[Bf-blender-cvs] [97414fb484f] master: GHOST/Wayland: support adding/removing global objects at runtime

Campbell Barton noreply at git.blender.org
Tue Oct 25 04:19:50 CEST 2022


Commit: 97414fb484fe37b8da69a2b570a659535bbf2a0f
Author: Campbell Barton
Date:   Mon Oct 24 20:33:33 2022 +1100
Branches: master
https://developer.blender.org/rB97414fb484fe37b8da69a2b570a659535bbf2a0f

GHOST/Wayland: support adding/removing global objects at runtime

Share logic for adding/removing global objects and freeing them on exit.

Refactor object registration add/remove into an array of callbacks
to localize logic into generic functions for each kind of interface.

Also corrects own error where the primary clipboard manager wasn't
being destroyed on exit.

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

M	intern/ghost/intern/GHOST_SystemWayland.cpp

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

diff --git a/intern/ghost/intern/GHOST_SystemWayland.cpp b/intern/ghost/intern/GHOST_SystemWayland.cpp
index 67f475a2963..65b4e890d88 100644
--- a/intern/ghost/intern/GHOST_SystemWayland.cpp
+++ b/intern/ghost/intern/GHOST_SystemWayland.cpp
@@ -89,6 +89,15 @@ static void gwl_seat_capability_pointer_disable(GWL_Seat *seat);
 static void gwl_seat_capability_keyboard_disable(GWL_Seat *seat);
 static void gwl_seat_capability_touch_disable(GWL_Seat *seat);
 
+static bool gwl_registry_entry_remove_by_name(GWL_Display *display,
+                                              uint32_t name,
+                                              int *r_interface_slot);
+
+struct GWL_RegistryHandler;
+static int gwl_registry_handler_interface_slot_max();
+static const struct GWL_RegistryHandler *gwl_registry_handler_from_interface_slot(
+    int interface_slot);
+
 /* -------------------------------------------------------------------- */
 /** \name Local Defines
  *
@@ -138,6 +147,8 @@ static bool use_gnome_confine_hack = false;
 #  define USE_GNOME_NEEDS_LIBDECOR_HACK
 #endif
 
+#define WL_NAME_UNSET uint32_t(-1)
+
 /** \} */
 
 /* -------------------------------------------------------------------- */
@@ -527,6 +538,7 @@ static void gwl_libdecor_system_destroy(GWL_LibDecor_System *decor)
 {
   if (decor->context) {
     libdecor_unref(decor->context);
+    decor->context = nullptr;
   }
   delete decor;
 }
@@ -534,16 +546,21 @@ static void gwl_libdecor_system_destroy(GWL_LibDecor_System *decor)
 
 struct GWL_XDG_Decor_System {
   struct xdg_wm_base *shell = nullptr;
+  uint32_t shell_name = WL_NAME_UNSET;
+
   struct zxdg_decoration_manager_v1 *manager = nullptr;
+  uint32_t manager_name = WL_NAME_UNSET;
 };
 
-static void gwl_xdg_decor_system_destroy(GWL_XDG_Decor_System *decor)
+static void gwl_xdg_decor_system_destroy(struct GWL_Display *display, GWL_XDG_Decor_System *decor)
 {
   if (decor->manager) {
-    zxdg_decoration_manager_v1_destroy(decor->manager);
+    gwl_registry_entry_remove_by_name(display, decor->manager_name, nullptr);
+    GHOST_ASSERT(decor->manager == nullptr, "Internal registry error");
   }
   if (decor->shell) {
-    xdg_wm_base_destroy(decor->shell);
+    gwl_registry_entry_remove_by_name(display, decor->shell_name, nullptr);
+    GHOST_ASSERT(decor->shell == nullptr, "Internal registry error");
   }
   delete decor;
 }
