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

using clampChroma within a conversion sequence results in invalid values #129

Open
Enteleform opened this issue Jul 22, 2021 · 18 comments
Open

Comments

@Enteleform
Copy link

Issue

Using clampChroma within a conversion sequence results in a color object with invalid values.

Expected

Post-clampChroma conversions result in color objects with valid values as defined by culorijs.org/color-spaces.

Example

import {rgb, oklch, clampChroma} from "culori"

const max = {l:1, c:0.322, h:360}

rgb(
  clampChroma(
    oklch({
      l: (max.l * 0.5),
      c: (max.c * 0.5),
      h: (max.h * 0.5),
    })
  )
)

Result

oklch > clampChroma > rgb

r has a negative value, which is outside of the rgb color-space's documented {min:0, max:1} range.

{mode:'rgb', r:-0.00015269415291914584, g:0.4633768089712706, b:0.40462053745496246}

Other Values

oklch

{mode:'oklch', l:0.5, c:0.161, h:180}

oklch > clampChroma

{mode:'oklch', l:0.5074222643221025, c:0.09214221649333439, h:179.92987163513519}

oklch > rgb

{mode:'rgb', r:-1.1275082865428603, g:0.4943967336505179, b:0.40272542671134454}
@danburzo
Copy link
Collaborator

Thanks for the report @Enteleform! The clamped RGB color is expected to have at least one of its channels very close to the range boundary, and in this case it seems a small conversion error is causing it to go very slightly under 0.

culori.clampChroma() works on the lch color space and returns the clamped version in the color's original space, so the sequence rgb(clampChroma(oklch(color))) involves several conversions:

  1. oklch -> lch, which is (roughly) oklch -> rgb -> lch,
  2. (clamped) lch -> oklch, that is lch -> rgb -> oklch, and finally
  3. oklch -> rgb

In this case, however, the crux of the problem seems to be that the result of culori.clampChroma is not actually displayable:

let lchColor = culori.lch({mode:'oklch', l:0.5, c:0.161, h:180});
let clamped = culori.clampChroma(lchColor);
culori.displayable(clamped); // => false

...which is odd. I'll take a look to see what's going on.

@danburzo
Copy link
Collaborator

So, the clampChroma() method made the following assumption: there is a value for c for which we know the color is displayable, we're going to iterate towards it until the search interval becomes very small. At this point, for a small enough interval, the color should be displayable. But it didn't guarantee you landed on a displayable color on the last iteration. With the fix above we use the last color in the interation we know to be displayable (sacrificing a tiny bit of chroma in the process).

@Enteleform
Copy link
Author

@danburzo Awesome, thanks for the quick fix!

@Enteleform
Copy link
Author

@danburzo Hey I think I might have found some more issues with the chroma clamping.

Video Demonstration:


I wrote a small wrapper for culori that uses unit intervals [0-1] for [r,g,b,h,c,l,a, etc.], but I do have some tests implemented that ensure accurate conversions. I'll do a bit more debugging tomorrow to double check that the issues aren't being caused by anything on my end & let you know how it goes.

If I do narrow the issue down to something within culori, I'd be open to setting up a screen sharing session with you if you think it might lead to any insights or want to check out any of the generated data.


This is the function I'm calling in the demo:

Color.Range({
  count,
  color:  {h:(hue/100), c:(chroma/100), l:0},
  result: "Hex6",
  modify: {
    l: {ease:"Linear", range:{from:(min/100), to:(max/100)}},
  },
})

The count, min, max, hue, & chroma variables are being passed to a Svelte component via Storybook's controls.

h and c remain static for each generated range, and l values are being generated from min to max via the easing library, after which all of the resulting color objects are passed through my wrapper which converts the unit interval values to culori values and then runs them through a similar color conversion process as initially detailed in this issue, finally applying the generated hex values to the HTML elements.

@Enteleform
Copy link
Author

Enteleform commented Jul 24, 2021

Ok; I rewrote what I was doing in the previous demo in its most basic form, using only native culori functions. The issues are still apparent, and are actually more pronounced in other color spaces.

Also, the jch space has all of its values being reduced to black.

Findings

My initial test on the oklch space yielded the folowing results, where {x:hue, y:luminance}.

InitialTest

 
The main issues I'm noticing are:

  • decreases in luminance where there should be increases, particularly noticeable at [0.64, 0.2] and [0.75, 0.3].
  • inconsistent termination of the ranges as seen at [[0.24, 1.0], [0.25, 1.0]] and [[0.64, 1.0], [0.65, 1.0]], while [0.0, 1.0] and [0.75, 1.0] exhibit values in accordance with expectations.

 
Another thing I noticed is that in addition to L deviations, there seems to be some variance in the resulting H values:

[                             // {x:0.0}
  {l:0.13, c:0.05, h:29.22 }, // #170000
  {l:0.07, c:0.03, h:0.67  }, // #040001
  {l:0.17, c:0.07, h:1.14  }, // #25000E
  {l:0.28, c:0.11, h:0.73  }, // #4F0025
  {l:0.38, c:0.15, h:0.39  }, // #7D003F
  {l:0.49, c:0.2,  h:0.22  }, // #AE005A
  {l:0.59, c:0.24, h:0.11  }, // #E20077
  {l:0.69, c:0.22, h:0.11  }, // #FF4D95
  {l:0.78, c:0.14, h:0.16  }, // #FF8FB3
  {l:0.87, c:0.07, h:0.16  }, // #FFC2D3
  {l:0.97, c:0.02, h:359.89}, // #FFF0F3
]

I then decided to write a more exhaustive test for all of the cyclic color spaces, of which I took some snapshots while increasing the chroma value to see if it revealed any relationships/patterns in the issue. I also exceeded the valid range of colors (L * 1.0) for the sake of further observing said patterns.

Below are some animations I put together with the snapshots, both within valid range & exceeding valid range.

The only animation with a linear time : L scale is the first one. The others have progressively increasing chroma intervals to avoid excessive effort & rendering time.

{L:[0-1]/11, withinRange:true}

L-11 (ValidRange)

{L:[0-1]/11, withinRange:false}

L-11 (ValidRange + ExtendedRange)

{L:[0-1]/201, withinRange:true}

L-201 (ValidRange)

{L:[0-1]/201, withinRange:false}

L-201 (ValidRange + ExtendedRange)

Resources

Files

Code

Initial Test, oklch Only
import {oklch, formatHex, clampChroma} from "culori"


const max = {l:1, c:0.322, h:360}

const c  = 1
const ls = [0, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1]
const hs = [0.00, 0.24, 0.25, 0.64, 0.65, 0.75] // chosen for demonstration of specific issues

const colors = 
  hs.map(h => (
    ls
      .map(l     => ({l:(l*max.l), c:(c*max.c), h:(h*max.h)}))
      .map(color => formatHex(clampChroma(oklch(color)))     )
  ))
Extended Test, All Cyclic Color Spaces
import * as Culori              from "culori"
import {clampChroma, formatHex} from "culori"


function UnitIntervalRange(count:number)
  {return new Array(count).fill(0).map((x, i) => i * (1/(count-1)))}

const h_Count = 201
const l_Count = 11

const c        = 1
const h_Values = UnitIntervalRange(h_Count)
const l_Values = UnitIntervalRange(l_Count)

const data = [
  {colorSpace:"cubehelix", max:{h:360, c:4.6143,  l:1    }, h_Offset:-30, key:{c:"s", l:"l"}},
  {colorSpace:"dlch",      max:{h:360, c:51.484,  l:100  }, h_Offset:0,   key:{c:"c", l:"l"}},
  {colorSpace:"hsi",       max:{h:360, c:1,       l:1    }, h_Offset:-30, key:{c:"s", l:"i"}},
  {colorSpace:"hsl",       max:{h:360, c:1,       l:1    }, h_Offset:-30, key:{c:"s", l:"l"}},
  {colorSpace:"hsv",       max:{h:360, c:1,       l:1    }, h_Offset:-30, key:{c:"s", l:"v"}},
  {colorSpace:"jch",       max:{h:360, c:0.190,   l:0.221}, h_Offset:0,   key:{c:"c", l:"l"}},
  {colorSpace:"lch",       max:{h:360, c:131.008, l:100  }, h_Offset:0,   key:{c:"c", l:"l"}},
  {colorSpace:"lch65",     max:{h:360, c:133.807, l:100  }, h_Offset:0,   key:{c:"c", l:"l"}},
  {colorSpace:"lchuv",     max:{h:360, c:176.609, l:100  }, h_Offset:0,   key:{c:"c", l:"l"}},
  {colorSpace:"oklch",     max:{h:360, c:0.322,   l:1    }, h_Offset:0,   key:{c:"c", l:"l"}},
]

const colors: {colorSpace:string, rows:string[][]}[] = []

data.forEach( ({colorSpace, max, h_Offset, key}) => {
  const convert = Culori[colorSpace]
  const rows: string[][] = []
  colors.push({colorSpace, rows})

  h_Values.forEach( (h) => {
    const row: string[] = []
    rows.push(row)

    l_Values.forEach( (l) => {
      const color = {
        h:       ((h * max.h) + h_Offset),
        [key.c]: ((c * max.c)           ),
        [key.l]: ((l * max.l)           ),
      }

      let converted = formatHex(clampChroma(convert(color)))
      row.push(converted)
    })
  })
})

