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

Wgpu 0.20 #13186

Merged
merged 8 commits into from
Jun 14, 2024
Merged

Wgpu 0.20 #13186

merged 8 commits into from
Jun 14, 2024

Conversation

Elabajaba
Copy link
Contributor

@Elabajaba Elabajaba commented May 2, 2024

Currently blocked on gfx-rs/wgpu#5774

Objective

Update to wgpu 0.20

Solution

Update to wgpu 0.20 and naga_oil 0.14.

Testing

Tested a few different examples on linux (vulkan, webgl2, webgpu) and windows (dx12 + vulkan) and they worked.


Changelog

Migration Guide

@alice-i-cecile alice-i-cecile added C-Dependencies A change to the crates that Bevy depends on X-Uncontroversial This work is generally agreed upon D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes A-Rendering Drawing game state to the screen S-Blocked This cannot move forward until something else changes labels May 2, 2024
@Elabajaba Elabajaba marked this pull request as ready for review May 28, 2024 21:01
@Elabajaba
Copy link
Contributor Author

Technically we should wait for gfx-rs/wgpu#5681 to be backported before landing this, but that's supposed to be merged and released tomorrow (May 29th).

@alice-i-cecile alice-i-cecile added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Blocked This cannot move forward until something else changes labels May 28, 2024
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.

I tested a bunch of 2d and 3d examples on windows 10 with a 4080 and everything worked correctly.

Would be nice not having TODOs, but they can be addressed later. We should still ship 0.14 with this

LGTM

@Elabajaba
Copy link
Contributor Author

I tested a bunch of 2d and 3d examples on windows 10 with a 4080 and everything worked correctly.

Would be nice not having TODOs, but they can be addressed later. We should still ship 0.14 with this

LGTM

The Todos shouldn't have any effect anyways, since they're for pipeline override constants which don't work on web currently (and aren't yet supported in naga_oil).

@SIGSTACKFAULT
Copy link
Contributor

SIGSTACKFAULT commented May 29, 2024

examples/3d_shapes dies of SIGBUS after ~5s on NixOS Unstable, RX6600.

2024-05-28.20-49-41.mp4

with --features wayland it brought Plasma down with it. The audio hitch is consistent.


EDIT: i relogged and now i can't reproduce it.

@alice-i-cecile alice-i-cecile added the S-Needs-Testing Testing must be done before this is safe to merge label May 29, 2024
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.

Tested a bunch of 3d examples with VK + DX12, lgtm.

2024-05-29T06:50:45.203720Z  INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "Windows 11 Home", kernel: "22631", cpu: "Intel(R) Core(TM) Ultra 7 155H", core_count: "16", memory: "15.6 GiB" }
2024-05-29T06:50:45.241110Z  INFO bevy_winit::system: Creating new window "App" (Entity { index: 0, generation: 1 })
2024-05-29T06:50:45.572420Z  INFO bevy_render::renderer: AdapterInfo { name: "Intel(R) Arc(TM) Graphics", vendor: 32902, device: 32085, device_type: IntegratedGpu, driver: "", driver_info: "", backend: Dx12 }

@killercup
Copy link
Contributor

Tested a bunch of examples on an Apple M1 Pro and they all looked good.

@Henauxg
Copy link
Contributor

Henauxg commented May 29, 2024

Tested on Windows 11 with a 4070 + both Vulkan & Dx12.

  • Edit: Nevermind, checked the wrong branch... Examples are working fine for me.

@IQuick143
Copy link
Contributor

Did testing on Linux (NVIDIA, KDE, XOrg):

The two_passes example does not render anything:
image
Also causes major lag to the whole system (UI & Audio froze up for a few seconds) when opening and closing the window.

Equivalent behaviour in the split_screen example.

Similar hangs occur when a lot of resizing is done.

