omgovich / colord

👑 A tiny yet powerful tool for high-performance color manipulations and conversions
https://colord.omgovich.ru
MIT License
1.63k stars 48 forks source link

deltaE00 implementation #77

Closed EricRovell closed 2 years ago

EricRovell commented 2 years ago

There is the DeltaE2000 implementation. It works correctly, the results were tested here.

But we have a problem. Despite the calculations are correct, we have wrong values, just a bit, but always wrong. I spend a lot of time trying to find out why and I found the problem. I was testing LAB / LAB comparison and I found out that if you create Colord instance using LAB color, and get LAB object back - values won't be the same.

Here the test that is unsuccessful:

expect(round(colord({ l: 28, a: 50, b: 64 })
  .deltaE00({ l: 33, a: -10, b: 98 }), 5))
   .toBe(37.81158); // result is 35.65174

I took these LAB values to get LAB object back and go this:

colord({ l: 28, a: 50, b: 64 }) // -> { l: 28.27, a: 50.61, b: 42.13 }
colord({ l: 33, a: -10, b: 98 }) // -> { l: 33.56, a: -6.01, b: 40.81 }

Now testing these values I have successfull test case:

expect(round(colord({ l: 28.27, a: 50.61, b: 42.13 })
  .deltaE00({ l: 33.56, a: -6.01, b: 40.81 }), 5))
  .toBe(35.65174);

I think the problem is with discrete values of the RGB color we use to store the color state. Having discrete values as the main state we have discrete values for all the color models and we lost some accuracy.

If I do something wrong, please tell me, I don't quite sure if I am doing everything correct. In case if the problem is real, I think we should return integer RGB values to the end consumer and use float values for model transformations.

P.S. The docs is not ready yet in this PR. Will do, but I don't think we need them yet.

omgovich commented 2 years ago

Hi! Thanks a lot!

Yeah, the library has several drawbacks since it uses RGB internally. I'm planning to migrate to XYZ as a master model soon.

leeoniya commented 2 years ago

@omgovich

have a look at these, too:

https://bottosson.github.io/posts/oklab/ https://bottosson.github.io/posts/colorpicker/

omgovich commented 2 years ago

https://bottosson.github.io/posts/oklab/ https://bottosson.github.io/posts/colorpicker/

Hi @leeoniya! Thanks for sharing these links! Looks breathtaking!

We might implement Oklab as a separate plugin, but in the primary API and plugins, we rely on the widely used standards that are close to CSS Specs (CIE LAB, CIE LCH, for example, are added to CSS Color Level 4+ drafts). So I think, we should use CIE LAB / CIE DeltaE to perform the color comparison.

EricRovell commented 2 years ago

Hi! Thanks a lot!

Yeah, the library has several drawbacks since it uses RGB internally. I'm planning to migrate to XYZ as a master model soon.

Alright, sounds awesome. I will try to polish this PR then.

omgovich commented 2 years ago

I think the accuracy is good anyway. Enough for most of the tasks.

EricRovell commented 2 years ago

I think the accuracy is good anyway. Enough for most of the tasks.

It is not big, about 2% of lost accuracy. But it bugs me that everywhere the values are the same and our tests won't have them. To finish this PR, I will do calibrated tests and include accurate values in the comments to replace when XYZ master color feature is ready.

EricRovell commented 2 years ago

Well, LAB plugin got some weight 😄

EricRovell commented 2 years ago

I think the next step I should I should better test examples to cover some branching logic in deltaE00 function. A bit hard, as these variables are calculated in mid air, but I will try.

EricRovell commented 2 years ago

There is one line still not covered, no luck yet.

omgovich commented 2 years ago

You can just make it a oneliner so CI won't trigger warnings: image

codecov-commenter commented 2 years ago

Codecov Report

Merging #77 (6e56bf4) into master (9fe0e2b) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #77   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           39        40    +1     
  Lines          524       574   +50     
  Branches        91       124   +33     
=========================================
+ Hits           524       574   +50     
Impacted Files Coverage Δ
src/get/getPerceivedDifference.ts 100.00% <100.00%> (ø)
src/plugins/lab.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9fe0e2b...6e56bf4. Read the comment docs.

EricRovell commented 2 years ago

Thanks a lot @EricRovell! This is huge! My new favorite feature of the library)

Always glad to help! 😁