Bug 1405359 - Stop passing around the clip id cache in all the functions. r=jrmuizel
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 24 Oct 2017 18:45:04 -0400
changeset 388054 1345767fb2d502a9ab705ca73ccecbc7f5209c99
parent 388053 aaa39a5309985d35debb0aaeaa3316dc074aa608
child 388055 1921d545ea3933100baae89600c18b2209f8a434
push id32739
push useracraciun@mozilla.com
push dateWed, 25 Oct 2017 09:29:21 +0000
treeherdermozilla-central@252a8528c5ab [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 - 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