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

Move the stage that acquires a new texture from the swap chain as late as possible in the rendering pipeline. #16964

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

Conversation

pcwalton
Copy link
Contributor

Currently, we retrieve a new texture from the pipeline in prepare_windows, which executes during the ManageViews rendering phase. This is inefficient if we're GPU bound, because ManageViews happens before queue_material_meshes and
batch_and_prepare_binned_render_phase among other frequently-slow systems. Retrieving a new texture from the swap chain can block for considerable time, during which we don't want the CPU to be sitting idle. Additionally, we don't want the GPU to be sitting idle while the CPU works on queue_material_meshes and so forth.

This commit fixes the issue by moving the acquisition of a new swap chain texture late in the pipeline, to a new phase PrepareWindows that occurs right before render graph evaluation. Because render graph evaluation needs to bind the swap chain textures in order to issue drawing commands, this is the last possible moment where it's safe to do so.

This shows Bistro on main with shadows enabled before these changes. Note that queue_material_meshes and batch_and_prepare_binned_render_phase occur after prepare_windows and therefore the GPU is idle during this time.
Screenshot 2024-12-24 185330

This shows Bistro with these changes applied and shadows enabled. Observe that queue_material_meshes and batch_and_prepare_binned_render_phase are moved before prepare_windows, allowing for simultaneous GPU/CPU execution.
Screenshot 2024-12-24 190909

@pcwalton pcwalton requested review from JMS55 and IceSentry December 25, 2024 00:12
@pcwalton pcwalton added A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 25, 2024
as possible in the rendering pipeline.

Currently, we retrieve a new texture from the pipeline in
`prepare_windows`, which executes during the `ManageViews` rendering
phase. This is inefficient if we're GPU bound, because `ManageViews`
happens before `queue_material_meshes` and
`batch_and_prepare_binned_render_phase` among other frequently-slow
systems. Retrieving a new texture from the swap chain can block for
considerable time, during which we don't want the CPU to be sitting
idle. Additionally, we don't want the GPU to be sitting idle while the
CPU works on `queue_material_meshes` and so forth.

This commit fixes the issue by moving the acquisition of a new swap
chain texture late in the pipeline, to a new phase `PrepareWindows` that
occurs right before render graph evaluation. Because render graph
evaluation needs to bind the swap chain textures in order to issue
drawing commands, this is the last possible moment where it's safe to do
so.
@JMS55
Copy link
Contributor

JMS55 commented Dec 25, 2024

Do we want this? Not sure how the synchronization is implemented in wgpu, but for DirectX12 I did this: https://github.com/JMS55/bevy_solari_directx/blob/main/src/swapchain.rs#L70-L76

Maybe @cwfitzgerald can weigh in on when the best time to block on the swapchain in wgpu is for good frame pacing / minimizing latency / maximizing FPS.

@pcwalton
Copy link
Contributor Author

I don't really understand why you'd want to not wait until the last possible moment. Right now the GPU is sitting idle for a long time and that makes no sense to me.

@pcwalton
Copy link
Contributor Author

pcwalton commented Dec 25, 2024

To be clear, not having this definitely loses us FPS in Bistro if vsync is off.

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 S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants