DWG: isolate proxy conversion failures and normalize proxy names

crates/dwg/src/lib.rs

@@ -109,7 +109,9 @@ mod native {
         }
         let mut notes = read_notes(&outcome.stats);
         let mut doc = outcome.document;
-        guarded(AssertUnwindSafe(|| super::proxy_transport::prepare(&mut doc))).map_err(|_| "the DWG proxy conversion failed".to_string())??;
+        notes.extend(
+            guarded(AssertUnwindSafe(|| super::proxy_transport::prepare(&mut doc))).map_err(|_| "the DWG proxy conversion failed".to_string())?,
+        );
         let orphans = guarded(AssertUnwindSafe(|| post_process(&mut doc))).map_err(|_| "the DWG post-processing failed on this file".to_string())?;
         if orphans > 0 {
             let (s, were) = if orphans == 1 { ("", "was") } else { ("s", "were") };
@@ -737,7 +739,6 @@ mod native {
                 mb(Limits::DEFAULT.max_dxf_bytes as u64)
             ));
         }
-        let classes = super::proxy_transport::read_classes(dxf)?;
         let data = dxf.to_vec();
         let mut doc = std::panic::catch_unwind(move || acadrust::DxfReader::from_reader(Cursor::new(data)).and_then(|r| r.read()))
             .map_err(|_| "the DXF→DWG conversion failed".to_string())?
@@ -750,7 +751,7 @@ mod native {
             None if doc.version < MIN_DWG_VERSION => doc.version = MIN_DWG_VERSION,
             None => {}
         }