Other than that all 3d examples (except meshlets and clearcoat which I haven't tested) work.

@Elabajaba
Copy link
Contributor Author

Is there an issue / PR we can link to that's blocking this? I saw that @Elabajaba ran into some Vulkan issues that pushed this to 0.15.

I tracked down what's causing the crash (using multiple command encoders) and made a wgpu issue for it gfx-rs/wgpu#5774.

@cwfitzgerald
Copy link

v0.20.1 is out now, thanks @Elabajaba for pushing this through!

@JMS55
Copy link
Contributor

JMS55 commented Jun 13, 2024

We should check if this is fixed now:

// HACK: Parallel command encoding is currently bugged on AMD + Windows + Vulkan with wgpu 0.19.1

@mockersf
Copy link
Member

no issue in CI after the wgpu 0.20.1 update on the examples 🎉

@@ -17,7 +17,7 @@ bevy_reflect = { path = "../bevy_reflect", version = "0.14.0-dev", features = [
bytemuck = { version = "1", features = ["derive"] }
serde = { version = "1.0", features = ["derive"], optional = true }
thiserror = "1.0"
wgpu-types = { version = "0.19", default-features = false, optional = true }
wgpu-types = { version = "0.20", default-features = false, optional = true }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
wgpu-types = { version = "0.20", default-features = false, optional = true }
wgpu-types = { version = "0.20.1", default-features = false, optional = true }

If 0.20.0 was so problematic, we should prevent it from being pulled in by making 0.20.1 the minimum version.

@@ -70,15 +70,15 @@ codespan-reporting = "0.11.0"
# It is enabled for now to avoid having to do a significant overhaul of the renderer just for wasm.
# When the 'atomics' feature is enabled `fragile-send-sync-non-atomic` does nothing
# and Bevy instead wraps `wgpu` types to verify they are not used off their origin thread.
wgpu = { version = "0.19.3", default-features = false, features = [
wgpu = { version = "0.20", default-features = false, features = [
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
wgpu = { version = "0.20", default-features = false, features = [
wgpu = { version = "0.20.1", default-features = false, features = [

@alice-i-cecile
Copy link
Member

I'm planning to merge this in right at the start of the 0.15 cycle.

@xangelix
Copy link

I'm planning to merge this in right at the start of the 0.15 cycle.

Is it possible we could get this in 0.14 and address any stability issues, if they happen, in further 0.14.x releases?
The upcoming release of egui will be on wgpu 0.20 and I'd like to keep the wgpu versions in sync if possible. emilk/egui#4560

@alice-i-cecile
Copy link
Member

I'll check with the rendering SMEs and other maintainers :)

@alice-i-cecile alice-i-cecile modified the milestones: 0.15, 0.14 Jun 13, 2024
@alice-i-cecile
Copy link
Member

Great; there's agreement to ship this in 0.14. Let me do a quick pass then let's get this merged.

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Blocked This cannot move forward until something else changes labels Jun 13, 2024
@alice-i-cecile
Copy link
Member

Diff looks reasonable, and it seems like the other issues are now resolved. Merging!

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 13, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 13, 2024
@xangelix
Copy link

Great; there's agreement to ship this in 0.14. Let me do a quick pass then let's get this merged.

Awesome, thanks so much!

@mockersf mockersf added this pull request to the merge queue Jun 14, 2024
Merged via the queue into bevyengine:main with commit 2825ac8 Jun 14, 2024
32 checks passed
mockersf added a commit that referenced this pull request Jun 14, 2024
Currently blocked on gfx-rs/wgpu#5774

# Objective

Update to wgpu 0.20

## Solution

Update to wgpu 0.20 and naga_oil 0.14.

## Testing

Tested a few different examples on linux (vulkan, webgl2, webgpu) and
windows (dx12 + vulkan) and they worked.

---

## Changelog

- Updated to wgpu 0.20. Note that we don't currently support wgpu's new
pipeline overridable constants, as they don't work on web currently and
need some more changes to naga_oil (and are somewhat redundant with
naga_oil's shader defs). See wgpu's changelog for more
https://github.com/gfx-rs/wgpu/blob/trunk/CHANGELOG.md#v0200-2024-04-28

## Migration Guide

TODO

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: François Mockers <mockersf@gmail.com>
@re0312 re0312 mentioned this pull request Jul 24, 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-Dependencies A change to the crates that Bevy depends on D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon
Projects
None yet
Development

Successfully merging this pull request may close these issues.