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-5] Testable but untested sections #6824

Closed
8 of 10 tasks
svgeesus opened this issue Nov 22, 2021 · 11 comments
Closed
8 of 10 tasks

[css-color-5] Testable but untested sections #6824

svgeesus opened this issue Nov 22, 2021 · 11 comments
Labels
css-color-5 Color modification

Comments

@svgeesus
Copy link
Contributor

svgeesus commented Nov 22, 2021

Has 2 simple tests, both marked tentative and a very comprehensive parsing test

Has some tests. Spec examples would be a start.

Now has some tests

Hard to test reliably, but at least an @supports test

Has some tests, using computed value

@weinig
Copy link
Contributor

weinig commented Nov 27, 2021

I have a few for parsing and serialization and color-mixing I would be happy to convert to wpt and donate if you would find them helpful (some are being updated for the recent changes right now, so may be a bit out of date):

https://github.com/WebKit/WebKit/blob/main/LayoutTests/fast/css/parsing-color-contrast.html
https://github.com/WebKit/WebKit/blob/main/LayoutTests/fast/css/parsing-color-mix.html
https://github.com/WebKit/WebKit/blob/main/LayoutTests/fast/css/parsing-color-function.html
https://github.com/WebKit/WebKit/blob/main/LayoutTests/fast/css/parsing-relative-color-syntax.html
https://github.com/WebKit/WebKit/blob/main/LayoutTests/fast/css/parsing-lab-colors.html

My biggest concern is with floating point precision differences between browsers in some of them (since precision of component storage is only defined with a minimum bound among other issues).

Let me know and I will make some time to get out a PR.

@svgeesus
Copy link
Contributor Author

Yes, these would be super helpful please do.

My biggest concern is with floating point precision differences between browsers in some of them (since precision of component storage is only defined with a minimum bound

Maybe round to the minimum bound then compare observed - expected < epsilon?

among other issues).

Other issues?

@svgeesus
Copy link
Contributor Author

Please do add the rel=help pointing to the actual spec sections, though.

So for example on the color-mix() test add

<link rel="help" href="https://drafts.csswg.org/css-color-5/#color-mix"/>

@weinig
Copy link
Contributor

weinig commented Nov 29, 2021

among other issues).

Other issues?

I meant things like the precision of the color conversions themselves, for issues like how many digits of significance are used for the matrices, since the exact algorithms are not defined and floating point operations can introduce error with small differences. That said, I think there is far more benefit in having the tests now, and figuring out how to solve those than not.

@svgeesus
Copy link
Contributor Author

Oh, I see. Yes, the general problem with floats and precision, especially cumulative roundoff error.

Agree it is far better to have good tests now and improve them over time, than perfect tests never.

@svgeesus
Copy link
Contributor Author

These are all merged now except that @tabatkins asked for changes on RCS and I don't understand why. He comments about not producing integer values, yet they are not required by spec because serializing says that decimal values are allowed in rgb()

@tabatkins
Copy link
Member

It appears you understand my comments exactly backwards. ^_^ Sam's tests were testing serialization of %s like 10% or 25%, which produce non-integer values when converted to numbers, while their expected results were integers (which is allowed but not required, as you note). So I was asking them to either allow multiple correct values, both rounded and unrounded, or ensure that all %s tested were multiples of 20%. (They did the latter, so we're good to go now; I merged the tests.)

@svgeesus
Copy link
Contributor Author

Aha, got it.

@svgeesus
Copy link
Contributor Author

svgeesus commented Dec 23, 2021

I have a PR for an initial @color-profile test - web-platform-tests/wpt#32157 uses a red-green swapped ICC profile. Now merged

@svgeesus
Copy link
Contributor Author

Closing since the spec is now fairly well tested, with the exception of device-cmyk() which seems hard to produce a reftest for, as uncalibrated color can't be directly compared.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-color-5 Color modification
Projects
None yet
Development

No branches or pull requests

3 participants