Bug 1475413 - Prefer using old items for uninvalidated frames during display list merging. r?miko
MozReview-Commit-ID: 14gc2IBJT1o
--- 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)));