Changing the bounding box for show press and tap gesture

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 23d092a2082d5770ab82e24433c75aa78b487afb..041bb4407c1c4f4c30d84c9fba641a0d78d97811 100644
--- a/ui/events/gesture_detection/gesture_provider.cc
+++ b/ui/events/gesture_detection/gesture_provider.cc
@@ -11,6 +11,7 @@
 #include "ui/events/event_constants.h"
 #include "ui/events/gesture_detection/gesture_event_data.h"
 #include "ui/events/gesture_detection/motion_event.h"
+#include "ui/gfx/point.h"

[jdduke (slow), 2014/09/05 20:55:56]

Hmm, I looked for the gfx::Point usage, but I only see PointF?

[lanwei, 2014/09/08 15:46:37]

I changed to #include "ui/gfx/geometry/point_f.h"
Done.

 
 namespace ui {
 namespace {
@@ -36,93 +37,6 @@ const char* GetMotionEventActionName(MotionEvent::Action action) {
   return "";
 }
 
-gfx::RectF GetBoundingBox(const MotionEvent& event) {
-  // Can't use gfx::RectF::Union, as it ignores touches with a radius of 0.
-  float left = std::numeric_limits<float>::max();
-  float top = std::numeric_limits<float>::max();
-  float right = -std::numeric_limits<float>::max();
-  float bottom = -std::numeric_limits<float>::max();
-  for (size_t i = 0; i < event.GetPointerCount(); ++i) {
-    float diameter = event.GetTouchMajor(i);
-    float x = event.GetX(i) - diameter / 2;
-    float y = event.GetY(i) - diameter / 2;
-    left = std::min(left, x);
-    right = std::max(right, x + diameter);
-    top = std::min(top, y);
-    bottom = std::max(bottom, y + diameter);
-  }
-  return gfx::RectF(left, top, right - left, bottom - top);
-}
-
-GestureEventData CreateGesture(const GestureEventDetails& details,
-                               int motion_event_id,
-                               MotionEvent::ToolType primary_tool_type,
-                               base::TimeTicks time,
-                               float x,
-                               float y,
-                               float raw_x,
-                               float raw_y,
-                               size_t touch_point_count,
-                               const gfx::RectF& bounding_box) {
-  return GestureEventData(details,
-                          motion_event_id,
-                          primary_tool_type,
-                          time,
-                          x,
-                          y,
-                          raw_x,
-                          raw_y,
-                          touch_point_count,
-                          bounding_box);
-}
-
-GestureEventData CreateGesture(EventType type,
-                               int motion_event_id,
-                               MotionEvent::ToolType primary_tool_type,
-                               base::TimeTicks time,
-                               float x,
-                               float y,
-                               float raw_x,
-                               float raw_y,
-                               size_t touch_point_count,
-                               const gfx::RectF& bounding_box) {
-  return GestureEventData(GestureEventDetails(type, 0, 0),
-                          motion_event_id,
-                          primary_tool_type,
-                          time,
-                          x,
-                          y,
-                          raw_x,
-                          raw_y,
-                          touch_point_count,
-                          bounding_box);
-}
-
-GestureEventData CreateGesture(const GestureEventDetails& details,
-                               const MotionEvent& event) {
-  return GestureEventData(details,
-                          event.GetId(),
-                          event.GetToolType(),
-                          event.GetEventTime(),
-                          event.GetX(),
-                          event.GetY(),
-                          event.GetRawX(),
-                          event.GetRawY(),
-                          event.GetPointerCount(),
-                          GetBoundingBox(event));
-}
-
-GestureEventData CreateGesture(EventType type, const MotionEvent& event) {
-  return CreateGesture(GestureEventDetails(type, 0, 0), event);
-}
-
-GestureEventData CreateTapGesture(EventType type, const MotionEvent& event) {
-  // Set the tap count to 1 even for ET_GESTURE_DOUBLE_TAP, in order to be
-  // consistent with double tap behavior on a mobile viewport. See
-  // crbug.com/234986 for context.
-  return CreateGesture(GestureEventDetails(type, 1, 0), event);
-}
-
 gfx::RectF ClampBoundingBox(const gfx::RectF& bounds,
                             float min_length,
                             float max_length) {
@@ -173,7 +87,9 @@ class GestureProvider::GestureListenerImpl
         ignore_multitouch_zoom_events_(false),
         ignore_single_tap_(false),
         pinch_event_sent_(false),
-        scroll_event_sent_(false) {}
+        scroll_event_sent_(false),
+        max_diameter_before_show_press_(0),
+        show_press_event_sent_(false) {}
 
   void OnTouchEvent(const MotionEvent& event) {
     const bool in_scale_gesture = IsScaleGestureDetectionInProgress();
@@ -188,7 +104,10 @@ class GestureProvider::GestureListenerImpl
       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);
@@ -202,6 +121,12 @@ class GestureProvider::GestureListenerImpl
         Send(CreateGesture(ET_GESTURE_SCROLL_END, event));
       current_down_time_ = base::TimeTicks();
     }
+

[jdduke (slow), 2014/09/05 20:55:56]

Nit: Might as well make this an else if (action ==...

[lanwei, 2014/09/08 15:46:37]

Done.

+    if (action == MotionEvent::ACTION_MOVE) {
+      if (!show_press_event_sent_)

[jdduke (slow), 2014/09/05 20:55:56]

Maybe |if(!show_press_event_sent_ && !scroll_event_sent_)| ?

[lanwei, 2014/09/08 15:46:37]

Done.

+        max_diameter_before_show_press_ =
+            std::max(max_diameter_before_show_press_, event.GetTouchMajor());
+    }
   }
 
   void Send(GestureEventData gesture) {
@@ -297,7 +222,7 @@ class GestureProvider::GestureListenerImpl
                          detector.GetFocusX() + e.GetRawOffsetX(),
                          detector.GetFocusY() + e.GetRawOffsetY(),
                          e.GetPointerCount(),
-                         GetBoundingBox(e)));
+                         GetBoundingBox(e, ET_GESTURE_PINCH_BEGIN)));
     }
 
     if (std::abs(detector.GetCurrentSpan() - detector.GetPreviousSpan()) <
@@ -331,7 +256,7 @@ class GestureProvider::GestureListenerImpl
                        detector.GetFocusX() + e.GetRawOffsetX(),
                        detector.GetFocusY() + e.GetRawOffsetY(),
                        e.GetPointerCount(),
-                       GetBoundingBox(e)));
+                       GetBoundingBox(e, pinch_details.type())));
     return true;
   }
 
