Repository navigation
feat(linux/wlgrab): report HDR from the output's image description - #5615
superuser404notfound wants to merge 1 commit into
Conversation
c1e06c7 to
708c9f8
Compare
|
Pushed a revision for the SonarCloud gate. Of the 26 findings, 10 were fair and are fixed; the other 16 are the shape of the Wayland C protocol and are marked rather than worked around. Fixed
Marked with
Re-checked after the change: still compiles at The two Read the Docs failures look unrelated to this change: both builds end in |
708c9f8 to
ae2a529
Compare
display_t::is_hdr() defaults to false in platform/common.h and is overridden in two places: kmsgrab, from the connector's HDR_OUTPUT_METADATA property, and the PipeWire path, from the SPA colorimetry. wlgrab overrides neither, so colorspace_from_client_config() always takes the SDR branch and a client asking for HDR over capture = wlr receives a 10-bit encode of Rec. 709. That is the right answer today, because the backend has no way to know better. It does not have to stay that way: colour-management-v1 publishes the output's whole image description to any client that asks, it carries more than the DRM property does, and it is available on a virtual output exactly as it is on a physical one. So bind wp_color_manager_v1, read the captured output's image description once when the capture is set up, and answer is_hdr() and get_hdr_metadata() from it. This is the job pipewire.cpp already does with the SPA colorimetry, over the Wayland protocol instead. Bound at version 1 deliberately: everything read here is in the first version, and a later one only adds events that would need handlers. The unit conversions are commented where they happen. The protocol carries CIE 1931 coordinates multiplied by a million and SS_HDR_METADATA carries them multiplied by fifty thousand; the luminances already agree, minimum in ten-thousandths of a nit and the rest in whole nits. A compositor without the protocol, or one that will not describe the output, leaves the colour unknown, which reads as SDR everywhere it is used - the behaviour every wlroots compositor had before this existed.
ae2a529 to
af9b521
Compare
|
|
I built Sunshine with this PR and tried it on Hyprland 0.56.2 with an NVIDIA card (RTX 5090, driver 615.71.09), capturing a headless output with To get HDR on a headless output at all, Hyprland only accepted it from a The problem is that stock Hyprland still hands screencopy clients an sRGB frame even when the output is HDR. So the Mac got sRGB data tagged as PQ: way too bright, oversaturated, and warm whites turning red. I dug through Hyprland and found three places responsible: I patched Hyprland so that clients bound to One more thing: a virtual output has no EDID, so I had to set its luminances by hand. Otherwise the metadata that reaches the client doesn't mean anything. The Hyprland patch and notes are here if anyone wants them: https://github.com/Jhonfel/sunshine-hyprland-virtual-display/tree/client-resolution-hdr#hdr-experimental Thanks for this, it was the missing piece on the Sunshine side. |
End-to-end HDR10 from HEADLESS to Moonlight works with two patches and some Hyprland config; ship the patches and document the whole chain: - patches/sunshine-pr5615-wlgrab-hdr.diff: Sunshine reads the captured output's image description (LizardByte/Sunshine#5615). - patches/hyprland-0.56.2-screencopy-hdr-passthrough.patch: new misc:screencopy_hdr_passthrough. Stock Hyprland converts screencopy to sRGB, keeps the mirror of a PQ output in sRGB and, during a capture, uses an unmodified SDR copy with white at 80 nits. With the option, colour-management-aware clients get the output's own description unconverted, SDR settings applied. - render:use_fp16 = 0: the default auto mode composes the HDR output in a linear FP16 buffer and the capture copied from it is washed out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When ENABLE_HDR=true and the client requests HDR (SUNSHINE_CLIENT_HDR), switch HEADLESS to 10-bit BT.2020/PQ. Hyprland only applies HDR to an output from a monitorv2 block (supports_hdr, bitdepth 10, cm hdr) at config load, so connect writes that block to a sourced file and reloads; reapply leaves the mode alone during an HDR session and disconnect clears the block. Sunshine only streams it as HDR with LizardByte/Sunshine#5615, which reads the output's image description.
End-to-end HDR10 from HEADLESS to Moonlight works with two patches and some Hyprland config; ship the patches and document the whole chain: - patches/sunshine-pr5615-wlgrab-hdr.diff: Sunshine reads the captured output's image description (LizardByte/Sunshine#5615). - patches/hyprland-0.56.2-screencopy-hdr-passthrough.patch: new misc:screencopy_hdr_passthrough. Stock Hyprland converts screencopy to sRGB, keeps the mirror of a PQ output in sRGB and, during a capture, uses an unmodified SDR copy with white at 80 nits. With the option, colour-management-aware clients get the output's own description unconverted, SDR settings applied. - render:use_fp16 = 0: the default auto mode composes the HDR output in a linear FP16 buffer and the capture copied from it is washed out.
ReenigneArcher
left a comment
There was a problem hiding this comment.
The HDR detection needs to describe the captured pixels, remain valid when the output changes color encoding, and preserve the metadata fallback for older version-1 implementations. The inline findings describe the triggering cases and proposed corrections.
Validation: git diff --check and clang-format --dry-run --Werror passed. A temporary GTest harness using the PR's query, callbacks, and metadata methods with mocked Wayland event delivery ran five tests: four passed (missing manager, failed description, explicit metadata conversion, and range clamping); the omitted-target-primaries regression failed, returning zero RGB primaries and white point. This was focused logic validation on macOS, not a full Linux build or hardware HDR test.
Repository requirements: the PR adds no GTests. Please add tests for the new query/metadata behavior and color-change handling, including the fallback case, and add the required Doxygen blocks for the new callback functions and listener tables.
Verification limit for the maintainer: the CI, lint, CodeQL, and GH-Pages runs contain no job results. The failed Read the Docs build timed out waiting for a check run, and screenshot publishing failed because its source CI run had no screenshot artifacts. Those statuses are not code findings. SonarCloud passed, but reports 0% coverage on new code.
Merge status: the current head af9b521 is mergeable against master; no conflicts were reported.
| bool is_hdr() override { | ||
| return hdr_color.known && | ||
| hdr_color.primaries == WP_COLOR_MANAGER_V1_PRIMARIES_BT2020 && | ||
| hdr_color.transfer_function == WP_COLOR_MANAGER_V1_TRANSFER_FUNCTION_ST2084_PQ; |
There was a problem hiding this comment.
[P1] Verify the capture encoding before advertising HDR
An output image description does not establish the encoding of a wlr-screencopy buffer. For example, stock Hyprland 0.56.2 explicitly sets the DMA-BUF capture framebuffer to DEFAULT_SRGB_IMAGE_DESCRIPTION, even when the monitor description is BT.2020/PQ. This predicate therefore returns true for SDR capture pixels; colorspace_from_client_config() then tags the encode as PQ/BT.2020 without converting those pixels, producing incorrect brightness and colors on Moonlight (also reproduced in the existing PR discussion). Please gate HDR on a capture path that guarantees or negotiates PQ/BT.2020 frames, and retain SDR for compositors that return sRGB. Binding wp_color_manager_v1 alone is insufficient.
| } | ||
|
|
||
| wp_image_description_v1_destroy(desc); | ||
| wp_color_management_output_v1_destroy(cm_output); |
There was a problem hiding this comment.
[P2] Keep output color-change notifications alive during capture
Destroying cm_output here removes the only image_description_changed subscription, and its callback is currently a no-op. If HDR is toggled while streaming without changing the resolution or DMA-BUF format, snapshot() keeps returning ok, so the encoder continues using the cached transfer function and metadata until the client reconnects. In particular, turning HDR off can leave SDR frames tagged as PQ. Please retain the output object and listener for the display lifetime and have a color-description change trigger capture_e::reinit so the encode session is rebuilt with the new description.
| static void color_info_primaries(void *data, wp_image_description_info_v1 *, std::int32_t r_x, std::int32_t r_y, std::int32_t g_x, std::int32_t g_y, std::int32_t b_x, std::int32_t b_y, std::int32_t w_x, std::int32_t w_y) { // NOSONAR: the event's shape is fixed by the Wayland C protocol - eight coordinates, the proxy and the user pointer (cpp:S107, cpp:S5008) | ||
| // The primary colour volume, which is not what Moonlight's structure wants: | ||
| // that is mastering display metadata, and it arrives in target_primaries. |
There was a problem hiding this comment.
[P2] Preserve primary coordinates as a fallback for older implementations
The wayland-protocols 1.41 target_primaries event documentation says that event is omitted when the target and primary volumes coincide. This version is explicitly included in the PR's compatibility testing. Discarding the primary coordinates here leaves target_primaries all zero in that case, while is_hdr() still returns true and get_hdr_metadata() exports zero RGB primaries and white point. The focused GTest reproduction returned 0/0 for the BT.2020 red primary instead of 35400/14600. Please collect the primary coordinates and use them when no target_primaries event arrives; keep explicitly supplied target coordinates authoritative, and add a regression test. Newer protocol text requires the event unconditionally, but that does not cover older implementations following the earlier event documentation.
|
Thanks for the review. P1 is right, and it is the one that decides this PR. wlr-screencopy has no way to negotiate or report the encoding of the buffer it hands out. On wlroots 0.20 the copy happens to be a blit of the output buffer, which is why it worked in my setup. Hyprland renders the capture into an sRGB framebuffer, as the report above shows, and wlroots plans to give capture its own render pass as well (work item 4108). So the output description is the wrong source, and nothing in this protocol offers a better one. The path that can guarantee it is ext-image-copy-capture-v1 together with the capture colour management protocol (wayland-protocols MR 531 merged as xx-image-capture-color-management-v1, MR 448 is the ext version). There the client asks for the image description of the capture source and the compositor converts. wlgrab would need the ext-image-copy-capture port first. P2 and P3 are both correct too. I checked the 1.41 text for target_primaries, the event is indeed omitted when the volumes coincide. Both would carry over to a version built on the capture protocol, along with the tests and Doxygen blocks. I am converting this to a draft rather than patching around P1. If you would prefer an opt-in setting that trusts the output description, default off, I can do that instead, but I do not think it is the better fix. (Written with AI assistance.) |



What this changes
display_t::is_hdr()defaults tofalseinplatform/common.hand is overridden in two places:kmsgrab.cpp, from the connector'sHDR_OUTPUT_METADATAproperty, andpipewire.cpp, from the SPA colorimetry.wlgrab.cppoverrides neither, socolorspace_from_client_config()always takes the SDR branch:A client that asks for HDR over
capture = wlrtherefore gets a 10-bit encode of Rec. 709 and is correctly told HDR is off.That is the right answer today, because the backend has no way to know better. It does not have to stay that way: colour-management-v1 publishes the output's whole image description to any client that asks, it carries more than the DRM property does, and it is available on a virtual output exactly as on a physical one.
So this binds
wp_color_manager_v1, reads the captured output's image description once when the capture is set up — which is also every moment the answer could have changed, since the encode session is rebuilt on reconnect and on reinit — and answersis_hdr()andget_hdr_metadata()from it. It is the jobpipewire.cppalready does with the SPA colorimetry, over the Wayland protocol instead.Bound at version 1 deliberately: everything read here is in the first version, and a later one only adds events that would need handlers.
Fallback
A compositor without the protocol, or one that will not describe the output, leaves the colour unknown, which reads as SDR everywhere it is used. That is the behaviour every wlroots compositor had before this existed, so nothing regresses.
Testing
Compiles at
-Wall -Werroragainst protocol headers generated from wayland-protocols 1.41 and 1.49, which is version 1 and version 3 ofwp_color_manager_v1; the designated initialisers are there so it survives both, since 1.49 adds aready2event and grows the listener struct.The metadata conversion is checked against BT.2020's red primary, 0.708 and 0.292, which has to come out as 35400 and 14600 in
SS_HDR_METADATA's units.On hardware: a headless sway 1.12 session on the Vulkan renderer, in an Incus container, on an RTX 4080 under the proprietary driver, streaming to a Moonlight client with HDR enabled.
The picture is correct, checked by eye and not only in the log. The output was at 2250x1206 at 120 Hz, adopted from the client mid-stream, so HDR survives a mode change.
One dependency worth stating
This reports what the compositor says. For a headless output to be able to say anything, wlroots has to accept that a virtual output can present BT.2020 and PQ, which it does not today — the capability is read from a monitor's EDID and a headless output has no monitor. That is a ten-line change and is in flight separately at https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/5453.
On a compositor whose output can already be in HDR, this change stands on its own.