Use a RwLock to cache inline_content_sizes() (#34232)

In order to support size keywords in block layout, we may need to call
`inline_content_sizes()` in order to compute the min/max-content sizes.
But this required a mutable reference in order the update the cache,
and in various places we already had mutable references.

So this switches the cache into a RwLock to avoid needing mutable refs.
Note OnceCell wouldn't work because it's not thread-safe.

Signed-off-by: Oriol Brufau <obrufau@igalia.com>
This commit is contained in:
Oriol Brufau 2024-11-13 10:56:02 +01:00 committed by GitHub
parent c00804190c
commit 9102644470
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 76 additions and 74 deletions

View file

@ -171,7 +171,7 @@ where
let non_replaced = NonReplacedFormattingContext {
base_fragment_info: info.into(),
style: info.style.clone(),
content_sizes_result: None,
content_sizes_result: Default::default(),
contents: NonReplacedFormattingContextContents::Flow(
block_formatting_context,
),

View file

@ -6,10 +6,10 @@ use std::cell::Cell;
use std::cmp::Ordering;
use app_units::Au;
use atomic_refcell::AtomicRefMut;
use atomic_refcell::AtomicRef;
use itertools::izip;
use rayon::iter::{
IndexedParallelIterator, IntoParallelRefMutIterator, ParallelDrainRange, ParallelIterator,
IndexedParallelIterator, IntoParallelRefIterator, ParallelDrainRange, ParallelIterator,
};
use style::computed_values::position::T as Position;
use style::logical_geometry::Direction;
@ -56,7 +56,7 @@ struct FlexContext<'a> {
/// A flex item with some intermediate results
struct FlexItem<'a> {
box_: &'a mut FlexItemBox,
box_: &'a FlexItemBox,
content_box_size: FlexRelativeVec2<AuOrAuto>,
content_min_size: FlexRelativeVec2<Au>,
content_max_size: FlexRelativeVec2<Option<Au>>,
@ -399,7 +399,7 @@ impl FlexContainer {
)
)]
pub fn inline_content_sizes(
&mut self,
&self,
layout_context: &LayoutContext,
constraint_space: &ConstraintSpace,
) -> InlineContentSizesResult {
@ -416,7 +416,7 @@ impl FlexContainer {
}
fn cross_content_sizes(
&mut self,
&self,
layout_context: &LayoutContext,
containing_block_for_children: &IndefiniteContainingBlock,
) -> InlineContentSizesResult {
@ -429,13 +429,13 @@ impl FlexContainer {
let mut sizes = ContentSizes::zero();
let mut depends_on_block_constraints = false;
for kid in self.children.iter() {
let kid = &mut *kid.borrow_mut();
let kid = &*kid.borrow();
match kid {
FlexLevelBox::FlexItem(item) => {
// TODO: For the max-content size we should distribute items into
// columns, and sum the column sizes and gaps.
// TODO: Use the proper automatic minimum size.
let ifc = &mut item.independent_formatting_context;
let ifc = &item.independent_formatting_context;
let result = ifc.outer_inline_content_sizes(
layout_context,
containing_block_for_children,
@ -476,7 +476,7 @@ impl FlexContainer {
let container_is_horizontal = self.style.writing_mode.is_horizontal();
for kid in self.children.iter() {
let kid = &mut *kid.borrow_mut();
let kid = &*kid.borrow();
match kid {
FlexLevelBox::FlexItem(item) => {
sum_of_flex_grow_factors += item.style().get_position().flex_grow.0;
@ -664,13 +664,13 @@ impl FlexContainer {
.children
.iter()
.map(|arcrefcell| {
let borrowed = arcrefcell.borrow_mut();
let borrowed = arcrefcell.borrow();
match &*borrowed {
FlexLevelBox::OutOfFlowAbsolutelyPositionedBox(absolutely_positioned) => {
FlexContent::AbsolutelyPositionedBox(absolutely_positioned.clone())
},
FlexLevelBox::FlexItem(_) => {
let item = AtomicRefMut::map(borrowed, |child| match child {
let item = AtomicRef::map(borrowed, |child| match child {
FlexLevelBox::FlexItem(item) => item,
_ => unreachable!(),
});
@ -681,7 +681,7 @@ impl FlexContainer {
})
.collect::<Vec<_>>();
let flex_item_boxes = flex_items.iter_mut().map(|child| &mut **child);
let flex_item_boxes = flex_items.iter().map(|child| &**child);
let flex_items = flex_item_boxes
.map(|flex_item_box| FlexItem::new(&flex_context, flex_item_box))
.collect::<Vec<_>>();
@ -1088,7 +1088,7 @@ fn allocate_free_cross_space_for_flex_line(
}
impl<'a> FlexItem<'a> {
fn new(flex_context: &FlexContext, box_: &'a mut FlexItemBox) -> Self {
fn new(flex_context: &FlexContext, box_: &'a FlexItemBox) -> Self {
let containing_block = flex_context.containing_block;
let parent_writing_mode = containing_block.style.writing_mode;
let item_writing_mode = box_.style().writing_mode;
@ -1321,7 +1321,7 @@ struct InitialFlexLineLayout<'a> {
impl InitialFlexLineLayout<'_> {
fn new<'items>(
flex_context: &FlexContext,
mut items: Vec<FlexItem<'items>>,
items: Vec<FlexItem<'items>>,
outer_hypothetical_main_sizes_sum: Au,
container_main_size: Au,
main_gap: Au,
@ -1335,7 +1335,7 @@ impl InitialFlexLineLayout<'_> {
// https://drafts.csswg.org/css-flexbox/#algo-cross-item
let layout_results = items
.par_iter_mut()
.par_iter()
.zip(&item_used_main_sizes)
.map(|(item, used_main_size)| {
item.layout(*used_main_size, flex_context, None, None)
@ -1833,7 +1833,7 @@ impl FlexItem<'_> {
/// > size and the given available space, treating `auto` as `fit-content`.
#[allow(clippy::too_many_arguments)]
fn layout(
&mut self,
&self,
used_main_size: Au,
flex_context: &FlexContext,
used_cross_size_override: Option<Au>,
@ -2205,7 +2205,7 @@ impl FlexItem<'_> {
impl FlexItemBox {
fn main_content_size_info<'a>(
&mut self,
&self,
layout_context: &LayoutContext,
containing_block: &IndefiniteContainingBlock,
container_is_horizontal: bool,
@ -2214,7 +2214,7 @@ impl FlexItemBox {
) -> FlexItemBoxInlineContentSizesInfo {
let flex_axis = config.flex_axis;
let main_start_cross_start = config.main_start_cross_start_sides_are;
let style = self.style().clone();
let style = &self.style();
let item_writing_mode = style.writing_mode;
let item_is_horizontal = item_writing_mode.is_horizontal();
let cross_axis_is_item_block_axis =
@ -2241,7 +2241,7 @@ impl FlexItemBox {
cross: padding_border.cross,
} + margin_auto_is_zero.sum_by_axis();
let item_with_auto_cross_size_stretches_to_container_size =
config.item_with_auto_cross_size_stretches_to_container_size(&style, &margin);
config.item_with_auto_cross_size_stretches_to_container_size(style, &margin);
let automatic_min_size = self.automatic_min_size(
layout_context,
containing_block,
@ -2274,7 +2274,7 @@ impl FlexItemBox {
block: content_min_box_size.block.auto_is(|| automatic_min_size),
}
};
let block_content_size_callback = |item: &mut FlexItemBox| {
let block_content_size_callback = |item: &FlexItemBox| {
item.layout_for_block_content_size(
flex_context_getter(),
&pbm,
@ -2434,7 +2434,7 @@ impl FlexItemBox {
/// This is an implementation of <https://drafts.csswg.org/css-flexbox/#min-size-auto>.
#[allow(clippy::too_many_arguments)]
fn automatic_min_size(
&mut self,
&self,
layout_context: &LayoutContext,
containing_block: &IndefiniteContainingBlock,
cross_axis_is_item_block_axis: bool,
@ -2443,14 +2443,11 @@ impl FlexItemBox {
max_size: FlexRelativeVec2<Option<Au>>,
pbm_auto_is_zero: &FlexRelativeVec2<Au>,
auto_cross_size_stretches_to_container_size: bool,
block_content_size_callback: impl FnOnce(&mut FlexItemBox) -> Au,
block_content_size_callback: impl FnOnce(&FlexItemBox) -> Au,
) -> Au {
// FIXME(stshine): Consider more situations when auto min size is not needed.
if self
.independent_formatting_context
.style()
.establishes_scroll_container()
{
let style = &self.independent_formatting_context.style();
if style.establishes_scroll_container() {
return Au::zero();
}
@ -2503,7 +2500,7 @@ impl FlexItemBox {
// > preferred aspect ratio, by any definite minimum and maximum cross sizes converted through the
// > aspect ratio.
let main_content_size = if cross_axis_is_item_block_axis {
let writing_mode = self.independent_formatting_context.style().writing_mode;
let writing_mode = style.writing_mode;
let constraint_space = ConstraintSpace::new(cross_size, writing_mode);
self.independent_formatting_context
.inline_content_sizes(layout_context, &constraint_space, containing_block)
@ -2540,7 +2537,7 @@ impl FlexItemBox {
/// <https://drafts.csswg.org/css-flexbox/#algo-main-item>
#[allow(clippy::too_many_arguments)]
fn flex_base_size(
&mut self,
&self,
layout_context: &LayoutContext,
containing_block: &IndefiniteContainingBlock,
container_definite_inner_size: FlexRelativeVec2<Option<Au>>,
@ -2551,9 +2548,9 @@ impl FlexItemBox {
padding_border_sums: FlexRelativeVec2<Au>,
pbm_auto_is_zero: &FlexRelativeVec2<Au>,
item_with_auto_cross_size_stretches_to_container_size: bool,
block_content_size_callback: impl FnOnce(&mut FlexItemBox) -> Au,
block_content_size_callback: impl FnOnce(&FlexItemBox) -> Au,
) -> (Au, bool) {
let flex_item = &mut self.independent_formatting_context;
let flex_item = &self.independent_formatting_context;
let style = flex_item.style();
let used_flex_basis = match &style.get_position().flex_basis {
@ -2655,7 +2652,7 @@ impl FlexItemBox {
let flex_basis = if cross_axis_is_item_block_axis {
// The main axis is the inline axis, so we can get the content size from the normal
// preferred widths calculation.
let writing_mode = flex_item.style().writing_mode;
let writing_mode = style.writing_mode;
let constraint_space = ConstraintSpace::new(
SizeConstraint::new(
content_box_size.cross.non_auto(),
@ -2696,7 +2693,7 @@ impl FlexItemBox {
)
)]
fn layout_for_block_content_size(
&mut self,
&self,
flex_context: &FlexContext,
padding_border_margin: &PaddingBorderMargin,
mut content_box_size: LogicalVec2<AuOrAuto>,
@ -2711,7 +2708,7 @@ impl FlexItemBox {
.collects_for_nearest_positioned_ancestor(),
);
match &mut self.independent_formatting_context {
match &self.independent_formatting_context {
IndependentFormattingContext::Replaced(replaced) => {
content_box_size.inline = content_box_size.inline.map(|v| v.max(Au::zero()));
if intrinsic_sizing_mode == IntrinsicSizingMode::Size {

View file

@ -891,7 +891,7 @@ impl FloatBox {
/// the float containing block formatting context. A later step adjusts the position
/// to be relative to the containing block.
pub fn layout(
&mut self,
&self,
layout_context: &LayoutContext,
positioning_context: &mut PositioningContext,
containing_block: &ContainingBlock,

View file

@ -1656,7 +1656,7 @@ impl InlineFormattingContext {
}
for item in self.inline_items.iter() {
let item = &mut *item.borrow_mut();
let item = &*item.borrow();
// Any new box should flush a pending hard line break.
if !matches!(item, InlineItem::EndInlineBox) {
@ -1684,7 +1684,7 @@ impl InlineFormattingContext {
},
));
},
InlineItem::OutOfFlowFloatBox(ref mut float_box) => {
InlineItem::OutOfFlowFloatBox(ref float_box) => {
float_box.layout_into_line_items(&mut layout);
},
}
@ -1915,7 +1915,7 @@ impl InlineContainerState {
impl IndependentFormattingContext {
fn layout_into_line_items(
&mut self,
&self,
layout: &mut InlineFormattingContextLayout,
offset_in_text: usize,
bidi_level: Level,
@ -2071,7 +2071,7 @@ impl IndependentFormattingContext {
}
impl FloatBox {
fn layout_into_line_items(&mut self, layout: &mut InlineFormattingContextLayout) {
fn layout_into_line_items(&self, layout: &mut InlineFormattingContextLayout) {
let fragment = self.layout(
layout.layout_context,
layout.positioning_context,
@ -2211,7 +2211,7 @@ impl<'layout_data> ContentSizesComputation<'layout_data> {
inline_formatting_context: &InlineFormattingContext,
) -> InlineContentSizesResult {
for inline_item in inline_formatting_context.inline_items.iter() {
self.process_item(&mut inline_item.borrow_mut(), inline_formatting_context);
self.process_item(&inline_item.borrow(), inline_formatting_context);
}
self.forced_line_break();
@ -2223,7 +2223,7 @@ impl<'layout_data> ContentSizesComputation<'layout_data> {
fn process_item(
&mut self,
inline_item: &mut InlineItem,
inline_item: &InlineItem,
inline_formatting_context: &InlineFormattingContext,
) {
match inline_item {

View file

@ -370,10 +370,10 @@ fn calculate_inline_content_size_for_block_level_boxes(
containing_block: &IndefiniteContainingBlock,
) -> InlineContentSizesResult {
let get_box_info = |box_: &ArcRefCell<BlockLevelBox>| {
match &mut *box_.borrow_mut() {
match &*box_.borrow() {
BlockLevelBox::OutOfFlowAbsolutelyPositionedBox(_) |
BlockLevelBox::OutsideMarker { .. } => None,
BlockLevelBox::OutOfFlowFloatBox(ref mut float_box) => {
BlockLevelBox::OutOfFlowFloatBox(ref float_box) => {
let inline_content_sizes_result = float_box.contents.outer_inline_content_sizes(
layout_context,
containing_block,
@ -404,7 +404,7 @@ fn calculate_inline_content_size_for_block_level_boxes(
// Instead, we treat it like an independent block with 'clear: both'.
Some((inline_content_sizes_result, Float::None, Clear::Both))
},
BlockLevelBox::Independent(ref mut independent) => {
BlockLevelBox::Independent(ref independent) => {
let inline_content_sizes_result = independent.outer_inline_content_sizes(
layout_context,
containing_block,
@ -606,7 +606,7 @@ fn layout_block_level_children_in_parallel(
.map(|child_box| {
let mut child_positioning_context =
PositioningContext::new_for_subtree(collects_for_nearest_positioned_ancestor);
let fragment = child_box.borrow_mut().layout(
let fragment = child_box.borrow().layout(
layout_context,
&mut child_positioning_context,
containing_block,
@ -646,7 +646,7 @@ fn layout_block_level_children_sequentially(
.iter()
.map(|child_box| {
let positioning_context_length_before_layout = positioning_context.len();
let mut fragment = child_box.borrow_mut().layout(
let mut fragment = child_box.borrow().layout(
layout_context,
positioning_context,
containing_block,
@ -670,7 +670,7 @@ fn layout_block_level_children_sequentially(
impl BlockLevelBox {
fn layout(
&mut self,
&self,
layout_context: &LayoutContext,
positioning_context: &mut PositioningContext,
containing_block: &ContainingBlock,
@ -1997,7 +1997,7 @@ fn block_size_is_zero_or_intrinsic(size: &StyleSize, containing_block: &Containi
impl IndependentFormattingContext {
pub(crate) fn layout_float_or_atomic_inline(
&mut self,
&self,
layout_context: &LayoutContext,
child_positioning_context: &mut PositioningContext,
containing_block: &ContainingBlock,

View file

@ -2,6 +2,8 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
use std::sync::RwLock;
use app_units::Au;
use serde::Serialize;
use servo_arc::Arc;
@ -38,7 +40,7 @@ pub(crate) struct NonReplacedFormattingContext {
#[serde(skip_serializing)]
pub style: Arc<ComputedValues>,
/// If it was requested during construction
pub content_sizes_result: Option<(SizeConstraint, InlineContentSizesResult)>,
pub content_sizes_result: RwLock<Option<(SizeConstraint, InlineContentSizesResult)>>,
pub contents: NonReplacedFormattingContextContents,
}
@ -157,7 +159,7 @@ impl IndependentFormattingContext {
Self::NonReplaced(NonReplacedFormattingContext {
style: Arc::clone(&node_and_style_info.style),
base_fragment_info,
content_sizes_result: None,
content_sizes_result: RwLock::default(),
contents,
})
},
@ -188,7 +190,7 @@ impl IndependentFormattingContext {
}
pub(crate) fn inline_content_sizes(
&mut self,
&self,
layout_context: &LayoutContext,
constraint_space: &ConstraintSpace,
containing_block: &IndefiniteContainingBlock,
@ -206,7 +208,7 @@ impl IndependentFormattingContext {
}
pub(crate) fn outer_inline_content_sizes(
&mut self,
&self,
layout_context: &LayoutContext,
containing_block: &IndefiniteContainingBlock,
auto_minimum: &LogicalVec2<Au>,
@ -276,30 +278,33 @@ impl NonReplacedFormattingContext {
}
pub(crate) fn inline_content_sizes(
&mut self,
&self,
layout_context: &LayoutContext,
constraint_space: &ConstraintSpace,
) -> InlineContentSizesResult {
if let Some((previous_cb_block_size, result)) = self.content_sizes_result {
if let Ok(Some((previous_cb_block_size, result))) =
self.content_sizes_result.read().as_deref()
{
if !result.depends_on_block_constraints ||
previous_cb_block_size == constraint_space.block_size
*previous_cb_block_size == constraint_space.block_size
{
return result;
return *result;
}
// TODO: Should we keep multiple caches for various block sizes?
}
self.content_sizes_result
.insert((
constraint_space.block_size,
self.contents
.inline_content_sizes(layout_context, constraint_space),
))
.1
let writer = self.content_sizes_result.write();
let result = self
.contents
.inline_content_sizes(layout_context, constraint_space);
if let Ok(mut cache) = writer {
*cache = Some((constraint_space.block_size, result));
}
result
}
pub(crate) fn outer_inline_content_sizes(
&mut self,
&self,
layout_context: &LayoutContext,
containing_block: &IndefiniteContainingBlock,
auto_minimum: &LogicalVec2<Au>,
@ -317,7 +322,7 @@ impl NonReplacedFormattingContext {
impl NonReplacedFormattingContextContents {
pub(crate) fn inline_content_sizes(
&mut self,
&self,
layout_context: &LayoutContext,
constraint_space: &ConstraintSpace,
) -> InlineContentSizesResult {

View file

@ -449,8 +449,8 @@ impl HoistedAbsolutelyPositionedBox {
let cbis = containing_block.size.inline;
let cbbs = containing_block.size.block;
let containing_block_writing_mode = containing_block.style.writing_mode;
let mut absolutely_positioned_box = self.absolutely_positioned_box.borrow_mut();
let context = &mut absolutely_positioned_box.context;
let absolutely_positioned_box = self.absolutely_positioned_box.borrow();
let context = &absolutely_positioned_box.context;
let style = context.style().clone();
let containing_block = &containing_block.into();
let pbm = style.padding_border_margin(containing_block);

View file

@ -137,7 +137,7 @@ impl Table {
IndependentFormattingContext::NonReplaced(NonReplacedFormattingContext {
base_fragment_info: (&anonymous_info).into(),
style: grid_and_wrapper_style,
content_sizes_result: None,
content_sizes_result: Default::default(),
contents: NonReplacedFormattingContextContents::Table(table),
})
}
@ -858,7 +858,7 @@ where
context: ArcRefCell::new(NonReplacedFormattingContext {
style: info.style.clone(),
base_fragment_info: info.into(),
content_sizes_result: None,
content_sizes_result: Default::default(),
contents,
}),
};

View file

@ -772,7 +772,7 @@ impl<'a> TableLayout<'a> {
}
/// Compute CAPMIN: <https://drafts.csswg.org/css-tables/#capmin>
fn compute_caption_minimum_inline_size(&mut self, layout_context: &LayoutContext) -> Au {
fn compute_caption_minimum_inline_size(&self, layout_context: &LayoutContext) -> Au {
let containing_block = IndefiniteContainingBlock {
size: LogicalVec2 {
inline: AuOrAuto::Auto,
@ -784,7 +784,7 @@ impl<'a> TableLayout<'a> {
.captions
.iter()
.map(|caption| {
let mut context = caption.context.borrow_mut();
let context = caption.context.borrow();
context
.outer_inline_content_sizes(
layout_context,
@ -1605,7 +1605,7 @@ impl<'a> TableLayout<'a> {
}
fn layout_caption(
&mut self,
&self,
caption: &TableCaption,
table_pbm: &PaddingBorderMargin,
layout_context: &LayoutContext,
@ -2186,7 +2186,7 @@ impl<'a> TableLayout<'a> {
}
fn make_fragments_for_columns_and_column_groups(
&mut self,
&self,
dimensions: &TableAndTrackDimensions,
fragments: &mut Vec<Fragment>,
) {
@ -2630,7 +2630,7 @@ impl Table {
)
)]
pub(crate) fn inline_content_sizes(
&mut self,
&self,
layout_context: &LayoutContext,
constraint_space: &ConstraintSpace,
) -> InlineContentSizesResult {