Bug 1411249 - Handle yet more clipping cases. r=mstange
authorKartikaya Gupta <kgupta@mozilla.com>
Fri, 27 Oct 2017 13:22:16 -0400
changeset 388752 aa6f720f86f0bad16603df02791095d79311ad88
parent 388751 d76b2163ba1ae8021d9ac555f670e88acb3a23c9
child 388753 8b8017ae9cc103830e28e9209a063756059b4a18
child 388773 b66c75b4aa617fc7a37c889f0fc9a9934c6f07d6
child 388854 6982043bdd3519981fba7817977542cc80600a33
push id54255
push userkgupta@mozilla.com
push dateFri, 27 Oct 2017 17:38:59 +0000
treeherderautoland@aa6f720f86f0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1411249, 1373802
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1411249 - Handle yet more clipping cases. r=mstange This extends the fix in bug 1373802 to account for extra levels of display item nesting. If those extra intermediate display items don't push any clips we still want to pick up the ClipAndScroll from the enclosing ancestor that has it. MozReview-Commit-ID: AmxRz4fBKnX
gfx/layers/wr/ScrollingLayersHelper.cpp
gfx/layers/wr/ScrollingLayersHelper.h
layout/reftests/async-scrolling/fixed-pos-scrolled-clip-5-ref.html
layout/reftests/async-scrolling/fixed-pos-scrolled-clip-5.html
layout/reftests/async-scrolling/reftest.list
--- a/gfx/layers/wr/ScrollingLayersHelper.cpp
+++ b/gfx/layers/wr/ScrollingLayersHelper.cpp
@@ -113,24 +113,25 @@ ScrollingLayersHelper::BeginItem(nsDispl
   // If the leafmost ASR is not the same as the item's ASR then we are dealing
   // with a case where the item's clip chain is scrolled by something other than
   // the item's ASR. So for those cases we need to use the ClipAndScroll API.
   bool needClipAndScroll = (leafmostId != scrollId);
 
   // The other scenario where we need to push a ClipAndScroll is when we are
   // in a nested display item where the enclosing item pushed a ClipAndScroll,
   // and our clip chain extends from that item's clip chain. To check this we
-  // want to make sure that (a) we are InsideClipAndScroll(), and (b) nothing
+  // want to make sure that (a) we are inside a ClipAndScroll, and (b) nothing
   // else was pushed onto mBuilder's stack since that ClipAndScroll.
   if (!needClipAndScroll &&
-      InsideClipAndScroll() &&
       mBuilder->TopmostScrollId() == scrollId &&
       !mBuilder->TopmostIsClip()) {
-    MOZ_ASSERT(mItemClipStack.back().mClipAndScroll->first == scrollId);
-    needClipAndScroll = true;
+    if (auto cs = EnclosingClipAndScroll()) {
+      MOZ_ASSERT(cs->first == scrollId);
+      needClipAndScroll = true;
+    }
   }
 
   // If we don't need a ClipAndScroll, ensure the item's ASR is at the top of
   // the scroll stack
   if (!needClipAndScroll && mBuilder->TopmostScrollId() != scrollId) {
     MOZ_ASSERT(leafmostId == scrollId); // because !needClipAndScroll
     clips.mScrollId = Some(scrollId);
   }
@@ -287,30 +288,30 @@ ScrollingLayersHelper::RecurseAndDefineC
         // pointer will be null for the nested item but the containing wrapper's
         // clip will be on the stack already and we can pick it up from there.
         // Another way of thinking about this is that if the clip chain were
         // "fully completed" then aChain->mParent wouldn't be null but would point
         // to the clip corresponding to mBuilder->TopmostClipId(), and we would
         // have gone into the |aChain->mParent->mASR == aAsr| branch above.
         ancestorIds.first = Nothing();
         ancestorIds.second = mBuilder->TopmostClipId();
-      } else if (InsideClipAndScroll()) {
+      } else if (auto cs = EnclosingClipAndScroll()) {
         // If aChain->mASR is already the topmost scroll layer on the stack, but
         // it was pushed as part of a "clip and scroll" entry (i.e. because an
         // item had a clip scrolled by a different ASR than the item itself),
         // then we have need to propagate that behaviour as well. For example if
         // the enclosing display item pushed a ClipAndScroll with (scrollid=S,
         // clipid=C), then then clip we're defining here (call it D) needs to be
         // defined as a child of C, and we'll need to push the ClipAndScroll
         // (S, D) for this item. This hunk of code ensures that we define D
         // as a child of C, and when we set the needClipAndScroll flag elsewhere
         // in this file we make sure to set it for this scenario.
-        MOZ_ASSERT(mItemClipStack.back().mClipAndScroll->first == scrollId);
+        MOZ_ASSERT(cs->first == scrollId);
         ancestorIds.first = Nothing();
-        ancestorIds.second = mItemClipStack.back().mClipAndScroll->second;
+        ancestorIds.second = cs->second;
       }
     }
   }
   // At most one of the ancestor pair should be defined here, and the one that
   // is defined will be the parent clip for the new clip that we're defining.
   MOZ_ASSERT(!(ancestorIds.first && ancestorIds.second));
 
   LayoutDeviceRect clip = LayoutDeviceRect::FromAppUnits(
@@ -440,20 +441,31 @@ ScrollingLayersHelper::RecurseAndDefineA
   mBuilder->DefineScrollLayer(scrollId, ancestorIds.first, ancestorIds.second,
       aSc.ToRelativeLayoutRect(contentRect),
       aSc.ToRelativeLayoutRect(clipBounds));
 
   ids.first = Some(scrollId);
   return ids;
 }
 
