Fix discrete Slider snapping to wrong value when tapping lower-half tick marks by hkarmoush · Pull Request #188012 · flutter/flutter

hkarmoush · 2026-06-15T10:53:20Z

Two independent bugs caused a discrete Slider (or RangeSlider) to snap to the wrong division when the user tapped a lower-half tick mark:

Bug 1 — Coordinate system mismatch (dominant cause)

Tick marks and the thumb are painted in a padded coordinate space:

dx = trackLeft + i/d * (trackWidth − padding) + padding/2

where padding = trackHeight (4 px on M2's RoundedRectSliderTrackShape, up to 16 px on M3's GappedSliderTrackShape). But tap detection inverted a different, unpadded formula, so the snap midpoints in tap space diverged from the visual midpoints between tick marks. Lower-half ticks consistently landed in the wrong snap bucket.

Fix: _getValueFromGlobalPosition now uses the padded inverse formula when isDiscrete && trackShape.isRounded. The drag-delta normalizer in _handleDragUpdate is updated to match so a tap followed immediately by a drag is consistent.

Bug 2 — IEEE 754 precision in _discretize

0.35 * 10 = 3.4999…9 in double arithmetic, so .round() returns 3 instead of 4 at the exact midpoint between ticks 3 and 4. A 1e-10 epsilon guard before rounding corrects this without biasing values that are genuinely below the midpoint.

Both fixes are applied to Slider and RangeSlider.

Pre-launch Checklist

google-cla · 2026-06-15T10:53:36Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

gemini-code-assist

Code Review

