From c5742ff43e13b942a5f7a5faecc3b9bf108709e1 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Mon, 30 Sep 2024 19:43:57 +0100 Subject: [PATCH] Simplified `ui_stack_system` (#9889) # Objective `ui_stack_system` generates a tree of `StackingContexts` which it then flattens to get the `UiStack`. But there's no need to construct a new tree. We can query for nodes with a global `ZIndex`, add those nodes to the root nodes list and then build the `UiStack` from a walk of the existing layout tree, ignoring any branches that have a global `Zindex`. Fixes #9877 ## Solution Split the `ZIndex` enum into two separate components, `ZIndex` and `GlobalZIndex` Query for nodes with a `GlobalZIndex`, add those nodes to the root nodes list and then build the `UiStack` from a walk of the existing layout tree, filtering branches by `Without` so we don't revisit nodes. ``` cargo run --profile stress-test --features trace_tracy --example many_buttons ``` ui-stack-system-walk-split-enum (Yellow is this PR, red is main) --- ## Changelog `Zindex` * The `ZIndex` enum has been split into two separate components `ZIndex` (which replaces `ZIndex::Local`) and `GlobalZIndex` (which replaces `ZIndex::Global`). An entity can have both a `ZIndex` and `GlobalZIndex`, in comparisons `ZIndex` breaks ties if two `GlobalZIndex` values are equal. `ui_stack_system` * Instead of generating a tree of `StackingContexts`, query for nodes with a `GlobalZIndex`, add those nodes to the root nodes list and then build the `UiStack` from a walk of the existing layout tree, filtering branches by `Without Co-authored-by: Alice Cecile Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com> --- crates/bevy_dev_tools/src/fps_overlay.rs | 22 +- crates/bevy_ui/src/stack.rs | 298 +++++++++++++---------- crates/bevy_ui/src/ui_node.rs | 33 ++- examples/3d/shadow_biases.rs | 18 +- examples/stress_tests/bevymark.rs | 18 +- examples/ui/z_index.rs | 56 +++-- 6 files changed, 243 insertions(+), 202 deletions(-) diff --git a/crates/bevy_dev_tools/src/fps_overlay.rs b/crates/bevy_dev_tools/src/fps_overlay.rs index d97af217ba..97d0c3989a 100644 --- a/crates/bevy_dev_tools/src/fps_overlay.rs +++ b/crates/bevy_dev_tools/src/fps_overlay.rs @@ -16,11 +16,11 @@ use bevy_render::view::Visibility; use bevy_text::{Font, Text, TextSection, TextStyle}; use bevy_ui::{ node_bundles::{NodeBundle, TextBundle}, - PositionType, Style, ZIndex, + GlobalZIndex, PositionType, Style, }; use bevy_utils::default; -/// Global [`ZIndex`] used to render the fps overlay. +/// [`GlobalZIndex`] used to render the fps overlay. /// /// We use a number slightly under `i32::MAX` so you can render on top of it if you really need to. pub const FPS_OVERLAY_ZINDEX: i32 = i32::MAX - 32; @@ -83,16 +83,18 @@ struct FpsText; fn setup(mut commands: Commands, overlay_config: Res) { commands - .spawn(NodeBundle { - style: Style { - // We need to make sure the overlay doesn't affect the position of other UI nodes - position_type: PositionType::Absolute, + .spawn(( + NodeBundle { + style: Style { + // We need to make sure the overlay doesn't affect the position of other UI nodes + position_type: PositionType::Absolute, + ..default() + }, + // Render overlay on top of everything ..default() }, - // Render overlay on top of everything - z_index: ZIndex::Global(FPS_OVERLAY_ZINDEX), - ..default() - }) + GlobalZIndex(FPS_OVERLAY_ZINDEX), + )) .with_children(|c| { c.spawn(( TextBundle::from_sections([ diff --git a/crates/bevy_ui/src/stack.rs b/crates/bevy_ui/src/stack.rs index 30b6c8e44a..1d1b28df8c 100644 --- a/crates/bevy_ui/src/stack.rs +++ b/crates/bevy_ui/src/stack.rs @@ -3,7 +3,7 @@ use bevy_ecs::prelude::*; use bevy_hierarchy::prelude::*; -use crate::{Node, ZIndex}; +use crate::{GlobalZIndex, Node, ZIndex}; /// The current UI stack, which contains all UI nodes ordered by their depth (back-to-front). /// @@ -15,69 +15,75 @@ pub struct UiStack { pub uinodes: Vec, } -/// Caches stacking context buffers for use in [`ui_stack_system`]. #[derive(Default)] -pub(crate) struct StackingContextCache { - inner: Vec, +pub(crate) struct ChildBufferCache { + pub inner: Vec>, } -impl StackingContextCache { - fn pop(&mut self) -> StackingContext { +impl ChildBufferCache { + fn pop(&mut self) -> Vec<(Entity, i32)> { 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); + fn push(&mut self, vec: Vec<(Entity, i32)>) { + self.inner.push(vec); } } -#[derive(Default)] -struct StackingContext { - entries: Vec, -} - -struct StackingContextEntry { - z_index: i32, - entity: Entity, - stack: StackingContext, -} - /// Generates the render stack for UI nodes. /// -/// First generate a UI node tree (`StackingContext`) based on z-index. -/// Then flatten that tree into back-to-front ordered `UiStack`. -pub(crate) fn ui_stack_system( - mut cache: Local, +/// Create a list of root nodes from unparented entities and entities with a `GlobalZIndex` component. +/// Then build the `UiStack` from a walk of the existing layout trees starting from each root node, +/// filtering branches by `Without`so that we don't revisit nodes. +#[allow(clippy::too_many_arguments)] +pub fn ui_stack_system( + mut cache: Local, + mut root_nodes: Local>, mut ui_stack: ResMut, - root_node_query: Query, Without)>, - zindex_query: Query<&ZIndex, With>, + root_node_query: Query< + (Entity, Option<&GlobalZIndex>, Option<&ZIndex>), + (With, Without), + >, + zindex_global_node_query: Query< + (Entity, &GlobalZIndex, Option<&ZIndex>), + (With, With), + >, children_query: Query<&Children>, + zindex_query: Query, (With, Without)>, mut update_query: Query<&mut Node>, ) { - // Generate `StackingContext` tree - 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, - &mut global_context, - None, - &mut total_entry_count, - ); + ui_stack.uinodes.clear(); + for (id, global_zindex, maybe_zindex) in zindex_global_node_query.iter() { + root_nodes.push(( + id, + ( + global_zindex.0, + maybe_zindex.map(|zindex| zindex.0).unwrap_or(0), + ), + )); } - // Flatten `StackingContext` into `UiStack` - ui_stack.uinodes.clear(); - ui_stack.uinodes.reserve(total_entry_count); - fill_stack_recursively(&mut cache, &mut ui_stack.uinodes, &mut global_context); - cache.push(global_context); + for (id, maybe_global_zindex, maybe_zindex) in root_node_query.iter() { + root_nodes.push(( + id, + ( + maybe_global_zindex.map(|zindex| zindex.0).unwrap_or(0), + maybe_zindex.map(|zindex| zindex.0).unwrap_or(0), + ), + )); + } + + root_nodes.sort_by_key(|(_, z)| *z); + + for (root_entity, _) in root_nodes.drain(..) { + update_uistack_recursive( + &mut cache, + root_entity, + &children_query, + &zindex_query, + &mut ui_stack.uinodes, + ); + } for (i, entity) in ui_stack.uinodes.iter().enumerate() { if let Ok(mut node) = update_query.get_mut(*entity) { @@ -86,67 +92,28 @@ pub(crate) fn ui_stack_system( } } -/// Generate z-index based UI node tree -fn insert_context_hierarchy( - cache: &mut StackingContextCache, - zindex_query: &Query<&ZIndex, With>, +fn update_uistack_recursive( + cache: &mut ChildBufferCache, + node_entity: Entity, children_query: &Query<&Children>, - entity: Entity, - global_context: &mut StackingContext, - parent_context: Option<&mut StackingContext>, - total_entry_count: &mut usize, + zindex_query: &Query, (With, Without)>, + ui_stack: &mut Vec, ) { - let mut new_context = cache.pop(); + ui_stack.push(node_entity); - if let Ok(children) = children_query.get(entity) { - // Reserve space for all children. In practice, some may not get pushed since - // nodes with `ZIndex::Global` are pushed to the global (root) context. - new_context.entries.reserve_exact(children.len()); - - for entity in children { - insert_context_hierarchy( - cache, - zindex_query, - children_query, - *entity, - global_context, - Some(&mut new_context), - total_entry_count, - ); + if let Ok(children) = children_query.get(node_entity) { + let mut child_buffer = cache.pop(); + child_buffer.extend(children.iter().filter_map(|child_entity| { + zindex_query + .get(*child_entity) + .ok() + .map(|zindex| (*child_entity, zindex.map(|zindex| zindex.0).unwrap_or(0))) + })); + child_buffer.sort_by_key(|k| k.1); + for (child_entity, _) in child_buffer.drain(..) { + update_uistack_recursive(cache, child_entity, children_query, zindex_query, ui_stack); } - } - - // The node will be added either to global/parent based on its z-index type: global/local. - let z_index = zindex_query.get(entity).unwrap_or(&ZIndex::Local(0)); - let (entity_context, z_index) = match z_index { - ZIndex::Local(value) => (parent_context.unwrap_or(global_context), *value), - ZIndex::Global(value) => (global_context, *value), - }; - - *total_entry_count += 1; - entity_context.entries.push(StackingContextEntry { - z_index, - entity, - stack: new_context, - }); -} - -/// Flatten `StackingContext` (z-index based UI node tree) into back-to-front entities list -fn fill_stack_recursively( - cache: &mut StackingContextCache, - result: &mut Vec, - 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 mut entry in stack.entries.drain(..) { - // Parent node renders before/behind child nodes - result.push(entry.entity); - fill_stack_recursively(cache, result, &mut entry.stack); - cache.push(entry.stack); + cache.push(child_buffer); } } @@ -160,15 +127,35 @@ mod tests { }; use bevy_hierarchy::{BuildChildren, ChildBuild}; - use crate::{Node, UiStack, ZIndex}; + use crate::{GlobalZIndex, Node, UiStack, ZIndex}; use super::ui_stack_system; #[derive(Component, PartialEq, Debug, Clone)] struct Label(&'static str); - fn node_with_zindex(name: &'static str, z_index: ZIndex) -> (Label, Node, ZIndex) { - (Label(name), Node::default(), z_index) + fn node_with_global_and_local_zindex( + name: &'static str, + global_zindex: i32, + local_zindex: i32, + ) -> (Label, Node, GlobalZIndex, ZIndex) { + ( + Label(name), + Node::default(), + GlobalZIndex(global_zindex), + ZIndex(local_zindex), + ) + } + + fn node_with_global_zindex( + name: &'static str, + global_zindex: i32, + ) -> (Label, Node, GlobalZIndex) { + (Label(name), Node::default(), GlobalZIndex(global_zindex)) + } + + fn node_with_zindex(name: &'static str, zindex: i32) -> (Label, Node, ZIndex) { + (Label(name), Node::default(), ZIndex(zindex)) } fn node_without_zindex(name: &'static str) -> (Label, Node) { @@ -188,24 +175,24 @@ mod tests { let mut queue = CommandQueue::default(); let mut commands = Commands::new(&mut queue, &world); - commands.spawn(node_with_zindex("0", ZIndex::Global(2))); + commands.spawn(node_with_global_zindex("0", 2)); commands - .spawn(node_with_zindex("1", ZIndex::Local(1))) + .spawn(node_with_zindex("1", 1)) .with_children(|parent| { parent .spawn(node_without_zindex("1-0")) .with_children(|parent| { parent.spawn(node_without_zindex("1-0-0")); parent.spawn(node_without_zindex("1-0-1")); - parent.spawn(node_with_zindex("1-0-2", ZIndex::Local(-1))); + parent.spawn(node_with_zindex("1-0-2", -1)); }); parent.spawn(node_without_zindex("1-1")); parent - .spawn(node_with_zindex("1-2", ZIndex::Global(-1))) + .spawn(node_with_global_zindex("1-2", -1)) .with_children(|parent| { parent.spawn(node_without_zindex("1-2-0")); - parent.spawn(node_with_zindex("1-2-1", ZIndex::Global(-3))); + parent.spawn(node_with_global_zindex("1-2-1", -3)); parent .spawn(node_without_zindex("1-2-2")) .with_children(|_| ()); @@ -227,7 +214,7 @@ mod tests { }); }); - commands.spawn(node_with_zindex("3", ZIndex::Global(-2))); + commands.spawn(node_with_global_zindex("3", -2)); queue.apply(&mut world); @@ -243,25 +230,74 @@ mod tests { .map(|entity| query.get(&world, *entity).unwrap().clone()) .collect::>(); let expected_result = vec![ - Label("1-2-1"), // ZIndex::Global(-3) - Label("3"), // ZIndex::Global(-2) - Label("1-2"), // ZIndex::Global(-1) - Label("1-2-0"), - Label("1-2-2"), - Label("1-2-3"), - Label("2"), - Label("2-0"), - Label("2-1"), - Label("2-1-0"), - Label("1"), // ZIndex::Local(1) - Label("1-0"), - Label("1-0-2"), // ZIndex::Local(-1) - Label("1-0-0"), - Label("1-0-1"), - Label("1-1"), - Label("1-3"), - Label("0"), // ZIndex::Global(2) + (Label("1-2-1")), // GlobalZIndex(-3) + (Label("3")), // GlobalZIndex(-2) + (Label("1-2")), // GlobalZIndex(-1) + (Label("1-2-0")), + (Label("1-2-2")), + (Label("1-2-3")), + (Label("2")), + (Label("2-0")), + (Label("2-1")), + (Label("2-1-0")), + (Label("1")), // ZIndex(1) + (Label("1-0")), + (Label("1-0-2")), // ZIndex(-1) + (Label("1-0-0")), + (Label("1-0-1")), + (Label("1-1")), + (Label("1-3")), + (Label("0")), // GlobalZIndex(2) ]; assert_eq!(actual_result, expected_result); } + + #[test] + fn test_with_equal_global_zindex_zindex_decides_order() { + let mut world = World::default(); + world.init_resource::(); + + let mut queue = CommandQueue::default(); + let mut commands = Commands::new(&mut queue, &world); + commands.spawn(node_with_global_and_local_zindex("0", -1, 1)); + commands.spawn(node_with_global_and_local_zindex("1", -1, 2)); + commands.spawn(node_with_global_and_local_zindex("2", 1, 3)); + commands.spawn(node_with_global_and_local_zindex("3", 1, -3)); + commands + .spawn(node_without_zindex("4")) + .with_children(|builder| { + builder.spawn(node_with_global_and_local_zindex("5", 0, -1)); + builder.spawn(node_with_global_and_local_zindex("6", 0, 1)); + builder.spawn(node_with_global_and_local_zindex("7", -1, -1)); + builder.spawn(node_with_global_zindex("8", 1)); + }); + + queue.apply(&mut world); + + let mut schedule = Schedule::default(); + schedule.add_systems(ui_stack_system); + schedule.run(&mut world); + + let mut query = world.query::<&Label>(); + let ui_stack = world.resource::(); + let actual_result = ui_stack + .uinodes + .iter() + .map(|entity| query.get(&world, *entity).unwrap().clone()) + .collect::>(); + + let expected_result = vec![ + (Label("7")), + (Label("0")), + (Label("1")), + (Label("5")), + (Label("4")), + (Label("6")), + (Label("3")), + (Label("8")), + (Label("2")), + ]; + + assert_eq!(actual_result, expected_result); + } } diff --git a/crates/bevy_ui/src/ui_node.rs b/crates/bevy_ui/src/ui_node.rs index c4af28e4c7..dac55d0dd4 100644 --- a/crates/bevy_ui/src/ui_node.rs +++ b/crates/bevy_ui/src/ui_node.rs @@ -2052,26 +2052,21 @@ pub struct CalculatedClip { /// appear in the UI hierarchy. In such a case, the last node to be added to its parent /// will appear in front of its siblings. /// -/// Internally, nodes with a global z-index share the stacking context of root UI nodes -/// (nodes that have no parent). Because of this, there is no difference between using -/// `ZIndex::Local(n)` and `ZIndex::Global(n)` for root nodes. -/// -/// Nodes without this component will be treated as if they had a value of `ZIndex::Local(0)`. -#[derive(Component, Copy, Clone, Debug, PartialEq, Eq, Reflect)] -#[reflect(Component, Default, Debug, PartialEq)] -pub enum ZIndex { - /// Indicates the order in which this node should be rendered relative to its siblings. - Local(i32), - /// Indicates the order in which this node should be rendered relative to root nodes and - /// all other nodes that have a global z-index. - Global(i32), -} -impl Default for ZIndex { - fn default() -> Self { - Self::Local(0) - } -} +/// Nodes without this component will be treated as if they had a value of [`ZIndex(0)`]. +#[derive(Component, Copy, Clone, Debug, Default, PartialEq, Eq, Reflect)] +#[reflect(Component, Default, Debug, PartialEq)] +pub struct ZIndex(pub i32); + +/// `GlobalZIndex` allows a [`Node`] entity anywhere in the UI hierarchy to escape the implicit draw ordering of the UI's layout tree and +/// be rendered above or below other UI nodes. +/// Nodes with a `GlobalZIndex` of greater than 0 will be drawn on top of nodes without a `GlobalZIndex` or nodes with a lower `GlobalZIndex`. +/// Nodes with a `GlobalZIndex` of less than 0 will be drawn below nodes without a `GlobalZIndex` or nodes with a greater `GlobalZIndex`. +/// +/// If two Nodes have the same `GlobalZIndex`, the node with the greater [`ZIndex`] will be drawn on top. +#[derive(Component, Copy, Clone, Debug, Default, PartialEq, Eq, Reflect)] +#[reflect(Component, Default, Debug, PartialEq)] +pub struct GlobalZIndex(pub i32); /// Used to add rounded corners to a UI node. You can set a UI node to have uniformly /// rounded corners or specify different radii for each corner. If a given radius exceeds half diff --git a/examples/3d/shadow_biases.rs b/examples/3d/shadow_biases.rs index e0bb9cde1e..ffcc1e70e6 100644 --- a/examples/3d/shadow_biases.rs +++ b/examples/3d/shadow_biases.rs @@ -116,16 +116,18 @@ fn setup( let style = TextStyle::default(); commands - .spawn(NodeBundle { - style: Style { - position_type: PositionType::Absolute, - padding: UiRect::all(Val::Px(5.0)), + .spawn(( + NodeBundle { + style: Style { + position_type: PositionType::Absolute, + padding: UiRect::all(Val::Px(5.0)), + ..default() + }, + background_color: Color::BLACK.with_alpha(0.75).into(), ..default() }, - z_index: ZIndex::Global(i32::MAX), - background_color: Color::BLACK.with_alpha(0.75).into(), - ..default() - }) + GlobalZIndex(i32::MAX), + )) .with_children(|c| { c.spawn(TextBundle::from_sections([ TextSection::new("Controls:\n", style.clone()), diff --git a/examples/stress_tests/bevymark.rs b/examples/stress_tests/bevymark.rs index e0959cc930..97c9115fcf 100644 --- a/examples/stress_tests/bevymark.rs +++ b/examples/stress_tests/bevymark.rs @@ -270,16 +270,18 @@ fn setup( commands.spawn(Camera2dBundle::default()); commands - .spawn(NodeBundle { - style: Style { - position_type: PositionType::Absolute, - padding: UiRect::all(Val::Px(5.0)), + .spawn(( + NodeBundle { + style: Style { + position_type: PositionType::Absolute, + padding: UiRect::all(Val::Px(5.0)), + ..default() + }, + background_color: Color::BLACK.with_alpha(0.75).into(), ..default() }, - z_index: ZIndex::Global(i32::MAX), - background_color: Color::BLACK.with_alpha(0.75).into(), - ..default() - }) + GlobalZIndex(i32::MAX), + )) .with_children(|c| { c.spawn(( TextBundle::from_sections([ diff --git a/examples/ui/z_index.rs b/examples/ui/z_index.rs index 475af2d27c..f3a1a1f0b0 100644 --- a/examples/ui/z_index.rs +++ b/examples/ui/z_index.rs @@ -62,7 +62,7 @@ fn setup(mut commands: Commands) { // spawn a node with a positive local z-index of 2. // it will show above other nodes in the gray container. parent.spawn(NodeBundle { - z_index: ZIndex::Local(2), + z_index: ZIndex(2), background_color: BLUE.into(), style: Style { position_type: PositionType::Absolute, @@ -78,7 +78,7 @@ fn setup(mut commands: Commands) { // spawn a node with a negative local z-index. // it will show under other nodes in the gray container. parent.spawn(NodeBundle { - z_index: ZIndex::Local(-1), + z_index: ZIndex(-1), background_color: LIME.into(), style: Style { position_type: PositionType::Absolute, @@ -94,36 +94,40 @@ fn setup(mut commands: Commands) { // spawn a node with a positive global z-index of 1. // it will show above all other nodes, because it's the highest global z-index in this example. // by default, boxes all share the global z-index of 0 that the gray container is added to. - parent.spawn(NodeBundle { - z_index: ZIndex::Global(1), - background_color: PURPLE.into(), - style: Style { - position_type: PositionType::Absolute, - left: Val::Px(15.0), - bottom: Val::Px(10.0), - width: Val::Px(100.), - height: Val::Px(60.), - ..default() + parent.spawn(( + NodeBundle { + background_color: PURPLE.into(), + style: Style { + position_type: PositionType::Absolute, + left: Val::Px(15.0), + bottom: Val::Px(10.0), + width: Val::Px(100.), + height: Val::Px(60.), + ..default() + }, + ..Default::default() }, - ..default() - }); + GlobalZIndex(1), + )); // spawn a node with a negative global z-index of -1. // this will show under all other nodes including its parent, because it's the lowest global z-index // in this example. - parent.spawn(NodeBundle { - z_index: ZIndex::Global(-1), - background_color: YELLOW.into(), - style: Style { - position_type: PositionType::Absolute, - left: Val::Px(-15.0), - bottom: Val::Px(-15.0), - width: Val::Px(100.), - height: Val::Px(125.), - ..default() + parent.spawn(( + NodeBundle { + background_color: YELLOW.into(), + style: Style { + position_type: PositionType::Absolute, + left: Val::Px(-15.0), + bottom: Val::Px(-15.0), + width: Val::Px(100.), + height: Val::Px(125.), + ..default() + }, + ..Default::default() }, - ..default() - }); + GlobalZIndex(-1), + )); }); }); }