From 570860ec5f783ee42d7649c8b094596258bba814 Mon Sep 17 00:00:00 2001
From: Emil Ernerfeldt <emilernerfeldt@gmail.com>
Date: Wed, 14 Oct 2020 00:29:11 +0200
Subject: [PATCH] [refactor] Create helper struct Mutex (with double-lock
 detection)

---
 egui/src/context.rs     | 56 ++++++++++++++++-------------------------
 egui/src/lib.rs         |  1 +
 egui/src/mutex.rs       | 49 ++++++++++++++++++++++++++++++++++++
 egui/src/paint/font.rs  |  7 ++++--
 egui/src/paint/fonts.rs |  2 +-
 egui/src/ui.rs          |  6 ++---
 egui_web/src/lib.rs     |  2 +-
 7 files changed, 82 insertions(+), 41 deletions(-)
 create mode 100644 egui/src/mutex.rs

diff --git a/egui/src/context.rs b/egui/src/context.rs
index 2463fa1c..921ec60b 100644
--- a/egui/src/context.rs
+++ b/egui/src/context.rs
@@ -3,12 +3,14 @@ use std::sync::{
     Arc,
 };
 
-use {
-    ahash::AHashMap,
-    parking_lot::{Mutex, MutexGuard},
-};
+use ahash::AHashMap;
 
-use crate::{animation_manager::AnimationManager, paint::*, *};
+use crate::{
+    animation_manager::AnimationManager,
+    mutex::{Mutex, MutexGuard},
+    paint::*,
+    *,
+};
 
 #[derive(Clone, Copy, Default)]
 struct PaintStats {
@@ -60,15 +62,15 @@ pub struct Context {
 impl Clone for Context {
     fn clone(&self) -> Self {
         Context {
-            options: Mutex::new(lock(&self.options, "options").clone()),
+            options: self.options.clone(),
             fonts: self.fonts.clone(),
             memory: self.memory.clone(),
             animation_manager: self.animation_manager.clone(),
             input: self.input.clone(),
-            graphics: Mutex::new(self.graphics.lock().clone()),
-            output: Mutex::new(self.output.lock().clone()),
-            used_ids: Mutex::new(self.used_ids.lock().clone()),
-            paint_stats: Mutex::new(*self.paint_stats.lock()),
+            graphics: self.graphics.clone(),
+            output: self.output.clone(),
+            used_ids: self.used_ids.clone(),
+            paint_stats: self.paint_stats.clone(),
             repaint_requests: self.repaint_requests.load(SeqCst).into(),
         }
     }
@@ -84,15 +86,15 @@ impl Context {
     }
 
     pub fn memory(&self) -> MutexGuard<'_, Memory> {
-        lock(&self.memory, "memory")
+        self.memory.lock()
     }
 
     pub fn graphics(&self) -> MutexGuard<'_, GraphicLayers> {
-        lock(&self.graphics, "graphics")
+        self.graphics.lock()
     }
 
     pub fn output(&self) -> MutexGuard<'_, Output> {
-        lock(&self.output, "output")
+        self.output.lock()
     }
 
     /// Call this if there is need to repaint the UI, i.e. if you are showing an animation.
