Repository navigation
Pin -jvm-default, and settle how the dialog and convertToArgb fail - #366
Merged
Merged
Conversation
Nothing set -jvm-default, so the libraries took whatever mode the Kotlin compiler defaults to. A release defaulting to no-compatibility would drop ColorPickerStrings$DefaultImpls, and a library compiled with -jvm-default=disable that implements ColorPickerStrings would then fail at run time. Pinning enable keeps the bytecode as it is, so the API dumps do not change. Switching the pin to no-compatibility removes DefaultImpls from the Android classes as well as the JVM dump, so it reaches both targets.
Left to its default, the initial space was spaces.first(), so an empty list threw NoSuchElementException, while every other bad argument to the dialog and its state throws IllegalArgumentException. Changing the type once 2.0.0 ships would break a caller that catches it.
Through the sRGB curve's table a NaN channel already packed as 0. But a gamut with any other curve, and sRGB mapped to itself, packed through roundToInt, which throws IllegalArgumentException on NaN. So the same NaN color drew black or threw, depending on the route.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three behaviours 2.0.0 would otherwise freeze:
-jvm-defaultis pinned toenableon the JVM and Android targets. Nothing set it, so a Kotlin release defaulting tono-compatibilitywould dropColorPickerStrings$DefaultImplsfrom the ABI. The dumps are unchanged. With the pin switched tono-compatibility,jvmApiCheckfails on the missingDefaultImplsand the Android classes lack it too.spacesthrowsIllegalArgumentExceptionfromColorPickerDialogState,rememberColorPickerDialogStateand bothColorPickerDialogoverloads. The defaultinitialSpacewasspaces.first(), which threwNoSuchElementException.convertToArgbpacks a NaN channel as 0 on every route. Only the sRGB table did; sRGB mapped to itself, and every other curve, threw fromroundToInt.apiCheck,checkNoMaterial, the iOS compile, the sample and the screenshot baselines pass unchanged.