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

Reduce steady-state allocations in ui_stack_system #12413

Merged
merged 7 commits into from
Mar 12, 2024
Merged
Changes from 6 commits
Commits
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
40 changes: 34 additions & 6 deletions crates/bevy_ui/src/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,24 @@ pub struct UiStack {
pub uinodes: Vec<Entity>,
}

#[derive(Default)]
pub struct StackingContextCache {
Copy link
Member

Choose a reason for hiding this comment

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

This needs docs if it's public. I would probably add docs for internal users and doc(hidden) this.

Copy link
Contributor Author

@UkoeHB UkoeHB Mar 11, 2024

Choose a reason for hiding this comment

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

It's public because the system is public even though it isn't actually exported. The visibility controls in this module don't make any sense.

EDIT: Added a commit to clean up visibility in this module.

inner: Vec<StackingContext>,
}

impl StackingContextCache {
fn pop(&mut self) -> StackingContext {
self.inner.pop().unwrap_or_default()
}

fn push(&mut self, mut context: StackingContext) {
for entry in context.entries.drain(..) {
self.push(entry.stack);
}
self.inner.push(context);
}
}

#[derive(Default)]
struct StackingContext {
pub entries: Vec<StackingContextEntry>,
Expand All @@ -31,18 +49,20 @@ struct StackingContextEntry {
/// First generate a UI node tree (`StackingContext`) based on z-index.
/// Then flatten that tree into back-to-front ordered `UiStack`.
pub fn ui_stack_system(
mut cache: Local<StackingContextCache>,
mut ui_stack: ResMut<UiStack>,
root_node_query: Query<Entity, (With<Node>, Without<Parent>)>,
zindex_query: Query<&ZIndex, With<Node>>,
children_query: Query<&Children>,
mut update_query: Query<&mut Node>,
) {
// Generate `StackingContext` tree
let mut global_context = StackingContext::default();
let mut global_context = cache.pop();
let mut total_entry_count: usize = 0;

for entity in &root_node_query {
insert_context_hierarchy(
&mut cache,
&zindex_query,
&children_query,
entity,
Expand All @@ -55,7 +75,8 @@ pub fn ui_stack_system(
// Flatten `StackingContext` into `UiStack`
ui_stack.uinodes.clear();
ui_stack.uinodes.reserve(total_entry_count);
fill_stack_recursively(&mut ui_stack.uinodes, &mut global_context);
fill_stack_recursively(&mut cache, &mut ui_stack.uinodes, &mut global_context);
cache.push(global_context);

for (i, entity) in ui_stack.uinodes.iter().enumerate() {
if let Ok(mut node) = update_query.get_mut(*entity) {
Expand All @@ -66,14 +87,15 @@ pub fn ui_stack_system(

/// Generate z-index based UI node tree
fn insert_context_hierarchy(
cache: &mut StackingContextCache,
zindex_query: &Query<&ZIndex, With<Node>>,
children_query: &Query<&Children>,
entity: Entity,
global_context: &mut StackingContext,
parent_context: Option<&mut StackingContext>,
total_entry_count: &mut usize,
) {
let mut new_context = StackingContext::default();
let mut new_context = cache.pop();

if let Ok(children) = children_query.get(entity) {
// Reserve space for all children. In practice, some may not get pushed since
Expand All @@ -82,6 +104,7 @@ fn insert_context_hierarchy(

for entity in children {
insert_context_hierarchy(
cache,
zindex_query,
children_query,
*entity,
Expand All @@ -108,16 +131,21 @@ fn insert_context_hierarchy(
}

/// Flatten `StackingContext` (z-index based UI node tree) into back-to-front entities list
fn fill_stack_recursively(result: &mut Vec<Entity>, stack: &mut StackingContext) {
fn fill_stack_recursively(
cache: &mut StackingContextCache,
result: &mut Vec<Entity>,
stack: &mut StackingContext,
) {
// Sort entries by ascending z_index, while ensuring that siblings
// with the same local z_index will keep their ordering. This results
// in `back-to-front` ordering, low z_index = back; high z_index = front.
stack.entries.sort_by_key(|e| e.z_index);

for entry in &mut stack.entries {
for mut entry in stack.entries.drain(..) {
// Parent node renders before/behind child nodes
result.push(entry.entity);
fill_stack_recursively(result, &mut entry.stack);
fill_stack_recursively(cache, result, &mut entry.stack);
cache.push(entry.stack);
}
}

Expand Down