Bug 1459441 - Make sure we build the full display list when we have blend containers in order to get the correct sorting for them. r=mstange, a=RyanVM
authorMatt Woodrow <mwoodrow@mozilla.com>
Thu, 10 May 2018 11:39:12 +1200
changeset 473352 a5b962b8bb3d9a9301430bc47ce975e5279b2043
parent 473351 064b3540e831abdb746ecb36f50c7d8cc01762d0
child 473353 2932a4b5f7e77885014ce42d09ae14e3096f37b6
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange, RyanVM
bugs1459441
milestone61.0
Bug 1459441 - Make sure we build the full display list when we have blend containers in order to get the correct sorting for them. r=mstange, a=RyanVM MozReview-Commit-ID: ECTU7enMb1r
layout/generic/nsFrame.cpp
layout/generic/nsIFrame.h
layout/painting/RetainedDisplayListBuilder.cpp
layout/painting/nsDisplayList.cpp
layout/painting/nsDisplayList.h
layout/reftests/display-list/invalidated-blendmode-sorting-ref.html
layout/reftests/display-list/invalidated-blendmode-sorting.html
layout/reftests/display-list/reftest.list
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -2935,23 +2935,16 @@ nsIFrame::BuildDisplayListForStackingCon
   if (HasOverrideDirtyRegion() && !aBuilder->InInvalidSubtree() && !IsFrameModified()) {
     nsDisplayListBuilder::DisplayListBuildingData* data =
       GetProperty(nsDisplayListBuilder::DisplayListBuildingRect());
     if (data) {
       dirtyRect = data->mDirtyRect.Intersect(visibleRect);
       hasOverrideDirtyRect = true;
     }
   }
