Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-06 06:00:15 +01:00
Code Issues Projects Releases Packages Wiki Activity

layout: Fix depends_on_block_constraints logic (#38318)

Browse source 
The logic was wrong, sometimes we weren't setting it to true on flex
containers that needed it, and then as a workaround we were setting it
to to true on flex items that didn't need it.

For example, this testcase had 5 cache misses when stretching the items,
now we will avoid laying them out again:
```html
<div style="display: flex">
  <div></div>  <div></div>  <div></div>  <div></div>  <div></div>
</div>
```

Also, the workaround wasn't always working, e.g. it failed to stretch
the green element here:
```html
<div style="display: flex; min-height: 200px">
  <div>
    <div style="display: flex; height: 100%; background-color: red">
      <div style="width: 200px; background-color: green;"></div>
    </div>
  </div>
</div>
```

Testing: Adding new test
Fixes: #38023

Signed-off-by: Oriol Brufau <obrufau@igalia.com>
This commit is contained in:
Oriol Brufau 2025-07-29 11:59:54 +02:00 • committed by GitHub
parent a5d4c49ec6
commit 3d2f0d1be5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 50 additions and 36 deletions
Download patch file Download diff file Expand all files Collapse all files

19
components/layout/flexbox/layout.rs
View file

@ -627,7 +627,6 @@ impl FlexContainer {
layout_context: &LayoutContext,
positioning_context: &mut PositioningContext,
containing_block: &ContainingBlock,
depends_on_block_constraints: bool,
lazy_block_size: &LazySize,
) -> CacheableLayoutResult {
let mut flex_context = FlexContext {
@ -953,6 +952,20 @@ impl FlexContainer {
.or(all_baselines.last),
};
// TODO: `depends_on_block_constraints` could be false in some corner cases
// in order to improve performance.
// - In a single-line column container where all items have the grow and shrink
// factors set to zero and the flex basis doesn't depend on block constraints,
// and `justify-content` is `start` or equivalent.
// This is unlikely because the flex shrink factor defaults to 1.
// - In a single-line row container where all items have `align-self: start` or
// equivalent, and the cross size doesn't depend on block constraints.
// This is unlikely because `align-self` stretches by default.
// - In a multi-line row container where `align-content` is `start` or equivalent,
// and no item cross size depends on block constraints.
// This is unlikely because `align-content` defaults to `stretch`.
let depends_on_block_constraints = true;
CacheableLayoutResult {
fragments,
content_block_size,
@ -1880,9 +1893,6 @@ impl FlexItem<'_> {
&item_as_containing_block,
containing_block,
self.preferred_aspect_ratio,
flex_axis == FlexAxis::Column ||
self.cross_size_stretches_to_line ||
self.depends_on_block_constraints,
&lazy_block_size,
);
let CacheableLayoutResult {
@ -2597,7 +2607,6 @@ impl FlexItemBox {
&item_as_containing_block,
flex_context.containing_block,
preferred_aspect_ratio,
false, /* depends_on_block_constraints */
&LazySize::intrinsic(),
)
.content_block_size

9
components/layout/flow/mod.rs
View file

@ -346,7 +346,6 @@ impl OutsideMarker {
layout_context,
positioning_context,
&containing_block_for_children,
false, /* depends_on_block_constraints */
);
let max_inline_size =
@ -429,7 +428,6 @@ impl BlockFormattingContext {
layout_context: &LayoutContext,
positioning_context: &mut PositioningContext,
containing_block: &ContainingBlock,
depends_on_block_constraints: bool,
) -> CacheableLayoutResult {
let mut sequential_layout_state = if self.contains_floats || !layout_context.use_rayon {
Some(SequentialLayoutState::new(containing_block.size.inline))
@ -470,8 +468,7 @@ impl BlockFormattingContext {
clearance.unwrap_or_default(),
content_inline_size_for_table: None,
baselines: flow_layout.baselines,
depends_on_block_constraints: depends_on_block_constraints ||
flow_layout.depends_on_block_constraints,
depends_on_block_constraints: flow_layout.depends_on_block_constraints,
specific_layout_info: None,
collapsible_margins_in_children: CollapsedBlockMargins::zero(),
}
@ -1241,7 +1238,6 @@ impl IndependentFormattingContext {
&containing_block_for_children,
containing_block,
preferred_aspect_ratio,
false, /* depends_on_block_constraints */
&lazy_block_size,
);
@ -1432,7 +1428,6 @@ impl IndependentFormattingContext {
},
containing_block,
preferred_aspect_ratio,
false, /* depends_on_block_constraints */
&lazy_block_size,
);
@ -1498,7 +1493,6 @@ impl IndependentFormattingContext {
},
containing_block,
preferred_aspect_ratio,
false, /* depends_on_block_constraints */
&lazy_block_size,
);
@ -2326,7 +2320,6 @@ impl IndependentFormattingContext {
&containing_block_for_children,
containing_block,
preferred_aspect_ratio,
false, /* depends_on_block_constraints */
&lazy_block_size,
);

1
components/layout/flow/root.rs
View file

@ -225,7 +225,6 @@ impl BoxTree {
layout_context,
&mut positioning_context,
&(&initial_containing_block).into(),
false, /* depends_on_block_constraints */
);
let mut root_fragments = independent_layout.fragments.into_iter().collect::<Vec<_>>();

10
components/layout/formatting_contexts.rs
View file

@ -254,7 +254,6 @@ impl IndependentFormattingContext {
containing_block_for_children: &ContainingBlock,
containing_block: &ContainingBlock,
preferred_aspect_ratio: Option<AspectRatio>,
depends_on_block_constraints: bool,
lazy_block_size: &LazySize,
) -> CacheableLayoutResult {
match &self.contents {
@ -263,20 +262,17 @@ impl IndependentFormattingContext {
containing_block_for_children,
preferred_aspect_ratio,
&self.base,
depends_on_block_constraints,
lazy_block_size,
),
IndependentFormattingContextContents::Flow(bfc) => bfc.layout(
layout_context,
positioning_context,
containing_block_for_children,
depends_on_block_constraints,
),
IndependentFormattingContextContents::Flex(fc) => fc.layout(
layout_context,
positioning_context,
containing_block_for_children,
depends_on_block_constraints,
lazy_block_size,
),
IndependentFormattingContextContents::Grid(fc) => fc.layout(
@ -290,7 +286,6 @@ impl IndependentFormattingContext {
positioning_context,
containing_block_for_children,
containing_block,
depends_on_block_constraints,
),
}
}
@ -307,7 +302,6 @@ impl IndependentFormattingContext {
containing_block_for_children: &ContainingBlock,
containing_block: &ContainingBlock,
preferred_aspect_ratio: Option<AspectRatio>,
depends_on_block_constraints: bool,
lazy_block_size: &LazySize,
) -> CacheableLayoutResult {
if let Some(cache) = self.base.cached_layout_result.borrow().as_ref() {
@ -316,8 +310,7 @@ impl IndependentFormattingContext {
containing_block_for_children.size.inline &&
(cache.containing_block_for_children_size.block ==
containing_block_for_children.size.block ||
!(cache.result.depends_on_block_constraints ||
depends_on_block_constraints))
!cache.result.depends_on_block_constraints)
{
positioning_context.append(cache.positioning_context.clone());
return cache.result.clone();
@ -337,7 +330,6 @@ impl IndependentFormattingContext {
containing_block_for_children,
containing_block,
preferred_aspect_ratio,
depends_on_block_constraints,
lazy_block_size,
);

1
components/layout/positioned.rs
View file

@ -584,7 +584,6 @@ impl HoistedAbsolutelyPositionedBox {
&containing_block_for_children,
containing_block,
preferred_aspect_ratio,
false, /* depends_on_block_constraints */
&lazy_block_size,
);

3
components/layout/replaced.rs
View file

@ -451,7 +451,6 @@ impl ReplacedContents {
containing_block_for_children: &ContainingBlock,
preferred_aspect_ratio: Option<AspectRatio>,
base: &LayoutBoxBase,
depends_on_block_constraints: bool,
lazy_block_size: &LazySize,
) -> CacheableLayoutResult {
let writing_mode = base.style.writing_mode;
@ -472,7 +471,7 @@ impl ReplacedContents {
collapsible_margins_in_children: CollapsedBlockMargins::zero(),
content_block_size,
content_inline_size_for_table: None,
depends_on_block_constraints,
depends_on_block_constraints: false,
fragments: self.make_fragments(layout_context, &base.style, size),
specific_layout_info: None,
}

10
components/layout/table/layout.rs
View file

@ -1114,7 +1114,6 @@ impl<'a> TableLayout<'a> {
layout_context,
&mut positioning_context,
&containing_block_for_children,
false, /* depends_on_block_constraints */
);
Some(CellLayout {
@ -1545,15 +1544,10 @@ impl<'a> TableLayout<'a> {
positioning_context: &mut PositioningContext,
containing_block_for_children: &ContainingBlock,
containing_block_for_table: &ContainingBlock,
depends_on_block_constraints: bool,
) -> CacheableLayoutResult {
let table_writing_mode = containing_block_for_children.style.writing_mode;
self.compute_border_collapse(table_writing_mode);
let layout_style = self.table.layout_style(Some(&self));
let depends_on_block_constraints = depends_on_block_constraints ||
layout_style
.content_box_sizes_and_padding_border_margin(&containing_block_for_table.into())
.depends_on_block_constraints;
self.pbm = layout_style
.padding_border_margin_with_writing_mode_and_containing_block_inline_size(
@ -1592,7 +1586,7 @@ impl<'a> TableLayout<'a> {
content_block_size: Zero::zero(),
content_inline_size_for_table: None,
baselines: Baselines::default(),
depends_on_block_constraints,
depends_on_block_constraints: true,
specific_layout_info: Some(SpecificLayoutInfo::TableWrapper),
collapsible_margins_in_children: CollapsedBlockMargins::zero(),
};
@ -2700,14 +2694,12 @@ impl Table {
positioning_context: &mut PositioningContext,
containing_block_for_children: &ContainingBlock,
containing_block_for_table: &ContainingBlock,
depends_on_block_constraints: bool,
) -> CacheableLayoutResult {
TableLayout::new(self).layout(
layout_context,
positioning_context,
containing_block_for_children,
containing_block_for_table,
depends_on_block_constraints,
)
}
}

1
components/layout/taffy/layout.rs
View file

@ -205,7 +205,6 @@ impl taffy::LayoutPartialTree for TaffyContainerContext<'_> {
&content_box_size_override,
containing_block,
preferred_aspect_ratio,
false, /* depends_on_block_constraints */
&lazy_block_size,
);