From efcd73727fbacd758836135c497ff9c197de67a0 Mon Sep 17 00:00:00 2001 From: Panos <25664832+mechpanos@users.noreply.github.com> Date: Sat, 5 Sep 2026 10:48:47 +0300 Subject: [PATCH 1/4] perf: select a block's contents from an index, not by scanning the drawing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switching to a paper layout took 6.9 s, of which the switch itself was 8.66 ms; the rest landed in the next frame. Part of it was this: wires-build total=334.0ms visible=313.3 entities=43 wires=43 313 ms of a 334 ms resident build, to gather the 43 entities of a paper sheet — because the selection walked all 1 166 357 entities in the document and asked `belongs_to_visible_block` about each one. The same cost is paid on every resident rebuild of any block, so it also shows up as ~315 ms in every model-space rebuild. Those membership rules depend only on owner handles and the block records, so one pass answers them for every block at once. `block_members` resolves them per `geometry_epoch`, mirroring the predicate exactly: a non-null owner is the block; a null owner falls back to the reverse map built from the block records; an entity neither naming nor named by any block is unowned. Two things kept deliberately: * The index is filled by walking `document.entities()`, so each block's list is already in document order. That is draw order for the wire pass, where submission order decides alpha blending, so the wires come out in exactly the order the full scan produced. * The permissive legacy-DXF case — no block record enumerates anything, and unowned entities count towards every block — still takes the full walk. Splicing unowned entities onto a block's list would have changed their relative order. Adds a test asserting the index and the old scan agree, in order, across adds and modifies. Co-Authored-By: Claude Opus 5 --- src/scene/mod.rs | 137 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 136 insertions(+), 1 deletion(-) diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 9642523f1..8c93fa5bf 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,18 @@ 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>, /// 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 +2092,7 @@ 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), layout_type_names_cache: RefCell::new(None), dependency_index_cache: RefCell::new(None), associative_hatch_source_cache: RefCell::new(None), @@ -8929,11 +8945,36 @@ impl Scene { } } 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. 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. + let members = self.block_members(); + let (_, by_block, _) = &*members; + let claimed = by_block.get(&block_handle).map(Vec::as_slice).unwrap_or(&[]); + 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); @@ -9182,6 +9223,48 @@ entities={} memo_hit={} memo_miss={} wires={} memo={}", } /// 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. + 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 +10480,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; From 67ea5fa2dc6695ee01355cbbe46e6386e946144a Mon Sep 17 00:00:00 2001 From: Panos <25664832+mechpanos@users.noreply.github.com> Date: Sat, 5 Sep 2026 11:02:00 +0300 Subject: [PATCH 2/4] perf: report which path the visible-entity selection took MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The block member index cut the paper sheet's selection from 313 ms to 112 ms but left model space at 296 ms, which is not what a shared per-epoch index should do — the second call in a frame ought to be free. Either the drawing falls into the permissive legacy-DXF case, where the predicate still needs the full walk, or the index is being rebuilt between the two calls. The wires-build line now says which path ran and how many candidates it considered, so one run settles it. Co-Authored-By: Claude Opus 5 --- src/scene/mod.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 8c93fa5bf..9c38d148c 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -8901,6 +8901,9 @@ 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. + let mut visible_path = "quadtree"; + 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 @@ -9200,7 +9203,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, @@ -9217,6 +9220,8 @@ entities={} memo_hit={} memo_miss={} wires={} memo={}", memo_misses, wires.len(), if memo_active { "on" } else { "off" }, + visible_path, + visible_candidates, ); } wires From 1f14ba35d17431d7f105b0269eb4c53c6e029516 Mon Sep 17 00:00:00 2001 From: Panos <25664832+mechpanos@users.noreply.github.com> Date: Sat, 5 Sep 2026 11:22:49 +0300 Subject: [PATCH 3/4] perf: index the entities the quadtree cannot hold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After querying the quadtree and appending the unbounded entities, the quadtree path has to add the ones the tree cannot index — Insert, Viewport, Block, BlockEnd — and did so by walking every entity in the document. On a 1.17 M-entity drawing that is a full walk per resident rebuild to find a few thousand, which defeats the point of having a spatial index on that path at all. Membership depends only on the entity's type, so it is resolved once per `geometry_epoch` and reused. The list is built by walking `document.entities()`, so it stays in document order and the wires come out in the order the loop produced before — which matters, because that is draw order for a pass that alpha-blends. This is the same treatment the previous commit gave the block-membership path. Which of the two a layout switch actually takes is still unmeasured: the diagnostic added to report it was defaulted rather than assigned, so its `visible_path=quadtree` output was a constant. The next commit fixes that. Both branches carried an avoidable full walk either way. Co-Authored-By: Claude Opus 5 --- src/scene/mod.rs | 50 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 9c38d148c..2a151ba04 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -1848,6 +1848,13 @@ pub struct Scene { /// `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 @@ -2093,6 +2100,7 @@ impl Scene { 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), @@ -8902,7 +8910,7 @@ 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. - let mut visible_path = "quadtree"; + let mut visible_path = "quadtree+index"; let mut visible_candidates = 0usize; // Phase 2.1 — quadtree-driven candidate selection. When a view @@ -8941,10 +8949,19 @@ 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 @@ -9239,6 +9256,29 @@ entities={} memo_hit={} memo_miss={} wires={} memo={} visible_path={} candidates /// * 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(); From 4e6a9c517773074012f38803a1ddaf1cc31ba255 Mon Sep 17 00:00:00 2001 From: Panos <25664832+mechpanos@users.noreply.github.com> Date: Sat, 5 Sep 2026 11:28:53 +0300 Subject: [PATCH 4/4] fix: make the visible-path diagnostic actually report a path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous version declared `visible_path` with a default of "quadtree" and never assigned it in any branch, so every run printed `visible_path=quadtree candidates=0` no matter which of the three branches ran. Clippy's "does not need to be mutable" on the declaration was the tell. All three branches now name themselves — `quadtree`, `full-scan`, `block-index` — and report how many candidates they considered. The binding is left uninitialised so the compiler rejects any branch that forgets, which is the property the first version lacked. The full-scan branch reports no count: obtaining one means walking the document a second time, on the one branch already doing that walk. Co-Authored-By: Claude Opus 5 --- src/scene/mod.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 2a151ba04..28cab5091 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -8910,7 +8910,10 @@ 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. - let mut visible_path = "quadtree+index"; + // 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 @@ -8920,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, @@ -8932,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) { @@ -8955,7 +8960,7 @@ impl Scene { // way. { let unindexable = self.unindexable_handles(); - visible_candidates = unindexable.1.len(); + visible_candidates += unindexable.1.len(); for &h in &unindexable.1 { if let Some(e) = self.document.get_entity(h) { if visibility_ok(e) { @@ -8970,6 +8975,7 @@ impl Scene { // 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)) @@ -8983,9 +8989,11 @@ impl Scene { // 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) {