From 94a805ab4b463cf802e0420d6d339c89f1d22ef5 Mon Sep 17 00:00:00 2001 From: Geoffrey McRae Date: Sun, 19 Jul 2026 03:43:26 +1000 Subject: [PATCH] wayland: make HDR descriptions asynchronous Publish HDR metadata through a locked request snapshot and wait for image-description readiness before using it. Commit encoding changes with matching content, preserve the active description during same-format metadata replacements, and handle creation failures without protocol errors. --- client/displayservers/Wayland/colormgmt.c | 17 ++ client/displayservers/Wayland/gl.c | 215 ++++++++++++++++------ client/displayservers/Wayland/wayland.c | 7 +- client/displayservers/Wayland/wayland.h | 48 +++-- 4 files changed, 210 insertions(+), 77 deletions(-) diff --git a/client/displayservers/Wayland/colormgmt.c b/client/displayservers/Wayland/colormgmt.c index f0669c7e..16b35be6 100644 --- a/client/displayservers/Wayland/colormgmt.c +++ b/client/displayservers/Wayland/colormgmt.c @@ -105,6 +105,23 @@ void waylandColorMgmtFree(void) if (!wlWm.colorManager) return; + if (wlWm.hdrImageCreator) + { + wp_image_description_creator_params_v1_destroy(wlWm.hdrImageCreator); + wlWm.hdrImageCreator = NULL; + } + if (wlWm.hdrImageDesc) + { + wp_image_description_v1_destroy(wlWm.hdrImageDesc); + wlWm.hdrImageDesc = NULL; + } + wlWm.hdrImageDescReady = false; + if (wlWm.colorSurface) + { + wp_color_management_surface_v1_destroy(wlWm.colorSurface); + wlWm.colorSurface = NULL; + } + wp_color_manager_v1_destroy(wlWm.colorManager); wlWm.colorManager = NULL; } diff --git a/client/displayservers/Wayland/gl.c b/client/displayservers/Wayland/gl.c index acdbf2f4..44b710fb 100644 --- a/client/displayservers/Wayland/gl.c +++ b/client/displayservers/Wayland/gl.c @@ -72,31 +72,32 @@ EGLDisplay waylandGetEGLDisplay(void) static void applyHDRPending(void) { - if (wlWm.pendingHDRApply) + enum WaylandHDRPendingAction action; + struct WaylandHDRParameters params; + + LG_LOCK(wlWm.pendingHDRLock); + action = wlWm.pendingHDRAction; + params = wlWm.pendingHDR; + wlWm.pendingHDRAction = WAYLAND_HDR_PENDING_NONE; + LG_UNLOCK(wlWm.pendingHDRLock); + + if (action == WAYLAND_HDR_PENDING_APPLY) { - wlWm.pendingHDRApply = false; - atomic_thread_fence(memory_order_acquire); waylandSetHDRImageDescription( - wlWm.pendingHDRDisplayPrimary, wlWm.pendingHDRWhitePoint, - wlWm.pendingHDRMaxDisplayLuminance, - wlWm.pendingHDRMinDisplayLuminance, - wlWm.pendingHDRMaxCLL, - wlWm.pendingHDRMaxFALL, - wlWm.pendingHDRPQ, - wlWm.pendingHDRMetadata); + params.displayPrimary, params.whitePoint, + params.maxDisplayLuminance, + params.minDisplayLuminance, + params.maxCLL, + params.maxFALL, + params.pq, + params.metadata); } - else if (wlWm.pendingHDRClear) - { - wlWm.pendingHDRClear = false; + else if (action == WAYLAND_HDR_PENDING_CLEAR) waylandClearHDRImageDescription(); - } } void waylandClearHDRImageDescription(void) { - if (!wlWm.hdrActive) - return; - if (wlWm.hdrImageCreator) { wp_image_description_creator_params_v1_destroy(wlWm.hdrImageCreator); @@ -107,16 +108,103 @@ void waylandClearHDRImageDescription(void) wp_image_description_v1_destroy(wlWm.hdrImageDesc); wlWm.hdrImageDesc = NULL; } - if (wlWm.colorSurface) + wlWm.hdrImageDescReady = false; + + if (wlWm.colorSurface && atomic_load(&wlWm.hdrActive)) + wp_color_management_surface_v1_unset_image_description(wlWm.colorSurface); + + atomic_store(&wlWm.hdrActive, false); + DEBUG_INFO("HDR image description removed from surface"); +} + +static void hdrImageDescriptionReady(struct wp_image_description_v1 * desc) +{ + if (desc != wlWm.hdrImageDesc) + return; + + if (!wlWm.colorSurface) { - wp_color_management_surface_v1_destroy(wlWm.colorSurface); - wlWm.colorSurface = NULL; + DEBUG_WARN("HDR image description became ready without a color surface"); + wp_image_description_v1_destroy(desc); + wlWm.hdrImageDesc = NULL; + return; } - wlWm.hdrActive = false; - DEBUG_INFO("HDR image description cleared"); + // Defer attachment until just after the current surface commit. EGL can + // then switch to native HDR for the next render, and that native frame and + // its image description become active in the same following commit. + wlWm.hdrImageDescReady = true; + DEBUG_INFO("HDR image description is ready (%s)", + wlWm.hdrImageDescPQ ? "PQ" : "scRGB"); + app_invalidateWindow(true); + waylandStopWaitFrame(); } +static void activateReadyHDRImageDescription(void) +{ + if (!wlWm.hdrImageDesc || !wlWm.hdrImageDescReady || !wlWm.colorSurface) + return; + + struct wp_image_description_v1 * desc = wlWm.hdrImageDesc; + wp_color_management_surface_v1_set_image_description( + wlWm.colorSurface, desc, + WP_COLOR_MANAGER_V1_RENDER_INTENT_PERCEPTUAL); + atomic_store(&wlWm.hdrActivePQ, wlWm.hdrImageDescPQ); + atomic_store(&wlWm.hdrActive, true); + + // set_image_description has copy semantics; the protocol object is no + // longer needed once it has been attached to the pending surface state. + wlWm.hdrImageDesc = NULL; + wlWm.hdrImageDescReady = false; + wp_image_description_v1_destroy(desc); + + DEBUG_INFO("HDR image description pending next surface commit (%s)", + atomic_load(&wlWm.hdrActivePQ) ? "PQ" : "scRGB"); + app_invalidateWindow(true); + waylandStopWaitFrame(); +} + +static void hdrImageDescriptionFailed(void * data, + struct wp_image_description_v1 * desc, uint32_t cause, const char * message) +{ + (void)data; + if (desc != wlWm.hdrImageDesc) + return; + + DEBUG_WARN("Failed to create HDR image description (cause:%u): %s", + cause, message); + wlWm.hdrImageDesc = NULL; + wlWm.hdrImageDescReady = false; + wp_image_description_v1_destroy(desc); + app_invalidateWindow(true); + waylandStopWaitFrame(); +} + +static void hdrImageDescriptionReadyV1(void * data, + struct wp_image_description_v1 * desc, uint32_t identity) +{ + (void)data; + (void)identity; + hdrImageDescriptionReady(desc); +} + +static void hdrImageDescriptionReadyV2(void * data, + struct wp_image_description_v1 * desc, uint32_t identityHi, + uint32_t identityLo) +{ + (void)data; + (void)identityHi; + (void)identityLo; + hdrImageDescriptionReady(desc); +} + +static const struct wp_image_description_v1_listener hdrImageDescListener = +{ + .failed = hdrImageDescriptionFailed, + .ready = hdrImageDescriptionReadyV1, + .ready2 = hdrImageDescriptionReadyV2, +}; + void waylandEGLSwapBuffers(EGLDisplay display, EGLSurface surface, const struct Rect * damage, int count) { if (!wlWm.swapWithDamage.init) @@ -133,6 +221,7 @@ void waylandEGLSwapBuffers(EGLDisplay display, EGLSurface surface, const struct waylandPresentationFrame(); applyHDRPending(); swapWithDamage(&wlWm.swapWithDamage, display, surface, damage, count); + activateReadyHDRImageDescription(); if (wlWm.needsResize) { @@ -242,7 +331,8 @@ void waylandSetHDRImageDescription(const uint16_t displayPrimary[3][2], return; } - // Clean up any previous description (allows re-application with updated metadata) + // Cancel only an in-flight replacement. The active surface description is + // retained until this replacement is ready. if (wlWm.hdrImageCreator) { wp_image_description_creator_params_v1_destroy(wlWm.hdrImageCreator); @@ -253,13 +343,26 @@ void waylandSetHDRImageDescription(const uint16_t displayPrimary[3][2], wp_image_description_v1_destroy(wlWm.hdrImageDesc); wlWm.hdrImageDesc = NULL; } - if (wlWm.colorSurface) + wlWm.hdrImageDescReady = false; + // If the encoding changed, remove the old description in the same surface + // commit that presents the software-mapped transition frame. + if (atomic_load(&wlWm.hdrActive) && + atomic_load(&wlWm.hdrActivePQ) != hdrPQ) { - wp_color_management_surface_v1_destroy(wlWm.colorSurface); - wlWm.colorSurface = NULL; + wp_color_management_surface_v1_unset_image_description(wlWm.colorSurface); + atomic_store(&wlWm.hdrActive, false); } - wlWm.hdrActive = false; + if (!wlWm.colorSurface) + { + wlWm.colorSurface = + wp_color_manager_v1_get_surface(wlWm.colorManager, wlWm.surface); + if (!wlWm.colorSurface) + { + DEBUG_WARN("Failed to get color management surface"); + return; + } + } wlWm.hdrImageCreator = wp_color_manager_v1_create_parametric_creator(wlWm.colorManager); @@ -332,22 +435,12 @@ void waylandSetHDRImageDescription(const uint16_t displayPrimary[3][2], return; } - wlWm.colorSurface = - wp_color_manager_v1_get_surface(wlWm.colorManager, wlWm.surface); - if (!wlWm.colorSurface) - { - DEBUG_WARN("Failed to get color management surface"); - wp_image_description_v1_destroy(wlWm.hdrImageDesc); - wlWm.hdrImageDesc = NULL; - return; - } + wlWm.hdrImageDescPQ = hdrPQ; + wlWm.hdrImageDescReady = false; + wp_image_description_v1_add_listener( + wlWm.hdrImageDesc, &hdrImageDescListener, NULL); - wp_color_management_surface_v1_set_image_description( - wlWm.colorSurface, wlWm.hdrImageDesc, - WP_COLOR_MANAGER_V1_RENDER_INTENT_PERCEPTUAL); - - wlWm.hdrActive = true; - DEBUG_INFO("HDR image description set on surface (%s, %s, " + DEBUG_INFO("HDR image description requested (%s, %s, " "maxLum:%u cd/m² minLum:%u (0.0001 cd/m²) maxCLL:%u maxFALL:%u)", hdrPQ ? "PQ" : "scRGB", hdrPQ ? "BT.2020" : "sRGB", maxDisplayLuminance, minDisplayLuminance, maxCLL, maxFALL); @@ -369,31 +462,31 @@ bool waylandRequestHDR(const uint16_t displayPrimary[3][2], if (!hdrPQ && !wlWm.cmHasPrimariesSRGB) return false; - wlWm.pendingHDRPQ = hdrPQ; - wlWm.pendingHDRMetadata = hdrMetadata; + atomic_store(&wlWm.hdrRequestedPQ, hdrPQ); + atomic_store(&wlWm.hdrRequested, true); - // Write all metadata before setting the apply flag to ensure the - // render thread sees the complete HDR metadata when it observes - // pendingHDRApply == true. - memcpy(wlWm.pendingHDRDisplayPrimary, displayPrimary, - sizeof(wlWm.pendingHDRDisplayPrimary)); - memcpy(wlWm.pendingHDRWhitePoint, whitePoint, - sizeof(wlWm.pendingHDRWhitePoint)); - wlWm.pendingHDRMaxDisplayLuminance = maxDisplayLuminance; - wlWm.pendingHDRMinDisplayLuminance = minDisplayLuminance; - wlWm.pendingHDRMaxCLL = maxCLL; - wlWm.pendingHDRMaxFALL = maxFALL; - - wlWm.pendingHDRClear = false; - atomic_thread_fence(memory_order_release); - wlWm.pendingHDRApply = true; + LG_LOCK(wlWm.pendingHDRLock); + wlWm.pendingHDR.pq = hdrPQ; + wlWm.pendingHDR.metadata = hdrMetadata; + memcpy(wlWm.pendingHDR.displayPrimary, displayPrimary, + sizeof(wlWm.pendingHDR.displayPrimary)); + memcpy(wlWm.pendingHDR.whitePoint, whitePoint, + sizeof(wlWm.pendingHDR.whitePoint)); + wlWm.pendingHDR.maxDisplayLuminance = maxDisplayLuminance; + wlWm.pendingHDR.minDisplayLuminance = minDisplayLuminance; + wlWm.pendingHDR.maxCLL = maxCLL; + wlWm.pendingHDR.maxFALL = maxFALL; + wlWm.pendingHDRAction = WAYLAND_HDR_PENDING_APPLY; + LG_UNLOCK(wlWm.pendingHDRLock); return true; } void waylandRequestClearHDR(void) { - wlWm.pendingHDRClear = true; - wlWm.pendingHDRApply = false; + atomic_store(&wlWm.hdrRequested, false); + LG_LOCK(wlWm.pendingHDRLock); + wlWm.pendingHDRAction = WAYLAND_HDR_PENDING_CLEAR; + LG_UNLOCK(wlWm.pendingHDRLock); } #ifdef ENABLE_OPENGL diff --git a/client/displayservers/Wayland/wayland.c b/client/displayservers/Wayland/wayland.c index b1066086..7d59a9c1 100644 --- a/client/displayservers/Wayland/wayland.c +++ b/client/displayservers/Wayland/wayland.c @@ -108,6 +108,7 @@ static bool getCompositor(char * dst, size_t size) static bool waylandInit(const LG_DSInitParams params) { memset(&wlWm, 0, sizeof(wlWm)); + LG_LOCK_INIT(wlWm.pendingHDRLock); wlWm.desktop = WL_Desktops[0]; wlWm.hdrWhiteLevels = (LG_DSHDRWhiteLevels) { @@ -211,6 +212,7 @@ static void waylandFree(void) waylandRegistryFree(); waylandCursorFree(); wl_display_disconnect(wlWm.display); + LG_LOCK_FREE(wlWm.pendingHDRLock); } static bool waylandGetProp(LG_DSProperty prop, void * ret) @@ -223,7 +225,10 @@ static bool waylandGetProp(LG_DSProperty prop, void * ret) if (prop == LG_DS_NATIVE_HDR) { - *(bool *)ret = wlWm.cmCanDoHDR; + const bool active = atomic_load(&wlWm.hdrActive); + const bool requested = atomic_load(&wlWm.hdrRequested); + *(bool *)ret = active && requested && + atomic_load(&wlWm.hdrActivePQ) == atomic_load(&wlWm.hdrRequestedPQ); return true; } diff --git a/client/displayservers/Wayland/wayland.h b/client/displayservers/Wayland/wayland.h index 7b8911ef..a79c1682 100644 --- a/client/displayservers/Wayland/wayland.h +++ b/client/displayservers/Wayland/wayland.h @@ -91,6 +91,25 @@ struct xkb_context; struct xkb_keymap; struct xkb_state; +enum WaylandHDRPendingAction +{ + WAYLAND_HDR_PENDING_NONE, + WAYLAND_HDR_PENDING_APPLY, + WAYLAND_HDR_PENDING_CLEAR, +}; + +struct WaylandHDRParameters +{ + bool pq; + bool metadata; + uint16_t displayPrimary[3][2]; + uint16_t whitePoint[2]; + uint32_t maxDisplayLuminance; + uint32_t minDisplayLuminance; + uint32_t maxCLL; + uint32_t maxFALL; +}; + struct WaylandDSState { bool pointerGrabbed; @@ -189,8 +208,14 @@ struct WaylandDSState struct wp_image_description_v1 * hdrImageDesc; struct wp_image_description_creator_params_v1 * hdrImageCreator; - // Set to true when an HDR image description is active on the surface - bool hdrActive; + // Active and requested encodings are atomic because getProp runs on the + // renderer while description events are dispatched by Wayland. + _Atomic(bool) hdrActive; + _Atomic(bool) hdrActivePQ; + _Atomic(bool) hdrRequested; + _Atomic(bool) hdrRequestedPQ; + bool hdrImageDescPQ; + bool hdrImageDescReady; // wp_color_manager_v1 feature advertisement tracking. // Set to true after the done event for the color-manager has been received @@ -215,19 +240,12 @@ struct WaylandDSState // set by the active desktop backend during shellInit void * xdgToplevel; - // Pending HDR format to apply (set by frame thread, applied in swap buffers). - // pendingHDRApply and pendingHDRClear are accessed from multiple threads - // without a lock, so they must be atomic. - _Atomic(bool) pendingHDRApply; - _Atomic(bool) pendingHDRClear; - bool pendingHDRPQ; - bool pendingHDRMetadata; - uint16_t pendingHDRDisplayPrimary[3][2]; - uint16_t pendingHDRWhitePoint[2]; - uint32_t pendingHDRMaxDisplayLuminance; - uint32_t pendingHDRMinDisplayLuminance; - uint32_t pendingHDRMaxCLL; - uint32_t pendingHDRMaxFALL; + // Pending HDR format is published by the frame thread and consumed by the + // render thread. Keep the action and all metadata in one locked snapshot so + // a replacement request cannot tear a request already being consumed. + LG_Lock pendingHDRLock; + enum WaylandHDRPendingAction pendingHDRAction; + struct WaylandHDRParameters pendingHDR; LGEvent * frameEvent;