From 655c0c1676f68bd0ed6e19be3cad6a9ca44acda4 Mon Sep 17 00:00:00 2001 From: Tiernan DeFranco <126631791+TiernanDeFranco@users.noreply.github.com> Date: Wed, 6 May 2026 23:53:08 -0700 Subject: [PATCH] drop persistent world matrix cache and add overwrite test --- .../perro_runtime/src/runtime/state.rs | 4 - .../perro_runtime/src/runtime/transforms.rs | 54 +-------- .../unit/rt_ctx_nodes_transform_api_tests.rs | 105 ++++++++++++++++++ 3 files changed, 111 insertions(+), 52 deletions(-) diff --git a/perro_source/runtime_project/perro_runtime/src/runtime/state.rs b/perro_source/runtime_project/perro_runtime/src/runtime/state.rs index 5a07bc500..cac89d2b9 100644 --- a/perro_source/runtime_project/perro_runtime/src/runtime/state.rs +++ b/perro_source/runtime_project/perro_runtime/src/runtime/state.rs @@ -53,10 +53,8 @@ pub(crate) struct TransformRuntimeState { pub(crate) transform_visit_flags: Vec, pub(crate) transform_visit_indices: Vec, pub(crate) global_transform_2d: Vec, - pub(crate) global_transform_2d_valid: Vec, pub(crate) global_transform_2d_generation: Vec, pub(crate) global_transform_3d: Vec, - pub(crate) global_transform_3d_valid: Vec, pub(crate) global_transform_3d_generation: Vec, pub(crate) global_chain_scratch: Vec, pub(crate) dirty_indices_scratch: Vec, @@ -70,10 +68,8 @@ impl TransformRuntimeState { transform_visit_flags: Vec::new(), transform_visit_indices: Vec::new(), global_transform_2d: Vec::new(), - global_transform_2d_valid: Vec::new(), global_transform_2d_generation: Vec::new(), global_transform_3d: Vec::new(), - global_transform_3d_valid: Vec::new(), global_transform_3d_generation: Vec::new(), global_chain_scratch: Vec::new(), dirty_indices_scratch: Vec::new(), diff --git a/perro_source/runtime_project/perro_runtime/src/runtime/transforms.rs b/perro_source/runtime_project/perro_runtime/src/runtime/transforms.rs index b093fee41..33e09f721 100644 --- a/perro_source/runtime_project/perro_runtime/src/runtime/transforms.rs +++ b/perro_source/runtime_project/perro_runtime/src/runtime/transforms.rs @@ -68,15 +68,10 @@ impl Runtime { .global_transform_2d .resize(index + 1, Transform2D::IDENTITY); } - if self.transforms.global_transform_2d_valid.len() <= index { - self.transforms - .global_transform_2d_valid - .resize(index + 1, 0); - } if self.transforms.global_transform_2d_generation.len() <= index { self.transforms .global_transform_2d_generation - .resize(index + 1, 0); + .resize(index + 1, u32::MAX); } } @@ -87,41 +82,21 @@ impl Runtime { .global_transform_3d .resize(index + 1, Transform3D::IDENTITY); } - if self.transforms.global_transform_3d_valid.len() <= index { - self.transforms - .global_transform_3d_valid - .resize(index + 1, 0); - } if self.transforms.global_transform_3d_generation.len() <= index { self.transforms .global_transform_3d_generation - .resize(index + 1, 0); + .resize(index + 1, u32::MAX); } } fn is_global_2d_cache_valid_for_id(&self, id: NodeID) -> bool { let index = id.index() as usize; - if self - .transforms - .global_transform_2d_valid - .get(index) - .copied() - .unwrap_or(0) - == 0 - { - return false; - } - if self - .transforms + self.transforms .global_transform_2d_generation .get(index) .copied() .unwrap_or(u32::MAX) - != id.generation() - { - return false; - } - true + == id.generation() } #[inline] @@ -132,27 +107,12 @@ impl Runtime { fn is_global_3d_cache_valid_for_id(&self, id: NodeID) -> bool { let index = id.index() as usize; - if self - .transforms - .global_transform_3d_valid - .get(index) - .copied() - .unwrap_or(0) - == 0 - { - return false; - } - if self - .transforms + self.transforms .global_transform_3d_generation .get(index) .copied() .unwrap_or(u32::MAX) - != id.generation() - { - return false; - } - true + == id.generation() } #[inline] @@ -231,7 +191,6 @@ impl Runtime { }; let index = chain_id.index() as usize; self.transforms.global_transform_2d[index] = global; - self.transforms.global_transform_2d_valid[index] = 1; self.transforms.global_transform_2d_generation[index] = chain_id.generation(); self.dirty.clear_transform_dirty(chain_id, Spatial::TwoD); parent_world = world; @@ -317,7 +276,6 @@ impl Runtime { }; let index = chain_id.index() as usize; self.transforms.global_transform_3d[index] = global; - self.transforms.global_transform_3d_valid[index] = 1; self.transforms.global_transform_3d_generation[index] = chain_id.generation(); self.dirty.clear_transform_dirty(chain_id, Spatial::ThreeD); parent_world = world; diff --git a/perro_source/runtime_project/perro_runtime/tests/unit/rt_ctx_nodes_transform_api_tests.rs b/perro_source/runtime_project/perro_runtime/tests/unit/rt_ctx_nodes_transform_api_tests.rs index f27697547..a4e5dd60f 100644 --- a/perro_source/runtime_project/perro_runtime/tests/unit/rt_ctx_nodes_transform_api_tests.rs +++ b/perro_source/runtime_project/perro_runtime/tests/unit/rt_ctx_nodes_transform_api_tests.rs @@ -216,3 +216,108 @@ fn remove_node_removes_entire_subtree() { assert!(runtime.nodes.get(grandchild_id).is_none()); assert!(!runtime.remove_node(root_id)); } + +#[test] +fn removed_node_cache_does_not_leak_to_reused_slot_3d() { + let mut runtime = Runtime::new(); + + let mut original = Node3D::new(); + original.transform.position = Vector3::new(3.0, 4.0, 5.0); + let original_id = runtime + .nodes + .insert(SceneNode::new(SceneNodeData::Node3D(original))); + runtime.mark_transform_dirty_recursive(original_id); + let original_global = runtime + .get_global_transform_3d(original_id) + .expect("original global must exist"); + assert!(approx(original_global.position.x, 3.0)); + assert!(runtime.remove_node(original_id)); + + let mut reused = None; + for i in 0..2048 { + let mut node = Node3D::new(); + node.transform.position = Vector3::new(-11.0, i as f32, 7.0); + let id = runtime + .nodes + .insert(SceneNode::new(SceneNodeData::Node3D(node))); + if id.index() == original_id.index() && id.generation() != original_id.generation() { + reused = Some((id, i as f32)); + break; + } + } + + let (reused_id, expected_y) = reused.expect("slot must be reused with new generation"); + runtime.mark_transform_dirty_recursive(reused_id); + let reused_global = runtime + .get_global_transform_3d(reused_id) + .expect("reused global must exist"); + assert!(approx(reused_global.position.x, -11.0)); + assert!(approx(reused_global.position.y, expected_y)); + assert!(approx(reused_global.position.z, 7.0)); +} + +#[test] +fn removed_node_cache_does_not_leak_to_reused_slot_2d() { + let mut runtime = Runtime::new(); + + let mut original = Node2D::new(); + original.transform.position = Vector2::new(8.0, 9.0); + let original_id = runtime + .nodes + .insert(SceneNode::new(SceneNodeData::Node2D(original))); + runtime.mark_transform_dirty_recursive(original_id); + let original_global = runtime + .get_global_transform_2d(original_id) + .expect("original global must exist"); + assert!(approx(original_global.position.x, 8.0)); + assert!(runtime.remove_node(original_id)); + + let mut reused = None; + for i in 0..2048 { + let mut node = Node2D::new(); + node.transform.position = Vector2::new(-2.0, i as f32 * 2.0); + let id = runtime + .nodes + .insert(SceneNode::new(SceneNodeData::Node2D(node))); + if id.index() == original_id.index() && id.generation() != original_id.generation() { + reused = Some((id, i as f32 * 2.0)); + break; + } + } + + let (reused_id, expected_y) = reused.expect("slot must be reused with new generation"); + runtime.mark_transform_dirty_recursive(reused_id); + let reused_global = runtime + .get_global_transform_2d(reused_id) + .expect("reused global must exist"); + assert!(approx(reused_global.position.x, -2.0)); + assert!(approx(reused_global.position.y, expected_y)); +} + +#[test] +fn recompute_overwrites_old_cached_global_transform_3d() { + let mut runtime = Runtime::new(); + let mut node = Node3D::new(); + node.transform.position = Vector3::new(1.0, 2.0, 3.0); + let id = runtime + .nodes + .insert(SceneNode::new(SceneNodeData::Node3D(node))); + + runtime.mark_transform_dirty_recursive(id); + let first = runtime + .get_global_transform_3d(id) + .expect("first global must exist"); + assert!(approx(first.position.x, 1.0)); + + let _ = runtime.with_base_node_mut::(id, |node| { + node.transform.position = Vector3::new(9.0, 8.0, 7.0); + }); + runtime.mark_transform_dirty_recursive(id); + + let second = runtime + .get_global_transform_3d(id) + .expect("second global must exist"); + assert!(approx(second.position.x, 9.0)); + assert!(approx(second.position.y, 8.0)); + assert!(approx(second.position.z, 7.0)); +}