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.

1 files changed, 159 insertions, 86 deletions

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,
@@ -4555,26 +4586,16 @@ static const GWL_RegistryHandler gwl_registry_handlers[] = {
         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,
         gwl_registry_wp_tablet_manager_remove,
@@ -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 uint32_t version)
 {
   /* Log last since it's useful to know if the interface was handled or not. */
-
   GWL_Display *display = static_cast<GWL_Display *>(data);
-  /* The interface name that is ensured not to be freed. */
-  GWL_RegisteryAdd_Params params = {
-      .wl_registry = wl_registry,
-      .name = name,
-      .version = version,
-      .interface_slot = 0,
-  };
 
-  bool found = false, added = false;
-  for (const GWL_RegistryHandler *handler = gwl_registry_handlers; handler->interface_p != nullptr;
-       handler++) {
-    if (!STREQ(interface, *handler->interface_p)) {
-      continue;
+  const int interface_slot = gwl_registry_handler_interface_slot_from_string(interface);
+  bool added = false;
+
+  if (interface_slot != -1) {
+    /* Defer registration until later (running on startup). */
+    if (display->registry_add_deferred) {
+      GWL_RegistryAdd_Deferred *reg_add = new GWL_RegistryAdd_Deferred;
+      reg_add->name = name;
+      reg_add->version = version;
+
+      /* Add to the list. */
+      GWL_RegistryAdd_Deferred **reg_p = &display->registry_add_deferred[interface_slot];
+      reg_add->next = *reg_p;
+      *reg_p = reg_add;
+      return;
     }
+
+    const GWL_RegistryHandler *handler = &gwl_registry_handlers[interface_slot];
     const GWL_RegistryEntry *registry_entry_prev = display->registry_entry;
-    params.interface_slot = int(handler - gwl_registry_handlers);
+
+    /* The interface name that is ensured not to be freed. */
+    GWL_RegisteryAdd_Params params = {
+        .wl_registry = wl_registry,
+        .name = name,
+        .version = version,
+        .interface_slot = interface_slot,
+    };
 
     handler->add_fn(display, &params);
 
     added = display->registry_entry != registry_entry_prev;
-    found = true;
-    break;
   }
-
-  /* Not found. */
-  if (!found) {
+  else {
+    /* Not found. */
 #ifdef USE_GNOME_NEEDS_LIBDECOR_HACK
     if (STRPREFIX(interface, "gtk_shell")) { /* `gtk_shell1` at time of writing. */
       /* Only require `libdecor` when built with X11 support,
@@ -4665,7 +4729,7 @@ static void global_handle_add(void *data,
   CLOG_INFO(LOG,
             2,
             "add %s(interface=%s, version=%u, name=%u)",
-            found ? (added ? "" : "(found but not added)") : "(skipped), ",
+            (interface_slot != -1) ? (added ? "" : "(found but not added)") : "(skipped), ",
             interface,
             version,
             name);
@@ -4685,14 +4749,20 @@ static void global_handle_remove(void *data,
                                  const uint32_t name)
 {
   GWL_Display *display = static_cast<GWL_Display *>(data);
+
+  /* Deferred registration only happens on startup, ensure interfaces are never removed
+   * when in the process of adding, while it seems unlikely compositors would do this.
+   * Better check as it's unsupported (it could be supported if this assert is ever reached). */
+  GHOST_ASSERT(!display->registry_add_deferred,
+               "Interface removal while deferred adding not supported");
+
   int interface_slot = 0;
   bool found = gwl_registry_entry_remove_by_name(display, name, &interface_slot);
   CLOG_INFO(LOG,
             2,
-            "remove (name=%u, interface=%s), %s",
+            "remove (name=%u, interface=%s)",
             name,
-            *gwl_registry_handlers[interface_slot].interface_p,
-            found ? "(known)" : "(unknown)");
+            found ? *gwl_registry_handlers[interface_slot].interface_p : "(unknown)");
 }
 
 static const struct wl_registry_listener registry_listener = {
@@ -4727,13 +4797,33 @@ GHOST_SystemWayland::GHOST_SystemWayland(bool background)
   display_->xdg_decor = new GWL_XDG_Decor_System;
 
   /* Register interfaces. */
-  struct wl_registry *registry = wl_display_get_registry(display_->wl_display);
-  wl_registry_add_listener(registry, &registry_listener, display_);
-  /* Call callback for registry listener. */
-  wl_display_roundtrip(display_->wl_display);
-  /* Call callbacks for registered listeners. */
-  wl_display_roundtrip(display_->wl_display);
-  wl_registry_destroy(registry);
+  {
+    constexpr size_t registry_add_num = ARRAY_SIZE(gwl_registry_handlers) - 1;
+    GWL_RegistryAdd_Deferred *registry_add_table[registry_add_num] = {nullptr};
+    display_->registry_add_deferred = registry_add_table;
+
+    struct wl_registry *registry = wl_display_get_registry(display_->wl_display);
+    wl_registry_add_listener(registry, &registry_listener, display_);
+    /* Call callback for registry listener. */
+    wl_display_roundtrip(display_->wl_display);
+    /* Call callbacks for registered listeners. */
+    wl_display_roundtrip(display_->wl_display);
+
+    /* Run #global_handle_add again without deferred registration (for predictable order). */
+    display_->registry_add_deferred = nullptr;
+
+    for (size_t interface_slot = 0; interface_slot < registry_add_num; interface_slot++) {
+      const char *interface = *gwl_registry_handlers[interface_slot].interface_p;
+      while (registry_add_table[interface_slot]) {
+        GWL_RegistryAdd_Deferred *reg_add = registry_add_table[interface_slot];
+        global_handle_add(display_, registry, reg_add->name, interface, reg_add->version);
+        registry_add_table[interface_slot] = reg_add->next;
+        delete reg_add;
+      }
+    }
+
+    wl_registry_destroy(registry);
+  }
 
 #ifdef WITH_GHOST_WAYLAND_LIBDECOR
   /* Ignore windowing requirements when running in background mode,
@@ -4785,23 +4875,6 @@ GHOST_SystemWayland::GHOST_SystemWayland(bool background)
       throw std::runtime_error("Wayland: unable to access xdg_shell!");
     }
   }
-
-  /* Register data device per seat for IPC between Wayland clients. */
-  if (display_->wl_data_device_manager) {
-    for (GWL_Seat *seat : display_->seats) {
-      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) {
-    for (GWL_Seat *seat : display_->seats) {
-      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);
-    }
-  }
 }
 
 GHOST_SystemWayland::~GHOST_SystemWayland()