Bug 1503447 - Remove Nothing() semantic from ASR overrides r=kats
authorDzmitry Malyshau <dmalyshau@mozilla.com>
Wed, 14 Nov 2018 14:15:16 +0000
changeset 446193 a7dc664cc98546de9e3740caa2d0228efa439ed2
parent 446192 2cb84935ae6a2bf96035a16c480bc40a08848e5d
child 446194 02a5b2ef21e926eb336f92b23fcd639111d62ca4
push id35038
push userrmaries@mozilla.com
push dateWed, 14 Nov 2018 22:12:17 +0000
treeherdermozilla-central@4e1b2b7e0c37 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs1503447
milestone65.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 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
.gitignore
gfx/layers/wr/ClipManager.cpp
gfx/layers/wr/ClipManager.h
gfx/layers/wr/WebRenderCommandBuilder.cpp
gfx/layers/wr/WebRenderCommandBuilder.h
layout/painting/nsDisplayList.cpp
--- a/.gitignore
+++ b/.gitignore
@@ -8,16 +8,18 @@ TAGS
 tags
 compile_commands.json
 # Ignore ID generated by idutils and un-ignore id directory (for Indonesian locale)
 ID
 !id/
 .DS_Store*
 *.pdb
 *.egg-info
+# Filesystem temporaries
+.fuse_hidden*
 
 # Vim swap files.
 .*.sw[a-z]
 .sw[a-z]
 
 # Emacs directory variable files.
 **/.dir-locals.el
 
--- a/gfx/layers/wr/ClipManager.cpp
+++ b/gfx/layers/wr/ClipManager.cpp
@@ -53,19 +53,24 @@ ClipManager::EndBuild()
   MOZ_ASSERT(mASROverride.empty());
   MOZ_ASSERT(mItemClipStack.empty());
 }
 
 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);
   if (!mItemClipStack.empty()) {
     clips.CopyOutputsFrom(mItemClipStack.top());
   }
   mItemClipStack.push(clips);
 }
@@ -73,31 +78,36 @@ ClipManager::BeginList(const StackingCon
 void
 ClipManager::EndList(const StackingContextHelper& aStackingContext)
 {
   MOZ_ASSERT(!mItemClipStack.empty());
   mItemClipStack.top().Unapply(mBuilder);
   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
   mCacheStack.emplace();
 }
 
 void
 ClipManager::PopOverrideForASR(const ActiveScrolledRoot* aASR)
@@ -107,17 +117,17 @@ ClipManager::PopOverrideForASR(const Act
 
   Maybe<wr::WrClipId> scrollId = GetScrollLayer(aASR);
   MOZ_ASSERT(scrollId.isSome());
 
   auto it = mASROverride.find(*scrollId);
   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);
   }
 }
 
 Maybe<wr::WrClipId>
 ClipManager::ClipIdAfterOverride(const Maybe<wr::WrClipId>& aClipId)
