Bug 1416065 - Ensure that override dirty rects are properly removed from frames r=mattwoodrow
authorMiko Mynttinen <mikokm@gmail.com>
Fri, 10 Nov 2017 01:32:29 +0100
changeset 444522 ad1fdeb7cbb8c0b53905a1b9428efd6560e9559e
parent 444521 5fd62ef74ab1513563db32f2fbc27ae1c81cc734
child 444523 4e6d62124ec0c017f219f47a91e5e7bc8320fa98
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow
bugs1416065
milestone58.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 1416065 - Ensure that override dirty rects are properly removed from frames r=mattwoodrow MozReview-Commit-ID: 8uLgDFxl5MV
layout/base/nsLayoutUtils.cpp
layout/painting/RetainedDisplayListBuilder.cpp
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -1395,38 +1395,46 @@ nsLayoutUtils::InvalidateForDisplayPortC
     frame = do_QueryFrame(frame->GetScrollTargetFrame());
   }
 
   if (changed && frame) {
     // It is important to call SchedulePaint on the same frame that we set the dirty
     // rect properties on so we can find the frame later to remove the properties.
     frame->SchedulePaint();
 
+    if (!gfxPrefs::LayoutRetainDisplayList()) {
+      return;
+    }
+
     nsIFrame* displayRoot = nsLayoutUtils::GetDisplayRootFrame(frame);
     RetainedDisplayListBuilder* retainedBuilder =
       displayRoot->GetProperty(RetainedDisplayListBuilder::Cached());
-    if (retainedBuilder) {
-      nsRect* rect =
-        frame->GetProperty(nsDisplayListBuilder::DisplayListBuildingDisplayPortRect());
-      if (!rect) {
-        rect = new nsRect();
-        frame->SetProperty(nsDisplayListBuilder::DisplayListBuildingDisplayPortRect(), rect);
-        frame->SetHasOverrideDirtyRegion(true);
-      }
-      if (aHadDisplayPort) {
-        // We only need to build a display list for any new areas added
-        nsRegion newRegion(aNewDisplayPort);
-        newRegion.SubOut(aOldDisplayPort);
-        rect->UnionRect(*rect, newRegion.GetBounds());
-      } else {
-        rect->UnionRect(*rect, aNewDisplayPort);
-      }
-    }
-  }
-
+
+    if (!retainedBuilder) {
+      return;
+    }
+
+    nsRect* rect =
+      frame->GetProperty(nsDisplayListBuilder::DisplayListBuildingDisplayPortRect());
+
+    if (!rect) {
+      rect = new nsRect();
+      frame->SetProperty(nsDisplayListBuilder::DisplayListBuildingDisplayPortRect(), rect);
+      frame->SetHasOverrideDirtyRegion(true);
+    }
+
+    if (aHadDisplayPort) {
+      // We only need to build a display list for any new areas added
+      nsRegion newRegion(aNewDisplayPort);
+      newRegion.SubOut(aOldDisplayPort);
+      rect->UnionRect(*rect, newRegion.GetBounds());
+    } else {
+      rect->UnionRect(*rect, aNewDisplayPort);
+    }
+  }
 }
 
 bool
 nsLayoutUtils::SetDisplayPortMargins(nsIContent* aContent,
                                      nsIPresShell* aPresShell,
                                      const ScreenMargin& aMargins,
                                      uint32_t aPriority,
                                      RepaintMode aRepaintMode)
--- a/layout/painting/RetainedDisplayListBuilder.cpp
+++ b/layout/painting/RetainedDisplayListBuilder.cpp
@@ -755,30 +755,47 @@ ShouldBuildPartial(nsTArray<nsIFrame*>& 
         type == LayoutFrameType::Scrollbar) {
       return false;
     }
   }
 
   return true;
 }
 
+static void
+ClearFrameProps(nsTArray<nsIFrame*>& aFrames)
+{
+  for (nsIFrame* f : aFrames) {
+    if (f->HasOverrideDirtyRegion()) {
+      f->SetHasOverrideDirtyRegion(false);
+      f->DeleteProperty(nsDisplayListBuilder::DisplayListBuildingRect());
+      f->DeleteProperty(nsDisplayListBuilder::DisplayListBuildingDisplayPortRect());
+    }
+
+    f->SetFrameIsModified(false);
+  }
+}
+
 bool
 RetainedDisplayListBuilder::AttemptPartialUpdate(nscolor aBackstop)
 {
   mBuilder.RemoveModifiedWindowDraggingRegion();
   if (mBuilder.ShouldSyncDecodeImages()) {
     MarkFramesWithItemsAndImagesModified(&mList);
   }
 
   mBuilder.EnterPresShell(mBuilder.RootReferenceFrame());
 
   nsTArray<nsIFrame*> modifiedFrames =
     GetModifiedFrames(mBuilder.RootReferenceFrame());
 
-  const bool shouldBuildPartial = ShouldBuildPartial(modifiedFrames);
+  // Do not allow partial builds if the retained display list is empty, or if
+  // ShouldBuildPartial heuristic fails.
+  const bool shouldBuildPartial =
+    !mList.IsEmpty() && ShouldBuildPartial(modifiedFrames);
 
   if (mPreviousCaret != mBuilder.GetCaretFrame()) {
     if (mPreviousCaret) {
       if (mBuilder.MarkFrameModifiedDuringBuilding(mPreviousCaret)) {
         modifiedFrames.AppendElement(mPreviousCaret);
       }
     }
 
@@ -790,18 +807,19 @@ RetainedDisplayListBuilder::AttemptParti
 
     mPreviousCaret = mBuilder.GetCaretFrame();
   }
 
   nsRect modifiedDirty;
   AnimatedGeometryRoot* modifiedAGR = nullptr;
   nsTArray<nsIFrame*> framesWithProps;
   bool merged = false;
-  if (shouldBuildPartial && !mList.IsEmpty() &&
-      ComputeRebuildRegion(modifiedFrames, &modifiedDirty, &modifiedAGR, &framesWithProps)) {
+  if (shouldBuildPartial &&
+      ComputeRebuildRegion(modifiedFrames, &modifiedDirty,
+                           &modifiedAGR, &framesWithProps)) {
     modifiedDirty.IntersectRect(modifiedDirty, mBuilder.RootReferenceFrame()->GetVisualOverflowRectRelativeToSelf());
 
     PreProcessDisplayList(&mList, modifiedAGR);
 
     nsDisplayList modifiedDL(&mBuilder);
     if (!modifiedDirty.IsEmpty() || !framesWithProps.IsEmpty()) {
       mBuilder.SetDirtyRect(modifiedDirty);
       mBuilder.SetPartialUpdate(true);
@@ -833,26 +851,16 @@ RetainedDisplayListBuilder::AttemptParti
     //printf_stderr("Painting --- Merged list:\n");
     //nsFrame::PrintDisplayList(&mBuilder, mList);
 
     merged = true;
   }
 
   mBuilder.LeavePresShell(mBuilder.RootReferenceFrame(), &mList);
 
-  // TODO: Do we mark frames as modified during displaylist building? If
-  // we do this isn't gonna work.
-  for (nsIFrame* f : modifiedFrames) {
-    if (f) {
-      f->SetFrameIsModified(false);
-    }
-  }
-
-  // Override dirty regions should only exist during this function. We set them up during
-  // ComputeRebuildRegion, and clear them here.
-  for (nsIFrame* f: framesWithProps) {
-    f->SetHasOverrideDirtyRegion(false);
-    f->DeleteProperty(nsDisplayListBuilder::DisplayListBuildingRect());
-    f->DeleteProperty(nsDisplayListBuilder::DisplayListBuildingDisplayPortRect());
-  }
+  // We set the override dirty regions during ComputeRebuildRegion or in
+  // nsLayoutUtils::InvalidateForDisplayPortChange. The display port change also
+  // marks the frame modified, so those regions are cleared here as well.
+  ClearFrameProps(modifiedFrames);
+  ClearFrameProps(framesWithProps);
 
   return merged;
 }