Bug 1398706 - Always create new clip ids in ScrollingLayersHelper if it's inside a mask. r=kats
authorEthan Lin <ethlin@mozilla.com>
Tue, 19 Sep 2017 17:05:22 +0800
changeset 382403 804199553c13fd14c266f08cb77816863e60d7f0
parent 382402 83da60c1a86ca2507b16ee047771b6d387ab299d
child 382404 56facbd1d507619532f85af1d6ac24bfab2c9842
push id32558
push userkwierso@gmail.com
push dateFri, 22 Sep 2017 21:29:46 +0000
treeherdermozilla-central@61e58a7d800b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs1398706
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 1398706 - Always create new clip ids in ScrollingLayersHelper if it's inside a mask. r=kats MozReview-Commit-ID: Hhsbgpqddo1
gfx/layers/wr/ScrollingLayersHelper.cpp
gfx/webrender_bindings/WebRenderAPI.cpp
gfx/webrender_bindings/WebRenderAPI.h
layout/painting/nsDisplayList.cpp
--- a/gfx/layers/wr/ScrollingLayersHelper.cpp
+++ b/gfx/layers/wr/ScrollingLayersHelper.cpp
@@ -221,25 +221,27 @@ ScrollingLayersHelper::DefineAndPushChai
   // pushed onto the WR clip stack. Note that the recursion can invalidate the
   // iterator `it`.
   DefineAndPushChain(aChain->mParent, aBuilder, aStackingContext, aAppUnitsPerDevPixel, aCache);
 
   if (!aChain->mClip.HasClip()) {
     // This item in the chain is a no-op, skip over it
     return;
   }
-  if (!clipId) {
+  if (!clipId || aBuilder.HasMaskClip()) {
     // If we don't have a clip id for this chain item yet, define the clip in WR
     // and save the id
     LayoutDeviceRect clip = LayoutDeviceRect::FromAppUnits(
         aChain->mClip.GetClipRect(), aAppUnitsPerDevPixel);
     nsTArray<wr::WrComplexClipRegion> wrRoundedRects;
     aChain->mClip.ToWrComplexClipRegions(aAppUnitsPerDevPixel, aStackingContext, wrRoundedRects);
     clipId = Some(aBuilder.DefineClip(aStackingContext.ToRelativeLayoutRect(clip), &wrRoundedRects));
-    aCache[aChain] = clipId.value();
+    if (!aBuilder.HasMaskClip()) {
+      aCache[aChain] = clipId.value();
+    }
   }
   // Finally, push the clip onto the WR stack
   MOZ_ASSERT(clipId);
   aBuilder.PushClip(clipId.value());
   mPushedClips.push_back(wr::ScrollOrClipId(clipId.value()));
 }
 
 bool
--- a/gfx/webrender_bindings/WebRenderAPI.cpp
+++ b/gfx/webrender_bindings/WebRenderAPI.cpp
@@ -617,16 +617,17 @@ void
 WebRenderAPI::RunOnRenderThread(UniquePtr<RendererEvent> aEvent)
 {
   auto event = reinterpret_cast<uintptr_t>(aEvent.release());
   wr_api_send_external_event(mDocHandle, event);
 }
 
 DisplayListBuilder::DisplayListBuilder(PipelineId aId,
                                        const wr::LayoutSize& aContentSize)
