Bug 1405359 - Stop passing around the clip id cache in all the functions. r?jrmuizel draft
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 24 Oct 2017 18:45:04 -0400
changeset 685846 dee159d4fcb1f784637eff4c2b34ed99315d5742
parent 685845 9c1bf5ec6b26d19db70ff38fa892c11097b9d5aa
child 685847 06d5d76f6bc29e8f6bfc40c7e665d943286b0b82
push id86016
push userkgupta@mozilla.com
push dateWed, 25 Oct 2017 01:53:44 +0000
reviewersjrmuizel
bugs1405359
milestone58.0a1
Bug 1405359 - Stop passing around the clip id cache in all the functions. r?jrmuizel Instead just keep a ref to it as a member variable. No functional change. MozReview-Commit-ID: 9jSBdZRIGuV
gfx/layers/wr/ScrollingLayersHelper.cpp
gfx/layers/wr/ScrollingLayersHelper.h
--- a/gfx/layers/wr/ScrollingLayersHelper.cpp
+++ b/gfx/layers/wr/ScrollingLayersHelper.cpp
@@ -15,16 +15,17 @@ 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.
   // The leafmost clip is trivially going to be aItem->GetClipChain().
@@ -32,17 +33,17 @@ ScrollingLayersHelper::ScrollingLayersHe
   // recursively define all the clips and scroll layers with the appropriate
   // parents, but will not actually push anything onto the WR stack.
   const ActiveScrolledRoot* leafmostASR = aItem->GetActiveScrolledRoot();
   if (aItem->GetClipChain()) {
     leafmostASR = ActiveScrolledRoot::PickDescendant(leafmostASR,
         aItem->GetClipChain()->mASR);
   }
   auto ids = DefineClipChain(aItem, leafmostASR, aItem->GetClipChain(),
-      auPerDevPixel, aStackingContext, aCache);
+      auPerDevPixel, aStackingContext);
 
   // Now that stuff is defined, we need to ensure the right items are on the
   // stack. We need this primarily for the WR display items that will be
   // generated while processing aItem. However those display items only care
   // about the topmost clip on the stack. If that were all we cared about we
   // would only need to push one thing here and we would be done. However, we
   // also care about the ScrollingLayersHelper instance that might be created
   // for nested display items, in the case where aItem is a wrapper item. The
