Invalid ui node size when creating tree with 4-level depth

[jkb0o]

Bevy version

0.10, 38a08d9

Relevant system information

rustc 1.67.1
AdapterInfo { name: "Apple M1",  device_type: IntegratedGpu, backend: Metal }

What you did

I create multiple nested ui nodes with depth level at least 4 and provide to them different style params (padding, margin and size):

commands.spawn(NodeBundle {
    background_color: Color::WHITE.into(),
    style: Style {
        // top-level margin affects the invalid behaviour, uncomment to see
        // margin: UiRect::all(Val::Px(20.)),
        size: Size::all(Val::Px(200.)),
        align_content: AlignContent::FlexStart,
        ..default()
    },
    ..default()
}).with_children(|parent| {
    for _ in 0..4 {
        parent.spawn(NodeBundle {
            background_color: Color::RED.into(),
            style: Style {
                min_size: Size::all(Val::Px(40.)),
                margin: UiRect::all(Val::Px(5.0)),
                padding: UiRect::all(Val::Px(5.0)),
                ..default()
            },
            ..default()
        }).with_children(|parent| {
            parent.spawn(NodeBundle {
                background_color: Color::GREEN.into(),
                style: Style {
                    size: Size::all(Val::Percent(100.)),
                    padding: UiRect::all(Val::Px(5.0)),
                    ..default()
                },
                ..default()
            })
            
            //  Uncoment this to see the invalid behaviour:

            // .with_children(|parent| {
            //     parent.spawn(NodeBundle {
            //         ..default()
            //     });
            // })
            ;
        });
    }
});

ui_height_0_10.zip

What went wrong

Adding additional child nodes (uncomment the code) makes the parent nodes too big:

3 levels of nodes (expected):

4 level of nodes (invalid):

Additional information

Changing the top-level margin affects the invalid behaviour (nodes goes bit smaller).

[nicoburns]

Transliterating this into HTML, I actually get something much closer to the "invalid" behaviour in Chrome (with or without the 4th nested node). In chrome the margin makes no difference other than adding some spacing around the edge:

HTML repro:

<div id="test-root" style="display: flex;width: 200px; height: 200px; align-content: start;margin: 20px">
  <div style="display: flex;min-width: 40px;min-height:40px;margin:5px;padding:5px;background-color:red">
    <div style="display: flex;width:100%;height:100%;padding:5px;background-color:green">
      <div></div>
    </div>
  </div>
  <div style="display: flex;min-width: 40px;min-height:40px;margin:5px;padding:5px;background-color:red">
    <div style="display: flex;width:100%;height:100%;padding:5px;background-color:green">
      <div></div>
    </div>
  </div>
  <div style="display: flex;min-width: 40px;min-height:40px;margin:5px;padding:5px;background-color:red">
    <div style="display: flex;width:100%;height:100%;padding:5px;background-color:green">
      <div></div>
    </div>
  </div>
  <div style="display: flex;min-width: 40px;min-height:40px;margin:5px;padding:5px;background-color:red">
    <div style="display: flex;width:100%;height:100%;padding:5px;background-color:green">
      <div></div>
    </div>
  </div>
</div>

[ickshonpe]

I think the second image is correct (or at least closer), and the first one is fixed by #7946.

[jkb0o]

@nicoburns Well, I ok with both behaviours if it solid, e.g. doesn't depend on the depth of tree.

[jkb0o]

@ickshonpe the issue created for the latest bevy, the version after the fix you noticed.

[nicoburns]

As @jkb0o said, this is running against main, so it can't be #7946. I actually think there are at least two bugs here:

1. The 3 level behaviour is incorrect: the nodes should be tall like in the 4 level version. This is presumably because Taffy's leaf algorithm (which is used for nodes with no children) is not resolving percentages correctly or something like that.

2. The nodes in 4 level version seem to be overflowing their container, which they don't in chrome. I'm getting a gentest failure for this so it's definitely not quite right.

[ickshonpe]

Weird, I'm seeing it in main now. In 0.9.1, you consistently get four long rects like the HTML example.

[nicoburns]

Chrome dev tools solve the mystery of bug 2:

Taffy wasn't ignoring align_content when flex-wrap was set to nowrap (the default).

(@jkb0o if you want to do alignment for a single-line flex container you want align-items not align-content).

[ickshonpe]

Maybe we should remove AlignContent from Style and change FlexWrap to:

pub enum FlexWrap {
    /// Single line, will overflow if needed.
    NoWrap,
    /// Multiple lines, if needed.
    Wrap(AlignContent),
    /// Same as [`FlexWrap::Wrap`] but new lines will appear before the previous one.
    WrapReverse(AlignContent),
}

or something similar.

[nicoburns]

I think that will just be confusing as the API will no longer match CSS. Also, it won't work very well with CSS Grid which also uses AlignContent but not FlexWrap. What I think we ought to do is move towards providing the same kind of lints as Chrome DevTools do above (ideally in some kind of devtools of our own).

[ickshonpe]

Yeah, some sort of visual inspection tool would be great.

[nicoburns]

@jkb0o Taffy v0.3.6 has been released which includes a fix for this issue. You should be able to get it by running cargo update :)

[jkb0o]

Cool, thanks! Got the same behaviour for all variants now!

[ickshonpe]

This still seems wrong:

use taffy::prelude::*;

fn main() {
    
    let p = Style {
        size: Size {
            width: points(20.),
            height: points(20.),
        },
        padding: Rect {
            left: points(5.),
            right: points(5.),
            top: points(5.),
            bottom: points(5.),
        },
        ..Default::default()
    };

    let c = Style {
        min_size: Size {
            width: percent(1.),
            height: auto(),
        },
        padding: Rect {
            left: points(5.),
            right: points(5.),
            top: points(5.),
            bottom: points(5.),
        },
        ..Default::default()
    };
    
    let mut taffy = Taffy::new();
    let child = taffy.new_leaf(c).unwrap();
    let parent = taffy.new_with_children(p, &[child]).unwrap();

    taffy.compute_layout(
            parent, 
            Size { width: AvailableSpace::Definite(100.0),  height: AvailableSpace::Definite(100.0) }
        )
        .ok();

    println!("{:#?}", taffy.layout(parent));
    println!("{:#?}", taffy.layout(child));
}

result:

Ok(
    Layout {
        order: 0,
        size: Size {
            width: 20.0,
            height: 20.0,
        },
        location: Point {
            x: 0.0,
            y: 0.0,
        },
    },
)
Ok(
    Layout {
        order: 0,
        size: Size {
            width: 10.0,
            height: 10.0,
        },
        location: Point {
            x: 5.0,
            y: 5.0,
        },
    },
)

The inner node has a size of 10x10, despite having a min-width of 100% which should resolve to a 20 unit width?

Recreated using CSS in chrome and edge, the inner child has a size of 20x10

<style>
    .g1 {
        display: flex;
        background: red;
        width: 20;
        height: 20;
        padding: 5 5 5 5;
    }
    
    .g2 {
        display: flex;
        background: green;
        min-width: 100%;
        padding: 5 5 5 5;
    }
</style>

<div class="root"> 
    <div class="g1">
        <div class="g2"></div> 
    </div>
</div>

output:

[nicoburns]

@ickshonpe That HTML is invalid. You need to specify 20px not just 20 and you need to specify 5px not just 5. You can see in the Chrome devtools that the styles are not being applied:

Correcting the HTML to:

<div id="test-root" style="width:20px;height:20px;padding:5px"> 
  <div style="min-width: 100%;padding:5px"></div> 
</div>

I get an inner size of 10px x 10px in Chrome. And turning this HTML into a Taffy gentest, it passes.

---

I would encourage you to make use of Taffy's gentest infrastructure yourself. If you install chromedriver and create a test file (by duplicating an existing one) in the test_fixtures directory of Taffy's repo, the you can:

• Open it in Chrome, taking advantage of the base stylesheet that the tests include.
• Run cargo gentest followed by cargo test to compare Taffy's output to Chrome's (if the output doesn't match then the newly generated test will fail)

More details in https://github.com/DioxusLabs/taffy/blob/main/CONTRIBUTING.md