Skip to content

Use add_command_buffer_generation_task for transparent and transmissive 3D passes #20308

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

fairhill1
Copy link

Objective

Solution

Refactors MainTransparentPass3dNode and MainTransmissivePass3dNode to use the async command buffer generation pattern with add_command_buffer_generation_task(), matching the approach already used by MainOpaquePass3dNode.

Changes:

  • MainTransparentPass3dNode: Convert to async pattern with command encoder
  • MainTransmissivePass3dNode: Convert to async pattern while preserving screen space specular transmission steps logic
  • Both nodes now follow the same performance optimization pattern used by the opaque pass

This change should improve performance by allowing render command generation to happen in parallel rather than sequentially on the render thread.

Testing

  • Needs benchmarking
  • Tested with existing 3D rendering examples to ensure no visual regressions
  • Verified that transparent and transmissive materials continue to render correctly
  • Confirmed that the async command buffer generation works as expected
  • No compilation errors or runtime issues observed

Reviewers can test by:

  1. Running any 3D examples with transparent materials (e.g., transparency_3d)
  2. Running examples with transmissive materials to verify screen space transmission still works
  3. Checking that performance characteristics match the opaque pass behavior
  4. Verifying no visual differences in rendered output

…ve 3D passes, matching the approach already used by MainOpaquePass3dNode.

Needs review and brenchmarking
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times S-Needs-Benchmarking This set of changes needs performance benchmarking to double-check that they help S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 28, 2025
@alice-i-cecile alice-i-cecile added X-Uncontroversial This work is generally agreed upon D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes labels Jul 28, 2025
Copy link
Contributor

@JMS55 JMS55 left a comment

Choose a reason for hiding this comment

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

A little hard to review the diff, but I think this looks good.

// NOTE: Scoped to drop the mutable borrow of render_context
#[cfg(feature = "trace")]
let _main_transmissive_pass_3d_span = info_span!("main_transmissive_pass_3d").entered();

if !transmissive_phase.items.is_empty() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be an early return to reduce nesting. Nothing else happens if this check is false

);

let render_pass =
command_encoder.begin_render_pass(&RenderPassDescriptor {
Copy link
Contributor

Choose a reason for hiding this comment

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

The descriptor should be constructed once before the if steps > 0. It's the same in both branches. That would probably help reduce the diff size too.

Copy link
Contributor

@IceSentry IceSentry left a comment

Choose a reason for hiding this comment

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

A few nitpicks for the transmissive pass that should help make the code shorter/easier to review

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-Performance A change motivated by improving speed, memory usage or compile times D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Benchmarking This set of changes needs performance benchmarking to double-check that they help S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Uncontroversial This work is generally agreed upon
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Consider using add_command_buffer_generation_task() for MainTransparentPass3dNode and MainTransmissivePass3dNode
4 participants