Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Unexpected NaNs with HSV + toSRGB() #67

Closed
iamcalledrob opened this issue Dec 24, 2024 · 0 comments · Fixed by #68
Closed

Unexpected NaNs with HSV + toSRGB() #67

iamcalledrob opened this issue Dec 24, 2024 · 0 comments · Fixed by #68

Comments

@iamcalledrob
Copy link

iamcalledrob commented Dec 24, 2024

Not a bug per-se, but an unexpected developer API so leaving some feedback!

HSV with a NaN hue is convertible to a sane SRGB value

HSV(h = Float.NaN, s = 0f, v = 0f, alpha = 0f).toSRGB()
-> RGB(r=0.0, g=0.0, b=0.0, alpha=0.0, space=sRGB)

However, HSV with a NaN hue and a non-zero saturation component converts to (NaN, NaN, NaN, alpha)

HSV(h = Float.NaN, s = 0.1f, v = 0f, alpha = 0f).toSRGB()
-> RGB(r=NaN, g=NaN, b=NaN, alpha=0.0, space=sRGB)

I suppose this is technically correct (can't saturate a hue of NaN), but this is a source of unexpected runtime bugs whenever user input is involved.

I would propose that the s component is ignored when the hue is NaN, rather than returning a broken color. This is a particular issue when calling toComposeColor(), which will cause a crash.

In my case I had code like the following, which worked until someone chooses a monochrome color.

fun androidx.compose.ui.graphics.Color.withMaxSaturation() = toColormathColor().toHSV().copy(s = 1f).toComposeColor()

Color(0xFF5B7A63).withMaxSaturation() // OK
Color(0xFF6E6E6E).withMaxSaturation() // IllegalArgumentException: red = NaN, green = NaN, blue = NaN, alpha = 1.0 outside the range for sRGB IEC61966-2.1 (id=0, model=Rgb)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant