Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-color] css/css-color/parsing/color-computed-relative-color.html assumes conversion to hwb is lossless #45912

Closed
weinig opened this issue Apr 25, 2024 · 5 comments
Labels

Comments

@weinig
Copy link
Contributor

weinig commented Apr 25, 2024

The test css/css-color/parsing/color-computed-relative-color.html currently assumes that conversion to hwb is lossless, which the current specification doesn't seem support. For example the fuzzy test on line 794:

fuzzy_test_computed_color(`color(from hwb(from color(${colorSpace} 0.99 0.88 0.77) h w b) ${colorSpace} x y z)`, `color(${colorSpace} 0.99 0.88 0.77)`, 0.0001);`

(where ${colorSpace} is either xyz-d65 or xyz-d50).

Using colorjs.io as a proxy, just taking the inner most part and seeing the possible values of color(xyz-d65 0.99 0.88 0.77) -> https://colorjs.io/apps/convert/?color=color(xyz-d65%200.99%200.88%200.77)&precision=4

you can see that the hwb representation is hwb(183.3 118.4% 15.14%) -> https://colorjs.io/apps/convert/?color=hwb(183.3%20118.4%25%2015.14%25)&precision=4

which round trips back to xyz-d65 as color(xyz-d65 0.7235 0.7612 0.829).

Totally possible I am interpreting something in the spec language incorrect here.

@nt1m
Copy link
Member

nt1m commented Apr 25, 2024

@svgeesus
Copy link
Contributor

Yeah hwb() is even worse than hsl() in that respect. For colors a little outside sRGB it is lossless but it suddenly falls apart quite quickly.

Those hwb() round-trip subtests should be deleted. The rgb() and hsl() ones appear correct.

@svgeesus
Copy link
Contributor

Btw @weinig I strongly suggest double-checking the to-xyz and from-xyz matrices, the chromatic adaptation matrices, and the matrices to and from oklab in WebKit against the ones in the sample code. There have been several rounds of revisions and we believe the ones in the sample code are 64-bit float accurate, but it may be that WebKit is using some older ones which (while not giving visibly different colors) will be subject to small round-off errors which can accumulate to the point of failing a test if there are many color conversion stages.

Same advice for Blink and Gecko but given how WebKit was such an early adopter the chance of the matrices being older ones is greater, I suspect.

@weinig
Copy link
Contributor Author

weinig commented Apr 27, 2024

Btw @weinig I strongly suggest double-checking the to-xyz and from-xyz matrices, the chromatic adaptation matrices, and the matrices to and from oklab in WebKit against the ones in the sample code

On it! WebKit/WebKit#27824

weinig added a commit to weinig/wpt that referenced this issue Apr 27, 2024
…assumes conversion to hwb is lossless

web-platform-tests#45912

Remove invalid test case that assumes out of gamut conversions to hwb are lossless.
svgeesus pushed a commit that referenced this issue May 6, 2024
…assumes conversion to hwb is lossless

#45912

Remove invalid test case that assumes out of gamut conversions to hwb are lossless.
@nt1m
Copy link
Member

nt1m commented May 13, 2024

I'm going to assume this was fixed by #45940, but feel free to reopen if that is not the case.

@nt1m nt1m closed this as completed May 13, 2024
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 14, 2024
…computed-relative-color.html assumes conversion to hwb is lossless, a=testonly

Automatic update from web-platform-tests
[css-color] css/css-color/parsing/color-computed-relative-color.html assumes conversion to hwb is lossless
web-platform-tests/wpt#45912

Remove invalid test case that assumes out of gamut conversions to hwb are lossless.

--

wpt-commits: 4dc8d6977d3c4e0d36f1b941652e0542bd86928b
wpt-pr: 45940
ErichDonGubler pushed a commit to ErichDonGubler/firefox that referenced this issue May 15, 2024
…computed-relative-color.html assumes conversion to hwb is lossless, a=testonly

Automatic update from web-platform-tests
[css-color] css/css-color/parsing/color-computed-relative-color.html assumes conversion to hwb is lossless
web-platform-tests/wpt#45912

Remove invalid test case that assumes out of gamut conversions to hwb are lossless.

--

wpt-commits: 4dc8d6977d3c4e0d36f1b941652e0542bd86928b
wpt-pr: 45940
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants