Bug 1462961 - Ensure the WebRenderLayerScrollData structure matches the APZ requirements. r?jrmuizel draft
authorKartikaya Gupta <kgupta@mozilla.com>
Wed, 23 May 2018 16:08:18 -0400
changeset 798967 b6cb6ef58608f70b8497e2bf7466130c4c9d033f
parent 798966 0cb4adf27c7c4fd7cab62043938dbdff4573a4d3
push id110903
push userkgupta@mozilla.com
push dateWed, 23 May 2018 20:09:12 +0000
reviewersjrmuizel
bugs1462961
milestone62.0a1
Bug 1462961 - Ensure the WebRenderLayerScrollData structure matches the APZ requirements. r?jrmuizel In cases where we were carrying down a transform from a nsDisplayTransform and then applying it to a child item's WRLSD, we could get incorrect behaviour when that child item had a different ASR (and therefore had a different APZ instance). This patch corrects that behaviour by ensuring that when we run into this case we fall back to creating two WRLSD items instead of one, where the new WRLSD is the parent of the other one, and holds the transform. The child WRLSD has the data from the child item's ASR. MozReview-Commit-ID: BE6HuZjwc8v
gfx/layers/wr/WebRenderCommandBuilder.cpp
--- a/gfx/layers/wr/WebRenderCommandBuilder.cpp
+++ b/gfx/layers/wr/WebRenderCommandBuilder.cpp
@@ -1366,21 +1366,62 @@ WebRenderCommandBuilder::CreateWebRender
         // Pop the thing we pushed before the recursion, so the topmost item on
         // the stack is enclosing display item's ASR (or the stack is empty)
         mAsrStack.pop_back();
         const ActiveScrolledRoot* stopAtAsr =
             mAsrStack.empty() ? nullptr : mAsrStack.back();
 
         int32_t descendants = mLayerScrollData.size() - layerCountBeforeRecursing;
 
+        // A deferred transform item is a nsDisplayTransform for which we did
+        // not create a dedicated WebRenderLayerScrollData item at the point
+        // that we encountered the item. Instead, we "deferred" the transform
+        // from that item to combine it into the WebRenderLayerScrollData produced
+        // by child display items. However, in the case where we have a child
+        // display item with a different ASR than the nsDisplayTransform item,
+        // we cannot do this, because it will not conform to APZ's expectations
+        // with respect to how the APZ tree ends up structured. In particular,
+        // the GetTransformToThis() for the child APZ (which is created for the
+        // child item's ASR) will not include the transform when we actually do
+        // want it to.
+        // When we run into this scenario, we solve it by creating two
+        // WebRenderLayerScrollData items; one that just holds the transform,
+        // that we deferred, and a child WebRenderLayerScrollData item that
+        // holds the scroll metadata for the child's ASR.
         Maybe<nsDisplayTransform*> deferred = aSc.GetDeferredTransformItem();
-        mLayerScrollData.emplace_back();
-        mLayerScrollData.back().Initialize(mManager->GetScrollData(), item,
-            descendants, stopAtAsr,
-            deferred ? Some((*deferred)->GetTransform().GetMatrix()) : Nothing());
+        if (deferred && (*deferred)->GetActiveScrolledRoot() != item->GetActiveScrolledRoot()) {
+          // This creates the child WebRenderLayerScrollData for |item|, but
+          // omits the transform (hence the Nothing() as the last argument to
+          // Initialize(...)). We also need to make sure that the ASR from
+          // the deferred transform item is not on this node, so we use that
+          // ASR as the "stop at" ASR for this WebRenderLayerScrollData.
+          mLayerScrollData.emplace_back();
+          mLayerScrollData.back().Initialize(mManager->GetScrollData(), item,
+              descendants, (*deferred)->GetActiveScrolledRoot(), Nothing());
+
+          // The above WebRenderLayerScrollData will also be a descendant of
+          // the transform-holding WebRenderLayerScrollData we create below.
+          descendants++;
+
+          // This creates the WebRenderLayerScrollData for the deferred transform
+          // item. This holds the transform matrix. and the remaining ASRs
+          // needed to complete the ASR chain (i.e. the ones from the stopAtAsr
+          // down to the deferred transform item's ASR, which must be "between"
+          // stopAtAsr and |item|'s ASR in the ASR tree.
+          mLayerScrollData.emplace_back();
+          mLayerScrollData.back().Initialize(mManager->GetScrollData(), *deferred,
+              descendants, stopAtAsr, Some((*deferred)->GetTransform().GetMatrix()));
+        } else {
+          // This is the "simple" case where we don't need to create two
+          // WebRenderLayerScrollData items; we can just create one that also
+          // holds the deferred transform matrix, if any.
+          mLayerScrollData.emplace_back();
+          mLayerScrollData.back().Initialize(mManager->GetScrollData(), item,
+              descendants, stopAtAsr, deferred ? Some((*deferred)->GetTransform().GetMatrix()) : Nothing());
+        }
       } else if (mLayerScrollData.size() != layerCountBeforeRecursing &&
                  !eventRegions.IsEmpty()) {
         // We are not forcing a new layer for |item|, but we did create some
         // layers while recursing. In this case, we need to make sure any
         // event regions that we were carrying end up on the right layer. So we
         // do an event region "flush" but retroactively; i.e. the event regions
         // end up on the layer that was mLayerScrollData.back() prior to the
         // recursion.