From 946af8745855fdb6eff8c5340c965b48a307c529 Mon Sep 17 00:00:00 2001 From: ffishman <20211677+ffishman@users.noreply.github.com> Date: Mon, 2 Nov 2026 08:54:58 -0300 Subject: [PATCH] A proxy whose class the file doesn't name is reported as not saved From 2013 the DWG keeps another program's proxy entity with the class its group 91 names, so the loss report skipped every ACAD_PROXY_ENTITY. Saving drops a proxy whose group 91 names no class of the file's CLASSES, though, so that one is now counted. The group 91 helpers move to cadcraft_doc::foreign so the engine can ask Foreign::proxy_class. --- crates/doc/src/foreign.rs | 32 +++++++++++++++++++ crates/engine/src/cmd/file.rs | 6 ++-- crates/engine/tests/save_reports_not_saved.rs | 23 +++++++++++-- crates/io/src/dxf_raw.rs | 28 +--------------- 4 files changed, 56 insertions(+), 33 deletions(-) diff --git a/crates/doc/src/foreign.rs b/crates/doc/src/foreign.rs index 80ebbc3a3..d71b21b47 100644 --- a/crates/doc/src/foreign.rs +++ b/crates/doc/src/foreign.rs @@ -82,6 +82,12 @@ pub struct Foreign { } impl Foreign { + /// The class of the file's CLASSES a proxy's group 91 names (see [`class_number`]), if any. + /// Saving as DWG keeps a proxy with that class and leaves out one whose class it can't name. + pub fn proxy_class(&self, tags: &[RawTag]) -> Option<&str> { + class_index(class_number(tags.iter().map(|r| (r.code, r.value.as_str())))).and_then(|i| self.class_order.get(i)).map(String::as_str) + } + pub fn is_empty(&self) -> bool { self.objects.is_empty() && self.dictionary.is_empty() @@ -98,3 +104,29 @@ impl Foreign { self.data_store.iter().filter(|t| t.code == 320).filter_map(|t| Handle::parse_hex(t.value.trim())).collect() } } + +/// Where a proxy's class number is: its first group 91 outside the `102 {NAME` … `102 }` groups +/// (an application's can hold a 91 of its own) and before any xdata. +pub fn class_group>(groups: impl IntoIterator) -> Option { + let mut in_group = false; + for (i, (code, value)) in groups.into_iter().enumerate() { + match code { + 1000.. => return None, + 102 => in_group = value.as_ref().trim().starts_with('{'), + 91 if !in_group => return Some(i), + _ => {} + } + } + None +} + +/// A proxy's class number (see [`class_group`]). +pub fn class_number>(groups: impl IntoIterator + Clone) -> Option { + let i = class_group(groups.clone())?; + groups.into_iter().nth(i).and_then(|(_, v)| v.as_ref().trim().parse().ok()) +} + +/// The place in CLASSES a class number names (500 is the first class). +pub fn class_index(number: Option) -> Option { + usize::try_from(number?.checked_sub(500)?).ok() +} diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index d9014b13b..0f9bb28c4 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -733,8 +733,8 @@ const MODELER_TYPES: [&str; 3] = ["3DSOLID", "REGION", "BODY"]; /// [`not_saved`] for a file saved as `version`. From 2013 on, a 3D solid, region or body the data /// store has a record for keeps its geometry and is not counted, and neither is another program's -/// proxy entity: the DWG writes it with its class (before 2013 it has no class to name, and the -/// DWG leaves it out). +/// proxy entity whose class the file names: the DWG writes it with that class. Before 2013, or +/// when its group 91 names no class of the file, the DWG leaves it out and it is counted. pub fn not_saved_as(d: &Drawing, version: Version) -> std::collections::BTreeMap { let mut out: std::collections::BTreeMap = std::collections::BTreeMap::new(); let stored = if version >= Version::R2013 { d.foreign.data_store_entities() } else { Default::default() }; @@ -744,7 +744,7 @@ pub fn not_saved_as(d: &Drawing, version: Version) -> std::collections::BTreeMap if MODELER_TYPES.iter().any(|t| u.dxf_type.eq_ignore_ascii_case(t)) && stored.contains(&e.handle) { continue; } - if version >= Version::R2013 && u.dxf_type.eq_ignore_ascii_case("ACAD_PROXY_ENTITY") { + if version >= Version::R2013 && u.dxf_type.eq_ignore_ascii_case("ACAD_PROXY_ENTITY") && d.foreign.proxy_class(&u.tags).is_some() { 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 6753151fd..c847dad88 100644 --- a/crates/engine/tests/save_reports_not_saved.rs +++ b/crates/engine/tests/save_reports_not_saved.rs @@ -83,18 +83,35 @@ 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()); } -/// Another program's proxy entity reaches a 2013 or later DWG with its class; before 2013 the DWG -/// leaves it out and says so. +/// Another program's proxy entity reaches a 2013 or later DWG with the class its group 91 names; +/// before 2013, or when the file has no such class, the DWG leaves it out and says so. #[test] fn proxy_entities_are_reported_only_before_2013() { use cadcraft_engine::cmd::file::{Version, not_saved_as}; + use cadcraft_engine::doc::foreign::Foreign; + let proxy = |class: i64| { + EntityKind::Unknown(Unknown { + dxf_type: "ACAD_PROXY_ENTITY".into(), + tags: vec![ + RawTag { code: 8, value: "0".into() }, + RawTag { code: 90, value: "498".into() }, + RawTag { code: 91, value: class.to_string() }, + ], + }) + }; let mut d = Drawing::new_imperial(); - d.add(&Space::Model, Common::default(), unknown("ACAD_PROXY_ENTITY")).unwrap(); + d.add(&Space::Model, Common::default(), proxy(500)).unwrap(); d.add(&Space::Model, Common::default(), unknown("MLINE")).unwrap(); + d.foreign = std::sync::Arc::new(Foreign { class_order: vec!["MYAPP_THING".into()], ..Foreign::default() }); for v in [Version::R2013, Version::R2018] { assert_eq!(not_saved_as(&d, v), [("MLINE".to_string(), 1)].into_iter().collect(), "{v:?}"); } for v in [Version::R2004, Version::R2010] { assert_eq!(not_saved_as(&d, v), [("ACAD_PROXY_ENTITY".to_string(), 1), ("MLINE".to_string(), 1)].into_iter().collect(), "{v:?}"); } + // A class number the file's CLASSES don't have, or none at all: the DWG can't name the class, + // leaves the proxy out, and says so from 2013 on too. + d.add(&Space::Model, Common::default(), proxy(501)).unwrap(); + d.add(&Space::Model, Common::default(), unknown("ACAD_PROXY_ENTITY")).unwrap(); + assert_eq!(not_saved_as(&d, Version::R2018), [("ACAD_PROXY_ENTITY".to_string(), 2), ("MLINE".to_string(), 1)].into_iter().collect()); } diff --git a/crates/io/src/dxf_raw.rs b/crates/io/src/dxf_raw.rs index 7857d3be3..96b236dba 100644 --- a/crates/io/src/dxf_raw.rs +++ b/crates/io/src/dxf_raw.rs @@ -16,7 +16,7 @@ use std::borrow::Cow; use std::collections::{BTreeMap, HashMap, HashSet}; use std::sync::Arc; -use cadcraft_doc::foreign::{Foreign, ForeignExtra, ForeignKey, ForeignObject}; +use cadcraft_doc::foreign::{Foreign, ForeignExtra, ForeignKey, ForeignObject, class_group, class_index, class_number}; use cadcraft_doc::{Drawing, Entity, EntityKind, Handle, RawTag, Unknown}; use cadcraft_dxf::{Section, Tag}; @@ -559,32 +559,6 @@ fn is_proxy(kind: &str) -> bool { matches!(up(kind).as_str(), "ACAD_PROXY_OBJECT" | "ACAD_PROXY_ENTITY") } -/// Where a proxy's class number is: its first group 91 outside the `102 {NAME` … `102 }` groups -/// (an application's can hold a 91 of its own) and before any xdata. -fn class_group>(groups: impl IntoIterator) -> Option { - let mut in_group = false; - for (i, (code, value)) in groups.into_iter().enumerate() { - match code { - 1000.. => return None, - 102 => in_group = value.as_ref().trim().starts_with('{'), - 91 if !in_group => return Some(i), - _ => {} - } - } - None -} - -/// A proxy's class number (see [`class_group`]). -fn class_number>(groups: impl IntoIterator + Clone) -> Option { - let i = class_group(groups.clone())?; - groups.into_iter().nth(i).and_then(|(_, v)| v.as_ref().trim().parse().ok()) -} - -/// The place in CLASSES a class number names (500 is the first class). -fn class_index(number: Option) -> Option { - usize::try_from(number?.checked_sub(500)?).ok() -} - /// Set a proxy's class number (see [`class_group`]). fn set_class_number(tags: &mut [Tag], n: i64) { if let Some(t) = class_group(tags.iter().map(|t| (t.code, t.str()))).and_then(|i| tags.get_mut(i)) {