-  // Always build the entire display list if we previously had a blend
-  // container since a partial build might make us think we no longer
-  // need the container even though the merged result will.
-  if (aBuilder->IsRetainingDisplayList() && BuiltBlendContainer()) {
-    dirtyRect = visibleRect;
-    aBuilder->MarkFrameModifiedDuringBuilding(this);
-  }
 
   bool usingFilter = StyleEffects()->HasFilters();
   bool usingMask = nsSVGIntegrationUtils::UsingMaskOrClipPathForFrame(this);
   bool usingSVGEffects = usingFilter || usingMask;
 
   nsRect visibleRectOutsideSVGEffects = visibleRect;
   nsDisplayList hoistedScrollInfoItemsStorage;
   if (usingSVGEffects) {
@@ -3103,52 +3096,27 @@ nsIFrame::BuildDisplayListForStackingCon
     // When we gain/remove a blend container item, we need to mark this frame
     // as invalid and have the full display list for merging to track
     // the change correctly.
     // It seems really hard to track this in advance, as the bookkeeping
     // required to note which stacking contexts have blend descendants
     // is complex and likely to be buggy.
     // Instead we're doing the sad thing, detecting it afterwards, and just
     // repeating display list building if it changed.
-
-    // If we changed whether we're going to build a blend mode item,
-    // then we need to make sure we're marked as invalid and we've built
-    // the full display list.
-    if (aBuilder->ContainsBlendMode() != BuiltBlendContainer() &&
+    // We have to repeat building for the entire display list (or at least
+    // the outer stacking context), since we need to mark this frame as invalid
+    // to remove any existing content that isn't wrapped in the blend container,
+    // and then we need to build content infront/behind the blend container
+    // to get correct positioning during merging.
+    if (aBuilder->ContainsBlendMode() &&
         aBuilder->IsRetainingDisplayList()) {
-      SetBuiltBlendContainer(aBuilder->ContainsBlendMode());
-
-      // If we did a partial build then delete all the items we just built
-      // and repeat building with the full area.
       if (!aBuilder->GetDirtyRect().Contains(aBuilder->GetVisibleRect())) {
-        aBuilder->MarkCurrentFrameModifiedDuringBuilding();
-        set.DeleteAll(aBuilder);
-
-        if (eventRegions) {
-          eventRegions->Destroy(aBuilder);
-          eventRegions = MakeDisplayItem<nsDisplayLayerEventRegions>(aBuilder, this);
-          eventRegions->AddFrame(aBuilder, this);
-          aBuilder->SetLayerEventRegions(eventRegions);
-        }
-
-        aBuilder->BuildCompositorHitTestInfoIfNeeded(this,
-                                                     set.BorderBackground(),
-                                                     true);
-
-        // If this is the root frame, then the previous call to
-        // MarkAbsoluteFramesForDisplayList might have stored some fixed
-        // background data. Clear that now.
-        if (!GetParent()) {
-          aBuilder->ClearFixedBackgroundDisplayData();
-        }
-
-        MarkAbsoluteFramesForDisplayList(aBuilder);
-        aBuilder->Check();
-        BuildDisplayList(aBuilder, set);
-        aBuilder->Check();
+        aBuilder->SetPartialBuildFailed(true);
+      } else {
+        aBuilder->SetDisablePartialUpdates(true);
       }
     }
 
     if (eventRegions) {
       // If the event regions item ended up empty, throw it away rather than
       // adding it to the display list.
       if (!eventRegions->IsEmpty()) {
         set.BorderBackground()->AppendToBottom(eventRegions);
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -603,17 +603,16 @@ public:
     , mParentIsWrapperAnonBox(false)
     , mIsWrapperBoxNeedingRestyle(false)
     , mReflowRequestedForCharDataChange(false)
     , mForceDescendIntoIfVisible(false)
     , mBuiltDisplayList(false)
     , mFrameIsModified(false)
     , mHasOverrideDirtyRegion(false)
     , mMayHaveWillChangeBudget(false)
-    , mBuiltBlendContainer(false)
     , mIsPrimaryFrame(false)
     , mMayHaveTransformAnimation(false)
     , mMayHaveOpacityAnimation(false)
     , mAllDescendantsAreInvisible(false)
   {
     mozilla::PodZero(&mOverflow);
   }
 
@@ -4136,19 +4135,16 @@ public:
   void SetFrameIsModified(bool aFrameIsModified) { mFrameIsModified = aFrameIsModified; }
 
   bool HasOverrideDirtyRegion() { return mHasOverrideDirtyRegion; }
   void SetHasOverrideDirtyRegion(bool aHasDirtyRegion) { mHasOverrideDirtyRegion = aHasDirtyRegion; }
 
   bool MayHaveWillChangeBudget() { return mMayHaveWillChangeBudget; }
   void SetMayHaveWillChangeBudget(bool aHasBudget) { mMayHaveWillChangeBudget = aHasBudget; }
 
-  bool BuiltBlendContainer() { return mBuiltBlendContainer; }
-  void SetBuiltBlendContainer(bool aBuilt) { mBuiltBlendContainer = aBuilt; }
-
   /**
    * Returns the set of flags indicating the properties of the frame that the
    * compositor might care about for hit-testing purposes. Note that this function
    * must be called during Gecko display list construction time (i.e while the
    * frame tree is being traversed) because that is when the display list builder
    * has the necessary state set up correctly.
    */
   mozilla::gfx::CompositorHitTestInfo GetCompositorHitTestInfo(nsDisplayListBuilder* aBuilder);
@@ -4336,22 +4332,16 @@ protected:
   bool mHasOverrideDirtyRegion : 1;
 
   /**
    * True if frame has will-change, and currently has display
    * items consuming some of the will-change budget.
    */
   bool mMayHaveWillChangeBudget : 1;
 
-  /**
-   * True if we built an nsDisplayBlendContainer last time
-   * we did retained display list building for this frame.
-   */
-  bool mBuiltBlendContainer : 1;
-
 private:
   /**
    * True if this is the primary frame for mContent.
    */
   bool mIsPrimaryFrame : 1;
 
   bool mMayHaveTransformAnimation : 1;
   bool mMayHaveOpacityAnimation : 1;
@@ -4363,17 +4353,17 @@ private:
    * fact, all descendants are invisible.
    * For example; an element is visibility:visible and has a visibility:hidden
    * child. This flag is stil false in such case.
    */
   bool mAllDescendantsAreInvisible : 1;
 
 protected:
 
-  // There is no gap left here.
+  // There is a 1-bit gap left here.
 
   // Helpers
   /**
    * Can we stop inside this frame when we're skipping non-rendered whitespace?
    * @param  aForward [in] Are we moving forward (or backward) in content order.
    * @param  aOffset [in/out] At what offset into the frame to start looking.
    *         on output - what offset was reached (whether or not we found a place to stop).
    * @return STOP: An appropriate offset was found within this frame,
--- a/layout/painting/RetainedDisplayListBuilder.cpp
+++ b/layout/painting/RetainedDisplayListBuilder.cpp
@@ -1146,16 +1146,23 @@ RetainedDisplayListBuilder::AttemptParti
   if (!modifiedDL.IsEmpty()) {
     nsLayoutUtils::AddExtraBackgroundItems(mBuilder, modifiedDL, mBuilder.RootReferenceFrame(),
                                            nsRect(nsPoint(0, 0), mBuilder.RootReferenceFrame()->GetSize()),
                                            mBuilder.RootReferenceFrame()->GetVisualOverflowRectRelativeToSelf(),
                                            aBackstop);
   }
   mBuilder.SetPartialUpdate(false);
 
+  if (mBuilder.PartialBuildFailed()) {
+    mBuilder.SetPartialBuildFailed(false);
+    mBuilder.LeavePresShell(mBuilder.RootReferenceFrame(), List());
+    mList.ClearDAG();
+    return PartialUpdateResult::Failed;
+  }
+
   if (aChecker) {
     aChecker->Set(&modifiedDL, "TM");
   }
 
   //printf_stderr("Painting --- Modified list (dirty %d,%d,%d,%d):\n",
   //              modifiedDirty.x, modifiedDirty.y, modifiedDirty.width, modifiedDirty.height);
   //nsFrame::PrintDisplayList(&mBuilder, modifiedDL);
 
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -999,17 +999,18 @@ nsDisplayListBuilder::nsDisplayListBuild
       mWindowDraggingAllowed(false),
       mIsBuildingForPopup(nsLayoutUtils::IsPopup(aReferenceFrame)),
       mForceLayerForScrollParent(false),
       mAsyncPanZoomEnabled(nsLayoutUtils::AsyncPanZoomEnabled(aReferenceFrame)),
       mBuildingInvisibleItems(false),
       mHitTestIsForVisibility(false),
       mIsBuilding(false),
       mInInvalidSubtree(false),
-      mDisablePartialUpdates(false)
+      mDisablePartialUpdates(false),
+      mPartialBuildFailed(false)
 {
   MOZ_COUNT_CTOR(nsDisplayListBuilder);
 
   const bool useWRHitTest =
     gfxPrefs::WebRenderHitTest() && gfxVars::UseWebRender();
 
   mBuildCompositorHitTestInfo = mAsyncPanZoomEnabled && IsForPainting() &&
     (useWRHitTest || gfxPrefs::SimpleEventRegionItems());
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -836,16 +836,19 @@ public:
    * Return true if we're currently building a display list for a
    * nested presshell.
    */
   bool IsInSubdocument() { return mPresShellStates.Length() > 1; }
 
   void SetDisablePartialUpdates(bool aDisable) { mDisablePartialUpdates = aDisable; }
   bool DisablePartialUpdates() { return mDisablePartialUpdates; }
 
+  void SetPartialBuildFailed(bool aFailed) { mPartialBuildFailed = aFailed; }
+  bool PartialBuildFailed() { return mPartialBuildFailed; }
+
   /**
    * Return true if we're currently building a display list for the presshell
    * of a chrome document, or if we're building the display list for a popup.
    */
   bool IsInChromeDocumentOrPopup() {
     return mIsInChromePresContext || mIsBuildingForPopup;
   }
 
@@ -1989,16 +1992,17 @@ private:
   bool                           mAsyncPanZoomEnabled;
   bool                           mBuildingInvisibleItems;
   bool                           mHitTestIsForVisibility;
   bool                           mIsBuilding;
   bool                           mInInvalidSubtree;
   bool                           mBuildCompositorHitTestInfo;
   bool                           mLessEventRegionItems;
   bool                           mDisablePartialUpdates;
+  bool                           mPartialBuildFailed;
 };
 
 class nsDisplayItem;
 class nsDisplayList;
 class RetainedDisplayList;
 /**
  * nsDisplayItems are put in singly-linked lists rooted in an nsDisplayList.
  * nsDisplayItemLink holds the link. The lists are linked from lowest to
new file mode 100644
--- /dev/null
+++ b/layout/reftests/display-list/invalidated-blendmode-sorting-ref.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+  div {
+    width: 200px;
+    height: 200px;
+  }
+  .sort-marker {
+    position: fixed;
+    background-color: green;
+  }
+  .wrapper {
+    position: absolute;
+    isolation: isolate;
+  }
+  .inner {
+    position: absolute;
+    background-color: blue;
+  }
+</style>
+</head>
+<body>
+  <div class="sort-marker"></div>
+  <div class="wrapper">
+    <div class="inner" style="left: 5px; top: 5px; mix-blend-mode:screen"></div>
+    <div class="inner" id="move" style="left: 221px;"></div>
+  </div>
+  <div class="sort-marker" style="left: 20px; top: 20px;"></div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/display-list/invalidated-blendmode-sorting.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+<style>
+  div {
+    width: 200px;
+    height: 200px;
+  }
+  .sort-marker {
+    position: fixed;
+    background-color: green;
+  }
+  .wrapper {
+    position: absolute;
+    isolation: isolate;
+  }
+  .inner {
+    position: absolute;
+    background-color: blue;
+  }
+</style>
+</head>
+<body>
+  <div class="sort-marker"></div>
+  <div class="wrapper">
+    <div class="inner" style="left: 5px; top: 5px; mix-blend-mode:screen"></div>
+    <div class="inner" id="move" style="left: 220px;"></div>
+  </div>
+  <div class="sort-marker" style="left: 20px; top: 20px;"></div>
+</body>
+<script>
+  function doTest() {
+    document.getElementById("move").style.left = "221px";
+    document.documentElement.removeAttribute("class");
+  }
+  document.addEventListener("MozReftestInvalidate", doTest);
+</script>
+</html>
--- a/layout/reftests/display-list/reftest.list
+++ b/layout/reftests/display-list/reftest.list
@@ -5,16 +5,17 @@ skip-if(!retainedDisplayList) == retaine
 skip-if(!retainedDisplayList||!asyncPan) == retained-dl-async-scrolled-1.html retained-dl-async-scrolled-1-ref.html
 skip-if(!retainedDisplayList) == retained-dl-remove-for-ancestor-change-1.html retained-dl-remove-for-ancestor-change-1-ref.html
 skip-if(!retainedDisplayList) == retained-dl-scroll-out-of-view-1.html retained-dl-scroll-out-of-view-1-ref.html
 skip-if(!retainedDisplayList) == retained-dl-displayport-1.html retained-dl-displayport-1-ref.html
 skip-if(!retainedDisplayList) == retained-dl-prerender-transform-1.html retained-dl-prerender-transform-1-ref.html
 == retained-dl-wrap-list.html retained-dl-wrap-list-ref.html
 == retained-dl-zindex-1.html retained-dl-zindex-1-ref.html
 == retained-dl-zindex-2.html retained-dl-zindex-2-ref.html
+== invalidated-blendmode-sorting.html invalidated-blendmode-sorting-ref.html
 fuzzy(1,235200) == 1413073.html 1413073-ref.html
 == 1416291.html 1416291-ref.html
 == 1417601-1.html 1417601-1-ref.html
 == 1418945-1.html 1418945-1-ref.html
 skip-if(Android) == 1428993-1.html 1428993-1-ref.html
 == 1420480-1.html 1420480-1-ref.html
 == 1428993-2.html 1428993-2-ref.html
 needs-focus == 1429027-1.html 1429027-1-ref.html