Merge pull request #2661 from storytold/land/2474

DXF: leave SAT-text solids out of 2013+ files, and ask before a save does (#2474)

crates/doc/src/foreign.rs

@@ -89,12 +89,88 @@ 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<Handle> {
-        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(data_store_owner(rec).and_then(|t| Handle::parse_hex(t.value.trim())));
+            }
+        }
+        out
     }
 }
 
+/// 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<Handle>) -> bool {
+    ["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
+/// ([`sat_text_acis`]), counted by DXF type, across model space, layouts and block definitions.
+pub fn sat_text_dropped(d: &crate::Drawing) -> BTreeMap<String, usize> {
+    let stored = d.foreign.data_store_entities();
+    let mut out = BTreeMap::new();
+    // 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)
+        {
+            let n: &mut usize = out.entry(u.dxf_type.to_ascii_uppercase()).or_default();
+            *n = n.saturating_add(1);
+        }
+    }
+    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('|')
+}
+
+/// 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.
+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;
+    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
+}
+
+/// 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)
+}
+
 impl Foreign {
     /// The kept objects by handle (the first of any that share one).
     fn object_index(&self) -> BTreeMap<Handle, &ForeignObject> {

crates/engine/src/cmd/exchange.rs

@@ -118,7 +118,6 @@ fn run_export(s: &mut Session, p: &Value) -> Result<Value> {
         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<Value> {
         }
         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)
 }
 

crates/engine/src/cmd/file.rs

@@ -695,8 +695,11 @@ fn run_saveas(s: &mut Session, p: &Value) -> Result<Value> {
 /// 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) }
 }
 
@@ -725,6 +728,8 @@ fn run_bytes(s: &mut Session, p: &Value) -> Result<Value> {
     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)
 }
@@ -756,7 +761,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 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) || (version < Version::R2013 && cadcraft_doc::foreign::has_sat_text(&u.tags)))
+            {
                 continue;
             }
             if DWG_WRITES.contains(&u.dxf_type.as_str()) {
@@ -775,20 +784,39 @@ pub fn not_saved_message(lost: &std::collections::BTreeMap<String, usize>) -> Op
         return None;
     }
     let list: Vec<String> = 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(", ")
+    ))
 }
 
 /// What saving the active drawing to `path` would leave out, before anything is written: empty
 /// 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<String, usize> {
+    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<std::path::Path>) -> bool {
@@ -800,6 +828,29 @@ fn is_drawing_file(path: impl AsRef<std::path::Path>) -> bool {
     Format::from_path(path).is_some_and(Format::is_drawing)
 }
 
+/// Whether `path` is a DXF file.
+fn is_dxf_file(path: impl AsRef<std::path::Path>) -> 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) {
+    // 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;
+    }
+    let list: Vec<String> = 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`.
 pub(super) fn report_not_saved(s: &mut Session, r: &mut Value, lost: &std::collections::BTreeMap<String, usize>) {
     let Some(msg) = not_saved_message(lost) else { return };
@@ -810,14 +861,17 @@ pub(super) fn report_not_saved(s: &mut Session, r: &mut Value, lost: &std::colle
 }
 
 /// 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<Value>) -> Result<Value> {
+fn save_reporting(s: &mut Session, p: &Value, run: fn(&mut Session, &Value) -> Result<Value>, asked: Option<Version>) -> Result<Value> {
     // 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) {
         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);
     }
     if r.get("path").and_then(Value::as_str).is_some_and(is_drawing_file) {
         report_fonts(s, &mut r);
@@ -857,11 +911,12 @@ fn report_fonts(s: &mut Session, r: &mut Value) {
 }
 
 fn run_qsave_reporting(s: &mut Session, p: &Value) -> Result<Value> {
-    save_reporting(s, p, run_save)
+    save_reporting(s, p, run_save, None)
 }
 
 fn run_saveas_reporting(s: &mut Session, p: &Value) -> Result<Value> {
-    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+/";

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;
 
@@ -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.
@@ -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);
 }
@@ -115,6 +115,113 @@ fn solids_whose_geometry_the_data_store_keeps_are_not_reported_from_2013() {
     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);
+
+    // 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);
+    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\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();
+    };
+    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]
+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 }));
+}
+
+/// 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_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<RawTag>| 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();
+    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");
+}
+
 /// The DWG bridge writes OLE objects (`ole2frame_roundtrip` in the io tests), so saving one as
 /// DWG reports nothing for them, at any version, while other unmodelled types are still listed.
 /// The type is matched as stored: the bridge gets it as stored too, and acadrust reads only the

crates/io/src/dxf_raw.rs

@@ -603,17 +603,13 @@ pub(crate) fn data_store(tags: &mut Vec<Tag>, 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));
+        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));
         }
-        i = end;
     }
     if records == 0 {
         return;
@@ -850,7 +846,7 @@ fn unknown_handles(d: &Drawing) -> HashSet<String> {
 }
 
 /// Put the drawing's foreign data back into a written file's tags (see the module comment).
-pub(crate) fn finish(tags: &mut Vec<Tag>, d: &Drawing, reactors: &HashMap<Handle, Vec<String>>) {
+pub(crate) fn finish(tags: &mut Vec<Tag>, d: &Drawing, reactors: &HashMap<Handle, Vec<String>>, 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();
@@ -879,7 +875,7 @@ pub(crate) fn finish(tags: &mut Vec<Tag>, d: &Drawing, reactors: &HashMap<Handle
     }
     let renumber = |old: Option<i64>| 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())) {
@@ -1023,7 +1019,14 @@ pub(crate) fn finish(tags: &mut Vec<Tag>, d: &Drawing, reactors: &HashMap<Handle
 /// 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<ForeignKey, String>) -> HashMap<usize, Vec<(String, Vec<Tag>)>> {
+fn plan_unknown(
+    d: &Drawing,
+    recs: &[Rec],
+    tags: &[Tag],
+    roles: &HashMap<ForeignKey, String>,
+    modern: bool,
+) -> HashMap<usize, Vec<(String, Vec<Tag>)>> {
+    let stored = d.foreign.data_store_entities();
     let mut out: HashMap<usize, Vec<(String, Vec<Tag>)>> = HashMap::new();
     let body = |r: &Rec| tags.get(r.start + 1..r.end).unwrap_or(&[]);
     let mut at: HashMap<String, usize> = HashMap::new();
@@ -1061,6 +1064,9 @@ fn plan_unknown(d: &Drawing, recs: &[Rec], tags: &[Tag], roles: &HashMap<Foreign
         for e in store.iter() {
             match &e.kind {
                 EntityKind::Unknown(u) => {
+                    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));

crates/io/src/dxf_write/sections.rs

@@ -1137,7 +1137,9 @@ pub(super) fn tags_for(d: &Drawing, version: Version, target: Version, frozen_34
     if let Some(t) = w.t.get_mut(seed_pos + 1) {
         *t = Tag::s(5, seed);
     }
-    crate::dxf_raw::finish(&mut w.t, d, &w.reactors);
+    // 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, &w.reactors, version >= Version::R2013);
     crate::dxf_raw::data_store(&mut w.t, d, target >= Version::R2013);
     add_class_instance_counts(&mut w.t, target);
     mark_xrefs(&mut w.t, d);

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

crates/io/tests/it/acis_data_store.rs

@@ -101,3 +101,119 @@ 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());
+}
+
+/// 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");
+}
+
+/// 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<RawTag>| 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());
+}
+
+/// 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());
+}
+
+/// 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");
+}

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() {