@@ -381,7 +306,7 @@ class GestureProvider::GestureListenerImpl
                          e1.GetRawX(),
                          e1.GetRawY(),
                          e2.GetPointerCount(),
-                         GetBoundingBox(e2)));
+                         GetBoundingBox(e2, scroll_details.type())));
       DCHECK(scroll_event_sent_);
     }
 
@@ -394,12 +319,12 @@ class GestureProvider::GestureListenerImpl
     }
 
     if (distance_x || distance_y) {
-      const gfx::RectF bounding_box = GetBoundingBox(e2);
+      GestureEventDetails scroll_details(
+          ET_GESTURE_SCROLL_UPDATE, -distance_x, -distance_y);
+      const gfx::RectF bounding_box = GetBoundingBox(e2, scroll_details.type());
       const gfx::PointF center = bounding_box.CenterPoint();
       const gfx::PointF raw_center =
           center + gfx::Vector2dF(e2.GetRawOffsetX(), e2.GetRawOffsetY());
-      GestureEventDetails scroll_details(
-          ET_GESTURE_SCROLL_UPDATE, -distance_x, -distance_y);
       Send(CreateGesture(scroll_details,
                          e2.GetId(),
                          e2.GetToolType(),
@@ -469,12 +394,13 @@ class GestureProvider::GestureListenerImpl
                        e1.GetRawX(),
                        e1.GetRawY(),
                        e2.GetPointerCount(),
-                       GetBoundingBox(e2)));
+                       GetBoundingBox(e2, two_finger_tap_details.type())));
     return true;
   }
 
   virtual void OnShowPress(const MotionEvent& e) OVERRIDE {
     GestureEventDetails show_press_details(ET_GESTURE_SHOW_PRESS, 0, 0);
+    show_press_event_sent_ = true;
     Send(CreateGesture(show_press_details, e));
   }
 
