Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid ui node size when creating tree with 5-level depth and 100% height #7994

Closed
jkb0o opened this issue Mar 9, 2023 · 9 comments
Closed
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior

Comments

@jkb0o
Copy link
Contributor

jkb0o commented Mar 9, 2023

Bevy version

bevy: 0.11.0-dev 21ddc60
taffy: 0.3.6

What you did

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

commands.spawn(NodeBundle {
    background_color: Color::WHITE.into(),
    style: Style { 
        margin: UiRect::all(Val::Px(20.)),
        size: Size::new(Val::Percent(20.), Val::Px(200.)),
        align_items: AlignItems::FlexStart,
        justify_content: JustifyContent::FlexStart,
        ..default()
    },
    ..default()
}).with_children(|parent| {
    for idx 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.)),
                padding: UiRect::all(Val::Px(5.)),
                ..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()
            })
            .with_children(|parent| {
                let mut size = Size::default();
                size.width = Val::Percent(100.);
                if idx >= 2 {
                    // Commenting this line makes all nodes
                    // behaves the same
                    size.height = Val::Percent(100.);
                }
                parent.spawn(NodeBundle {
                    background_color: Color::BLUE.into(),
                    style: Style {
                        size,
                        padding: UiRect::all(Val::Px(5.)),
                        ..default()
                    },
                    ..default()
                })
                // Commenting this makes all nodes bahaves the same
                // no matter what the size of blue node
                .with_children(|parent| {
                    parent.spawn(NodeBundle::default());
                })
                ;
            })
            ;
        });
    }
});

What went wrong

Setting height to the blue node to Val::Percent(100.) makes the outer nodes grow. This happens only if the blue node has another child. On the screenshot the first two nodes use default height, the rest two nodes has 100% height:

Screenshot 2023-03-09 at 08 07 12

I expect all nodes are the same size in both cases: if I specify the 100% height to the blue one or if I add extra empty children to blue nodes.

Example project: ui_height_0_11.zip

@jkb0o jkb0o added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Mar 9, 2023
@alice-i-cecile alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets and removed S-Needs-Triage This issue needs to be labelled labels Mar 9, 2023
@nicoburns
Copy link
Contributor

Ah, excellent. I look forward to tomorrow's issue with 6-level depth! In all seriousness though, I don't think it's actually the depth that's the issue here.

In Chrome, I get the following:

Screenshot 2023-03-09 at 19 31 58

@nicoburns
Copy link
Contributor

Ok, so I have a couple of reduced failing test cases:

Test Case 1

<div id="test-root" style="min-height:40px">
  <div style="height:100%;width: 40px;background: red;"></div>
</div>

This fails because Taffy allows the percentage size of the inner node to resolve against the min-height of the outer node and Chrome doesn't.

Test Case 2

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

This is basically the same as the original example in the description of this issue. This seems to happen because one of the inner percentage heights (or possibly the inner stretch) is somehow resolving against the available space from the outermost node (although margins/paddings are being subtracted).

Thoughts

Unfortunately the solution to this one doesn't seem so simple to me. So it may take a while to be fixed.

@jkb0o If you're looking to work around this in the meantime, I would recommend not trying to resolve percentages against min-height or min-width. This generally doesn't seem to be a well-supported use case in web layout.

nicoburns added a commit to nicoburns/taffy that referenced this issue Mar 9, 2023
@nicoburns
Copy link
Contributor

A couple more comments:

@jkb0o
Copy link
Contributor Author

jkb0o commented Mar 10, 2023

@nicoburns Thank you for your attention!

This appears to be a long-standing bug rather than a regression. My tests cases also fail with Taffy 0.2.

It could be a regression, I tried the same example with bevy 0.9.1 with taffy 0.1.0, and the layout looks perfect:
Screenshot 2023-03-10 at 07 32 20
ui_height_0_9.zip

If you're looking to work around this in the meantime, I would recommend not trying to resolve percentages against min-height or min-width. This generally doesn't seem to be a well-supported use case in web layout.

Should I avoid min-size / percentage sizes at any levels to be sure the layout doesn't break? So I can't use 100% height if some grandparent node has min-height defined? I do not understand how to keep things simple in complex ui then. I have something like this for example (it is completely broken right now, and I can't find any workarounds):

party-editor

@nicoburns
Copy link
Contributor

Taffy v0.3.7 is a partial fix for this issue. It at least prevents the case where the node gets super tall.

@nicoburns
Copy link
Contributor

Should I avoid min-size / percentage sizes at any levels to be sure the layout doesn't break? So I can't use 100% height if some grandparent node has min-height defined? I do not understand how to keep things simple in complex ui then.

There are two possible ways to work around this:

  • Use stretch alignment instead of height: 100%. This works fine with min-height
  • Add a height (perhaps a percentage height?) to the nodes which have a min-height it's the combination of min-height and height: auto (the default) that is causing issues here.

Medium term, it looks like CSS Grid would be perfect for your UI (being that it's a grid). The layout algorithms for this are all implemented. They just need to be wired up Bevy, so this should he available sooner rather than later.

@jkb0o
Copy link
Contributor Author

jkb0o commented Mar 10, 2023

The example project now looks good! Thank you!

@jkb0o jkb0o closed this as completed Mar 10, 2023
@jkb0o
Copy link
Contributor Author

jkb0o commented Mar 10, 2023

Almost all my examples looks good now as well! Thank you!

@nicoburns
Copy link
Contributor

@jkb0o Just to warn you: things may end up looking more like the chrome screenshot above in future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

No branches or pull requests

3 participants