Bug 1451461 - Make pinch locking unaffected by input sensitivity. r=botond

Implements a 50ms buffer of scale events in APZC, so pinch locking code can
consider gesture movement over a fixed length of time. Previously, pinch
locking was sensitive to event frequency (which is determined by the
sensitivity of the input device).

  * New class InputBuffer wraps std::deque
  * New field APZC::mPinchEventBuffer
  * New gfxPref APZPinchLockBufferMaxAge

--HG--
extra : rebase_source : a44da4c3ce971fee1b0f401357f47f994a9145df

Author: jlogandavison

gfx/layers/apz/src/AsyncPanZoomController.cpp

@@ -415,6 +415,13 @@ typedef PlatformSpecificStateBase
  * and scrolls more than apz.pinch_lock.scroll_lock_threshold.\n
  * Units: (real-world, i.e. screen) inches measured between two touch points
  *
+ * \li\b apz.pinch_lock.buffer_max_age
+ * To ensure that pinch locking threshold calculations are not affected by
+ * variations in touch screen sensitivity, calculations draw from a buffer of
+ * recent events. This preference specifies the maximum time that events are
+ * held in this buffer.
+ * Units: milliseconds
+ *
  * \li\b apz.popups.enabled
  * Determines whether APZ is used for XUL popup widgets with remote content.
  * Ideally, this should always be true, but it is currently not well tested, and
