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
--- 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)