From 86d010eaac0d458a528d91705c0a2118f80fa528 Mon Sep 17 00:00:00 2001 From: SBaksa Date: Fri, 30 Oct 2026 14:59:16 +1100 Subject: [PATCH] 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()); +}