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.

Sign up for GitHub

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

Jump to bottom

WASM renderer isn't thread safe #9304

Closed
Elabajaba opened this issue Jul 30, 2023 · 0 comments · Fixed by #12205
Closed

WASM renderer isn't thread safe #9304

Elabajaba opened this issue Jul 30, 2023 · 0 comments · Fixed by #12205
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior O-Web Specific to web (WASM) builds P-High This is particularly urgent, and deserves immediate attention P-Unsound A bug that results in undefined compiler behavior
Milestone
0.14

Comments

@Elabajaba
Copy link
Contributor

Bevy version

0.12 dev + wgpu 0.17

What went wrong

Wgpu types have been pretending to be Send + Sync on wasm despite being !Send + !Sync as it assumed wasm threading wasn't a thing, and it made using wpgu for native and wasm significantly easier.

Wasm threads are now starting to be used elsewhere, so wgpu has now changed it so that the types are still Send + Sync on native, but are now !Send + !Sync on wasm meaning that the renderer is now full of panics when targeting wasm because we're using #[derive(Resource)] on types that are !Send + !Sync

Additional information

See https://github.com/gfx-rs/wgpu/blob/trunk/CHANGELOG.md#wgpu-types-now-send-sync-on-wasm and gfx-rs/wgpu#3691
@Elabajaba Elabajaba added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jul 30, 2023
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen P-Unsound A bug that results in undefined compiler behavior and removed S-Needs-Triage This issue needs to be labelled labels Jul 30, 2023
@Elabajaba Elabajaba mentioned this issue Jul 30, 2023
Merged
@nicopap nicopap added the O-Web Specific to web (WASM) builds label Jul 31, 2023
github-merge-queue bot pushed a commit that referenced this issue Oct 9, 2023
@Elabajaba
wgpu 0.17 (#9302)
665dbcb 
~~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
regnarock pushed a commit to regnarock/bevy that referenced this issue Oct 13, 2023
@Elabajaba
wgpu 0.17 (bevyengine#9302)
8ada9d8 
~~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
@Elabajaba Elabajaba mentioned this issue Oct 17, 2023
Open
ameknite pushed a commit to ameknite/bevy that referenced this issue Nov 6, 2023
@Elabajaba @ameknite
wgpu 0.17 (bevyengine#9302)
1c7854b 
~~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 issue Jan 9, 2024
@Elabajaba
wgpu 0.17 (bevyengine#9302)
f643290 
~~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
@james7132 james7132 added this to the 0.14 milestone Feb 19, 2024
@james7132 james7132 added the P-High This is particularly urgent, and deserves immediate attention label Feb 19, 2024
@james7132 james7132 mentioned this issue Mar 23, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Mar 25, 2024
@kettle11 @mockersf
@alice-i-cecile @daxpedda
Get Bevy building for WebAssembly with multithreading (#12205)
b359740 
# Objective

This gets Bevy building on Wasm when the `atomics` flag is enabled. This
does not yet multithread Bevy itself, but it allows Bevy users to use a
crate like `wasm_thread` to spawn their own threads and manually
parallelize work. This is a first step towards resolving #4078 . Also
fixes #9304.

This provides a foothold so that Bevy contributors can begin to think
about multithreaded Wasm's constraints and Bevy can work towards changes
to get the engine itself multithreaded.

Some flags need to be set on the Rust compiler when compiling for Wasm
multithreading. Here's what my build script looks like, with the correct
flags set, to test out Bevy examples on web:

```bash
set -e
RUSTFLAGS='-C target-feature=+atomics,+bulk-memory,+mutable-globals' \
     cargo build --example breakout --target wasm32-unknown-unknown -Z build-std=std,panic_abort --release
 wasm-bindgen --out-name wasm_example \
   --out-dir examples/wasm/target \
   --target web target/wasm32-unknown-unknown/release/examples/breakout.wasm
 devserver --header Cross-Origin-Opener-Policy='same-origin' --header Cross-Origin-Embedder-Policy='require-corp' --path examples/wasm
```

A few notes:

1. `cpal` crashes immediately when the `atomics` flag is set. That is
patched in RustAudio/cpal#837, but not yet in
the latest crates.io release.

That can be temporarily worked around by patching Cpal like so:
```toml
[patch.crates-io]
cpal = { git = "https://github.com/RustAudio/cpal" }
```

2. When testing out `wasm_thread` you need to enable the `es_modules`
feature.

## Solution

The largest obstacle to compiling Bevy with `atomics` on web is that
`wgpu` types are _not_ Send and Sync. Longer term Bevy will need an
approach to handle that, but in the near term Bevy is already configured
to be single-threaded on web.

Therefor it is enough to wrap `wgpu` types in a
`send_wrapper::SendWrapper` that _is_ Send / Sync, but panics if
accessed off the `wgpu` thread.

---

## Changelog

- `wgpu` types that are not `Send` are wrapped in
`send_wrapper::SendWrapper` on Wasm + 'atomics'
- CommandBuffers are not generated in parallel on Wasm + 'atomics'

## Questions
- Bevy should probably add CI checks to make sure this doesn't regress.
Should that go in this PR or a separate PR? **Edit:** Added checks to
build Wasm with atomics

---------

Co-authored-by: François <mockersf@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: daxpedda <daxpedda@gmail.com>
Co-authored-by: François <francois.mockers@vleue.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior O-Web Specific to web (WASM) builds P-High This is particularly urgent, and deserves immediate attention P-Unsound A bug that results in undefined compiler behavior
Projects
None yet
Milestone
0.14
Development

Successfully merging a pull request may close this issue.

Get Bevy building for WebAssembly with multithreading kettle11/bevy
4 participants
@Elabajaba @james7132 @alice-i-cecile @nicopap