diff --git a/CHANGELOG.md b/CHANGELOG.md index 5904913292..0906f71bc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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`) diff --git a/src/compute/grid/track_sizing.rs b/src/compute/grid/track_sizing.rs index a141cc2670..067d6365d2 100644 --- a/src/compute/grid/track_sizing.rs +++ b/src/compute/grid/track_sizing.rs @@ -362,10 +362,13 @@ pub(super) fn track_sizing_algorithm( 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 @@ -796,6 +799,9 @@ fn resolve_intrinsic_track_sizes( 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 { @@ -856,6 +862,11 @@ fn resolve_intrinsic_track_sizes( } 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) @@ -918,6 +929,9 @@ fn resolve_intrinsic_track_sizes( 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)]; @@ -950,6 +964,9 @@ fn resolve_intrinsic_track_sizes( 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)]; @@ -973,6 +990,9 @@ fn resolve_intrinsic_track_sizes( || (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)]; @@ -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: &mut Tree, axis: AbstractAxis, axis_tracks: &mut [GridTrack], + other_axis_tracks: &[GridTrack], items: &mut [GridItem], axis_min_size: Option, axis_max_size: Option, axis_available_space_for_expansion: AvailableSpace, + inner_node_size: Size>, + get_track_size_estimate: impl Fn(&GridTrack, Option, &Tree) -> Option, ) { + 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: @@ -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)) diff --git a/src/compute/grid/types/grid_item.rs b/src/compute/grid/types/grid_item.rs index cd6c0b86a6..1ef3af8d2d 100644 --- a/src/compute/grid/types/grid_item.rs +++ b/src/compute/grid/types/grid_item.rs @@ -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 { diff --git a/test_fixtures/grid/grid_fr_row_max_content_uses_column_width.html b/test_fixtures/grid/grid_fr_row_max_content_uses_column_width.html new file mode 100644 index 0000000000..18a0f546f4 --- /dev/null +++ b/test_fixtures/grid/grid_fr_row_max_content_uses_column_width.html @@ -0,0 +1,18 @@ + + + + + + + Test description + + + + +
+
HHHHH​HHHH
+
HHHHH​HHHH
+
+ + + diff --git a/tests/hand_written/caching.rs b/tests/hand_written/caching.rs index 3d2f2fa44c..ccabcbf072 100644 --- a/tests/hand_written/caching.rs +++ b/tests/hand_written/caching.rs @@ -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); + } } diff --git a/tests/xml/grid/grid_fr_row_max_content_uses_column_width__border_box_ltr.xml b/tests/xml/grid/grid_fr_row_max_content_uses_column_width__border_box_ltr.xml new file mode 100644 index 0000000000..fc92aef042 --- /dev/null +++ b/tests/xml/grid/grid_fr_row_max_content_uses_column_width__border_box_ltr.xml @@ -0,0 +1,19 @@ + + + +
+ + HHHHH​HHHH + + + HHHHH​HHHH + +
+ + + + + + + + diff --git a/tests/xml/grid/grid_fr_row_max_content_uses_column_width__border_box_rtl.xml b/tests/xml/grid/grid_fr_row_max_content_uses_column_width__border_box_rtl.xml new file mode 100644 index 0000000000..c0a02c9de1 --- /dev/null +++ b/tests/xml/grid/grid_fr_row_max_content_uses_column_width__border_box_rtl.xml @@ -0,0 +1,19 @@ + + + +
+ + HHHHH​HHHH + + + HHHHH​HHHH + +
+ + + + + + + + diff --git a/tests/xml/grid/grid_fr_row_max_content_uses_column_width__content_box_ltr.xml b/tests/xml/grid/grid_fr_row_max_content_uses_column_width__content_box_ltr.xml new file mode 100644 index 0000000000..b22ab75bc6 --- /dev/null +++ b/tests/xml/grid/grid_fr_row_max_content_uses_column_width__content_box_ltr.xml @@ -0,0 +1,19 @@ + + + +
+ + HHHHH​HHHH + + + HHHHH​HHHH + +
+ + + + + + + + diff --git a/tests/xml/grid/grid_fr_row_max_content_uses_column_width__content_box_rtl.xml b/tests/xml/grid/grid_fr_row_max_content_uses_column_width__content_box_rtl.xml new file mode 100644 index 0000000000..1c353b045f --- /dev/null +++ b/tests/xml/grid/grid_fr_row_max_content_uses_column_width__content_box_rtl.xml @@ -0,0 +1,19 @@ + + + +
+ + HHHHH​HHHH + + + HHHHH​HHHH + +
+ + + + + + + + diff --git a/tests/xml/mod.rs b/tests/xml/mod.rs index 952b88c2e9..b7b1e9a6d2 100644 --- a/tests/xml/mod.rs +++ b/tests/xml/mod.rs @@ -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() {