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 · 9707996 · 9707996
1 parent 8063a34
commit 9707996
Show file tree

Hide file tree

Showing 20 changed files with 832 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,6 +228,11 @@ 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 {

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>
diff --git a/test_fixtures/grid_max_width_greater_than_max_content.html b/test_fixtures/grid_max_width_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 id="test-root" style="display: grid; grid-template-columns: max-content;">
+ <div style="display: grid; max-width: 400px; 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>