Skip to content

Hdr fixes#3023

Open
kode54 wants to merge 19 commits into
WayfireWM:masterfrom
kode54:hdr-fixes
Open

Hdr fixes#3023
kode54 wants to merge 19 commits into
WayfireWM:masterfrom
kode54:hdr-fixes

Conversation

@kode54
Copy link
Copy Markdown
Contributor

@kode54 kode54 commented May 9, 2026

This branch was:

Co-authored-by: Claude Opus 4.7 (1M context) noreply@anthropic.com


Some of the changes were hand edits or tweaks, such as the gamma 2.2 change to the add_rect color, which required multiple rewrites before I realized what was up with that:

  • The target blend operation is alpha blended with pre-multiplied alpha, so the color that is coming in needs to have its R/G/B divided by the alpha before adjusting it, then scaled by the color multiplier in linear space, then converted back to sRGB, then pre-multiplied by the alpha again. I opted to use gamma 2.2 instead of piecewise sRGB for the space conversions.
  • There was a place where the agent decided to try setting a NULL preferred image description, which I discovered is not the legal way to unset the preferred image description. So instead, I opted to write it to set a fallback of sRGB / gamma 2.2, in the case where a "best" format is not found for a surface.
  • All the rest of the code did what it was supposed to right out of the generator, so it didn't need any major alterations.
  • Except for Uncrustify. I had to fix up a few files with that, and the line breaking comment wrap found a bug in the Uncrustify implementation that needs to run a second pass to check for discontinuity with equals sign spacing. I'll report that on ammen99's uncrustify repository.

Comment thread src/output/render-manager.cpp Outdated
Comment thread src/api/wayfire/render.hpp
@kode54
Copy link
Copy Markdown
Contributor Author

kode54 commented May 10, 2026

Okay, this has an outstanding issue with direct scanout of size mismatched XWayland surfaces, possibly also size mismatched Wayland surfaces, I've committed a fix, will push it once I verify it works correctly.

Copy link
Copy Markdown
Member

@ammen99 ammen99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, I am highly doubtful about some of the latest changes. Why skip the zero-copy path if we don't need HDR / special color transfer functions? Maybe the best solution is something like this:

  • Extend get_texture() to also include a parameter saying which color spaces the caller expects
  • Make sure get_texture() does not go the zero-copy path if the texture does not match the expected color space
  • In the GLES2 codepath, expect SRGB or whatever
  • In the Vulkan codepath, say we support all transfer functions.

Comment thread plugins/wobbly/wobbly.cpp
Comment thread src/api/wayfire/render.hpp Outdated
Comment thread src/view/view-3d.cpp Outdated
Comment thread plugins/cube/cube.cpp Outdated
Comment thread plugins/common/workspace-wall.cpp Outdated
Comment thread src/output/render-manager.cpp Outdated
* colors. Cached so that it is not recreated each frame.
*/
wlr_color_transform *output_inverse_eotf = nullptr;
wlr_color_transfer_function output_inverse_eotf_tf = (wlr_color_transfer_function)0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is 0 a valid color transfer function? if not, seems like a candidate for std::optional<wlr_color_transfer_function> instead of invalid values.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still remains a point for improvement

Comment thread src/output/render-manager.cpp Outdated
Comment thread src/render.cpp Outdated
@kode54
Copy link
Copy Markdown
Contributor Author

kode54 commented May 10, 2026

In general, I am highly doubtful about some of the latest changes. Why skip the zero-copy path if we don't need HDR / special color transfer functions? Maybe the best solution is something like this:

  • Extend get_texture() to also include a parameter saying which color spaces the caller expects
  • Make sure get_texture() does not go the zero-copy path if the texture does not match the expected color space
  • In the GLES2 codepath, expect SRGB or whatever

The altered wlroots code base expects linear in the GLES2 code path as well. And maybe it's worth just getting rid of this branch instead, since I doubt that will ever be merged into wlroots, considering it breaks on Nvidia.

  • In the Vulkan codepath, say we support all transfer functions.

@kode54
Copy link
Copy Markdown
Contributor Author

kode54 commented May 10, 2026

It's quite possible I should just discard everything after 2443c5f ... since that's mostly adapting everything to the GLES2 rewrite that will likely never be approved or employed anywhere.

@kode54
Copy link
Copy Markdown
Contributor Author

kode54 commented May 14, 2026

Could you please review again, I'm not sure how much of the above comments are regarding code I decided to move elsewhere for future dealings.

Comment thread src/api/wayfire/render.hpp
Comment thread src/view/wlr-surface-node.cpp Outdated
Copy link
Copy Markdown
Member

@ammen99 ammen99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are still a few small comments, but overall lgtm. If you fix them and are generally happy with how the PR works on your monitors, we can merge.

