Headless renderer example has been added #13006

bugsweeper · 2024-04-17T11:45:23Z

Objective

Fixes #11457.
Fixes #22.

Solution

Based on another headless application

Changelog

Adopted to bevy 0.14

bugsweeper · 2024-04-17T12:28:21Z

@alice-i-cecile May I ask you for a review?

IceSentry

Just a quick review of the first couple of things I saw, this isn't a comprehensive review.

The code seems to do a lot of things and I'd prefer if it could be streamlined a bit to be easier to follow, but it's hard to know if there are parts of it that aren't as important because it lacks documentation of what all the pieces are.

examples/app/headless_renderer.rs

bugsweeper · 2024-04-25T09:06:05Z

@IceSentry , may I ask you for a rereview?

examples/app/headless_renderer.rs

Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com>

mockersf · 2024-04-27T13:29:14Z

could you add objects to the scene, so that it doesn't render just a black picture?

the example panics for me:


Adding CaptureFramePlugin
Saving image to: "bevy/test_images"
thread 'Compute Task Pool (3)' panicked at examples/app/headless_renderer.rs:300:22:
Failed to send data to main world: "SendError(..)"
stack backtrace:
   0: rust_begin_unwind
             at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/std/src/panicking.rs:647:5
   1: core::panicking::panic_fmt
             at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/panicking.rs:72:14
   2: core::result::unwrap_failed
             at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/result.rs:1649:5
   3: core::result::Result<T,E>::expect
             at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/result.rs:1030:23
   4: headless_renderer::frame_capture::image_copy::receive_image_from_buffer
             at ./examples/app/headless_renderer.rs:298:17
   5: core::ops::function::FnMut::call_mut
             at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/ops/function.rs:166:5
   6: core::ops::function::impls::<impl core::ops::function::FnMut<A> for &mut F>::call_mut
             at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/ops/function.rs:294:13
   7: <Func as bevy_ecs::system::function_system::SystemParamFunction<fn(F0,F1,F2) .> Out>>::run::call_inner
             at ./crates/bevy_ecs/src/system/function_system.rs:665:21
   8: <Func as bevy_ecs::system::function_system::SystemParamFunction<fn(F0,F1,F2) .> Out>>::run
             at ./crates/bevy_ecs/src/system/function_system.rs:668:17
   9: <bevy_ecs::system::function_system::FunctionSystem<Marker,F> as bevy_ecs::system::system::System>::run_unsafe
             at ./crates/bevy_ecs/src/system/function_system.rs:507:19
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
Encountered a panic in system `headless_renderer::frame_capture::image_copy::receive_image_from_buffer`!

Is it possible to not surface that panic?

examples/app/headless_renderer.rs

mockersf · 2024-04-27T13:31:20Z

examples/app/headless_renderer.rs

+                            let number = rng.gen::<u128>();
+                            let image_path = images_dir.join(format!("{number:032X}.png"));


Could you use an incrementing number instead of a random value?

replace println to info Co-authored-by: François Mockers <francois.mockers@vleue.com>

bugsweeper · 2024-04-29T09:40:55Z

thread 'Compute Task Pool (3)' panicked at examples/app/headless_renderer.rs:300:22:
Failed to send data to main world: "SendError(..)"
Is it possible to not surface that panic?

The reason of this panic is attempt to send image data to nonexistent receiver, because MainWorld destroyed while RenderWorld still renders. For such case I left comment
// This could fail on app exit, if Main world clears resources while Render world still renders
Simple solution is ignoring Result of that send (there is .expect("...") now), more complex solution is to notify RenderWorld that an exit event has emitted, may be via some Atomic<bool>.

bugsweeper · 2024-04-29T16:51:23Z

@mockersf May I ask you for a rereview?

pcwalton

Seems fine.

tychedelia

I agree with the comment about the nested modules being unnecessary for an example but otherwise lgtm.

IceSentry · 2024-05-05T20:36:37Z

examples/app/headless_renderer.rs

+            let mut cpu_image = Image {
+                texture_descriptor: TextureDescriptor {
+                    label: None,
+                    size,
+                    dimension: TextureDimension::D2,
+                    format: TextureFormat::Rgba8UnormSrgb,
+                    mip_level_count: 1,
+                    sample_count: 1,
+                    usage: TextureUsages::COPY_DST | TextureUsages::TEXTURE_BINDING,
+                    view_formats: &[],
+                },
+                ..Default::default()
+            };
+            cpu_image.resize(size);


This can be simplified a little with this:

Suggested change

let mut cpu_image = Image {

texture_descriptor: TextureDescriptor {

label: None,

size,

dimension: TextureDimension::D2,

format: TextureFormat::Rgba8UnormSrgb,

mip_level_count: 1,

sample_count: 1,

usage: TextureUsages::COPY_DST | TextureUsages::TEXTURE_BINDING,

view_formats: &[],

},

..Default::default()

};