@@ -707,9 +724,13 @@ struct GWL_Seat {
 /** \name Internal #GWL_Display Type (#wl_display & #wl_compositor wrapper)
  * \{ */
 
+struct GWL_RegistryEntry;
+
 struct GWL_Display {
   GHOST_SystemWayland *system = nullptr;
 
+  struct GWL_RegistryEntry *registry_entry = nullptr;
+
   struct wl_display *wl_display = nullptr;
   struct wl_compositor *wl_compositor = nullptr;
 
@@ -738,6 +759,178 @@ struct GWL_Display {
 
 /** \} */
 
+/* -------------------------------------------------------------------- */
+/** \name Internal #GWL_RegistryHandler
+ * \{ */
+
+struct GWL_RegisteryAdd_Params {
+  struct GWL_Display *display = nullptr;
+  struct wl_registry *wl_registry = nullptr;
+  uint32_t name = 0;
+  uint32_t version = 0;
+  /** Index within `gwl_registry_handlers`. */
+  int interface_slot = 0;
+};
+
+/**
+ * Add callback for object registry.
+ * \param display: The display which holes a reference to the global object.
+ * \param params: Various arguments needed for registration.
+ */
+using GWL_RegistryHandler_AddFn = void (*)(GWL_Display *display,
+                                           const GWL_RegisteryAdd_Params *params);
+
+/**
+ * Remove callback for object registry.
+ * \param display: The display which holes a reference to the global object.
+ * \param user_data: Optional reference to a sub element of `display`,
+ * use for outputs or seats for e.g. when the display may hold multiple references.
+ * \param on_exit: Enabled when freeing on exit.
+ * When true the consistency of references between objects should be kept valid.
+ * Otherwise it can be assumed that all objects will be freed and none will be used again,
+ * so there is no need to ensure a valid state.
+ */
+using GWL_RegistryEntry_FreeFn = void (*)(GWL_Display *display, void *user_data, bool on_exit);
+
+struct GWL_RegistryHandler {
+  /** Pointer to the name (not the name it's self), needed as the values aren't set on startup. */
+  const char *const *interface_p = nullptr;
+
+  GWL_RegistryHandler_AddFn add_fn = nullptr;
+  GWL_RegistryEntry_FreeFn remove_fn = nullptr;
+};
+
+/** \} */
+
+/* -------------------------------------------------------------------- */
+/** \name Internal #GWL_RegistryEntry
+ * \{ */
+
+/**
+ * Registered global objects can be removed by the compositor,
+ * these entries are a registry of objects and callbacks to properly remove them.
+ * These are also used to  remove all registered objects before exiting.
+ */
+struct GWL_RegistryEntry {
+  GWL_RegistryEntry *next = nullptr;
+  /**
+   * Optional pointer passed to `remove_fn`, typically the container in #GWL_Display
+   * in cases multiple instances of the same interface are supported.
+   */
+  void *user_data = nullptr;
+  /**
+   * A unique identifier used as a handle by `wl_registry_listener.global_remove`.
+   */
+  uint32_t name = WL_NAME_UNSET;
+  /**
+   * The index in `gwl_registry_handlers`,
+   * useful for accessing the interface name (for logging for example).
+   */
+  int interface_slot = 0;
+};
+
+static void gwl_registry_entry_add(GWL_Display *display,
+                                   const int interface_slot,
+                                   const uint32_t name,
+                                   void *user_data)
+{
+  GWL_RegistryEntry *reg = new GWL_RegistryEntry;
+
+  reg->interface_slot = interface_slot;
+  reg->name = name;
+  reg->user_data = user_data;
+
+  reg->next = display->registry_entry;
+  display->registry_entry = reg;
+}
+
+static bool gwl_registry_entry_remove_by_name(GWL_Display *display,
+                                              uint32_t name,
+                                              int *r_interface_slot)
+{
+  GWL_RegistryEntry *reg = display->registry_entry;
+  GWL_RegistryEntry **reg_link_p = &display->registry_entry;
+  bool found = false;
+
+  if (r_interface_slot) {
+    *r_interface_slot = -1;
+  }
+
+  while (reg) {
+    if (reg->name == name) {
+      GWL_RegistryEntry *reg_next = reg->next;
+      const GWL_RegistryHandler *handler = gwl_registry_handler_from_interface_slot(
+          reg->interface_slot);
+      handler->remove_fn(display, reg->user_data, false);
+      if (r_interface_slot) {
+        *r_interface_slot = reg->interface_slot;
+      }
+      delete reg;
+      *reg_link_p = reg_next;
+      found = true;
+      break;
+    }
+    reg_link_p = &reg->next;
+    reg = reg->next;
+  }
+  return found;
+}
+
+static bool gwl_registry_entry_remove_by_interface_slot(GWL_Display *display,
+                                                        int interface_slot,
+                                                        bool on_exit)
+{
+  GWL_RegistryEntry *reg = display->registry_entry;
+  GWL_RegistryEntry **reg_link_p = &display->registry_entry;
+  bool found = false;
+
+  while (reg) {
+    if (reg->interface_slot == interface_slot) {
+      GWL_RegistryEntry *reg_next = reg->next;
+      const GWL_RegistryHandler *handler = gwl_registry_handler_from_interface_slot(
+          interface_slot);
+      handler->remove_fn(display, reg->user_data, on_exit);
+      delete reg;
+      *reg_link_p = reg_next;
+      reg = reg_next;
+      found = true;
+      continue;
+    }
+    reg_link_p = &reg->next;
+    reg = reg->next;
+  }
+  return found;
+}
+
+/**
+ * Remove all global objects (on exit).
+ */
+static void gwl_registry_entry_remove_all(GWL_Display *display)
+{
+  const bool on_exit = true;
+
+  /* NOTE(@campbellbarton): Free by slot instead of simply looping over
+   * `display->registry_entry` so the order of freeing is always predictable.
+   * Otherwise global objects would be feed in the order they are registered.
+   * While this works in my tests, it could cause difficult to reproduce bugs
+   * where lesser used compositors or changes to existing compositors could
+   * crash on exit based on the order of freeing objects is out of our control.
+   *
+   * To give a concrete example of how this could fail, it's possible removing
+   * a tablet interface could reference the pointer interface, or the output interface.
+   * Even though references between interfaces shouldn't be necessary in most cases
+   * when `on_exit` is enabled. */
+  int interface_slot = gwl_registry_handler_interface_slot_max();
+  while (interface_slot--) {
+    gwl_registry_entry_remove_by_interface_slot(display, interface_slot, on_exit);
+  }
+
+  GHOST_ASSERT(display->registry_entry == nullptr, "Failed to remove all entries!");
+  display->registry_entry = nullptr;
+}
+
+/** \} */
+
 /* -------------------------------------------------------------------- */
 /** \name Private Utility Functions
  * \{ */
@@ -790,125 +983,22 @@ static GWL_SeatStatePointer *seat_state_pointer_from_cursor_surface(GWL_Seat *se
 
 static void display_destroy(GWL_Display *display)
 {
-  if (display->wl_data_device_manager) {
-    wl_data_device_manager_destroy(display->wl_data_device_manager);
-  }
-
-  if (display->wp_tablet_manager) {
-    zwp_tablet_manager_v2_destroy(display->wp_tablet_manager);
-  }
-
-  for (GWL_Output *output : display->outputs) {
-    wl_output_destroy(output->wl_output);
-    delete output;
-  }
-
-  for (GWL_Seat *seat : display->seats) {
-
-    /* First handle members that require locking.
-     * While highly unlikely, it's possible they are being used while this function runs. */
-    {
-      std::lock_guard lock{seat->data_source_mutex};
-      if (seat->data_source) {
-        gwl_simple_buffer_free_data(&seat->data_source->buffer_out);
-        if (seat->data_source->wl_source) {
-          wl_data_source_destroy(seat->data_source->wl_source);
-        }
-        delete seat->data_source;
-      }
-    }
-
-    {
-      std::lock_guard lock{seat->data_offer_dnd_mutex};
-      if (seat->data_offer_dnd) {
-        wl_data_offer_destroy(seat->data_offer_dnd->id);
-        delete seat->data_

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list