Qix- / color-convert

Plain color conversion functions in JavaScript
MIT License
739 stars 96 forks source link

Inaccurate xyz to lab. #100

Open forrestli74 opened 2 years ago

forrestli74 commented 2 years ago

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- commented 2 years ago

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- commented 2 years ago

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

Qix- commented 2 years ago

c1576fe38cb83063c03ce6bf23410aa6ecc59079 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 commented 2 years ago

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- commented 2 years ago

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

Qix- commented 2 years ago

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 commented 2 years ago

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

forrestli74 commented 2 years ago

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.