Skip to content

Commit

Permalink
Bug 1373802 - Handle more clipping cases. r=mstange
Browse files Browse the repository at this point in the history 
We already support cases where we have scrolling clips applied to fixed
items. However if we had to build nested clips inside those items, then
those nested clips would not properly inherit from the scrolling clips.
This patch addresses that case.

MozReview-Commit-ID: CWp1x0EsyaP

--HG--
extra : rebase_source : f8c80ace2da39edebaabd5339670a68038a18489
  • Loading branch information
@staktrace
staktrace committed Oct 27, 2017
1 parent 86f9c84 commit 3d6ab72
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 21 deletions.
67 changes: 51 additions & 16 deletions gfx/layers/wr/ScrollingLayersHelper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,19 @@ ScrollingLayersHelper::BeginItem(nsDisplayItem* aItem,
// 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
// 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 we don't need a ClipAndScroll, ensure the item's ASR is at the top of
// the scroll stack
if (!needClipAndScroll && mBuilder->TopmostScrollId() != scrollId) {
Expand Down Expand Up @@ -261,23 +274,39 @@ ScrollingLayersHelper::RecurseAndDefineClip(nsDisplayItem* aItem,
}
} else {
MOZ_ASSERT(!ancestorIds.second);
// If aChain->mASR is already the topmost scroll layer on the stack, but
// but there was another clip pushed *on top* of that ASR, then that clip
// shares the ASR, and we need to make our clip a child of that clip, which
// in turn will already be a descendant of the correct ASR.
// This covers the cases where e.g. the Gecko display list has nested items,
// and the clip chain on the nested item implicitly extends from the clip
// chain on the containing wrapper item. In this case the aChain->mParent
// 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.
FrameMetrics::ViewID scrollId = aChain->mASR ? nsLayoutUtils::ViewIDForASR(aChain->mASR) : FrameMetrics::NULL_SCROLL_ID;
if (mBuilder->TopmostScrollId() == scrollId && mBuilder->TopmostIsClip()) {
ancestorIds.first = Nothing();
ancestorIds.second = mBuilder->TopmostClipId();
if (mBuilder->TopmostScrollId() == scrollId) {
if (mBuilder->TopmostIsClip()) {
// If aChain->mASR is already the topmost scroll layer on the stack, but
// but there was another clip pushed *on top* of that ASR, then that clip
// shares the ASR, and we need to make our clip a child of that clip, which
// in turn will already be a descendant of the correct ASR.
// This covers the cases where e.g. the Gecko display list has nested items,
// and the clip chain on the nested item implicitly extends from the clip
// chain on the containing wrapper item. In this case the aChain->mParent
// 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()) {
// 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);
ancestorIds.first = Nothing();
ancestorIds.second = mItemClipStack.back().mClipAndScroll->second;
}
}
}
// At most one of the ancestor pair should be defined here, and the one that
Expand Down Expand Up @@ -416,6 +445,12 @@ ScrollingLayersHelper::RecurseAndDefineAsr(nsDisplayItem* aItem,
return ids;
}

bool
ScrollingLayersHelper::InsideClipAndScroll() const
{
return !mItemClipStack.empty() && mItemClipStack.back().mClipAndScroll.isSome();
}

ScrollingLayersHelper::~ScrollingLayersHelper()
{
MOZ_ASSERT(!mBuilder);
Expand Down
2 changes: 2 additions & 0 deletions gfx/layers/wr/ScrollingLayersHelper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ class ScrollingLayersHelper
int32_t aAppUnitsPerDevPixel,
const StackingContextHelper& aSc);

bool InsideClipAndScroll() 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
Expand Down
10 changes: 5 additions & 5 deletions layout/reftests/async-scrolling/reftest.list
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ fuzzy-if(skiaContent,1,11300) skip-if(!asyncPan) == position-fixed-in-scroll-con
skip-if(!asyncPan) == position-fixed-inside-sticky-1.html position-fixed-inside-sticky-1-ref.html
skip-if(!asyncPan) == position-fixed-inside-sticky-2.html position-fixed-inside-sticky-2-ref.html
fuzzy(1,60000) skip-if(!asyncPan) == group-opacity-surface-size-1.html group-opacity-surface-size-1-ref.html
fuzzy-if(Android,1,197) skip-if(!asyncPan) == position-sticky-transformed.html position-sticky-transformed-ref.html # bug 1366295 for webrender
fuzzy-if(Android,1,197) skip-if(!asyncPan) == position-sticky-transformed.html position-sticky-transformed-ref.html
skip-if(!asyncPan) == offscreen-prerendered-active-opacity.html offscreen-prerendered-active-opacity-ref.html
fuzzy-if(Android,6,4) fuzzy-if(skiaContent&&!Android,1,34) skip-if(!asyncPan) == offscreen-clipped-blendmode-1.html offscreen-clipped-blendmode-ref.html
fuzzy-if(Android,6,4) skip-if(!asyncPan) == offscreen-clipped-blendmode-2.html offscreen-clipped-blendmode-ref.html
Expand All @@ -50,10 +50,10 @@ pref(apz.disable_for_scroll_linked_effects,true) skip-if(!asyncPan) == disable-a
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) fails-if(webrender) skip-if(!asyncPan) == fixed-pos-scrolled-clip-1.html fixed-pos-scrolled-clip-1-ref.html # bug 1373802 for webrender
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 1373802 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 1373802 for webrender
fuzzy-if(Android,6,8) fails-if(webrender) skip-if(!asyncPan) == fixed-pos-scrolled-clip-4.html fixed-pos-scrolled-clip-4-ref.html # bug 1373802 for webrender
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
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

Expand Down

0 comments on commit 3d6ab72

Please sign in to comment.