-bool
-ScrollingLayersHelper::InsideClipAndScroll() const
+Maybe<ScrollingLayersHelper::ClipAndScroll>
+ScrollingLayersHelper::EnclosingClipAndScroll() const
 {
-  return !mItemClipStack.empty() && mItemClipStack.back().mClipAndScroll.isSome();
+  for (auto it = mItemClipStack.rbegin(); it != mItemClipStack.rend(); it++) {
+    if (it->mClipAndScroll) {
+      return it->mClipAndScroll;
+    }
+    // If an entry in the stack pushed a single clip or scroll without pushing
+    // a mClipAndScroll, we abort because we are effectively no longer inside
+    // a ClipAndScroll
+    if (it->mClipId || it->mScrollId) {
+      break;
+    }
+  }
+  return Nothing();
 }
 
 ScrollingLayersHelper::~ScrollingLayersHelper()
 {
   MOZ_ASSERT(!mBuilder);
   MOZ_ASSERT(mCache.empty());
   MOZ_ASSERT(mItemClipStack.empty());
 }
--- a/gfx/layers/wr/ScrollingLayersHelper.h
+++ b/gfx/layers/wr/ScrollingLayersHelper.h
@@ -39,16 +39,18 @@ public:
   void BeginList();
   void EndList();
 
   void BeginItem(nsDisplayItem* aItem,
                  const StackingContextHelper& aStackingContext);
   ~ScrollingLayersHelper();
 
 private:
+  typedef std::pair<FrameMetrics::ViewID, Maybe<wr::WrClipId>> ClipAndScroll;
+
   std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>>
   DefineClipChain(nsDisplayItem* aItem,
                   const ActiveScrolledRoot* aAsr,
                   const DisplayItemClipChain* aChain,
                   int32_t aAppUnitsPerDevPixel,
                   const StackingContextHelper& aStackingContext);
 
   std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>>
@@ -60,17 +62,17 @@ private:
 
   std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>>
   RecurseAndDefineAsr(nsDisplayItem* aItem,
                       const ActiveScrolledRoot* aAsr,
                       const DisplayItemClipChain* aChain,
                       int32_t aAppUnitsPerDevPixel,
                       const StackingContextHelper& aSc);
 
-  bool InsideClipAndScroll() const;
+  Maybe<ClipAndScroll> EnclosingClipAndScroll() const;
 
   // Note: two DisplayItemClipChain* A and B might actually be "equal" (as per
   // DisplayItemClipChain::Equal(A, B)) even though they are not the same pointer
   // (A != B). In this hopefully-rare case, they will get separate entries
   // in this map when in fact we could collapse them. However, to collapse
   // them involves writing a custom hash function for the pointer type such that
   // A and B hash to the same things whenever DisplayItemClipChain::Equal(A, B)
   // is true, and that will incur a performance penalty for all the hashmap
