From 449f38adf2837c0e3663914f65aeaf595faa3139 Mon Sep 17 00:00:00 2001 From: Konkitoman Date: Wed, 27 Sep 2023 00:21:06 +0300 Subject: [PATCH] This is a temporary fix to not unsafe impl Sync + Send for backends The problem is that when we call Context::create_viewport_sync can be on any thread because egui::Context is Sync + Send but the backend is not! So we want the callback to the backend to be thread local! The problem that this adds is that now you cannot create more then one egui::Context, because the thread local variabile is static and i don't know how to store a non static LocalKey on egui::ContextImpl --- crates/eframe/src/native/run.rs | 10 --------- crates/egui/src/context.rs | 38 ++++++++++++++++++--------------- crates/egui/src/viewport.rs | 5 ++--- 3 files changed, 23 insertions(+), 30 deletions(-) diff --git a/crates/eframe/src/native/run.rs b/crates/eframe/src/native/run.rs index 73c9397e8..ab602965e 100644 --- a/crates/eframe/src/native/run.rs +++ b/crates/eframe/src/native/run.rs @@ -538,11 +538,6 @@ mod glow_integration { builders: HashMap, } - #[allow(unsafe_code)] - unsafe impl Sync for GlutinWindowContext {} - #[allow(unsafe_code)] - unsafe impl Send for GlutinWindowContext {} - impl GlutinWindowContext { /// There is a lot of complexity with opengl creation, so prefer extensive logging to get all the help we can to debug issues. /// @@ -1889,11 +1884,6 @@ mod wgpu_integration { #[derive(Clone)] pub struct Viewports(Arc>>); - #[allow(unsafe_code)] - unsafe impl Send for Viewports {} - #[allow(unsafe_code)] - unsafe impl Sync for Viewports {} - impl std::ops::Deref for Viewports { type Target = Arc>>; diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 7ad5a65ec..6f71970a9 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -172,6 +172,12 @@ impl Repaint { // ---------------------------------------------------------------------------- +thread_local! { + static EGUI_RENDER_SYNC: RwLock>> = RwLock::new(None); +} + +// ---------------------------------------------------------------------------- + #[derive(Default)] struct ContextImpl { /// `None` until the start of the first frame. @@ -201,8 +207,6 @@ struct ContextImpl { viewports: HashMap, viewport_commands: Vec<(ViewportId, ViewportCommand)>, - render_sync: Option>>, - viewport_counter: u64, is_desktop: bool, force_embedding: bool, @@ -467,9 +471,6 @@ impl Default for Context { s.write(|ctx| { ctx.force_embedding = true; - ctx.render_sync = Some(Arc::new(Box::new(move |ctx, _builder, _pair, render| { - render(ctx); - }))); }); s @@ -2554,15 +2555,16 @@ impl Context { /// When a viewport sync is created will be rendered by this function /// /// Look in `crates/eframe/native/run.rs` and search for ``set_render_sync_callback`` to see for what is used! + #[allow(clippy::unused_self)] pub fn set_render_sync_callback( &self, callback: impl for<'a> Fn(&Context, ViewportBuilder, ViewportIdPair, Box) - + Send - + Sync + 'static, ) { let callback = Box::new(callback); - self.write(|ctx| ctx.render_sync = Some(Arc::new(callback))); + EGUI_RENDER_SYNC.with(|render_sync| { + *render_sync.write() = Some(callback); + }); } /// If this is true no other native windows will be created @@ -2644,7 +2646,7 @@ impl Context { ) -> T { if !self.force_embedding() { let mut id_pair = ViewportIdPair::MAIN; - let render_sync = self.write(|ctx| { + self.write(|ctx| { id_pair.parent = ctx.viewport_id(); if let Some(window) = ctx.viewports.get_mut(&viewport_builder.id) { window.builder = viewport_builder.clone(); @@ -2666,18 +2668,20 @@ impl Context { }, ); } - - ctx.render_sync.clone() }); let mut out = None; { let out = &mut out; - render_sync.unwrap()( - self, - viewport_builder, - id_pair, - Box::new(move |context| *out = Some(func(context))), - ); + EGUI_RENDER_SYNC.with(|render_sync|{ + let render_sync = render_sync.read(); + let render_sync = render_sync.as_ref().expect("No EGUI_RENDER_SYNC callback on this thread, if you try to use Context::create_viewport_sync you cannot do that in other thread! If that is not the issue your egui intrecration is invalid or do not support sync viewports!"); + render_sync( + self, + viewport_builder, + id_pair, + Box::new(move |context| *out = Some(func(context))), + ); + }); } out.expect("egui backend is implemented incorrectly! Context::set_render_sync_callback") diff --git a/crates/egui/src/viewport.rs b/crates/egui/src/viewport.rs index 3eed7c3a1..96c4e6036 100644 --- a/crates/egui/src/viewport.rs +++ b/crates/egui/src/viewport.rs @@ -51,9 +51,8 @@ impl std::ops::Deref for ViewportIdPair { /// This is used to render an async viewport pub type ViewportRender = dyn Fn(&Context) + Sync + Send; -pub type ViewportRenderSyncCallback = dyn for<'a> Fn(&Context, ViewportBuilder, ViewportIdPair, Box) - + Send - + Sync; +pub type ViewportRenderSyncCallback = + dyn for<'a> Fn(&Context, ViewportBuilder, ViewportIdPair, Box); /// The filds in this struct should not be change directly, but is not problem tho! /// Every thing is wrapped in Option<> indicates that thing should not be changed!