From c61d648dc7ae54e1fabaf108dded415b7dae4f7c Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Wed, 13 May 2026 23:02:48 +0300 Subject: [PATCH] fix(scene): unfreeze open on huge xref Inserts A real DWG with an xref containing ~74k sub-entities froze on the first render: Insert tessellation exploded the block, style-resolved every sub serially, and one bad sub could hang the loop forever. - Insert short-circuits to an insertion marker when the referenced block has more than INSERT_SUB_LIMIT (5_000) sub-entities. Documented as temporary in RENDERING_OPTIMIZATION.md. - Re-run purge_corrupt_entities after resolve_xrefs so parser-garbage inside xref content is caught (initial purge runs before xref load). - build_derived_caches: single pass over entities collecting handles per kind; tessellation reads via doc.get_entity instead of cloning every EntityType. Image decode now parallel too. Co-Authored-By: Claude Opus 4.7 --- RENDERING_OPTIMIZATION.md | 38 ++++++++++++++ src/app/update.rs | 11 ++++ src/scene/mod.rs | 102 ++++++++++++++++++++++++++------------ 3 files changed, 119 insertions(+), 32 deletions(-) diff --git a/RENDERING_OPTIMIZATION.md b/RENDERING_OPTIMIZATION.md index d7974ada4..a86a588d0 100644 --- a/RENDERING_OPTIMIZATION.md +++ b/RENDERING_OPTIMIZATION.md @@ -272,4 +272,42 @@ Phase 4.2 Hi-Z occlusion high complexity, 3D only, last proportional to visible fraction. - **Phase 2 target:** Pan/zoom frame cost O(visible) not O(total). - **Phase 3 target:** Complex solid scene at far zoom → mesh triangle count < 10% of full detail. + +--- + +## TEMPORARY: Insert sub-entity count limit (revert when better fix lands) + +`tessellate_entity` in `src/scene/mod.rs` has a guard that short-circuits any +Insert whose referenced block contains more than `INSERT_SUB_LIMIT` (currently +5,000) sub-entities. The Insert is rendered as an insertion marker only — no +explode, no sub tessellation. + +**Why this exists:** A real-world DWG was found where an xref block contained +~74k sub-entities. Opening the file froze the UI on the first render because +`tessellate_entity` exploded the block and serially style-resolved every +sub-entity (and any single bad sub could hang tessellation forever). The guard +made the file openable. + +**Why it should go away:** The limit hides legitimate geometry. Large xrefs are +common in civil/infrastructure drawings — the user shouldn't have to see them +as crosses. + +**Proper fixes that would let us drop this guard:** +1. Per-block tessellation cache — tessellate a block once, then per-Insert + apply the transform to cached wire vertices. Helps when the same block is + inserted multiple times; doesn't fully solve single-Insert huge blocks. +2. Lazy / background Insert tessellation — show marker first, fill in detail + on a background thread, redraw when ready. +3. Per-sub-entity hardening — find why a single sub can hang tessellation + (likely Spline NURBS evaluation or malformed boundary edge) and fix the + root cause; combine with sub-count budget per frame instead of a hard cap. +4. xref attach metadata respected — DWG carries an "unloaded" state for xref + blocks; honour it instead of always inlining xref content on open. + +When any of (1)–(3) lands, delete the `INSERT_SUB_LIMIT` block in +`tessellate_entity` (search for `INSERT_SUB_LIMIT`). + +Related companion fix in `src/app/update.rs`: after `resolve_xrefs`, a second +`purge_corrupt_entities` pass runs against the newly inlined xref content. +That one is not a hack — it should stay even after the Insert guard goes. - **Phase 4 target:** GPU-cull overhead < 0.5 ms for 1M entity scene. diff --git a/src/app/update.rs b/src/app/update.rs index 4514e0dbb..8afcfe9ef 100644 --- a/src/app/update.rs +++ b/src/app/update.rs @@ -132,6 +132,17 @@ impl H7CAD { 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, + ); + if extra_dropped > 0 { + self.command_line.push_error(&format!( + "Warning: {extra_dropped} corrupt xref entities dropped" + )); + } for info in &xrefs { match info.status { crate::io::xref::XrefStatus::Loaded => { diff --git a/src/scene/mod.rs b/src/scene/mod.rs index ed1f8384e..bcc5f4d2e 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -120,25 +120,36 @@ pub fn build_derived_caches(doc: &CadDocument) -> DerivedCaches { use rayon::prelude::*; + // Single pass over entities — collect only handles per kind. No clones. + // Heavy tessellation runs in parallel below, reading entities via + // `doc.get_entity(h)` (O(1) HashMap lookup). + let mut hatch_handles: Vec = Vec::new(); + let mut image_handles: Vec = Vec::new(); + let mut mesh_handles: Vec = Vec::new(); + for e in doc.entities() { + let h = e.common().handle; + match e { + EntityType::Hatch(_) | EntityType::Solid(_) => hatch_handles.push(h), + EntityType::RasterImage(_) => image_handles.push(h), + EntityType::Solid3D(_) | EntityType::Region(_) | EntityType::Body(_) => { + mesh_handles.push(h) + } + _ => {} + } + } + // hatches - let hatch_entries: Vec<(Handle, EntityType)> = doc - .entities() - .filter_map(|e| match e { - EntityType::Hatch(h2) => Some((h2.common.handle, e.clone())), - EntityType::Solid(s) => Some((s.common.handle, e.clone())), - _ => None, - }) - .collect(); - let hatches: HashMap = hatch_entries - .into_par_iter() - .filter_map(|(handle, kind)| { - let owner = kind.common().owner_handle; + let hatches: HashMap = hatch_handles + .par_iter() + .filter_map(|&handle| { + let e = doc.get_entity(handle)?; + let owner = e.common().owner_handle; let offset = if owner == model_block { world_offset } else { [0.0; 3] }; - let model = match &kind { + let model = match e { EntityType::Hatch(dxf) => { let color = tessellate::aci_to_rgba(&dxf.common.color); Scene::hatch_model_from_dxf(dxf, color, offset) @@ -154,11 +165,11 @@ pub fn build_derived_caches(doc: &CadDocument) -> DerivedCaches { .collect(); // images - let images: HashMap = doc - .entities() - .filter_map(|e| { - if let EntityType::RasterImage(img) = e { - ImageModel::from_raster_image(img).map(|m| (img.common.handle, m)) + let images: HashMap = image_handles + .par_iter() + .filter_map(|&handle| { + if let EntityType::RasterImage(img) = doc.get_entity(handle)? { + ImageModel::from_raster_image(img).map(|m| (handle, m)) } else { None } @@ -166,20 +177,12 @@ pub fn build_derived_caches(doc: &CadDocument) -> DerivedCaches { .collect(); // meshes (parallel tessellation) - let mesh_entries: Vec<(Handle, EntityType)> = doc - .entities() - .filter_map(|e| match e { - EntityType::Solid3D(_) | EntityType::Region(_) | EntityType::Body(_) => { - Some((e.common().handle, e.clone())) - } - _ => None, - }) - .collect(); - let meshes: HashMap = mesh_entries - .into_par_iter() - .filter_map(|(handle, entity)| { - let color = tessellate::aci_to_rgba(&entity.common().color); - let model = match &entity { + let meshes: HashMap = mesh_handles + .par_iter() + .filter_map(|&handle| { + let e = doc.get_entity(handle)?; + let color = tessellate::aci_to_rgba(&e.common().color); + let model = match e { EntityType::Solid3D(s) => solid3d_tess::tessellate_solid3d(s, color), EntityType::Region(r) => solid3d_tess::tessellate_region(r, color), EntityType::Body(b) => solid3d_tess::tessellate_body(b, color), @@ -3814,6 +3817,41 @@ fn tessellate_entity( } if let EntityType::Insert(ins) = e { + // Bail out for huge blocks (typically xref imports): exploding + + // tessellating tens of thousands of sub-entities freezes the UI, and + // any single bad sub can hang tessellation forever. Draw only an + // insertion marker so the user sees the block exists. + const INSERT_SUB_LIMIT: usize = 5_000; + let sub_count = document + .block_records + .get(&ins.block_name) + .map(|br| br.entity_handles.len()) + .unwrap_or(0); + if sub_count > INSERT_SUB_LIMIT { + let [ox, oy, oz] = world_offset; + let ip = glam::Vec3::new( + (ins.insert_point.x - ox) as f32, + (ins.insert_point.y - oy) as f32, + (ins.insert_point.z - oz) as f32, + ); + return vec![WireModel { + name: h.value().to_string(), + points: vec![], + color: entity_color, + selected: sel, + aci, + pattern_length: 0.0, + pattern: [0.0; 8], + line_weight_px: 1.0, + snap_pts: vec![(ip, wire_model::SnapHint::Insertion)], + tangent_geoms: vec![], + key_vertices: vec![], + aabb: WireModel::UNBOUNDED_AABB, + plinegen: true, + vp_scissor: None, + fill_tris: vec![], + }]; + } // Resolve the INSERT's own style so ByBlock sub-entities can inherit it. let (ins_color, ins_pat_len, ins_pat, ins_lw_px, _) = render::render_style_for(document, e); let ins_color = render::adapt_to_bg(ins_color, bg_color);