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

HCY Color wheel and HCYColor HSV interconversion bug fix [ Patch included :D ] #1167

Open
ChengduLittleA opened this issue Dec 10, 2021 · 10 comments
Labels
cat.Widget Issue relates to a widget type.Enhancement Issue requests feature.
Milestone

Comments

@ChengduLittleA
Copy link

ChengduLittleA commented Dec 10, 2021

Update: I managed to fix all the problems described in this thread. Scroll to the bottom to see patch and description.

:D

Description of the problem

Because I was researching about the perpetual brightness and chroma, and I always felt that the HCY picker isn't quite "flat". They always show some strips with rather high chroma. (But better than HSY' picker in Krita because the saturation is much more comfortable) In theoy it should be very flat as the values are perpetually adjusted, as in Y' (Luminance).

图片

Looks to me Kirta handles this much more correctly? The color wheel is flat, and also the blue Y slider looks much more linear as well.

图片

Also I think somewhere in the conversion there's a problem, and this is how you can produce it:

  1. Select this a very dark blue colour in HCY picker, doesn't matter if it's exact or not, just somewhere on that blue stripe when brightness is very low.
  2. Use an opaque brush to paint, like "Pointy ink", make sure it doesn't have color variation.
  3. Pick the color you just painted.
  4. Immadiately you can see the color picker position jumps up a little bit.
  5. After like maybe two or three times the color picker is already at the top of the wheel.

Peek 2021-12-10 19-05

This does look like a precision issue (because internally it's 16bit fixed point?), but I think this is also related to the conversion method used in HCYColor, because the resulting circle is not exactly "flat" looking. If we use the screenshot of the color wheel and put it into GIMP, desaturate it using HCY' (Luminance option) and Luma, we can actually see when using Luma the desaturated result is much closer to flat, which considering bit depth, is consistent with MyPaint HCY conversion algorithm (which is luma).

So the question becomes, can we have Y' (Luminance) as our picker model? Because it's even closer to perpetual brightness.

Basic system details

MyPaint version: [Doesn't really matter, it's latest, and this part hasn't seen changes since forever]
Operating system: Xubuntu 20.04
Desktop environment: XFCE

@ChengduLittleA
Copy link
Author

Also another problem:

If libmypaint uses 16bit fixed point linear color space, then Y (luma) and Y' (luminance) should be the same value. Then why would a gamma-corrected canvas display and color picker show otherwise (where Y stay the same but perpetually it's non uniform?)

@ChengduLittleA
Copy link
Author

ChengduLittleA commented Dec 10, 2021

Oh I see the problem is actually mentioned long before in here... (WE REALLY NEED TO SOLVE THIS?) #890

I dug in the colour representation all day on the problem with Y, Y' and L*. Here's what I found:

  • if you draw the color slider/picker thing with rec709 coefficients, then you get a screenshot and put it into GIMP, use desaturate-Luma, it will be equal brightness.
  • use rec601 (as default) there doesn't show equal brightness in either "Luminance" or "Luma" mode in GIMP desaturation (because according to gimp manual, it uses 709's coeff).
  • Krita does show correctly with HSY' picker with rec709 coefficients and gamma 2.2 in GIMP desaturate-LUMINANCE mode, however, according to wiki explanation, it should show correctly in GIMP desaturate-LUMA mode. So either Krita is actually using L*/Y there, or GIMP is doing it wrong.

image

I think we should use L* (as in Lab) or Y (But I messed around with code I can't seem to find a way to get a "gamma corrected Y" solution? My math isn't good.), yet I don't know how to get rgb (be it linear or gamma-ed) from a L* or Y correctly.

from Wikipedia https://en.wikipedia.org/wiki/Relative_luminance :

