wpt icon indicating copy to clipboard operation
wpt copied to clipboard

[css-color] Tests assume old hsl()/hsla() clipping behavior or depend on unclear gamut mapping to sRGB

Open weinig opened this issue 10 months ago • 10 comments

There are few tests that assume the old hsl()/hsla() clipping behavior (specifically, clipping saturation values greater than 100% to 100%):

css/css-color/t424-hsl-clip-outside-gamut-b.xht css/css-color/t425-hsla-clip-outside-device-gamut-b.xht

html/canvas/element/fill-and-stroke-styles/2d.fillStyle.parse.hsl-clamp-1.html html/canvas/element/fill-and-stroke-styles/2d.fillStyle.parse.hsla-clamp-1.html html/canvas/offscreen/fill-and-stroke-styles/2d.fillStyle.parse.hsl-clamp-1.html html/canvas/offscreen/fill-and-stroke-styles/2d.fillStyle.parse.hsl-clamp-1.worker.html html/canvas/offscreen/fill-and-stroke-styles/2d.fillStyle.parse.hsla-clamp-1.html html/canvas/offscreen/fill-and-stroke-styles/2d.fillStyle.parse.hsla-clamp-1.worker.html

The tests will pass if the host is running with an sRGB display with some gamut mappings, but it doesn't seem like that is something the tests should be depending on.

weinig avatar Apr 20 '24 16:04 weinig

@mysteryDate

nt1m avatar Apr 21 '24 07:04 nt1m

cc @tiaanl @svgeesus @mdubet as well

nt1m avatar Apr 23 '24 05:04 nt1m

Yes, I'd be pro deleting them as the debate around gamut mapping is still swirling, though clipping like this is almost certainly not what we want to be doing.

They're generated here: https://github.com/web-platform-tests/wpt/blob/21bc8079a5e0013c7f72d7629ccc18d20740e2d0/html/canvas/tools/yaml/element/meta.yaml#L393

mysteryDate avatar Apr 24 '24 16:04 mysteryDate

Agreed, until we're clear on what exactly to expect from colors falling outside of some gamut we should avoid testing for it.

tiaanl avatar Apr 24 '24 22:04 tiaanl

These tests are based on two things in CSS Color 3, section 4.2.4 which apply to both HSL and HSLA:

If saturation is less than 0%, implementations must clip it to 0%.

This was discussed in the CSS Color 4 context:

  • https://github.com/w3c/csswg-drafts/issues/9222

and as a result, for interop, modern syntax HSL and HSLA also require clipping negative saturation.

However the paragraph continues:

If the resulting value is outside the device gamut, implementations must clip it to the device gamut. This clipping should preserve the hue when possible, but is otherwise undefined. (In other words, the clipping is different from applying the rules for clipping of RGB colors after applying the algorithm below for converting HSL to RGB.)

On the face of it this is tautologous: a display is not required to produce colors it can't produce. The mechanism is intentionally unspecified, so the actual result can't be directly tested; in practice implementations of Color 3 clipped each RGB component to [0, 255] and also assumed all displays were sRGB; and that is what the WPT assume and test for. Which is incorrect. The tests note this explicitly:

<p><strong>WARNING: This test assumes that the device gamut is sRGB
	(as it will be for many CRT monitors).</strong></p>

In terms of what do do about it, in my view subtests that check for negative saturation becoming 0 should be kept because this is also in CSS Color 4:

For historical reasons, if the saturation is less than 0% it is clamped to 0% at parsed-value time, before being converted to an sRGB color.

and all the subtests which assume an sRGB display and assume a particular method to force colors into the display gamut should be deleted, as the current tests go beyond what is specified.

These subtests contradict the resolution from

  • https://github.com/w3c/csswg-drafts/issues/8444

specifically, if a Display P3 color gets converted to HSL then it now round trips, and should be displayed as that color not forced into sRGB on a P3 display.

svgeesus avatar Apr 25 '24 17:04 svgeesus

Also, the slimmed-down tests

css/css-color/t424-hsl-clip-outside-gamut-b.xht css/css-color/t425-hsla-clip-outside-device-gamut-b.xht

should be renamed

css/css-color/hsl-clamp-negative-saturation.html css/css-color/hsla-clamp-negative-saturation.html

and should link to the relevant clause in CSS Color 4, which replaces CSS Color 3 according to the CSS Snapshot.

svgeesus avatar Apr 25 '24 17:04 svgeesus

They're generated here:

https://github.com/web-platform-tests/wpt/blob/21bc8079a5e0013c7f72d7629ccc18d20740e2d0/html/canvas/tools/yaml/element/meta.yaml#L393

Looks like those tests all check for saturation greater than 100 (so, are wrong) and there are no tests for clamping negative saturation. So an easy fix would be to change that generator to use a negative saturation, and check the rgb value comes from hsl with a zero saturation.

svgeesus avatar Apr 25 '24 18:04 svgeesus

We do have some modern-syntax CSS tests for clamping negative HSL saturation, because of this change

svgeesus avatar Apr 25 '24 18:04 svgeesus

I put up a PR for the ones discussed so far.

In addition though, there are

css/css-color/lch-009.html css/css-color/lch-010.html css/css-color/oklch-009.html css/css-color/oklch-010.html

which depend on gamut mapping always mapping lightness levels of 100% always mapping to srgb white, and lightness levels of 0% always mapping to srgb black.

weinig avatar Apr 27 '24 18:04 weinig

@wenig thanks for fixing the WPT tests we discussed

In addition though, there are

css/css-color/lch-009.html css/css-color/lch-010.html css/css-color/oklch-009.html css/css-color/oklch-010.html

which depend on gamut mapping always mapping lightness levels of 100% always mapping to srgb white, and lightness levels of 0% always mapping to srgb black.

Currently in the spec we have

In Lab, the first argument specifies the CIE Lightness, L. This is a number between 0% or 0 and 100% or 100 Values less than 0% or 0 must be clamped to 0% at parsed-value time; values greater than 100% or 100 are clamped to 100% at parsed-value time.

and

If the lightness of a Lab color (after clamping) is 0%, or 100% the color will be displayed as black, or white, respectively due to gamut mapping to the display.

The second quoted part was changed to be that way because previously there was a discontinuity: 1.0 would give white while 0.9999 would not. Expressing it as a consequence of gamut mapping removed that discontinuity, while not relying on any details of the gamut mapping beyond lightness preservation. Just the natural consequence in standard dynamic range that media white is the brightest displayable color.

It isn't clear to me how to express that in a better way in the spec.

svgeesus avatar Aug 15 '24 10:08 svgeesus