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

UI node layout doesn't refresh when patching entity children #9492

Closed
viridia opened this issue Aug 19, 2023 · 6 comments
Closed

UI node layout doesn't refresh when patching entity children #9492

viridia opened this issue Aug 19, 2023 · 6 comments
Labels
A-Transform Translations, rotations and scales A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior

Comments

@viridia
Copy link
Contributor

viridia commented Aug 19, 2023

Bevy version

0.11.1

What you did

I'm writing a UI system that has a "virtual DOM" similar to what React does. This entails doing a "diff" between the old and new node hierarchy, and then incrementally patching the tree of entities based on what's changed.

What I have discovered is that Bevy UI doesn't like this very much. Specifically, when I have a node that has some number of children, and I replace some of the children but not others, the layout gets messed up. Even more significant, is that if I resize the window even a tiny bit, the layout fixes itself. So something is not getting refreshed.

Specifically, what I am doing is (pseudo-code):

  • For a given entity
    • visit each child to see if that child changed.
      • If the child changed, replace it with a new child entity and despawn the old one and its descendants.
      • If the child did not change, keep it
    • Now take the vector of all the new child ids - both the ones that were kept from before, and the ones that are new - and call parent.replace_children().
    • Finally, recurse downward and do the same for each child element.

What went wrong

What I'm seeing is that the node layout is random, nodes appearing displaced from where they should be, nodes off the edge of the screen and so on.

Additional information

The problem only seems to manifest when I enable mixing of new and old children. I can disable the bug in one of two ways: either by re-creating the node tree every time (by disabling the diff algorithm), or by not mutating the tree at all.

I realize that you'll probably want a reproducible case. I can provide this, but it's a considerable amount of work and will take some time. That is, I can easily demonstrate the bug in my current github repo, but if you want a "minimal" example I'll need to spend time cutting out all of the parts of the game that aren't relevant to the bug, which can go on a branch or something.

Let me know what you need.

@viridia viridia added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Aug 19, 2023
@ickshonpe
Copy link
Contributor

ickshonpe commented Aug 19, 2023

I think this example captures at least some of your issues:

use bevy::prelude::*;
use bevy::utils::HashMap;

fn main() {
    let mut app = App::new();
    
    app.add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .add_systems(Update, replacement)
        .run();
}

fn setup(mut commands: Commands) {
    commands.spawn(Camera2dBundle::default());
    let root_id = commands.spawn(NodeBundle {
        style: Style {
            flex_direction: FlexDirection::Column,
            justify_content: JustifyContent::Center,
            margin: UiRect::horizontal(Val::Auto),
            row_gap: Val::Px(25.),
            ..Default::default()
        },
        background_color: Color::WHITE.into(),
        ..Default::default()
    })
    .id();

    let mut container = root_id;

    for i in 0..4 {
        let wrapper_id = commands.spawn(NodeBundle {
            style: Style {
                flex_direction: FlexDirection::Column,
                align_items: AlignItems::Center,
                border: UiRect::all(Val::Px(10.)),
                ..Default::default()
            },
            border_color: BorderColor(Color::YELLOW),
            background_color: BackgroundColor(Color::RED),
            ..Default::default()
        })
        .id();
        let row_id = commands.spawn(NodeBundle {
            style: Style {
                flex_direction: FlexDirection::Row,
                column_gap:  Val::Px(25.),
                padding: UiRect::all(Val::Px(25.)),
                ..Default::default()
            },
            background_color: Color::BLACK.into(),            
            ..Default::default()
        }).with_children(|commands| {
            for j in 0..4 {
                commands.spawn(NodeBundle {
                    style: Style {
                        width: Val::Px(70.),
                        height: Val::Px(70.),
                        align_items: AlignItems::Center,
                        justify_content: JustifyContent::Center,
                        ..Default::default()
                    },
                    background_color: Color::WHITE.into(),
                    ..Default::default()
                }).with_children(|commands| {
                    commands.spawn(TextBundle::from_section(format!("{i}, {j}"), TextStyle { font_size: 25., color: Color::BLACK, ..Default::default() }));
                });
            
            }
        })
        .id();
        commands.entity(wrapper_id).add_child(row_id);
        commands.entity(container).add_child(wrapper_id);
        container = wrapper_id;
    }        
}

