Improve tap; add double tap; add tests

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

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 import 'dart:async';
-import 'dart:ui' as ui;
 
 import 'arena.dart';
 import 'constants.dart';
@@ -14,117 +13,145 @@ import 'tap.dart';
 class DoubleTapGestureRecognizer extends DisposableArenaMember {
   static int sInstances = 0;
 
-  DoubleTapGestureRecognizer({ this.router, this.onDoubleTap }) {
-    _instance = sInstances++;
-  }
+  DoubleTapGestureRecognizer({ this.router, this.onDoubleTap });
+
+  // Implementation notes:
+  // The double tap recognizer can be in one of four states. There's no
+  // explicit enum for the states, because they are already captured by
+  // the state of existing fields.  Specifically:
+  // Waiting on first tap: In this state, the _trackers list is empty, and
+  // _firstTap is null.
+  // First tap in progress: In this state, the _trackers list contains all
+  // the states for taps that have begun but not completed. This list can
+  // have more than one entry if two pointers begin to tap.
+  // Waiting on second tap: In this state, one of the in-progress taps has
+  // completed successfully. The _trackers list is again empty, and
+  // _firstTap records the successful tap.
+  // Second tap in progress: Much like the "first tap in progress" state, but
+  // _firstTap is non-null.  If a tap completes successfully while in this
+  // state, the callback is invoked and the state is reset.
+  // There are various other scenarios that cause the state to reset:
+  // - All in-progress taps are rejected (by time, distance, pointercancel, etc)
+  // - The long timer between taps expires
+  // - The gesture arena decides we have been rejected wholesale
 
   PointerRouter router;
   GestureTapCallback onDoubleTap;
 
-  int _numTaps = 0;
-  int _instance = 0;
-  bool _isTrackingPointer = false;
-  int _pointer;
-  ui.Point _initialPosition;
-  Timer _tapTimer;
   Timer _doubleTapTimer;
-  GestureArenaEntry _entry = null;
+  TapTracker _firstTap;
+  Map<int, TapTracker> _trackers = new Map<int, TapTracker>();
 
   void addPointer(PointerInputEvent event) {
-    message("add pointer");
-    if (_initialPosition != null && !_isWithinTolerance(event)) {
-      message("reset");
-      _reset();
-    }
-    _pointer = event.pointer;
-    _initialPosition = _getPoint(event);
-    _isTrackingPointer = false;
-    _startTapTimer();
+    // Ignore out-of-bounds second taps
+    if (_firstTap != null &&
+        !_firstTap.isWithinTolerance(event, kDoubleTapTouchSlop))
+      return;
     _stopDoubleTapTimer();
-    _startTrackingPointer();
-    if (_entry == null) {
-      message("register entry");
-      _entry = GestureArena.instance.add(event.pointer, this);
-    }
-  }
-
-  void message(String s) {
-    print("Double tap " + _instance.toString() + ": " + s);
+    TapTracker tracker = new TapTracker(
+      event: event,
+      entry: GestureArena.instance.add(event.pointer, this)
+    );
+    _trackers[event.pointer] = tracker;
+    tracker.startTimer(() => _reject(tracker));
+    tracker.startTrackingPointer(router, handleEvent);
   }
 
   void handleEvent(PointerInputEvent event) {
-    message("handle event");
+    TapTracker tracker = _trackers[event.pointer];
+    assert(tracker != null);
     if (event.type == 'pointerup') {
-      _numTaps++;
-      _stopTapTimer();
-      _stopTrackingPointer();
-      if (_numTaps == 1) {
-        message("start long timer");
-        _startDoubleTapTimer();
-      } else if (_numTaps == 2) {
-        message("start found second tap");
-        _entry.resolve(GestureDisposition.accepted);
-      }
-    } else if (event.type == 'pointermove' && !_isWithinTolerance(event)) {
-      message("outside tap tolerance");
-      _entry.resolve(GestureDisposition.rejected);
+      if (_firstTap == null)
+        _registerFirstTap(tracker);
+      else
+        _registerSecondTap(tracker);
+    } else if (event.type == 'pointermove' &&
+        !tracker.isWithinTolerance(event, kTouchSlop)) {
+      _reject(tracker);
     } else if (event.type == 'pointercancel') {
-      message("cancel");
-      _entry.resolve(GestureDisposition.rejected);
+      _reject(tracker);
     }
   }
 
-  void acceptGesture(int pointer) {
-    message("accepted");
-    _reset();
-    _entry = null;
-    print ("Entry is assigned null");
-    onDoubleTap?.call();
-  }
+  void acceptGesture(int pointer) {}
 
   void rejectGesture(int pointer) {
-    message("rejected");
-    _reset();
-    _entry = null;
-    print ("Entry is assigned null");
+    TapTracker tracker = _trackers[pointer];
+    // If tracker isn't in the list, check if this is the first tap tracker
+    if (tracker == null &&
+        _firstTap != null &&
+        _firstTap.pointer == pointer)
+      tracker = _firstTap;
+    // If tracker is still null, we rejected ourselves already
+    if (tracker != null)
+      _reject(tracker);
+  }
+
+  void _reject(TapTracker tracker) {
+    _trackers.remove(tracker.pointer);
+    tracker.entry.resolve(GestureDisposition.rejected);
+    _freezeTracker(tracker);
+    // If the first tap is in progress, and we've run out of taps to track,
+    // reset won't have any work to do.  But if we're in the second tap, we need
+    // to clear intermediate state.
+    if (_firstTap != null &&
+        (_trackers.isEmpty || tracker == _firstTap))
+      _reset();
   }
 
   void dispose() {
-    _entry?.resolve(GestureDisposition.rejected);
+    _reset();
     router = null;
   }
 
   void _reset() {
-    _numTaps = 0;
-    _initialPosition = null;
-    _stopTapTimer();
     _stopDoubleTapTimer();
-    _stopTrackingPointer();
+    if (_firstTap != null) {
+      // Note, order is important below in order for the resolve -> reject logic
+      // to work properly
+      TapTracker tracker = _firstTap;
+      _firstTap = null;
+      _reject(tracker);
+      GestureArena.instance.release(tracker.pointer);
+    }
+    _clearTrackers();
   }
 
-  void _startTapTimer() {
-    if (_tapTimer == null) {
-      _tapTimer = new Timer(
-        kTapTimeout,
-        () => _entry.resolve(GestureDisposition.rejected)
-      );
-    }
+  void _registerFirstTap(TapTracker tracker) {
+    _startDoubleTapTimer();
+    GestureArena.instance.hold(tracker.pointer);
+    // Note, order is important below in order for the clear -> reject logic to
+    // work properly.
+    _freezeTracker(tracker);
+    _trackers.remove(tracker.pointer);
+    _clearTrackers();
+    _firstTap = tracker;
   }
 
-  void _stopTapTimer() {
-    if (_tapTimer != null) {
-      _tapTimer.cancel();
-      _tapTimer = null;
-    }
+  void _registerSecondTap(TapTracker tracker) {
+    _firstTap.entry.resolve(GestureDisposition.accepted);
+    tracker.entry.resolve(GestureDisposition.accepted);
+    _freezeTracker(tracker);
+    _trackers.remove(tracker.pointer);
+    onDoubleTap?.call();
+    _reset();
+  }
+
+  void _clearTrackers() {
+    List<TapTracker> localTrackers = new List.from(_trackers.values);
+    for (TapTracker tracker in localTrackers)
+      _reject(tracker);
+    assert(_trackers.isEmpty);
+  }
+
+  void _freezeTracker(TapTracker tracker) {
+    tracker.stopTimer();
+    tracker.stopTrackingPointer(router, handleEvent);
   }
 
   void _startDoubleTapTimer() {
-    if (_doubleTapTimer == null) {
-      _doubleTapTimer = new Timer(
-        kDoubleTapTimeout,
-        () => _entry.resolve(GestureDisposition.rejected)
-      );
-    }
+    if (_doubleTapTimer == null)
+      _doubleTapTimer = new Timer(kDoubleTapTimeout, () => _reset());
   }
 
   void _stopDoubleTapTimer() {
@@ -134,27 +161,4 @@ class DoubleTapGestureRecognizer extends DisposableArenaMember {
     }
   }
 
-  void _startTrackingPointer() {
-    if (!_isTrackingPointer) {
-      _isTrackingPointer = true;
-      router.addRoute(_pointer, handleEvent);
-    }
-  }
-
-  void _stopTrackingPointer() {
-    if (_isTrackingPointer) {
-      _isTrackingPointer = false;
-      router.removeRoute(_pointer, handleEvent);
-    }
-  }
-
-  ui.Point _getPoint(PointerInputEvent event) {
-    return new ui.Point(event.x, event.y);
-  }
-
-  bool _isWithinTolerance(PointerInputEvent event) {
-    ui.Offset offset = _getPoint(event) - _initialPosition;
-    return offset.distance <= kDoubleTapTouchSlop;
-  }
-
 }

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

@@ -2,6 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'dart:ui' as ui;
+
+export 'dart:ui' show Point;
+
 /// Base class for input events.
 class InputEvent {
 
@@ -67,4 +71,5 @@ class PointerInputEvent extends InputEvent {
   final double orientation;
   final double tilt;
 
+  ui.Point get position => new ui.Point(x, y);
 }

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

@@ -8,6 +8,7 @@ import 'dart:ui' as ui;
 import 'arena.dart';
 import 'constants.dart';
 import 'events.dart';
+import 'pointer_router.dart';
 import 'recognizer.dart';
 
 typedef void GestureTapCallback();
@@ -17,103 +18,113 @@ enum TapResolution {
   cancel
 }
 
-class _TapGesture {
-  _TapGesture({ this.gestureRecognizer, PointerInputEvent event }) {
-    assert(event.type == 'pointerdown');
-    _pointer = event.pointer;
-    _isTrackingPointer = false;
-    _initialPosition = _getPoint(event);
-    _entry = GestureArena.instance.add(_pointer, gestureRecognizer);
-    _wonArena = false;
-    _didTap = false;
-    _startTimer();
-    _startTrackingPointer();
+/// TapTracker helps track individual tap sequences as part of a
+/// larger gesture.
+class TapTracker {
+
+  TapTracker({ PointerInputEvent event, this.entry })
+  : pointer = event.pointer,
+    initialPosition = event.position,
+    isTrackingPointer = false {
+      assert(event.type == 'pointerdown');
+    }
+
+  int pointer;
+  ui.Point initialPosition;
+  bool isTrackingPointer;
+  Timer timer;
+  GestureArenaEntry entry;
+
+  void startTimer(void callback()) {
+    if (timer == null) {
+      timer = new Timer(kTapTimeout, callback);
+    }
   }
 
+  void stopTimer() {
+    if (timer != null) {
+      timer.cancel();
+      timer = null;
+    }
+  }
+
+  void startTrackingPointer(PointerRouter router, PointerRoute route) {
+    if (!isTrackingPointer) {
+      isTrackingPointer = true;
+      router.addRoute(pointer, route);
+    }
+  }
+
+  void stopTrackingPointer(PointerRouter router, PointerRoute route) {
+    if (isTrackingPointer) {
+      isTrackingPointer = false;
+      router.removeRoute(pointer, route);
+    }
+  }
+
+  bool isWithinTolerance(PointerInputEvent event, double tolerance) {
+    ui.Offset offset = event.position - initialPosition;
+    return offset.distance <= tolerance;
+  }
+
+}
+
+/// TapGesture represents a full gesture resulting from a single tap
+/// sequence. Tap gestures are passive, meaning that they will not
+/// pre-empt any other arena member in play.
+class TapGesture extends TapTracker {
+
+  TapGesture({ this.gestureRecognizer, PointerInputEvent event })
+  : super(event: event) {
+      entry = GestureArena.instance.add(event.pointer, gestureRecognizer);
+      _wonArena = false;
+      _didTap = false;
+      startTimer(() => cancel());
+      startTrackingPointer(gestureRecognizer.router, handleEvent);
+    }
+
   TapGestureRecognizer gestureRecognizer;
 
-  int _pointer;
-  bool _isTrackingPointer;
-  ui.Point _initialPosition;
-  GestureArenaEntry _entry;
-  Timer _deadline;
   bool _wonArena;
   bool _didTap;
 
   void handleEvent(PointerInputEvent event) {
-    print("Tap gesture handleEvent");
-    assert(event.pointer == _pointer);
-    if (event.type == 'pointermove' && !_isWithinTolerance(event)) {
-      _entry.resolve(GestureDisposition.rejected);
+    assert(event.pointer == pointer);
+    if (event.type == 'pointermove' && !isWithinTolerance(event, kTouchSlop)) {
+      cancel();
     } else if (event.type == 'pointercancel') {
-      _entry.resolve(GestureDisposition.rejected);
+      cancel();
     } else if (event.type == 'pointerup') {
-      _stopTimer();
-      _stopTrackingPointer();
+      stopTimer();
+      stopTrackingPointer(gestureRecognizer.router, handleEvent);
       _didTap = true;
       _check();
     }
   }
 
   void accept() {
-    print("Tap gesture accept");
     _wonArena = true;
     _check();
   }
 
   void reject() {
-    print("Tap gesture reject");
-    _stopTimer();
-    _stopTrackingPointer();
-    gestureRecognizer._resolveTap(_pointer, TapResolution.cancel);
+    stopTimer();
+    stopTrackingPointer(gestureRecognizer.router, handleEvent);
+    gestureRecognizer._resolveTap(pointer, TapResolution.cancel);
   }
 
-  void abort() {
-    _entry.resolve(GestureDisposition.rejected);
+  void cancel() {
+    // If we won the arena already, then _entry is resolved, so resolving
+    // again is a no-op. But we still need to clean up our own state.
+    if (_wonArena)
+      reject();
+    else
+      entry.resolve(GestureDisposition.rejected);
   }
 
   void _check() {
     if (_wonArena && _didTap)
-      gestureRecognizer._resolveTap(_pointer, TapResolution.tap);
-  }
-
-  void _startTimer() {
-    if (_deadline == null) {
-      _deadline = new Timer(
-        kTapTimeout,
-        () => _entry.resolve(GestureDisposition.rejected)
-      );
-    }
-  }
-
-  void _stopTimer() {
-    if (_deadline != null) {
-      _deadline.cancel();
-      _deadline = null;
-    }
-  }
-
-  void _startTrackingPointer() {
-    if (!_isTrackingPointer) {
-      _isTrackingPointer = true;
-      gestureRecognizer.router.addRoute(_pointer, handleEvent);
-    }
-  }
-
-  void _stopTrackingPointer() {
-    if (_isTrackingPointer) {
-      _isTrackingPointer = false;
-      gestureRecognizer.router.removeRoute(_pointer, handleEvent);
-    }
-  }
-
-  ui.Point _getPoint(PointerInputEvent event) {
-    return new ui.Point(event.x, event.y);
-  }
-
-  bool _isWithinTolerance(PointerInputEvent event) {
-    ui.Offset offset = _getPoint(event) - _initialPosition;
-    return offset.distance <= kTouchSlop;
+      gestureRecognizer._resolveTap(pointer, TapResolution.tap);
   }
 
 }
@@ -126,10 +137,10 @@ class TapGestureRecognizer extends DisposableArenaMember {
   GestureTapCallback onTapDown;
   GestureTapCallback onTapCancel;
 
-  Map<int, _TapGesture> _gestureMap = new Map<int, _TapGesture>();
+  Map<int, TapGesture> _gestureMap = new Map<int, TapGesture>();
 
   void addPointer(PointerInputEvent event) {
-    _gestureMap[event.pointer] = new _TapGesture(
+    _gestureMap[event.pointer] = new TapGesture(
       gestureRecognizer: this,
       event: event
     );
@@ -153,9 +164,9 @@ class TapGestureRecognizer extends DisposableArenaMember {
   }
 
   void dispose() {
-    List<_TapGesture> localGestures = new List.from(_gestureMap.values);
-    for (_TapGesture gesture in localGestures)
-      gesture.abort();
+    List<TapGesture> localGestures = new List.from(_gestureMap.values);
+    for (TapGesture gesture in localGestures)
+      gesture.cancel();
     // Rejection of each gesture should cause it to be removed from our map
     assert(_gestureMap.isEmpty);
     router = null;

packages/unit/test/gestures/tap_test.dart

 import 'package:flutter/gestures.dart';
+import 'package:quiver/testing/async.dart';
 import 'package:test/test.dart';
 
+class TestGestureArenaMember extends GestureArenaMember {
+  void acceptGesture(Object key) {}
+  void rejectGesture(Object key) {}
+}
+
 void main() {
+
+  // Down/up pair 1: normal tap sequence
+  final PointerInputEvent down1 = new PointerInputEvent(
+    pointer: 1,
+    type: 'pointerdown',
+    x: 10.0,
+    y: 10.0
+  );
+
+  final PointerInputEvent up1 = new PointerInputEvent(
+    pointer: 1,
+    type: 'pointerup',
+    x: 11.0,
+    y: 9.0
+  );
+
+  // Down/up pair 2: normal tap sequence far away from pair 1
+  final PointerInputEvent down2 = new PointerInputEvent(
+    pointer: 2,
+    type: 'pointerdown',
+    x: 30.0,
+    y: 30.0
+  );
+
+  final PointerInputEvent up2 = new PointerInputEvent(
+    pointer: 2,
+    type: 'pointerup',
+    x: 31.0,
+    y: 29.0
+  );
+
+  // Down/move/up sequence 3: intervening motion
+  final PointerInputEvent down3 = new PointerInputEvent(
+    pointer: 3,
+    type: 'pointerdown',
+    x: 10.0,
+    y: 10.0
+  );
+
+  final PointerInputEvent move3 = new PointerInputEvent(
+    pointer: 3,
+    type: 'pointermove',
+    x: 25.0,
+    y: 25.0
+  );
+
+  final PointerInputEvent up3 = new PointerInputEvent(
+    pointer: 3,
+    type: 'pointerup',
+    x: 25.0,
+    y: 25.0
+  );
+
   test('Should recognize tap', () {
     PointerRouter router = new PointerRouter();
     TapGestureRecognizer tap = new TapGestureRecognizer(router: router);
@@ -11,29 +70,163 @@ void main() {
       tapRecognized = true;
     };
 
-    PointerInputEvent down = new PointerInputEvent(
-      pointer: 5,
-      type: 'pointerdown',
-      x: 10.0,
-      y: 10.0
-    );
+    tap.addPointer(down1);
+    GestureArena.instance.close(1);
+    expect(tapRecognized, isFalse);
+    router.route(down1);
+    expect(tapRecognized, isFalse);
+
+    router.route(up1);
+    expect(tapRecognized, isTrue);
+    GestureArena.instance.sweep(1);
+    expect(tapRecognized, isTrue);
+
+    tap.dispose();
+  });
+
+  test('Should recognize two overlapping taps', () {
+    PointerRouter router = new PointerRouter();
+    TapGestureRecognizer tap = new TapGestureRecognizer(router: router);
+
+    int tapsRecognized = 0;
+    tap.onTap = () {
+      tapsRecognized++;
+    };
 
-    tap.addPointer(down);
-    GestureArena.instance.close(5);
+    tap.addPointer(down1);
+    GestureArena.instance.close(1);
+    expect(tapsRecognized, 0);
+    router.route(down1);
+    expect(tapsRecognized, 0);
+
+    tap.addPointer(down2);
+    GestureArena.instance.close(2);
+    expect(tapsRecognized, 0);
+    router.route(down1);
+    expect(tapsRecognized, 0);
+
+
+    router.route(up1);
+    expect(tapsRecognized, 1);
+    GestureArena.instance.sweep(1);
+    expect(tapsRecognized, 1);
+
+    router.route(up2);
+    expect(tapsRecognized, 2);
+    GestureArena.instance.sweep(2);
+    expect(tapsRecognized, 2);
+
+    tap.dispose();
+  });
+
+  test('Distance cancels tap', () {
+    PointerRouter router = new PointerRouter();
+    TapGestureRecognizer tap = new TapGestureRecognizer(router: router);
+
+    bool tapRecognized = false;
+    tap.onTap = () {
+      tapRecognized = true;
+    };
+
+    tap.addPointer(down3);
+    GestureArena.instance.close(3);
     expect(tapRecognized, isFalse);
-    router.route(down);
+    router.route(down3);
     expect(tapRecognized, isFalse);
 
-    PointerInputEvent up = new PointerInputEvent(
-      pointer: 5,
-      type: 'pointerup',
-      x: 11.0,
-      y: 9.0
-    );
+    router.route(move3);
+    expect(tapRecognized, isFalse);
+    router.route(up3);
+    expect(tapRecognized, isFalse);
+    GestureArena.instance.sweep(3);
+    expect(tapRecognized, isFalse);
 
-    router.route(up);
+    tap.dispose();
+  });
+
+  test('Timeout cancels tap', () {
+    PointerRouter router = new PointerRouter();
+    TapGestureRecognizer tap = new TapGestureRecognizer(router: router);
+
+    bool tapRecognized = false;
+    tap.onTap = () {
+      tapRecognized = true;
+    };
+
+    new FakeAsync().run((FakeAsync async) {
+      tap.addPointer(down1);
+      GestureArena.instance.close(1);
+      expect(tapRecognized, isFalse);
+      router.route(down1);
+      expect(tapRecognized, isFalse);
+
+      async.elapse(new Duration(milliseconds: 500));
+      expect(tapRecognized, isFalse);
+      router.route(up1);
+      expect(tapRecognized, isFalse);
+      GestureArena.instance.sweep(1);
+      expect(tapRecognized, isFalse);
+    });
+
+    tap.dispose();
+  });
+
+  test('Should yield to other arena members', () {
+    PointerRouter router = new PointerRouter();
+    TapGestureRecognizer tap = new TapGestureRecognizer(router: router);
+
+    bool tapRecognized = false;
+    tap.onTap = () {
+      tapRecognized = true;
+    };
+
+    tap.addPointer(down1);
+    TestGestureArenaMember member = new TestGestureArenaMember();
+    GestureArenaEntry entry = GestureArena.instance.add(1, member);
+    GestureArena.instance.hold(1);
+    GestureArena.instance.close(1);
+    expect(tapRecognized, isFalse);
+    router.route(down1);
+    expect(tapRecognized, isFalse);
+
+    router.route(up1);
+    expect(tapRecognized, isFalse);
+    GestureArena.instance.sweep(1);
+    expect(tapRecognized, isFalse);
+
+    entry.resolve(GestureDisposition.accepted);
+    expect(tapRecognized, isFalse);
+
+    tap.dispose();
+  });
+
+  test('Should trigger on release of held arena', () {
+    PointerRouter router = new PointerRouter();
+    TapGestureRecognizer tap = new TapGestureRecognizer(router: router);
+
+    bool tapRecognized = false;
+    tap.onTap = () {
+      tapRecognized = true;
+    };
+
+    tap.addPointer(down1);
+    TestGestureArenaMember member = new TestGestureArenaMember();
+    GestureArenaEntry entry = GestureArena.instance.add(1, member);
+    GestureArena.instance.hold(1);
+    GestureArena.instance.close(1);
+    expect(tapRecognized, isFalse);
+    router.route(down1);
+    expect(tapRecognized, isFalse);
+
+    router.route(up1);
+    expect(tapRecognized, isFalse);
+    GestureArena.instance.sweep(1);
+    expect(tapRecognized, isFalse);
+
+    entry.resolve(GestureDisposition.rejected);
     expect(tapRecognized, isTrue);
 
     tap.dispose();
   });
+
 }