diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 9642523f1..28cab5091 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -1495,6 +1495,9 @@ pub enum ViewportRefreshScope { All, } +/// `(geometry_epoch, entities per block, entities no block claims)`. +type BlockMembers = (u64, HashMap>, Vec); + pub struct Scene { pub camera: Rc>, /// View saved immediately before the latest navigation operation. ZOOM @@ -1833,6 +1836,25 @@ pub struct Scene { /// Reverse map: entity_handle → block_record_handle, built from entity_handles lists. /// Keyed by geometry_epoch. Eliminates the O(B) fallback scan in belongs_to_visible_block. entity_block_map_cache: RefCell)>>, + /// Candidate entities per block, keyed by `geometry_epoch`. + /// + /// Selecting a block's contents used to walk every entity in the document + /// and ask `belongs_to_visible_block` about each: 313 ms of a 334 ms + /// resident build for a paper sheet holding 43 entities, on a drawing with + /// 1.17 M. The membership rules only depend on owner handles and the block + /// records, so one pass answers them for every block at once. + /// + /// The second field holds entities no block claims, which + /// `belongs_to_visible_block` treats as belonging to everything — but only + /// when no block record enumerates its contents at all. + block_members_cache: RefCell>, + /// Handles of entities the quadtree cannot index — Insert, Viewport, Block, + /// BlockEnd — in document order, keyed by `geometry_epoch`. + /// + /// The quadtree path has to append these regardless of the view, and did so + /// by walking every entity in the document: ~300 ms of every resident + /// rebuild on a 1.17 M-entity drawing, to find a few thousand. + unindexable_cache: RefCell)>>, /// Distinct entity type names present in a layout, keyed by /// (geometry_epoch, layout block). The status bar asks for this on every /// frame to populate the selection-filter menu, but the answer only @@ -2077,6 +2099,8 @@ impl Scene { annotation_affects_wires: std::cell::Cell::new(None), model_extents_cache: RefCell::new(None), entity_block_map_cache: RefCell::new(None), + block_members_cache: RefCell::new(None), + unindexable_cache: RefCell::new(None), layout_type_names_cache: RefCell::new(None), dependency_index_cache: RefCell::new(None), associative_hatch_source_cache: RefCell::new(None), @@ -8885,6 +8909,12 @@ impl Scene { let sort_cache_ms = crate::perf::elapsed_ms(t_fn); let t_visible = perf.then(iced::time::Instant::now); + // Which path the selection took, and how many candidates it looked at. + // Left uninitialised on purpose: every branch below must set it, and + // the compiler enforces that. A default here is what made the first + // version of this diagnostic report a constant. + let visible_path: &str; + let mut visible_candidates = 0usize; // Phase 2.1 — quadtree-driven candidate selection. When a view // AABB exists (Model layout with a settled camera), only iterate @@ -8893,6 +8923,7 @@ impl Scene { // Paper space and the first-frame "settle" path fall back to the // full doc scan — preserving prior behaviour. let visible: Vec<&EntityType> = if let Some(local_view) = view_aabb { + visible_path = "quadtree"; let view_wcs: [f64; 4] = [ local_view[0] as f64, local_view[1] as f64, @@ -8905,6 +8936,7 @@ impl Scene { }; let mut out: Vec<&EntityType> = Vec::with_capacity(candidates.len() + unbounded.len() + 16); + visible_candidates = candidates.len() + unbounded.len(); for h in candidates { if let Some(e) = self.document.get_entity(h) { if visibility_ok(e) { @@ -8922,18 +8954,55 @@ impl Scene { } } // Inserts/Viewports/Block/BlockEnd — handled by their own paths - // (block expansion, viewport rendering); always candidates. - for e in self.document.entities() { - if is_unindexable_entity(e) && visibility_ok(e) { - out.push(e); + // (block expansion, viewport rendering); always candidates. Taken + // from the per-epoch list rather than by walking the document, + // which preserves document order because the list is built that + // way. + { + let unindexable = self.unindexable_handles(); + visible_candidates += unindexable.1.len(); + for &h in &unindexable.1 { + if let Some(e) = self.document.get_entity(h) { + if visibility_ok(e) { + out.push(e); + } + } } } out - } else { + } else if block_handle.is_null() || self.entity_block_map().is_empty() { + // Either every entity qualifies, or the drawing declares no block + // contents anywhere and `belongs_to_visible_block` turns permissive + // — the legacy-DXF case. Both need the full walk, and both must + // keep document order, which is draw order for the wire pass. + visible_path = "full-scan"; self.document .entities() .filter(|e| visibility_ok(e)) .collect() + } else { + // Ask the per-epoch membership index for this block's candidates + // instead of testing every entity in the document. For a paper + // sheet holding 43 entities in a 1.17 M-entity drawing that is the + // difference between 313 ms and nothing measurable. + // + // The index is filled by walking `document.entities()`, so each + // 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"; + let members = self.block_members(); + let (_, by_block, _) = &*members; + let claimed = by_block.get(&block_handle).map(Vec::as_slice).unwrap_or(&[]); + visible_candidates = claimed.len(); + let mut out: Vec<&EntityType> = Vec::with_capacity(claimed.len()); + for &handle in claimed { + if let Some(entity) = self.document.get_entity(handle) { + if visibility_ok(entity) { + out.push(entity); + } + } + } + out }; let visible_ms = crate::perf::elapsed_ms(t_visible); @@ -9159,7 +9228,7 @@ impl Scene { crate::perf_record!( "[perf] wires-build total={:.1}ms sort_cache={:.1} visible={:.1} blk_cache={:.1} \ build={:.1} [classify={:.1} hits={:.1} tess={:.1} materialize={:.1}] sort={:.1}({}) \ -entities={} memo_hit={} memo_miss={} wires={} memo={}", +entities={} memo_hit={} memo_miss={} wires={} memo={} visible_path={} candidates={}", crate::perf::elapsed_ms(t_fn), sort_cache_ms, visible_ms, @@ -9176,12 +9245,79 @@ entities={} memo_hit={} memo_miss={} wires={} memo={}", memo_misses, wires.len(), if memo_active { "on" } else { "off" }, + visible_path, + visible_candidates, ); } wires } /// Decide whether an entity should be drawn as direct content of `block_handle`. + /// Entities each block may contain, and the ones no block claims. + /// + /// Mirrors `belongs_to_visible_block` exactly, resolved once per + /// `geometry_epoch` for every block instead of per entity per query: + /// + /// * a non-null owner *is* the block, so the entity is that block's; + /// * a null owner falls back to the reverse map built from the block + /// records' `entity_handles`; + /// * an entity neither names nor is named by any block is "unowned", and + /// counts towards every block only when no block record enumerates its + /// contents at all — the legacy-DXF case that predicate allows. + /// Entities the quadtree cannot index, in document order, resolved once per + /// `geometry_epoch`. Membership depends only on the entity's type, so an + /// edit that does not add or remove one leaves the list unchanged. + fn unindexable_handles(&self) -> std::cell::Ref<'_, (u64, Vec)> { + { + let cache = self.unindexable_cache.borrow(); + if cache.as_ref().is_some_and(|(epoch, _)| *epoch == self.geometry_epoch) { + drop(cache); + return std::cell::Ref::map(self.unindexable_cache.borrow(), |c| { + c.as_ref().unwrap() + }); + } + } + let handles: Vec = self + .document + .entities() + .filter(|entity| is_unindexable_entity(entity)) + .map(|entity| entity.common().handle) + .collect(); + *self.unindexable_cache.borrow_mut() = Some((self.geometry_epoch, handles)); + std::cell::Ref::map(self.unindexable_cache.borrow(), |c| c.as_ref().unwrap()) + } + + fn block_members(&self) -> std::cell::Ref<'_, BlockMembers> { + { + 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() + }); + } + } + let mut members: HashMap> = HashMap::default(); + let mut unowned: Vec = Vec::new(); + { + let map = self.entity_block_map(); + for entity in self.document.entities() { + let common = entity.common(); + let owner = common.owner_handle; + if !owner.is_null() { + members.entry(owner).or_default().push(common.handle); + } else if let Some(&block) = map.get(&common.handle) { + members.entry(block).or_default().push(common.handle); + } else { + unowned.push(common.handle); + } + } + } + *self.block_members_cache.borrow_mut() = + Some((self.geometry_epoch, members, unowned)); + std::cell::Ref::map(self.block_members_cache.borrow(), |c| c.as_ref().unwrap()) + } + fn belongs_to_visible_block( &self, entity_handle: Handle, @@ -10397,6 +10533,58 @@ mod journal_tests { // Differential oracle: the incrementally-patched entity index must always // equal a from-scratch rebuild after any add / move / erase. + #[test] + fn block_member_index_matches_the_full_scan() { + use acadrust::entities::Line; + use acadrust::types::Vector3; + + fn line(a: (f64, f64), b: (f64, f64)) -> EntityType { + EntityType::Line(Line::from_points( + Vector3::new(a.0, a.1, 0.0), + Vector3::new(b.0, b.1, 0.0), + )) + } + + // What the membership index reports for `block`, in order. + fn indexed(s: &Scene, block: Handle) -> Vec { + let members = s.block_members(); + let (_, by_block, _) = &*members; + by_block.get(&block).cloned().unwrap_or_default() + } + + // What testing every entity the old way reports, in document order. + fn scanned(s: &Scene, block: Handle) -> Vec { + s.document + .entities() + .filter(|entity| { + let common = entity.common(); + s.belongs_to_visible_block(common.handle, common.owner_handle, block) + }) + .map(|entity| entity.common().handle) + .collect() + } + + let mut s = Scene::new(); + let block = s.model_space_block_handle(); + let h1 = s.add_entity(line((0.0, 0.0), (10.0, 10.0))); + let h2 = s.add_entity(line((5.0, 5.0), (20.0, 3.0))); + let _ = (h1, h2); + + // Order matters: it is the wire pass's draw order. + assert_eq!(indexed(&s, block), scanned(&s, block), "after adds"); + + let h3 = s.add_entity(line((100.0, 100.0), (140.0, 90.0))); + assert_eq!(indexed(&s, block), scanned(&s, block), "after another add"); + + // A modify must not reorder the block's contents either. + { + let mut moved = line((900.0, 900.0), (950.0, 970.0)); + moved.common_mut().handle = h3; + s.update_entity(moved); + } + assert_eq!(indexed(&s, block), scanned(&s, block), "after modify"); + } + #[test] fn entity_index_incremental_matches_full_rebuild() { use acadrust::entities::Line;