+  : mMaskClipCount(0)
 {
   MOZ_COUNT_CTOR(DisplayListBuilder);
   mWrState = wr_state_new(aId, aContentSize);
 }
 
 DisplayListBuilder::~DisplayListBuilder()
 {
   MOZ_COUNT_DTOR(DisplayListBuilder);
@@ -702,31 +703,35 @@ DisplayListBuilder::DefineClip(const wr:
   WRDL_LOG("DefineClip id=%" PRIu64 " r=%s m=%p b=%s complex=%zu\n", mWrState,
       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, bool aRecordInStack)
+DisplayListBuilder::PushClip(const wr::WrClipId& aClipId, bool aMask)
 {
   wr_dp_push_clip(mWrState, aClipId.id);
   WRDL_LOG("PushClip id=%" PRIu64 "\n", mWrState, aClipId.id);
-  if (aRecordInStack) {
+  if (!aMask) {
     mClipIdStack.push_back(aClipId);
+  } else {
+    mMaskClipCount++;
   }
 }
 
 void
-DisplayListBuilder::PopClip(bool aRecordInStack)
+DisplayListBuilder::PopClip(bool aMask)
 {
   WRDL_LOG("PopClip id=%" PRIu64 "\n", mWrState, mClipIdStack.back().id);
-  if (aRecordInStack) {
+  if (!aMask) {
     mClipIdStack.pop_back();
+  } else {
+    mMaskClipCount--;
   }
   wr_dp_pop_clip(mWrState);
 }
 
 wr::WrStickyId
 DisplayListBuilder::DefineStickyFrame(const wr::LayoutRect& aContentRect,
                                       const wr::StickySideConstraint* aTop,
                                       const wr::StickySideConstraint* aRight,
--- a/gfx/webrender_bindings/WebRenderAPI.h
+++ b/gfx/webrender_bindings/WebRenderAPI.h
@@ -221,18 +221,18 @@ public:
                            const wr::MixBlendMode& aMixBlendMode,
                            const nsTArray<wr::WrFilterOp>& aFilters,
                            bool aIsBackfaceVisible);
   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, bool aRecordInStack = true);
-  void PopClip(bool aRecordInStack = true);
+  void PushClip(const wr::WrClipId& aClipId, bool aMask = false);
+  void PopClip(bool aMask = false);
 
   wr::WrStickyId DefineStickyFrame(const wr::LayoutRect& aContentRect,
                                    const wr::StickySideConstraint* aTop,
                                    const wr::StickySideConstraint* aRight,
                                    const wr::StickySideConstraint* aBottom,
                                    const wr::StickySideConstraint* aLeft);
   void PushStickyFrame(const wr::WrStickyId& aStickyId);
   void PopStickyFrame();
@@ -396,31 +396,38 @@ public:
   layers::FrameMetrics::ViewID TopmostScrollId();
   // Returns the scroll id that was pushed just before the given scroll id. This
   // function returns Nothing() if the given scrollid has not been encountered,
   // or if it is the rootmost scroll id (and therefore has no ancestor).
   Maybe<layers::FrameMetrics::ViewID> ParentScrollIdFor(layers::FrameMetrics::ViewID aScrollId);
 
   // Try to avoid using this when possible.
   wr::WrState* Raw() { return mWrState; }
+
+  // Return true if the current clip stack has any mask type clip.
+  bool HasMaskClip() { return mMaskClipCount > 0; }
+
 protected:
   wr::WrState* mWrState;
 
   // Track the stack of clip ids and scroll layer ids that have been pushed
   // (by PushClip and PushScrollLayer, respectively) and are still active.
   // This is helpful for knowing e.g. what the ancestor scroll id of a particular
   // scroll id is, and doing other "queries" of current state.
   std::vector<wr::WrClipId> mClipIdStack;
   std::vector<layers::FrameMetrics::ViewID> mScrollIdStack;
 
   // Track the parent scroll id of each scroll id that we encountered. A
   // Nothing() value indicates a root scroll id. We also use this structure to
   // ensure that we don't define a particular scroll layer multiple times.
   std::unordered_map<layers::FrameMetrics::ViewID, Maybe<layers::FrameMetrics::ViewID>> mScrollParents;
 
+  // The number of mask clips that are in the stack.
+  uint32_t mMaskClipCount;
+
   friend class WebRenderAPI;
 };
 
 Maybe<wr::ImageFormat>
 SurfaceFormatToImageFormat(gfx::SurfaceFormat aFormat);
 
 } // namespace wr
 } // namespace mozilla
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -9150,23 +9150,23 @@ nsDisplayMask::CreateWebRenderCommands(m
   Maybe<wr::WrImageMask> mask = aManager->BuildWrMaskImage(this, aBuilder, aResources,
                                                            aSc, aDisplayListBuilder, bounds);
   if (mask) {
     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);
+    aBuilder.PushClip(clipId, /*aMask*/ true);
   }
 
   nsDisplaySVGEffects::CreateWebRenderCommands(aBuilder, aResources, aSc, aManager, aDisplayListBuilder);
 
   if (mask) {
-    aBuilder.PopClip(/*aRecordInStack*/ false);
+    aBuilder.PopClip(/*aMask*/ true);
   }
 
   return true;
 }
 
 #ifdef MOZ_DUMP_PAINTING
 void
 nsDisplayMask::PrintEffects(nsACString& aTo)