[css-color-4][css-color-5] rgb-to-hwb algorithm disagrees indirectly with relative-color-out-of-gamut and color-mix-out-of-gamut expectations

[squelart]

Bias warning: I've made the title as neutral as possible, but I personally think the specs are at fault and the tests are correct (though additional tests would be welcome).
Note that I'm not an expert in colorspaces and CSS and specs, so my bias may be wrong; also I may use incorrect terminology below.

tl;dr: The rgb-to-hwb algorithm uses rgb-to-hsl as-is, which produces a rotated hue for some out-of-gamut colors with negative saturation, while some relative-color tests like hwb(from lab(100 104.3 -50.9) h w b) (if they internally convert via rgb) expect non-rotated hue.
I will argue that the function definition in https://drafts.csswg.org/css-color-4/#rgb-to-hwb needs to be tweaked.

First, about the "indirect" tests: Safari fails tests like hwb(from lab(100 104.3 -50.9) h w b) because internally it converts from lab to rgb first, before using the rgb-to-hwb algorithm to compute the final hwb color.
However, I think this internal implementation converting via rgb is not at issue here, because we could add another simple test case that goes directly from rgb to hwb, e.g.:

  fuzzy_test_computed_color(
    `hwb(from color(srgb 1.59343 0.58802 1.40564) h w b / alpha)`,
    `color(srgb 1.59343 0.58802 1.40564)`);

This also fails in Safari with an actual output color(srgb 0.587758 1.593503 0.775713).

Next, about the test expectations.
Focusing on this relative-color-out-of-gamut.html test case:

  fuzzy_test_computed_color(
    `hwb(from lab(100 104.3 -50.9) h w b)`,
    `color(srgb 1.59343 0.58802 1.40564)`);

I believe the expectation from lab to srgb is correct, because I can reproduce it in a number of ways:

• Using the macOS ColorSync Utility.
• On https://www.mathworks.com/help/images/ref/lab2rgb.html
• On https://www.colorspaceconverter.com/converter/lab-to-rgb , https://colordesigner.io/convert/labtorgb , https://www.colormine.org/convert/lab-to-rgb#google_vignette , https://products.aspose.app/svg/color-converter/lab-to-rgb#google_vignette (all clipped, but tending towards the expected extra-red half-green extra-blue color.)

Assuming the test expectations are correct, I'll now focus on the rgb-to-hwb conversion:
https://drafts.csswg.org/css-color-4/#rgb-to-hwb defines the rgbToHwb function, and its first step is to call rgbToHsl.
https://drafts.csswg.org/css-color-4/#rgb-to-hsl defines rgbToHsl. Just above it there's this note:

Special care is taken to deal with intermediate negative values of saturation, which can be produced by colors far outside the sRGB gamut.

And in the rgbToHsl function definition this is reflected in this bit of code:

    // Very out of gamut colors can produce negative saturation
    // If so, just rotate the hue by 180 and use a positive saturation
    // see https://github.com/w3c/csswg-drafts/issues/9222
    if (sat < 0) {
        hue += 180;
        sat = Math.abs(sat);
    }

Notice how the hue is rotated 180 degrees, and the saturation is negated. And it probably makes sense to normalize HSL colors that way.

However, back in rgbToHwb, only the resulting hue component is kept, the saturation and lightness are simply ignored.
I believe that this is where things go wrong, because rgbToHsl could have flipped the hue and negated the saturation, but we only use that flipped hue and have lost the corresponding information stored in the saturation.
And this explains why we get something like rgb(0.6, 1.6, 0.8), which seems to have an opposite hue to the expected rgb(1.6, 0.6, 1.4).

My proposed solution would be to compute the hue in a similar way, except that we wouldn't need to compute the saturation (nor lightness), and the hue would never be rotated/flipped.

To illustrate this, I've written this codepen: https://codepen.io/squelart/pen/MWMoeoB
It uses the function definitions straight from https://drafts.csswg.org/css-color-4 , and tests rgb -> hwb -> rgb.
We can notice how our hero test color doesn't survive the round trip. I've added a few test cases with a decreasing red component, it looks like there's a cut-off between 1.4 and 1.5. [edit: I've added more output to the codepen, cut-off around red>1.41198, with the HSL saturation dipping into big negative numbers.]

In the codepen I've also added a modified rgb-to-hwb function that implements my suggested rgb-to-hue sub-function (which derives from rgb-to-hsl). And this one shows a working round trip.
(And I'm working on a similar fix in WebKit: WebKit/WebKit#31636 , but I'll wait for this discussion here to be resolved first.)

In conclusion:

• I believe the function definition in https://drafts.csswg.org/css-color-4/#rgb-to-hwb needs to be tweaked to avoid flipping the hue.
• More out-of-gamut test cases (related to css-colors-5) could be useful, to cover more colorspace pairs, and more colors.

