[Bf-blender-cvs] [e75be000e3e] master: GHOST/Wayland: replace deferred registration with an update callback
Campbell Barton
noreply at git.blender.org
Thu Oct 27 10:25:56 CEST 2022
Commit: e75be000e3e83b53e0fc18c5719f41de37bd6a75
Author: Campbell Barton
Date: Thu Oct 27 17:24:51 2022 +1100
Branches: master
https://developer.blender.org/rBe75be000e3e83b53e0fc18c5719f41de37bd6a75
GHOST/Wayland: replace deferred registration with an update callback
There were two issues caused by deferred registration (added by [0]),
one crash on startup (T102075), another unreported issue with the GLX/EGL
context failing to initialize. Unfortunately I'm unable to reproduce the
errors but it seems likely deferring interface registration is not well
supported so this commit uses an alternative solution to some interfaces
depending on others for initialization.
Instead of relying on the order of registration, a separate "update"
callback has been added which is called after binding interfaces.
This has the advantage that it can be called when adding/removing
interfaces at run-time to avoid the dangling pointers being left in
locally allocated structures. In practice adding/removing interfaces
happens so rarely (only with "outputs" as far as I'm aware) that this
benefit is theoretical at the moment.
This should resolve T102075.
[0]: 9fe9705bc0af9d881905d75957c8dd467f8c8cb3
===================================================================
M intern/ghost/intern/GHOST_SystemWayland.cpp
===================================================================
diff --git a/intern/ghost/intern/GHOST_SystemWayland.cpp b/intern/ghost/intern/GHOST_SystemWayland.cpp
index 87e5f9bb8d5..d4eabfb815d 100644
--- a/intern/ghost/intern/GHOST_SystemWayland.cpp
+++ b/intern/ghost/intern/GHOST_SystemWayland.cpp
@@ -763,17 +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.
+ * True when initializing registration, while updating all other entries wont cause problems,
+ * it will preform many redundant update calls.
*/
- struct GWL_RegistryAdd_Deferred **registry_add_deferred = nullptr;
- /** Registry entries, kept to allow removal at run-time. */
+ bool registry_skip_update_all = false;
+
+ /** Registry entries, kept to allow updating & removal at run-time. */
struct GWL_RegistryEntry *registry_entry = nullptr;
struct wl_registry *wl_registry = nullptr;
@@ -852,21 +852,42 @@ static void gwl_display_destroy(GWL_Display *display)
* \{ */
struct GWL_RegisteryAdd_Params {
- struct GWL_Display *display = nullptr;
uint32_t name = 0;
- uint32_t version = 0;
/** Index within `gwl_registry_handlers`. */
int interface_slot = 0;
+ uint32_t version = 0;
};
/**
* Add callback for object registry.
+ * \note Any operations that depend on other interfaces being registered must be performed in the
+ * #GWL_RegistryHandler_UpdateFn callback as the order interfaces are added is out of our control.
+ *
* \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);
+struct GWL_RegisteryUpdate_Params {
+ uint32_t name = 0;
+ /** Index within `gwl_registry_handlers`. */
+ int interface_slot = 0;
+ uint32_t version = 0;
+
+ /** Set to #GWL_RegistryEntry.user_data. */
+ void *user_data = nullptr;
+};
+
+/**
+ * Optional update callback to refresh internal data when another interface has been added/removed.
+ *
+ * \param display: The display which holes a reference to the global object.
+ * \param params: Various arguments needed for updating.
+ */
+using GWL_RegistryHandler_UpdateFn = void (*)(GWL_Display *display,
+ const GWL_RegisteryUpdate_Params *params);
+
/**
* Remove callback for object registry.
* \param display: The display which holes a reference to the global object.
@@ -883,7 +904,11 @@ 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;
+ /** Add the interface. */
GWL_RegistryHandler_AddFn add_fn = nullptr;
+ /** Optional update the interface (when other interfaces have been added/removed). */
+ GWL_RegistryHandler_UpdateFn update_fn = nullptr;
+ /** Remove the interface. */
GWL_RegistryEntry_RemoveFn remove_fn = nullptr;
};
@@ -909,6 +934,10 @@ struct GWL_RegistryEntry {
* A unique identifier used as a handle by `wl_registry_listener.global_remove`.
*/
uint32_t name = WL_NAME_UNSET;
+ /**
+ * Version passed by the add callback.
+ */
+ uint32_t version;
/**
* The index in `gwl_registry_handlers`,
* useful for accessing the interface name (for logging for example).
@@ -917,14 +946,14 @@ struct GWL_RegistryEntry {
};
static void gwl_registry_entry_add(GWL_Display *display,
- const int interface_slot,
- const uint32_t name,
+ const GWL_RegisteryAdd_Params *params,
void *user_data)
{
GWL_RegistryEntry *reg = new GWL_RegistryEntry;
- reg->interface_slot = interface_slot;
- reg->name = name;
+ reg->interface_slot = params->interface_slot;
+ reg->name = params->name;
+ reg->version = params->version;
reg->user_data = user_data;
reg->next = display->registry_entry;
@@ -1016,6 +1045,49 @@ static void gwl_registry_entry_remove_all(GWL_Display *display)
display->registry_entry = nullptr;
}
+/**
+ * Run GWL_RegistryHandler.update_fn an all registered interface instances.
+ * This is needed to refresh the state of interfaces that may reference other interfaces.
+ * Called when interfaces are added/removed.
+ *
+ * \param interface_slot_exclude: Skip updating slots of this type.
+ * Note that while harmless dependencies only exist between different types,
+ * so there is no reason to update all other outputs that an output was removed (for e.g.).
+ * Pass as -1 to update all slots.
+ *
+ * NOTE(@campbellbarton): Updating all other items on a single change is typically worth avoiding.
+ * In practice this isn't a problem as so there are so few elements in `display->registry_entry`,
+ * so few use update functions and adding/removal at runtime is rarely called (plugging/unplugging)
+ * hardware for e.g. So while it's possible to store dependency links to avoid unnecessary
+ * looping over data - so it ends up being a non issue.
+ */
+static void gwl_registry_entry_update_all(GWL_Display *display, const int interface_slot_exclude)
+{
+ GHOST_ASSERT(interface_slot_exclude == -1 || (uint(interface_slot_exclude) <
+ uint(gwl_registry_handler_interface_slot_max())),
+ "Invalid exclude slot");
+
+ for (GWL_RegistryEntry *reg = display->registry_entry; reg; reg = reg->next) {
+ if (reg->interface_slot == interface_slot_exclude) {
+ continue;
+ }
+ const GWL_RegistryHandler *handler = gwl_registry_handler_from_interface_slot(
+ reg->interface_slot);
+ if (handler->update_fn == nullptr) {
+ continue;
+ }
+
+ GWL_RegisteryUpdate_Params params = {
+ .name = reg->name,
+ .interface_slot = reg->interface_slot,
+ .version = reg->version,
+
+ .user_data = reg->user_data,
+ };
+ handler->update_fn(display, ¶ms);
+ }
+}
+
/** \} */
/* -------------------------------------------------------------------- */
@@ -4153,8 +4225,7 @@ static void gwl_registry_compositor_add(GWL_Display *display,
{
display->wl_compositor = static_cast<wl_compositor *>(
wl_registry_bind(display->wl_registry, params->name, &wl_compositor_interface, 3));
-
- gwl_registry_entry_add(display, params->interface_slot, params->name, nullptr);
+ gwl_registry_entry_add(display, params, nullptr);
}
static void gwl_registry_compositor_remove(GWL_Display *display,
void * /*user_data*/,
@@ -4175,8 +4246,7 @@ static void gwl_registry_xdg_wm_base_add(GWL_Display *display,
wl_registry_bind(display->wl_registry, params->name, &xdg_wm_base_interface, 1));
xdg_wm_base_add_listener(decor.shell, &shell_listener, nullptr);
decor.shell_name = params->name;
-
- gwl_registry_entry_add(display, params->interface_slot, params->name, nullptr);
+ gwl_registry_entry_add(display, params, nullptr);
}
static void gwl_registry_xdg_wm_base_remove(GWL_Display *display,
void * /*user_data*/,
@@ -4199,8 +4269,7 @@ static void gwl_registry_xdg_decoration_manager_add(GWL_Display *display,
decor.manager = static_cast<zxdg_decoration_manager_v1 *>(wl_registry_bind(
display->wl_registry, params->name, &zxdg_decoration_manager_v1_interface, 1));
decor.manager_name = params->name;
-
- gwl_registry_entry_add(display, params->interface_slot, params->name, nullptr);
+ gwl_registry_entry_add(display, params, nullptr);
}
static void gwl_registry_xdg_decoration_manager_remove(GWL_Display *display,
void * /*user_data*/,
@@ -4221,13 +4290,7 @@ static void gwl_registry_xdg_output_manager_add(GWL_Display *display,
{
display->xdg_output_manager = static_cast<zxdg_output_manager_v1 *>(
wl_registry_bind(display->wl_registry, params->name, &zxdg_output_manager_v1_interface, 2));
- for (GWL_Output *output : display->outputs) {
- output->xdg_output = zxdg_output_manager_v1_get_xdg_output(display->xdg_output_manager,
- output->wl_output);
- zxdg_output_v1_add_listener(output->xdg_output, &xdg_output_listener, output);
- }
-
- gwl_registry_entry_add(display, params->interface_slot, params->name, nullptr);
+ gwl_registry_entry_add(display, params, nullptr);
}
static void gwl_registry_xdg_output_manager_remove(GWL_Display *display,
void * /*user_data*/,
@@ -4236,10 +4299,6 @@ static void gwl_registry_xdg_output_manager_remove(GWL_Display *display,
struct zxdg_output_manager_v1 **value_p = &display->xdg_output_manager;
zxdg_output_manager_v1_destroy(*value_p);
*value_p = nullptr;
-
- for (GWL_Output *output : display->outputs) {
- output->xdg_output = nullptr;
- }
}
/* #GWL_Display.wl_output */
@@ -4255,15 +4314,22 @@ static void gwl_registry_wl_output_add(GWL_Display *display, const GWL_Registery
display->outputs.push_back(output);
wl_output_add_listener(output->wl_output, &output_listener, output);
-
+ gwl_registry_entry_add(display, params, static_cast<void *>(output));
+}
+static void gwl_registry_wl_output_update(GWL_Display *display,
+ const GWL_RegisteryUpdate_Params *params)
+{
+ GWL_Output *output = static_cast<GWL_Output *>(params->user_data);
if (display->xdg_output_manager) {
- output->xdg_output = zxdg_output_manager_v1_get_xdg_output(display->xdg_output_manager,
- output->wl_output);
- zxdg_output_v1_add_listener(output->xdg_output, &xdg_output_listener, output);
+ if (output->xdg_output == nullptr) {
+ output->xdg_output = zxdg_output_manager_v1_get_xdg_output(display->xdg_ou
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list