Bug 1365088 - Avoid calling HitTestingTreeNode::Untransform() twice for each node during hit testing. r=kats
authorBotond Ballo <botond@mozilla.com>
Wed, 17 May 2017 12:49:23 -0400
changeset 579694 7108c4b57bdd3ada41dc57ab022825a1057b899d
parent 579653 85e5d15c31691c89b82d6068c26260416493071f
child 579733 3fdd6a00215ba8bfc9a717d3ab0e7ba0e21d4700
push id59336
push userbballo@mozilla.com
push dateWed, 17 May 2017 16:57:07 +0000
reviewerskats
bugs1365088
milestone55.0a1
Bug 1365088 - Avoid calling HitTestingTreeNode::Untransform() twice for each node during hit testing. r=kats The patch also adds another gtest that exercises hit testing, and fixes a coordinate space bug in the gtest fixture. MozReview-Commit-ID: 3QYTofkKSZj
gfx/layers/apz/src/APZCTreeManager.cpp
gfx/layers/apz/src/HitTestingTreeNode.cpp
gfx/layers/apz/src/HitTestingTreeNode.h
gfx/layers/apz/test/gtest/APZCTreeManagerTester.h
gfx/layers/apz/test/gtest/TestHitTesting.cpp
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -1831,46 +1831,48 @@ APZCTreeManager::GetAPZCAtPoint(HitTesti
                                 HitTestingTreeNode** aOutScrollbarNode)
 {
   mTreeLock.AssertCurrentThreadOwns();
 
   // This walks the tree in depth-first, reverse order, so that it encounters
   // APZCs front-to-back on the screen.
   HitTestingTreeNode* resultNode;
   HitTestingTreeNode* root = aNode;
-  std::stack<ParentLayerPoint> hitTestPoints;
-  hitTestPoints.push(aHitTestPoint);
+  std::stack<LayerPoint> hitTestPoints;
+  hitTestPoints.push(ViewAs<LayerPixel>(aHitTestPoint,
+      PixelCastJustification::MovingDownToChildren));
 
   ForEachNode<ReverseIterator>(root,
       [&hitTestPoints](HitTestingTreeNode* aNode) {
-        if (aNode->IsOutsideClip(hitTestPoints.top())) {
+        ParentLayerPoint hitTestPointForParent = ViewAs<ParentLayerPixel>(hitTestPoints.top(),
+            PixelCastJustification::MovingDownToChildren);
+        if (aNode->IsOutsideClip(hitTestPointForParent)) {
           // If the point being tested is outside the clip region for this node
           // then we don't need to test against this node or any of its children.
           // Just skip it and move on.
           APZCTM_LOG("Point %f %f outside clip for node %p\n",
             hitTestPoints.top().x, hitTestPoints.top().y, aNode);
           return TraversalFlag::Skip;
         }
         // First check the subtree rooted at this node, because deeper nodes
         // are more "in front".
-        Maybe<LayerPoint> hitTestPointForChildLayers = aNode->Untransform(hitTestPoints.top());
+        Maybe<LayerPoint> hitTestPoint = aNode->Untransform(hitTestPointForParent);
         APZCTM_LOG("Transformed ParentLayer point %s to layer %s\n",
-                Stringify(hitTestPoints.top()).c_str(),
-                hitTestPointForChildLayers ? Stringify(hitTestPointForChildLayers.ref()).c_str() : "nil");
-        if (!hitTestPointForChildLayers) {
+                Stringify(hitTestPointForParent).c_str(),
+                hitTestPoint ? Stringify(hitTestPoint.ref()).c_str() : "nil");
+        if (!hitTestPoint) {
           return TraversalFlag::Skip;
         }
-        hitTestPoints.push(ViewAs<ParentLayerPixel>(hitTestPointForChildLayers.ref(),
-            PixelCastJustification::MovingDownToChildren));
+        hitTestPoints.push(hitTestPoint.ref());
         return TraversalFlag::Continue;
       },
       [&resultNode, &hitTestPoints, &aOutHitResult](HitTestingTreeNode* aNode) {
+        HitTestResult hitResult = aNode->HitTest(hitTestPoints.top());
         hitTestPoints.pop();
-        HitTestResult hitResult = aNode->HitTest(hitTestPoints.top());
-        APZCTM_LOG("Testing ParentLayer point %s against node %p\n",
+        APZCTM_LOG("Testing Layer point %s against node %p\n",
                 Stringify(hitTestPoints.top()).c_str(), aNode);
         if (hitResult != HitTestResult::HitNothing) {
           resultNode = aNode;
           // If event regions are disabled, *aOutHitResult will be HitLayer
           *aOutHitResult = hitResult;
           return TraversalFlag::Abort;
         }
         return TraversalFlag::Continue;
--- a/gfx/layers/apz/src/HitTestingTreeNode.cpp
+++ b/gfx/layers/apz/src/HitTestingTreeNode.cpp
@@ -258,32 +258,23 @@ HitTestingTreeNode::Untransform(const Pa
   Maybe<ParentLayerToLayerMatrix4x4> inverse = transform.MaybeInverse();
   if (inverse) {
     return UntransformBy(inverse.ref(), aPoint);
   }
   return Nothing();
 }
 
 HitTestResult
-HitTestingTreeNode::HitTest(const ParentLayerPoint& aPoint) const
+HitTestingTreeNode::HitTest(const LayerPoint& aPoint) const
 {
-  // This should only ever get called if the point is inside the clip region
-  // for this node.
-  MOZ_ASSERT(!IsOutsideClip(aPoint));
-
   if (mOverride & EventRegionsOverride::ForceEmptyHitRegion) {
     return HitTestResult::HitNothing;
   }
 
-  // convert into Layer coordinate space
-  Maybe<LayerPoint> pointInLayerPixels = Untransform(aPoint);
-  if (!pointInLayerPixels) {
-    return HitTestResult::HitNothing;
-  }
-  auto point = LayerIntPoint::Round(pointInLayerPixels.ref());
+  auto point = LayerIntPoint::Round(aPoint);
 
   // test against event regions in Layer coordinate space
   if (!mEventRegions.mHitRegion.Contains(point.x, point.y)) {
     return HitTestResult::HitNothing;
   }
   if ((mOverride & EventRegionsOverride::ForceDispatchToContent) ||
       mEventRegions.mDispatchToContentHitRegion.Contains(point.x, point.y))
   {
--- a/gfx/layers/apz/src/HitTestingTreeNode.h
+++ b/gfx/layers/apz/src/HitTestingTreeNode.h
@@ -105,17 +105,17 @@ public:
   void SetFixedPosData(FrameMetrics::ViewID aFixedPosTarget);
   FrameMetrics::ViewID GetFixedPosTarget() const;
 
   /* Convert aPoint into the LayerPixel space for the layer corresponding to
    * this node. */
   Maybe<LayerPoint> Untransform(const ParentLayerPoint& aPoint) const;
   /* Assuming aPoint is inside the clip region for this node, check which of the
    * event region spaces it falls inside. */
-  HitTestResult HitTest(const ParentLayerPoint& aPoint) const;
+  HitTestResult HitTest(const LayerPoint& aPoint) const;
   /* Returns the mOverride flag. */
   EventRegionsOverride GetEventRegionsOverride() const;
 
   /* Debug helpers */
   void Dump(const char* aPrefix = "") const;
 
 private:
   void SetApzcParent(AsyncPanZoomController* aApzc);
--- a/gfx/layers/apz/test/gtest/APZCTreeManagerTester.h
+++ b/gfx/layers/apz/test/gtest/APZCTreeManagerTester.h
@@ -91,18 +91,19 @@ protected:
           RoundedToInt(metrics.GetCompositionBounds().TopLeft().ToUnknownPoint()),
           scrollRect.Size()));
       aLayer->SetEventRegions(er);
     }
   }
 
   static void SetScrollableFrameMetrics(Layer* aLayer, FrameMetrics::ViewID aScrollId,
                                         CSSRect aScrollableRect = CSSRect(-1, -1, -1, -1)) {
-    ParentLayerIntRect compositionBounds = ViewAs<ParentLayerPixel>(
-        aLayer->GetVisibleRegion().ToUnknownRegion().GetBounds());
+    ParentLayerIntRect compositionBounds =
+        RoundedToInt(aLayer->GetLocalTransformTyped().
+            TransformBounds(LayerRect(aLayer->GetVisibleRegion().GetBounds())));
     ScrollMetadata metadata = BuildScrollMetadata(aScrollId, aScrollableRect,
         ParentLayerRect(compositionBounds));
     aLayer->SetScrollMetadata(metadata);
     aLayer->SetClipRect(Some(compositionBounds));
     SetEventRegionsBasedOnBottommostMetrics(aLayer);
   }
 
   void SetScrollHandoff(Layer* aChild, Layer* aParent) {
--- a/gfx/layers/apz/test/gtest/TestHitTesting.cpp
+++ b/gfx/layers/apz/test/gtest/TestHitTesting.cpp
@@ -273,16 +273,40 @@ TEST_F(APZHitTestingTester, HitTesting2)
   hit = GetTargetAPZC(ScreenPoint(25, 25));
   EXPECT_EQ(apzcroot, hit.get());
   // transformToApzc doesn't unapply the root's own async transform
   EXPECT_EQ(ParentLayerPoint(25, 25), transformToApzc.TransformPoint(ScreenPoint(25, 25)));
   // transformToGecko unapplies the full async transform of -100 pixels
   EXPECT_EQ(ScreenPoint(25, 25), transformToGecko.TransformPoint(ParentLayerPoint(25, 25)));
 }
 
+TEST_F(APZHitTestingTester, HitTesting3) {
+  const char* layerTreeSyntax = "c(t)";
+  // LayerID                     0 1
+  nsIntRegion layerVisibleRegions[] = {
+      nsIntRegion(IntRect(0,0,200,200)),
+      nsIntRegion(IntRect(0,0,50,50))
+  };
+  Matrix4x4 transforms[] = {
+      Matrix4x4(),
+      Matrix4x4::Scaling(2, 2, 1)
+  };
+  root = CreateLayerTree(layerTreeSyntax, layerVisibleRegions, transforms, lm, layers);
+  // No actual room to scroll
+  SetScrollableFrameMetrics(root, FrameMetrics::START_SCROLL_ID, CSSRect(0, 0, 200, 200));
+  SetScrollableFrameMetrics(layers[1], FrameMetrics::START_SCROLL_ID + 1, CSSRect(0, 0, 50, 50));
+
+  ScopedLayerTreeRegistration registration(manager, 0, root, mcc);
+
+  manager->UpdateHitTestingTree(0, root, false, 0, 0);
+
+  RefPtr<AsyncPanZoomController> hit = GetTargetAPZC(ScreenPoint(75, 75));
+  EXPECT_EQ(ApzcOf(layers[1]), hit.get());
+}
+
 TEST_F(APZHitTestingTester, ComplexMultiLayerTree) {
   CreateComplexMultiLayerTree();
   ScopedLayerTreeRegistration registration(manager, 0, root, mcc);
   manager->UpdateHitTestingTree(0, root, false, 0, 0);
 
   /* The layer tree looks like this:
 
                 0