Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add capability to render to cpu buffer or double buffer #4572

Closed
wants to merge 11 commits into from

Conversation

DGriffin91
Copy link
Contributor

@DGriffin91 DGriffin91 commented Apr 24, 2022

Objective

Provides a portion of the functionally needed for #22

The goal of the PR is to provide a straightforward way to:

  • Render to a cpu buffer that can be used to output images files.
  • Render to a double buffer, allowing the image to be easily used as a texture in the same render layer.
  • Simplify rendering to a texture. Doing so without having to directly manipulate the render graph.

There are three examples showing each of these respective methods:
examples/3d/frame_capture_cpu.rs
examples/3d/frame_capture_double_buffer.rs
examples/3d/frame_capture_gpu.rs

The FrameCapturePlugin is also setup to allow use of multiple, simultaneous, renders using a variety of targets.

The location of the FrameCapturePlugin was lightly discussed on discord. There isn't currently a great place for utility plugins like this one.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Apr 24, 2022
@DGriffin91 DGriffin91 changed the title Add capability to render to cpu image buffer or double buffer Add capability to render to cpu buffer or double buffer Apr 24, 2022
@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen and removed S-Needs-Triage This issue needs to be labelled labels Apr 24, 2022
use bevy::render::renderer::RenderDevice;

#[derive(Component, Default)]
pub struct CaptureCamera1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why this needs to be numbered?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a left over from when I was testing using multiple cameras. I'll ditch the number.

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Apr 24, 2022

Super cool! A few comments on the examples:

  1. Please add quick module level doc comments at the top of each explaining what their trying to do.
  2. It feels very awkward that these are in the 3D folder. There's fundamentally nothing in them that wouldn't work for 2D. @mockersf, are you on board with creating a rendering examples folder? I know we've discussed this before, but this is a clear case of the need.
  3. Can we save the rendered textures to file? If so, that would be incredibly valuable to demonstrate in an example in the root level tests folder to enable the creation and validation of "golden images", which should be stable across changes.

@DGriffin91
Copy link
Contributor Author

@alice-i-cecile

  1. Please add quick module level doc comments at the top of each explaining what their trying to do.

Just added some doc comments. Is what I added the sort of thing you are looking for?

  1. It feels very awkward that these are in the 3D folder.

Agreed. I'll have to do some testing to make sure these can work in 2D as well.

  1. Can we save the rendered textures to file?

Yep! That's what the frame_capture_cpu example currently does, and is part of the motivation for adding this feature.

@alice-i-cecile
Copy link
Member

WRT the comments, see #4438 :)

