Use fragile to remove Send + Sync impls

melody-rs · Jan 4, 2024 · 8a4af0a · 8a4af0a
1 parent 3471b50
commit 8a4af0a
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 60 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -102,7 +102,7 @@ puffin = "0.18"
 raw-window-handle = "0.5.0"
 
 parking_lot = { version = "0.12.1", features = [
-    "nightly", # This is required for parking_lot to work properly in WebAssembly builds with atomics support
+    "nightly",            # This is required for parking_lot to work properly in WebAssembly builds with atomics support
     "deadlock_detection",
 ] }
 once_cell = "1.18.0"
@@ -113,6 +113,7 @@ oneshot = "0.1.6"
 futures-lite = "2.1.0"
 async-std = "1.12.0"
 pin-project = "1"
+fragile = "2.0"
 
 poll-promise = { version = "0.3.0" }
 

diff --git a/crates/components/Cargo.toml b/crates/components/Cargo.toml
@@ -43,3 +43,5 @@ color-eyre.workspace = true
 qp-trie.workspace = true
 indextree = "4.6.0"
 lexical-sort = "0.3.1"
+
+fragile.workspace = true
diff --git a/crates/components/src/tilepicker.rs b/crates/components/src/tilepicker.rs
@@ -15,6 +15,7 @@
 // You should have received a copy of the GNU General Public License
 // along with Luminol.  If not, see <http://www.gnu.org/licenses/>.
 
+use fragile::Fragile;
 use itertools::Itertools;
 use std::sync::Arc;
 use std::time::Duration;
@@ -37,36 +38,30 @@ struct Resources {
     collision: luminol_graphics::collision::Collision,
 }
 
+// wgpu types are not Send + Sync on webassembly, so we use fragile to make sure we never access any wgpu resources across thread boundaries
 struct Callback {
-    resources: Arc<Resources>,
-    graphics_state: Arc<luminol_graphics::GraphicsState>,
+    resources: Fragile<Arc<Resources>>,
+    graphics_state: Fragile<Arc<luminol_graphics::GraphicsState>>,
 
     coll_enabled: bool,
 }
 
-//? SAFETY:
-//? wgpu resources are not Send + Sync on wasm, but egui_wgpu::CallbackTrait requires Send + Sync (because egui::Context is Send + Sync)
-//? as long as this callback does not leave the thread it was created on on wasm (which it shouldn't be) these are ok.
-#[allow(unsafe_code)]
-unsafe impl Send for Callback {}
-#[allow(unsafe_code)]
-unsafe impl Sync for Callback {}
-
 impl luminol_egui_wgpu::CallbackTrait for Callback {
     fn paint<'a>(
         &'a self,
         _info: egui::PaintCallbackInfo,
         render_pass: &mut wgpu::RenderPass<'a>,
         _callback_resources: &'a luminol_egui_wgpu::CallbackResources,
     ) {
-        self.resources
+        let resources = self.resources.get();
+        let graphics_state = self.graphics_state.get();
+
+        resources
             .tiles
-            .draw(&self.graphics_state, &[true], None, render_pass);
+            .draw(graphics_state, &[true], None, render_pass);
 
         if self.coll_enabled {
-            self.resources
-                .collision
-                .draw(&self.graphics_state, render_pass);
+            resources.collision.draw(graphics_state, render_pass);
         }
     }
 }