@@ -87,17 +89,17 @@ private:
     ItemClips(const ActiveScrolledRoot* aAsr,
               const DisplayItemClipChain* aChain);
 
     const ActiveScrolledRoot* mAsr;
     const DisplayItemClipChain* mChain;
 
     Maybe<FrameMetrics::ViewID> mScrollId;
     Maybe<wr::WrClipId> mClipId;
-    Maybe<std::pair<FrameMetrics::ViewID, Maybe<wr::WrClipId>>> mClipAndScroll;
+    Maybe<ClipAndScroll> mClipAndScroll;
 
     void Apply(wr::DisplayListBuilder* aBuilder);
     void Unapply(wr::DisplayListBuilder* aBuilder);
     bool HasSameInputs(const ItemClips& aOther);
   };
 
   std::vector<ItemClips> mItemClipStack;
 };
new file mode 100644
--- /dev/null
+++ b/layout/reftests/async-scrolling/fixed-pos-scrolled-clip-5-ref.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html lang="en-US"><head>
+<meta http-equiv="content-type" content="text/html; charset=UTF-8"><title>The scrolled clip on the fixed element is not respected</title>
+
+</head><body><div style="
+    position: absolute;
+    top: 0;
+    left: 0;
+    background-color: lime;
+    width: 100%;
+    height: 200px;">
+</div>
+</body></html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/async-scrolling/fixed-pos-scrolled-clip-5.html
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html lang="en-US"><head>
+<meta http-equiv="content-type" content="text/html; charset=UTF-8"><title>The scrolled clip on the fixed element is not respected</title>
+
+</head><body><div style="
+    position: absolute;
+    top: 0;
+    left: 0;
+    clip: rect(auto auto auto auto);
+    width: 100%;
+    height: 200px;">
+
+    <div style="background-image: linear-gradient(lime, lime);
+        position: fixed;
+        top: 0;
+        left: 0;
+        height: 100%;
+        width: 100%;
+        transform: translateZ(0px)">
+    </div>
+
+</div>
+</body></html>
--- a/layout/reftests/async-scrolling/reftest.list
+++ b/layout/reftests/async-scrolling/reftest.list
@@ -49,16 +49,17 @@ fuzzy-if(Android,7,4) skip-if(!asyncPan)
 pref(apz.disable_for_scroll_linked_effects,true) skip-if(!asyncPan) == disable-apz-for-sle-pages.html disable-apz-for-sle-pages-ref.html
 fuzzy-if(browserIsRemote&&d2d,1,19) skip-if(!asyncPan) == background-blend-mode-1.html background-blend-mode-1-ref.html
 skip-if(Android||!asyncPan) != opaque-fractional-displayport-1.html about:blank
 skip-if(Android||!asyncPan) != opaque-fractional-displayport-2.html about:blank
 fuzzy-if(Android,6,4) skip-if(!asyncPan) == fixed-pos-scrolled-clip-1.html fixed-pos-scrolled-clip-1-ref.html
 fuzzy-if(Android,6,8) fails-if(webrender) skip-if(!asyncPan) == fixed-pos-scrolled-clip-2.html fixed-pos-scrolled-clip-2-ref.html   # bug 1377187 for webrender
 fuzzy-if(Android,6,8) fails-if(webrender) skip-if(!asyncPan) == fixed-pos-scrolled-clip-3.html fixed-pos-scrolled-clip-3-ref.html   # bug 1377187 for webrender
 fuzzy-if(Android,6,8) skip-if(!asyncPan) == fixed-pos-scrolled-clip-4.html fixed-pos-scrolled-clip-4-ref.html
+skip-if(!asyncPan) == fixed-pos-scrolled-clip-5.html fixed-pos-scrolled-clip-5-ref.html
 fuzzy-if(Android,6,4) skip-if(!asyncPan) == position-sticky-scrolled-clip-1.html position-sticky-scrolled-clip-1-ref.html
 fuzzy-if(Android,6,4) skip == position-sticky-scrolled-clip-2.html position-sticky-scrolled-clip-2-ref.html # bug ?????? - incorrectly applying clip to sticky contents
 
 # for the following tests, we want to disable the low-precision buffer
 # as it will expand the displayport beyond what the test specifies in
 # its reftest-displayport attributes, and interfere with where we expect
 # checkerboarding to occur
 default-preferences pref(layers.low-precision-buffer,false)