Bug 1503447 - Remove Nothing() semantic from ASR overrides r=kats

Previously, the ASR overrides contained Maybe<ClipId>, where Nothing() corresponded to taking the top of the clip/scroll stack instead of overriding. This change removes the associated complexity by ensuring that we always provide the ClipId.

Differential Revision: https://phabricator.services.mozilla.com/D11813

--HG--
extra : moz-landing-system : lando

.gitignore

@@ -13,6 +13,8 @@ ID
 .DS_Store*
 *.pdb
 *.egg-info
+# Filesystem temporaries
+.fuse_hidden*
 
 # Vim swap files.
 .*.sw[a-z]

gfx/layers/wr/ClipManager.cpp

@@ -58,9 +58,14 @@ void
 ClipManager::BeginList(const StackingContextHelper& aStackingContext)
 {
   if (aStackingContext.AffectsClipPositioning()) {
-    PushOverrideForASR(
-        mItemClipStack.empty() ? nullptr : mItemClipStack.top().mASR,
-        aStackingContext.ReferenceFrameId());
+    if (aStackingContext.ReferenceFrameId()) {
+      PushOverrideForASR(
+          mItemClipStack.empty() ? nullptr : mItemClipStack.top().mASR,
+          aStackingContext.ReferenceFrameId().ref());
+    } else {
+      // Start a new cache
+      mCacheStack.emplace();
+    }
   }
 
   ItemClips clips(nullptr, nullptr, false);
@@ -78,21 +83,26 @@ ClipManager::EndList(const StackingContextHelper& aStackingContext)
   mItemClipStack.pop();
 
   if (aStackingContext.AffectsClipPositioning()) {
-    PopOverrideForASR(
+    if (aStackingContext.ReferenceFrameId()) {
+      PopOverrideForASR(
         mItemClipStack.empty() ? nullptr : mItemClipStack.top().mASR);
+    } else {
+      MOZ_ASSERT(!mCacheStack.empty());
+      mCacheStack.pop();
+    }
   }
 }
 
 void
 ClipManager::PushOverrideForASR(const ActiveScrolledRoot* aASR,
-                                const Maybe<wr::WrClipId>& aClipId)
+                                const wr::WrClipId& aClipId)
 {
   Maybe<wr::WrClipId> scrollId = GetScrollLayer(aASR);
   MOZ_ASSERT(scrollId.isSome());
 
   CLIP_LOG("Pushing override %zu -> %s\n", scrollId->id,
-      aClipId ? Stringify(aClipId->id).c_str() : "(none)");
-  auto it = mASROverride.insert({ *scrollId, std::stack<Maybe<wr::WrClipId>>() });
+      Stringify(aClipId->id).c_str());
+  auto it = mASROverride.insert({ *scrollId, std::stack<wr::WrClipId>() });
   it.first->second.push(aClipId);
 
   // Start a new cache
@@ -112,7 +122,7 @@ ClipManager::PopOverrideForASR(const ActiveScrolledRoot* aASR)
   MOZ_ASSERT(it != mASROverride.end());
   MOZ_ASSERT(!(it->second.empty()));
   CLIP_LOG("Popping override %zu -> %s\n", scrollId->id,
-      it->second.top() ? Stringify(it->second.top()->id).c_str() : "(none)");
+      Stringify(it->second.top()->id).c_str());
   it->second.pop();
   if (it->second.empty()) {
     mASROverride.erase(it);
@@ -132,7 +142,7 @@ ClipManager::ClipIdAfterOverride(const Maybe<wr::WrClipId>& aClipId)
   MOZ_ASSERT(!it->second.empty());
   CLIP_LOG("Overriding %zu with %s\n", aClipId->id,
       it->second.top() ? Stringify(it->second.top()->id).c_str() : "(none)");
-  return it->second.top();
+  return Some(it->second.top());
 }
 
 void

gfx/layers/wr/ClipManager.h

@@ -66,7 +66,7 @@ public:
   ~ClipManager();
 
   void PushOverrideForASR(const ActiveScrolledRoot* aASR,
-                          const Maybe<wr::WrClipId>& aClipId);
+                          const wr::WrClipId& aClipId);
   void PopOverrideForASR(const ActiveScrolledRoot* aASR);
 
 private:
