From 8b133ceab2656d547dba10a68355df7251af4cc8 Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Sun, 12 Jul 2026 01:17:26 +0300 Subject: [PATCH] fix(layout): create the CTAB variable so the exact paper tab round-trips MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Saving from a non-first paper layout still reopened on the first paper tab: set_saved_active_layout only updated an existing CTAB entry, but documents authored here never carried one, so CTAB was never written and the reader fell back to $TILEMODE (which only records model-vs-paper) → the first paper layout. set_saved_active_layout now creates the CTAB DICTIONARYVAR under the root named-object dictionary when it is absent (updating in place otherwise). The root dictionary is taken from the header handle, or found by scanning for the dictionary that holds ACAD_LAYOUT when a from-scratch document has not populated the header handle yet. The writers persist it (they serialize the document's objects and root-dictionary entries; the root-dict rebuild in CadDocument::build() is not run on save). Adds tests/active_space_roundtrip.rs, including a full DXF save→reload round-trip asserting both $TILEMODE and CTAB survive. Co-Authored-By: Claude Opus 4.8 --- src/io/mod.rs | 43 ++++++++++++++++-- tests/active_space_roundtrip.rs | 79 +++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 3 deletions(-) create mode 100644 tests/active_space_roundtrip.rs diff --git a/src/io/mod.rs b/src/io/mod.rs index b735e1fa..0d645928 100644 --- a/src/io/mod.rs +++ b/src/io/mod.rs @@ -538,10 +538,47 @@ pub fn saved_active_layout(doc: &CadDocument) -> Option { } /// Record `name` as the active layout tab (`CTAB`) so the next save round-trips -/// which space was open. No-op when the document has no `CTAB` entry yet; the -/// `$TILEMODE` header (model vs paper) remains the guaranteed fallback. +/// which space was open. Updates the existing `CTAB` variable in place, or +/// creates one under the root named-object dictionary when the drawing never +/// carried it (e.g. a document authored here from scratch) — otherwise the exact +/// paper layout would be lost and reopening fell back to the first paper tab. pub fn set_saved_active_layout(doc: &mut CadDocument, name: &str) { - set_vardict_value(doc, "CTAB", name); + use acadrust::objects::{DictionaryVariable, ObjectType}; + if let Some(h) = vardict_handle(doc, "CTAB") { + if let Some(ObjectType::DictionaryVariable(v)) = doc.objects.get_mut(&h) { + v.value = name.to_string(); + } + 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, + } + }; + let handle = doc.allocate_handle(); + let mut var = DictionaryVariable::new("CTAB", name); + var.handle = handle; + var.owner_handle = root; + doc.objects.insert(handle, ObjectType::DictionaryVariable(var)); + if let Some(ObjectType::Dictionary(rd)) = doc.objects.get_mut(&root) { + rd.entries.push(("CTAB".to_string(), handle)); + } } /// Materialise the current-style choices into their format-specific storage diff --git a/tests/active_space_roundtrip.rs b/tests/active_space_roundtrip.rs new file mode 100644 index 00000000..7387be1e --- /dev/null +++ b/tests/active_space_roundtrip.rs @@ -0,0 +1,79 @@ +// The active space (Model vs a paper layout) a drawing was saved in must +// round-trip: `set_current_layout` mirrors it into the document as $TILEMODE +// (`header.show_model_space`) and the CTAB current-tab variable, and the loader +// restores it. Regression guard for the "always reopens in Model / the first +// paper layout" bugs. + +use OpenCADStudio::scene::Scene; + +#[test] +fn switching_to_paper_records_tilemode_and_ctab() { + let mut scene = Scene::new(); + + // A paper layout is active → TILEMODE says paper, CTAB names the tab. + scene.set_current_layout("Layout1".to_string()); + assert!( + !scene.document.header.show_model_space, + "$TILEMODE should record paper space when a layout is active" + ); + assert_eq!( + OpenCADStudio::io::saved_active_layout(&scene.document).as_deref(), + Some("Layout1"), + "CTAB must be created/updated so the exact paper tab round-trips (not \ + just the first paper layout)" + ); + + // Back to Model → TILEMODE flips, CTAB follows. + scene.set_current_layout("Model".to_string()); + assert!( + scene.document.header.show_model_space, + "$TILEMODE should record model space in the Model tab" + ); + assert_eq!( + OpenCADStudio::io::saved_active_layout(&scene.document).as_deref(), + Some("Model"), + ); +} + +#[test] +fn active_paper_layout_survives_a_dxf_save_and_reload() { + let mut scene = Scene::new(); + scene.set_current_layout("Layout1".to_string()); + + // Full file round-trip: write to DXF bytes, read them back. + let bytes = OpenCADStudio::io::save_to_bytes(&scene.document, "dxf", scene.document.version) + .expect("save to DXF bytes"); + let doc = OpenCADStudio::io::load_bytes("roundtrip.dxf", bytes).expect("reload DXF bytes"); + + assert!( + !doc.header.show_model_space, + "$TILEMODE must persist paper space across a DXF save/reload" + ); + assert_eq!( + OpenCADStudio::io::saved_active_layout(&doc).as_deref(), + Some("Layout1"), + "CTAB must persist the exact active tab across a DXF save/reload" + ); +} + +#[test] +fn ctab_is_created_when_absent_then_updated_in_place() { + let mut scene = Scene::new(); + let doc = &mut scene.document; + + // A brand-new document carries no CTAB entry. + assert_eq!(OpenCADStudio::io::saved_active_layout(doc), None); + + // First write creates it; a second write must update in place, not stack a + // duplicate entry that a reader could resolve to the stale value. + OpenCADStudio::io::set_saved_active_layout(doc, "Layout2"); + assert_eq!( + OpenCADStudio::io::saved_active_layout(doc).as_deref(), + Some("Layout2") + ); + OpenCADStudio::io::set_saved_active_layout(doc, "Layout3"); + assert_eq!( + OpenCADStudio::io::saved_active_layout(doc).as_deref(), + Some("Layout3") + ); +}