Parse rgb() like other color functions #40426
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wpt-pr-bot
approved these changes
Jun 7, 2023
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-4598329
branch
2 times, most recently
from
ac9072c to
8cf2133
Compare
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-4598329
branch
3 times, most recently
from
c54a4d5 to
b2fb50a
Compare
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-4598329
branch
2 times, most recently
from
4130e2d to
f4b7add
Compare
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-4598329
branch
2 times, most recently
from
da7ad6b to
3562728
Compare
chromium-wpt-export-bot
changed the title
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-4598329
branch
7 times, most recently
from
f12bf02 to
b651ab9
Compare
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-4598329
branch
4 times, most recently
from
f1d7a44 to
2c60737
Compare
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 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4598329 Reviewed-by: Florin Malita <[email protected]> Reviewed-by: Steinar H Gunderson <[email protected]> Commit-Queue: Aaron Krajeski <[email protected]> Cr-Commit-Position: refs/heads/main@{#1166616}
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-4598329
branch
from
2c60737 to
dbda57a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4598329
Reviewed-by: Florin Malita <[email protected]>
Reviewed-by: Steinar H Gunderson <[email protected]>
Commit-Queue: Aaron Krajeski <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1166616}