diff --git a/ROADMAP.md b/ROADMAP.md index d4b9d6bc0..a0026315a 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -27,7 +27,7 @@ Source-scan summary (references): **Goal:** measurably halve the wall time between "Open" click and "first frame" for a 50 MB DWG. -### 1.1 Drop the second `purge_corrupt_entities` +### 1.1 Drop the second `purge_corrupt_entities` ✅ DONE Today `purge_corrupt_entities` runs once on the [background thread](src/io/mod.rs#L62-L74) and again in the @@ -56,7 +56,9 @@ list back. The UI thread only emits log lines. - corrupt-entity detection, - hatch / image / mesh handle lists, - AABB accumulation for `world_offset` (currently a separate pass inside - `compute_world_offset`). + `compute_world_offset`). ✅ the world_offset AABB scan is now folded into + the cache-handle walk (see 2.4); corrupt-detect + hatch/image/mesh planning + remain a follow-up. Target: three `O(N)` passes → one. @@ -131,7 +133,7 @@ On the first frame emit a **coarse**-tol wire pass (e.g. 4× the normal tol); refine to full tol on the second frame. The user sees *something* within 16 ms; detail snaps in smoothly afterwards. -### 2.4 Merge the world-offset scan into the single-pass walk +### 2.4 Merge the world-offset scan into the single-pass walk ✅ DONE [`compute_world_offset`](src/scene/mod.rs#L128) walks the whole MSPACE AABB when the header is unreliable. That scan should join the single-pass diff --git a/src/app/commands.rs b/src/app/commands.rs index fecefdd1c..c006bb1c5 100644 --- a/src/app/commands.rs +++ b/src/app/commands.rs @@ -641,7 +641,7 @@ impl OpenCADStudio { // Reload all xrefs for the current drawing. if let Some(path) = &self.tabs[i].current_path.clone() { if let Some(base_dir) = path.parent() { - let infos = crate::io::xref::resolve_xrefs( + let (infos, _dropped) = crate::io::xref::resolve_xrefs( &mut self.tabs[i].scene.document, base_dir, ); diff --git a/src/app/update.rs b/src/app/update.rs index a8bec8c37..2f11cebd0 100644 --- a/src/app/update.rs +++ b/src/app/update.rs @@ -172,14 +172,13 @@ impl OpenCADStudio { // Auto-resolve XREFs relative to the opened file's directory. if let Some(base_dir) = path.parent() { - let xrefs = - crate::io::xref::resolve_xrefs(&mut self.tabs[i].scene.document, base_dir); // xref content arrives un-purged: parser-garbage entities // inside the referenced file can trigger infinite loops in - // tessellation. Run the corrupt-entity guard again. - let extra_dropped = crate::io::purge_corrupt_entities( - &mut self.tabs[i].scene.document, - ); + // tessellation. `resolve_xrefs` runs the corrupt-entity + // guard inline as it merges each xref, so no second + // full-document walk is needed here. + let (xrefs, extra_dropped) = + crate::io::xref::resolve_xrefs(&mut self.tabs[i].scene.document, base_dir); if extra_dropped > 0 { self.command_line.push_error(&format!( "Warning: {extra_dropped} corrupt xref entities dropped" diff --git a/src/io/mod.rs b/src/io/mod.rs index 87467fee4..ffdafab22 100644 --- a/src/io/mod.rs +++ b/src/io/mod.rs @@ -256,7 +256,7 @@ fn finite_vec3(v: &acadrust::types::Vector3) -> bool { } /// Returns true if the entity looks like parser garbage and should be dropped. -fn is_entity_corrupt(e: &EntityType) -> bool { +pub(crate) fn is_entity_corrupt(e: &EntityType) -> bool { use acadrust::entities::EntityType as E; // Reject polylines at or above this vertex count. Even valid drawings // rarely use this many — and parser desync produces exactly-100_000-vertex diff --git a/src/io/xref.rs b/src/io/xref.rs index 46135fbe1..a65eb81f8 100644 --- a/src/io/xref.rs +++ b/src/io/xref.rs @@ -35,8 +35,12 @@ pub struct XrefInfo { /// `base_dir`, and populate each xref block with entities from the /// referenced file. /// -/// Returns a list of [`XrefInfo`] describing each xref block found. -pub fn resolve_xrefs(doc: &mut CadDocument, base_dir: &Path) -> Vec { +/// Returns a list of [`XrefInfo`] describing each xref block found, plus the +/// number of corrupt xref entities dropped during merge. Purging happens +/// inline as each xref's entities are merged — xref content is parser output +/// just like the host doc, so it gets the same corrupt-entity guard. Folding +/// it in here avoids a second full-document `entities()` walk after resolve. +pub fn resolve_xrefs(doc: &mut CadDocument, base_dir: &Path) -> (Vec, usize) { // Auto-resolve every xref — frustum + LOD culling keep GPU cost bounded. let xref_entries: Vec<(String, String, Handle)> = doc .block_records @@ -46,6 +50,7 @@ pub fn resolve_xrefs(doc: &mut CadDocument, base_dir: &Path) -> Vec { .collect(); let mut result = Vec::new(); + let mut dropped = 0usize; for (block_name, raw_path, br_handle) in xref_entries { let resolved = resolve_path(&raw_path, base_dir); @@ -56,7 +61,7 @@ pub fn resolve_xrefs(doc: &mut CadDocument, base_dir: &Path) -> Vec { Err(_) => XrefStatus::NotFound, Ok(xref_doc) => { ensure_block_entities(doc, &block_name); - merge_xref_into_block(doc, &block_name, br_handle, xref_doc); + dropped += merge_xref_into_block(doc, &block_name, br_handle, xref_doc); XrefStatus::Loaded } }, @@ -71,7 +76,7 @@ pub fn resolve_xrefs(doc: &mut CadDocument, base_dir: &Path) -> Vec { }); } - result + (result, dropped) } /// Try to build an absolute path from a raw xref path string. @@ -135,12 +140,13 @@ fn ensure_block_entities(doc: &mut CadDocument, block_name: &str) { /// — which doesn't know about the xref's layers — and silently falls /// back to WHITE / 1 px line weight. AutoCAD's BIND command uses the /// same naming scheme. +/// Returns the number of corrupt entities skipped during the merge. fn merge_xref_into_block( doc: &mut CadDocument, xref_block_name: &str, br_handle: Handle, xref_doc: CadDocument, -) { +) -> usize { let prefix = xref_block_name; // Carry the xref's INSUNITS onto the host BlockRecord. Used at INSERT @@ -223,7 +229,15 @@ fn merge_xref_into_block( .cloned() .collect(); + let mut dropped = 0usize; for mut entity in entities { + // Drop parser-garbage entities (bad normals / vertex counts / inf + // coords) before they enter the host doc — they trigger huge + // allocations and unbounded recursion in the wire pipeline. + if super::is_entity_corrupt(&entity) { + dropped += 1; + continue; + } // Remap layer / linetype names so the host's resolver hits the // copies we just inserted. { @@ -262,6 +276,7 @@ fn merge_xref_into_block( set_handle(&mut entity, Handle::NULL); let _ = doc.add_entity(entity); } + dropped } fn is_sentinel_linetype(name: &str) -> bool { diff --git a/src/modules/insert/xattach.rs b/src/modules/insert/xattach.rs index 3e70a12ee..36e555a08 100644 --- a/src/modules/insert/xattach.rs +++ b/src/modules/insert/xattach.rs @@ -116,7 +116,7 @@ pub fn prepare_xref_block(scene: &mut Scene, path: &str) -> String { // Resolve the XREF content immediately. let path_buf = std::path::PathBuf::from(path); if let Some(base_dir) = path_buf.parent() { - crate::io::xref::resolve_xrefs(&mut scene.document, base_dir); + let _ = crate::io::xref::resolve_xrefs(&mut scene.document, base_dir); } block_name diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 0d2e071d5..89f9f2e54 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -146,16 +146,19 @@ pub fn build_derived_caches(doc: &CadDocument) -> DerivedCaches { // filters that `block_cache::build_defn` already uses for block defns) // and prefer the entity-scan when the header center drifts more than // 10× its own half-span away from the entity centroid. - let (world_offset, local_extent_max) = compute_world_offset(doc, model_block); - use rayon::prelude::*; - // Single pass over entities — collect only handles per kind. No clones. + // Single pass over entities does triple duty: classify cache-kind handle + // lists (hatch / image / mesh) AND accumulate per-entity centroids for the + // world_offset median. Folding the offset scan in here collapses what were + // two O(N) `entities()` walks (offset scan + handle collection) into one. // Heavy tessellation runs in parallel below, reading entities via - // `doc.get_entity(h)` (O(1) HashMap lookup). + // `doc.get_entity(h)` (O(1) HashMap lookup); no clones in this pass. + let prep = offset_prep(doc, model_block); let mut hatch_handles: Vec = Vec::new(); let mut image_handles: Vec = Vec::new(); let mut mesh_handles: Vec = Vec::new(); + let mut centers: Vec<[f64; 3]> = Vec::new(); for e in doc.entities() { let h = e.common().handle; match e { @@ -166,7 +169,11 @@ pub fn build_derived_caches(doc: &CadDocument) -> DerivedCaches { } _ => {} } + if let Some(c) = offset_centroid(e, model_block, &prep) { + centers.push(c); + } } + let (world_offset, local_extent_max) = world_offset_from_centers(centers, &doc.header); // Default bg adaptation target at load: the model background (paper // bg is only relevant after the user enters a paper layout, and @@ -233,34 +240,28 @@ pub fn build_derived_caches(doc: &CadDocument) -> DerivedCaches { } } -/// Pick the model-space precision-preserving offset and the `fit_all` -/// outlier-rejection limit. -/// -/// Tries header `$EXTMIN/$EXTMAX` first, then cross-checks against a direct -/// MSPACE entity AABB scan. The entity scan wins when the header is invalid -/// (sentinel / sub-empty) or when the header center has drifted more than -/// 10× its own half-span from the entity centroid (a stale-extents DXF). -fn compute_world_offset( - doc: &acadrust::CadDocument, - model_block: Handle, -) -> ([f64; 3], f32) { - // Mirrors `block_cache::SANE_EXTENT` — wire coords past this magnitude - // are treated as corruption rather than precision-relevant geometry. - const SANE_EXTENT: f64 = 1.0e8; +/// Mirrors `block_cache::SANE_EXTENT` — wire coords past this magnitude +/// are treated as corruption rather than precision-relevant geometry. +const WORLD_OFFSET_SANE_EXTENT: f64 = 1.0e8; - // The filter here MUST agree with `belongs_to_visible_block` (the - // render-time filter): if rendering treats an entity as MSPACE but - // we skip it here, our offset misses the geometry that's actually on - // screen, and direct WCS-coordinate wires drag f32 precision to its - // knees. Likewise, including block-defn entities that the render - // path strictly drops would pull the centroid toward block-local - // origins. - // - // Authoritative path: if the model BlockRecord enumerates its - // entities via `entity_handles`, use that set directly. Falls back - // to the legacy permissive interpretation when no BlockRecord - // enumerates anything (legacy DXF without group-code 330) — match - // `belongs_to_visible_block`'s permissive default for that case. +/// MSPACE-membership prep shared by the world-offset centroid scan. +/// +/// The filter here MUST agree with `belongs_to_visible_block` (the +/// render-time filter): if rendering treats an entity as MSPACE but we skip +/// it here, our offset misses on-screen geometry and direct WCS-coordinate +/// wires drag f32 precision to its knees. Conversely, including block-defn +/// entities the render path drops would pull the centroid toward block-local +/// origins. +struct OffsetPrep { + /// `Some` when the model BlockRecord enumerates its entities; the offset + /// scan uses this set directly. `None` falls back to the legacy + /// permissive owner-based interpretation. + mspace_set: Option>, + any_enumerated: bool, + owned_by_other_block: std::collections::HashSet, +} + +fn offset_prep(doc: &acadrust::CadDocument, model_block: Handle) -> OffsetPrep { let model_br = doc .block_records .iter() @@ -281,76 +282,89 @@ fn compute_world_offset( } else { std::collections::HashSet::new() }; + OffsetPrep { mspace_set, any_enumerated, owned_by_other_block } +} - // ── Pass 1: collect per-entity centroids ───────────────────────────── - // Min/max midpoint is wrecked by a single bogus entity at WCS distance - // (e.g. a Ray with bad direction, an orphan reference at WCS x=-510k - // when the real drawing sits at WCS x=+510k — the midpoint lands - // halfway between, far from both clusters). Per-entity centroids let - // us take the median, which is immune to single outliers regardless - // of how far they sit. - let mut centers: Vec<[f64; 3]> = Vec::new(); - for e in doc.entities() { - let c = e.common(); - let h = c.handle; - let include = if let Some(ref set) = mspace_set { - set.contains(&h) - } else if c.owner_handle == model_block { - true - } else if !c.owner_handle.is_null() { - false - } else if owned_by_other_block.contains(&h) { - false - } else { - // owner null + h not enumerated by any block: legacy permissive - // when no block enumerated at all, strict drop otherwise (same - // as belongs_to_visible_block). - !any_enumerated - }; - if !include { - continue; - } - // Skip block-defn sentinels and AttributeDefinition — same as - // block_cache::build_defn. Their bboxes don't represent drawable - // MSPACE geometry. - if matches!( - e, - EntityType::Block(_) - | EntityType::BlockEnd(_) - | EntityType::AttributeDefinition(_) - ) { - continue; - } - let (bmin, bmax) = match e { - EntityType::Insert(ins) => (ins.insert_point, ins.insert_point), - _ => { - let bb = e.as_entity().bounding_box(); - (bb.min, bb.max) - } - }; - // Empty-entity placeholder (Polyline/Hatch/Spline/Mesh with no - // vertices). Including these would pull the centroid toward origin - // and destroy precision on UTM-authored content. - if bmin.x == 0.0 - && bmin.y == 0.0 - && bmin.z == 0.0 - && bmax.x == 0.0 - && bmax.y == 0.0 - && bmax.z == 0.0 - { - continue; - } - let cx = (bmin.x + bmax.x) * 0.5; - let cy = (bmin.y + bmax.y) * 0.5; - let cz = (bmin.z + bmax.z) * 0.5; - if !cx.is_finite() || !cy.is_finite() || !cz.is_finite() { - continue; - } - if cx.abs() > SANE_EXTENT || cy.abs() > SANE_EXTENT { - continue; - } - centers.push([cx, cy, cz]); +/// Per-entity centroid for the world-offset scan, or `None` if the entity is +/// not MSPACE geometry / has no usable bbox. Single-outlier-robust because +/// the caller takes the median of these per-entity centroids rather than a +/// global min/max midpoint. +fn offset_centroid( + e: &EntityType, + model_block: Handle, + prep: &OffsetPrep, +) -> Option<[f64; 3]> { + let c = e.common(); + let h = c.handle; + let include = if let Some(ref set) = prep.mspace_set { + set.contains(&h) + } else if c.owner_handle == model_block { + true + } else if !c.owner_handle.is_null() { + false + } else if prep.owned_by_other_block.contains(&h) { + false + } else { + // owner null + h not enumerated by any block: legacy permissive + // when no block enumerated at all, strict drop otherwise (same + // as belongs_to_visible_block). + !prep.any_enumerated + }; + if !include { + return None; } + // Skip block-defn sentinels and AttributeDefinition — same as + // block_cache::build_defn. Their bboxes don't represent drawable + // MSPACE geometry. + if matches!( + e, + EntityType::Block(_) | EntityType::BlockEnd(_) | EntityType::AttributeDefinition(_) + ) { + return None; + } + let (bmin, bmax) = match e { + EntityType::Insert(ins) => (ins.insert_point, ins.insert_point), + _ => { + let bb = e.as_entity().bounding_box(); + (bb.min, bb.max) + } + }; + // Empty-entity placeholder (Polyline/Hatch/Spline/Mesh with no + // vertices). Including these would pull the centroid toward origin + // and destroy precision on UTM-authored content. + if bmin.x == 0.0 + && bmin.y == 0.0 + && bmin.z == 0.0 + && bmax.x == 0.0 + && bmax.y == 0.0 + && bmax.z == 0.0 + { + return None; + } + let cx = (bmin.x + bmax.x) * 0.5; + let cy = (bmin.y + bmax.y) * 0.5; + let cz = (bmin.z + bmax.z) * 0.5; + if !cx.is_finite() || !cy.is_finite() || !cz.is_finite() { + return None; + } + if cx.abs() > WORLD_OFFSET_SANE_EXTENT || cy.abs() > WORLD_OFFSET_SANE_EXTENT { + return None; + } + Some([cx, cy, cz]) +} + +/// Pick the model-space precision-preserving offset and the `fit_all` +/// outlier-rejection limit from the collected per-entity `centers`. +/// +/// Prefers the entity-centroid median; cross-checks against header +/// `$EXTMIN/$EXTMAX` only as a fallback when the entity scan found nothing. +/// `centers` is gathered by the caller's single entity walk (see +/// [`build_derived_caches`]) so no separate AABB pass is needed. +fn world_offset_from_centers( + centers: Vec<[f64; 3]>, + header: &acadrust::document::HeaderVariables, +) -> ([f64; 3], f32) { + const SANE_EXTENT: f64 = WORLD_OFFSET_SANE_EXTENT; let entity_ok = !centers.is_empty(); // Median of per-entity centroids → robust drawing center. @@ -381,10 +395,9 @@ fn compute_world_offset( (0.0, 0.0, 0.0, 0.0) }; - // ── Pass 2: read header extents ────────────────────────────────────── - let h = &doc.header; - let hmin = h.model_space_extents_min; - let hmax = h.model_space_extents_max; + // ── Header extents (fallback only) ─────────────────────────────────── + let hmin = header.model_space_extents_min; + let hmax = header.model_space_extents_max; let header_ok = hmin.x < hmax.x && hmin.y < hmax.y && hmin.x.abs() < SANE_EXTENT