This pull request updates the discrete slider and range slider implementations to ensure tap and drag coordinates align correctly on rounded tracks, and adds a small epsilon before rounding to prevent IEEE 754 precision errors. It also introduces corresponding regression tests. The review feedback recommends updating the comment style of a private helper class in the test file to use triple-slash documentation comments (///) instead of double-slash comments (//), adhering to the repository's style guide.

gemini-code-assist · 2026-06-15T10:54:57Z

+// Records the global center of each tick mark as it is painted.
+// Used in regression tests for https://github.com/flutter/flutter/issues/184391
+// to obtain actual tick pixel positions without replicating track-geometry math.


According to the Flutter Style Guide, public-quality documentation should use /// instead of //, even for private members.

/// Records the global center of each tick mark as it is painted. /// /// Used in regression tests for https://github.com/flutter/flutter/issues/184391 /// to obtain actual tick pixel positions without replicating track-geometry math.

References

Use /// for public-quality documentation, even on private members. ^(link)

Piinks · 2026-06-24T19:28:50Z

I've marked this PR as not ready to port to flutter/packages yet.
We'll provide instructions to move this change over to material_ui/cupertino_ui once ready to receive PRs. Thank you!

…ick marks Fixes flutter#184391 Two bugs caused discrete Slider taps to land on the wrong division: 1. Coordinate system mismatch: tick marks and the thumb are painted at `trackLeft + i/d * (trackWidth - padding) + padding/2`, but tap detection inverted a different, unpadded formula. On rounded tracks (RoundedRectSliderTrackShape, GappedSliderTrackShape) this pushes lower-half tap midpoints into the wrong snap bucket. The mismatch is 4 px on M2 and up to 16 px on M3. Fix: _getValueFromGlobalPosition now uses the same padded formula when isDiscrete && trackShape.isRounded. The drag-delta normalizer in _handleDragUpdate is updated to match so tap-then-drag is consistent. 2. IEEE 754 precision: 0.35 * 10 = 3.4999…9 in double arithmetic, so .round() returns 3 instead of 4 at the midpoint between ticks 3 and 4. A 1e-10 epsilon guard in _discretize corrects this without biasing values that are genuinely below the midpoint. Both fixes are applied to Slider and RangeSlider. Five regression tests are added to slider_test.dart covering M2 LTR, M3 LTR, RTL, drag continuity, and the IEEE 754 midpoint case.

dkwingsmt

Thanks for your investigation. I'm generally ok with the approach. But can you provide more details on why you think the IEEE 754 precision is an issue? It should be almost impossible for regular users to touch at exactly 3.5, and they shouldn't notice or complain about it even if they do.

dkwingsmt · 2026-07-18T02:10:00Z

  double _getValueFromGlobalPosition(Offset globalPosition) {
-    final double visualPosition =
-        (globalToLocal(globalPosition).dx - _trackRect.left) / _trackRect.width;
+    final double localDx = globalToLocal(globalPosition).dx;
+    final double visualPosition;
+    // Tick marks and the thumb are painted in an adjusted coordinate space that
+    // accounts for track height padding on discrete rounded tracks:
+    //   dx = trackLeft + value * (trackWidth - padding) + padding/2
+    // Invert that same formula here so tap coordinates align with the visual
+    // tick positions. Without this, the snap midpoints in tap space diverge
+    // from the visual midpoints between tick marks, biasing lower-half taps
+    // toward the wrong (lower) division.
+    if (isDiscrete && _sliderTheme.trackShape!.isRounded) {
+      final double padding = _trackRect.height;
+      final double adjustedWidth = _trackRect.width - padding;
+      if (adjustedWidth <= 0.0) {
+        visualPosition = 0.5;
+      } else {
+        visualPosition = (localDx - _trackRect.left - padding / 2) / adjustedWidth;
+      }
+    } else {
+      visualPosition = (localDx - _trackRect.left) / _trackRect.width;
+    }
    return _getValueFromVisualPosition(visualPosition);
  }


I think this logic is over-complicated. I propose simplifying it to follows, with additional doc:

// Converts a global screen coordinate into a normalized visual position (0.0 to 1.0). // // For discrete rounded tracks, tick marks and thumb centers are painted with // padding equal to `trackHeight` (half-height padding on each end): // ```dart // dx = trackLeft + padding / 2 + visualPosition * (trackWidth - padding) // ``` // Inverting this formula aligns tap coordinates with visual tick mark centers, // preventing tap midpoints from diverging from visual midpoints. // // For non-discrete or non-rounded tracks, `padding` is 0.0, mapping directly // across `trackWidth`. // // Returns 0.5 (neutral midpoint) when `adjustedWidth` is non-positive to // prevent division by zero. double _getValueFromGlobalPosition(Offset globalPosition) { final double localDx = globalToLocal(globalPosition).dx; final double padding = (isDiscrete && _sliderTheme.trackShape!.isRounded) ? _trackRect.height : 0.0; final double adjustedWidth = _trackRect.width - padding; final double visualPosition = adjustedWidth <= 0.0 ? 0.5 : (localDx - _trackRect.left - padding / 2) / adjustedWidth; return _getValueFromVisualPosition(visualPosition); }

Done — extracted the padding calc into _discreteRoundedTrackPadding and flattened this method as suggested (dbd277b).

On the IEEE 754 question: the regression test taps the actual pixel midpoint between ticks 3 and 4, which computes to 0.34999999999999997 (not 0.35) from real screen-coordinate math, not an idealized decimal. Without the epsilon that rounds down to the wrong tick — and tapping the midpoint between two ticks is a completely normal interaction, not an edge case.

dkwingsmt · 2026-07-18T02:10:51Z

+          final double effectiveWidth = (isDiscrete && _sliderTheme.trackShape!.isRounded)
+              ? _trackRect.width - _trackRect.height
+              : _trackRect.width;


It seems to me that we can extract

final double padding = (isDiscrete && _sliderTheme.trackShape!.isRounded) ? _trackRect.height : 0.0;

to be a separate getter.

Done in dbd277b — _discreteRoundedTrackPadding now backs both call sites.

Extracts the repeated isDiscrete/isRounded padding calculation into a single _discreteRoundedTrackPadding getter and flattens the branching in _getValueFromGlobalPosition, per dkwingsmt's review suggestion.

dkwingsmt · 2026-07-23T17:40:33Z

I experimented by removing the three cases of + 1e-10 and running slider_test.dart and I see no test cases failing. Can you be more specific on which regression test (or maybe platform?) you're noticing the failure?

Also, while tapping any point on the slider is possible for a user, the user won't notice whether they've tapped 3.499999 or 3.500001, therefore behavior around it isn't particularly important, making the 1e-10 offset appearing arbitrary.

…pping The IEEE-754 comment was wrong (0.35 * 10 is exactly 3.5, not 3.4999...), and the regression test tapped a tick pair (3-4) that isn't actually epsilon-sensitive on this geometry, so it passed identically with or without the fix. The real effect is upstream float error in the pixel-to-fraction conversion landing a few ULPs below an exact tie between two divisions. Rewrote the comments to describe that, dropped the redundant clampDouble wrapper, and replaced the test with one that loops over every tick midpoint and genuinely fails without the epsilon.

hkarmoush · 2026-07-25T07:17:46Z

You're right, and I dug into why: 0.35 * 10 is actually exactly 3.5 in IEEE-754, so that comment was just wrong. The real effect is different — converting a tapped pixel to a value can land a few ULPs below an exact tie between two ticks, which then rounds down instead of up. It's narrow (only matters if you tap the exact midpoint pixel) but it's real — I rewrote the comment and swapped the regression test for one that actually catches it (the old one happened to test a tick pair that wasn't affected). Pushed the fix.

dkwingsmt

I still don't think the behavior at exactly x.5 matters. Whether it rounds up or down doesn't affect practical user experience, meanwhile it complicates the logic. It's not what the original issue complained about, and is not checked by any current unit tests. If you still think this is a real problem, I'd prefer filing a new issue to describe it, which will make the subsequent PR clearer.

I do appreciate the remaining fix as well as the new unit tests, especially the loop test that verifies the snapping behavior comprehensively - it's essential to discrete sliders!

I recommend removing the changes of + 1e-10 and changing the test case Discrete Slider tapping the exact midpoint between two ticks snaps up to checking x.1 values and x.9 values.

dkwingsmt pointed out the +1e-10 epsilon on exact tick-midpoint ties is unverified by any test and imperceptible to users, and asked for it to be dropped in favor of covering near-tick taps instead. Reworks the midpoint regression test to tap 10%/90% of the way between ticks rather than the exact midpoint, which exercises the snapping behavior without depending on tie-breaking rounding.

hkarmoush · 2026-07-28T15:47:05Z

@dkwingsmt Removed the + 1e-10 epsilon from all three _discretize call sites in 25b4c97, and reworked the exact-midpoint regression test to tap 10%/90% of the way between adjacent ticks instead of the exact tie — that exercises the snapping behavior without depending on tie-breaking rounding. All 175 slider/range-slider tests pass and dart analyze is clean.

Thanks for pushing on this — you're right that it wasn't worth the complexity for a case no user can perceive.

dkwingsmt

LGTM. Thank you!

AbdeMohlbi · 2026-07-30T10:23:33Z

+  );
+}
+
+/// Records the global center of each tick mark as it is painted.


nit: AFAIK we do not usually explain why a certain helper class is used in tests, @dkwingsmt what do you think about that ?

I'd say it's a bit excessive to mention the issue name twice here but I don't mind documenting the helper class. Sometimes it can take a while to understand the helper classes to understand the test cases. :)

QuncCccccc · 2026-08-12T05:09:41Z

+
+      await tester.pumpWidget(
+        MaterialApp(
+          theme: ThemeData(useMaterial3: true),


nit: this is the default value so we can remove this line.

Suggested change

theme: ThemeData(useMaterial3: true),

QuncCccccc · 2026-08-12T05:11:21Z

+
+      await tester.pumpWidget(
+        MaterialApp(
+          theme: ThemeData(useMaterial3: false),


Do we only test M2? Actually M3 might be more important to test since M2 has become a out of dated style. We can remove this line as well to test the default value - M3.

Suggested change

theme: ThemeData(useMaterial3: false),

QuncCccccc · 2026-08-12T05:11:35Z

+
+      await tester.pumpWidget(
+        MaterialApp(
+          theme: ThemeData(useMaterial3: false),


QuncCccccc · 2026-08-12T05:12:15Z

+
+      await tester.pumpWidget(
+        MaterialApp(
+          theme: ThemeData(useMaterial3: false),


Suggested change

theme: ThemeData(useMaterial3: false),

QuncCccccc

Could you also update PR description to reflect the latest change?

flutter-dashboard · 2026-08-24T11:37:31Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #188012 at sha 25b4c97

github-actions · 2026-08-24T11:37:39Z

This pull request contains changes to Material or Cupertino, which are currently frozen in this repository.

Changes should be made in material_ui and/or cupertino_ui in the flutter/packages repository.

Please refer to #188444 for instructions.

Piinks · 2026-08-24T16:37:06Z

Hi @hkarmoush this PR is now ready to port over to material_ui.
See guidance and instructions in #188444
Once the PR is created in flutter/packages, this can be closed.

flutter-dashboard · 2026-09-13T16:43:37Z

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

hkarmoush · 2026-09-18T14:24:04Z

Ported to `material_ui` in flutter/packages#12927 now that the Material/Cupertino code freeze has lifted. Closing this in favor of that PR — see #188444 for the porting process.

…ick marks Port of flutter/flutter#188012. Tick marks and the thumb on discrete rounded tracks are painted in a coordinate space padded by half a track height on each end, but the gesture math used an unpadded formula, so lower-half tick taps could snap to the wrong (lower) division. Mirrors the padded geometry in _getValueFromGlobalPosition (Slider and RangeSlider) and in the drag-delta normalizer so tap and drag stay in the same coordinate space.

… the track ends (#12927) Ports flutter/flutter#188012 to `material_ui` now that the Material/Cupertino code freeze has lifted, per the instructions in flutter/flutter#188444. Original PR description: --- Fixes flutter/flutter#194855 Tick marks and the thumb on discrete rounded tracks are *painted* in a padded coordinate space: ``` dx = trackLeft + i/d * (trackWidth − padding) + padding/2 ``` where `padding = trackHeight` (4 px on M2's `RoundedRectSliderTrackShape`, up to 16 px on M3's `GappedSliderTrackShape`). But tap detection inverted a different, unpadded formula, so the snap midpoints in tap space diverged from the visual midpoints between tick marks. Lower-half ticks consistently landed in the wrong snap bucket. Fix: `_getValueFromGlobalPosition` now uses the padded inverse formula when `isDiscrete && trackShape.isRounded`. The drag-delta normalizer in `_handleDragUpdate` is updated to match so a tap followed immediately by a drag is consistent. Applied to both `Slider` and `RangeSlider`. ## Tests Added regression tests to `slider_test.dart` and `range_slider_test.dart`. They fail without the fix and pass with it: - `Slider` snaps at the visual midpoint between every pair of ticks, tapping 0.05px either side of the midpoint, for the default Material 3 track (LTR and RTL) and the gapped track from `Slider.year2023: false` (LTR and RTL), which has the largest padding. Tapping exactly on a tick center can't expose the bug, because the old and new mappings differ by only a couple of pixels there. - A full-track drag after a tap on a 100-division gapped slider, which covers the drag delta normalizer. - `RangeSlider` snaps at the visual midpoint between ticks for both thumbs. - A tick-center sanity check for `Slider`. All of `slider_test.dart` and `range_slider_test.dart` pass locally. --- Fixes flutter/flutter#194855 Closes flutter/flutter#188012 (superseded by this PR) ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.

github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages labels Jun 15, 2026

gemini-code-assist Bot reviewed Jun 15, 2026

View reviewed changes

hkarmoush force-pushed the fix/184391-slider-discrete-tap-wrong-value branch 2 times, most recently from 32b6601 to 822cd1c Compare June 15, 2026 11:08

hkarmoush mentioned this pull request Jun 15, 2026

[flutter_tools] fix: don't rebuild unit_test_assets when nothing changed #187751

Closed

10 tasks done

dkwingsmt self-requested a review June 17, 2026 18:19

Piinks added the Decoupling: Not ready to port yet Instructions will be provided when this is ready to move to flutter/packages. label Jun 24, 2026

hkarmoush force-pushed the fix/184391-slider-discrete-tap-wrong-value branch 2 times, most recently from 2768ebe to 82cd056 Compare July 15, 2026 11:47

dkwingsmt reviewed Jul 18, 2026

View reviewed changes

hkarmoush added 2 commits July 23, 2026 11:43

Simplify discrete-rounded-track padding logic per review feedback

dbd277b

Extracts the repeated isDiscrete/isRounded padding calculation into a single _discreteRoundedTrackPadding getter and flattens the branching in _getValueFromGlobalPosition, per dkwingsmt's review suggestion.

Merge branch 'master' into fix/184391-slider-discrete-tap-wrong-value

6bf982f

hkarmoush and others added 2 commits July 27, 2026 02:26

Merge branch 'master' into fix/184391-slider-discrete-tap-wrong-value

2c4bfa6

Merge branch 'master' into fix/184391-slider-discrete-tap-wrong-value

391219c

dkwingsmt requested changes Jul 27, 2026

View reviewed changes

dkwingsmt self-requested a review July 28, 2026 17:33

dkwingsmt added the CICD Run CI/CD label Jul 28, 2026

dkwingsmt approved these changes Jul 28, 2026

View reviewed changes

dkwingsmt requested a review from QuncCccccc July 29, 2026 18:19

AbdeMohlbi reviewed Jul 30, 2026

View reviewed changes

QuncCccccc reviewed Aug 12, 2026

View reviewed changes

flutter-dashboard Bot added the will affect goldens Changes to golden files label Aug 24, 2026

Piinks added Decoupling: Port to flutter/packages This PR is ready to be ported to flutter/packages. We will provide instructions to do so. and removed Decoupling: Not ready to port yet Instructions will be provided when this is ready to move to flutter/packages. labels Aug 24, 2026

hkarmoush mentioned this pull request Sep 18, 2026

[material_ui] Fix discrete Slider selecting the neighboring tick near the track ends flutter/packages#12927

Merged

8 of 11 tasks

hkarmoush closed this Sep 18, 2026

Conversation

hkarmoush commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

google-cla Bot commented Jun 15, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

Piinks commented Jun 24, 2026

Uh oh!

dkwingsmt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkwingsmt commented Jul 23, 2026

Uh oh!

hkarmoush commented Jul 25, 2026

Uh oh!

dkwingsmt left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hkarmoush commented Jul 28, 2026

Uh oh!

dkwingsmt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

QuncCccccc left a comment

Choose a reason for hiding this comment

Uh oh!

flutter-dashboard Bot commented Aug 24, 2026

Uh oh!

github-actions Bot commented Aug 24, 2026

Uh oh!

Piinks commented Aug 24, 2026

Uh oh!

flutter-dashboard Bot commented Sep 13, 2026

Uh oh!

hkarmoush commented Sep 18, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hkarmoush commented Jun 15, 2026 •

edited

Loading

dkwingsmt left a comment •

edited

Loading