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

Inaccurate xyz to lab. #100

Open
forrestli74 opened this issue Jul 8, 2022 · 8 comments
Open

Inaccurate xyz to lab. #100

forrestli74 opened this issue Jul 8, 2022 · 8 comments
Labels

Comments

@forrestli74
Copy link

The constants used in xyz to lab conversions are not accurate...
As an example, places where we use 0.008856 are actually supposed to be (6/29)^3. There are a lot more constants that are not accurate. I didn't go through all of them.

This has caused problems for me because I'm building a color-related library that's implemented in multiple languages. It converts rgb to lab color space, do some basic arithmetic and then convert back to rgb. (Yes, I used the xyz.lab.raw version) For javascript, I used color-convert as a dependency. And this has produced inconsistent results for different implementations.

@Qix-
Copy link
Owner

Qix- commented Jul 8, 2022

There have been a number of PRs regarding this in the past, Actually, no there haven't. That constant's been there since the package was first created.

Do you have some reference for the (6/29)^3 figure? I'm not an expert with lab/xyz.

If this is truly the case, happy to accept a PR that constructs the constant at boot time and uses that instead of the hardcoded value.

@Qix- Qix- added the bug label Jul 8, 2022
@Qix-
Copy link
Owner

Qix- commented Jul 8, 2022

Ah actually that's pretty well documented. Neat, thanks for the heads up. Will fix.

@Qix-
Copy link
Owner

Qix- commented Jul 8, 2022

c1576fe should fix it; as for the other constants, I'm not sure they can be adjusted based on some cursory research. Further, some of the more specific ones are hitting limits of the IEEE standard anyway.

Let me know if that's adequate or if any others should be calculated like that. If not, I'll cut a release for you.

Thanks again!

@forrestli74
Copy link
Author

Wow, thanks for the quick fix! Could you also change 7.787 to 1/3*(4/29)^(-2) (ref: lab wiki) ?
rgb to xyz is also inaccurate based on my manual testing, but I'm still figuring out what needs to be changed.

One other thing is that any reason there are no tests broken?

@Qix-
Copy link
Owner

Qix- commented Jul 8, 2022

The tests are using the default (rounded) versions of calls. Probably best, anyway, given IEEE inaccuracies, especially at such a fine-grained level.

@Qix-
Copy link
Owner

Qix- commented Jul 8, 2022

The change you requested gives me a drastically different result (** is the power operator here):

> 1/3*(4/29)**-2
17.520833333333332

Am I misunderstanding the formula? I would have expected it to be mostly the same, if not a bit more precise.

@forrestli74
Copy link
Author

Sorry. Typo. 1/3*(6/29)^(-2)
Reference: https://en.m.wikipedia.org/wiki/CIELAB_color_space

@forrestli74
Copy link
Author

Too lazy to figure out what's wrong with rgb to xyz. Feel free to close this once 1/3*(6/29)^(-2) is fixed.

Things I used for manual testing the convertion:

  1. https://www.easyrgb.com/en/convert.php#inputFORM
  2. https://docs.rs/palette/latest/palette/

To document my findings for rgb to xyz conversion:

For #FABD2F, both external tool produced xyz: 58.139, 56.929, 10.615 (with rounding)
While color-convert produced: 58.13502713482779, 56.924421562846604, 10.61278897125713

I don't think it's too fine grained though. My library converts rgb to lab color space, do some basic arithmetic and then convert back to 0-255 integer version of rgb. And some rgb components is off by 2. Too lazy to write a minimum example though.

If you are worried about IEEE inaccuracy, maybe set a difference threshold when asserting. Threshold of 1 is not justified just to account for IEEE inaccuracy. But not a high priority though.

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

2 participants