Skip to content

Commit

Permalink
layout: Improve logic for block size of table (servo#34947)
Browse files Browse the repository at this point in the history
The containing block for children already has the size coming from the
style and the rules of the parent formatting context, so no need to try
to recompute it.

This allows removing a bunch of functions, and fixes some problems when
the table is a flex item.

Signed-off-by: Oriol Brufau <[email protected]>
  • Loading branch information
Loirooriol authored Jan 13, 2025
1 parent f66cd17 commit 4332c1e
Show file tree
Hide file tree
Showing 10 changed files with 8 additions and 119 deletions.
16 changes: 0 additions & 16 deletions components/layout_2020/geom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -733,22 +733,6 @@ impl From<StyleMaxSize> for Size<LengthPercentage> {
}

impl LogicalVec2<Size<LengthPercentage>> {
pub(crate) fn percentages_relative_to(
&self,
containing_block: &ContainingBlock,
) -> LogicalVec2<Size<Au>> {
self.map_inline_and_block_axes(
|inline_size| inline_size.map(|lp| lp.to_used_value(containing_block.size.inline)),
|block_size| {
block_size
.maybe_map(|lp| {
lp.maybe_to_used_value(containing_block.size.block.to_definite())
})
.unwrap_or_default()
},
)
}

pub(crate) fn maybe_percentages_relative_to_basis(
&self,
basis: &LogicalVec2<Option<Au>>,
Expand Down
66 changes: 0 additions & 66 deletions components/layout_2020/style_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,31 +263,11 @@ pub(crate) trait ComputedValuesExt {
&self,
containing_block_writing_mode: WritingMode,
) -> LogicalVec2<Size<LengthPercentage>>;
fn content_box_size(
&self,
containing_block: &ContainingBlock,
pbm: &PaddingBorderMargin,
) -> LogicalVec2<Size<Au>>;
fn content_box_size_deprecated(
&self,
containing_block: &ContainingBlock,
pbm: &PaddingBorderMargin,
) -> LogicalVec2<AuOrAuto>;
fn content_box_size_for_box_size(
&self,
box_size: LogicalVec2<Size<Au>>,
pbm: &PaddingBorderMargin,
) -> LogicalVec2<Size<Au>>;
fn content_min_box_size(
&self,
containing_block: &ContainingBlock,
pbm: &PaddingBorderMargin,
) -> LogicalVec2<Size<Au>>;
fn content_min_box_size_deprecated(
&self,
containing_block: &ContainingBlock,
pbm: &PaddingBorderMargin,
) -> LogicalVec2<AuOrAuto>;
fn content_min_box_size_for_min_size(
&self,
box_size: LogicalVec2<Size<Au>>,
Expand Down Expand Up @@ -418,26 +398,6 @@ impl ComputedValuesExt for ComputedValues {
)
}

fn content_box_size(
&self,
containing_block: &ContainingBlock,
pbm: &PaddingBorderMargin,
) -> LogicalVec2<Size<Au>> {
let box_size = self
.box_size(containing_block.style.writing_mode)
.percentages_relative_to(containing_block);
self.content_box_size_for_box_size(box_size, pbm)
}

fn content_box_size_deprecated(
&self,
containing_block: &ContainingBlock,
pbm: &PaddingBorderMargin,
) -> LogicalVec2<AuOrAuto> {
self.content_box_size(containing_block, pbm)
.map(Size::to_auto_or)
}

fn content_box_size_for_box_size(
&self,
box_size: LogicalVec2<Size<Au>>,
Expand All @@ -454,32 +414,6 @@ impl ComputedValuesExt for ComputedValues {
}
}

fn content_min_box_size(
&self,
containing_block: &ContainingBlock,
pbm: &PaddingBorderMargin,
) -> LogicalVec2<Size<Au>> {
let min_size = self
.min_box_size(containing_block.style.writing_mode)
.map_inline_and_block_sizes(
|lp| lp.to_used_value(containing_block.size.inline),
|lp| {
let cbbs = containing_block.size.block.to_definite();
lp.to_used_value(cbbs.unwrap_or_else(Au::zero))
},
);
self.content_min_box_size_for_min_size(min_size, pbm)
}

fn content_min_box_size_deprecated(
&self,
containing_block: &ContainingBlock,
pbm: &PaddingBorderMargin,
) -> LogicalVec2<AuOrAuto> {
self.content_min_box_size(containing_block, pbm)
.map(Size::to_auto_or)
}

fn content_min_box_size_for_min_size(
&self,
min_box_size: LogicalVec2<Size<Au>>,
Expand Down
27 changes: 8 additions & 19 deletions components/layout_2020/table/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ use style::values::computed::{
BorderStyle, LengthPercentage as ComputedLengthPercentage, Percentage,
};
use style::values::generics::box_::{GenericVerticalAlign as VerticalAlign, VerticalAlignKeyword};
use style::values::generics::length::GenericLengthPercentageOrAuto::{Auto, LengthPercentage};
use style::Zero;

use super::{
Expand Down Expand Up @@ -1550,24 +1549,17 @@ impl<'a> TableLayout<'a> {
&mut self,
mut row_sizes: Vec<Au>,
containing_block_for_children: &ContainingBlock,
containing_block_for_table: &ContainingBlock,
) {
// The table content height is the maximum of the computed table height from style and the
// sum of computed row heights from row layout plus size from borders and spacing.
// When block-size doesn't compute to auto, `containing_block_for children` will have
// the resulting length, properly clamped between min-block-size and max-block-size.
let style = &self.table.style;
let table_height_from_style = match style
.content_box_size_deprecated(containing_block_for_table, &self.pbm)
.block
{
LengthPercentage(_) => containing_block_for_children.size.block.to_auto_or(),
Auto => style
.content_min_box_size_deprecated(containing_block_for_table, &self.pbm)
.block
.map(Au::from),
}
.auto_is(Au::zero);
// TODO: for `height: stretch`, the block size of the containing block is the available
// space for the entire table wrapper, but here we are using that amount for the table grid.
// Therefore, if there is a caption, this will cause overflow. Gecko and WebKit have the
// same problem, but not Blink.
let table_height_from_style = match containing_block_for_children.size.block {
SizeConstraint::Definite(size) => size,
SizeConstraint::MinMax(min, _) => min,
};

let block_border_spacing = self.table.total_border_spacing().block;
let table_height_from_rows = row_sizes.iter().sum::<Au>() + block_border_spacing;
Expand Down Expand Up @@ -1756,7 +1748,6 @@ impl<'a> TableLayout<'a> {
positioning_context,
&containing_block_for_logical_conversion,
containing_block_for_children,
containing_block_for_table,
);

// Take the baseline of the grid fragment, after adjusting it to be in the coordinate system
Expand Down Expand Up @@ -1853,7 +1844,6 @@ impl<'a> TableLayout<'a> {
positioning_context: &mut PositioningContext,
containing_block_for_logical_conversion: &ContainingBlock,
containing_block_for_children: &ContainingBlock,
containing_block_for_table: &ContainingBlock,
) -> BoxFragment {
self.distributed_column_widths = self.distribute_width_to_columns();
self.layout_cells_in_row(
Expand All @@ -1866,7 +1856,6 @@ impl<'a> TableLayout<'a> {
self.compute_table_height_and_final_row_heights(
first_layout_row_heights,
containing_block_for_children,
containing_block_for_table,
);

assert_eq!(self.table.size.height, self.row_sizes.len());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
[table-align-self-stretch.html]
[.item 1]
expected: FAIL

[.item 3]
expected: FAIL

[.item 5]
expected: FAIL

Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

0 comments on commit 4332c1e

Please sign in to comment.