-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
Precision lets scientific notation leak for small values #388
Comments
Are you saying that a conversion from sRGB This is what I get for black and white: > new Color('#000000').to('oklch').coords
[ 0, 0, NaN ]
> new Color('#ffffff').to('oklch').coords
[ 1.0000000000000002, 4.996003610813204e-16, NaN ] This matches what I get in their live playground: https://colorjs.io/notebook/ |
Yes, you are right. I also get [0, 0, NaN] now. Not sure what I did. |
What is the status? |
Your issue is not reproducible, and then it also seems you are now getting what you should, so there is nothing to look into. The issue has been closed. |
I am confused. My test with black was obviously wrong, but both colors do not return the expected result. Black:
White
My problem was not that both return the same color, but that the conversion from black and white return invalid colors. |
Unfortunately, it was not clear from your filed issue what your issue was, so let me explain.
Now, I have found an issue, so I'll reopen this issue. What I am experiencing is that rounding to a given precision isn't quite working. I think this is a new bug introduced by #384. This seems to specifically affect very small numbers using scientific notation. > new Color('red').to('oklch').toString({precision: 1})
'oklch(0.6 0.3 30)'
> new Color('red').to('oklch').toString({precision: 3})
'oklch(0.628 0.258 29.2)'
> new Color('red').to('oklch').toString({precision: 16})
'oklch(0.6279553639214311 0.2576833038053608 29.23388027962784)'
> new Color('white').to('oklch').toString({precision: 3})
'oklch(1 5e-16 none)'
> new Color('white').to('oklch').toString({precision: 1})
'oklch(1 5e-16 none)'
> new Color('white').to('oklch').toString({precision: 16})
'oklch(1 4.996003610813204e-16 none)' In their defense, precision is working much better, but it now specifically has an issue with very, very small numbers that JS will display with scientific notation. What we should get, because >>> Color('red').convert('oklch').to_string(precision=1)
'oklch(0.6 0.3 30)'
>>> Color('red').convert('oklch').to_string(precision=3)
'oklch(0.628 0.258 29.2)'
>>> Color('red').convert('oklch').to_string(precision=16)
'oklch(0.6279553639214313 0.2576833038053606 29.2338802796279)'
>>> Color('white').convert('oklch').to_string(precision=1)
'oklch(1 0 0)'
>>> Color('white').convert('oklch').to_string(precision=3)
'oklch(1 0 0)'
>>> Color('white').convert('oklch').to_string(precision=16)
'oklch(1 0.0000000000000005 0)' |
I see. Thanks for your awesome explanation |
Your first problem is that you have mixed up black and white. Black should give [0, 0, NaN] and white would give [1, 0, NaN]. Your second is that you seem to believe Oklab (and thus, Oklch) Lightness goes from 0 to 100. It does not, the range is 0 to 1. See the actual defining document: However, I can see why you think that: Oklch.com is using percentage values (but leaving out the percent sign from the picker!) Thirdly, an undefined hue is correct for achromatic colors. As @facelessuser said, rounding should be catching those tiny almost-zero numbers, but the scientific notation is leaking out and this is a Color.js problem that we need to fix. |
My white / black mistakes are just from copy-pasting. But I do not really understand oklab yet, but it looks interesting to generate colors for things elements like avatars. The only motivation was to convert a color, that I got from a color-input to something that CSS understands, because the CSS framework uses oklab. If there is a good reason why it should not work out of the box, it would be great to have documentation, an alternative conversion, a flag or something that works for people that are not experts in color spaces ;) Keep up the good work and thanks again for the explanation. |
I have to put the variable into a CSS variable without the oklch part, but I guess I can just use the "serializeNumber" function for the coords. |
@facelessuser could you please edit the title of this issue to something that reflects the actual issue and apply suitable labels? Thanks! |
Thanks @facelessuser! |
It seems to me we should check whether the absolute value of the number is less than 0.5 E So for example if precision is |
@LeaVerou I'm guessing it is how the language serializes very small floats, I ran into the same thing with Python. You can strictly control the serialization. I have some ideas. I'll post back once cobble something together. |
The thing is, this only happens when people read the coords directly, which is not really something we can intercept, and we don't want to lose any precision in our internal representation. When serializing, we do limit precision. Solutions I see:
|
This is to set the stage for option 3 here: #388 (comment) but also implements a performance improvement when the color passed *happens* to be in the same color space as the one specified.
@LeaVerou Okay, so I'm looking at two different things.
Now you get sane results. Again, the removal of scientific notation from serialization doesn't have to be included. ![]()
I think coords should always return the actual coords. If they want to get a certain precision, they can use the utility |
Updated the code in my original post to fix some issues. If I moved forward with any of this, I'd obviously run some more tests. |
I have a PR for the precision fix up: #416. If it is desired to force serialization to not use scientific notation, just let me know; otherwise, I will keep changes to just to the precision fix. |
I guess the most important example after the fix: > new Color('white').to('oklch').toString()
'oklch(100% 0 none)'
> new Color('black').to('oklch').toString()
'oklch(0% 0 none)' Coords will still be in the raw form, but you can set their precision as shown below: > let coords = []
undefined
> new Color('white').to('oklch').coords.forEach(c => {coords.push(Color.util.toPrecision(c, 5))})
undefined
> coords
[ 1, 0, NaN ] |
…vements (#413) * `colorSpace.equals()` improvements - JSDoc - Handle comparison with id * `getAll()`: Make color space optional This is to set the stage for option 3 here: #388 (comment) but also implements a performance improvement when the color passed *happens* to be in the same color space as the one specified. * Update types for getAll --------- Co-authored-by: Jonny Gerig Meyer <jonny@oddbird.net>
Hi,
I have very little experience with color spaces. I found your solution for a simple theme designer that i build for my app using DaisyUI.
They store the colors in oklch and when the user pickers a rgb color, using a color picker I have to make the conversion. It is actually super easy:
It works fine, except for black and white:
Except black and white (
#00000
and `#ffffff)1.0000000000000002 4.996003610813204e-16 NaN
1.0000000000000002 4.996003610813204e-16 NaN
The text was updated successfully, but these errors were encountered: