Skip to content

Upgrade to wgpu v24 #17542

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
merged 22 commits into from
Feb 9, 2025
Merged

Upgrade to wgpu v24 #17542

merged 22 commits into from
Feb 9, 2025

Conversation

JMS55
Copy link
Contributor

@JMS55 JMS55 commented Jan 26, 2025

Didn't remove WgpuWrapper. Not sure if it's needed or not still.

Testing

  • Did you test these changes? If so, how? Example runner
  • Are there any parts that need more testing? Web (portable atomics thingy?), DXC.

Migration Guide

  • Bevy has upgraded to wgpu v24.
  • When using the DirectX 12 rendering backend, the new priority system for choosing a shader compiler is as follows:
    • If the WGPU_DX12_COMPILER environment variable is set at runtime, it is used
    • Else if the new statically-linked-dxc feature is enabled, a custom version of DXC will be statically linked into your app at compile time.
    • Else Bevy will look in the app's working directory for dxcompiler.dll and dxil.dll at runtime.
    • Else if they are missing, Bevy will fall back to FXC (not recommended)

@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Jan 26, 2025
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Dependencies A change to the crates that Bevy depends on X-Uncontroversial This work is generally agreed upon S-Needs-Testing Testing must be done before this is safe to merge labels Jan 26, 2025
@JMS55 JMS55 requested a review from atlv24 January 27, 2025 05:39
@JMS55 JMS55 added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Jan 30, 2025
Copy link
Contributor

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

@JMS55 JMS55 marked this pull request as ready for review January 30, 2025 04:27
@JMS55
Copy link
Contributor Author

JMS55 commented Jan 30, 2025

I've changed how wgpu works with DX12 so that it no longer uses FXC.

If wgpu ever defaults to DX12 (I think I've seen people have that happen, despite the fact that I thought VK was supposed to be the default) then it's going to try and use a dynamic DXC and crash since it'll be missing. Users can enable the static DXC feature to fix that, or switch to VK, but it might be a little confusing.

Eventually once wgpu's DX12 backend is more stable we should consider making static-dxc on by default, and removing vulkan (and OpenGL) by default for smaller binary sizes.

@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-Needs-Testing Testing must be done before this is safe to merge labels Jan 30, 2025
@alice-i-cecile
Copy link
Member

Kicking off an example run: bevyengine/bevy-example-runner#108

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.

Assuming the example runner passes LGTM

@JMS55
Copy link
Contributor Author

JMS55 commented Jan 31, 2025

@alice-i-cecile good to merge?

@JMS55
Copy link
Contributor Author

JMS55 commented Feb 8, 2025

@pcwalton please review the previous two commits for gpu preprocessing changes.

Copy link
Contributor

@pcwalton pcwalton left a comment

Choose a reason for hiding this comment

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

LGTM!

@JMS55 JMS55 removed the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Feb 8, 2025
@JMS55
Copy link
Contributor Author

JMS55 commented Feb 8, 2025

The only issue remaining is texture_binding_array fails on macOS on the example runner. Not sure if there's a way to check logs or anything...

The fact that it only fails on macOS is probably a wgpu bug though. I'd like to merge this PR, we can look into it later.

@ChristopherBiscardi
Copy link
Contributor

on m1 mac

2025-02-08T21:47:30.165803Z  INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "macOS 14.2.1 Sonoma", kernel: "23.2.0", cpu: "Apple M1 Max", core_count: "10", memory: "64.0 GiB" }
2025-02-08T21:47:30.260362Z  INFO bevy_render::renderer: AdapterInfo { name: "Apple M1 Max", vendor: 0, device: 0, device_type: IntegratedGpu, driver: "", driver_info: "", backend: Metal }

texture_binding_array

screenshot-2025-02-08-at-13 47 06@2x

pcss

screenshot-2025-02-08-at-13 47 33@2x

ssr

❯ cargo run --example ssr
   Compiling bevy v0.16.0-dev (/Users/chris/github/bevyengine/bevy)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 5.93s
     Running `target/debug/examples/ssr`
