Bug 1425565 - Use the container ASR for sticky items when building WebRender clips. r?mstange draft
authorKartikaya Gupta <kgupta@mozilla.com>
Fri, 18 May 2018 15:19:47 -0400
changeset 797238 6302abe6fd66fb581974b11650f3c42349a34f4b
parent 796870 11ee70f24ea52c4dc4f113593c288f4a6dc92c55
child 797239 7d9c66e8c2a3bb239460547c3ffae22ec7fbad66
push id110430
push userkgupta@mozilla.com
push dateFri, 18 May 2018 19:20:24 +0000
reviewersmstange
bugs1425565
milestone62.0a1
Bug 1425565 - Use the container ASR for sticky items when building WebRender clips. r?mstange Even if the sticky item has a fixed descendant, we want to use the sticky container item's "real" ASR when computing the WR clips because otherwise the WR item doesn't get attached to the correct scrolling layer. MozReview-Commit-ID: JVnzEIBeLKp
gfx/layers/wr/ClipManager.cpp
layout/generic/nsFrame.cpp
layout/painting/nsDisplayList.cpp
layout/painting/nsDisplayList.h
--- a/gfx/layers/wr/ClipManager.cpp
+++ b/gfx/layers/wr/ClipManager.cpp
@@ -141,16 +141,24 @@ ClipManager::ClipIdAfterOverride(const M
 void
 ClipManager::BeginItem(nsDisplayItem* aItem,
                        const StackingContextHelper& aStackingContext)
 {
   CLIP_LOG("processing item %p\n", aItem);
 
   const DisplayItemClipChain* clip = aItem->GetClipChain();
   const ActiveScrolledRoot* asr = aItem->GetActiveScrolledRoot();
+  if (aItem->GetType() == DisplayItemType::TYPE_STICKY_POSITION) {
+    // For sticky position items, the ASR is computed differently depending
+    // on whether the item has a fixed descendant or not. But for WebRender
+    // purposes we always want to use the ASR that would have been used if it
+    // didn't have fixed descendants, which is stored as the "container ASR" on
+    // the sticky item.
+    asr = static_cast<nsDisplayStickyPosition*>(aItem)->GetContainerASR();
+  }
 
   ItemClips clips(asr, clip);
   MOZ_ASSERT(!mItemClipStack.empty());
   if (clips.HasSameInputs(mItemClipStack.top())) {
     // Early-exit because if the clips are the same as aItem's previous sibling,
     // then we don't need to do do the work of popping the old stuff and then
     // pushing it right back on for the new item. Note that if aItem doesn't
     // have a previous sibling, that means BeginList would have been called
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -3397,20 +3397,26 @@ nsIFrame::BuildDisplayListForStackingCon
     // For position:sticky, the clip needs to be applied both to the sticky
     // container item and to the contents. The container item needs the clip
     // because a scrolled clip needs to move independently from the sticky
     // contents, and the contents need the clip so that they have finite
     // clipped bounds with respect to the container item's ASR. The latter is
     // a little tricky in the case where the sticky item has both fixed and
     // non-fixed descendants, because that means that the sticky container
     // item's ASR is the ASR of the fixed descendant.
+    // For WebRender display list building, though, we still want to know the
+    // the ASR that the sticky container item would normally have, so we stash
+    // that on the display item as the "container ASR" (i.e. the normal ASR of
+    // the container item, excluding the special behaviour induced by fixed
+    // descendants).
     const ActiveScrolledRoot* stickyASR =
       ActiveScrolledRoot::PickAncestor(containerItemASR, aBuilder->CurrentActiveScrolledRoot());
     resultList.AppendToTop(
-        MakeDisplayItem<nsDisplayStickyPosition>(aBuilder, this, &resultList, stickyASR));
+        MakeDisplayItem<nsDisplayStickyPosition>(aBuilder, this, &resultList,
+          stickyASR, aBuilder->CurrentActiveScrolledRoot()));
     if (aCreatedContainerItem) {
       *aCreatedContainerItem = true;
     }
   }
 
   /* If there's blending, wrap up the list in a blend-mode item. Note
    * that opacity can be applied before blending as the blend color is
    * not affected by foreground opacity (only background alpha).
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -7477,18 +7477,20 @@ nsDisplayTableFixedPosition::CreateForFi
   temp.AppendToTop(aImage);
 
   return MakeDisplayItem<nsDisplayTableFixedPosition>(aBuilder, aFrame, &temp, aIndex + 1, aAncestorFrame);
 }
 
 nsDisplayStickyPosition::nsDisplayStickyPosition(nsDisplayListBuilder* aBuilder,
                                                  nsIFrame* aFrame,
                                                  nsDisplayList* aList,
-                                                 const ActiveScrolledRoot* aActiveScrolledRoot)
+                                                 const ActiveScrolledRoot* aActiveScrolledRoot,
+                                                 const ActiveScrolledRoot* aContainerASR)
   : nsDisplayOwnLayer(aBuilder, aFrame, aList, aActiveScrolledRoot)
+  , mContainerASR(aContainerASR)
 {
   MOZ_COUNT_CTOR(nsDisplayStickyPosition);
 }
 
 void
 nsDisplayStickyPosition::SetClipChain(const DisplayItemClipChain* aClipChain,
                                       bool aStore)
 {
@@ -7714,24 +7716,24 @@ nsDisplayStickyPosition::CreateWebRender
       NSAppUnitsToFloatPixels(appliedOffset.y, auPerDevPixel)
     };
     wr::WrClipId id = aBuilder.DefineStickyFrame(wr::ToRoundedLayoutRect(bounds),
         topMargin.ptrOr(nullptr), rightMargin.ptrOr(nullptr),
         bottomMargin.ptrOr(nullptr), leftMargin.ptrOr(nullptr),
         vBounds, hBounds, applied);
 
     aBuilder.PushClip(id);
-    aManager->CommandBuilder().PushOverrideForASR(GetActiveScrolledRoot(), Some(id));
+    aManager->CommandBuilder().PushOverrideForASR(mContainerASR, Some(id));
   }
 
   nsDisplayOwnLayer::CreateWebRenderCommands(aBuilder, aResources, aSc,
       aManager, aDisplayListBuilder);
 
   if (stickyScrollContainer) {
-    aManager->CommandBuilder().PopOverrideForASR(GetActiveScrolledRoot());
+    aManager->CommandBuilder().PopOverrideForASR(mContainerASR);
     aBuilder.PopClip();
   }
 
   return true;
 }
 
 nsDisplayScrollInfoLayer::nsDisplayScrollInfoLayer(
   nsDisplayListBuilder* aBuilder,
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -5811,20 +5811,22 @@ public:
  * A display item used to represent sticky position elements. The contents
  * gets its own layer and creates a stacking context, and the layer will have
  * position-related metadata set on it.
  */
 class nsDisplayStickyPosition : public nsDisplayOwnLayer {
 public:
   nsDisplayStickyPosition(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame,
                           nsDisplayList* aList,
-                          const ActiveScrolledRoot* aActiveScrolledRoot);
+                          const ActiveScrolledRoot* aActiveScrolledRoot,
+                          const ActiveScrolledRoot* aContainerASR);
   nsDisplayStickyPosition(nsDisplayListBuilder* aBuilder,
                           const nsDisplayStickyPosition& aOther)
     : nsDisplayOwnLayer(aBuilder, aOther)
+    , mContainerASR(aOther.mContainerASR)
   {}
 
 #ifdef NS_BUILD_REFCNT_LOGGING
   virtual ~nsDisplayStickyPosition();
 #endif
 
   void SetClipChain(const DisplayItemClipChain* aClipChain, bool aStore) override;
   virtual nsDisplayWrapList* Clone(nsDisplayListBuilder* aBuilder) const override
@@ -5850,16 +5852,27 @@ public:
     return HasSameTypeAndClip(aItem) && mFrame == aItem->Frame();
   }
 
   virtual bool CreateWebRenderCommands(mozilla::wr::DisplayListBuilder& aBuilder,
                                        mozilla::wr::IpcResourceUpdateQueue& aResources,
                                        const StackingContextHelper& aSc,
                                        mozilla::layers::WebRenderLayerManager* aManager,
                                        nsDisplayListBuilder* aDisplayListBuilder) override;
+
+  const ActiveScrolledRoot* GetContainerASR() const
+  {
+    return mContainerASR;
+  }
+
+private:
+  // This stores the ASR that this sticky container item would have assuming it
+  // has no fixed descendants. This may be the same as the ASR returned by
+  // GetActiveScrolledRoot(), or it may be a descendant of that.
+  const ActiveScrolledRoot* mContainerASR;
 };
 
 class nsDisplayFixedPosition : public nsDisplayOwnLayer {
 public:
   nsDisplayFixedPosition(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame,
                          nsDisplayList* aList,
                          const ActiveScrolledRoot* aActiveScrolledRoot);
   nsDisplayFixedPosition(nsDisplayListBuilder* aBuilder,