Improve flex layout free space distribution #422
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes the issues in #419, by changing the flex layout implementation to more closely match the algorithm outlined in the spec.
Apart from the actual space distribution algorithm, I've also had to change how some of its per-flex-item inputs are calculated.
flex_item::main_sizeis now set to the item's hypothetical main size before space distribution.scaled_flex_shrink_factoris now correctly calculated from the inner base size, instead of the outer one.There is a new
flex_item::clamp_statefield, which is used when fixing min/max violations. It could also be used to tell why an item was sized in a particular way, kinda like how the developer tools in browsers show clamping. Maybe the dump function could be extended in the future to also print things about the state of render items.I've also noticed, that
box-sizingwas not correctly handled for min/max/base sizes, which was causing some test failures after changing the algorithm. That's also fixed now.There is still one
box-sizingrelated problem I know of, that I haven't fixed. When the flex container itself usesborder-box, percentage min/max/base sizes are calculated relative to the container's border box size, but they should be relative to the container's inner size. But this is not really related to the free space distribution algorithm, so I would say it's out of scope for this PR.With these changes, there are a total of 22 failing tests. Most of these have actually been improved, the "failed" outputs match how other browsers work, the originals were slightly off.
There is 1 exception,
flex/table-as-item-stretch-cross-size-3.htmis now failing. This case has a column direction flex container, with a table in it, with90pxbody height, and a10pxhigh caption. This results in a min size of100pxand a base size of90px. The hypothetical main size is100px. This also ends up being the used main size, because the flex container has infinite inner main size, but the same thing would happen if it didn't, as the table is inflexible. The table is then rendered with a size of100px, which is supposed to be its outer size, but it ends up being used as its table box size. So it's oversized by the size of the caption. The caption size should be subtracted from the main size before rendering, like the padding/border/margin, but I don't really know how to do that currently.The case originally worked, because the min size wasn't taken into account, so it ended up with a used main size of
90px.This seems like a pretty niche thing though, so I would suggest also considering this out of scope now, and opening issues for the outstanding problems.
Closes #419