Bug 1405359 - Make ScrollingLayersHelper a more stateful class. r?jrmuizel draft
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 24 Oct 2017 18:46:44 -0400
changeset 685848 afacbf2bff2d3160ee38a9b3769c485953d297ba
parent 685847 06d5d76f6bc29e8f6bfc40c7e665d943286b0b82
child 685849 ee406b85224d8d5bbc19a8e00edf14e76e90bf77
push id86016
push userkgupta@mozilla.com
push dateWed, 25 Oct 2017 01:53:44 +0000
reviewersjrmuizel
bugs1405359
milestone58.0a1
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: GEZzCGbVqB1
gfx/layers/wr/ScrollingLayersHelper.cpp
gfx/layers/wr/ScrollingLayersHelper.h
gfx/layers/wr/WebRenderCommandBuilder.cpp
gfx/layers/wr/WebRenderCommandBuilder.h
gfx/layers/wr/WebRenderLayerManager.cpp
--- a/gfx/layers/wr/ScrollingLayersHelper.cpp
+++ b/gfx/layers/wr/ScrollingLayersHelper.cpp
@@ -1,30 +1,50 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #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,
-                                             wr::DisplayListBuilder& aBuilder,
-                                             const StackingContextHelper& aStackingContext,
-                                             WebRenderCommandBuilder::ClipIdMap& aCache,
-                                             bool aApzEnabled)
-  : mBuilder(&aBuilder)
-  , mCache(aCache)
+ScrollingLayersHelper::ScrollingLayersHelper()
+  : mBuilder(nullptr)
+{
+}
+
+void
+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();
 
   // 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.
   // The leafmost clip is trivially going to be aItem->GetClipChain().
@@ -53,42 +73,44 @@ ScrollingLayersHelper::ScrollingLayersHe
   FrameMetrics::ViewID scrollId = aItem->GetActiveScrolledRoot()
       ? nsLayoutUtils::ViewIDForASR(aItem->GetActiveScrolledRoot())
       : FrameMetrics::NULL_SCROLL_ID;
   // If the leafmost ASR is not the same as the item's ASR then we are dealing
   // 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);
 
+  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
   // some other ASR).
   if (needClipAndScroll) {
     // 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>>
 ScrollingLayersHelper::DefineClipChain(nsDisplayItem* aItem,
                                        const ActiveScrolledRoot* aAsr,
                                        const DisplayItemClipChain* aChain,
                                        int32_t aAppUnitsPerDevPixel,
                                        const StackingContextHelper& aStackingContext)
@@ -349,19 +371,30 @@ ScrollingLayersHelper::RecurseAndDefineA
   mBuilder->DefineScrollLayer(scrollId, ancestorIds.first, ancestorIds.second,
       aSc.ToRelativeLayoutRect(contentRect),
       aSc.ToRelativeLayoutRect(clipBounds));
 
   ids.first = Some(scrollId);
   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
 ScrollingLayersHelper::ItemClips::Apply(wr::DisplayListBuilder* aBuilder)
 {
   if (mScrollId) {
     aBuilder->PushScrollLayer(mScrollId.ref());
   }
--- a/gfx/layers/wr/ScrollingLayersHelper.h
+++ b/gfx/layers/wr/ScrollingLayersHelper.h
@@ -1,40 +1,47 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef GFX_SCROLLINGLAYERSHELPER_H
 #define GFX_SCROLLINGLAYERSHELPER_H
 
+#include <unordered_map>
+
 #include "mozilla/Attributes.h"
-#include "mozilla/layers/WebRenderCommandBuilder.h"
+
+class nsDisplayItem;
 
 namespace mozilla {
 
+struct ActiveScrolledRoot;
 struct DisplayItemClipChain;
 
 namespace wr {
 class DisplayListBuilder;
 }
 
 namespace layers {
 
 struct FrameMetrics;
 class StackingContextHelper;
 
-class MOZ_RAII ScrollingLayersHelper
+class ScrollingLayersHelper
 {
 public:
-  ScrollingLayersHelper(nsDisplayItem* aItem,
-                        wr::DisplayListBuilder& aBuilder,
-                        const StackingContextHelper& aStackingContext,
-                        WebRenderCommandBuilder::ClipIdMap& aCache,
-                        bool aApzEnabled);
+  ScrollingLayersHelper();
+
+  void BeginBuild(wr::DisplayListBuilder& aBuilder);
+  void EndBuild();
+
+  void BeginItem(nsDisplayItem* aItem,
+                 const StackingContextHelper& aStackingContext);
+  void EndItem(nsDisplayItem* aItem);
   ~ScrollingLayersHelper();
 
 private:
   std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>>
   DefineClipChain(nsDisplayItem* aItem,
                   const ActiveScrolledRoot* aAsr,
                   const DisplayItemClipChain* aChain,
                   int32_t aAppUnitsPerDevPixel,
@@ -49,27 +56,39 @@ private:
 
   std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>>
   RecurseAndDefineAsr(nsDisplayItem* aItem,
                       const ActiveScrolledRoot* aAsr,
                       const DisplayItemClipChain* aChain,
                       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;
     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;
+  std::vector<ItemClips> mItemClipStack;
 };
 
 } // namespace layers
 } // namespace mozilla
 
 #endif
--- a/gfx/layers/wr/WebRenderCommandBuilder.cpp
+++ b/gfx/layers/wr/WebRenderCommandBuilder.cpp
@@ -59,16 +59,17 @@ WebRenderCommandBuilder::BuildWebRenderC
   { // scoping for StackingContextHelper RAII
 
     StackingContextHelper sc;
     mParentCommands.Clear();
     aScrollData = WebRenderScrollData();
     MOZ_ASSERT(mLayerScrollData.empty());
     mLastCanvasDatas.Clear();
     mLastAsr = nullptr;
+    mScrollingHelper.BeginBuild(aBuilder);
 
     {
       StackingContextHelper pageRootSc(sc, aBuilder);
       CreateWebRenderCommandsFromDisplayList(aDisplayList, aDisplayListBuilder,
                                              pageRootSc, aBuilder, aResourceUpdates);
     }
 
     // Make a "root" layer data that has everything else as descendants
@@ -89,17 +90,17 @@ WebRenderCommandBuilder::BuildWebRenderC
     }
     // Append the WebRenderLayerScrollData items into WebRenderScrollData
     // in reverse order, from topmost to bottommost. This is in keeping with
     // the semantics of WebRenderScrollData.
     for (auto i = mLayerScrollData.crbegin(); i != mLayerScrollData.crend(); i++) {
       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.
     RemoveUnusedAndResetWebRenderUserData();
   }
 
   mManager->WrBridge()->AddWebRenderParentCommands(mParentCommands);
 }
@@ -208,26 +209,24 @@ WebRenderCommandBuilder::CreateWebRender
       // If we're going to create a new layer data for this item, stash the
       // ASR so that if we recurse into a sublist they will know where to stop
       // walking up their ASR chain when building scroll metadata.
       if (forceNewLayerData) {
         mAsrStack.push_back(asr);
       }
     }
 
-    { // ensure the scope of ScrollingLayersHelper is maintained
-      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.
-      if (!item->CreateWebRenderCommands(aBuilder, aResources, aSc, mManager,
-                                         aDisplayListBuilder)) {
-        PushItemAsImage(item, aBuilder, aResources, aSc, aDisplayListBuilder);
-      }
+    mScrollingHelper.BeginItem(item, aSc);
+    // Note: this call to CreateWebRenderCommands can recurse back into
+    // this function if the |item| is a wrapper for a sublist.
+    if (!item->CreateWebRenderCommands(aBuilder, aResources, aSc, mManager,
+                                       aDisplayListBuilder)) {
+      PushItemAsImage(item, aBuilder, aResources, aSc, aDisplayListBuilder);
     }
+    mScrollingHelper.EndItem(item);
 
     if (apzEnabled) {
       if (forceNewLayerData) {
         // Pop the thing we pushed before the recursion, so the topmost item on
         // the stack is enclosing display item's ASR (or the stack is empty)
         mAsrStack.pop_back();
         const ActiveScrolledRoot* stopAtAsr =
             mAsrStack.empty() ? nullptr : mAsrStack.back();
--- a/gfx/layers/wr/WebRenderCommandBuilder.h
+++ b/gfx/layers/wr/WebRenderCommandBuilder.h
@@ -2,16 +2,17 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef GFX_WEBRENDERCOMMANDBUILDER_H
 #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"
 #include "nsDisplayList.h"
 #include "nsIFrame.h"
 
 namespace mozilla {
 
@@ -140,32 +141,19 @@ public:
 
     if (T::Type() == WebRenderUserData::UserDataType::eCanvas) {
       mLastCanvasDatas.PutEntry(data->AsCanvasData());
     }
     RefPtr<T> res = static_cast<T*>(data.get());
     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.
   nsTArray<WebRenderParentCommand> mParentCommands;
 
   // We use this as a temporary data structure while building the mScrollData
   // inside a layers-free transaction.
   std::vector<WebRenderLayerScrollData> mLayerScrollData;
--- a/gfx/layers/wr/WebRenderLayerManager.cpp
+++ b/gfx/layers/wr/WebRenderLayerManager.cpp
@@ -7,17 +7,16 @@
 
 #include "BasicLayers.h"
 #include "gfxPrefs.h"
 #include "GeckoProfiler.h"
 #include "LayersLogging.h"
 #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"
 #include "mozilla/layers/UpdateImageHelper.h"
 #include "nsDisplayList.h"
 #include "WebRenderCanvasRenderer.h"
 
 namespace mozilla {