Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for aspect ratio and min/max sizes #317

Merged
merged 40 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
0bff4e2
Add aspect ratio support to gentest script + add aspect ratio support…
nicoburns Jan 4, 2023
2801c19
Add maybe_apply_aspect_ratio helper to Size<Option<f32>>
nicoburns Jan 4, 2023
1dea4d0
Apply aspect ratio to leaf node style resolution
nicoburns Jan 4, 2023
fd88cf4
Apply aspect ratio when doing final placement of grid items
nicoburns Jan 4, 2023
da30c98
Make cargo gentest compile generator in release mode
nicoburns Jan 4, 2023
cd7a563
Add tests for aspect ratio of leaf nodes in grid containers
nicoburns Jan 4, 2023
d74c903
Apply aspect ratio to grid container styles
nicoburns Jan 4, 2023
796cf13
Apply aspect ratio to automatic minimum size
nicoburns Jan 4, 2023
fd57d84
Add tests for aspect ratio of content-sized leaf nodes
nicoburns Jan 4, 2023
fbc92dc
Use main size for flex-basis if flex-basis is not set
nicoburns Jan 4, 2023
807624e
Resolve size, min_size, max_size against aspect ratio when generating…
nicoburns Jan 4, 2023
b2f0905
Add gentests for aspect-ratio in flex-row (align-start)
nicoburns Jan 4, 2023
8efe05c
Move aspect ratio handling from text measure func to leaf.rs
nicoburns Jan 4, 2023
81ac768
Tweak clamping of flex item minimum sizes
nicoburns Jan 4, 2023
214b4e6
Add aspect ratio tests for flex-columns with align-items: start
nicoburns Jan 4, 2023
e2af292
FLexbox: don't apply aspect ratio to max_width when clamping stretche…
nicoburns Jan 4, 2023
03c9167
Add tests for aspect ratio with align-items:stretch (flexbox)
nicoburns Jan 4, 2023
a3895d5
Apply aspect ratio to absolute children of flex container
nicoburns Jan 5, 2023
63f12a2
Remove duplicate test
nicoburns Jan 5, 2023
9a81357
Namespace grid aspect ratio tests under grid
nicoburns Jan 5, 2023
1dcf6db
Absolute positioning: Apply aspect ratio to inset-generated width bef…
nicoburns Jan 7, 2023
8b92331
Add tests for aspect ratio height/width for absolutely positioned nodes
nicoburns Jan 7, 2023
5c66c23
Exclude hidden items from absolute positioning
nicoburns Jan 7, 2023
b147f78
Apply min/max sizes when finally positioning grid items
nicoburns Jan 7, 2023
c20a307
Add tests for interaction between inset and aspect ratio for absolute…
nicoburns Jan 8, 2023
a2ef38c
Always apply margins to absolutely positioned grid items
nicoburns Jan 8, 2023
befdef1
Rewrite flex item absolute positioning
nicoburns Jan 8, 2023
e9568cf
Take border into account when absolutely positioning grid items relat…
nicoburns Jan 8, 2023
fad8919
Add absolute positioning margin test
nicoburns Jan 8, 2023
f063cc0
Apply suggestions from code review
nicoburns Jan 8, 2023
01fadec
Update release notes
nicoburns Jan 8, 2023
29161f0
Add test for absolutely positioned fallback to static position of gri…
nicoburns Jan 8, 2023
db1bc72
Implement Add for Rect where the inner type implements Add
nicoburns Jan 8, 2023
dec6bc0
Apply aspect ratio and min/max size to flex container size
nicoburns Jan 8, 2023
0319a30
Cleanup TODOs in the flexbox algorithm
nicoburns Jan 8, 2023
e1ef9d8
Remove one more TODO
nicoburns Jan 8, 2023
1419e31
Add min/max size tests for absolutely positioned flex items
nicoburns Jan 11, 2023
94df9c5
Move absolute flex positioning aspect ratio tests under absolute name…
nicoburns Jan 11, 2023
1ded18f
Clamp available_space by min/max size for final layout
nicoburns Jan 11, 2023
355477e
Add tests for applying aspect ratio to min/max size of flex items
nicoburns Jan 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 22 additions & 31 deletions src/compute/flexbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,11 @@ fn compute_preliminary(
}

// TODO: Add step 4 according to spec: https://www.w3.org/TR/css-flexbox-1/#algo-main-container
nicoburns marked this conversation as resolved.
Show resolved Hide resolved

// 4. Determine the main size of the flex container

// This has already been done as part of compute_constants. The inner size is exposed as constants.node_inner_size.

// 9.3. Main Size Determination

// 5. Collect flex items into flex lines.
Expand Down Expand Up @@ -248,8 +253,7 @@ fn compute_preliminary(
resolve_flexible_lengths(tree, line, &constants, original_gap);
}

// TODO: Cleanup and make according to spec
// Not part of the spec from what i can see but seems correct
// Find the inner and outer used main_size of the container
constants.container_size.set_main(
constants.dir,
known_dimensions.main(constants.dir).unwrap_or({
Expand All @@ -262,7 +266,6 @@ fn compute_preliminary(
}
}),
);

