From 7d1fca000b918d6da3ca2b84ff7ab70668368090 Mon Sep 17 00:00:00 2001 From: Karim Jerbi Date: Mon, 5 Oct 2026 00:07:21 +0100 Subject: [PATCH] fix(io): materialize missing BLOCK/ENDBLK sentinels on load The DXF reader keeps marker identities on the block record but never materializes the entities, so block_entity_handle/block_end_handle dangle after every DXF load (reopened baked *D dimension blocks hit this every time). Re-create missing sentinels at load finalize, reusing the file handle when free and minting otherwise. Idempotent and DWG-safe: loads whose markers already resolve are untouched. Pinned by dimlinear_survives_save_reload_with_baked_geometry. --- src/io/mod.rs | 79 ++++++++++++++++++++++++++++++ src/modules/draw/modify/explode.rs | 73 +++++++++++++++++++++++++++ 2 files changed, 152 insertions(+) diff --git a/src/io/mod.rs b/src/io/mod.rs index 85cfe2906..3a4c9d52a 100644 --- a/src/io/mod.rs +++ b/src/io/mod.rs @@ -1036,6 +1036,7 @@ fn finalize_loaded_outcome( ) -> Result { let doc = &mut outcome.document; normalize_block_origins(doc); + materialize_missing_block_markers(doc); normalize_knotless_splines(doc); if outcome.stats.source_format == Some(codec::SourceFormat::Dxf) { fix_dxf_dimension_rotations(doc); @@ -1240,6 +1241,84 @@ fn normalize_block_origins(doc: &mut CadDocument) { } } +/// The DXF section reader keeps BLOCK/ENDBLK marker identities on the block +/// record but never materializes the sentinel entities, so +/// `block_entity_handle`/`block_end_handle` dangle after every DXF load +/// (reopened baked `*D` dimension blocks hit this every time). Re-create any +/// missing sentinel so each record points at a real Block/BlockEnd again. +/// Idempotent: records whose markers already resolve (DWG loads, live docs) +/// are untouched. File handles are reused when still free so a repair + +/// resave stays handle-stable; otherwise a fresh handle is minted and the +/// record is pointed at it. +fn materialize_missing_block_markers(doc: &mut CadDocument) { + use codec::entities::{Block, BlockEnd}; + use codec::{EntityType, Handle}; + + struct Work { + name: String, + record_handle: Handle, + base_point: codec::types::Vector3, + block_handle: Handle, + end_handle: Handle, + need_block: bool, + need_end: bool, + } + let work: Vec = doc + .block_records + .iter() + .map(|record| Work { + name: record.name.clone(), + record_handle: record.handle, + base_point: record.base_point, + block_handle: record.block_entity_handle, + end_handle: record.block_end_handle, + need_block: !matches!( + doc.get_entity(record.block_entity_handle), + Some(EntityType::Block(_)) + ), + need_end: !matches!( + doc.get_entity(record.block_end_handle), + Some(EntityType::BlockEnd(_)) + ), + }) + .filter(|w| w.need_block || w.need_end) + .collect(); + + for w in work { + if w.need_block { + let mut marker = Block::new(&w.name, w.base_point); + // Reuse the file handle only if nothing else claims it; an + // explicit handle is respected by add_entity (bumping the counter + // past it), otherwise a NULL handle mints a fresh one below. + if !w.block_handle.is_null() && doc.get_entity(w.block_handle).is_none() { + marker.common.handle = w.block_handle; + } + marker.common.owner_handle = w.record_handle; + if let Ok(handle) = doc.add_entity(EntityType::Block(marker)) { + if handle != w.block_handle { + if let Some(record) = doc.block_records.get_mut(&w.name) { + record.block_entity_handle = handle; + } + } + } + } + if w.need_end { + let mut marker = BlockEnd::new(); + if !w.end_handle.is_null() && doc.get_entity(w.end_handle).is_none() { + marker.common.handle = w.end_handle; + } + marker.common.owner_handle = w.record_handle; + if let Ok(handle) = doc.add_entity(EntityType::BlockEnd(marker)) { + if handle != w.end_handle { + if let Some(record) = doc.block_records.get_mut(&w.name) { + record.block_end_handle = handle; + } + } + } + } + } +} + /// A RasterImage entity stores its file path on the linked ImageDefinition, /// often as the original author's absolute path (e.g. a Windows / VMware share) /// that doesn't exist on this machine. Resolve each image to a usable path: diff --git a/src/modules/draw/modify/explode.rs b/src/modules/draw/modify/explode.rs index b8631c67c..dc21485aa 100644 --- a/src/modules/draw/modify/explode.rs +++ b/src/modules/draw/modify/explode.rs @@ -1970,4 +1970,77 @@ mod tests { assert!((line.start.x - 2.0).abs() < 1e-12); assert!((line.end.x - 2.0).abs() < 1e-12); } + + // RED (TDD): a real DIMLINEAR must survive save -> reload with its baked + // *D geometry intact and a clean handle graph (no duplicates, no dangling + // owners). Headless codec round-trip, no GUI needed. + #[test] + fn dimlinear_survives_save_reload_with_baked_geometry() { + let mut failures = Vec::new(); + for ext in ["dxf", "dwg"] { + let mut doc = CadDocument::new(); + let mut d = + DimensionLinear::new(Vector3::new(0.0, 0.0, 0.0), Vector3::new(10.0, 0.0, 0.0)); + d.definition_point = Vector3::new(0.0, 5.0, 0.0); + d.base.text_middle_point = Vector3::new(5.0, 5.0, 0.0); + doc.add_entity(EntityType::Dimension(Dimension::Linear(d))) + .unwrap(); + + let path = std::env::temp_dir().join(format!( + "ocs_dim_rt_{}.{ext}", + std::process::id() + )); + crate::io::save_as_version(&doc, &path, codec::DxfVersion::AC1032) + .expect("save"); + let loaded = crate::io::load_file(&path).expect("load"); + let _ = std::fs::remove_file(&path); + + let mut seen = std::collections::HashSet::new(); + for e in loaded.entities() { + if !seen.insert(e.common().handle) { + failures.push(format!( + "{ext}: duplicate handle {:?}", + e.common().handle + )); + } + } + let dims: Vec<_> = loaded + .entities() + .filter_map(|e| match e { + EntityType::Dimension(d) => Some(d.clone()), + _ => None, + }) + .collect(); + if dims.len() != 1 { + failures.push(format!("{ext}: dimension entity lost")); + continue; + } + let block_name = dims[0].base().block_name.clone(); + if block_name.trim().is_empty() { + failures.push(format!("{ext}: dimension came back blockless")); + continue; + } + let Some(rec) = loaded.block_records.get(&block_name) else { + failures.push(format!("{ext}: *D block {block_name} missing after reload")); + continue; + }; + let owned: Vec = { + let mut h = rec.entity_handles.clone(); + h.push(rec.block_entity_handle); + h.push(rec.block_end_handle); + h + }; + if rec.entity_handles.is_empty() { + failures.push(format!("{ext}: baked block has no sub-entities")); + } + for h in &owned { + if h.is_null() || loaded.get_entity(*h).is_none() { + failures.push(format!( + "{ext}: dangling handle {h:?} in baked block {block_name}" + )); + } + } + } + assert!(failures.is_empty(), "round-trip failures:\n{}", failures.join("\n")); + } }