Closed Bug 1417601 Opened 2 years ago Closed 2 years ago
OOF placeholders might be skipped in nested stacking context when retaining a display list
This is an existing bug that a patch for bug 1415485 uncovered.
When a modified frame is within a stacking context, we try to create a display list with empty dirty rect to contain the changes to that stacking context. The empty dirty rect causes us to not mark OOF frames , unless we first call ForceDescendIntoIfVisible() on them. We do this in PreProcessDisplayList . Unfortunately this fails, if the placeholder for the OOF frame is inside another stacking context that is a descendant of the stacking context that the modified frame is in.  https://searchfox.org/mozilla-central/rev/bab833ebeef6b2202e71f81d225b968283521fd6/layout/painting/nsDisplayList.cpp#1183  https://searchfox.org/mozilla-central/rev/a662f122c37704456457a526af90db4e3c0fd10e/layout/painting/RetainedDisplayListBuilder.cpp#118
Comment on attachment 8929226 [details] Bug 1417601 - Override dirty rect for OOF frames with containing block outside a modified stacking context https://reviewboard.mozilla.org/r/200502/#review205648 ::: layout/painting/nsDisplayList.cpp:1120 (Diff revision 1) > return FindAnimatedGeometryRootFor(aItem->Frame()); > } > > +static bool > +AnyAncestorOrPlaceholderIsModified(nsIFrame* aFrame, > + nsIFrame* aStopAtFrame = nullptr) So, for DisplayListClipState we refer to 'ContainingBlockDescendants' and 'ContentDescendants' to distinguish between taking placeholder replacements into account or not. We could do something similar here, maybe AnyContentAncestorModified? ::: layout/painting/nsDisplayList.cpp:1193 (Diff revision 1) > + // have an empty dirty rect set. > + // If the nearest stacking context for the modified frame is an ancestor of > + // of it, and if the stacking context is a descendant of the containing block > + // of this OOF frame, we override the dirty rect to ensure that the frame will > + // get marked. > + if (hasEmptyDirtyRect && I think we always want to set dirty = visible if we have a modified ancestor, even if the dirty rect isn't empty. I think it might be possible to have an unrelated invalidation that gives a dirty rect that partial covers us, but one of our descendants would be excluded by it.
Attachment #8929226 - Flags: review?(matt.woodrow) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/1b2c62ece7a3 Override dirty rect for OOF frames with containing block outside a modified stacking context r=mattwoodrow
Comment on attachment 8929226 [details] Bug 1417601 - Override dirty rect for OOF frames with containing block outside a modified stacking context Approval Request Comment [Feature/Bug causing the regression]: bug 1352499. This is code that is preffed off, but we want to run a shield study enabling the pref. [User impact if declined]: None, preffed off code. [Is this code covered by automated tests?]: Yes, when the pref is enabled. [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: Code is preffed off. [String changes made/needed]: None
Attachment #8929226 - Flags: approval-mozilla-beta?
Comment on attachment 8929226 [details] Bug 1417601 - Override dirty rect for OOF frames with containing block outside a modified stacking context Take this to support the shield study in 58. Beta58+.
Attachment #8929226 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.