@danburzo danburzo reopened this Jul 25, 2021
@danburzo
Copy link
Collaborator

Wow, thank you for the detailed troubleshooting! I'll look carefully into the data tomorrow morning to get to the root(s) of the issues. In the meantime I have started to make some changes in this PR:

  • Allow clampChroma to be performed in other color spaces containing a Chroma dimension
  • Check that the conversion from sRGB to { LCh, OKLCh, JzCzHz } works as expected when starting with an out-of-gamut color (e.g. one of the r, g, b channels is negative).

@Enteleform
Copy link
Author

@danburzo

Awesome, I'll try out the changes when I get a chance.

Currently building out my testing sandbox a bit. Made it a bit more dynamic by exposing all of the relevant variables as Storybook controls, generalized the conversion process to allow testing of arbitrary libraries, and added some options to manipulate the layout to get different perspectives.

Still got a bit of work to do, but here are a few screenshots of its current state. Don't really have a proper git/hosting/sharing workflow down yet, but lmk if you're interested in getting any particular state/data captures we can either do a screen share or I can maybe figure out the best way to share a functioning version. (I'm kinda mid-process of building out a personal stack, part of which is dynamic theming - thus the quest for perceptually uniform color generation. New to Svelte/Vite/Storybook/Tailwind/etc., so everything is only partially set up.)

chrome_2021-07-25_14-29-10
chrome_2021-07-25_14-29-23
chrome_2021-07-25_14-29-27
chrome_2021-07-25_14-29-34
chrome_2021-07-25_14-39-09

@danburzo
Copy link
Collaborator

danburzo commented Jul 25, 2021

I'm currently working through the data you provided. The color table you provided with chromaClamp(oklch()):

Screenshot 2021-07-25 at 21 40 57

This converts the colors to lch, then tries to find the closest chroma that's displayable while maintaining the lch hue constant, which doesn't necessarily result in the oklch hue to remain constant. The fact that oklch has its own Chroma and Hue dimensions is incidental — nonetheless it may become confusing.

When doing clampChroma in the color's original color space, clampChroma(oklch(...), 'oklch') we get rid of the hue drift, and the Lightness increases monotonically:

Screenshot 2021-07-25 at 21 41 08

Now, a remaining problem in both tables is the row corresponding to the OKLCh Lightness = 1. This is due to a quirk/side-effect in OKLab, in that sRGB white has L: 0.9999882345920056, not L: 1. So any OKLCh color with L: 1 will be too bright to be displayable in sRGB on any Chroma (even 0), so clampChroma falls back to clampRgb, and the row appears out of order with the others. I should probably update the OKLab / OKLCh Lightness range on the Color Spaces to be more accurate. (Also, there are other ways to clamp out-of-gamut colors that may be better for this particular situation)

@Enteleform
Copy link
Author

Enteleform commented Jul 26, 2021

@danburzo Hey, just built the latest repo state & tried clampChroma(oklch(...), 'oklch'), but I'm still getting similar results.
Edit: nevermind, this turned out to be a dependency conflict with yarn installing from specific commits.

Just getting started for the day, down to chat if you're available. Email me @ [email protected] if you want to exchange info & get something set up on Discord/Slack/Zoom/etc.

Image

chrome_2021-07-26_05-45-54

@Enteleform
Copy link
Author

Here are the latest results:

chrome_2021-07-26_06-14-30
chrome_2021-07-26_06-14-40

@danburzo
Copy link
Collaborator

I've wrote an interactive notebook to explore clampChroma and it seems that the algorithm works as well as it can work when the color's space corresponds to the space where we do the chroma search. Here's a video of oklch will the slider increasing the chroma (the dots mark out-of-gamut colors):

oklch.mp4

(As mentioned in a previous comment, the chroma-clamping algorithm is not necessarily the best from a perceptual point of view)

When the color's space and the chroma-finding space are different, we see a bunch of chromatic aberrations — probably due to the intermediary rgb representation containing out-of-gamut values — that warrant further investigation at some point. Here's clampChroma(oklch(...), 'lch') for a high chroma:

Screenshot 2021-07-26 at 15 26 21

danburzo added a commit that referenced this issue Jul 26, 2021
@danburzo
Copy link
Collaborator

The color aberrations should be improved with the latest changes on main.

@Enteleform
Copy link
Author

Looks good, definite improvement!

I am getting some deviation at the very end of the high-L range of a few spaces, but I only applied the more precise {L:0.999...} value that you mentioned to oklch because I wasn't sure if it also applied to other spaces.

Could you provide the non-truncated values for all spaces? Might be handy for some users to have access to them via culorijs.org/color-spaces. Maybe via a PopOver/Tooltip or something if you don't want to cruft up the UI with numbers. Or maybe as a separate data page with direct access to min/max values without all the other documentation.

It turns out that the jch no-values issue from before was just due to an error in the documentation. The j parameter is currently documented as l.

Also, I'm still wondering:

  • If the mode parameter is necessary or if it could just be referenced from the color automatically.
    • Is there any reason why a user might need to do something like clampChroma({...otherValues, mode:"x"}, "y")?
  • How should the spaces that are still getting distorted be handled? Should they just be passed through clampChroma without any modification? Or is there some other expected outcome that should be implemented? According to the docs, it seemingly accepts any color object without limitation, so I would expect that it will either perform clamping when possible or just pass the color through unaffected.

jch Error @ Docs

image

Conversion Values

image

No Clamp

chrome_2021-07-26_14-13-11

clampChroma(color)

chrome_2021-07-26_14-14-09

clampChroma(color, mode)

chrome_2021-07-26_14-13-33

@danburzo
Copy link
Collaborator

danburzo commented Jul 26, 2021

Could you provide the non-truncated values for all spaces? Might be handy for some users to have access to them via culorijs.org/color-spaces. Maybe via a PopOver/Tooltip or something if you don't want to cruft up the UI with numbers. Or maybe as a separate data page with direct access to min/max values without all the other documentation.

The ranges are available programatically with culori.getModeDefinition(mode).ranges (currently undocumented), with the caveat that the definition object might see a few changes before the stable 1.x release. (Any breaking change will be documented in the release notes). The ranges are approximate because they're not generated analytically, they're just min-max values found by a three-nested-for-loop routine that tries several sRGB values.

It turns out that the jch no-values issue from before was just due to an error in the documentation. The j parameter is currently documented as l.

Oops, good catch! That's a typo.

If the mode parameter is necessary or if it could just be referenced from the color automatically. Is there any reason why a user might need to do something like clampChroma({...otherValues, mode:"x"}, "y")?

clampChroma can only work in a mode that has a Chroma dimension. Some modes, such as hsl, hsv, etc. don't apply, so we need some sort of fallback. With the current API, you can do something like:

culori.clampChroma(color, color.c !== undefined ? color.mode : 'lch');

Without the second argument, lch would have to be hard-coded inside clampChroma and unchangeable.

How should the spaces that are still getting distorted be handled? Should they just be passed through clampChroma without any modification? Or is there some other expected outcome that should be implemented? According to the docs, it seemingly accepts any color object without limitation, so I would expect that it will either perform clamping when possible or just pass the color through unaffected.

Technically, when clampChroma can't find a displayable color, it falls back to clampRgb, so you should see an useful result on any color. From your screenshots, I'm seeing clampChroma(color) is still exhibiting some issues with jch and lchuv — which needs to be sorted out — but otherwise it looks like it's doing a decent job.

@Enteleform
Copy link
Author

Enteleform commented Jul 26, 2021

The ranges are available programatically with culori.getModeDefinition(mode).ranges

Those seem to match up with the values I had already entered from the docs. However, getModeDefinition("lchuv").ranges returned {c:[0, 131.008]} rather than the documented 176.609.

Other than that; I figured that maybe there was some discrepancy between what I thought were truncated values VS some more precise values that were actually in the code, which was potentially causing this issue:

image

@danburzo
Copy link
Collaborator

The ranges in the definition files / color-spaces page were a bit neglected because they were mostly informative. I've redone some computation and updated docs/definition files accordingly in: b3af2b5

As the code stands right now on main, clampChroma(color, color.c !== undefined ? color.mode : 'lch') should look pretty good. Some combinations (for example clampChroma(jch(), 'lch')) are still problematic for non-obvious reasons.

@Enteleform
Copy link
Author

Enteleform commented Jul 26, 2021

Oh nice, that resolved a weird issue I was having with the jch space where I couldn't get a perfect red hue by applying an offset to H. For some reason it was making an abrupt jump between [high-blue, low-green] and [low-blue, high-green] before I updated the script with the new values.

Now all the colorspaces are perfectly lined up in the sandbox 😁👍

image

@Enteleform
Copy link
Author

Enteleform commented Jul 26, 2021

Here are the full spaces, still getting that drop in L at the end of some of them.

(also added "lch" as mode for the non-C spaces as shown in your example)

image

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

No branches or pull requests

2 participants