From 1d882fce9e644a6b0aa1fe82ede6c7405ca50efa Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Tue, 24 Mar 2026 23:51:49 +0000 Subject: [PATCH 1/2] fix: clean up orphaned BVH nodes when remove creates a partial root When multiple removes occur without an intervening refit, orphaned wide nodes accumulate in the nodes vector. If the root is then reduced to a single leaf (partial root), those orphaned nodes remain, causing optimize_incremental to operate on a corrupt tree and eventually crash. Truncate nodes and parents to 1 after creating a partial root, since only node[0] is reachable in that state. Co-Authored-By: Claude Opus 4.6 --- src/partitioning/bvh/bvh_tree.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/partitioning/bvh/bvh_tree.rs b/src/partitioning/bvh/bvh_tree.rs index 17ddbf3a..07e0893e 100644 --- a/src/partitioning/bvh/bvh_tree.rs +++ b/src/partitioning/bvh/bvh_tree.rs @@ -2383,6 +2383,14 @@ impl Bvh { // Now we can just clear the right leaf. self.nodes[0].right = BvhNode::zeros(); + + // Clean up orphaned nodes. With a partial root, only node[0] is + // reachable. Previous removes may have left orphaned wide nodes + // that were waiting for refit to compact them. If we don't truncate + // here, the tree appears as a single-leaf tree with unreachable + // nodes, which corrupts optimize_incremental. + self.nodes.truncate(1); + self.parents.truncate(1); } else { // The sibling isn’t a leaf. It becomes the new root at index 0. self.nodes[0] = self.nodes[self.nodes[sibling].children as usize]; From 763d2a65d20afa0ce1878a1008aa9e34e48765a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Crozet?= Date: Thu, 28 May 2026 19:09:47 +0200 Subject: [PATCH 2/2] chore: add non-regression test --- src/partitioning/bvh/bvh_tests.rs | 39 ++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/src/partitioning/bvh/bvh_tests.rs b/src/partitioning/bvh/bvh_tests.rs index 143d3170..404990bd 100644 --- a/src/partitioning/bvh/bvh_tests.rs +++ b/src/partitioning/bvh/bvh_tests.rs @@ -1,6 +1,8 @@ use crate::bounding_volume::Aabb; use crate::math::{Real, Vector}; -use crate::partitioning::{Bvh, BvhBuildStrategy, BvhNode, BvhNodeIndex, TraversalAction}; +use crate::partitioning::{ + Bvh, BvhBuildStrategy, BvhNode, BvhNodeIndex, BvhWorkspace, TraversalAction, +}; fn make_test_aabb(i: usize) -> Aabb { Aabb::from_half_extents(Vector::splat(i as Real).into(), Vector::splat(1.0)) @@ -242,3 +244,38 @@ fn bvh_build_and_removal() { } } } + +#[test] +fn bvh_remove_to_partial_root_then_optimize() { + // Regression test for the bug reported in #409 where `Bvh::remove` would leave orphaned + // wide nodes in `self.nodes`/`self.parents` after reducing the tree to a + // partial root (a single surviving leaf at node 0). Earlier removals on a + // tree with more than one wide node would intentionally leave orphans for + // the next `refit` to compact, but if a partial root was created before + // any refit, those orphans remained reachable from `self.nodes` and + // `optimize_incremental` would walk them as if they were live, crashing + // on the corrupt structure. + // + // We pick enough leaves to ensure at least one orphan-leaving removal + // (`wide_node_index != 0`) before the final partial-root removal. + let leaves: std::vec::Vec<_> = (0..10).map(make_test_aabb).collect(); + let mut bvh = Bvh::from_leaves(BvhBuildStrategy::Binned, &leaves); + + // Remove all but the last leaf, without ever calling refit in between. + for i in 0..(leaves.len() as u32 - 1) { + bvh.remove(i); + } + + // After the final remove, the tree should be a partial root with exactly + // one surviving leaf, and no orphaned wide nodes left over. + assert_eq!(bvh.leaf_count(), 1); + + // Without the fix this call walks the orphan-laden tree as if it were + // live and ends up corrupting/crashing on the partial root. + let mut workspace = BvhWorkspace::default(); + bvh.optimize_incremental(&mut workspace); + + assert_eq!(bvh.nodes.len(), 1); + assert_eq!(bvh.parents.len(), 1); + bvh.assert_well_formed(); +}