Bug 1405359 - Make ScrollingLayersHelper a more stateful class. r=jrmuizel

This makes ScrollingLayersHelper a non-RAII type class, and instead adds
methods to notify it of when we start processing a new transaction or a
new display item within the transaction. This patch has no functional
changes, it's non-obvious refactoring.

MozReview-Commit-ID: 3yq9sPiHMge

--HG--
extra : rebase_source : 286423f56de59211e320f015cb1004a1e98332b8

gfx/layers/wr/ScrollingLayersHelper.cpp

@@ -5,21 +5,41 @@
 
 #include "mozilla/layers/ScrollingLayersHelper.h"
 
+#include "DisplayItemClipChain.h"
 #include "FrameMetrics.h"
 #include "mozilla/layers/StackingContextHelper.h"
 #include "mozilla/webrender/WebRenderAPI.h"
+#include "nsDisplayList.h"
 #include "UnitTransforms.h"
 
 namespace mozilla {
 namespace layers {
 
-ScrollingLayersHelper::ScrollingLayersHelper(nsDisplayItem* aItem,
+ScrollingLayersHelper::ScrollingLayersHelper()
-                                             wr::DisplayListBuilder& aBuilder,
+  : mBuilder(nullptr)
-                                             const StackingContextHelper& aStackingContext,
+{
-                                             WebRenderCommandBuilder::ClipIdMap& aCache,
+}
-                                             bool aApzEnabled)
+
-  : mBuilder(&aBuilder)
+void
-  , mCache(aCache)
+ScrollingLayersHelper::BeginBuild(wr::DisplayListBuilder& aBuilder)
+{
+  MOZ_ASSERT(!mBuilder);
+  mBuilder = &aBuilder;
+  MOZ_ASSERT(mCache.empty());
+  MOZ_ASSERT(mItemClipStack.empty());
+}
+
+void
+ScrollingLayersHelper::EndBuild()
+{
+  mBuilder = nullptr;
+  mCache.clear();
+  MOZ_ASSERT(mItemClipStack.empty());
+}
+
+void
+ScrollingLayersHelper::BeginItem(nsDisplayItem* aItem,
+                                 const StackingContextHelper& aStackingContext)
 {
   int32_t auPerDevPixel = aItem->Frame()->PresContext()->AppUnitsPerDevPixel();
 
@@ -58,16 +78,17 @@ ScrollingLayersHelper::ScrollingLayersHelper(nsDisplayItem* aItem,
   // the item's ASR. So for those cases we need to use the ClipAndScroll API.
   bool needClipAndScroll = (leafmostId != scrollId);
 
+  ItemClips clips;
   // 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
-    mItemClips.mScrollId = Some(scrollId);
+    clips.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) {
-    mItemClips.mClipId = ids.second;
+    clips.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
@@ -76,14 +97,15 @@ ScrollingLayersHelper::ScrollingLayersHelper(nsDisplayItem* aItem,
     // 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;
+    Maybe<wr::WrClipId> clipId = clips.mClipId;
     if (!clipId) {
       clipId = mBuilder->TopmostClipId();
     }
-    mItemClips.mClipAndScroll = Some(std::make_pair(scrollId, clipId));
+    clips.mClipAndScroll = Some(std::make_pair(scrollId, clipId));
   }
 
-  mItemClips.Apply(mBuilder);
+  clips.Apply(mBuilder);
+  mItemClipStack.push_back(clips);
 }
 
 std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>>
@@ -354,9 +376,20 @@ ScrollingLayersHelper::RecurseAndDefineAsr(nsDisplayItem* aItem,
   return ids;
 }
 
+void
+ScrollingLayersHelper::EndItem(nsDisplayItem* aItem)
+{
+  MOZ_ASSERT(!mItemClipStack.empty());
+  ItemClips& clips = mItemClipStack.back();
+  clips.Unapply(mBuilder);
+  mItemClipStack.pop_back();
+}
+
 ScrollingLayersHelper::~ScrollingLayersHelper()
 {
-  mItemClips.Unapply(mBuilder);
+  MOZ_ASSERT(!mBuilder);
+  MOZ_ASSERT(mCache.empty());
+  MOZ_ASSERT(mItemClipStack.empty());
 }
 
 void

gfx/layers/wr/ScrollingLayersHelper.h

@@ -7,10 +7,12 @@
 #define GFX_SCROLLINGLAYERSHELPER_H
 
 #include "mozilla/Attributes.h"
-#include "mozilla/layers/WebRenderCommandBuilder.h"
+
+class nsDisplayItem;
 
 namespace mozilla {
 
+struct ActiveScrolledRoot;
 struct DisplayItemClipChain;
 
 namespace wr {
@@ -22,14 +24,17 @@ namespace layers {
 struct FrameMetrics;
 class StackingContextHelper;
 
-class MOZ_RAII ScrollingLayersHelper
+class ScrollingLayersHelper
 {
 public:
-  ScrollingLayersHelper(nsDisplayItem* aItem,
+  ScrollingLayersHelper();
-                        wr::DisplayListBuilder& aBuilder,
+
-                        const StackingContextHelper& aStackingContext,
+  void BeginBuild(wr::DisplayListBuilder& aBuilder);
-                        WebRenderCommandBuilder::ClipIdMap& aCache,
+  void EndBuild();
-                        bool aApzEnabled);
+
+  void BeginItem(nsDisplayItem* aItem,
+                 const StackingContextHelper& aStackingContext);
+  void EndItem(nsDisplayItem* aItem);
   ~ScrollingLayersHelper();
 
 private:
@@ -54,8 +59,20 @@ private:
                       int32_t aAppUnitsPerDevPixel,
                       const StackingContextHelper& aSc);
 
+  // Note: two DisplayItemClipChain* A and B might actually be "equal" (as per
+  // DisplayItemClipChain::Equal(A, B)) even though they are not the same pointer
+  // (A != B). In this hopefully-rare case, they will get separate entries
+  // in this map when in fact we could collapse them. However, to collapse
+  // them involves writing a custom hash function for the pointer type such that
+  // A and B hash to the same things whenever DisplayItemClipChain::Equal(A, B)
+  // is true, and that will incur a performance penalty for all the hashmap
+  // operations, so is probably not worth it. With the current code we might
+  // end up creating multiple clips in WR that are effectively identical but
+  // have separate clip ids. Hopefully this won't happen very often.
+  typedef std::unordered_map<const DisplayItemClipChain*, wr::WrClipId> ClipIdMap;
+
   wr::DisplayListBuilder* mBuilder;
-  WebRenderCommandBuilder::ClipIdMap& mCache;
+  ClipIdMap mCache;
 
   struct ItemClips {
     Maybe<FrameMetrics::ViewID> mScrollId;
@@ -66,7 +83,7 @@ private:
     void Unapply(wr::DisplayListBuilder* aBuilder);
   };
 
-  ItemClips mItemClips;
+  std::vector<ItemClips> mItemClipStack;
 };
 
 } // namespace layers

gfx/layers/wr/WebRenderCommandBuilder.cpp

@@ -64,6 +64,7 @@ WebRenderCommandBuilder::BuildWebRenderCommands(wr::DisplayListBuilder& aBuilder
     MOZ_ASSERT(mLayerScrollData.empty());
     mLastCanvasDatas.Clear();
     mLastAsr = nullptr;
+    mScrollingHelper.BeginBuild(aBuilder);
 
     {
       StackingContextHelper pageRootSc(sc, aBuilder);
@@ -94,7 +95,7 @@ WebRenderCommandBuilder::BuildWebRenderCommands(wr::DisplayListBuilder& aBuilder
       aScrollData.AddLayerData(*i);
     }
     mLayerScrollData.clear();
-    mClipIdCache.clear();
+    mScrollingHelper.EndBuild();
 
     // Remove the user data those are not displayed on the screen and
     // also reset the data to unused for next transaction.
@@ -213,16 +214,14 @@ WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList(nsDisplayList* a
       }
     }
 
-    { // ensure the scope of ScrollingLayersHelper is maintained
+    mScrollingHelper.BeginItem(item, aSc);
-      ScrollingLayersHelper clip(item, aBuilder, aSc, mClipIdCache, apzEnabled);
+    // Note: this call to CreateWebRenderCommands can recurse back into
-
+    // this function if the |item| is a wrapper for a sublist.
-      // Note: this call to CreateWebRenderCommands can recurse back into
+    if (!item->CreateWebRenderCommands(aBuilder, aResources, aSc, mManager,
-      // this function if the |item| is a wrapper for a sublist.
+                                       aDisplayListBuilder)) {
-      if (!item->CreateWebRenderCommands(aBuilder, aResources, aSc, mManager,
+      PushItemAsImage(item, aBuilder, aResources, aSc, aDisplayListBuilder);
-                                         aDisplayListBuilder)) {
-        PushItemAsImage(item, aBuilder, aResources, aSc, aDisplayListBuilder);
-      }
     }
+    mScrollingHelper.EndItem(item);
 
     if (apzEnabled) {
       if (forceNewLayerData) {

gfx/layers/wr/WebRenderCommandBuilder.h

@@ -7,6 +7,7 @@
 #define GFX_WEBRENDERCOMMANDBUILDER_H
 
 #include "mozilla/webrender/WebRenderAPI.h"
+#include "mozilla/layers/ScrollingLayersHelper.h"
 #include "mozilla/layers/WebRenderMessages.h"
 #include "mozilla/layers/WebRenderScrollData.h"
 #include "mozilla/layers/WebRenderUserData.h"
@@ -145,22 +146,9 @@ public:
     return res.forget();
   }
 
-public:
-  // Note: two DisplayItemClipChain* A and B might actually be "equal" (as per
-  // DisplayItemClipChain::Equal(A, B)) even though they are not the same pointer
-  // (A != B). In this hopefully-rare case, they will get separate entries
-  // in this map when in fact we could collapse them. However, to collapse
-  // them involves writing a custom hash function for the pointer type such that
-  // A and B hash to the same things whenever DisplayItemClipChain::Equal(A, B)
-  // is true, and that will incur a performance penalty for all the hashmap
-  // operations, so is probably not worth it. With the current code we might
-  // end up creating multiple clips in WR that are effectively identical but
-  // have separate clip ids. Hopefully this won't happen very often.
-  typedef std::unordered_map<const DisplayItemClipChain*, wr::WrClipId> ClipIdMap;
-
 private:
   WebRenderLayerManager* mManager;
-  ClipIdMap mClipIdCache;
+  ScrollingLayersHelper mScrollingHelper;
 
   // These fields are used to save a copy of the display list for
   // empty transactions in layers-free mode.

gfx/layers/wr/WebRenderLayerManager.cpp

@@ -12,7 +12,6 @@
 #include "mozilla/gfx/DrawEventRecorder.h"
 #include "mozilla/layers/CompositorBridgeChild.h"
 #include "mozilla/layers/IpcResourceUpdateQueue.h"
-#include "mozilla/layers/ScrollingLayersHelper.h"
 #include "mozilla/layers/StackingContextHelper.h"
 #include "mozilla/layers/TextureClient.h"
 #include "mozilla/layers/WebRenderBridgeChild.h"