Backed out changeset 154c415885b8 (bug 1405359)
authorSebastian Hengst <archaeopteryx@coole-files.de>
Wed, 25 Oct 2017 00:16:49 +0200
changeset 685842 1d6b1bfb9e23e7acc6e73a6395619f3bfbce8948
parent 685841 79d7006caad1628cd502a789d87418b5c3972f26
child 685843 870e07ee24cf9af05c0e790afd2918ec6e63cf52
push id86016
push userkgupta@mozilla.com
push dateWed, 25 Oct 2017 01:53:44 +0000
bugs1405359
milestone58.0a1
backs out154c415885b818ca3de8f21c44639994bddeab5d
Backed out changeset 154c415885b8 (bug 1405359)
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,50 +1,30 @@
 /* -*- 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()
-  : 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)
+ScrollingLayersHelper::ScrollingLayersHelper(nsDisplayItem* aItem,
+                                             wr::DisplayListBuilder& aBuilder,
+                                             const StackingContextHelper& aStackingContext,
+                                             WebRenderCommandBuilder::ClipIdMap& aCache,
+                                             bool aApzEnabled)
+  : mBuilder(&aBuilder)
+  , 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.
   // The leafmost clip is trivially going to be aItem->GetClipChain().
@@ -73,44 +53,42 @@ ScrollingLayersHelper::BeginItem(nsDispl
   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
-    clips.mScrollId = Some(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) {
-    clips.mClipId = ids.second;
+    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) {
     // 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 = clips.mClipId;
+    Maybe<wr::WrClipId> clipId = mItemClips.mClipId;
     if (!clipId) {
       clipId = mBuilder->TopmostClipId();
     }
-    clips.mClipAndScroll = Some(std::make_pair(scrollId, clipId));
+    mItemClips.mClipAndScroll = Some(std::make_pair(scrollId, clipId));
   }
 
-  clips.Apply(mBuilder);
-  mItemClipStack.push_back(clips);
+  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)
@@ -371,30 +349,19 @@ 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()
 {
-  MOZ_ASSERT(!mBuilder);
-  MOZ_ASSERT(mCache.empty());
-  MOZ_ASSERT(mItemClipStack.empty());
+  mItemClips.Unapply(mBuilder);
 }
 
 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,44 +2,39 @@
  * 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"
-
-class nsDisplayItem;
+#include "mozilla/layers/WebRenderCommandBuilder.h"
 
 namespace mozilla {
 
-struct ActiveScrolledRoot;
 struct DisplayItemClipChain;
 
 namespace wr {
 class DisplayListBuilder;
 }
 
 namespace layers {
 
 struct FrameMetrics;
 class StackingContextHelper;
 
-class ScrollingLayersHelper
+class MOZ_RAII ScrollingLayersHelper
 {
 public:
-  ScrollingLayersHelper();
-
-  void BeginBuild(wr::DisplayListBuilder& aBuilder);
-  void EndBuild();
-
-  void BeginItem(nsDisplayItem* aItem,
-                 const StackingContextHelper& aStackingContext);
-  void EndItem(nsDisplayItem* aItem);
+  ScrollingLayersHelper(nsDisplayItem* aItem,
+                        wr::DisplayListBuilder& aBuilder,
+                        const StackingContextHelper& aStackingContext,
+                        WebRenderCommandBuilder::ClipIdMap& aCache,
+                        bool aApzEnabled);
   ~ScrollingLayersHelper();
 
 private:
   std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>>
   DefineClipChain(nsDisplayItem* aItem,
                   const ActiveScrolledRoot* aAsr,
                   const DisplayItemClipChain* aChain,
                   int32_t aAppUnitsPerDevPixel,
@@ -54,39 +49,27 @@ 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;
-  ClipIdMap mCache;
+  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);
   };
 
-  std::vector<ItemClips> mItemClipStack;
+  ItemClips mItemClips;
 };
 
 } // namespace layers
 } // namespace mozilla
 
 #endif
--- a/gfx/layers/wr/WebRenderCommandBuilder.cpp
+++ b/gfx/layers/wr/WebRenderCommandBuilder.cpp
@@ -59,17 +59,16 @@ 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
@@ -90,17 +89,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();
-    mScrollingHelper.EndBuild();
+    mClipIdCache.clear();
 
     // 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);
 }
@@ -209,24 +208,26 @@ 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);
       }
     }
 
-    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);
+    { // 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.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,17 +2,16 @@
  * 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 {
 
@@ -141,19 +140,32 @@ 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;
-  ScrollingLayersHelper mScrollingHelper;
+  ClipIdMap mClipIdCache;
 
   // 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,16 +7,17 @@
 
 #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 {