@@ -556,6 +482,103 @@ class GestureProvider::GestureListenerImpl
     Send(CreateGesture(long_press_details, e));
   }
 
+  GestureEventData CreateGesture(const GestureEventDetails& details,
+                                 int motion_event_id,
+                                 MotionEvent::ToolType primary_tool_type,
+                                 base::TimeTicks time,
+                                 float x,
+                                 float y,
+                                 float raw_x,
+                                 float raw_y,
+                                 size_t touch_point_count,
+                                 const gfx::RectF& bounding_box) {
+    return GestureEventData(details,
+                            motion_event_id,
+                            primary_tool_type,
+                            time,
+                            x,
+                            y,
+                            raw_x,
+                            raw_y,
+                            touch_point_count,
+                            bounding_box);
+  }
+
+  GestureEventData CreateGesture(EventType type,
+                                 int motion_event_id,
+                                 MotionEvent::ToolType primary_tool_type,
+                                 base::TimeTicks time,
+                                 float x,
+                                 float y,
+                                 float raw_x,
+                                 float raw_y,
+                                 size_t touch_point_count,
+                                 const gfx::RectF& bounding_box) {
+    return GestureEventData(GestureEventDetails(type, 0, 0),
+                            motion_event_id,
+                            primary_tool_type,
+                            time,
+                            x,
+                            y,
+                            raw_x,
+                            raw_y,
+                            touch_point_count,
+                            bounding_box);
+  }
+
+  GestureEventData CreateGesture(const GestureEventDetails& details,
+                                 const MotionEvent& event) {
+    return GestureEventData(details,
+                            event.GetId(),
+                            event.GetToolType(),
+                            event.GetEventTime(),
+                            event.GetX(),
+                            event.GetY(),
+                            event.GetRawX(),
+                            event.GetRawY(),
+                            event.GetPointerCount(),
+                            GetBoundingBox(event, details.type()));
+  }
+
+  GestureEventData CreateGesture(EventType type, const MotionEvent& event) {
+    return CreateGesture(GestureEventDetails(type, 0, 0), event);
+  }
+
+  GestureEventData CreateTapGesture(EventType type, const MotionEvent& event) {
+    // Set the tap count to 1 even for ET_GESTURE_DOUBLE_TAP, in order to be
+    // consistent with double tap behavior on a mobile viewport. See
+    // crbug.com/234986 for context.
+    return CreateGesture(GestureEventDetails(type, 1, 0), event);
+  }
+
+  gfx::RectF GetBoundingBox(const MotionEvent& event, EventType type) {
+    // Can't use gfx::RectF::Union, as it ignores touches with a radius of 0.
+    float left = std::numeric_limits<float>::max();
+    float top = std::numeric_limits<float>::max();
+    float right = -std::numeric_limits<float>::max();
+    float bottom = -std::numeric_limits<float>::max();
+    for (size_t i = 0; i < event.GetPointerCount(); ++i) {
+      float x, y, diameter;
+      if (type == ET_GESTURE_SHOW_PRESS || type == ET_GESTURE_TAP ||

[tdresser, 2014/09/08 13:24:36]

Lets add a comment explaining why we special case these gesture types.

[lanwei, 2014/09/08 15:46:37]

Done.

+          type == ET_GESTURE_TAP_UNCONFIRMED) {

[jdduke (slow), 2014/09/05 20:55:56]

Let's add a |DCHECK_EQ(0U, i)| here to be sure (I don't see that validated
anywhere else, maybe it's unnecessary?).

[lanwei, 2014/09/08 15:46:37]

It is a good idea that we check if it is single fingle.
Done.

+        diameter = max_diameter_before_show_press_;
+        x = tap_down_point_.x();
+        y = tap_down_point_.y();
+      } else {
+        diameter = event.GetTouchMajor(i);
+        x = event.GetX(i);
+        y = event.GetY(i);
+      }
+      x = x - diameter / 2;
+      y = y - diameter / 2;
+      left = std::min(left, x);
+      right = std::max(right, x + diameter);
+      top = std::min(top, y);
+      bottom = std::max(bottom, y + diameter);
+    }
+    return gfx::RectF(left, top, right - left, bottom - top);
+  }
+
   void SetDoubleTapEnabled(bool enabled) {
     DCHECK(!IsDoubleTapInProgress());
     gesture_detector_.SetDoubleTapListener(enabled ? this : NULL);
@@ -621,6 +644,14 @@ class GestureProvider::GestureListenerImpl
   bool pinch_event_sent_;
   bool scroll_event_sent_;
 
+  // The maximum touch diameter before show press gesture is sent.
+  float max_diameter_before_show_press_;
+
+  gfx::PointF tap_down_point_;
+
+  // Tracks whether ET_GESTURE_SHOW_PRESS event has been sent.

[tdresser, 2014/09/08 13:24:36]

Lets be a bit more specific here.

Something like:
Tracks whether an ET_GESTURE_SHOW_PRESS event has been sent for this touch
sequence.

+  bool show_press_event_sent_;
+
   DISALLOW_COPY_AND_ASSIGN(GestureListenerImpl);
 };
 
