Add support for rect-based event targeting in views

ui/views/view.cc

Index: ui/views/view.cc
diff --git a/ui/views/view.cc b/ui/views/view.cc
index 81f0207bec7cfd07fc2ef3beefb6f6becada10a8..1047a0851ea7f61d776f740deb88f8fe58f6356a 100644
--- a/ui/views/view.cc
+++ b/ui/views/view.cc
@@ -63,6 +63,11 @@ const bool kContextMenuOnMousePress = false;
 const bool kContextMenuOnMousePress = true;
 #endif
 
+// The minimum percentage of a view's area that needs to be covered by a rect
+// representing a touch region in order for that view to be considered by the
+// views fuzzing algorithm.
+static const float kViewsFuzzingOverlap = 0.6;
+
 // Saves the drawing state, and restores the state when going out of scope.
 class ScopedCanvas {
  public:
@@ -700,6 +705,32 @@ void View::ConvertPointToTarget(const View* source,
 }
 
 // static
+void View::ConvertRectToTarget(const View* source,
+                               const View* target,
+                               gfx::Rect* rect) {
+  if (source == target)
+    return;
+
+  // |source| can be NULL.
+  const View* root = GetHierarchyRoot(target);
+  if (source) {
+    CHECK_EQ(GetHierarchyRoot(source), root);
+
+    if (source != root)
+      source->ConvertRectForAncestor(root, rect);
+  }
+
+  if (target != root)
+    target->ConvertRectFromAncestor(root, rect);
+
+  // API defines NULL |source| as returning the point in screen coordinates.
+  if (!source) {
+    rect->set_origin(rect->origin() -
+        root->GetWidget()->GetClientAreaBoundsInScreen().OffsetFromOrigin());
+  }
+}
+
+// static
 void View::ConvertPointToWidget(const View* src, gfx::Point* p) {
   DCHECK(src);
   DCHECK(p);
@@ -826,19 +857,65 @@ bool View::get_use_acceleration_when_possible() {
 // Input -----------------------------------------------------------------------
 
 View* View::GetEventHandlerForPoint(const gfx::Point& point) {

[sky, 2013/09/06 19:56:47]

Is there a reason to keep this? Keep it is error prone in so far as subclasses
that override one really need to override both.

[tdanderson, 2013/09/09 22:15:39]

I am keeping it as a convenience so that existing callers of
GetEventHandlerForPoint() don't need to do the work of converting a point to a
1x1 rect themselves. I have made View::GetEventHandlerForPoint() non-virtual, so
subclasses will only need to override View::GetEventHandlerForRect().

-  // Walk the child Views recursively looking for the View that most
-  // tightly encloses the specified point.
+  return GetEventHandlerForRect(gfx::Rect(point, gfx::Size(1, 1)));
+}
+
+View* View::GetEventHandlerForRect(const gfx::Rect& rect) {

[sky, 2013/09/06 19:56:47]

I kind of think the return value should be changed so that it returns not only
the view but also the hit region, or maybe a percent. That way you can end early
and you know the region of the view considered hit.

[tdanderson, 2013/09/09 22:15:39]

Please see my reply to your comment at line 897.

+  // |rect_view| represents the current best candidate to return
+  // if rect-based targeting (i.e., fuzzing) is used.
+  // |rect_view_distance| is used to keep track of the distance
+  // between the center point of |rect_view| and the center
+  // point of |rect|.
+  View* rect_view = NULL;
+  int rect_view_distance = INT_MAX;
+
+  // |point_view| represents the view that would have been returned
+  // from this function call if point-based targeting were used.
+  View* point_view = NULL;
+
   for (int i = child_count() - 1; i >= 0; --i) {
     View* child = child_at(i);
+
+    // Ignore any children which are invisible or do not intersect |rect|.
     if (!child->visible())
       continue;
+    gfx::Rect rect_in_child_coords(rect);
+    ConvertRectToTarget(this, child, &rect_in_child_coords);
+    if (!child->HitTestRect(rect_in_child_coords))
+      continue;
 
-    gfx::Point point_in_child_coords(point);
-    ConvertPointToTarget(this, child, &point_in_child_coords);
-    if (child->HitTestPoint(point_in_child_coords))
-      return child->GetEventHandlerForPoint(point_in_child_coords);
+    View* cur_view = child->GetEventHandlerForRect(rect_in_child_coords);
+
+    if (UsePointBasedTargeting(rect))
+      return cur_view;
+
+    gfx::Rect cur_rect(cur_view->GetLocalBounds());
+    ConvertRectToTarget(cur_view, this, &cur_rect);
+    if (PercentCoveredBy(cur_rect, rect) >= kViewsFuzzingOverlap) {

[sky, 2013/09/06 19:56:47]

Doesn't this assume the view completely fills its space, which is not
necessarily true? For example, tabs definitely don't fill their space.

[tdanderson, 2013/09/09 22:15:39]

Yes, good point... I am going to try and use the hit test mask (if applicable)
instead of GetLocalBounds(). Hopefully doing so will also resolve the problem I
have about occluded tab close buttons in the tab  strip being targeted.

+      // |cur_view| is a suitable candidate for rect-based targeting.
+      // Check to see if it is the closest suitable candidate so far.
+      gfx::Point touch_center(rect.CenterPoint());

[sky, 2013/09/06 19:56:47]

Can't you return some where in here if the view completely covers the area?

[tdanderson, 2013/09/09 22:15:39]

I don't think an early return is possible by just looking at the overlap
percentages because:

- It is possible for more than one view to completely cover |rect| if one of
these views is enclosed by the other, so an early return is not guaranteed to
give the correct result.
- It is possible for |rect| to completely cover more than one view, so even if
PercentCoveredBy(cur_rect, rect) == 1.0, that does not mean that |cur_view| is
the one that should be returned (distance is used to break 'ties' among views of
this type).

For this reason I don't think it would be helpful to change the return type of
View::GetEventHandlerForRect() as you suggested on line 863.

[sky, 2013/09/10 16:47:41]

99% of the views don't allow overlapping children, meaning you could early out
if you knew this. But I'm hesitant to add in API to determine if a view allows
overlapping children unless we identify this as a hotspot (which I doubt it is).
So, ok, make sense.

That said, your implementation does need to early out for the 1x1 case (the
mouse case), as we get way more mouse events than touch events.

+      // Terry: change this to just use distance between center points.
+      int cur_dist = DistanceSquaredFromCenterLineToPoint(touch_center,
+                                                          cur_rect);
+      if (!rect_view || cur_dist < rect_view_distance) {
+        rect_view = cur_view;
+        rect_view_distance = cur_dist;
+      }
+    } else if (!rect_view && !point_view) {
+      // Rect-based targeting has not yielded any candidates so far. Check
+      // if point-based targeting would have selected |cur_view|.
+      gfx::Point point_in_child_coords(rect.CenterPoint());
+      ConvertPointToTarget(this, child, &point_in_child_coords);
+      if (child->HitTestPoint(point_in_child_coords))

[sky, 2013/09/06 19:56:47]

HitTestPoint isn't enough, you really need to be invoking
GetEventHandlerForPoint here.

[tdanderson, 2013/09/09 22:15:39]

Done.

+        point_view = cur_view;
+    }
   }
-  return this;
+
+  if (UsePointBasedTargeting(rect) || (!rect_view && !point_view))
+    return this;
+
+  return rect_view ? rect_view : point_view;
 }
 
 View* View::GetTooltipHandlerForPoint(const gfx::Point& point) {
@@ -1269,6 +1346,52 @@ int View::GetLineScrollIncrement(ScrollView* scroll_view,
   return 0;
 }
 
+// Views fuzzing ---------------------------------------------------------------
+
+// static
+bool View::UsePointBasedTargeting(const gfx::Rect& rect) {
+  return rect.width() == 1 && rect.height() == 1;
+}
+
+// static
+float View::PercentCoveredBy(const gfx::Rect& rect_1, const gfx::Rect& rect_2) {
+  gfx::Rect intersection(rect_1);
+  intersection.Intersect(rect_2);
+  float intersection_area = intersection.size().GetArea();
+  float rect_1_area = rect_1.size().GetArea();
+  return rect_1_area ? intersection_area / rect_1_area : 0;
+}
+
+// The positive distance from |pos| to the nearest endpoint of the interval
+// [start, end] is returned if |pos| lies within the interval, otherwise
+// 0 is returned.
+int DistanceToInterval(int pos, int start, int end) {
+  if (pos < start)
+    return start - pos;
+  if (pos > end)
+    return pos - end;
+  return 0;
+}
+
+// static
+int View::DistanceSquaredFromCenterLineToPoint(const gfx::Point& point,
+                                               const gfx::Rect& target_rect) {
+  gfx::Point center_point = target_rect.CenterPoint();
+  int dx = center_point.x() - point.x();
+  int dy = center_point.y() - point.y();
+
+  if (target_rect.width() > target_rect.height()) {
+    dx = DistanceToInterval(point.x(),
+                            target_rect.x() + (target_rect.height() / 2),
+                            target_rect.right() - (target_rect.height() / 2));
+  } else {
+    dy = DistanceToInterval(point.y(),
+                            target_rect.y() + (target_rect.width() / 2),
+                            target_rect.bottom() - (target_rect.width() / 2));
+  }
+  return (dx * dx) + (dy * dy);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // View, protected:
 
@@ -2014,6 +2137,27 @@ bool View::ConvertPointFromAncestor(const View* ancestor,
   return result;
 }
 
+bool View::ConvertRectForAncestor(const View* ancestor,
+                                  gfx::Rect* rect) const {
+  gfx::Transform trans;
+  // TODO(sad): Have some way of caching the transformation results.
+  bool result = GetTransformRelativeTo(ancestor, &trans);
+  gfx::RectF r(*rect);
+  trans.TransformRect(&r);
+  *rect = gfx::ToFlooredRect(r);
+  return result;
+}
+
+bool View::ConvertRectFromAncestor(const View* ancestor,
+                                   gfx::Rect* rect) const {
+  gfx::Transform trans;
+  bool result = GetTransformRelativeTo(ancestor, &trans);
+  gfx::RectF r(*rect);
+  trans.TransformRectReverse(&r);
+  *rect = gfx::ToFlooredRect(r);
+  return result;
+}
+
 // Accelerated painting --------------------------------------------------------
 
 void View::CreateLayer() {