Fix Bvh::optimize_incremental non-determinism (#403)

* Fix Bvh::optimize_incremental being non-deterministic when restored from a serialized snapshot, due to significant state being stored in BvhWorkspace which cannot be serialized (#402)

* Fix typo in BvhIncrementalOptimizationState description (#403)

* chore: add comment for the new optimization field

* cargo fmt

---------

Co-authored-by: Sébastien Crozet <sebcrozet@dimforge.com>

Author: raysplaceinspace

src/partitioning/bvh/bvh_optimize.rs

@@ -240,15 +240,16 @@ impl Bvh {
         }
 
         workspace.rebuild_leaves.clear();
-        workspace.rebuild_frame_index = workspace.rebuild_frame_index.overflowing_add(1).0;
-        let config = self.optimization_config(workspace.rebuild_frame_index);
+        self.optimization.rebuild_frame_index =
+            self.optimization.rebuild_frame_index.overflowing_add(1).0;
+        let config = self.optimization_config(self.optimization.rebuild_frame_index);
 
         /*
          * Subtree optimizations.
          */
         // let t0 = core::time::Instant::now();
         let num_leaves = self.nodes[0].leaf_count();
-        let mut start_index = workspace.rebuild_start_index;
+        let mut start_index = self.optimization.rebuild_start_index;
 
         // println!("Max candidate leaf count = {}", max_candidate_leaf_count);
         self.find_optimization_roots(
@@ -266,7 +267,7 @@ impl Bvh {
             //       to reach the target subtree count.
         }
 
-        workspace.rebuild_start_index = start_index;
+        self.optimization.rebuild_start_index = start_index;
 
         // println!(
         //     "Num refinement candidates: {}, list: {:?}",
@@ -522,6 +523,19 @@ impl Bvh {
     }
 }
 
+/// The optimization state used by `Bvh::optimize_incremental`.
+/// This allows each call to `optimize_incremental` to continue from where the last one left off.
+#[derive(Clone, Debug, Default)]
+#[cfg_attr(feature = "serde-serialize", derive(Serialize, Deserialize))]
+#[cfg_attr(
+    feature = "rkyv",
+    derive(rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)
+)]
+pub(super) struct BvhIncrementalOptimizationState {
+    pub(super) rebuild_frame_index: u32,
+    pub(super) rebuild_start_index: u32,
+}
+
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 enum RootOptimizationMode {
     PriorityQueue,

src/partitioning/bvh/bvh_tree.rs

@@ -1,3 +1,4 @@
+use super::bvh_optimize::BvhIncrementalOptimizationState;
 use super::BvhOptimizationHeapEntry;
 use crate::bounding_volume::{Aabb, BoundingVolume};
 use crate::math::{Real, Vector};
@@ -136,8 +137,6 @@ pub enum BvhBuildStrategy {
 pub struct BvhWorkspace {
     pub(super) refit_tmp: BvhNodeVec,
     pub(super) rebuild_leaves: Vec<BvhNode>,
-    pub(super) rebuild_frame_index: u32,
-    pub(super) rebuild_start_index: u32,
     pub(super) optimization_roots: Vec<u32>,
     pub(super) queue: BinaryHeap<BvhOptimizationHeapEntry>,
     pub(super) dequeue: VecDeque<u32>,
@@ -1752,6 +1751,9 @@ pub struct Bvh {
     // We don’t store this in `Self::nodes` since it’s only useful for node removal.
     pub(super) parents: Vec<BvhNodeIndex>,
     pub(super) leaf_node_indices: VecMap<BvhNodeIndex>,
+    // NOTE: this cannot be in the workspace as we need this to survive serialization/deserialization
+    //       to maintain determinism.
+    pub(super) optimization: BvhIncrementalOptimizationState,
 }
 
 impl Bvh {
@@ -2186,6 +2188,7 @@ impl Bvh {
             nodes,
             parents,
             leaf_node_indices,
+            optimization: _,
         } = self;
         nodes.capacity() * size_of::<BvhNodeWide>()
             + parents.capacity() * size_of::<BvhNodeIndex>()