kode54 and others added 14 commits May 23, 2026 23:42
Also store color representation v1 pointer.
For future proofing, in case they become useful to
the render setup.
Defaulting to sRGB/gamma22 for non-matching parameters.
Since the wlroots Vulkan renderer already supports intermediate
float16 render pass, take advantage of it, and support allocating
other intermediate buffers with a float16 hint, otherwise falling
back on fixed 16 or fixed 8 where the others are unavailable.
Plugin-managed linear-space buffers (Expo's per-workspace aux, the
cube's per-workspace framebuffers, the grid crossfade snapshot, view
snapshots, and transformer inner_content) had no opinion about their
underlying format. With an HDR (PQ) output, the per-source luminance
multiplier in render_pass_t::add_texture scales PQ contents up to
~49.26 in the SDR-relative linear domain (PQ peak / SDR reference
white) before they land in those buffers. An 8-bit linear backing
clipped that to 1.0, dimming HDR highlights.

Pass hdr_linear through buffer_allocation_hints_t at each of those
sites, gated on the output's committed transfer function being
ST2084_PQ. SDR outputs keep their 8-bit allocations.

transformer_base_node_t::get_updated_contents gains an optional
wf::output_t* parameter so it can make the same decision; the call
site in transformer_render_instance_t::get_texture forwards _shown_on.
The default nullptr preserves the existing behavior for any
out-of-tree callers.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Direct scanout commits the surface buffer straight to the primary plane
via wlr_output_state_set_buffer, which leaves buffer_src_box / buffer_dst_box
unset and lets wlroots default both to the buffer's pixel dimensions. On
surfaces where the dimensions do not match the output exactly, some
drivers may malfunction and ignore the position and/or dimensions of the
direct scanout operation.

Fall back to compositing for these surfaces so the renderer can sample
the buffer correctly.
wlr_output_state_set_image_description() neither sets allow_reconfiguration
nor triggers wlroots' empty-buffer allocation path, so HDR toggles produced
atomic commits with no fresh primary buffer and no DRM_MODE_ATOMIC_ALLOW_MODESET
flag. amdgpu rejected those, leaving HDR_OUTPUT_METADATA unapplied. Re-pin the
current render format and set allow_reconfiguration on the pending state so
wlroots attaches a new primary FB and the DRM backend issues a modeset commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The wlroots Vulkan two-pass renderer composites every add_texture into
an FP16 blend image with sRGB primaries (it builds a sRGB-target
absolute-colorimetric matrix per source primaries in pass.c). Wayfire's
output color_transform was a pure linear→inverse-EOTF transform, which
the second pass applies with an identity color matrix — leaving the
final output buffer with sRGB primaries values that an HDR display
configured for Rec.2020 then over-saturates.

Build a 2-stage [matrix(sRGB→output_primaries), inverse_eotf] pipeline
when the output's committed image description advertises non-sRGB
primaries (e.g. BT.2020 on HDR), and fall back to the bare inverse_eotf
otherwise. The wlroots Vulkan renderer accepts that pipeline shape via
unwrap_color_transform's COLOR_TRANSFORM_PIPELINE branch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Plugins computing the FP16-backing hint for linear aux buffers each
inlined the same image-description / PQ-transfer check with a different
verbose comment. Move the check to a single helper in render.hpp and
keep the rationale comment there.

v2: Amended to move the checker function to the right place.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kode54 and others added 3 commits May 24, 2026 01:31
wlr_output_add_software_cursors_to_render_pass submits each cursor via
wlr_render_pass_add_texture with only texture / src_box / dst_box / clip
/ transform set — no .primaries, .transfer_function, or
.luminance_multiplier. wlroots therefore treats cursor pixels as already
in the output's color space. On PQ outputs the sRGB cursor values are
encoded as ~10000 cd/m² absolute Rec.2020, producing dazzlingly bright
and oversaturated cursors.

Open-code the cursor iteration so each cursor can carry the same sRGB-
primaries + gamma2.2 tagging wlr_surface_node applies to regular
surfaces, plus the SDR→PQ luminance multiplier from
compute_luminance_multiplier. SDR outputs are unaffected: the multiplier
resolves to 1.0 and the primaries conversion is identity.

The cursor box is built in scaled fb-pixel coords and run through
wlr_box_transform exactly as wlroots' helper does, avoiding the
floor/ceil rounding errors that a logical-coord roundtrip would
introduce on fractionally-scaled outputs.

compute_luminance_multiplier is promoted from a static helper in
render.cpp to wf:: so render-manager can call it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ammen99
Copy link
Copy Markdown
Member

ammen99 commented May 24, 2026

@kode54 I added some more commits to fix the remaining issues, please take a look at my latest changes, test and if it still works ok, I will merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants