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.17 #9302

Merged
merged 8 commits into from
Oct 9, 2023
Merged

wgpu 0.17 #9302

merged 8 commits into from
Oct 9, 2023

Conversation

Elabajaba
Copy link
Contributor

@Elabajaba Elabajaba commented Jul 29, 2023

Currently blocked on an upstream bug that causes crashes when minimizing/resizing on dx12 gfx-rs/wgpu#3967 wgpu 0.17.1 is out which fixes it

Objective

Keep wgpu up to date.

Solution

Update wgpu and naga_oil.

Currently this depends on an unreleased (and unmerged) branch of naga_oil, and hasn't been properly tested yet.

The wgpu side of this seems to have been an extremely trivial upgrade (all the upgrade work seems to be in naga_oil). This also lets us remove the workarounds for pack/unpack4x8unorm in the SSAO shaders.

Lets us close the dx12 part of #8888

related: #9304


Changelog

Update to wgpu 0.17 and naga_oil 0.9

@Elabajaba
Copy link
Contributor Author

Elabajaba commented Jul 29, 2023

Looks like wasm basically needs a rewrite of the renderer...

edit: https://github.com/gfx-rs/wgpu/blob/trunk/CHANGELOG.md#wgpu-types-now-send-sync-on-wasm

edit2: I've enabled the fragile-send-sync-non-atomic-wasm for now as we don't use threads on WASM.

@Elabajaba
Copy link
Contributor Author

I'm not sure if we should add a panic somewhere in the renderer in case someone compiles for wasm with -Ctarget-feature=+atomics to warn users that it isn't currently supported, or just leave it for now until bevy supports wasm threading.

@Selene-Amanita Selene-Amanita added A-Rendering Drawing game state to the screen C-Dependencies A change to the crates that Bevy depends on labels Jul 30, 2023
crates/bevy_render/Cargo.toml Outdated Show resolved Hide resolved
crates/bevy_render/Cargo.toml Outdated Show resolved Hide resolved
@JMS55 JMS55 mentioned this pull request Aug 20, 2023
@Elabajaba Elabajaba marked this pull request as ready for review August 27, 2023 02:58
@Elabajaba
Copy link
Contributor Author

Elabajaba commented Aug 27, 2023

Looks like minimizing is broken on dx12 for w/e reason, just panics immediately when I minimize a window.

edit: just checked, minimizing works on main.

edit 2: I can reproduce this upstream in wgpu

edit 3: looks like it's gfx-rs/wgpu#3967 and will also crash when resizing the window. This appears to be a dx12 only issue.

@Elabajaba
Copy link
Contributor Author

Elabajaba commented Sep 3, 2023

gfx-rs/wgpu#4106 is the upstream PR that fixes the crash, once that lands and there's a patch release this should be ready.

@JMS55
Copy link
Contributor

JMS55 commented Sep 24, 2023

@Elabajaba status update on this PR?

@Elabajaba
Copy link
Contributor Author

Still waiting on a patch release for wgpu.

@mockersf
Copy link
Member

wgpu 0.17.1 was just released 🎉

@Elabajaba
Copy link
Contributor Author

I can't reproduce the windows CI failure for the minimising example locally both on a dedicated GPU and forcing it to use WARP.

@JMS55 JMS55 added this to the 0.12 milestone Sep 28, 2023
@hymm
Copy link
Contributor

hymm commented Sep 28, 2023

can't reproduce on my laptop

AdapterInfo { name: "NVIDIA GeForce RTX 3070 Ti Laptop GPU", vendor: 4318, device: 9376, device_type: DiscreteGpu, driver: "", driver_info: "", backend: Dx12 }

@alice-i-cecile alice-i-cecile 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 Sep 30, 2023
@JMS55
Copy link
Contributor

JMS55 commented Oct 1, 2023

Can't reproduce either

2023-10-01T20:09:55.009018Z  INFO bevy_render::renderer: AdapterInfo { name: "NVIDIA GeForce RTX 3080", vendor: 4318, device: 8710, device_type: DiscreteGpu, driver: "NVIDIA", driver_info: "536.67", backend: Vulkan }
2023-10-01T20:09:55.953721Z  INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "Windows 11 Home", kernel: "22621", cpu: "AMD Ryzen 5 2600 Six-Core Processor", core_count: "6", memory: "15.9 GiB" }

@mockersf
Copy link
Member

mockersf commented Oct 1, 2023

re triggered CI, it's still failing with the same error:

