From 2175f47f8f0ca12525a9f95839d05f248755ddb6 Mon Sep 17 00:00:00 2001 From: Miko Mynttinen Date: Fri, 28 Jan 2022 16:30:55 +0000 Subject: [PATCH] Bug 1751965 - Remove destroyed display items from reused display items list r=mstange Differential Revision: https://phabricator.services.mozilla.com/D137235 --- layout/base/nsLayoutUtils.cpp | 8 ++-- .../painting/RetainedDisplayListBuilder.cpp | 43 ++++--------------- layout/painting/RetainedDisplayListBuilder.h | 6 +-- layout/painting/nsDisplayList.cpp | 26 +++++++++++ layout/painting/nsDisplayList.h | 26 +++++++++++ 5 files changed, 68 insertions(+), 41 deletions(-) diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index 414e7226799d..e3893adeed4c 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -3336,16 +3336,16 @@ void nsLayoutUtils::PaintFrame(gfxContext* aRenderingContext, nsIFrame* aFrame, } if (doFullRebuild) { - list->DeleteAll(builder); - list->RestoreState(); - if (useRetainedBuilder) { retainedBuilder->ClearFramesWithProps(); + retainedBuilder->ClearReuseableDisplayItems(); mozilla::RDLUtils::AssertFrameSubtreeUnmodified( builder->RootReferenceFrame()); - MOZ_ASSERT(retainedBuilder->List()->IsEmpty()); } + list->DeleteAll(builder); + list->RestoreState(); + builder->ClearRetainedWindowRegions(); builder->ClearWillChangeBudgets(); diff --git a/layout/painting/RetainedDisplayListBuilder.cpp b/layout/painting/RetainedDisplayListBuilder.cpp index 0596b8604dd1..b3371ae38fad 100644 --- a/layout/painting/RetainedDisplayListBuilder.cpp +++ b/layout/painting/RetainedDisplayListBuilder.cpp @@ -1507,7 +1507,8 @@ MOZ_NEVER_INLINE_DEBUG void ReuseStackingContextItem( aItem->UpdateBounds(aBuilder); } - DL_LOGD("Retaining display item %p", aItem); + aBuilder->AddReusableDisplayItem(aItem); + DL_LOGD("Reusing display item %p", aItem); } bool IsSupportedFrameType(const nsIFrame* aFrame) { @@ -1558,10 +1559,8 @@ bool IsReuseableStackingContextItem(nsDisplayItem* aItem) { * linked and collected in given |aOutItems| array. */ void CollectStackingContextItems(nsDisplayListBuilder* aBuilder, - nsDisplayList* aList, - nsTArray& aOutItems, - nsIFrame* aOuterFrame, int aDepth = 0, - bool aParentReused = false) { + nsDisplayList* aList, nsIFrame* aOuterFrame, + int aDepth = 0, bool aParentReused = false) { nsDisplayList out; while (nsDisplayItem* item = aList->RemoveBottom()) { @@ -1593,8 +1592,8 @@ void CollectStackingContextItems(nsDisplayListBuilder* aBuilder, const bool isStackingContextItem = IsReuseableStackingContextItem(item); if (item->GetChildren()) { - CollectStackingContextItems(aBuilder, item->GetChildren(), aOutItems, - item->Frame(), aDepth + 1, + CollectStackingContextItems(aBuilder, item->GetChildren(), item->Frame(), + aDepth + 1, aParentReused || isStackingContextItem); } @@ -1604,7 +1603,6 @@ void CollectStackingContextItems(nsDisplayListBuilder* aBuilder, out.AppendToTop(item); } else if (isStackingContextItem) { // |item| is a stacking context item that can be reused. - aOutItems.AppendElement(item); ReuseStackingContextItem(aBuilder, item); } else { // |item| is inside a container item that will be destroyed later. @@ -1622,27 +1620,6 @@ void CollectStackingContextItems(nsDisplayListBuilder* aBuilder, aList->RestoreState(); } -/** - * Destroys the retained stacking context items that have not been reused and - * clears the array. - */ -void ClearPreviousItems(nsDisplayListBuilder* aBuilder, - nsTArray& aItems) { - const size_t total = aItems.Length(); - size_t reused = 0; - for (auto* item : aItems) { - if (item->IsReusedItem()) { - reused++; - item->SetReusable(); - } else { - item->Destroy(aBuilder); - } - } - - DL_LOGI("RDL - Reused %zu of %zu SC display items", reused, total); - aItems.Clear(); -} - } // namespace RDL bool RetainedDisplayListBuilder::TrySimpleUpdate( @@ -1652,10 +1629,8 @@ bool RetainedDisplayListBuilder::TrySimpleUpdate( return false; } - MOZ_ASSERT(mPreviousItems.IsEmpty()); RDL::MarkAllAncestorFrames(aModifiedFrames, aOutFramesWithProps); - RDL::CollectStackingContextItems(&mBuilder, &mList, mPreviousItems, - RootReferenceFrame()); + RDL::CollectStackingContextItems(&mBuilder, &mList, RootReferenceFrame()); return true; } @@ -1746,7 +1721,7 @@ PartialUpdateResult RetainedDisplayListBuilder::AttemptPartialUpdate( if (mBuilder.PartialBuildFailed()) { DL_LOGI("RDL - Partial update failed!"); mBuilder.LeavePresShell(RootReferenceFrame(), nullptr); - RDL::ClearPreviousItems(&mBuilder, mPreviousItems); + mBuilder.ClearReuseableDisplayItems(); mList.DeleteAll(&mBuilder); modifiedDL.DeleteAll(&mBuilder); Metrics()->mPartialUpdateFailReason = PartialUpdateFailReason::Content; @@ -1775,7 +1750,7 @@ PartialUpdateResult RetainedDisplayListBuilder::AttemptPartialUpdate( } else { MOZ_ASSERT(mList.IsEmpty()); mList = std::move(modifiedDL); - RDL::ClearPreviousItems(&mBuilder, mPreviousItems); + mBuilder.ClearReuseableDisplayItems(); result = PartialUpdateResult::Updated; } diff --git a/layout/painting/RetainedDisplayListBuilder.h b/layout/painting/RetainedDisplayListBuilder.h index 64f49056bff6..d949e42e02e0 100644 --- a/layout/painting/RetainedDisplayListBuilder.h +++ b/layout/painting/RetainedDisplayListBuilder.h @@ -195,6 +195,9 @@ struct RetainedDisplayListBuilder { * the frames in the modified frame lists. */ void ClearFramesWithProps(); + + void ClearReuseableDisplayItems() { mBuilder.ClearReuseableDisplayItems(); } + void AddSizeOfIncludingThis(nsWindowSizes&) const; NS_DECLARE_FRAME_PROPERTY_DELETABLE(Cached, RetainedDisplayListBuilder) @@ -272,9 +275,6 @@ struct RetainedDisplayListBuilder { RetainedDisplayList mList; WeakFrame mPreviousCaret; RetainedDisplayListMetrics mMetrics; - - // Stores reusable items collected during display list preprocessing. - nsTArray mPreviousItems; }; namespace RDLUtils { diff --git a/layout/painting/nsDisplayList.cpp b/layout/painting/nsDisplayList.cpp index 08b9758f080c..712d4b36c3c7 100644 --- a/layout/painting/nsDisplayList.cpp +++ b/layout/painting/nsDisplayList.cpp @@ -2008,6 +2008,32 @@ void nsDisplayListBuilder::BuildCompositorHitTestInfoIfNeeded( } } +void nsDisplayListBuilder::AddReusableDisplayItem(nsDisplayItem* aItem) { + mReuseableItems.Insert(aItem); +} + +void nsDisplayListBuilder::RemoveReusedDisplayItem(nsDisplayItem* aItem) { + MOZ_ASSERT(aItem->IsReusedItem()); + mReuseableItems.Remove(aItem); +} + +void nsDisplayListBuilder::ClearReuseableDisplayItems() { + const size_t total = mReuseableItems.Count(); + + size_t reused = 0; + for (auto* item : mReuseableItems) { + if (item->IsReusedItem()) { + reused++; + item->SetReusable(); + } else { + item->Destroy(this); + } + } + + DL_LOGI("RDL - Reused %zu of %zu SC display items", reused, total); + mReuseableItems.Clear(); +} + void nsDisplayListBuilder::ReuseDisplayItem(nsDisplayItem* aItem) { const auto* previous = mCurrentContainerASR; const auto* asr = aItem->GetActiveScrolledRoot(); diff --git a/layout/painting/nsDisplayList.h b/layout/painting/nsDisplayList.h index 12eb82dce6ec..0ea3f1aa4be4 100644 --- a/layout/painting/nsDisplayList.h +++ b/layout/painting/nsDisplayList.h @@ -1699,6 +1699,24 @@ class nsDisplayListBuilder { return mIsReusingStackingContextItems; } + /** + * Adds display item |aItem| to the reuseable display items set. + */ + void AddReusableDisplayItem(nsDisplayItem* aItem); + + /** + * Removes display item |aItem| from the reuseable display items set. + * This is needed because display items are sometimes deleted during + * display list building. + * Called by |nsDisplayItem::Destroy()| when the item has been reused. + */ + void RemoveReusedDisplayItem(nsDisplayItem* aItem); + + /** + * Clears the reuseable display items set. + */ + void ClearReuseableDisplayItems(); + /** * Marks the given display item |aItem| as reused, and updates the necessary * display list builder state. @@ -1900,6 +1918,9 @@ class nsDisplayListBuilder { bool mIsForContent; bool mIsReusingStackingContextItems; + + // Stores reusable items collected during display list preprocessing. + nsTHashSet mReuseableItems; }; class nsDisplayItem; @@ -2094,6 +2115,11 @@ class nsDisplayItem : public nsDisplayItemLink { if (aBuilder->IsForPainting() && aBuilder->IsForContent()) { DL_LOGV("Destroying display item %p (%s)", this, Name()); } + + if (IsReusedItem()) { + aBuilder->RemoveReusedDisplayItem(this); + } + this->~nsDisplayItem(); aBuilder->Destroy(type, this); }