Skip to content

Commit 68e4ba4

Browse files
chipweinbergerchristopherfujino
authored andcommitted
[Velocity Tracker] Fix: Issue 97761: Flutter Scrolling does not match iOS; inadvertent scrolling when user lifts up finger (flutter#132291)
## Issue **Issue:** flutter#97761 https://github.com/flutter/flutter/assets/1863934/53c5e0df-b85a-483c-a17d-bddd18db3aa9 ## The Cause: The bug is very simple to understand - `velocity_tracker.dart` **only adds new samples while your finger is moving**. **Therefore**, if you move your finger quickly & (important) stop suddenly with no extra movement, the last 3 samples will all be > 0 dy. Regardless of how long you wait, you will get movement when you lift up your finger. **Logs from velocity_tracker.dart:** Notice: all 3 `_previousVelocityAt` are `dy > 0` despite a 2 second delay since the last scroll ``` // start moving finger flutter: addPosition dy:-464.0 flutter: addPosition dy:-465.0 flutter: addPosition dy:-466.0 flutter: addPosition dy:-467.0 flutter: addPosition dy:-468.0 flutter: addPosition dy:-469.0 flutter: addPosition dy:-470.0 // stop moving finger here, keep it still for 2 seconds & lift it up flutter: _previousVelocityAt(-2) samples(-467.0, -468.0)) dy:-176.772140710624 flutter: _previousVelocityAt(-1) samples(-468.0, -469.0)) dy:-375.0937734433609 flutter: _previousVelocityAt(0) samples(-469.0, -470.0)) dy:-175.71604287471447 flutter: primaryVelocity DragEndDetails(Velocity(0.0, -305.5)).primaryVelocity flutter: createBallisticSimulation pixels 464.16666666666663 velocity 305.4699824197211 ``` ## The Fix **There are 3 options to fix it:** A. sample uniformly *per unit time* (a larger more risky change, hurts battery life) B. consider elapsed time since the last sample. If greater than X, assume no more velocity. (easy & just as valid) C. similar to B, but instead add "ghost samples" of velocity zero, and run calculations as normal (a bit tricker, of dubious benefit imo) **For Option B I considered two approaches:** 1. _get the current timestamp and compare to event timestamp._ This is tricky because events are documented to use an arbitrary timescale & I wasn't able to find the code that generates the timestamps. This approach could be considered more. 2. _get a new timestamp using Stopwatch and compare now vs when the last sample was added._ This is the solution implemented here. There is a limitation in that we don't know when addSamples is called relative to the event. But, this estimation is already on a very low latency path & still it gives us a *minimum* time bound which is sufficient for comparison. **This PR chooses the simplest of the all solutions. Please try it our yourself, it completely solves the problem �** Option _B.1_ would be a nice alternative as well, if we can define and access the same timesource as the pointer tracker in a maintainable simple way. ## After Fix https://github.com/flutter/flutter/assets/1863934/be50d8e7-d5da-495a-a4af-c71bc541cbe3
1 parent 722b9a1 commit 68e4ba4

File tree

2 files changed

+56
-1
lines changed

2 files changed

+56
-1
lines changed

packages/flutter/lib/src/gestures/velocity_tracker.dart

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,12 +149,17 @@ class VelocityTracker {
149149
/// The kind of pointer this tracker is for.
150150
final PointerDeviceKind kind;
151151

152+
// Time difference since the last sample was added
153+
final Stopwatch _sinceLastSample = Stopwatch();
154+
152155
// Circular buffer; current sample at _index.
153156
final List<_PointAtTime?> _samples = List<_PointAtTime?>.filled(_historySize, null);
154157
int _index = 0;
155158

156159
/// Adds a position as the given time to the tracker.
157160
void addPosition(Duration time, Offset position) {
161+
_sinceLastSample.start();
162+
_sinceLastSample.reset();
158163
_index += 1;
159164
if (_index == _historySize) {
160165
_index = 0;
@@ -169,6 +174,16 @@ class VelocityTracker {
169174
///
170175
/// Returns null if there is no data on which to base an estimate.
171176
VelocityEstimate? getVelocityEstimate() {
177+
// no recent user movement?
178+
if (_sinceLastSample.elapsedMilliseconds > VelocityTracker._assumePointerMoveStoppedMilliseconds) {
179+
return const VelocityEstimate(
180+
pixelsPerSecond: Offset.zero,
181+
confidence: 1.0,
182+
duration: Duration.zero,
183+
offset: Offset.zero,
184+
);
185+
}
186+
172187
final List<double> x = <double>[];
173188
final List<double> y = <double>[];
174189
final List<double> w = <double>[];
@@ -195,7 +210,7 @@ class VelocityTracker {
195210
final double age = (newestSample.time - sample.time).inMicroseconds.toDouble() / 1000;
196211
final double delta = (sample.time - previousSample.time).inMicroseconds.abs().toDouble() / 1000;
197212
previousSample = sample;
198-
if (age > _horizonMilliseconds || delta > _assumePointerMoveStoppedMilliseconds) {
213+
if (age > _horizonMilliseconds || delta > VelocityTracker._assumePointerMoveStoppedMilliseconds) {
199214
break;
200215
}
201216

@@ -288,6 +303,8 @@ class IOSScrollViewFlingVelocityTracker extends VelocityTracker {
288303

289304
@override
290305
void addPosition(Duration time, Offset position) {
306+
_sinceLastSample.start();
307+
_sinceLastSample.reset();
291308
assert(() {
292309
final _PointAtTime? previousPoint = _touchSamples[_index];
293310
if (previousPoint == null || previousPoint.time <= time) {
@@ -326,6 +343,16 @@ class IOSScrollViewFlingVelocityTracker extends VelocityTracker {
326343

327344
@override
328345
VelocityEstimate getVelocityEstimate() {
346+
// no recent user movement?
347+
if (_sinceLastSample.elapsedMilliseconds > VelocityTracker._assumePointerMoveStoppedMilliseconds) {
348+
return const VelocityEstimate(
349+
pixelsPerSecond: Offset.zero,
350+
confidence: 1.0,
351+
duration: Duration.zero,
352+
offset: Offset.zero,
353+
);
354+
}
355+
329356
// The velocity estimated using this expression is an approximation of the
330357
// scroll velocity of an iOS scroll view at the moment the user touch was
331358
// released, not the final velocity of the iOS pan gesture recognizer
@@ -387,6 +414,16 @@ class MacOSScrollViewFlingVelocityTracker extends IOSScrollViewFlingVelocityTrac
387414

388415
@override
389416
VelocityEstimate getVelocityEstimate() {
417+
// no recent user movement?
418+
if (_sinceLastSample.elapsedMilliseconds > VelocityTracker._assumePointerMoveStoppedMilliseconds) {
419+
return const VelocityEstimate(
420+
pixelsPerSecond: Offset.zero,
421+
confidence: 1.0,
422+
duration: Duration.zero,
423+
offset: Offset.zero,
424+
);
425+
}
426+
390427
// The velocity estimated using this expression is an approximation of the
391428
// scroll velocity of a macOS scroll view at the moment the user touch was
392429
// released.

packages/flutter/test/gestures/velocity_tracker_test.dart

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,4 +144,22 @@ void main() {
144144
}
145145
}
146146
});
147+
148+
test('Assume zero velocity when there are no recent samples', () async {
149+
final IOSScrollViewFlingVelocityTracker tracker = IOSScrollViewFlingVelocityTracker(PointerDeviceKind.touch);
150+
Offset position = Offset.zero;
151+
Duration time = Duration.zero;
152+
const Offset positionDelta = Offset(0, -1);
153+
const Duration durationDelta = Duration(seconds: 1);
154+
155+
for (int i = 0; i < 10; i+=1) {
156+
position += positionDelta;
157+
time += durationDelta;
158+
tracker.addPosition(time, position);
159+
}
160+
161+
await Future<void>.delayed(const Duration(milliseconds: 50));
162+
163+
expect(tracker.getVelocity().pixelsPerSecond, Offset.zero);
164+
});
147165
}

0 commit comments

Comments
 (0)