Bug 1464737 - Make nsDisplayPerspective simpler by using the transform frame as mFrame. r?miko draft
authorMatt Woodrow <mwoodrow@mozilla.com>
Fri, 01 Jun 2018 14:15:36 +1200
changeset 802556 f876c538a8403460854617df95670589980735d1
parent 802528 42880a726964a0bd66e2f636931e8322eae86ef7
push id111908
push usermwoodrow@mozilla.com
push dateFri, 01 Jun 2018 02:15:57 +0000
reviewersmiko
bugs1464737
milestone62.0a1
Bug 1464737 - Make nsDisplayPerspective simpler by using the transform frame as mFrame. r?miko MozReview-Commit-ID: CDjdjE2xCzG
layout/base/crashtests/1464737.html
layout/base/crashtests/crashtests.list
layout/generic/nsFrame.cpp
layout/painting/FrameLayerBuilder.cpp
layout/painting/nsDisplayList.cpp
layout/painting/nsDisplayList.h
new file mode 100644
--- /dev/null
+++ b/layout/base/crashtests/1464737.html
@@ -0,0 +1,7 @@
+<style>
+.cl { -webkit-transform-style: preserve-3d }
+:not(mask) { -webkit-perspective: 0px }
+:root { columns: 0px }
+</style>
+<textarea class="cl"></textarea>
+<p class="cl">z
--- a/layout/base/crashtests/crashtests.list
+++ b/layout/base/crashtests/crashtests.list
@@ -533,8 +533,9 @@ load 1452839.html
 load 1453196.html
 load 1453342.html
 load 1453702.html
 pref(dom.webcomponents.shadowdom.enabled,true) load 1461749.html
 load 1461812.html
 load 1462412.html
 load 1463940.html
 pref(dom.webcomponents.shadowdom.enabled,true) HTTP load 1464641.html
