Add opt-out fragile-send-sync-non-atomic-wasm feature for wgpu (#5098)

Note this will break people depending on eframe or egui-wgpu with --no-default-features. I don't know what to do about that to be honest. * Closes #4914 * [x] I have followed the instructions in the PR template --------- Co-authored-by: Andreas Reich <[email protected]>
emilk · Sep 13, 2024 · 08f5eb3 · 08f5eb3
1 parent f658f8b
commit 08f5eb3
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 6 deletions.
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -5,6 +5,7 @@ name: Rust
 env:
   RUSTFLAGS: -D warnings
   RUSTDOCFLAGS: -D warnings
+  NIGHTLY_VERSION: nightly-2024-09-11
 
 jobs:
   fmt-crank-check-test:
@@ -113,6 +114,25 @@ jobs:
       - name: clippy wasm32
         run: ./scripts/clippy_wasm.sh
 
+  # requires a different toolchain from the other checks (nightly)
+  check_wasm_atomics:
+    name: Check wasm32+atomics
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@v4
+      - run: sudo apt-get update && sudo apt-get install libgtk-3-dev libatk1.0-dev
+
+      - name: Set up cargo cache
+        uses: Swatinem/rust-cache@v2
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{env.NIGHTLY_VERSION}}
+          targets: wasm32-unknown-unknown
+          components: rust-src
+
+      - name: Check wasm32+atomics eframe with wgpu
+        run: RUSTFLAGS='-C target-feature=+atomics' cargo +${{env.NIGHTLY_VERSION}} check -p eframe --lib --no-default-features --features wgpu --target wasm32-unknown-unknown -Z build-std=std,panic_abort
+
   # ---------------------------------------------------------------------------
 
   cargo-deny:

diff --git a/Cargo.toml b/Cargo.toml
@@ -92,10 +92,7 @@ web-time = "1.1.0" # Timekeeping for native and web
 wasm-bindgen = "0.2"
 wasm-bindgen-futures = "0.4"
 web-sys = "0.3.70"
-wgpu = { version = "22.1.0", default-features = false, features = [
-    # Make the renderer `Sync` even on wasm32, because it makes the code simpler:
-    "fragile-send-sync-non-atomic-wasm",
-] }
+wgpu = { version = "22.1.0", default-features = false }
 windows-sys = "0.52"
 winit = { version = "0.30.5", default-features = false }
 

diff --git a/crates/eframe/Cargo.toml b/crates/eframe/Cargo.toml
@@ -39,6 +39,7 @@ default = [
   "web_screen_reader",
   "winit/default",
   "x11",
+  "egui-wgpu?/fragile-send-sync-non-atomic-wasm",
 ]
 
 ## Enable platform accessibility API implementations through [AccessKit](https://accesskit.dev/).
@@ -185,7 +186,11 @@ objc2-app-kit = { version = "0.2.0", features = [
 # windows:
 [target.'cfg(any(target_os = "windows"))'.dependencies]
 winapi = { version = "0.3.9", features = ["winuser"] }
-windows-sys = { workspace = true, features = ["Win32_Foundation", "Win32_UI_Shell", "Win32_System_Com"] }
+windows-sys = { workspace = true, features = [
+  "Win32_Foundation",
+  "Win32_UI_Shell",
+  "Win32_System_Com",
+] }
 
 # -------------------------------------------
 # web:

diff --git a/crates/egui-wgpu/Cargo.toml b/crates/egui-wgpu/Cargo.toml
@@ -31,7 +31,7 @@ all-features = true
 rustdoc-args = ["--generate-link-to-definition"]
 
 [features]
-default = []
+default = ["fragile-send-sync-non-atomic-wasm"]
 
 ## Enable profiling with the [`puffin`](https://docs.rs/puffin) crate.
 puffin = ["dep:puffin"]
@@ -45,6 +45,12 @@ wayland = ["winit?/wayland"]
 ## Enables x11 support for winit.
 x11 = ["winit?/x11"]
 
+## Make the renderer `Sync` on wasm, exploiting that by default wasm isn't multithreaded.
+## It may make code easier, expecially when targeting both native and web.
+## On native most wgpu objects are send and sync, on the web they are not (by nature of the WebGPU specification).
+## This is not supported in [multithreaded WASM](https://gpuweb.github.io/gpuweb/explainer/#multithreading-transfer).
+## Thus that usage is guarded against with compiler errors in wgpu.
+fragile-send-sync-non-atomic-wasm = ["wgpu/fragile-send-sync-non-atomic-wasm"]
 
 [dependencies]
 egui = { workspace = true, default-features = false }

diff --git a/crates/egui-wgpu/src/lib.rs b/crates/egui-wgpu/src/lib.rs
@@ -173,6 +173,9 @@ impl RenderState {
             dithering,
         );
 
+        // On wasm, depending on feature flags, wgpu objects may or may not implement sync.
+        // It doesn't make sense to switch to Rc for that special usecase, so simply disable the lint.
+        #[allow(clippy::arc_with_non_send_sync)]
         Ok(Self {
             adapter: Arc::new(adapter),
             #[cfg(not(target_arch = "wasm32"))]

diff --git a/crates/egui-wgpu/src/renderer.rs b/crates/egui-wgpu/src/renderer.rs
@@ -7,8 +7,19 @@ use epaint::{emath::NumExt, PaintCallbackInfo, Primitive, Vertex};
 
 use wgpu::util::DeviceExt as _;
 
+// Only implements Send + Sync on wasm32 in order to allow storing wgpu resources on the type map.
+#[cfg(not(all(
+    target_arch = "wasm32",
+    not(feature = "fragile-send-sync-non-atomic-wasm"),
+)))]
 /// You can use this for storage when implementing [`CallbackTrait`].
 pub type CallbackResources = type_map::concurrent::TypeMap;
+#[cfg(all(
+    target_arch = "wasm32",
+    not(feature = "fragile-send-sync-non-atomic-wasm"),
+))]
+/// You can use this for storage when implementing [`CallbackTrait`].
+pub type CallbackResources = type_map::TypeMap;
 
 /// You can use this to do custom [`wgpu`] rendering in an egui app.
 ///
@@ -1017,6 +1028,11 @@ impl ScissorRect {
     }
 }
 
+// Look at the feature flag for an explanation.
+#[cfg(not(all(
+    target_arch = "wasm32",
+    not(feature = "fragile-send-sync-non-atomic-wasm"),
+)))]
 #[test]
 fn renderer_impl_send_sync() {
     fn assert_send_sync<T: Send + Sync>() {}