Bug 203592 – Align CSS hsl() -> rgba() color conversion with the spec

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Simon Fraser (smfr)]

Align CSS hsl() -> rgba() color conversion with the spec

[Simon Fraser (smfr)]

Created attachment 382233 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 382241 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/56724731>

[Simon Fraser (smfr)]

https://trac.webkit.org/r251750

[Truitt Savell]

The changes in https://trac.webkit.org/changeset/251750/webkit

has broken http/wpt/css/css-images-4/conic-gradient-parsing.html

History:
https://results.webkit.org/?suite=layout-tests&test=http%2Fwpt%2Fcss%2Fcss-images-4%2Fconic-gradient-parsing.html

Diff:
--- /Volumes/Data/slave/mojave-release-tests-wk1/build/layout-test-results/http/wpt/css/css-images-4/conic-gradient-parsing-expected.txt
+++ /Volumes/Data/slave/mojave-release-tests-wk1/build/layout-test-results/http/wpt/css/css-images-4/conic-gradient-parsing-actual.txt
@@ -1,4 +1,4 @@
 
-PASS Test conic-gradient parsing 
+FAIL Test conic-gradient parsing assert_equals: Testing round trip of conic-gradient(hsl(0,0%,75%), hsl(0,0%,25%)) expected "conic-gradient(rgb(191, 191, 191), rgb(63, 63, 63))" but got "conic-gradient(rgb(191, 191, 191), rgb(64, 64, 64))"
 PASS Test repeating-conic-gradient parsing 
 

This is easily reproducible by running it in iterations.

[Aakash Jain]

This probably also broke:
fast/css/hsla-color.html and fast/css/hsl-color.html

Flakiness dashboard: https://results.webkit.org/?suite=layout-tests&suite=layout-tests&suite=layout-tests&test=fast%2Fcss%2Fhsl-color.html&test=fast%2Fcss%2Fhsla-color.html&test=http%2Fwpt%2Fcss%2Fcss-images-4%2Fconic-gradient-parsing.html

[Simon Fraser (smfr)]

Already fixed.

[Truitt Savell]

fast/css/hsl-color.html is still broken on Windows and is slowing down that EWS:

History:
https://results.webkit.org/?suite=layout-tests&test=fast%2Fcss%2Fhsl-color.html&platform=win

Diff:
--- /home/buildbot/worker/win10-release-tests/build/layout-test-results/fast/css/hsl-color-expected.txt
+++ /home/buildbot/worker/win10-release-tests/build/layout-test-results/fast/css/hsl-color-actual.txt
@@ -15,13 +15,13 @@
       RenderBlock {H1} at (0,175) size 769x38 [color=#0000FF]
         RenderText {#text} at (0,0) size 570x37
           text run at (0,0) width 570: "This should be blue: hsl(240, 100%, 50%)"
-      RenderBlock {H1} at (0,233) size 769x38 [color=#7FFF7F]
+      RenderBlock {H1} at (0,233) size 769x38 [color=#80FF80]
         RenderText {#text} at (0,0) size 658x37
           text run at (0,0) width 658: "This should be light green: hsl(120, 100%, 75%)"
-      RenderBlock {H1} at (0,292) size 769x38 [color=#007F00]
+      RenderBlock {H1} at (0,292) size 769x38 [color=#008000]
         RenderText {#text} at (0,0) size 661x37
           text run at (0,0) width 661: "This should be dark green: hsl(120, 100%, 25%)"
-      RenderBlock {H1} at (0,350) size 769x38 [color=#3FBF3F]
+      RenderBlock {H1} at (0,350) size 769x38 [color=#40BF40]
         RenderText {#text} at (0,0) size 659x37
           text run at (0,0) width 659: "This should be pastel green: hsl(120, 50%, 50%)"
       RenderBlock (anonymous) at (0,409) size 769x37
@@ -40,32 +40,32 @@
       RenderBlock (anonymous) at (0,583) size 769x19
         RenderText {#text} at (0,0) size 470x18
           text run at (0,0) width 470: "Check percentages less than 0% (negative values), should be limited to 0%"
-      RenderBlock {H1} at (0,622) size 769x38 [color=#7F7F7F]
+      RenderBlock {H1} at (0,622) size 769x38 [color=#808080]
         RenderText {#text} at (0,0) size 539x37
           text run at (0,0) width 539: "This should be grey: hsl(120, 0%, 50%)"
-      RenderBlock {H1} at (0,681) size 769x38 [color=#7F7F7F]
+      RenderBlock {H1} at (0,681) size 769x38 [color=#808080]
         RenderText {#text} at (0,0) size 641x37
           text run at (0,0) width 641: "This should be grey, too: hsl(120, -100%, 50%)"
       RenderBlock (anonymous) at (0,739) size 769x37
         RenderText {#text} at (0,0) size 755x36
           text run at (0,0) width 755: "Check Hue values that are larger than 360, should be normalized back to a value between 0 and 360. As Hue values are"
           text run at (0,18) width 451: "in degrees, there is no maximum like percentages, they are loop around."
-      RenderBlock {H1} at (0,797) size 769x38 [color=#007F00]
+      RenderBlock {H1} at (0,797) size 769x38 [color=#008000]
         RenderText {#text} at (0,0) size 661x37
           text run at (0,0) width 661: "This should be dark green: hsl(120, 100%, 25%)"
-      RenderBlock {H1} at (0,855) size 769x38 [color=#007F00]
+      RenderBlock {H1} at (0,855) size 769x38 [color=#008000]
         RenderText {#text} at (0,0) size 720x37
           text run at (0,0) width 720: "This should be dark green, too: hsl(480, 100%, 25%)"
       RenderBlock (anonymous) at (0,914) size 769x18
         RenderText {#text} at (0,0) size 258x18
           text run at (0,0) width 258: "Check Hue values with a negative angle."
-      RenderBlock {H1} at (0,953) size 769x38 [color=#7FFF7F]
+      RenderBlock {H1} at (0,953) size 769x38 [color=#80FF80]
         RenderText {#text} at (0,0) size 658x37
           text run at (0,0) width 658: "This should be light green: hsl(120, 100%, 75%)"
-      RenderBlock {H1} at (0,1011) size 769x38 [color=#7FFF7F]
+      RenderBlock {H1} at (0,1011) size 769x38 [color=#80FF80]
         RenderText {#text} at (0,0) size 728x37
           text run at (0,0) width 728: "This should be light green, too: hsl(-240, 100%, 75%)"
-      RenderBlock {H1} at (0,1070) size 769x38 [color=#7FFF7F]
+      RenderBlock {H1} at (0,1070) size 769x38 [color=#80FF80]
         RenderText {#text} at (0,0) size 728x37
           text run at (0,0) width 728: "This should be light green, too: hsl(-600, 100%, 75%)"
       RenderBlock (anonymous) at (0,1128) size 769x19

[Simon Fraser (smfr)]

Could you just land new Windows results?

[Truitt Savell]

Updated test results in https://trac.webkit.org/changeset/251865/webkit

[Darin Adler]

Comment on attachment 382241 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=382241&action=review

> Source/WebCore/platform/graphics/Color.cpp:108
> +    return makeRGBA(
> +        round(floatResult.components[0] * scaleFactor),
> +        round(floatResult.components[1] * scaleFactor),
> +        round(floatResult.components[2] * scaleFactor),
> +        round(floatResult.components[3] * scaleFactor));

I think it would be better to use std::round. The ::round function used here takes and returns a double so we do "float * float -> double -> float". If we used std::round then it would stay a float.