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.

Sign up for GitHub

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

Preserve the order of window child nodes between updates #9764

Closed
wants to merge 1 commit into from
Closed

Preserve the order of window child nodes between updates #9764

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions crates/bevy_ui/src/layout/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use bevy_hierarchy::{Children, Parent};
use bevy_log::warn;
use bevy_math::Vec2;
use bevy_transform::components::Transform;
use bevy_utils::HashMap;
use bevy_utils::{HashMap, HashSet};
use bevy_window::{PrimaryWindow, Window, WindowResolution, WindowScaleFactorChanged};
use std::fmt;
use taffy::{prelude::Size, style_helpers::TaffyMaxContent, Taffy};
Expand Down Expand Up @@ -164,11 +164,25 @@ without UI components as a child of an entity with UI components, results may be
parent_window: Entity,
children: impl Iterator<Item = Entity>,
) {
let taffy_node = self.window_nodes.get(&parent_window).unwrap();
let child_nodes = children
let parent_node = *self.window_nodes.get(&parent_window).unwrap();

let new_window_children = children
.map(|e| *self.entity_to_taffy.get(&e).unwrap())
.collect::<Vec<taffy::node::Node>>();
self.taffy.set_children(*taffy_node, &child_nodes).unwrap();
.collect::<Vec<_>>();

let window_children = if let Ok(mut window_children) = self.taffy.children(parent_node) {
let child_set = new_window_children.iter().copied().collect::<HashSet<_>>();
let mut unique = HashSet::new();
window_children.extend(new_window_children);
window_children.retain(|child| child_set.contains(child) && unique.insert(*child));
window_children
} else {
new_window_children
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this branch ever execute in practice? It seems like taffy.children unconditionally returns Ok.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like taffy.children unconditionally returns Ok

It currently does but arguably shouldn't, as it will panic if the node doesn't exist (note that if the node exists in Taffy, then the children definitely will as children is default initialized to an empty Vec). Likely Taffy's API will split/duplicated into panicking and result-returning variants at some point. See DioxusLabs/taffy#519

};

self.taffy
.set_children(parent_node, &window_children)
.unwrap();
}

/// Compute the layout for each window entity's corresponding root node in the layout.
Expand Down