@@ -238,8 +233,8 @@ impl Tilepicker {
             .add(luminol_egui_wgpu::Callback::new_paint_callback(
                 absolute_scroll_rect,
                 Callback {
-                    resources: self.resources.clone(),
-                    graphics_state: graphics_state.clone(),
+                    resources: Fragile::new(self.resources.clone()),
+                    graphics_state: Fragile::new(graphics_state.clone()),
                     coll_enabled,
                 },
             ));

diff --git a/crates/graphics/Cargo.toml b/crates/graphics/Cargo.toml
@@ -40,3 +40,5 @@ camino.workspace = true
 
 luminol-data.workspace = true
 luminol-filesystem.workspace = true
+
+fragile.workspace = true
diff --git a/crates/graphics/src/event.rs b/crates/graphics/src/event.rs
@@ -17,6 +17,8 @@
 
 use std::sync::Arc;
 
+use fragile::Fragile;
+
 use crate::{quad::Quad, sprite::Sprite, tiles::Atlas, viewport::Viewport, GraphicsState};
 
 pub struct Event {
@@ -25,27 +27,23 @@ pub struct Event {
     pub sprite_size: egui::Vec2,
 }
 
+// wgpu types are not Send + Sync on webassembly, so we use fragile to make sure we never access any wgpu resources across thread boundaries
 struct Callback {
-    sprite: Arc<Sprite>,
-    graphics_state: Arc<GraphicsState>,
+    sprite: Fragile<Arc<Sprite>>,
+    graphics_state: Fragile<Arc<GraphicsState>>,
 }
 
-//? SAFETY:
-//? wgpu resources are not Send + Sync on wasm, but egui_wgpu::CallbackTrait requires Send + Sync (because egui::Context is Send + Sync)
-//? as long as this callback does not leave the thread it was created on on wasm (which it shouldn't be) these are ok.
-#[allow(unsafe_code)]
-unsafe impl Send for Callback {}
-#[allow(unsafe_code)]
-unsafe impl Sync for Callback {}
-
 impl luminol_egui_wgpu::CallbackTrait for Callback {
     fn paint<'a>(
         &'a self,
         _info: egui::PaintCallbackInfo,
         render_pass: &mut wgpu::RenderPass<'a>,
         _callback_resources: &'a luminol_egui_wgpu::CallbackResources,
     ) {
-        self.sprite.draw(&self.graphics_state, render_pass);
+        let sprite = self.sprite.get();
+        let graphics_state = self.graphics_state.get();
+
+        sprite.draw(graphics_state, render_pass);
     }
 }
 
@@ -140,8 +138,8 @@ impl Event {
         painter.add(luminol_egui_wgpu::Callback::new_paint_callback(
             rect,
             Callback {
-                sprite: self.sprite.clone(),
-                graphics_state,
+                sprite: Fragile::new(self.sprite.clone()),
+                graphics_state: Fragile::new(graphics_state),
             },
         ));
     }

diff --git a/crates/graphics/src/map.rs b/crates/graphics/src/map.rs
@@ -19,6 +19,8 @@ use std::sync::Arc;
 
 use std::time::Duration;
 
+use fragile::Fragile;
+
 use crate::{collision::Collision, tiles::Tiles, viewport::Viewport, GraphicsState, Plane};
 
 pub struct Map {
@@ -39,50 +41,42 @@ struct Resources {
     collision: Collision,
 }
 
+// wgpu types are not Send + Sync on webassembly, so we use fragile to make sure we never access any wgpu resources across thread boundaries
 struct Callback {
-    resources: Arc<Resources>,
-    graphics_state: Arc<GraphicsState>,
+    resources: Fragile<Arc<Resources>>,
+    graphics_state: Fragile<Arc<GraphicsState>>,
 
     pano_enabled: bool,
     enabled_layers: Vec<bool>,
     selected_layer: Option<usize>,
 }
 
 struct OverlayCallback {
-    resources: Arc<Resources>,
-    graphics_state: Arc<GraphicsState>,
+    resources: Fragile<Arc<Resources>>,
+    graphics_state: Fragile<Arc<GraphicsState>>,
 
     fog_enabled: bool,
     coll_enabled: bool,
 }
 
-//? SAFETY:
-//? wgpu resources are not Send + Sync on wasm, but egui_wgpu::CallbackTrait requires Send + Sync (because egui::Context is Send + Sync)
-//? as long as this callback does not leave the thread it was created on on wasm (which it shouldn't be) these are ok.
-#[allow(unsafe_code)]
-unsafe impl Send for Callback {}
-#[allow(unsafe_code)]
-unsafe impl Sync for Callback {}
-#[allow(unsafe_code)]
-unsafe impl Send for OverlayCallback {}
-#[allow(unsafe_code)]
-unsafe impl Sync for OverlayCallback {}
-
 impl luminol_egui_wgpu::CallbackTrait for Callback {
     fn paint<'a>(
         &'a self,
         _info: egui::PaintCallbackInfo,
         render_pass: &mut wgpu::RenderPass<'a>,
         _callback_resources: &'a luminol_egui_wgpu::CallbackResources,
     ) {
+        let resources = self.resources.get();
+        let graphics_state = self.graphics_state.get();
+
         if self.pano_enabled {
-            if let Some(panorama) = &self.resources.panorama {
-                panorama.draw(&self.graphics_state, render_pass);
+            if let Some(panorama) = &resources.panorama {
+                panorama.draw(graphics_state, render_pass);
             }
         }
 
-        self.resources.tiles.draw(
-            &self.graphics_state,
+        resources.tiles.draw(
+            graphics_state,
             &self.enabled_layers,
             self.selected_layer,
             render_pass,
@@ -97,16 +91,17 @@ impl luminol_egui_wgpu::CallbackTrait for OverlayCallback {
         render_pass: &mut wgpu::RenderPass<'a>,
         _callback_resources: &'a luminol_egui_wgpu::CallbackResources,
     ) {
+        let resources = self.resources.get();
+        let graphics_state = self.graphics_state.get();
+
         if self.fog_enabled {
-            if let Some(fog) = &self.resources.fog {
-                fog.draw(&self.graphics_state, render_pass);
+            if let Some(fog) = &resources.fog {
+                fog.draw(graphics_state, render_pass);
             }
         }
 
         if self.coll_enabled {
-            self.resources
-                .collision
-                .draw(&self.graphics_state, render_pass);
+            resources.collision.draw(graphics_state, render_pass);
         }
     }
 }
@@ -242,8 +237,8 @@ impl Map {
         painter.add(luminol_egui_wgpu::Callback::new_paint_callback(
             rect,
             Callback {
-                resources: self.resources.clone(),
-                graphics_state,
+                resources: Fragile::new(self.resources.clone()),
+                graphics_state: Fragile::new(graphics_state),
 
                 pano_enabled: self.pano_enabled,
                 enabled_layers: self.enabled_layers.clone(),
@@ -261,8 +256,8 @@ impl Map {
         painter.add(luminol_egui_wgpu::Callback::new_paint_callback(
             rect,
             OverlayCallback {
-                resources: self.resources.clone(),
-                graphics_state,
+                resources: Fragile::new(self.resources.clone()),
+                graphics_state: Fragile::new(graphics_state),
 
                 fog_enabled: self.fog_enabled,
                 coll_enabled: self.coll_enabled,
Original file line number	Diff line number	Diff line change
Expand Up		@@ -40,3 +40,5 @@ camino.workspace = true

		luminol-data.workspace = true
		luminol-filesystem.workspace = true

		fragile.workspace = true