Fix interference in fling-scrolling from cross-axis motion (flutter#122338)

Fix interference in fling-scrolling from cross-axis motion

Author: gnprice

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

@@ -216,14 +216,18 @@ typedef GestureDragUpdateCallback = void Function(DragUpdateDetails details);
 class DragEndDetails {
   /// Creates details for a [GestureDragEndCallback].
   ///
+  /// If [primaryVelocity] is non-null, its value must match one of the
+  /// coordinates of `velocity.pixelsPerSecond` and the other coordinate
+  /// must be zero.
+  ///
   /// The [velocity] argument must not be null.
   DragEndDetails({
     this.velocity = Velocity.zero,
     this.primaryVelocity,
   }) : assert(
          primaryVelocity == null
-           || primaryVelocity == velocity.pixelsPerSecond.dx
-           || primaryVelocity == velocity.pixelsPerSecond.dy,
+           || (primaryVelocity == velocity.pixelsPerSecond.dx && velocity.pixelsPerSecond.dy == 0)
+           || (primaryVelocity == velocity.pixelsPerSecond.dy && velocity.pixelsPerSecond.dx == 0),
        );
 
   /// The velocity the pointer was moving when it stopped contacting the screen.

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

@@ -147,6 +147,14 @@ abstract class DragGestureRecognizer extends OneSequenceGestureRecognizer {
   /// The distance traveled by the pointer since the last update is provided in
   /// the callback's `details` argument, which is a [DragUpdateDetails] object.
   ///
+  /// If this gesture recognizer recognizes movement on a single axis (a
+  /// [VerticalDragGestureRecognizer] or [HorizontalDragGestureRecognizer]),
+  /// then `details` will reflect movement only on that axis and its
+  /// [DragUpdateDetails.primaryDelta] will be non-null.
+  /// If this gesture recognizer recognizes movement in all directions
+  /// (a [PanGestureRecognizer]), then `details` will reflect movement on
+  /// both axes and its [DragUpdateDetails.primaryDelta] will be null.
+  ///
   /// See also:
   ///
   ///  * [allowedButtonsFilter], which decides which button will be allowed.
@@ -162,6 +170,14 @@ abstract class DragGestureRecognizer extends OneSequenceGestureRecognizer {
   /// The velocity is provided in the callback's `details` argument, which is a
   /// [DragEndDetails] object.
   ///
+  /// If this gesture recognizer recognizes movement on a single axis (a
+  /// [VerticalDragGestureRecognizer] or [HorizontalDragGestureRecognizer]),
+  /// then `details` will reflect movement only on that axis and its
+  /// [DragEndDetails.primaryVelocity] will be non-null.
+  /// If this gesture recognizer recognizes movement in all directions
+  /// (a [PanGestureRecognizer]), then `details` will reflect movement on
+  /// both axes and its [DragEndDetails.primaryVelocity] will be null.
+  ///
   /// See also:
   ///
   ///  * [allowedButtonsFilter], which decides which button will be allowed.
@@ -258,6 +274,13 @@ abstract class DragGestureRecognizer extends OneSequenceGestureRecognizer {
   /// inertia, for example.
   bool isFlingGesture(VelocityEstimate estimate, PointerDeviceKind kind);
 
+  /// Determines if a gesture is a fling or not, and if so its effective velocity.
+  ///
+  /// A fling calls its gesture end callback with a velocity, allowing the
+  /// provider of the callback to respond by carrying the gesture forward with
+  /// inertia, for example.
+  DragEndDetails? _considerFling(VelocityEstimate estimate, PointerDeviceKind kind);
+
   Offset _getDeltaForDetails(Offset delta);
   double? _getPrimaryValueFromOffset(Offset value);
   bool _hasSufficientGlobalDistanceToAccept(PointerDeviceKind pointerDeviceKind, double? deviceTouchSlop);
@@ -504,33 +527,21 @@ abstract class DragGestureRecognizer extends OneSequenceGestureRecognizer {
     }
 
     final VelocityTracker tracker = _velocityTrackers[pointer]!;
+    final VelocityEstimate? estimate = tracker.getVelocityEstimate();
 
-    final DragEndDetails details;
+    DragEndDetails? details;
     final String Function() debugReport;
-
-    final VelocityEstimate? estimate = tracker.getVelocityEstimate();
-    if (estimate != null && isFlingGesture(estimate, tracker.kind)) {
-      final Velocity velocity = Velocity(pixelsPerSecond: estimate.pixelsPerSecond)
-        .clampMagnitude(minFlingVelocity ?? kMinFlingVelocity, maxFlingVelocity ?? kMaxFlingVelocity);
-      details = DragEndDetails(
-        velocity: velocity,
-        primaryVelocity: _getPrimaryValueFromOffset(velocity.pixelsPerSecond),
-      );
-      debugReport = () {
-        return '$estimate; fling at $velocity.';
-      };
+    if (estimate == null) {
+      debugReport = () => 'Could not estimate velocity.';
     } else {
-      details = DragEndDetails(
-        primaryVelocity: 0.0,
-      );
-      debugReport = () {
-        if (estimate == null) {
-          return 'Could not estimate velocity.';
-        }
-        return '$estimate; judged to not be a fling.';
-      };
+      details = _considerFling(estimate, tracker.kind);
+      debugReport = (details != null)
+        ? () => '$estimate; fling at ${details!.velocity}.'
+        : () => '$estimate; judged to not be a fling.';
     }
-    invokeCallback<void>('onEnd', () => onEnd!(details), debugReport: debugReport);
+    details ??= DragEndDetails(primaryVelocity: 0.0);
+
+    invokeCallback<void>('onEnd', () => onEnd!(details!), debugReport: debugReport);
   }
 
   void _checkCancel() {
@@ -578,6 +589,19 @@ class VerticalDragGestureRecognizer extends DragGestureRecognizer {
     return estimate.pixelsPerSecond.dy.abs() > minVelocity && estimate.offset.dy.abs() > minDistance;
   }
 
+  @override
+  DragEndDetails? _considerFling(VelocityEstimate estimate, PointerDeviceKind kind) {
+    if (!isFlingGesture(estimate, kind)) {
+      return null;
+    }
+    final double maxVelocity = maxFlingVelocity ?? kMaxFlingVelocity;
+    final double dy = clampDouble(estimate.pixelsPerSecond.dy, -maxVelocity, maxVelocity);
+    return DragEndDetails(
+      velocity: Velocity(pixelsPerSecond: Offset(0, dy)),
+      primaryVelocity: dy,
+    );
+  }
+
   @override
   bool _hasSufficientGlobalDistanceToAccept(PointerDeviceKind pointerDeviceKind, double? deviceTouchSlop) {
     return _globalDistanceMoved.abs() > computeHitSlop(pointerDeviceKind, gestureSettings);
@@ -620,6 +644,19 @@ class HorizontalDragGestureRecognizer extends DragGestureRecognizer {
     return estimate.pixelsPerSecond.dx.abs() > minVelocity && estimate.offset.dx.abs() > minDistance;
   }
 
+  @override
+  DragEndDetails? _considerFling(VelocityEstimate estimate, PointerDeviceKind kind) {
+    if (!isFlingGesture(estimate, kind)) {
+      return null;
+    }
+    final double maxVelocity = maxFlingVelocity ?? kMaxFlingVelocity;
+    final double dx = clampDouble(estimate.pixelsPerSecond.dx, -maxVelocity, maxVelocity);
+    return DragEndDetails(
+      velocity: Velocity(pixelsPerSecond: Offset(dx, 0)),
+      primaryVelocity: dx,
+    );
+  }
+
   @override
   bool _hasSufficientGlobalDistanceToAccept(PointerDeviceKind pointerDeviceKind, double? deviceTouchSlop) {
     return _globalDistanceMoved.abs() > computeHitSlop(pointerDeviceKind, gestureSettings);
@@ -660,6 +697,16 @@ class PanGestureRecognizer extends DragGestureRecognizer {
         && estimate.offset.distanceSquared > minDistance * minDistance;
   }
 
+  @override
+  DragEndDetails? _considerFling(VelocityEstimate estimate, PointerDeviceKind kind) {
+    if (!isFlingGesture(estimate, kind)) {
+      return null;
+    }
+    final Velocity velocity = Velocity(pixelsPerSecond: estimate.pixelsPerSecond)
+        .clampMagnitude(minFlingVelocity ?? kMinFlingVelocity, maxFlingVelocity ?? kMaxFlingVelocity);
+    return DragEndDetails(velocity: velocity);
+  }
+
   @override
   bool _hasSufficientGlobalDistanceToAccept(PointerDeviceKind pointerDeviceKind, double? deviceTouchSlop) {
     return _globalDistanceMoved.abs() > computePanSlop(pointerDeviceKind, gestureSettings);

packages/flutter/test/gestures/drag_test.dart

@@ -569,6 +569,86 @@ void main() {
     expect(primaryVelocity, velocity.pixelsPerSecond.dx);
   });
 
+  /// Drag the pointer at the given velocity, and return the details
+  /// the recognizer passes to onEnd.
+  ///
+  /// This method will mutate `recognizer.onEnd`.
+  DragEndDetails performDragToEnd(GestureTester tester, DragGestureRecognizer recognizer, Offset pointerVelocity) {
+    late DragEndDetails actual;
+    recognizer.onEnd = (DragEndDetails details) {
+      actual = details;
+    };
+    final TestPointer pointer = TestPointer();
+    final PointerDownEvent down = pointer.down(Offset.zero);
+    recognizer.addPointer(down);
+    tester.closeArena(pointer.pointer);
+    tester.route(down);
+    tester.route(pointer.move(pointerVelocity * 0.025, timeStamp: const Duration(milliseconds: 25)));
+    tester.route(pointer.move(pointerVelocity * 0.050, timeStamp: const Duration(milliseconds: 50)));
+    tester.route(pointer.up(timeStamp: const Duration(milliseconds: 50)));
+    return actual;
+  }
+
+  testGesture('Clamp max pan velocity in 2D, isotropically', (GestureTester tester) {
+    final PanGestureRecognizer recognizer = PanGestureRecognizer();
+    addTearDown(recognizer.dispose);
+
+    void checkDrag(Offset pointerVelocity, Offset expectedVelocity) {
+      final DragEndDetails actual = performDragToEnd(tester, recognizer, pointerVelocity);
+      expect(actual.velocity.pixelsPerSecond, offsetMoreOrLessEquals(expectedVelocity, epsilon: 0.1));
+      expect(actual.primaryVelocity, isNull);
+    }
+
+    checkDrag(const Offset(  400.0,   400.0), const Offset(  400.0,   400.0));
+    checkDrag(const Offset( 2000.0, -2000.0), const Offset( 2000.0, -2000.0));
+    checkDrag(const Offset(-8000.0, -8000.0), const Offset(-5656.9, -5656.9));
+    checkDrag(const Offset(-8000.0,  6000.0), const Offset(-6400.0,  4800.0));
+    checkDrag(const Offset(-9000.0,     0.0), const Offset(-8000.0,     0.0));
+    checkDrag(const Offset(-9000.0, -1000.0), const Offset(-7951.1, - 883.5));
+    checkDrag(const Offset(-1000.0,  9000.0), const Offset(- 883.5,  7951.1));
+    checkDrag(const Offset(    0.0,  9000.0), const Offset(    0.0,  8000.0));
+  });
+
+  testGesture('Clamp max vertical-drag velocity vertically', (GestureTester tester) {
+    final VerticalDragGestureRecognizer recognizer = VerticalDragGestureRecognizer();
+    addTearDown(recognizer.dispose);
+
+    void checkDrag(Offset pointerVelocity, double expectedVelocity) {
+      final DragEndDetails actual = performDragToEnd(tester, recognizer, pointerVelocity);
+      expect(actual.primaryVelocity, moreOrLessEquals(expectedVelocity, epsilon: 0.1));
+      expect(actual.velocity.pixelsPerSecond.dx, 0.0);
+      expect(actual.velocity.pixelsPerSecond.dy, actual.primaryVelocity);
+    }
+
+    checkDrag(const Offset(  500.0,   400.0),   400.0);
+    checkDrag(const Offset( 3000.0, -2000.0), -2000.0);
+    checkDrag(const Offset(-9000.0, -9000.0), -8000.0);
+    checkDrag(const Offset(-9000.0,     0.0),     0.0);
+    checkDrag(const Offset(-9000.0,  1000.0),  1000.0);
+    checkDrag(const Offset(-1000.0, -9000.0), -8000.0);
+    checkDrag(const Offset(    0.0, -9000.0), -8000.0);
+  });
+
+  testGesture('Clamp max horizontal-drag velocity horizontally', (GestureTester tester) {
+    final HorizontalDragGestureRecognizer recognizer = HorizontalDragGestureRecognizer();
+    addTearDown(recognizer.dispose);
+
+    void checkDrag(Offset pointerVelocity, double expectedVelocity) {
+      final DragEndDetails actual = performDragToEnd(tester, recognizer, pointerVelocity);
+      expect(actual.primaryVelocity, moreOrLessEquals(expectedVelocity, epsilon: 0.1));
+      expect(actual.velocity.pixelsPerSecond.dx, actual.primaryVelocity);
+      expect(actual.velocity.pixelsPerSecond.dy, 0.0);
+    }
+
+    checkDrag(const Offset(  500.0,   400.0),   500.0);
+    checkDrag(const Offset( 3000.0, -2000.0),  3000.0);
+    checkDrag(const Offset(-9000.0, -9000.0), -8000.0);
+    checkDrag(const Offset(-9000.0,     0.0), -8000.0);
+    checkDrag(const Offset(-9000.0,  1000.0), -8000.0);
+    checkDrag(const Offset(-1000.0, -9000.0), -1000.0);
+    checkDrag(const Offset(    0.0, -9000.0),     0.0);
+  });
+
   testGesture('Synthesized pointer events are ignored for velocity tracking', (GestureTester tester) {
     final HorizontalDragGestureRecognizer drag = HorizontalDragGestureRecognizer() ..dragStartBehavior = DragStartBehavior.down;
     addTearDown(drag.dispose);