Optimise flexbox layouts with min/max sizes (#413)
* Remove 2-pass min/max code path + implement main size min/max clamping within determine_container_main_size method
* Fix typo in flex_grow_within_constrained_min_max_column test
* Implement cross-size min/max clamping inline in algorithm
* Remove debug log
* Remove commented code
da796c6e408e255c97df2be8656c270de69c13b7 · by Nico Burns · (3 years ago) · parent 7c2c77842 · browse the tree at this commit
Semantic delta · 9
tests/generated/flex_grow_within_constrained_min_max_column.rs
diff --git a/src/compute/flexbox.rs b/src/compute/flexbox.rs
index a5b3916a2..c72416232 100644
--- a/src/compute/flexbox.rs
+++ b/src/compute/flexbox.rs
@@ -143,6 +143,10 @@ struct AlgoConstants {
/// Is the wrap direction inverted
is_wrap_reverse: bool,
+ /// The item's min_size style
+ min_size: Size<Option<f32>>,
+ /// The item's max_size style
+ max_size: Size<Option<f32>>,
/// The margin of this section
margin: Rect<f32>,
/// The border of this section
@@ -179,10 +183,6 @@ pub fn compute(
run_mode: RunMode,
) -> SizeAndBaselines {
let style = tree.style(node);
- let has_min_max_sizes = style.min_size.width.is_defined()
- || style.min_size.height.is_defined()
- || style.max_size.width.is_defined()
- || style.max_size.height.is_defined();
// Pull these out earlier to avoid borrowing issues
let aspect_ratio = style.aspect_ratio;
@@ -206,35 +206,9 @@ pub fn compute(
}
}
- if styled_based_known_dimensions.both_axis_defined() || !has_min_max_sizes {
- #[cfg(feature = "debug")]
- NODE_LOGGER.log("FLEX: single-pass");
- compute_preliminary(tree, node, styled_based_known_dimensions, parent_size, available_space, run_mode)
- } else {
- #[cfg(feature = "debug")]
- NODE_LOGGER.log("FLEX: two-pass");
- let first_pass = compute_preliminary(
- tree,
- node,
- styled_based_known_dimensions,
- parent_size,
- available_space,
- RunMode::ComputeSize,
- )
- .size;
-
- let clamped_first_pass_size = first_pass.maybe_clamp(min_size, max_size);
-
- compute_preliminary(
- tree,
- node,
- styled_based_known_dimensions
- .zip_map(clamped_first_pass_size, |known, first_pass| known.or_else(|| first_pass.into())),
- parent_size,
- available_space,
- run_mode,
- )
- }
+ #[cfg(feature = "debug")]
+ NODE_LOGGER.log("FLEX: single-pass");
+ compute_preliminary(tree, node, styled_based_known_dimensions, parent_size, available_space, run_mode)
}
/// Compute a preliminary size for an item
@@ -451,6 +425,7 @@ fn compute_constants(
let is_wrap = matches!(style.flex_wrap, FlexWrap::Wrap | FlexWrap::WrapReverse);
let is_wrap_reverse = style.flex_wrap == FlexWrap::WrapReverse;
+ let aspect_ratio = style.aspect_ratio;
let margin = style.margin.resolve_or_zero(parent_size.width);
let padding = style.padding.resolve_or_zero(parent_size.width);
let border = style.border.resolve_or_zero(parent_size.width);
@@ -473,6 +448,8 @@ fn compute_constants(
is_column,
is_wrap,
is_wrap_reverse,
+ min_size: style.min_size.maybe_resolve(parent_size).maybe_apply_aspect_ratio(aspect_ratio),
+ max_size: style.max_size.maybe_resolve(parent_size).maybe_apply_aspect_ratio(aspect_ratio),
margin,
border,
gap,
@@ -836,6 +813,8 @@ fn determine_container_main_size(
lines: &mut Vec<FlexLine<'_>>,
constants: &mut AlgoConstants,
) {
+ let main_padding_border = constants.padding_border.main_axis_sum(constants.dir);
+
let outer_main_size: f32 = constants.node_outer_size.main(constants.dir).unwrap_or_else(|| {
match main_axis_available_space {
AvailableSpace::Definite(main_axis_available_space) => {
@@ -855,7 +834,7 @@ fn determine_container_main_size(
})
.max_by(|a, b| a.total_cmp(b))
.unwrap_or(0.0);
- let size = longest_line_length + constants.padding_border.main_axis_sum(constants.dir);
+ let size = longest_line_length + main_padding_border;
if lines.len() > 1 {
f32_max(size, main_axis_available_space)
} else {
@@ -879,7 +858,7 @@ fn determine_container_main_size(
})
.max_by(|a, b| a.total_cmp(b))
.unwrap_or(0.0);
- longest_line_length + constants.padding_border.main_axis_sum(constants.dir)
+ longest_line_length + main_padding_border
}
AvailableSpace::MinContent | AvailableSpace::MaxContent => {
// Define a base main_size variable. This is mutated once for iteration over the outer
@@ -949,14 +928,12 @@ fn determine_container_main_size(
// Ultimately, this was not found by reading the spec, but by trial and error fixing tests to align with Webkit/Firefox output.
// (see the `flex_basis_unconstraint_row` and `flex_basis_uncontraint_column` generated tests which demonstrate this)
if constants.is_row {
- content_main_size
- .maybe_clamp(style_min, style_max)
- .max(constants.padding_border.main_axis_sum(constants.dir))
+ content_main_size.maybe_clamp(style_min, style_max).max(main_padding_border)
} else {
content_main_size
.max(item.flex_basis)
.maybe_clamp(style_min, style_max)
- .max(constants.padding_border.main_axis_sum(constants.dir))
+ .max(main_padding_border)
}
}
};
@@ -1017,10 +994,15 @@ fn determine_container_main_size(
main_size = f32_max(main_size, item_main_size_sum + gap_sum)
}
- main_size + constants.padding_border.main_axis_sum(constants.dir)
+ main_size + main_padding_border
}
}
});
+
+ let outer_main_size = outer_main_size
+ .maybe_clamp(constants.min_size.main(constants.dir), constants.max_size.main(constants.dir))
+ .max(main_padding_border);
+
// let outer_main_size = inner_main_size + constants.padding_border.main_axis_sum(constants.dir);
let inner_main_size = outer_main_size - constants.padding_border.main_axis_sum(constants.dir);
constants.container_size.set_main(constants.dir, outer_main_size);
@@ -1368,9 +1350,15 @@ fn calculate_cross_size(flex_lines: &mut [FlexLine], node_size: Size<Option<f32>
AlignContent::Stretch | AlignContent::SpaceEvenly | AlignContent::SpaceAround
))
{
- flex_lines[0].cross_size =
- (node_size.cross(constants.dir).maybe_sub(constants.padding_border.cross_axis_sum(constants.dir)))
- .unwrap_or(0.0);
+ let cross_axis_padding_border = constants.padding_border.cross_axis_sum(constants.dir);
+ let cross_min_size = constants.min_size.cross(constants.dir);
+ let cross_max_size = constants.max_size.cross(constants.dir);
+ flex_lines[0].cross_size = node_size
+ .cross(constants.dir)
+ .maybe_clamp(cross_min_size, cross_max_size)
+ .maybe_sub(cross_axis_padding_border)
+ .maybe_max(0.0)
+ .unwrap_or(0.0);
} else {
for line in flex_lines.iter_mut() {
// 1. Collect all the flex items whose inline-axis is parallel to the main-axis, whose
@@ -1413,15 +1401,23 @@ fn calculate_cross_size(flex_lines: &mut [FlexLine], node_size: Size<Option<f32>
/// increase the cross size of each flex line by equal amounts such that the sum of their cross sizes exactly equals the flex container’s inner cross size.
#[inline]
fn handle_align_content_stretch(flex_lines: &mut [FlexLine], node_size: Size<Option<f32>>, constants: &AlgoConstants) {
- if constants.align_content == AlignContent::Stretch && node_size.cross(constants.dir).is_some() {
+ if constants.align_content == AlignContent::Stretch {
+ let cross_axis_padding_border = constants.padding_border.cross_axis_sum(constants.dir);
+ let cross_min_size = constants.min_size.cross(constants.dir);
+ let cross_max_size = constants.max_size.cross(constants.dir);
+ let container_min_inner_cross = node_size
+ .cross(constants.dir)
+ .or(cross_min_size)
+ .maybe_clamp(cross_min_size, cross_max_size)
+ .maybe_sub(cross_axis_padding_border)
+ .maybe_max(0.0)
+ .unwrap_or(0.0);
+
let total_cross_axis_gap = sum_axis_gaps(constants.gap.cross(constants.dir), flex_lines.len());
- let total_cross: f32 = flex_lines.iter().map(|line| line.cross_size).sum::<f32>() + total_cross_axis_gap;
- let inner_cross =
- (node_size.cross(constants.dir).maybe_sub(constants.padding_border.cross_axis_sum(constants.dir)))
- .unwrap_or(0.0);
+ let lines_total_cross: f32 = flex_lines.iter().map(|line| line.cross_size).sum::<f32>() + total_cross_axis_gap;
- if total_cross < inner_cross {
- let remaining = inner_cross - total_cross;
+ if lines_total_cross < container_min_inner_cross {
+ let remaining = container_min_inner_cross - lines_total_cross;
let addition = remaining / flex_lines.len() as f32;
flex_lines.iter_mut().for_each(|line| line.cross_size += addition);
}
@@ -1665,9 +1661,12 @@ fn determine_container_cross_size(
let total_line_cross_size: f32 = flex_lines.iter().map(|line| line.cross_size).sum::<f32>();
let padding_border_sum = constants.padding_border.cross_axis_sum(constants.dir);
+ let min_cross_size = constants.min_size.cross(constants.dir);
+ let max_cross_size = constants.max_size.cross(constants.dir);
let outer_container_size = node_size
.cross(constants.dir)
.unwrap_or(total_line_cross_size + total_cross_axis_gap + padding_border_sum)
+ .maybe_clamp(min_cross_size, max_cross_size)
.max(padding_border_sum);
let inner_container_size = outer_container_size - padding_border_sum;
diff --git a/src/style/dimension.rs b/src/style/dimension.rs
index 21e3d7f1c..d4caade8e 100644
--- a/src/style/dimension.rs
+++ b/src/style/dimension.rs
@@ -135,11 +135,6 @@ impl From<LengthPercentageAuto> for Dimension {
}
impl Dimension {
- /// Is this value defined?
- pub(crate) fn is_defined(self) -> bool {
- matches!(self, Dimension::Points(_) | Dimension::Percent(_))
- }
-
/// Get Points value if value is Points variant
#[cfg(feature = "grid")]
pub fn into_option(self) -> Option<f32> {
diff --git a/test_fixtures/flex_grow_within_constrained_min_max_column.html b/test_fixtures/flex_grow_within_constrained_min_max_column.html
index 51c19d853..9cb289f1c 100644
--- a/test_fixtures/flex_grow_within_constrained_min_max_column.html
+++ b/test_fixtures/flex_grow_within_constrained_min_max_column.html
@@ -9,7 +9,7 @@
<head/>
<body>
-<div id="test-root" style="min-height: 100px; max-height: 200px flex-direction: column;">
+<div id="test-root" style="min-height: 100px; max-height: 200px; flex-direction: column;">
<div style="flex-grow:1;"></div>
<div style="height: 50px;"></div>
</div>
diff --git a/tests/generated/flex_grow_within_constrained_min_max_column.rs b/tests/generated/flex_grow_within_constrained_min_max_column.rs
index c83aafc73..8e0fb2fb9 100644
--- a/tests/generated/flex_grow_within_constrained_min_max_column.rs
+++ b/tests/generated/flex_grow_within_constrained_min_max_column.rs
@@ -14,7 +14,9 @@ fn flex_grow_within_constrained_min_max_column() {
let node = taffy
.new_with_children(
taffy::style::Style {
+ flex_direction: taffy::style::FlexDirection::Column,
min_size: taffy::geometry::Size { width: auto(), height: taffy::style::Dimension::Points(100f32) },
+ max_size: taffy::geometry::Size { width: auto(), height: taffy::style::Dimension::Points(200f32) },
..Default::default()
},
&[node0, node1],
@@ -31,12 +33,12 @@ fn flex_grow_within_constrained_min_max_column() {
assert_eq!(location.y, 0f32, "y of node {:?}. Expected {}. Actual {}", node.data(), 0f32, location.y);
let Layout { size, location, .. } = taffy.layout(node0).unwrap();
assert_eq!(size.width, 0f32, "width of node {:?}. Expected {}. Actual {}", node0.data(), 0f32, size.width);
- assert_eq!(size.height, 100f32, "height of node {:?}. Expected {}. Actual {}", node0.data(), 100f32, size.height);
+ assert_eq!(size.height, 50f32, "height of node {:?}. Expected {}. Actual {}", node0.data(), 50f32, size.height);
assert_eq!(location.x, 0f32, "x of node {:?}. Expected {}. Actual {}", node0.data(), 0f32, location.x);
assert_eq!(location.y, 0f32, "y of node {:?}. Expected {}. Actual {}", node0.data(), 0f32, location.y);
let Layout { size, location, .. } = taffy.layout(node1).unwrap();
assert_eq!(size.width, 0f32, "width of node {:?}. Expected {}. Actual {}", node1.data(), 0f32, size.width);
assert_eq!(size.height, 50f32, "height of node {:?}. Expected {}. Actual {}", node1.data(), 50f32, size.height);
assert_eq!(location.x, 0f32, "x of node {:?}. Expected {}. Actual {}", node1.data(), 0f32, location.x);
- assert_eq!(location.y, 0f32, "y of node {:?}. Expected {}. Actual {}", node1.data(), 0f32, location.y);
+ assert_eq!(location.y, 50f32, "y of node {:?}. Expected {}. Actual {}", node1.data(), 50f32, location.y);
}