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

Redundant copies of MeshUniform #11770

Open
JMS55 opened this issue Feb 7, 2024 · 2 comments
Open

Redundant copies of MeshUniform #11770

JMS55 opened this issue Feb 7, 2024 · 2 comments
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times P-High This is particularly urgent, and deserves immediate attention X-Controversial There is active debate or serious implications around merging this PR

Comments

@JMS55
Copy link
Contributor

JMS55 commented Feb 7, 2024

Since #9685, each instance of batch_and_prepare_render_phase will append a copy of MeshUniform to the GpuArrayBuffer<MeshUniform> resource for each entity x phase. For 2 cameras, each with 3 phases (shadow, prepass, main pass), that's 6 total copies per entity... Additionally, each batch_and_prepare_render_phase cannot run in parallel with each other due to ResMut<GpuArrayBuffer>.

This was kind of intended as it makes it easy for each draw to find the correct MeshUniform in the shader, but it does have the above downsides.

@JMS55 JMS55 added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times P-High This is particularly urgent, and deserves immediate attention labels Feb 7, 2024
@JMS55 JMS55 added this to the 0.14 milestone Feb 7, 2024
@JMS55 JMS55 added the X-Controversial There is active debate or serious implications around merging this PR label Feb 7, 2024
@james7132
Copy link
Member

james7132 commented Feb 11, 2024

Additionally, each batch_and_prepare_render_phase cannot run in parallel with each other due to ResMut<GpuArrayBuffer>.

This is potentially avoidable by splitting the resource based on phase, though I'm not sure how that would play out on the GPU side. Though that doesn't address the issue of the duplicate copies.

@james7132
Copy link
Member

When trying to accelerate encase encoding, I found that the memory copy and encoding costs aren't the bottleneck in the batching systems. It may come down to the fact that we're computing the matrix inverses, potentially redundantly, across multiple render phases.

@JMS55 JMS55 removed this from the 0.14 milestone Apr 25, 2024
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-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times P-High This is particularly urgent, and deserves immediate attention X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

No branches or pull requests

2 participants