Closed Atari2 closed 4 months ago
Hello @Atari2, we'll take a look into this issue, but rounding to a whole number isn't appropriate in all cases.
Could you describe your use case? There are several ways you can solve the issue.
Hello @Atari2, we'll take a look into this issue, but rounding to a whole number isn't appropriate in all cases.
Could you describe your use case? There are several ways you can solve the issue.
To be clear, I'm not asking for rounding to integers to avoid this problem, I'm saying that checking for strict equality with floating point numbers is asking for trouble, as this helpful site describes. So the check shouldn't just be
xSpacingMultiplier % 1f == 0f
(emphasis on the ==), but, at the very least, something like the following:
(1f - (xSpacingMultiplier % 1f)).absoluteValue <= xSpacingMultiplier.ulp)
.
I'm saying this because, as seen in my reproduction case, your check errors out even with values that should be valid (1.9, 1.35 and 2.59 are all multiples of 0.01 (xStep)).
My use case is just a very simple bar graph with an array of doubles on the X axis and some ints on the Y axis, imagine the code I provided as reproduction but with many more values in the columnSeries and some labels, that's literally all it is.
Link to kotlin playground that shows the math here
I'm posting a new comment because the wording of the old one could be misinterpreted as suggesting that we weren't going to fully resolve the issue.
Thanks for the suggestion. This doesn't cover all cases, so we've implemented a slightly different solution. An update will be available shortly.
Vico 2.0.0-alpha.21 fixes this. Thanks again for the suggestion!
It's fixed in Vico 1.15.0 Alpha 2 as well.
How to reproduce
To reproduce, simply create a jetpack compose activity project in android studio and try to preview this composable, and observe the crash.
Observed behavior
The code is doing this calculation https://github.com/patrykandpatrick/vico/blob/7ead489e30b4f36947ed0b9d76d5fa8c373d8186/vico/core/src/main/java/com/patrykandpatrick/vico/core/cartesian/layer/ColumnCartesianLayer.kt#L130 and then immediately checks https://github.com/patrykandpatrick/vico/blob/7ead489e30b4f36947ed0b9d76d5fa8c373d8186/vico/core/src/main/java/com/patrykandpatrick/vico/core/cartesian/layer/ColumnCartesianLayer.kt#L131
However sometimes this calculation, due to floating point rounding errors, can lead to unwanted results, in my case,
entry.x
was 1.9,chartValues,minX
was 1.35 andchartValues.xStep
is 0.01. This means thatxSpacingMultiplier
comes out being 54.999996 instead of the 55 and the checks fails.Expected behavior
I would expect rounding issues to be taken into account in this calculation to prevent errors like these.
Vico version(s)
2.0.0-alpha.19
Android version(s)
Android 14.0
Additional information
No response