w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.47k stars 658 forks source link

[css-color-4] Gamut Mapping Clarifications #9651

Closed jamesnw closed 10 months ago

jamesnw commented 10 months ago

In implementing the Gamut Mapping Algorithm in Color.js, I found a few areas where clarification in the pseudocode could be helpful. If any of these are too pedantic for pseudocode, feel free to ignore.

  1. In step 1, replace return |origin| with convert |origin| to |destination| and return it as the gamut mapped color. The explicit conversion is the important bit, the "as the gamut mapped color" is for parity with the other returns.
  2. In step 10, defining clip, should we change it to specify it clamps to the range of the color channel, instead of 0-1? This may be a moot point, as it appears that only unbounded spaces which don't go through this portion have channel ranges besides 0-1.
  3. In steps 3 and 4, where we return white/black, should we specify white/black instead of 1's and 0's? Like # 2, this may be a moot point, as only unbounded spaces appear to have other channel values for white/black.
  4. Nit: In step 10, defining clip, there's an extra pipe character at the end of clip(color)|
svgeesus commented 10 months ago
  1. In step 1, replace return |origin| with convert |origin| to |destination| and return it as the gamut mapped color. The explicit conversion is the important bit, the "as the gamut mapped color" is for parity with the other returns.

Yes, the algorithm should consistently return the gamut mapped color in the destination color space, regardless of the particular return point within the algorithm.

svgeesus commented 10 months ago

Changes to step 1 are now done

svgeesus commented 10 months ago

2. In step 10, defining clip, should we change it to specify it clamps to the range of the color channel, instead of 0-1? This may be a moot point, as it appears that only unbounded spaces which don't go through this portion have channel ranges besides 0-1.

I notice that the introductory text about the scope and applicability of the CSS GMA is in an inconsistent state. It started off as being widely applicable (for example, it used to be required before conversion to HSL and HWB). Explanatory notes were added to say that a different algorithm would be required:

and so the section title describes the primary use case: 13.2. CSS Gamut Mapping to an RGB Destination

Despite that, the pseudo-code is written to assume an arbitrary (colorimetric) destination color space, and the first line of the pseudo-code says what to do if destination is unbounded, such as XYZ-D65, XYZ-D50, Lab, LCH, Oklab, Oklch.

And so, as you point out, what to do if destination is bounded but the coordinate limits of in-gamut colors are not [0,1]. The suggestion to return [1 1 1 origin.alpha] was, I believe, first made here and a little later, here and in both cases I think an RGB color space whose gamut bounds are [0, 1] was assumed.

In practice, for the color spaces defined in CSS Color 4, I think that is all that is left once (XYZ-D65, XYZ-D50, Lab, LCH, Oklab, Oklch) is excluded.

Looking now (to check for upwards compatibility) at CSS Color HDR, then

Looking too at what is in Color.js,

I think that there is value in keeping the wider generality of the CSS GMA, which does mean that continuing to specify [1,1,1,origin.alpha] will work for the "gamut map to RGB display" case, but is sometimes going to be incorrect in the wider case.

svgeesus commented 10 months ago

It seems that

if the Lightness of |origin_Oklch| is greater than or equal to 100%,
convert `oklab(1 0 0 / origin.alpha)` to |destination| and return it as the gamut mapped color

(and oklab(1 0 0 / origin.alpha) for "less than than or equal to 0%")

would be clear, and correct in all these cases.

oklab(1 0 0) oklab(0 0 0)

(Note to self, check why Color.js claims color(acescc -0.3584 -0.3584 -0.3584) is gamut mapped); bounds rounded off?

svgeesus commented 10 months ago

@jamesnw @weinig @mysteryDate @facelessuser @LeaVerou any comments on returning oklab(1 0 0 / origin.alpha) and oklab(0 0 0 / origin.alpha) respectively?

facelessuser commented 10 months ago

any comments on returning oklab(1 0 0 / origin.alpha) and oklab(0 0 0 / origin.alpha) respectively?

Only if Oklab is using a corrected matrix where oklab(1 0 0 / origin.alpha) is equivalent to color(srgb 1 1 1 / origin.alpha). Currently, that is not the case in the spec.

svgeesus commented 10 months ago

Thanks for the nudge, corrected

mysteryDate commented 10 months ago

any comments on returning oklab(1 0 0 / origin.alpha) and oklab(0 0 0 / origin.alpha) respectively?

Only if Oklab is using a corrected matrix where oklab(1 0 0 / origin.alpha) is equivalent to color(srgb 1 1 1 / origin.alpha). Currently, that is not the case in the spec.

Does this change the values for all oklab colors? Or is the adjustment small enough that it mostly will look like rounding errors?

facelessuser commented 10 months ago

They should be the same up to ~32 bit precision. Changes in the matrix are mainly out in the 64 bit portion.

You can see, the inverse matrix before just had garbage in the 64bit portion:

const LabtoLMS_M = [
    [ 0.99999999845051981432,  0.39633779217376785678,   0.21580375806075880339  ],
    [ 1.0000000088817607767,  -0.1055613423236563494,   -0.063854174771705903402 ],
    [ 1.0000000546724109177,  -0.089484182094965759684, -1.2914855378640917399   ]
];

Now the new matrix will give you a proper LMS of [1, 1, 1] for oklab(1 0 0) which matches the the XYZ to LMS portion of the transform. Before, this was off for 64bit values, which is why all achromatics had garbage in the 64bit part.

const LabtoLMS_M = [
    [ 1.0000000000000000,  0.3963377773761749,  0.2158037573099136 ],
    [ 1.0000000000000000, -0.1055613458156586, -0.0638541728258133 ],
    [ 1.0000000000000000, -0.0894841775298119, -1.2914855480194092 ]
];
svgeesus commented 10 months ago

Or is the adjustment small enough that it mostly will look like rounding errors?

The adjustment simply reduces the rounding errors. It is way below the level which would produce a visible difference in a single color conversion.

svgeesus commented 10 months ago

2. In step 10, defining clip, should we change it to specify it clamps to the range of the color channel, instead of 0-1? This may be a moot point, as it appears that only unbounded spaces which don't go through this portion have channel ranges besides 0-1.

On the one hand, the current text is very precise:

<li>let clip(|color|) be a function which converts |color| to |destination|,
    converts all negative components to zero,
    converts all components greater that one to one,
    and returns the result

On the other hand, it could be expanded to cover a destination which is not bounded [0,1] like this:

<li>let clip(|color|) be a function which converts |color| to |destination|,
    converts all components less than the lower bound of the reference range to the lower bound,
    converts all components greater than the upper bound of the reference range to the upper bound,
    and returns the result

Does that seem clear and understandable?

jamesnw commented 10 months ago

I think that is clear and understandable. One potential alternative would be-

<li>let clip(|color|) be a function which converts |color| to |destination|,
    clamps each component to the bounds of the reference range for that component
    and returns the result

Or is this functionally different from clamping?

Potential nits that I don't think warrant changes, and are fairly unlikely to cause confusion, but are here for documentation-

converts all components less than the lower bound of the reference range for the component to the lower bound,
svgeesus commented 10 months ago

Closed by https://github.com/w3c/csswg-drafts/commit/5d806c11651ebab3a97815a29e1965f479d6c69e