Relative luminance should not be confused with luma Y ′ {\displaystyle Y^{\prime }} {\displaystyle Y^{\prime }} (Y prime), which is a weighted sum of nonlinear (gamma encoded) R′G′B′ components, where in some implementations the weighting coefficients are applied to the gamma encoded signal. Some colorspaces that use luma include Y′UV, Y′IQ, and Y′CbCr. To determine relative luminance, The Y ′ {\displaystyle Y^{\prime }} {\displaystyle Y^{\prime }} must be used with the subcomponents to create the gamma encoded R′G′B′ components, which are then linearized to RGB by inverting the gamma correction. These linearized RGB channels can then have the appropriate linear coefficients applied (based on the primary chromaticities) and summed to relative luminance Y {\displaystyle Y} Y.

I think the best solution now is to have a look at how Krita gets that color and try to steal use the same method.

Also, A native 16bit canvas would get around these problems, but I see legacy code for gamma/eotf/whatever thing all over the place so I imagine that could be some work...

@briend Any ideas?

@ChengduLittleA
Copy link
Author

ChengduLittleA commented Dec 10, 2021

@ChengduLittleA
Copy link
Author

image

Additionally... Another weird thing, Lab* should have equal Luminance(?) across a/b field and in krita display it isn't really the case :(

@ChengduLittleA
Copy link
Author

ChengduLittleA commented Dec 11, 2021

Update: I used the HSY' code from Krita, it gives exactly the same result as the one in mypaint... I think it's gonna largely be due to lcms. I'm compiling Krita and try to mess with the code and see if it gives any more detail.

Update 2: Indeed the brightness is corrected using lcms (where it corrects to a certain profile, srgb? I still not understand why it needs to be corrected again, maybe it's for srgb->luminance correction with D65 or whatever, I need to check that part of the lcms code, it's likely gonna be converted to YCC/XYZ model then lab or something like that). I commented out the lcms delinearize code and immediately it produced the same result (And verifiable in GIMP using Luma desaturation). So if we are to solve this, we need to use lcms in some capacity, at least in the color wheel.

