Bug 1529992 - disable the MergeClipLeaf optimization for all shadows properly. r=kats
☠☠ backed out by 81be11ad9dfb ☠ ☠
authorAlexis Beingessner <a.beingessner@gmail.com>
Mon, 15 Apr 2019 17:25:34 +0000
changeset 469539 87b64e169b1b
parent 469538 ddf29d68efb2
child 469540 c6c39f570b1f
push id35874
push userccoroiu@mozilla.com
push dateTue, 16 Apr 2019 04:04:58 +0000
treeherdermozilla-central@be3f40425b52 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs1529992
milestone68.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 1529992 - disable the MergeClipLeaf optimization for all shadows properly. r=kats Differential Revision: https://phabricator.services.mozilla.com/D27114
gfx/layers/wr/ClipManager.cpp
gfx/webrender_bindings/WebRenderAPI.cpp
gfx/webrender_bindings/WebRenderAPI.h
--- a/gfx/layers/wr/ClipManager.cpp
+++ b/gfx/layers/wr/ClipManager.cpp
@@ -150,26 +150,20 @@ wr::WrSpaceAndClipChain ClipManager::Swi
     asr = static_cast<nsDisplayStickyPosition*>(aItem)->GetContainerASR();
   }
 
   // In most cases we can combine the leaf of the clip chain with the clip rect
   // of the display item. This reduces the number of clip items, which avoids
   // some overhead further down the pipeline.
   bool separateLeaf = false;
   if (clip && clip->mASR == asr && clip->mClip.GetRoundedRectCount() == 0) {
-    if (type == DisplayItemType::TYPE_TEXT) {
-      // Text with shadows interprets the text display item clip rect and
-      // clips from the clip chain differently.
-      separateLeaf = !aItem->Frame()->StyleText()->HasTextShadow();
-    } else {
-      // Container display items are not currently supported because the clip
-      // rect of a stacking context is not handled the same as normal display
-      // items.
-      separateLeaf = aItem->GetChildren() == nullptr;
-    }
+    // Container display items are not currently supported because the clip
+    // rect of a stacking context is not handled the same as normal display
+    // items.
+    separateLeaf = aItem->GetChildren() == nullptr;
   }
 
   ItemClips clips(asr, clip, separateLeaf);
   MOZ_ASSERT(!mItemClipStack.empty());
   if (clips.HasSameInputs(mItemClipStack.top())) {
     // Early-exit because if the clips are the same as aItem's previous sibling,
     // then we don't need to do do the work of popping the old stuff and then
     // pushing it right back on for the new item. Note that if aItem doesn't
--- a/gfx/webrender_bindings/WebRenderAPI.cpp
+++ b/gfx/webrender_bindings/WebRenderAPI.cpp
@@ -1089,21 +1089,58 @@ void DisplayListBuilder::PushLine(const 
                   aLine.wavyLineThickness, aLine.orientation, &aLine.color,
                   aLine.style);
 }
 
 void DisplayListBuilder::PushShadow(const wr::LayoutRect& aRect,
                                     const wr::LayoutRect& aClip,
                                     bool aIsBackfaceVisible,
                                     const wr::Shadow& aShadow) {
-  wr_dp_push_shadow(mWrState, aRect, MergeClipLeaf(aClip), aIsBackfaceVisible,
+  // Local clip_rects are translated inside of shadows, as they are assumed to
+  // be part of the element drawing itself, and not a parent frame clipping it.
+  // As such, it is not sound to apply the MergeClipLeaf optimization inside of
+  // shadows. So we disable the optimization when we encounter a shadow.
+  // Shadows don't span frames, so we don't have to worry about MergeClipLeaf
+  // being re-enabled mid-shadow. The optimization is restored in PopAllShadows.
+  SuspendClipLeafMerging();
+  wr_dp_push_shadow(mWrState, aRect, aClip, aIsBackfaceVisible,
                     &mCurrentSpaceAndClipChain, aShadow);
 }
 
-void DisplayListBuilder::PopAllShadows() { wr_dp_pop_all_shadows(mWrState); }
+void DisplayListBuilder::PopAllShadows() {
+  wr_dp_pop_all_shadows(mWrState);
+  ResumeClipLeafMerging();
+}
+
+void DisplayListBuilder::SuspendClipLeafMerging() {
+  if (mClipChainLeaf) {
+    // No one should reinitialize mClipChainLeaf while we're suspended
+    MOZ_ASSERT(!mSuspendedClipChainLeaf);
+
+    mSuspendedClipChainLeaf = mClipChainLeaf;
+    mSuspendedSpaceAndClipChain = Some(mCurrentSpaceAndClipChain);
+
+    // Clip is implicitly parented by mCurrentSpaceAndClipChain
+    auto clipId = DefineClip(Nothing(), *mClipChainLeaf);
+    auto clipChainId = DefineClipChain({ clipId });
+
+    mCurrentSpaceAndClipChain.clip_chain = clipChainId.id;
+    mClipChainLeaf = Nothing();
+  }
+}
+
+void DisplayListBuilder::ResumeClipLeafMerging() {
+  if (mSuspendedClipChainLeaf) {
+    mCurrentSpaceAndClipChain = *mSuspendedSpaceAndClipChain;
+    mClipChainLeaf = mSuspendedClipChainLeaf;
+
+    mSuspendedClipChainLeaf = Nothing();
+    mSuspendedSpaceAndClipChain = Nothing();
+  }
+}
 
 void DisplayListBuilder::PushBoxShadow(
     const wr::LayoutRect& aRect, const wr::LayoutRect& aClip,
     bool aIsBackfaceVisible, const wr::LayoutRect& aBoxBounds,
     const wr::LayoutVector2D& aOffset, const wr::ColorF& aColor,
     const float& aBlurRadius, const float& aSpreadRadius,
     const wr::BorderRadius& aBorderRadius,
     const wr::BoxShadowClipMode& aClipMode) {
--- a/gfx/webrender_bindings/WebRenderAPI.h
+++ b/gfx/webrender_bindings/WebRenderAPI.h
@@ -586,31 +586,40 @@ class DisplayListBuilder final {
  protected:
   wr::LayoutRect MergeClipLeaf(const wr::LayoutRect& aClip) {
     if (mClipChainLeaf) {
       return wr::IntersectLayoutRect(*mClipChainLeaf, aClip);
     }
     return aClip;
   }
 
+  // See the implementation of PushShadow for details on these methods.
+  void SuspendClipLeafMerging();
+  void ResumeClipLeafMerging();
+
   wr::WrState* mWrState;
 
   // Track each scroll id that we encountered. We use this structure to
   // ensure that we don't define a particular scroll layer multiple times,
   // as that results in undefined behaviour in WR.
   std::unordered_map<layers::ScrollableLayerGuid::ViewID, wr::WrSpaceAndClip>
       mScrollIds;
 
   wr::WrSpaceAndClipChain mCurrentSpaceAndClipChain;
 
   // Contains the current leaf of the clip chain to be merged with the
   // display item's clip rect when pushing an item. May be set to Nothing() if
   // there is no clip rect to merge with.
   Maybe<wr::LayoutRect> mClipChainLeaf;
 
+  // Versions of the above that are on hold while SuspendClipLeafMerging is on
+  // (see the implementation of PushShadow for details).
+  Maybe<wr::WrSpaceAndClipChain> mSuspendedSpaceAndClipChain;
+  Maybe<wr::LayoutRect> mSuspendedClipChainLeaf;
+
   RefPtr<layout::TextDrawTarget> mCachedTextDT;
   RefPtr<gfxContext> mCachedContext;
 
   FixedPosScrollTargetTracker* mActiveFixedPosTracker;
 
   NonDefaultRenderRootArray<UniquePtr<DisplayListBuilder>> mSubBuilders;
   wr::PipelineId mPipelineId;
   wr::LayoutSize mContentSize;