@@ -127,15 +129,15 @@ impl Context {
     /// Will become active at the start of the next frame.
     /// `pixels_per_point` will be ignored (overwritten at start of each frame with the contents of input)
     pub fn set_fonts(&self, font_definitions: FontDefinitions) {
-        lock(&self.options, "options").font_definitions = font_definitions;
+        self.options.lock().font_definitions = font_definitions;
     }
 
     pub fn style(&self) -> Arc<Style> {
-        lock(&self.options, "options").style.clone()
+        self.options.lock().style.clone()
     }
 
     pub fn set_style(&self, style: impl Into<Arc<Style>>) {
-        lock(&self.options, "options").style = style.into();
+        self.options.lock().style = style.into();
     }
 
     pub fn pixels_per_point(&self) -> f32 {
@@ -183,7 +185,7 @@ impl Context {
         self.used_ids.lock().clear();
 
         self.input = std::mem::take(&mut self.input).begin_frame(new_raw_input);
-        let mut font_definitions = lock(&self.options, "options").font_definitions.clone();
+        let mut font_definitions = self.options.lock().font_definitions.clone();
         font_definitions.pixels_per_point = self.input.pixels_per_point();
         let same_as_current = match &self.fonts {
             None => false,
@@ -220,7 +222,7 @@ impl Context {
     }
 
     fn paint(&self) -> PaintJobs {
-        let mut paint_options = lock(&self.options, "options").paint_options;
+        let mut paint_options = self.options.lock().paint_options;
         paint_options.aa_size = 1.0 / self.pixels_per_point();
         let paint_commands = self.drain_paint_lists();
         let num_primitives = paint_commands.len();
@@ -532,9 +534,9 @@ impl Context {
         CollapsingHeader::new("Painting")
             .default_open(true)
             .show(ui, |ui| {
-                let mut paint_options = lock(&self.options, "options").paint_options;
+                let mut paint_options = self.options.lock().paint_options;
                 paint_options.ui(ui);
-                lock(&self.options, "options").paint_options = paint_options;
+                self.options.lock().paint_options = paint_options;
             });
     }
 
@@ -651,17 +653,3 @@ impl PaintStats {
         ui.label(format!("Triangles: {}", self.num_triangles));
     }
 }
-
-#[cfg(debug_assertions)]
-fn lock<'m, T>(mutex: &'m Mutex<T>, what: &'static str) -> MutexGuard<'m, T> {
-    // TODO: detect if we are trying to lock the same mutex *from the same thread*.
-    // at the moment we just panic on any double-locking of a mutex (so no multithreaded support in debug builds)
-    mutex
-        .try_lock()
-        .unwrap_or_else(|| panic!("The Mutex for {} is already locked. Probably a bug", what))
-}
-
-#[cfg(not(debug_assertions))]
-fn lock<'m, T>(mutex: &'m Mutex<T>, _what: &'static str) -> MutexGuard<'m, T> {
-    mutex.lock()
-}
diff --git a/egui/src/lib.rs b/egui/src/lib.rs
index 32182471..e5e09541 100644
--- a/egui/src/lib.rs
+++ b/egui/src/lib.rs
@@ -60,6 +60,7 @@ mod layout;
 pub mod math;
 mod memory;
 pub mod menu;
+pub mod mutex;
 pub mod paint;
 mod painter;
 mod style;
diff --git a/egui/src/mutex.rs b/egui/src/mutex.rs
new file mode 100644
index 00000000..5604f66f
--- /dev/null
+++ b/egui/src/mutex.rs
@@ -0,0 +1,49 @@
+//! Helper module for a Mutex that
+//! detects double-locking on the same thread in debug mode.
+
+// TODO: feature-flag for `parking_lot` vs `std::sync::Mutex`.
+pub use parking_lot::MutexGuard;
+
+#[derive(Default)]
+pub struct Mutex<T>(parking_lot::Mutex<T>);
+
+impl<T> Mutex<T> {
+    #[inline(always)]
+    pub fn new(val: T) -> Self {
+        Self(parking_lot::Mutex::new(val))
+    }
+
+    #[cfg(debug_assertions)]
+    pub fn lock(&self) -> MutexGuard<'_, T> {
+        // TODO: detect if we are trying to lock the same mutex from the same thread (bad)
+        // vs locking it from another thread (fine).
+        // At the moment we just panic on any double-locking of a mutex (so no multithreaded support in debug builds)
+        self.0
+            .try_lock()
+            .expect("The Mutex is already locked. Probably a bug")
+    }
+
+    #[inline(always)]
+    #[cfg(not(debug_assertions))]
+    pub fn lock(&self) -> MutexGuard<'_, T> {
+        self.0.lock()
+    }
+}
+
+impl<T> Clone for Mutex<T>
+where
+    T: Clone,
+{
+    fn clone(&self) -> Self {
+        Self::new(self.lock().clone())
+    }
+}
+
+// impl<T> PartialEq for Mutex<T>
+// where
+//     T: PartialEq,
+// {
+//     fn eq(&self, other: &Self) -> bool {
+//         std::ptr::eq(self, other) || self.lock().eq(&other.lock())
+//     }
+// }
diff --git a/egui/src/paint/font.rs b/egui/src/paint/font.rs
index 8e0f8547..3435adc3 100644
--- a/egui/src/paint/font.rs
+++ b/egui/src/paint/font.rs
@@ -2,11 +2,14 @@ use std::sync::Arc;
 
 use {
     ahash::AHashMap,
-    parking_lot::{Mutex, RwLock},
+    parking_lot::RwLock,
     rusttype::{point, Scale},
 };
 
-use crate::math::{vec2, Vec2};
+use crate::{
+    math::{vec2, Vec2},
+    mutex::Mutex,
+};
 
 use super::texture_atlas::TextureAtlas;
 
diff --git a/egui/src/paint/fonts.rs b/egui/src/paint/fonts.rs
index e03109e9..ae1e0769 100644
--- a/egui/src/paint/fonts.rs
+++ b/egui/src/paint/fonts.rs
@@ -4,7 +4,7 @@ use std::{
     sync::Arc,
 };
 
-use parking_lot::Mutex;
+use crate::mutex::Mutex;
 
 use super::{
     font::Font,
diff --git a/egui/src/ui.rs b/egui/src/ui.rs
index 7bbb5f67..251136ca 100644
--- a/egui/src/ui.rs
+++ b/egui/src/ui.rs
@@ -2,7 +2,7 @@
 
 use std::{hash::Hash, sync::Arc};
 
-use crate::{color::*, containers::*, layout::*, paint::*, widgets::*, *};
+use crate::{color::*, containers::*, layout::*, mutex::MutexGuard, paint::*, widgets::*, *};
 
 /// Represents a region of the screen
 /// with a type of layout (horizontal or vertical).
@@ -154,13 +154,13 @@ impl Ui {
 
     /// The `Memory` of the `Context` associated with the `Ui`.
     /// Equivalent to `.ctx().memory()`.
-    pub fn memory(&self) -> parking_lot::MutexGuard<'_, Memory> {
+    pub fn memory(&self) -> MutexGuard<'_, Memory> {
         self.ctx().memory()
     }
 
     /// The `Output` of the `Context` associated with the `Ui`.
     /// Equivalent to `.ctx().output()`.
-    pub fn output(&self) -> parking_lot::MutexGuard<'_, Output> {
+    pub fn output(&self) -> MutexGuard<'_, Output> {
         self.ctx().output()
     }
 
diff --git a/egui_web/src/lib.rs b/egui_web/src/lib.rs
index 3c7048d5..6644f2c7 100644
--- a/egui_web/src/lib.rs
+++ b/egui_web/src/lib.rs
@@ -6,7 +6,7 @@ pub mod webgl;
 
 pub use backend::*;
 
-use parking_lot::Mutex;
+use egui::mutex::Mutex;
 use std::sync::Arc;
 use wasm_bindgen::prelude::*;