+load 1464737.html
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -2664,20 +2664,19 @@ FrameParticipatesIn3DContext(nsIFrame* a
   MOZ_ASSERT(frame == aAncestor);
   return true;
 }
 
 static bool
 ItemParticipatesIn3DContext(nsIFrame* aAncestor, nsDisplayItem* aItem)
 {
   nsIFrame* transformFrame;
-  if (aItem->GetType() == DisplayItemType::TYPE_TRANSFORM) {
+  if (aItem->GetType() == DisplayItemType::TYPE_TRANSFORM ||
+      aItem->GetType() == DisplayItemType::TYPE_PERSPECTIVE) {
     transformFrame = aItem->Frame();
-  } else if (aItem->GetType() == DisplayItemType::TYPE_PERSPECTIVE) {
-    transformFrame = static_cast<nsDisplayPerspective*>(aItem)->TransformFrame();
   } else {
     return false;
   }
   if (aAncestor == transformFrame) {
     return true;
   }
   return FrameParticipatesIn3DContext(aAncestor, transformFrame);
 }
@@ -2861,17 +2860,16 @@ nsIFrame::BuildDisplayListForStackingCon
   nsRect visibleRect = aBuilder->GetVisibleRect();
   nsRect dirtyRect = aBuilder->GetDirtyRect();
 
   const bool isTransformed = IsTransformed(disp);
   const bool hasPerspective = isTransformed && HasPerspective(disp);
   const bool extend3DContext = Extend3DContext(disp, effectSet);
   const bool combines3DTransformWithAncestors =
     (extend3DContext || isTransformed) && Combines3DTransformWithAncestors(disp);
-  const bool childrenHavePerspective = ChildrenHavePerspective(disp);
 
   Maybe<nsDisplayListBuilder::AutoPreserves3DContext> autoPreserves3DContext;
   if (extend3DContext && !combines3DTransformWithAncestors) {
     // Start a new preserves3d context to keep informations on
     // nsDisplayListBuilder.
     autoPreserves3DContext.emplace(aBuilder);
     // Save dirty rect on the builder to avoid being distorted for
     // multiple transforms along the chain.
@@ -2880,25 +2878,16 @@ nsIFrame::BuildDisplayListForStackingCon
     // We rebuild everything within preserve-3d and don't try
     // to retain, so override the dirty rect now.
     if (aBuilder->IsRetainingDisplayList()) {
       dirtyRect = visibleRect;
       aBuilder->SetDisablePartialUpdates(true);
     }
   }
 
-  // nsDisplayPerspective items use an index to keep their PerFrameKey unique.
-  // We need to make sure we build all of them for them to be consistent, so
-  // rebuild all items if we have perspective. Bug 1431249 should remove
-  // this requirement.
-  if (aBuilder->IsRetainingDisplayList() && childrenHavePerspective) {
-    dirtyRect = visibleRect;
-    aBuilder->SetDisablePartialUpdates(true);
-  }
-
   // reset blend mode so we can keep track if this stacking context needs have
   // a nsDisplayBlendContainer. Set the blend mode back when the routine exits
   // so we keep track if the parent stacking context needs a container too.
   AutoSaveRestoreContainsBlendMode autoRestoreBlendMode(*aBuilder);
   aBuilder->SetContainsBlendMode(false);
 
   nsRect visibleRectOutsideTransform = visibleRect;
   bool allowAsyncAnimation = false;
@@ -3059,18 +3048,16 @@ nsIFrame::BuildDisplayListForStackingCon
     clipForMask = ComputeClipForMaskItem(aBuilder, this, !useOpacity);
   }
 
   nsDisplayListCollection set(aBuilder);
   {
     DisplayListClipState::AutoSaveRestore nestedClipState(aBuilder);
     nsDisplayListBuilder::AutoInTransformSetter
       inTransformSetter(aBuilder, inTransform);
-    nsDisplayListBuilder::AutoSaveRestorePerspectiveIndex
-      perspectiveIndex(aBuilder, childrenHavePerspective);
     nsDisplayListBuilder::AutoFilterASRSetter
       filterASRSetter(aBuilder, usingFilter);
 
     CheckForApzAwareEventHandlers(aBuilder, this);
 
     Maybe<nsRect> contentClip =
       GetClipPropClipRect(disp, effects, GetSize());
 
@@ -3377,19 +3364,17 @@ nsIFrame::BuildDisplayListForStackingCon
     resultList.AppendToTop(transformItem);
 
     if (hasPerspective) {
       if (clipCapturedBy == ContainerItemType::ePerspective) {
         clipState.Restore();
       }
       resultList.AppendToTop(
         MakeDisplayItem<nsDisplayPerspective>(
-          aBuilder, this,
-          GetContainingBlock(0, disp)->GetContent()->GetPrimaryFrame(),
-          &resultList));
+          aBuilder, this, &resultList));
     }
 
     if (aCreatedContainerItem) {
       *aCreatedContainerItem = true;
     }
   }
 
   if (clipCapturedBy == ContainerItemType::eOwnLayerForTransformWithRoundedClip) {
--- a/layout/painting/FrameLayerBuilder.cpp
+++ b/layout/painting/FrameLayerBuilder.cpp
@@ -4630,17 +4630,17 @@ ContainerState::ProcessDisplayItems(nsDi
         // Perspective items have a single child item, an nsDisplayTransform.
         // If the perspective item is scrolled, but the perspective-inducing
         // frame is outside the scroll frame (indicated by item->Frame()
         // being outside that scroll frame), we have to take special care to
         // make APZ scrolling work properly. APZ needs us to put the scroll
         // frame's FrameMetrics on our child transform ContainerLayer instead.
         // It's worth investigating whether this ASR adjustment can be done at
         // display item creation time.
-        scrollMetadataASR = GetASRForPerspective(scrollMetadataASR, item->Frame());
+        scrollMetadataASR = GetASRForPerspective(scrollMetadataASR, item->Frame()->GetContainingBlock(nsIFrame::SKIP_SCROLLED_FRAME));
         params.mScrollMetadataASR = scrollMetadataASR;
         itemASR = scrollMetadataASR;
       }
 
       // Just use its layer.
       // Set layerContentsVisibleRect.width/height to -1 to indicate we
       // currently don't know. If BuildContainerLayerFor gets called by
       // item->BuildLayer, this will be set to a proper rect.
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -993,17 +993,16 @@ nsDisplayListBuilder::nsDisplayListBuild
       mUsedAGRBudget(0),
       mDirtyRect(-1,-1,-1,-1),
       mGlassDisplayItem(nullptr),
       mScrollInfoItemsForHoisting(nullptr),
       mActiveScrolledRootForRootScrollframe(nullptr),
       mMode(aMode),
       mCurrentScrollParentId(FrameMetrics::NULL_SCROLL_ID),
       mCurrentScrollbarTarget(FrameMetrics::NULL_SCROLL_ID),
-      mPerspectiveItemIndex(0),
       mSVGEffectsBuildingDepth(0),
       mFilterASR(nullptr),
       mContainsBlendMode(false),
       mIsBuildingScrollbar(false),
       mCurrentScrollbarWillHaveLayer(false),
       mBuildCaret(aBuildCaret),
       mRetainingDisplayList(aRetainingDisplayList),
       mPartialUpdate(false),
@@ -2966,17 +2965,17 @@ void nsDisplayList::HitTest(nsDisplayLis
 
     bool snap;
     nsRect r = item->GetBounds(aBuilder, &snap).Intersect(aRect);
     auto itemType = item->GetType();
     bool same3DContext =
       (itemType == DisplayItemType::TYPE_TRANSFORM &&
        static_cast<nsDisplayTransform*>(item)->IsParticipating3DContext()) ||
       (itemType == DisplayItemType::TYPE_PERSPECTIVE &&
-       static_cast<nsDisplayPerspective*>(item)->TransformFrame()->Extend3DContext());
+       item->Frame()->Extend3DContext());
     if (same3DContext &&
         (itemType != DisplayItemType::TYPE_TRANSFORM ||
          !static_cast<nsDisplayTransform*>(item)->IsLeafOf3DContext())) {
       if (!item->GetClip().MayIntersect(aRect)) {
         continue;
       }
       AutoTArray<nsIFrame*, 1> neverUsed;
       // Start gethering leaves of the 3D rendering context, and
@@ -9153,42 +9152,36 @@ nsDisplayTransform::WriteDebugInfo(std::
     aStream << " extends-3d-context";
   }
   if (mFrame->Combines3DTransformWithAncestors()) {
     aStream << " combines-3d-with-ancestors";
   }
 }
 
 nsDisplayPerspective::nsDisplayPerspective(nsDisplayListBuilder* aBuilder,
-                                           nsIFrame* aTransformFrame,
-                                           nsIFrame* aPerspectiveFrame,
+                                           nsIFrame* aFrame,
                                            nsDisplayList* aList)
-  : nsDisplayItem(aBuilder, aPerspectiveFrame)
-  , mList(aBuilder, aPerspectiveFrame, aList, true)
-  , mTransformFrame(aTransformFrame)
-  , mIndex(aBuilder->AllocatePerspectiveItemIndex())
+  : nsDisplayItem(aBuilder, aFrame)
+  , mList(aBuilder, aFrame, aList, true)
 {
   MOZ_ASSERT(mList.GetChildren()->Count() == 1);
   MOZ_ASSERT(mList.GetChildren()->GetTop()->GetType() == DisplayItemType::TYPE_TRANSFORM);
-
-  if (aBuilder->IsRetainingDisplayList()) {
-    mTransformFrame->AddDisplayItem(this);
-  }
+  mAnimatedGeometryRoot = aBuilder->FindAnimatedGeometryRootFor(mFrame->GetContainingBlock(nsIFrame::SKIP_SCROLLED_FRAME));
 }
 
 already_AddRefed<Layer>
 nsDisplayPerspective::BuildLayer(nsDisplayListBuilder *aBuilder,
                                  LayerManager *aManager,
                                  const ContainerLayerParameters& aContainerParameters)
 {
   float appUnitsPerPixel = mFrame->PresContext()->AppUnitsPerDevPixel();
 
   Matrix4x4 perspectiveMatrix;
   DebugOnly<bool> hasPerspective =
-    nsDisplayTransform::ComputePerspectiveMatrix(mTransformFrame, appUnitsPerPixel,
+    nsDisplayTransform::ComputePerspectiveMatrix(mFrame, appUnitsPerPixel,
                                                  perspectiveMatrix);
   MOZ_ASSERT(hasPerspective, "Why did we create nsDisplayPerspective?");
 
   /*
    * ClipListToRange can remove our child after we were created.
    */
   if (!mList.GetChildren()->GetTop()) {
     return nullptr;
@@ -9241,17 +9234,17 @@ nsDisplayPerspective::CreateWebRenderCom
                                               mozilla::wr::IpcResourceUpdateQueue& aResources,
                                               const StackingContextHelper& aSc,
                                               WebRenderLayerManager* aManager,
                                               nsDisplayListBuilder* aDisplayListBuilder)
 {
   float appUnitsPerPixel = mFrame->PresContext()->AppUnitsPerDevPixel();
   Matrix4x4 perspectiveMatrix;
   DebugOnly<bool> hasPerspective =
-    nsDisplayTransform::ComputePerspectiveMatrix(mTransformFrame, appUnitsPerPixel,
+    nsDisplayTransform::ComputePerspectiveMatrix(mFrame, appUnitsPerPixel,
                                                  perspectiveMatrix);
   MOZ_ASSERT(hasPerspective, "Why did we create nsDisplayPerspective?");
 
   /*
    * ClipListToRange can remove our child after we were created.
    */
   if (!mList.GetChildren()->GetTop()) {
     return false;
@@ -9287,22 +9280,16 @@ nsDisplayPerspective::CreateWebRenderCom
                            gfx::CompositionOp::OP_OVER,
                            !BackfaceIsHidden(),
                            true);
 
   return mList.CreateWebRenderCommands(aBuilder, aResources, sc,
                                        aManager, aDisplayListBuilder);
 }
 
-int32_t
-nsDisplayPerspective::ZIndex() const
-{
-  return ZIndexForFrame(mTransformFrame);
-}
-
 nsDisplayItemGeometry*
 nsCharClipDisplayItem::AllocateGeometry(nsDisplayListBuilder* aBuilder)
 {
   return new nsCharClipGeometry(this, aBuilder);
 }
 
 void
 nsCharClipDisplayItem::ComputeInvalidationRegion(nsDisplayListBuilder* aBuilder,
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -1189,41 +1189,16 @@ public:
     ~AutoFilterASRSetter() {
       mBuilder->mFilterASR = mOldValue;
     }
   private:
     nsDisplayListBuilder* mBuilder;
     const ActiveScrolledRoot* mOldValue;
   };
 
-  class AutoSaveRestorePerspectiveIndex {
-  public:
-    AutoSaveRestorePerspectiveIndex(nsDisplayListBuilder* aBuilder,
-                                    const bool aChildrenHavePerspective)
-      : mBuilder(nullptr)
-    {
-      if (aChildrenHavePerspective) {
-        mBuilder = aBuilder;
-        mCachedItemIndex = aBuilder->mPerspectiveItemIndex;
-        aBuilder->mPerspectiveItemIndex = 0;
-      }
-    }
-
-    ~AutoSaveRestorePerspectiveIndex()
-    {
-      if (mBuilder) {
-        mBuilder->mPerspectiveItemIndex = mCachedItemIndex;
-      }
-    }
-
-  private:
-    nsDisplayListBuilder* mBuilder;
-    uint32_t mCachedItemIndex;
-  };
-
   /**
    * A helper class to temporarily set the value of mCurrentScrollParentId.
    */
   class AutoCurrentScrollParentIdSetter {
   public:
     AutoCurrentScrollParentIdSetter(nsDisplayListBuilder* aBuilder, ViewID aScrollId)
       : mBuilder(aBuilder)
       , mOldValue(aBuilder->mCurrentScrollParentId)
@@ -1636,18 +1611,16 @@ public:
   /**
    * mContainsBlendMode is true if we processed a display item that
    * has a blend mode attached. We do this so we can insert a
    * nsDisplayBlendContainer in the parent stacking context.
    */
   void SetContainsBlendMode(bool aContainsBlendMode) { mContainsBlendMode = aContainsBlendMode; }
   bool ContainsBlendMode() const { return mContainsBlendMode; }
 
-  uint32_t AllocatePerspectiveItemIndex() { return mPerspectiveItemIndex++; }
-
   DisplayListClipState& ClipState() { return mClipState; }
   const ActiveScrolledRoot* CurrentActiveScrolledRoot() { return mCurrentActiveScrolledRoot; }
   const ActiveScrolledRoot* CurrentAncestorASRStackingContextContents() { return mCurrentContainerASR; }
 
   /**
    * Add the current frame to the will-change budget if possible and
    * remeber the outcome. Subsequent calls to IsInWillChangeBudget
    * will return the same value as return here.
@@ -1817,16 +1790,17 @@ private:
    */
   bool ShouldBuildCompositorHitTestInfo(const nsIFrame* aFrame,
                                         const mozilla::gfx::CompositorHitTestInfo& aInfo,
                                         const bool aBuildNew) const;
 
   friend class nsDisplayCanvasBackgroundImage;
   friend class nsDisplayBackgroundImage;
   friend class nsDisplayFixedPosition;
+  friend class nsDisplayPerspective;
   AnimatedGeometryRoot* FindAnimatedGeometryRootFor(nsDisplayItem* aItem);
 
   friend class nsDisplayItem;
   friend class nsDisplayOwnLayer;
   friend struct RetainedDisplayListBuilder;
   AnimatedGeometryRoot* FindAnimatedGeometryRootFor(nsIFrame* aFrame);
 
   AnimatedGeometryRoot* WrapAGRForFrame(nsIFrame* aAnimatedGeometryRoot,
@@ -1958,17 +1932,16 @@ private:
   std::list<DisplayItemClipChain*> mClipChainsToDestroy;
   nsTArray<nsDisplayItem*> mTemporaryItems;
   const ActiveScrolledRoot*      mActiveScrolledRootForRootScrollframe;
   nsDisplayListBuilderMode       mMode;
   ViewID                         mCurrentScrollParentId;
   ViewID                         mCurrentScrollbarTarget;
   MaybeScrollDirection           mCurrentScrollbarDirection;
   Preserves3DContext             mPreserves3DCtx;
-  uint32_t                       mPerspectiveItemIndex;
   int32_t                        mSVGEffectsBuildingDepth;
   // When we are inside a filter, the current ASR at the time we entered the
   // filter. Otherwise nullptr.
   const ActiveScrolledRoot*      mFilterASR;
   bool                           mContainsBlendMode;
   bool                           mIsBuildingScrollbar;
   bool                           mCurrentScrollbarWillHaveLayer;
   bool                           mBuildCaret;
@@ -6764,28 +6737,20 @@ private:
  */
 class nsDisplayPerspective : public nsDisplayItem
 {
   typedef mozilla::gfx::Point3D Point3D;
 
 public:
   NS_DISPLAY_DECL_NAME("nsDisplayPerspective", TYPE_PERSPECTIVE)
 
-  nsDisplayPerspective(nsDisplayListBuilder* aBuilder, nsIFrame* aTransformFrame,
-                       nsIFrame* aPerspectiveFrame,
+  nsDisplayPerspective(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame,
                        nsDisplayList* aList);
   ~nsDisplayPerspective()
   {
-    if (mTransformFrame) {
-      mTransformFrame->RemoveDisplayItem(this);
-    }
-  }
-
-  virtual uint32_t GetPerFrameKey() const override {
-    return (mIndex << TYPE_BITS) | nsDisplayItem::GetPerFrameKey();
   }
 
   virtual void HitTest(nsDisplayListBuilder* aBuilder, const nsRect& aRect,
                        HitTestState* aState, nsTArray<nsIFrame*> *aOutFrames) override
   {
     return mList.HitTest(aBuilder, aRect, aState, aOutFrames);
   }
 
@@ -6860,50 +6825,37 @@ public:
     mList.SetActiveScrolledRoot(aActiveScrolledRoot);
   }
 
   virtual nsRect GetComponentAlphaBounds(nsDisplayListBuilder* aBuilder) const override
   {
     return mList.GetComponentAlphaBounds(aBuilder);
   }
 
-  nsIFrame* TransformFrame() { return mTransformFrame; }
-
-  virtual nsIFrame* FrameForInvalidation() const override { return mTransformFrame; }
-
-  virtual int32_t ZIndex() const override;
-
   virtual void
   DoUpdateBoundsPreserves3D(nsDisplayListBuilder* aBuilder) override {
     if (mList.GetChildren()->GetTop()) {
       static_cast<nsDisplayTransform*>(mList.GetChildren()->GetTop())->DoUpdateBoundsPreserves3D(aBuilder);
     }
   }
 
   virtual void Destroy(nsDisplayListBuilder* aBuilder) override
   {
     mList.GetChildren()->DeleteAll(aBuilder);
     nsDisplayItem::Destroy(aBuilder);
   }
 
-  virtual bool HasDeletedFrame() const override { return !mTransformFrame || nsDisplayItem::HasDeletedFrame(); }
-
   virtual void RemoveFrame(nsIFrame* aFrame) override
   {
-    if (aFrame == mTransformFrame) {
-      mTransformFrame = nullptr;
-    }
     nsDisplayItem::RemoveFrame(aFrame);
     mList.RemoveFrame(aFrame);
   }
 
 private:
   nsDisplayWrapList mList;
-  nsIFrame* mTransformFrame;
-  uint32_t mIndex;
 };
 
 /**
  * This class adds basic support for limiting the rendering (in the inline axis
  * of the writing mode) to the part inside the specified edges.  It's a base
  * class for the display item classes that do the actual work.
  * The two members, mVisIStartEdge and mVisIEndEdge, are relative to the edges
  * of the frame's scrollable overflow rectangle and are the amount to suppress