Fix: Make CSS Grid algorithm correctly apply max width/height and ava…

…ilable space when it is the root node (#491) * Add viewport control to gentest setup * Add tests for max-width and available space on a CSS Grid root node * Only treat definite available grid space as indefinite when container size is indefinite, and only do so for the final two track sizing steps. (cherry picked from commit 9958624)
DioxusLabs · Aug 14, 2023 · ae87fd0 · ae87fd0
1 parent 3803328
commit ae87fd0
Show file tree

Hide file tree

Showing 20 changed files with 834 additions and 58 deletions.
diff --git a/scripts/gentest/src/main.rs b/scripts/gentest/src/main.rs
@@ -10,6 +10,47 @@ use quote::{format_ident, quote};
 use serde_json::Value;
 use syn::Ident;
 
+macro_rules! dim_quoted_renamed {
+    ($obj:ident, $in_name:ident, $out_name:ident, $value_mapper:ident, $default:expr) => {
+        let $out_name = match $obj.get(stringify!($in_name)) {
+            Some(Value::Object(ref value)) => {
+                let dim = $value_mapper(value);
+                quote!($out_name: #dim,)
+            }
+            _ => {
+                let dim = $default;
+                quote!($out_name: #dim,)
+            }
+        };
+    };
+}
+
+macro_rules! dim_quoted {
+    ($obj:ident, $dim_name:ident, $value_mapper: ident, $default:expr) => {
+        dim_quoted_renamed!($obj, $dim_name, $dim_name, $value_mapper, $default)
+    };
+}
+
+macro_rules! edges_quoted {
+    ($style:ident, $val:ident, $value_mapper:ident, $default_value: expr) => {
+        let $val = match $style[stringify!($val)] {
+            Value::Object(ref value) => {
+                dim_quoted!(value, left, $value_mapper, $default_value);
+                dim_quoted!(value, right, $value_mapper, $default_value);
+                dim_quoted!(value, top, $value_mapper, $default_value);
+                dim_quoted!(value, bottom, $value_mapper, $default_value);
+
+                let edges = quote!(taffy::geometry::Rect {
+                    #left #right #top #bottom
+                });
+
+                quote!($val: #edges,)
+            },
+            _ => quote!(),
+        };
+    };
+}
+
 #[tokio::main]
 async fn main() {
     env_logger::init();
@@ -127,6 +168,20 @@ fn generate_test(name: impl AsRef<str>, description: &Value) -> TokenStream {
 
     let set_rounding_mode = if use_rounding { quote!() } else { quote!(taffy.disable_rounding();) };
 
+    // Compute available space
+    let viewport = &description["viewport"];
+    let available_space = if viewport["width"]["unit"] == "max-content" && viewport["height"]["unit"] == "max-content" {
+        quote!(taffy::geometry::Size::MAX_CONTENT)
+    } else {
+        dim_quoted!(viewport, width, generate_available_space, quote!(taffy::style::AvailableSpace::MAX_CONTENT));
+        dim_quoted!(viewport, height, generate_available_space, quote!(taffy::style::AvailableSpace::MAX_CONTENT));
+        quote!(
+            taffy::geometry::Size {
+                #width #height
+            }
+        )
+    };
+
     quote!(
         #[test]
         fn #name() {
@@ -136,7 +191,7 @@ fn generate_test(name: impl AsRef<str>, description: &Value) -> TokenStream {
             let mut taffy = taffy::Taffy::new();
             #set_rounding_mode
             #node_description
-            taffy.compute_layout(node, taffy::geometry::Size::MAX_CONTENT).unwrap();
+            taffy.compute_layout(node, #available_space).unwrap();
 
             println!("\nComputed tree:");
             taffy::debug::print_tree(&taffy, node);
@@ -191,47 +246,6 @@ fn generate_assertions(ident: &str, node: &Value, use_rounding: bool) -> TokenSt
     }
 }
 
-macro_rules! dim_quoted_renamed {
-    ($obj:ident, $in_name:ident, $out_name:ident, $value_mapper:ident, $default:expr) => {
-        let $out_name = match $obj.get(stringify!($in_name)) {
-            Some(Value::Object(ref value)) => {
-                let dim = $value_mapper(value);
-                quote!($out_name: #dim,)
-            }
-            _ => {
-                let dim = $default;
-                quote!($out_name: #dim,)
-            }
-        };
-    };
-}
-
-macro_rules! dim_quoted {
-    ($obj:ident, $dim_name:ident, $value_mapper: ident, $default:expr) => {
-        dim_quoted_renamed!($obj, $dim_name, $dim_name, $value_mapper, $default)
-    };
-}
-
-macro_rules! edges_quoted {
-    ($style:ident, $val:ident, $value_mapper:ident, $default_value: expr) => {
-        let $val = match $style[stringify!($val)] {
-            Value::Object(ref value) => {
-                dim_quoted!(value, left, $value_mapper, $default_value);
-                dim_quoted!(value, right, $value_mapper, $default_value);
-                dim_quoted!(value, top, $value_mapper, $default_value);
-                dim_quoted!(value, bottom, $value_mapper, $default_value);
-
-                let edges = quote!(taffy::geometry::Rect {
-                    #left #right #top #bottom
-                });
-
-                quote!($val: #edges,)
-            },
-            _ => quote!(),
-        };
-    };
-}
-
 fn generate_node(ident: &str, node: &Value) -> TokenStream {
     let style = &node["style"];
 
@@ -660,6 +674,24 @@ fn generate_dimension(dimen: &serde_json::Map<String, Value>) -> TokenStream {
     }
 }
 
+fn generate_available_space(dimen: &serde_json::Map<String, Value>) -> TokenStream {
+    let unit = dimen.get("unit").unwrap();
+    let value = || dimen.get("value").unwrap().as_f64().unwrap() as f32;
+
+    match unit {
+        Value::String(ref unit) => match unit.as_ref() {
+            "max-content" => quote!(taffy::style::AvailableSpace::MaxContent),
+            "min-content" => quote!(taffy::style::AvailableSpace::MaxContent),
+            "points" => {
+                let value = value();
+                quote!(taffy::style::AvailableSpace::Definite(#value))
+            }
+            _ => unreachable!(),
+        },
+        _ => unreachable!(),
+    }
+}
+
 fn generate_grid_auto_flow(auto_flow: &serde_json::Map<String, Value>) -> TokenStream {
     let direction = auto_flow.get("direction").unwrap().as_str().unwrap();
     let algorithm = auto_flow.get("algorithm").unwrap().as_str().unwrap();

diff --git a/scripts/gentest/test_base_style.css b/scripts/gentest/test_base_style.css
@@ -31,6 +31,15 @@ body > * {
   border-color: red;
 }
 
+.viewport {
+  display: flex;
+  align-items: start;
+  justify-content: start;
+  /* Defaults: can be overriden in individual tests */
+  width: max-content;
+  height: max-content;
+}
+
 #test-root {
   font-family: ahem;
   line-height: 1;

diff --git a/scripts/gentest/test_helper.js b/scripts/gentest/test_helper.js
@@ -81,6 +81,20 @@ class TrackSizingParser {
 
 }
 
+function parseViewportConstraint(e) {
+  if (e.parentNode.classList.contains('viewport')) {
+    return {
+      width: parseDimension(e.parentNode.style.width || 'max-content'),
+      height: parseDimension(e.parentNode.style.height || 'max-content'),
+    }
+  } else {
+    return {
+      width: { unit: 'max-content' },
+      height: { unit: 'max-content' },
+    }
+  }
+}
+
 function parseRepetition(input) {
   if (input === "auto-fill") return { unit: 'auto-fill' };
   if (input === "auto-fit") return { unit: 'auto-fit' };
@@ -283,6 +297,8 @@ function describeElement(e) {
     // Whether the test should enable rounding
     useRounding: e.getAttribute("data-test-rounding") !== "false",
 
+    viewport: parseViewportConstraint(e),
+
     children: Array.from(e.children).map(c => describeElement(c)),
   }
 }

diff --git a/src/compute/grid/mod.rs b/src/compute/grid/mod.rs
@@ -149,14 +149,10 @@ pub fn compute(
 
     let constrained_available_space = known_dimensions
         .or(size)
-        .maybe_clamp(min_size, max_size)
         .map(|size| size.map(AvailableSpace::Definite))
-        .unwrap_or(available_space.map(|space| match space {
-            // Available grid space should not depend on Definite available space as a grid is allowed
-            // to expand beyond it's available space
-            AvailableSpace::Definite(_) => AvailableSpace::MaxContent,
-            _ => space,
-        }));
+        .unwrap_or(available_space)
+        .maybe_clamp(min_size, max_size)
+        .maybe_max(padding_border_size);
 
     let available_grid_space = Size {
         width: constrained_available_space
@@ -167,7 +163,7 @@ pub fn compute(
             .map_definite_value(|space| space - padding.vertical_axis_sum() - border.vertical_axis_sum()),
     };
 
-    let outer_node_size = known_dimensions.or(size.maybe_clamp(min_size, max_size).or(min_size));
+    let outer_node_size = known_dimensions.or(size).maybe_clamp(min_size, max_size).maybe_max(padding_border_size);
     let mut inner_node_size = Size {
         width: outer_node_size.width.map(|space| space - padding.horizontal_axis_sum() - border.horizontal_axis_sum()),
         height: outer_node_size.height.map(|space| space - padding.vertical_axis_sum() - border.vertical_axis_sum()),
@@ -232,16 +228,23 @@ pub fn compute(
     let initial_row_sum = rows.iter().map(|track| track.base_size).sum::<f32>();
     inner_node_size.height = inner_node_size.height.or_else(|| initial_row_sum.into());
 
+    #[cfg(feature = "debug")]
+    NODE_LOGGER.labelled_debug_log("initial_column_sum", initial_column_sum);
+    #[cfg(feature = "debug")]
+    NODE_LOGGER.labelled_debug_log("initial_row_sum", initial_row_sum);
+
     // 6. Compute container size
     let resolved_style_size = known_dimensions.or(style.size.maybe_resolve(parent_size));
     let container_border_box = Size {
         width: resolved_style_size
             .get(AbstractAxis::Inline)
             .unwrap_or_else(|| initial_column_sum + padding.horizontal_axis_sum() + border.horizontal_axis_sum())
+            .maybe_clamp(min_size.width, max_size.width)
             .max(padding_border_size.width),
         height: resolved_style_size
             .get(AbstractAxis::Block)
             .unwrap_or_else(|| initial_row_sum + padding.vertical_axis_sum() + border.vertical_axis_sum())
+            .maybe_clamp(min_size.height, max_size.height)
             .max(padding_border_size.height),
     };
     let container_content_box = Size {

diff --git a/src/compute/grid/track_sizing.rs b/src/compute/grid/track_sizing.rs
@@ -327,6 +327,19 @@ pub(super) fn track_sizing_algorithm<Tree: LayoutTree>(
     // Distributes free space (if any) to tracks with FINITE growth limits, up to their limits.
     maximise_tracks(axis_tracks, inner_node_size.get(axis), available_grid_space.get(axis));
 
+    // For the purpose of the final two expansion steps ("Expand Flexible Tracks" and "Stretch auto Tracks"), we only want to expand
+    // into space generated by the grid container's size (as defined by either it's preferred size style or by it's parent node through
+    // something like stretch alignment), not just any available space. To do this we map definite available space to AvailableSpace::MaxContent
+    // in the case that inner_node_size is None
+    let axis_available_space_for_expansion = if let Some(available_space) = inner_node_size.get(axis) {
+        AvailableSpace::Definite(available_space)
+    } else {
+        match available_grid_space.get(axis) {
+            AvailableSpace::MinContent => AvailableSpace::MinContent,
+            AvailableSpace::MaxContent | AvailableSpace::Definite(_) => AvailableSpace::MaxContent,
+        }
+    };
+
     // 11.7. Expand Flexible Tracks
     // This step sizes flexible tracks using the largest value it can assign to an fr without exceeding the available space.
     expand_flexible_tracks(
@@ -336,13 +349,13 @@ pub(super) fn track_sizing_algorithm<Tree: LayoutTree>(
         items,
         axis_min_size,
         axis_max_size,
-        available_grid_space,
+        axis_available_space_for_expansion,
         inner_node_size,
     );
 
     // 11.8. Stretch auto Tracks
     // This step expands tracks that have an auto max track sizing function by dividing any remaining positive, definite free space equally amongst them.
-    stretch_auto_tracks(axis, axis_tracks, axis_min_size, available_grid_space);
+    stretch_auto_tracks(axis_tracks, axis_min_size, axis_available_space_for_expansion);
 }
 
 /// Whether it is a minimum or maximum size's space being distributed
@@ -1083,11 +1096,11 @@ fn expand_flexible_tracks(
     items: &mut [GridItem],
     axis_min_size: Option<f32>,
     axis_max_size: Option<f32>,
-    available_grid_space: Size<AvailableSpace>,
+    axis_available_space_for_expansion: AvailableSpace,
     inner_node_size: Size<Option<f32>>,
 ) {
     // First, find the grid’s used flex fraction:
-    let flex_fraction = match available_grid_space.get(axis) {
+    let flex_fraction = match axis_available_space_for_expansion {
         // If the free space is zero:
         //    The used flex fraction is zero.
         // Otherwise, if the free space is a definite length:
@@ -1237,10 +1250,9 @@ fn find_size_of_fr(tracks: &[GridTrack], space_to_fill: f32) -> f32 {
 /// This step expands tracks that have an auto max track sizing function by dividing any remaining positive, definite free space equally amongst them.
 #[inline(always)]
 fn stretch_auto_tracks(
-    axis: AbstractAxis,
     axis_tracks: &mut [GridTrack],
     axis_min_size: Option<f32>,
-    available_grid_space: Size<AvailableSpace>,
+    axis_available_space_for_expansion: AvailableSpace,
 ) {
     let num_auto_tracks =
         axis_tracks.iter().filter(|track| track.max_track_sizing_function == MaxTrackSizingFunction::Auto).count();
@@ -1249,8 +1261,8 @@ fn stretch_auto_tracks(
 
         // If the free space is indefinite, but the grid container has a definite min-width/height
         // use that size to calculate the free space for this step instead.
-        let free_space = if available_grid_space.get(axis).is_definite() {
-            available_grid_space.get(axis).compute_free_space(used_space)
+        let free_space = if axis_available_space_for_expansion.is_definite() {
+            axis_available_space_for_expansion.compute_free_space(used_space)
         } else {
             match axis_min_size {
                 Some(size) => size - used_space,

diff --git a/test_fixtures/grid_available_space_greater_than_max_content.html b/test_fixtures/grid_available_space_greater_than_max_content.html
@@ -0,0 +1,20 @@
+<!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 class="viewport" style="width: 400px;">
+  <div id="test-root" style="display: grid; grid-template-columns: auto auto;">
+    <div>HH&ZeroWidthSpace;HH&ZeroWidthSpace;HH&ZeroWidthSpace;HH</div>
+    <div>HH&ZeroWidthSpace;HH&ZeroWidthSpace;HH&ZeroWidthSpace;HH</div>
+  </div>
+</div>
+
+</body>
+</html>
diff --git a/test_fixtures/grid_available_space_smaller_than_max_content.html b/test_fixtures/grid_available_space_smaller_than_max_content.html
@@ -0,0 +1,20 @@
+<!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 class="viewport" style="width: 80px;">
+  <div id="test-root" style="display: grid; grid-template-columns: auto auto;">
+    <div>HH&ZeroWidthSpace;HH&ZeroWidthSpace;HH&ZeroWidthSpace;HH</div>
+    <div>HH&ZeroWidthSpace;HH&ZeroWidthSpace;HH&ZeroWidthSpace;HH</div>
+  </div>
+</div>
+
+</body>
+</html>
diff --git a/test_fixtures/grid_available_space_smaller_than_min_content.html b/test_fixtures/grid_available_space_smaller_than_min_content.html
@@ -0,0 +1,20 @@
+<!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 class="viewport" style="width: 60px;">
+  <div id="test-root" style="display: grid; grid-template-columns: auto auto;">
+    <div>HHHH&ZeroWidthSpace;HHHH</div>
+    <div>HHHH&ZeroWidthSpace;HHHH</div>
+  </div>
+</div>
+
+</body>
+</html>