Avoid deadlocks by using lambdas for context lock (#2625)

ctx.input().key_pressed(Key::A) -> ctx.input(|i| i.key_pressed(Key::A))

Author: emilk

.gitignore

@@ -1,6 +1,7 @@
+.DS_Store
 **/target
 **/target_ra
+**/target_wasm
 /.*.json
 /.vscode
 /media/*
-.DS_Store

CHANGELOG.md

@@ -5,6 +5,10 @@ NOTE: [`epaint`](crates/epaint/CHANGELOG.md), [`eframe`](crates/eframe/CHANGELOG
 
 
 ## Unreleased
+* ⚠️ BREAKING: `egui::Context` now use closures for locking ([#2625](https://github.com/emilk/egui/pull/2625)):
+  * `ctx.input().key_pressed(Key::A)` -> `ctx.input(|i| i.key_pressed(Key::A))`
+  * `ui.memory().toggle_popup(popup_id)` -> `ui.memory_mut(|mem| mem.toggle_popup(popup_id))`
+
 ### Added ⭐
 * Add `Response::drag_started_by` and `Response::drag_released_by` for convenience, similar to `dragged` and `dragged_by` ([#2507](https://github.com/emilk/egui/pull/2507)).
 * Add `PointerState::*_pressed` to check if the given button was pressed in this frame ([#2507](https://github.com/emilk/egui/pull/2507)).
@@ -18,6 +22,7 @@ NOTE: [`epaint`](crates/epaint/CHANGELOG.md), [`eframe`](crates/eframe/CHANGELOG
 * Add `ProgressBar::fill` if you want to set the fill color manually. ([#2618](https://github.com/emilk/egui/pull/2618)).
 * Add `Button::rounding` to enable round buttons ([#2616](https://github.com/emilk/egui/pull/2616)).
 * Add `WidgetVisuals::optional_bg_color` - set it to `Color32::TRANSPARENT` to hide button backgrounds ([#2621](https://github.com/emilk/egui/pull/2621)).
+* Add `Context::screen_rect` and `Context::set_cursor_icon` ([#2625](https://github.com/emilk/egui/pull/2625)).
 
 ### Changed 🔧
 * Improved plot grid appearance ([#2412](https://github.com/emilk/egui/pull/2412)).

Cargo.lock

@@ -371,6 +371,8 @@ egui uses the builder pattern for construction widgets. For instance: `ui.add(La
 
 Instead of using matching `begin/end` style function calls (which can be error prone) egui prefers to use `FnOnce` closures passed to a wrapping function. Lambdas are a bit ugly though, so I'd like to find a nicer solution to this. More discussion of this at <https://github.com/emilk/egui/issues/1004#issuecomment-1001650754>.
 
+egui uses a single `RwLock` for short-time locks on each access of `Context` data. This is to leave implementation simple and transactional and allow users to run their UI logic in parallel. Instead of creating mutex guards, egui uses closures passed to a wrapping function, e.g. `ctx.input(|i| i.key_down(Key::A))`. This is to make it less likely that a user would accidentally double-lock the `Context`, which would lead to a deadlock.
+
 ### Inspiration
 
 The one and only [Dear ImGui](https://github.com/ocornut/imgui) is a great Immediate Mode GUI for C++ which works with many backends. That library revolutionized how I think about GUI code and turned GUI programming from something I hated to do to something I now enjoy.
@@ -396,6 +398,7 @@ Notable contributions by:
 * [@mankinskin](https://github.com/mankinskin): [Context menus](https://github.com/emilk/egui/pull/543).
 * [@t18b219k](https://github.com/t18b219k): [Port glow painter to web](https://github.com/emilk/egui/pull/868).
 * [@danielkeller](https://github.com/danielkeller): [`Context` refactor](https://github.com/emilk/egui/pull/1050).
+* [@MaximOsipenko](https://github.com/MaximOsipenko): [`Context` lock refactor](https://github.com/emilk/egui/pull/2625).
 * And [many more](https://github.com/emilk/egui/graphs/contributors?type=a).
 
 egui is licensed under [MIT](LICENSE-MIT) OR [Apache-2.0](LICENSE-APACHE).

README.md

@@ -55,7 +55,7 @@ persistence = [
 ## `eframe` will call `puffin::GlobalProfiler::lock().new_frame()` for you
 puffin = ["dep:puffin", "egui_glow?/puffin", "egui-wgpu?/puffin"]
 
-## Enable screen reader support (requires `ctx.options().screen_reader = true;`)
+## Enable screen reader support (requires `ctx.options_mut(|o| o.screen_reader = true);`)
 screen_reader = ["egui-winit/screen_reader", "tts"]
 
 ## If set, eframe will look for the env-var `EFRAME_SCREENSHOT_TO` and write a screenshot to that location, and then quit.

crates/eframe/Cargo.toml

@@ -173,7 +173,7 @@ pub trait App {
     }
 
     /// If `true` a warm-up call to [`Self::update`] will be issued where
-    /// `ctx.memory().everything_is_visible()` will be set to `true`.
+    /// `ctx.memory(|mem| mem.everything_is_visible())` will be set to `true`.
     ///
     /// This can help pre-caching resources loaded by different parts of the UI, preventing stutter later on.
     ///

crates/eframe/src/epi.rs

@@ -257,7 +257,8 @@ impl EpiIntegration {
     ) -> Self {
         let egui_ctx = egui::Context::default();
 
-        *egui_ctx.memory() = load_egui_memory(storage.as_deref()).unwrap_or_default();
+        let memory = load_egui_memory(storage.as_deref()).unwrap_or_default();
+        egui_ctx.memory_mut(|mem| *mem = memory);
 
         let native_pixels_per_point = window.scale_factor() as f32;
 
@@ -315,11 +316,12 @@ impl EpiIntegration {
 
     pub fn warm_up(&mut self, app: &mut dyn epi::App, window: &winit::window::Window) {
         crate::profile_function!();
-        let saved_memory: egui::Memory = self.egui_ctx.memory().clone();
-        self.egui_ctx.memory().set_everything_is_visible(true);
+        let saved_memory: egui::Memory = self.egui_ctx.memory(|mem| mem.clone());
+        self.egui_ctx
+            .memory_mut(|mem| mem.set_everything_is_visible(true));
         let full_output = self.update(app, window);
         self.pending_full_output.append(full_output); // Handle it next frame
-        *self.egui_ctx.memory() = saved_memory; // We don't want to remember that windows were huge.
+        self.egui_ctx.memory_mut(|mem| *mem = saved_memory); // We don't want to remember that windows were huge.
         self.egui_ctx.clear_animations();
     }
 
@@ -446,7 +448,8 @@ impl EpiIntegration {
             }
             if _app.persist_egui_memory() {
                 crate::profile_scope!("egui_memory");
-                epi::set_value(storage, STORAGE_EGUI_MEMORY_KEY, &*self.egui_ctx.memory());
+                self.egui_ctx
+                    .memory(|mem| epi::set_value(storage, STORAGE_EGUI_MEMORY_KEY, mem));
             }
             {
                 crate::profile_scope!("App::save");

crates/eframe/src/native/epi_integration.rs

@@ -313,10 +313,11 @@ impl AppRunner {
 
     pub fn warm_up(&mut self) -> Result<(), JsValue> {
         if self.app.warm_up_enabled() {
-            let saved_memory: egui::Memory = self.egui_ctx.memory().clone();
-            self.egui_ctx.memory().set_everything_is_visible(true);
+            let saved_memory: egui::Memory = self.egui_ctx.memory(|m| m.clone());
+            self.egui_ctx
+                .memory_mut(|m| m.set_everything_is_visible(true));
             self.logic()?;
-            *self.egui_ctx.memory() = saved_memory; // We don't want to remember that windows were huge.
+            self.egui_ctx.memory_mut(|m| *m = saved_memory); // We don't want to remember that windows were huge.
             self.egui_ctx.clear_animations();
         }
         Ok(())
@@ -388,7 +389,7 @@ impl AppRunner {
     }
 
     fn handle_platform_output(&mut self, platform_output: egui::PlatformOutput) {
-        if self.egui_ctx.options().screen_reader {
+        if self.egui_ctx.options(|o| o.screen_reader) {
             self.screen_reader
                 .speak(&platform_output.events_description());
         }

crates/eframe/src/web/backend.rs

@@ -15,7 +15,7 @@ pub fn load_memory(ctx: &egui::Context) {
     if let Some(memory_string) = local_storage_get("egui_memory_ron") {
         match ron::from_str(&memory_string) {
             Ok(memory) => {
-                *ctx.memory() = memory;
+                ctx.memory_mut(|m| *m = memory);
             }
             Err(err) => {
                 tracing::error!("Failed to parse memory RON: {}", err);
@@ -29,7 +29,7 @@ pub fn load_memory(_: &egui::Context) {}
 
 #[cfg(feature = "persistence")]
 pub fn save_memory(ctx: &egui::Context) {
-    match ron::to_string(&*ctx.memory()) {
+    match ctx.memory(|mem| ron::to_string(mem)) {
         Ok(ron) => {
             local_storage_set("egui_memory_ron", &ron);
         }

crates/eframe/src/web/storage.rs

@@ -615,7 +615,7 @@ impl State {
         egui_ctx: &egui::Context,
         platform_output: egui::PlatformOutput,
     ) {
-        if egui_ctx.options().screen_reader {
+        if egui_ctx.options(|o| o.screen_reader) {
             self.screen_reader
                 .speak(&platform_output.events_description());
         }

crates/egui-winit/src/lib.rs

@@ -5,7 +5,7 @@
 use crate::*;
 
 /// State that is persisted between frames.
-// TODO(emilk): this is not currently stored in `memory().data`, but maybe it should be?
+// TODO(emilk): this is not currently stored in `Memory::data`, but maybe it should be?
 #[derive(Clone, Copy, Debug)]
 #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
 pub(crate) struct State {
@@ -231,7 +231,7 @@ impl Area {
 
         let layer_id = LayerId::new(order, id);
 
-        let state = ctx.memory().areas.get(id).copied();
+        let state = ctx.memory(|mem| mem.areas.get(id).copied());
         let is_new = state.is_none();
         if is_new {
             ctx.request_repaint(); // if we don't know the previous size we are likely drawing the area in the wrong place
@@ -278,7 +278,7 @@ impl Area {
             // Important check - don't try to move e.g. a combobox popup!
             if movable {
                 if move_response.dragged() {
-                    state.pos += ctx.input().pointer.delta();
+                    state.pos += ctx.input(|i| i.pointer.delta());
                 }
 
                 state.pos = ctx
@@ -288,9 +288,9 @@ impl Area {
 
             if (move_response.dragged() || move_response.clicked())
                 || pointer_pressed_on_area(ctx, layer_id)
-                || !ctx.memory().areas.visible_last_frame(&layer_id)
+                || !ctx.memory(|m| m.areas.visible_last_frame(&layer_id))
             {
-                ctx.memory().areas.move_to_top(layer_id);
+                ctx.memory_mut(|m| m.areas.move_to_top(layer_id));
                 ctx.request_repaint();
             }
 
@@ -329,7 +329,7 @@ impl Area {
         }
 
         let layer_id = LayerId::new(self.order, self.id);
-        let area_rect = ctx.memory().areas.get(self.id).map(|area| area.rect());
+        let area_rect = ctx.memory(|mem| mem.areas.get(self.id).map(|area| area.rect()));
         if let Some(area_rect) = area_rect {
             let clip_rect = ctx.available_rect();
             let painter = Painter::new(ctx.clone(), layer_id, clip_rect);
@@ -358,7 +358,7 @@ impl Prepared {
     }
 
     pub(crate) fn content_ui(&self, ctx: &Context) -> Ui {
-        let screen_rect = ctx.input().screen_rect();
+        let screen_rect = ctx.screen_rect();
 
         let bounds = if let Some(bounds) = self.drag_bounds {
             bounds.intersect(screen_rect) // protect against infinite bounds
@@ -410,29 +410,29 @@ impl Prepared {
 
         state.size = content_ui.min_rect().size();
 
-        ctx.memory().areas.set_state(layer_id, state);
+        ctx.memory_mut(|m| m.areas.set_state(layer_id, state));
 
         move_response
     }
 }
 
 fn pointer_pressed_on_area(ctx: &Context, layer_id: LayerId) -> bool {
     if let Some(pointer_pos) = ctx.pointer_interact_pos() {
-        let any_pressed = ctx.input().pointer.any_pressed();
+        let any_pressed = ctx.input(|i| i.pointer.any_pressed());
         any_pressed && ctx.layer_id_at(pointer_pos) == Some(layer_id)
     } else {
         false
     }
 }
 
 fn automatic_area_position(ctx: &Context) -> Pos2 {
-    let mut existing: Vec<Rect> = ctx
-        .memory()
-        .areas
-        .visible_windows()
-        .into_iter()
-        .map(State::rect)
-        .collect();
+    let mut existing: Vec<Rect> = ctx.memory(|mem| {
+        mem.areas
+            .visible_windows()
+            .into_iter()
+            .map(State::rect)
+            .collect()
+    });
     existing.sort_by_key(|r| r.left().round() as i32);
 
     let available_rect = ctx.available_rect();

crates/egui/src/containers/area.rs

@@ -26,13 +26,14 @@ pub struct CollapsingState {
 
 impl CollapsingState {
     pub fn load(ctx: &Context, id: Id) -> Option<Self> {
-        ctx.data()
-            .get_persisted::<InnerState>(id)
-            .map(|state| Self { id, state })
+        ctx.data_mut(|d| {
+            d.get_persisted::<InnerState>(id)
+                .map(|state| Self { id, state })
+        })
     }
 
     pub fn store(&self, ctx: &Context) {
-        ctx.data().insert_persisted(self.id, self.state);
+        ctx.data_mut(|d| d.insert_persisted(self.id, self.state));
     }
 
     pub fn id(&self) -> Id {
@@ -64,7 +65,7 @@ impl CollapsingState {
 
     /// 0 for closed, 1 for open, with tweening
     pub fn openness(&self, ctx: &Context) -> f32 {
-        if ctx.memory().everything_is_visible() {
+        if ctx.memory(|mem| mem.everything_is_visible()) {
             1.0
         } else {
             ctx.animate_bool(self.id, self.state.open)

crates/egui/src/containers/collapsing_header.rs

@@ -242,18 +242,13 @@ fn combo_box_dyn<'c, R>(
 ) -> InnerResponse<Option<R>> {
     let popup_id = button_id.with("popup");
 
-    let is_popup_open = ui.memory().is_popup_open(popup_id);
+    let is_popup_open = ui.memory(|m| m.is_popup_open(popup_id));
 
-    let popup_height = ui
-        .ctx()
-        .memory()
-        .areas
-        .get(popup_id)
-        .map_or(100.0, |state| state.size.y);
+    let popup_height = ui.memory(|m| m.areas.get(popup_id).map_or(100.0, |state| state.size.y));
 
     let above_or_below =
         if ui.next_widget_position().y + ui.spacing().interact_size.y + popup_height
-            < ui.ctx().input().screen_rect().bottom()
+            < ui.ctx().screen_rect().bottom()
         {
             AboveOrBelow::Below
         } else {
@@ -334,7 +329,7 @@ fn combo_box_dyn<'c, R>(
     });
 
     if button_response.clicked() {
-        ui.memory().toggle_popup(popup_id);
+        ui.memory_mut(|mem| mem.toggle_popup(popup_id));
     }
     let inner = crate::popup::popup_above_or_below_widget(
         ui,