tawasuyu

tawasuyu/taffy

A high performance rust-powered UI layout library

HTML 231Rust 10TOML 3CSS 1348 Definitions

Fix CSS Grid "auto track" / placement bugs (#481)

* Fix divide by zero when using grid_auto_rows/grid_auto_columns with zero
"negative implicit" tracks

* Fix over-allocation in the CellOccupancyMatrix when auto-placing in grids
that contain negative implicit tracks.

* Fix axis conflation when grid_auto_flow is column

* Fix computation of auto track offset when initializing negative implicit tracks

* Add basic tests for grid_auto_rows and grid_auto_columns

8a04e1a8e933a9f51dad658bc40588b0af479ddf · by Nico Burns · (3 years ago) · parent 10652b66f · browse the tree at this commit

Semantic delta · 7

src/compute/grid/explicit_grid.rs

src/compute/grid/placement.rs

src/compute/grid/types/cell_occupancy.rs

tests/generated/grid_auto_columns.rs

tests/generated/grid_auto_rows.rs

tests/generated/mod.rs

8 files · +229 −16

diff --git a/src/compute/grid/explicit_grid.rs b/src/compute/grid/explicit_grid.rs
index af4b5a18e..42624f5aa 100644
--- a/src/compute/grid/explicit_grid.rs
+++ b/src/compute/grid/explicit_grid.rs
@@ -7,7 +7,6 @@ use crate::style_helpers::TaffyAuto;
 use crate::util::sys::{GridTrackVec, Vec};
 use crate::util::MaybeMath;
 use crate::util::ResolveOrZero;
-use core::cmp::{max, min};
 
 #[cfg(not(feature = "std"))]
 use num_traits::float::FloatCore;
@@ -194,15 +193,15 @@ pub(super) fn initialize_grid_tracks(
     tracks.push(GridTrack::gutter(gap));
 
     // Create negative implicit tracks
-    if auto_tracks.is_empty() {
-        let iter = core::iter::repeat(NonRepeatedTrackSizingFunction::AUTO);
-        create_implicit_tracks(tracks, counts.negative_implicit, iter, gap)
-    } else {
-        let max_count = max(auto_tracks.len(), counts.negative_implicit as usize);
-        let min_count = min(auto_tracks.len(), counts.negative_implicit as usize);
-        let offset = max_count % min_count;
-        let iter = auto_tracks.iter().copied().cycle().skip(offset);
-        create_implicit_tracks(tracks, counts.negative_implicit, iter, gap)
+    if counts.negative_implicit > 0 {
+        if auto_tracks.is_empty() {
+            let iter = core::iter::repeat(NonRepeatedTrackSizingFunction::AUTO);
+            create_implicit_tracks(tracks, counts.negative_implicit, iter, gap)
+        } else {
+            let offset = auto_tracks.len() - (counts.negative_implicit as usize % auto_tracks.len());
+            let iter = auto_tracks.iter().copied().cycle().skip(offset);
+            create_implicit_tracks(tracks, counts.negative_implicit, iter, gap)
+        }
     }
 
     let mut current_track_index = (counts.negative_implicit) as usize;
diff --git a/src/compute/grid/placement.rs b/src/compute/grid/placement.rs
index 132426cd6..f9fe6c5ad 100644
--- a/src/compute/grid/placement.rs
+++ b/src/compute/grid/placement.rs
@@ -113,9 +113,11 @@ pub(super) fn place_grid_items<'a, ChildIter>(
 
     // 4. Position the remaining grid items
     // (which either have definite position only in the secondary axis or indefinite positions in both axis)
-    let x_neg_tracks = cell_occupancy_matrix.track_counts(AbsoluteAxis::Horizontal).negative_implicit as i16;
-    let y_neg_tracks = cell_occupancy_matrix.track_counts(AbsoluteAxis::Vertical).negative_implicit as i16;
-    let grid_start_position = (OriginZeroLine(-x_neg_tracks), OriginZeroLine(-y_neg_tracks));
+    let primary_axis = grid_auto_flow.primary_axis();
+    let secondary_axis = primary_axis.other_axis();
+    let primary_neg_tracks = cell_occupancy_matrix.track_counts(primary_axis).negative_implicit as i16;
+    let secondary_neg_tracks = cell_occupancy_matrix.track_counts(secondary_axis).negative_implicit as i16;
+    let grid_start_position = (OriginZeroLine(-primary_neg_tracks), OriginZeroLine(-secondary_neg_tracks));
     let mut grid_position = grid_start_position;
     let mut idx = 0;
     children_iter()
diff --git a/src/compute/grid/types/cell_occupancy.rs b/src/compute/grid/types/cell_occupancy.rs
index cf05648a5..5e011c64c 100644
--- a/src/compute/grid/types/cell_occupancy.rs
+++ b/src/compute/grid/types/cell_occupancy.rs
@@ -91,10 +91,9 @@ impl CellOccupancyMatrix {
     fn expand_to_fit_range(&mut self, row_range: Range<i16>, col_range: Range<i16>) {
         // Calculate number of rows and columns missing to accomodate ranges (if any)
         let req_negative_rows = min(row_range.start, 0);
-        let req_positive_rows = max(row_range.end - self.rows.explicit as i16 - self.rows.positive_implicit as i16, 0);
+        let req_positive_rows = max(row_range.end - self.rows.len() as i16, 0);
         let req_negative_cols = min(col_range.start, 0);
-        let req_positive_cols =
-            max(col_range.end - self.columns.explicit as i16 - self.columns.positive_implicit as i16, 0);
+        let req_positive_cols = max(col_range.end - self.columns.len() as i16, 0);
 
         let old_row_count = self.rows.len();
         let old_col_count = self.columns.len();
diff --git a/test_fixtures/grid_auto_columns.html b/test_fixtures/grid_auto_columns.html
new file mode 100644
index 000000000..7e717bef0
--- /dev/null
+++ b/test_fixtures/grid_auto_columns.html
@@ -0,0 +1,24 @@
+<!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-auto-flow: column; grid-template-rows: 100px; grid-template-columns: 40px; grid-auto-columns: 10px 20px 30px;">
+  <div style="grid-column-start: -3;"></div>
+  <div></div>
+  <div></div>
+  <div></div>
+  <div></div>
+  <div></div>
+  <div></div>
+  <div></div>
+</div>
+
+</body>
+</html>
\ No newline at end of file
diff --git a/test_fixtures/grid_auto_rows.html b/test_fixtures/grid_auto_rows.html
new file mode 100644
index 000000000..19f55d5bc
--- /dev/null
+++ b/test_fixtures/grid_auto_rows.html
@@ -0,0 +1,24 @@
+<!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: 100px; grid-template-rows: 40px; grid-auto-rows: 10px 20px 30px;">
+  <div style="grid-row-start: -4;"></div>
+  <div></div>
+  <div></div>
+  <div></div>
+  <div></div>
+  <div></div>
+  <div></div>
+  <div></div>
+</div>
+
+</body>
+</html>
\ No newline at end of file
diff --git a/tests/generated/grid_auto_columns.rs b/tests/generated/grid_auto_columns.rs
new file mode 100644
index 000000000..8c20713a6
--- /dev/null
+++ b/tests/generated/grid_auto_columns.rs
@@ -0,0 +1,81 @@
+#[test]
+fn grid_auto_columns() {
+    #[allow(unused_imports)]
+    use taffy::{prelude::*, tree::Layout};
+    let mut taffy = taffy::Taffy::new();
+    let node0 = taffy
+        .new_leaf(taffy::style::Style {
+            grid_column: taffy::geometry::Line { start: line(-3i16), end: taffy::style::GridPlacement::Auto },
+            ..Default::default()
+        })
+        .unwrap();
+    let node1 = taffy.new_leaf(taffy::style::Style { ..Default::default() }).unwrap();
+    let node2 = taffy.new_leaf(taffy::style::Style { ..Default::default() }).unwrap();
+    let node3 = taffy.new_leaf(taffy::style::Style { ..Default::default() }).unwrap();
+    let node4 = taffy.new_leaf(taffy::style::Style { ..Default::default() }).unwrap();
+    let node5 = taffy.new_leaf(taffy::style::Style { ..Default::default() }).unwrap();
+    let node6 = taffy.new_leaf(taffy::style::Style { ..Default::default() }).unwrap();
+    let node7 = taffy.new_leaf(taffy::style::Style { ..Default::default() }).unwrap();
+    let node = taffy
+        .new_with_children(
+            taffy::style::Style {
+                display: taffy::style::Display::Grid,
+                grid_template_rows: vec![length(100f32)],
+                grid_template_columns: vec![length(40f32)],
+                grid_auto_columns: vec![length(10f32), length(20f32), length(30f32)],
+                grid_auto_flow: taffy::style::GridAutoFlow::Column,
+                ..Default::default()
+            },
+            &[node0, node1, node2, node3, node4, node5, node6, node7],
+        )
+        .unwrap();
+    taffy.compute_layout(node, taffy::geometry::Size::MAX_CONTENT).unwrap();
+    println!("\nComputed tree:");
+    taffy::util::print_tree(&taffy, node);
+    println!();
+    let Layout { size, location, .. } = taffy.layout(node).unwrap();
+    assert_eq!(size.width, 190f32, "width of node {:?}. Expected {}. Actual {}", node, 190f32, size.width);
+    assert_eq!(size.height, 100f32, "height of node {:?}. Expected {}. Actual {}", node, 100f32, size.height);
+    assert_eq!(location.x, 0f32, "x of node {:?}. Expected {}. Actual {}", node, 0f32, location.x);
+    assert_eq!(location.y, 0f32, "y of node {:?}. Expected {}. Actual {}", node, 0f32, location.y);
+    let Layout { size, location, .. } = taffy.layout(node0).unwrap();
+    assert_eq!(size.width, 30f32, "width of node {:?}. Expected {}. Actual {}", node0, 30f32, size.width);
+    assert_eq!(size.height, 100f32, "height of node {:?}. Expected {}. Actual {}", node0, 100f32, size.height);
+    assert_eq!(location.x, 0f32, "x of node {:?}. Expected {}. Actual {}", node0, 0f32, location.x);
+    assert_eq!(location.y, 0f32, "y of node {:?}. Expected {}. Actual {}", node0, 0f32, location.y);
+    let Layout { size, location, .. } = taffy.layout(node1).unwrap();
+    assert_eq!(size.width, 40f32, "width of node {:?}. Expected {}. Actual {}", node1, 40f32, size.width);
+    assert_eq!(size.height, 100f32, "height of node {:?}. Expected {}. Actual {}", node1, 100f32, size.height);
+    assert_eq!(location.x, 30f32, "x of node {:?}. Expected {}. Actual {}", node1, 30f32, location.x);
+    assert_eq!(location.y, 0f32, "y of node {:?}. Expected {}. Actual {}", node1, 0f32, location.y);
+    let Layout { size, location, .. } = taffy.layout(node2).unwrap();
+    assert_eq!(size.width, 10f32, "width of node {:?}. Expected {}. Actual {}", node2, 10f32, size.width);
+    assert_eq!(size.height, 100f32, "height of node {:?}. Expected {}. Actual {}", node2, 100f32, size.height);
+    assert_eq!(location.x, 70f32, "x of node {:?}. Expected {}. Actual {}", node2, 70f32, location.x);
+    assert_eq!(location.y, 0f32, "y of node {:?}. Expected {}. Actual {}", node2, 0f32, location.y);
+    let Layout { size, location, .. } = taffy.layout(node3).unwrap();
+    assert_eq!(size.width, 20f32, "width of node {:?}. Expected {}. Actual {}", node3, 20f32, size.width);
+    assert_eq!(size.height, 100f32, "height of node {:?}. Expected {}. Actual {}", node3, 100f32, size.height);
+    assert_eq!(location.x, 80f32, "x of node {:?}. Expected {}. Actual {}", node3, 80f32, location.x);
+    assert_eq!(location.y, 0f32, "y of node {:?}. Expected {}. Actual {}", node3, 0f32, location.y);
+    let Layout { size, location, .. } = taffy.layout(node4).unwrap();
+    assert_eq!(size.width, 30f32, "width of node {:?}. Expected {}. Actual {}", node4, 30f32, size.width);
+    assert_eq!(size.height, 100f32, "height of node {:?}. Expected {}. Actual {}", node4, 100f32, size.height);
+    assert_eq!(location.x, 100f32, "x of node {:?}. Expected {}. Actual {}", node4, 100f32, location.x);
+    assert_eq!(location.y, 0f32, "y of node {:?}. Expected {}. Actual {}", node4, 0f32, location.y);
+    let Layout { size, location, .. } = taffy.layout(node5).unwrap();
+    assert_eq!(size.width, 10f32, "width of node {:?}. Expected {}. Actual {}", node5, 10f32, size.width);
+    assert_eq!(size.height, 100f32, "height of node {:?}. Expected {}. Actual {}", node5, 100f32, size.height);
+    assert_eq!(location.x, 130f32, "x of node {:?}. Expected {}. Actual {}", node5, 130f32, location.x);
+    assert_eq!(location.y, 0f32, "y of node {:?}. Expected {}. Actual {}", node5, 0f32, location.y);
+    let Layout { size, location, .. } = taffy.layout(node6).unwrap();
+    assert_eq!(size.width, 20f32, "width of node {:?}. Expected {}. Actual {}", node6, 20f32, size.width);
+    assert_eq!(size.height, 100f32, "height of node {:?}. Expected {}. Actual {}", node6, 100f32, size.height);
+    assert_eq!(location.x, 140f32, "x of node {:?}. Expected {}. Actual {}", node6, 140f32, location.x);
+    assert_eq!(location.y, 0f32, "y of node {:?}. Expected {}. Actual {}", node6, 0f32, location.y);
+    let Layout { size, location, .. } = taffy.layout(node7).unwrap();
+    assert_eq!(size.width, 30f32, "width of node {:?}. Expected {}. Actual {}", node7, 30f32, size.width);
+    assert_eq!(size.height, 100f32, "height of node {:?}. Expected {}. Actual {}", node7, 100f32, size.height);
+    assert_eq!(location.x, 160f32, "x of node {:?}. Expected {}. Actual {}", node7, 160f32, location.x);
+    assert_eq!(location.y, 0f32, "y of node {:?}. Expected {}. Actual {}", node7, 0f32, location.y);
+}
diff --git a/tests/generated/grid_auto_rows.rs b/tests/generated/grid_auto_rows.rs
new file mode 100644
index 000000000..692120078
--- /dev/null
+++ b/tests/generated/grid_auto_rows.rs
@@ -0,0 +1,80 @@
+#[test]
+fn grid_auto_rows() {
+    #[allow(unused_imports)]
+    use taffy::{prelude::*, tree::Layout};
+    let mut taffy = taffy::Taffy::new();
+    let node0 = taffy
+        .new_leaf(taffy::style::Style {
+            grid_row: taffy::geometry::Line { start: line(-4i16), end: taffy::style::GridPlacement::Auto },
+            ..Default::default()
+        })
+        .unwrap();
+    let node1 = taffy.new_leaf(taffy::style::Style { ..Default::default() }).unwrap();
+    let node2 = taffy.new_leaf(taffy::style::Style { ..Default::default() }).unwrap();
+    let node3 = taffy.new_leaf(taffy::style::Style { ..Default::default() }).unwrap();
+    let node4 = taffy.new_leaf(taffy::style::Style { ..Default::default() }).unwrap();
+    let node5 = taffy.new_leaf(taffy::style::Style { ..Default::default() }).unwrap();
+    let node6 = taffy.new_leaf(taffy::style::Style { ..Default::default() }).unwrap();
+    let node7 = taffy.new_leaf(taffy::style::Style { ..Default::default() }).unwrap();
+    let node = taffy
+        .new_with_children(
+            taffy::style::Style {
+                display: taffy::style::Display::Grid,
+                grid_template_rows: vec![length(40f32)],
+                grid_template_columns: vec![length(100f32)],
+                grid_auto_rows: vec![length(10f32), length(20f32), length(30f32)],
+                ..Default::default()
+            },
+            &[node0, node1, node2, node3, node4, node5, node6, node7],
+        )
+        .unwrap();
+    taffy.compute_layout(node, taffy::geometry::Size::MAX_CONTENT).unwrap();
+    println!("\nComputed tree:");
+    taffy::util::print_tree(&taffy, node);
+    println!();
+    let Layout { size, location, .. } = taffy.layout(node).unwrap();
+    assert_eq!(size.width, 100f32, "width of node {:?}. Expected {}. Actual {}", node, 100f32, size.width);
+    assert_eq!(size.height, 180f32, "height of node {:?}. Expected {}. Actual {}", node, 180f32, size.height);
+    assert_eq!(location.x, 0f32, "x of node {:?}. Expected {}. Actual {}", node, 0f32, location.x);
+    assert_eq!(location.y, 0f32, "y of node {:?}. Expected {}. Actual {}", node, 0f32, location.y);
+    let Layout { size, location, .. } = taffy.layout(node0).unwrap();
+    assert_eq!(size.width, 100f32, "width of node {:?}. Expected {}. Actual {}", node0, 100f32, size.width);
+    assert_eq!(size.height, 20f32, "height of node {:?}. Expected {}. Actual {}", node0, 20f32, size.height);
+    assert_eq!(location.x, 0f32, "x of node {:?}. Expected {}. Actual {}", node0, 0f32, location.x);
+    assert_eq!(location.y, 0f32, "y of node {:?}. Expected {}. Actual {}", node0, 0f32, location.y);
+    let Layout { size, location, .. } = taffy.layout(node1).unwrap();
+    assert_eq!(size.width, 100f32, "width of node {:?}. Expected {}. Actual {}", node1, 100f32, size.width);
+    assert_eq!(size.height, 30f32, "height of node {:?}. Expected {}. Actual {}", node1, 30f32, size.height);
+    assert_eq!(location.x, 0f32, "x of node {:?}. Expected {}. Actual {}", node1, 0f32, location.x);
+    assert_eq!(location.y, 20f32, "y of node {:?}. Expected {}. Actual {}", node1, 20f32, location.y);
+    let Layout { size, location, .. } = taffy.layout(node2).unwrap();
+    assert_eq!(size.width, 100f32, "width of node {:?}. Expected {}. Actual {}", node2, 100f32, size.width);
+    assert_eq!(size.height, 40f32, "height of node {:?}. Expected {}. Actual {}", node2, 40f32, size.height);
+    assert_eq!(location.x, 0f32, "x of node {:?}. Expected {}. Actual {}", node2, 0f32, location.x);
+    assert_eq!(location.y, 50f32, "y of node {:?}. Expected {}. Actual {}", node2, 50f32, location.y);
+    let Layout { size, location, .. } = taffy.layout(node3).unwrap();
+    assert_eq!(size.width, 100f32, "width of node {:?}. Expected {}. Actual {}", node3, 100f32, size.width);
+    assert_eq!(size.height, 10f32, "height of node {:?}. Expected {}. Actual {}", node3, 10f32, size.height);
+    assert_eq!(location.x, 0f32, "x of node {:?}. Expected {}. Actual {}", node3, 0f32, location.x);
+    assert_eq!(location.y, 90f32, "y of node {:?}. Expected {}. Actual {}", node3, 90f32, location.y);
+    let Layout { size, location, .. } = taffy.layout(node4).unwrap();
+    assert_eq!(size.width, 100f32, "width of node {:?}. Expected {}. Actual {}", node4, 100f32, size.width);
+    assert_eq!(size.height, 20f32, "height of node {:?}. Expected {}. Actual {}", node4, 20f32, size.height);
+    assert_eq!(location.x, 0f32, "x of node {:?}. Expected {}. Actual {}", node4, 0f32, location.x);
+    assert_eq!(location.y, 100f32, "y of node {:?}. Expected {}. Actual {}", node4, 100f32, location.y);
+    let Layout { size, location, .. } = taffy.layout(node5).unwrap();
+    assert_eq!(size.width, 100f32, "width of node {:?}. Expected {}. Actual {}", node5, 100f32, size.width);
+    assert_eq!(size.height, 30f32, "height of node {:?}. Expected {}. Actual {}", node5, 30f32, size.height);
+    assert_eq!(location.x, 0f32, "x of node {:?}. Expected {}. Actual {}", node5, 0f32, location.x);
+    assert_eq!(location.y, 120f32, "y of node {:?}. Expected {}. Actual {}", node5, 120f32, location.y);
+    let Layout { size, location, .. } = taffy.layout(node6).unwrap();
+    assert_eq!(size.width, 100f32, "width of node {:?}. Expected {}. Actual {}", node6, 100f32, size.width);
+    assert_eq!(size.height, 10f32, "height of node {:?}. Expected {}. Actual {}", node6, 10f32, size.height);
+    assert_eq!(location.x, 0f32, "x of node {:?}. Expected {}. Actual {}", node6, 0f32, location.x);
+    assert_eq!(location.y, 150f32, "y of node {:?}. Expected {}. Actual {}", node6, 150f32, location.y);
+    let Layout { size, location, .. } = taffy.layout(node7).unwrap();
+    assert_eq!(size.width, 100f32, "width of node {:?}. Expected {}. Actual {}", node7, 100f32, size.width);
+    assert_eq!(size.height, 20f32, "height of node {:?}. Expected {}. Actual {}", node7, 20f32, size.height);
+    assert_eq!(location.x, 0f32, "x of node {:?}. Expected {}. Actual {}", node7, 0f32, location.x);
+    assert_eq!(location.y, 160f32, "y of node {:?}. Expected {}. Actual {}", node7, 160f32, location.y);
+}
diff --git a/tests/generated/mod.rs b/tests/generated/mod.rs
index 691b69583..9b9a83243 100644
--- a/tests/generated/mod.rs
+++ b/tests/generated/mod.rs
@@ -447,6 +447,8 @@ mod grid_aspect_ratio_overriden_by_explicit_sizes;
 #[cfg(feature = "grid")]
 mod grid_aspect_ratio_overriden_by_explicit_sizes_flex;
 #[cfg(feature = "grid")]
+mod grid_auto_columns;
+#[cfg(feature = "grid")]
 mod grid_auto_columns_fixed_width;
 #[cfg(feature = "grid")]
 mod grid_auto_fill_fixed_size;
@@ -455,6 +457,8 @@ mod grid_auto_fill_with_empty_auto_track;
 #[cfg(feature = "grid")]
 mod grid_auto_fit_with_empty_auto_track;
 #[cfg(feature = "grid")]
+mod grid_auto_rows;
+#[cfg(feature = "grid")]
 mod grid_auto_single_item;
 #[cfg(feature = "grid")]
 mod grid_auto_single_item_fixed_width;