Bug 1475413 - Prefer using old items for uninvalidated frames during display list merging. r=miko
authorMatt Woodrow <mwoodrow@mozilla.com>
Fri, 13 Jul 2018 12:26:36 +1200
changeset 426596 ae6e73727685
parent 426595 03ecdc85a126
child 426597 d48e40cba0b4
push id34276
push userncsoregi@mozilla.com
push dateSat, 14 Jul 2018 09:41:08 +0000
treeherdermozilla-central@04dd259d71db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiko
bugs1475413
milestone63.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 1475413 - Prefer using old items for uninvalidated frames during display list merging. r=miko MozReview-Commit-ID: 14gc2IBJT1o
layout/painting/RetainedDisplayListBuilder.cpp
--- a/layout/painting/RetainedDisplayListBuilder.cpp
+++ b/layout/painting/RetainedDisplayListBuilder.cpp
@@ -230,17 +230,16 @@ UpdateASR(nsDisplayItem* aItem,
     ActiveScrolledRoot::PickAncestor(wrapList->GetFrameActiveScrolledRoot(),
                                      aContainerASR.value()));
 }
 
 void
 OldItemInfo::AddedMatchToMergedList(RetainedDisplayListBuilder* aBuilder,
                                     MergedListIndex aIndex)
 {
-  mItem->Destroy(aBuilder->Builder());
   AddedToMergedList(aIndex);
 }
 
 void
 OldItemInfo::Discard(RetainedDisplayListBuilder* aBuilder,
                      nsTArray<MergedListIndex>&& aDirectPredecessors)
 {
   MOZ_ASSERT(!IsUsed());
@@ -285,41 +284,86 @@ public:
     OldListIndex oldIndex;
     if (!HasModifiedFrame(aNewItem) &&
         HasMatchingItemInOldList(aNewItem, &oldIndex)) {
       nsDisplayItem* oldItem = mOldItems[oldIndex.val].mItem;
       MOZ_DIAGNOSTIC_ASSERT(oldItem->GetPerFrameKey() == aNewItem->GetPerFrameKey() &&
                             oldItem->Frame() == aNewItem->Frame());
       if (!mOldItems[oldIndex.val].IsChanged()) {
         MOZ_DIAGNOSTIC_ASSERT(!mOldItems[oldIndex.val].IsUsed());
+        nsDisplayItem* destItem = ShouldUseNewItem(aNewItem) ? aNewItem : oldItem;
         if (aNewItem->GetChildren()) {
           Maybe<const ActiveScrolledRoot*> containerASRForChildren;
           if (mBuilder->MergeDisplayLists(aNewItem->GetChildren(),
                                           oldItem->GetChildren(),
-                                          aNewItem->GetChildren(),
+                                          destItem->GetChildren(),
                                           containerASRForChildren,
                                           aNewItem->GetPerFrameKey())) {
-            aNewItem->InvalidateCachedChildInfo();
+            destItem->InvalidateCachedChildInfo();
             mResultIsModified = true;
 
           }
-          UpdateASR(aNewItem, containerASRForChildren);
-          aNewItem->UpdateBounds(mBuilder->Builder());
+          UpdateASR(destItem, containerASRForChildren);
+          destItem->UpdateBounds(mBuilder->Builder());
         }
 
         AutoTArray<MergedListIndex, 2> directPredecessors = ProcessPredecessorsOfOldNode(oldIndex);
-        MergedListIndex newIndex = AddNewNode(aNewItem, Some(oldIndex), directPredecessors, aPreviousItem);
+        MergedListIndex newIndex = AddNewNode(destItem, Some(oldIndex), directPredecessors, aPreviousItem);
         mOldItems[oldIndex.val].AddedMatchToMergedList(mBuilder, newIndex);
+        if (destItem == aNewItem) {
+          oldItem->Destroy(mBuilder->Builder());
+        } else {
+          aNewItem->Destroy(mBuilder->Builder());
+        }
         return newIndex;
       }
     }
     mResultIsModified = true;
     return AddNewNode(aNewItem, Nothing(), Span<MergedListIndex>(), aPreviousItem);
   }
 
+  bool ShouldUseNewItem(nsDisplayItem* aNewItem)
+  {
+    // Generally we want to use the old item when the frame isn't marked as modified
+    // so that any cached information on the item (or referencing the item) gets
+    // retained. Quite a few FrameLayerBuilder performance improvements benefit
+    // by this.
+    // Sometimes, however, we can end up where the new item paints something different
+    // from the old item, even though we haven't modified the frame, and it's hard to
+    // fix. In these cases we just always use the new item to be safe.
+    DisplayItemType type = aNewItem->GetType();
+    if (type == DisplayItemType::TYPE_CANVAS_BACKGROUND_COLOR ||
+        type == DisplayItemType::TYPE_SOLID_COLOR) {
+      // The canvas background color item can paint the color from another
+      // frame, and even though we schedule a paint, we don't mark the canvas
+      // frame as invalid.
+      return true;
+    } else if (type == DisplayItemType::TYPE_TABLE_BORDER_COLLAPSE) {
+      // We intentionally don't mark the root table frame as modified when a subframe
+      // changes, even though the border collapse item for the root frame is what paints
+      // the changed border. Marking the root frame as modified would rebuild display
+      // items for the whole table area, and we don't want that.
+      return true;
+    } else if (type == DisplayItemType::TYPE_TEXT_OVERFLOW) {
+      // Text overflow marker items are created with the wrapping block as their frame,
+      // and have an index value to note which line they are created for. Their rendering
+      // can change if the items on that line change, which may not mark the block as modified.
+      // We rebuild them if we build any item on the line, so we should always get new items
+      // if they might have changed rendering, and it's easier to just use the new items
+      // rather than computing if we actually need them.
+      return true;
+    } else if (type == DisplayItemType::TYPE_SUBDOCUMENT) {
+      // nsDisplaySubDocument::mShouldFlatten can change without an invalidation
+      // (and is the reason we unconditionally build the subdocument item), so always
+      // use the new one to make sure we get the right value.
+      return true;
+    }
+    return false;
+  }
+
   RetainedDisplayList Finalize() {
     for (size_t i = 0; i < mOldDAG.Length(); i++) {
       if (mOldItems[i].IsUsed()) {
         continue;
       }
 
       AutoTArray<MergedListIndex, 2> directPredecessors =
         ResolveNodeIndexesOldToMerged(mOldDAG.GetDirectPredecessors(OldListIndex(i)));