Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Changed

- Grid: intrinsic track sizing no longer measures an item's min-/max-content contribution in a step where none of the item's spanned tracks can receive that contribution (e.g. items spanning only `minmax(0, 1fr)` or fixed tracks). This matches Blink and avoids redundant, sometimes very expensive, measurement of large subtrees under a min-content constraint.

### Fixed

- Grid: items with an `auto` start line and a definite end line (e.g. `grid-column: auto / 1`) no longer cause a phantom zero-sized positive implicit track to be created. This previously caused `grid_template_columns()`/`grid_template_rows()` to serialize an extra `0px` track (e.g. `10px 0px` instead of `10px`)
Expand Down
34 changes: 29 additions & 5 deletions src/compute/grid/track_sizing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,10 +362,13 @@ pub(super) fn track_sizing_algorithm<Tree: LayoutPartialTree>(
tree,
axis,
axis_tracks,
other_axis_tracks,
items,
axis_min_size,
axis_max_size,
axis_available_space_for_expansion,
inner_node_size,
get_track_size_estimate,
);

// 11.8. Stretch auto Tracks
Expand Down Expand Up @@ -796,6 +799,9 @@ fn resolve_intrinsic_track_sizes<Tree: LayoutPartialTree>(
let has_min_or_max_content_min_track_sizing_function =
move |track: &GridTrack| track.min_track_sizing_function.is_min_or_max_content();
for item in batch.iter_mut() {
if !item.spans_track_matching(axis, axis_tracks, has_min_or_max_content_min_track_sizing_function) {
continue;
}
let space = item_sizer.min_content_contribution(item, axis_tracks);
let tracks = &mut axis_tracks[item.track_range_excluding_lines(axis)];
if space > 0.0 {
Expand Down Expand Up @@ -856,6 +862,11 @@ fn resolve_intrinsic_track_sizes<Tree: LayoutPartialTree>(
}

for item in batch.iter_mut() {
if !item.spans_track_matching(axis, axis_tracks, |track| {
has_auto_min_track_sizing_function(track) || has_max_content_min_track_sizing_function(track)
}) {
continue;
}
let axis_max_content_size = item_sizer.max_content_contribution(item, axis_tracks);
let limit = item.spanned_track_limit(axis, axis_tracks, axis_inner_node_size, &|val, basis| {
item_sizer.calc(val, basis)
Expand Down Expand Up @@ -918,6 +929,9 @@ fn resolve_intrinsic_track_sizes<Tree: LayoutPartialTree>(
let has_max_content_min_track_sizing_function =
move |track: &GridTrack| track.min_track_sizing_function.is_max_content();
for item in batch.iter_mut() {
if !item.spans_track_matching(axis, axis_tracks, has_max_content_min_track_sizing_function) {
continue;
}
let axis_max_content_size = item_sizer.max_content_contribution(item, axis_tracks);
let space = axis_max_content_size;
let tracks = &mut axis_tracks[item.track_range_excluding_lines(axis)];
Expand Down Expand Up @@ -950,6 +964,9 @@ fn resolve_intrinsic_track_sizes<Tree: LayoutPartialTree>(
let has_intrinsic_max_track_sizing_function =
move |track: &GridTrack| !track.max_track_sizing_function.has_definite_value(axis_inner_node_size);
for item in batch.iter_mut() {
if !item.spans_track_matching(axis, axis_tracks, has_intrinsic_max_track_sizing_function) {
continue;
}
let axis_min_content_size = item_sizer.min_content_contribution(item, axis_tracks);
let space = axis_min_content_size;
let tracks = &mut axis_tracks[item.track_range_excluding_lines(axis)];
Expand All @@ -973,6 +990,9 @@ fn resolve_intrinsic_track_sizes<Tree: LayoutPartialTree>(
|| (track.max_track_sizing_function.uses_percentage() && axis_inner_node_size.is_none())
};
for item in batch.iter_mut() {
if !item.spans_track_matching(axis, axis_tracks, has_max_content_max_track_sizing_function) {
continue;
}
let axis_max_content_size = item_sizer.max_content_contribution(item, axis_tracks);
let space = axis_max_content_size;
let tracks = &mut axis_tracks[item.track_range_excluding_lines(axis)];
Expand Down Expand Up @@ -1252,15 +1272,21 @@ fn maximise_tracks(
/// This step sizes flexible tracks using the largest value it can assign to an fr without exceeding the available space.
#[allow(clippy::too_many_arguments)]
#[inline(always)]
fn expand_flexible_tracks(
tree: &mut impl LayoutPartialTree,
fn expand_flexible_tracks<Tree: LayoutPartialTree>(
tree: &mut Tree,
axis: AbstractAxis,
axis_tracks: &mut [GridTrack],
other_axis_tracks: &[GridTrack],
items: &mut [GridItem],
axis_min_size: Option<f32>,
axis_max_size: Option<f32>,
axis_available_space_for_expansion: AvailableSpace,
inner_node_size: Size<Option<f32>>,
get_track_size_estimate: impl Fn(&GridTrack, Option<f32>, &Tree) -> Option<f32>,
) {
let mut item_sizer =
IntrinsicSizeMeasurer { tree, other_axis_tracks, axis, inner_node_size, get_track_size_estimate };

// First, find the grid’s used flex fraction:
let flex_fraction = match axis_available_space_for_expansion {
// If the free space is zero:
Expand Down Expand Up @@ -1304,10 +1330,8 @@ fn expand_flexible_tracks(
.iter_mut()
.filter(|item| item.crosses_flexible_track(axis))
.map(|item| {
let max_content_contribution = item_sizer.max_content_contribution(item, axis_tracks);
let tracks = &axis_tracks[item.track_range_excluding_lines(axis)];
// TODO: plumb estimate of other axis size (known_dimensions) in here rather than just passing Size::NONE?
let max_content_contribution =
item.max_content_contribution_cached(axis, tree, Size::NONE, Size::NONE);
find_size_of_fr(tracks, max_content_contribution)
})
.max_by(|a, b| a.total_cmp(b))
Expand Down
10 changes: 10 additions & 0 deletions src/compute/grid/types/grid_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,16 @@ impl GridItem {
(indexes.start as usize + 1)..(indexes.end as usize)
}

/// Whether any track spanned by this item in the specified axis satisfies `predicate`
pub fn spans_track_matching(
&self,
axis: AbstractAxis,
axis_tracks: &[GridTrack],
predicate: impl Fn(&GridTrack) -> bool,
) -> bool {
axis_tracks[self.track_range_excluding_lines(axis)].iter().any(predicate)
}

/// Returns the number of tracks that this item spans in the specified axis
pub fn span(&self, axis: AbstractAxis) -> u16 {
match axis {
Expand Down
18 changes: 18 additions & 0 deletions test_fixtures/grid/grid_fr_row_max_content_uses_column_width.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<!DOCTYPE html>
<html lang="en">
<head>
<script src="../../scripts/gentest/test_helper.js"></script>
<link rel="stylesheet" type="text/css" href="../../scripts/gentest/test_base_style.css">
<title>
Test description
</title>
</head>
<body>

<div id="test-root" style="display: grid; grid-template-columns: 50px; grid-template-rows: minmax(10px, 1fr) minmax(min-content, 50px);">
<div>HHHHH&ZeroWidthSpace;HHHH</div>
<div>HHHHH&ZeroWidthSpace;HHHH</div>
</div>

</body>
</html>
30 changes: 30 additions & 0 deletions tests/hand_written/caching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,34 @@ mod caching {
assert_eq!(layout.location.y, 19.0 * index as f32);
}
}
/// An item whose spanned tracks cannot receive its min-/max-content contribution in any
/// intrinsic track sizing step (definite min, flexible max) must not be measured for it.
#[test]
#[cfg(feature = "grid")]
fn grid_item_spanning_only_unaffected_tracks_is_not_measured() {
let mut taffy = new_test_tree();

let text = TestNodeContext::ahem_text("HH HH HH HH".to_string(), taffy_test_helpers::WritingMode::Horizontal);
let leaf_style =
Style { grid_column: taffy::geometry::Line { start: line(2), end: line(3) }, ..Default::default() };
let leaf = taffy.new_leaf_with_context(leaf_style, text).unwrap();
let grid = taffy
.new_with_children(
Style {
display: Display::Grid,
size: Size { width: length(400.0), height: length(50.0) },
grid_template_columns: vec![length(100.0), minmax(length(0.0), fr(1.0))],
grid_template_rows: vec![length(50.0)],
..Default::default()
},
&[leaf],
)
.unwrap();

taffy.compute_layout_with_measure(grid, Size::MAX_CONTENT, test_measure_function).unwrap();

// Only the final layout pass measures the leaf; track sizing does not.
assert_eq!(taffy.layout(leaf).unwrap().size.width, 300.0);
assert_eq!(taffy.get_node_context_mut(leaf).unwrap().count, 1);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<test name="grid_fr_row_max_content_uses_column_width__border_box_ltr" use-rounding="true">
<viewport width="max-content" height="max-content"/>
<input>
<div display="grid" direction="ltr" grid-template-rows="minmax(10px, 1fr) minmax(min-content, 50px)" grid-template-columns="50px">
<text direction="ltr">
HHHHH​HHHH
</text>
<text direction="ltr">
HHHHH​HHHH
</text>
</div>
</input>
<expectations>
<node x="0" y="0" width="50" height="70" resolved-rows="20px 50px" resolved-columns="50px">
<node x="0" y="0" width="50" height="20"/>
<node x="0" y="20" width="50" height="50"/>
</node>
</expectations>
</test>
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<test name="grid_fr_row_max_content_uses_column_width__border_box_rtl" use-rounding="true">
<viewport width="max-content" height="max-content"/>
<input>
<div display="grid" direction="rtl" grid-template-rows="minmax(10px, 1fr) minmax(min-content, 50px)" grid-template-columns="50px">
<text direction="rtl">
HHHHH​HHHH
</text>
<text direction="rtl">
HHHHH​HHHH
</text>
</div>
</input>
<expectations>
<node x="0" y="0" width="50" height="70" resolved-rows="20px 50px" resolved-columns="50px">
<node x="0" y="0" width="50" height="20"/>
<node x="0" y="20" width="50" height="50"/>
</node>
</expectations>
</test>
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<test name="grid_fr_row_max_content_uses_column_width__content_box_ltr" use-rounding="true">
<viewport width="max-content" height="max-content"/>
<input>
<div display="grid" box-sizing="content-box" direction="ltr" grid-template-rows="minmax(10px, 1fr) minmax(min-content, 50px)" grid-template-columns="50px">
<text box-sizing="content-box" direction="ltr">
HHHHH​HHHH
</text>
<text box-sizing="content-box" direction="ltr">
HHHHH​HHHH
</text>
</div>
</input>
<expectations>
<node x="0" y="0" width="50" height="70" resolved-rows="20px 50px" resolved-columns="50px">
<node x="0" y="0" width="50" height="20"/>
<node x="0" y="20" width="50" height="50"/>
</node>
</expectations>
</test>
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<test name="grid_fr_row_max_content_uses_column_width__content_box_rtl" use-rounding="true">
<viewport width="max-content" height="max-content"/>
<input>
<div display="grid" box-sizing="content-box" direction="rtl" grid-template-rows="minmax(10px, 1fr) minmax(min-content, 50px)" grid-template-columns="50px">
<text box-sizing="content-box" direction="rtl">
HHHHH​HHHH
</text>
<text box-sizing="content-box" direction="rtl">
HHHHH​HHHH
</text>
</div>
</input>
<expectations>
<node x="0" y="0" width="50" height="70" resolved-rows="20px 50px" resolved-columns="50px">
<node x="0" y="0" width="50" height="20"/>
<node x="0" y="20" width="50" height="50"/>
</node>
</expectations>
</test>
24 changes: 24 additions & 0 deletions tests/xml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27463,6 +27463,30 @@ mod grid {
crate::run_xml_test("grid", "grid_fr_no_sized_items_indefinite__content_box_rtl");
}

#[cfg(feature = "grid")]
#[test]
fn grid_fr_row_max_content_uses_column_width__border_box_ltr() {
crate::run_xml_test("grid", "grid_fr_row_max_content_uses_column_width__border_box_ltr");
}

#[cfg(feature = "grid")]
#[test]
fn grid_fr_row_max_content_uses_column_width__content_box_ltr() {
crate::run_xml_test("grid", "grid_fr_row_max_content_uses_column_width__content_box_ltr");
}

#[cfg(feature = "grid")]
#[test]
fn grid_fr_row_max_content_uses_column_width__border_box_rtl() {
crate::run_xml_test("grid", "grid_fr_row_max_content_uses_column_width__border_box_rtl");
}

#[cfg(feature = "grid")]
#[test]
fn grid_fr_row_max_content_uses_column_width__content_box_rtl() {
crate::run_xml_test("grid", "grid_fr_row_max_content_uses_column_width__content_box_rtl");
}

#[cfg(feature = "grid")]
#[test]
fn grid_fr_single_item_indefinite__border_box_ltr() {
Expand Down