From bc8429b28b30d1eb4c8ac9a9ba99d4725aeb256a Mon Sep 17 00:00:00 2001 From: Timothy Nikkel Date: Fri, 28 Aug 2026 10:19:18 +0000 Subject: [PATCH] Bug 2067338. Misc cleanups to preserve3d wrapping display list code. r=layout-reviewers,emilio Follow up to review comments in bug 2066084. WrapSeparatorTransform didn't convey that it only wraps the non-participants accumulated so far, and that it does nothing when there are none. Rename it to FlushNonParticipantsIntoSeparatorTransform and say the rest in a comment. The index was passed by value and every caller incremented it, so an index was used up each time there was nothing to wrap. Pass it by reference and consume it only when an item is actually created. Differential Revision: https://phabricator.services.mozilla.com/D322031 --- layout/generic/nsIFrame.cpp | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/layout/generic/nsIFrame.cpp b/layout/generic/nsIFrame.cpp index 9491a5a86851..4291864bfa9d 100644 --- a/layout/generic/nsIFrame.cpp +++ b/layout/generic/nsIFrame.cpp @@ -3064,17 +3064,23 @@ static nsIFrame* BackfaceHidden3DParticipantFor(nsIFrame* aAncestor, return nullptr; } -static void WrapSeparatorTransform(nsDisplayListBuilder* aBuilder, - nsIFrame* aFrame, - nsDisplayList* aNonParticipants, - nsDisplayList* aParticipants, int aIndex, - nsDisplayItem** aSeparator) { +/** + * Flushes the non-participants accumulated so far into a separator transform + * item on aFrame, and moves that item over to aParticipants. It does not touch + * anything already in aParticipants, and it is a no-op when nothing has + * accumulated. This should be called whenever there is a switch between + * the participants and non-participants. + */ +static void FlushNonParticipantsIntoSeparatorTransform( + nsDisplayListBuilder* aBuilder, nsIFrame* aFrame, + nsDisplayList* aNonParticipants, nsDisplayList* aParticipants, int& aIndex, + nsDisplayItem** aSeparator) { if (aNonParticipants->IsEmpty()) { return; } nsDisplayTransform* item = MakeDisplayItemWithIndex( - aBuilder, aFrame, aIndex, aNonParticipants, aBuilder->GetVisibleRect()); + aBuilder, aFrame, aIndex++, aNonParticipants, aBuilder->GetVisibleRect()); if (*aSeparator == nullptr && item) { *aSeparator = item; @@ -3920,8 +3926,9 @@ void nsIFrame::BuildDisplayListForStackingContext( if (ItemParticipatesIn3DContext(this, item) && !item->GetClip().HasClip()) { // The frame of this item participates the same 3D context. - WrapSeparatorTransform(aBuilder, this, &nonparticipants, - &participants, index++, &separator); + FlushNonParticipantsIntoSeparatorTransform( + aBuilder, this, &nonparticipants, &participants, index, + &separator); participants.AppendToTop(item); } else if (nsIFrame* backfaceHidden = @@ -3930,8 +3937,9 @@ void nsIFrame::BuildDisplayListForStackingContext( // leaf keyed on that participant rather than adding it to the shared // separator below, which is keyed on us and so would be culled only // when our own backface is turned away. - WrapSeparatorTransform(aBuilder, this, &nonparticipants, - &participants, index++, &separator); + FlushNonParticipantsIntoSeparatorTransform( + aBuilder, this, &nonparticipants, &participants, index, + &separator); nsDisplayList itemList(aBuilder); itemList.AppendToTop(item); @@ -3949,8 +3957,8 @@ void nsIFrame::BuildDisplayListForStackingContext( nonparticipants.AppendToTop(item); } } - WrapSeparatorTransform(aBuilder, this, &nonparticipants, &participants, - index++, &separator); + FlushNonParticipantsIntoSeparatorTransform( + aBuilder, this, &nonparticipants, &participants, index, &separator); if (separator) { createdContainer = true;