From f347a20a4c4e8411b1f80aaf34c2cb7f9833fdf5 Mon Sep 17 00:00:00 2001 From: Kevin Griffin Date: Wed, 22 Jul 2026 12:56:36 -0700 Subject: [PATCH 1/4] perf(scene): return the memoized wire set by Arc, not by deep copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit entity_wires() cloned the entire wire set — every point, fill, and snap buffer of every entity — on each call, although the set is already memoized behind an Arc and none of the 18 call sites mutates it. On large mesh imports (#358: 42M faces, ~210 MB of wire content per 1M triangles) that is a multi-GB allocation + memcpy on every pick, snap, selection, or plot pass. Return the Arc instead. Call sites iterate as before; the two window-plot paths that build a culled owned subset now clone only the wires inside the plot window, and the print pipeline moves the Arc into its async task. Measured (tests/mem_probe_meshent.rs, 1M-triangle MESH, release): resident after entity_wires() drops 758 -> 547 MB (866 -> 646 B/face). Co-Authored-By: Claude Fable 5 --- src/app/update/file.rs | 10 ++++++---- src/io/print_to_printer.rs | 8 ++++---- src/scene/mod.rs | 14 +++++++++----- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/app/update/file.rs b/src/app/update/file.rs index 0569b7f78..c968f6200 100644 --- a/src/app/update/file.rs +++ b/src/app/update/file.rs @@ -1335,10 +1335,11 @@ pub(super) fn on_open_file(&mut self) -> Task { let (wx0, wy0, wx1, wy1) = (x0 as f32, y0 as f32, x1 as f32, y1 as f32); let wires: Vec<_> = scene .entity_wires() - .into_iter() + .iter() .filter(|w| { w.aabb[0] <= wx1 && w.aabb[2] >= wx0 && w.aabb[1] <= wy1 && w.aabb[3] >= wy0 }) + .cloned() .collect(); let hatches = scene.paper_canvas_hatches(); let wipeouts = scene.paper_canvas_wipeouts(); @@ -1388,7 +1389,7 @@ pub(super) fn on_open_file(&mut self) -> Task { pub(super) fn layout_plot_params( &self, ) -> ( - Vec, + std::sync::Arc>, Vec, Vec, f64, @@ -1496,7 +1497,7 @@ pub(super) fn on_open_file(&mut self) -> Task { let mut x1 = f32::NEG_INFINITY; let mut y1 = f32::NEG_INFINITY; let mut any = false; - for w in scene.entity_wires() { + for w in scene.entity_wires().iter() { let picked = crate::scene::Scene::handle_from_wire_name(&w.name) .is_some_and(|h| set.contains(&h)); if !picked { @@ -2150,8 +2151,9 @@ pub(super) fn on_open_file(&mut self) -> Task { let (wx0, wy0, wx1, wy1) = (x0 as f32, y0 as f32, x1 as f32, y1 as f32); let wires: Vec<_> = scene .entity_wires() - .into_iter() + .iter() .filter(|w| w.aabb[0] <= wx1 && w.aabb[2] >= wx0 && w.aabb[1] <= wy1 && w.aabb[3] >= wy0) + .cloned() .collect(); let hatches: Vec<_> = scene.paper_canvas_hatches().as_ref().clone(); let wipeouts: Vec<_> = scene.paper_canvas_wipeouts().as_ref().clone(); diff --git a/src/io/print_to_printer.rs b/src/io/print_to_printer.rs index 1baa17908..586eee0f9 100644 --- a/src/io/print_to_printer.rs +++ b/src/io/print_to_printer.rs @@ -36,7 +36,7 @@ pub struct PrintOptions { // is native-only; the web build gets a stub so the call site still compiles. #[cfg(target_arch = "wasm32")] pub async fn print_wires( - _wires: Vec, + _wires: std::sync::Arc>, _hatches: Vec, _wipeouts: Vec, _paper_w: f64, @@ -57,7 +57,7 @@ pub fn list_printers() -> Vec { #[cfg(target_arch = "wasm32")] #[allow(clippy::too_many_arguments)] pub async fn print_wires_with( - _wires: Vec, + _wires: std::sync::Arc>, _hatches: Vec, _wipeouts: Vec, _paper_w: f64, @@ -87,7 +87,7 @@ pub fn print_existing_pdf(_path: &std::path::Path, _opts: &PrintOptions) -> Resu /// Returns `Ok(printer_name)` on success or `Err(message)` on failure. #[cfg(not(target_arch = "wasm32"))] pub async fn print_wires( - wires: Vec, + wires: std::sync::Arc>, hatches: Vec, wipeouts: Vec, paper_w: f64, @@ -150,7 +150,7 @@ pub fn list_printers() -> Vec { #[cfg(not(target_arch = "wasm32"))] #[allow(clippy::too_many_arguments)] pub async fn print_wires_with( - wires: Vec, + wires: std::sync::Arc>, hatches: Vec, wipeouts: Vec, paper_w: f64, diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 5fc17cfc4..c498108a2 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -2851,9 +2851,9 @@ impl Scene { /// Collect closed polygon outlines (world XY) from the current layout. pub fn closed_outlines(&self) -> Vec> { self.entity_wires() - .into_iter() + .iter() .filter_map(|wire| { - let pts = wire.points; + let pts = &wire.points; if pts.len() < 4 { return None; } @@ -2872,7 +2872,7 @@ impl Scene { // boundary commands) see one vertex per corner — not the doubled // ring that otherwise shows two grips at every corner. let mut ring: Vec<[f32; 2]> = Vec::with_capacity(pts.len()); - for p in &pts { + for p in pts { if !p[0].is_finite() || !p[1].is_finite() { continue; } @@ -3342,8 +3342,12 @@ impl Scene { } /// Build WireModels from all document entities + optional preview wire. - pub fn entity_wires(&self) -> Vec { - (*self.entity_wires_arc()).clone() + /// + /// Returns the memoized set by `Arc` — callers only ever iterate it, and + /// the previous per-call deep clone was a full copy of every wire buffer + /// (hundreds of MB on large mesh imports, #358). + pub fn entity_wires(&self) -> Arc> { + self.entity_wires_arc() } /// Per-entity normalized draw-order depth, keyed by entity handle value. From 5aecdff12e10ebd83f2e280e668a932272547806 Mon Sep 17 00:00:00 2001 From: Kevin Griffin Date: Wed, 22 Jul 2026 12:59:07 -0700 Subject: [PATCH 2/4] perf(mesh): skip per-vertex snap tables above 50k vertices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The subdivision MESH converter emitted one snap candidate and one key vertex per mesh vertex — ~56 B/vertex retained in every copy of the wire set. On NWD-scale imports (#358: tens of millions of vertices) that is GBs spent on snap targets far too dense to distinguish on screen. PolyfaceMesh and PolygonMesh already ship empty tables at any size. Above 50,000 vertices, ship none. Smaller meshes (including the #358 repro sample at 34.6k vertices per mesh) keep vertex snap unchanged. Measured (mem_probe_meshent, 1M-triangle MESH): 646 -> 589 B/face resident; wire content 210 -> 183 MB per 1M triangles. Co-Authored-By: Claude Fable 5 --- src/entities/mesh.rs | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/entities/mesh.rs b/src/entities/mesh.rs index 9a1c74df4..28310501e 100644 --- a/src/entities/mesh.rs +++ b/src/entities/mesh.rs @@ -685,12 +685,23 @@ impl TruckConvertible for Mesh { } } - let snap_pts: Vec<(glam::DVec3, SnapHint)> = self - .vertices - .iter() - .map(|v| (glam::DVec3::new(v.x, v.y, v.z), SnapHint::Node)) - .collect(); - let key_vertices: Vec<[f64; 3]> = self.vertices.iter().map(|v| [v.x, v.y, v.z]).collect(); + // Per-vertex snap tables cost ~56 B/vertex and are retained in every + // copy of the wire set. On NWD-scale imports (#358: tens of millions + // of mesh vertices) that is GBs for snap targets far too dense to + // pick apart on screen — so past this size ship none, which is what + // PolyfaceMesh / PolygonMesh already do at any size. + const SNAP_TABLE_MAX_VERTICES: usize = 50_000; + let (snap_pts, key_vertices) = if self.vertices.len() > SNAP_TABLE_MAX_VERTICES { + (Vec::new(), Vec::new()) + } else { + ( + self.vertices + .iter() + .map(|v| (glam::DVec3::new(v.x, v.y, v.z), SnapHint::Node)) + .collect(), + self.vertices.iter().map(|v| [v.x, v.y, v.z]).collect(), + ) + }; Some(TruckEntity { pick_tris: Vec::new(), From e9198a47e5aabd9e1b0a52f8dda2b2fd07d803d3 Mon Sep 17 00:00:00 2001 From: Kevin Griffin Date: Wed, 22 Jul 2026 13:03:01 -0700 Subject: [PATCH 3/4] perf(render): don't duplicate the wire set when no Face3D wire exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-epoch Face3D/other split cloned every wire into two owned lists even when the face3d list came out empty — for pure 2-D drawings and mesh imports (#358) the 'other' list was a wire-for-wire copy of the entire base set, held resident in split_cache alongside the original. Cache None as an 'everything is other' marker and hand the render the base set itself for that frame. The base Arc is deliberately not stored in the cache: try_resident_patch can only apply an incremental patch while the resident Arc is uniquely held, so pinning it there would silently degrade every edit to a full rebuild. Co-Authored-By: Claude Fable 5 --- src/scene/mod.rs | 8 +++++++- src/scene/view/render.rs | 39 ++++++++++++++++++++++++++------------- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/src/scene/mod.rs b/src/scene/mod.rs index c498108a2..729944b00 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -1061,9 +1061,15 @@ pub struct Scene { /// [`WIRE_CONTENT_GEN`] id. `split_face3d_wires` is an O(N) per-wire /// handle lookup + clone that otherwise re-runs every frame. A map, not a /// slot: a paper frame walks several sources (sheet + viewports). + /// + /// `None` for the second element means "no Face3D wire in this set — use + /// the base set itself". The base `Arc` is deliberately NOT stored here: + /// `try_resident_patch` can only move a resident set out for an + /// incremental patch while its `Arc` is uniquely held, so pinning it in + /// this cache would silently force a full rebuild on every edit (#358). #[allow(clippy::type_complexity)] split_cache: - RefCell>, Arc>)>>, + RefCell>, Option>>)>>, /// Cached `selected ∪ hover` handle set for the GPU xray overlay, keyed by /// `selection_generation`. Rebuilt only when the selection changes so /// `build_primitive` doesn't clone the set every frame. diff --git a/src/scene/view/render.rs b/src/scene/view/render.rs index 1a710f690..491d342fa 100644 --- a/src/scene/view/render.rs +++ b/src/scene/view/render.rs @@ -1228,9 +1228,18 @@ impl Scene { // each frame — for every source, since all ids are stable now. let (face3d_wires, other_arc) = { let cached = { self.split_cache.borrow().get(&wire_content_id).cloned() }; - cached.unwrap_or_else(|| { - let (f, o) = split_face3d_wires(&base_arc, &self.document); - let (fa, oa) = (Arc::new(f), Arc::new(o)); + let (fa, oa) = cached.unwrap_or_else(|| { + // No Face3D wire at all (pure 2-D drawings, mesh imports): + // "others" would be a wire-for-wire copy of the base set — + // mark it `None` and use the base set directly instead of + // duplicating it (#358). The base Arc itself must not be + // stored in the cache (see the `split_cache` field docs). + let (fa, oa) = if base_arc.iter().any(|w| is_face3d_wire(w, &self.document)) { + let (f, o) = split_face3d_wires(&base_arc, &self.document); + (Arc::new(f), Some(Arc::new(o))) + } else { + (Arc::new(Vec::new()), None) + }; let mut c = self.split_cache.borrow_mut(); // Ids change on rebuild, so old keys die naturally; the cap // just bounds pathological churn. @@ -1239,7 +1248,8 @@ impl Scene { } c.insert(wire_content_id, (fa.clone(), oa.clone())); (fa, oa) - }) + }); + (fa, oa.unwrap_or_else(|| Arc::clone(&base_arc))) }; // Base wire set — the cached `other` Arc directly, never cloned to // append overlays. Preview / interim wires ride in their own small @@ -1541,9 +1551,19 @@ pub(crate) fn resolve_pattern( (pat_len, pat) } +/// Whether a wire belongs to a Face3D entity, by document handle lookup — +/// so no changes to WireModel are needed. +fn is_face3d_wire(w: &WireModel, document: &acadrust::CadDocument) -> bool { + w.name + .parse::() + .ok() + .and_then(|v| document.get_entity(Handle::new(v))) + .map(|e| matches!(e, EntityType::Face3D(_))) + .unwrap_or(false) +} + /// Partition a wire list into (face3d_wires, other_wires). /// -/// Uses a document handle lookup so no changes to WireModel are needed. /// O(N) per geometry epoch — acceptable since it runs once per epoch. fn split_face3d_wires( wires: &[WireModel], @@ -1552,14 +1572,7 @@ fn split_face3d_wires( let mut face3d = Vec::new(); let mut others = Vec::new(); for w in wires { - let is_face3d = w - .name - .parse::() - .ok() - .and_then(|v| document.get_entity(Handle::new(v))) - .map(|e| matches!(e, EntityType::Face3D(_))) - .unwrap_or(false); - if is_face3d { + if is_face3d_wire(w, document) { face3d.push(w.clone()); } else { others.push(w.clone()); From ead2f5dd0b4c15688016b28fdf5389f47ab5f30e Mon Sep 17 00:00:00 2001 From: Kevin Griffin Date: Wed, 22 Jul 2026 13:07:41 -0700 Subject: [PATCH 4/4] test: pass the depth map BlockCache::build now requires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit text_font_rendering.rs still called BlockCache::build with the pre- depth-map signature, so 'cargo test' failed to compile on main. Pass an empty map — the test exercises glyph tessellation, not draw order. Co-Authored-By: Claude Fable 5 --- tests/text_font_rendering.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/text_font_rendering.rs b/tests/text_font_rendering.rs index f156412ea..5ec178192 100644 --- a/tests/text_font_rendering.rs +++ b/tests/text_font_rendering.rs @@ -49,7 +49,7 @@ fn expand_block_mtext( let ins = Insert::new("LABEL_BLOCK", insert_at); doc.add_entity(EntityType::Insert(ins.clone())).unwrap(); - let cache = BlockCache::build(&doc, 1.0, [0.0, 0.0, 0.0, 1.0]); + let cache = BlockCache::build(&doc, 1.0, [0.0, 0.0, 0.0, 1.0], &Default::default()); expand_insert( &cache, &ins,