From 04bd49c7153de6c09fe768257bdc9ad6b958bd15 Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Mon, 13 Jul 2026 16:28:21 +0300 Subject: [PATCH] fix(io): resolve root named-objects dict robustly on foreign DWGs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some DWGs written by other programs leave the root named-objects dictionary unrecoverable: the header pointer names a non-dictionary and the real NOD child dicts are owned by a table-control handle, so nothing has owner==NULL. Navigating the root then silently no-ops, and registering a new named-object entry (page setup, annotation scale, CTAB) against the missing root orphans it — it never round-trips. Add shared annotative::root_named_dict_handle(): resolve the header pointer, else adopt the richest owner==NULL dict, else synthesise a fresh root and write it back. Route ensure_plotsettings_dict, add_scale's scalelist-create branch, and set_saved_active_layout through it. Resolve the plot-settings dict via the PlotSettings owner when the header pointer is stale, mirroring scalelist_dict_handle. Verified page setups survive a DWG save/reload on the repro file with layouts intact. Co-Authored-By: Claude Opus 4.8 --- src/io/mod.rs | 26 ++----- src/scene/annotative.rs | 47 ++++++++++++ src/scene/mod.rs | 6 +- src/scene/page_setup.rs | 52 ++++++++++--- tests/page_setup_root_repair.rs | 128 ++++++++++++++++++++++++++++++++ 5 files changed, 227 insertions(+), 32 deletions(-) create mode 100644 tests/page_setup_root_repair.rs diff --git a/src/io/mod.rs b/src/io/mod.rs index 0d645928b..999d137f9 100644 --- a/src/io/mod.rs +++ b/src/io/mod.rs @@ -550,27 +550,11 @@ pub fn set_saved_active_layout(doc: &mut CadDocument, name: &str) { } return; } - // Attach a new CTAB entry to the root named-object dictionary. Prefer the - // header handle; fall back to scanning for the root dict (the one holding - // `ACAD_LAYOUT`) since a from-scratch document leaves the header handle null - // until it is built. No root dict at all → `$TILEMODE` still records - // model-vs-paper. - let named = doc.header.named_objects_dict_handle; - let root = if !named.is_null() && doc.objects.contains_key(&named) { - named - } else { - match doc.objects.iter().find_map(|(h, o)| match o { - ObjectType::Dictionary(d) - if d.entries.iter().any(|(k, _)| k.eq_ignore_ascii_case("ACAD_LAYOUT")) => - { - Some(*h) - } - _ => None, - }) { - Some(h) => h, - None => return, - } - }; + // Attach a new CTAB entry to the root named-object dictionary. Resolve it + // robustly (or synthesise one) so the current-tab record persists even on a + // from-scratch document, or a foreign DWG whose header root pointer is + // unresolvable. See `annotative::root_named_dict_handle`. + let root = crate::scene::annotative::root_named_dict_handle(doc); let handle = doc.allocate_handle(); let mut var = DictionaryVariable::new("CTAB", name); var.handle = handle; diff --git a/src/scene/annotative.rs b/src/scene/annotative.rs index 353777751..228e34d2d 100644 --- a/src/scene/annotative.rs +++ b/src/scene/annotative.rs @@ -18,6 +18,53 @@ pub fn as_dict(doc: &CadDocument, handle: Handle) -> Option<&Dictionary> { } } +/// Resolve the drawing's root named-objects dictionary, creating one if the +/// file has none reachable. +/// +/// The canonical pointer is `header.named_objects_dict_handle`, but DWGs +/// written by some programs leave it dangling — pointing at a handle that never +/// loaded, or at a non-dictionary — while the real named-object sub-dictionaries +/// (`ACAD_LAYOUT`, `ACAD_SCALELIST`, …) are instead owned by an unrelated handle +/// that is not a dictionary. When the pointer can't be resolved we adopt any +/// top-level (`owner == NULL`) dictionary as the root; failing that we synthesise +/// a fresh, empty root so that *registering* a new named-object entry (a page +/// setup, an annotation scale, the `CTAB` variable) actually persists instead of +/// silently no-opping against a missing dictionary. +/// +/// Idempotent: the resolved or created handle is written back to the header, so +/// later calls return the same dictionary rather than minting another root. On a +/// well-formed drawing this returns the existing root untouched. +pub fn root_named_dict_handle(doc: &mut CadDocument) -> Handle { + let h = doc.header.named_objects_dict_handle; + if matches!(doc.objects.get(&h), Some(ObjectType::Dictionary(_))) { + return h; + } + // A top-level dictionary is already present (the standard root shape) — adopt + // the richest one (matching the DWG writer's own root heuristic) and repair + // the stale header pointer. + if let Some(root) = doc + .objects + .iter() + .filter_map(|(k, o)| match o { + ObjectType::Dictionary(d) if d.owner.is_null() => Some((*k, d.entries.len())), + _ => None, + }) + .max_by_key(|&(_, n)| n) + .map(|(k, _)| k) + { + doc.header.named_objects_dict_handle = root; + return root; + } + // Nothing reachable — build a fresh, empty root named-objects dictionary. + let nh = doc.allocate_handle(); + let mut d = Dictionary::new(); + d.handle = nh; + d.owner = Handle::NULL; + doc.objects.insert(nh, ObjectType::Dictionary(d)); + doc.header.named_objects_dict_handle = nh; + nh +} + /// Set the per-object annotative flag on the entity types that carry one /// (MTEXT, MULTILEADER). Turning it off also strips the per-object annotation /// context and legacy markers via [`clear_annotation_context`] so the object diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 54ddc942a..7f72b6d6a 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -1967,7 +1967,11 @@ impl Scene { let scalelist_h = match self.scalelist_dict_handle() { Some(h) => h, None => { - let root_h = self.document.header.named_objects_dict_handle; + // Resolve (or synthesise) the root robustly — a drawing whose + // header root pointer is unresolvable would otherwise register + // the new dictionary against a missing root and silently orphan + // it. See `annotative::root_named_dict_handle`. + let root_h = crate::scene::annotative::root_named_dict_handle(&mut self.document); let h = self.document.allocate_handle(); let mut d = Dictionary::new(); d.handle = h; diff --git a/src/scene/page_setup.rs b/src/scene/page_setup.rs index ef2f5e5e7..a09db2e6c 100644 --- a/src/scene/page_setup.rs +++ b/src/scene/page_setup.rs @@ -9,10 +9,34 @@ use acadrust::objects::{Dictionary, ObjectType, PlotSettings}; use acadrust::Handle; impl Scene { + /// Handle of the `ACAD_PLOTSETTINGS` dictionary, located robustly. + /// + /// The canonical path is the header's `acad_plotsettings_dict_handle`, but + /// DWGs written by other programs don't always leave that pointer resolvable + /// (the header handle points at no loaded dictionary — see + /// [`crate::scene::annotative::root_named_dict_handle`]). In that case fall + /// back to the dictionary that owns the drawing's `PlotSettings` objects, + /// mirroring [`Scene::scalelist_dict_handle`]. Returns `None` when the + /// drawing genuinely has no named page setups. + fn plotsettings_dict_handle(&self) -> Option { + let dh = self.document.header.acad_plotsettings_dict_handle; + if matches!(self.document.objects.get(&dh), Some(ObjectType::Dictionary(_))) { + return Some(dh); + } + let owner = self.document.objects.values().find_map(|o| match o { + ObjectType::PlotSettings(ps) => Some(ps.owner), + _ => None, + })?; + matches!( + self.document.objects.get(&owner), + Some(ObjectType::Dictionary(_)) + ) + .then_some(owner) + } + /// Names of the document's named page setups, in dictionary order. pub fn page_setup_names(&self) -> Vec { - let dh = self.document.header.acad_plotsettings_dict_handle; - match self.document.objects.get(&dh) { + match self.plotsettings_dict_handle().and_then(|h| self.document.objects.get(&h)) { Some(ObjectType::Dictionary(d)) => d.entries.iter().map(|(k, _)| k.clone()).collect(), _ => Vec::new(), } @@ -20,7 +44,7 @@ impl Scene { /// Clone the named page setup's `PlotSettings`, or `None` if absent. pub fn page_setup_get(&self, name: &str) -> Option { - let dh = self.document.header.acad_plotsettings_dict_handle; + let dh = self.plotsettings_dict_handle()?; let ObjectType::Dictionary(d) = self.document.objects.get(&dh)? else { return None; }; @@ -32,13 +56,17 @@ impl Scene { } /// Handle of the `ACAD_PLOTSETTINGS` dictionary, creating it (and its entry - /// in the root named-objects dictionary) if the drawing has none yet. + /// in the root named-objects dictionary) if the drawing has none yet. The + /// root itself is resolved (or synthesised) robustly so registration + /// persists even on drawings whose header root pointer is unresolvable. fn ensure_plotsettings_dict(&mut self) -> Handle { - let dh = self.document.header.acad_plotsettings_dict_handle; - if matches!(self.document.objects.get(&dh), Some(ObjectType::Dictionary(_))) { + if let Some(dh) = self.plotsettings_dict_handle() { + // Keep the header pointer in sync with the located dictionary so the + // writer and later reads agree on it. + self.document.header.acad_plotsettings_dict_handle = dh; return dh; } - let root = self.document.header.named_objects_dict_handle; + let root = crate::scene::annotative::root_named_dict_handle(&mut self.document); let mut dict = Dictionary::new(); dict.handle = self.document.allocate_handle(); dict.owner = root; @@ -56,7 +84,7 @@ impl Scene { /// Handle of the named page setup, if it exists. fn page_setup_handle(&self, name: &str) -> Option { - let dh = self.document.header.acad_plotsettings_dict_handle; + let dh = self.plotsettings_dict_handle()?; let ObjectType::Dictionary(d) = self.document.objects.get(&dh)? else { return None; }; @@ -85,7 +113,9 @@ impl Scene { /// Remove the named page setup (object + dictionary entry). No-op if absent. pub fn page_setup_delete(&mut self, name: &str) { - let dict_handle = self.document.header.acad_plotsettings_dict_handle; + let Some(dict_handle) = self.plotsettings_dict_handle() else { + return; + }; let handle = if let Some(ObjectType::Dictionary(d)) = self.document.objects.get_mut(&dict_handle) { @@ -109,7 +139,9 @@ impl Scene { if self.page_setup_handle(new).is_some() { return; // name collision } - let dict_handle = self.document.header.acad_plotsettings_dict_handle; + let Some(dict_handle) = self.plotsettings_dict_handle() else { + return; + }; let mut renamed = None; if let Some(ObjectType::Dictionary(d)) = self.document.objects.get_mut(&dict_handle) { if let Some(e) = d.entries.iter_mut().find(|(k, _)| k == old) { diff --git a/tests/page_setup_root_repair.rs b/tests/page_setup_root_repair.rs new file mode 100644 index 000000000..fdd2f5d0a --- /dev/null +++ b/tests/page_setup_root_repair.rs @@ -0,0 +1,128 @@ +// Some DWGs written by other programs leave the header's root named-objects +// dictionary pointer unresolvable — it names a handle that never loaded (or a +// non-dictionary), while the real named-object sub-dictionaries are owned by an +// unrelated handle. Navigating that root then silently no-ops, so registering a +// new named-object entry (a page setup, the CTAB current-tab variable, an +// annotation scale) would vanish instead of persisting. +// +// `annotative::root_named_dict_handle` resolves the root robustly and, when it +// truly can't be found, synthesises a fresh one so registration sticks. These +// guards exercise that repair through the page-setup and CTAB write paths. + +use acadrust::objects::{ObjectType, PlotSettings}; +use acadrust::Handle; +use OpenCADStudio::scene::Scene; + +/// Break a document the way a foreign DWG does: dangle the header root pointer +/// and re-home the real root under a non-dictionary owner, so neither the +/// pointer nor the `owner == NULL` heuristic can rediscover it. +fn break_root_pointer(scene: &mut Scene) { + let doc = &mut scene.document; + let real_root = doc.header.named_objects_dict_handle; + if let Some(ObjectType::Dictionary(d)) = doc.objects.get_mut(&real_root) { + // Owner points at the dimstyle-control handle (0x0A) — present in the + // file but not a dictionary, exactly like the repro drawing. + d.owner = Handle::new(0x0A); + } + doc.header.named_objects_dict_handle = Handle::new(0x00DE_AD00); + doc.header.acad_plotsettings_dict_handle = Handle::new(0x00BA_D000); +} + +#[test] +fn page_setup_persists_when_root_pointer_is_unresolvable() { + let mut scene = Scene::new(); + break_root_pointer(&mut scene); + + scene.page_setup_save("Metric-A4", PlotSettings::new("Metric-A4")); + + // The setup is retrievable again… + assert!( + scene.page_setup_names().contains(&"Metric-A4".to_string()), + "saved page setup must be listed back" + ); + assert!( + scene.page_setup_get("Metric-A4").is_some(), + "saved page setup must be resolvable" + ); + + // …and it is linked into a *resolvable* root named-objects dictionary, so it + // will round-trip through a save rather than dangling. + let root_h = scene.document.header.named_objects_dict_handle; + let ObjectType::Dictionary(root) = scene + .document + .objects + .get(&root_h) + .expect("root handle must resolve to a loaded object") + else { + panic!("root handle must resolve to a dictionary"); + }; + assert!( + root.entries.iter().any(|(k, _)| k == "ACAD_PLOTSETTINGS"), + "ACAD_PLOTSETTINGS must be registered in the (repaired) root NOD" + ); +} + +#[test] +fn page_setup_survives_a_dwg_roundtrip_after_root_repair() { + let mut scene = Scene::new(); + break_root_pointer(&mut scene); + scene.page_setup_save("Wide", PlotSettings::new("Wide")); + + // Full DWG write/read cycle: the repaired root must carry the setup across. + let bytes = OpenCADStudio::io::save_to_bytes(&scene.document, "dwg", scene.document.version) + .expect("save to DWG bytes"); + let doc = OpenCADStudio::io::load_bytes("roundtrip.dwg", bytes).expect("reload DWG bytes"); + + let mut reloaded = Scene::new(); + reloaded.document = doc; + assert!( + reloaded.page_setup_names().contains(&"Wide".to_string()), + "page setup must persist across a DWG save/reload once the root is repaired" + ); +} + +#[test] +fn existing_but_unlinked_plot_settings_are_still_listed() { + // A drawing whose ACAD_PLOTSETTINGS dictionary exists but whose header + // pointer to it is stale: the setups must still be discoverable via the + // PlotSettings objects' owner, mirroring the scale-list fallback. + let mut scene = Scene::new(); + scene.page_setup_save("Draft", PlotSettings::new("Draft")); + // Now stale the header pointer (leave the dictionary + object intact). + scene.document.header.acad_plotsettings_dict_handle = Handle::new(0x00BA_D000); + + assert!( + scene.page_setup_names().contains(&"Draft".to_string()), + "page setup must be found via the PlotSettings owner when the header \ + plot-settings pointer is stale" + ); +} + +#[test] +fn ctab_is_created_against_a_repaired_root() { + let mut scene = Scene::new(); + break_root_pointer(&mut scene); + + // Switching the active layout records CTAB; with a broken root pointer this + // used to silently drop the record. + scene.set_current_layout("Layout1".to_string()); + assert_eq!( + OpenCADStudio::io::saved_active_layout(&scene.document).as_deref(), + Some("Layout1"), + "CTAB must persist even when the root pointer needed repair" + ); + + let root_h = scene.document.header.named_objects_dict_handle; + let ObjectType::Dictionary(root) = scene + .document + .objects + .get(&root_h) + .expect("root must resolve") + else { + panic!("root must be a dictionary"); + }; + assert!( + root.entries.iter().any(|(k, _)| k == "CTAB"), + "CTAB must be registered in the repaired root NOD" + ); +}