[Bf-blender-cvs] [9f0e9f36be9] blender-v3.4-release: GHOST/Wayland: extend on comments regarding locking

[Campbell Barton]

Commit: 9f0e9f36be99461ed9076f5e0c9fcc33251d6b7d
Author: Campbell Barton
Date:   Wed Nov 16 10:09:39 2022 +1100
Branches: blender-v3.4-release
https://developer.blender.org/rB9f0e9f36be99461ed9076f5e0c9fcc33251d6b7d

GHOST/Wayland: extend on comments regarding locking

===================================================================

M	intern/ghost/intern/GHOST_WindowWayland.cpp
M	intern/ghost/intern/GHOST_WindowWayland.h

===================================================================

diff --git a/intern/ghost/intern/GHOST_WindowWayland.cpp b/intern/ghost/intern/GHOST_WindowWayland.cpp
index 909304577c3..9a916f9bd1b 100644
--- a/intern/ghost/intern/GHOST_WindowWayland.cpp
+++ b/intern/ghost/intern/GHOST_WindowWayland.cpp
@@ -115,7 +115,10 @@ struct GWL_Window {
 #endif
   WGL_XDG_Decor_Window *xdg_decor = nullptr;
 
-  /** The current value of frame, copied from `frame_pending` when applying updates. */
+  /**
+   * The current value of frame, copied from `frame_pending` when applying updates.
+   * This avoids the need for locking when reading from `frame`.
+   */
   GWL_WindowFrame frame;
   GWL_WindowFrame frame_pending;
 
@@ -543,14 +546,13 @@ static void frame_handle_configure(struct libdecor_frame *frame,
     win->libdecor->configured = true;
   }
 
+  /* Apply the changes. */
   {
     GWL_Window *win = static_cast<GWL_Window *>(data);
 #  ifdef USE_EVENT_BACKGROUND_THREAD
     GHOST_SystemWayland *system = win->ghost_system;
     const bool is_main_thread = system->main_thread_id == std::this_thread::get_id();
     if (!is_main_thread) {
-      /* NOTE(@campbellbarton): this only gets one redraw,
-       * I could not find a case where this causes problems. */
       gwl_window_pending_actions_tag(win, PENDING_FRAME_CONFIGURE);
     }
     else
diff --git a/intern/ghost/intern/GHOST_WindowWayland.h b/intern/ghost/intern/GHOST_WindowWayland.h
index e530d29f6d6..c43f7ca008c 100644
--- a/intern/ghost/intern/GHOST_WindowWayland.h
+++ b/intern/ghost/intern/GHOST_WindowWayland.h
@@ -23,13 +23,32 @@
  *
  * Solve this using a thread that handles events, locking must be performed as follows:
  *
- * - Lock #GWL_Display.server_mutex to prevent wl_display_dispatch / wl_display_roundtrip
+ * - Lock #GWL_Display.server_mutex to prevent #wl_display_dispatch / #wl_display_roundtrip
  *   running from multiple threads at once.
- *   GHOST functions that communicate with WAYLAND must use this lock to to be thread safe.
+ *
+ *   Even though WAYLAND functions that communicate with `wl_display_*` have their own locks,
+ *   GHOST functions that communicate with WAYLAND must use this lock to be thread safe.
+ *
+ *   Without this reading/writing values such as changing the cursor or setting the window size
+ *   could conflict with WAYLAND callbacks running in a separate thread.
+ *   So the `server_mutex` ensures either GHOST or WAYLAND is manipulating this data,
+ *   having two WAYLAND callbacks accessing the data at once isn't a problem.
+ *
+ *   \warning Some operations such as #ContextEGL creation/deletion & swap-buffers may call
+ *   #wl_display_dispatch indirectly, so it's important to take care to lock the `server_mutex`,
+ *   before accessing these functions too.
+ *
+ *   \warning An unfortunate side-effect of this is care needs to be taken not to call public
+ *   GHOST functions from each other in situations that would otherwise be supported.
+ *   As both using a `server_mutex` results in a dead-lock. In some cases this means the
+ *   implementation and the public function may need to be split.
  *
  * - Lock #GWL_Display.timer_mutex when WAYLAND callbacks manipulate timers.
  *
  * - Lock #GWL_Display.events_pending_mutex before manipulating #GWL_Display.events_pending.
+ *
+ * - Lock #GWL_Window.frame_pending_mutex before changing window size & frame settings,
+ *   this is flushed in #GHOST_WindowWayland::pending_actions_handle.
  */
 #define USE_EVENT_BACKGROUND_THREAD