Ensure Okhsl and Okhsv return undefined hues for achromatic colors #517
Conversation
✅ Deploy Preview for colorjs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
facelessuser
force-pushed
the
bugfix/ok-achromatic
branch
from
9829553
to
af37eb5
Compare
|
Will need updating due to the switch from |
|
Yeah, I was hoping to get in before all that. I'll rebase and update it probably tomorrow if I have time. |
|
@facelessuser let me know if I can help! |
facelessuser
force-pushed
the
bugfix/ok-achromatic
branch
from
2e15d98
to
ce85731
Compare
|
I've rebased and updated to use null. |
|
@LeaVerou Okhsl and Okhsv is duplicating logic? Which part? Did you mean to comment in the Oklrab PR? I could see the claim that that PR was duplicating code. |
|
Perhaps it's not exactly duplication, but the conversion of these spaces to/from base is very weird, it seems to be going through sRGB Linear somehow? If so, why not just define sRGB linear as the base? |
|
It goes through linear sRGB as that is the gamut the Okhsl and Okhsv space are converting to. This is required to find the cusp of the hue slice of Oklab in relation to linear sRGB. It is specifically part of the conversion algorithm, but the algorithm also requires Oklab to calculate the initial chroma and lightness. That's just how the algorithm works. I need to convert to both Oklab and linear sRGB, I can't just pick one. If I require the base to be sRGB linear, then I'll also have to convert it to Oklab right away. |
|
To be honest, I'm pretty indifferent as to whether the base is Oklab or linear sRGB. If you want the base to be different, I can do it, and instead of having the Oklab to sRGB linear conversion, I'll add the linear sRGB to Oklab conversion. It's all the same to me. It all amounts to pretty much the same thing. |
|
For context, the original algorithm did conversion back and forth between llnear sRGB and Oklab, but I've made the initial base Oklab and only require a conversion to sRGB linear, not from, simplifying the code. |
|
If it needs both color spaces, then I guess up to you. |
|
I'll leave it as is unless a specific need arises requiring such a change. |
Fixes #516