From d2bdbc69a0fe18d53efdcba37141d0ec05db4b7b Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 13:11:19 -0400 Subject: [PATCH 01/33] Promote UiSurface to mod --- crates/bevy_ui/src/layout/debug.rs | 2 +- crates/bevy_ui/src/layout/mod.rs | 222 +---------------------- crates/bevy_ui/src/layout/ui_surface.rs | 223 ++++++++++++++++++++++++ crates/bevy_ui/src/lib.rs | 5 +- 4 files changed, 233 insertions(+), 219 deletions(-) create mode 100644 crates/bevy_ui/src/layout/ui_surface.rs diff --git a/crates/bevy_ui/src/layout/debug.rs b/crates/bevy_ui/src/layout/debug.rs index 8d5f27e169eca..ee727625179c8 100644 --- a/crates/bevy_ui/src/layout/debug.rs +++ b/crates/bevy_ui/src/layout/debug.rs @@ -1,4 +1,4 @@ -use crate::UiSurface; +use crate::layout::ui_surface::UiSurface; use bevy_ecs::prelude::Entity; use bevy_utils::HashMap; use std::fmt::Write; diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index 7ff8a59229fca..c27e685b288f0 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -1,5 +1,6 @@ mod convert; pub mod debug; +mod ui_surface; use crate::{ContentSize, DefaultUiCamera, Node, Outline, Style, TargetCamera, UiScale}; use bevy_ecs::{ @@ -19,8 +20,9 @@ use bevy_utils::tracing::warn; use bevy_utils::{default, HashMap, HashSet}; use bevy_window::{PrimaryWindow, Window, WindowScaleFactorChanged}; use std::fmt; -use taffy::{tree::LayoutTree, Taffy}; +use taffy::{Taffy, tree::LayoutTree}; use thiserror::Error; +use ui_surface::UiSurface; pub struct LayoutContext { pub scale_factor: f32, @@ -41,218 +43,6 @@ impl LayoutContext { } } -#[derive(Debug, Clone, PartialEq, Eq)] -struct RootNodePair { - // The implicit "viewport" node created by Bevy - implicit_viewport_node: taffy::node::Node, - // The root (parentless) node specified by the user - user_root_node: taffy::node::Node, -} - -#[derive(Resource)] -pub struct UiSurface { - entity_to_taffy: EntityHashMap, - camera_entity_to_taffy: EntityHashMap>, - camera_roots: EntityHashMap>, - taffy: Taffy, -} - -fn _assert_send_sync_ui_surface_impl_safe() { - fn _assert_send_sync() {} - _assert_send_sync::>(); - _assert_send_sync::(); - _assert_send_sync::(); -} - -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_roots", &self.camera_roots) - .finish() - } -} - -impl Default for UiSurface { - fn default() -> Self { - let mut taffy = Taffy::new(); - taffy.disable_rounding(); - Self { - entity_to_taffy: Default::default(), - camera_entity_to_taffy: Default::default(), - camera_roots: Default::default(), - taffy, - } - } -} - -impl UiSurface { - /// Retrieves the Taffy node associated with the given UI node entity and updates its style. - /// If no associated Taffy node exists a new Taffy node is inserted into the Taffy layout. - pub fn upsert_node(&mut self, entity: Entity, style: &Style, context: &LayoutContext) { - let mut added = false; - let taffy = &mut self.taffy; - let taffy_node = self.entity_to_taffy.entry(entity).or_insert_with(|| { - added = true; - taffy.new_leaf(convert::from_style(context, style)).unwrap() - }); - - if !added { - self.taffy - .set_style(*taffy_node, convert::from_style(context, style)) - .unwrap(); - } - } - - /// Update the `MeasureFunc` of the taffy node corresponding to the given [`Entity`] if the node exists. - pub fn try_update_measure( - &mut self, - entity: Entity, - measure_func: taffy::node::MeasureFunc, - ) -> Option<()> { - let taffy_node = self.entity_to_taffy.get(&entity)?; - - self.taffy.set_measure(*taffy_node, Some(measure_func)).ok() - } - - /// Update the children of the taffy node corresponding to the given [`Entity`]. - pub fn update_children(&mut self, entity: Entity, children: &Children) { - let mut taffy_children = Vec::with_capacity(children.len()); - for child in children { - if let Some(taffy_node) = self.entity_to_taffy.get(child) { - taffy_children.push(*taffy_node); - } else { - warn!( - "Unstyled child in a UI entity hierarchy. You are using an entity \ -without UI components as a child of an entity with UI components, results may be unexpected." - ); - } - } - - let taffy_node = self.entity_to_taffy.get(&entity).unwrap(); - self.taffy - .set_children(*taffy_node, &taffy_children) - .unwrap(); - } - - /// Removes children from the entity's taffy node if it exists. Does nothing otherwise. - pub fn try_remove_children(&mut self, entity: Entity) { - if let Some(taffy_node) = self.entity_to_taffy.get(&entity) { - self.taffy.set_children(*taffy_node, &[]).unwrap(); - } - } - - /// Removes the measure from the entity's taffy node if it exists. Does nothing otherwise. - pub fn try_remove_measure(&mut self, entity: Entity) { - if let Some(taffy_node) = self.entity_to_taffy.get(&entity) { - self.taffy.set_measure(*taffy_node, None).unwrap(); - } - } - - /// Set the ui node entities without a [`Parent`] as children to the root node in the taffy layout. - pub fn set_camera_children( - &mut self, - camera_id: 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 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.camera_roots.insert(camera_id, new_roots); - } - - /// 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 { - return; - }; - - 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(root_nodes.implicit_viewport_node, available_space) - .unwrap(); - } - } - - /// Removes each camera entity from the internal map and then removes their associated node from taffy - 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(); - } - } - } - } - - /// Removes each entity from the internal map and then removes their 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(); - } - } - } - - /// Get the layout geometry for the taffy node corresponding to the ui node [`Entity`]. - /// Does not compute the layout geometry, `compute_window_layouts` should be run before using this function. - pub fn get_layout(&self, entity: Entity) -> Result<&taffy::layout::Layout, LayoutError> { - if let Some(taffy_node) = self.entity_to_taffy.get(&entity) { - self.taffy - .layout(*taffy_node) - .map_err(LayoutError::TaffyError) - } else { - warn!( - "Styled child in a non-UI entity hierarchy. You are using an entity \ -with UI components as a child of an entity without UI components, results may be unexpected." - ); - Err(LayoutError::InvalidHierarchy) - } - } -} - #[derive(Debug, Error)] pub enum LayoutError { #[error("Invalid hierarchy")] @@ -538,7 +328,7 @@ mod tests { use crate::ui_layout_system; use crate::update::update_target_camera_system; use crate::ContentSize; - use crate::UiSurface; + use crate::layout::ui_surface::UiSurface; use bevy_asset::AssetEvent; use bevy_asset::Assets; use bevy_core_pipeline::core_2d::Camera2dBundle; @@ -551,8 +341,8 @@ 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_math::{vec2, Rect, UVec2, Vec2}; + use bevy_hierarchy::{BuildWorldChildren, Children, despawn_with_children_recursive, Parent}; + use bevy_math::{Rect, UVec2, vec2, Vec2}; use bevy_render::camera::ManualTextureViews; use bevy_render::camera::OrthographicProjection; use bevy_render::prelude::Camera; diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs new file mode 100644 index 0000000000000..346172a52d767 --- /dev/null +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -0,0 +1,223 @@ +use bevy_ecs::entity::{Entity, EntityHashMap}; +use bevy_ecs::prelude::Resource; +use taffy::Taffy; +use bevy_hierarchy::{Children, Parent}; +use bevy_math::UVec2; +use bevy_utils::default; +use bevy_utils::tracing::warn; +use std::fmt; +use taffy::prelude::LayoutTree; +use crate::{LayoutContext, LayoutError, Style}; +use crate::layout::convert; + +#[derive(Debug, Clone, PartialEq, Eq)] +struct RootNodePair { + // The implicit "viewport" node created by Bevy + implicit_viewport_node: taffy::node::Node, + // The root (parentless) node specified by the user + user_root_node: taffy::node::Node, +} + +#[derive(Resource)] +pub struct UiSurface { + entity_to_taffy: EntityHashMap, + camera_entity_to_taffy: EntityHashMap>, + camera_roots: EntityHashMap>, + taffy: Taffy, +} + +fn _assert_send_sync_ui_surface_impl_safe() { + fn _assert_send_sync() {} + _assert_send_sync::>(); + _assert_send_sync::(); + _assert_send_sync::(); +} + +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_roots", &self.camera_roots) + .finish() + } +} + +impl Default for UiSurface { + fn default() -> Self { + let mut taffy = Taffy::new(); + taffy.disable_rounding(); + Self { + entity_to_taffy: Default::default(), + camera_entity_to_taffy: Default::default(), + camera_roots: Default::default(), + taffy, + } + } +} + +impl UiSurface { + /// Retrieves the Taffy node associated with the given UI node entity and updates its style. + /// If no associated Taffy node exists a new Taffy node is inserted into the Taffy layout. + pub fn upsert_node(&mut self, entity: Entity, style: &Style, context: &LayoutContext) { + let mut added = false; + let taffy = &mut self.taffy; + let taffy_node = self.entity_to_taffy.entry(entity).or_insert_with(|| { + added = true; + taffy.new_leaf(convert::from_style(context, style)).unwrap() + }); + + if !added { + self.taffy + .set_style(*taffy_node, convert::from_style(context, style)) + .unwrap(); + } + } + + /// Update the `MeasureFunc` of the taffy node corresponding to the given [`Entity`] if the node exists. + pub fn try_update_measure( + &mut self, + entity: Entity, + measure_func: taffy::node::MeasureFunc, + ) -> Option<()> { + let taffy_node = self.entity_to_taffy.get(&entity)?; + + self.taffy.set_measure(*taffy_node, Some(measure_func)).ok() + } + + /// Update the children of the taffy node corresponding to the given [`Entity`]. + pub fn update_children(&mut self, entity: Entity, children: &Children) { + let mut taffy_children = Vec::with_capacity(children.len()); + for child in children { + if let Some(taffy_node) = self.entity_to_taffy.get(child) { + taffy_children.push(*taffy_node); + } else { + warn!( + "Unstyled child in a UI entity hierarchy. You are using an entity \ +without UI components as a child of an entity with UI components, results may be unexpected." + ); + } + } + + let taffy_node = self.entity_to_taffy.get(&entity).unwrap(); + self.taffy + .set_children(*taffy_node, &taffy_children) + .unwrap(); + } + + /// Removes children from the entity's taffy node if it exists. Does nothing otherwise. + pub fn try_remove_children(&mut self, entity: Entity) { + if let Some(taffy_node) = self.entity_to_taffy.get(&entity) { + self.taffy.set_children(*taffy_node, &[]).unwrap(); + } + } + + /// Removes the measure from the entity's taffy node if it exists. Does nothing otherwise. + pub fn try_remove_measure(&mut self, entity: Entity) { + if let Some(taffy_node) = self.entity_to_taffy.get(&entity) { + self.taffy.set_measure(*taffy_node, None).unwrap(); + } + } + + /// Set the ui node entities without a [`Parent`] as children to the root node in the taffy layout. + pub fn set_camera_children( + &mut self, + camera_id: 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 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.camera_roots.insert(camera_id, new_roots); + } + + /// 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 { + return; + }; + + 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(root_nodes.implicit_viewport_node, available_space) + .unwrap(); + } + } + + /// Removes each camera entity from the internal map and then removes their associated node from taffy + 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(); + } + } + } + } + + /// Removes each entity from the internal map and then removes their 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(); + } + } + } + + /// Get the layout geometry for the taffy node corresponding to the ui node [`Entity`]. + /// Does not compute the layout geometry, `compute_window_layouts` should be run before using this function. + pub fn get_layout(&self, entity: Entity) -> Result<&taffy::layout::Layout, LayoutError> { + if let Some(taffy_node) = self.entity_to_taffy.get(&entity) { + self.taffy + .layout(*taffy_node) + .map_err(LayoutError::TaffyError) + } else { + warn!( + "Styled child in a non-UI entity hierarchy. You are using an entity \ +with UI components as a child of an entity without UI components, results may be unexpected." + ); + Err(LayoutError::InvalidHierarchy) + } + } +} diff --git a/crates/bevy_ui/src/lib.rs b/crates/bevy_ui/src/lib.rs index a8f127980d3fc..09dfb31e3d834 100644 --- a/crates/bevy_ui/src/lib.rs +++ b/crates/bevy_ui/src/lib.rs @@ -42,8 +42,8 @@ use widget::UiImageSize; pub mod prelude { #[doc(hidden)] pub use crate::{ - geometry::*, node_bundles::*, ui_material::*, ui_node::*, widget::Button, widget::Label, - Interaction, UiMaterialPlugin, UiScale, + geometry::*, Interaction, node_bundles::*, ui_material::*, ui_node::*, UiMaterialPlugin, + UiScale, widget::Button, widget::Label, }; // `bevy_sprite` re-exports for texture slicing #[doc(hidden)] @@ -55,6 +55,7 @@ use bevy_ecs::prelude::*; use bevy_input::InputSystem; use bevy_render::RenderApp; use bevy_transform::TransformSystem; +use layout::ui_surface::UiSurface; use stack::ui_stack_system; pub use stack::UiStack; use update::{update_clipping_system, update_target_camera_system}; From 57d32616812ac695cc4423cb890b7128b7dc3a61 Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 13:16:14 -0400 Subject: [PATCH 02/33] Update visibility for ui_surface, RootNodePair, and UiSurface --- crates/bevy_ui/src/layout/mod.rs | 2 +- crates/bevy_ui/src/layout/ui_surface.rs | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index c27e685b288f0..7d34c4596148e 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -1,6 +1,6 @@ mod convert; pub mod debug; -mod ui_surface; +pub(crate) mod ui_surface; use crate::{ContentSize, DefaultUiCamera, Node, Outline, Style, TargetCamera, UiScale}; use bevy_ecs::{ diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 346172a52d767..8b39b45035c33 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -11,19 +11,19 @@ use crate::{LayoutContext, LayoutError, Style}; use crate::layout::convert; #[derive(Debug, Clone, PartialEq, Eq)] -struct RootNodePair { +pub struct RootNodePair { // The implicit "viewport" node created by Bevy - implicit_viewport_node: taffy::node::Node, + pub(super) implicit_viewport_node: taffy::node::Node, // The root (parentless) node specified by the user - user_root_node: taffy::node::Node, + pub(super) user_root_node: taffy::node::Node, } #[derive(Resource)] pub struct UiSurface { - entity_to_taffy: EntityHashMap, - camera_entity_to_taffy: EntityHashMap>, - camera_roots: EntityHashMap>, - taffy: Taffy, + pub(super) entity_to_taffy: EntityHashMap, + pub(super) camera_entity_to_taffy: EntityHashMap>, + pub(super) camera_roots: EntityHashMap>, + pub(super) taffy: Taffy, } fn _assert_send_sync_ui_surface_impl_safe() { From 9353506f11189c1bff03d221b21f867db51995c4 Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 13:20:03 -0400 Subject: [PATCH 03/33] Optimize imports in bevy_ui/layout --- crates/bevy_ui/src/layout/debug.rs | 9 ++++-- crates/bevy_ui/src/layout/mod.rs | 37 ++++++++++++++----------- crates/bevy_ui/src/layout/ui_surface.rs | 9 ++++-- 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/crates/bevy_ui/src/layout/debug.rs b/crates/bevy_ui/src/layout/debug.rs index ee727625179c8..67e7a205b2915 100644 --- a/crates/bevy_ui/src/layout/debug.rs +++ b/crates/bevy_ui/src/layout/debug.rs @@ -1,10 +1,13 @@ -use crate::layout::ui_surface::UiSurface; -use bevy_ecs::prelude::Entity; -use bevy_utils::HashMap; use std::fmt::Write; + use taffy::prelude::Node; use taffy::tree::LayoutTree; +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. pub fn print_ui_layout_tree(ui_surface: &UiSurface) { let taffy_to_entity: HashMap = ui_surface diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index 7d34c4596148e..6b8b36b454c20 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -1,8 +1,8 @@ -mod convert; -pub mod debug; -pub(crate) mod ui_surface; +use std::fmt; + +use taffy::{Taffy, tree::LayoutTree}; +use thiserror::Error; -use crate::{ContentSize, DefaultUiCamera, Node, Outline, Style, TargetCamera, UiScale}; use bevy_ecs::{ change_detection::{DetectChanges, DetectChangesMut}, entity::{Entity, EntityHashMap}, @@ -16,14 +16,17 @@ use bevy_hierarchy::{Children, Parent}; use bevy_math::{UVec2, Vec2}; use bevy_render::camera::{Camera, NormalizedRenderTarget}; use bevy_transform::components::Transform; -use bevy_utils::tracing::warn; use bevy_utils::{default, HashMap, HashSet}; +use bevy_utils::tracing::warn; use bevy_window::{PrimaryWindow, Window, WindowScaleFactorChanged}; -use std::fmt; -use taffy::{Taffy, tree::LayoutTree}; -use thiserror::Error; use ui_surface::UiSurface; +use crate::{ContentSize, DefaultUiCamera, Node, Outline, Style, TargetCamera, UiScale}; + +mod convert; +pub mod debug; +pub(crate) mod ui_surface; + pub struct LayoutContext { pub scale_factor: f32, pub physical_size: Vec2, @@ -323,12 +326,8 @@ fn round_layout_coords(value: Vec2) -> Vec2 { #[cfg(test)] mod tests { - use crate::layout::round_layout_coords; - use crate::prelude::*; - use crate::ui_layout_system; - use crate::update::update_target_camera_system; - use crate::ContentSize; - use crate::layout::ui_surface::UiSurface; + use taffy::tree::LayoutTree; + use bevy_asset::AssetEvent; use bevy_asset::Assets; use bevy_core_pipeline::core_2d::Camera2dBundle; @@ -348,15 +347,21 @@ mod tests { use bevy_render::prelude::Camera; use bevy_render::texture::Image; use bevy_transform::prelude::{GlobalTransform, Transform}; - use bevy_utils::prelude::default; use bevy_utils::HashMap; + use bevy_utils::prelude::default; use bevy_window::PrimaryWindow; use bevy_window::Window; use bevy_window::WindowCreated; use bevy_window::WindowResized; use bevy_window::WindowResolution; use bevy_window::WindowScaleFactorChanged; - use taffy::tree::LayoutTree; + + use crate::ContentSize; + use crate::layout::round_layout_coords; + use crate::layout::ui_surface::UiSurface; + use crate::prelude::*; + use crate::ui_layout_system; + use crate::update::update_target_camera_system; #[test] fn round_layout_coords_must_round_ties_up() { diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 8b39b45035c33..666ddcba28ac7 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -1,12 +1,15 @@ +use std::fmt; + +use taffy::prelude::LayoutTree; +use taffy::Taffy; + use bevy_ecs::entity::{Entity, EntityHashMap}; use bevy_ecs::prelude::Resource; -use taffy::Taffy; use bevy_hierarchy::{Children, Parent}; use bevy_math::UVec2; use bevy_utils::default; use bevy_utils::tracing::warn; -use std::fmt; -use taffy::prelude::LayoutTree; + use crate::{LayoutContext, LayoutError, Style}; use crate::layout::convert; From 0ab5686f01679668070f0c619f3e10dd54989bc9 Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 13:24:30 -0400 Subject: [PATCH 04/33] Remove unused imports in bevy_ui/layout --- crates/bevy_ui/src/layout/mod.rs | 9 +++------ crates/bevy_ui/src/layout/ui_surface.rs | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index 6b8b36b454c20..3b0532d806be9 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -1,22 +1,19 @@ -use std::fmt; - -use taffy::{Taffy, tree::LayoutTree}; use thiserror::Error; use bevy_ecs::{ change_detection::{DetectChanges, DetectChangesMut}, - entity::{Entity, EntityHashMap}, + entity::Entity, event::EventReader, query::{With, Without}, removal_detection::RemovedComponents, - system::{Query, Res, ResMut, Resource, SystemParam}, + system::{Query, Res, ResMut, SystemParam}, world::Ref, }; use bevy_hierarchy::{Children, Parent}; use bevy_math::{UVec2, Vec2}; use bevy_render::camera::{Camera, NormalizedRenderTarget}; use bevy_transform::components::Transform; -use bevy_utils::{default, HashMap, HashSet}; +use bevy_utils::{HashMap, HashSet}; use bevy_utils::tracing::warn; use bevy_window::{PrimaryWindow, Window, WindowScaleFactorChanged}; use ui_surface::UiSurface; diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 666ddcba28ac7..345c9ab730e72 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -5,7 +5,7 @@ use taffy::Taffy; use bevy_ecs::entity::{Entity, EntityHashMap}; use bevy_ecs::prelude::Resource; -use bevy_hierarchy::{Children, Parent}; +use bevy_hierarchy::Children; use bevy_math::UVec2; use bevy_utils::default; use bevy_utils::tracing::warn; From 042b21ccce3a7d670c4a27ffb1dfb1891560acf1 Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 13:25:40 -0400 Subject: [PATCH 05/33] Apply rustfmt in bevy_ui/layout --- crates/bevy_ui/src/layout/mod.rs | 10 +++++----- crates/bevy_ui/src/layout/ui_surface.rs | 2 +- crates/bevy_ui/src/lib.rs | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index 3b0532d806be9..62d60f50cbb18 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -13,8 +13,8 @@ use bevy_hierarchy::{Children, Parent}; use bevy_math::{UVec2, Vec2}; use bevy_render::camera::{Camera, NormalizedRenderTarget}; use bevy_transform::components::Transform; -use bevy_utils::{HashMap, HashSet}; use bevy_utils::tracing::warn; +use bevy_utils::{HashMap, HashSet}; use bevy_window::{PrimaryWindow, Window, WindowScaleFactorChanged}; use ui_surface::UiSurface; @@ -337,15 +337,15 @@ mod tests { use bevy_ecs::schedule::Schedule; use bevy_ecs::system::RunSystemOnce; use bevy_ecs::world::World; - use bevy_hierarchy::{BuildWorldChildren, Children, despawn_with_children_recursive, Parent}; - use bevy_math::{Rect, UVec2, vec2, Vec2}; + use bevy_hierarchy::{despawn_with_children_recursive, BuildWorldChildren, Children, Parent}; + use bevy_math::{vec2, Rect, UVec2, Vec2}; use bevy_render::camera::ManualTextureViews; use bevy_render::camera::OrthographicProjection; use bevy_render::prelude::Camera; use bevy_render::texture::Image; use bevy_transform::prelude::{GlobalTransform, Transform}; - use bevy_utils::HashMap; use bevy_utils::prelude::default; + use bevy_utils::HashMap; use bevy_window::PrimaryWindow; use bevy_window::Window; use bevy_window::WindowCreated; @@ -353,12 +353,12 @@ mod tests { use bevy_window::WindowResolution; use bevy_window::WindowScaleFactorChanged; - use crate::ContentSize; use crate::layout::round_layout_coords; use crate::layout::ui_surface::UiSurface; use crate::prelude::*; use crate::ui_layout_system; use crate::update::update_target_camera_system; + use crate::ContentSize; #[test] fn round_layout_coords_must_round_ties_up() { diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 345c9ab730e72..128f3a7f6e5f4 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -10,8 +10,8 @@ use bevy_math::UVec2; use bevy_utils::default; use bevy_utils::tracing::warn; -use crate::{LayoutContext, LayoutError, Style}; use crate::layout::convert; +use crate::{LayoutContext, LayoutError, Style}; #[derive(Debug, Clone, PartialEq, Eq)] pub struct RootNodePair { diff --git a/crates/bevy_ui/src/lib.rs b/crates/bevy_ui/src/lib.rs index 09dfb31e3d834..4ebfeb8e86df7 100644 --- a/crates/bevy_ui/src/lib.rs +++ b/crates/bevy_ui/src/lib.rs @@ -42,8 +42,8 @@ use widget::UiImageSize; pub mod prelude { #[doc(hidden)] pub use crate::{ - geometry::*, Interaction, node_bundles::*, ui_material::*, ui_node::*, UiMaterialPlugin, - UiScale, widget::Button, widget::Label, + geometry::*, node_bundles::*, ui_material::*, ui_node::*, widget::Button, widget::Label, + Interaction, UiMaterialPlugin, UiScale, }; // `bevy_sprite` re-exports for texture slicing #[doc(hidden)] From be7e6a26290e009c54bbdee474c2f59d02f5242d Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 13:36:27 -0400 Subject: [PATCH 06/33] Simplify GlobalTransform update in bevy_ui/layout tests Remove the need to manually update GlobalTransform in bevy_ui/layout tests --- crates/bevy_ui/src/layout/mod.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index 62d60f50cbb18..ccff47ae38067 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -343,7 +343,8 @@ mod tests { use bevy_render::camera::OrthographicProjection; use bevy_render::prelude::Camera; use bevy_render::texture::Image; - use bevy_transform::prelude::{GlobalTransform, Transform}; + use bevy_transform::prelude::GlobalTransform; + use bevy_transform::systems::{propagate_transforms, sync_simple_transforms}; use bevy_utils::prelude::default; use bevy_utils::HashMap; use bevy_window::PrimaryWindow; @@ -399,6 +400,8 @@ mod tests { update_target_camera_system, apply_deferred, ui_layout_system, + sync_simple_transforms, + propagate_transforms, ) .chain(), ); @@ -697,15 +700,11 @@ mod tests { ui_schedule.run(&mut world); let overlap_check = world - .query_filtered::<(Entity, &Node, &mut GlobalTransform, &Transform), Without>() - .iter_mut(&mut world) + .query_filtered::<(Entity, &Node, &GlobalTransform), Without>() + .iter(&world) .fold( Option::<(Rect, bool)>::None, - |option_rect, (entity, node, mut global_transform, transform)| { - // fix global transform - for some reason the global transform isn't populated yet. - // might be related to how these specific tests are working directly with World instead of App - *global_transform = GlobalTransform::from(transform.compute_affine()); - let global_transform = &*global_transform; + |option_rect, (entity, node, global_transform)| { let current_rect = node.logical_rect(global_transform); assert!( current_rect.height().abs() + current_rect.width().abs() > 0., From d761918dcffd071851f7b4eb1c1f93f2ca0361a5 Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 13:45:32 -0400 Subject: [PATCH 07/33] Add missing asserts and Debug fields in UiSurface from #12268 and #12698 --- crates/bevy_ui/src/layout/ui_surface.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 128f3a7f6e5f4..6657779e53d46 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -32,6 +32,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::(); } @@ -40,6 +42,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() } From 971577961bf2740e60f13180b40319092225125f Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 14:50:54 -0400 Subject: [PATCH 08/33] Widen type for parameter children in UiSurface::update_children --- crates/bevy_ui/src/layout/ui_surface.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 6657779e53d46..ee086d7f4852a 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -5,7 +5,6 @@ use taffy::Taffy; 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; @@ -91,7 +90,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) { From d65c0b228ced2736d09cee14d318a9ddf547d543 Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 15:21:30 -0400 Subject: [PATCH 09/33] Add tests for bevy_ui/layout/ui_surface --- crates/bevy_ui/src/layout/ui_surface.rs | 379 ++++++++++++++++++++++++ 1 file changed, 379 insertions(+) diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index ee086d7f4852a..d2fb17296987d 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -225,4 +225,383 @@ with UI components as a child of an entity without UI components, results may be Err(LayoutError::InvalidHierarchy) } } + + #[cfg(test)] + /// Tries to get the associated camera entity by iterating over `camera_entity_to_taffy` + fn get_associated_camera_entity(&self, root_node_entity: Entity) -> Option { + for (&camera_entity, root_node_map) in self.camera_entity_to_taffy.iter() { + if root_node_map.contains_key(&root_node_entity) { + return Some(camera_entity); + } + } + None + } + + #[cfg(test)] + /// Tries to get the root node pair for a given root node entity + fn get_root_node_pair(&self, root_node_entity: Entity) -> Option<&RootNodePair> { + // If `get_associated_camera_entity_from_ui_entity` returns `None`, + // it's not also guaranteed for camera_roots to not contain a reference + // to the root nodes taffy node + // + // `camera_roots` could still theoretically contain a reference to entities taffy node + // unless other writes/reads are proven to be atomic in nature + // so if they are out of sync then something else is wrong + let camera_entity = self.get_associated_camera_entity(root_node_entity)?; + self.get_root_node_pair_exact(root_node_entity, camera_entity) + } + + #[cfg(test)] + /// Tries to get the root node pair for a given root node entity with the specified camera entity + fn get_root_node_pair_exact( + &self, + root_node_entity: Entity, + camera_entity: Entity, + ) -> Option<&RootNodePair> { + let root_node_pairs = self.camera_roots.get(&camera_entity)?; + let root_node_taffy = self.entity_to_taffy.get(&root_node_entity)?; + root_node_pairs + .iter() + .find(|&root_node_pair| root_node_pair.user_root_node == *root_node_taffy) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ContentSize, FixedMeasure}; + use bevy_math::Vec2; + #[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); + } + + const DUMMY_LAYOUT_CONTEXT: LayoutContext = LayoutContext { + scale_factor: 1.0, + physical_size: Vec2::ONE, + min_size: 0.0, + max_size: 1.0, + }; + + trait IsRootNodePairValid { + fn is_root_node_pair_valid(&self, root_node_pair: &RootNodePair) -> bool; + } + + impl IsRootNodePairValid for Taffy { + fn is_root_node_pair_valid(&self, root_node_pair: &RootNodePair) -> bool { + self.parent(root_node_pair.user_root_node) + == Some(root_node_pair.implicit_viewport_node) + } + } + + #[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(root_node_entity, &style, &DUMMY_LAYOUT_CONTEXT); + + // 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(root_node_entity, &style, &DUMMY_LAYOUT_CONTEXT); + + // 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 = ui_surface + .get_root_node_pair_exact(root_node_entity, camera_entity) + .expect("expected root node pair"); + assert!(ui_surface.taffy.is_root_node_pair_valid(root_node_pair)); + + // test duplicate insert 2 + ui_surface.upsert_node(root_node_entity, &style, &DUMMY_LAYOUT_CONTEXT); + + // 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 = ui_surface + .get_root_node_pair_exact(root_node_entity, camera_entity) + .expect("expected root node pair"); + assert!(ui_surface.taffy.is_root_node_pair_valid(root_node_pair)); + } + + #[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(root_node_entity, &style, &DUMMY_LAYOUT_CONTEXT); + + // 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 = ui_surface + .get_root_node_pair_exact(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 + assert!(!ui_surface.camera_roots.contains_key(&camera_entity)); + + // root node pair should be removed + let root_node_pair = ui_surface.get_root_node_pair_exact(root_node_entity, camera_entity); + assert_eq!(root_node_pair, None); + } + + #[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(root_node_entity, &style, &DUMMY_LAYOUT_CONTEXT); + + 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 = ui_surface + .get_root_node_pair_exact(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 + 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(root_node_entity, &style, &DUMMY_LAYOUT_CONTEXT); + let mut content_size = ContentSize::default(); + content_size.set(FixedMeasure { size: Vec2::ONE }); + let measure_func = content_size.measure_func.take().unwrap(); + assert!(ui_surface + .try_update_measure(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(root_node_entity, &style, &DUMMY_LAYOUT_CONTEXT); + ui_surface.upsert_node(child_entity, &style, &DUMMY_LAYOUT_CONTEXT); + + 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)); + } + + #[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(root_node_entity, &style, &DUMMY_LAYOUT_CONTEXT); + ui_surface.upsert_node(child_entity, &style, &DUMMY_LAYOUT_CONTEXT); + + 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 = ui_surface + .get_root_node_pair_exact(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).unwrap(), + 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 + + 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).unwrap(), + 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).unwrap(), + 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(root_node_entity, &style, &DUMMY_LAYOUT_CONTEXT); + + 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 c730631922f815accb98e347495cf67f875dab7d Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 18:49:38 -0400 Subject: [PATCH 10/33] Rename RootNodePair to RootNodeData --- crates/bevy_ui/src/layout/ui_surface.rs | 84 ++++++++++++------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index d2fb17296987d..7718868aabeb1 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -13,7 +13,7 @@ use crate::layout::convert; use crate::{LayoutContext, LayoutError, Style}; #[derive(Debug, Clone, PartialEq, Eq)] -pub struct RootNodePair { +pub struct RootNodeData { // The implicit "viewport" node created by Bevy pub(super) implicit_viewport_node: taffy::node::Node, // The root (parentless) node specified by the user @@ -24,7 +24,7 @@ pub struct RootNodePair { pub struct UiSurface { pub(super) entity_to_taffy: EntityHashMap, pub(super) camera_entity_to_taffy: EntityHashMap>, - pub(super) camera_roots: EntityHashMap>, + pub(super) camera_roots: EntityHashMap>, pub(super) taffy: Taffy, } @@ -32,7 +32,7 @@ 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::(); } @@ -162,7 +162,7 @@ without UI components as a child of an entity with UI components, results may be .or_insert_with(|| self.taffy.new_leaf(viewport_style.clone()).unwrap()); self.taffy.add_child(viewport_node, node).unwrap(); - RootNodePair { + RootNodeData { implicit_viewport_node: viewport_node, user_root_node: node, } @@ -238,8 +238,8 @@ with UI components as a child of an entity without UI components, results may be } #[cfg(test)] - /// Tries to get the root node pair for a given root node entity - fn get_root_node_pair(&self, root_node_entity: Entity) -> Option<&RootNodePair> { + /// Tries to get the root node data for a given root node entity + fn get_root_node_data(&self, root_node_entity: Entity) -> Option<&RootNodeData> { // If `get_associated_camera_entity_from_ui_entity` returns `None`, // it's not also guaranteed for camera_roots to not contain a reference // to the root nodes taffy node @@ -248,21 +248,21 @@ with UI components as a child of an entity without UI components, results may be // unless other writes/reads are proven to be atomic in nature // so if they are out of sync then something else is wrong let camera_entity = self.get_associated_camera_entity(root_node_entity)?; - self.get_root_node_pair_exact(root_node_entity, camera_entity) + self.get_root_node_data_exact(root_node_entity, camera_entity) } #[cfg(test)] - /// 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 with the specified camera entity + fn get_root_node_data_exact( &self, root_node_entity: Entity, camera_entity: Entity, - ) -> Option<&RootNodePair> { - let root_node_pairs = self.camera_roots.get(&camera_entity)?; + ) -> Option<&RootNodeData> { + let root_node_datas = self.camera_roots.get(&camera_entity)?; let root_node_taffy = self.entity_to_taffy.get(&root_node_entity)?; - root_node_pairs + root_node_datas .iter() - .find(|&root_node_pair| root_node_pair.user_root_node == *root_node_taffy) + .find(|&root_node_data| root_node_data.user_root_node == *root_node_taffy) } } @@ -287,14 +287,14 @@ mod tests { max_size: 1.0, }; - trait IsRootNodePairValid { - fn is_root_node_pair_valid(&self, root_node_pair: &RootNodePair) -> bool; + trait IsRootNodeDataValid { + fn is_root_node_data_valid(&self, root_node_data: &RootNodeData) -> bool; } - impl IsRootNodePairValid for Taffy { - fn is_root_node_pair_valid(&self, root_node_pair: &RootNodePair) -> bool { - self.parent(root_node_pair.user_root_node) - == Some(root_node_pair.implicit_viewport_node) + impl IsRootNodeDataValid for Taffy { + fn is_root_node_data_valid(&self, root_node_data: &RootNodeData) -> bool { + self.parent(root_node_data.user_root_node) + == Some(root_node_data.implicit_viewport_node) } } @@ -324,11 +324,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 = ui_surface - .get_root_node_pair_exact(root_node_entity, camera_entity) - .expect("expected root node pair"); - assert!(ui_surface.taffy.is_root_node_pair_valid(root_node_pair)); + // root node data should now exist + let root_node_data = ui_surface + .get_root_node_data_exact(root_node_entity, camera_entity) + .expect("expected root node data"); + assert!(ui_surface.taffy.is_root_node_data_valid(root_node_data)); // test duplicate insert 2 ui_surface.upsert_node(root_node_entity, &style, &DUMMY_LAYOUT_CONTEXT); @@ -336,11 +336,11 @@ 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 = ui_surface - .get_root_node_pair_exact(root_node_entity, camera_entity) - .expect("expected root node pair"); - assert!(ui_surface.taffy.is_root_node_pair_valid(root_node_pair)); + // root node data should be unaffected + let root_node_data = ui_surface + .get_root_node_data_exact(root_node_entity, camera_entity) + .expect("expected root node data"); + assert!(ui_surface.taffy.is_root_node_data_valid(root_node_data)); } #[test] @@ -364,14 +364,14 @@ mod tests { .unwrap() .contains_key(&root_node_entity)); assert!(ui_surface.camera_roots.contains_key(&camera_entity)); - let root_node_pair = ui_surface - .get_root_node_pair_exact(root_node_entity, camera_entity) - .expect("expected root node pair"); + let root_node_data = ui_surface + .get_root_node_data_exact(root_node_entity, camera_entity) + .expect("expected root node data"); assert!(ui_surface .camera_roots .get(&camera_entity) .unwrap() - .contains(root_node_pair)); + .contains(root_node_data)); ui_surface.remove_camera_entities([camera_entity]); @@ -385,9 +385,9 @@ mod tests { return; // TODO: can't pass the test if we continue - not implemented assert!(!ui_surface.camera_roots.contains_key(&camera_entity)); - // root node pair should be removed - let root_node_pair = ui_surface.get_root_node_pair_exact(root_node_entity, camera_entity); - assert_eq!(root_node_pair, None); + // root node data should be removed + let root_node_data = ui_surface.get_root_node_data_exact(root_node_entity, camera_entity); + assert_eq!(root_node_data, None); } #[test] @@ -407,14 +407,14 @@ mod tests { .get(&camera_entity) .unwrap() .contains_key(&root_node_entity)); - let root_node_pair = ui_surface - .get_root_node_pair_exact(root_node_entity, camera_entity) + let root_node_data = ui_surface + .get_root_node_data_exact(root_node_entity, camera_entity) .unwrap(); assert!(ui_surface .camera_roots .get(&camera_entity) .unwrap() - .contains(root_node_pair)); + .contains(root_node_data)); ui_surface.remove_entities([root_node_entity]); assert!(!ui_surface.entity_to_taffy.contains_key(&root_node_entity)); @@ -508,9 +508,9 @@ mod tests { "child of root node should not be associated with camera" ); - let _root_node_pair = ui_surface - .get_root_node_pair_exact(root_node_entity, camera_entity) - .expect("expected root node pair"); + let _root_node_data = ui_surface + .get_root_node_data_exact(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(); From 32ee4aab4f42d08911994d9be3a05bf66813e7b9 Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 18:52:15 -0400 Subject: [PATCH 11/33] Rename camera_id to camera_entity --- crates/bevy_ui/src/layout/ui_surface.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 7718868aabeb1..e0812953b7a3a 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -126,7 +126,7 @@ without UI components as a child of an entity with UI components, results may be /// Set the ui node entities without a [`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 { @@ -142,8 +142,8 @@ without UI components as a child of an entity with UI components, results may be ..default() }; - 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 camera_root_node_map = self.camera_entity_to_taffy.entry(camera_entity).or_default(); + let existing_roots = self.camera_roots.entry(camera_entity).or_default(); let mut new_roots = Vec::new(); for entity in children { let node = *self.entity_to_taffy.get(&entity).unwrap(); @@ -170,7 +170,7 @@ without UI components as a child of an entity with UI components, results may be new_roots.push(root_node); } - self.camera_roots.insert(camera_id, new_roots); + self.camera_roots.insert(camera_entity, new_roots); } /// Compute the layout for each window entity's corresponding root node in the layout. From 207067cc0f4a4582d5168e673be4bb16c3d6efb0 Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 18:55:00 -0400 Subject: [PATCH 12/33] Extract default viewport style into inline fn --- crates/bevy_ui/src/layout/ui_surface.rs | 31 ++++++++++++++----------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index e0812953b7a3a..3527e9ed57bee 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -12,6 +12,22 @@ use bevy_utils::tracing::warn; use crate::layout::convert; use crate::{LayoutContext, LayoutError, Style}; +#[inline(always)] +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 RootNodeData { // The implicit "viewport" node created by Bevy @@ -129,19 +145,6 @@ without UI components as a child of an entity with UI components, results may be 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 camera_root_node_map = self.camera_entity_to_taffy.entry(camera_entity).or_default(); let existing_roots = self.camera_roots.entry(camera_entity).or_default(); let mut new_roots = Vec::new(); @@ -159,7 +162,7 @@ without UI components as a child of an entity with UI components, results may be let viewport_node = *camera_root_node_map .entry(entity) - .or_insert_with(|| self.taffy.new_leaf(viewport_style.clone()).unwrap()); + .or_insert_with(|| self.taffy.new_leaf(default_viewport_style()).unwrap()); self.taffy.add_child(viewport_node, node).unwrap(); RootNodeData { From 54ca22017c31aebef0b98355c3f0801107dac981 Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 18:59:25 -0400 Subject: [PATCH 13/33] Deprecate user_root_node in RootNodeData --- crates/bevy_ui/src/layout/ui_surface.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 3527e9ed57bee..7a9e324ebad5e 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -32,6 +32,7 @@ fn default_viewport_style() -> taffy::style::Style { pub struct RootNodeData { // The implicit "viewport" node created by Bevy pub(super) implicit_viewport_node: taffy::node::Node, + #[deprecated] // The root (parentless) node specified by the user pub(super) user_root_node: taffy::node::Node, } From 2d93de396e008b7ba46febb097f6c23dd500f7ba Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 19:31:23 -0400 Subject: [PATCH 14/33] Implement updated map structure in UiSurface --- crates/bevy_ui/src/layout/debug.rs | 25 +- crates/bevy_ui/src/layout/mod.rs | 12 +- crates/bevy_ui/src/layout/ui_surface.rs | 419 ++++++++++++++++-------- 3 files changed, 302 insertions(+), 154 deletions(-) diff --git a/crates/bevy_ui/src/layout/debug.rs b/crates/bevy_ui/src/layout/debug.rs index 67e7a205b2915..1a8b0b5263b3e 100644 --- a/crates/bevy_ui/src/layout/debug.rs +++ b/crates/bevy_ui/src/layout/debug.rs @@ -13,22 +13,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 ccff47ae38067..460ac1fa716d3 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -200,7 +200,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, @@ -494,7 +494,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(); @@ -508,9 +508,9 @@ mod tests { let ui_surface = world.resource::(); assert!(ui_surface - .camera_entity_to_taffy + .camera_root_nodes .contains_key(&camera_entity)); - assert_eq!(ui_surface.camera_entity_to_taffy.len(), 1); + assert_eq!(ui_surface.camera_root_nodes.len(), 1); world.despawn(ui_entity); world.despawn(camera_entity); @@ -520,9 +520,9 @@ mod tests { let ui_surface = world.resource::(); assert!(!ui_surface - .camera_entity_to_taffy + .camera_root_nodes .contains_key(&camera_entity)); - assert!(ui_surface.camera_entity_to_taffy.is_empty()); + assert!(ui_surface.camera_root_nodes.is_empty()); } #[test] diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 7a9e324ebad5e..f0a61e843e131 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -3,7 +3,7 @@ use std::fmt; use taffy::prelude::LayoutTree; use taffy::Taffy; -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; @@ -30,6 +30,7 @@ fn default_viewport_style() -> taffy::style::Style { #[derive(Debug, Clone, PartialEq, Eq)] pub struct RootNodeData { + pub(super) camera_entity: Option, // The implicit "viewport" node created by Bevy pub(super) implicit_viewport_node: taffy::node::Node, #[deprecated] @@ -40,16 +41,16 @@ pub struct RootNodeData { #[derive(Resource)] pub struct UiSurface { pub(super) entity_to_taffy: EntityHashMap, - pub(super) camera_entity_to_taffy: EntityHashMap>, - pub(super) camera_roots: EntityHashMap>, + pub(super) root_node_data: EntityHashMap, + pub(super) camera_root_nodes: EntityHashMap, pub(super) taffy: Taffy, } 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::(); } @@ -58,8 +59,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() } } @@ -70,8 +71,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, } } @@ -80,21 +81,101 @@ impl Default for UiSurface { impl UiSurface { /// Retrieves the Taffy node associated with the given UI node entity and updates its style. /// If no associated Taffy node exists a new Taffy node is inserted into the Taffy layout. - pub fn upsert_node(&mut self, entity: Entity, style: &Style, context: &LayoutContext) { + pub fn upsert_node( + &mut self, + ui_node_entity: Entity, + style: &Style, + context: &LayoutContext, + ) { let mut added = false; - let taffy = &mut self.taffy; - let taffy_node = self.entity_to_taffy.entry(entity).or_insert_with(|| { - added = true; - taffy.new_leaf(convert::from_style(context, style)).unwrap() - }); + let taffy_node = *self + .entity_to_taffy + .entry(ui_node_entity) + .or_insert_with(|| { + added = true; + self.taffy + .new_leaf(convert::from_style(context, style)) + .unwrap() + }); if !added { self.taffy - .set_style(*taffy_node, convert::from_style(context, style)) + .set_style(taffy_node, convert::from_style(context, style)) .unwrap(); } } + /// 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) { + if let Some(mut removed) = self.root_node_data.remove(root_node_entity) { + if let Some(camera_entity) = removed.camera_entity.take() { + if let Some(root_node_entities) = self.camera_root_nodes.get_mut(&camera_entity) { + root_node_entities.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) { + self.remove_root_node_viewport(ui_node_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); + } + } + + pub(super) fn demote_ui_node(&mut self, target_entity: &Entity, parent_entity: &Entity) { + if let Some(mut root_node_data) = self.root_node_data.remove(target_entity) { + if let Some(camera_entity) = root_node_data.camera_entity.take() { + if let Some(ui_set) = self.camera_root_nodes.get_mut(&camera_entity) { + ui_set.remove(target_entity); + } + } + self.taffy + .remove(root_node_data.implicit_viewport_node) + .unwrap(); + let parent_taffy = self.entity_to_taffy.get(parent_entity).unwrap(); + self.taffy + .add_child(*parent_taffy, root_node_data.user_root_node) + .unwrap(); + } + } + + pub(super) fn promote_ui_node(&mut self, target_entity: &Entity) { + self.root_node_data + .entry(*target_entity) + .or_insert_with(|| { + let user_root_node = *self.entity_to_taffy.get(target_entity).unwrap(); + let implicit_viewport_node = self + .taffy + .new_with_children(default_viewport_style(), &[user_root_node]) + .unwrap(); + RootNodeData { + camera_entity: None, + implicit_viewport_node, + user_root_node, + } + }); + } + /// Update the `MeasureFunc` of the taffy node corresponding to the given [`Entity`] if the node exists. pub fn try_update_measure( &mut self, @@ -140,77 +221,178 @@ without UI components as a child of an entity with UI components, results may be } } + fn mark_root_node_as_orphaned(&mut self, root_node_entity: &Entity) { + if let Some(root_node_data) = self.root_node_data.get_mut(root_node_entity) { + // mark it as orphaned + if let Some(old_camera_entity) = root_node_data.camera_entity.take() { + if let Some(root_nodes) = self.camera_root_nodes.get_mut(&old_camera_entity) { + root_nodes.remove(root_node_entity); + } + } + } + } + + 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; + let root_node_data = 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, + user_root_node, + } + }); + + if !added { + let option_old_camera_entity = root_node_data.camera_entity.replace(camera_entity); + // if we didn't insert, lets check to make the camera reference is the same + if Some(camera_entity) != option_old_camera_entity { + if let Some(old_camera_entity) = option_old_camera_entity { + // camera reference is not the same so remove it from the old set + if let Some(root_node_set) = self.camera_root_nodes.get_mut(&old_camera_entity) + { + root_node_set.remove(&ui_root_node_entity); + } + } + + self.camera_root_nodes + .entry(camera_entity) + .or_default() + .insert(ui_root_node_entity); + } + } + + root_node_data + } + /// Set the ui node entities without a [`Parent`] as children to the root node in the taffy layout. pub fn set_camera_children( &mut self, camera_entity: Entity, children: impl Iterator, ) { - let camera_root_node_map = self.camera_entity_to_taffy.entry(camera_entity).or_default(); - let existing_roots = self.camera_roots.entry(camera_entity).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 removed_children = self.camera_root_nodes.entry(camera_entity).or_default(); + let mut removed_children = removed_children.clone(); - let viewport_node = *camera_root_node_map - .entry(entity) - .or_insert_with(|| self.taffy.new_leaf(default_viewport_style()).unwrap()); - self.taffy.add_child(viewport_node, node).unwrap(); + for ui_entity in children { + let root_node_data = self.create_or_update_root_node_data(&ui_entity, &camera_entity); - RootNodeData { - implicit_viewport_node: viewport_node, - user_root_node: node, + if let Some(old_camera) = root_node_data.camera_entity.replace(camera_entity) { + if old_camera != camera_entity { + if let Some(old_siblings_set) = self.camera_root_nodes.get_mut(&old_camera) { + old_siblings_set.remove(&ui_entity); } - }); - new_roots.push(root_node); + } + } + let Some(root_node_data) = self.root_node_data.get_mut(&ui_entity) else { + unreachable!("impossible since root_node_data was created in create_or_update_root_node_data"); + }; + + // fix taffy relationships + { + if let Some(parent) = self.taffy.parent(root_node_data.user_root_node) { + self.taffy + .remove_child(parent, root_node_data.user_root_node) + .unwrap(); + } + + self.taffy + .add_child( + root_node_data.implicit_viewport_node, + root_node_data.user_root_node, + ) + .unwrap(); + } + + self.camera_root_nodes + .entry(camera_entity) + .or_default() + .insert(ui_entity); + + removed_children.remove(&ui_entity); } - self.camera_roots.insert(camera_entity, new_roots); + for orphan in removed_children.iter() { + if let Some(root_node_data) = self.root_node_data.get_mut(orphan) { + // mark as orphan + if let Some(camera_entity) = root_node_data.camera_entity.take() { + if let Some(children_set) = self.camera_root_nodes.get_mut(&camera_entity) { + children_set.remove(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 { + // Compute the layout for each window entity's corresponding root node in the layout. + 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 Some(root_node_data) = self.root_node_data.get(&root_node_entity) else { + continue; + }; + 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(root_nodes.implicit_viewport_node, available_space) + .compute_layout( + root_node_data.implicit_viewport_node, + available_space, + ) .unwrap(); } } - /// Removes each camera entity from the internal map and then removes their associated node from taffy + /// 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); } } @@ -231,42 +413,15 @@ with UI components as a child of an entity without UI components, results may be } #[cfg(test)] - /// Tries to get the associated camera entity by iterating over `camera_entity_to_taffy` + /// Tries to get the associated camera entity from root node data fn get_associated_camera_entity(&self, root_node_entity: Entity) -> Option { - for (&camera_entity, root_node_map) in self.camera_entity_to_taffy.iter() { - if root_node_map.contains_key(&root_node_entity) { - return Some(camera_entity); - } - } - None + self.get_root_node_data(root_node_entity)?.camera_entity } #[cfg(test)] /// Tries to get the root node data for a given root node entity fn get_root_node_data(&self, root_node_entity: Entity) -> Option<&RootNodeData> { - // If `get_associated_camera_entity_from_ui_entity` returns `None`, - // it's not also guaranteed for camera_roots to not contain a reference - // to the root nodes taffy node - // - // `camera_roots` could still theoretically contain a reference to entities taffy node - // unless other writes/reads are proven to be atomic in nature - // so if they are out of sync then something else is wrong - let camera_entity = self.get_associated_camera_entity(root_node_entity)?; - self.get_root_node_data_exact(root_node_entity, camera_entity) - } - - #[cfg(test)] - /// Tries to get the root node data for a given root node entity with the specified camera entity - fn get_root_node_data_exact( - &self, - root_node_entity: Entity, - camera_entity: Entity, - ) -> Option<&RootNodeData> { - let root_node_datas = self.camera_roots.get(&camera_entity)?; - let root_node_taffy = self.entity_to_taffy.get(&root_node_entity)?; - root_node_datas - .iter() - .find(|&root_node_data| root_node_data.user_root_node == *root_node_taffy) + self.root_node_data.get(&root_node_entity) } } @@ -279,8 +434,8 @@ mod tests { 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); } @@ -330,7 +485,7 @@ mod tests { // root node data should now exist let root_node_data = ui_surface - .get_root_node_data_exact(root_node_entity, camera_entity) + .get_root_node_data(root_node_entity) .expect("expected root node data"); assert!(ui_surface.taffy.is_root_node_data_valid(root_node_data)); @@ -342,7 +497,7 @@ mod tests { // root node data should be unaffected let root_node_data = ui_surface - .get_root_node_data_exact(root_node_entity, camera_entity) + .get_root_node_data(root_node_entity) .expect("expected root node data"); assert!(ui_surface.taffy.is_root_node_data_valid(root_node_data)); } @@ -360,22 +515,19 @@ mod tests { ui_surface.set_camera_children(camera_entity, [root_node_entity].into_iter()); assert!(ui_surface - .camera_entity_to_taffy + .camera_root_nodes .contains_key(&camera_entity)); assert!(ui_surface - .camera_entity_to_taffy - .get(&camera_entity) - .unwrap() + .root_node_data .contains_key(&root_node_entity)); - assert!(ui_surface.camera_roots.contains_key(&camera_entity)); - let root_node_data = ui_surface - .get_root_node_data_exact(root_node_entity, camera_entity) + let _root_node_data = ui_surface + .get_root_node_data(root_node_entity) .expect("expected root node data"); assert!(ui_surface - .camera_roots + .camera_root_nodes .get(&camera_entity) .unwrap() - .contains(root_node_data)); + .contains(&root_node_entity)); ui_surface.remove_camera_entities([camera_entity]); @@ -384,13 +536,13 @@ mod tests { // `camera_roots` and `camera_entity_to_taffy` should no longer contain entries for `camera_entity` assert!(!ui_surface - .camera_entity_to_taffy + .camera_root_nodes .contains_key(&camera_entity)); - return; // TODO: can't pass the test if we continue - not implemented - assert!(!ui_surface.camera_roots.contains_key(&camera_entity)); + + assert!(!ui_surface.camera_root_nodes.contains_key(&camera_entity)); // root node data should be removed - let root_node_data = ui_surface.get_root_node_data_exact(root_node_entity, camera_entity); + let root_node_data = ui_surface.get_root_node_data(root_node_entity); assert_eq!(root_node_data, None); } @@ -407,35 +559,24 @@ 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_data = ui_surface - .get_root_node_data_exact(root_node_entity, camera_entity) + .contains(&root_node_entity)); + let _root_node_data = ui_surface + .get_root_node_data(root_node_entity) .unwrap(); - assert!(ui_surface - .camera_roots - .get(&camera_entity) - .unwrap() - .contains(root_node_data)); 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 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()); @@ -497,23 +638,23 @@ 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_data = ui_surface - .get_root_node_data_exact(root_node_entity, camera_entity) + .get_root_node_data(root_node_entity) .expect("expected root node data"); assert_eq!(ui_surface.taffy.parent(child_taffy), Some(root_taffy_node)); @@ -531,22 +672,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 - 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" ); @@ -566,18 +705,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" ); @@ -603,7 +742,7 @@ mod tests { ui_surface.upsert_node(root_node_entity, &style, &DUMMY_LAYOUT_CONTEXT); - 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 28b24438fc72d3737747e5819b1433e24f7eb670 Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 21:25:56 -0400 Subject: [PATCH 15/33] Remove user_root_node field in favor of using entity_to_taffy --- crates/bevy_ui/src/layout/ui_surface.rs | 42 +++++++++++-------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index f0a61e843e131..cdb8618da0d41 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -33,9 +33,6 @@ pub struct RootNodeData { pub(super) camera_entity: Option, // The implicit "viewport" node created by Bevy pub(super) implicit_viewport_node: taffy::node::Node, - #[deprecated] - // The root (parentless) node specified by the user - pub(super) user_root_node: taffy::node::Node, } #[derive(Resource)] @@ -153,8 +150,9 @@ impl UiSurface { .remove(root_node_data.implicit_viewport_node) .unwrap(); let parent_taffy = self.entity_to_taffy.get(parent_entity).unwrap(); + let child_taffy = self.entity_to_taffy.get(target_entity).unwrap(); self.taffy - .add_child(*parent_taffy, root_node_data.user_root_node) + .add_child(*parent_taffy, *child_taffy) .unwrap(); } } @@ -171,7 +169,6 @@ impl UiSurface { RootNodeData { camera_entity: None, implicit_viewport_node, - user_root_node, } }); } @@ -262,7 +259,6 @@ without UI components as a child of an entity with UI components, results may be RootNodeData { camera_entity: Some(camera_entity), implicit_viewport_node, - user_root_node, } }); @@ -313,16 +309,17 @@ without UI components as a child of an entity with UI components, results may be // fix taffy relationships { - if let Some(parent) = self.taffy.parent(root_node_data.user_root_node) { + 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, root_node_data.user_root_node) + .remove_child(parent, taffy_node) .unwrap(); } self.taffy .add_child( root_node_data.implicit_viewport_node, - root_node_data.user_root_node, + taffy_node, ) .unwrap(); } @@ -446,14 +443,19 @@ mod tests { max_size: 1.0, }; - trait IsRootNodeDataValid { - fn is_root_node_data_valid(&self, root_node_data: &RootNodeData) -> bool; + trait HasValidRootNodeData { + fn has_valid_root_node_data(&self, root_node_entity: &Entity) -> bool; } - impl IsRootNodeDataValid for Taffy { - fn is_root_node_data_valid(&self, root_node_data: &RootNodeData) -> bool { - self.parent(root_node_data.user_root_node) - == Some(root_node_data.implicit_viewport_node) + impl HasValidRootNodeData for UiSurface { + fn has_valid_root_node_data(&self, root_node_entity: &Entity) -> bool { + let Some(&taffy_node) = self.entity_to_taffy.get(root_node_entity) else { + return false; + }; + let Some(root_node_data) = self.root_node_data.get(root_node_entity) else { + return false; + }; + self.taffy.parent(taffy_node) == Some(root_node_data.implicit_viewport_node) } } @@ -484,10 +486,7 @@ mod tests { assert_eq!(ui_surface.taffy.total_node_count(), 2); // root node data should now exist - let root_node_data = ui_surface - .get_root_node_data(root_node_entity) - .expect("expected root node data"); - assert!(ui_surface.taffy.is_root_node_data_valid(root_node_data)); + assert!(ui_surface.has_valid_root_node_data(&root_node_entity)); // test duplicate insert 2 ui_surface.upsert_node(root_node_entity, &style, &DUMMY_LAYOUT_CONTEXT); @@ -496,10 +495,7 @@ mod tests { assert_eq!(ui_surface.taffy.total_node_count(), 2); // root node data should be unaffected - let root_node_data = ui_surface - .get_root_node_data(root_node_entity) - .expect("expected root node data"); - assert!(ui_surface.taffy.is_root_node_data_valid(root_node_data)); + assert!(ui_surface.has_valid_root_node_data(&root_node_entity)); } #[test] From 94d258e649ef864399885156ebd1c9e1c064fae2 Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 21:27:19 -0400 Subject: [PATCH 16/33] Apply rustfmt in bevy_ui/layout --- crates/bevy_ui/src/layout/mod.rs | 8 ++--- crates/bevy_ui/src/layout/ui_surface.rs | 47 ++++++------------------- 2 files changed, 13 insertions(+), 42 deletions(-) diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index 460ac1fa716d3..b7585b8b365b4 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -507,9 +507,7 @@ 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.contains_key(&camera_entity)); assert_eq!(ui_surface.camera_root_nodes.len(), 1); world.despawn(ui_entity); @@ -519,9 +517,7 @@ 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.contains_key(&camera_entity)); assert!(ui_surface.camera_root_nodes.is_empty()); } diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index cdb8618da0d41..cec215cfe0474 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -78,12 +78,7 @@ impl Default for UiSurface { impl UiSurface { /// Retrieves the Taffy node associated with the given UI node entity and updates its style. /// If no associated Taffy node exists a new Taffy node is inserted into the Taffy layout. - pub fn upsert_node( - &mut self, - ui_node_entity: Entity, - style: &Style, - context: &LayoutContext, - ) { + pub fn upsert_node(&mut self, ui_node_entity: Entity, style: &Style, context: &LayoutContext) { let mut added = false; let taffy_node = *self .entity_to_taffy @@ -121,9 +116,7 @@ impl UiSurface { root_node_entities.remove(root_node_entity); } } - self.taffy - .remove(removed.implicit_viewport_node) - .unwrap(); + self.taffy.remove(removed.implicit_viewport_node).unwrap(); } } @@ -151,9 +144,7 @@ impl UiSurface { .unwrap(); let parent_taffy = self.entity_to_taffy.get(parent_entity).unwrap(); let child_taffy = self.entity_to_taffy.get(target_entity).unwrap(); - self.taffy - .add_child(*parent_taffy, *child_taffy) - .unwrap(); + self.taffy.add_child(*parent_taffy, *child_taffy).unwrap(); } } @@ -311,16 +302,11 @@ without UI components as a child of an entity with UI components, results may be { 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(); + self.taffy.remove_child(parent, taffy_node).unwrap(); } self.taffy - .add_child( - root_node_data.implicit_viewport_node, - taffy_node, - ) + .add_child(root_node_data.implicit_viewport_node, taffy_node) .unwrap(); } @@ -367,10 +353,7 @@ without UI components as a child of an entity with UI components, results may be } self.taffy - .compute_layout( - root_node_data.implicit_viewport_node, - available_space, - ) + .compute_layout(root_node_data.implicit_viewport_node, available_space) .unwrap(); } } @@ -510,12 +493,8 @@ 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)); + assert!(ui_surface.root_node_data.contains_key(&root_node_entity)); let _root_node_data = ui_surface .get_root_node_data(root_node_entity) .expect("expected root node data"); @@ -531,10 +510,8 @@ 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_root_nodes - .contains_key(&camera_entity)); - + assert!(!ui_surface.camera_root_nodes.contains_key(&camera_entity)); + assert!(!ui_surface.camera_root_nodes.contains_key(&camera_entity)); // root node data should be removed @@ -559,9 +536,7 @@ mod tests { .get(&camera_entity) .unwrap() .contains(&root_node_entity)); - let _root_node_data = ui_surface - .get_root_node_data(root_node_entity) - .unwrap(); + let _root_node_data = ui_surface.get_root_node_data(root_node_entity).unwrap(); ui_surface.remove_entities([root_node_entity]); assert!(!ui_surface.entity_to_taffy.contains_key(&root_node_entity)); From 22c3d53897b2159d69dfe9cd51a129693ccfd6b1 Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 21:46:47 -0400 Subject: [PATCH 17/33] Expand tests to cover different methods of despawn --- crates/bevy_ui/src/layout/mod.rs | 79 +++++++++++++++++++++++++++++--- 1 file changed, 72 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index b7585b8b365b4..3a78834bb579a 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -448,33 +448,98 @@ 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()); + } + + #[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()); + } + + #[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); } #[test] From ca744e03d0822e5a6b6606cb1f23a6f9c670edfc Mon Sep 17 00:00:00 2001 From: bstriker Date: Fri, 29 Mar 2024 21:47:21 -0400 Subject: [PATCH 18/33] Add support demoting root nodes into normal ui nodes --- crates/bevy_ui/src/layout/mod.rs | 45 ++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index 3a78834bb579a..59f15e51ee77e 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -4,7 +4,7 @@ use bevy_ecs::{ change_detection::{DetectChanges, DetectChangesMut}, entity::Entity, event::EventReader, - query::{With, Without}, + query::{Added, With, Without}, removal_detection::RemovedComponents, system::{Query, Res, ResMut, SystemParam}, world::Ref, @@ -73,6 +73,7 @@ pub fn ui_layout_system( style_query: Query<(Entity, Ref