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

[chore] Port missing Chromatic adaptation tests #397

Open
Tracked by #399
lloydk opened this issue Feb 3, 2024 · 3 comments
Open
Tracked by #399

[chore] Port missing Chromatic adaptation tests #397

lloydk opened this issue Feb 3, 2024 · 3 comments
Labels
help wanted Extra attention is needed Topic: testing

Comments

@lloydk
Copy link
Collaborator

lloydk commented Feb 3, 2024

There are 13 tests in the old Chromatic adaptation test suite and 2 in the new test suite

@LeaVerou
Copy link
Member

LeaVerou commented Feb 4, 2024

Wow, no idea how that happened! Thanks!

Can you edit your comment with links to both files? (in general if you can add links to these issues, it would help). Thanks!

@lloydk
Copy link
Collaborator Author

lloydk commented Feb 4, 2024

Actually, the tests for this suite have been ported but most of them are commented out. Most of the commented out tests fail if they're run.

Old tests:
https://github.com/color-js/color.js/blob/main/tests/adapt.html

New Tests
https://github.com/color-js/color.js/blob/main/test/adapt.js

@LeaVerou LeaVerou changed the title Port missing Chromatic adaptation tests [chore] Port missing Chromatic adaptation tests Feb 4, 2024
@LeaVerou LeaVerou added the help wanted Extra attention is needed label Feb 4, 2024
@svgeesus
Copy link
Member

svgeesus commented Feb 4, 2024

This is because, as the commented-out tests say:

description: These test the linear Bradford adaptations from CATs.js against the matrices <a href="http://www.brucelindbloom.com/index.html?Eqn_ChromAdapt.html">published by Lindbloom</a>

The trouble is that

  • Lindbloom uses low precision rounded-off matrices
  • Lindbloom uses a slightly different definition of D65 and D50, thus gets different results
  • Lindbloom is AFAICT the only source for the "expected" values for adaptation to and from other whitepoints

There would be value in recalculating these to a higher precision, and using those values as "expected" to test for later regressions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed Topic: testing
Projects
None yet
Development

No branches or pull requests

3 participants