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

Nodes are not constrained to container #7946

Closed
rparrett opened this issue Mar 7, 2023 · 6 comments · Fixed by #7948
Closed

Nodes are not constrained to container #7946

rparrett opened this issue Mar 7, 2023 · 6 comments · Fixed by #7948
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior
Milestone

Comments

@rparrett
Copy link
Contributor

rparrett commented Mar 7, 2023

Bevy version

0.10, main
bisected to 76058bc

Relevant system information

AdapterInfo { name: "Apple M1 Max", vendor: 0, device: 0, device_type: IntegratedGpu, driver: "", driver_info: "", backend: Metal }
SystemInfo { os: "MacOS 13.1 ", kernel: "22.2.0", cpu: "Apple M1 Max", core_count: "10", memory: "64.0 GiB" }

scale_factor is 2., but the behavior is the same when overridden to 1.

What you did

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_startup_system(setup)
        .run();
}

pub const DIALOG_BACKGROUND: Color = Color::rgb(0.2, 0.2, 0.2);
pub const BUTTON_BACKGROUND: Color = Color::rgb(0.15, 0.15, 0.15);

fn setup(mut commands: Commands) {
    commands.spawn(Camera2dBundle::default());

    // dialog
    commands
        .spawn((NodeBundle {
            style: Style {
                size: Size::new(Val::Px(320.0), Val::Px(300.0)),
                margin: UiRect::all(Val::Auto),
                padding: UiRect::all(Val::Px(20.0)),
                flex_direction: FlexDirection::Column,
                ..Default::default()
            },
            background_color: DIALOG_BACKGROUND.into(),
            ..Default::default()
        },))
        .with_children(|parent| {
            // horizontal flex container for "buttons"
            parent
                .spawn(NodeBundle {
                    style: Style {
                        size: Size::new(Val::Percent(100.0), Val::Px(70.0)),
                        flex_direction: FlexDirection::Row,
                        justify_content: JustifyContent::Center,
                        align_items: AlignItems::Center,
                        ..Default::default()
                    },
                    ..Default::default()
                })
                .with_children(|parent| {
                    // "button" 1
                    parent.spawn(NodeBundle {
                        style: Style {
                            size: Size::new(Val::Percent(100.0), Val::Percent(100.0)),
                            ..Default::default()
                        },
                        background_color: BUTTON_BACKGROUND.into(),
                        ..Default::default()
                    });

                    // "button" 2
                    parent.spawn(NodeBundle {
                        style: Style {
                            size: Size::new(Val::Percent(100.0), Val::Percent(100.0)),
                            margin: UiRect {
                                left: Val::Px(10.0),
                                ..Default::default()
                            },
                            ..Default::default()
                        },
                        background_color: BUTTON_BACKGROUND.into(),
                        ..Default::default()
                    });
                });
        });
}

What went wrong

Post-Taffy-3
image

Pre-Taffy-3 and 0.9 (what I expected)
image

@rparrett rparrett added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Mar 7, 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 7, 2023
@alice-i-cecile alice-i-cecile added this to the 0.11 milestone Mar 7, 2023
@rparrett
Copy link
Contributor Author

rparrett commented Mar 7, 2023

Not sure if this is a regression or user-error, but this behaved "as expected" in Bevy 0.9.

In html/css, this sort of thing seems to behave like bevy 0.9, so probably a regression.

@nicoburns
Copy link
Contributor

I cannot reproduce this in Taffy. Using the following HTML:

<div id="test-root" style="width: 320px; height: 300px;margin: auto;padding: 20px;flex-direction: column;">
  <div style="width: 100%;height: 70px;justify-content: center;align-items: center;">
    <div style="width: 100%;height: 100%;"></div>
    <div style="width: 100%;height: 100%;margin-left: 10px"></div>
  </div>
</div>

I get the following rendering in Chrome:

Screenshot 2023-03-07 at 15 04 42

And the Taffy gentest passes with the following output:

Computed tree:
TREE
└──  FLEX [x: 0    y: 0    width: 320  height: 300 ] (4v1)
    └──  FLEX [x: 20   y: 20   width: 280  height: 70  ] (3v1)
        ├──  LEAF [x: 0    y: 0    width: 135  height: 70  ] (1v1)
        └──  LEAF [x: 145  y: 0    width: 135  height: 70  ] (2v1)

Possibly the issue could be somewhere in the code that interfaces with Taffy in Bevy?


P.S. with the latest bevy you can set gap on the parent instead of setting a left margin on one of children if you want to.

@rparrett
Copy link
Contributor Author

rparrett commented Mar 7, 2023

Possibly the issue could be somewhere in the code that interfaces with Taffy in Bevy?

Thanks for the analysis! Looks like you're right.

P.S. with the latest bevy you can set gap on the parent instead of setting a left margin on one of children if you want to.

Looking forward to it, but needed to test Bevy 0.9 as well.

@rparrett
Copy link
Contributor Author

rparrett commented Mar 7, 2023

Side note: I am surprised that we don't have a built-in example that exhibits this bug. They all look okay.

@nicoburns
Copy link
Contributor

nicoburns commented Mar 7, 2023

Side note: I am surprised that we don't have a built-in example that exhibits this bug. They all look okay.

I'm not so surprised. It only exhibits because you're relying on shrinking (and setting the min-size prevents shrinking below that size). But use of shrinking is quite rare (note that there is nothing special about the 100% value you're using for the width of the inner nodes here. It renders identically whatever value you choose there so long as both nodes get the same value).

@rparrett
Copy link
Contributor Author

rparrett commented Mar 7, 2023

I guess it's not surprising that we don't have the same exact bug described above. But if both min_size and max_size were not being passed along to taffy properly, then it seems like our usages of them in other examples are not meaningful, which is surprising.

Just in case it's useful for someone else finding this bug, I was able to work around this issue in my case by

  • removing size from the buttons
  • setting flex_grow: 1.0 on my buttons
  • setting AlignItems: AlignItems::Stretch on the "button container"

@mockersf mockersf modified the milestones: 0.11, 0.10.1 Mar 8, 2023
alice-i-cecile pushed a commit that referenced this issue Apr 24, 2023
# Objective

Add a simple example demonstrating how to use size constraints.

Related to #7946

# Solution

<img width="827" alt="Capture"
src="https://user-images.githubusercontent.com/27962798/223741566-4b8eca99-c450-42b5-a40e-a414858c8310.PNG">

# Changelog
* Added the example `size_constraints`
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

Successfully merging a pull request may close this issue.

4 participants