From a2f551eb2dda6ca35262d1c4a3b3f8d27f9ffdb5 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Wed, 1 Oct 2025 08:50:54 +0200 Subject: [PATCH] layout: Clone static position rectangles when caching in `IndependentFormattingContext` (#39591) Hoisted absolutes that are cached in IndependentFormattingContexts may have their HoistedSharedFragment reused between layouts. This means that the original value of the adjusted static positioning rectangles need to be preserved in cache. This change makes it so that these values are cloned when being retrieved from the cache. Further adjustments up the tree will now no longer affected the version in teh cache. Testing: This should cause `/_mozilla/css/stacked_layers.html` to no longer be flaky. Fixes: #39548. Fixes: #39439. Signed-off-by: Martin Robinson Co-authored-by: Oriol Brufau --- components/layout/flow/mod.rs | 2 +- .../fragment_tree/hoisted_shared_fragment.rs | 47 ++++---------- components/layout/positioned.rs | 63 +++++++++++++------ 3 files changed, 56 insertions(+), 56 deletions(-) diff --git a/components/layout/flow/mod.rs b/components/layout/flow/mod.rs index 8bc775f79ec..bec8fb63f9c 100644 --- a/components/layout/flow/mod.rs +++ b/components/layout/flow/mod.rs @@ -2153,7 +2153,7 @@ impl<'container> PlacementState<'container> { Fragment::AbsoluteOrFixedPositioned(fragment) => { // The alignment of absolutes in block flow layout is always "start", so the size of // the static position rectangle does not matter. - fragment.borrow_mut().static_position_rect = LogicalRect { + fragment.borrow_mut().original_static_position_rect = LogicalRect { start_corner: LogicalVec2 { block: (self.current_margin.solve() + self.current_block_direction_position), diff --git a/components/layout/fragment_tree/hoisted_shared_fragment.rs b/components/layout/fragment_tree/hoisted_shared_fragment.rs index fa3862058da..adff53b1708 100644 --- a/components/layout/fragment_tree/hoisted_shared_fragment.rs +++ b/components/layout/fragment_tree/hoisted_shared_fragment.rs @@ -4,54 +4,31 @@ use app_units::Au; use malloc_size_of_derive::MallocSizeOf; -use style::logical_geometry::WritingMode; -use style::values::specified::align::AlignFlags; use super::Fragment; -use crate::geom::{LogicalVec2, PhysicalRect, PhysicalVec}; +use crate::geom::PhysicalRect; /// A reference to a Fragment which is shared between `HoistedAbsolutelyPositionedBox` /// and its placeholder `AbsoluteOrFixedPositionedFragment` in the original tree position. /// This will be used later in order to paint this hoisted box in tree order. -#[derive(MallocSizeOf)] +#[derive(Default, MallocSizeOf)] pub(crate) struct HoistedSharedFragment { pub fragment: Option, - /// The "static-position rect" of this absolutely positioned box. This is defined by the - /// layout mode from which the box originates. + /// The original "static-position rect" of this absolutely positioned box. This is + /// defined by the layout mode from which the box originates. As this fragment is + /// hoisted up the tree this rectangle will be adjusted by the offsets of all + /// ancestors between the tree position of the absolute and the containing block for + /// absolutes that it is laid out in. /// /// See - pub static_position_rect: PhysicalRect, - /// The resolved alignment values used for aligning this absolutely positioned element - /// if the "static-position rect" ends up being the "inset-modified containing block". - /// These values are dependent on the layout mode (currently only interesting for - /// flexbox). - pub resolved_alignment: LogicalVec2, - /// This is the [`WritingMode`] of the original parent of the element that created this - /// hoisted absolutely-positioned fragment. This helps to interpret the offset for - /// static positioning. If the writing mode is right-to-left or bottom-to-top, the static - /// offset needs to be adjusted by the absolutely positioned element's inline size. - pub original_parent_writing_mode: WritingMode, + pub original_static_position_rect: PhysicalRect, } impl HoistedSharedFragment { - pub(crate) fn new( - static_position_rect: PhysicalRect, - resolved_alignment: LogicalVec2, - original_parent_writing_mode: WritingMode, - ) -> Self { - HoistedSharedFragment { - fragment: None, - static_position_rect, - resolved_alignment, - original_parent_writing_mode, + pub(crate) fn new(original_static_position_rect: PhysicalRect) -> Self { + Self { + fragment: Default::default(), + original_static_position_rect, } } } - -impl HoistedSharedFragment { - /// `inset: auto`-positioned elements do not know their precise position until after - /// they're hoisted. This lets us adjust auto values after the fact. - pub(crate) fn adjust_offsets(&mut self, offset: &PhysicalVec) { - self.static_position_rect = self.static_position_rect.translate(*offset); - } -} diff --git a/components/layout/positioned.rs b/components/layout/positioned.rs index 4869910773c..03eb8cdaf2c 100644 --- a/components/layout/positioned.rs +++ b/components/layout/positioned.rs @@ -40,11 +40,30 @@ pub(crate) struct AbsolutelyPositionedBox { #[derive(Clone, MallocSizeOf)] pub(crate) struct HoistedAbsolutelyPositionedBox { absolutely_positioned_box: ArcRefCell, - /// A reference to a Fragment which is shared between this `HoistedAbsolutelyPositionedBox` /// and its placeholder `AbsoluteOrFixedPositionedFragment` in the original tree position. /// This will be used later in order to paint this hoisted box in tree order. pub fragment: ArcRefCell, + /// The adjusted "static-position rect" of this absolutely positioned box. This is + /// defined by the layout mode from which the box originates. This is the + /// [`HoistedSharedFragment::original_static_position_rect`] adjusted by the offests + /// of ancestors between the tree position of the absolute and the + /// [`PostioningContext`] that holds this [`HoistedAbsolutelyPositionedBox`]. + /// + /// If the value is `None`, the original static position rect has not been adjusted yet. + /// + /// See + pub adjusted_static_position_rect: Option>, + /// The resolved alignment values used for aligning this absolutely positioned element + /// if the "static-position rect" ends up being the "inset-modified containing block". + /// These values are dependent on the layout mode (currently only interesting for + /// flexbox). + pub resolved_alignment: LogicalVec2, + /// This is the [`WritingMode`] of the original parent of the element that created this + /// hoisted absolutely-positioned fragment. This helps to interpret the offset for + /// static positioning. If the writing mode is right-to-left or bottom-to-top, the static + /// offset needs to be adjusted by the absolutely positioned element's inline size. + pub original_parent_writing_mode: WritingMode, } impl AbsolutelyPositionedBox { @@ -74,16 +93,15 @@ impl AbsolutelyPositionedBox { pub(crate) fn to_hoisted( absolutely_positioned_box: ArcRefCell, - static_position_rectangle: PhysicalRect, + static_position_rect: PhysicalRect, resolved_alignment: LogicalVec2, original_parent_writing_mode: WritingMode, ) -> HoistedAbsolutelyPositionedBox { HoistedAbsolutelyPositionedBox { - fragment: ArcRefCell::new(HoistedSharedFragment::new( - static_position_rectangle, - resolved_alignment, - original_parent_writing_mode, - )), + fragment: ArcRefCell::new(HoistedSharedFragment::new(static_position_rect)), + adjusted_static_position_rect: None, + resolved_alignment, + original_parent_writing_mode, absolutely_positioned_box, } } @@ -154,11 +172,8 @@ impl PositioningContext { self.absolutes .iter_mut() .skip(index.0) - .for_each(|hoisted_fragment| { - hoisted_fragment - .fragment - .borrow_mut() - .adjust_offsets(offset) + .for_each(|hoisted_box| { + hoisted_box.adjust_static_position_with_offset(offset); }) } @@ -447,13 +462,12 @@ impl HoistedAbsolutelyPositionedBox { let is_table = layout_style.is_table(); let is_table_or_replaced = is_table || context.is_replaced(); let preferred_aspect_ratio = context.preferred_aspect_ratio(&pbm.padding_border_sums); - let shared_fragment = self.fragment.borrow(); // The static position rect was calculated assuming that the containing block would be // established by the content box of some ancestor, but the actual containing block is // established by the padding box. So we need to add the padding of that ancestor. - let mut static_position_rect = shared_fragment - .static_position_rect + let mut static_position_rect = self + .static_position_rect() .outer_rect(-containing_block_padding); static_position_rect.size = static_position_rect.size.max(PhysicalSize::zero()); let static_position_rect = static_position_rect.to_logical(containing_block); @@ -465,7 +479,7 @@ impl HoistedAbsolutelyPositionedBox { let inline_box_offsets = box_offset.inline_sides(); let inline_alignment = match inline_box_offsets.either_specified() { true => style.clone_justify_self().0.0, - false => shared_fragment.resolved_alignment.inline, + false => self.resolved_alignment.inline, }; let inline_axis_solver = AbsoluteAxisSolver { @@ -479,7 +493,7 @@ impl HoistedAbsolutelyPositionedBox { box_offsets: inline_box_offsets, static_position_rect_axis: static_position_rect.get_axis(Direction::Inline), alignment: inline_alignment, - flip_anchor: shared_fragment.original_parent_writing_mode.is_bidi_ltr() != + flip_anchor: self.original_parent_writing_mode.is_bidi_ltr() != containing_block_writing_mode.is_bidi_ltr(), is_table_or_replaced, }; @@ -489,7 +503,7 @@ impl HoistedAbsolutelyPositionedBox { let block_box_offsets = box_offset.block_sides(); let block_alignment = match block_box_offsets.either_specified() { true => style.clone_align_self().0.0, - false => shared_fragment.resolved_alignment.block, + false => self.resolved_alignment.block, }; let block_axis_solver = AbsoluteAxisSolver { axis: Direction::Block, @@ -607,13 +621,13 @@ impl HoistedAbsolutelyPositionedBox { let inline_origin = inline_axis_solver.origin_for_margin_box( margin_rect_size.inline, style.writing_mode, - shared_fragment.original_parent_writing_mode, + self.original_parent_writing_mode, containing_block_writing_mode, ); let block_origin = block_axis_solver.origin_for_margin_box( margin_rect_size.block, style.writing_mode, - shared_fragment.original_parent_writing_mode, + self.original_parent_writing_mode, containing_block_writing_mode, ); @@ -656,6 +670,15 @@ impl HoistedAbsolutelyPositionedBox { context.base.set_fragment(fragment.clone()); fragment } + + fn static_position_rect(&self) -> PhysicalRect { + self.adjusted_static_position_rect + .unwrap_or_else(|| self.fragment.borrow().original_static_position_rect) + } + + fn adjust_static_position_with_offset(&mut self, offset: &PhysicalVec) { + self.adjusted_static_position_rect = Some(self.static_position_rect().translate(*offset)); + } } #[derive(Clone, Copy, Debug)]