2025-02-08T21:48:00.292415Z  INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "macOS 14.2.1 Sonoma", kernel: "23.2.0", cpu: "Apple M1 Max", core_count: "10", memory: "64.0 GiB" }
2025-02-08T21:48:00.386114Z  INFO bevy_render::renderer: AdapterInfo { name: "Apple M1 Max", vendor: 0, device: 0, device_type: IntegratedGpu, driver: "", driver_info: "", backend: Metal }
2025-02-08T21:48:01.014102Z  INFO bevy_winit::system: Creating new window Bevy Screen Space Reflections Example (0v1)
2025-02-08T21:48:06.615888Z ERROR wgpu::backend::wgpu_core: Shader translation error for stage ShaderStages(FRAGMENT): mapping of ResourceBinding { group: 0, binding: 17 } is missing
2025-02-08T21:48:06.615916Z ERROR wgpu::backend::wgpu_core: Please report it to https://github.com/gfx-rs/wgpu
2025-02-08T21:48:06.615937Z ERROR wgpu::backend::wgpu_core: Handling wgpu errors as fatal by default
thread 'Compute Task Pool (2)' panicked at /Users/chris/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-24.0.1/src/backend/wgpu_core.rs:1303:26:
wgpu error: Validation Error

Caused by:
  In Device::create_render_pipeline, label = 'deferred_lighting_pipeline'
    Internal error in ShaderStages(FRAGMENT) shader: mapping of ResourceBinding { group: 0, binding: 17 } is missing


note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Encountered a panic in system `bevy_render::render_resource::pipeline_cache::PipelineCache::process_pipeline_queue_system`!
thread '<unnamed>' panicked at /Users/chris/github/bevyengine/bevy/crates/bevy_render/src/render_resource/pipeline_cache.rs:577:28:
index out of bounds: the len is 0 but the index is 14
Encountered a panic in system `bevy_render::renderer::render_system`!

Copy link
Contributor

@ickshonpe ickshonpe left a comment

Choose a reason for hiding this comment

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

Doesn't break anything with UI or text rendering, I don't know anything about anything else at all.

@JMS55
Copy link
Contributor Author

JMS55 commented Feb 8, 2025

2025-02-08T21:48:06.615888Z ERROR wgpu::backend::wgpu_core: Shader translation error for stage ShaderStages(FRAGMENT): mapping of ResourceBinding { group: 0, binding: 17 } is missing
2025-02-08T21:48:06.615916Z ERROR wgpu::backend::wgpu_core: Please report it to https://github.com/gfx-rs/wgpu
2025-02-08T21:48:06.615937Z ERROR wgpu::backend::wgpu_core: Handling wgpu errors as fatal by default

Seems like a wgpu bug

@superdump superdump added this pull request to the merge queue Feb 8, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 8, 2025
@JMS55 JMS55 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 Feb 8, 2025
@superdump
Copy link
Contributor

superdump commented Feb 9, 2025

CI broke on this in the windows dx12 example runner:

2025-02-08T23:12:40.192270Z  INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "Windows Server 2022 Datacenter", kernel: "20348", cpu: "AMD EPYC 7763 64-Core Processor", core_count: "2", memory: "16.0 GiB" }    

thread 'main' panicked at crates\bevy_render\src\renderer\mod.rs:197:10:
Unable to find a GPU! Make sure you have installed required drivers!
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: process didn't exit successfully: `target\debug\examples\ambiguity_detection.exe` (exit code: 101)

@mockersf
Copy link
Member

mockersf commented Feb 9, 2025

JMS55#32 should fix windows dx12 CI

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Feb 9, 2025
Merged via the queue into bevyengine:main with commit 669d139 Feb 9, 2025
32 checks passed
mrchantey pushed a commit to mrchantey/bevy that referenced this pull request Feb 17, 2025
Didn't remove WgpuWrapper. Not sure if it's needed or not still.

## Testing

- Did you test these changes? If so, how? Example runner
- Are there any parts that need more testing? Web (portable atomics
thingy?), DXC.

## Migration Guide
- Bevy has upgraded to [wgpu
v24](https://github.com/gfx-rs/wgpu/blob/trunk/CHANGELOG.md#v2400-2025-01-15).
- When using the DirectX 12 rendering backend, the new priority system
for choosing a shader compiler is as follows:
- If the `WGPU_DX12_COMPILER` environment variable is set at runtime, it
is used
- Else if the new `statically-linked-dxc` feature is enabled, a custom
version of DXC will be statically linked into your app at compile time.
- Else Bevy will look in the app's working directory for
`dxcompiler.dll` and `dxil.dll` at runtime.
- Else if they are missing, Bevy will fall back to FXC (not recommended)

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: IceSentry <c.giguere42@gmail.com>
Co-authored-by: François Mockers <francois.mockers@vleue.com>
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 M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide 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
Status: Done
Development

Successfully merging this pull request may close these issues.