[Bf-blender-cvs] [9fe9705bc0a] master: GHOST/Wayland: use a predictable order for interface registration

Campbell Barton noreply at git.blender.org
Wed Oct 26 03:20:25 CEST 2022


Commit: 9fe9705bc0af9d881905d75957c8dd467f8c8cb3
Author: Campbell Barton
Date:   Wed Oct 26 11:39:49 2022 +1100
Branches: master
https://developer.blender.org/rB9fe9705bc0af9d881905d75957c8dd467f8c8cb3

GHOST/Wayland: use a predictable order for interface registration

Defer interface registration so all known interfaces can be called in
the order defined by the array of supported types.

Without this, the compositor defined the order of registration so it
wasn't possible to rely on registration functions to depend on other
interfaces.

This caused initialization for 'seats' to be moved out of the
register callback to ensure multiple interfaces were initialized.
This isn't good for readability or maintenance since it meant the
add/remove callbacks didn't act on matching data.

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

M	intern/ghost/intern/GHOST_SystemWayland.cpp

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

diff --git a/intern/ghost/intern/GHOST_SystemWayland.cpp b/intern/ghost/intern/GHOST_SystemWayland.cpp
index 23f7c5060c0..4e3689ddfc2 100644
--- a/intern/ghost/intern/GHOST_SystemWayland.cpp
+++ b/intern/ghost/intern/GHOST_SystemWayland.cpp
@@ -96,6 +96,7 @@ static void gwl_registry_entry_remove_all(GWL_Display *display);
 
 struct GWL_RegistryHandler;
 static int gwl_registry_handler_interface_slot_max();
+static int gwl_registry_handler_interface_slot_from_string(const char *interface);
 static const struct GWL_RegistryHandler *gwl_registry_handler_from_interface_slot(
     int interface_slot);
 
@@ -142,7 +143,7 @@ static bool use_gnome_confine_hack = false;
 /**
  * KDE (plasma 5.26.1) has a bug where the cursor surface needs to be committed
  * (via `wl_surface_commit`) when it was hidden and is being set to visible again, see: T102048.
- * TODO: report this bug up-stream.
+ * See: https://bugs.kde.org/show_bug.cgi?id=461001
  */
 #define USE_KDE_TABLET_HIDDEN_CURSOR_HACK
 
