-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
TrackedRenderPass Ergonomics #7227
Conversation
bcdef6b
to
bb7b5d4
Compare
.set_label("bloom_prefilter_pass") | ||
.add_color_attachment(view) | ||
.begin() | ||
.set_camera_viewport(camera) |
There was a problem hiding this comment.
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> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Todo: needs documentation
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, | ||
) | ||
} |
There was a problem hiding this comment.
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.
IMO we should consider merging |
26dc71f
to
4ff6233
Compare
…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
4ff6233
to
23b9e18
Compare
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)):
Example
First, let me show you a comparison between the old and the new API.
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.ViewTarget
and theViewDepthTexture
into a single struct (in a follow up PR).2. The
RenderPhase
s are now executed directly using theTrackedRenderPass
, with the API described above.TrackedRenderPass
is associated with aview_entity
.view_entity
s in the render method ofDraw
functions. Instead, they can be accessed directly from theTrackedRenderPass
if they are needed.3. Changed
TrackedRenderPass::set_camera_viewport
to take a&ExtractedCamera
instead of a&Viewport
4. Chainable
TrackedRenderPass
methodsTrackedRenderPass
return a mutable reference to Self.5. Added
load_op
method toClearColorConfig
Changelog
Todo
Migration Guide
Todo