cm: add ICC profile loading #12711
cm: add ICC profile loading #12711
Conversation
|
@fufexan can we get nix |
|
can you guys let me know if this works well? I have done very limited testing. |
Do not see anything in the logs showing it is loading the ICC profile, or rather nothing icc at all. What am I doing wrong there? |
|
oh oops, I think I forgot v2 |
|
my bad should work now |
|
it should dump what we were able to decode from the icc into the log like so if it loaded successfully 
|
|
Yup, it does attempt to load now. Unfortunately looks like the only ICC profile for my monitor is split-trc, so I won't be able to assist much with testing here.
|
|
I've added logging of the trc to the debug output, can you post the icc data dump here? |
|
Sure, here you go! |
|
thanks, despite xmas :P |
|
This should work. I've loaded my ICC profile and it does seem to change the colors on my panel a bit, but without a colorimeter I have no way of verifying if it's correct. Please load your ICC and let me know. |
|
It does seem to change the colors. I've ordered a colorimeter to verify though, should have it within a week. |
|
talk about dedication. I do think its wrong, sway's is different. |
|
@UjinT34 would you mind checking the LUT shader changes? I'm quite sure I am doing something wrong there. Shadows are a tad too bright I think. Can't say for sure without a colorimeter but they do look a bit flat and weird. |
|
I checked KDE, colors shift a tad but not as much as we do. Something's wrong. |
|
I am not that familiar with math around icc luts. |
|
fixed, but it's again surfacing that nightmare where kitty sets the DEFAULTS and yet it changes how we render shit. I need to do something about this... |
|
nvm its not fixed it just had a bug where it wouldnt apply cm curves fuck my life |
|
in general @UjinT34 shouldn't we compose to a linear SRGB buffer internally and then do a final CM pass on the entire image? Why are we doing it per-surface? This would be desirable as ICC shifts colors and will mess up e.g. color pickers. We should screencopy un-icc'd buffers. |
|
I've pushed a completely different approach with a 3D LUT instead, which also changes how we render CM. This will need some testing. This needs an HDR-kill-switch internally (HDR should disable ICC, currently it's likely the other way round) This needs people to run this shit and report any visual glitches or bugs, feel free to drive this @Flat thanks for your testing, if you could compare how this looks vs KDE it would be great. Sway's ICC is busted on my end, but KWin's works properly. |
|
I've decided to use Gamma 2.2 for internal storage just like kwin does, this lifts up the shadows a bit and looks more accurate in my opinion. |
|
Yay! This is my use case! I have a colorimeter, good wide gamut display, its ICC profile, and the display itself has a pretty accurate sRGB clamp which I measured. I hope to have some time to test this PR this weekend. Meanwhile the only program I've found that applies ICC profile correctly (for sRGB clamp) is https://github.com/ledoge/novideo_srgb. But its for Windows. Every Linux DE implementation I tried was way off color-wise, at least the last time I checked. |
|
Looks like that does help, also just realized think I profiled these with cm set to wide, so probably need to redo those as well |
|
cm, x is overridden by icc, doesn't do anything. Almost all cm options get overridden |
|
Yes, I mean when I created the ICC profile, it was set to wide, which would have been changing the base color readings used to create the profile. |
|
ah, right. Idk much about profiling but that could be an issue |
|
Just recreated them, not much different honestly. I think the ICC support looks good to me, unless someone with more experience doing this in Linux comes along. Wayland is pretty much unsupported by about the only calibration tool available on Linux, for calibration anyway. Profiling seems to work ok. See eoyilmaz/displaycal-py3#133 for further info on that. |
|
Just tested this PR briefly, and found that HDR screenshare with If I full-screen the app before opening OBS, the screenshare will fail. If I open OBS and share the screen, then full-screen the app, the app will output as if my display doesn't support HDR (extremely gray, washed-out). It also has a ~25% chance of just crashing instead when I do that. Works fine if the app is windowed, and works again the moment I close OBS. |
|
screenshare is a bit wonk atm yes |
|
@Flat do you mean that KDE and Hyprland are now not much different? |
Correct. I did not measure again, but they visually look about the same. |
|
Actual measurements (Normalized to White X + Y + Z) Hyprland KWin |
|
Took the time to profile in X11, which is known to work, and get some better human readable verification reports. X11 comes out almost spot on. X11 Monitor Measurement Report.html |
src/helpers/cm/ICC.cpp
Outdated
| // decided it'd be great to have both 18 and 20 | ||
| // FUCK YOU | ||
| size_t tableOff = 20; | ||
| if (raw.size() < tableOff + tableBytes) |
There was a problem hiding this comment.
This may not be correct, this ICC profile with vcgt is not loaded because of this check. I flipped the comparison to load it and it seems to load the rest of the table at offset 18
iccdump
tag 7:
sig 'vcgt'
type 'vcgt'
offset 976
size 1554
VideoCardGammaTable:
channels = 3
entries = 256
entrysize = 2
DEBUG ]: ============= Begin ICC load =============
DEBUG ]: ICC size: 969536 bytes
DEBUG ]: Building a 33³ 3D LUT
DEBUG ]: 3D LUT constructed, size 107811
DEBUG ]: readVCGT16: table has 3 channels, 256 entries, and entry size of 2
DEBUG ]: readVCGT16: table is likely offset 18 not 20, re-reading
DEBUG ]: readVCGT16: red channel: [0, 257, ... 65278, 65535]
DEBUG ]: readVCGT16: green channel: [0, 257, ... 65278, 65535]
DEBUG ]: readVCGT16: blue channel: [0, 257, ... 65278, 65535]
DEBUG ]: ============= End ICC load =============
There was a problem hiding this comment.
can you drop the icc profile so I can inspect it?
There was a problem hiding this comment.
Linked in the above comment, GitHub won't let me upload .icc so it's tar-ed
There was a problem hiding this comment.
thanks, I thought it was the log
There was a problem hiding this comment.
latest commit should allow your vcgt ramps to load. Can you check color accuracy now?
There was a problem hiding this comment.
If they loaded correctly when I changed that check, the readings are about the same, but if you expect it to be different, I can measure again in a few minutes
There was a problem hiding this comment.
DEBUG ]: ============= Begin ICC load =============
DEBUG ]: ICC size: 968640 bytes
DEBUG ]: Building a 33³ 3D LUT
DEBUG ]: 3D LUT constructed, size 107811
DEBUG ]: readVCGT16: table has 3 channels, 256 entries, and entry size of 2
DEBUG ]: readVCGT16: table is too short, but off = 18 fits. Attempting offset = 18
DEBUG ]: readVCGT16: red channel: [0, 257, ... 65278, 65535]
DEBUG ]: readVCGT16: green channel: [0, 257, ... 65278, 65535]
DEBUG ]: readVCGT16: blue channel: [0, 257, ... 65278, 65535]
DEBUG ]: ============= End ICC load =============
|
Just verified on Gnome Wayland, and it looks like their color management is correct, if that may help to reference as well. |
|
I can feel an imbalance in the force... I feel this will be a painful battle |
|
Try creating the profile with |
|
he created the profile on X11 iirc |
|
Correct, profile was created in X11, using DisplayCal/ArgyllCMS which disables any loaded profiles and resets video card gamma table before creating the profile. |
|
i don't have a calibration tool or anything but I do have icc profiles for my monitors, one of them loads fine, the other doesn't i set these whilst hyprland was open, but hyprland coredumps if i launch btw just from my eyes things look too dark but i could be wrong and stupid and wrong and stupid (did i mention that last one already?) |
This seems to be the case with Gamma 2.2 vs. other compositors (KDE, Sway) that use the same |
|
@fxzzi can you get a debug stacktrace |
|
errr is this what you're looking for also this icc profile crashes too: |
Looks like it needs a rebase to include 5faa66d |
|
probably but icc profile still fails to load either way |
|
@fxzzi idk why lcms fails no clue icc profile bork |
i'm not too sure i thikn hyprland might be parsing something in specific icc's wrong, because on my secondary monitor with its ICC profile everything looks pretty great, but on my main monitor which is an oled dark colours are definitely crushed again idk how this stuff works so grain of salt |
|
to be honest we're on par with KDE but if anyone knows what gnum does differently to get the accuracy actually good, lmk, otherwise idk much about how to push this forward |
|
So... after following the steps here https://planet.kde.org/xavers-blog-2024-07-15-how-to-profile-your-display-in-the-plasma-wayland-session/ I've measured the profile created by that (Display-DP-2-Wayland.tar.gz) on Gnome, KWin, and Hyprland. Doing the profile this way, Gnome is way off, and KDE is the most correct. Hyprland is close, but there is something wrong with the transform on the blue channel, specifically test patch 29. I think if that can be tracked down it would be correct, at least against KDE. I think it would take a bit more in terms of Wayland color management protocols to match X11. |
6 participants
Adds loading of icc profiles for the properties we support
ref #9064
Usage
v2:
icc = /full/pathv1:
, icc, /full/pathInfo
This also changes how we handle CM in general, where our intermediate buffer (offload) is in sRGB, and we CM at the end.
This should also make it possible to have actual, proper pixel-perfect screencopy in a follow-up MR (as we can copy the offloaded fb for screencopy purposes) - But that's for a follow-up MR
Requires: