-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[Merged by Bors] - update wgpu to 0.13 #5168
Conversation
How does the performance compare? |
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've done a full pass, and all changes are uncontroversial migration changes or very minor cleanups.
Very rough performance numbers (fps before / fps after) many_sprites: 45 / 45 Probably some improvements here, but they're modest :) All tested examples are functioning normally 🎉 |
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.
The general changes look OK, some open questions and a bit more cleanup suggested. Cannot comment on the texture related changes.
A deeper dive seems to show some contradictory results. I profiled latest main and this PR merged against |
I'm getting a panic running 3d_scene with dx12 on windows 11. Panic Text
|
I see a very small regression in main_opaque_pass_3d, much smaller than yours @james7132
That message was already mentioned in #4461 (comment), it was my understanding that it was fixed on wgpu side |
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.
Mostly looks good. A couple of small changes, and a couple of questions.
@@ -1363,100 +1242,184 @@ pub fn ktx2_format_to_texture_format( | |||
ktx2::Format::EAC_R11G11_SNORM_BLOCK => TextureFormat::EacRg11Snorm, | |||
ktx2::Format::ASTC_4x4_UNORM_BLOCK | ktx2::Format::ASTC_4x4_SRGB_BLOCK => { |
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.
We could also map from these ktx2 formats to the TextureFormat::Astc { block, .. }
member somehow... I don't know if the code would be cleaner. Just a thought and I won't block (HA!) on it.
I've approved this, but doing a quick test with The median frame time is 0.6ms worse, which is about 4%, and the mean is almost 2ms worse, which is about 12.5%. It visually looks like it just stutters more, and I thought I observed that the CPU and GPU usage were both much higher. If I also focus on |
As far as I can tell the workaround was removed. Not sure if is supposed to be fixed on windows' side. |
As a note, I've done another run with main merged in so I'm not missing any potential optimisations from main, and some of the other systems maybe switch places, though I think more due to run to run variance, but the main_pass_opaque_3d timings hold steady with the performance regression. |
I remembered one thing that @cwfitzgerald mentioned during the wgpu 0.13 development cycle about tracking resources when doing command encoding to detect if something needed to be rebound or not. We use TrackedRenderPass to detect whether something is already bound and so then we skip rebinding. wgpu already did this for pipelines in 0.12 and in 0.13 also does it for bind groups. This means that if a bind group changes, both we and wgpu detect this. So we should remove this detection from TrackedRenderPass for pipelines (if we do) and bind groups and just let wgpu handle avoiding unnecessary pipeline/bind group rebindings. I don’t know if that accounts for the difference but maybe it could. |
We should also run some traces with wgpu’s spans enabled. |
I don't think this is it. I tried disabling bind group tracking in TrackedRenderPass and saw a 10ms/frame regression: |
Controversial label added because this should be tackled carefully and @cart should make the final call on whether we merge ASAP or try to fix perf first. I want to upgrade for 0.8 even with the performance regressions, but it would be a shame if we can't get those fixed. |
Good call! I wanted to indicate the performance regression in the labels somehow but didn't want to label it with performance and regression as that would suggest that this PR fixes a problem. I agree controversial expresses the PR needs some more scrutiny for some reason. |
bors r+ |
# Objective - Update wgpu to 0.13 - ~~Wait, is wgpu 0.13 released? No, but I had most of the changes already ready since playing with webgpu~~ well it has been released now - Also update parking_lot to 0.12 and naga to 0.9 ## Solution - Update syntax for wgsl shaders https://github.com/gfx-rs/wgpu/blob/master/CHANGELOG.md#wgsl-syntax - Add a few options, remove some references: https://github.com/gfx-rs/wgpu/blob/master/CHANGELOG.md#other-breaking-changes - fragment inputs should now exactly match vertex outputs for locations, so I added exports for those to be able to reuse them gfx-rs/wgpu#2704
Build failed: |
bors retry |
# Objective - Update wgpu to 0.13 - ~~Wait, is wgpu 0.13 released? No, but I had most of the changes already ready since playing with webgpu~~ well it has been released now - Also update parking_lot to 0.12 and naga to 0.9 ## Solution - Update syntax for wgsl shaders https://github.com/gfx-rs/wgpu/blob/master/CHANGELOG.md#wgsl-syntax - Add a few options, remove some references: https://github.com/gfx-rs/wgpu/blob/master/CHANGELOG.md#other-breaking-changes - fragment inputs should now exactly match vertex outputs for locations, so I added exports for those to be able to reuse them gfx-rs/wgpu#2704
Build failed: |
bors retry |
# Objective - Update wgpu to 0.13 - ~~Wait, is wgpu 0.13 released? No, but I had most of the changes already ready since playing with webgpu~~ well it has been released now - Also update parking_lot to 0.12 and naga to 0.9 ## Solution - Update syntax for wgsl shaders https://github.com/gfx-rs/wgpu/blob/master/CHANGELOG.md#wgsl-syntax - Add a few options, remove some references: https://github.com/gfx-rs/wgpu/blob/master/CHANGELOG.md#other-breaking-changes - fragment inputs should now exactly match vertex outputs for locations, so I added exports for those to be able to reuse them gfx-rs/wgpu#2704
# Objective - The line shader missed the wgpu 0.13 update (#5168) and does not work in it's current state ## Solution - update the shader
# Objective - The line shader missed the wgpu 0.13 update (#5168) and does not work in it's current state ## Solution - update the shader
# Objective - The line shader missed the wgpu 0.13 update (#5168) and does not work in it's current state ## Solution - update the shader
# Objective - The line shader missed the wgpu 0.13 update (#5168) and does not work in it's current state ## Solution - update the shader
# Objective - The line shader missed the wgpu 0.13 update (#5168) and does not work in it's current state ## Solution - update the shader
# Objective - The line shader missed the wgpu 0.13 update (#5168) and does not work in it's current state ## Solution - update the shader
# Objective - Update wgpu to 0.13 - ~~Wait, is wgpu 0.13 released? No, but I had most of the changes already ready since playing with webgpu~~ well it has been released now - Also update parking_lot to 0.12 and naga to 0.9 ## Solution - Update syntax for wgsl shaders https://github.com/gfx-rs/wgpu/blob/master/CHANGELOG.md#wgsl-syntax - Add a few options, remove some references: https://github.com/gfx-rs/wgpu/blob/master/CHANGELOG.md#other-breaking-changes - fragment inputs should now exactly match vertex outputs for locations, so I added exports for those to be able to reuse them gfx-rs/wgpu#2704
# Objective - The line shader missed the wgpu 0.13 update (bevyengine#5168) and does not work in it's current state ## Solution - update the shader
# Objective - Update wgpu to 0.13 - ~~Wait, is wgpu 0.13 released? No, but I had most of the changes already ready since playing with webgpu~~ well it has been released now - Also update parking_lot to 0.12 and naga to 0.9 ## Solution - Update syntax for wgsl shaders https://github.com/gfx-rs/wgpu/blob/master/CHANGELOG.md#wgsl-syntax - Add a few options, remove some references: https://github.com/gfx-rs/wgpu/blob/master/CHANGELOG.md#other-breaking-changes - fragment inputs should now exactly match vertex outputs for locations, so I added exports for those to be able to reuse them gfx-rs/wgpu#2704
# Objective - The line shader missed the wgpu 0.13 update (bevyengine#5168) and does not work in it's current state ## Solution - update the shader
# Objective - Update wgpu to 0.13 - ~~Wait, is wgpu 0.13 released? No, but I had most of the changes already ready since playing with webgpu~~ well it has been released now - Also update parking_lot to 0.12 and naga to 0.9 ## Solution - Update syntax for wgsl shaders https://github.com/gfx-rs/wgpu/blob/master/CHANGELOG.md#wgsl-syntax - Add a few options, remove some references: https://github.com/gfx-rs/wgpu/blob/master/CHANGELOG.md#other-breaking-changes - fragment inputs should now exactly match vertex outputs for locations, so I added exports for those to be able to reuse them gfx-rs/wgpu#2704
# Objective - The line shader missed the wgpu 0.13 update (bevyengine#5168) and does not work in it's current state ## Solution - update the shader
Objective
Wait, is wgpu 0.13 released? No, but I had most of the changes already ready since playing with webgpuwell it has been released nowSolution