@@ -704,21 +735,23 @@ void GestureProvider::OnTouchEventHandlingBegin(const MotionEvent& event) {
     case MotionEvent::ACTION_DOWN:
       current_down_event_ = event.Clone();
       if (gesture_begin_end_types_enabled_)
-        gesture_listener_->Send(CreateGesture(ET_GESTURE_BEGIN, event));
+        gesture_listener_->Send(
+            gesture_listener_->CreateGesture(ET_GESTURE_BEGIN, event));
       break;
     case MotionEvent::ACTION_POINTER_DOWN:
       if (gesture_begin_end_types_enabled_) {
         const int action_index = event.GetActionIndex();
-        gesture_listener_->Send(CreateGesture(ET_GESTURE_BEGIN,
-                                              event.GetId(),
-                                              event.GetToolType(),
-                                              event.GetEventTime(),
-                                              event.GetX(action_index),
-                                              event.GetY(action_index),
-                                              event.GetRawX(action_index),
-                                              event.GetRawY(action_index),
-                                              event.GetPointerCount(),
-                                              GetBoundingBox(event)));
+        gesture_listener_->Send(gesture_listener_->CreateGesture(
+            ET_GESTURE_BEGIN,
+            event.GetId(),
+            event.GetToolType(),
+            event.GetEventTime(),
+            event.GetX(action_index),
+            event.GetY(action_index),
+            event.GetRawX(action_index),
+            event.GetRawY(action_index),
+            event.GetPointerCount(),
+            gesture_listener_->GetBoundingBox(event, ET_GESTURE_BEGIN)));
       }
       break;
     case MotionEvent::ACTION_POINTER_UP:
@@ -734,7 +767,8 @@ void GestureProvider::OnTouchEventHandlingEnd(const MotionEvent& event) {
     case MotionEvent::ACTION_UP:
     case MotionEvent::ACTION_CANCEL: {
       if (gesture_begin_end_types_enabled_)
-        gesture_listener_->Send(CreateGesture(ET_GESTURE_END, event));
+        gesture_listener_->Send(
+            gesture_listener_->CreateGesture(ET_GESTURE_END, event));
 
       current_down_event_.reset();
 
@@ -743,7 +777,8 @@ void GestureProvider::OnTouchEventHandlingEnd(const MotionEvent& event) {
     }
     case MotionEvent::ACTION_POINTER_UP:
       if (gesture_begin_end_types_enabled_)
-        gesture_listener_->Send(CreateGesture(ET_GESTURE_END, event));
+        gesture_listener_->Send(
+            gesture_listener_->CreateGesture(ET_GESTURE_END, event));
       break;
     case MotionEvent::ACTION_DOWN:
     case MotionEvent::ACTION_POINTER_DOWN: