Bug 1405359 - Make ScrollingLayersHelper a more stateful class. r=jrmuizel
☠☠ backed out by 1d6b1bfb9e23 ☠ ☠
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 24 Oct 2017 16:15:00 -0400
changeset 387978 154c415885b818ca3de8f21c44639994bddeab5d
parent 387977 f6ce05f8e699674a01d0df99a6a36c73191ba273
child 387979 988d6a397ea8eb440e7fd521f818202124de8417
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 - 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
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
@@ -2,39 +2,44 @@
  * 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 "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 +54,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 {