Bug 1416065 - Ensure that override dirty rects are properly removed from frames draft
authorMiko Mynttinen <mikokm@gmail.com>
Fri, 10 Nov 2017 01:32:29 +0100
changeset 695903 eb1e9cbb60fe35cf8ca537ac68a16457d428dca4
parent 694943 f63559d7e6a570e4e73ba367964099394248e18d
child 739743 ee587237bf07a36e9f7685903a301f7224d6c909
push id88587
push userbmo:mikokm@gmail.com
push dateFri, 10 Nov 2017 00:54:09 +0000
bugs1416065
milestone58.0a1
Bug 1416065 - Ensure that override dirty rects are properly removed from frames 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
@@ -714,30 +714,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);
       }
     }
 
@@ -749,18 +766,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);
@@ -792,26 +810,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;
 }