Bug 1390804 - When pushing a mask clip, don't record it in DisplayListBuilder's clip stack. r?ethlin draft
authorKartikaya Gupta <kgupta@mozilla.com>
Thu, 17 Aug 2017 13:54:25 -0400
changeset 648385 662b04e85be83f0dd9d2f1940daf561815e44f39
parent 648384 e365137fa61bfd729617ba1ebf9f1ed79facd1f2
child 726812 4902514d1062ce15682c007abaf5baaae58a7c2d
push id74741
push userkgupta@mozilla.com
push dateThu, 17 Aug 2017 17:54:56 +0000
reviewersethlin
bugs1390804
milestone57.0a1
Bug 1390804 - When pushing a mask clip, don't record it in DisplayListBuilder's clip stack. r?ethlin Recording mask clips in the clip stack changes the value of TopmostClipId() which confuses the code in ScrollingLayersHelper. The mask clip can be thought of as an "out-of-band" clip that ScrollingLayersHelper doesn't need to know about. This patch adds a mechanism for pushing such "out-of-band" clips without touching the clip stack. MozReview-Commit-ID: 8Zeqtigk0cj
gfx/webrender_bindings/WebRenderAPI.cpp
gfx/webrender_bindings/WebRenderAPI.h
layout/painting/nsDisplayList.cpp
--- a/gfx/webrender_bindings/WebRenderAPI.cpp
+++ b/gfx/webrender_bindings/WebRenderAPI.cpp
@@ -682,28 +682,32 @@ DisplayListBuilder::DefineClip(const wr:
   WRDL_LOG("DefineClip id=%" PRIu64 " r=%s m=%p b=%s complex=%d\n", clip_id,
       Stringify(aClipRect).c_str(), aMask,
       aMask ? Stringify(aMask->rect).c_str() : "none",
       aComplex ? aComplex->Length() : 0);
   return wr::WrClipId { clip_id };
 }
 
 void
-DisplayListBuilder::PushClip(const wr::WrClipId& aClipId)
+DisplayListBuilder::PushClip(const wr::WrClipId& aClipId, bool aRecordInStack)
 {
   wr_dp_push_clip(mWrState, aClipId.id);
   WRDL_LOG("PushClip id=%" PRIu64 "\n", aClipId.id);
-  mClipIdStack.push_back(aClipId);
+  if (aRecordInStack) {
+    mClipIdStack.push_back(aClipId);
+  }
 }
 
 void
-DisplayListBuilder::PopClip()
+DisplayListBuilder::PopClip(bool aRecordInStack)
 {
   WRDL_LOG("PopClip id=%" PRIu64 "\n", mClipIdStack.back().id);
-  mClipIdStack.pop_back();
+  if (aRecordInStack) {
+    mClipIdStack.pop_back();
+  }
   wr_dp_pop_clip(mWrState);
 }
 
 void
 DisplayListBuilder::PushBuiltDisplayList(BuiltDisplayList &dl)
 {
   wr_dp_push_built_display_list(mWrState,
                                 dl.dl_desc,
--- a/gfx/webrender_bindings/WebRenderAPI.h
+++ b/gfx/webrender_bindings/WebRenderAPI.h
@@ -176,18 +176,18 @@ public:
                            const gfx::Matrix4x4* aPerspective,
                            const wr::MixBlendMode& aMixBlendMode,
                            const nsTArray<wr::WrFilterOp>& aFilters);
   void PopStackingContext();
 
   wr::WrClipId DefineClip(const wr::LayoutRect& aClipRect,
                           const nsTArray<wr::WrComplexClipRegion>* aComplex = nullptr,
                           const wr::WrImageMask* aMask = nullptr);
-  void PushClip(const wr::WrClipId& aClipId);
-  void PopClip();
+  void PushClip(const wr::WrClipId& aClipId, bool aRecordInStack = true);
+  void PopClip(bool aRecordInStack = true);
 
   void PushBuiltDisplayList(wr::BuiltDisplayList &dl);
 
   void PushScrollLayer(const layers::FrameMetrics::ViewID& aScrollId,
                        const wr::LayoutRect& aContentRect, // TODO: We should work with strongly typed rects
                        const wr::LayoutRect& aClipRect);
   void PopScrollLayer();
 
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -9002,24 +9002,28 @@ nsDisplayMask::CreateWebRenderCommands(m
   bool snap;
   float appUnitsPerDevPixel = mFrame->PresContext()->AppUnitsPerDevPixel();
   nsRect displayBound = GetBounds(aDisplayListBuilder, &snap);
   LayerRect bounds = ViewAs<LayerPixel>(LayoutDeviceRect::FromAppUnits(displayBound, appUnitsPerDevPixel),
                                         PixelCastJustification::WebRenderHasUnitResolution);
 
   Maybe<wr::WrImageMask> mask = aManager->BuildWrMaskImage(this, aBuilder, aSc, aDisplayListBuilder, bounds);
   if (mask) {
-    aBuilder.PushClip(aBuilder.DefineClip(
-        aSc.ToRelativeLayoutRect(bounds), nullptr, mask.ptr()));
+    wr::WrClipId clipId = aBuilder.DefineClip(
+        aSc.ToRelativeLayoutRect(bounds), nullptr, mask.ptr());
+    // Don't record this clip push in aBuilder's internal clip stack, because
+    // otherwise any nested ScrollingLayersHelper instances that are created
+    // will get confused about which clips are pushed.
+    aBuilder.PushClip(clipId, /*aRecordInStack*/ false);
   }
 
   nsDisplaySVGEffects::CreateWebRenderCommands(aBuilder, aSc, aParentCommands, aManager, aDisplayListBuilder);
 
   if (mask) {
-    aBuilder.PopClip();
+    aBuilder.PopClip(/*aRecordInStack*/ false);
   }
 
   return true;
 }
 
 #ifdef MOZ_DUMP_PAINTING
 void
 nsDisplayMask::PrintEffects(nsACString& aTo)