(The high saturation is also because krita uses H[S]Y' not H[C]Y). I'm then gonna see if I can get that lcms function to work with mypaint hcy functions.

Note that the brightness slider is actually not linear in this screenshot, the middle point only has rgb about 54. The behavior in MyPaint is actually correct, it only lacks that final mystery profile stage.

图片

@ChengduLittleA
Copy link
Author

Update: I found the problem is actually related to cairo drawing. The mixing isn't linear. I draw the gradients manually and it matches rec709 perfectly. If I do linear rec 709 it's perfect in grayscale brightness, but now the color picker will jump to a lower Y if I select some colors on the edge... still investigating.

@ChengduLittleA
Copy link
Author

OK I fixed everything now by myself XD

I'm using rec709 coeffs for the palette, but it can be changed to others.

I'm gonna update a diff here in a short while

Actual problems, the way to correctly handle them:

  1. DO NOT use cairo to mix the palette, because it's a non-linear mix. (It mixes in sRGB numerically, which would certainly lead to darker gray region)
  2. Do rec. 709 Y transformation in linear space, then transform them back. It's not perfect flat but numerically it matches luminance desaturation algorithm which is currently the closest to perception. Very dark blue and reds are a bit brighter than the calculation, and yellow is a little bit darker. if we want perfect flat we may need to use a LUT which is too overkill.
  3. And WHO WROTE THIS:

图片

@ChengduLittleA ChengduLittleA changed the title Can we have HC[Y'] instead of HCY as the picker model? HCY Color wheel and HCYColor HSV interconversion bug fix [ Patch included :D ] Dec 11, 2021
@ChengduLittleA
Copy link
Author

Here's the patch: HCY Color wheel correct display.

This patch contains:

  1. HCYColor bug fix in to/from HSV operations.
  2. Using correct color for HCY Color wheel instead of cario mixing.
diff --git a/gui/colors/adjbases.py b/gui/colors/adjbases.py
index 1b5b726c..80a943a3 100644
--- a/gui/colors/adjbases.py
+++ b/gui/colors/adjbases.py
@@ -1180,6 +1180,14 @@ class HueSaturationWheelMixin(object):
         if mgr:
             theta = mgr.undistort_hue(theta)
         return self.color_at_normalized_polar_pos(r, theta)
+    
+    def add_color_stops(self, hue, gr, nstops, gamma):
+        nstops = int(nstops) + 2
+        for s in xrange(nstops+1):
+            a = 1 * (((nstops - s) / nstops) ** gamma)
+            stop = s / nstops
+            rgb = self.color_at_normalized_polar_pos(a, hue).get_rgb()
+            gr.add_color_stop_rgba(stop, rgb[0], rgb[1], rgb[2], 1)
 
     def render_background_cb(self, cr, wd, ht, icon_border=None):
         """Renders the offscreen bg, for `ColorAdjusterWidget` impls.
@@ -1229,32 +1237,17 @@ class HueSaturationWheelMixin(object):
                 x, y = cr.get_current_point()
                 cr.line_to(0, 0)
                 cr.close_path()
-                lg = cairo.LinearGradient(radius, 0, (x + radius) / 2, y)
-                lg.add_color_stop_rgba(0, rgb[0], rgb[1], rgb[2], 1.0)
-                lg.add_color_stop_rgba(1, rgb[0], rgb[1], rgb[2], 0.0)
+                lg = cairo.LinearGradient(x,y, 0,0)
+                for i in range(0,sat_slices+1):
+                    percent = i/(sat_slices)
+                    rgb_ = self.color_at_normalized_polar_pos(1-percent, h).get_rgb()
+                    self.add_color_stops(h,lg,sat_slices,sat_gamma)
                 cr.set_source(lg)
-                cr.fill()
-            if ih < steps:
-                # Forward solid
-                cr.arc(0, 0, radius, 0, step_angle)
-                x, y = cr.get_current_point()
-                cr.line_to(0, 0)
-                cr.close_path()
-                cr.set_source_rgb(*rgb)
                 cr.stroke_preserve()
                 cr.fill()
             cr.rotate(step_angle)
         cr.restore()
 
-        # Cheeky approximation of the right desaturation gradients
-        rg = cairo.RadialGradient(0, 0, 0, 0, 0, radius)
-        add_distance_fade_stops(rg, ref_grey.get_rgb(),
-                                nstops=sat_slices,
-                                gamma=1.0 / sat_gamma)
-        cr.set_source(rg)
-        cr.arc(0, 0, radius, 0, 2 * math.pi)
-        cr.fill()
-
         # Tangoesque inner border
         cr.set_source_rgba(*self.EDGE_HIGHLIGHT_RGBA)
         cr.set_line_width(self.EDGE_HIGHLIGHT_WIDTH)
diff --git a/lib/color.py b/lib/color.py
index cd46cfea..f0cad504 100644
--- a/lib/color.py
+++ b/lib/color.py
@@ -24,6 +24,7 @@ with an adjuster does its type change to match the control's color space.
 from __future__ import division, print_function
 import re
 import colorsys
+import lib.hsluv
 
 from lib.gibindings import GdkPixbuf
 
@@ -523,11 +524,11 @@ class HCYColor (UIColor):
                 c = color.c
                 y = color.y
             else:
-                h, s, v = color.get_hsv()
-                _, c, y = RGB_to_HCY(colorsys.hsv_to_rgb(h, s, v))
+                hv, s, v = color.get_hsv()
+                h, c, y = RGB_to_HCY(colorsys.hsv_to_rgb(hv, s, v))
                 # Special case to retain chroma/saturation when moving to black
                 if v == 0 and c == 0 and s != 0:
-                    _, c, _ = RGB_to_HCY(colorsys.hsv_to_rgb(h, s, 0.01))
+                    h, c, _ = RGB_to_HCY(colorsys.hsv_to_rgb(hv, s, 0.01))
         if hcy is not None:
             h, c, y = hcy
         self.h = h  #: Read/write hue angle, scaled to the range 0.0 to 1.0
@@ -542,8 +543,8 @@ class HCYColor (UIColor):
         h, s, v = colorsys.rgb_to_hsv(*rgb)
         # Special case to retain chroma/saturation when moving to black
         if s == 0 and v == 0 and self.c != 0:
-            _, s, _ = colorsys.rgb_to_hsv(*HCY_to_RGB((self.h, self.c, 0.01)))
-        return self.h, s, v
+            h, s, _ = colorsys.rgb_to_hsv(*HCY_to_RGB((self.h, self.c, 0.01)))
+        return h, s, v
 
     def get_rgb(self):
         return HCY_to_RGB((self.h, self.c, self.y))
@@ -785,14 +786,23 @@ def YCbCr_to_RGB_BT601(YCbCr):
 # ref Levkowitz H., Herman G.T., "GLHS: a generalized lightness, hue, and
 #     saturation color model"
 
-# For consistency, use the same weights that the Color and Luminosity layer
-# blend modes use, as also used by brushlib's Colorize brush blend mode. We
-# follow http://www.w3.org/TR/compositing/ here. BT.601 YCbCr has a nearly
-# identical definition of luma.
+# Using Rec.709 luma for being closer to perpetual brightness.
+# Because HCY color is only used in color picker, so it doesn't matter
+# if it's not the same with the layer compositors.
 
-_HCY_RED_LUMA = 0.3
-_HCY_GREEN_LUMA = 0.59
-_HCY_BLUE_LUMA = 0.11
+_HCY_RED_LUMA = 0.2126
+_HCY_GREEN_LUMA = 0.7152
+_HCY_BLUE_LUMA = 0.0722
+
+def delinearize(L):
+    if L<0.018:
+        return 4.5*L
+    return 1.099*(L**0.45)-0.099
+
+def linearize(V):
+    if V<0.081:
+        return V/4.5
+    return ((V+0.099)/1.099)**(1/0.45)
 
 
 def RGB_to_HCY(rgb):
@@ -804,6 +814,9 @@ def RGB_to_HCY(rgb):
 
     """
     r, g, b = rgb
+    r=linearize(r)
+    g=linearize(g)
+    b=linearize(b)
 
     # Luma is just a weighted sum of the three components.
     y = _HCY_RED_LUMA*r + _HCY_GREEN_LUMA*g + _HCY_BLUE_LUMA*b
@@ -832,6 +845,9 @@ def RGB_to_HCY(rgb):
     else:
         # For the derivation, see the GLHS paper.
         c = max((y-n)/y, (p-y)/(1-y))
+    
+    y= delinearize(y)
+    
     return h, c, y
 
 
@@ -854,9 +870,7 @@ def HCY_to_RGB(hcy):
 
     """
     h, c, y = hcy
-
-    if c == 0:
-        return y, y, y
+    y=linearize(y)
 
     h %= 1.0
     h *= 6.0
@@ -894,6 +908,10 @@ def HCY_to_RGB(hcy):
         p = y + (1-y)*c
         o = y + (1-y)*c*(th-tm)/(1-tm)
         n = y - (1-y)*c*tm/(1-tm)
+        
+    p=delinearize(p)
+    o=delinearize(o)
+    n=delinearize(n)
 
     # Back to RGB order
     if h < 1:

@ChengduLittleA
Copy link
Author

In the mean time I also found out HSLuv could be even more useful, because Luv is supposedly flatter than Y', and it can apply different color matrix to transform RGB to XYZ, like Adobe/CIE Dxx/whatever. I'm gonna make a similar color picker for using it.

@AesaraB AesaraB added type.Enhancement Issue requests feature. cat.Widget Issue relates to a widget labels Jan 17, 2024
@AesaraB AesaraB added this to the v2.02 milestone Jan 17, 2024
@dreamlayers
Copy link

Working with a weighed sum of r, g and b only makes sense if you are dealing with linear values. Summing sRGB values doesn't make sense. So, the use of linearize() and delinearize() makes sense and gives better results.

Theoretically, https://en.wikipedia.org/wiki/CIELAB_color_space#Cylindrical_model makes more sense when considering human perception. But then you run into the problem of how only a part of that can be represented by sRGB. The general problem is shown by how you can display a very pure blue, but it will be dim. You can only get a nice circle if you limit saturation and brightness.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat.Widget Issue relates to a widget type.Enhancement Issue requests feature.
Development

No branches or pull requests

3 participants