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

style.min_size doesn't override max_size on the cross axis #6737

Closed
ickshonpe opened this issue Nov 23, 2022 · 6 comments
Closed

style.min_size doesn't override max_size on the cross axis #6737

ickshonpe opened this issue Nov 23, 2022 · 6 comments
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior S-Blocked This cannot move forward until something else changes

Comments

@ickshonpe
Copy link
Contributor

ickshonpe commented Nov 23, 2022

Bevy version

0.9

What you did

use bevy::prelude::*;

pub fn setup(
    mut commands: Commands,
) {
    commands.spawn(Camera2dBundle::default());
    commands.spawn(NodeBundle {
        style: Style {
            min_size: Size::new(Val::Px(100.0), Val::Px(100.0)),
            size: Size::new(Val::Px(50.0), Val::Px(50.0)),
            max_size: Size::new(Val::Px(10.0), Val::Px(10.0)),
            ..Default::default()
        },
        background_color: BackgroundColor(Color::RED),
        ..Default::default()
    });
}

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

What went wrong

It should draw a 100 x 100 square.
Instead, a 100 wide x 10 tall rectangle is drawn.

Additional information

According to the Flexbox spec, min_size is supposed to override max_size if max_size < min_size.

The problem is with the calculation of the length of the cross-axis, if the Flex direction is set to FlexDirection::Column you get a 10 wide x 100 tall box instead.

related issue: #5502

@ickshonpe ickshonpe added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Nov 23, 2022
@alice-i-cecile alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets S-Blocked This cannot move forward until something else changes and removed S-Needs-Triage This issue needs to be labelled labels Nov 23, 2022
@alice-i-cecile
Copy link
Member

Can you reproduce this upstream in taffy, and then file a bug for us there?

@ickshonpe
Copy link
Contributor Author

Okay, I'll have a go.

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Nov 24, 2022

It seems to be fixed in Taffy main, except that max_size overrides min_size but I'm not sure if that's intentional.

Is this a bug?

use taffy::prelude::*;

fn main() {
    let mut taffy = Taffy::new();
    let child = taffy.new_leaf(
        Style {
            size: Size { 
                width: Dimension::Points(2.), 
                height: Dimension::Points(2.) 
            },                
            ..Default::default()
        }
    ).unwrap();

    let parent = taffy.new_with_children(Style {
            size: Size {
                width: Dimension::Points(1.), 
                height: Dimension::Points(1.)
            },
            ..Default::default()        
        }, &[child]
    ).unwrap();

    let _ = taffy.compute_layout(parent, Size {
        width: AvailableSpace::Definite(1.),
        height: AvailableSpace::Definite(1.),
    });

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

output:

parent size = Size { width: 1.0, height: 1.0 }
child size = Size { width: 1.0, height: 2.0 }

@alice-i-cecile
Copy link
Member

@TimJentzsch, you're much better at these flexbox details than I am :)

@TimJentzsch
Copy link
Contributor

I agree that this looks like a bug, from experiments in the browser I can confirm that the priority seems to be min_size > max_size > size.

Also confirmed by the MDN docs:

max-width overrides width, but min-width overrides max-width

@Weibye
Copy link
Contributor

Weibye commented Jan 12, 2023

This has been solved by #6743 and is now working. Running the reproduce-code gives the correct results:

image

2023-01-12T21:42:42.881107Z  INFO ui_test: Node { calculated_size: Vec2(100.0, 100.0) }

@Weibye Weibye closed this as completed Jan 12, 2023
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 S-Blocked This cannot move forward until something else changes
Projects
None yet
Development

No branches or pull requests

4 participants