@@ -81,26 +82,24 @@ ScrollingLayersHelper::ScrollingLayersHe
   }
 }
 
 std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>>
 ScrollingLayersHelper::DefineClipChain(nsDisplayItem* aItem,
                                        const ActiveScrolledRoot* aAsr,
                                        const DisplayItemClipChain* aChain,
                                        int32_t aAppUnitsPerDevPixel,
-                                       const StackingContextHelper& aStackingContext,
-                                       WebRenderCommandBuilder::ClipIdMap& aCache)
+                                       const StackingContextHelper& aStackingContext)
 {
   // This is the main entry point for defining the clip chain for a display
   // item. This function recursively walks up the ASR chain and the display
   // item's clip chain to define all the ASRs and clips necessary. Each level
   // of the recursion defines one item, if it hasn't been defined already.
   // The |aAsr| and |aChain| parameters are the important ones to track during
-  // the recursion; the rest of the parameters don't change (although |aCache|
-  // might be updated with new things).
+  // the recursion; the rest of the parameters don't change.
   // At each level of the recursion, the return value is the pair of identifiers
   // that correspond to aAsr and aChain, respectively.
 
   // These are the possible cases when recursing:
   //
   // aAsr is null, aChain is null     => base case; return
   // aAsr is non-null, aChain is null => recurse(aAsr->mParent, null),
   //                                     then define aAsr
@@ -116,65 +115,64 @@ ScrollingLayersHelper::DefineClipChain(n
   // on the ASR chain and one that recurses on the clip chain; that's what the
   // code below does.
 
   // in all of these cases, this invariant should hold:
   //   PickDescendant(aChain->mASR, aAsr) == aAsr
   MOZ_ASSERT(!aChain || ActiveScrolledRoot::PickDescendant(aChain->mASR, aAsr) == aAsr);
 
   if (aChain && aChain->mASR == aAsr) {
-    return RecurseAndDefineClip(aItem, aAsr, aChain, aAppUnitsPerDevPixel, aStackingContext, aCache);
+    return RecurseAndDefineClip(aItem, aAsr, aChain, aAppUnitsPerDevPixel, aStackingContext);
   }
   if (aAsr) {
-    return RecurseAndDefineAsr(aItem, aAsr, aChain, aAppUnitsPerDevPixel, aStackingContext, aCache);
+    return RecurseAndDefineAsr(aItem, aAsr, aChain, aAppUnitsPerDevPixel, aStackingContext);
   }
 
   MOZ_ASSERT(!aChain && !aAsr);
 
   return std::make_pair(Nothing(), Nothing());
 }
 
 std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>>
 ScrollingLayersHelper::RecurseAndDefineClip(nsDisplayItem* aItem,
                                             const ActiveScrolledRoot* aAsr,
                                             const DisplayItemClipChain* aChain,
                                             int32_t aAppUnitsPerDevPixel,
-                                            const StackingContextHelper& aSc,
-                                            WebRenderCommandBuilder::ClipIdMap& aCache)
+                                            const StackingContextHelper& aSc)
 {
   MOZ_ASSERT(aChain);
 
   // This will hold our return value
   std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>> ids;
 
   if (mBuilder->HasExtraClip()) {
-    // We can't use aCache directly. However if there's an out-of-band clip that
+    // We can't use mCache directly. However if there's an out-of-band clip that
     // was pushed on top of aChain, we should return the id for that OOB clip,
     // so that anything we want to define as a descendant of aChain we actually
     // end up defining as a descendant of the OOB clip.
     ids.second = mBuilder->GetCacheOverride(aChain);
   } else {
-    auto it = aCache.find(aChain);
-    if (it != aCache.end()) {
+    auto it = mCache.find(aChain);
+    if (it != mCache.end()) {
       ids.second = Some(it->second);
     }
   }
   if (ids.second) {
     // If we've already got an id for this clip, we can early-exit
     if (aAsr) {
       FrameMetrics::ViewID scrollId = nsLayoutUtils::ViewIDForASR(aAsr);
       MOZ_ASSERT(mBuilder->IsScrollLayerDefined(scrollId));
       ids.first = Some(scrollId);
     }
     return ids;
   }
 
   // If not, recurse to ensure all the ancestors are defined
   auto ancestorIds = DefineClipChain(
-      aItem, aAsr, aChain->mParent, aAppUnitsPerDevPixel, aSc, aCache);
+      aItem, aAsr, aChain->mParent, aAppUnitsPerDevPixel, aSc);
   ids = ancestorIds;
 
   if (!aChain->mClip.HasClip()) {
     // This item in the chain is a no-op, skip over it
     return ids;
   }
 
   // Now we need to figure out whether the new clip we're defining should be
@@ -227,80 +225,78 @@ ScrollingLayersHelper::RecurseAndDefineC
   nsTArray<wr::ComplexClipRegion> wrRoundedRects;
   aChain->mClip.ToComplexClipRegions(aAppUnitsPerDevPixel, aSc, wrRoundedRects);
 
   // Define the clip
   wr::WrClipId clipId = mBuilder->DefineClip(
       ancestorIds.first, ancestorIds.second,
       aSc.ToRelativeLayoutRect(clip), &wrRoundedRects);
   if (!mBuilder->HasExtraClip()) {
-    aCache[aChain] = clipId;
+    mCache[aChain] = clipId;
   }
 
   ids.second = Some(clipId);
   return ids;
 }
 
 std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>>
 ScrollingLayersHelper::RecurseAndDefineAsr(nsDisplayItem* aItem,
                                            const ActiveScrolledRoot* aAsr,
                                            const DisplayItemClipChain* aChain,
                                            int32_t aAppUnitsPerDevPixel,
-                                           const StackingContextHelper& aSc,
-                                           WebRenderCommandBuilder::ClipIdMap& aCache)
+                                           const StackingContextHelper& aSc)
 {
   MOZ_ASSERT(aAsr);
 
   // This will hold our return value
   std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>> ids;
 
   FrameMetrics::ViewID scrollId = nsLayoutUtils::ViewIDForASR(aAsr);
   if (mBuilder->IsScrollLayerDefined(scrollId)) {
     // If we've already defined this scroll layer before, we can early-exit
     ids.first = Some(scrollId);
     if (aChain) {
       if (mBuilder->HasExtraClip()) {
         ids.second = mBuilder->GetCacheOverride(aChain);
       } else {
-        auto it = aCache.find(aChain);
-        if (it == aCache.end()) {
+        auto it = mCache.find(aChain);
+        if (it == mCache.end()) {
           // Degenerate case, where there are two clip chain items that are
           // fundamentally the same but are different objects and so we can't
           // find it in the cache via hashing. Linear search for it instead.
           // XXX This shouldn't happen very often but it might still turn out
           // to be a performance cliff, so we should figure out a better way to
           // deal with this.
-          for (it = aCache.begin(); it != aCache.end(); it++) {
+          for (it = mCache.begin(); it != mCache.end(); it++) {
             if (DisplayItemClipChain::Equal(aChain, it->first)) {
               break;
             }
           }
         }
-        // If |it == aCache.end()| here then we have run into a case where the
+        // If |it == mCache.end()| here then we have run into a case where the
         // scroll layer was previously defined a specific parent clip, and
         // now here it has a different parent clip. Gecko can create display
         // lists like this because it treats the ASR chain and clipping chain
         // more independently, but we can't yet represent this in WR. This is
         // tracked by bug 1409442. For now we'll just leave ids.second as
         // Nothing() which will effectively ignore the clip |aChain|. Once WR
         // supports multiple ancestors on a scroll layer we can deal with this
         // better. The layout/reftests/text/wordwrap-08.html has a Text display
         // item that exercises this case.
-        if (it != aCache.end()) {
+        if (it != mCache.end()) {
           ids.second = Some(it->second);
         }
       }
     }
     return ids;
   }
 
   // If not, recurse to ensure all the ancestors are defined
   auto ancestorIds = DefineClipChain(
-      aItem, aAsr->mParent, aChain, aAppUnitsPerDevPixel, aSc,
-      aCache);
+      aItem, aAsr->mParent, aChain, aAppUnitsPerDevPixel, aSc);
   ids = ancestorIds;
 
   Maybe<ScrollMetadata> metadata = aAsr->mScrollableFrame->ComputeScrollMetadata(
       nullptr, aItem->ReferenceFrame(), ContainerLayerParameters(), nullptr);
   MOZ_ASSERT(metadata);
   FrameMetrics& metrics = metadata->GetMetrics();
 
   if (!metrics.IsScrollable()) {
--- a/gfx/layers/wr/ScrollingLayersHelper.h
+++ b/gfx/layers/wr/ScrollingLayersHelper.h
@@ -33,36 +33,34 @@ public:
   ~ScrollingLayersHelper();
 
 private:
   std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>>
   DefineClipChain(nsDisplayItem* aItem,
                   const ActiveScrolledRoot* aAsr,
                   const DisplayItemClipChain* aChain,
                   int32_t aAppUnitsPerDevPixel,
-                  const StackingContextHelper& aStackingContext,
-                  WebRenderCommandBuilder::ClipIdMap& aCache);
+                  const StackingContextHelper& aStackingContext);
 
   std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>>
   RecurseAndDefineClip(nsDisplayItem* aItem,
                        const ActiveScrolledRoot* aAsr,
                        const DisplayItemClipChain* aChain,
                        int32_t aAppUnitsPerDevPixel,
-                       const StackingContextHelper& aSc,
-                       WebRenderCommandBuilder::ClipIdMap& aCache);
+                       const StackingContextHelper& aSc);
 
   std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>>
   RecurseAndDefineAsr(nsDisplayItem* aItem,
                       const ActiveScrolledRoot* aAsr,
                       const DisplayItemClipChain* aChain,
                       int32_t aAppUnitsPerDevPixel,
-                      const StackingContextHelper& aSc,
-                      WebRenderCommandBuilder::ClipIdMap& aCache);
+                      const StackingContextHelper& aSc);
 
   wr::DisplayListBuilder* mBuilder;
   bool mPushedClipAndScroll;
   std::vector<wr::ScrollOrClipId> mPushedClips;
+  WebRenderCommandBuilder::ClipIdMap& mCache;
 };
 
 } // namespace layers
 } // namespace mozilla
 
 #endif