Yoga does not respect automatic minimum size for flexbox items #1409
Comments
|
|
|
Stupid bot didn't notice that repro has been provided. |
You should provide a link to a repro or a Snack, not a snippet. |
|
Interestingly From the CSS spec, Flexbox says this should be the content-based minimum size. So in the correct case, we would be sizing the box according to that, then distribute the rest of the free space accordingly, like the expected example. https://www.w3.org/TR/css-flexbox-1/#min-size-auto Yoga will incorporate max-content into I think this might also depend on #1298 where right now Yoga doesn't actually have a way to ask for Arguably, if we want to build for conformance, this might not be the right choice. But it would maybe be worth seeing how much this trick is actually saving on performance in a real app, if it was an intentional design decision. FYI @necolas, that we should maybe put |
NickGerleman
changed the title
flex-basis works like min-width: 0px
|
@NickGerleman Huge thanks for looking into this! Do you know of any workarounds? |
|
The workaround for consistency would be set an explicit minimum size, since Yoga will not determine one automatically. That's not really fixing the root issue, admittedly. |
|
I was interested in what the performance impact of implementing "automatic minimum size" might be, so I've done a little informal investigation into the issue. MethodologyModifying Taffy's benchmarks, which include head-to-head comparison with Yoga via the rust bindings (yoga-rs) with various style combinations designed to stress the min-content sizing part of the algorithm. In particular, I tested a lot of trees that had Findings / Conclusions
Benchmark resultsTaffy's standard benchmarksThese are Taffy's current standard benchmarks which we have committed to the repo. The idea of including them here is that they are useful for giving us an insight into what performance in easier / less degenerate cases than the above. Although arguably they measure a bit too easy of a scenario. The use the following style generation function: fn create_style<R: Rng>(rng: &mut R) -> Style {
fn gen_dimension<R: Rng>(rng: &mut R) -> Dimension {
let switch: f32 = rng.gen_range(0.0..=1.0);
if switch < 0.2 {
Dimension::Auto
} else if switch < 0.8 {
Dimension::Length(rng.gen_range(0.0..500.0))
} else {
Dimension::Percent(rng.gen_range(0.0..1.0))
}
}
Style {
size: Size {
width: gen_dimension(rng),
height: gen_dimension(rng),
},
..Default::default()
}
}
My conclusion from these numbers: although Taffy is a bit faster, the numbers are more similar than they are different (with the exception of the unrealistically large 100k node test). Stress TestThe results below pertain to trees of nodes where styles were creating using the following function (this is using Taffy/Rust notation, but hopefully it should be pretty clear what it's doing): fn create_style<R: Rng>(rng: &mut R) -> Style {
let is_row = rng.gen::<f32>() < 0.5;
Style {
flex_direction: if is_row {
FlexDirection::Row
} else {
FlexDirection::Column
},
flex_grow: 1.0,
margin: Rect {
left: LengthPercentageAuto::Length(10.0),
right: LengthPercentageAuto::Length(10.0),
top: LengthPercentageAuto::Length(10.0),
bottom: LengthPercentageAuto::Length(10.0),
},
..Default::default()
}
}these styles were picked for inclusion here because they were amongst the slowest results I could find for both libraries.
It was interesting (and surprising!) to me that Yoga actually seems to scale worse in this more difficult scenario (note in particular the numbers in the "deep" benchmarks), despite containing algorithm shortcuts designed to facilitate this kind of scalability. Whether that's because the algorithm shortcuts don't help much, or because there are other aspects of Taffy's implementation that compensate for it using a slower algorithm I'm not sure. But I figure that in many ways it doesn't matter too much, because either could be implemented in Yoga. These benchmarks are definitely a bit artificial, and I'd love to run some numbers on some real-world views/trees, but hopefully they give us some idea of what might be possible :) Let me know if there are any other style combinations you'd like me to benchmark. |
|
Thanks for the helpful context. Apart from the general "is it possible for this to be efficient", comparing Yoga to previous baseline is probably what we need to do here. Re benchmarks, I definitely agree with the sentiment of what we have right now veering too far into synthetic. I've been at times tempted to see if we can dump a tree from a real-world app, and make a fixture out of it, so we can test laying out a real-world tree (or different real-world trees), along with incremental mutation and relayout. We do have a couple of ~10k node real-world UIs that have pretty different characteristics. I've also noticed, at least for the case of For caching, beyond what Yoga does, we recently discovered RN Fabric can invalidate the layout cache much more often than it needs to. In RN there is a Yoga node per ShadowNode, and a series of bugs which leads to sometimes restoring previous ShadowNodes, containing pre-layout information, into new trees, on React mutation. In the most serious cases, this could lead to small UI changes having catastrophic effect to layout cache, and effectively removing most of the previous work. facebook/react-native@ef43846 was one of the changes to help here, but there are more coming, and some discussion on the general model (RN Fabric heavily relying on Yoga private APIs is another thing I'd like to get us out of). |
|
Apart from cost of flex layout, I think part of the performance concern here is for laying out text. I think if we wanted to implement this correctly, we could still shortcut min-content measurement in most cases I think. If flex basis is based on content, we already have a max-content measurement. If we don’t need to shrink, and didn’t otherwise have an explicit flex basis, we know we are already at least min-content sized in the main axis. |
|
Downside, the elegant way to do this involves breaking every Yoga measure function already in existence, since now they need to know about min-content. Or adding a secondary function. |
Description
Actual (all items are equal width):
Expected (1 and 2 should shrink):
React Native Version
0.72.4
Output of
npx react-native infoSteps to reproduce
React Native:
Web equivalent:
Snack, screenshot, or link to a repository
Repro has been provided above.
The text was updated successfully, but these errors were encountered: