Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simple color management #9506

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Simple color management #9506

wants to merge 16 commits into from

Conversation

UjinT34
Copy link
Contributor

@UjinT34 UjinT34 commented Feb 27, 2025

Describe your PR, what does it fix/add?

Added a simple CM shader and relevant settings.
New monitor rule cm:

  • auto: subject to change. srgb for 8bpc, wide for 10bpc if supported
  • srgb: default, sRGB primaries
  • wide: wide color gamut, BT2020 primaries
  • edid: primaries from edid (known to be inaccurate)
  • hdr: wide color gamut and HDR PQ transfer function
  • hdredid: same as hdr with edid primaries

New monitor rule sdrbrightness: brightness multiplier for SDR -> HDR mapping (sane value 1.0 - 2.0)
New monitor rule sdrsaturation: saturation modifier for SDR -> HDR mapping (sane value 0.9 - 1.1)

Removed experimental:wide_color_gamut
Removed experimental:hdr
Added render:cm_fs_passthrough to disable hdr metadata changes for fullscreen apps to avoid modesetting and black screen during the switch

Partially implements #9064

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

Desktop HDR has some weird interactions with blur and other effects.
SDR might also have incorrect interactions with blur and other effects. In general should be unnoticeable.
Old shaders are kept in place but no longer used.
New CM shader might be slow.
Some color transformations might be incorrect.
Interaction with screen capture or mirroring was not tested.
TEXTURE_EXTERNAL isn't supported, probably unused.

Is it ready for merging, or does it need work?

Ready for merging.

  • Convert primaries
  • Decode to linear
    • SRGB
    • GAMMA22
    • GAMMA28
    • ST2084_PQ
    • HLG
  • Encode from linear
    • SRGB
    • GAMMA22
    • GAMMA28
    • ST2084_PQ
    • HLG
  • Handle luminances
  • Mark relevant proto features as supported
  • Add a way to change output image description
  • Add a setting for fullscreen passthrough

@UjinT34
Copy link
Contributor Author

UjinT34 commented Feb 28, 2025

Ready for testing. SDR -> HDR with experimental:hdr = 1 should have correct colors but has some weird flickers. HDR -> SDR should work fine. Fullscreen apps should work the same.
Blur, fade animations, dimaround, custom screen shaders and external CM plugins might be broken.

@github-actions github-actions bot added the config label Mar 1, 2025
@UjinT34 UjinT34 marked this pull request as ready for review March 1, 2025 19:39
@abihf
Copy link

abihf commented Mar 2, 2025

Hi @UjinT34 , I tried to build from this branch but it crash on start up.

[ERR] Failed to link shader: Screen shader parser: Error compiling shader: 0:208(23): error: no matching function for call to `sqrt(int)'; candidates are:
0:208(23): error:    float sqrt(float)
0:208(23): error:    vec2 sqrt(vec2)
0:208(23): error:    vec3 sqrt(vec3)
0:208(23): error:    vec4 sqrt(vec4)
0:208(11): error: operands to arithmetic operators must be numeric
0:208(7): error: no matching function for call to `pow(error, vec3)'; candidates are:
0:208(7): error:    float pow(float, float)
0:208(7): error:    vec2 pow(vec2, vec2)
0:208(7): error:    vec3 pow(vec3, vec3)
0:208(7): error:    vec4 pow(vec4, vec4)

hyprlandCrashReport180489.txt

Copy link

@abihf abihf left a comment

Choose a reason for hiding this comment

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

this might be the problem

@abihf
Copy link

abihf commented Mar 2, 2025

just curious question: why are you not using lcms2 like gamescope or weston?

@UjinT34
Copy link
Contributor Author

UjinT34 commented Mar 2, 2025

just curious question: why are you not using lcms2 like gamescope or weston?

No idea how to use it in our case. Might be useful for some matrix calculations but most of the work is inside shaders

@abihf
Copy link

abihf commented Mar 3, 2025

seems like this lib support icc, might be useful for next iteration.

@lkcv
Copy link

lkcv commented Mar 3, 2025

Is it expected for non-HDR content to be really dim when HDR is active? The colors are still correct and look like they do when cm is set to wide, but wide is a bit brighter for SDR content. When cm is set to hdr, the cursor and window borders are very bright, but everything else (except for HDR games) is dim.

@UjinT34
Copy link
Contributor Author

UjinT34 commented Mar 3, 2025

Is it expected for non-HDR content to be really dim when HDR is active? The colors are still correct and look like they do when cm is set to wide, but wide is a bit brighter for SDR content. When cm is set to hdr, the cursor and window borders are very bright, but everything else (except for HDR games) is dim.