-        guarded(AssertUnwindSafe(|| super::proxy_transport::restore_classes(&mut doc, classes)))
+        guarded(AssertUnwindSafe(|| super::proxy_transport::restore_classes(&mut doc, || super::proxy_transport::read_classes(dxf))))
             .map_err(|_| "the DXF proxy class conversion failed".to_string())??;
         // The fix-ups run on the third-party reader's document, so a panic in one is an error, not a crash.
         let unwritten = guarded(AssertUnwindSafe(|| {

crates/dwg/src/proxy_transport.rs

@@ -12,16 +12,16 @@ fn class_number(index: usize) -> Result<i16, String> {
     index.checked_add(500).and_then(|n| i16::try_from(n).ok()).ok_or_else(|| "DXF: too many application classes".into())
 }
 
-/// Replace the dependency's private registered-class envelope with a standard proxy.
-pub(super) fn prepare(doc: &mut CadDocument) -> Result<(), String> {
+/// Convert representable application records and report only the records left out.
+pub(super) fn prepare(doc: &mut CadDocument) -> Vec<String> {
     let classes: Vec<_> = doc.classes.iter().cloned().collect();
     let mut numbers = HashMap::new();
     let mut identities = HashMap::new();
     for (index, class) in classes.iter().enumerate() {
-        let number = class_number(index)?;
-        numbers.entry(i32::from(class.class_number)).and_modify(|v| *v = None).or_insert(Some(i32::from(number)));
+        let number = class_number(index).ok();
+        numbers.entry(i32::from(class.class_number)).and_modify(|v| *v = None).or_insert(number.map(i32::from));
         let identity = (class.dxf_name.as_str(), class.cpp_class_name.as_str(), class.is_an_entity);
-        identities.entry(identity).and_modify(|v| *v = None).or_insert(Some(number));
+        identities.entry(identity).and_modify(|v| *v = None).or_insert(number);
     }
     let identity = |dxf: &str, cpp: &str, entity: bool| {
         identities
@@ -29,7 +29,7 @@ pub(super) fn prepare(doc: &mut CadDocument) -> Result<(), String> {
             .copied()
             .flatten()
             .map(i32::from)
-            .ok_or_else(|| format!("DWG: missing or ambiguous application CLASS for {dxf} ({cpp})"))
+            .ok_or_else(|| format!("missing or ambiguous application CLASS for {dxf:.80} ({cpp:.80})"))
     };
     let source = doc.dwg_source_version.unwrap_or(doc.version).version_code();
     let dwg_version = match source {
@@ -41,10 +41,10 @@ pub(super) fn prepare(doc: &mut CadDocument) -> Result<(), String> {
         1024 => 29,
         1027 => 31,
         1032 => 33,
-        _ => return Ok(()),
+        _ => return Vec::new(),
     };
     let maintenance = i32::from(doc.maintenance_version);
-    let remap = |id: i32| numbers.get(&id).copied().flatten().ok_or_else(|| format!("DWG: proxy names missing application class {id}"));
+    let remap = |id: i32| numbers.get(&id).copied().flatten().ok_or_else(|| format!("proxy names missing or ambiguous application class {id}"));
     // Only mutate proxies: entities_mut invalidates the decoder's verbatim records for every entity.
     let handles: Vec<_> = doc
         .entities()
@@ -57,46 +57,81 @@ pub(super) fn prepare(doc: &mut CadDocument) -> Result<(), String> {
             _ => None,
         })
         .collect();
+    let mut notes = Vec::new();
+    let mut entity_count = 0usize;
+    let mut object_count = 0usize;
     for handle in handles {
-        let text_payload = match doc.get_entity(handle) {
-            Some(EntityType::Extended(e)) if matches!(e.data, ExtendedEntityData::RegisteredClass(_)) => {
-                if let ExtendedEntityData::RegisteredClass(data) = &e.data
-                    && !data.properties.is_empty()
-                {
-                    return Err(format!("DWG: cannot transport semantic properties of {} as an opaque proxy", data.dxf_name));
+        let converted = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+            let Some(EntityType::Extended(entity)) = doc.get_entity(handle) else { return Ok(None) };
+            let data = match &entity.data {
+                ExtendedEntityData::Proxy(data) => {
+                    let mut proxy = data.clone();
+                    proxy.class_id = remap(proxy.class_id)?;
+                    ExtendedEntityData::Proxy(proxy)
+                }
+                ExtendedEntityData::RegisteredClass(data) => {
+                    if !data.properties.is_empty() {
+                        return Err(format!("cannot transport semantic properties of {:.80} as an opaque proxy", data.dxf_name));
+                    }
+                    let class_id = identity(&data.dxf_name, &data.cpp_class_name, true)?;
+                    let text_payload = entity.common.raw_record.as_deref().map(registered_text).transpose()?.unwrap_or_default();
+                    ExtendedEntityData::Proxy(ProxyEntityData {
+                        proxy_id: 498,
+                        class_id,
+                        dxf_subclass: String::new(),
+                        version: (maintenance << 16) | dwg_version,
+                        dwg_version,
+                        maintenance_version: maintenance,
+                        from_dxf: false,
+                        graphics: ProxyPayload::from_bytes(entity.common.graphic_data.as_deref().unwrap_or_default()),
+                        payload: data.payload.clone(),
+                        text_payload,
+                        object_ids: data.object_ids.clone(),
+                    })
+                }
+                _ => return Ok(None),
+            };
+            Ok(Some(data))
+        }))
+        .unwrap_or_else(|_| Err("application record conversion failed".into()));
+        match converted {
+            Ok(Some(data)) => {
+                if let Some(EntityType::Extended(entity)) = doc.get_entity_mut(handle) {
+                    entity.data = data;
                 }
-                e.common.raw_record.as_deref().map(registered_text).transpose()?.unwrap_or_default()
             }
-            _ => ProxyPayload::default(),
-        };
-        let Some(EntityType::Extended(entity)) = doc.get_entity_mut(handle) else { continue };
-        match &mut entity.data {
-            ExtendedEntityData::Proxy(data) => data.class_id = remap(data.class_id)?,
-            ExtendedEntityData::RegisteredClass(data) => {
-                let class_id = identity(&data.dxf_name, &data.cpp_class_name, true)?;
-                entity.data = ExtendedEntityData::Proxy(ProxyEntityData {
-                    proxy_id: 498,
-                    class_id,
-                    dxf_subclass: String::new(),
-                    version: (maintenance << 16) | dwg_version,
-                    dwg_version,
-                    maintenance_version: maintenance,
-                    from_dxf: false,
-                    graphics: ProxyPayload::from_bytes(entity.common.graphic_data.as_deref().unwrap_or_default()),
-                    payload: data.payload.clone(),
-                    text_payload,
-                    object_ids: data.object_ids.clone(),
-                });
+            Ok(None) => {}
+            Err(why) => {
+                doc.remove_entity(handle);
+                entity_count = entity_count.saturating_add(1);
+                skipped_note(&mut notes, "entity", handle, &why);
             }
-            _ => {}
         }
     }
-    for object in doc.objects.values_mut() {
+    doc.objects.retain(|handle, object| {
         if let ObjectType::ProxyObject(proxy) = object {
-            proxy.class_id = remap(proxy.class_id)?;
+            match remap(proxy.class_id) {
+                Ok(id) => proxy.class_id = id,
+                Err(why) => {
+                    object_count = object_count.saturating_add(1);
+                    skipped_note(&mut notes, "object", *handle, &why);
+                    return false;
+                }
+            }
         }
+        true
+    });
+    if entity_count > 0 || object_count > 0 {
+        notes.insert(0, format!("DWG: partial read; application proxy records left out (entities: {entity_count}, objects: {object_count})"));
+    }
+    notes
+}
+
+fn skipped_note(notes: &mut Vec<String>, kind: &str, handle: acadrust::types::Handle, why: &str) {
+    if notes.len() < 6 {
+        let why: String = why.chars().take(256).collect();
+        notes.push(format!("DWG: proxy {kind} {handle} left out: {why}"));
     }
-    Ok(())
 }
 
 /// The decoder's registered-class representation omits the separate R2007+ string stream.
@@ -191,7 +226,7 @@ fn read_class(tags: &[Tag], number: i16) -> Result<DxfClass, String> {
 }
 
 /// Keep source class positions and metadata, adding missing decoder defaults only afterwards.
-pub(super) fn restore_classes(doc: &mut CadDocument, classes: Vec<DxfClass>) -> Result<(), String> {
+pub(super) fn restore_classes(doc: &mut CadDocument, source: impl FnOnce() -> Result<Vec<DxfClass>, String>) -> Result<(), String> {
     let mut proxies = Vec::new();
     for entity in doc.entities() {
         if let EntityType::Extended(e) = entity
@@ -208,6 +243,7 @@ pub(super) fn restore_classes(doc: &mut CadDocument, classes: Vec<DxfClass>) ->
     if proxies.is_empty() {
         return Ok(());
     }
+    let classes = source()?;
     for (number, entity) in proxies {
         let index = number.checked_sub(500).and_then(|n| usize::try_from(n).ok());
         if !index.and_then(|i| classes.get(i)).is_some_and(|c| c.is_an_entity == entity) {
@@ -286,7 +322,7 @@ mod tests {
             assert_eq!(classes[0].instance_count, 7);
             assert!(classes[0].was_zombie);
             let mut doc = proxy_doc();
-            restore_classes(&mut doc, classes.clone()).unwrap();
+            restore_classes(&mut doc, || Ok(classes.clone())).unwrap();
             assert_eq!(doc.classes.iter().take(2).cloned().collect::<Vec<_>>(), classes);
         }
     }
@@ -302,11 +338,47 @@ mod tests {
             }
             let mut class = DxfClass::new_entity("ENTITY", "Entity");
             class.class_number = 500;
-            assert!(restore_classes(&mut doc, vec![class]).is_err());
+            assert!(restore_classes(&mut doc, || Ok(vec![class])).is_err());
         }
-        assert!(restore_classes(&mut proxy_doc(), vec![DxfClass::new("A", "A"), DxfClass::new("B", "B")]).is_err());
+        assert!(restore_classes(&mut proxy_doc(), || Ok(vec![DxfClass::new("A", "A"), DxfClass::new("B", "B")])).is_err());
     }
 
+    #[test]
+    fn unrepresentable_proxies_keep_other_geometry_and_bounded_diagnostics() {
+        use acadrust::entities::{Line, RawRecord};
+        use acadrust::objects::ProxyObject;
+        use acadrust::types::{Handle, Vector3};
+        for count in [1, 10] {
+            let mut doc = proxy_doc();
+            if let Some(EntityType::Extended(e)) = doc.entities_mut().next()
+                && let ExtendedEntityData::Proxy(p) = &mut e.data
+            {
+                p.class_id = i32::MAX;
+            }
+            let template = doc.entities().next().unwrap().clone();
+            for _ in 1..count {
+                let mut entity = template.clone();
+                entity.common_mut().handle = Handle::NULL;
+                doc.add_entity(entity).unwrap();
+            }
+            let object = Handle::new(u64::MAX - 1);
+            assert!(!doc.objects.contains_key(&object));
+            doc.objects.insert(object, ObjectType::ProxyObject(ProxyObject { handle: object, class_id: i32::MAX, ..Default::default() }));
+            let line = doc.add_entity(EntityType::Line(Line::from_points(Vector3::new(2.0, 3.0, 0.0), Vector3::new(12.0, 13.0, 0.0)))).unwrap();
+            let raw = RawRecord { data: vec![0xAA, 0x55], handle_bits: 0, version: DxfVersion::AC1032 };
+            doc.get_entity_mut(line).unwrap().common_mut().raw_record = Some(std::sync::Arc::new(raw.clone()));
+            let notes = prepare(&mut doc);
+            assert_eq!(notes.len(), (count + 1).min(6) + 1);
+            assert!(notes.iter().all(|n| n.starts_with("DWG:")));
+            assert!(notes[0].contains(&format!("entities: {count}, objects: 1")));
+            assert_eq!(doc.entities().count(), 1);
+            assert!(!doc.objects.contains_key(&object));
+            let Some(EntityType::Line(actual)) = doc.get_entity(line) else { panic!("readable line") };
+            assert_eq!(actual.start, Vector3::new(2.0, 3.0, 0.0));
+            assert_eq!(actual.end, Vector3::new(12.0, 13.0, 0.0));
+            assert_eq!(doc.get_entity(line).unwrap().common().raw_record.as_deref(), Some(&raw));
+        }
+    }
     #[test]
     fn opaque_registered_class_ambiguity_is_not_guessed() {
         let mut doc = CadDocument::new();
@@ -323,11 +395,12 @@ mod tests {
             }),
         })))
         .unwrap();
-        assert!(prepare(&mut doc).unwrap_err().contains("ambiguous"));
+        assert!(prepare(&mut doc).iter().any(|n| n.contains("ambiguous")));
+        assert_eq!(doc.entities().count(), 0);
         assert!(class_number(usize::MAX).is_err());
     }
     #[test]
-    fn semantic_records_keep_their_verbatim_data_and_mixed_payloads_are_rejected() {
+    fn semantic_records_keep_verbatim_data_and_mixed_payloads_are_reported() {
         use acadrust::entities::RawRecord;
         use acadrust::objects::{SemanticProperty, SemanticPropertyValue};
         for payload in [ProxyPayload::default(), ProxyPayload::from_bits(&[0x80], 1)] {
@@ -352,10 +425,11 @@ mod tests {
             doc.get_entity_mut(handle).unwrap().common_mut().raw_record = Some(std::sync::Arc::new(raw.clone()));
             assert_eq!(doc.get_entity(handle).unwrap().common().raw_record.as_deref(), Some(&raw));
             if payload.bit_count == 0 {
-                prepare(&mut doc).unwrap();
+                assert!(prepare(&mut doc).is_empty());
                 assert_eq!(doc.get_entity(handle).unwrap().common().raw_record.as_deref(), Some(&raw));
             } else {
-                assert!(prepare(&mut doc).unwrap_err().contains("semantic properties"));
+                assert!(prepare(&mut doc).iter().any(|n| n.contains("semantic properties")));
+                assert!(doc.get_entity(handle).is_none());
             }
         }
     }
@@ -411,7 +485,7 @@ mod string_stream_tests {
                 object_ids: vec![],
             });
             entity.common.raw_record = Some(raw);
-            prepare(&mut decoded).unwrap();
+            assert!(prepare(&mut decoded).is_empty());
             for _ in 0..2 {
                 let mut dxf = Vec::new();
                 acadrust::DxfWriter::new(&decoded).write_to_writer(&mut dxf).unwrap();

crates/io/src/dxf_write.rs

@@ -127,11 +127,17 @@ pub(crate) fn dwg_bridge(d: &Drawing, dwg: Version) -> String {
     cadcraft_dxf::write_ascii(&tags)
 }
 
-/// Subclass markers the DWG bridge knows by another spelling (it matches them exactly):
-/// AutoCAD writes a dynamic block lookup parameter's as `AcDbBlockLookUpParameter`, the bridge
-/// reads `AcDbBlockLookupParameter`; otherwise the parameter loses its name and its lookup
-/// action in the DWG, and AutoCAD's lookups stop working.
+/// Proxy type names and lookup-parameter subclass names must match the bridge reader's spelling.
+/// Ordinary DXF output retains source spellings.
 fn bridge_markers(tags: &mut [Tag]) {
+    for tag in tags.iter_mut().filter(|tag| tag.code == 0) {
+        for name in ["ACAD_PROXY_ENTITY", "ACAD_PROXY_OBJECT"] {
+            if tag.str().trim().eq_ignore_ascii_case(name) {
+                *tag = Tag::s(0, name);
+                break;
+            }
+        }
+    }
     for t in tags.iter_mut().filter(|t| t.code == 100) {
         if t.str().trim().eq_ignore_ascii_case("AcDbBlockLookupParameter") {
             *t = Tag::s(100, "AcDbBlockLookupParameter");

crates/io/tests/registered_proxy.rs

@@ -187,3 +187,48 @@ fn proxies_inside_block_definitions_survive_repeated_saves() {
         assert!(block.entities.iter().any(|e| matches!(e.kind, EntityKind::Unknown(_))));
     }
 }
+
+#[test]
+fn unused_malformed_classes_do_not_block_plain_geometry() {
+    for (name, cpp, flags, count) in [("", "Unused", 0, 0), ("UNUSED", "", 0, 0), ("UNUSED", "Unused", 65536, 0), ("UNUSED", "Unused", 0, i64::MAX)] {
+        let dxf = format!(
+            "0\nSECTION\n2\nHEADER\n9\n$ACADVER\n1\nAC1032\n0\nENDSEC\n0\nSECTION\n2\nCLASSES\n0\nCLASS\n1\n{name}\n2\n{cpp}\n3\nSynthetic\n90\n{flags}\n91\n{count}\n281\n1\n0\nENDSEC\n0\nSECTION\n2\nENTITIES\n0\nLINE\n5\n10\n100\nAcDbEntity\n8\n0\n100\nAcDbLine\n10\n2\n20\n3\n11\n12\n21\n13\n0\nENDSEC\n0\nEOF\n"
+        );
+        let bytes = cadcraft_dwg::dxf_to_dwg_as(dxf.as_bytes(), "AC1032").unwrap();
+        let drawing = cadcraft_io::read(&bytes, "plain.dwg").unwrap();
+        let lines: Vec<_> = drawing.model.iter().filter_map(|e| if let EntityKind::Line(l) = &e.kind { Some(l) } else { None }).collect();
+        assert_eq!(lines.len(), 1);
+        assert_eq!(lines[0].a.x, 2.0);
+        assert_eq!(lines[0].a.y, 3.0);
+        assert_eq!(lines[0].b.x, 12.0);
+        assert_eq!(lines[0].b.y, 13.0);
+    }
+}
+
+#[test]
+fn proxy_spelling_is_preserved_in_dxf_and_normalized_only_for_dwg() {
+    let source = String::from_utf8(duplicate_class_dxf()).unwrap();
+    for (entity, object) in [("acad_proxy_entity", "acad_proxy_object"), ("AcAd_PrOxY_EnTiTy", "AcAd_PrOxY_ObJeCt")] {
+        let dxf = source.replace("ACAD_PROXY_ENTITY", entity).replace("ACAD_PROXY_OBJECT", object);
+        let drawing = cadcraft_io::read(dxf.as_bytes(), "source.dxf").unwrap();
+        assert_eq!(drawing.entity_count(), 3);
+        let out = cadcraft_io::write_with(&drawing, "copy.dxf", &SaveOptions { version: Some(Version::R2018), ..Default::default() }).unwrap();
+        let tags = cadcraft_dxf::parse(&out).unwrap();
+        assert!(tags.iter().any(|t| t.code == 0 && t.str() == entity));
+        let bytes = cadcraft_io::write_with(&drawing, "copy.dwg", &SaveOptions { version: Some(Version::R2018), ..Default::default() }).unwrap();
+        let decoded = cadcraft_dwg::dwg_to_dxf(&bytes).unwrap();
+        let tags = cadcraft_dxf::parse(&decoded).unwrap();
+        let proxies: Vec<_> = cadcraft_dxf::sections(&tags)
+            .iter()
+            .flat_map(|s| cadcraft_dxf::records(&s.tags))
+            .filter(|(kind, _)| kind.starts_with("ACAD_PROXY_"))
+            .collect();
+        assert_eq!(proxies.len(), 3);
+        for (_, tags) in proxies {
+            assert!(tags.iter().any(|t| t.code == 310 && t.str() == "A580"));
+            assert!(tags.iter().any(|t| matches!(t.code, 93 | 161) && t.i64() == 9));
+        }
+        let recovered = cadcraft_io::read(&bytes, "copy.dwg").unwrap();
+        assert_eq!(recovered.entity_count(), 3);
+    }
+}