From b699e81f96127140d7918a77f8974858971bd0cf Mon Sep 17 00:00:00 2001 From: bstriker Date: Sun, 12 May 2024 23:20:28 -0400 Subject: [PATCH 1/4] #12803 Changes (squashed) Apply rustfmt Move test_initialization next to other tests Add doc comment to is_root_node_pair_valid Move helper methods into the single test case where they are used Add tests for helper methods Remove trait helpers to reduce complexity of tests Mark specific test functions as unreachable Move ui_surface test only methods into mod tests as trait Fix tests after rebase Add tests for bevy_ui/layout/ui_surface Widen type for parameter children in UiSurface::update_children Add missing asserts and Debug fields in UiSurface from #12268 and #12698 --- crates/bevy_ui/src/layout/ui_surface.rs | 418 +++++++++++++++++++++++- 1 file changed, 416 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 2ba6eed374383..9bba310adb4ba 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -4,7 +4,6 @@ use taffy::TaffyTree; use bevy_ecs::entity::{Entity, EntityHashMap}; use bevy_ecs::prelude::Resource; -use bevy_hierarchy::Children; use bevy_math::UVec2; use bevy_utils::default; use bevy_utils::tracing::warn; @@ -31,6 +30,8 @@ pub struct UiSurface { fn _assert_send_sync_ui_surface_impl_safe() { fn _assert_send_sync() {} _assert_send_sync::>(); + _assert_send_sync::>>(); + _assert_send_sync::>>(); _assert_send_sync::>(); _assert_send_sync::(); } @@ -39,6 +40,7 @@ impl fmt::Debug for UiSurface { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.debug_struct("UiSurface") .field("entity_to_taffy", &self.entity_to_taffy) + .field("camera_entity_to_taffy", &self.camera_entity_to_taffy) .field("camera_roots", &self.camera_roots) .finish() } @@ -112,7 +114,7 @@ impl UiSurface { } /// Update the children of the taffy node corresponding to the given [`Entity`]. - pub fn update_children(&mut self, entity: Entity, children: &Children) { + pub fn update_children(&mut self, entity: Entity, children: &[Entity]) { let mut taffy_children = Vec::with_capacity(children.len()); for child in children { if let Some(taffy_node) = self.entity_to_taffy.get(child) { @@ -271,3 +273,415 @@ with UI components as a child of an entity without UI components, results may be } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ContentSize, FixedMeasure}; + use bevy_math::Vec2; + use taffy::TraversePartialTree; + + const TEST_LAYOUT_CONTEXT: LayoutContext = LayoutContext { + scale_factor: 1.0, + physical_size: Vec2::ONE, + min_size: 0.0, + max_size: 1.0, + }; + + /// Checks if the parent of the `user_root_node` in a `RootNodePair` + /// is correctly assigned as the `implicit_viewport_node`. + fn is_root_node_pair_valid( + taffy_tree: &TaffyTree, + root_node_pair: &RootNodePair, + ) -> bool { + taffy_tree.parent(root_node_pair.user_root_node) + == Some(root_node_pair.implicit_viewport_node) + } + + /// Tries to get the root node pair for a given root node entity with the specified camera entity + fn get_root_node_pair_exact( + ui_surface: &UiSurface, + root_node_entity: Entity, + camera_entity: Entity, + ) -> Option<&RootNodePair> { + let root_node_pairs = ui_surface.camera_roots.get(&camera_entity)?; + let root_node_taffy = ui_surface.entity_to_taffy.get(&root_node_entity)?; + root_node_pairs + .iter() + .find(|&root_node_pair| root_node_pair.user_root_node == *root_node_taffy) + } + + #[test] + fn test_initialization() { + let ui_surface = UiSurface::default(); + assert!(ui_surface.entity_to_taffy.is_empty()); + assert!(ui_surface.camera_entity_to_taffy.is_empty()); + assert!(ui_surface.camera_roots.is_empty()); + assert_eq!(ui_surface.taffy.total_node_count(), 0); + } + + #[test] + fn test_upsert() { + let mut ui_surface = UiSurface::default(); + let camera_entity = Entity::from_raw(0); + let root_node_entity = Entity::from_raw(1); + let style = Style::default(); + + // standard upsert + ui_surface.upsert_node(&TEST_LAYOUT_CONTEXT, root_node_entity, &style, None); + + // should be inserted into taffy + assert_eq!(ui_surface.taffy.total_node_count(), 1); + assert!(ui_surface.entity_to_taffy.contains_key(&root_node_entity)); + + // test duplicate insert 1 + ui_surface.upsert_node(&TEST_LAYOUT_CONTEXT, root_node_entity, &style, None); + + // node count should not have increased + assert_eq!(ui_surface.taffy.total_node_count(), 1); + + // assign root node to camera + ui_surface.set_camera_children(camera_entity, vec![root_node_entity].into_iter()); + + // each root node will create 2 taffy nodes + assert_eq!(ui_surface.taffy.total_node_count(), 2); + + // root node pair should now exist + let root_node_pair = get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity) + .expect("expected root node pair"); + assert!(is_root_node_pair_valid(&ui_surface.taffy, root_node_pair)); + + // test duplicate insert 2 + ui_surface.upsert_node(&TEST_LAYOUT_CONTEXT, root_node_entity, &style, None); + + // node count should not have increased + assert_eq!(ui_surface.taffy.total_node_count(), 2); + + // root node pair should be unaffected + let root_node_pair = get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity) + .expect("expected root node pair"); + assert!(is_root_node_pair_valid(&ui_surface.taffy, root_node_pair)); + } + + #[test] + fn test_get_root_node_pair_exact() { + /// Attempts to find the camera entity that holds a reference to the given root node entity + fn get_associated_camera_entity( + ui_surface: &UiSurface, + root_node_entity: Entity, + ) -> Option { + for (&camera_entity, root_node_map) in ui_surface.camera_entity_to_taffy.iter() { + if root_node_map.contains_key(&root_node_entity) { + return Some(camera_entity); + } + } + None + } + + /// Attempts to find the root node pair corresponding to the given root node entity + fn get_root_node_pair( + ui_surface: &UiSurface, + root_node_entity: Entity, + ) -> Option<&RootNodePair> { + let camera_entity = get_associated_camera_entity(ui_surface, root_node_entity)?; + get_root_node_pair_exact(ui_surface, root_node_entity, camera_entity) + } + + let mut ui_surface = UiSurface::default(); + let camera_entity = Entity::from_raw(0); + let root_node_entity = Entity::from_raw(1); + let style = Style::default(); + + ui_surface.upsert_node(&TEST_LAYOUT_CONTEXT, root_node_entity, &style, None); + + // assign root node to camera + ui_surface.set_camera_children(camera_entity, [root_node_entity].into_iter()); + + assert_eq!( + get_associated_camera_entity(&ui_surface, root_node_entity), + Some(camera_entity) + ); + assert_eq!( + get_associated_camera_entity(&ui_surface, Entity::from_raw(2)), + None + ); + + let root_node_pair = get_root_node_pair(&ui_surface, root_node_entity); + assert!(root_node_pair.is_some()); + assert_eq!( + Some(root_node_pair.unwrap().user_root_node).as_ref(), + ui_surface.entity_to_taffy.get(&root_node_entity) + ); + + assert_eq!( + get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity), + root_node_pair + ); + } + + #[allow(unreachable_code)] + #[test] + fn test_remove_camera_entities() { + let mut ui_surface = UiSurface::default(); + let camera_entity = Entity::from_raw(0); + let root_node_entity = Entity::from_raw(1); + let style = Style::default(); + + ui_surface.upsert_node(&TEST_LAYOUT_CONTEXT, root_node_entity, &style, None); + + // assign root node to camera + ui_surface.set_camera_children(camera_entity, [root_node_entity].into_iter()); + + assert!(ui_surface + .camera_entity_to_taffy + .contains_key(&camera_entity)); + assert!(ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&root_node_entity)); + assert!(ui_surface.camera_roots.contains_key(&camera_entity)); + let root_node_pair = get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity) + .expect("expected root node pair"); + assert!(ui_surface + .camera_roots + .get(&camera_entity) + .unwrap() + .contains(root_node_pair)); + + ui_surface.remove_camera_entities([camera_entity]); + + // should not affect `entity_to_taffy` + assert!(ui_surface.entity_to_taffy.contains_key(&root_node_entity)); + + // `camera_roots` and `camera_entity_to_taffy` should no longer contain entries for `camera_entity` + assert!(!ui_surface + .camera_entity_to_taffy + .contains_key(&camera_entity)); + + return; // TODO: can't pass the test if we continue - not implemented (remove allow(unreachable_code)) + + assert!(!ui_surface.camera_roots.contains_key(&camera_entity)); + + // root node pair should be removed + let root_node_pair = get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity); + assert_eq!(root_node_pair, None); + } + + #[allow(unreachable_code)] + #[test] + fn test_remove_entities() { + let mut ui_surface = UiSurface::default(); + let camera_entity = Entity::from_raw(0); + let root_node_entity = Entity::from_raw(1); + let style = Style::default(); + + ui_surface.upsert_node(&TEST_LAYOUT_CONTEXT, root_node_entity, &style, None); + + ui_surface.set_camera_children(camera_entity, [root_node_entity].into_iter()); + + assert!(ui_surface.entity_to_taffy.contains_key(&root_node_entity)); + assert!(ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&root_node_entity)); + let root_node_pair = + get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity).unwrap(); + assert!(ui_surface + .camera_roots + .get(&camera_entity) + .unwrap() + .contains(root_node_pair)); + + ui_surface.remove_entities([root_node_entity]); + assert!(!ui_surface.entity_to_taffy.contains_key(&root_node_entity)); + + return; // TODO: can't pass the test if we continue - not implemented (remove allow(unreachable_code)) + + assert!(!ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&root_node_entity)); + assert!(!ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&root_node_entity)); + assert!(ui_surface + .camera_roots + .get(&camera_entity) + .unwrap() + .is_empty()); + } + + #[test] + fn test_try_update_measure() { + let mut ui_surface = UiSurface::default(); + let root_node_entity = Entity::from_raw(1); + let style = Style::default(); + + ui_surface.upsert_node(&TEST_LAYOUT_CONTEXT, root_node_entity, &style, None); + let mut content_size = ContentSize::default(); + content_size.set(NodeMeasure::Fixed(FixedMeasure { size: Vec2::ONE })); + let measure_func = content_size.measure.take().unwrap(); + assert!(ui_surface + .update_node_context(root_node_entity, measure_func) + .is_some()); + } + + #[test] + fn test_update_children() { + let mut ui_surface = UiSurface::default(); + let root_node_entity = Entity::from_raw(1); + let child_entity = Entity::from_raw(2); + let style = Style::default(); + + ui_surface.upsert_node(&TEST_LAYOUT_CONTEXT, root_node_entity, &style, None); + ui_surface.upsert_node(&TEST_LAYOUT_CONTEXT, child_entity, &style, None); + + ui_surface.update_children(root_node_entity, &[child_entity]); + + let parent_node = *ui_surface.entity_to_taffy.get(&root_node_entity).unwrap(); + let child_node = *ui_surface.entity_to_taffy.get(&child_entity).unwrap(); + assert_eq!(ui_surface.taffy.parent(child_node), Some(parent_node)); + } + + #[allow(unreachable_code)] + #[test] + fn test_set_camera_children() { + let mut ui_surface = UiSurface::default(); + let camera_entity = Entity::from_raw(0); + let root_node_entity = Entity::from_raw(1); + let child_entity = Entity::from_raw(2); + let style = Style::default(); + + ui_surface.upsert_node(&TEST_LAYOUT_CONTEXT, root_node_entity, &style, None); + ui_surface.upsert_node(&TEST_LAYOUT_CONTEXT, child_entity, &style, None); + + let root_taffy_node = *ui_surface.entity_to_taffy.get(&root_node_entity).unwrap(); + let child_taffy = *ui_surface.entity_to_taffy.get(&child_entity).unwrap(); + + // set up the relationship manually + ui_surface + .taffy + .add_child(root_taffy_node, child_taffy) + .unwrap(); + + ui_surface.set_camera_children(camera_entity, [root_node_entity].into_iter()); + + assert!( + ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&root_node_entity), + "root node not associated with camera" + ); + assert!( + !ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&child_entity), + "child of root node should not be associated with camera" + ); + + let _root_node_pair = + get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity) + .expect("expected root node pair"); + + assert_eq!(ui_surface.taffy.parent(child_taffy), Some(root_taffy_node)); + let root_taffy_children = ui_surface.taffy.children(root_taffy_node).unwrap(); + assert!( + root_taffy_children.contains(&child_taffy), + "root node is not a parent of child node" + ); + assert_eq!( + ui_surface.taffy.child_count(root_taffy_node), + 1, + "expected root node child count to be 1" + ); + + // clear camera's root nodes + ui_surface.set_camera_children(camera_entity, Vec::::new().into_iter()); + + return; // TODO: can't pass the test if we continue - not implemented (remove allow(unreachable_code)) + + assert!( + !ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&root_node_entity), + "root node should have been unassociated with camera" + ); + assert!( + !ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&child_entity), + "child of root node should not be associated with camera" + ); + + let root_taffy_children = ui_surface.taffy.children(root_taffy_node).unwrap(); + assert!( + root_taffy_children.contains(&child_taffy), + "root node is not a parent of child node" + ); + assert_eq!( + ui_surface.taffy.child_count(root_taffy_node), + 1, + "expected root node child count to be 1" + ); + + // re-associate root node with camera + ui_surface.set_camera_children(camera_entity, vec![root_node_entity].into_iter()); + + assert!( + ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&root_node_entity), + "root node should have been re-associated with camera" + ); + assert!( + !ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&child_entity), + "child of root node should not be associated with camera" + ); + + let child_taffy = ui_surface.entity_to_taffy.get(&child_entity).unwrap(); + let root_taffy_children = ui_surface.taffy.children(root_taffy_node).unwrap(); + assert!( + root_taffy_children.contains(child_taffy), + "root node is not a parent of child node" + ); + assert_eq!( + ui_surface.taffy.child_count(root_taffy_node), + 1, + "expected root node child count to be 1" + ); + } + + #[test] + fn test_compute_camera_layout() { + let mut ui_surface = UiSurface::default(); + let camera_entity = Entity::from_raw(0); + let root_node_entity = Entity::from_raw(1); + let style = Style::default(); + + ui_surface.upsert_node(&TEST_LAYOUT_CONTEXT, root_node_entity, &style, None); + + ui_surface.compute_camera_layout(camera_entity, UVec2::new(800, 600)); + + let taffy_node = ui_surface.entity_to_taffy.get(&root_node_entity).unwrap(); + assert!(ui_surface.taffy.layout(*taffy_node).is_ok()); + } +} From 90afc38a2bcf96ccd0dee69a2cb3cd297525077f Mon Sep 17 00:00:00 2001 From: bstriker Date: Mon, 13 May 2024 18:16:00 -0400 Subject: [PATCH 2/4] #12804 changed Update internal mappings (squashed) Move assertion above panic triggering line Remove demotion logic to simplify scope Remove potentially duplicated context update loop Remove duplicated ContentSize removal handler Restore assertion in ui tracking tests Remove promotion test Satisfy linter Update test Remove promotion helper Apply rustfmt Fix documentation Add missing back ticks in doc comment Underscore unused variables Rebase fixes Add regression test for recursive despawn on ui nodes Apply rustfmt in bevy_ui/layout/debug Remove message from unreachable! Add/update documentation for UiSurface Make missing root_node_data in compute_camera_layout an error Fix promote_ui_node, add a test, and set to only be in cfg(test) Update documentation Use replace_camera_association to reduce redundant code Use mark_root_node_as_orphaned Add explicit taffy node counts to tests Add test for promoting normal ui nodes into root nodes Add support demoting root nodes into normal ui nodes Expand tests to cover different methods of despawn Remove user_root_node field in favor of using entity_to_taffy Replace is_root_node_data_valid helper with has_valid_root_node_data Implement updated map structure in UiSurface Deprecate user_root_node in RootNodeData Extract default viewport style into inline fn Rename camera_id to camera_entity --- crates/bevy_ui/src/layout/debug.rs | 25 +- crates/bevy_ui/src/layout/mod.rs | 204 +++++++++- crates/bevy_ui/src/layout/ui_surface.rs | 475 +++++++++++++++--------- 3 files changed, 503 insertions(+), 201 deletions(-) diff --git a/crates/bevy_ui/src/layout/debug.rs b/crates/bevy_ui/src/layout/debug.rs index 47b02396a6816..32845b9839ab7 100644 --- a/crates/bevy_ui/src/layout/debug.rs +++ b/crates/bevy_ui/src/layout/debug.rs @@ -12,22 +12,31 @@ pub fn print_ui_layout_tree(ui_surface: &UiSurface) { let taffy_to_entity: HashMap = ui_surface .entity_to_taffy .iter() - .map(|(entity, node)| (*node, *entity)) - .collect(); - for (&entity, roots) in &ui_surface.camera_roots { - let mut out = String::new(); - for root in roots { + .map(|(&ui_entity, &taffy_node)| (taffy_node, ui_entity)) + .collect::>(); + for (&camera_entity, root_node_set) in ui_surface.camera_root_nodes.iter() { + bevy_utils::tracing::info!("Layout tree for camera entity: {camera_entity}"); + for &root_node_entity in root_node_set.iter() { + let Some(implicit_viewport_node) = ui_surface + .root_node_data + .get(&root_node_entity) + .map(|rnd| rnd.implicit_viewport_node) + else { + continue; + }; + let mut out = String::new(); print_node( ui_surface, &taffy_to_entity, - entity, - root.implicit_viewport_node, + camera_entity, + implicit_viewport_node, false, String::new(), &mut out, ); + + bevy_utils::tracing::info!("{out}"); } - bevy_utils::tracing::info!("Layout tree for camera entity: {entity:?}\n{out}"); } } diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index 43d767acdecbc..bbaf643f30d0b 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -220,7 +220,7 @@ pub fn ui_layout_system( for (camera_id, camera) in &camera_layout_info { let inverse_target_scale_factor = camera.scale_factor.recip(); - ui_surface.compute_camera_layout(*camera_id, camera.size); + ui_surface.compute_camera_layout(camera_id, camera.size); for root in &camera.root_nodes { update_uinode_geometry_recursive( *root, @@ -357,7 +357,10 @@ mod tests { use bevy_ecs::schedule::Schedule; use bevy_ecs::system::RunSystemOnce; use bevy_ecs::world::World; - use bevy_hierarchy::{despawn_with_children_recursive, BuildWorldChildren, Children, Parent}; + use bevy_hierarchy::{ + despawn_with_children_recursive, BuildChildren, BuildWorldChildren, Children, + DespawnRecursiveExt, Parent, + }; use bevy_math::{vec2, Rect, UVec2, Vec2}; use bevy_render::camera::ManualTextureViews; use bevy_render::camera::OrthographicProjection; @@ -468,33 +471,102 @@ mod tests { } } - #[test] - fn ui_surface_tracks_ui_entities() { - let (mut world, mut ui_schedule) = setup_ui_test_world(); - - ui_schedule.run(&mut world); + fn _track_ui_entity_setup(world: &mut World, ui_schedule: &mut Schedule) -> (Entity, Entity) { + ui_schedule.run(world); // no UI entities in world, none in UiSurface let ui_surface = world.resource::(); assert!(ui_surface.entity_to_taffy.is_empty()); + assert!(ui_surface.root_node_data.is_empty()); let ui_entity = world.spawn(NodeBundle::default()).id(); // `ui_layout_system` should map `ui_entity` to a ui node in `UiSurface::entity_to_taffy` - ui_schedule.run(&mut world); + ui_schedule.run(world); let ui_surface = world.resource::(); assert!(ui_surface.entity_to_taffy.contains_key(&ui_entity)); assert_eq!(ui_surface.entity_to_taffy.len(), 1); + assert!(ui_surface.root_node_data.contains_key(&ui_entity)); + assert_eq!(ui_surface.root_node_data.len(), 1); + + let child_entity = world.spawn(NodeBundle::default()).id(); + world.commands().entity(ui_entity).add_child(child_entity); + + // `ui_layout_system` should add `child_entity` as a child of `ui_entity` + ui_schedule.run(world); + + let ui_surface = world.resource::(); + assert!(ui_surface.entity_to_taffy.contains_key(&child_entity)); + assert_eq!(ui_surface.entity_to_taffy.len(), 2); + assert!( + !ui_surface.root_node_data.contains_key(&child_entity), + "child should not have been added as a root node" + ); + assert_eq!(ui_surface.root_node_data.len(), 1); + let ui_taffy = ui_surface.entity_to_taffy.get(&ui_entity).unwrap(); + let child_taffy = ui_surface.entity_to_taffy.get(&child_entity).unwrap(); + assert_eq!( + ui_surface.taffy.parent(*child_taffy), + Some(*ui_taffy), + "expected to be child of root node" + ); + + (ui_entity, child_entity) + } + + #[test] + fn ui_surface_tracks_ui_entities_despawn() { + let (mut world, mut ui_schedule) = setup_ui_test_world(); + let (ui_entity, _child_entity) = _track_ui_entity_setup(&mut world, &mut ui_schedule); world.despawn(ui_entity); // `ui_layout_system` should remove `ui_entity` from `UiSurface::entity_to_taffy` ui_schedule.run(&mut world); + let ui_surface = world.resource::(); + assert!(!ui_surface.entity_to_taffy.contains_key(&ui_entity)); + assert_eq!(ui_surface.entity_to_taffy.len(), 1); + assert!(!ui_surface.root_node_data.contains_key(&ui_entity)); + assert!(ui_surface.root_node_data.is_empty()); + assert_eq!(ui_surface.taffy.total_node_count(), 1); + } + + #[test] + fn ui_surface_tracks_ui_entities_despawn_recursive() { + let (mut world, mut ui_schedule) = setup_ui_test_world(); + let (ui_entity, _child_entity) = _track_ui_entity_setup(&mut world, &mut ui_schedule); + + despawn_with_children_recursive(&mut world, ui_entity); + + // `ui_layout_system` should remove `ui_entity` and `child_entity` from `UiSurface::entity_to_taffy` + ui_schedule.run(&mut world); + let ui_surface = world.resource::(); assert!(!ui_surface.entity_to_taffy.contains_key(&ui_entity)); assert!(ui_surface.entity_to_taffy.is_empty()); + assert!(!ui_surface.root_node_data.contains_key(&ui_entity)); + assert!(ui_surface.root_node_data.is_empty()); + assert_eq!(ui_surface.taffy.total_node_count(), 0); + } + + #[test] + fn ui_surface_tracks_ui_entities_despawn_descendants() { + let (mut world, mut ui_schedule) = setup_ui_test_world(); + let (ui_entity, _child_entity) = _track_ui_entity_setup(&mut world, &mut ui_schedule); + + world.commands().entity(ui_entity).despawn_descendants(); + + // `ui_layout_system` should remove `child_entity` from `UiSurface::entity_to_taffy` + ui_schedule.run(&mut world); + + let ui_surface = world.resource::(); + assert!(ui_surface.entity_to_taffy.contains_key(&ui_entity)); + assert_eq!(ui_surface.entity_to_taffy.len(), 1); + assert!(ui_surface.root_node_data.contains_key(&ui_entity)); + assert_eq!(ui_surface.root_node_data.len(), 1); + assert_eq!(ui_surface.taffy.total_node_count(), 2); } #[test] @@ -514,7 +586,7 @@ mod tests { // no UI entities in world, none in UiSurface let ui_surface = world.resource::(); - assert!(ui_surface.camera_entity_to_taffy.is_empty()); + assert!(ui_surface.camera_root_nodes.is_empty()); // respawn camera let camera_entity = world.spawn(Camera2dBundle::default()).id(); @@ -527,10 +599,8 @@ mod tests { ui_schedule.run(&mut world); let ui_surface = world.resource::(); - assert!(ui_surface - .camera_entity_to_taffy - .contains_key(&camera_entity)); - assert_eq!(ui_surface.camera_entity_to_taffy.len(), 1); + assert!(ui_surface.camera_root_nodes.contains_key(&camera_entity)); + assert_eq!(ui_surface.camera_root_nodes.len(), 1); world.despawn(ui_entity); world.despawn(camera_entity); @@ -539,10 +609,8 @@ mod tests { ui_schedule.run(&mut world); let ui_surface = world.resource::(); - assert!(!ui_surface - .camera_entity_to_taffy - .contains_key(&camera_entity)); - assert!(ui_surface.camera_entity_to_taffy.is_empty()); + assert!(!ui_surface.camera_root_nodes.contains_key(&camera_entity)); + assert!(ui_surface.camera_root_nodes.is_empty()); } #[test] @@ -567,6 +635,7 @@ mod tests { let ui_surface = world.resource::(); + assert_eq!(ui_surface.taffy.total_node_count(), 0); // `ui_node` is removed, attempting to retrieve a style for `ui_node` panics let _ = ui_surface.taffy.style(ui_node); } @@ -671,6 +740,7 @@ mod tests { // all nodes should have been deleted let ui_surface = world.resource::(); assert!(ui_surface.entity_to_taffy.is_empty()); + assert_eq!(ui_surface.taffy.total_node_count(), 0); } /// regression test for >=0.13.1 root node layouts @@ -1081,4 +1151,104 @@ mod tests { ui_schedule.run(&mut world); } + + struct DespawnTestEntityReference { + parent_entity: Entity, + child1_entity: Entity, + child2_entity: Entity, + } + fn recursive_despawn_setup() -> (World, Schedule, DespawnTestEntityReference) { + let (mut world, mut ui_schedule) = setup_ui_test_world(); + + let mut child1_entity = None; + let mut child2_entity = None; + let parent_entity = world + .spawn(NodeBundle::default()) + .with_children(|children| { + child1_entity = Some( + children + .spawn(NodeBundle::default()) + .with_children(|children| { + child2_entity = Some(children.spawn(NodeBundle::default()).id()); + }) + .id(), + ); + }) + .id(); + + ui_schedule.run(&mut world); + + let ui_surface = world.get_resource::().unwrap(); + // 1 for root node, 1 for implicit viewport node + // 2 children + assert_eq!(ui_surface.taffy.total_node_count(), 4); + + ( + world, + ui_schedule, + DespawnTestEntityReference { + parent_entity, + child1_entity: child1_entity.expect("expected child 1"), + child2_entity: child2_entity.expect("expected child 2"), + }, + ) + } + + #[test] + fn test_recursive_despawn_on_parent() { + let ( + mut world, + mut ui_schedule, + DespawnTestEntityReference { + parent_entity, + child1_entity, + child2_entity, + }, + ) = recursive_despawn_setup(); + + let ui_surface = world.get_resource::().unwrap(); + + let parent_taffy = *ui_surface.entity_to_taffy.get(&parent_entity).unwrap(); + let child1_taffy = *ui_surface.entity_to_taffy.get(&child1_entity).unwrap(); + let child2_taffy = *ui_surface.entity_to_taffy.get(&child2_entity).unwrap(); + assert_eq!(ui_surface.taffy.parent(child2_taffy), Some(child1_taffy)); + assert_eq!(ui_surface.taffy.parent(child1_taffy), Some(parent_taffy)); + + world.commands().entity(parent_entity).despawn_recursive(); + + ui_schedule.run(&mut world); + + let ui_surface = world.get_resource::().unwrap(); + // all nodes should be removed + assert_eq!(ui_surface.taffy.total_node_count(), 0); + } + + #[test] + fn test_recursive_despawn_on_child() { + let ( + mut world, + mut ui_schedule, + DespawnTestEntityReference { + parent_entity, + child1_entity, + child2_entity, + }, + ) = recursive_despawn_setup(); + + let ui_surface = world.get_resource::().unwrap(); + + let parent_taffy = *ui_surface.entity_to_taffy.get(&parent_entity).unwrap(); + let child1_taffy = *ui_surface.entity_to_taffy.get(&child1_entity).unwrap(); + let child2_taffy = *ui_surface.entity_to_taffy.get(&child2_entity).unwrap(); + assert_eq!(ui_surface.taffy.parent(child2_taffy), Some(child1_taffy)); + assert_eq!(ui_surface.taffy.parent(child1_taffy), Some(parent_taffy)); + + world.commands().entity(child1_entity).despawn_recursive(); + + ui_schedule.run(&mut world); + + let ui_surface = world.get_resource::().unwrap(); + // only root node and implicit viewport left + assert_eq!(ui_surface.taffy.total_node_count(), 2); + } } diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 9bba310adb4ba..0733e7234b0b7 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -2,7 +2,7 @@ use std::fmt; use taffy::TaffyTree; -use bevy_ecs::entity::{Entity, EntityHashMap}; +use bevy_ecs::entity::{Entity, EntityHashMap, EntityHashSet}; use bevy_ecs::prelude::Resource; use bevy_math::UVec2; use bevy_utils::default; @@ -11,27 +11,69 @@ use bevy_utils::tracing::warn; use crate::layout::convert; use crate::{LayoutContext, LayoutError, Measure, NodeMeasure, Style}; +#[inline(always)] +/// Style used for `implicit_viewport_node` +fn default_viewport_style() -> taffy::style::Style { + taffy::style::Style { + display: taffy::style::Display::Grid, + // Note: Taffy percentages are floats ranging from 0.0 to 1.0. + // So this is setting width:100% and height:100% + size: taffy::geometry::Size { + width: taffy::style::Dimension::Percent(1.0), + height: taffy::style::Dimension::Percent(1.0), + }, + align_items: Some(taffy::style::AlignItems::Start), + justify_items: Some(taffy::style::JustifyItems::Start), + ..default() + } +} + #[derive(Debug, Clone, PartialEq, Eq)] -pub struct RootNodePair { - // The implicit "viewport" node created by Bevy +/// Stores reference data to quickly identify: +/// - Its associated camera +/// - Its parent `implicit_viewport_node` taffy node +pub struct RootNodeData { + /// Associated camera `Entity` + /// + /// inferred by components: `TargetCamera`, `IsDefaultUiCamera` + /// + /// "Orphans" are root nodes not assigned to a camera. + /// Root nodes might temporarily enter an orphan state as they transition between cameras + /// The reason for this is to prevent us from prematurely recreating taffy nodes + /// and allowing for the entities to be cleaned up when they are requested to be removed by the ECS + pub(super) camera_entity: Option, + /// The implicit "viewport" node created by Bevy + /// + /// This forces the root nodes to behave independently to other root nodes. + /// Just as if they were set to `PositionType::Absolute` + /// + /// This must be manually removed on `Entity` despawn + /// or else it will survive in the taffy tree with no references pub(super) implicit_viewport_node: taffy::NodeId, - // The root (parentless) node specified by the user - pub(super) user_root_node: taffy::NodeId, } #[derive(Resource)] +/// Manages state and hierarchy for ui entities pub struct UiSurface { + /// Maps `Entity` to its corresponding taffy node + /// + /// Maintains an entry for each root ui node (parentless), and any of its children + /// + /// (does not include the `implicit_viewport_node`) pub(super) entity_to_taffy: EntityHashMap, - pub(super) camera_entity_to_taffy: EntityHashMap>, - pub(super) camera_roots: EntityHashMap>, + /// Maps root ui node (parentless) `Entity` to its corresponding `RootNodeData` + pub(super) root_node_data: EntityHashMap, + /// Maps camera `Entity` to an associated `EntityHashSet` of root nodes (parentless) + pub(super) camera_root_nodes: EntityHashMap, + /// Manages the UI Node Tree pub(super) taffy: TaffyTree, } fn _assert_send_sync_ui_surface_impl_safe() { fn _assert_send_sync() {} _assert_send_sync::>(); - _assert_send_sync::>>(); - _assert_send_sync::>>(); + _assert_send_sync::>(); + _assert_send_sync::>(); _assert_send_sync::>(); _assert_send_sync::(); } @@ -40,8 +82,8 @@ impl fmt::Debug for UiSurface { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.debug_struct("UiSurface") .field("entity_to_taffy", &self.entity_to_taffy) - .field("camera_entity_to_taffy", &self.camera_entity_to_taffy) - .field("camera_roots", &self.camera_roots) + .field("root_node_data", &self.root_node_data) + .field("camera_root_nodes", &self.camera_root_nodes) .finish() } } @@ -52,8 +94,8 @@ impl Default for UiSurface { taffy.disable_rounding(); Self { entity_to_taffy: Default::default(), - camera_entity_to_taffy: Default::default(), - camera_roots: Default::default(), + root_node_data: Default::default(), + camera_root_nodes: Default::default(), taffy, } } @@ -147,70 +189,152 @@ without UI components as a child of an entity with UI components, results may be } } + /// Removes camera association to root node + /// Shorthand for calling `replace_camera_association(root_node_entity, None)` + fn mark_root_node_as_orphaned(&mut self, root_node_entity: &Entity) { + self.replace_camera_association(*root_node_entity, None); + } + + /// Reassigns or removes a root node's associated camera entity + /// `Some(camera_entity)` - Updates camera association to root node + /// `None` - Removes camera association to root node + /// Does not check to see if they are the same before performing operations + fn replace_camera_association( + &mut self, + root_node_entity: Entity, + new_camera_entity_option: Option, + ) { + if let Some(root_node_data) = self.root_node_data.get_mut(&root_node_entity) { + // Clear existing camera association, if any + if let Some(old_camera_entity) = root_node_data.camera_entity.take() { + let prev_camera_root_nodes = self.camera_root_nodes.get_mut(&old_camera_entity); + if let Some(prev_camera_root_nodes) = prev_camera_root_nodes { + prev_camera_root_nodes.remove(&root_node_entity); + } + } + + // Establish new camera association, if provided + if let Some(camera_entity) = new_camera_entity_option { + root_node_data.camera_entity.replace(camera_entity); + self.camera_root_nodes + .entry(camera_entity) + .or_default() + .insert(root_node_entity); + } + } + } + + /// Creates or updates a root node + fn create_or_update_root_node_data( + &mut self, + root_node_entity: &Entity, + camera_entity: &Entity, + ) -> &mut RootNodeData { + let user_root_node = *self.entity_to_taffy.get(root_node_entity).expect("create_or_update_root_node_data called before ui_root_node_entity was added to taffy tree or was previously removed"); + let ui_root_node_entity = *root_node_entity; + let camera_entity = *camera_entity; + + let mut added = false; + + // creates mutable borrow on self that lives as long as the result + let _ = self + .root_node_data + .entry(ui_root_node_entity) + .or_insert_with(|| { + added = true; + + self.camera_root_nodes + .entry(camera_entity) + .or_default() + .insert(ui_root_node_entity); + + let implicit_viewport_node = self.taffy.new_leaf(default_viewport_style()).unwrap(); + + self.taffy + .add_child(implicit_viewport_node, user_root_node) + .unwrap(); + + RootNodeData { + camera_entity: Some(camera_entity), + implicit_viewport_node, + } + }); + + if !added { + self.replace_camera_association(ui_root_node_entity, Some(camera_entity)); + } + + self.root_node_data + .get_mut(root_node_entity) + .unwrap_or_else(|| unreachable!()) + } + /// Set the ui node entities without a [`bevy_hierarchy::Parent`] as children to the root node in the taffy layout. pub fn set_camera_children( &mut self, - camera_id: Entity, + camera_entity: Entity, children: impl Iterator, ) { - let viewport_style = taffy::style::Style { - display: taffy::style::Display::Grid, - // Note: Taffy percentages are floats ranging from 0.0 to 1.0. - // So this is setting width:100% and height:100% - size: taffy::geometry::Size { - width: taffy::style::Dimension::Percent(1.0), - height: taffy::style::Dimension::Percent(1.0), - }, - align_items: Some(taffy::style::AlignItems::Start), - justify_items: Some(taffy::style::JustifyItems::Start), - ..default() - }; + let removed_children = self.camera_root_nodes.entry(camera_entity).or_default(); + let mut removed_children = removed_children.clone(); + + for ui_entity in children { + // creates mutable borrow on self that lives as long as the result + let _ = self.create_or_update_root_node_data(&ui_entity, &camera_entity); + + // drop the mutable borrow on self by re-fetching + let root_node_data = self + .root_node_data + .get(&ui_entity) + .unwrap_or_else(|| unreachable!()); + + // fix taffy relationships + { + let taffy_node = *self.entity_to_taffy.get(&ui_entity).unwrap(); + if let Some(parent) = self.taffy.parent(taffy_node) { + self.taffy.remove_child(parent, taffy_node).unwrap(); + } - let camera_root_node_map = self.camera_entity_to_taffy.entry(camera_id).or_default(); - let existing_roots = self.camera_roots.entry(camera_id).or_default(); - let mut new_roots = Vec::new(); - for entity in children { - let node = *self.entity_to_taffy.get(&entity).unwrap(); - let root_node = existing_roots - .iter() - .find(|n| n.user_root_node == node) - .cloned() - .unwrap_or_else(|| { - if let Some(previous_parent) = self.taffy.parent(node) { - // remove the root node from the previous implicit node's children - self.taffy.remove_child(previous_parent, node).unwrap(); - } - - let viewport_node = *camera_root_node_map - .entry(entity) - .or_insert_with(|| self.taffy.new_leaf(viewport_style.clone()).unwrap()); - self.taffy.add_child(viewport_node, node).unwrap(); - - RootNodePair { - implicit_viewport_node: viewport_node, - user_root_node: node, - } - }); - new_roots.push(root_node); + self.taffy + .add_child(root_node_data.implicit_viewport_node, taffy_node) + .unwrap(); + } + + removed_children.remove(&ui_entity); } - self.camera_roots.insert(camera_id, new_roots); + for orphan in removed_children.iter() { + self.mark_root_node_as_orphaned(orphan); + } } /// Compute the layout for each window entity's corresponding root node in the layout. - pub fn compute_camera_layout(&mut self, camera: Entity, render_target_resolution: UVec2) { - let Some(camera_root_nodes) = self.camera_roots.get(&camera) else { + pub fn compute_camera_layout( + &mut self, + camera_entity: &Entity, + render_target_resolution: UVec2, + ) { + let Some(root_nodes) = self.camera_root_nodes.get(camera_entity) else { return; }; + for &root_node_entity in root_nodes.iter() { + let available_space = taffy::geometry::Size { + width: taffy::style::AvailableSpace::Definite(render_target_resolution.x as f32), + height: taffy::style::AvailableSpace::Definite(render_target_resolution.y as f32), + }; + + let root_node_data = self + .root_node_data + .get(&root_node_entity) + .expect("root_node_data missing"); + + if root_node_data.camera_entity.is_none() { + panic!("internal map out of sync"); + } - let available_space = taffy::geometry::Size { - width: taffy::style::AvailableSpace::Definite(render_target_resolution.x as f32), - height: taffy::style::AvailableSpace::Definite(render_target_resolution.y as f32), - }; - for root_nodes in camera_root_nodes { self.taffy .compute_layout_with_measure( - root_nodes.implicit_viewport_node, + root_node_data.implicit_viewport_node, available_space, |known_dimensions: taffy::Size>, available_space: taffy::Size, @@ -237,23 +361,51 @@ without UI components as a child of an entity with UI components, results may be } } - /// Removes each camera entity from the internal map and then removes their associated node from taffy + /// Disassociates the camera from all of its assigned root nodes and removes their viewport nodes + /// Removes entry in `camera_root_nodes` + pub(super) fn remove_camera(&mut self, camera_entity: &Entity) { + if let Some(root_node_entities) = self.camera_root_nodes.remove(camera_entity) { + for root_node_entity in root_node_entities { + self.remove_root_node_viewport(&root_node_entity); + } + }; + } + + /// Disassociates the root node from the assigned camera (if any) and removes the viewport node from taffy + /// Removes entry in `root_node_data` + pub(super) fn remove_root_node_viewport(&mut self, root_node_entity: &Entity) { + self.mark_root_node_as_orphaned(root_node_entity); + if let Some(removed) = self.root_node_data.remove(root_node_entity) { + self.taffy.remove(removed.implicit_viewport_node).unwrap(); + } + } + + /// Removes the ui node from the taffy tree, and if it's a root node it also calls `remove_root_node_viewport` + pub(super) fn remove_ui_node(&mut self, ui_node_entity: &Entity) { + if let Some(taffy_node) = self.entity_to_taffy.remove(ui_node_entity) { + self.taffy.remove(taffy_node).unwrap(); + } + // remove root node entry if this is a root node + if self.root_node_data.contains_key(ui_node_entity) { + self.remove_root_node_viewport(ui_node_entity); + } + } + + /// Removes specified camera entities by disassociating them from their associated `implicit_viewport_node` + /// in the internal map, and subsequently removes the `implicit_viewport_node` + /// from the `taffy` layout engine for each. pub fn remove_camera_entities(&mut self, entities: impl IntoIterator) { for entity in entities { - if let Some(camera_root_node_map) = self.camera_entity_to_taffy.remove(&entity) { - for (_, node) in camera_root_node_map.iter() { - self.taffy.remove(*node).unwrap(); - } - } + self.remove_camera(&entity); } } - /// Removes each entity from the internal map and then removes their associated node from taffy + /// Removes the specified entities from the internal map while + /// removing their `implicit_viewport_node` from taffy, + /// and then subsequently removes their entry from `entity_to_taffy` and associated node from taffy pub fn remove_entities(&mut self, entities: impl IntoIterator) { for entity in entities { - if let Some(node) = self.entity_to_taffy.remove(&entity) { - self.taffy.remove(node).unwrap(); - } + self.remove_ui_node(&entity); } } @@ -288,35 +440,38 @@ mod tests { max_size: 1.0, }; - /// Checks if the parent of the `user_root_node` in a `RootNodePair` + /// Checks if the parent of the `user_root_node` in a `RootNodeData` /// is correctly assigned as the `implicit_viewport_node`. - fn is_root_node_pair_valid( - taffy_tree: &TaffyTree, - root_node_pair: &RootNodePair, - ) -> bool { - taffy_tree.parent(root_node_pair.user_root_node) - == Some(root_node_pair.implicit_viewport_node) + fn has_valid_root_node_data(ui_surface: &UiSurface, root_node_entity: &Entity) -> bool { + let Some(&root_node_taffy_node_id) = ui_surface.entity_to_taffy.get(root_node_entity) + else { + return false; + }; + let Some(root_node_data) = ui_surface.root_node_data.get(root_node_entity) else { + return false; + }; + ui_surface.taffy.parent(root_node_taffy_node_id) + == Some(root_node_data.implicit_viewport_node) } - /// Tries to get the root node pair for a given root node entity with the specified camera entity - fn get_root_node_pair_exact( + /// Tries to get the root node data for a given root node entity + /// and asserts it matches the provided camera entity + fn get_root_node_data_exact( ui_surface: &UiSurface, root_node_entity: Entity, camera_entity: Entity, - ) -> Option<&RootNodePair> { - let root_node_pairs = ui_surface.camera_roots.get(&camera_entity)?; - let root_node_taffy = ui_surface.entity_to_taffy.get(&root_node_entity)?; - root_node_pairs - .iter() - .find(|&root_node_pair| root_node_pair.user_root_node == *root_node_taffy) + ) -> Option<&RootNodeData> { + let root_node_data = ui_surface.root_node_data.get(&root_node_entity)?; + assert_eq!(root_node_data.camera_entity, Some(camera_entity)); + Some(root_node_data) } #[test] fn test_initialization() { let ui_surface = UiSurface::default(); assert!(ui_surface.entity_to_taffy.is_empty()); - assert!(ui_surface.camera_entity_to_taffy.is_empty()); - assert!(ui_surface.camera_roots.is_empty()); + assert!(ui_surface.root_node_data.is_empty()); + assert!(ui_surface.camera_root_nodes.is_empty()); assert_eq!(ui_surface.taffy.total_node_count(), 0); } @@ -346,10 +501,11 @@ mod tests { // each root node will create 2 taffy nodes assert_eq!(ui_surface.taffy.total_node_count(), 2); - // root node pair should now exist - let root_node_pair = get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity) - .expect("expected root node pair"); - assert!(is_root_node_pair_valid(&ui_surface.taffy, root_node_pair)); + // root node data should now exist + let _root_node_data = + get_root_node_data_exact(&ui_surface, root_node_entity, camera_entity) + .expect("expected root node data"); + assert!(has_valid_root_node_data(&ui_surface, &root_node_entity)); // test duplicate insert 2 ui_surface.upsert_node(&TEST_LAYOUT_CONTEXT, root_node_entity, &style, None); @@ -357,34 +513,29 @@ mod tests { // node count should not have increased assert_eq!(ui_surface.taffy.total_node_count(), 2); - // root node pair should be unaffected - let root_node_pair = get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity) - .expect("expected root node pair"); - assert!(is_root_node_pair_valid(&ui_surface.taffy, root_node_pair)); + // root node data should be unaffected + let _root_node_data = + get_root_node_data_exact(&ui_surface, root_node_entity, camera_entity) + .expect("expected root node data"); + assert!(has_valid_root_node_data(&ui_surface, &root_node_entity)); } #[test] - fn test_get_root_node_pair_exact() { + fn test_get_root_node_data_exact() { /// Attempts to find the camera entity that holds a reference to the given root node entity fn get_associated_camera_entity( ui_surface: &UiSurface, root_node_entity: Entity, ) -> Option { - for (&camera_entity, root_node_map) in ui_surface.camera_entity_to_taffy.iter() { - if root_node_map.contains_key(&root_node_entity) { - return Some(camera_entity); - } - } - None + get_root_node_data(ui_surface, root_node_entity)?.camera_entity } - /// Attempts to find the root node pair corresponding to the given root node entity - fn get_root_node_pair( + /// Attempts to find the root node data corresponding to the given root node entity + fn get_root_node_data( ui_surface: &UiSurface, root_node_entity: Entity, - ) -> Option<&RootNodePair> { - let camera_entity = get_associated_camera_entity(ui_surface, root_node_entity)?; - get_root_node_pair_exact(ui_surface, root_node_entity, camera_entity) + ) -> Option<&RootNodeData> { + ui_surface.root_node_data.get(&root_node_entity) } let mut ui_surface = UiSurface::default(); @@ -406,20 +557,19 @@ mod tests { None ); - let root_node_pair = get_root_node_pair(&ui_surface, root_node_entity); - assert!(root_node_pair.is_some()); + let root_node_data = + get_root_node_data(&ui_surface, root_node_entity).expect("expected root node data"); assert_eq!( - Some(root_node_pair.unwrap().user_root_node).as_ref(), - ui_surface.entity_to_taffy.get(&root_node_entity) + Some(root_node_data), + ui_surface.root_node_data.get(&root_node_entity), ); assert_eq!( - get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity), - root_node_pair + get_root_node_data_exact(&ui_surface, root_node_entity, camera_entity), + Some(root_node_data), ); } - #[allow(unreachable_code)] #[test] fn test_remove_camera_entities() { let mut ui_surface = UiSurface::default(); @@ -432,22 +582,17 @@ mod tests { // assign root node to camera ui_surface.set_camera_children(camera_entity, [root_node_entity].into_iter()); + assert!(ui_surface.camera_root_nodes.contains_key(&camera_entity)); + assert!(ui_surface.root_node_data.contains_key(&root_node_entity)); + assert!(ui_surface.camera_root_nodes.contains_key(&camera_entity)); + let _root_node_data = + get_root_node_data_exact(&ui_surface, root_node_entity, camera_entity) + .expect("expected root node data"); assert!(ui_surface - .camera_entity_to_taffy - .contains_key(&camera_entity)); - assert!(ui_surface - .camera_entity_to_taffy + .camera_root_nodes .get(&camera_entity) .unwrap() - .contains_key(&root_node_entity)); - assert!(ui_surface.camera_roots.contains_key(&camera_entity)); - let root_node_pair = get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity) - .expect("expected root node pair"); - assert!(ui_surface - .camera_roots - .get(&camera_entity) - .unwrap() - .contains(root_node_pair)); + .contains(&root_node_entity)); ui_surface.remove_camera_entities([camera_entity]); @@ -455,20 +600,15 @@ mod tests { assert!(ui_surface.entity_to_taffy.contains_key(&root_node_entity)); // `camera_roots` and `camera_entity_to_taffy` should no longer contain entries for `camera_entity` - assert!(!ui_surface - .camera_entity_to_taffy - .contains_key(&camera_entity)); + assert!(!ui_surface.camera_root_nodes.contains_key(&camera_entity)); - return; // TODO: can't pass the test if we continue - not implemented (remove allow(unreachable_code)) + assert!(!ui_surface.camera_root_nodes.contains_key(&camera_entity)); - assert!(!ui_surface.camera_roots.contains_key(&camera_entity)); - - // root node pair should be removed - let root_node_pair = get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity); - assert_eq!(root_node_pair, None); + // root node data should be removed + let root_node_data = get_root_node_data_exact(&ui_surface, root_node_entity, camera_entity); + assert_eq!(root_node_data, None); } - #[allow(unreachable_code)] #[test] fn test_remove_entities() { let mut ui_surface = UiSurface::default(); @@ -482,35 +622,21 @@ mod tests { assert!(ui_surface.entity_to_taffy.contains_key(&root_node_entity)); assert!(ui_surface - .camera_entity_to_taffy + .camera_root_nodes .get(&camera_entity) .unwrap() - .contains_key(&root_node_entity)); - let root_node_pair = - get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity).unwrap(); - assert!(ui_surface - .camera_roots - .get(&camera_entity) - .unwrap() - .contains(root_node_pair)); + .contains(&root_node_entity)); ui_surface.remove_entities([root_node_entity]); assert!(!ui_surface.entity_to_taffy.contains_key(&root_node_entity)); - return; // TODO: can't pass the test if we continue - not implemented (remove allow(unreachable_code)) - assert!(!ui_surface - .camera_entity_to_taffy + .camera_root_nodes .get(&camera_entity) .unwrap() - .contains_key(&root_node_entity)); - assert!(!ui_surface - .camera_entity_to_taffy - .get(&camera_entity) - .unwrap() - .contains_key(&root_node_entity)); + .contains(&root_node_entity)); assert!(ui_surface - .camera_roots + .camera_root_nodes .get(&camera_entity) .unwrap() .is_empty()); @@ -548,7 +674,6 @@ mod tests { assert_eq!(ui_surface.taffy.parent(child_node), Some(parent_node)); } - #[allow(unreachable_code)] #[test] fn test_set_camera_children() { let mut ui_surface = UiSurface::default(); @@ -573,24 +698,24 @@ mod tests { assert!( ui_surface - .camera_entity_to_taffy + .camera_root_nodes .get(&camera_entity) .unwrap() - .contains_key(&root_node_entity), + .contains(&root_node_entity), "root node not associated with camera" ); assert!( !ui_surface - .camera_entity_to_taffy + .camera_root_nodes .get(&camera_entity) .unwrap() - .contains_key(&child_entity), + .contains(&child_entity), "child of root node should not be associated with camera" ); - let _root_node_pair = - get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity) - .expect("expected root node pair"); + let _root_node_data = + get_root_node_data_exact(&ui_surface, root_node_entity, camera_entity) + .expect("expected root node data"); assert_eq!(ui_surface.taffy.parent(child_taffy), Some(root_taffy_node)); let root_taffy_children = ui_surface.taffy.children(root_taffy_node).unwrap(); @@ -607,22 +732,20 @@ mod tests { // clear camera's root nodes ui_surface.set_camera_children(camera_entity, Vec::::new().into_iter()); - return; // TODO: can't pass the test if we continue - not implemented (remove allow(unreachable_code)) - assert!( !ui_surface - .camera_entity_to_taffy + .camera_root_nodes .get(&camera_entity) .unwrap() - .contains_key(&root_node_entity), + .contains(&root_node_entity), "root node should have been unassociated with camera" ); assert!( !ui_surface - .camera_entity_to_taffy + .camera_root_nodes .get(&camera_entity) .unwrap() - .contains_key(&child_entity), + .contains(&child_entity), "child of root node should not be associated with camera" ); @@ -642,18 +765,18 @@ mod tests { assert!( ui_surface - .camera_entity_to_taffy + .camera_root_nodes .get(&camera_entity) .unwrap() - .contains_key(&root_node_entity), + .contains(&root_node_entity), "root node should have been re-associated with camera" ); assert!( !ui_surface - .camera_entity_to_taffy + .camera_root_nodes .get(&camera_entity) .unwrap() - .contains_key(&child_entity), + .contains(&child_entity), "child of root node should not be associated with camera" ); @@ -679,7 +802,7 @@ mod tests { ui_surface.upsert_node(&TEST_LAYOUT_CONTEXT, root_node_entity, &style, None); - ui_surface.compute_camera_layout(camera_entity, UVec2::new(800, 600)); + ui_surface.compute_camera_layout(&camera_entity, UVec2::new(800, 600)); let taffy_node = ui_surface.entity_to_taffy.get(&root_node_entity).unwrap(); assert!(ui_surface.taffy.layout(*taffy_node).is_ok()); From 73459f57911175eb071aff4013e11d6179a0b6df Mon Sep 17 00:00:00 2001 From: bstriker Date: Mon, 13 May 2024 17:35:46 -0400 Subject: [PATCH 3/4] Make UiSurface fields private --- crates/bevy_ui/src/layout/debug.rs | 8 +- crates/bevy_ui/src/layout/mod.rs | 136 ++++++++++++------------ crates/bevy_ui/src/layout/ui_surface.rs | 25 ++++- 3 files changed, 93 insertions(+), 76 deletions(-) diff --git a/crates/bevy_ui/src/layout/debug.rs b/crates/bevy_ui/src/layout/debug.rs index 32845b9839ab7..7dd8ab8072e92 100644 --- a/crates/bevy_ui/src/layout/debug.rs +++ b/crates/bevy_ui/src/layout/debug.rs @@ -10,15 +10,15 @@ use crate::layout::ui_surface::UiSurface; /// Prints a debug representation of the computed layout of the UI layout tree for each window. pub fn print_ui_layout_tree(ui_surface: &UiSurface) { let taffy_to_entity: HashMap = ui_surface - .entity_to_taffy + .entity_to_taffy() .iter() .map(|(&ui_entity, &taffy_node)| (taffy_node, ui_entity)) .collect::>(); - for (&camera_entity, root_node_set) in ui_surface.camera_root_nodes.iter() { + for (&camera_entity, root_node_set) in ui_surface.camera_root_nodes().iter() { bevy_utils::tracing::info!("Layout tree for camera entity: {camera_entity}"); for &root_node_entity in root_node_set.iter() { let Some(implicit_viewport_node) = ui_surface - .root_node_data + .root_node_data() .get(&root_node_entity) .map(|rnd| rnd.implicit_viewport_node) else { @@ -50,7 +50,7 @@ fn print_node( lines_string: String, acc: &mut String, ) { - let tree = &ui_surface.taffy; + let tree = &ui_surface.taffy(); let layout = tree.layout(node).unwrap(); let style = tree.style(node).unwrap(); diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index bbaf643f30d0b..9386cb6f197e9 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -476,8 +476,8 @@ mod tests { // no UI entities in world, none in UiSurface let ui_surface = world.resource::(); - assert!(ui_surface.entity_to_taffy.is_empty()); - assert!(ui_surface.root_node_data.is_empty()); + assert!(ui_surface.entity_to_taffy().is_empty()); + assert!(ui_surface.root_node_data().is_empty()); let ui_entity = world.spawn(NodeBundle::default()).id(); @@ -485,10 +485,10 @@ mod tests { ui_schedule.run(world); let ui_surface = world.resource::(); - assert!(ui_surface.entity_to_taffy.contains_key(&ui_entity)); - assert_eq!(ui_surface.entity_to_taffy.len(), 1); - assert!(ui_surface.root_node_data.contains_key(&ui_entity)); - assert_eq!(ui_surface.root_node_data.len(), 1); + assert!(ui_surface.entity_to_taffy().contains_key(&ui_entity)); + assert_eq!(ui_surface.entity_to_taffy().len(), 1); + assert!(ui_surface.root_node_data().contains_key(&ui_entity)); + assert_eq!(ui_surface.root_node_data().len(), 1); let child_entity = world.spawn(NodeBundle::default()).id(); world.commands().entity(ui_entity).add_child(child_entity); @@ -497,17 +497,17 @@ mod tests { ui_schedule.run(world); let ui_surface = world.resource::(); - assert!(ui_surface.entity_to_taffy.contains_key(&child_entity)); - assert_eq!(ui_surface.entity_to_taffy.len(), 2); + assert!(ui_surface.entity_to_taffy().contains_key(&child_entity)); + assert_eq!(ui_surface.entity_to_taffy().len(), 2); assert!( - !ui_surface.root_node_data.contains_key(&child_entity), + !ui_surface.root_node_data().contains_key(&child_entity), "child should not have been added as a root node" ); - assert_eq!(ui_surface.root_node_data.len(), 1); - let ui_taffy = ui_surface.entity_to_taffy.get(&ui_entity).unwrap(); - let child_taffy = ui_surface.entity_to_taffy.get(&child_entity).unwrap(); + assert_eq!(ui_surface.root_node_data().len(), 1); + let ui_taffy = ui_surface.entity_to_taffy().get(&ui_entity).unwrap(); + let child_taffy = ui_surface.entity_to_taffy().get(&child_entity).unwrap(); assert_eq!( - ui_surface.taffy.parent(*child_taffy), + ui_surface.taffy().parent(*child_taffy), Some(*ui_taffy), "expected to be child of root node" ); @@ -526,11 +526,11 @@ mod tests { ui_schedule.run(&mut world); let ui_surface = world.resource::(); - assert!(!ui_surface.entity_to_taffy.contains_key(&ui_entity)); - assert_eq!(ui_surface.entity_to_taffy.len(), 1); - assert!(!ui_surface.root_node_data.contains_key(&ui_entity)); - assert!(ui_surface.root_node_data.is_empty()); - assert_eq!(ui_surface.taffy.total_node_count(), 1); + assert!(!ui_surface.entity_to_taffy().contains_key(&ui_entity)); + assert_eq!(ui_surface.entity_to_taffy().len(), 1); + assert!(!ui_surface.root_node_data().contains_key(&ui_entity)); + assert!(ui_surface.root_node_data().is_empty()); + assert_eq!(ui_surface.taffy().total_node_count(), 1); } #[test] @@ -544,11 +544,11 @@ mod tests { ui_schedule.run(&mut world); let ui_surface = world.resource::(); - assert!(!ui_surface.entity_to_taffy.contains_key(&ui_entity)); - assert!(ui_surface.entity_to_taffy.is_empty()); - assert!(!ui_surface.root_node_data.contains_key(&ui_entity)); - assert!(ui_surface.root_node_data.is_empty()); - assert_eq!(ui_surface.taffy.total_node_count(), 0); + assert!(!ui_surface.entity_to_taffy().contains_key(&ui_entity)); + assert!(ui_surface.entity_to_taffy().is_empty()); + assert!(!ui_surface.root_node_data().contains_key(&ui_entity)); + assert!(ui_surface.root_node_data().is_empty()); + assert_eq!(ui_surface.taffy().total_node_count(), 0); } #[test] @@ -562,11 +562,11 @@ mod tests { ui_schedule.run(&mut world); let ui_surface = world.resource::(); - assert!(ui_surface.entity_to_taffy.contains_key(&ui_entity)); - assert_eq!(ui_surface.entity_to_taffy.len(), 1); - assert!(ui_surface.root_node_data.contains_key(&ui_entity)); - assert_eq!(ui_surface.root_node_data.len(), 1); - assert_eq!(ui_surface.taffy.total_node_count(), 2); + assert!(ui_surface.entity_to_taffy().contains_key(&ui_entity)); + assert_eq!(ui_surface.entity_to_taffy().len(), 1); + assert!(ui_surface.root_node_data().contains_key(&ui_entity)); + assert_eq!(ui_surface.root_node_data().len(), 1); + assert_eq!(ui_surface.taffy().total_node_count(), 2); } #[test] @@ -586,7 +586,7 @@ mod tests { // no UI entities in world, none in UiSurface let ui_surface = world.resource::(); - assert!(ui_surface.camera_root_nodes.is_empty()); + assert!(ui_surface.camera_root_nodes().is_empty()); // respawn camera let camera_entity = world.spawn(Camera2dBundle::default()).id(); @@ -599,8 +599,8 @@ mod tests { ui_schedule.run(&mut world); let ui_surface = world.resource::(); - assert!(ui_surface.camera_root_nodes.contains_key(&camera_entity)); - assert_eq!(ui_surface.camera_root_nodes.len(), 1); + assert!(ui_surface.camera_root_nodes().contains_key(&camera_entity)); + assert_eq!(ui_surface.camera_root_nodes().len(), 1); world.despawn(ui_entity); world.despawn(camera_entity); @@ -609,8 +609,8 @@ mod tests { ui_schedule.run(&mut world); let ui_surface = world.resource::(); - assert!(!ui_surface.camera_root_nodes.contains_key(&camera_entity)); - assert!(ui_surface.camera_root_nodes.is_empty()); + assert!(!ui_surface.camera_root_nodes().contains_key(&camera_entity)); + assert!(ui_surface.camera_root_nodes().is_empty()); } #[test] @@ -625,7 +625,7 @@ mod tests { // retrieve the ui node corresponding to `ui_entity` from ui surface let ui_surface = world.resource::(); - let ui_node = ui_surface.entity_to_taffy[&ui_entity]; + let ui_node = ui_surface.entity_to_taffy()[&ui_entity]; world.despawn(ui_entity); @@ -635,9 +635,9 @@ mod tests { let ui_surface = world.resource::(); - assert_eq!(ui_surface.taffy.total_node_count(), 0); + assert_eq!(ui_surface.taffy().total_node_count(), 0); // `ui_node` is removed, attempting to retrieve a style for `ui_node` panics - let _ = ui_surface.taffy.style(ui_node); + let _ = ui_surface.taffy().style(ui_node); } #[test] @@ -650,10 +650,10 @@ mod tests { ui_schedule.run(&mut world); let ui_surface = world.resource::(); - let ui_parent_node = ui_surface.entity_to_taffy[&ui_parent_entity]; + let ui_parent_node = ui_surface.entity_to_taffy()[&ui_parent_entity]; // `ui_parent_node` shouldn't have any children yet - assert_eq!(ui_surface.taffy.child_count(ui_parent_node), 0); + assert_eq!(ui_surface.taffy().child_count(ui_parent_node), 0); let mut ui_child_entities = (0..10) .map(|_| { @@ -668,23 +668,23 @@ mod tests { // `ui_parent_node` should have children now let ui_surface = world.resource::(); assert_eq!( - ui_surface.entity_to_taffy.len(), + ui_surface.entity_to_taffy().len(), 1 + ui_child_entities.len() ); assert_eq!( - ui_surface.taffy.child_count(ui_parent_node), + ui_surface.taffy().child_count(ui_parent_node), ui_child_entities.len() ); let child_node_map = HashMap::from_iter( ui_child_entities .iter() - .map(|child_entity| (*child_entity, ui_surface.entity_to_taffy[child_entity])), + .map(|child_entity| (*child_entity, ui_surface.entity_to_taffy()[child_entity])), ); // the children should have a corresponding ui node and that ui node's parent should be `ui_parent_node` for node in child_node_map.values() { - assert_eq!(ui_surface.taffy.parent(*node), Some(ui_parent_node)); + assert_eq!(ui_surface.taffy().parent(*node), Some(ui_parent_node)); } // delete every second child @@ -699,21 +699,21 @@ mod tests { let ui_surface = world.resource::(); assert_eq!( - ui_surface.entity_to_taffy.len(), + ui_surface.entity_to_taffy().len(), 1 + ui_child_entities.len() ); assert_eq!( - ui_surface.taffy.child_count(ui_parent_node), + ui_surface.taffy().child_count(ui_parent_node), ui_child_entities.len() ); // the remaining children should still have nodes in the layout tree for child_entity in &ui_child_entities { let child_node = child_node_map[child_entity]; - assert_eq!(ui_surface.entity_to_taffy[child_entity], child_node); - assert_eq!(ui_surface.taffy.parent(child_node), Some(ui_parent_node)); + assert_eq!(ui_surface.entity_to_taffy()[child_entity], child_node); + assert_eq!(ui_surface.taffy().parent(child_node), Some(ui_parent_node)); assert!(ui_surface - .taffy + .taffy() .children(ui_parent_node) .unwrap() .contains(&child_node)); @@ -722,11 +722,11 @@ mod tests { // the nodes of the deleted children should have been removed from the layout tree for deleted_child_entity in &deleted_children { assert!(!ui_surface - .entity_to_taffy + .entity_to_taffy() .contains_key(deleted_child_entity)); let deleted_child_node = child_node_map[deleted_child_entity]; assert!(!ui_surface - .taffy + .taffy() .children(ui_parent_node) .unwrap() .contains(&deleted_child_node)); @@ -739,8 +739,8 @@ mod tests { // all nodes should have been deleted let ui_surface = world.resource::(); - assert!(ui_surface.entity_to_taffy.is_empty()); - assert_eq!(ui_surface.taffy.total_node_count(), 0); + assert!(ui_surface.entity_to_taffy().is_empty()); + assert_eq!(ui_surface.taffy().total_node_count(), 0); } /// regression test for >=0.13.1 root node layouts @@ -903,7 +903,7 @@ mod tests { } fn get_taffy_node_count(world: &World) -> usize { - world.resource::().taffy.total_node_count() + world.resource::().taffy().total_node_count() } let (mut world, mut ui_schedule) = setup_ui_test_world(); @@ -1015,10 +1015,10 @@ mod tests { ui_schedule.run(&mut world); let ui_surface = world.resource::(); - let ui_node = ui_surface.entity_to_taffy[&ui_entity]; + let ui_node = ui_surface.entity_to_taffy()[&ui_entity]; // a node with a content size should have taffy context - assert!(ui_surface.taffy.get_node_context(ui_node).is_some()); + assert!(ui_surface.taffy().get_node_context(ui_node).is_some()); let layout = ui_surface.get_layout(ui_entity).unwrap(); assert_eq!(layout.size.width, content_size.x); assert_eq!(layout.size.height, content_size.y); @@ -1029,7 +1029,7 @@ mod tests { let ui_surface = world.resource::(); // a node without a content size should not have taffy context - assert!(ui_surface.taffy.get_node_context(ui_node).is_none()); + assert!(ui_surface.taffy().get_node_context(ui_node).is_none()); // Without a content size, the node has no width or height constraints so the length of both dimensions is 0. let layout = ui_surface.get_layout(ui_entity).unwrap(); @@ -1181,7 +1181,7 @@ mod tests { let ui_surface = world.get_resource::().unwrap(); // 1 for root node, 1 for implicit viewport node // 2 children - assert_eq!(ui_surface.taffy.total_node_count(), 4); + assert_eq!(ui_surface.taffy().total_node_count(), 4); ( world, @@ -1208,11 +1208,11 @@ mod tests { let ui_surface = world.get_resource::().unwrap(); - let parent_taffy = *ui_surface.entity_to_taffy.get(&parent_entity).unwrap(); - let child1_taffy = *ui_surface.entity_to_taffy.get(&child1_entity).unwrap(); - let child2_taffy = *ui_surface.entity_to_taffy.get(&child2_entity).unwrap(); - assert_eq!(ui_surface.taffy.parent(child2_taffy), Some(child1_taffy)); - assert_eq!(ui_surface.taffy.parent(child1_taffy), Some(parent_taffy)); + let parent_taffy = *ui_surface.entity_to_taffy().get(&parent_entity).unwrap(); + let child1_taffy = *ui_surface.entity_to_taffy().get(&child1_entity).unwrap(); + let child2_taffy = *ui_surface.entity_to_taffy().get(&child2_entity).unwrap(); + assert_eq!(ui_surface.taffy().parent(child2_taffy), Some(child1_taffy)); + assert_eq!(ui_surface.taffy().parent(child1_taffy), Some(parent_taffy)); world.commands().entity(parent_entity).despawn_recursive(); @@ -1220,7 +1220,7 @@ mod tests { let ui_surface = world.get_resource::().unwrap(); // all nodes should be removed - assert_eq!(ui_surface.taffy.total_node_count(), 0); + assert_eq!(ui_surface.taffy().total_node_count(), 0); } #[test] @@ -1237,11 +1237,11 @@ mod tests { let ui_surface = world.get_resource::().unwrap(); - let parent_taffy = *ui_surface.entity_to_taffy.get(&parent_entity).unwrap(); - let child1_taffy = *ui_surface.entity_to_taffy.get(&child1_entity).unwrap(); - let child2_taffy = *ui_surface.entity_to_taffy.get(&child2_entity).unwrap(); - assert_eq!(ui_surface.taffy.parent(child2_taffy), Some(child1_taffy)); - assert_eq!(ui_surface.taffy.parent(child1_taffy), Some(parent_taffy)); + let parent_taffy = *ui_surface.entity_to_taffy().get(&parent_entity).unwrap(); + let child1_taffy = *ui_surface.entity_to_taffy().get(&child1_entity).unwrap(); + let child2_taffy = *ui_surface.entity_to_taffy().get(&child2_entity).unwrap(); + assert_eq!(ui_surface.taffy().parent(child2_taffy), Some(child1_taffy)); + assert_eq!(ui_surface.taffy().parent(child1_taffy), Some(parent_taffy)); world.commands().entity(child1_entity).despawn_recursive(); @@ -1249,6 +1249,6 @@ mod tests { let ui_surface = world.get_resource::().unwrap(); // only root node and implicit viewport left - assert_eq!(ui_surface.taffy.total_node_count(), 2); + assert_eq!(ui_surface.taffy().total_node_count(), 2); } } diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 0733e7234b0b7..1bd4072a58eda 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -60,13 +60,13 @@ pub struct UiSurface { /// Maintains an entry for each root ui node (parentless), and any of its children /// /// (does not include the `implicit_viewport_node`) - pub(super) entity_to_taffy: EntityHashMap, + entity_to_taffy: EntityHashMap, /// Maps root ui node (parentless) `Entity` to its corresponding `RootNodeData` - pub(super) root_node_data: EntityHashMap, + root_node_data: EntityHashMap, /// Maps camera `Entity` to an associated `EntityHashSet` of root nodes (parentless) - pub(super) camera_root_nodes: EntityHashMap, + camera_root_nodes: EntityHashMap, /// Manages the UI Node Tree - pub(super) taffy: TaffyTree, + taffy: TaffyTree, } fn _assert_send_sync_ui_surface_impl_safe() { @@ -426,6 +426,23 @@ with UI components as a child of an entity without UI components, results may be } } +// Expose readonly accessors for tests in mod +#[cfg(test)] +impl UiSurface { + pub(super) fn entity_to_taffy(&self) -> &EntityHashMap { + &self.entity_to_taffy + } + pub(super) fn root_node_data(&self) -> &EntityHashMap { + &self.root_node_data + } + pub(super) fn camera_root_nodes(&self) -> &EntityHashMap { + &self.camera_root_nodes + } + pub(super) fn taffy(&self) -> &TaffyTree { + &self.taffy + } +} + #[cfg(test)] mod tests { use super::*; From 8b3f47c5f7ad91486460341096f82c0013ca0a14 Mon Sep 17 00:00:00 2001 From: bstriker Date: Mon, 13 May 2024 18:17:31 -0400 Subject: [PATCH 4/4] Move debug print into UiSurface to access private fields --- crates/bevy_ui/src/layout/debug.rs | 106 ++-------------- crates/bevy_ui/src/layout/ui_surface.rs | 155 +++++++++++++++++++++++- 2 files changed, 163 insertions(+), 98 deletions(-) diff --git a/crates/bevy_ui/src/layout/debug.rs b/crates/bevy_ui/src/layout/debug.rs index 7dd8ab8072e92..439632b494738 100644 --- a/crates/bevy_ui/src/layout/debug.rs +++ b/crates/bevy_ui/src/layout/debug.rs @@ -1,101 +1,17 @@ -use std::fmt::Write; - -use taffy::{NodeId, TraversePartialTree}; - -use bevy_ecs::prelude::Entity; -use bevy_utils::HashMap; - use crate::layout::ui_surface::UiSurface; -/// Prints a debug representation of the computed layout of the UI layout tree for each window. +/// Prints a debug representation of the computed layout of the UI layout tree for each camera. +#[deprecated( + since = "0.13.3", + note = "use `ui_surface.ui_layout_tree_debug_string()` instead" +)] pub fn print_ui_layout_tree(ui_surface: &UiSurface) { - let taffy_to_entity: HashMap = ui_surface - .entity_to_taffy() - .iter() - .map(|(&ui_entity, &taffy_node)| (taffy_node, ui_entity)) - .collect::>(); - for (&camera_entity, root_node_set) in ui_surface.camera_root_nodes().iter() { - bevy_utils::tracing::info!("Layout tree for camera entity: {camera_entity}"); - for &root_node_entity in root_node_set.iter() { - let Some(implicit_viewport_node) = ui_surface - .root_node_data() - .get(&root_node_entity) - .map(|rnd| rnd.implicit_viewport_node) - else { - continue; - }; - let mut out = String::new(); - print_node( - ui_surface, - &taffy_to_entity, - camera_entity, - implicit_viewport_node, - false, - String::new(), - &mut out, - ); - - bevy_utils::tracing::info!("{out}"); + let debug_layout_tree = match ui_surface.ui_layout_tree_debug_string() { + Ok(output) => output, + Err(err) => { + bevy_utils::tracing::error!("Failed to print ui layout tree: {err}"); + return; } - } -} - -/// Recursively navigates the layout tree printing each node's information. -fn print_node( - ui_surface: &UiSurface, - taffy_to_entity: &HashMap, - entity: Entity, - node: NodeId, - has_sibling: bool, - lines_string: String, - acc: &mut String, -) { - let tree = &ui_surface.taffy(); - let layout = tree.layout(node).unwrap(); - let style = tree.style(node).unwrap(); - - let num_children = tree.child_count(node); - - let display_variant = match (num_children, style.display) { - (_, taffy::style::Display::None) => "NONE", - (0, _) => "LEAF", - (_, taffy::style::Display::Flex) => "FLEX", - (_, taffy::style::Display::Grid) => "GRID", - (_, taffy::style::Display::Block) => "BLOCK", }; - - let fork_string = if has_sibling { - "├── " - } else { - "└── " - }; - writeln!( - acc, - "{lines}{fork} {display} [x: {x:<4} y: {y:<4} width: {width:<4} height: {height:<4}] ({entity:?}) {measured}", - lines = lines_string, - fork = fork_string, - display = display_variant, - x = layout.location.x, - y = layout.location.y, - width = layout.size.width, - height = layout.size.height, - measured = if tree.get_node_context(node).is_some() { "measured" } else { "" } - ).ok(); - let bar = if has_sibling { "│ " } else { " " }; - let new_string = lines_string + bar; - - // Recurse into children - for (index, child_node) in tree.children(node).unwrap().iter().enumerate() { - let has_sibling = index < num_children - 1; - let child_entity = taffy_to_entity.get(child_node).unwrap(); - print_node( - ui_surface, - taffy_to_entity, - *child_entity, - *child_node, - has_sibling, - new_string.clone(), - acc, - ); - } + bevy_utils::tracing::info!("{debug_layout_tree}"); } diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 1bd4072a58eda..eb0f7da84ac3b 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -1,12 +1,12 @@ use std::fmt; -use taffy::TaffyTree; +use taffy::{NodeId, TaffyTree, TraversePartialTree}; use bevy_ecs::entity::{Entity, EntityHashMap, EntityHashSet}; use bevy_ecs::prelude::Resource; use bevy_math::UVec2; -use bevy_utils::default; use bevy_utils::tracing::warn; +use bevy_utils::{default, HashMap}; use crate::layout::convert; use crate::{LayoutContext, LayoutError, Measure, NodeMeasure, Style}; @@ -424,6 +424,100 @@ with UI components as a child of an entity without UI components, results may be Err(LayoutError::InvalidHierarchy) } } + + /// Returns a debug representation of the computed layout of the UI layout tree for each camera. + pub fn ui_layout_tree_debug_string(&self) -> Result { + use std::fmt::Write; + let mut output = String::new(); + let taffy_to_entity: HashMap = self + .entity_to_taffy + .iter() + .map(|(&ui_entity, &taffy_node)| (taffy_node, ui_entity)) + .collect::>(); + for (&camera_entity, root_node_set) in self.camera_root_nodes.iter() { + writeln!(output, "Layout tree for camera entity: {camera_entity}")?; + for &root_node_entity in root_node_set.iter() { + let Some(implicit_viewport_node) = self + .root_node_data + .get(&root_node_entity) + .map(|rnd| rnd.implicit_viewport_node) + else { + continue; + }; + self.write_node( + &taffy_to_entity, + camera_entity, + implicit_viewport_node, + false, + String::new(), + &mut output, + )?; + } + } + Ok(output) + } + + /// Recursively navigates the layout tree writing each node's information to the output/acc. + fn write_node( + &self, + taffy_to_entity: &HashMap, + entity: Entity, + node: NodeId, + has_sibling: bool, + lines_string: String, + acc: &mut String, + ) -> fmt::Result { + use std::fmt::Write; + let tree = &self.taffy; + let layout = tree.layout(node).unwrap(); + let style = tree.style(node).unwrap(); + + let num_children = tree.child_count(node); + + let display_variant = match (num_children, style.display) { + (_, taffy::style::Display::None) => "NONE", + (0, _) => "LEAF", + (_, taffy::style::Display::Flex) => "FLEX", + (_, taffy::style::Display::Grid) => "GRID", + (_, taffy::style::Display::Block) => "BLOCK", + }; + + let fork_string = if has_sibling { + "├── " + } else { + "└── " + }; + writeln!( + acc, + "{lines}{fork} {display} [x: {x:<4} y: {y:<4} width: {width:<4} height: {height:<4}] ({entity:?}) {measured}", + lines = lines_string, + fork = fork_string, + display = display_variant, + x = layout.location.x, + y = layout.location.y, + width = layout.size.width, + height = layout.size.height, + measured = if tree.get_node_context(node).is_some() { "measured" } else { "" } + )?; + let bar = if has_sibling { "│ " } else { " " }; + let new_string = lines_string + bar; + + // Recurse into children + for (index, child_node) in tree.children(node).unwrap().iter().enumerate() { + let has_sibling = index < num_children - 1; + let child_entity = taffy_to_entity.get(child_node).unwrap(); + self.write_node( + taffy_to_entity, + *child_entity, + *child_node, + has_sibling, + new_string.clone(), + acc, + )?; + } + + Ok(()) + } } // Expose readonly accessors for tests in mod @@ -446,7 +540,7 @@ impl UiSurface { #[cfg(test)] mod tests { use super::*; - use crate::{ContentSize, FixedMeasure}; + use crate::{ContentSize, FixedMeasure, Val}; use bevy_math::Vec2; use taffy::TraversePartialTree; @@ -824,4 +918,59 @@ mod tests { let taffy_node = ui_surface.entity_to_taffy.get(&root_node_entity).unwrap(); assert!(ui_surface.taffy.layout(*taffy_node).is_ok()); } + + #[test] + fn test_ui_layout_tree_debug_string() { + let mut ui_surface = UiSurface::default(); + + let camera_entity = Entity::from_raw(0); + let root_node_entity = Entity::from_raw(1); + let child_entity = Entity::from_raw(2); + let style = Style::default(); + + ui_surface.upsert_node(&TEST_LAYOUT_CONTEXT, root_node_entity, &style, None); + ui_surface.upsert_node(&TEST_LAYOUT_CONTEXT, child_entity, &style, None); + + let root_taffy_node = *ui_surface.entity_to_taffy.get(&root_node_entity).unwrap(); + let child_taffy = *ui_surface.entity_to_taffy.get(&child_entity).unwrap(); + // set up the relationship manually + ui_surface + .taffy + .add_child(root_taffy_node, child_taffy) + .unwrap(); + ui_surface.set_camera_children(camera_entity, [root_node_entity].into_iter()); + + let camera_entity2 = Entity::from_raw(3); + let root_node_entity2 = Entity::from_raw(4); + let style = Style { + top: Val::Px(1.), + left: Val::Px(1.), + width: Val::Percent(100.), + height: Val::Percent(100.), + ..default() + }; + ui_surface.upsert_node( + &TEST_LAYOUT_CONTEXT, + root_node_entity2, + &style, + Some(NodeMeasure::Fixed(FixedMeasure { size: Vec2::ONE })), + ); + ui_surface.set_camera_children(camera_entity2, [root_node_entity2].into_iter()); + + ui_surface.compute_camera_layout(&camera_entity, UVec2::ONE); + ui_surface.compute_camera_layout(&camera_entity2, UVec2::ONE); + + let debug_string = ui_surface + .ui_layout_tree_debug_string() + .expect("expected debug string"); + let lines = debug_string.lines().collect::>(); + + assert!(lines[0].starts_with("Layout tree for camera entity: 0v1|")); + assert_eq!(lines[1], "└── GRID [x: 0 y: 0 width: 1 height: 1 ] (Entity { index: 0, generation: 1 }) "); + assert_eq!(lines[2], " └── FLEX [x: 0 y: 0 width: 0 height: 0 ] (Entity { index: 1, generation: 1 }) "); + assert_eq!(lines[3], " └── LEAF [x: 0 y: 0 width: 0 height: 0 ] (Entity { index: 2, generation: 1 }) "); + assert!(lines[4].starts_with("Layout tree for camera entity: 3v1|")); + assert_eq!(lines[5], "└── GRID [x: 0 y: 0 width: 1 height: 1 ] (Entity { index: 3, generation: 1 }) "); + assert_eq!(lines[6], " └── LEAF [x: 1 y: 1 width: 1 height: 1 ] (Entity { index: 4, generation: 1 }) measured"); + } }