From b6c1ab74f99dc4c66ab38ba63178fed5a1e441ba Mon Sep 17 00:00:00 2001 From: SBaksa Date: Fri, 30 Oct 2026 10:38:00 +1100 Subject: [PATCH 01/14] DXF: leave SAT-text solids out of files of 2013 and later, with a save note (#2334) REGION, 3DSOLID and BODY read from a DXF 2000 hold SAT text; DXF 2013 and later keep ACIS geometry in the data store section, so writing the text there made a file other readers reject. Those entities are now left out of 2013+ DXF output and counted in the save note; 2010 and earlier, and DWG, are unchanged. Test: sat_text_solids_are_left_out_of_dxf_2013_and_later. --- crates/doc/src/foreign.rs | 26 +++++++++++++++++++++++++ crates/engine/src/cmd/file.rs | 26 +++++++++++++++++++++++++ crates/io/src/dxf_raw.rs | 16 ++++++++++++--- crates/io/src/dxf_write/sections.rs | 2 +- crates/io/tests/it/acis_data_store.rs | 28 +++++++++++++++++++++++++++ 5 files changed, 94 insertions(+), 4 deletions(-) diff --git a/crates/doc/src/foreign.rs b/crates/doc/src/foreign.rs index e0ebb81a4..3b6aa8ef9 100644 --- a/crates/doc/src/foreign.rs +++ b/crates/doc/src/foreign.rs @@ -94,3 +94,29 @@ impl Foreign { self.data_store.iter().filter(|t| t.code == 320).filter_map(|t| Handle::parse_hex(t.value.trim())).collect() } } + +/// Whether a solid modeler entity (3DSOLID, REGION, BODY) holds its ACIS geometry as SAT text +/// (groups 1 and 3), which DXF 2013 and later no longer has: those files keep it in the data +/// store section, so an entity with SAT text and no record there can't be written to one (#2334). +pub fn sat_text_acis(u: &crate::Unknown, handle: Handle, stored: &std::collections::BTreeSet) -> bool { + ["3DSOLID", "REGION", "BODY"].iter().any(|t| u.dxf_type.eq_ignore_ascii_case(t)) + && !stored.contains(&handle) + && u.tags.iter().take_while(|r| r.code < 1000).any(|r| r.code == 1 || r.code == 3) +} + +/// The entities a DXF of 2013 or later leaves out because they hold SAT-text ACIS data +/// ([`sat_text_acis`]), counted by DXF type, across model space, layouts and block definitions. +pub fn sat_text_dropped(d: &crate::Drawing) -> BTreeMap { + let stored = d.foreign.data_store_entities(); + let mut out = BTreeMap::new(); + let stores = std::iter::once(&d.model).chain(d.layouts.iter().map(|l| &l.entities)).chain(d.blocks.values().map(|b| &b.entities)); + for e in stores.flat_map(|s| s.iter()) { + if let crate::EntityKind::Unknown(u) = &e.kind + && sat_text_acis(u, e.handle, &stored) + { + let n: &mut usize = out.entry(u.dxf_type.to_ascii_uppercase()).or_default(); + *n = n.saturating_add(1); + } + } + out +} diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 3a5296e88..5b012bb4e 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -520,6 +520,8 @@ fn run_bytes(s: &mut Session, p: &Value) -> Result { let mut r = json!({ "data": base64_encode(&bytes), "bytes": bytes.len() }); if drops_unknown(format!("drawing.{fmt}")) { report_not_saved(s, &mut r, &lost); + } else if version >= Version::R2013 && is_dxf_file(format!("drawing.{fmt}")) { + report_sat_text_dropped(s, &mut r); } Ok(r) } @@ -575,6 +577,28 @@ fn is_drawing_file(path: impl AsRef) -> bool { Format::from_path(path).is_some_and(Format::is_drawing) } +/// Whether `path` is a DXF file. +fn is_dxf_file(path: impl AsRef) -> bool { + Format::from_path(path) == Some(Format::Dxf) +} + +/// Say on the command line which solid modeler entities a DXF of 2013 or later left out because +/// they hold SAT-text ACIS data, which such files keep in a data store instead (#2334). +fn report_sat_text_dropped(s: &mut Session, r: &mut Value) { + let lost = s.doc().map(cadcraft_doc::foreign::sat_text_dropped).unwrap_or_default(); + if lost.is_empty() { + return; + } + let list: Vec = lost.iter().map(|(t, n)| format!("{n} {t}")).collect(); + s.echo(format!( + "Not saved in the DXF file (2013 and later keep ACIS geometry in a data store these entities have no record in; save as 2010 or earlier to keep them): {}", + list.join(", ") + )); + if let Some(o) = r.as_object_mut() { + o.insert("notSaved".into(), json!(lost)); + } +} + /// Say on the command line what the file couldn't keep, and list it in the result's `notSaved`. fn report_not_saved(s: &mut Session, r: &mut Value, lost: &std::collections::BTreeMap) { let Some(msg) = not_saved_message(lost) else { return }; @@ -593,6 +617,8 @@ fn save_reporting(s: &mut Session, p: &Value, run: fn(&mut Session, &Value) -> R let mut r = run(s, p)?; if r.get("path").and_then(Value::as_str).is_some_and(drops_unknown) { report_not_saved(s, &mut r, &lost); + } else if version >= Version::R2013 && r.get("path").and_then(Value::as_str).is_some_and(is_dxf_file) { + report_sat_text_dropped(s, &mut r); } Ok(r) } diff --git a/crates/io/src/dxf_raw.rs b/crates/io/src/dxf_raw.rs index 0154b01d3..5a2aa398c 100644 --- a/crates/io/src/dxf_raw.rs +++ b/crates/io/src/dxf_raw.rs @@ -848,7 +848,7 @@ fn unknown_handles(d: &Drawing) -> HashSet { } /// Put the drawing's foreign data back into a written file's tags (see the module comment). -pub(crate) fn finish(tags: &mut Vec, d: &Drawing) { +pub(crate) fn finish(tags: &mut Vec, d: &Drawing, modern: bool) { let f = d.foreign.as_ref(); let unknown = unknown_handles(d); let xd: XdataOf = all_entities(d).filter(|e| !e.common.xdata.is_empty()).map(|e| (e.handle, e.common.xdata.as_slice())).collect(); @@ -877,7 +877,7 @@ pub(crate) fn finish(tags: &mut Vec, d: &Drawing) { } let renumber = |old: Option| class_index(old).and_then(|i| f.class_order.get(i)).and_then(|n| numbers.get(n).copied()); // Entities of types CADCraft doesn't model, placed among the written ones. - let mut planned = plan_unknown(d, &recs, tags, &roles); + let mut planned = plan_unknown(d, &recs, tags, &roles, modern); for list in planned.values_mut() { list.retain_mut(|(_, t)| { if !t.first().is_some_and(|z| is_proxy(&z.str())) { @@ -1019,7 +1019,14 @@ pub(crate) fn finish(tags: &mut Vec, d: &Drawing) { /// Where the entities of types CADCraft doesn't model go: each before the next written entity of /// its space or block (else at the end of it), as `(section, record)` lists by the index of the /// record they precede. Owners come from the written block records. -fn plan_unknown(d: &Drawing, recs: &[Rec], tags: &[Tag], roles: &HashMap) -> HashMap)>> { +fn plan_unknown( + d: &Drawing, + recs: &[Rec], + tags: &[Tag], + roles: &HashMap, + modern: bool, +) -> HashMap)>> { + let stored = d.foreign.data_store_entities(); let mut out: HashMap)>> = HashMap::new(); let body = |r: &Rec| tags.get(r.start + 1..r.end).unwrap_or(&[]); let mut at: HashMap = HashMap::new(); @@ -1057,6 +1064,9 @@ fn plan_unknown(d: &Drawing, recs: &[Rec], tags: &[Tag], roles: &HashMap { + if modern && cadcraft_doc::foreign::sat_text_acis(u, e.handle, &stored) { + continue; + } let t = unknown_entity(u, e, &owner, paper); if !t.is_empty() { pending.push((sec.to_string(), t)); diff --git a/crates/io/src/dxf_write/sections.rs b/crates/io/src/dxf_write/sections.rs index fe865515b..90214b401 100644 --- a/crates/io/src/dxf_write/sections.rs +++ b/crates/io/src/dxf_write/sections.rs @@ -1115,7 +1115,7 @@ pub(super) fn tags_for(d: &Drawing, version: Version, target: Version) -> Vec= Version::R2013); crate::dxf_raw::data_store(&mut w.t, d, target >= Version::R2013); add_class_instance_counts(&mut w.t); mark_xrefs(&mut w.t, d); diff --git a/crates/io/tests/it/acis_data_store.rs b/crates/io/tests/it/acis_data_store.rs index 74b9809ce..36c947892 100644 --- a/crates/io/tests/it/acis_data_store.rs +++ b/crates/io/tests/it/acis_data_store.rs @@ -101,3 +101,31 @@ fn dwg_keeps_the_solids_geometry() { let again = cadcraft_io::read(&dwg, "x.dwg").unwrap(); assert_eq!(again.foreign.data_store_entities().len(), 2); } + +/// A solid with SAT-text ACIS data (an R2000-style file) can't be written to a DXF of 2013 or +/// later, whose files keep that data in the data store: it is left out there and counted, while +/// 2010 and earlier keep it, and solids with a data store record are kept everywhere (#2334). +#[test] +fn sat_text_solids_are_left_out_of_dxf_2013_and_later() { + let sat = "0\nSECTION\n2\nHEADER\n9\n$ACADVER\n1\nAC1015\n0\nENDSEC\n0\nSECTION\n2\nENTITIES\n\ + 0\nREGION\n5\n2B\n100\nAcDbEntity\n8\n0\n100\nAcDbModelerGeometry\n70\n1\n1\n400 1 1 0\n1\n16 ZWCAD\n3\nEnd-of-ACIS-data\n\ + 0\n3DSOLID\n5\n2C\n100\nAcDbEntity\n8\n0\n100\nAcDbModelerGeometry\n70\n1\n1\n400 1 1 0\n3\nEnd-of-ACIS-data\n\ + 0\nLINE\n5\n2D\n8\n0\n10\n0\n20\n0\n11\n1\n21\n1\n0\nENDSEC\n0\nEOF\n"; + let d = cadcraft_io::read_dxf(sat.as_bytes()).unwrap(); + let count = |v: cadcraft_io::Version, ty: &str| { + let out = cadcraft_io::write_with(&d, "x.dxf", &cadcraft_io::SaveOptions { version: Some(v), binary: false }).unwrap(); + let t = String::from_utf8(out).unwrap().replace("\r\n", "\n"); + t.matches(&format!("\n0\n{ty}\n")).count() + t.matches(&format!("\n 0\n{ty}\n")).count() + }; + assert_eq!(cadcraft_doc::foreign::sat_text_dropped(&d), [("3DSOLID".to_string(), 1), ("REGION".to_string(), 1)].into()); + for v in [cadcraft_io::Version::R2000, cadcraft_io::Version::R2010] { + assert_eq!((count(v, "REGION"), count(v, "3DSOLID")), (1, 1), "{v:?} keeps them"); + } + for v in [cadcraft_io::Version::R2013, cadcraft_io::Version::R2018] { + assert_eq!((count(v, "REGION"), count(v, "3DSOLID")), (0, 0), "{v:?} leaves them out"); + assert_eq!(count(v, "LINE"), 1, "{v:?} keeps the rest"); + } + // Solids with a data store record keep their geometry in 2013 and later. + let stored = cadcraft_io::read_dxf(dxf().as_bytes()).unwrap(); + assert!(cadcraft_doc::foreign::sat_text_dropped(&stored).is_empty()); +} From 2eecaef6c2958cc78422c9600426fdc0f5b81004 Mon Sep 17 00:00:00 2001 From: SBaksa Date: Fri, 30 Oct 2026 10:41:07 +1100 Subject: [PATCH 02/14] Parity checklist: regenerated after merging main --- docs/parity-checklist.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index a6250d5dc..162a59e3b 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,10 +1,10 @@ # Parity checklist -> **Generated:** 2026-10-25 by `cargo xtask parity`. Do not edit by hand. +> **Generated:** 2026-10-29 by `cargo xtask parity`. Do not edit by hand. Menu-breadth parity: a reference menu item counts as covered when a CADCraft command is registered under the same menu label. It measures breadth, not depth; see [target-app-parity.md](target-app-parity.md) for the weighted assessment. -**Menu breadth: 297 / 491 items (60%)** · 442 registered commands. +**Menu breadth: 298 / 491 items (61%)** · 444 registered commands. | Menu | Items | Covered | % | |---|---|---|---| From 49fdbff529dea0f64e7d88a77dfedabe632a939d Mon Sep 17 00:00:00 2001 From: SBaksa Date: Fri, 30 Oct 2026 11:23:09 +1100 Subject: [PATCH 03/14] Test: saving SAT-text solids as DXF 2013 reports them, 2010 does not (#2334) --- crates/engine/tests/save_reports_not_saved.rs | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/crates/engine/tests/save_reports_not_saved.rs b/crates/engine/tests/save_reports_not_saved.rs index 615028a21..f231f8ace 100644 --- a/crates/engine/tests/save_reports_not_saved.rs +++ b/crates/engine/tests/save_reports_not_saved.rs @@ -3,7 +3,7 @@ //! writes them back as read: nothing to report. use cadcraft_engine::Session; -use cadcraft_engine::cmd::file::{IoHooks, set_io}; +use cadcraft_engine::cmd::file::{IoHooks, SaveAs, set_io, set_write_as}; use cadcraft_engine::doc::{Block, Common, Drawing, EntityKind, RawTag, Space, Unknown}; use serde_json::json; @@ -82,3 +82,33 @@ fn solids_whose_geometry_the_data_store_keeps_are_not_reported_from_2013() { // Before 2013 a file has no data store: every solid loses its geometry. assert_eq!(not_saved_as(&d, Version::R2010), [("3DSOLID".to_string(), 2), ("REGION".to_string(), 1)].into_iter().collect()); } + +/// A DXF of 2013 or later can't hold a solid's SAT-text geometry (it lives in the data store +/// there), so the save leaves it out and says so; 2010 and earlier keep it (#2334). +#[test] +fn saving_sat_text_solids_as_dxf_2013_reports_them() { + set_io(IoHooks { read: |_, _| Err("no reader in this test".into()), write: |_, _| Ok(b"0\nEOF\n".to_vec()), plot: None }); + set_write_as(|_, _, _: &SaveAs| Ok(b"x".to_vec())); + let mut d = Drawing::new_imperial(); + let sat = EntityKind::Unknown(Unknown { + dxf_type: "REGION".into(), + tags: vec![ + RawTag { code: 8, value: "0".into() }, + RawTag { code: 1, value: "400 1 1 0".into() }, + RawTag { code: 3, value: "End-of-ACIS-data".into() }, + ], + }); + d.add(&Space::Model, Common::default(), sat).unwrap(); + let mut s = Session::empty(); + s.open_drawing(d, "region.dxf", None); + + let path = std::env::temp_dir().join(format!("cadcraft-sat-note-{}.dxf", std::process::id())); + let r = s.execute("saveas", &json!({ "path": path.to_string_lossy(), "version": 2018 })).unwrap(); + assert_eq!(r["notSaved"], json!({ "REGION": 1 })); + assert!(s.log.iter().any(|l| l.contains("Not saved in the DXF file") && l.contains("1 REGION")), "{:?}", s.log); + + let n = s.log.len(); + let r = s.execute("saveas", &json!({ "path": path.to_string_lossy(), "version": 2010 })).unwrap(); + let _ = std::fs::remove_file(&path); + assert!(r.get("notSaved").is_none() && s.log.len() == n); +} From 89771962fcbe5f238613d107c3dfe2a1a82292a8 Mon Sep 17 00:00:00 2001 From: SBaksa Date: Fri, 30 Oct 2026 11:26:25 +1100 Subject: [PATCH 04/14] Parity checklist: regenerated after merging main --- docs/parity-checklist.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 162a59e3b..53ffffc96 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Parity checklist -> **Generated:** 2026-10-29 by `cargo xtask parity`. Do not edit by hand. +> **Generated:** 2026-10-30 by `cargo xtask parity`. Do not edit by hand. Menu-breadth parity: a reference menu item counts as covered when a CADCraft command is registered under the same menu label. It measures breadth, not depth; see [target-app-parity.md](target-app-parity.md) for the weighted assessment. From 1013b482a81067f7d9f6e1e4247070ba99b8b37b Mon Sep 17 00:00:00 2001 From: SBaksa Date: Fri, 30 Oct 2026 12:26:09 +1100 Subject: [PATCH 05/14] DXF: SAT-text note only for DXF output, from the host drawing, without xref contents (#2334) The DWG bridge writes R2000 text for the DWG writer, which keeps SAT-text solids: the skip is now keyed on the DXF's declared version, not the DWG target. The save note reads the version and count from the host drawing while the Block Editor is open, and leaves out xref blocks (path or xref| name), which the DXF writer empties anyway; the xref name rule is shared with the writer. --- crates/doc/src/foreign.rs | 10 ++++++- crates/engine/src/cmd/file.rs | 10 ++++--- crates/engine/tests/save_reports_not_saved.rs | 27 +++++++++++++++++++ crates/io/src/dxf_write/sections.rs | 4 ++- crates/io/src/dxf_xref.rs | 2 +- crates/io/tests/it/acis_data_store.rs | 27 +++++++++++++++++++ 6 files changed, 74 insertions(+), 6 deletions(-) diff --git a/crates/doc/src/foreign.rs b/crates/doc/src/foreign.rs index 3b6aa8ef9..60000c6dd 100644 --- a/crates/doc/src/foreign.rs +++ b/crates/doc/src/foreign.rs @@ -109,7 +109,9 @@ pub fn sat_text_acis(u: &crate::Unknown, handle: Handle, stored: &std::collectio pub fn sat_text_dropped(d: &crate::Drawing) -> BTreeMap { let stored = d.foreign.data_store_entities(); let mut out = BTreeMap::new(); - let stores = std::iter::once(&d.model).chain(d.layouts.iter().map(|l| &l.entities)).chain(d.blocks.values().map(|b| &b.entities)); + // The writer saves xrefs as references: their blocks (and `xref|` ones) hold nothing in the file. + let own_blocks = d.blocks.values().filter(|b| b.xref_path.is_none() && !xref_dependent(&b.name)); + let stores = std::iter::once(&d.model).chain(d.layouts.iter().map(|l| &l.entities)).chain(own_blocks.map(|b| &b.entities)); for e in stores.flat_map(|s| s.iter()) { if let crate::EntityKind::Unknown(u) = &e.kind && sat_text_acis(u, e.handle, &stored) @@ -120,3 +122,9 @@ pub fn sat_text_dropped(d: &crate::Drawing) -> BTreeMap { } out } + +/// Whether a symbol name belongs to an attached xref (`xref|name`): the DXF writer leaves such +/// symbols out, and the contents of xref blocks too ([`Block::xref_path`](crate::Block::xref_path)). +pub fn xref_dependent(name: &str) -> bool { + name.contains('|') +} diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 5b012bb4e..5d89a196e 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -490,8 +490,11 @@ fn run_saveas(s: &mut Session, p: &Value) -> Result { /// The version a drawing is saved in at `path` when none is asked for: its `$ACADVER` (2018 by /// default), and for DWG at least 2004 (the io layer's rule: R2000 DWG has no true colours). fn drawing_version(s: &Session, path: &str) -> Version { - let v = - s.doc().ok().and_then(|d| d.header.get("ACADVER").and_then(cadcraft_doc::HVal::as_str).and_then(Version::for_acadver)).unwrap_or_default(); + let v = s + .state() + .ok() + .and_then(|st| st.drawing().header.get("ACADVER").and_then(cadcraft_doc::HVal::as_str).and_then(Version::for_acadver)) + .unwrap_or_default(); if is_dxf(path) { v } else { v.max(Version::R2004) } } @@ -585,7 +588,8 @@ fn is_dxf_file(path: impl AsRef) -> bool { /// Say on the command line which solid modeler entities a DXF of 2013 or later left out because /// they hold SAT-text ACIS data, which such files keep in a data store instead (#2334). fn report_sat_text_dropped(s: &mut Session, r: &mut Value) { - let lost = s.doc().map(cadcraft_doc::foreign::sat_text_dropped).unwrap_or_default(); + // The drawing a file holds: the host while the Block Editor is open. + let lost = s.state().map(|st| cadcraft_doc::foreign::sat_text_dropped(st.drawing())).unwrap_or_default(); if lost.is_empty() { return; } diff --git a/crates/engine/tests/save_reports_not_saved.rs b/crates/engine/tests/save_reports_not_saved.rs index f231f8ace..839c2489b 100644 --- a/crates/engine/tests/save_reports_not_saved.rs +++ b/crates/engine/tests/save_reports_not_saved.rs @@ -112,3 +112,30 @@ fn saving_sat_text_solids_as_dxf_2013_reports_them() { let _ = std::fs::remove_file(&path); assert!(r.get("notSaved").is_none() && s.log.len() == n); } + +/// In the Block Editor the file still holds the host drawing: its solids and version decide the +/// note, not the edited block's (#2334). +#[test] +fn the_note_counts_the_host_drawing_while_a_block_is_edited() { + set_io(IoHooks { read: |_, _| Err("no reader in this test".into()), write: |_, _| Ok(b"0\nEOF\n".to_vec()), plot: None }); + set_write_as(|_, _, _: &SaveAs| Ok(b"x".to_vec())); + let mut d = Drawing::new_imperial(); + let sat = EntityKind::Unknown(Unknown { + dxf_type: "REGION".into(), + tags: vec![ + RawTag { code: 8, value: "0".into() }, + RawTag { code: 1, value: "400 1 1 0".into() }, + RawTag { code: 3, value: "End-of-ACIS-data".into() }, + ], + }); + d.add(&Space::Model, Common::default(), sat).unwrap(); + d.blocks.insert("Door".into(), std::sync::Arc::new(Block::new("Door"))); + let mut s = Session::empty(); + s.open_drawing(d, "region.dxf", None); + s.execute("bedit", &json!({ "name": "Door" })).unwrap(); + + let path = std::env::temp_dir().join(format!("cadcraft-sat-edit-{}.dxf", std::process::id())); + let r = s.execute("saveas", &json!({ "path": path.to_string_lossy(), "version": 2018 })).unwrap(); + let _ = std::fs::remove_file(&path); + assert_eq!(r["notSaved"], json!({ "REGION": 1 })); +} diff --git a/crates/io/src/dxf_write/sections.rs b/crates/io/src/dxf_write/sections.rs index 90214b401..815f4eb32 100644 --- a/crates/io/src/dxf_write/sections.rs +++ b/crates/io/src/dxf_write/sections.rs @@ -1115,7 +1115,9 @@ pub(super) fn tags_for(d: &Drawing, version: Version, target: Version) -> Vec= Version::R2013); + // Only a DXF declares the version it is written for: the DWG bridge writes R2000 text for the + // DWG writer, which keeps SAT-text solids (#2334). + crate::dxf_raw::finish(&mut w.t, d, version >= Version::R2013); crate::dxf_raw::data_store(&mut w.t, d, target >= Version::R2013); add_class_instance_counts(&mut w.t); mark_xrefs(&mut w.t, d); diff --git a/crates/io/src/dxf_xref.rs b/crates/io/src/dxf_xref.rs index f98a044f7..9ea7904cd 100644 --- a/crates/io/src/dxf_xref.rs +++ b/crates/io/src/dxf_xref.rs @@ -13,7 +13,7 @@ use cadcraft_doc::Drawing; /// Whether a symbol name belongs to an attached xref (`xref|name`). pub(crate) fn dependent(name: &str) -> bool { - name.contains('|') + cadcraft_doc::foreign::xref_dependent(name) } /// The drawing as it is saved: xref blocks emptied, xref-dependent symbols other than layers and diff --git a/crates/io/tests/it/acis_data_store.rs b/crates/io/tests/it/acis_data_store.rs index 36c947892..308b532d9 100644 --- a/crates/io/tests/it/acis_data_store.rs +++ b/crates/io/tests/it/acis_data_store.rs @@ -129,3 +129,30 @@ fn sat_text_solids_are_left_out_of_dxf_2013_and_later() { let stored = cadcraft_io::read_dxf(dxf().as_bytes()).unwrap(); assert!(cadcraft_doc::foreign::sat_text_dropped(&stored).is_empty()); } + +/// A DWG keeps SAT-text solids at any version (its writer handles them), and a DXF's note leaves +/// out the contents of xref blocks, which the DXF doesn't hold anyway (#2334). +#[test] +fn sat_text_solids_stay_in_dwg_2018_and_xref_contents_are_not_counted() { + let sat = "0\nSECTION\n2\nHEADER\n9\n$ACADVER\n1\nAC1015\n0\nENDSEC\n0\nSECTION\n2\nENTITIES\n\ + 0\nREGION\n5\n2B\n100\nAcDbEntity\n8\n0\n100\nAcDbModelerGeometry\n70\n1\n1\n400 1 1 0\n3\nEnd-of-ACIS-data\n\ + 0\nLINE\n5\n2D\n8\n0\n10\n0\n20\n0\n11\n1\n21\n1\n0\nENDSEC\n0\nEOF\n"; + let d = cadcraft_io::read_dxf(sat.as_bytes()).unwrap(); + let opts = cadcraft_io::SaveOptions { version: Some(cadcraft_io::Version::R2018), binary: false }; + let dwg = cadcraft_io::write_with(&d, "x.dwg", &opts).unwrap(); + let back = cadcraft_io::read(&dwg, "x.dwg").unwrap(); + let unknown = |d: &cadcraft_doc::Drawing| d.model.iter().filter(|e| matches!(&e.kind, EntityKind::Unknown(u) if u.dxf_type == "REGION")).count(); + assert_eq!(unknown(&back), 1, "the DWG 2018 keeps the region"); + + // Xref blocks (and `xref|` ones) hold no entities in the file: not reported as left out. + let mut x = d.clone(); + let region = d.model.iter().find(|e| matches!(&e.kind, EntityKind::Unknown(_))).cloned().unwrap(); + let mut host = cadcraft_doc::Block::new("REF"); + host.xref_path = Some("other.dwg".into()); + host.entities.push((*region).clone()); + x.blocks.insert("REF".into(), std::sync::Arc::new(host)); + let mut dep = cadcraft_doc::Block::new("REF|PART"); + dep.entities.push((*region).clone()); + x.blocks.insert("REF|PART".into(), std::sync::Arc::new(dep)); + assert_eq!(cadcraft_doc::foreign::sat_text_dropped(&x), [("REGION".to_string(), 1)].into(), "only the model-space region"); +} From fc8270d8c52acd6dba69e221b889193b1a497739 Mon Sep 17 00:00:00 2001 From: SBaksa Date: Fri, 30 Oct 2026 12:47:03 +1100 Subject: [PATCH 06/14] Parity checklist: regenerated after merging main --- docs/parity-checklist.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index d0bd6f967..53ffffc96 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,7 +1,6 @@ # Parity checklist -> **Generated:** 2026-10-29 by `cargo xtask parity`. Do not edit by hand. -> **Generated:** 2026-10-28 by `cargo xtask parity`. Do not edit by hand. +> **Generated:** 2026-10-30 by `cargo xtask parity`. Do not edit by hand. Menu-breadth parity: a reference menu item counts as covered when a CADCraft command is registered under the same menu label. It measures breadth, not depth; see [target-app-parity.md](target-app-parity.md) for the weighted assessment. From 86d010eaac0d458a528d91705c0a2118f80fa528 Mon Sep 17 00:00:00 2001 From: SBaksa Date: Fri, 30 Oct 2026 14:59:16 +1100 Subject: [PATCH 07/14] DXF: SAT text is groups 1 and 3 of the modeler subclass; only ASM_Data records keep geometry (#2334) sat_text_acis now looks for groups 1 and 3 inside AcDbModelerGeometry only (all groups when a file has no subclass markers), and the data store counts an entity as stored only when its ACDSRECORD holds ASM_Data. The writer and the reader share one record splitter (data_store_records). --- crates/doc/src/foreign.rs | 46 ++++++++++++++++++++++++--- crates/io/src/dxf_raw.rs | 6 +--- crates/io/tests/it/acis_data_store.rs | 29 +++++++++++++++++ 3 files changed, 71 insertions(+), 10 deletions(-) diff --git a/crates/doc/src/foreign.rs b/crates/doc/src/foreign.rs index 60000c6dd..133c525e2 100644 --- a/crates/doc/src/foreign.rs +++ b/crates/doc/src/foreign.rs @@ -89,9 +89,17 @@ impl Foreign { && self.data_store.is_empty() } - /// The entities the data store has records for (their 320 handles). + /// The entities with a geometry record in the data store: an `ACDSRECORD` naming the entity + /// (320) that holds its `ASM_Data`. A 320 without one doesn't keep an entity's geometry. pub fn data_store_entities(&self) -> std::collections::BTreeSet { - self.data_store.iter().filter(|t| t.code == 320).filter_map(|t| Handle::parse_hex(t.value.trim())).collect() + let mut out = std::collections::BTreeSet::new(); + for rec in data_store_records(&self.data_store) { + let is_record = rec.first().is_some_and(|t| t.value.trim().eq_ignore_ascii_case("ACDSRECORD")); + if is_record && rec.iter().any(|t| t.code == 2 && t.value.trim().eq_ignore_ascii_case("ASM_Data")) { + out.extend(rec.iter().filter(|t| t.code == 320).filter_map(|t| Handle::parse_hex(t.value.trim()))); + } + } + out } } @@ -99,9 +107,7 @@ impl Foreign { /// (groups 1 and 3), which DXF 2013 and later no longer has: those files keep it in the data /// store section, so an entity with SAT text and no record there can't be written to one (#2334). pub fn sat_text_acis(u: &crate::Unknown, handle: Handle, stored: &std::collections::BTreeSet) -> bool { - ["3DSOLID", "REGION", "BODY"].iter().any(|t| u.dxf_type.eq_ignore_ascii_case(t)) - && !stored.contains(&handle) - && u.tags.iter().take_while(|r| r.code < 1000).any(|r| r.code == 1 || r.code == 3) + ["3DSOLID", "REGION", "BODY"].iter().any(|t| u.dxf_type.eq_ignore_ascii_case(t)) && !stored.contains(&handle) && has_sat_text(&u.tags) } /// The entities a DXF of 2013 or later leaves out because they hold SAT-text ACIS data @@ -128,3 +134,33 @@ pub fn sat_text_dropped(d: &crate::Drawing) -> BTreeMap { pub fn xref_dependent(name: &str) -> bool { name.contains('|') } + +/// The records of a data store section's groups: each starts at a `0` group and runs to the next +/// (the section's own groups before the first record are not part of one). +pub fn data_store_records(tags: &[RawTag]) -> Vec<&[RawTag]> { + let mut out = Vec::new(); + let mut start = tags.iter().position(|t| t.code == 0).unwrap_or(tags.len()); + while start < tags.len() { + let end = tags.iter().skip(start + 1).position(|t| t.code == 0).map_or(tags.len(), |k| start + 1 + k); + out.push(tags.get(start..end).unwrap_or(&[])); + start = end; + } + out +} + +/// Whether an entity's groups hold SAT text: groups 1 and 3 inside its `AcDbModelerGeometry` +/// subclass. A file of a version before the subclass markers has no `100` groups: all of its +/// groups (before the extended data) are the entity's. +fn has_sat_text(tags: &[RawTag]) -> bool { + let own = tags.iter().take_while(|r| r.code < 1000); + let marked = tags.iter().take_while(|r| r.code < 1000).any(|r| r.code == 100); + let mut in_modeler = !marked; + for r in own { + if r.code == 100 { + in_modeler = r.value.trim().eq_ignore_ascii_case("AcDbModelerGeometry"); + } else if in_modeler && (r.code == 1 || r.code == 3) { + return true; + } + } + false +} diff --git a/crates/io/src/dxf_raw.rs b/crates/io/src/dxf_raw.rs index 5a2aa398c..7856a24b8 100644 --- a/crates/io/src/dxf_raw.rs +++ b/crates/io/src/dxf_raw.rs @@ -601,17 +601,13 @@ pub(crate) fn data_store(tags: &mut Vec, d: &Drawing, modern: bool) { let first = ds.iter().position(|t| t.code == 0).unwrap_or(ds.len()); out.extend(ds.iter().take(first).map(tag)); let mut records = 0usize; - let mut i = first; - while i < ds.len() { - let end = ds.iter().skip(i + 1).position(|t| t.code == 0).map_or(ds.len(), |k| i + 1 + k); - let rec = ds.get(i..end).unwrap_or(&[]); + for rec in cadcraft_doc::foreign::data_store_records(ds) { let is_record = rec.first().is_some_and(|t| t.value.trim().eq_ignore_ascii_case("ACDSRECORD")); let owner = rec.iter().find(|t| t.code == 320).map(|t| norm(&t.value)); if !is_record || owner.as_ref().is_some_and(|h| written.contains(h)) { records += usize::from(is_record); out.extend(rec.iter().map(tag)); } - i = end; } if records == 0 { return; diff --git a/crates/io/tests/it/acis_data_store.rs b/crates/io/tests/it/acis_data_store.rs index 308b532d9..4f8f455b4 100644 --- a/crates/io/tests/it/acis_data_store.rs +++ b/crates/io/tests/it/acis_data_store.rs @@ -156,3 +156,32 @@ fn sat_text_solids_stay_in_dwg_2018_and_xref_contents_are_not_counted() { x.blocks.insert("REF|PART".into(), std::sync::Arc::new(dep)); assert_eq!(cadcraft_doc::foreign::sat_text_dropped(&x), [("REGION".to_string(), 1)].into(), "only the model-space region"); } + +/// Only groups 1 and 3 of the modeler geometry subclass are SAT text, and only a record holding +/// the entity's ASM_Data keeps its geometry in the data store (#2334). +#[test] +fn sat_text_is_told_by_its_subclass_and_a_geometry_record() { + use cadcraft_doc::foreign::{Foreign, sat_text_acis}; + use cadcraft_doc::{Handle, RawTag, Unknown}; + let tag = |code: i32, value: &str| RawTag { code, value: value.into() }; + let solid = |tags: Vec| Unknown { dxf_type: "3DSOLID".into(), tags }; + let none = std::collections::BTreeSet::new(); + let h = Handle(0x2B); + // A stray 1 or 3 outside AcDbModelerGeometry is not SAT text. + let stray = + solid(vec![tag(100, "AcDbEntity"), tag(1, "name"), tag(100, "AcDbModelerGeometry"), tag(70, "1"), tag(100, "AcDb3dSolid"), tag(3, "x")]); + assert!(!sat_text_acis(&stray, h, &none)); + let real = solid(vec![tag(100, "AcDbEntity"), tag(100, "AcDbModelerGeometry"), tag(70, "1"), tag(1, "400 1 1 0"), tag(100, "AcDb3dSolid")]); + assert!(sat_text_acis(&real, h, &none)); + // Without subclass markers (an old file) the groups are the entity's own. + assert!(sat_text_acis(&solid(vec![tag(8, "0"), tag(1, "400 1 1 0")]), h, &none)); + + // A 320 in the data store without an ASM_Data record doesn't keep the geometry. + let store = |asm: bool| { + let mut t = vec![tag(0, "ACDSRECORD"), tag(90, "1"), tag(320, "2B")]; + t.push(tag(2, if asm { "ASM_Data" } else { "Other" })); + Foreign { data_store: t, ..Foreign::default() } + }; + assert!(store(true).data_store_entities().contains(&h)); + assert!(store(false).data_store_entities().is_empty()); +} From 590b97a3211743a780de5996f7e2dadb1918879d Mon Sep 17 00:00:00 2001 From: SBaksa Date: Fri, 30 Oct 2026 18:42:24 +1100 Subject: [PATCH 08/14] Save note: QSAVE reports by the version it writes, not a `version` it ignores (#2334) QSAVE takes no version and writes the drawing's own, but the SAT-text note read a `version` passed to it, so `qsave {version: 2010}` on a 2018 drawing skipped the note while still leaving the solids out. Only SAVEAS's requested version now picks the reporting branch. Test: saving_sat_text_solids_as_dxf_2013_reports_them (qsave case fails without the fix). --- crates/engine/src/cmd/file.rs | 10 ++++++---- crates/engine/tests/save_reports_not_saved.rs | 5 +++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 28c8d66be..e91bbd9da 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -613,10 +613,11 @@ fn report_not_saved(s: &mut Session, r: &mut Value, lost: &std::collections::BTr } /// Save, then report the entities the file couldn't keep (nothing is dropped silently). -fn save_reporting(s: &mut Session, p: &Value, run: fn(&mut Session, &Value) -> Result) -> Result { +fn save_reporting(s: &mut Session, p: &Value, run: fn(&mut Session, &Value) -> Result, asked: Option) -> Result { // What the file will hold depends on the version it is saved as (the data store from 2013). + // QSAVE takes no `version` and keeps the drawing's own; SAVEAS writes the one asked for. let path = str_param(p, "path").map(str::to_string).or_else(|| s.state().ok().and_then(|st| st.path.clone())).unwrap_or_default(); - let version = save_options("save", p).ok().and_then(|o| o.version).unwrap_or_else(|| drawing_version(s, &path)); + let version = asked.unwrap_or_else(|| drawing_version(s, &path)); let lost = s.doc().map(|d| not_saved_as(d, version)).unwrap_or_default(); let mut r = run(s, p)?; if r.get("path").and_then(Value::as_str).is_some_and(drops_unknown) { @@ -628,11 +629,12 @@ fn save_reporting(s: &mut Session, p: &Value, run: fn(&mut Session, &Value) -> R } fn run_qsave_reporting(s: &mut Session, p: &Value) -> Result { - save_reporting(s, p, run_save) + save_reporting(s, p, run_save, None) } fn run_saveas_reporting(s: &mut Session, p: &Value) -> Result { - save_reporting(s, p, run_saveas) + let asked = save_options("saveas", p)?.version; + save_reporting(s, p, run_saveas, asked) } const B64: &[u8; 64] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; diff --git a/crates/engine/tests/save_reports_not_saved.rs b/crates/engine/tests/save_reports_not_saved.rs index 839c2489b..5472254a1 100644 --- a/crates/engine/tests/save_reports_not_saved.rs +++ b/crates/engine/tests/save_reports_not_saved.rs @@ -107,6 +107,11 @@ fn saving_sat_text_solids_as_dxf_2013_reports_them() { assert_eq!(r["notSaved"], json!({ "REGION": 1 })); assert!(s.log.iter().any(|l| l.contains("Not saved in the DXF file") && l.contains("1 REGION")), "{:?}", s.log); + // QSAVE takes no version: a `version` passed to it doesn't change what it writes (the + // drawing's own 2018), so the note still comes. + let r = s.execute("qsave", &json!({ "path": path.to_string_lossy(), "version": 2010 })).unwrap(); + assert_eq!(r["notSaved"], json!({ "REGION": 1 })); + let n = s.log.len(); let r = s.execute("saveas", &json!({ "path": path.to_string_lossy(), "version": 2010 })).unwrap(); let _ = std::fs::remove_file(&path); From 7baa9703818416d132e88e0978c8de83a75ef648 Mon Sep 17 00:00:00 2001 From: SBaksa Date: Fri, 30 Oct 2026 19:18:41 +1100 Subject: [PATCH 09/14] Save note: a data store record stores its first entity; a DWG keeps SAT-text solids (#2334) The writer keeps an ACDSRECORD by its first 320 only, so data_store_entities now credits that handle alone (one shared rule, data_store_owner). The DWG writer converts a solid's SAT text itself at any version, so not_saved_as no longer lists solids that hold SAT text as left out of a DWG. --- crates/doc/src/foreign.rs | 10 ++++++++-- crates/engine/src/cmd/file.rs | 6 +++++- crates/engine/tests/save_reports_not_saved.rs | 15 +++++++++++++++ crates/io/src/dxf_raw.rs | 2 +- crates/io/tests/it/acis_data_store.rs | 11 +++++++++++ 5 files changed, 40 insertions(+), 4 deletions(-) diff --git a/crates/doc/src/foreign.rs b/crates/doc/src/foreign.rs index 133c525e2..dbae3ab49 100644 --- a/crates/doc/src/foreign.rs +++ b/crates/doc/src/foreign.rs @@ -96,7 +96,7 @@ impl Foreign { for rec in data_store_records(&self.data_store) { let is_record = rec.first().is_some_and(|t| t.value.trim().eq_ignore_ascii_case("ACDSRECORD")); if is_record && rec.iter().any(|t| t.code == 2 && t.value.trim().eq_ignore_ascii_case("ASM_Data")) { - out.extend(rec.iter().filter(|t| t.code == 320).filter_map(|t| Handle::parse_hex(t.value.trim()))); + out.extend(data_store_owner(rec).and_then(|t| Handle::parse_hex(t.value.trim()))); } } out @@ -151,7 +151,7 @@ pub fn data_store_records(tags: &[RawTag]) -> Vec<&[RawTag]> { /// Whether an entity's groups hold SAT text: groups 1 and 3 inside its `AcDbModelerGeometry` /// subclass. A file of a version before the subclass markers has no `100` groups: all of its /// groups (before the extended data) are the entity's. -fn has_sat_text(tags: &[RawTag]) -> bool { +pub fn has_sat_text(tags: &[RawTag]) -> bool { let own = tags.iter().take_while(|r| r.code < 1000); let marked = tags.iter().take_while(|r| r.code < 1000).any(|r| r.code == 100); let mut in_modeler = !marked; @@ -164,3 +164,9 @@ fn has_sat_text(tags: &[RawTag]) -> bool { } false } + +/// The group naming the entity a data store record belongs to: its first `320`. A record is kept +/// or dropped by this handle alone, in the writer and wherever the store is read. +pub fn data_store_owner(record: &[RawTag]) -> Option<&RawTag> { + record.iter().find(|t| t.code == 320) +} diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index e91bbd9da..0bccbfb4f 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -550,7 +550,11 @@ pub fn not_saved_as(d: &Drawing, version: Version) -> std::collections::BTreeMap let stores = std::iter::once(&d.model).chain(d.layouts.iter().map(|l| &l.entities)).chain(d.blocks.values().map(|b| &b.entities)); for e in stores.flat_map(|s| s.iter()) { if let cadcraft_doc::EntityKind::Unknown(u) = &e.kind { - if MODELER_TYPES.iter().any(|t| u.dxf_type.eq_ignore_ascii_case(t)) && stored.contains(&e.handle) { + // A DWG writer converts SAT text to its own form at any version, and from 2013 a data + // store record keeps the geometry too: neither loses the solid. + if MODELER_TYPES.iter().any(|t| u.dxf_type.eq_ignore_ascii_case(t)) + && (stored.contains(&e.handle) || cadcraft_doc::foreign::has_sat_text(&u.tags)) + { continue; } let n = out.entry(u.dxf_type.clone()).or_insert(0); diff --git a/crates/engine/tests/save_reports_not_saved.rs b/crates/engine/tests/save_reports_not_saved.rs index 5472254a1..d50b1a503 100644 --- a/crates/engine/tests/save_reports_not_saved.rs +++ b/crates/engine/tests/save_reports_not_saved.rs @@ -144,3 +144,18 @@ fn the_note_counts_the_host_drawing_while_a_block_is_edited() { let _ = std::fs::remove_file(&path); assert_eq!(r["notSaved"], json!({ "REGION": 1 })); } + +/// A DWG writer converts a solid's SAT text itself (at any version), so a DWG save doesn't list +/// solids that hold it as left out; ones with neither SAT text nor a data store record are (#2334). +#[test] +fn a_dwg_keeps_solids_that_hold_sat_text() { + use cadcraft_engine::cmd::file::{Version, not_saved_as}; + let tag = |code: i32, value: &str| RawTag { code, value: value.into() }; + let solid = |tags: Vec| EntityKind::Unknown(Unknown { dxf_type: "3DSOLID".into(), tags }); + let mut d = Drawing::new_imperial(); + d.add(&Space::Model, Common::default(), solid(vec![tag(100, "AcDbModelerGeometry"), tag(1, "400 1 1 0")])).unwrap(); + d.add(&Space::Model, Common::default(), solid(vec![tag(100, "AcDbModelerGeometry"), tag(70, "1")])).unwrap(); + for v in [Version::R2010, Version::R2018] { + assert_eq!(not_saved_as(&d, v), [("3DSOLID".to_string(), 1)].into_iter().collect(), "{v:?}"); + } +} diff --git a/crates/io/src/dxf_raw.rs b/crates/io/src/dxf_raw.rs index 7856a24b8..9efb5b07d 100644 --- a/crates/io/src/dxf_raw.rs +++ b/crates/io/src/dxf_raw.rs @@ -603,7 +603,7 @@ pub(crate) fn data_store(tags: &mut Vec, d: &Drawing, modern: bool) { let mut records = 0usize; for rec in cadcraft_doc::foreign::data_store_records(ds) { let is_record = rec.first().is_some_and(|t| t.value.trim().eq_ignore_ascii_case("ACDSRECORD")); - let owner = rec.iter().find(|t| t.code == 320).map(|t| norm(&t.value)); + let owner = cadcraft_doc::foreign::data_store_owner(rec).map(|t| norm(&t.value)); if !is_record || owner.as_ref().is_some_and(|h| written.contains(h)) { records += usize::from(is_record); out.extend(rec.iter().map(tag)); diff --git a/crates/io/tests/it/acis_data_store.rs b/crates/io/tests/it/acis_data_store.rs index 4f8f455b4..5bb4bc8ee 100644 --- a/crates/io/tests/it/acis_data_store.rs +++ b/crates/io/tests/it/acis_data_store.rs @@ -185,3 +185,14 @@ fn sat_text_is_told_by_its_subclass_and_a_geometry_record() { assert!(store(true).data_store_entities().contains(&h)); assert!(store(false).data_store_entities().is_empty()); } + +/// The writer keeps a data store record by its first 320 only, so only that entity counts as +/// stored (#2334). +#[test] +fn a_data_store_record_stores_only_its_first_entity() { + use cadcraft_doc::foreign::Foreign; + use cadcraft_doc::{Handle, RawTag}; + let tag = |code: i32, value: &str| RawTag { code, value: value.into() }; + let store = Foreign { data_store: vec![tag(0, "ACDSRECORD"), tag(320, "2B"), tag(320, "2C"), tag(2, "ASM_Data")], ..Foreign::default() }; + assert_eq!(store.data_store_entities(), [Handle(0x2B)].into()); +} From 282ad11e60f22d4d7bc93d300eeff985a508df8a Mon Sep 17 00:00:00 2001 From: SBaksa Date: Sat, 31 Oct 2026 12:28:21 +1100 Subject: [PATCH 10/14] Save note: a DWG loses SAT-text solids from 2013 on, and the note says so (#2334) A real DWG write and read-back keeps a SAT-text REGION's groups up to 2010 and returns it without them at 2013 and 2018, so the SAT-text skip in not_saved_as applies before 2013 only. The DWG note now says solids are kept in a DXF only when saved as 2010 or earlier. --- crates/engine/src/cmd/file.rs | 11 ++++++---- crates/engine/tests/save_reports_not_saved.rs | 14 ++++++------- crates/io/tests/it/acis_data_store.rs | 21 +++++++++++++++++++ 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 0bccbfb4f..683bd26f8 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -550,10 +550,10 @@ pub fn not_saved_as(d: &Drawing, version: Version) -> std::collections::BTreeMap let stores = std::iter::once(&d.model).chain(d.layouts.iter().map(|l| &l.entities)).chain(d.blocks.values().map(|b| &b.entities)); for e in stores.flat_map(|s| s.iter()) { if let cadcraft_doc::EntityKind::Unknown(u) = &e.kind { - // A DWG writer converts SAT text to its own form at any version, and from 2013 a data - // store record keeps the geometry too: neither loses the solid. + // A DWG before 2013 writes a solid's SAT text as it is; from 2013 the geometry has to be + // in the data store, and SAT text alone is lost (#2334). if MODELER_TYPES.iter().any(|t| u.dxf_type.eq_ignore_ascii_case(t)) - && (stored.contains(&e.handle) || cadcraft_doc::foreign::has_sat_text(&u.tags)) + && (stored.contains(&e.handle) || (version < Version::R2013 && cadcraft_doc::foreign::has_sat_text(&u.tags))) { continue; } @@ -570,7 +570,10 @@ pub fn not_saved_message(lost: &std::collections::BTreeMap) -> Op return None; } let list: Vec = lost.iter().map(|(t, n)| format!("{n} {t}")).collect(); - Some(format!("Not saved in the DWG file (its writer can't write these object types yet; DXF keeps them): {}", list.join(", "))) + Some(format!( + "Not saved in the DWG file (its writer can't write these object types yet; DXF keeps them, solids only when saved as 2010 or earlier): {}", + list.join(", ") + )) } /// Whether saving to `path` leaves out the entities [`not_saved`] counts: DWG does (the DWG diff --git a/crates/engine/tests/save_reports_not_saved.rs b/crates/engine/tests/save_reports_not_saved.rs index d50b1a503..295877d5c 100644 --- a/crates/engine/tests/save_reports_not_saved.rs +++ b/crates/engine/tests/save_reports_not_saved.rs @@ -29,7 +29,7 @@ fn save_lists_the_entities_the_file_does_not_keep() { let r = s.execute("saveas", &json!({ "path": path.to_string_lossy() })).unwrap(); let _ = std::fs::remove_file(&path); assert_eq!(r["notSaved"], json!({ "3DSOLID": 1, "MLINE": 2, "REGION": 1 })); - let msg = "Not saved in the DWG file (its writer can't write these object types yet; DXF keeps them): 1 3DSOLID, 2 MLINE, 1 REGION"; + let msg = "Not saved in the DWG file (its writer can't write these object types yet; DXF keeps them, solids only when saved as 2010 or earlier): 1 3DSOLID, 2 MLINE, 1 REGION"; assert!(s.log.iter().any(|l| l == msg), "{:?}", s.log); // DXF keeps them. @@ -145,17 +145,17 @@ fn the_note_counts_the_host_drawing_while_a_block_is_edited() { assert_eq!(r["notSaved"], json!({ "REGION": 1 })); } -/// A DWG writer converts a solid's SAT text itself (at any version), so a DWG save doesn't list -/// solids that hold it as left out; ones with neither SAT text nor a data store record are (#2334). +/// A DWG before 2013 writes a solid's SAT text as it is, so a save there doesn't list a solid that +/// holds it; from 2013 the geometry must be in the data store and SAT text alone is lost, so the +/// save lists it. A solid with neither is always listed (#2334). #[test] -fn a_dwg_keeps_solids_that_hold_sat_text() { +fn a_dwg_keeps_sat_text_solids_only_before_2013() { use cadcraft_engine::cmd::file::{Version, not_saved_as}; let tag = |code: i32, value: &str| RawTag { code, value: value.into() }; let solid = |tags: Vec| EntityKind::Unknown(Unknown { dxf_type: "3DSOLID".into(), tags }); let mut d = Drawing::new_imperial(); d.add(&Space::Model, Common::default(), solid(vec![tag(100, "AcDbModelerGeometry"), tag(1, "400 1 1 0")])).unwrap(); d.add(&Space::Model, Common::default(), solid(vec![tag(100, "AcDbModelerGeometry"), tag(70, "1")])).unwrap(); - for v in [Version::R2010, Version::R2018] { - assert_eq!(not_saved_as(&d, v), [("3DSOLID".to_string(), 1)].into_iter().collect(), "{v:?}"); - } + assert_eq!(not_saved_as(&d, Version::R2010), [("3DSOLID".to_string(), 1)].into_iter().collect(), "2010 keeps the SAT-text solid"); + assert_eq!(not_saved_as(&d, Version::R2018), [("3DSOLID".to_string(), 2)].into_iter().collect(), "2018 loses it"); } diff --git a/crates/io/tests/it/acis_data_store.rs b/crates/io/tests/it/acis_data_store.rs index 5bb4bc8ee..f0ad2039c 100644 --- a/crates/io/tests/it/acis_data_store.rs +++ b/crates/io/tests/it/acis_data_store.rs @@ -196,3 +196,24 @@ fn a_data_store_record_stores_only_its_first_entity() { let store = Foreign { data_store: vec![tag(0, "ACDSRECORD"), tag(320, "2B"), tag(320, "2C"), tag(2, "ASM_Data")], ..Foreign::default() }; assert_eq!(store.data_store_entities(), [Handle(0x2B)].into()); } + +/// A real DWG round trip of a SAT-text REGION: the DWG writer keeps the SAT groups up to 2010, and +/// from 2013 they come back empty (the solid's geometry is not in the file), which the DWG save +/// note has to say (#2334). +#[test] +fn a_dwg_keeps_a_sat_text_region_only_before_2013() { + let sat = "0\nSECTION\n2\nHEADER\n9\n$ACADVER\n1\nAC1015\n0\nENDSEC\n0\nSECTION\n2\nENTITIES\n\ + 0\nREGION\n5\n2B\n100\nAcDbEntity\n8\n0\n100\nAcDbModelerGeometry\n70\n1\n1\n400 1 1 0\n3\nEnd-of-ACIS-data\n0\nENDSEC\n0\nEOF\n"; + let d = cadcraft_io::read_dxf(sat.as_bytes()).unwrap(); + let holds_sat = |v: cadcraft_io::Version| { + let dwg = cadcraft_io::write_with(&d, "x.dwg", &cadcraft_io::SaveOptions { version: Some(v), binary: false }).unwrap(); + let back = cadcraft_io::read(&dwg, "x.dwg").unwrap(); + let region = back.model.iter().find_map(|e| match &e.kind { + EntityKind::Unknown(u) if u.dxf_type == "REGION" => Some(u.clone()), + _ => None, + }); + region.is_some_and(|u| cadcraft_doc::foreign::has_sat_text(&u.tags)) + }; + assert!(holds_sat(cadcraft_io::Version::R2010), "2010 keeps the SAT text"); + assert!(!holds_sat(cadcraft_io::Version::R2018), "2018 comes back without it"); +} From 57e6a89d10a8bff40391fcfa0a697d71ea8ab26d Mon Sep 17 00:00:00 2001 From: SBaksa Date: Sat, 31 Oct 2026 12:31:34 +1100 Subject: [PATCH 11/14] Parity checklist: regenerated after merging main --- docs/parity-checklist.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 53ffffc96..f842c51a9 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,10 +1,10 @@ # Parity checklist -> **Generated:** 2026-10-30 by `cargo xtask parity`. Do not edit by hand. +> **Generated:** 2026-10-31 by `cargo xtask parity`. Do not edit by hand. Menu-breadth parity: a reference menu item counts as covered when a CADCraft command is registered under the same menu label. It measures breadth, not depth; see [target-app-parity.md](target-app-parity.md) for the weighted assessment. -**Menu breadth: 298 / 491 items (61%)** · 444 registered commands. +**Menu breadth: 298 / 491 items (61%)** · 446 registered commands. | Menu | Items | Covered | % | |---|---|---|---| From 97679d8cffa6d0d57b84d52eb25b18a14d429ebd Mon Sep 17 00:00:00 2001 From: Tanish Manku Date: Mon, 2 Nov 2026 14:29:51 -0700 Subject: [PATCH 12/14] Ask before a save drops solids from a 2013+ DXF (#2334) The interactive save only knew about DWG, so overwriting a 2013+ DXF lost SAT-text solids on a command-line note. loss_on_save now lists them, and EXPORT reports them with the DXF wording. --- crates/engine/src/cmd/exchange.rs | 3 +- crates/engine/src/cmd/file.rs | 18 ++++++++++- crates/engine/tests/save_reports_not_saved.rs | 32 +++++++++++++++++++ crates/ui-egui/src/save_loss.rs | 10 ++++-- 4 files changed, 58 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/exchange.rs b/crates/engine/src/cmd/exchange.rs index d4993566a..1be19a408 100644 --- a/crates/engine/src/cmd/exchange.rs +++ b/crates/engine/src/cmd/exchange.rs @@ -118,7 +118,6 @@ fn run_export(s: &mut Session, p: &Value) -> Result { return Err(bad("export", "`format` must be dxf, dwg, svg, png or pdf (or give a `path` with one of those extensions)")); } let file = format!("export.{format}"); - let lost = super::file::loss_on_save(s, &file); let bytes = (hooks.write)(s.doc()?, &file).map_err(|e| bad("export", e))?; let mut r = match path { Some(path) => { @@ -127,7 +126,7 @@ fn run_export(s: &mut Session, p: &Value) -> Result { } None => json!({ "data": base64_encode(&bytes), "format": format, "bytes": bytes.len() }), }; - super::file::report_not_saved(s, &mut r, &lost); + super::file::report_loss(s, &mut r, &file); Ok(r) } diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 80244bc21..012a1d381 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -794,13 +794,29 @@ pub fn not_saved_message(lost: &std::collections::BTreeMap) -> Op /// when the format keeps everything. The interactive save asks with this; [`save_reporting`] /// reports the same list afterwards to callers that never get a dialog. pub fn loss_on_save(s: &Session, path: &str) -> std::collections::BTreeMap { + let version = drawing_version(s, path); + if is_dxf_file(path) && version >= Version::R2013 { + // The drawing a file holds: the host while the Block Editor is open. + return s.state().map(|st| cadcraft_doc::foreign::sat_text_dropped(st.drawing())).unwrap_or_default(); + } if !drops_unknown(path) { return Default::default(); } - let version = drawing_version(s, path); s.doc().map(|d| not_saved_as(d, version)).unwrap_or_default() } +/// Report what the file `path`, just written at the drawing's own version, left out. +pub(super) fn report_loss(s: &mut Session, r: &mut Value, path: &str) { + if is_dxf_file(path) { + if drawing_version(s, path) >= Version::R2013 { + report_sat_text_dropped(s, r); + } + } else { + let lost = loss_on_save(s, path); + report_not_saved(s, r, &lost); + } +} + /// Whether saving to `path` leaves out the entities [`not_saved`] counts: DWG does (the DWG /// bridge drops types it doesn't know); DXF writes them back as read. pub fn drops_unknown(path: impl AsRef) -> bool { diff --git a/crates/engine/tests/save_reports_not_saved.rs b/crates/engine/tests/save_reports_not_saved.rs index cd73f637d..984d0d635 100644 --- a/crates/engine/tests/save_reports_not_saved.rs +++ b/crates/engine/tests/save_reports_not_saved.rs @@ -150,6 +150,38 @@ fn saving_sat_text_solids_as_dxf_2013_reports_them() { assert!(r.get("notSaved").is_none() && s.log.len() == n); } +/// The interactive save asks with the same list, and EXPORT reports it, so overwriting a 2013+ +/// DXF never drops solids on a command-line note alone (#2334). +#[test] +fn a_dxf_2013_loss_is_known_before_saving() { + use cadcraft_engine::cmd::file::loss_on_save; + set_io(IoHooks { read: |_, _| Err("no reader in this test".into()), write: |_, _| Ok(b"0 +EOF +".to_vec()), plot: None }); + let sat = |d: &mut Drawing| { + let tags = vec![RawTag { code: 8, value: "0".into() }, RawTag { code: 1, value: "400 1 1 0".into() }]; + d.add(&Space::Model, Common::default(), EntityKind::Unknown(Unknown { dxf_type: "REGION".into(), tags })).unwrap(); + }; + let mut modern = Drawing::new_imperial(); + modern.header.set_str("ACADVER", "AC1032"); + sat(&mut modern); + let mut s = Session::empty(); + s.open_drawing(modern, "region.dxf", None); + assert_eq!(loss_on_save(&s, "region.dxf"), [("REGION".to_string(), 1)].into_iter().collect()); + + let path = std::env::temp_dir().join(format!("cadcraft-sat-export-{}.dxf", std::process::id())); + let r = s.execute("export", &json!({ "path": path.to_string_lossy() })).unwrap(); + let _ = std::fs::remove_file(&path); + assert_eq!(r["notSaved"], json!({ "REGION": 1 })); + assert!(s.log.iter().any(|l| l.contains("Not saved in the DXF file")), "{:?}", s.log); + + let mut old = Drawing::new_imperial(); + old.header.set_str("ACADVER", "AC1024"); + sat(&mut old); + s.open_drawing(old, "region.dxf", None); + assert!(loss_on_save(&s, "region.dxf").is_empty(), "2010 keeps SAT text"); +} + /// In the Block Editor the file still holds the host drawing: its solids and version decide the /// note, not the edited block's (#2334). #[test] diff --git a/crates/ui-egui/src/save_loss.rs b/crates/ui-egui/src/save_loss.rs index 50e77978b..2b0a28c57 100644 --- a/crates/ui-egui/src/save_loss.rs +++ b/crates/ui-egui/src/save_loss.rs @@ -1,5 +1,6 @@ //! Asking before a save throws content away. DWG can't yet hold everything a drawing read from -//! another program keeps (`cadcraft_engine::cmd::file::not_saved`), and QSAVE replaces the file +//! another program keeps (`cadcraft_engine::cmd::file::not_saved`), a DXF of 2013 or later can't +//! hold a solid's SAT text (`cadcraft_doc::foreign::sat_text_dropped`), and QSAVE replaces the file //! that still has it, so File ▸ Save, its shortcut or typing QSAVE says what would go first: //! Save Anyway / Save As... / Cancel. Agreeing lasts for that same loss on that same file, so //! later saves in the session don't ask again; the file no longer holds what was agreed to. @@ -120,7 +121,12 @@ pub fn prompt(app: &mut CadApp, ctx: &egui::Context) { ui.label(RichText::new(crate::tf!("Saving {name} will remove content", name = file)).strong()); ui.add_space(4.0); ui.label(crate::tf!("This format can't keep these objects yet: {objects}.", objects = pending.summary())); - ui.label(RichText::new(crate::tl!("DXF keeps them. Use Save As to keep them in the drawing.")).color(t.text_dim)); + let keeps = if path.extension().is_some_and(|e| e.eq_ignore_ascii_case("dxf")) { + crate::tl!("DXF 2010 and earlier keep them. Use Save As to keep them in the drawing.") + } else { + crate::tl!("DXF keeps them. Use Save As to keep them in the drawing.") + }; + ui.label(RichText::new(keeps).color(t.text_dim)); let on_disk = if save_keeps_current(path) { crate::tf!("The current file is kept as {name} until the next save.", name = backup) } else if isavebak() { From 7690d36bb0aa43c73d266c56af828e985f2afb10 Mon Sep 17 00:00:00 2001 From: Tanish Manku Date: Mon, 2 Nov 2026 14:31:06 -0700 Subject: [PATCH 13/14] fmt --- crates/engine/tests/save_reports_not_saved.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/engine/tests/save_reports_not_saved.rs b/crates/engine/tests/save_reports_not_saved.rs index 984d0d635..d38e13e0b 100644 --- a/crates/engine/tests/save_reports_not_saved.rs +++ b/crates/engine/tests/save_reports_not_saved.rs @@ -155,9 +155,16 @@ fn saving_sat_text_solids_as_dxf_2013_reports_them() { #[test] fn a_dxf_2013_loss_is_known_before_saving() { use cadcraft_engine::cmd::file::loss_on_save; - set_io(IoHooks { read: |_, _| Err("no reader in this test".into()), write: |_, _| Ok(b"0 + set_io(IoHooks { + read: |_, _| Err("no reader in this test".into()), + write: |_, _| { + Ok(b"0 EOF -".to_vec()), plot: None }); +" + .to_vec()) + }, + plot: None, + }); let sat = |d: &mut Drawing| { let tags = vec![RawTag { code: 8, value: "0".into() }, RawTag { code: 1, value: "400 1 1 0".into() }]; d.add(&Space::Model, Common::default(), EntityKind::Unknown(Unknown { dxf_type: "REGION".into(), tags })).unwrap(); From 86f3fdc9e99454fe096f94b33e8aa864d62fc158 Mon Sep 17 00:00:00 2001 From: Tanish Manku Date: Mon, 2 Nov 2026 14:34:47 -0700 Subject: [PATCH 14/14] Test: the DWG note's wording, and one-line fake bytes --- crates/engine/tests/save_reports_not_saved.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/crates/engine/tests/save_reports_not_saved.rs b/crates/engine/tests/save_reports_not_saved.rs index d38e13e0b..80192bf53 100644 --- a/crates/engine/tests/save_reports_not_saved.rs +++ b/crates/engine/tests/save_reports_not_saved.rs @@ -60,14 +60,14 @@ fn export_and_wblock_say_what_a_dwg_leaves_out() { let mut s = Session::empty(); s.open_drawing(d, "foreign.dxf", None); let file = |ext: &str| std::env::temp_dir().join(format!("cadcraft-leaves-out-{}.{ext}", std::process::id())).to_string_lossy().into_owned(); - let msg = "Not saved in the DWG file (its writer can't write these object types yet; DXF keeps them): 1 ACME_WIDGET"; + let said = |l: &String| l.starts_with("Not saved in the DWG file") && l.ends_with(": 1 ACME_WIDGET"); let (dwg, dxf) = (file("dwg"), file("dxf")); for (cmd, p) in [("export", json!({ "path": dwg })), ("wblock", json!({ "path": dwg, "handles": [h] }))] { s.log.clear(); let r = s.execute(cmd, &p).unwrap(); assert_eq!(r["notSaved"], json!({ "ACME_WIDGET": 1 }), "{cmd}"); - assert!(s.log.iter().any(|l| l == msg), "{cmd}: {:?}", s.log); + assert!(s.log.iter().any(said), "{cmd}: {:?}", s.log); } // -WBLOCK takes the objects out of the drawing, so last. let r = s.execute("-wblock", &json!({ "path": dwg, "handles": [h] })).unwrap(); @@ -79,7 +79,7 @@ fn export_and_wblock_say_what_a_dwg_leaves_out() { for (cmd, p) in [("export", json!({ "path": dxf })), ("wblock", json!({ "path": dxf, "handles": [h] }))] { s.log.clear(); assert!(s.execute(cmd, &p).unwrap().get("notSaved").is_none(), "{cmd}"); - assert!(!s.log.iter().any(|l| l == msg), "{cmd}: {:?}", s.log); + assert!(!s.log.iter().any(said), "{cmd}: {:?}", s.log); } let _ = std::fs::remove_file(&dxf); } @@ -155,16 +155,7 @@ fn saving_sat_text_solids_as_dxf_2013_reports_them() { #[test] fn a_dxf_2013_loss_is_known_before_saving() { use cadcraft_engine::cmd::file::loss_on_save; - set_io(IoHooks { - read: |_, _| Err("no reader in this test".into()), - write: |_, _| { - Ok(b"0 -EOF -" - .to_vec()) - }, - plot: None, - }); + set_io(IoHooks { read: |_, _| Err("no reader in this test".into()), write: |_, _| Ok(b"0\nEOF\n".to_vec()), plot: None }); let sat = |d: &mut Drawing| { let tags = vec![RawTag { code: 8, value: "0".into() }, RawTag { code: 1, value: "400 1 1 0".into() }]; d.add(&Space::Model, Common::default(), EntityKind::Unknown(Unknown { dxf_type: "REGION".into(), tags })).unwrap();