Scroll on main if impl-hit testing isn't guaranteed to be correct

cc/trees/layer_tree_host_common.cc

 // Copyright 2011 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 "cc/trees/layer_tree_host_common.h"
 
 #include <algorithm>
 
 #include "base/debug/trace_event.h"
 #include "cc/base/math_util.h"
@@ skipping 2419 matching lines @@
       return true;
 
     current_layer = current_layer->parent();
   }
 
   // If we have finished walking all ancestors without having already exited,
   // then the point is not clipped by any ancestors.
   return false;
 }
 
+LayerImpl* LayerTreeHostCommon::FindFirstScrollingLayerThatIsHitByPoint(

[enne (OOO), 2014/04/15 21:25:54]

Ew, code duplication.  You're going to want something like this for raycasting
touch regions too.

How about a base::Callback or or templatize this with some class that has a bool
Query(LayerImpl*, gfx::PointF) function?

[Ian Vollick, 2014/04/15 23:27:27]

Yeah, it's pretty gross. Haste makes.. this cl. :)

It's not so cute, but I refactored the common bits in the loop to cut down on
duplication. WDYT?

+    const gfx::PointF& screen_space_point,
+    const LayerImplList& render_surface_layer_list) {
+  LayerImpl* found_layer = NULL;
+
+  typedef LayerIterator<LayerImpl> LayerIteratorType;
+  LayerIteratorType end = LayerIteratorType::End(&render_surface_layer_list);
+  for (LayerIteratorType it =
+           LayerIteratorType::Begin(&render_surface_layer_list);
+       it != end;
+       ++it) {
+    // We don't want to consider render_surfaces for hit testing.
+    if (!it.represents_itself())
+      continue;
+
+    LayerImpl* current_layer = (*it);
+
+    gfx::RectF content_rect(current_layer->content_bounds());
+    if (!PointHitsRect(screen_space_point,
+                       current_layer->screen_space_transform(),
+                       content_rect))
+      continue;
+
+    // At this point, we think the point does hit the layer, but we need to walk
+    // up the parents to ensure that the layer was not clipped in such a way
+    // that the hit point actually should not hit the layer.
+    if (PointIsClippedBySurfaceOrClipRect(screen_space_point, current_layer))
+      continue;
+
+    // Skip the HUD layer.

[enne (OOO), 2014/04/15 21:25:54]

Sadness.

[Ian Vollick, 2014/04/15 23:27:27]

yeh.

+    if (current_layer == current_layer->layer_tree_impl()->hud_layer())
+      continue;
+
+    if (current_layer->scrollable()) {
+      found_layer = current_layer;

[enne (OOO), 2014/04/15 21:25:54]

readability/debuggability nit: just return current_layer here, and return NULL
instead of break and at the end?

[Ian Vollick, 2014/04/15 23:27:27]

Done.

+      break;
+    }
+
+    if (current_layer->contents_opaque())

[enne (OOO), 2014/04/15 21:25:54]

I think for correctness, it is always ok to return NULL from this function
(since it will punt to the main thread).  However, what case is this solving?

A (scrollable)
-B (not scrollable child, scrolls with A, opaque)

For a point that hits A+B, you would return NULL from this function and scroll
on the main thread.  That seems inefficient.  Why not just return A from this
function and not have this case?

[Ian Vollick, 2014/04/15 23:27:27]

Good point. This was a stale idea from the algorithm Rick and I had been
discussing, but it's not actually helpful here. Removed.

+      break;
+  }
+
+  // This can potentially return NULL, which means the screen_space_point did
+  // not successfully hit test any layers, not even the root layer.
+  return found_layer;
+}
+
 LayerImpl* LayerTreeHostCommon::FindLayerThatIsHitByPoint(
     const gfx::PointF& screen_space_point,
     const LayerImplList& render_surface_layer_list) {
   LayerImpl* found_layer = NULL;
 
   typedef LayerIterator<LayerImpl> LayerIteratorType;
   LayerIteratorType end = LayerIteratorType::End(&render_surface_layer_list);
   for (LayerIteratorType
            it = LayerIteratorType::Begin(&render_surface_layer_list);
        it != end;
@@ skipping 65 matching lines @@
   // At this point, we think the point does hit the touch event handler region
   // on the layer, but we need to walk up the parents to ensure that the layer
   // was not clipped in such a way that the hit point actually should not hit
   // the layer.
   if (PointIsClippedBySurfaceOrClipRect(screen_space_point, layer_impl))
     return false;
 
   return true;
 }
 }  // namespace cc