Comment on lines 13 to 20
bevy_app = { path = "../bevy_app", version = "0.8.0-dev" }
bevy_asset = { path = "../bevy_asset", version = "0.8.0-dev" }
bevy_ecs = { path = "../bevy_ecs", version = "0.8.0-dev" }
bevy_utils = { path = "../bevy_utils", version = "0.8.0-dev" }
bevy_render = { path = "../bevy_render", version = "0.8.0-dev" }
bevy_math = { path = "../bevy_math", version = "0.8.0-dev" }
bevy_core = { path = "../bevy_core", version = "0.8.0-dev" }
bevy_core_pipeline = { path = "../bevy_core_pipeline", version = "0.8.0-dev" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you reorder them alphabetically and check that they are all needed? I'm not sure for bevy_utils, bevy_math or bevy_core


let gpu_image = images.add(image);

let padded_bytes_per_row = RenderDevice::align_copy_bytes_per_row(width as usize) * 4;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not happy about the * 4 there (and the other time there's a 4). Some texture formats have less than 4 components, and some uses more than one byte per component.
Should this be * format.describe().components as usize * format.describe().block_size as usize instead?

Copy link
Contributor Author

@DGriffin91 DGriffin91 Apr 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.

Based on this: ImageDataLayout I think it may be:

let padded_bytes_per_row = RenderDevice::align_copy_bytes_per_row(
    (gpu_image.size.width as usize / format.block_dimensions.0 as usize) * format.block_size as usize,
);

It may be still more complex. I'm not dealing with rows_per_image yet. But I don't know if that matters here since I would think the texture would always have only one layer in this context.

This is what Bevy does for prepare_asset in image.rs

And it looks like pixel_size() just does info.type_size * info.num_components. I'm not sure if this ends up being always correct if the texture is compressed.

There's also the issue of the buffer being larger than the user might expect since it's aligned to 256 bytes

This is fine for both GPU examples since it's what the gpu expects on both ends. But the one that copies to the CPU results in row padded data.

Do we crop the buffer for the user? If so, I'm not sure exactly what the most efficient way of doing that is. Each row is potentially too long, so we can't just slice off the end of the flat buffer.

Comment on lines 227 to 229
RenderPhase::<Opaque3d>::default(),
RenderPhase::<AlphaMask3d>::default(),
RenderPhase::<Transparent3d>::default(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be possible to have it work also for 2d / ui?

bevy::camera::TargetBuffer::CPUBuffer(target_buffer) => {
target_buffer.get(&render_device, |buf| {
image::save_buffer(
format!("../test{i}.png"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will write every frame to the same file, with a good chance of leaving it corrupted (with 120 writes per second on my laptop)

could this system be put on a tilmestep, maybe every second, and changing the filename every time?

@alice-i-cecile
Copy link
Member

@DGriffin91 does this fix #22? If so, please add it to the PR description.

@DGriffin91
Copy link
Contributor Author

DGriffin91 commented Apr 27, 2022

@alice-i-cecile It should at least help with the "copy texture data from the gpu to an image file" portion. I just tried making a headless example and haven't been able to.

If I manually add plugins:

.add_plugin(CorePlugin)
.add_plugin(TransformPlugin)
.add_plugin(HierarchyPlugin)
.add_plugin(AssetPlugin)
.add_plugin(ScenePlugin)
.add_plugin(RenderPlugin)
.add_plugin(CorePipelinePlugin)
.add_plugin(GltfPlugin)
.add_plugin(PbrPlugin)

I get an error on this line:

let windows = app.world.resource_mut::<bevy_window::Windows>();

thread 'main' panicked at 'Requested resource bevy_window::windows::Windows does not exist in the `World`. 
                Did you forget to add it using `app.add_resource` / `app.init_resource`?
                Resources are also implicitly added via `app.add_event`,
                and can be added by plugins.', crates\bevy_render\src\lib.rs:128:41

And if I try the wgpu backend: None from the headless_defaults.rs example:

.insert_resource(WgpuSettings {
    backends: None,
    ..default()
})
.add_plugins(DefaultPlugins)

I get the error:

thread 'main' panicked at 'Sub-App with label 'RenderApp' does not exist', 
[...]\bevy_app\src\app.rs:884:27

So I'm not sure if headless rendering in generally possible in bevy yet. And I also don't know if making headless rendering possible is in the scope of this PR.

@alice-i-cecile
Copy link
Member

Sounds good. Headless rendering has been a bit of a headache for a long time, so that doesn't surprise me. Fixing that is definitely out of scope.

@mockersf
Copy link
Member

mockersf commented May 2, 2022

waiting on the "high level camera driven render targets" PR

@YoshieraHuang
Copy link
Contributor

YoshieraHuang commented May 25, 2022

@alice-i-cecile It should at least help with the "copy texture data from the gpu to an image file" portion. I just tried making a headless example and haven't been able to.

If I manually add plugins:

.add_plugin(CorePlugin)
.add_plugin(TransformPlugin)
.add_plugin(HierarchyPlugin)
.add_plugin(AssetPlugin)
.add_plugin(ScenePlugin)
.add_plugin(RenderPlugin)
.add_plugin(CorePipelinePlugin)
.add_plugin(GltfPlugin)
.add_plugin(PbrPlugin)

I get an error on this line:

let windows = app.world.resource_mut::<bevy_window::Windows>();

thread 'main' panicked at 'Requested resource bevy_window::windows::Windows does not exist in the `World`. 
                Did you forget to add it using `app.add_resource` / `app.init_resource`?
                Resources are also implicitly added via `app.add_event`,
                and can be added by plugins.', crates\bevy_render\src\lib.rs:128:41

And if I try the wgpu backend: None from the headless_defaults.rs example:

.insert_resource(WgpuSettings {
    backends: None,
    ..default()
})
.add_plugins(DefaultPlugins)

I get the error:

thread 'main' panicked at 'Sub-App with label 'RenderApp' does not exist', 
[...]\bevy_app\src\app.rs:884:27

So I'm not sure if headless rendering in generally possible in bevy yet. And I also don't know if making headless rendering possible is in the scope of this PR.

Only the WinitPlugin should be disabled to prevent the windowing. This script works with the frame_capture_cpu example:

App::new()
...
        .add_plugins_with(DefaultPlugins, |group| group.disable::<WinitPlugin>() )
...

However, no frame is rendered and then the app exits, since WinitPlugin provides built-in eventloop to trigger app to run forever. An explicit runner should be provided.

Update:
ScheduleRunnerPlugin provides a standalone runner. Add ScheduleRunnerPlugin to the app and the app starts to output png files. Here is the current main function:

    App::new()
        .insert_resource(Msaa { samples: 4 }) // Use 4x MSAA
        .insert_resource(AmbientLight {
            color: Color::WHITE,
            brightness: 1.0 / 5.0f32,
        })
        .add_plugins_with(DefaultPlugins, |group| group.disable::<WinitPlugin>())
        .add_plugin(CameraTypePlugin::<CaptureCamera>::default())
        .add_plugin(FrameCapturePlugin)
        .insert_resource(ScheduleRunnerSettings{
            run_mode: RunMode::Loop { wait: Some(Duration::from_millis(100)) }
        })
        .add_plugin(ScheduleRunnerPlugin)
        .add_startup_system(setup)
        .add_system(animate_light_direction)
        .add_system(save_img)
        .run();

The ScheduleRunnerSettings is used to determine how fast this app ticks.

However, the output file may be empty or corrputed if the app ends using Ctrl+C. If you monitor the output file when the app runs, you can see the size of file jumping from zero to about 100 kb repeatedly.
Recording 2022-05-25 at 21 38 40 (1)

@mockersf
Copy link
Member

@DGriffin91 #4898 has been merged, could you update this PR? I would love to see it move forward.

@zimond
Copy link

zimond commented Jun 16, 2022

I tested this PR on 2d camera and it just works (by changing 3d subgraph rendering to 2d, etc). I think maybe the whole extract_camera_phases thing is creating redundant code, as it is identical to what is done for 2d/3d camera

@DGriffin91
Copy link
Contributor Author

I tested this PR on 2d camera and it just works (by changing 3d subgraph rendering to 2d, etc). I think maybe the whole extract_camera_phases thing is creating redundant code, as it is identical to what is done for 2d/3d camera

If I remove this (below) from extract_camera_phases with the frame_capture_cpu example, it doesn't output the 3D model and I just get a blank grey png.

.insert_bundle((
    RenderPhase::<Opaque3d>::default(),
    RenderPhase::<AlphaMask3d>::default(),
    RenderPhase::<Transparent3d>::default(),
))

@zimond
Copy link

zimond commented Jun 16, 2022

@DGriffin91 Yes, I didn't mean it's unnecessary, it's just a duplication of what is done in camera3d pipeline.

Anyway, I think we could extract the logic or at least generalize this method to support 2d/ui cameras, e.g. for 2d camera:

.insert_bundle((
    RenderPhase::<Transparent2d>::default()
))

By the way this is the feature I really want & thanks for your hard work.

@DGriffin91
Copy link
Contributor Author

@zimond Gotcha, that makes sense. And thank you! We are discussing how to get this setup with the new camera driven stuff on discord here in case you are interested: https://discord.com/channels/691052431525675048/986788850535915540/

@DGriffin91 DGriffin91 force-pushed the frame_capture branch 2 times, most recently from f74b4c4 to 379515d Compare June 17, 2022 00:16
@@ -194,6 +199,31 @@ impl Node for MainPass3dNode {
}
}

// TODO: should this live here or in a separate node?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate node imo.

@@ -247,6 +247,8 @@ pub enum RenderTarget {
Window(WindowId),
/// Image to which the camera's view is rendered.
Image(Handle<Image>),
/// Buffered Image to which the camera's view is rendered.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this description could be clearer. The first image is used for rendering into and is then copied into the second image, right?

// TODO: don't just have duplicated code between MainPass3dNode/MainPass2dNode
if let RenderTarget::BufferedImage(image_handle, buffer_image_handle) = &camera.target {
let gpu_images = world.get_resource::<RenderAssets<Image>>().unwrap();
let gpu_image = gpu_images.get(&image_handle).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe source and destination images? i.e. src_image, dst_image

Comment on lines +57 to +60
usage: TextureUsages::TEXTURE_BINDING
| TextureUsages::COPY_DST
| TextureUsages::COPY_SRC
| TextureUsages::RENDER_ATTACHMENT,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one only needs RENDER_ATTACHMENT | COPY_SRC.

Comment on lines +76 to +79
usage: TextureUsages::TEXTURE_BINDING
| TextureUsages::COPY_DST
| TextureUsages::COPY_SRC
| TextureUsages::RENDER_ATTACHMENT,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one only needs COPY_DST | TEXTURE_BINDING.

@paulkre
Copy link

paulkre commented Sep 5, 2022

I'm looking forward to using this feature. I hope it gets merged soon!

@paulkre
Copy link

paulkre commented Sep 6, 2022

Is it possible to save a PNG image with the wgpu::Texture struct? I only found a way to do this with the wgpu::Buffer which requires the use of wgpu::CommandEncoder::copy_texture_to_buffer method.

@DGriffin91
Copy link
Contributor Author

DGriffin91 commented Sep 6, 2022

@paulkre Check out this newer PR. It outputs PNG images: #5550

You should be able to use image_copy.rs in bevy 0.8 as a plugin iirc.

Yes I am using copy_texture_to_buffer

@alice-i-cecile
Copy link
Member

@DGriffin91 shall I close this out in favor of #5550?

@DGriffin91
Copy link
Contributor Author

@alice-i-cecile Yep, I think that makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants