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

Crash in bevy_ui with specific node hierarchy #16765

Open
romamik opened this issue Dec 11, 2024 · 6 comments
Open

Crash in bevy_ui with specific node hierarchy #16765

romamik opened this issue Dec 11, 2024 · 6 comments
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior P-Crash A sudden unexpected crash S-Needs-Design This issue requires design work to think about how it would best be accomplished

Comments

@romamik
Copy link
Contributor

romamik commented Dec 11, 2024

Bevy version 0.15.0

(0.14.2 also had this issue)

What you did

I created a UI node hierarchy, that looks like this:

    <div width=184 height=288 align_self=center justify_self=center>
        <div>
            <div position=absolute width=100% height=100% flex_direction=column>
                <div margin_left=10 margin_right=10>
                    <text text="hello world"/>
                </div>
            </div>
        </div>
    </div>

I know, it kinda does not make sense as there is a node with 100% size and parent size is not defined, but that should not crash anyway...

The code:

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins((DefaultPlugins.set::<AssetPlugin>(AssetPlugin {
            watch_for_changes_override: Some(true),
            ..Default::default()
        }),))
        .add_systems(Startup, setup)
        .run();
}

fn setup(mut commands: Commands) {
    commands.spawn((Camera2d::default(), IsDefaultUiCamera));

    // <div width=184 height=288 align_self=center justify_self=center>
    //   <div>
    //     <div position=absolute size=100% flex_direction=column>
    //       <div margin_x=10>
    //         <text text="hello world"/>
    //       </div>
    //     </div>
    //   </div>
    // </div>
    commands
        .spawn(Node {
            width: Val::Px(184.0),
            height: Val::Px(288.0),
            align_self: AlignSelf::Center,
            justify_self: JustifySelf::Center,
            ..Default::default()
        })
        .with_children(|parent| {
            parent
                .spawn(Node {
                    ..Default::default()
                })
                .with_children(|parent| {
                    parent
                        .spawn(Node {
                            position_type: PositionType::Absolute,
                            width: Val::Percent(100.0),
                            height: Val::Percent(100.0),
                            flex_direction: FlexDirection::Column,
                            ..Default::default()
                        })
                        .with_children(|parent| {
                            parent
                                .spawn(Node {
                                    margin: UiRect {
                                        left: Val::Px(10.0),
                                        right: Val::Px(10.0),
                                        ..Default::default()
                                    },
                                    ..Default::default()
                                })
                                .with_children(|parent| {
                                    parent.spawn(Text("hello world".to_string()));
                                });
                        });
                });
        });
}

What went wrong

It crashes.

rustlib/src/rust/library/core/src/num/f32.rs:1406:9:
min > max, or either was NaN. min = 0.0, max = -10.0
stack backtrace:
   0: rust_begin_unwind
             at /rustc/55a22d2a63334e0faff0202b72a31ce832b56125/library/std/src/panicking.rs:665:5
   1: core::panicking::panic_fmt
             at /rustc/55a22d2a63334e0faff0202b72a31ce832b56125/library/core/src/panicking.rs:74:14
   2: core::f32::<impl f32>::clamp
             at /Users/romamik/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/num/f32.rs:1406:9
   3: bevy_ui::ui_node::BorderRadius::resolve_single_corner
             at /Users/romamik/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ui-0.15.0/src/ui_node.rs:2354:9
   4: bevy_ui::ui_node::BorderRadius::resolve
             at /Users/romamik/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ui-0.15.0/src/ui_node.rs:2373:23
   5: bevy_ui::layout::ui_layout_system::update_uinode_geometry_recursive
             at /Users/romamik/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ui-0.15.0/src/layout/mod.rs:382:64
   6: bevy_ui::layout::ui_layout_system::update_uinode_geometry_recursive
             at /Users/romamik/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ui-0.15.0/src/layout/mod.rs:445:17
   7: bevy_ui::layout::ui_layout_system::update_uinode_geometry_recursive
             at /Users/romamik/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ui-0.15.0/src/layout/mod.rs:445:17
   8: bevy_ui::layout::ui_layout_system::update_uinode_geometry_recursive
             at /Users/romamik/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ui-0.15.0/src/layout/mod.rs:445:17
   9: bevy_ui::layout::ui_layout_system
             at /Users/romamik/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ui-0.15.0/src/layout/mod.rs:301:13
  10: <bevy_ecs::system::function_system::FunctionSystem<Marker,F> as bevy_ecs::system::system::System>::run_unsafe
@romamik romamik added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Dec 11, 2024
@alice-i-cecile alice-i-cecile added P-Crash A sudden unexpected crash A-UI Graphical user interfaces, styles, layouts, and widgets and removed S-Needs-Triage This issue needs to be labelled labels Dec 12, 2024
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Dec 12, 2024

Can you please test this on #16780? This looks like it might be a duplicate of #16639.

@BenjaminBrienen BenjaminBrienen added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Dec 12, 2024
@romamik
Copy link
Contributor Author

romamik commented Dec 12, 2024

No, it is not fixed by this PR, it still crashes with the same error.

It actually is different, because in that issue the problem is with parent not being removed from child when manipulating hierarchy (or something like that).

While in my case it calculates some value during layout and then assert fails.

@romamik
Copy link
Contributor Author

romamik commented Dec 12, 2024

I've done some debugging:
With that layout, taffy returns width=-20 for one of the nodes. I've tested with the latest version from git.

my test code
// put it in examples folder in taffy
mod common {
    pub mod image;
    pub mod text;
}

use common::{
    image::ImageContext,
    text::{TextContext, WritingMode},
};
use taffy::prelude::*;

enum NodeContext {
    Text(TextContext),
    Image(ImageContext),
}

fn main() -> Result<(), taffy::TaffyError> {
    let mut taffy: TaffyTree<NodeContext> = TaffyTree::new();
    /*
        1<div width=184 height=288 align_self=center justify_self=center>
        2    <div>
        3        <div position=absolute width=100% height=100% flex_direction=column>
        4            <div margin_left=10 margin_right=10>
        5                <text text="hello world"/>
                    </div>
                </div>
            </div>
        </div>
    */

    let node1 = taffy.new_leaf(Style {
        size: Size { width: Dimension::Length(184.0), height: Dimension::Length(288.0) },
        align_self: Some(AlignSelf::Center),
        justify_self: Some(JustifySelf::Center),
        ..Default::default()
    })?;

    let node2 = taffy.new_leaf(Style { ..Default::default() })?;
    taffy.add_child(node1, node2)?;

    let node3 = taffy.new_leaf(Style {
        position: Position::Absolute,
        size: Size { width: Dimension::Percent(1.0), height: Dimension::Percent(1.0) },
        flex_direction: FlexDirection::Column,
        ..Default::default()
    })?;
    taffy.add_child(node2, node3)?;

    let node4 = taffy.new_leaf(Style {
        margin: Rect {
            left: LengthPercentageAuto::Length(10.0),
            right: LengthPercentageAuto::Length(10.0),
            top: LengthPercentageAuto::Auto,
            bottom: LengthPercentageAuto::Auto,
        },
        ..Default::default()
    })?;
    taffy.add_child(node3, node4)?;

    let text = taffy.new_leaf_with_context(
        Style::default(),
        NodeContext::Text(TextContext { text_content: "Hello world".into(), writing_mode: WritingMode::Horizontal }),
    )?;
    taffy.add_child(node4, text)?;

    println!("Compute layout with 100x100 viewport:");
    taffy.compute_layout(
        node1,
        Size { height: AvailableSpace::Definite(100.0), width: AvailableSpace::Definite(100.0) },
    )?;
    println!("node: {:#?}", taffy.layout(node1)?);
    println!("child: {:#?}", taffy.layout(node2)?);

    println!("Compute layout with undefined (infinite) viewport:");
    taffy.compute_layout(node1, Size::MAX_CONTENT)?;
    println!("node: {:#?}", taffy.layout(node1)?);
    println!("child: {:#?}", taffy.layout(node2)?);

    taffy.print_tree(node1);

    Ok(())
}

And this leads to crash in bevy code here because node_size.min_element() returns -20 the clamp is called with arguments: clamp(0.0, -10.0) which is impossible to perform:

pub fn resolve_single_corner(
radius: Val,
node_size: Vec2,
viewport_size: Vec2,
scale_factor: f32,
) -> f32 {
match radius {
Val::Auto => 0.,
Val::Px(px) => px * scale_factor,
Val::Percent(percent) => node_size.min_element() * percent / 100.,
Val::Vw(percent) => viewport_size.x * percent / 100.,
Val::Vh(percent) => viewport_size.y * percent / 100.,
Val::VMin(percent) => viewport_size.min_element() * percent / 100.,
Val::VMax(percent) => viewport_size.max_element() * percent / 100.,
}
.clamp(0., 0.5 * node_size.min_element())

I am not sure if this is a bug in taffy or is it normal for it to return negative size.

@alice-i-cecile
Copy link
Member

Negative widths being returned is surprising to me, but @nicoburns will know better if that's a taffy bug or something that Bevy needs to be robust to.

@alice-i-cecile alice-i-cecile added S-Needs-Investigation This issue requires detective work to figure out what's going wrong S-Needs-Design This issue requires design work to think about how it would best be accomplished and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged S-Needs-Investigation This issue requires detective work to figure out what's going wrong labels Dec 12, 2024
@ickshonpe
Copy link
Contributor

Yeah we assume that the node size returned from Taffy is always positive. Afaik the sizes should never be negative.
Maybe until there's a proper fix we could just clamp the size equal or above zero when updating the layout and emit a warning instead of panicking when we find a negative extent .

@nicoburns
Copy link
Contributor

Negative node size seems wrong to me. I'll need to look into why this is happening in this case.

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 P-Crash A sudden unexpected crash S-Needs-Design This issue requires design work to think about how it would best be accomplished
Projects
None yet
Development

No branches or pull requests

5 participants