To be complete: There's the possibility that my argument is incorrect, and in fact the css-color-4 conversion functions should be taken as authoritative and correct, in which case the test expectations should be updated.

[nt1m]

cc @mysteryDate @weinig @svgeesus @tiaanl

[squelart]

As seen on https://codepen.io/squelart/pen/MWMoeoB , the resulting HWB color has a negative blackness, so #10368 may be related, but it seems more focused on hwb-to-rgb.

[facelessuser]

Since the hue in HWB is used without saturation context, you definitely don't want the corrected hue because whiteness and blackness are not calculated with the adjusted saturation but are calculated directly from the sRGB values. I would agree that this is a bug to use the adjusted hue in HWB. This is a good catch.

[romainmenke]

I also think you analysis is correct :)

The flipping of the hue was introduced only for hsl if I recall correctly.
It was discussed here: #9222
As far as I can see hwb was not mentioned there.

So I think it was an oversight that the algorithm was changed without also considering the effects it would have on conversions to hwb.

I've tested your suggested fix and it seems fine to me.

---

@squelart This variant is based on the WebKit code, after your proposed changes would be applied

/**
 * @param {number} red - Red component 0..1
 * @param {number} green - Green component 0..1
 * @param {number} blue - Blue component 0..1
 * @return {number[]} Array of HWB values: Hue as degrees 0..360, Whiteness and Blackness in reference range [0,100]
 */
function rgbToHwb(red, green, blue) {
    var white = Math.min(red, green, blue);
    var black = 1 - Math.max(red, green, blue);
    return([rgbToHue(red, green, blue), white*100, black*100]);
}

/**
 * @param {number} red - Red component 0..1
 * @param {number} green - Green component 0..1
 * @param {number} blue - Blue component 0..1
 * @return {number} Hue as degrees 0..360
 */
function rgbToHue(red, green, blue) {
	const max = Math.max(red, green, blue);
	const min = Math.min(red, green, blue);
	let hue = NaN;
	const d = max - min;

	if (d !== 0) {
		switch (max) {
			case red: hue = ((green - blue) / d); break;
			case green: hue = (blue - red) / d + 2; break;
			case blue: hue = (red - green) / d + 4;
		}

		hue = hue * 60;
	}

	if (hue >= 360) {
		hue -= 360;
	} else if (hue < 0) {
		hue += 360;
	}

	return hue;
}

[squelart]

Thank you @facelessuser and @romainmenke . Is that enough agreement here to proceed?

What's the process for updating the specs? (I'd be fine if someone wanted to go ahead and do it, otherwise I can have a go if appropriate.)

And any thoughts on adding more coverage in out-of-gamut tests? Any process to follow for that?

[facelessuser]

As the person who suggested the hue flip for HSL in the first place, I can confidently say HWB should not flip the hue unless whiteness and blackness account for it, and since there is not currently a way to do so, the hue should not be flipped for HWB. So, I would say a change for HWB needs to be made to avoid the hue flip for good round tripping. I would be surprised if there was push back.

[svgeesus]

However, back in rgbToHwb, only the resulting hue component is kept, the saturation and lightness are simply ignored.
I believe that this is where things go wrong, because rgbToHsl could have flipped the hue and negated the saturation, but we only use that flipped hue and have lost the corresponding information stored in the saturation.

Yes, spot on. I agree that HWB should not flip the hue.

[svgeesus]

Starting from CSS Color 4 rgbToHSL and removing the negative saturation check, to create an rgbToHue, I end up with

case red:   hue = (green - blue) / d + (green < blue ? 6 : 0); break;

while the codepen from @squelart has

case red:   hue = (green - blue) / d; break;

[squelart]

Good eye @svgeesus !
I actually implemented my proposed fix before diving deeper into root causes, and followed what another correct-looking implementation was doing. It didn't have this + (green < blue ? 6 : 0) trick, but instead a later catch-all if (hue < 0) { hue += 360; }, which seemed a bit clearer, and safer to me because it could catch other potential negative hues from other branches.
But if you think that this rgbToHsl-derived code is 100% safe and preferable, I'd be happy to go with that optimization.

Back to my rambling about tests, it'd be nice to have more test cases that would cover all possible code paths (in both versions), to ensure that this optimization doesn't break anything.

[svgeesus]

which seemed a bit clearer, and safer to me because it could catch other potential negative hues from other branches.

Thanks for the explanation. I agree it is clearer to simply test for hues greater than 360.

[squelart]

My first PR! #10718
I can't add reviewers or set labels myself, hopefully people here can help. (And of course please let me know if I've done anything incorrectly.)

[svgeesus]

closed by ead8668