constants.inner_container_size.set_main(
constants.dir,
constants.container_size.main(constants.dir) - constants.padding_border.main_axis_sum(constants.dir),
Expand Down Expand Up @@ -390,25 +393,25 @@ fn compute_constants(
let is_column = dir.is_column();
let is_wrap_reverse = style.flex_wrap == FlexWrap::WrapReverse;

let aspect_ratio = style.aspect_ratio;
let size = style.size.maybe_resolve(parent_size).maybe_apply_aspect_ratio(aspect_ratio);
let min_size = style.size.maybe_resolve(parent_size).maybe_apply_aspect_ratio(aspect_ratio);
let max_size = style.size.maybe_resolve(parent_size).maybe_apply_aspect_ratio(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);
let align_items = style.align_items.unwrap_or(crate::style::AlignItems::Stretch);

let padding_border = Rect {
left: padding.left + border.left,
right: padding.right + border.right,
top: padding.top + border.top,
bottom: padding.bottom + border.bottom,
};
let padding_border = padding + border;

let node_outer_size = Size {
width: known_dimensions
.width
.or_else(|| style.size.width.maybe_resolve(parent_size.width).maybe_sub(margin.horizontal_axis_sum())),
height: known_dimensions
.height
.or_else(|| style.size.height.maybe_resolve(parent_size.height).maybe_sub(margin.vertical_axis_sum())),
.or_else(|| size.width.maybe_sub(margin.horizontal_axis_sum()).maybe_clamp(min_size.width, max_size.width)),
height: known_dimensions.height.or_else(|| {
size.height.maybe_sub(margin.vertical_axis_sum()).maybe_clamp(min_size.height, max_size.height)
}),
};

let node_inner_size = Size {
Expand Down Expand Up @@ -555,7 +558,6 @@ fn determine_flex_base_size(
available_space: Size<AvailableSpace>,
flex_items: &mut Vec<FlexItem>,
) {
// TODO - this does not follow spec. See the TODOs below
for child in flex_items.iter_mut() {
let child_style = tree.style(child.node);

Expand All @@ -582,15 +584,16 @@ fn determine_flex_base_size(
// size the item under that constraint. The flex base size is the item’s
// resulting main size.

// TODO - Probably need to cover this case in future
// This is covered by the implementation of E below, which passes the available_space constraint
// through to the child size computation. It may need a separate implementation if/when D is implemented.

// D. Otherwise, if the used flex basis is content or depends on its
// available space, the available main size is infinite, and the flex item’s
// inline axis is parallel to the main axis, lay the item out using the rules
// for a box in an orthogonal flow [CSS3-WRITING-MODES]. The flex base size
// is the item’s max-content main size.

// TODO - Probably need to cover this case in future
// TODO if/when vertical writing modes are supported

// E. Otherwise, size the item into the available space using its used flex basis
// in place of its main size, treating a value of content as max-content.
Expand Down Expand Up @@ -783,7 +786,6 @@ fn resolve_flexible_lengths(

// TODO this should really only be set inside the if-statement below but
// that causes the target_main_size to never be set for some items

child
.outer_target_size
.set_main(constants.dir, child.target_size.main(constants.dir) + child.margin.main_axis_sum(constants.dir));
Expand Down Expand Up @@ -1792,8 +1794,8 @@ mod tests {
#![allow(clippy::redundant_clone)]

use crate::{
geometry::Size,
math::MaybeMath,
prelude::{Rect, Size},
resolve::ResolveOrZero,
style::{FlexWrap, Style},
Taffy,
Expand All @@ -1811,7 +1813,6 @@ mod tests {
let parent_size = Size::NONE;

let constants = super::compute_constants(tree.style(node_id).unwrap(), node_size, parent_size);
// let constants = super::compute_constants(&tree.nodes[node_id], node_size, parent_size);

assert!(constants.dir == style.flex_direction);
assert!(constants.is_row == style.flex_direction.is_row());
Expand All @@ -1822,21 +1823,11 @@ mod tests {
assert_eq!(constants.margin, margin);

let border = style.border.resolve_or_zero(parent_size);
assert_eq!(constants.border, border);

let padding = style.padding.resolve_or_zero(parent_size);

// TODO: Replace with something less hardcoded?
let padding_border = Rect {
left: padding.left + border.left,
right: padding.right + border.right,
top: padding.top + border.top,
bottom: padding.bottom + border.bottom,
};

let padding_border = padding + border;
assert_eq!(constants.border, border);
assert_eq!(constants.padding_border, padding_border);

// TODO: Replace with something less hardcoded?
let inner_size = Size {
width: node_size.width.maybe_sub(padding_border.horizontal_axis_sum()),
height: node_size.height.maybe_sub(padding_border.vertical_axis_sum()),
Expand Down
13 changes: 13 additions & 0 deletions src/geometry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,19 @@ impl<T: Default> Default for Rect<T> {
}
}

impl<U, T: Add<U>> Add<Rect<U>> for Rect<T> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This deserves a note in the release notes; it's surprisingly useful.

type Output = Rect<T::Output>;

fn add(self, rhs: Rect<U>) -> Self::Output {
Rect {
left: self.left + rhs.left,
right: self.right + rhs.right,
top: self.top + rhs.top,
bottom: self.bottom + rhs.bottom,
}
}
}

impl<T> Rect<T> {
/// Applies the function `f` to all four sides of the rect
///
Expand Down