Bug 1416991 - Fix perspective indexing in partial DL builds - r?mattwoodrow draft
authorGerald Squelart <gsquelart@mozilla.com>
Wed, 13 Dec 2017 05:41:13 -0600
changeset 711765 f48645fa09d6088a05a530dee4ef47ad2c1319c0
parent 711692 8062887ff0d9382ea84177f2c21f62dc0e613d9e
child 743865 51dc6570f6c28b51d1f0d6ece379075a74696b73
push id93134
push usergsquelart@mozilla.com
push dateThu, 14 Dec 2017 16:43:00 +0000
reviewersmattwoodrow
bugs1416991
milestone59.0a1
Bug 1416991 - Fix perspective indexing in partial DL builds - r?mattwoodrow Perspective item indices (used to produce unique per-frame keys) were generated by incrementing a counter in the builder when building a perspective display item. This caused problems with retained display lists, because an unmodified perspective could be skipped during a partial build, causing other perspectives to be incorrectly numbered and then incorrectly merged with the previous retained display list. To fix this, we need to always increment the counter if there is likely to be a perspective, before that item may be skipped. MozReview-Commit-ID: Edn7lUOLuPw
layout/generic/nsFrame.cpp
layout/painting/nsDisplayList.cpp
layout/painting/nsDisplayList.h
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -3473,16 +3473,22 @@ nsIFrame::BuildDisplayListForChild(nsDis
   if (doingShortcut) {
     // This is the shortcut for frames been handled along the common
     // path, the most common one of THE COMMON CASE mentioned later.
     MOZ_ASSERT(child->Type() != LayoutFrameType::Placeholder);
     MOZ_ASSERT(!aBuilder->GetSelectedFramesOnly() &&
                !aBuilder->GetIncludeAllOutOfFlows(),
                "It should be held for painting to window");
 
+    if (child->HasPerspective()) {
+      // We need to allocate a perspective index before a potential early
+      // return below.
+      aBuilder->AllocatePerspectiveItemIndex();
+    }
+
     // dirty rect in child-relative coordinates
     nsRect dirty = aBuilder->GetDirtyRect() - child->GetOffsetTo(this);
     nsRect visible = aBuilder->GetVisibleRect() - child->GetOffsetTo(this);
 
     if (!DescendIntoChild(aBuilder, child, visible, dirty)) {
       return;
     }
 
@@ -3576,16 +3582,22 @@ nsIFrame::BuildDisplayListForChild(nsDis
       // to enter to reach other out-of-flow frames that are visible.
       visible.SetEmpty();
       dirty.SetEmpty();
     }
     pseudoStackingContext = true;
     awayFromCommonPath = true;
   }
 
+  if (child->HasPerspective()) {
+    // We need to allocate a perspective index before a potential early
+    // return below.
+    aBuilder->AllocatePerspectiveItemIndex();
+  }
+
   NS_ASSERTION(!child->IsPlaceholderFrame(),
                "Should have dealt with placeholders already");
   if (aBuilder->GetSelectedFramesOnly() &&
       child->IsLeaf() &&
       !aChild->IsSelected()) {
     return;
   }
 
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -9124,17 +9124,17 @@ nsDisplayTransform::WriteDebugInfo(std::
 
 nsDisplayPerspective::nsDisplayPerspective(nsDisplayListBuilder* aBuilder,
                                            nsIFrame* aTransformFrame,
                                            nsIFrame* aPerspectiveFrame,
                                            nsDisplayList* aList)
   : nsDisplayItem(aBuilder, aPerspectiveFrame)
   , mList(aBuilder, aPerspectiveFrame, aList)
   , mTransformFrame(aTransformFrame)
-  , mIndex(aBuilder->AllocatePerspectiveItemIndex())
+  , mIndex(aBuilder->PerspectiveItemIndex())
 {
   MOZ_ASSERT(mList.GetChildren()->Count() == 1);
   MOZ_ASSERT(mList.GetChildren()->GetTop()->GetType() == DisplayItemType::TYPE_TRANSFORM);
 
   mTransformFrame->AddDisplayItem(this);
 }
 
 already_AddRefed<Layer>
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -1530,17 +1530,18 @@ 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++; }
+  void AllocatePerspectiveItemIndex() { ++mPerspectiveItemIndex; }
+  uint32_t PerspectiveItemIndex() const { 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