Optimise flexbox layouts with min/max sizes #413
Conversation
Weibye
left a comment
There was a problem hiding this comment.
If I understand this change correctly it is about reusing some calculated values we already know, instead of re-calculating them multiple times?
Weibye
left a comment
There was a problem hiding this comment.
The change looks good, in the sense that I think I understand what is happening here. It's been a while since I looked too closely at all of this 😅
@Weibye It's effectively calculating two values (the size of the container in each dimension) earlier in the algorithm which means that min/max constraints on those two sizes can be applied earlier in the algorithm. Which means that we have a fully resolved final size for the container before we apply things like stretch and percentage sizing to child nodes. Which means that we don't have to redo this step in event that min/max constraints change those sizes. I feel like it should always have worked like this, but it wasn't really possible until #354 was implemented, as before that PR we didn't have a container size to apply the min/max constraints to until after we'd already applied stretch/percentage sizing. |
Yeah, now I see it, thanks for the explanation! |
8a83254 to
6c541cc
Compare
* Fix layout of direct flex children with `display:none` set (#380) * Add test for toggling display:none * Add test for toggling display:none on a flexbox child * Fix layout direct flex children with `display:none` set * Add test for toggling display:none on flexbox container * Fix setting display:none on a grid child (#382) * Prepare for 0.3.5 release (#381) * Prepare for 0.3.5 release * Add grid PR to release notes * Ignore align_content in non-wrapping flexbox containers (#383) * Add tests for bevyengine/bevy#7976 * Simplify code in compute_constants * Ignore align_contents if flex_wrap is set to no_wrap * Remove commented out code * Prepare for v0.3.6 release (#384) * Use mutable slice rather mutable vec parameter * Reduce style accesses (#386) * Use cached flex_grow and flex_shrink values when resolving flexible lengths * Cache auto margins on FlexItem struct * Use constants.is_wrap instead of re-resolving style * Use cached align_content value rather than re-resolving style * Cache justify_content style in AlgoConstants * Add failing tests for borders flooring node size * Add failing tests for padding flooring node size * Fix divide by zero in main size determination * Add seperate leaf and flexbox tests for padding/border flooring node size * Fix padding/border flooring leaf node size * Fix flexbox children being floored by padding/border * Combine padding and border tests + make edge dimensions uneven * Add padding/border floor absolute child size * Add padding/border floor node size tests for grid * Add grid container test for padding/border flooring node size * Fix padding/border flooring size of absolutely positioned children * Rename grid padding/border tests so that they are all located together * Make padding/border floor node size for grid children * Make padding/border floor node size for grid containers * Apply aspect ratio to leaf nodes whose size is determined by padding/border sum * Add test for padding/border not affecting flex basis * Convert flex basis determination to use break from block * Add test for flex-basis 0 with flex grow * Floor flex-basis limits by preferred size style value when determining flex container main size * Floor outer flex-basis by padding_border sum (floors inner flex-basis at 0) * Remove commented out code * Add missing 0 * Test cases for bevyengine/bevy#8017 and Taffy #387 * Prevent percentage sizes from contributing a flex item's min-content size * Prepare for 0.3.7 release (#389) * Prepare for 0.3.7 release * Fix duplicate content in the changelog * Fix markdown lint * Upgrade to better Github Actions for faster and better maintained CI (#390) * Use taiki-e/install-action for installing cargo-deny * Update CI to use dtolnay/rust-toolchain action * Enable CI for 0.3.x branch * Fix documentation CI * Dummy Cargo.toml change * Add fully reduced test case for #387 (#391) * Fix wrapping nodes generating an incorrect min-content size (#395) * Add tests for bevyengine/bevy#8082 * Debug log parent size * Pass correct cross-axis parent_size/available_space when computing a flex item's min-content contribution * Prepare for 0.3.8 release (#396) * Don't allow cached results to be used for sizings with greater available space (#397) * Don't allow cached results to be used for sizings with greater available space * Update caching tests to use tree 100 nodes deep (relax permitted measure count to 7) * Prepare for 0.3.9 release (#398) * Prepare for 0.3.9 release * Add bevyengine/bevy#8124 to release notes * Allow multiple syn versions * Tree creation benchmarks (#401) * Add tree creation benchmarks * Rename allocation benchmark to tree_creation * Update rstest requirement from 0.16.0 to 0.17.0 (#402) Updates the requirements on [rstest](https://github.com/la10736/rstest) to permit the latest version. - [Release notes](https://github.com/la10736/rstest/releases) - [Changelog](https://github.com/la10736/rstest/blob/master/CHANGELOG.md) - [Commits](la10736/rstest@0.16.0...0.17.0) --- updated-dependencies: - dependency-name: rstest dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Update syn requirement from 1.0.7 to 2.0.4 (#403) Updates the requirements on [syn](https://github.com/dtolnay/syn) to permit the latest version. - [Release notes](https://github.com/dtolnay/syn/releases) - [Commits](dtolnay/syn@1.0.7...2.0.4) --- updated-dependencies: - dependency-name: syn dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix: available space in the presence of a min width (#407) * Add `with_main` and `with_cross` methods to Size * Add test for min_width > available_space * Clamp available cross space by min/max height when computing flex basis * Clamp available cross space by min/max size when computing hypothetical cross size * Bump measure func call counts to 8 * Prepare for 0.3.10 release (#411) * Fix import lints (#416) (cherry picked from commit 5522573) * Add caching to CI (#418) * 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 * Prepare for 0.3.11 release (#419) --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Andreas Weibye <13300393+Weibye@users.noreply.github.com> Co-authored-by: TimJentzsch <commits@timjen.net>
Objective
Optimise performance of Flexbox layouts with min/max sizes.
I also fixed a test that I noticed had a missing semicolon which caused two of the styles to not be applied.
Context
Currently the flexbox algorithm does a second pass of the entire layout algorithm (for that node) in the case that it has min- and max- sizes.
Notes
Currently we do not have benchmarks testing layouts with min/max sizes. Benchmarks results on the existing benchmarks are pretty neutral. UPDATE: I did a quick test locally by adding a min size to the setup code for our existing benchmarks, and this causes an exponential blowup on
main. Fixed by this PR :)