[css-color-4] Update lab and lch tests to use rectangles #27202

weinig · 2021-01-15T18:22:19Z

When running the lab and lch test in WebKit, the use of text to try and match colors caused small differences in antialiasing causing the tests to fail. By changing the tests to use rectangles for comparisons, it is much easier to see that they are passing or failing and don't have these antialiasing issues.

css/css-color/lch-004-ref.html

svgeesus

I agree that solid rectangular patches of color are more accurate (the angle subtended is greater) and more testable (measuring the patch is possible) than text.

svgeesus

I see some lint errors, which although trivial will block mergin. Please fix them.

css/css-color/lab-006.html:11: Whitespace at EOL (TRAILING WHITESPACE)
css/css-color/lab-005.html:11: Whitespace at EOL (TRAILING WHITESPACE)
css/css-color/lch-006.html:11: Whitespace at EOL (TRAILING WHITESPACE)
css/css-color/lch-004.html:11: Whitespace at EOL (TRAILING WHITESPACE)
css/css-color/lab-004.html:11: Whitespace at EOL (TRAILING WHITESPACE)
css/css-color/lab-007.html:11: Whitespace at EOL (TRAILING WHITESPACE)

svgeesus

Overall this looks good. Since you were seeing small roundoff errors I recalculated all of the conversions and, while they were mostly correct to three decimal places, some showed truncation rather than rounding and a few showed cumulative roundoff error that affected the second or, in one case, the first decimal place.
So, sorry to be a pain but please add in these more accurate conversions, just so we can be sure roundoff is not causing an issue.

svgeesus · 2021-01-17T11:54:20Z

css/css-color/lab-001.html

 <meta name="assert" content="lab() with no alpha">
 <style>
-    .test {color: lab(46.277% -47.562 48.583)} /* green (sRGB #008000) converted to Lab */
+    .test { background-color: red; width: 12em; height: 12em; }
+    .test { background-color: lab(46.277% -47.562 48.583); } /* green (sRGB #008000) converted to Lab */


Checking that conversion, let's use a more precise one. The existing 3dp value has rounding error (two values truncated rather than rounded)

lab(46.2775% -47.5621 48.5837)

svgeesus · 2021-01-17T11:56:31Z

css/css-color/lab-004-ref.html

@@ -2,10 +2,9 @@
 <meta charset="utf-8">
 <title>CSS Color 4: Specifying Lab and LCH</title>
 <style>
-    .match { color: rgb(75.62% 30.45% 47.56%)} /* lab(50 50 0) converted to sRGB */
+    .test { background-color: rgb(75.62% 30.45% 47.56%); width: 12em; height: 12em; } /* lab(50% 50 0) converted to sRGB */


Checking again, let's use

rgb(75.6208% 30.4487% 47.5634%)

svgeesus · 2021-01-17T11:57:08Z

css/css-color/lab-004.html

-    .test { color: lab(50% 50 0)}
-    .match { color: rgb(75.62%, 30.45%, 47.56%)} /* lab(50 50 0) converted to sRGB */
+    .test { background-color: red; width: 12em; height: 6em; margin-top: 0; }
+    .ref { background-color: rgb(75.62%, 30.45%, 47.56%); width: 12em; height: 6em; margin-bottom: 0; }/* lab(50% 50 0) converted to sRGB */


and the same here,

rgb(75.6208% 30.4487% 47.5634%)

svgeesus · 2021-01-17T11:58:13Z

css/css-color/lab-005-ref.html

@@ -2,10 +2,9 @@
 <meta charset="utf-8">
 <title>CSS Color 4: Specifying Lab and LCH</title>
 <style>
-    .match { color: rgb(10.79%, 75.55%, 66.40%)} /* lab(70 -45 0) converted to sRGB */
+    .test { background-color: rgb(10.79%, 75.55%, 66.40%); width: 12em; height: 12em; } /* lab(70% -45 0) converted to sRGB */


recalculating,

rgb(10.7906% 75.5567% 66.3982%)

svgeesus · 2021-01-17T11:58:29Z

