From dd1b08ef8b8711fa1ee89c09d96e6999cf2e810b Mon Sep 17 00:00:00 2001 From: SvenTS <73037851+SvenTS@users.noreply.github.com> Date: Thu, 26 Nov 2020 01:55:55 +0100 Subject: [PATCH] Improve ui depth system (#905) * Add test for ui-z system * Remove generic hierarchy runner and refactor ui z-system * Remove different handling for childless nodes Having an empty children list should be the same as having no child component. * Further simplify system after change --- .../bevy_transform/src/hierarchy/hierarchy.rs | 33 +--- crates/bevy_ui/src/update.rs | 154 +++++++++++++++--- 2 files changed, 128 insertions(+), 59 deletions(-) diff --git a/crates/bevy_transform/src/hierarchy/hierarchy.rs b/crates/bevy_transform/src/hierarchy/hierarchy.rs index e9ec1070bc..0668dadf01 100644 --- a/crates/bevy_transform/src/hierarchy/hierarchy.rs +++ b/crates/bevy_transform/src/hierarchy/hierarchy.rs @@ -1,38 +1,7 @@ use crate::components::{Children, Parent}; -use bevy_ecs::{Command, Commands, Entity, Query, Resources, World}; +use bevy_ecs::{Command, Commands, Entity, Resources, World}; use bevy_utils::tracing::debug; -pub fn run_on_hierarchy( - children_query: &Query<&Children>, - state: &mut S, - entity: Entity, - parent_result: Option, - mut previous_result: Option, - run: &mut dyn FnMut(&mut S, Entity, Option, Option) -> Option, -) -> Option -where - T: Clone, -{ - let parent_result = run(state, entity, parent_result, previous_result); - previous_result = None; - if let Ok(children) = children_query.get(entity) { - for child in children.iter().cloned() { - previous_result = run_on_hierarchy( - children_query, - state, - child, - parent_result.clone(), - previous_result, - run, - ); - } - } else { - previous_result = parent_result; - } - - previous_result -} - #[derive(Debug)] pub struct DespawnRecursive { entity: Entity, diff --git a/crates/bevy_ui/src/update.rs b/crates/bevy_ui/src/update.rs index b0dde1566e..498a487596 100644 --- a/crates/bevy_ui/src/update.rs +++ b/crates/bevy_ui/src/update.rs @@ -1,49 +1,149 @@ use super::Node; use bevy_ecs::{Entity, Query, With, Without}; -use bevy_transform::{ - hierarchy, - prelude::{Children, Parent, Transform}, -}; +use bevy_transform::prelude::{Children, Parent, Transform}; pub const UI_Z_STEP: f32 = 0.001; pub fn ui_z_system( root_node_query: Query, Without)>, - mut node_query: Query<(Entity, &mut Transform), With>, + mut node_query: Query<&mut Transform, With>, children_query: Query<&Children>, ) { let mut current_global_z = 0.0; - for entity in root_node_query.iter() { - if let Some(result) = hierarchy::run_on_hierarchy( + current_global_z = update_hierarchy( &children_query, &mut node_query, entity, - Some(current_global_z), - Some(current_global_z), - &mut update_node_entity, - ) { - current_global_z = result; + current_global_z, + current_global_z, + ); + } +} + +fn update_hierarchy( + children_query: &Query<&Children>, + node_query: &mut Query<&mut Transform, With>, + entity: Entity, + parent_global_z: f32, + mut current_global_z: f32, +) -> f32 { + current_global_z += UI_Z_STEP; + if let Ok(mut transform) = node_query.get_mut(entity) { + transform.translation.z = current_global_z - parent_global_z; + } + if let Ok(children) = children_query.get(entity) { + let current_parent_global_z = current_global_z; + for child in children.iter().cloned() { + current_global_z = update_hierarchy( + children_query, + node_query, + child, + current_parent_global_z, + current_global_z, + ); } } + current_global_z } +#[cfg(test)] +mod tests { + use bevy_ecs::{Commands, IntoSystem, Resources, Schedule, World}; + use bevy_transform::{components::Transform, hierarchy::BuildChildren}; -fn update_node_entity( - node_query: &mut Query<(Entity, &mut Transform), With>, - entity: Entity, - parent_result: Option, - previous_result: Option, -) -> Option { - let mut z = UI_Z_STEP; - let parent_global_z = parent_result.unwrap(); - if let Some(previous_global_z) = previous_result { - z += previous_global_z - parent_global_z; - }; - let global_z = z + parent_global_z; + use crate::Node; - if let Ok(mut transform) = node_query.get_component_mut::(entity) { - transform.translation.z = z; + use super::{ui_z_system, UI_Z_STEP}; + + fn node_with_transform(name: &str) -> (String, Node, Transform) { + (name.to_owned(), Node::default(), Transform::default()) } - Some(global_z) + fn node_without_transform(name: &str) -> (String, Node) { + (name.to_owned(), Node::default()) + } + + fn get_steps(transform: &Transform) -> u32 { + (transform.translation.z / UI_Z_STEP).round() as u32 + } + + #[test] + fn test_ui_z_system() { + let mut world = World::default(); + let mut resources = Resources::default(); + let mut commands = Commands::default(); + commands.set_entity_reserver(world.get_entity_reserver()); + + commands.spawn(node_with_transform("0")); + + commands + .spawn(node_with_transform("1")) + .with_children(|parent| { + parent + .spawn(node_with_transform("1-0")) + .with_children(|parent| { + parent.spawn(node_with_transform("1-0-0")); + parent.spawn(node_without_transform("1-0-1")); + parent.spawn(node_with_transform("1-0-2")); + }); + parent.spawn(node_with_transform("1-1")); + parent + .spawn(node_without_transform("1-2")) + .with_children(|parent| { + parent.spawn(node_with_transform("1-2-0")); + parent.spawn(node_with_transform("1-2-1")); + parent + .spawn(node_with_transform("1-2-2")) + .with_children(|_| ()); + parent.spawn(node_with_transform("1-2-3")); + }); + parent.spawn(node_with_transform("1-3")); + }); + + commands + .spawn(node_without_transform("2")) + .with_children(|parent| { + parent + .spawn(node_with_transform("2-0")) + .with_children(|_parent| ()); + parent + .spawn(node_with_transform("2-1")) + .with_children(|parent| { + parent.spawn(node_with_transform("2-1-0")); + }); + }); + commands.apply(&mut world, &mut resources); + + let mut schedule = Schedule::default(); + schedule.add_stage("update"); + schedule.add_system_to_stage("update", ui_z_system.system()); + schedule.initialize(&mut world, &mut resources); + schedule.run(&mut world, &mut resources); + + let mut actual_result = world + .query::<(&String, &Transform)>() + .map(|(name, transform)| (name.clone(), get_steps(transform))) + .collect::>(); + actual_result.sort_unstable_by_key(|(name, _)| name.clone()); + let expected_result = vec![ + ("0".to_owned(), 1), + ("1".to_owned(), 1), + ("1-0".to_owned(), 1), + ("1-0-0".to_owned(), 1), + // 1-0-1 has no transform + ("1-0-2".to_owned(), 3), + ("1-1".to_owned(), 5), + // 1-2 has no transform + ("1-2-0".to_owned(), 1), + ("1-2-1".to_owned(), 2), + ("1-2-2".to_owned(), 3), + ("1-2-3".to_owned(), 4), + ("1-3".to_owned(), 11), + // 2 has no transform + ("2-0".to_owned(), 1), + ("2-1".to_owned(), 2), + ("2-1-0".to_owned(), 1), + ]; + assert_eq!(actual_result, expected_result); + } }