Default white brightness is only 80. Seems to be low for most cases. Will add a configurable multiplier in the future. Borders, hw cursor, blur and some other stuff doesn't have a CM atm.
HDR content should look fine with every setting combination (sdr/hdr and windowed/fullscreen). With HDR output mode SDR content which goes through CM will be too dim and which doesn't will be too bright.
With the current state it is better to have non-hdr cm and render:cm_fs_passthrough = true.
For now cm,hdr or cm,hdredid can be used with render:cm_fs_passthrough = false to test conversion from content HDR settings to reported monitor HDR capabilities.

@Soliprem Soliprem mentioned this pull request Mar 4, 2025
36 tasks
@abihf
Copy link

abihf commented Mar 4, 2025

I replace 80 to 300 and it looks better. But I agree, configuration like sdrnit would be nice.

@9oOzv
Copy link

9oOzv commented Mar 4, 2025

Hi,

I installed hyprland the other day for the first time and I've been testing this out for a bit.

Seems to be a lot better than the previous experimental:hdr = 1. Setting SDR luminance to 300-400 makes the image bright as it should be.

Should this map SDR programs correctly to BT2020? My browser (and possibly other SDR programs (?)) still seem too saturated. At least oranges seem too red. I don't know color management and color spaces well enough to figure out what exactly the problem is. What I've gathered:

  • sourcePrimaries seem to be sRGB. targetPrimaries are BT2020.
    • Tried logging these before render.
  • The shader code converting primaries seem to be running
    • Played around with the shader code and changing convertPrimaries does affect the image
  • On chromium, forcing color profile to ITU-R BT.2020 seems to fix the colors (Quite sure sourcePrimaries still evaluates to sRGB).
  • Alternatively, setting gamut on my TV manually to "Adobe RGB" (only other option the settings menu has), seems to fix the color (or at least it is much closer to what I think it should).

Is this just how it is supposed to work, or are the primaries not being mapped correctly? Maybe it is just the browser doing weird things (?). Though I do not really understand, how would a client outputting sRGB end up looking too saturated on a wider gamut monitor. If the colors are correctly mapped.

@UjinT34
Copy link
Contributor Author

UjinT34 commented Mar 4, 2025

I've tried several simple SDR brightness modifications. The image becomes brighter but I don't like the resulting quality. Probably needs a more sophisticated approach than simple multiplication.

A client outputs an integer value. Maximum red in sRGB and maximum red in BT2020 will have the same value. But should look different. convertPrimaries should change an sRGB maximum red to closest similar looking value in BT2020 which is lower than maximum. Current implementation does that but the effect is very subtle for some reason. Might be some issues with drm properties which aren't touched by this PR.
Chromium with forced BT2020 probably does its own conversion reducing the values and then they are reduced again by HL.

@UjinT34
Copy link
Contributor Author

UjinT34 commented Mar 5, 2025

@9oOzv primaries conversion should work better now

@9oOzv
Copy link

9oOzv commented Mar 5, 2025

@9oOzv primaries conversion should work better now

Great. I think it looks closer now.

Though I do still notice some weird tint here and there compared to all my SDR monitor. Some pinks tint towards orange. Some skin tones towards green (I think... quite difficult to say actually other than it looks a bit off still). Different enough for me to believe it is not just calibration issues.

Nonetheless, I think with the addition of 'sdr brightness' setting, I think this would make Hyprland much more usable on HDR monitors.

@Soliprem
Copy link
Contributor

Soliprem commented Mar 6, 2025

iirc Plasma has values for sdr brightness and color intensity. This could (at least temporarily) take the burden of calibration away from the maintainers. Making strongly recommended defaults for calibration but letting them be configured could be an easy "out" in terms of figuring out details. But this is just my two cents, and I'm a total compositor/c++ noob

@9oOzv
Copy link

9oOzv commented Mar 6, 2025

I think having color intensity config for SDR would be nice at some point, as sRGB colors, even if correctly mapped, can look disappointingly unsaturated compared to wide gamut content on the same screen,. Or compared to the regular SDR monitors, that we are used to and often are much more saturated.

But getting the primaries conversion correct should probably be the first step. I do not think it is correct quite yet, but I cannot tell for sure just by looking at the picture.

@Soliprem
Copy link
Contributor

Soliprem commented Mar 6, 2025

general/total brightness is very low, even if colors are improved since the last commit (at least on my monitor)

@UjinT34
Copy link
Contributor Author

UjinT34 commented Mar 6, 2025

At this point I am pretty sure that conversion between primaries with the same white point is correct. No white point compensation for now. Other stuff which happens before and/or after the conversion might throw it off.
10bpc is not enough to map all 8bpc sRGB values to 10bpc BT2020. Some precision will be lost in favor of wider range.

Added two new monitor rules to control SDR -> HDR brightness and saturation. bitdepth,10,cm,hdr,sdrbrightness,1.2,sdrsaturation,1.0 works for me.

@lkcv
Copy link

lkcv commented Mar 6, 2025

With the latest changes, the saturation when using auto or wide seems to have been greatly reduced. The colors are very vivid if I roll back to (58cb7e7).

Update:
Regressed by ecb62f2

@UjinT34
Copy link
Contributor Author

UjinT34 commented Mar 6, 2025

With the latest changes, the saturation when using auto or wide seems to have been greatly reduced. The colors are very vivid if I roll back to (58cb7e7). I'll try to bisect in a bit.

Vivid colors are oversaturated. You might have some monitor settings that compensated that oversaturation and now they cause undersaturation.

@lkcv
Copy link

lkcv commented Mar 6, 2025

I probably should have described the issue differently. Colors appear to have a yellow tint to them after that commit. For example, shades of pink tends to look like a peach color now.

Btw, I check my TV's color settings, and changed the color gamut and gamma curve values and they didn't have an effect on this.

@9oOzv
Copy link

9oOzv commented Mar 6, 2025

Looks good to me. sdrbrightness and sdrsaturation seems to work. For me, something like sdrbrightness, 1.35, sdrsaturation, 1.04 works quite good.

I am still not sure if the saturated pinks/magentas are missing or if my other monitors just are not showing true sRGB. But I guess that can be a separate issue if needed. Ill dig a bit deeper when/if I find time for it.

@9oOzv
Copy link

9oOzv commented Mar 6, 2025

I probably should have described the issue differently. Colors appear to have a yellow tint to them after that commit. For example, shades of pink tends to look like a peach color now.

Btw, I check my TV's color settings, and changed the color gamut and gamma curve values and they didn't have an effect on this.

This sounds exactly like what I was referring to in my previous comment

@Soliprem
Copy link
Contributor

Soliprem commented Mar 6, 2025

no notes from the last commit. LGTM

edit: found this tool that's kinda handy to check how colors look
https://www.eizo.be/monitor-test/

@Soliprem
Copy link
Contributor

Soliprem commented Mar 6, 2025

upon further checking/testing, there's something weird about greens specifically. Other colors look fine though.
Greens in particular look undersaturated (to me). This did not reflect on the monitor test I linked above, so I can't properly quantify it. It's also not a dealbreaker as hdr is marked as experimental, but worth checking out (maybe someone here has better testing tools to corroborate what I'm experiencing)

edit: yeah it feels either like the greens being undersaturated or as a slight yellow tint as said above, to me

@lkcv
Copy link

lkcv commented Mar 6, 2025

I probably should have described the issue differently. Colors appear to have a yellow tint to them after that commit. For example, shades of pink tends to look like a peach color now.
Btw, I check my TV's color settings, and changed the color gamut and gamma curve values and they didn't have an effect on this.

This sounds exactly like what I was referring to in my previous comment

Ah, I didn't see that at first. Is this happening for you when using auto/wide or only in hdr? For me, it happens in the former.

@9oOzv
Copy link

9oOzv commented Mar 6, 2025

I probably should have described the issue differently. Colors appear to have a yellow tint to them after that commit. For example, shades of pink tends to look like a peach color now.
Btw, I check my TV's color settings, and changed the color gamut and gamma curve values and they didn't have an effect on this.

This sounds exactly like what I was referring to in my previous comment

Ah, I didn't see that at first. Is this happening for you when using auto/wide or only in hdr? For me, it happens in the former.

I have only tried in HDR. I took a picture of my 3 monitors (top one is an HDR OLED TV (Hyprland), bottom left is an SDR one (Hyprland), Bottom right is SDR (attached to a Windows computer). The problem is quite apparent at some specific colors.


20250306_223437

@9oOzv
Copy link

9oOzv commented Mar 6, 2025

Just tested, and It is even worse in wide

@lkcv
Copy link

lkcv commented Mar 6, 2025

Shouldn't the primary conversion only happen for SDR->HDR and not in SDR-only scenarios, such as auto/wide?

@9oOzv
Copy link

9oOzv commented Mar 6, 2025

Shouldn't the primary conversion on happen for SDR->HDR and not in SDR-only scenarios, such as auto/wide?

I think it should happen for wide as well. SDR programs output sRGB. Monitor configured as wide is BT2020. So you need to convert from the programs sRGB primaries to the monitors BT2020 primaries. sRGB red is different from BT2020 red, green is different from green etc. So if you used the sRGB values as-is for the monitor, it would look wrong (too saturated).

You can always use srgb instead of wide, if you want the monitor use the RGB values from your programs without conversion

@lkcv
Copy link

lkcv commented Mar 6, 2025

Thanks for the explanation. I guess I was wondering why the recent primary conversion changes weren't specific to hdr cm.

I took some pictures of the issue - sorry for the angle, but the glare was pretty bad

Before commit:
20250306_162506

After commit:
20250306_163043

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

Successfully merging this pull request may close these issues.

5 participants