Saturation & Brightness/Value Gain using Oklab color space #1477
Conversation
|
Hello @xkns 👋 I'm the Hyperion Project Bot and I want to thank you for To help you and other users test your pull requests faster, 🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/2481085530 Of course, if you make changes to your PR, I will create a new link. Best regards, |
|
This pull request introduces 5 alerts when merging b874453 into 359618a - view on LGTM.com new alerts:
|
These are all in the Oklab reference implementation. Not sure how easy it is license wise to try to fix them. Edit: As the reference implementation is licensed under MIT it appears that we can modify and redistribute it under any license as long as the copyright is kept. But I'm not a lawyer. |
|
Hey @xkns I created a new link to your workflow artifacts: |
|
This pull request introduces 5 alerts when merging 5a295b9 into 359618a - view on LGTM.com new alerts:
|
|
I deeply hope that this PR is accepted. I compiled this and the saturation and brightness gain controls put Hyperion over the line for me. Would the translation to OKLAB facilitate the implementation of ICC profiles for calibration? I find that my particular LEDs render blue midtones as a rich cyan in a way that cannot be corrected through Hyperion's calibration options without ruining the white balance. |
I've been in contact with the maintainer (@Lord-Grey) and I'm optimistic that this will get merged at some point. We'll see how the code review goes...
We have also started to look into this issue. While I believe that it is theoretically possible to get a good color correction profile set using the current settings (color setpoints + gamma curves) we agree that it isn't very intuitive to do so. I don't think that importing ICC files is the solution here as that would require users to use some external software to generate those files. Fine tuning would also be very annoying as one would have to import the newly adjusted ICC file every single testing iteration. I'm sure that we can find a more user friendly way but that will be in a new PR. Whether or not Oklab will play a part in that is to be seen but the necessary facilities will be present when this PR is merged. |
|
Awesome to hear. I notice that the cyan, yellow, magenta, and white values appear to no longer work. I'm guessing they're discarded during conversion. That might need to be made more clear in ui.
…________________________________
From: xkns ***@***.***>
Sent: Thursday, July 7, 2022 10:51:47 PM
To: hyperion-project/hyperion.ng ***@***.***>
Cc: Chris Johnstone ***@***.***>; Comment ***@***.***>
Subject: Re: [hyperion-project/hyperion.ng] Saturation & Brightness/Value Gain using Oklab color space (PR #1477)
I deeply hope that this PR is accepted. I compiled this and the saturation and brightness gain controls put Hyperion over the line for me. I've been in contact with the maintainer ***@***.***) and I'm optimistic that this will get merged at
ZjQcmQRYFpfptBannerStart
CAUTION: This Email is from an EXTERNAL source. Ensure you trust this sender before clicking on any links or attachments.
ZjQcmQRYFpfptBannerEnd
I deeply hope that this PR is accepted. I compiled this and the saturation and brightness gain controls put Hyperion over the line for me.
I've been in contact with the maintainer ***@***.***<https://github.com/Lord-Grey>) and I'm optimistic that this will get merged at some point. We'll see how the code review goes...
Would the translation to OKLAB facilitate the implementation of ICC profiles for calibration? I find that my particular LEDs render blue midtones as a rich cyan in a way that cannot be corrected through Hyperion's calibration options without ruining the white balance.
We have also started to look into this issue. While I believe that it is theoretically possible to get a good color correction profile set using the current settings (color setpoints + gamma curves) we agree that it isn't very intuitive to do so.
I don't think that importing ICC files is the solution here as that would require users to use some external software to generate those files. Fine tuning would also be very annoying as one would have to import the newly adjusted ICC file every single testing iteration.
I'm sure that we can find a more user friendly way but that will be in a new PR. Whether or not Oklab will play a part in this is to be seen but the necessary facilities will be present when this PR is merged.
—
Reply to this email directly, view it on GitHub<#1477 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AFCPQMS7X6OGXF2OIEWN37LVS2ZEHANCNFSM5YQRPNGQ>.
You are receiving this because you commented.Message ID: ***@***.***>
Chris Johnstone
Production Print Specialist
Ricoh New Zealand Limited
Mobile: +64 21 849 115
www.ricoh.co.nz
ISO 14001 Environmentally certified | ISO 27001 Information Security Certified
|
|
Edit: Sorry I misunderstood what you were saying. You're talking about the color setpoint settings and not that certain input colors go missing at some point. The Okhsv transform is being applied before the color correction. It converts to Okhsv, then scales s (-aturation) and v (-alue) by the given factors and finally converts back to sRGB. The resulting color then runs through the color correction and all the other options before finally being output to the output device. While the color correction should still work you might need to re-adjust it a bit since most colors reaching the color correction could now be in a different region of the color gamut (e.g. they are more saturated now) which you might not have tuned for previously. A new color correction system could definitely help out here. The whole Okhsv transformation will be skipped when both saturation and brightness gain are set to their neutral value of |
|
@xkns Sorry, for only coming back on this now. a) As I am not too detailed in the Color transformation subject, maybe you could help me in understanding why the saturation and value gain are transformed before the RGB color transformations. By heart I would have expected that first Colors are mapped and then gains applied. Maybe I am missing some theory here.... b) What is the number range for saturation and value gain? c) Should the RGB+SatGain+ValGain an alternative way to the existing calibration rather then one on top? d) What would be the guidance for a standard user going forward? Sorry for those many questions, but I would like to get a better feels in which direction the target design should go. |
As this is MIT, we can modify the code. Good practice is to keep the original copyright, plus adding info on change
|
The order of operations should be irrelevant for hue and saturation. If you were to first saturate a color and then map it or first map it and then saturate the mapped color shouldn't make a significant difference (some rounding errors might occur). What does make a difference is when you are limiting the brightness through the existing mapper and were to then apply some brightness gain (>= 1.0). In that case you would exceed the maximum brightness as set by your mapping. There are basically three options here:
In theory you could construct some "super" matrix that does everything at once but I don't have a good enough understanding of the Okhsv definition / reference implementation to attempt such a thing.
Should you want to use percent instead all values would simply have to be multiplied by 100 (including the upper limits). Using other scales would also be possible but a simple multiplier is quite intuitive in my opinion.
ValGain is redundant in this sense because you should theoretically be able achieve the same effect by setting all three gamma values just right. What it can't do however is account for different physical light output by various color channels on the hardware. This is important for getting mixed colors (including gray tones) right. I'm not sure if I would count SatGain as a calibration measure or simply an aesthetic effect. It's not one of the "calibration settings" you would find in other pieces of software but you could argue that (especially in HDR situations) the LED color output might be lacking in saturation / looking washed out. In that sense it would calibrate something that other existing settings cannot account for.
This probably needs some additional discussion but I would say that saturation is something that a standard user has some intuition for. As it isn't really redundant there isn't really an alternative expert/non-expert setting to show instead. Brightness gain on the other hand is mostly identical (the curves diverge more as you get farther away from the neutral value) to gamma settings if all gamma settings have the same value. So the gamma settings could be regarded as the expert brightness gain. This is especially the case if we were to change the input curve to match the gamma formula.
Thanks for taking the time and sorry for messing up the JSON definitions. |
|
Hey @xkns I created a new link to your workflow artifacts: |
|
This pull request introduces 2 alerts when merging b2b06c0 into 8000c3e - view on LGTM.com new alerts:
|
|
Hey @xkns I created a new link to your workflow artifacts: |
|
This pull request introduces 1 alert when merging f181d9b into 8000c3e - view on LGTM.com new alerts:
|
|
Hey @xkns I created a new link to your workflow artifacts: |
|
Hey @xkns I created a new link to your workflow artifacts: |
|
Hey @xkns I created a new link to your workflow artifacts: |
|
@xkns Ready to merge from your perspective? |
Yes ready to merge |
3 participants
Summary
Adds Saturation & Value Gain settings to the UI under
Image Processingand adjusts the colors accordingly using Okhsv. As discussed on Discord this was done using a newOkhsvTransformclass.Fixes #822, #1092 and partially fixes #1142.
What kind of change does this PR introduce? (check at least one)
If changing the UI of web configuration, please provide the before/after screenshot:
Images
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing setups:
The PR fulfills these requirements:
Fixes: #xxx[,#xxx], where "xxx" is the issue number)If adding a new feature, the PR's description includes:
PLEASE DON'T FORGET TO ADD YOUR CHANGES TO CHANGELOG.MD
To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.
Other information: