From 1c35d7f8c19f3d2ac6da46e5846f4450bbbbf847 Mon Sep 17 00:00:00 2001 From: ffishman Date: Sat, 24 Oct 2026 02:53:49 -0300 Subject: [PATCH] Keep the active layout when saving DWG/DXF (#1735) * Keep the active layout when saving DWG/DXF The layout a file marks active (block *Paper_Space) was not kept on read, and on write *Paper_Space always went to the first tab, so every save made the first sheet active. Store it as Drawing::active_layout and write it as *Paper_Space again; tab order is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) * Match the active layout by name without regard to case paper_space_layouts and layout.rename now compare the active layout's name case-insensitively, as Drawing::layout() does, so an active layout stored as "s2" still resolves the "S2" tab. Co-Authored-By: Claude Opus 5.5 (1M context) * Test case-insensitive active layout matching Co-Authored-By: Claude Opus 5.5 (1M context) --------- Co-authored-by: Claude Opus 5.5 (1M context) Co-authored-by: ffishman <20211677+ffishman@users.noreply.github.com> Co-authored-by: Brandon Thomas --- contributors/people.toml | 4 ++ crates/doc/src/lib.rs | 16 ++++++ crates/engine/src/cmd/layout.rs | 14 ++++- crates/io/src/dxf_raw.rs | 3 +- crates/io/src/dxf_read.rs | 7 ++- crates/io/src/dxf_write.rs | 3 +- crates/io/tests/dwg_active_layout.rs | 82 ++++++++++++++++++++++++++++ 7 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 crates/io/tests/dwg_active_layout.rs diff --git a/contributors/people.toml b/contributors/people.toml index 856ddd8a..121a5d6f 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -76,3 +76,7 @@ real_name = "Gurparvez" [people.ffishman] sync_github_name = true + +real_name = "Rodrigo Gressler" +display_name = "Rodrigo" +sync_github_name = false diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index 8b1f66c3..eafea9e2 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -98,6 +98,9 @@ pub struct Drawing { pub blocks: BTreeMap>, pub model: EntityStore, pub layouts: Vec, + /// The layout a file marks active (its block is `*Paper_Space`, the others `*Paper_SpaceN`), + /// written active again. `None`, or a layout that no longer exists: the first by tab order. + pub active_layout: Option, pub views: Vec, /// Model space viewport configurations (VPORTS): the tiles shown and named ones. pub vports: Vec, @@ -171,6 +174,7 @@ impl Drawing { blocks: BTreeMap::new(), model: EntityStore::new(), layouts: vec![Layout::new("Layout1", 1), Layout::new("Layout2", 2)], + active_layout: None, views: Vec::new(), vports: Vec::new(), ucss: Vec::new(), @@ -298,6 +302,18 @@ impl Drawing { true } + /// The layouts in the order their paper space blocks are named in a file: the active one + /// (`*Paper_Space`) first, then the others by tab order (`*Paper_Space0`, `*Paper_Space1`, …). + pub fn paper_space_layouts(&self) -> Vec<&Layout> { + let mut ls: Vec<&Layout> = self.layouts.iter().collect(); + ls.sort_by_key(|l| l.tab_order); + if let Some(i) = self.active_layout.as_ref().and_then(|a| ls.iter().position(|l| l.name.eq_ignore_ascii_case(a))) { + let active = ls.remove(i); + ls.insert(0, active); + } + ls + } + pub fn space(&self, space: &Space) -> Option<&EntityStore> { match space { Space::Model => Some(&self.model), diff --git a/crates/engine/src/cmd/layout.rs b/crates/engine/src/cmd/layout.rs index f013ab03..a6150ed8 100644 --- a/crates/engine/src/cmd/layout.rs +++ b/crates/engine/src/cmd/layout.rs @@ -270,9 +270,13 @@ fn run_rename(s: &mut Session, p: &Value) -> Result { let from = layout_name("layout.rename", s.doc()?, &from)?; let to = str_param(p, "to").ok_or_else(|| bad("layout.rename", "`to` is required"))?; let to = check_name("layout.rename", s.doc()?, to, Some(&from))?; - if let Some(l) = s.doc_mut()?.layouts.iter_mut().find(|l| l.name == from) { + let d = s.doc_mut()?; + if let Some(l) = d.layouts.iter_mut().find(|l| l.name == from) { l.name = to.clone(); } + if d.active_layout.as_ref().is_some_and(|a| a.eq_ignore_ascii_case(&from)) { + d.active_layout = Some(to.clone()); + } let (old, new) = (Space::Paper(from.clone()), Space::Paper(to.clone())); let st = s.state_mut()?; for (sp, _) in st.views.iter_mut() { @@ -928,6 +932,14 @@ mod tests { assert!(s.doc().unwrap().layout("Layout3").is_none()); } + #[test] + fn rename_keeps_the_active_layout_whatever_its_case() { + let mut s = session_with_model(); + s.doc_mut().unwrap().active_layout = Some("layout1".into()); + s.execute("layout.rename", &json!({"from": "Layout1", "to": "Sheet A"})).unwrap(); + assert_eq!(s.doc().unwrap().active_layout.as_deref(), Some("Sheet A")); + } + #[test] fn rename_copy_delete() { let mut s = session_with_model(); diff --git a/crates/io/src/dxf_raw.rs b/crates/io/src/dxf_raw.rs index 129183ef..415398b7 100644 --- a/crates/io/src/dxf_raw.rs +++ b/crates/io/src/dxf_raw.rs @@ -913,8 +913,7 @@ fn plan_unknown(d: &Drawing, recs: &[Rec], tags: &[Tag], roles: &HashMap = d.layouts.iter().collect(); - layouts.sort_by_key(|l| l.tab_order); + let layouts = d.paper_space_layouts(); // (entities, owner, paper space, end of the space, section) let mut stores: Vec<(&cadcraft_doc::EntityStore, Option, bool, Option, &str)> = Vec::new(); let model_end = layouts.first().and_then(|l| first_at(&l.entities)).or(ents_end); diff --git a/crates/io/src/dxf_read.rs b/crates/io/src/dxf_read.rs index 48c03283..70642efa 100644 --- a/crates/io/src/dxf_read.rs +++ b/crates/io/src/dxf_read.rs @@ -1795,7 +1795,12 @@ fn read_budgeted(bytes: &[u8], mut table_cells_left: usize) -> Result { d.layouts.push(Layout::new("Layout1", 1)); paper_block_for.insert("*PAPER_SPACE".into(), "Layout1".into()); } - let first_layout = d.layouts.first().map(|l| l.name.clone()).unwrap_or_else(|| "Layout1".into()); + // The layout in `*Paper_Space` is the active one, kept when it isn't the first tab (the first + // is active by default); paper-space entities without an owner go there. + let active = paper_block_for.get("*PAPER_SPACE").cloned(); + let first_tab = d.layouts.iter().min_by_key(|l| l.tab_order).map(|l| l.name.clone()); + d.active_layout = active.clone().filter(|a| Some(a) != first_tab.as_ref()); + let first_layout = active.or_else(|| d.layouts.first().map(|l| l.name.clone())).unwrap_or_else(|| "Layout1".into()); // Blocks: model/paper space blocks hold entities in newer files. for (name, base, description, ents, xref) in blocks { let up = name.to_ascii_uppercase(); diff --git a/crates/io/src/dxf_write.rs b/crates/io/src/dxf_write.rs index bd75f70c..0434c706 100644 --- a/crates/io/src/dxf_write.rs +++ b/crates/io/src/dxf_write.rs @@ -1762,8 +1762,7 @@ fn tags_for(d: &Drawing, version: Version, target: Version) -> Vec { reactors: hatch_reactors(d), }; // Pre-allocate handles for block records. - let mut paper_layouts: Vec<&Layout> = d.layouts.iter().collect(); - paper_layouts.sort_by_key(|l| l.tab_order); + let paper_layouts = d.paper_space_layouts(); let ms_br = w.h(); // Xrefs are saved as references: without their objects and the symbols that came with them. let saved_view = crate::dxf_xref::saved_view(d); diff --git a/crates/io/tests/dwg_active_layout.rs b/crates/io/tests/dwg_active_layout.rs new file mode 100644 index 00000000..5093b2f5 --- /dev/null +++ b/crates/io/tests/dwg_active_layout.rs @@ -0,0 +1,82 @@ +//! A drawing whose active layout is not the first tab keeps it active when saved: its block is +//! the one written as `*Paper_Space` (the DWG header's paper space block), the others +//! `*Paper_SpaceN`. Saving used to make the first tab the active one again. + +use std::io::Cursor; + +use cadcraft_doc::{Common, Drawing, EntityKind, Layout, Line, Space}; +use cadcraft_geom::Vec3; +use cadcraft_io::{SaveOptions, Version, read, write_with}; + +fn three_sheets() -> Drawing { + let mut d = Drawing::new_metric(); + d.layouts = vec![Layout::new("S1", 1), Layout::new("S2", 2), Layout::new("S3", 3)]; + let line = |a: f64| EntityKind::Line(Line { a: Vec3::new(a, 0.0, 0.0), b: Vec3::new(a + 10.0, 10.0, 0.0) }); + d.add(&Space::Paper("S1".into()), Common::default(), line(0.0)).unwrap(); + d.add(&Space::Paper("S2".into()), Common::default(), line(20.0)).unwrap(); + d.add(&Space::Paper("S2".into()), Common::default(), line(40.0)).unwrap(); + d +} + +/// The layout the DWG marks active: the one whose block the header names as paper space. +fn dwg_active_layout(dwg: &[u8]) -> String { + let doc = acadrust::DwgReader::from_stream(Cursor::new(dwg.to_vec())).read().unwrap(); + let ps = doc.header.paper_space_block_handle; + doc.objects + .iter() + .find_map(|(_, o)| match o { + acadrust::objects::ObjectType::Layout(l) if l.block_record == ps => Some(l.name.clone()), + _ => None, + }) + .unwrap() +} + +fn dwg(d: &Drawing) -> Vec { + write_with(d, "case.dwg", &SaveOptions { version: Some(Version::R2018), binary: false }).unwrap() +} + +#[test] +fn the_active_layout_stays_active_through_dwg_saves() { + let mut d = three_sheets(); + d.active_layout = Some("S2".into()); + let first = dwg(&d); + assert_eq!(dwg_active_layout(&first), "S2"); + + // Opened again and saved again, as a drawing from another program would be. + let back = read(&first, "case.dwg").unwrap(); + assert_eq!(back.active_layout.as_deref(), Some("S2")); + let count = |d: &Drawing, n: &str| d.layouts.iter().find(|l| l.name == n).map_or(0, |l| l.entities.len()); + assert_eq!((count(&back, "S1"), count(&back, "S2"), count(&back, "S3")), (1, 2, 0)); + let mut tabs: Vec<(u32, &str)> = back.layouts.iter().map(|l| (l.tab_order, l.name.as_str())).collect(); + tabs.sort(); + assert_eq!(tabs.iter().map(|t| t.1).collect::>(), ["S1", "S2", "S3"], "the tab order is kept"); + + let second = dwg(&back); + assert_eq!(dwg_active_layout(&second), "S2"); + let again = read(&second, "case.dwg").unwrap(); + assert_eq!(again.active_layout.as_deref(), Some("S2")); + assert_eq!((count(&again, "S1"), count(&again, "S2"), count(&again, "S3")), (1, 2, 0)); +} + +#[test] +fn the_active_layout_survives_dxf_too() { + let mut d = three_sheets(); + d.active_layout = Some("S3".into()); + let dxf = write_with(&d, "case.dxf", &SaveOptions::default()).unwrap(); + let back = read(&dxf, "case.dxf").unwrap(); + assert_eq!(back.active_layout.as_deref(), Some("S3")); +} + +#[test] +fn without_an_active_layout_the_first_tab_is_active() { + let mut d = three_sheets(); + d.active_layout = Some("gone".into()); + assert_eq!(dwg_active_layout(&dwg(&d)), "S1"); +} + +#[test] +fn the_active_layout_name_is_matched_ignoring_case() { + let mut d = three_sheets(); + d.active_layout = Some("s2".into()); + assert_eq!(dwg_active_layout(&dwg(&d)), "S2"); +}