@@ -119,7 +119,7 @@ private:
   // be multiple different "Y" clips (in case of nested cache overrides), which
   // is why we need a stack.
   std::unordered_map<wr::WrClipId,
-                     std::stack<Maybe<wr::WrClipId>>,
+                     std::stack<wr::WrClipId>,
                      wr::WrClipId::HashFn> mASROverride;
 
   // This holds some clip state for a single nsDisplayItem

gfx/layers/wr/WebRenderCommandBuilder.cpp

@@ -445,7 +445,7 @@ struct DIGroup
       combined = clip.ApplyNonRoundedIntersection(geometry->ComputeInvalidationRegion());
       aData->mGeometry = std::move(geometry);
 
-      GP("matrix: %f %f\n", aMatrix._31, aMatrix._32); 
+      GP("matrix: %f %f\n", aMatrix._31, aMatrix._32);
       GP("frame invalid invalidate: %s\n", aItem->Name());
       GP("old rect: %d %d %d %d\n",
              aData->mRect.x,
@@ -1589,7 +1589,7 @@ WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList(nsDisplayList* a
 
 void
 WebRenderCommandBuilder::PushOverrideForASR(const ActiveScrolledRoot* aASR,
-                                            const Maybe<wr::WrClipId>& aClipId)
+                                            const wr::WrClipId& aClipId)
 {
   mClipManager.PushOverrideForASR(aASR, aClipId);
 }

gfx/layers/wr/WebRenderCommandBuilder.h

@@ -59,7 +59,7 @@ public:
                               const nsTArray<wr::WrFilterOp>& aFilters);
 
   void PushOverrideForASR(const ActiveScrolledRoot* aASR,
-                          const Maybe<wr::WrClipId>& aClipId);
+                          const wr::WrClipId& aClipId);
   void PopOverrideForASR(const ActiveScrolledRoot* aASR);
 
   Maybe<wr::ImageKey> CreateImageKey(nsDisplayItem* aItem,

layout/painting/nsDisplayList.cpp

@@ -7839,7 +7839,7 @@ nsDisplayStickyPosition::CreateWebRenderCommands(
                                  applied);
 
     aBuilder.PushClip(id);
-    aManager->CommandBuilder().PushOverrideForASR(mContainerASR, Some(id));
+    aManager->CommandBuilder().PushOverrideForASR(mContainerASR, id);
   }
 
   {
@@ -9473,7 +9473,7 @@ nsDisplayPerspective::CreateWebRenderCommands(
   Point3D roundedOrigin(NS_round(newOrigin.x), NS_round(newOrigin.y), 0);
 
   gfx::Matrix4x4 transformForSC = gfx::Matrix4x4::Translation(roundedOrigin);
-  
+
   nsIFrame* perspectiveFrame = mFrame->GetContainingBlock(nsIFrame::SKIP_SCROLLED_FRAME);
 
   nsTArray<mozilla::wr::WrFilterOp> filters;
@@ -10184,19 +10184,11 @@ nsDisplayMasksAndClipPaths::CreateWebRenderCommands(
                   /*aTransformForScrollData: */ Nothing(),
                   /*aClipNodeId: */ &clipId);
     sc = layer.ptr();
-    // The whole stacking context will be clipped by us, so no need to have any
-    // parent for the children context's clip.
-    aManager->CommandBuilder().PushOverrideForASR(GetActiveScrolledRoot(),
-                                                  Nothing());
   }
 
   nsDisplayEffectsBase::CreateWebRenderCommands(
     aBuilder, aResources, *sc, aManager, aDisplayListBuilder);
 
-  if (clip) {
-    aManager->CommandBuilder().PopOverrideForASR(GetActiveScrolledRoot());
-  }
-
   return true;
 }
 
@@ -10516,15 +10508,9 @@ nsDisplayFilters::CreateWebRenderCommands(
                            Nothing(),
                            &clipId);
 
-  // The whole stacking context will be clipped by us, so no need to have any
-  // parent for the children context's clip.
-  aManager->CommandBuilder().PushOverrideForASR(GetActiveScrolledRoot(),
-                                                Nothing());
-
   nsDisplayEffectsBase::CreateWebRenderCommands(
     aBuilder, aResources, sc, aManager, aDisplayListBuilder);
 
-  aManager->CommandBuilder().PopOverrideForASR(GetActiveScrolledRoot());
   return true;
 }