[Merged by Bors] - update wgpu to 0.13 #5168

mockersf · 2022-07-01T16:13:25Z

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 Check that all vertex outputs are consumed by the fragment shader gfx-rs/wgpu#2704

alice-i-cecile · 2022-07-01T16:15:35Z

How does the performance compare?

alice-i-cecile

I've done a full pass, and all changes are uncontroversial migration changes or very minor cleanups.

alice-i-cecile · 2022-07-01T17:50:23Z

Very rough performance numbers (fps before / fps after)

many_sprites: 45 / 45
many_cubes: 100 / 110
many_foxes: 100 / 100
many_lights: 120 / 130

Probably some improvements here, but they're modest :)

All tested examples are functioning normally 🎉

james7132

The general changes look OK, some open questions and a bit more cleanup suggested. Cannot comment on the texture related changes.

crates/bevy_asset/Cargo.toml

crates/bevy_pbr/src/render/mesh.rs

crates/bevy_render/src/renderer/render_device.rs

james7132 · 2022-07-01T20:20:20Z

Very rough performance numbers (fps before / fps after)

many_sprites: 45 / 45 many_cubes: 100 / 110 many_foxes: 100 / 100 many_lights: 120 / 130

Probably some improvements here, but they're modest :)

All tested examples are functioning normally 🎉

A deeper dive seems to show some contradictory results. I profiled latest main and this PR merged against many_cubes sphere and I'm seeing a ~500us regression in frame time, mostly coming from main_opaque_pass_3d.

hymm · 2022-07-01T20:45:41Z

I'm getting a panic running 3d_scene with dx12 on windows 11.

Panic Text

Running `target\release\examples\3d_scene.exe`
2022-07-01T20:42:30.569547Z  INFO bevy_render::renderer: AdapterInfo { name: "AMD Radeon RX 6600", vendor: 4098, device: 29695, device_type: DiscreteGpu, backend: Dx12 }
2022-07-01T20:42:30.630963Z ERROR wgpu::backend::direct: Handling wgpu errors as fatal by default
thread 'main' panicked at 'wgpu error: Validation Error

Caused by:
    In Device::create_texture
      note: label = `point_light_shadow_map_texture`
    Texture usages TEXTURE_BINDING are not allowed on a texture of type Depth32Float

mockersf · 2022-07-01T22:27:40Z

I see a very small regression in main_opaque_pass_3d, much smaller than yours @james7132

Texture usages TEXTURE_BINDING are not allowed on a texture of type Depth32Float

That message was already mentioned in #4461 (comment), it was my understanding that it was fixed on wgpu side

assets/shaders/custom_material_chromatic_aberration.wgsl

assets/shaders/custom_material_screenspace_texture.wgsl

superdump

Mostly looks good. A couple of small changes, and a couple of questions.

assets/shaders/array_texture.wgsl

crates/bevy_pbr/src/render/mesh.rs

crates/bevy_pbr/src/render/mesh_vertex_output.wgsl

crates/bevy_render/src/texture/ktx2.rs

superdump · 2022-07-02T12:26:46Z

crates/bevy_render/src/texture/ktx2.rs

