diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 68d04fb91..0faa7ab2d 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, }, ) } @@ -2272,6 +2279,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. @@ -8842,6 +8858,32 @@ impl Scene { frozen_layers: Option<&HashSet>, annotation_scale_handle: Option, all_visible: bool, + ) -> bool { + let layer = self.document.layers.get(&e.common().layer); + self.resident_entity_visible_with_layer( + e, + block_handle, + frozen_layers, + annotation_scale_handle, + all_visible, + layer, + ) + } + + /// `resident_entity_visible` with the layer already resolved. + /// + /// `Table::get` normalises the name with `to_uppercase`, which allocates, + /// and the selection calls this once per candidate — 444 938 allocations on + /// the reproducer, ~440 ms, to look up a few hundred distinct layers. A + /// caller in a loop resolves each name once and passes the result here. + fn resident_entity_visible_with_layer( + &self, + e: &EntityType, + block_handle: Handle, + frozen_layers: Option<&HashSet>, + annotation_scale_handle: Option, + all_visible: bool, + layer: Option<&acadrust::tables::layer::Layer>, ) -> bool { let c = e.common(); if c.invisible { @@ -8855,7 +8897,6 @@ impl Scene { if matches!(e, EntityType::Block(_) | EntityType::BlockEnd(_)) { return false; } - let layer = self.document.layers.get(&c.layer); if layer .map(|l| l.flags.off || l.flags.frozen) .unwrap_or(false) @@ -8882,8 +8923,8 @@ impl Scene { self.belongs_to_visible_block(c.handle, c.owner_handle, block_handle) } - fn wires_for_block_culled( - &self, + fn wires_for_block_culled<'doc>( + &'doc self, block_handle: Handle, view_aabb: Option<[f32; 4]>, wpp: Option, @@ -8937,13 +8978,45 @@ impl Scene { // Visibility test reused by both paths below — and by the resident // incremental patch, so a changed entity is included/excluded exactly as // a from-scratch build would (no divergence). - let visibility_ok = |e: &EntityType| { - self.resident_entity_visible( + // `Table::get` normalises with `to_uppercase`, so every layer lookup + // allocates. The selection asks once per candidate and a drawing has a + // few hundred layers, so the name is resolved once and remembered. + // Keys borrow the entity's own layer string, which lives in the + // document for the whole call. + type LayerRef<'a> = Option<&'a acadrust::tables::layer::Layer>; + let layer_of: RefCell>> = + RefCell::new(rustc_hash::FxHashMap::default()); + // Kill switch, so one binary can measure both paths on one drawing. + // Same shape as `OCS_NO_RESIDENT_MEMO`. + fn layer_memo_enabled() -> bool { + static EN: std::sync::OnceLock = std::sync::OnceLock::new(); + *EN.get_or_init(|| std::env::var("OCS_NO_LAYER_MEMO").is_err()) + } + let memo_on = layer_memo_enabled(); + let visibility_ok = |e: &'doc EntityType| { + let name = e.common().layer.as_str(); + if !memo_on { + return self.resident_entity_visible( + e, + block_handle, + frozen_layers, + annotation_scale_handle, + all_visible, + ); + } + let layer = match layer_of.borrow_mut().entry(name) { + std::collections::hash_map::Entry::Occupied(hit) => *hit.get(), + std::collections::hash_map::Entry::Vacant(slot) => { + *slot.insert(self.document.layers.get(name)) + } + }; + self.resident_entity_visible_with_layer( e, block_handle, frozen_layers, annotation_scale_handle, all_visible, + layer, ) }; @@ -8955,6 +9028,12 @@ impl Scene { // version of this diagnostic report a constant. let visible_path: &str; let mut visible_candidates: usize; + // `visible` costs ~400 ms over 444 937 candidates, and two things in + // the loop could own that: resolving each handle through the document's + // hash index, or the predicate, which looks a layer up **by name** — + // a string hash per entity. Timed separately, under PERF only. + let mut visible_index_ms = 0.0f64; + let mut visible_probe_ms = 0.0f64; // Phase 2.1 — quadtree-driven candidate selection. When a view // AABB exists (Model layout with a settled camera), only iterate @@ -9031,7 +9110,13 @@ impl Scene { // block's list is already in document order and the wires come out // in exactly the order the full scan produced. visible_path = "block-index"; + // The index itself: rebuilt by walking the whole document once per + // `geometry_epoch`, and the epoch moves on every edit. Timed + // separately because it is inside `visible` and is not per-entity + // work, so no per-candidate breakdown can ever account for it. + let t_members = perf.then(iced::time::Instant::now); let members = self.block_members(); + visible_index_ms = crate::perf::elapsed_ms(t_members); let (_, by_block) = &*members; let claimed = by_block.get(&block_handle).map(Vec::as_slice).unwrap_or(&[]); visible_candidates = claimed.len(); @@ -9043,6 +9128,24 @@ impl Scene { } } } + if perf { + // Resolve-only pass, for the share of `visible` that is the + // document's hash index rather than the predicate. Repeated + // rather than clocked per iteration, and PERF only. + // + // It runs after the real loop, so it reads a warm cache and + // undercounts what the first traversal actually paid. Treat it + // as a floor, not as the cost. + let t_probe = iced::time::Instant::now(); + let mut resolved = 0usize; + for &handle in claimed { + if self.document.get_entity(handle).is_some() { + resolved += 1; + } + } + std::hint::black_box(resolved); + visible_probe_ms = t_probe.elapsed().as_secs_f64() * 1000.0; + } out }; @@ -9287,7 +9390,8 @@ impl Scene { "[perf] wires-build total={:.1}ms sort_cache={:.1} visible={:.1} blk_cache={:.1} colors={:.1} \ build={:.1} [classify={:.1} hits={:.1} tess={:.1} materialize={:.1}] sort={:.1}({}) \ entities={} memo_hit={} memo_miss={} wires={} memo={} visible_path={} candidates={} \ -guard={:016x} guard_stale={} avp={} anno={:.4} anno_h={} all_vis={} sdf_gen={}", +guard={:016x} guard_stale={} avp={} anno={:.4} anno_h={} all_vis={} sdf_gen={} \ +vis_index={:.1} visible_probe={:.1}", crate::perf::elapsed_ms(t_fn), sort_cache_ms, visible_ms, @@ -9314,6 +9418,8 @@ guard={:016x} guard_stale={} avp={} anno={:.4} anno_h={} all_vis={} sdf_gen={}", annotation_scale_handle.map(|h| h.value()).unwrap_or(0), all_visible, crate::scene::text::sdf_atlas::generation(), + visible_index_ms, + visible_probe_ms, ); } wires @@ -9341,14 +9447,59 @@ guard={:016x} guard_stale={} avp={} anno={:.4} anno_h={} all_vis={} sdf_gen={}", } /// Resolve owner handles and block-record membership once per geometry epoch. + /// + /// The walk visits every entity in the drawing and the epoch moves on every + /// edit, so it used to run again for each one — 19 ms of a 55 ms selection + /// on a 586 k-entity drawing, to add a single handle to a single list. + /// + /// A lone addition is folded in instead. Only a lone one: entities are + /// appended to the document, so a single new handle belongs at the end of + /// its block's list, while `replay_since` returns several changes in no + /// particular order and each list is in document order because that is + /// draw order. Anything else — a removal, a modification that could move an + /// entity between blocks, more than one change — takes the full walk. fn block_members(&self) -> std::cell::Ref<'_, BlockMembers> { + let mut cached_epoch = None; { let cache = self.block_members_cache.borrow(); - if cache.as_ref().is_some_and(|(epoch, _)| *epoch == self.geometry_epoch) { - drop(cache); - return std::cell::Ref::map(self.block_members_cache.borrow(), |c| { - c.as_ref().unwrap() + if let Some((epoch, _)) = cache.as_ref() { + if *epoch == self.geometry_epoch { + drop(cache); + return std::cell::Ref::map(self.block_members_cache.borrow(), |c| { + c.as_ref().unwrap() + }); + } + cached_epoch = Some(*epoch); + } + } + + if let Some(since) = cached_epoch { + let lone_add = self.replay_since(since).filter(|deltas| { + deltas.len() == 1 && deltas[0].1 == ChangeKind::Added + }); + if let Some(deltas) = lone_add { + let handle = deltas[0].0; + // Resolve the owner before touching the cache: both lookups + // borrow, and `entity_block_map` is its own `RefCell`. + let owner = self.document.get_entity(handle).and_then(|entity| { + let owner = entity.common().owner_handle; + if !owner.is_null() { + Some(owner) + } else { + self.entity_block_map().get(&handle).copied() + } }); + let mut cache = self.block_members_cache.borrow_mut(); + if let Some((epoch, members)) = cache.as_mut() { + if let Some(owner) = owner { + members.entry(owner).or_default().push(handle); + } + *epoch = self.geometry_epoch; + drop(cache); + return std::cell::Ref::map(self.block_members_cache.borrow(), |c| { + c.as_ref().unwrap() + }); + } } } let mut members: HashMap> = HashMap::default(); @@ -10523,6 +10674,42 @@ mod journal_tests { // Differential oracle: the incrementally-patched entity index must always // equal a from-scratch rebuild after any add / move / erase. + #[test] + /// Folding a lone addition must land the handle where the full walk would + /// have put it — at the end of its block's list, because that list is in + /// document order and document order is draw order. + #[test] + fn folding_one_addition_matches_the_full_scan() { + use acadrust::entities::Line; + use acadrust::types::Vector3; + + let mut scene = Scene::new(); + fn add(scene: &mut Scene, x: f64) -> Handle { + scene.add_entity(EntityType::Line(Line::from_points( + Vector3::new(x, 0.0, 0.0), + Vector3::new(x + 1.0, 0.0, 0.0), + ))) + } + add(&mut scene, 0.0); + add(&mut scene, 1.0); + // Build the index, then add one more so the next call folds. + let block = scene.model_space_block_handle(); + let before = scene.block_members().1.get(&block).cloned().unwrap_or_default(); + let third = add(&mut scene, 2.0); + let folded = scene.block_members().1.get(&block).cloned().unwrap_or_default(); + + scene.block_members_cache.borrow_mut().take(); + let rebuilt = scene.block_members().1.get(&block).cloned().unwrap_or_default(); + + assert_eq!(folded, rebuilt, "the folded index must equal a full rebuild"); + assert_eq!( + folded.last(), + Some(&third), + "an appended entity belongs at the end of its block's list", + ); + assert_eq!(folded.len(), before.len() + 1); + } + #[test] fn block_member_index_matches_the_full_scan() { use acadrust::entities::Line; @@ -10575,6 +10762,119 @@ mod journal_tests { assert_eq!(indexed(&s, block), scanned(&s, block), "after modify"); } + /// The layer memo keys on the entity's **raw** layer string, while + /// `Table::get` normalises with `to_uppercase`. Two spellings of one layer + /// therefore take two memo slots, and both must resolve to that same layer + /// — otherwise the memo would hide geometry the unmemoized path shows. + /// + /// Covers the property the memo rests on (the table is case-insensitive) + /// and the predicate that reads it, not the memo's own bookkeeping. + #[test] + fn the_layer_memo_respects_case_insensitive_layer_names() { + use acadrust::entities::Line; + use acadrust::tables::layer::Layer; + use acadrust::types::Vector3; + + let mut scene = Scene::new(); + let mut hidden = Layer::new("Walls"); + hidden.flags.off = true; + scene.document.layers.add_or_replace(hidden); + + let mut on_layer = |layer: &str| { + let mut line = Line::from_points( + Vector3::new(0.0, 0.0, 0.0), + Vector3::new(1.0, 0.0, 0.0), + ); + line.common.layer = layer.to_string(); + scene.add_entity(EntityType::Line(line)) + }; + // Three spellings of the same, switched-off layer, plus one that is + // genuinely a different layer and stays visible. + on_layer("Walls"); + on_layer("WALLS"); + on_layer("walls"); + on_layer("Doors"); + + let block = scene.model_space_block_handle(); + let visible: Vec<_> = scene + .document + .entities() + .filter(|e| scene.resident_entity_visible(e, block, None, None, true)) + .map(|e| e.common().layer.clone()) + .collect(); + assert_eq!( + visible, + vec!["Doors".to_string()], + "every spelling of a switched-off layer must be hidden", + ); + + // The memo takes one slot per spelling; each must find the same layer. + let resolved: Vec<_> = ["Walls", "WALLS", "walls"] + .iter() + .map(|name| scene.document.layers.get(name).map(|l| l.name.clone())) + .collect(); + assert_eq!( + resolved, + vec![Some("Walls".to_string()); 3], + "the table is case-insensitive, which is what lets the memo key on \ + the raw name", + ); + } + + /// 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;