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

Fix Oklab from Oklch hue conversion #344

Merged
merged 1 commit into from
Jul 27, 2023
Merged

Conversation

tomcur
Copy link
Contributor

@tomcur tomcur commented Jul 26, 2023

The Oklch -> Oklab conversion currently inverts the hue by swapping the a and b components. This PR corrects the calculation and adds an Oklch -> Oklab -> Oklch roundtrip test.

The Oklch -> Oklab conversion inverted the hue by swapping the a and b
components. This corrects the calculation and adds an
Oklch -> Oklab -> Oklch roundtrip test.
@Ogeon
Copy link
Owner

Ogeon commented Jul 27, 2023

Hi, thank you for spotting this and for taking the time to fix it (with added tests too!). It may have been a naming confusion when replacing sin_cos. Either way, I'll let the tests run through, but it looks good to me. 👍

@Ogeon
Copy link
Owner

Ogeon commented Jul 27, 2023

Alright, it's all green, so let's merge this.

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 27, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit ee54371 into Ogeon:master Jul 27, 2023
@tomcur
Copy link
Contributor Author

tomcur commented Jul 27, 2023

Thank you for the quick merge!

@tomcur tomcur deleted the fix-oklch-hue branch July 27, 2023 13:45
@wash2
Copy link

wash2 commented Aug 3, 2023

Is there planned to be a release with this fix?

@Ogeon
Copy link
Owner

Ogeon commented Aug 3, 2023

I'll try to get it out within the next few days. I was initially going to add CAM16 (#342) in the same release, but it turned out to be a bit trickier than expected. I'll put it to the side for a bit and add a couple of missing casting traits, and then make the release.

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.

3 participants