Slop region check for multi-finger scroll

ui/events/gesture_detection/gesture_provider.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/events/gesture_detection/gesture_provider.h"
 
 #include <stddef.h>
 
 #include <cmath>
 
@@ skipping 100 matching lines @@
       current_down_time_ = event.GetEventTime();
       current_longpress_time_ = base::TimeTicks();
       ignore_single_tap_ = false;
       scroll_event_sent_ = false;
       pinch_event_sent_ = false;
       show_press_event_sent_ = false;
       gesture_detector_.set_longpress_enabled(true);
       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);
 
     if (action == MotionEvent::ACTION_UP ||
         action == MotionEvent::ACTION_CANCEL) {
       // Note: This call will have no effect if a fling was just generated, as
       // |Fling()| will have already signalled an end to touch-scrolling.
       if (scroll_event_sent_)
         Send(CreateGesture(ET_GESTURE_SCROLL_END, event));
       current_down_time_ = base::TimeTicks();
@@ skipping 145 matching lines @@
     GestureEventDetails tap_details(ET_GESTURE_TAP_DOWN);
     tap_details.set_device_type(GestureDeviceType::DEVICE_TOUCHSCREEN);
     Send(CreateGesture(tap_details, e));
 
     // Return true to indicate that we want to handle touch.
     return true;
   }
 
   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);
     if (snap_scroll_controller_.IsSnappingScrolls()) {
       if (snap_scroll_controller_.IsSnapHorizontal())
         distance_y = 0;
       else
         distance_x = 0;
     }
 
@@ skipping 333 matching lines @@
   bool InAnchoredScaleMode() const {
     return scale_gesture_detector_.InAnchoredScaleMode();
   }
 
   bool IsDoubleTapEnabled() const {
     return gesture_detector_.has_doubletap_listener();
   }
 
   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_;
 
   GestureDetector gesture_detector_;
   ScaleGestureDetector scale_gesture_detector_;
   SnapScrollController snap_scroll_controller_;
 
   base::TimeTicks current_down_time_;
 
   // Keeps track of the current GESTURE_LONG_PRESS event. If a context menu is
@@ skipping 180 matching lines @@
   // null'ing of the listener until the sequence has ended.
   if (current_down_event_)
     return;
 
   const bool double_tap_enabled =
       double_tap_support_for_page_ && double_tap_support_for_platform_;
   gesture_listener_->SetDoubleTapEnabled(double_tap_enabled);
 }
 
 }  //  namespace ui