search

BoxDragon / kolor

Color space conversion math made simple
38 stars 7 forks source link

Fix NaN produced during some XYZ to Oklab conversions #5

Closed kettle11 closed 3 years ago

kettle11 commented 3 years ago

I stumbled across this issue today. It seems like a very easy error to make when implementing Oklab and took a bit of time to track down the fix.

Negative numbers can be produced for the lms components during conversion. Raising those to 1. / 3. with powf can produce NaN values. Using cbrt instead avoids that and matches the expected values in the table "Table of example XYZ and Oklab pairs" on this page: https://bottosson.github.io/posts/oklab/

kabergstrom commented 3 years ago

Oh neat, thank you for this! Goes to show that we really need a proper test suite sooner rather than later..

Should we use cbrt for the similar operation in XYZ_to_CIELAB as well?

kettle11 commented 3 years ago

Should we use cbrt for the similar operation in XYZ_to_CIELAB as well?

That code looks like this:

if v > 0.008856 {
    v.powf(1.0 / 3.0)
}

Since v's definitely not negative then there's no risk of NaN values there. cbrt is probably faster though.