ajalt / colormath

Multiplatform Kotlin color conversion and manipulation
https://ajalt.github.io/colormath/
MIT License
308 stars 21 forks source link

The formatCssString method, used with unitsPercent = true, is not working with colors HSV or Oklch if they contain NaN value #54

Closed danielemaddaluno closed 7 months ago

danielemaddaluno commented 7 months ago

Here I paste a couple of codes written in Java to replicate the error that I'm getting. The point is that the same formatCssString is working fine if the unitsPercent are set to false. When you set them to true with a color HSV or Oklch, it will throw an exception.

The first code is for HSV:

Color color = CssParseKt.parse(Color.Companion, "#777777");
color = color.toHSV();
boolean unitsPercent = true;
String css = CssRenderKt.formatCssString(color, AngleUnit.AUTO, RenderCondition.AUTO, unitsPercent, false, false, false);
System.out.println(css);

The second for Oklch:

Color color2 = CssParseKt.parse(Color.Companion, "#777777");
color2 = color2.toOklch();
boolean unitsPercent2 = true;
String css2 = CssRenderKt.formatCssString(color2, AngleUnit.AUTO, RenderCondition.AUTO, unitsPercent2, false, false, false);
System.out.println(css2);

And this is the stacktrace:

Exception in thread "main" java.lang.IllegalArgumentException: Cannot round NaN value.
    at kotlin.math.MathKt__MathJVMKt.roundToInt(MathJVM.kt:1165)
    at com.github.ajalt.colormath.CssRenderKt.render(CssRender.kt:257)
    at com.github.ajalt.colormath.CssRenderKt.access$render(CssRender.kt:1)
    at com.github.ajalt.colormath.CssRenderKt$renderColorFunction$1$1.invoke(CssRender.kt:242)
    at com.github.ajalt.colormath.CssRenderKt$renderColorFunction$1$1.invoke(CssRender.kt:242)
    at kotlin.text.StringsKt__AppendableKt.appendElement(Appendable.kt:85)
    at kotlin.collections.CollectionsKt___CollectionsKt.joinTo(_Collections.kt:3490)
    at kotlin.collections.CollectionsKt___CollectionsKt.joinTo$default(_Collections.kt:3484)
    at com.github.ajalt.colormath.CssRenderKt.renderColorFunction(CssRender.kt:242)
    at com.github.ajalt.colormath.CssRenderKt.formatCssString(CssRender.kt:145)
danielemaddaluno commented 7 months ago

In this pull request I have just added a couple of tests in the CssRenderTest.kt, similar to the ones that I have written above.

danielemaddaluno commented 7 months ago

I think I have added the small edit to manage the NaN with percentUnits = true. Sorry @ajalt if I have made it separately with two separated pull request (one for tests and one for the fix). If you prefer I can add the two edits of the two files in a singular pull request, let me know.

ajalt commented 7 months ago

Thanks for the contribution! Yes, please combine them into one PR.