@@ -762,10 +763,17 @@ static GWL_SeatStatePointer *gwl_seat_state_pointer_from_cursor_surface(
  * \{ */
 
 struct GWL_RegistryEntry;
+struct GWL_RegistryAdd_Deferred;
 
 struct GWL_Display {
   GHOST_SystemWayland *system = nullptr;
 
+  /**
+   * An array of registration arguments aligned with `gwl_registry_handlers`.
+   * Only used during registration at startup.
+   */
+  struct GWL_RegistryAdd_Deferred **registry_add_deferred = nullptr;
+  /** Registry entries, kept to allow removal at run-time. */
   struct GWL_RegistryEntry *registry_entry = nullptr;
 
   struct wl_display *wl_display = nullptr;
@@ -2641,8 +2649,9 @@ static const struct zwp_pointer_gesture_swipe_v1_listener gesture_swipe_listener
 /* -------------------------------------------------------------------- */
 /** \name Listener (Touch Seat), #wl_touch_listener
  *
- * TODO(@campbellbarton): Only setup the callbacks for now as I don't have
- * hardware that generates touch events.
+ * NOTE(@campbellbarton): It's not clear if this interface is used by popular compositors.
+ * It looks like GNOME/KDE only support `zwp_pointer_gestures_v1_interface`.
+ * If this isn't used anywhere, it could be removed.
  * \{ */
 
 static CLG_LogRef LOG_WL_TOUCH = {"ghost.wl.handle.touch"};
@@ -3838,22 +3847,6 @@ static void seat_handle_capabilities(void *data,
   else {
     gwl_seat_capability_touch_disable(seat);
   }
-
-  /* TODO(@campbellbarton): this could be moved out elsewhere. */
-  if (seat->system) {
-    zwp_primary_selection_device_manager_v1 *primary_selection_device_manager =
-        seat->system->wp_primary_selection_manager();
-    if (primary_selection_device_manager) {
-      if (seat->wp_primary_selection_device == nullptr) {
-        seat->wp_primary_selection_device = zwp_primary_selection_device_manager_v1_get_device(
-            primary_selection_device_manager, seat->wl_seat);
-
-        zwp_primary_selection_device_v1_add_listener(seat->wp_primary_selection_device,
-                                                     &primary_selection_device_listener,
-                                                     &seat->primary_selection);
-      }
-    }
-  }
 }
 
 static void seat_handle_name(void *data, struct wl_seat * /*wl_seat*/, const char *name)
@@ -4299,6 +4292,30 @@ static void gwl_registry_wl_seat_add(GWL_Display *display, const GWL_RegisteryAd
   display->seats.push_back(seat);
   wl_seat_add_listener(seat->wl_seat, &seat_listener, seat);
 
+  /* Register data device per seat for IPC between Wayland clients. */
+  if (display->wl_data_device_manager) {
+    seat->wl_data_device = wl_data_device_manager_get_data_device(display->wl_data_device_manager,
+                                                                  seat->wl_seat);
+    wl_data_device_add_listener(seat->wl_data_device, &data_device_listener, seat);
+  }
+
+  if (display->wp_tablet_manager) {
+    seat->wp_tablet_seat = zwp_tablet_manager_v2_get_tablet_seat(display->wp_tablet_manager,
+                                                                 seat->wl_seat);
+    zwp_tablet_seat_v2_add_listener(seat->wp_tablet_seat, &tablet_seat_listener, seat);
+  }
+
+  if (display->wp_primary_selection_device_manager) {
+    if (seat->wp_primary_selection_device == nullptr) {
+      seat->wp_primary_selection_device = zwp_primary_selection_device_manager_v1_get_device(
+          display->wp_primary_selection_device_manager, seat->wl_seat);
+
+      zwp_primary_selection_device_v1_add_listener(seat->wp_primary_selection_device,
+                                                   &primary_selection_device_listener,
+                                                   &seat->primary_selection);
+    }
+  }
+
   gwl_registry_entry_add(display, params->interface_slot, params->name, static_cast<void *>(seat));
 }
 static void gwl_registry_wl_seat_remove(GWL_Display *display, void *user_data, const bool on_exit)
@@ -4530,20 +4547,34 @@ static void gwl_registry_wp_primary_selection_device_manager_remove(GWL_Display
 /**
  * Map interfaces to to initialization functions.
  *
- * \note This list also defines the order interfaces are freed: from last to first,
- * so the most fundamental objects such as the compositor are freed last.
+ * \note This list also defines the order interfaces added & removed.
+ * - On startup interface registration is performed from first to last.
+ * - On exit interface removal runs from last to first.
+ *
+ * In general fundamental, low level objects such as the compositor and shared memory
+ * should be declared earlier and other interfaces that may use them should be declared later.
+ *
+ * This is useful for predictable registration, especially when one interface depends on another.
+ * It also helps avoid potential bugs caused by undefined order of removal.
  */
 static const GWL_RegistryHandler gwl_registry_handlers[] = {
+    /* Low level interfaces. */
     {
         &wl_compositor_interface.name,
         gwl_registry_compositor_add,
         gwl_registry_compositor_remove,
     },
+    {
+        &wl_shm_interface.name,
+        gwl_registry_wl_shm_add,
+        gwl_registry_wl_shm_remove,
+    },
     {
         &xdg_wm_base_interface.name,
         gwl_registry_xdg_wm_base_add,
         gwl_registry_xdg_wm_base_remove,
     },
+    /* Managers. */
     {
         &zxdg_decoration_manager_v1_interface.name,
         gwl_registry_xdg_decoration_manager_add,
@@ -4554,26 +4585,16 @@ static const GWL_RegistryHandler gwl_registry_handlers[] = {
         gwl_registry_xdg_output_manager_add,
         gwl_registry_xdg_output_manager_remove,
     },
-    {
-        &wl_output_interface.name,
-        gwl_registry_wl_output_add,
-        gwl_registry_wl_output_remove,
-    },
-    {
-        &wl_seat_interface.name,
-        gwl_registry_wl_seat_add,
-        gwl_registry_wl_seat_remove,
-    },
-    {
-        &wl_shm_interface.name,
-        gwl_registry_wl_shm_add,
-        gwl_registry_wl_shm_remove,
-    },
     {
         &wl_data_device_manager_interface.name,
         gwl_registry_wl_data_device_manager_add,
         gwl_registry_wl_data_device_manager_remove,
     },
+    {
+        &zwp_primary_selection_device_manager_v1_interface.name,
+        gwl_registry_wp_primary_selection_device_manager_add,
+        gwl_registry_wp_primary_selection_device_manager_remove,
+    },
     {
         &zwp_tablet_manager_v2_interface.name,
         gwl_registry_wp_tablet_manager_add,
@@ -4584,6 +4605,7 @@ static const GWL_RegistryHandler gwl_registry_handlers[] = {
         gwl_registry_wp_relative_pointer_manager_add,
         gwl_registry_wp_relative_pointer_manager_remove,
     },
+    /* Higher level interfaces. */
     {
         &zwp_pointer_constraints_v1_interface.name,
         gwl_registry_wp_pointer_constraints_add,
@@ -4594,10 +4616,19 @@ static const GWL_RegistryHandler gwl_registry_handlers[] = {
         gwl_registry_wp_pointer_gestures_add,
         gwl_registry_wp_pointer_gestures_remove,
     },
+    /* Display outputs. */
     {
-        &zwp_primary_selection_device_manager_v1_interface.name,
-        gwl_registry_wp_primary_selection_device_manager_add,
-        gwl_registry_wp_primary_selection_device_manager_remove,
+        &wl_output_interface.name,
+        gwl_registry_wl_output_add,
+        gwl_registry_wl_output_remove,
+    },
+    /* Seats.
+     * Keep the seat near the end to ensure other types are created first.
+     * as the seat creates data based on other interfaces. */
+    {
+        &wl_seat_interface.name,
+        gwl_registry_wl_seat_add,
+        gwl_registry_wl_seat_remove,
     },
     {nullptr, nullptr, nullptr},
 };
@@ -4611,6 +4642,17 @@ static int gwl_registry_handler_interface_slot_max()
   return ARRAY_SIZE(gwl_registry_handlers) - 1;
 }
 
+static int gwl_registry_handler_interface_slot_from_string(const char *interface)
+{
+  for (const GWL_RegistryHandler *handler = gwl_registry_handlers; handler->interface_p != nullptr;
+       handler++) {
+    if (STREQ(interface, *handler->interface_p)) {
+      return int(handler - gwl_registry_handlers);
+    }
+  }
+  return -1;
+}
+
 static const GWL_RegistryHandler *gwl_registry_handler_from_interface_slot(int interface_slot)
 {
   GHOST_ASSERT(uint32_t(interface_slot) < uint32_t(gwl_registry_handler_interface_slot_max()),
@@ -4618,6 +4660,20 @@ static const GWL_RegistryHandler *gwl_registry_handler_from_interface_slot(int i
   return &gwl_registry_handlers[interface_slot];
 }
 
+/**
+ * Support deferred registration, needed so interface registration order on startup is predictable.
+ * (defined by the interface order in `gwl_registry_handlers`)
+ */
+struct GWL_RegistryAdd_Deferred {
+  GWL_RegistryAdd_Deferred *next = nullptr;
+
+  /* Arguments to #global_handle_add, excluding some known args:
+   * `data`,  `wl_registry` & `interface` are known by the caller
+   * and don't need to be stored here. */
+  uint32_t name = 0;
+  uint32_t version = 0;
+};
+
 static void global_handle_add(void *data,
                               struct wl_registry *wl_registry,
                               const uint32_t name,
@@ -4625,34 +4681,42 @@ static void global_handle_add(void *data,
                               const ui

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list