@@ -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 => {


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.

crates/bevy_sprite/src/mesh2d/mesh.rs

crates/bevy_sprite/src/mesh2d/mesh2d.wgsl

crates/bevy_sprite/src/mesh2d/mesh2d_vertex_output.wgsl

superdump · 2022-07-02T16:10:33Z

I've approved this, but doing a quick test with many_cubes -- sphere modified to use PresentMode::Immediate on an M1 Max I see ('this trace' is main in yellow, 'external trace' is this PR using wgpu 0.13 in red):

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 main_pass_opaque_3d, I see a median of 5.59ms on main, and 6.04ms on this PR which is 0.45ms worse and 8% slower. So I'd say we have a significant performance regression here. @cwfitzgerald FYI.

superdump · 2022-07-02T16:12:59Z

main:

this PR:

hymm · 2022-07-02T16:21:41Z

That message was already mentioned in #4461 (comment), it was my understanding that it was fixed on wgpu side

As far as I can tell the workaround was removed. Not sure if is supposed to be fixed on windows' side.

superdump · 2022-07-02T16:25:57Z

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.

superdump · 2022-07-02T18:51:10Z

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.

superdump · 2022-07-02T18:51:32Z

We should also run some traces with wgpu’s spans enabled.

james7132 · 2022-07-02T19:58:51Z

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.

I don't think this is it. I tried disabling bind group tracking in TrackedRenderPass and saw a 10ms/frame regression:

alice-i-cecile · 2022-07-02T21:03:00Z

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.

superdump · 2022-07-03T07:43:23Z

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.

cart · 2022-07-14T19:51:07Z

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

bors · 2022-07-14T20:21:57Z

Build failed:

run-examples-on-windows-dx12

cart · 2022-07-14T20:37:33Z

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

bors · 2022-07-14T21:08:25Z

Build failed:

run-examples-on-windows-dx12

cart · 2022-07-14T21:16:59Z

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

bors · 2022-07-14T21:42:48Z

Pull request successfully merged into main.

Build succeeded:

# 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

alice-i-cecile added A-Rendering Drawing game state to the screen C-Dependencies A change to the crates that Bevy depends on P-High This is particularly urgent, and deserves immediate attention labels Jul 1, 2022

alice-i-cecile added this to the Bevy 0.8 milestone Jul 1, 2022

alice-i-cecile approved these changes Jul 1, 2022

View reviewed changes

james7132 self-requested a review July 1, 2022 18:16

james7132 reviewed Jul 1, 2022

View reviewed changes

crates/bevy_asset/Cargo.toml Show resolved Hide resolved

crates/bevy_pbr/src/render/mesh.rs Show resolved Hide resolved

crates/bevy_render/src/renderer/render_device.rs Show resolved Hide resolved

james7132 approved these changes Jul 1, 2022

View reviewed changes

james7132 added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jul 1, 2022

james7132 requested a review from superdump July 1, 2022 19:27

alice-i-cecile mentioned this pull request Jul 1, 2022

Update naga requirement from 0.8.0 to 0.9.0 #5157

Closed

ManevilleF reviewed Jul 2, 2022

View reviewed changes

assets/shaders/custom_material_chromatic_aberration.wgsl Show resolved Hide resolved

assets/shaders/custom_material_screenspace_texture.wgsl Show resolved Hide resolved

ManevilleF approved these changes Jul 2, 2022

View reviewed changes

superdump requested changes Jul 2, 2022

View reviewed changes

superdump approved these changes Jul 2, 2022

View reviewed changes

alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Jul 2, 2022

mockersf added 3 commits July 14, 2022 21:19

use latest wgpu

9b1fdf6

remove depreaction and use fifo by default

e0c00c0

rename job in bors config

9a9eed6

mockersf force-pushed the wgpu-0.13 branch from ee65ec3 to 9a9eed6 Compare July 14, 2022 19:19

remove rand

228a683

increase timeout for windows

981ad9b

bors bot changed the title ~~update wgpu to 0.13~~ [Merged by Bors] - update wgpu to 0.13 Jul 14, 2022

bors bot closed this Jul 14, 2022

NiklasEi mentioned this pull request Jul 17, 2022

[Merged by Bors] - Fix line material shader #5348

Closed

bors bot pushed a commit that referenced this pull request Jul 17, 2022

Fix line material shader (#5348)

51ff814

# Objective - The line shader missed the wgpu 0.13 update (#5168) and does not work in it's current state ## Solution - update the shader

bors bot pushed a commit that referenced this pull request Jul 17, 2022

Fix line material shader (#5348)

ad18ca5

# Objective - The line shader missed the wgpu 0.13 update (#5168) and does not work in it's current state ## Solution - update the shader

bors bot pushed a commit that referenced this pull request Jul 17, 2022

Fix line material shader (#5348)

51ec03b

# Objective - The line shader missed the wgpu 0.13 update (#5168) and does not work in it's current state ## Solution - update the shader

bors bot pushed a commit that referenced this pull request Jul 17, 2022

Fix line material shader (#5348)

98fbc25

# Objective - The line shader missed the wgpu 0.13 update (#5168) and does not work in it's current state ## Solution - update the shader

bors bot pushed a commit that referenced this pull request Jul 17, 2022

Fix line material shader (#5348)

cedd89d

# Objective - The line shader missed the wgpu 0.13 update (#5168) and does not work in it's current state ## Solution - update the shader

bors bot pushed a commit that referenced this pull request Jul 17, 2022

Fix line material shader (#5348)

71368d4

# Objective - The line shader missed the wgpu 0.13 update (#5168) and does not work in it's current state ## Solution - update the shader

inodentry pushed a commit to IyesGames/bevy that referenced this pull request Aug 8, 2022

Fix line material shader (bevyengine#5348)

8ce91fc

# 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

james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022

Fix line material shader (bevyengine#5348)

50d3c98

# 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

ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023

Fix line material shader (bevyengine#5348)

1028128

# 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - update wgpu to 0.13 #5168

[Merged by Bors] - update wgpu to 0.13 #5168

mockersf commented Jul 1, 2022 •

edited

Loading

alice-i-cecile commented Jul 1, 2022

alice-i-cecile left a comment

alice-i-cecile commented Jul 1, 2022

james7132 left a comment

james7132 commented Jul 1, 2022

hymm commented Jul 1, 2022

mockersf commented Jul 1, 2022

superdump left a comment

superdump Jul 2, 2022

superdump commented Jul 2, 2022

superdump commented Jul 2, 2022

hymm commented Jul 2, 2022 •

edited

Loading

superdump commented Jul 2, 2022

superdump commented Jul 2, 2022

superdump commented Jul 2, 2022

james7132 commented Jul 2, 2022

alice-i-cecile commented Jul 2, 2022

superdump commented Jul 3, 2022

cart commented Jul 14, 2022

bors bot commented Jul 14, 2022

cart commented Jul 14, 2022

bors bot commented Jul 14, 2022

cart commented Jul 14, 2022

bors bot commented Jul 14, 2022

		@@ -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 => {

[Merged by Bors] - update wgpu to 0.13 #5168

[Merged by Bors] - update wgpu to 0.13 #5168

Conversation

mockersf commented Jul 1, 2022 • edited Loading

Objective

Solution

alice-i-cecile commented Jul 1, 2022

alice-i-cecile left a comment

Choose a reason for hiding this comment

alice-i-cecile commented Jul 1, 2022

james7132 left a comment

Choose a reason for hiding this comment

james7132 commented Jul 1, 2022

hymm commented Jul 1, 2022

mockersf commented Jul 1, 2022

superdump left a comment

Choose a reason for hiding this comment

superdump Jul 2, 2022

Choose a reason for hiding this comment

superdump commented Jul 2, 2022

superdump commented Jul 2, 2022

hymm commented Jul 2, 2022 • edited Loading

superdump commented Jul 2, 2022

superdump commented Jul 2, 2022

superdump commented Jul 2, 2022

james7132 commented Jul 2, 2022

alice-i-cecile commented Jul 2, 2022

superdump commented Jul 3, 2022

cart commented Jul 14, 2022

bors bot commented Jul 14, 2022

cart commented Jul 14, 2022

bors bot commented Jul 14, 2022

cart commented Jul 14, 2022

bors bot commented Jul 14, 2022

mockersf commented Jul 1, 2022 •

edited

Loading

hymm commented Jul 2, 2022 •

edited

Loading