2023-10-01T20:33:12.009607Z ERROR wgpu_hal::auxil::dxgi::exception: ID3D12CommandAllocator::Reset: A command allocator 0x000002ACC79B7860:'Unnamed ID3D12CommandAllocator Object' is being reset before previous executions associated with the allocator have completed. [ EXECUTION ERROR #552: COMMAND_ALLOCATOR_SYNC]    
2023-10-01T20:33:12.009841Z ERROR wgpu_hal::auxil::dxgi::exception: ID3D12Resource2::<final-release>: CORRUPTION: An ID3D12Resource object (0x000002ACC607F6E0:'(wgpu internal) Staging') is referenced by GPU operations in-flight on Command Queue (0x000002ACA6BA6CD0:'Unnamed ID3D12CommandQueue Object').  It is not safe to final-release objects that may have GPU operations pending.  This can result in application instability. [ EXECUTION ERROR #921: OBJECT_DELETED_WHILE_STILL_IN_USE]  

@Azorlogh
Copy link
Contributor

Azorlogh commented Oct 2, 2023

Just to let you know, wgpu-hal v0.17.1 is currently affected by another resize-related issue which prevents resizing on X11 + Vulkan.
It will be hopefully resolved soon in 0.17.2 as the fix PR is already there though

@cwfitzgerald
Copy link

Fixed in v0.17.2

@alice-i-cecile alice-i-cecile removed 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 Oct 4, 2023
@Elabajaba
Copy link
Contributor Author

Oof, the resizing example also works fine for me locally on a my gpu and on WARP. ($env:WGPU_ADAPTER_NAME="Microsoft Basic Render Driver", and changing

let adapter = instance
to use wgpu::util::initialize_adapter_from_env to test out warp locally)

@superdump superdump enabled auto-merge October 8, 2023 22:38
@alice-i-cecile alice-i-cecile 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 Oct 9, 2023
auto-merge was automatically disabled October 9, 2023 19:52

Head branch was pushed to by a user without write access

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 9, 2023
Merged via the queue into bevyengine:main with commit 665dbcb Oct 9, 2023
24 of 25 checks passed
@cart cart mentioned this pull request Oct 13, 2023
43 tasks
regnarock pushed a commit to regnarock/bevy that referenced this pull request Oct 13, 2023
~~Currently blocked on an upstream bug that causes crashes when
minimizing/resizing on dx12 gfx-rs/wgpu#3967
wgpu 0.17.1 is out which fixes it

# Objective

Keep wgpu up to date.

## Solution

Update wgpu and naga_oil.

Currently this depends on an unreleased (and unmerged) branch of
naga_oil, and hasn't been properly tested yet.

The wgpu side of this seems to have been an extremely trivial upgrade
(all the upgrade work seems to be in naga_oil). This also lets us remove
the workarounds for pack/unpack4x8unorm in the SSAO shaders.

Lets us close the dx12 part of
bevyengine#8888

related: bevyengine#9304

---

## Changelog

Update to wgpu 0.17 and naga_oil 0.9
ameknite pushed a commit to ameknite/bevy that referenced this pull request Nov 6, 2023
~~Currently blocked on an upstream bug that causes crashes when
minimizing/resizing on dx12 gfx-rs/wgpu#3967
wgpu 0.17.1 is out which fixes it

Keep wgpu up to date.

Update wgpu and naga_oil.

Currently this depends on an unreleased (and unmerged) branch of
naga_oil, and hasn't been properly tested yet.

The wgpu side of this seems to have been an extremely trivial upgrade
(all the upgrade work seems to be in naga_oil). This also lets us remove
the workarounds for pack/unpack4x8unorm in the SSAO shaders.

Lets us close the dx12 part of
bevyengine#8888

related: bevyengine#9304

---

Update to wgpu 0.17 and naga_oil 0.9
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
~~Currently blocked on an upstream bug that causes crashes when
minimizing/resizing on dx12 gfx-rs/wgpu#3967
wgpu 0.17.1 is out which fixes it

# Objective

Keep wgpu up to date.

## Solution

Update wgpu and naga_oil.

Currently this depends on an unreleased (and unmerged) branch of
naga_oil, and hasn't been properly tested yet.

The wgpu side of this seems to have been an extremely trivial upgrade
(all the upgrade work seems to be in naga_oil). This also lets us remove
the workarounds for pack/unpack4x8unorm in the SSAO shaders.

Lets us close the dx12 part of
bevyengine#8888

related: bevyengine#9304

---

## Changelog

Update to wgpu 0.17 and naga_oil 0.9
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 S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.