Change View::HitTest to View::HitTestRect

ui/views/view.cc

 // Copyright (c) 2012 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/views/view.h"
 
 #include <algorithm>
 
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "third_party/skia/include/core/SkRect.h"
 #include "ui/base/accessibility/accessibility_types.h"
 #include "ui/base/dragdrop/drag_drop_types.h"
 #include "ui/compositor/compositor.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/layer_animator.h"
 #include "ui/gfx/canvas.h"
 #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"
 #include "ui/views/drag_controller.h"
 #include "ui/views/layout/layout_manager.h"
 #include "ui/views/views_delegate.h"
 #include "ui/views/widget/native_widget_private.h"
 #include "ui/views/widget/root_view.h"
 #include "ui/views/widget/tooltip_manager.h"
 #include "ui/views/widget/widget.h"
@@ skipping 715 matching lines @@
 View* View::GetEventHandlerForPoint(const gfx::Point& point) {
   // Walk the child Views recursively looking for the View that most
   // tightly encloses the specified point.
   for (int i = child_count() - 1; i >= 0; --i) {
     View* child = child_at(i);
     if (!child->visible())
       continue;
 
     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;
 }
 
 gfx::NativeCursor View::GetCursor(const MouseEvent& event) {
 #if defined(OS_WIN) && !defined(USE_AURA)
   static HCURSOR arrow = LoadCursor(NULL, IDC_ARROW);
   return arrow;
 #else
   return gfx::kNullCursor;
 #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);
 #if defined(USE_AURA)
       // TODO: should we use this every where?
       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.
     return true;
   }
   // Outside our bounds.
   return false;
 }
 
 bool View::OnMousePressed(const MouseEvent& event) {
@@ skipping 1089 matching lines @@
 
   Widget* widget = GetWidget();
   if (widget)
     widget->UpdateRootLayers();
 }
 
 // Input -----------------------------------------------------------------------
 
 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;
 
   const bool enabled = enabled_;
   const bool result = OnMousePressed(event);
   // WARNING: we may have been deleted, don't use any View variables.
 
   if (!enabled)
     return result;
 
@@ skipping 24 matching lines @@
   // WARNING: we may have been deleted.
   return (context_menu_controller != NULL) || possible_drag;
 }
 
 void View::ProcessMouseReleased(const MouseEvent& event) {
   if (context_menu_controller_ && event.IsOnlyRightMouseButton()) {
     // Assume that if there is a context menu controller we won't be deleted
     // 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);
     }
   } else {
     OnMouseReleased(event);
   }
   // WARNING: we may have been deleted.
 }
 
 ui::TouchStatus View::ProcessTouchEvent(const TouchEvent& event) {
@@ skipping 150 matching lines @@
   gfx::Point widget_location(event.location());
   ConvertPointToWidget(this, &widget_location);
   GetWidget()->RunShellDrag(this, data, widget_location, drag_operations);
   return true;
 #else
   return false;
 #endif  // !defined(OS_MACOSX)
 }
 
 }  // namespace views