scrollable_overflow implementation
Objective
Replaces content_size: Size<f32> with scrollable_overflow: Rect<f32> and tries to match the definition of "scrollable overflow rectangle" in CSS Overflow 3, 2.2. Scrollable Overflow.
Fixes #871
- [ ] Changes that will affect external library users must update RELEASES.md before they will be merged.
Context
See #871.
Feedback wanted
I added several helper/utility Rect methods to geometry.rs. I wasn't sure if they are of sufficient generality to be methods directly on Rect or whether they should be helper functions in some utility module. Also, if they are Rect methods, I wasn't sure if they should be pub or just pub(crate). I saw examples of both, so I couldn't just do the same as the others.
I made some changes to gentest/src/main.rs (which unfortunately means that every generated file shows up in this commit) but I don't think they should be controversial:
- I pulled long string literals for leaf nodes out onto their own line to make rustfmt not throw up its hands in despair.
- I changed the assertion logic to check every node then assert once instead of asserting at each comparison. This way you can see all the things you got wrong, not just the first.
- And I (tentatively) turned on the
scroll_width/scroll_heightchecks for all nodes instead of limiting them to scroll containers. Hopefully I'll be able to fix the last few fails and won't have to turn that back off.
The actual logic for the scrollable overflow computations follows a very similar structure to the content_size computations that I replaced. There were some subtleties that took some trial-and-error to figure out, like the fact that the scrollable overflow from inflow child elements is expanded by the container's padding but the scrollable overflow from out-of-flow elements is taken as is. But what it is doing is pretty straight forward. The most non-obvious bit I think is that LayoutOutput only contains some of the scrollable overflow and it is up to the caller of tree.perform_child_layout to fill in the rest before calling tree.set_unrounded_layout. The compute_*_layout functions just don't have enough information to compute it all in one place.
And finally, I'm sure this stuff needs more test cases to be fully exercised, but I just don't know enough CSS to figure out all that might entail. I know a million times more about CSS layout now than I did when I started this work, but, well, that's like saying 10^-6 is a million times more than 10^-12: both are still small numbers.
Oh, something else I forgot to mention. By the time I got done with this work, the text "content_size" shows up pretty much only in #[cfg(feature = "content_size")] annotations. It really isn't the "content_size" feature any more. Couple of options:
- We could add a
content_sizefield back intoLayoutthat tries to duplicate the prior semantics. - We could change all my scrollable overflow stuff to gate on a new feature, say
scrollable_overflow. - We could do that but also put back all the
content_sizestuff so people could choose between the two feature sets. -
scrollable_overflowfeature with just the new stuff then acontent_sizefeature that requiresscrollable_overflowand just adds acontent_sizefield that tries to duplicate prior semantics.
I guess it boils down to separate questions:
- do we add a new scrollable_overflow feature?
- do we remove the old content_size feature?
- do we preserve the old
content_sizefield inLayout(and possiblyLayoutOutput) - if there is a
content_sizefield, do we try to compute it from the scrollable overflow or do we preserve both sets of code under different feature gates?
My personal bias is towards renaming the feature and pretending content_size never existed. But I come out of a career of smallish game shops where there were no external clients for the code so reducing maintenance burden was much more valuable than backwards compatibility.
Looking into the justify_content_row_max_width_and_margin test. Taffy's result seems to agree with Firefox but not Chrome here. Additionally, Chrome starts giving this results if overflow is set to hidden/scroll/auto.