@@ -127,17 +137,17 @@ ClipManager::ClipIdAfterOverride(const M
   }
   auto it = mASROverride.find(*aClipId);
   if (it == mASROverride.end()) {
     return 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
 ClipManager::BeginItem(nsDisplayItem* aItem,
                        const StackingContextHelper& aStackingContext)
 {
   CLIP_LOG("processing item %p\n", aItem);
 
--- a/gfx/layers/wr/ClipManager.h
+++ b/gfx/layers/wr/ClipManager.h
@@ -61,17 +61,17 @@ public:
   void BeginList(const StackingContextHelper& aStackingContext);
   void EndList(const StackingContextHelper& aStackingContext);
 
   void BeginItem(nsDisplayItem* aItem,
                  const StackingContextHelper& aStackingContext);
   ~ClipManager();
 
   void PushOverrideForASR(const ActiveScrolledRoot* aASR,
-                          const Maybe<wr::WrClipId>& aClipId);
+                          const wr::WrClipId& aClipId);
   void PopOverrideForASR(const ActiveScrolledRoot* aASR);
 
 private:
   Maybe<wr::WrClipId> ClipIdAfterOverride(const Maybe<wr::WrClipId>& aClipId);
 
   Maybe<wr::WrClipId>
   GetScrollLayer(const ActiveScrolledRoot* aASR);
 
@@ -114,17 +114,17 @@ private:
   // Any time ClipManager wants to define a new clip as a child of ASR X, it
   // should first check the cache overrides to see if there is a cache override
   // item ((a) or (b) above) that is already a child of X, and then define that
   // clip as a child of Y instead. This map stores X -> Y, which allows
   // ClipManager to do the necessary lookup. Note that there theoretically might
   // 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
   struct ItemClips {
     ItemClips(const ActiveScrolledRoot* aASR,
               const DisplayItemClipChain* aChain,
               bool aSeparateLeaf);
 
--- a/gfx/layers/wr/WebRenderCommandBuilder.cpp
+++ b/gfx/layers/wr/WebRenderCommandBuilder.cpp
@@ -440,17 +440,17 @@ struct DIGroup
       combined = aData->mClip.ApplyNonRoundedIntersection(aData->mGeometry->ComputeInvalidationRegion());
       combined.MoveBy(shift);
       combined.Or(combined, clip.ApplyNonRoundedIntersection(geometry->ComputeInvalidationRegion()));
       aData->mGeometry = std::move(geometry);
       */
       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,
              aData->mRect.y,
              aData->mRect.width,
              aData->mRect.height);
       InvalidateRect(aData->mRect.Intersect(mImageBounds));
       // We want to snap to outside pixels. When should we multiply by the matrix?
@@ -1584,17 +1584,17 @@ WebRenderCommandBuilder::CreateWebRender
   }
 
   mDumpIndent--;
   mClipManager.EndList(aSc);
 }
 
 void
 WebRenderCommandBuilder::PushOverrideForASR(const ActiveScrolledRoot* aASR,
-                                            const Maybe<wr::WrClipId>& aClipId)
+                                            const wr::WrClipId& aClipId)
 {
   mClipManager.PushOverrideForASR(aASR, aClipId);
 }
 
 void
 WebRenderCommandBuilder::PopOverrideForASR(const ActiveScrolledRoot* aASR)
 {
   mClipManager.PopOverrideForASR(aASR);
--- a/gfx/layers/wr/WebRenderCommandBuilder.h
+++ b/gfx/layers/wr/WebRenderCommandBuilder.h
@@ -54,17 +54,17 @@ public:
                               wr::IpcResourceUpdateQueue& aResourceUpdates,
                               nsDisplayList* aDisplayList,
                               nsDisplayListBuilder* aDisplayListBuilder,
                               WebRenderScrollData& aScrollData,
                               wr::LayoutSize& aContentSize,
                               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,
                                      ImageContainer* aContainer,
                                      mozilla::wr::DisplayListBuilder& aBuilder,
                                      mozilla::wr::IpcResourceUpdateQueue& aResources,
                                      mozilla::wr::ImageRendering aRendering,
                                      const StackingContextHelper& aSc,
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -7834,17 +7834,17 @@ nsDisplayStickyPosition::CreateWebRender
                                  rightMargin.ptrOr(nullptr),
                                  bottomMargin.ptrOr(nullptr),
                                  leftMargin.ptrOr(nullptr),
                                  vBounds,
                                  hBounds,
                                  applied);
 
     aBuilder.PushClip(id);
-    aManager->CommandBuilder().PushOverrideForASR(mContainerASR, Some(id));
+    aManager->CommandBuilder().PushOverrideForASR(mContainerASR, id);
   }
 
   {
     StackingContextHelper sc(aSc, GetActiveScrolledRoot(), aBuilder);
     nsDisplayWrapList::CreateWebRenderCommands(
       aBuilder, aResources, sc, aManager, aDisplayListBuilder);
   }
 
@@ -9468,17 +9468,17 @@ nsDisplayPerspective::CreateWebRenderCom
 
   Point3D newOrigin = Point3D(
     NSAppUnitsToFloatPixels(transform->ToReferenceFrame().x, appUnitsPerPixel),
     NSAppUnitsToFloatPixels(transform->ToReferenceFrame().y, appUnitsPerPixel),
     0.0f);
   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;
   StackingContextHelper sc(aSc,
                            GetActiveScrolledRoot(),
                            aBuilder,
                            filters,
                            LayoutDeviceRect(),
@@ -10179,29 +10179,21 @@ nsDisplayMasksAndClipPaths::CreateWebRen
                   /*aTransform: */ nullptr,
                   /*aPerspective: */ nullptr,
                   /*aMixBlendMode: */ gfx::CompositionOp::OP_OVER,
                   /*aBackfaceVisible: */ true,
                   /*aIsPreserve3D: */ false,
                   /*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;
 }
 
 Maybe<nsRect>
 nsDisplayMasksAndClipPaths::GetClipWithRespectToASR(
                               nsDisplayListBuilder* aBuilder,
                               const ActiveScrolledRoot* aASR) const
 {
@@ -10511,25 +10503,19 @@ nsDisplayFilters::CreateWebRenderCommand
                            nullptr,
                            nullptr,
                            gfx::CompositionOp::OP_OVER,
                            true,
                            false,
                            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;
 }
 
 #ifdef MOZ_DUMP_PAINTING
 void
 nsDisplayFilters::PrintEffects(nsACString& aTo)
 {
   nsIFrame* firstFrame =