From de08fb2afa5e8da1d910ed6be424649b6fbe9f0e Mon Sep 17 00:00:00 2001 From: Brett Striker Date: Tue, 15 Oct 2024 10:06:17 -0400 Subject: [PATCH] [bevy_ui/layout] Add tests, missing asserts, and missing debug fields for UiSurface (#12803) This is 3 of 5 iterative PR's that affect bevy_ui/layout - [x] Blocked by https://github.com/bevyengine/bevy/pull/12801 - [x] Blocked by https://github.com/bevyengine/bevy/pull/12802 --- # Objective - Add tests to `UiSurface` - Add missing asserts in `_assert_send_sync_ui_surface_impl_safe` - Add missing Debug field print for `camera_entity_to_taffy` ## Solution - Adds tests to `UiSurface` - Adds missing asserts in `_assert_send_sync_ui_surface_impl_safe` - Adds missing impl Debug field print for `camera_entity_to_taffy` --------- Co-authored-by: Alice Cecile --- crates/bevy_ui/src/layout/mod.rs | 65 +++- crates/bevy_ui/src/layout/ui_surface.rs | 410 ++++++++++++++++++++++++ crates/bevy_ui/src/lib.rs | 18 +- 3 files changed, 486 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index 492f1c1e0f2d4..504e2e02074b6 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -55,6 +55,16 @@ impl LayoutContext { } } +#[cfg(test)] +impl LayoutContext { + pub const TEST_CONTEXT: Self = Self { + scale_factor: 1.0, + physical_size: Vec2::new(1000.0, 1000.0), + min_size: 0.0, + max_size: 1000.0, + }; +} + impl Default for LayoutContext { fn default() -> Self { Self::DEFAULT @@ -513,7 +523,7 @@ mod tests { prelude::*, ui_layout_system, update::update_target_camera_system, - ContentSize, + ContentSize, LayoutContext, }; #[test] @@ -1234,4 +1244,57 @@ mod tests { ui_schedule.run(&mut world); } + + #[test] + fn test_ui_surface_compute_camera_layout() { + use bevy_ecs::prelude::ResMut; + + let (mut world, ..) = setup_ui_test_world(); + + let camera_entity = Entity::from_raw(0); + let root_node_entity = Entity::from_raw(1); + + struct TestSystemParam { + camera_entity: Entity, + root_node_entity: Entity, + } + + fn test_system( + params: In, + mut ui_surface: ResMut, + #[cfg(feature = "bevy_text")] mut computed_text_block_query: Query< + &mut bevy_text::ComputedTextBlock, + >, + #[cfg(feature = "bevy_text")] mut font_system: ResMut, + ) { + ui_surface.upsert_node( + &LayoutContext::TEST_CONTEXT, + params.root_node_entity, + &Style::default(), + None, + ); + + ui_surface.compute_camera_layout( + params.camera_entity, + UVec2::new(800, 600), + #[cfg(feature = "bevy_text")] + &mut computed_text_block_query, + #[cfg(feature = "bevy_text")] + &mut font_system.0, + ); + } + + let _ = world.run_system_once_with( + TestSystemParam { + camera_entity, + root_node_entity, + }, + test_system, + ); + + let ui_surface = world.resource::(); + + let taffy_node = ui_surface.entity_to_taffy.get(&root_node_entity).unwrap(); + assert!(ui_surface.taffy.layout(*taffy_node).is_ok()); + } } diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 4ffd2b467538a..24c9965d14ead 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -33,6 +33,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::(); } @@ -41,7 +43,9 @@ 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("taffy_children_scratch", &self.taffy_children_scratch) .finish() } } @@ -312,3 +316,409 @@ fn get_text_buffer<'a>( }; Some(computed.into_inner()) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ContentSize, FixedMeasure}; + use bevy_math::Vec2; + use taffy::TraversePartialTree; + + /// 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(&LayoutContext::TEST_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(&LayoutContext::TEST_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(&LayoutContext::TEST_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(&LayoutContext::TEST_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(&LayoutContext::TEST_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(&LayoutContext::TEST_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(&LayoutContext::TEST_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(&LayoutContext::TEST_CONTEXT, root_node_entity, &style, None); + ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, child_entity, &style, None); + + ui_surface.update_children(root_node_entity, vec![child_entity].into_iter()); + + 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(&LayoutContext::TEST_CONTEXT, root_node_entity, &style, None); + ui_surface.upsert_node(&LayoutContext::TEST_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] + #[cfg(not(feature = "bevy_text"))] + 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(&LayoutContext::TEST_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()); + } +} diff --git a/crates/bevy_ui/src/lib.rs b/crates/bevy_ui/src/lib.rs index 007959402ba8b..1cdd48aa5a1cc 100644 --- a/crates/bevy_ui/src/lib.rs +++ b/crates/bevy_ui/src/lib.rs @@ -46,6 +46,7 @@ use widget::UiImageSize; /// /// This includes the most common types in this crate, re-exported for your convenience. pub mod prelude { + #[cfg(feature = "bevy_text")] #[allow(deprecated)] #[doc(hidden)] pub use crate::widget::TextBundle; @@ -178,17 +179,22 @@ impl Plugin for UiPlugin { ui_focus_system.in_set(UiSystem::Focus).after(InputSystem), ); + let ui_layout_system_config = ui_layout_system + .in_set(UiSystem::Layout) + .before(TransformSystem::TransformPropagate); + + #[cfg(feature = "bevy_text")] + let ui_layout_system_config = ui_layout_system_config + // Text and Text2D operate on disjoint sets of entities + .ambiguous_with(bevy_text::update_text2d_layout) + .ambiguous_with(bevy_text::detect_text_needs_rerender::); + app.add_systems( PostUpdate, ( check_visibility::.in_set(VisibilitySystems::CheckVisibility), update_target_camera_system.in_set(UiSystem::Prepare), - ui_layout_system - .in_set(UiSystem::Layout) - .before(TransformSystem::TransformPropagate) - // Text and Text2D operate on disjoint sets of entities - .ambiguous_with(bevy_text::detect_text_needs_rerender::) - .ambiguous_with(bevy_text::update_text2d_layout), + ui_layout_system_config, ui_stack_system .in_set(UiSystem::Stack) // the systems don't care about stack index