fn replacement(
    input: Res<Input<KeyCode>>,
    mut commands: Commands,
    query: Query<(Entity, &Children), With<Node>> 
) { 
    if input.just_pressed(KeyCode::Space) {
        let mut children_map: HashMap<Entity, Vec<Entity>> = HashMap::default();

        query.for_each(|(entity, children)| {
            children_map.insert(entity, children.iter().cloned().collect());
        });

        for (entity, children) in children_map {    
            commands.entity(entity)
                .remove_children(&children)
                .replace_children(&children);
        }
    }
}

After startup:

9492_example_capture_1

Then after pressing space to trigger a global children replacement:

9492_example_capture_2

The text has disappeared and we can see a green leaf node in the top left corner.

Then after resizing the window, it resets back to the original state:

9492_example_capture_1

It's only the leaf nodes that get displaced. Perhaps it is because of how the UI queries for entities with changed Children and updates them, but doesn't query for entities with a changed Parent. It might be that I've already fixed this in one of my unmerged ui_layout_system PRs. IIRC there are a couple of subtle problems caused because we do removal detection, hierarchy change detection and hierarchy updates in the wrong order.

@ickshonpe
Copy link
Contributor

This wasn't what I thought it was initially at all, it's a bug in transform propagation. Issue #9517.

@alice-i-cecile alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets A-Transform Translations, rotations and scales and removed S-Needs-Triage This issue needs to be labelled labels Aug 21, 2023
@viridia
Copy link
Contributor Author

viridia commented Aug 21, 2023

This wasn't what I thought it was initially at all, it's a bug in transform propagation. Issue #9517.

I'm attempting to verify that this fixes my problem. Unfortunately I'm having trouble using the bevy main branch - bevy_trait_query no longer works (compilation error), and my UI code depends on that.

@ickshonpe
Copy link
Contributor

ickshonpe commented Aug 21, 2023

You could try a system that forces the transforms to update every frame. Something like this should work:

fn force_update(mut transforms: Query<&mut Transform>) {
   for mut transform in transforms.iter_mut() {
        transform.set_changed();
   }
}

Then, assuming that works, you could make some sort of replace_children_force_update EntityCommands extension method that replaces the children and forces a transform update for the replaced children.

@viridia
Copy link
Contributor Author

viridia commented Aug 21, 2023

You could try a system that forces the transforms to update every frame. Something like this should work:

Yep, that did the trick! Looks beautiful.

github-merge-queue bot pushed a commit that referenced this issue Aug 28, 2023
…tities query (#9518)

# Objective

`sync_simple_transforms` only checks for removed parents and doesn't
filter for `Without<Parent>`, so it overwrites the `GlobalTransform` of
non-orphan entities that were orphaned and then reparented since the
last update.

Introduced by #7264

## Solution

Filter for `Without<Parent>`.

Fixes #9517, #9492

## Changelog

`sync_simple_transforms`:
* Added a `Without<Parent>` filter to the orphaned entities query.
@viridia
Copy link
Contributor Author

viridia commented Dec 5, 2023

This was fixed, can be closed.

@JMS55 JMS55 closed this as completed Dec 16, 2023
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this issue Jan 9, 2024
…tities query (bevyengine#9518)

# Objective

`sync_simple_transforms` only checks for removed parents and doesn't
filter for `Without<Parent>`, so it overwrites the `GlobalTransform` of
non-orphan entities that were orphaned and then reparented since the
last update.

Introduced by bevyengine#7264

## Solution

Filter for `Without<Parent>`.

Fixes bevyengine#9517, bevyengine#9492

## Changelog

`sync_simple_transforms`:
* Added a `Without<Parent>` filter to the orphaned entities query.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Transform Translations, rotations and scales 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

4 participants