From 6b03e4cf17e5431475db039f3372d506b8efe830 Mon Sep 17 00:00:00 2001 From: Panos <25664832+mechpanos@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:07:29 +0300 Subject: [PATCH] perf: hand the loader's draw-order map to the scene MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The open sequence computes it twice: [perf] draw-depth 293.5ms entries=1166328 ← loader thread … [perf] draw-depth 302.0ms entries=1166328 ← main thread [perf] update 1427.3ms message=other `DrawDepthCache` is keyed on `geometry_epoch`, the loader builds it on its temporary scene, and `PreparedOpenGeometry` carried the wire set, the tessellation memo and the block definitions across — but not this. So the receiving scene walked all 1 166 328 entities again to rebuild a map that already existed, inside the message that finishes opening the file. It travels on exactly the terms the block definitions already do: the document is the loader's own and has not been touched between the loader finishing and the install, so its order labels describe the receiving scene exactly. Only the epoch is restamped. Tested by handing a computed map back into a scene through `install_prepared_open_geometry` and asserting both that it is served as-is and that it equals what that scene recomputes from scratch — the second assertion is the one that matters, since serving a map that does not describe the document would put every entity at the wrong draw order. Co-Authored-By: Claude Opus 5 --- src/scene/mod.rs | 72 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 07d219156..a7060ac7e 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -437,6 +437,10 @@ pub struct PreparedOpenGeometry { pub tess_guard: u64, /// Loader block definitions; the receiving scene supplies its own epoch. pub block_defns: Vec<(u64, Arc)>, + /// The loader's draw-order map. Computing it walks all 1.17 M entities and + /// costs ~300 ms; the loader already paid that, and the receiving scene + /// used to pay it again on the first frame. + pub(crate) draw_depths: Option, } impl std::fmt::Debug for PreparedOpenGeometry { @@ -557,7 +561,8 @@ fn inserted_draw_depth_label(order: &[DrawDepthEntry], position: usize) -> Optio } } -struct DrawDepthCache { +#[derive(Clone)] +pub(crate) struct DrawDepthCache { epoch: u64, depths: Arc>, /// Per-block stable order labels, including deleted tombstones retained for @@ -1026,6 +1031,7 @@ pub fn prepare_open_geometry( // Taken, not cloned: the scene is discarded on the next line. let tess_memo = std::mem::take(&mut *scene.resident_tess_memo.borrow_mut()); let tess_guard = scene.resident_tess_guard.get(); + let draw_depths = scene.draw_depth_cache.borrow_mut().take(); let block_defns: Vec<(u64, Arc)> = std::mem::take(&mut *scene.block_defn_cache.borrow_mut()) .into_iter() @@ -1040,6 +1046,7 @@ pub fn prepare_open_geometry( tess_memo, tess_guard, block_defns, + draw_depths, }, ) } @@ -2250,6 +2257,15 @@ impl Scene { *self.resident_tess_memo.borrow_mut() = prepared.tess_memo; self.resident_tess_guard.set(prepared.tess_guard); } + // The draw-order map, on the same terms as the block definitions + // below: the document is the loader's own, unmodified since, so its + // order labels describe this scene exactly. Only the epoch is this + // scene's. Without this the first frame recomputed it — a walk of + // every entity in the drawing, ~300 ms, for a map that already existed. + if let Some(mut depths) = prepared.draw_depths { + depths.epoch = self.geometry_epoch; + *self.draw_depth_cache.borrow_mut() = Some(depths); + } // Same for the block definitions, stamped with this scene's block // epoch: the document has not been touched between the loader // finishing and this call, so they describe it exactly. @@ -10543,6 +10559,60 @@ mod journal_tests { assert_eq!(indexed(&s, block), scanned(&s, block), "after modify"); } + /// The loader computes the draw-order map and the receiving scene used to + /// compute it again — a walk of every entity, ~300 ms, for a map that + /// already existed. Handing it over is only sound if it describes the + /// receiving scene's document exactly, so that is what this asserts. + #[test] + fn the_handed_over_draw_depth_map_matches_a_recompute() { + use acadrust::entities::{Circle, Line}; + use acadrust::types::Vector3; + + let mut scene = Scene::new(); + scene.add_entity(EntityType::Line(Line::from_points( + Vector3::new(0.0, 0.0, 0.0), + Vector3::new(1.0, 0.0, 0.0), + ))); + scene.add_entity(EntityType::Circle(Circle::new())); + scene.add_entity(EntityType::Line(Line::from_points( + Vector3::new(2.0, 0.0, 0.0), + Vector3::new(3.0, 0.0, 0.0), + ))); + + // What the loader would hand over, and what it built it from. + let expected = scene.draw_depth_map(); + let handed_over = scene + .draw_depth_cache + .borrow_mut() + .take() + .expect("the map was just computed, so it is cached"); + + // Installing it stands in for `install_prepared_open_geometry`: the + // epoch is the receiving scene's, the contents are the loader's. + scene.bump_geometry(); + scene.install_prepared_open_geometry(PreparedOpenGeometry { + wires: Arc::new(Vec::new()), + interaction_index: None, + tess_memo: HashMap::default(), + tess_guard: 0, + block_defns: Vec::new(), + draw_depths: Some(handed_over), + }); + assert_eq!( + *scene.draw_depth_map(), + *expected, + "the handed-over map must be served as-is", + ); + + // ...and it must be the map this scene would have produced itself. + *scene.draw_depth_cache.borrow_mut() = None; + assert_eq!( + *scene.draw_depth_map(), + *expected, + "a recompute must agree with what was handed over", + ); + } + #[test] fn entity_index_incremental_matches_full_rebuild() { use acadrust::entities::Line;