css/css-color/lab-005.html

-    .test { color: lab(70% -45 0)}
-    .match { color: rgb(10.79%, 75.55%, 66.40%)} /* lab(70 -45 0) converted to sRGB */
+    .test { background-color: red; width: 12em; height: 6em; margin-top: 0; }
+    .ref { background-color: rgb(10.79%, 75.55%, 66.40%); width: 12em; height: 6em; margin-bottom: 0; } /* lab(70% -45 0) converted to sRGB */


and here

rgb(10.7906% 75.5567% 66.3982%)

svgeesus · 2021-01-17T12:06:38Z

css/css-color/lch-005.html

-    .test { color: lch(70% 45 -180)}
-    .match { color: rgb(10.79%, 75.55%, 66.40%)} /* lch(70 45 180) converted to sRGB */
+    .test { background-color: red; width: 12em; height: 6em; margin-top: 0; }
+    .ref { background-color: rgb(10.79%, 75.55%, 66.40%); width: 12em; height: 6em; margin-bottom: 0; } /* lch(70% 45 180) converted to sRGB */


rgb(10.7906% 75.5567% 66.3982%)

svgeesus · 2021-01-17T12:07:24Z

css/css-color/lch-006-ref.html

+<meta charset="utf-8">
+<title>CSS Color 4: Specifying Lab and LCH</title>
+<style>
+    .test { background-color: rgb(76.62%, 66.36%, 5.58%); width: 12em; height: 12em; } /* lch(70% 70 90) converted to sRGB */


rgb(76.6254% 66.3607% 5.5775%)

svgeesus · 2021-01-17T12:07:42Z

css/css-color/lch-006.html

-    .test { color: lch(70% 70 90)}
-    .match { color: rgb(76.62%, 66.36%, 5.58%)} /* lch(70 70 90) converted to sRGB */
+    .test { background-color: red; width: 12em; height: 6em; margin-top: 0; }
+    .ref { background-color: rgb(76.62%, 66.36%, 5.58%); width: 12em; height: 6em; margin-bottom: 0; } /* lch(70% 70 90) converted to sRGB */


rgb(76.6254% 66.3607% 5.5775%)

svgeesus · 2021-01-17T12:08:44Z

css/css-color/lch-007-ref.html

+<meta charset="utf-8">
+<title>CSS Color 4: Specifying Lab and LCH</title>
+<style>
+    .test { background-color: rgb(12.8%, 53.1%, 92.5%); width: 12em; height: 12em; } /* lch(55% 60 270) converted to sRGB */


rgb(12.8128% 53.105% 92.7645%)

svgeesus · 2021-01-17T12:09:26Z

css/css-color/lch-007.html

-    .test { color: lch(56% 58 275)}
-    .match { color: rgb(12.81%, 53.10%, 92.76%)} /* lch(56 58 275) converted to sRGB */
+    .test { background-color: red; width: 12em; height: 6em; margin-top: 0; }
+    .ref { background-color: rgb(12.8%, 53.1%, 92.5%); width: 12em; height: 6em; margin-bottom: 0; } /* lch(55% 60 270) converted to sRGB */


rgb(12.8128% 53.105% 92.7645%)

weinig · 2021-01-17T18:29:41Z

Will do. How do you feel about updating these to use color(srgb ...) rather than rgb(...)?

At least for WebKit, that puts us down a more consistent path of using floats for everything, and will also work for tests of display-p3 and so on.

dbaron · 2021-01-17T18:38:10Z

Wouldn't that make the tests no longer work in a bunch of implementations that do support lab() and lch()? It seems like it might make sense to make a duplicate set of tests with that comparison, but replacing the existing ones seems suboptimal.

weinig · 2021-01-17T21:39:05Z

Wouldn't that make the tests no longer work in a bunch of implementations that do support lab() and lch()? It seems like it might make sense to make a duplicate set of tests with that comparison, but replacing the existing ones seems suboptimal.

Are there implementations that support lab() and lch() but not color(srgb ...)?

dbaron · 2021-01-17T21:52:48Z

Er, sorry, maybe not... I think I was getting things mixed up. (Though I'm not saying there aren't, either...)

But it wouldn't surprise me if there ended up being some in the future.

weinig · 2021-01-17T22:16:47Z

Fair enough, let's stick with rgb() since it doesn't seem to be a problem currently (though I haven't tried to see if things still pass with the extended precision values yet, will report back once I do).

That said, I want to try to articulate the benefit (for the WebKit implementation at least) of using color(srgb ...) as the reference rather than rgb(...) again, since I don't think I was very clear and perhaps this will lead to more specificity. I don't want anything I write below to be seen as things I think should be specified, just trying to document the current state of things.

Right now, WebKit has two ways of storing colors, either as a set of 4 8-bit integers (rgba-8888 style) or as a set of 4 32-bit floats (rgba-FFFF style). The decision on which format to use is currently entirely based on how the color was specified, with rgba(), rgb(), hsl(), hsla() and the name and hex notations all using the rgba-8888 format, where as color(srgb ...) uses the rgba-FFFF format.

This is observable in two ways.

The first way it is observable is via serialization. Colors stored as rgba-8888 will be serialized using the rgba() notation (or rgb() if alpha is 255), colors store as rgba-FFFF will be serialized as color(srgb ...).

The second way it is observable (though in this case, only by a user, not via script) is via the precision loss and how that might effect conversion when drawing the color into a backing store.

I am not sure there is all that much important rationale behind this current state. When the rgb() and rgba() notations were introduced, we only had rgba-8888, so that is how they have been for a long time. When we added support for color(srgb ...) it was in concert with adding support for color(display-p3 ...) and deeper buffers, so using rgba-FFFF made sense.

I think this raises some interesting questions about what aspects of these decisions should be specified (if any).

svgeesus · 2021-01-18T14:28:23Z

I think this raises some interesting questions about what aspects of these decisions should be specified (if any).

On that note, following CSSWG discussion, CSS Color 4 has this to say about precision when serializing:

4.7.2. Serializing sRGB values

hex colors
rgb() and rgba() values
hsl() and hsla() values
hwb() values
named colors

The precision with which sRGB component values are retained, and thus the number of significant figures in the serialized value, is not defined in this specification, but must at least be sufficient to round-trip eight bit values. Values must be rounded towards +∞, not truncated.

but

4.7.4 Serializing values of the color() function

The precision with which color() component values are retained, and thus the number of significant figures in the serialized value, is not defined in this specification, but for sRGB and CMYK colorspaces must at least be sufficient to round-trip values with eight bit precision; this will result in at least two decimal places unless trailing zeroes have been omitted.

For the predefined colorspaces other than sRGB, which have wider gamuts and thus require more bits to avoid banding, the minimum precision for round-tripping is as follows:

(which then has a minimum of 10 bits for P3, etc)

So spec-wise, because of legacy implementations, it is conformant for either rgb() or color(srgb) to return serialization to 8 bit precision. But this will result in roundoff error for the % form or for non-integer values in the <number> form.

svgeesus · 2021-01-18T14:31:08Z

I can certainly see the value in having a duplicate set of tests where the reference is color(srgb). Maybe named like lab-nnn-b.html so that pairs of tests are easier to see?

…risons to avoid antialiasing artifacts from text rendering causing failures.

weinig · 2021-01-21T18:04:21Z

Ok, these have all been updated with the recalculated values. Sorry for the delay. I kept things as rgb() for now.

…port wide color https://bugs.webkit.org/show_bug.cgi?id=220684 Reviewed by Simon Fraser. LayoutTests/imported/w3c: Update existing lab/lch tests to match new more precise values from web-platform-tests/wpt#27202 and add two new tests for colors outside the sRGB gamut that will be upstreamed to WPT after landing. * web-platform-tests/css/css-color/lab-001.html: * web-platform-tests/css/css-color/lab-002.html: * web-platform-tests/css/css-color/lab-003.html: * web-platform-tests/css/css-color/lab-004-expected.html: * web-platform-tests/css/css-color/lab-004.html: * web-platform-tests/css/css-color/lab-005-expected.html: * web-platform-tests/css/css-color/lab-005.html: * web-platform-tests/css/css-color/lab-006-expected.html: * web-platform-tests/css/css-color/lab-006.html: * web-platform-tests/css/css-color/lab-007-expected.html: * web-platform-tests/css/css-color/lab-007.html: * web-platform-tests/css/css-color/lab-008-expected.html: Added. * web-platform-tests/css/css-color/lab-008.html: Added. * web-platform-tests/css/css-color/lch-001.html: * web-platform-tests/css/css-color/lch-002.html: * web-platform-tests/css/css-color/lch-003.html: * web-platform-tests/css/css-color/lch-004-expected.html: * web-platform-tests/css/css-color/lch-004.html: * web-platform-tests/css/css-color/lch-005-expected.html: * web-platform-tests/css/css-color/lch-005.html: * web-platform-tests/css/css-color/lch-006-expected.html: * web-platform-tests/css/css-color/lch-006.html: * web-platform-tests/css/css-color/lch-007-expected.html: * web-platform-tests/css/css-color/lch-007.html: * web-platform-tests/css/css-color/lch-008-expected.html: Added. * web-platform-tests/css/css-color/lch-008.html: Added. Source/WebCore: To avoid clamping device independent colors to sRGB we convert colors without a direct CoreGraphics representation to ExtendedSRGB to ensure the full gamut is preserved. While CoreGraphics does support the Lab colorspace on some systems (Big Sur and later), to ensure all versions of WebKit can support wide color Lab this approach provides the most coverage. * platform/graphics/ColorConversion.cpp: * platform/graphics/ColorConversion.h: * platform/graphics/ColorTypes.h: Add support for ExtendedSRGBA and LinearExtendedSRGBA color types. * platform/graphics/cg/ColorCG.cpp: (WebCore::leakCGColor): Update fallback conversion to convert to an extended color space to avoid premature clamping. Canonical link: https://commits.webkit.org/233219@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@271712 268f45cc-cd09-0410-ab3c-d52691b4dbfc

svgeesus

Thanks, this all looks great now!

…port wide color https://bugs.webkit.org/show_bug.cgi?id=220684 Reviewed by Simon Fraser. LayoutTests/imported/w3c: Update existing lab/lch tests to match new more precise values from web-platform-tests/wpt#27202 and add two new tests for colors outside the sRGB gamut that will be upstreamed to WPT after landing. * web-platform-tests/css/css-color/lab-001.html: * web-platform-tests/css/css-color/lab-002.html: * web-platform-tests/css/css-color/lab-003.html: * web-platform-tests/css/css-color/lab-004-expected.html: * web-platform-tests/css/css-color/lab-004.html: * web-platform-tests/css/css-color/lab-005-expected.html: * web-platform-tests/css/css-color/lab-005.html: * web-platform-tests/css/css-color/lab-006-expected.html: * web-platform-tests/css/css-color/lab-006.html: * web-platform-tests/css/css-color/lab-007-expected.html: * web-platform-tests/css/css-color/lab-007.html: * web-platform-tests/css/css-color/lab-008-expected.html: Added. * web-platform-tests/css/css-color/lab-008.html: Added. * web-platform-tests/css/css-color/lch-001.html: * web-platform-tests/css/css-color/lch-002.html: * web-platform-tests/css/css-color/lch-003.html: * web-platform-tests/css/css-color/lch-004-expected.html: * web-platform-tests/css/css-color/lch-004.html: * web-platform-tests/css/css-color/lch-005-expected.html: * web-platform-tests/css/css-color/lch-005.html: * web-platform-tests/css/css-color/lch-006-expected.html: * web-platform-tests/css/css-color/lch-006.html: * web-platform-tests/css/css-color/lch-007-expected.html: * web-platform-tests/css/css-color/lch-007.html: * web-platform-tests/css/css-color/lch-008-expected.html: Added. * web-platform-tests/css/css-color/lch-008.html: Added. Source/WebCore: To avoid clamping device independent colors to sRGB we convert colors without a direct CoreGraphics representation to ExtendedSRGB to ensure the full gamut is preserved. While CoreGraphics does support the Lab colorspace on some systems (Big Sur and later), to ensure all versions of WebKit can support wide color Lab this approach provides the most coverage. * platform/graphics/ColorConversion.cpp: * platform/graphics/ColorConversion.h: * platform/graphics/ColorTypes.h: Add support for ExtendedSRGBA and LinearExtendedSRGBA color types. * platform/graphics/cg/ColorCG.cpp: (WebCore::leakCGColor): Update fallback conversion to convert to an extended color space to avoid premature clamping. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@271712 268f45cc-cd09-0410-ab3c-d52691b4dbfc

wpt-pr-bot added the css-color label Jan 15, 2021

wpt-pr-bot assigned tabatkins Jan 15, 2021

wpt-pr-bot requested review from dbaron, svgeesus and tabatkins January 15, 2021 18:22

svgeesus reviewed Jan 16, 2021

View reviewed changes

css/css-color/lch-004-ref.html Outdated Show resolved Hide resolved

svgeesus approved these changes Jan 16, 2021

View reviewed changes

svgeesus self-requested a review January 16, 2021 13:58

svgeesus requested changes Jan 16, 2021

View reviewed changes

weinig force-pushed the weinig/lab-lch-color-text-to-rect branch from b0d8096 to 26cd3f6 Compare January 16, 2021 18:25

weinig requested a review from svgeesus January 16, 2021 19:57

svgeesus requested changes Jan 17, 2021

View reviewed changes

svgeesus mentioned this pull request Jan 18, 2021

[css-color-4] What serialization should be used when using color(lab ...) syntax to specify a lab color? w3c/csswg-drafts#5825

Closed

Switch lab and lch tests to use rectangles rather than text for compa…

b4e5caa

…risons to avoid antialiasing artifacts from text rendering causing failures.

weinig force-pushed the weinig/lab-lch-color-text-to-rect branch from 26cd3f6 to b4e5caa Compare January 21, 2021 17:12

weinig requested a review from svgeesus January 21, 2021 18:04

svgeesus approved these changes Jan 22, 2021

View reviewed changes

svgeesus merged commit 755a4a0 into web-platform-tests:master Jan 22, 2021

svgeesus mentioned this pull request Jan 27, 2021

Change tests for legacy color formats to use a box, rather than text #27337

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[css-color-4] Update lab and lch tests to use rectangles #27202

[css-color-4] Update lab and lch tests to use rectangles #27202

weinig commented Jan 15, 2021

svgeesus left a comment

svgeesus left a comment

svgeesus left a comment

svgeesus Jan 17, 2021

svgeesus Jan 17, 2021

svgeesus Jan 17, 2021

svgeesus Jan 17, 2021

svgeesus Jan 17, 2021

svgeesus Jan 17, 2021

svgeesus Jan 17, 2021

svgeesus Jan 17, 2021

svgeesus Jan 17, 2021

svgeesus Jan 17, 2021

weinig commented Jan 17, 2021

dbaron commented Jan 17, 2021

weinig commented Jan 17, 2021

dbaron commented Jan 17, 2021

weinig commented Jan 17, 2021

svgeesus commented Jan 18, 2021

svgeesus commented Jan 18, 2021

weinig commented Jan 21, 2021

svgeesus left a comment

[css-color-4] Update lab and lch tests to use rectangles #27202

[css-color-4] Update lab and lch tests to use rectangles #27202

Conversation

weinig commented Jan 15, 2021

svgeesus left a comment

Choose a reason for hiding this comment

svgeesus left a comment

Choose a reason for hiding this comment

svgeesus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weinig commented Jan 17, 2021

dbaron commented Jan 17, 2021

weinig commented Jan 17, 2021

dbaron commented Jan 17, 2021

weinig commented Jan 17, 2021

svgeesus commented Jan 18, 2021

svgeesus commented Jan 18, 2021

weinig commented Jan 21, 2021

svgeesus left a comment

Choose a reason for hiding this comment