From 25b0760913e966acefb4dfaf1f58e9ba470dd21e Mon Sep 17 00:00:00 2001 From: Nova Date: Sun, 21 Sep 2025 04:17:45 -0700 Subject: [PATCH] refactor(spatial): general improvements and efficiency --- src/nodes/mod.rs | 7 +-- src/nodes/spatial/mod.rs | 106 +++++++++++++++++--------------------- src/nodes/spatial/zone.rs | 8 +-- 3 files changed, 53 insertions(+), 68 deletions(-) diff --git a/src/nodes/mod.rs b/src/nodes/mod.rs index 11f62e4..acddf66 100644 --- a/src/nodes/mod.rs +++ b/src/nodes/mod.rs @@ -126,12 +126,7 @@ impl Node { pub fn enabled(&self) -> bool { self.enabled.load(Ordering::Relaxed) && if let Ok(spatial) = self.get_aspect::() { - spatial - .global_transform() - .to_scale_rotation_translation() - .0 - .length_squared() - > 0.0 + spatial.visible() } else { true } diff --git a/src/nodes/spatial/mod.rs b/src/nodes/spatial/mod.rs index 0c4900b..b59678e 100644 --- a/src/nodes/spatial/mod.rs +++ b/src/nodes/spatial/mod.rs @@ -16,10 +16,9 @@ use bevy::render::primitives::Aabb; use color_eyre::eyre::OptionExt; use glam::{Mat4, Quat, Vec3, vec3a}; use mint::Vector3; -use parking_lot::Mutex; +use parking_lot::{Mutex, RwLock}; use rustc_hash::FxHashMap; use std::fmt::Debug; -use std::sync::atomic::Ordering; use std::sync::{Arc, OnceLock, Weak}; use std::{f32, ptr}; @@ -44,7 +43,7 @@ fn spawn_spatial_nodes(mut cmds: Commands) { for spatial in SPATIAL_REGISTRY .get_valid_contents() .into_iter() - .filter(|v| v.entity.lock().is_none()) + .filter(|v| v.entity.read().is_none()) { let entity = cmds .spawn((SpatialNode(Arc::downgrade(&spatial)), Name::new("Spatial"))) @@ -64,7 +63,7 @@ fn update_spatial_node_parenting( let Some(parent_entity) = spatial .get_parent() - .map(|v| v.entity.lock().as_ref().map(|v| v.0)) + .map(|v| v.entity.read().as_ref().map(|v| v.0)) else { continue; }; @@ -95,26 +94,11 @@ fn update_spatial_nodes(mut query: Query<(&mut BevyTransform, &SpatialNode, &mut let Some(spatial) = spatial_node.0.upgrade() else { return; }; - if spatial - .node() - .is_some_and(|v| !v.enabled.load(Ordering::Relaxed)) - { - if !matches!(*vis, Visibility::Hidden) { - *vis = Visibility::Hidden; - } - return; - } - let mat4 = - debug_span!("getting global transform").in_scope(|| spatial.global_transform()); - let (scale, _, _) = mat4.to_scale_rotation_translation(); - match (*vis, scale == Vec3::ZERO) { - (Visibility::Inherited | Visibility::Visible, true) => { - *vis = Visibility::Hidden; - } - (Visibility::Hidden, false) => { - *vis = Visibility::Inherited; - } - _ => {} + // Set visibility based on node enabled state + if spatial.node().is_some_and(|v| v.enabled()) { + *vis = Visibility::Inherited; + } else { + *vis = Visibility::Hidden; } *transform = BevyTransform::from_matrix(spatial.local_transform()); }); @@ -160,12 +144,11 @@ static ZONEABLE_REGISTRY: Registry = Registry::new(); pub struct Spatial { pub node: Weak, - entity: Mutex>, - parent: Mutex>>, - old_parent: Mutex>>, - transform: Mutex, - cached_global_transform: Mutex>, - zone: Mutex>, + entity: RwLock>, + parent: RwLock>>, + old_parent: RwLock>>, + transform: RwLock, + zone: RwLock>, children: Registry, pub bounding_box_calc: OnceLock Aabb>, } @@ -174,18 +157,17 @@ impl Spatial { pub fn new(node: Weak, parent: Option>, transform: Mat4) -> Arc { SPATIAL_REGISTRY.add(Spatial { node, - entity: Mutex::new(None), - parent: Mutex::new(parent), - old_parent: Mutex::new(None), - transform: Mutex::new(transform), - cached_global_transform: Mutex::new(None), - zone: Mutex::new(Weak::new()), + entity: RwLock::new(None), + parent: RwLock::new(parent), + old_parent: RwLock::new(None), + transform: RwLock::new(transform), + zone: RwLock::new(Weak::new()), children: Registry::new(), bounding_box_calc: OnceLock::default(), }) } pub fn set_entity(&self, entity: Entity) { - self.entity.lock().replace(entity.into()); + self.entity.write().replace(entity.into()); } pub fn add_to( node: &Arc, @@ -239,28 +221,37 @@ impl Spatial { bounds } - fn clear_cached_global_transform(&self) { - self.cached_global_transform.lock().take(); - for child in self.children.get_valid_contents() { - child.clear_cached_global_transform(); - } - } pub fn local_transform(&self) -> Mat4 { - *self.transform.lock() + *self.transform.read() + } + + /// Check if this node or any ancestor has zero scale (for visibility culling) + pub fn visible(&self) -> bool { + // Check parent chain + if let Some(parent) = self.get_parent() + && !parent.visible() + { + return false; + } + + // Check our own scale by looking at matrix column lengths + let mat = self.local_transform(); + let x_scale = mat.x_axis.length_squared(); + let y_scale = mat.y_axis.length_squared(); + let z_scale = mat.z_axis.length_squared(); + + !(x_scale == 0.0 || y_scale == 0.0 || z_scale == 0.0) } pub fn global_transform(&self) -> Mat4 { - *self.cached_global_transform.lock().get_or_insert_with(|| { - let parent_transform = self - .get_parent() - .as_deref() - .map(Self::global_transform) - .unwrap_or_default(); - parent_transform * self.local_transform() - }) + let parent_transform = self + .get_parent() + .as_deref() + .map(Self::global_transform) + .unwrap_or_default(); + parent_transform * self.local_transform() } pub fn set_local_transform(&self, transform: Mat4) { - self.clear_cached_global_transform(); - *self.transform.lock() = transform; + *self.transform.write() = transform; } pub fn set_local_transform_components( &self, @@ -321,16 +312,15 @@ impl Spatial { } fn get_parent(&self) -> Option> { - self.parent.lock().clone() + self.parent.read().clone() } fn set_parent(self: &Arc, new_parent: &Arc) { if let Some(parent) = self.get_parent() { parent.children.remove(self); } new_parent.children.add_raw(self); - self.clear_cached_global_transform(); - *self.parent.lock() = Some(new_parent.clone()); + *self.parent.write() = Some(new_parent.clone()); } pub fn set_spatial_parent(self: &Arc, parent: &Arc) -> Result<()> { @@ -354,7 +344,7 @@ impl Spatial { pub(self) fn zone_distance(&self) -> f32 { self.zone - .lock() + .read() .upgrade() .map(|zone| zone.field.clone()) .map(|field| field.distance(self, vec3a(0.0, 0.0, 0.0))) diff --git a/src/nodes/spatial/zone.rs b/src/nodes/spatial/zone.rs index a4a75a3..6d38bce 100644 --- a/src/nodes/spatial/zone.rs +++ b/src/nodes/spatial/zone.rs @@ -21,8 +21,8 @@ pub fn capture(spatial: &Arc, zone: &Arc) { } release(spatial); - *spatial.old_parent.lock() = spatial.get_parent(); - *spatial.zone.lock() = Arc::downgrade(zone); + *spatial.old_parent.write() = spatial.get_parent(); + *spatial.zone.write() = Arc::downgrade(zone); let Some(zone_node) = zone.spatial.node.upgrade() else { return; }; @@ -45,11 +45,11 @@ pub fn release(spatial: &Spatial) { }; let spatial = spatial_node.get_aspect::().unwrap(); - let Some(old_parent) = spatial.old_parent.lock().take() else { + let Some(old_parent) = spatial.old_parent.read().clone() else { return; }; let _ = spatial.set_spatial_parent_in_place(&old_parent); - let mut spatial_zone = spatial.zone.lock(); + let mut spatial_zone = spatial.zone.write(); if let Some(spatial_zone) = spatial_zone.upgrade() { spatial_zone.captured.remove_aspect(spatial.as_ref());