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

Add Okhsl and Okhsv #469

Merged
merged 7 commits into from Mar 6, 2024
Merged

Conversation

facelessuser
Copy link
Collaborator

@facelessuser facelessuser commented Mar 2, 2024

Resolves #312

@facelessuser facelessuser changed the title Add OkHsl Add Okhsl and Okhsv Mar 2, 2024
Copy link

netlify bot commented Mar 2, 2024

Deploy Preview for colorjs ready!

Name Link
🔨 Latest commit 896f5a4
🔍 Latest deploy log https://app.netlify.com/sites/colorjs/deploys/65e694e90c8fd100087cf633
😎 Deploy Preview https://deploy-preview-469--colorjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@facelessuser
Copy link
Collaborator Author

facelessuser commented Mar 2, 2024

This will add both Okhsl and Okhsv. It is implemented in a way that you could easily swap out coefficients and matrices and create Okhsl and Okhsv in other gamuts. Not that we want to do that ever, but that is why some things are done the way they are.

I am currently implementing it where lightness and saturation is in the range of 0 - 1 as the original author had done, but if it is decided we want 0 - 100, we can make that change.

I'll leave this in a draft until I've implemented both and added testing.

@facelessuser
Copy link
Collaborator Author

It should also be noted that Okhsl and Okhsv are only accurate to the 32 bit precision. So with the much more accurate 64 precision Oklab matrices, you get:

> new Color('red').to('okhsl').coords
[ 29.233880279627897, 1.0000000995016394, 0.5680846563197034 ]
> new Color('blue').to('okhsl').coords
[ 264.05202261636987, 1.0000000005848084, 0.3665653391870817 ]
> new Color('green').to('okhsl').coords
[ 142.4953450414439, 0.9999999797952728, 0.4437098579755183 ]

Okhsl and Okhsv also are an approximation, but the actual bounds of the model will cut out some sRGB colors and include some that are technically out of gamut. For this reason, I'm opting to set its gamut as itself. For practicality, it is suggested that people choose to gamut map in sRGB, the closest gamut and the one it aimed to target. We could set its gamut to sRGB, but then colors within the model near the edge will complain they are out of gamut. This is just my opinion and I'll let others decide what is best in this library.

@facelessuser facelessuser marked this pull request as ready for review March 5, 2024 03:46
@facelessuser
Copy link
Collaborator Author

I believe this is ready for review.

@LeaVerou LeaVerou requested a review from svgeesus March 5, 2024 07:22
@svgeesus
Copy link
Collaborator

svgeesus commented Mar 5, 2024

For this reason, I'm opting to set its gamut as itself. For practicality, it is suggested that people choose to gamut map in sRGB, the closest gamut and the one it aimed to target. We could set its gamut to sRGB, but then colors within the model near the edge will complain they are out of gamut.

That sounds like a good plan to me.

Copy link
Collaborator

@svgeesus svgeesus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good.

Thanks for providing, and abundantly documenting, the oklab matrix maker.

@svgeesus svgeesus merged commit 1a14ae5 into color-js:main Mar 6, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OKHSV and OKHSL
2 participants