@@ -800,6 +807,8 @@ AsyncPanZoomController::AsyncPanZoomController(
       mY(this),
       mPanDirRestricted(false),
       mPinchLocked(false),
+      mPinchEventBuffer(
+          TimeDuration::FromMilliseconds(gfxPrefs::APZPinchLockBufferMaxAge())),
       mZoomConstraints(false, false,
                        Metrics().GetDevPixelsPerCSSPixel() * kViewportMinScale /
                            ParentLayerToScreenScale(1),
@@ -1515,6 +1524,8 @@ nsEventStatus AsyncPanZoomController::OnScaleBegin(
   mLastZoomFocus =
       aEvent.mLocalFocusPoint - Metrics().GetCompositionBounds().TopLeft();
 
+  mPinchEventBuffer.push(aEvent);
+
   return nsEventStatus_eConsumeNoDefault;
 }
 
@@ -1530,22 +1541,8 @@ nsEventStatus AsyncPanZoomController::OnScale(const PinchGestureInput& aEvent) {
     return nsEventStatus_eConsumeNoDefault;
   }
 
-  ParentLayerCoord spanDistance =
-      fabsf(aEvent.mPreviousSpan - aEvent.mCurrentSpan);
-  ParentLayerPoint focusPoint, focusChange;
-  {
-    RecursiveMutexAutoLock lock(mRecursiveMutex);
-
-    focusPoint =
-        aEvent.mLocalFocusPoint - Metrics().GetCompositionBounds().TopLeft();
-    focusChange = mLastZoomFocus - focusPoint;
-    mLastZoomFocus = focusPoint;
-  }
-
-  HandlePinchLocking(
-      ToScreenCoordinates(ParentLayerPoint(0, spanDistance), focusPoint)
-          .Length(),
-      ToScreenCoordinates(focusChange, focusPoint));
+  mPinchEventBuffer.push(aEvent);
+  HandlePinchLocking();
   bool allowZoom = mZoomConstraints.mAllowZoom && !mPinchLocked;
 
   // If zooming is not allowed, this is a two-finger pan.
@@ -1581,8 +1578,12 @@ nsEventStatus AsyncPanZoomController::OnScale(const PinchGestureInput& aEvent) {
     RecursiveMutexAutoLock lock(mRecursiveMutex);
 
     CSSToParentLayerScale userZoom = Metrics().GetZoom().ToScaleFactor();
+    ParentLayerPoint focusPoint =
+        aEvent.mLocalFocusPoint - Metrics().GetCompositionBounds().TopLeft();
     CSSPoint cssFocusPoint = focusPoint / Metrics().GetZoom();
 
+    ParentLayerPoint focusChange = mLastZoomFocus - focusPoint;
+    mLastZoomFocus = focusPoint;
     // If displacing by the change in focus point will take us off page bounds,
     // then reduce the displacement such that it doesn't.
     focusChange.x -= mX.DisplacementWillOverscrollAmount(focusChange.x);
@@ -1701,6 +1702,8 @@ nsEventStatus AsyncPanZoomController::OnScaleEnd(
     UpdateSharedCompositorFrameMetrics();
   }
 
+  mPinchEventBuffer.clear();
+
   // Non-negative focus point would indicate that one finger is still down
   if (aEvent.mLocalFocusPoint != PinchGestureInput::BothFingersLifted()) {
     if (mZoomConstraints.mAllowZoom) {
@@ -2932,8 +2935,38 @@ void AsyncPanZoomController::HandlePanningUpdate(
   }
 }
 
-void AsyncPanZoomController::HandlePinchLocking(ScreenCoord spanDistance,
-                                                ScreenPoint focusChange) {
+void AsyncPanZoomController::HandlePinchLocking() {
+  // Focus change and span distance calculated from an event buffer
+  // Used to handle pinch locking irrespective of touch screen sensitivity
+  // Note: both values fall back to the same value as
+  //       their un-buffered counterparts if there is only one (the latest)
+  //       event in the buffer. ie: when the touch screen is dispatching
+  //       events slower than the lifetime of the buffer
+  ParentLayerCoord bufferedSpanDistance;
+  ParentLayerPoint focusPoint, bufferedFocusChange;
+  {
+    RecursiveMutexAutoLock lock(mRecursiveMutex);
+
+    focusPoint = mPinchEventBuffer.back().mLocalFocusPoint -
+                 Metrics().GetCompositionBounds().TopLeft();
+    ParentLayerPoint bufferedLastZoomFocus =
+        (mPinchEventBuffer.size() > 1)
+            ? mPinchEventBuffer.front().mLocalFocusPoint -
+                  Metrics().GetCompositionBounds().TopLeft()
+            : mLastZoomFocus;
+
+    bufferedFocusChange = bufferedLastZoomFocus - focusPoint;
+    bufferedSpanDistance = fabsf(mPinchEventBuffer.front().mPreviousSpan -
+                                 mPinchEventBuffer.back().mCurrentSpan);
+  }
+
+  // Convert to screen coordinates
+  ScreenCoord spanDistance =
+      ToScreenCoordinates(ParentLayerPoint(0, bufferedSpanDistance), focusPoint)
+          .Length();
+  ScreenPoint focusChange =
+      ToScreenCoordinates(bufferedFocusChange, focusPoint);
+
   if (mPinchLocked) {
     if (GetPinchLockMode() == PINCH_STICKY) {
       ScreenCoord spanBreakoutThreshold =

gfx/layers/apz/src/AsyncPanZoomController.h

@@ -28,6 +28,7 @@
 #include "nsRegion.h"
 #include "nsTArray.h"
 #include "PotentialCheckerboardDurationTracker.h"
+#include "RecentEventsBuffer.h"  // for RecentEventsBuffer
 
 #include "base/message_loop.h"
 
@@ -795,7 +796,7 @@ class AsyncPanZoomController {
   /**
    * Set and update the pinch lock
    */
-  void HandlePinchLocking(ScreenCoord spanDistance, ScreenPoint focusChange);
+  void HandlePinchLocking();
 
   /**
    * Sets up anything needed for panning. This takes us out of the "TOUCHING"
@@ -983,6 +984,12 @@ class AsyncPanZoomController {
   // is performing a two-finger pan rather than a pinch gesture
   bool mPinchLocked;
 
+  // Stores the pinch events that occured within a given timeframe. Used to
+  // calculate the focusChange and spanDistance within a fixed timeframe.
+  // RecentEventsBuffer is not threadsafe. Should only be accessed on the
+  // controller thread.
+  RecentEventsBuffer<PinchGestureInput> mPinchEventBuffer;
+
   // Most up-to-date constraints on zooming. These should always be reasonable
   // values; for example, allowing a min zoom of 0.0 can cause very bad things
   // to happen.

gfx/layers/apz/src/RecentEventsBuffer.h

@@ -0,0 +1,83 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_layers_RecentEventsBuffer_h
+#define mozilla_layers_RecentEventsBuffer_h
+
+#include <deque>
+
+#include "mozilla/TimeStamp.h"
+
+namespace mozilla {
+namespace layers {
+/**
+ * RecentEventsBuffer: maintains an age constrained buffer of events
+ *
+ * Intended for use with elements of type InputData, but the only requirement
+ * is a member "mTimeStamp" of type TimeStamp
+ */
+template <typename Event>
+class RecentEventsBuffer {
+ public:
+  explicit RecentEventsBuffer(TimeDuration maxAge);
+
+  void push(Event event);
+  void clear();
+
+  typedef typename std::deque<Event>::size_type size_type;
+  size_type size() { return mBuffer.size(); }
+
+  // Delegate to container for iterators
+  typedef typename std::deque<Event>::iterator iterator;
+  typedef typename std::deque<Event>::const_iterator const_iterator;
+  iterator begin() { return mBuffer.begin(); }
+  iterator end() { return mBuffer.end(); }
+  const_iterator cbegin() const { return mBuffer.cbegin(); }
+  const_iterator cend() const { return mBuffer.cend(); }
+
+  // Also delegate for front/back
+  typedef typename std::deque<Event>::reference reference;
+  typedef typename std::deque<Event>::const_reference const_reference;
+  reference front() { return mBuffer.front(); }
+  reference back() { return mBuffer.back(); }
+  const_reference front() const { return mBuffer.front(); }
+  const_reference back() const { return mBuffer.back(); }
+
+ private:
+  TimeDuration mMaxAge;
+  std::deque<Event> mBuffer;
+};
+
+template <typename Event>
+RecentEventsBuffer<Event>::RecentEventsBuffer(TimeDuration maxAge)
+    : mMaxAge(maxAge), mBuffer() {}
+
+template <typename Event>
+void RecentEventsBuffer<Event>::push(Event event) {
+  // Events must be pushed in chronological order
+  MOZ_ASSERT(mBuffer.empty() || mBuffer.back().mTimeStamp <= event.mTimeStamp);
+
+  mBuffer.push_back(event);
+
+  // Flush all events older than the given lifetime
+  TimeStamp bound = event.mTimeStamp - mMaxAge;
+  while (!mBuffer.empty()) {
+    if (mBuffer.front().mTimeStamp >= bound) {
+      break;
+    }
+    mBuffer.pop_front();
+  }
+}
+
+template <typename Event>
+void RecentEventsBuffer<Event>::clear() {
+  mBuffer.clear();
+}
+
+}  // namespace layers
+}  // namespace mozilla
+
+#endif  // mozilla_layers_RecentEventsBuffer_h

gfx/layers/apz/test/gtest/APZTestCommon.h

@@ -62,6 +62,15 @@ inline SingleTouchData CreateSingleTouchData(int32_t aIdentifier,
   return CreateSingleTouchData(aIdentifier, ScreenIntPoint(aX, aY));
 }
 
+inline PinchGestureInput CreatePinchGestureInput(
+    PinchGestureInput::PinchGestureType aType, const ScreenPoint& aFocus,
+    float aCurrentSpan, float aPreviousSpan, TimeStamp timestamp) {
+  ParentLayerPoint localFocus(aFocus.x, aFocus.y);
+  PinchGestureInput result(aType, 0, timestamp, ExternalPoint(0, 0), aFocus,
+                           aCurrentSpan, aPreviousSpan, 0);
+  return result;
+}
+
 template <class SetArg, class Storage>
 class ScopedGfxSetting {
  public:
@@ -450,6 +459,18 @@ class APZCTesterBase : public ::testing::Test {
       float aScale, int& inputId, bool aShouldTriggerPinch,
       nsTArray<uint32_t>* aAllowedTouchBehaviors);
 
+  template <class InputReceiver>
+  void PinchWithPinchInput(const RefPtr<InputReceiver>& aTarget,
+                           const ScreenIntPoint& aFocus,
+                           const ScreenIntPoint& aSecondFocus, float aScale,
+                           nsEventStatus (*aOutEventStatuses)[3] = nullptr);
+
+  template <class InputReceiver>
+  void PinchWithPinchInputAndCheckStatus(const RefPtr<InputReceiver>& aTarget,
+                                         const ScreenIntPoint& aFocus,
+                                         float aScale,
+                                         bool aShouldTriggerPinch);
+
  protected:
   RefPtr<MockContentControllerDelayed> mcc;
 };
@@ -798,6 +819,57 @@ void APZCTesterBase::PinchWithTouchInputAndCheckStatus(
   EXPECT_EQ(expectedMoveStatus, statuses[2]);
 }
 
+template <class InputReceiver>
+void APZCTesterBase::PinchWithPinchInput(
+    const RefPtr<InputReceiver>& aTarget, const ScreenIntPoint& aFocus,
+    const ScreenIntPoint& aSecondFocus, float aScale,
+    nsEventStatus (*aOutEventStatuses)[3]) {
+  const TimeDuration TIME_BETWEEN_PINCH_INPUT =
+      TimeDuration::FromMilliseconds(50);
+
+  nsEventStatus actualStatus = aTarget->ReceiveInputEvent(
+      CreatePinchGestureInput(PinchGestureInput::PINCHGESTURE_START, aFocus,
+                              10.0, 10.0, mcc->Time()),
+      nullptr);
+  if (aOutEventStatuses) {
+    (*aOutEventStatuses)[0] = actualStatus;
+  }
+  mcc->AdvanceBy(TIME_BETWEEN_PINCH_INPUT);
+
+  actualStatus = aTarget->ReceiveInputEvent(
+      CreatePinchGestureInput(PinchGestureInput::PINCHGESTURE_SCALE,
+                              aSecondFocus, 10.0 * aScale, 10.0, mcc->Time()),
+      nullptr);
+  if (aOutEventStatuses) {
+    (*aOutEventStatuses)[1] = actualStatus;
+  }
+  mcc->AdvanceBy(TIME_BETWEEN_PINCH_INPUT);
+
+  actualStatus = aTarget->ReceiveInputEvent(
+      CreatePinchGestureInput(
+          PinchGestureInput::PINCHGESTURE_END,
+          PinchGestureInput::BothFingersLifted<ScreenPixel>(), 10.0 * aScale,
+          10.0 * aScale, mcc->Time()),
+      nullptr);
+  if (aOutEventStatuses) {
+    (*aOutEventStatuses)[2] = actualStatus;
+  }
+}
+
+template <class InputReceiver>
+void APZCTesterBase::PinchWithPinchInputAndCheckStatus(
+    const RefPtr<InputReceiver>& aTarget, const ScreenIntPoint& aFocus,
+    float aScale, bool aShouldTriggerPinch) {
+  nsEventStatus statuses[3];  // scalebegin, scale, scaleend
+  PinchWithPinchInput(aTarget, aFocus, aFocus, aScale, &statuses);
+
+  nsEventStatus expectedStatus = aShouldTriggerPinch
+                                     ? nsEventStatus_eConsumeNoDefault
+                                     : nsEventStatus_eIgnore;
+  EXPECT_EQ(expectedStatus, statuses[0]);
+  EXPECT_EQ(expectedStatus, statuses[1]);
+}
+
 AsyncPanZoomController* TestAPZCTreeManager::NewAPZCInstance(
     LayersId aLayersId, GeckoContentController* aController) {
   MockContentControllerDelayed* mcc =

gfx/layers/apz/test/gtest/InputUtils.h

@@ -28,15 +28,6 @@
  * code to dispatch input events.
  */
 
-inline PinchGestureInput CreatePinchGestureInput(
-    PinchGestureInput::PinchGestureType aType, const ScreenPoint& aFocus,
-    float aCurrentSpan, float aPreviousSpan) {
-  ParentLayerPoint localFocus(aFocus.x, aFocus.y);
-  PinchGestureInput result(aType, 0, TimeStamp(), ExternalPoint(0, 0), aFocus,
-                           aCurrentSpan, aPreviousSpan, 0);
-  return result;
-}
-
 template <class InputReceiver>
 void SetDefaultAllowedTouchBehavior(const RefPtr<InputReceiver>& aTarget,
                                     uint64_t aInputBlockId,
@@ -86,50 +77,6 @@ nsEventStatus TouchUp(const RefPtr<InputReceiver>& aTarget,
   return aTarget->ReceiveInputEvent(mti, nullptr, nullptr);
 }
 
-template <class InputReceiver>
-void PinchWithPinchInput(const RefPtr<InputReceiver>& aTarget,
-                         const ScreenIntPoint& aFocus,
-                         const ScreenIntPoint& aSecondFocus, float aScale,
-                         nsEventStatus (*aOutEventStatuses)[3] = nullptr) {
-  nsEventStatus actualStatus = aTarget->ReceiveInputEvent(
-      CreatePinchGestureInput(PinchGestureInput::PINCHGESTURE_START, aFocus,
-                              10.0, 10.0),
-      nullptr);
-  if (aOutEventStatuses) {
-    (*aOutEventStatuses)[0] = actualStatus;
-  }
-  actualStatus = aTarget->ReceiveInputEvent(
-      CreatePinchGestureInput(PinchGestureInput::PINCHGESTURE_SCALE,
-                              aSecondFocus, 10.0 * aScale, 10.0),
-      nullptr);
-  if (aOutEventStatuses) {
-    (*aOutEventStatuses)[1] = actualStatus;
-  }
-  actualStatus = aTarget->ReceiveInputEvent(
-      CreatePinchGestureInput(
-          PinchGestureInput::PINCHGESTURE_END,
-          PinchGestureInput::BothFingersLifted<ScreenPixel>(), 10.0 * aScale,
-          10.0 * aScale),
-      nullptr);
-  if (aOutEventStatuses) {
-    (*aOutEventStatuses)[2] = actualStatus;
-  }
-}
-
-template <class InputReceiver>
-void PinchWithPinchInputAndCheckStatus(const RefPtr<InputReceiver>& aTarget,
-                                       const ScreenIntPoint& aFocus,
-                                       float aScale, bool aShouldTriggerPinch) {
-  nsEventStatus statuses[3];  // scalebegin, scale, scaleend
-  PinchWithPinchInput(aTarget, aFocus, aFocus, aScale, &statuses);
-
-  nsEventStatus expectedStatus = aShouldTriggerPinch
-                                     ? nsEventStatus_eConsumeNoDefault
-                                     : nsEventStatus_eIgnore;
-  EXPECT_EQ(expectedStatus, statuses[0]);
-  EXPECT_EQ(expectedStatus, statuses[1]);
-}
-
 template <class InputReceiver>
 nsEventStatus Wheel(const RefPtr<InputReceiver>& aTarget,
                     const ScreenIntPoint& aPoint, const ScreenPoint& aDelta,