diff --git a/lib/java/com/google/android/material/slider/BaseSlider.java b/lib/java/com/google/android/material/slider/BaseSlider.java index 57d4a4abac8..f551d49cb3b 100644 --- a/lib/java/com/google/android/material/slider/BaseSlider.java +++ b/lib/java/com/google/android/material/slider/BaseSlider.java @@ -114,6 +114,7 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.math.BigDecimal; +import java.math.RoundingMode; import java.text.NumberFormat; import java.text.ParseException; import java.util.ArrayList; @@ -727,6 +728,32 @@ private boolean isMultipleOfStepSize(double value) { return Math.abs(Math.round(result) - result) < THRESHOLD; } + /** + * Returns the number of steps between {@code valueFrom} and {@code valueTo}, dividing on the + * decimal values because {@code (valueTo - valueFrom) / stepSize} can land just below a whole + * number in float arithmetic. valueFrom=-1.5, valueTo=3.7 and stepSize=0.1 is 52 steps, but the + * float division gives 51.999996 and casting it to an int silently drops the last tick. + */ + private long getStepCount() { + return new BigDecimal(Float.toString(valueTo)) + .subtract(new BigDecimal(Float.toString(valueFrom)), DECIMAL64) + .divide(new BigDecimal(Float.toString(stepSize)), DECIMAL64) + .setScale(0, RoundingMode.HALF_UP) + .longValue(); + } + + /** + * Returns the value of the tick at {@code stepIndex}. Like {@link #valueLandsOnTick(float)} this + * works on the decimal values, so the result lands exactly on the tick instead of drifting. + */ + private float getStepValue(long stepIndex) { + return new BigDecimal(Float.toString(valueFrom)) + .add( + new BigDecimal(Float.toString(stepSize)).multiply(BigDecimal.valueOf(stepIndex)), + DECIMAL64) + .floatValue(); + } + private void validateStepSize() { if (stepSize > 0.0f && !valueLandsOnTick(valueTo)) { throw new IllegalStateException( @@ -3482,7 +3509,7 @@ private void resetThumbWidth() { private double snapPosition(float position) { if (stepSize > 0.0f) { - int stepCount = (int) ((valueTo - valueFrom) / stepSize); + long stepCount = getStepCount(); return Math.round(position * stepCount) / (double) stepCount; } @@ -3624,6 +3651,13 @@ private float getValueOfTouchPosition() { if (isRtl() || isVertical()) { position = 1 - position; } + + if (stepSize > 0.0f) { + // Scaling the snapped position back up by the value range loses the tick alignment in float + // arithmetic: valueFrom=0.2, valueTo=4.0 and stepSize=0.05 lands on 2.6499999 rather than + // 2.65. Rebuild the value from the tick index instead. + return getStepValue(Math.round(position * getStepCount())); + } return (float) (position * (valueTo - valueFrom) + valueFrom); } diff --git a/lib/javatests/com/google/android/material/slider/SliderRoundingErrorTest.java b/lib/javatests/com/google/android/material/slider/SliderRoundingErrorTest.java index 7d25aa8fcc7..f59b9ed384d 100644 --- a/lib/javatests/com/google/android/material/slider/SliderRoundingErrorTest.java +++ b/lib/javatests/com/google/android/material/slider/SliderRoundingErrorTest.java @@ -64,4 +64,45 @@ public void testKnownValues_snapTouchToValue_NoRoundingError() { assertThat(slider.getValue()).isEqualTo(i); } } + + @Test + public void testFractionalStepSize_snapTouchToValue_valueLandsExactlyOnTick() { + slider.setValueFrom(0.2f); + slider.setValueTo(4f); + slider.setStepSize(0.05f); + slider.setValues(1f); + + touchSliderAtValue(slider, 1f, MotionEvent.ACTION_DOWN); + shadowOf(getMainLooper()).idle(); + + // These came back as 1.4499999, 1.6999999, ... because the snapped position was scaled back + // up by the value range instead of being counted from valueFrom in whole steps. + float[] ticks = {1.45f, 1.7f, 1.95f, 2.4f, 2.65f, 2.9f, 3.15f, 3.4f, 3.65f, 3.9f}; + for (float tick : ticks) { + touchSliderAtValue(slider, tick, MotionEvent.ACTION_MOVE); + shadowOf(getMainLooper()).idle(); + assertThat(slider.getValue()).isEqualTo(tick); + } + } + + @Test + public void testFractionalRange_snapTouchToValue_stepCountIsNotTruncated() { + slider.setValueFrom(-1.5f); + slider.setValueTo(3.7f); + slider.setStepSize(0.1f); + slider.setValues(-1.5f); + + touchSliderAtValue(slider, -1.5f, MotionEvent.ACTION_DOWN); + shadowOf(getMainLooper()).idle(); + + // This range is exactly 52 steps, but (3.7 - -1.5) / 0.1 comes out as 51.999996 in float + // arithmetic, so the step count used to be truncated to 51 and every value landed off the + // 0.1 grid. Touching -1 used to give -0.9901961 and valueTo was never reachable. + float[] ticks = {-1f, -0.5f, 0f, 0.5f, 1f, 2.5f, 3.7f}; + for (float tick : ticks) { + touchSliderAtValue(slider, tick, MotionEvent.ACTION_MOVE); + shadowOf(getMainLooper()).idle(); + assertThat(slider.getValue()).isEqualTo(tick); + } + } }