From 8a4af0a88f3fe3f282fc8e89e20c577b74f184a4 Mon Sep 17 00:00:00 2001 From: Speak2Erase Date: Thu, 4 Jan 2024 10:04:03 -0800 Subject: [PATCH] Use fragile to remove Send + Sync impls --- Cargo.lock | 8 +++++ Cargo.toml | 3 +- crates/components/Cargo.toml | 2 ++ crates/components/src/tilepicker.rs | 29 +++++++--------- crates/graphics/Cargo.toml | 2 ++ crates/graphics/src/event.rs | 24 ++++++------- crates/graphics/src/map.rs | 53 +++++++++++++---------------- 7 files changed, 61 insertions(+), 60 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 01654f67..98a7be87 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1876,6 +1876,12 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fragile" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c2141d6d6c8512188a7891b4b01590a45f6dac67afb4f255c4124dbb86d4eaa" + [[package]] name = "futures" version = "0.3.29" @@ -3032,6 +3038,7 @@ version = "0.4.0" dependencies = [ "color-eyre", "egui", + "fragile", "glam", "indextree", "itertools", @@ -3207,6 +3214,7 @@ dependencies = [ "crossbeam", "dashmap", "egui", + "fragile", "glam", "image 0.24.7", "itertools", diff --git a/Cargo.toml b/Cargo.toml index e98a5149..a37f9af1 100644 --- 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 index 6f86213f..d2f46d72 100644 --- 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 index 52a49e7b..291fc38e 100644 --- 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 . +use fragile::Fragile; use itertools::Itertools; use std::sync::Arc; use std::time::Duration; @@ -37,21 +38,14 @@ 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, - graphics_state: Arc, + resources: Fragile>, + graphics_state: Fragile>, 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, @@ -59,14 +53,15 @@ impl luminol_egui_wgpu::CallbackTrait for Callback { 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 index 023a7bdd..193a1ddb 100644 --- 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 index bcfaa483..2d1d3751 100644 --- 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,19 +27,12 @@ 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, - graphics_state: Arc, + sprite: Fragile>, + graphics_state: Fragile>, } -//? 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, @@ -45,7 +40,10 @@ impl luminol_egui_wgpu::CallbackTrait for Callback { 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 index 9bade0ce..d77f2eec 100644 --- 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,9 +41,10 @@ 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, - graphics_state: Arc, + resources: Fragile>, + graphics_state: Fragile>, pano_enabled: bool, enabled_layers: Vec, @@ -49,25 +52,13 @@ struct Callback { } struct OverlayCallback { - resources: Arc, - graphics_state: Arc, + resources: Fragile>, + graphics_state: Fragile>, 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, @@ -75,14 +66,17 @@ impl luminol_egui_wgpu::CallbackTrait for Callback { 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,