Bug 1458598 - Override scrollframes with their descendant reference frames. r=mstange
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 08 May 2018 09:16:29 -0400
changeset 417340 0bb3414bd6af644addd5ea6c4b349c88e3a4a7b9
parent 417339 06170521ac0aeef10293ff08f8af93a7222ece9c
child 417341 e4cfb8924eea1c40da0a8b3a8974edacd773616a
push id33966
push useraciure@mozilla.com
push dateTue, 08 May 2018 22:55:45 +0000
treeherdermozilla-central@ecbcf736a436 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1458598
milestone62.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 1458598 - Override scrollframes with their descendant reference frames. r=mstange The test case has a fixed item A inside a scrollframe B which is inside a reference frame C which is inside the root scrollframe D. The ClipManager code currently uses D's scrollid as the scrolling ancestor for A, because the gecko display list's ASR is set up that way. However, we really want to set C as the scrolling ancestor, because otherwise the item A gets hoisted out of C and the transform from C doesn't get applied to it. This patch ensures that when we enter C, we install an override so that anything that would have used D's scrollid ends up using C's, which results in the correct behaviour. MozReview-Commit-ID: 31tscfT4xWW
gfx/layers/wr/ClipManager.cpp
gfx/layers/wr/StackingContextHelper.cpp
layout/reftests/async-scrolling/reftest.list
--- a/gfx/layers/wr/ClipManager.cpp
+++ b/gfx/layers/wr/ClipManager.cpp
@@ -54,17 +54,17 @@ ClipManager::EndBuild()
 }
 
 void
 ClipManager::BeginList(const StackingContextHelper& aStackingContext)
 {
   if (aStackingContext.AffectsClipPositioning()) {
     PushOverrideForASR(
         mItemClipStack.empty() ? nullptr : mItemClipStack.top().mASR,
-        Nothing());
+        aStackingContext.ReferenceFrameId());
   }
 
   ItemClips clips(nullptr, nullptr);
   if (!mItemClipStack.empty()) {
     clips.CopyOutputsFrom(mItemClipStack.top());
   }
   mItemClipStack.push(clips);
 }
@@ -212,17 +212,17 @@ ClipManager::BeginItem(nsDisplayItem* aI
     // If the clip's ASR is different, then we need to set the scroll id
     // explicitly to match the desired ASR.
     FrameMetrics::ViewID viewId = asr
         ? asr->GetViewId()
         : FrameMetrics::NULL_SCROLL_ID;
     Maybe<wr::WrClipId> scrollId =
         mBuilder->GetScrollIdForDefinedScrollLayer(viewId);
     MOZ_ASSERT(scrollId.isSome());
-    clips.mScrollId = scrollId;
+    clips.mScrollId = ClipIdAfterOverride(scrollId);
   } else {
     // If we don't have a clip at all, then we don't want to explicitly push
     // the ASR either, because as with the first clause of this if condition,
     // the item might get hoisted out of a stacking context that was pushed
     // between the |asr| and this |aItem|. Instead we just leave clips.mScrollId
     // empty and things seem to work out.
     // XXX: there might be cases where things don't just "work out", in which
     // case we might need to do something smarter here.
--- a/gfx/layers/wr/StackingContextHelper.cpp
+++ b/gfx/layers/wr/StackingContextHelper.cpp
@@ -68,20 +68,18 @@ StackingContextHelper::StackingContextHe
           aTransformPtr,
           aIsPreserve3D ? wr::TransformStyle::Preserve3D : wr::TransformStyle::Flat,
           aPerspectivePtr,
           wr::ToMixBlendMode(aMixBlendMode),
           aFilters,
           aBackfaceVisible,
           rasterSpace);
 
-  mAffectsClipPositioning =
-      (aTransformPtr && !aTransformPtr->IsIdentity()) ||
-      (aBounds.TopLeft() != LayoutDevicePoint()) ||
-      (aAnimation && aAnimation->effect_type == wr::WrAnimationType::Transform);
+  mAffectsClipPositioning = mReferenceFrameId.isSome() ||
+      (aBounds.TopLeft() != LayoutDevicePoint());
 }
 
 StackingContextHelper::~StackingContextHelper()
 {
   if (mBuilder) {
     mBuilder->PopStackingContext();
   }
 }
--- a/layout/reftests/async-scrolling/reftest.list
+++ b/layout/reftests/async-scrolling/reftest.list
@@ -53,17 +53,17 @@ fuzzy-if(Android,7,4) skip-if(!asyncPan)
 fuzzy-if(Android,7,4) fails-if(webrender) skip-if(!asyncPan) == perspective-scrolling-3.html perspective-scrolling-3-ref.html # bug 1361720 for webrender
 fuzzy-if(Android,7,4) skip-if(!asyncPan) == perspective-scrolling-4.html perspective-scrolling-4-ref.html
 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,20) 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) skip-if(!asyncPan) == fixed-pos-scrolled-clip-2.html fixed-pos-scrolled-clip-2-ref.html
-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 1458598 for webrender
+fuzzy-if(Android,6,8) skip-if(!asyncPan) == fixed-pos-scrolled-clip-3.html fixed-pos-scrolled-clip-3-ref.html
 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
 skip-if(!asyncPan) == position-sticky-bug1434250.html position-sticky-bug1434250-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
 skip-if(!asyncPan) == curtain-effect-1.html curtain-effect-1-ref.html
 
 # for the following tests, we want to disable the low-precision buffer