Bug 1416065 - Ensure that override dirty rects are properly removed from frames
MozReview-Commit-ID: 8uLgDFxl5MV
--- 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;
}