Bug 1405359 - Replace the mPushed* variables with a more encapsulated struct. r=jrmuizel
☠☠ backed out by 870e07ee24cf ☠ ☠
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 24 Oct 2017 16:15:00 -0400
changeset 387977 f6ce05f8e699674a01d0df99a6a36c73191ba273
parent 387976 cb49e178390c45386141460e3ca2901cc3dc6346
child 387978 154c415885b818ca3de8f21c44639994bddeab5d
push id53974
push userkgupta@mozilla.com
push dateTue, 24 Oct 2017 21:07:42 +0000
treeherderautoland@1593dfc4cf04 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs1405359
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 1405359 - Replace the mPushed* variables with a more encapsulated struct. r=jrmuizel Storing the per-item clip state in a struct like this will allow us to easily compare the desired clip state across items, so we can avoid doing unnecessary work when going from one item to the next. This patch has no functional changes, it's just refactoring. MozReview-Commit-ID: 49B6hmsWZ4V
gfx/layers/wr/ScrollingLayersHelper.cpp
gfx/layers/wr/ScrollingLayersHelper.h
--- a/gfx/layers/wr/ScrollingLayersHelper.cpp
+++ b/gfx/layers/wr/ScrollingLayersHelper.cpp
@@ -14,17 +14,16 @@ namespace mozilla {
 namespace layers {
 
 ScrollingLayersHelper::ScrollingLayersHelper(nsDisplayItem* aItem,
                                              wr::DisplayListBuilder& aBuilder,
                                              const StackingContextHelper& aStackingContext,
                                              WebRenderCommandBuilder::ClipIdMap& aCache,
                                              bool aApzEnabled)
   : mBuilder(&aBuilder)
-  , mPushedClipAndScroll(false)
   , mCache(aCache)
 {
   int32_t auPerDevPixel = aItem->Frame()->PresContext()->AppUnitsPerDevPixel();
 
   // There are two ASR chains here that we need to be fully defined. One is the
   // ASR chain pointed to by aItem->GetActiveScrolledRoot(). The other is the
   // ASR chain pointed to by aItem->GetClipChain()->mASR. We pick the leafmost
   // of these two chains because that one will include the other.
@@ -58,33 +57,38 @@ ScrollingLayersHelper::ScrollingLayersHe
   // with a case where the item's clip chain is scrolled by something other than
   // the item's ASR. So for those cases we need to use the ClipAndScroll API.
   bool needClipAndScroll = (leafmostId != scrollId);
 
   // 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) {
     MOZ_ASSERT(leafmostId == scrollId); // because !needClipAndScroll
-    mBuilder->PushScrollLayer(scrollId);
-    mPushedClips.push_back(wr::ScrollOrClipId(scrollId));
+    mItemClips.mScrollId = Some(scrollId);
   }
   // And ensure the leafmost clip, if scrolled by that ASR, is at the top of the
   // stack.
   if (ids.second && aItem->GetClipChain()->mASR == leafmostASR) {
-    mBuilder->PushClip(ids.second.ref());
-    mPushedClips.push_back(wr::ScrollOrClipId(ids.second.ref()));
+    mItemClips.mClipId = ids.second;
   }
   // If we need the ClipAndScroll, we want to replace the topmost scroll layer
   // with the item's ASR but preseve the topmost clip (which is scrolled by
   // some other ASR).
   if (needClipAndScroll) {
-    Maybe<wr::WrClipId> clipId = mBuilder->TopmostClipId();
-    mBuilder->PushClipAndScrollInfo(scrollId, clipId.ptrOr(nullptr));
-    mPushedClipAndScroll = true;
+    // If mClipId is set that means we want to push it such that it's going
+    // to be the TopmostClipId(), but we haven't actually pushed it yet.
+    // But we still want to take that instead of the actual current TopmostClipId().
+    Maybe<wr::WrClipId> clipId = mItemClips.mClipId;
+    if (!clipId) {
+      clipId = mBuilder->TopmostClipId();
+    }
+    mItemClips.mClipAndScroll = Some(std::make_pair(scrollId, clipId));
   }
+
+  mItemClips.Apply(mBuilder);
 }
 
 std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>>
 ScrollingLayersHelper::DefineClipChain(nsDisplayItem* aItem,
                                        const ActiveScrolledRoot* aAsr,
                                        const DisplayItemClipChain* aChain,
                                        int32_t aAppUnitsPerDevPixel,
                                        const StackingContextHelper& aStackingContext)
@@ -347,25 +351,42 @@ ScrollingLayersHelper::RecurseAndDefineA
       aSc.ToRelativeLayoutRect(clipBounds));
 
   ids.first = Some(scrollId);
   return ids;
 }
 
 ScrollingLayersHelper::~ScrollingLayersHelper()
 {
-  if (mPushedClipAndScroll) {
-    mBuilder->PopClipAndScrollInfo();
+  mItemClips.Unapply(mBuilder);
+}
+
+void
+ScrollingLayersHelper::ItemClips::Apply(wr::DisplayListBuilder* aBuilder)
+{
+  if (mScrollId) {
+    aBuilder->PushScrollLayer(mScrollId.ref());
+  }
+  if (mClipId) {
+    aBuilder->PushClip(mClipId.ref());
   }
-  while (!mPushedClips.empty()) {
-    wr::ScrollOrClipId id = mPushedClips.back();
-    if (id.is<wr::WrClipId>()) {
-      mBuilder->PopClip();
-    } else {
-      MOZ_ASSERT(id.is<FrameMetrics::ViewID>());
-      mBuilder->PopScrollLayer();
-    }
-    mPushedClips.pop_back();
+  if (mClipAndScroll) {
+    aBuilder->PushClipAndScrollInfo(mClipAndScroll->first,
+                                    mClipAndScroll->second.ptrOr(nullptr));
+  }
+}
+
+void
+ScrollingLayersHelper::ItemClips::Unapply(wr::DisplayListBuilder* aBuilder)
+{
+  if (mClipAndScroll) {
+    aBuilder->PopClipAndScrollInfo();
+  }
+  if (mClipId) {
+    aBuilder->PopClip();
+  }
+  if (mScrollId) {
+    aBuilder->PopScrollLayer();
   }
 }
 
 } // namespace layers
 } // namespace mozilla
--- a/gfx/layers/wr/ScrollingLayersHelper.h
+++ b/gfx/layers/wr/ScrollingLayersHelper.h
@@ -50,17 +50,26 @@ private:
   std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>>
   RecurseAndDefineAsr(nsDisplayItem* aItem,
                       const ActiveScrolledRoot* aAsr,
                       const DisplayItemClipChain* aChain,
                       int32_t aAppUnitsPerDevPixel,
                       const StackingContextHelper& aSc);
 
   wr::DisplayListBuilder* mBuilder;
-  bool mPushedClipAndScroll;
-  std::vector<wr::ScrollOrClipId> mPushedClips;
   WebRenderCommandBuilder::ClipIdMap& mCache;
+
+  struct ItemClips {
+    Maybe<FrameMetrics::ViewID> mScrollId;
+    Maybe<wr::WrClipId> mClipId;
+    Maybe<std::pair<FrameMetrics::ViewID, Maybe<wr::WrClipId>>> mClipAndScroll;
+
+    void Apply(wr::DisplayListBuilder* aBuilder);
+    void Unapply(wr::DisplayListBuilder* aBuilder);
+  };
+
+  ItemClips mItemClips;
 };
 
 } // namespace layers
 } // namespace mozilla
 
 #endif