From 3fc408200e65064a9e66a8f52b92a5dc43db44ee Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Tue, 18 May 2021 09:50:24 -0700 Subject: [PATCH] Revert "Gesture recognizer cleanup (#81884)" (#82831) This reverts commit e88a387b26b097d8516c92babe1287edede79206. --- packages/flutter/lib/src/gestures/eager.dart | 3 +- .../flutter/lib/src/gestures/force_press.dart | 6 +- .../flutter/lib/src/gestures/monodrag.dart | 4 +- .../flutter/lib/src/gestures/recognizer.dart | 67 +----- packages/flutter/lib/src/gestures/scale.dart | 4 +- .../lib/src/rendering/platform_view.dart | 4 +- .../test/gestures/recognizer_test.dart | 227 +----------------- 7 files changed, 33 insertions(+), 282 deletions(-) diff --git a/packages/flutter/lib/src/gestures/eager.dart b/packages/flutter/lib/src/gestures/eager.dart index cc1bddb01b..9f1dc02c87 100644 --- a/packages/flutter/lib/src/gestures/eager.dart +++ b/packages/flutter/lib/src/gestures/eager.dart @@ -27,7 +27,8 @@ class EagerGestureRecognizer extends OneSequenceGestureRecognizer { @override void addAllowedPointer(PointerDownEvent event) { - super.addAllowedPointer(event); + // We call startTrackingPointer as this is where OneSequenceGestureRecognizer joins the arena. + startTrackingPointer(event.pointer, event.transform); resolve(GestureDisposition.accepted); stopTrackingPointer(event.pointer); } diff --git a/packages/flutter/lib/src/gestures/force_press.dart b/packages/flutter/lib/src/gestures/force_press.dart index e9348c8b6b..4da8cf5d8a 100644 --- a/packages/flutter/lib/src/gestures/force_press.dart +++ b/packages/flutter/lib/src/gestures/force_press.dart @@ -216,14 +216,14 @@ class ForcePressGestureRecognizer extends OneSequenceGestureRecognizer { _ForceState _state = _ForceState.ready; @override - void addAllowedPointer(PointerDownEvent event) { + void addAllowedPointer(PointerEvent event) { // If the device has a maximum pressure of less than or equal to 1, it // doesn't have touch pressure sensing capabilities. Do not participate // in the gesture arena. - if (event.pressureMax <= 1.0) { + if (event is! PointerUpEvent && event.pressureMax <= 1.0) { resolve(GestureDisposition.rejected); } else { - super.addAllowedPointer(event); + startTrackingPointer(event.pointer, event.transform); if (_state == _ForceState.ready) { _state = _ForceState.possible; _lastPosition = OffsetPair.fromEventPosition(event); diff --git a/packages/flutter/lib/src/gestures/monodrag.dart b/packages/flutter/lib/src/gestures/monodrag.dart index 3a463cb8cf..6e14b94041 100644 --- a/packages/flutter/lib/src/gestures/monodrag.dart +++ b/packages/flutter/lib/src/gestures/monodrag.dart @@ -262,8 +262,8 @@ abstract class DragGestureRecognizer extends OneSequenceGestureRecognizer { } @override - void addAllowedPointer(PointerDownEvent event) { - super.addAllowedPointer(event); + void addAllowedPointer(PointerEvent event) { + startTrackingPointer(event.pointer, event.transform); _velocityTrackers[event.pointer] = velocityTrackerBuilder(event); if (_state == _DragState.ready) { _state = _DragState.possible; diff --git a/packages/flutter/lib/src/gestures/recognizer.dart b/packages/flutter/lib/src/gestures/recognizer.dart index f38ca93a9c..f353c923ab 100644 --- a/packages/flutter/lib/src/gestures/recognizer.dart +++ b/packages/flutter/lib/src/gestures/recognizer.dart @@ -248,32 +248,11 @@ abstract class OneSequenceGestureRecognizer extends GestureRecognizer { final Set _trackedPointers = HashSet(); @override - @protected - void addAllowedPointer(PointerDownEvent event) { - startTrackingPointer(event.pointer, event.transform); - } - - @override - @protected void handleNonAllowedPointer(PointerDownEvent event) { resolve(GestureDisposition.rejected); } /// Called when a pointer event is routed to this recognizer. - /// - /// This will be called for every pointer event while the pointer is being - /// tracked. Typically, this recognizer will start tracking the pointer in - /// [addAllowedPointer], which means that [handleEvent] will be called - /// starting with the [PointerDownEvent] that was passed to [addAllowedPointer]. - /// - /// See also: - /// - /// * [startTrackingPointer], which causes pointer events to be routed to - /// this recognizer. - /// * [stopTrackingPointer], which stops events from being routed to this - /// recognizer. - /// * [stopTrackingIfPointerNoLongerDown], which conditionally stops events - /// from being routed to this recognizer. @protected void handleEvent(PointerEvent event); @@ -359,11 +338,6 @@ abstract class OneSequenceGestureRecognizer extends GestureRecognizer { /// null if no transformation is necessary. /// /// Use [stopTrackingPointer] to remove the route added by this function. - /// - /// This method also adds this recognizer (or its [team] if it's non-null) to - /// the gesture arena for the specified pointer. - /// - /// This is called by [OneSequenceGestureRecognizer.addAllowedPointer]. @protected void startTrackingPointer(int pointer, [Matrix4? transform]) { GestureBinding.instance!.pointerRouter.addRoute(pointer, handleEvent, transform); @@ -492,27 +466,13 @@ abstract class PrimaryPointerGestureRecognizer extends OneSequenceGestureRecogni /// The current state of the recognizer. /// /// See [GestureRecognizerState] for a description of the states. - GestureRecognizerState get state => _state; - GestureRecognizerState _state = GestureRecognizerState.ready; + GestureRecognizerState state = GestureRecognizerState.ready; /// The ID of the primary pointer this recognizer is tracking. - /// - /// If this recognizer is no longer tracking any pointers, this field holds - /// the ID of the primary pointer this recognizer was most recently tracking. - /// This enables the recognizer to know which pointer it was most recently - /// tracking when [acceptGesture] or [rejectGesture] is called (which may be - /// called after the recognizer is no longer tracking a pointer if, e.g. - /// [GestureArenaManager.hold] has been called, or if there are other - /// recognizers keeping the arena open). - int? get primaryPointer => _primaryPointer; - int? _primaryPointer; + int? primaryPointer; /// The location at which the primary pointer contacted the screen. - /// - /// This will only be non-null while this recognizer is tracking at least - /// one pointer. - OffsetPair? get initialPosition => _initialPosition; - OffsetPair? _initialPosition; + OffsetPair? initialPosition; // Whether this pointer is accepted by winning the arena or as defined by // a subclass calling acceptGesture. @@ -521,11 +481,11 @@ abstract class PrimaryPointerGestureRecognizer extends OneSequenceGestureRecogni @override void addAllowedPointer(PointerDownEvent event) { - super.addAllowedPointer(event); + startTrackingPointer(event.pointer, event.transform); if (state == GestureRecognizerState.ready) { - _state = GestureRecognizerState.possible; - _primaryPointer = event.pointer; - _initialPosition = OffsetPair(local: event.localPosition, global: event.position); + state = GestureRecognizerState.possible; + primaryPointer = event.pointer; + initialPosition = OffsetPair(local: event.localPosition, global: event.position); if (deadline != null) _timer = Timer(deadline!, () => didExceedDeadlineWithEvent(event)); } @@ -568,8 +528,7 @@ abstract class PrimaryPointerGestureRecognizer extends OneSequenceGestureRecogni /// Override to be notified when [deadline] is exceeded. /// /// You must override this method or [didExceedDeadlineWithEvent] if you - /// supply a [deadline]. Subclasses that override this method must _not_ - /// call `super.didExceedDeadline()`. + /// supply a [deadline]. @protected void didExceedDeadline() { assert(deadline == null); @@ -579,8 +538,7 @@ abstract class PrimaryPointerGestureRecognizer extends OneSequenceGestureRecogni /// gesture. /// /// You must override this method or [didExceedDeadline] if you supply a - /// [deadline]. Subclasses that override this method must _not_ call - /// `super.didExceedDeadlineWithEvent(event)`. + /// [deadline]. @protected void didExceedDeadlineWithEvent(PointerDownEvent event) { didExceedDeadline(); @@ -598,7 +556,7 @@ abstract class PrimaryPointerGestureRecognizer extends OneSequenceGestureRecogni void rejectGesture(int pointer) { if (pointer == primaryPointer && state == GestureRecognizerState.possible) { _stopTimer(); - _state = GestureRecognizerState.defunct; + state = GestureRecognizerState.defunct; } } @@ -606,9 +564,7 @@ abstract class PrimaryPointerGestureRecognizer extends OneSequenceGestureRecogni void didStopTrackingLastPointer(int pointer) { assert(state != GestureRecognizerState.ready); _stopTimer(); - _state = GestureRecognizerState.ready; - _initialPosition = null; - _gestureAccepted = false; + state = GestureRecognizerState.ready; } @override @@ -641,7 +597,6 @@ abstract class PrimaryPointerGestureRecognizer extends OneSequenceGestureRecogni /// Usually, the [global] [Offset] is in the coordinate space of the screen /// after conversion to logical pixels and the [local] offset is the same /// [Offset], but transformed to a local coordinate space. -@immutable class OffsetPair { /// Creates a [OffsetPair] combining a [local] and [global] [Offset]. const OffsetPair({ diff --git a/packages/flutter/lib/src/gestures/scale.dart b/packages/flutter/lib/src/gestures/scale.dart index 5b864a4595..7e7e742b82 100644 --- a/packages/flutter/lib/src/gestures/scale.dart +++ b/packages/flutter/lib/src/gestures/scale.dart @@ -350,8 +350,8 @@ class ScaleGestureRecognizer extends OneSequenceGestureRecognizer { } @override - void addAllowedPointer(PointerDownEvent event) { - super.addAllowedPointer(event); + void addAllowedPointer(PointerEvent event) { + startTrackingPointer(event.pointer, event.transform); _velocityTrackers[event.pointer] = VelocityTracker.withKind(event.kind); if (_state == _ScaleState.ready) { _state = _ScaleState.possible; diff --git a/packages/flutter/lib/src/rendering/platform_view.dart b/packages/flutter/lib/src/rendering/platform_view.dart index 90a0ff5439..53894242d7 100644 --- a/packages/flutter/lib/src/rendering/platform_view.dart +++ b/packages/flutter/lib/src/rendering/platform_view.dart @@ -456,7 +456,7 @@ class _UiKitViewGestureRecognizer extends OneSequenceGestureRecognizer { @override void addAllowedPointer(PointerDownEvent event) { - super.addAllowedPointer(event); + startTrackingPointer(event.pointer, event.transform); for (final OneSequenceGestureRecognizer recognizer in _gestureRecognizers) { recognizer.addPointer(event); } @@ -544,7 +544,7 @@ class _PlatformViewGestureRecognizer extends OneSequenceGestureRecognizer { @override void addAllowedPointer(PointerDownEvent event) { - super.addAllowedPointer(event); + startTrackingPointer(event.pointer, event.transform); for (final OneSequenceGestureRecognizer recognizer in _gestureRecognizers) { recognizer.addPointer(event); } diff --git a/packages/flutter/test/gestures/recognizer_test.dart b/packages/flutter/test/gestures/recognizer_test.dart index 48fe20eaac..4aafb19a4e 100644 --- a/packages/flutter/test/gestures/recognizer_test.dart +++ b/packages/flutter/test/gestures/recognizer_test.dart @@ -2,48 +2,26 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:ui' show VoidCallback; - import 'package:flutter/gestures.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'gesture_tester.dart'; +class TestGestureRecognizer extends GestureRecognizer { + TestGestureRecognizer({ Object? debugOwner }) : super(debugOwner: debugOwner); -// Down/move/up pair 1: normal tap sequence -const PointerDownEvent down = PointerDownEvent( - pointer: 5, - position: Offset(10, 10), -); + @override + String get debugDescription => 'debugDescription content'; -const PointerMoveEvent move = PointerMoveEvent( - pointer: 5, - position: Offset(15, 15), -); + @override + void addPointer(PointerDownEvent event) { } -const PointerUpEvent up = PointerUpEvent( - pointer: 5, - position: Offset(15, 15), -); + @override + void acceptGesture(int pointer) { } -// Down/move/up pair 2: tap sequence with a large move in the middle -const PointerDownEvent down2 = PointerDownEvent( - pointer: 6, - position: Offset(10, 10), -); - -const PointerMoveEvent move2 = PointerMoveEvent( - pointer: 6, - position: Offset(100, 200), -); - -const PointerUpEvent up2 = PointerUpEvent( - pointer: 6, - position: Offset(100, 200), -); + @override + void rejectGesture(int pointer) { } +} void main() { - setUp(ensureGestureBinding); - test('GestureRecognizer smoketest', () { final TestGestureRecognizer recognizer = TestGestureRecognizer(debugOwner: 0); expect(recognizer, hasAGoodToStringDeep); @@ -86,187 +64,4 @@ void main() { ), ); }); - - group('PrimaryPointerGestureRecognizer', () { - testGesture('cleans up state after winning arena', (GestureTester tester) { - final List resolutions = []; - final IndefiniteGestureRecognizer indefinite = IndefiniteGestureRecognizer(); - final TestPrimaryPointerGestureRecognizer accepting = TestPrimaryPointerGestureRecognizer( - GestureDisposition.accepted, - onAcceptGesture: () => resolutions.add('accepted'), - onRejectGesture: () => resolutions.add('rejected'), - ); - expect(accepting.state, GestureRecognizerState.ready); - expect(accepting.primaryPointer, isNull); - expect(accepting.initialPosition, isNull); - expect(resolutions, []); - - indefinite.addPointer(down); - accepting.addPointer(down); - expect(accepting.state, GestureRecognizerState.possible); - expect(accepting.primaryPointer, 5); - expect(accepting.initialPosition!.global, down.position); - expect(accepting.initialPosition!.local, down.localPosition); - expect(resolutions, []); - - tester.closeArena(5); - tester.async.flushMicrotasks(); - tester.route(down); - tester.route(up); - expect(accepting.state, GestureRecognizerState.ready); - expect(accepting.primaryPointer, 5); - expect(accepting.initialPosition, isNull); - expect(resolutions, ['accepted']); - }); - - testGesture('cleans up state after losing arena', (GestureTester tester) { - final List resolutions = []; - final IndefiniteGestureRecognizer indefinite = IndefiniteGestureRecognizer(); - final TestPrimaryPointerGestureRecognizer rejecting = TestPrimaryPointerGestureRecognizer( - GestureDisposition.rejected, - onAcceptGesture: () => resolutions.add('accepted'), - onRejectGesture: () => resolutions.add('rejected'), - ); - expect(rejecting.state, GestureRecognizerState.ready); - expect(rejecting.primaryPointer, isNull); - expect(rejecting.initialPosition, isNull); - expect(resolutions, []); - - indefinite.addPointer(down); - rejecting.addPointer(down); - expect(rejecting.state, GestureRecognizerState.possible); - expect(rejecting.primaryPointer, 5); - expect(rejecting.initialPosition!.global, down.position); - expect(rejecting.initialPosition!.local, down.localPosition); - expect(resolutions, []); - - tester.closeArena(5); - tester.async.flushMicrotasks(); - tester.route(down); - tester.route(move); - expect(rejecting.state, GestureRecognizerState.defunct); - expect(rejecting.primaryPointer, 5); - expect(rejecting.initialPosition!.global, down.position); - expect(rejecting.initialPosition!.local, down.localPosition); - expect(resolutions, ['rejected']); - - tester.route(up); - expect(rejecting.state, GestureRecognizerState.ready); - expect(rejecting.primaryPointer, 5); - expect(rejecting.initialPosition, isNull); - expect(resolutions, ['rejected']); - }); - - testGesture('works properly when recycled', (GestureTester tester) { - final List resolutions = []; - final IndefiniteGestureRecognizer indefinite = IndefiniteGestureRecognizer(); - final TestPrimaryPointerGestureRecognizer accepting = TestPrimaryPointerGestureRecognizer( - GestureDisposition.accepted, - preAcceptSlopTolerance: 15, - postAcceptSlopTolerance: 1000, - onAcceptGesture: () => resolutions.add('accepted'), - onRejectGesture: () => resolutions.add('rejected'), - ); - - // Send one complete pointer sequence - indefinite.addPointer(down); - accepting.addPointer(down); - tester.closeArena(5); - tester.async.flushMicrotasks(); - tester.route(down); - tester.route(up); - expect(resolutions, ['accepted']); - resolutions.clear(); - - // Send a follow-on sequence that breaks preAcceptSlopTolerance - indefinite.addPointer(down2); - accepting.addPointer(down2); - tester.closeArena(6); - tester.async.flushMicrotasks(); - tester.route(down2); - tester.route(move2); - expect(resolutions, ['rejected']); - tester.route(up2); - expect(resolutions, ['rejected']); - }); - }); -} - -class TestGestureRecognizer extends GestureRecognizer { - TestGestureRecognizer({ Object? debugOwner }) : super(debugOwner: debugOwner); - - @override - String get debugDescription => 'debugDescription content'; - - @override - void addPointer(PointerDownEvent event) { } - - @override - void acceptGesture(int pointer) { } - - @override - void rejectGesture(int pointer) { } -} - -/// Gesture recognizer that adds itself to the gesture arena but never -/// resolves itself. -class IndefiniteGestureRecognizer extends GestureRecognizer { - @override - void addAllowedPointer(PointerDownEvent event) { - GestureBinding.instance!.gestureArena.add(event.pointer, this); - } - - @override - void acceptGesture(int pointer) { } - - @override - void rejectGesture(int pointer) { } - - @override - String get debugDescription => 'Unresolving'; -} - -/// Gesture recognizer that resolves with [resolution] when it handles an event -/// on the primary pointer of type [T] -class TestPrimaryPointerGestureRecognizer extends PrimaryPointerGestureRecognizer { - TestPrimaryPointerGestureRecognizer( - this.resolution, { - this.onAcceptGesture, - this.onRejectGesture, - double? preAcceptSlopTolerance, - double? postAcceptSlopTolerance, - }) : super( - preAcceptSlopTolerance: preAcceptSlopTolerance, - postAcceptSlopTolerance: postAcceptSlopTolerance, - ); - - final GestureDisposition resolution; - final VoidCallback? onAcceptGesture; - final VoidCallback? onRejectGesture; - - @override - void acceptGesture(int pointer) { - super.acceptGesture(pointer); - if (onAcceptGesture != null) { - onAcceptGesture!(); - } - } - - @override - void rejectGesture(int pointer) { - super.rejectGesture(pointer); - if (onRejectGesture != null) { - onRejectGesture!(); - } - } - - @override - void handlePrimaryPointer(PointerEvent event) { - if (event is T) { - resolve(resolution); - } - } - - @override - String get debugDescription => 'TestPrimaryPointer'; }