Bug 1521579 - Only transform one bounding rect for text nodes when computing scroll anchoring bounding rect. r=dholbert a=lizzard DEVEDITION_66_0b8_BUILD1 DEVEDITION_66_0b8_RELEASE FIREFOX_66_0b8_BUILD1 FIREFOX_66_0b8_RELEASE
authorRyan Hunt <rhunt@eqrion.net>
Wed, 30 Jan 2019 14:42:58 +0000
changeset 515961 bf56566e9cdbf447be4dfe4f59504f3d812e9441
parent 515960 87d73e0f898fdcadfeba5ac3ab53546231d1b4bb
child 515962 ad4148070fd490ebd121c899dfccb9a6908db2b5
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, lizzard
bugs1521579
milestone66.0
Bug 1521579 - Only transform one bounding rect for text nodes when computing scroll anchoring bounding rect. r=dholbert a=lizzard When visiting a text frame with many continuations, traversing ancestors to compute the transform to the ancestor scroll frame can become very hot. This commit changes the algorithm to translate all the text continuations to an ancestor that can then be transformed just once. Differential Revision: https://phabricator.services.mozilla.com/D17730
layout/generic/ScrollAnchorContainer.cpp
--- a/layout/generic/ScrollAnchorContainer.cpp
+++ b/layout/generic/ScrollAnchorContainer.cpp
@@ -93,26 +93,48 @@ static void SetAnchorFlags(const nsIFram
  * https://drafts.csswg.org/css-scroll-anchoring-1/#scroll-anchoring-bounding-rect
  * [2] https://github.com/w3c/csswg-drafts/issues/3478
  * [3] https://bugzilla.mozilla.org/show_bug.cgi?id=1519541
  */
 static nsRect FindScrollAnchoringBoundingRect(const nsIFrame* aScrollFrame,
                                               nsIFrame* aCandidate) {
   MOZ_ASSERT(nsLayoutUtils::IsProperAncestorFrame(aScrollFrame, aCandidate));
   if (!!Text::FromNodeOrNull(aCandidate->GetContent())) {
+    // This is a frame for a text node. The spec says we need to accumulate the
+    // union of all line boxes in the coordinate space of the scroll frame
+    // accounting for transforms.
+    //
+    // To do this, we translate and accumulate the overflow rect for each text
+    // continuation to the coordinate space of the nearest ancestor block
+    // frame. Then we transform the resulting rect into the coordinate space of
+    // the scroll frame.
+    //
+    // Transforms aren't allowed on non-replaced inline boxes, so we can assume
+    // that these text node continuations will have the same transform as their
+    // nearest block ancestor. And it should be faster to transform their union
+    // rather than individually transforming each overflow rect
+    //
+    // XXX for fragmented blocks, blockAncestor will be an ancestor only to the
+    //     text continuations in the first block continuation. GetOffsetTo
+    //     should continue to work, but is it correct with transforms or a
+    //     performance hazard?
+    nsIFrame* blockAncestor =
+        nsLayoutUtils::FindNearestBlockAncestor(aCandidate);
+    MOZ_ASSERT(
+        nsLayoutUtils::IsProperAncestorFrame(aScrollFrame, blockAncestor));
     nsRect bounding;
     for (nsIFrame* continuation = aCandidate->FirstContinuation(); continuation;
          continuation = continuation->GetNextContinuation()) {
       nsRect overflowRect =
           continuation->GetScrollableOverflowRectRelativeToSelf();
-      nsRect transformed = nsLayoutUtils::TransformFrameRectToAncestor(
-          continuation, overflowRect, aScrollFrame);
-      bounding = bounding.Union(transformed);
+      overflowRect += continuation->GetOffsetTo(blockAncestor);
+      bounding = bounding.Union(overflowRect);
     }
-    return bounding;
+    return nsLayoutUtils::TransformFrameRectToAncestor(blockAncestor, bounding,
+                                                       aScrollFrame);
   }
 
   nsRect borderRect = aCandidate->GetRectRelativeToSelf();
   nsRect overflowRect = aCandidate->GetScrollableOverflowRectRelativeToSelf();
 
   NS_ASSERTION(overflowRect.Contains(borderRect),
                "overflow rect must include border rect, and the clamping logic "
                "here depends on that");