cpu_image.resize(size);

let cpu_image = Image::new_fill(

size,

TextureDimension::D2,

&[0; 4],

TextureFormat::bevy_default(),

RenderAssetUsages::default(),

);

IceSentry · 2024-05-05T20:39:59Z

examples/app/headless_renderer.rs

+            let mut render_target_image = Image {
+                texture_descriptor: TextureDescriptor {
+                    label: None,
+                    size,
+                    dimension: TextureDimension::D2,
+                    format: TextureFormat::Rgba8UnormSrgb,
+                    mip_level_count: 1,
+                    sample_count: 1,
+                    usage: TextureUsages::COPY_SRC
+                        | TextureUsages::COPY_DST
+                        | TextureUsages::TEXTURE_BINDING
+                        | TextureUsages::RENDER_ATTACHMENT,
+                    view_formats: &[],
+                },
+                ..Default::default()
+            };
+            render_target_image.resize(size);


Suggested change

let mut render_target_image = Image {

texture_descriptor: TextureDescriptor {

label: None,

size,

dimension: TextureDimension::D2,

format: TextureFormat::Rgba8UnormSrgb,

mip_level_count: 1,

sample_count: 1,

usage: TextureUsages::COPY_SRC

| TextureUsages::COPY_DST

| TextureUsages::TEXTURE_BINDING

| TextureUsages::RENDER_ATTACHMENT,

view_formats: &[],

},

..Default::default()

};

render_target_image.resize(size);

let mut render_target_image = Image::new_fill(

size,

TextureDimension::D2,

&[0; 4],

TextureFormat::bevy_default(),

RenderAssetUsages::default(),

);

render_target_image .texture_descriptor.usage |= TextureUsages::RENDER_ATTACHMENT | TextureUsages::TEXTURE_BINDING;

IceSentry

I think there's a few things that can be improved to simplify this example a little, but it's probably better to merge it as-is and improve it later.

I haven't tested my suggestions, but I know they should roughly work. There might be some fmt issues.

alice-i-cecile · 2024-05-05T21:18:31Z

@bugsweeper those suggestions look solid, but once CI is green I'll merge this in either way :)

examples/app/headless_renderer.rs

mockersf · 2024-05-05T21:54:38Z

examples/app/headless_renderer.rs

+                            while image_path.exists() {
+                                number += 1;
+                                image_path = images_dir.join(format!("{number:03}.png"));
+                            }


you could keep a Local<u32> as a system parameter instead of iterating on every file. this has the potential to blow up

This example can be run multiple times with single_image option, Local helps only at first run with multiple files

overwriting files from a previous run seems better to me than iterating over every files. additional bonus is that we create less random files on their computer

Searching free filenames has been changed to Local usage

mockersf

interaction with the file system should be improved or commented

bugsweeper · 2024-05-06T04:26:09Z

@mockersf May I ask you for a rereview?

bugsweeper · 2024-05-06T07:06:48Z

@alice-i-cecile Ci is green now

bugsweeper added 3 commits April 17, 2024 14:32

Headless renderer example has been added

060eab9

Update examples list

5a2f443

Fix warnings

5fcba48

pablo-lua added A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples labels Apr 17, 2024

alice-i-cecile requested a review from mockersf April 17, 2024 15:22

IceSentry reviewed Apr 17, 2024

View reviewed changes

Fixes based on review feedback and according to gpu_readback example

40bb6d8

bugsweeper force-pushed the add-headless-render-to-file branch from 782ada3 to 40bb6d8 Compare April 24, 2024 11:24

bugsweeper requested a review from IceSentry April 24, 2024 12:45

Merge branch 'main' into add-headless-render-to-file

91bbe63

bugsweeper force-pushed the add-headless-render-to-file branch from e648765 to 4960008 Compare April 25, 2024 08:40

Fix ci build error

d85d6a7

bugsweeper force-pushed the add-headless-render-to-file branch from 4960008 to d85d6a7 Compare April 25, 2024 08:55

BD103 reviewed Apr 26, 2024

View reviewed changes

examples/app/headless_renderer.rs Outdated Show resolved Hide resolved

examples/app/headless_renderer.rs Show resolved Hide resolved

examples/app/headless_renderer.rs Outdated Show resolved Hide resolved

examples/app/headless_renderer.rs Outdated Show resolved Hide resolved

Apply suggestions from code review

19e1690

Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com>

mockersf reviewed Apr 27, 2024

View reviewed changes

examples/app/headless_renderer.rs Outdated Show resolved Hide resolved

mockersf reviewed Apr 27, 2024

View reviewed changes

examples/app/headless_renderer.rs Outdated Show resolved Hide resolved

mockersf reviewed Apr 27, 2024

View reviewed changes

Apply suggestions from code review

1bd7cba

replace println to info Co-authored-by: François Mockers <francois.mockers@vleue.com>

bugsweeper added 4 commits April 29, 2024 12:43

Merge branch 'main' into add-headless-render-to-file

1f717cb

Fixed panic on app exit

0475ec9

Filled scene to generate nonblack image

e5d805a

Using incrementing numbers instead of random values as filenames

946ae14

bugsweeper force-pushed the add-headless-render-to-file branch from 11edefd to 946ae14 Compare April 29, 2024 16:40

alice-i-cecile added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label May 5, 2024

pcwalton approved these changes May 5, 2024

View reviewed changes

tychedelia approved these changes May 5, 2024

View reviewed changes

alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 5, 2024

alice-i-cecile added this pull request to the merge queue May 5, 2024

alice-i-cecile mentioned this pull request May 5, 2024

Screenshots from any render target not just windows #12478

Closed

github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 5, 2024

IceSentry reviewed May 5, 2024

View reviewed changes

IceSentry approved these changes May 5, 2024

View reviewed changes

mockersf reviewed May 5, 2024

View reviewed changes

examples/app/headless_renderer.rs Outdated Show resolved Hide resolved

mockersf reviewed May 5, 2024

View reviewed changes

mockersf requested changes May 5, 2024

View reviewed changes

mockersf removed the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label May 5, 2024

Simplified image creation, fixed ci build

5c8e54b

bugsweeper requested a review from mockersf May 5, 2024 22:37

Interaction with file system has been commented

b74e238

bugsweeper force-pushed the add-headless-render-to-file branch from 08c6aff to b74e238 Compare May 5, 2024 22:45

Rewrite files instead of searching unocupied file numbers

c29cd0b

mockersf approved these changes May 6, 2024

View reviewed changes

mockersf added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label May 6, 2024

Removed nested modules, added comments

5436d6e

alice-i-cecile added this pull request to the merge queue May 8, 2024

Merged via the queue into bevyengine:main with commit d9d305d May 8, 2024
31 checks passed

cxreiff mentioned this pull request May 16, 2024

Skewed output when changing resolution of new headless_renderer example #13384

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Headless renderer example has been added #13006

Headless renderer example has been added #13006

bugsweeper commented Apr 17, 2024 •

edited

Loading

bugsweeper commented Apr 17, 2024

IceSentry left a comment •

edited

Loading

bugsweeper commented Apr 25, 2024

mockersf commented Apr 27, 2024

mockersf Apr 27, 2024

bugsweeper commented Apr 29, 2024 •

edited

Loading

bugsweeper commented Apr 29, 2024 •

edited

Loading

pcwalton left a comment

tychedelia left a comment

IceSentry May 5, 2024 •

edited

Loading

IceSentry May 5, 2024

IceSentry left a comment •

edited

Loading

alice-i-cecile commented May 5, 2024

mockersf May 5, 2024 •

edited

Loading

bugsweeper May 5, 2024

mockersf May 5, 2024

bugsweeper May 6, 2024

mockersf left a comment

bugsweeper commented May 6, 2024

bugsweeper commented May 6, 2024

		let number = rng.gen::<u128>();
		let image_path = images_dir.join(format!("{number:032X}.png"));

Headless renderer example has been added #13006

Headless renderer example has been added #13006

Conversation

bugsweeper commented Apr 17, 2024 • edited Loading

Objective

Solution

Changelog

bugsweeper commented Apr 17, 2024

IceSentry left a comment • edited Loading

Choose a reason for hiding this comment

bugsweeper commented Apr 25, 2024

mockersf commented Apr 27, 2024

mockersf Apr 27, 2024

Choose a reason for hiding this comment

bugsweeper commented Apr 29, 2024 • edited Loading

bugsweeper commented Apr 29, 2024 • edited Loading

pcwalton left a comment

Choose a reason for hiding this comment

tychedelia left a comment

Choose a reason for hiding this comment

IceSentry May 5, 2024 • edited Loading

Choose a reason for hiding this comment

IceSentry May 5, 2024

Choose a reason for hiding this comment

IceSentry left a comment • edited Loading

Choose a reason for hiding this comment

alice-i-cecile commented May 5, 2024

mockersf May 5, 2024 • edited Loading

Choose a reason for hiding this comment

bugsweeper May 5, 2024

Choose a reason for hiding this comment

mockersf May 5, 2024

Choose a reason for hiding this comment

bugsweeper May 6, 2024

Choose a reason for hiding this comment

mockersf left a comment

Choose a reason for hiding this comment

bugsweeper commented May 6, 2024

bugsweeper commented May 6, 2024

bugsweeper commented Apr 17, 2024 •

edited

Loading

IceSentry left a comment •

edited

Loading

bugsweeper commented Apr 29, 2024 •

edited

Loading

bugsweeper commented Apr 29, 2024 •

edited

Loading

IceSentry May 5, 2024 •

edited

Loading

IceSentry left a comment •

edited

Loading

mockersf May 5, 2024 •

edited

Loading