Skip to content

TrackedRenderPass Ergonomics #7227

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

Conversation

kurtkuehnert
Copy link
Contributor

@kurtkuehnert kurtkuehnert commented Jan 16, 2023

Objective

The current code for creating render passes and executing render phases is quite indentation-heavy and repetitive.

I have experimented with further improving the API, based on the proposal by @cart (posted in #7043 (comment)):

// This would create and set up a TrackedRenderPass, which would set the viewport, if it is
// configured on the camera. "pass" would need to be a wrapper over TrackedRenderPass.
let pass = camera.begin_render_pass(render_context, view_entity);
pass.render_phase(opaque_phase, world);

Example

First, let me show you a comparison between the old and the new API.

// Bevy Main
let mut render_pass = render_context.begin_tracked_render_pass(RenderPassDescriptor {
    label: Some("main_alpha_mask_pass_3d"),
    // NOTE: The alpha_mask pass loads the color buffer as well as overwriting it where appropriate.
    color_attachments: &[Some(target.get_color_attachment(Operations {
        load: LoadOp::Load,
        store: true,
    }))],
    depth_stencil_attachment: Some(RenderPassDepthStencilAttachment {
        view: &depth.view,
        // NOTE: The alpha mask pass loads the depth buffer and possibly overwrites it
        depth_ops: Some(Operations {
            load: LoadOp::Load,
            store: true,
        }),
        stencil_ops: None,
    }),
});

if let Some(viewport) = camera.viewport.as_ref() {
    render_pass.set_camera_viewport(viewport);
}

alpha_mask_phase.render(&mut render_pass, world, view_entity);

// this PR
render_context
    .render_pass(view_entity)
    .set_label("main_alpha_mask_pass_3d")
    .add_view_target(target)
    .set_color_ops(LoadOp::Load, true)
    .set_depth_stencil_attachment(&depth.view)
    .set_depth_ops(LoadOp::Load, true)
    .begin()
    .set_camera_viewport(camera)
    .render_phase(alpha_mask_phase, world);

Solution

Cart's solution is not directly realizable, since we have no central camera object.
Instead, I propose a builder-style API for setting up the TrackedRenderPass.

This PR comprises multiple improvements (which can be split apart and merged separately, if desired. I wanted to showcase them all at once first.)

1. I have added a TrackedRenderPassBuilder, which sets up the render pass's attachments.

  • In TrackedRenderPass driven RenderPhases #7080 I have tried implementing this as a single method similar to the one proposed by @cart. I do not think that this is the right design. Due to the many optional types, a builder-style API seems like the better fit.
  • The views of the attachments, are added separately from their operations, to keep nesting to a minimum and make use of default operations.
  • Let me know if you have got suggestions or differing opinions regarding this API.
  • Contrary to my previous attempts this new version supports all usages through the bevy code base.
  • I believe we should consider merging the ViewTarget and the ViewDepthTexture into a single struct (in a follow up PR).
// Bevy main
let mut render_pass = render_context.begin_tracked_render_pass(RenderPassDescriptor {
    label: Some("main_opaque_pass_3d"),
    // NOTE: The opaque pass loads the color
    // buffer as well as writing to it.
    color_attachments: &[Some(target.get_color_attachment(Operations {
        load: match camera_3d.clear_color {
            ClearColorConfig::Default => {
            LoadOp::Clear(world.resource::<ClearColor>().0.into())
            }
            ClearColorConfig::Custom(color) => LoadOp::Clear(color.into()),
            ClearColorConfig::None => LoadOp::Load,
        },
        store: true,
    }))],
    depth_stencil_attachment: Some(RenderPassDepthStencilAttachment {
        view: &depth.view,
        // NOTE: The opaque main pass loads the depth buffer and possibly overwrites it
        depth_ops: Some(Operations {
            load: if depth_prepass.is_some() || normal_prepass.is_some() {
                // if any prepass runs, it will generate a depth buffer so we should use it,
                // even if only the normal_prepass is used.
                Camera3dDepthLoadOp::Load
            } else {
                // NOTE: 0.0 is the far plane due to bevy's use of reverse-z projections.
                camera_3d.depth_load_op.clone()
            }
            .into(),
            store: true,
        }),
        stencil_ops: None,
    }),
});

// cart
let mut render_pass = camera.begin_render_pass(render_context, view_entity);

// #7080
let mut render_pass = TrackedRenderPass::create_for_camera(
    render_context,
    "main_alpha_mask_pass_3d",
    view_entity,
    target,
    Operations {
    load: LoadOp::Load,
    store: true,
    },
    Some(depth),
    Some(Operations {
    load: LoadOp::Load,
    store: true,
    }),
    &camera.viewport,
);

// this PR
let depth_load_op = if depth_prepass.is_some() || normal_prepass.is_some() {
    // if any prepass runs, it will generate a depth buffer so we should use it,
    // even if only the normal_prepass is used.
    LoadOp::Load
} else {
    camera_3d.depth_load_op.clone().into()
};

render_context
    .render_pass(view_entity)
    .set_label("main_opaque_pass_3d")
    .add_view_target(target)
    .set_color_ops(camera_3d.clear_color.load_op(world), true)
    .set_depth_stencil_attachment(&depth.view)
    .set_depth_ops(depth_load_op, true)
    .begin()
    .set_camera_viewport(camera)
    .render_phase(opaque_phase, world);

2. The RenderPhases are now executed directly using the TrackedRenderPass, with the API described above.

  • Therefore, each TrackedRenderPass is associated with a view_entity.
  • Additionally, this alleviates the need to pass around the view_entitys in the render method of Draw functions. Instead, they can be accessed directly from the TrackedRenderPass if they are needed.
  • I believe that conceptually this is now way more clear and discoverable. You execute zero or multiple render phases on a render pass.
// old
opaque_phase.render(&mut render_pass, world, view_entity);

// new
render_pass.render_phase(opaque_phase, world);

3. Changed TrackedRenderPass::set_camera_viewport to take a &ExtractedCamera instead of a &Viewport

  • this minor change avoids a lot of repetition
// bevy main
if let Some(viewport) = camera.viewport.as_ref() {
    render_pass.set_camera_viewport(viewport);
}
// this PR
render_pass.set_camera_viewport(camera);

4. Chainable TrackedRenderPass methods

  • For convenience, I have made all methods of the TrackedRenderPass return a mutable reference to Self.
  • This allows us to chain multiple of these calls.
// bevy main
downsampling_pass.set_render_pipeline(downsampling_pipeline);
downsampling_pass.set_bind_group(
     0, 
     &bind_groups.downsampling_bind_groups[mip as usize - 1],
     &[uniform_index.index()],
);
if let Some(viewport) = camera.viewport.as_ref() {
    downsampling_pass.set_camera_viewport(viewport);
}
downsampling_pass.draw(0..3, 0..1);

// this PR
downsampling_pass
    .set_camera_viewport(camera)
    .set_render_pipeline(downsampling_pipeline)
    .set_bind_group(
        0,
        &bind_groups.downsampling_bind_groups[mip as usize - 1], 
        &[uniform_index.index()],
     )
     .draw(0..3, 0..1);

5. Added load_op method to ClearColorConfig

  • This tiny change removes a lot of code duplication and indentation as well.
// Bevy Main
match camera_2d.clear_color {
    ClearColorConfig::Default => {
        LoadOp::Clear(world.resource::<ClearColor>().0.into())
    }
    ClearColorConfig::Custom(color) => LoadOp::Clear(color.into()),
    ClearColorConfig::None => LoadOp::Load,
}

// this PR
camera_2d.clear_color.load_op(world)
  • Add Documentation
  • Write Changelog
  • Write Migration Guide

Changelog

Todo

Migration Guide

Todo

@kurtkuehnert kurtkuehnert added A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jan 16, 2023
@kurtkuehnert kurtkuehnert force-pushed the tracked_render_pass_ergonomics branch from bcdef6b to bb7b5d4 Compare January 23, 2023 13:01
.set_label("bloom_prefilter_pass")
.add_color_attachment(view)
.begin()
.set_camera_viewport(camera)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should probably remove this line.

@@ -13,6 +14,17 @@ pub enum ClearColorConfig {
None,
}

impl ClearColorConfig {
#[inline]
pub fn load_op(&self, world: &World) -> LoadOp<RawColor> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Todo: needs documentation

Comment on lines +336 to +354
pub(crate) fn begin_tracked_render_pass<'a>(
&'a mut self,
descriptor: RenderPassDescriptor<'a, '_>,
view_entity: Entity,
) -> TrackedRenderPass<'a> {
// Cannot use command_encoder() as we need to split the borrow on self
let command_encoder = self.command_encoder.get_or_insert_with(|| {
self.render_device
.create_command_encoder(&wgpu::CommandEncoderDescriptor::default())
});
TrackedRenderPass::new(
&self.render_device,
command_encoder.begin_render_pass(&descriptor),
view_entity,
)
}
Copy link
Contributor Author

@kurtkuehnert kurtkuehnert Jan 23, 2023

Choose a reason for hiding this comment

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

This method is still required, due to the split borrow.

@kurtkuehnert
Copy link
Contributor Author

IMO we should consider merging TrackedRenderPass and RenderPass into a single abstraction.

@kurtkuehnert kurtkuehnert mentioned this pull request Feb 7, 2023
2 tasks
@kurtkuehnert kurtkuehnert force-pushed the tracked_render_pass_ergonomics branch from 26dc71f to 4ff6233 Compare February 14, 2023 08:31
…r API

updated builder API to support arbitrary attachments
TrackedRenderPass methods can be chained
associated TrackedRenderPass with the view Entity
added render_phase method to TrackedRenderPass
added TrackedRenderPassBuilder
changed set_camera_viewport method to take &Camera instead of &Viewport
@kurtkuehnert kurtkuehnert force-pushed the tracked_render_pass_ergonomics branch from 4ff6233 to 23b9e18 Compare March 9, 2023 14:01
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-Code-Quality A section of code that is hard to understand or change C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

1 participant