Skip to content

Commit

Permalink
Parse rgb() like other color functions
Browse files Browse the repository at this point in the history
Previously, rgb() and rgba() parsers were compressing all four color
channels and alpha to 8-bit integers. This is no longer necessary, as
blink::Color stores the parameters as floats.

As we move towards implementing relative color, the more unified the
color parsing is, the easier the eventual implementation will be.

After some discussion for Interop 2023, it was decided that non-finite
color parameters should round-trip and be resolved sanely:
http://github.com/w3c/csswg-drafts/issues/8629
This new requirement obviously puts us well out of 8-bit integer
territory.

Unfortunately, there are clearly still some code paths that are
independently compressing alpha to be an 8-bit integer and unless
we explicitly do this in the parser, many tests fail. A TODO has been
added to address this issue.

Here is the test change proposal: web-platform-tests/interop#369

Gradient tests with png expectations are pretty brittle, as evidenced
by the per-platform expectations. This change requires a rebaseline
because color channels are not squashed to [0,255] integer range at
the endpoints. So inputs like:
  conic-gradient(rgb(0%, 75%, 25%), rgb(0%, 25%, 75%));
No longer become equivalent to:
  conic-gradient(rgb(0, 191, 63), rgb(0, 63, 191));
25% of 255 is 63.75. 75% of 255 is 191.25.

Bug: 1452185
Change-Id: I1e89ad21d0e3007c1f51aa075728732c2856acc3
  • Loading branch information
mysteryDate authored and chromium-wpt-export-bot committed Jul 3, 2023
1 parent b87174e commit 9f05eb8
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 1 deletion.
19 changes: 19 additions & 0 deletions css/css-color/parsing/color-computed-rgb.html
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,25 @@
["rgba(0%, 0%, 0%, 0%)", "rgba(0, 0, 0, 0)", "Percent alpha values are accepted in rgb/rgba"],
["rgba(0%, 0%, 0%)", "rgb(0, 0, 0)", "RGB and RGBA are synonyms"],
["rgba(0, 0, 0)", "rgb(0, 0, 0)", "RGB and RGBA are synonyms"],

// calc(+/-infinity) remains infinity but calc(NaN) becomes zero for all channels.
// See: https://github.com/w3c/csswg-drafts/issues/8629
["rgb(calc(infinity), 0, 0)", "rgb(255, 0, 0)", "Red channel resolves positive infinity to 255"],
["rgb(0, calc(infinity), 0)", "rgb(0, 255, 0)", "Green channel resolves positive infinity to 255"],
["rgb(0, 0, calc(infinity))", "rgb(0, 0, 255)", "Blue channel resolves positive infinity to 255"],
["rgba(0, 0, 0, calc(infinity))", "rgb(0, 0, 0)", "Alpha channel resolves positive infinity to fully opaque"],
["rgb(calc(-infinity), 0, 0)", "rgb(0, 0, 0)", "Red channel resolves negative infinity to zero"],
["rgb(0, calc(-infinity), 0)", "rgb(0, 0, 0)", "Green channel resolves negative infinity to zero"],
["rgb(0, 0, calc(-infinity))", "rgb(0, 0, 0)", "Blue channel resolves negative infinity to zero"],
["rgba(0, 0, 0, calc(-infinity))", "rgba(0, 0, 0, 0)", "Alpha channel resolves negative infinity to fully transparent"],
["rgb(calc(NaN), 0, 0)", "rgb(0, 0, 0)", "Red channel resolves NaN to zero"],
["rgb(0, calc(NaN), 0)", "rgb(0, 0, 0)", "Green channel resolves NaN to zero"],
["rgb(0, 0, calc(NaN))", "rgb(0, 0, 0)", "Blue channel resolves NaN to zero"],
["rgba(0, 0, 0, calc(NaN))", "rgba(0, 0, 0, 0)", "Alpha channel resolves NaN to zero"],
["rgb(calc(0 / 0), 0, 0)", "rgb(0, 0, 0)", "Red channel resolves NaN equivalent calc statements to zero"],
["rgb(0, calc(0 / 0), 0)", "rgb(0, 0, 0)", "Green channel resolves NaN equivalent calc statements to zero"],
["rgb(0, 0, calc(0 / 0))", "rgb(0, 0, 0)", "Blue channel resolves NaN equivalent calc statements to zero"],
["rgba(0, 0, 0, calc(0 / 0))", "rgba(0, 0, 0, 0)", "Alpha channel resolves NaN equivalent calc statements to zero"],
];

for (const test of tests) {
Expand Down
18 changes: 18 additions & 0 deletions css/css-color/parsing/color-valid-rgb.html
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,24 @@
test_valid_value("color", "rgba(-20% 20% 40% / 50%)", "rgba(0, 51, 102, 0.5)");
test_valid_value("color", "rgba(257 30 40 / 50%)", "rgba(255, 30, 40, 0.5)");
test_valid_value("color", "rgba(250% 20% 40% / .5)", "rgba(255, 51, 102, 0.5)");

// Test non-finite values. See: https://github.com/w3c/csswg-drafts/issues/8629
test_valid_value("color", "rgb(calc(infinity), 0, 0)", "rgb(calc(infinity), 0, 0)");
test_valid_value("color", "rgb(0, calc(infinity), 0)", "rgb(0, calc(infinity), 0)");
test_valid_value("color", "rgb(0, 0, calc(infinity))", "rgb(0, 0, calc(infinity))");
test_valid_value("color", "rgba(0, 0, 0, calc(infinity))", "rgba(0, 0, 0, calc(infinity))");
test_valid_value("color", "rgb(calc(-infinity), 0, 0)", "rgb(calc(-infinity), 0, 0)");
test_valid_value("color", "rgb(0, calc(-infinity), 0)", "rgb(0, calc(-infinity), 0)");
test_valid_value("color", "rgb(0, 0, calc(-infinity))", "rgb(0, 0, calc(-infinity))");
test_valid_value("color", "rgba(0, 0, 0, calc(-infinity))", "rgba(0, 0, 0, calc(-infinity))");
test_valid_value("color", "rgb(calc(NaN), 0, 0)", "rgb(calc(NaN), 0, 0)");
test_valid_value("color", "rgb(0, calc(NaN), 0)", "rgb(0, calc(NaN), 0)");
test_valid_value("color", "rgb(0, 0, calc(NaN))", "rgb(0, 0, calc(NaN))");
test_valid_value("color", "rgba(0, 0, 0, calc(NaN))", "rgba(0, 0, 0, calc(NaN))");
test_valid_value("color", "rgb(calc(0 / 0), 0, 0)", "rgb(calc(NaN), 0, 0)");
test_valid_value("color", "rgb(0, calc(0 / 0), 0)", "rgb(0, calc(NaN), 0)");
test_valid_value("color", "rgb(0, 0, calc(0 / 0))", "rgb(0, 0, calc(NaN))");
test_valid_value("color", "rgba(0, 0, 0, calc(0 / 0))", "rgba(0, 0, 0, calc(NaN))");
</script>
</body>
</html>
1 change: 0 additions & 1 deletion css/support/computed-testcommon.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ function colorValuesAlmostEqual(color1, color2, float_epsilon, integer_epsilon)
// Return an array of all numbers in the color.
function getNumbers(color) {
const result = [];
// const entries = color.split(colorElementDividers);
color.split(colorElementDividers).forEach(element => {
const numberElement = parseFloat(element);
if (!isNaN(numberElement)) {
Expand Down

0 comments on commit 9f05eb8

Please sign in to comment.