Slop region check for multi-finger scroll

ui/events/gesture_detection/gesture_provider.cc

Index: ui/events/gesture_detection/gesture_provider.cc
diff --git a/ui/events/gesture_detection/gesture_provider.cc b/ui/events/gesture_detection/gesture_provider.cc
index 5db66be1a2ed0bb42e5eace9f5d66fa8c1c843f8..1e4eb12af840efe09b674e1f98809232a6cf7a74 100644
--- a/ui/events/gesture_detection/gesture_provider.cc
+++ b/ui/events/gesture_detection/gesture_provider.cc
@@ -118,7 +118,6 @@ class GestureProvider::GestureListenerImpl : public ScaleGestureListener,
       tap_down_point_ = gfx::PointF(event.GetX(), event.GetY());
       max_diameter_before_show_press_ = event.GetTouchMajor();
     }
-
     gesture_detector_.OnTouchEvent(event);
     scale_gesture_detector_.OnTouchEvent(event);
 
@@ -284,25 +283,17 @@ class GestureProvider::GestureListenerImpl : public ScaleGestureListener,
 
   bool OnScroll(const MotionEvent& e1,
                 const MotionEvent& e2,
+                const MotionEvent& secondary_pointer_down_event,
                 float raw_distance_x,
                 float raw_distance_y) override {
     float distance_x = raw_distance_x;
     float distance_y = raw_distance_y;
-    if (!scroll_event_sent_ && e2.GetPointerCount() == 1) {
-      // Remove the touch slop region from the first scroll event to
-      // avoid a jump. Touch slop isn't used for multi-finger
-      // gestures, so in those cases we don't subtract the slop.
-      float distance =
-          std::sqrt(distance_x * distance_x + distance_y * distance_y);
-      float epsilon = 1e-3f;
-      if (distance > epsilon) {
-        float ratio =
-            std::max(0.f,
-                     distance - config_.gesture_detector_config.touch_slop) /
-            distance;
-        distance_x *= ratio;
-        distance_y *= ratio;
-      }
+    if (!scroll_event_sent_ && e2.GetPointerCount() < 3) {
+      // Remove the touch slop region from the first scroll event to avoid a
+      // jump. Touch slop isn't used for scroll gestures with greater than 2
+      // pointers down, in those cases we don't subtract the slop.
+      SetScrollValues(e1, e2, secondary_pointer_down_event,
+                      distance_x, distance_y);
     }
 
     snap_scroll_controller_.UpdateSnapScrollMode(distance_x, distance_y);
@@ -656,6 +647,50 @@ class GestureProvider::GestureListenerImpl : public ScaleGestureListener,
 
   void SetIgnoreSingleTap(bool value) { ignore_single_tap_ = value; }
 
+  // Calculate the subtraction value for touch scrolls

[tdresser, 2016/06/28 15:24:19]

Missing period.

[sahel, 2016/06/29 16:26:19]

Done.

+  void SetScrollValues(const MotionEvent& ev1,
+        const MotionEvent& ev2, const MotionEvent& secondary_pointer_down_event,
+        float& scroll_x, float& scroll_y) {

[tdresser, 2016/06/28 15:24:19]

Maybe return a Point instead of using out parameters?

Let's name this a bit more precisely, something like - ComputeFirstScrollDelta?
I think the method comment could be a bit clearer too.

[sahel, 2016/06/29 16:26:19]

Done.

+
+    // If there are more than two down pointers, tapping is not possible.
+    // Slop region is not deducted.

[tdresser, 2016/06/28 15:24:19]

"tapping is not possible, so the slop region is not deducted"

Let's just make it a bit clearer that these are related.

[sahel, 2016/06/29 16:26:19]

Done.

+    if (ev2.GetPointerCount() > 2)
+      return;
+
+    const int id0 = ev1.GetPointerId(0);
+    const int ev_idx0 = ev2.GetPointerId(0) == id0 ? 0 : 1;
+
+    // Subtract the slop region from the first pointer move.
+    float dx0 = ev1.GetX() - ev2.GetX(ev_idx0);
+    float dy0 = ev1.GetY() - ev2.GetY(ev_idx0);
+    float distance0 = std::sqrt(dx0 * dx0 + dy0 * dy0);
+    float epsilon = 1e-3f;
+    if (distance0 > epsilon) {
+      float ratio = std::max(0.f, distance0 -
+                    config_.gesture_detector_config.touch_slop ) / distance0;
+      dx0 *= ratio;
+      dy0 *= ratio;
+    }
+    float dx1 = 0;
+    float dy1 = 0;
+    // Subtract the slop region from the second pointer move.
+    if (ev2.GetPointerCount() == 2 ) {
+      const int ev_idx1 = ev_idx0 == 0 ? 1 : 0;
+      const int idx1 = secondary_pointer_down_event.GetActionIndex();
+      dx1 = secondary_pointer_down_event.GetX(idx1) - ev2.GetX(ev_idx1);
+      dy1 = secondary_pointer_down_event.GetY(idx1) - ev2.GetY(ev_idx1);
+      float distance1 = std::sqrt(dx1 * dx1 + dy1 * dy1);
+      if (distance1 > epsilon) {
+        float ratio = std::max(0.f, distance1 -
+                      config_.gesture_detector_config.touch_slop ) / distance1;
+        dx1 *= ratio;
+        dy1 *= ratio;
+      }
+    }
+    scroll_x = (dx0 + dx1) / ev2.GetPointerCount();
+    scroll_y = (dy0 + dy1) / ev2.GetPointerCount();

[tdresser, 2016/06/28 15:24:19]

This makes sense to me, but we should definitely try it on a device or two.

One way to make it easier to see issues here would be to try a build with a
massive slop region size.

[sahel, 2016/06/29 16:26:19]

Sure, I'll play with the code, with a massive slop region.

+  }
+
   const GestureProvider::Config config_;
   GestureProviderClient* const client_;