Change View::HitTest to View::HitTestRect

ui/views/view.cc

Index: ui/views/view.cc
diff --git a/ui/views/view.cc b/ui/views/view.cc
index 80bd221359589cc850369a3bb9de561f8537f722..65510abe66baeb237fa52b979923f259cdc45925 100644
--- a/ui/views/view.cc
+++ b/ui/views/view.cc
@@ -22,6 +22,7 @@
 #include "ui/gfx/interpolated_transform.h"
 #include "ui/gfx/path.h"
 #include "ui/gfx/point3.h"
+#include "ui/gfx/skia_util.h"
 #include "ui/gfx/transform.h"
 #include "ui/views/background.h"
 #include "ui/views/context_menu_controller.h"
@@ -757,7 +758,7 @@ View* View::GetEventHandlerForPoint(const gfx::Point& point) {
 
     gfx::Point point_in_child_coords(point);
     View::ConvertPointToView(this, child, &point_in_child_coords);
-    if (child->HitTest(point_in_child_coords))
+    if (child->HitTest(gfx::Rect(point_in_child_coords, gfx::Size(0, 0))))
       return child->GetEventHandlerForPoint(point_in_child_coords);
   }
   return this;
@@ -772,8 +773,10 @@ gfx::NativeCursor View::GetCursor(const MouseEvent& event) {
 #endif
 }
 
-bool View::HitTest(const gfx::Point& l) const {
-  if (GetLocalBounds().Contains(l)) {
+bool View::HitTest(const gfx::Rect& r) const {
+  if (r.IsEmpty() ?
+      GetLocalBounds().Contains(r.x(), r.y()) :
+      GetLocalBounds().Intersects(r)) {

[tdanderson, 2012/08/07 16:18:28]

I made the decision to represent a point using a rect of width and height 0. But
on second thought, after making this change I realize that doing so introduces
the inconvenience of needing to call Contains if the rect is empty vs. calling
Intersects if it is not. If I instead represent a point as a rect having width
and height 1, Intersects can be called in both cases. Let me know if you agree
with this change and I will make it.

[sky, 2012/08/07 20:11:43]

I like whatever is easiest all around. If you say a rect of 1x1 is easier than
0x0, go with that.

[tdanderson, 2012/08/08 23:47:51]

I have gone with 1x1 instead of 0x0 so that HitTestRect can call Intersects in
all cases (rather than needing to call Contains in the special case of an empty
rect).

     if (HasHitTestMask()) {
       gfx::Path mask;
       GetHitTestMask(&mask);
@@ -782,11 +785,17 @@ bool View::HitTest(const gfx::Point& l) const {
       SkRegion clip_region;
       clip_region.setRect(0, 0, width(), height());
       SkRegion mask_region;
-      return mask_region.setPath(mask, clip_region) &&
-          mask_region.contains(l.x(), l.y());
+      if (!mask_region.setPath(mask, clip_region))
+        return false;
+      if (r.IsEmpty())
+        return mask_region.contains(r.x(), r.y());
+      return mask_region.intersects(RectToSkIRect(r));
 #elif defined(OS_WIN)
       base::win::ScopedRegion rgn(mask.CreateNativeRegion());
-      return !!PtInRegion(rgn, l.x(), l.y());
+      if (r.IsEmpty())
+        return !!PtInRegion(rgn, r.x(), r.y());
+      const RECT* rect(r.x(), r.y(), r.width(), r.height());
+      return !!RectInRegion(rgn, rect);

[tdanderson, 2012/08/07 16:18:28]

I have not tested that above two lines of code are correct.

[sky, 2012/08/07 20:11:43]

I don't think they are. Shouldn't rect be a value and you do &rect to
RectInRegion? Also, nuke the !! it's hard to read.

[tdanderson, 2012/08/08 23:47:51]

Changed. (I thought that the !! was needed to force the (int) BOOL to bool?)

[sky, 2012/08/09 00:30:14]

It is, I'm suggesting coverting to == or !=

[tdanderson, 2012/08/09 16:16:46]

Done.

 #endif
     }
     // No mask, but inside our bounds.
@@ -1896,8 +1905,10 @@ void View::DestroyLayer() {
 
 bool View::ProcessMousePressed(const MouseEvent& event, DragInfo* drag_info) {
   int drag_operations =
-      (enabled_ && event.IsOnlyLeftMouseButton() && HitTest(event.location())) ?
-      GetDragOperations(event.location()) : 0;
+      (enabled_ &&
+       event.IsOnlyLeftMouseButton() &&
+       HitTest(gfx::Rect(event.location(), gfx::Size(0, 0)))) ?
+       GetDragOperations(event.location()) : 0;
   ContextMenuController* context_menu_controller = event.IsRightMouseButton() ?
       context_menu_controller_ : 0;
 
@@ -1942,7 +1953,7 @@ void View::ProcessMouseReleased(const MouseEvent& event) {
     // from mouse released.
     gfx::Point location(event.location());
     OnMouseReleased(event);
-    if (HitTest(location)) {
+    if (HitTest(gfx::Rect(location, gfx::Size(0, 0)))) {
       ConvertPointToScreen(this, &location);
       ShowContextMenu(location, true);
     }