feat(engine): INSERT of a drawing file keeps an attribute definition's field (#2619)

* feat(engine): INSERT of a drawing file keeps an attribute definition's field

The attribute definitions of a drawing file read by INSERT name=file lost their
extension dictionary, so the ACAD_FIELD dictionary and the FIELD objects of an attribute
whose value is a field (the file name in a title block) were gone from the block.
adopt() now copies that object tree to the drawing under new handles, with the FIELD
class and a field list if the drawing has none.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

* foreign: index the kept objects once when walking and copying a dictionary tree

A file can keep many objects; a linear lookup per pointer grew with their product.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: andrewbrake <6925387+andrewbrake@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>

crates/doc/src/foreign.rs

@@ -6,7 +6,7 @@
 //! named object dictionary, a table record, a block, an entity); references to objects that are
 //! gone are dropped. Commands don't edit it.
 
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, BTreeSet};
 
 use crate::{Handle, RawTag};
 
@@ -94,3 +94,89 @@ impl Foreign {
         self.data_store.iter().filter(|t| t.code == 320).filter_map(|t| Handle::parse_hex(t.value.trim())).collect()
     }
 }
+
+impl Foreign {
+    /// The kept objects by handle (the first of any that share one).
+    fn object_index(&self) -> BTreeMap<Handle, &ForeignObject> {
+        let mut m = BTreeMap::new();
+        for o in &self.objects {
+            m.entry(o.handle).or_insert(o);
+        }
+        m
+    }
+
+    /// The kept object `root` and the kept objects it owns, directly or not (dictionary entries
+    /// and child pointers, groups 350 and 360), `root` first.
+    pub fn tree(&self, root: Handle) -> Vec<Handle> {
+        // Indexed once: a file can keep many objects, and a lookup per pointer would grow with
+        // their product.
+        let by_handle = self.object_index();
+        let mut out = vec![root];
+        let mut seen = BTreeSet::from([root]);
+        let mut i = 0;
+        while let Some(&h) = out.get(i) {
+            i += 1;
+            let Some(o) = by_handle.get(&h) else { continue };
+            for c in o.tags.iter().filter(|t| matches!(t.code, 350 | 360)).filter_map(|t| Handle::parse_hex(t.value.trim())) {
+                if out.len() < 4096 && by_handle.contains_key(&c) && seen.insert(c) {
+                    out.push(c);
+                }
+            }
+        }
+        out
+    }
+
+    /// Copy the objects of `tree` (see [`Foreign::tree`]) of `src` into this one under the handles
+    /// `fresh` (the same order; two more follow for a field list), `owner` taking the place of the
+    /// root's owner. References to objects outside the tree become null. Fields keep a field
+    /// list: this one gets a copy of the source's if it has none.
+    pub fn adopt_tree(&mut self, src: &Foreign, tree: &[Handle], fresh: &[Handle], owner: Handle) {
+        if fresh.len() < tree.len() + 2 {
+            return;
+        }
+        let map: BTreeMap<Handle, Handle> = tree.iter().copied().zip(fresh.iter().copied()).collect();
+        let by_handle = src.object_index();
+        let mut kinds = Vec::new();
+        for (n, h) in tree.iter().enumerate() {
+            let Some(o) = by_handle.get(h) else { continue };
+            let mut c = (*o).clone();
+            let Some(&new) = fresh.get(n) else { continue };
+            c.handle = new;
+            for t in &mut c.tags {
+                if t.code == 5 {
+                    t.value = new.hex();
+                } else if matches!(t.code, 320..=369 | 390..=399) {
+                    let to = Handle::parse_hex(t.value.trim()).and_then(|h| map.get(&h).copied()).or((n == 0 && t.code == 330).then_some(owner));
+                    t.value = to.map_or_else(|| "0".to_string(), Handle::hex);
+                }
+            }
+            kinds.push(c.kind.clone());
+            self.objects.push(c);
+        }
+        if let Some(list) = src.objects.iter().find(|o| o.kind == "FIELDLIST").filter(|_| !self.objects.iter().any(|o| o.kind == "FIELDLIST")) {
+            let (Some(&h), Some(&nod)) = (fresh.get(tree.len()), fresh.get(tree.len() + 1)) else { return };
+            let mut c = list.clone();
+            c.handle = h;
+            for t in &mut c.tags {
+                match t.code {
+                    5 => t.value = h.hex(),
+                    330 => t.value = "0".to_string(),
+                    _ => {}
+                }
+            }
+            if let Some(t) = c.tags.iter_mut().find(|t| t.code == 330) {
+                t.value = nod.hex();
+            }
+            self.targets.insert(nod, ("NOD".to_string(), String::new()));
+            self.dictionary.push(("ACAD_FIELDLIST".to_string(), h));
+            kinds.push(c.kind.clone());
+            self.objects.push(c);
+        }
+        for k in kinds {
+            let named = |c: &Vec<RawTag>| c.iter().any(|t| t.code == 1 && t.value.eq_ignore_ascii_case(&k));
+            if !self.classes.iter().any(named) {
+                self.classes.extend(src.classes.iter().filter(|c| named(c)).cloned());
+            }
+        }
+    }
+}

crates/engine/src/cmd/blocks/insert_file.rs

@@ -109,7 +109,7 @@ fn define_from(s: &mut Session, src: &Drawing, name: &str) -> Result<String> {
         nb.name = to.clone();
         nb.entities = Default::default();
         for e in b.entities.iter() {
-            let e = adopt(&mut d, e, &renames);
+            let e = adopt(&mut d, src, e, &renames);
             nb.entities.push(e);
         }
         d.blocks.insert(to.clone(), Arc::new(nb));
@@ -118,7 +118,7 @@ fn define_from(s: &mut Session, src: &Drawing, name: &str) -> Result<String> {
     b.base = src.header.point("INSBASE").filter(|p| p.x.is_finite() && p.y.is_finite() && p.z.is_finite()).unwrap_or_default();
     let mut ents = Vec::new();
     for e in src.model.iter() {
-        let e = adopt(&mut d, e, &renames);
+        let e = adopt(&mut d, src, e, &renames);
         d.ensure_layer(&e.common.layer);
         ents.push(e);
     }
@@ -183,9 +183,20 @@ fn block_names(d: &Drawing, src: &Drawing, defining: &str) -> BTreeMap<String, S
 
 /// A copy of a source entity for this drawing: a fresh handle, block references renamed, and
 /// links to other source objects (associative dimension points) dropped.
-fn adopt(d: &mut Drawing, e: &Entity, renames: &BTreeMap<String, String>) -> Entity {
+fn adopt(d: &mut Drawing, src: &Drawing, e: &Entity, renames: &BTreeMap<String, String>) -> Entity {
     let mut e = e.clone();
+    let old = e.handle;
     e.handle = d.new_handle();
+    // An attribute definition keeps its field: its extension dictionary and the objects in it.
+    if matches!(e.kind, EntityKind::AttDef(_))
+        && let Some(root) = src.foreign.entities.get(&old).and_then(|x| x.xdict)
+    {
+        let tree = src.foreign.tree(root);
+        let fresh: Vec<Handle> = (0..tree.len() + 2).map(|_| d.new_handle()).collect();
+        let foreign = Arc::make_mut(&mut d.foreign);
+        foreign.adopt_tree(&src.foreign, &tree, &fresh, e.handle);
+        foreign.entities.entry(e.handle).or_default().xdict = fresh.first().copied();
+    }
     match &mut e.kind {
         EntityKind::Insert(i) => {
             if let Some(n) = renames.get(&i.block) {

crates/engine/tests/insert_file_field.rs

@@ -0,0 +1,107 @@
+//! `INSERT name=file` keeps the field of an attribute definition: the definition's extension
+//! dictionary and the objects in it come along under new handles, so the attributes the block's
+//! references get can carry the field when the drawing is saved.
+
+use std::sync::Arc;
+
+use cadcraft_engine::Session;
+use cadcraft_engine::cmd::file::{IoHooks, set_io};
+use cadcraft_engine::doc::foreign::{ForeignExtra, ForeignObject};
+use cadcraft_engine::doc::{Attrib, Drawing, Entity, EntityKind, Handle, RawTag, Text};
+use cadcraft_engine::geom::Vec3;
+use serde_json::json;
+
+#[path = "support/temp_dir.rs"]
+mod temp_dir;
+
+use temp_dir::TempDir;
+
+fn tag(code: i32, value: &str) -> RawTag {
+    RawTag { code, value: value.into() }
+}
+
+fn object(handle: u64, kind: &str, tags: Vec<RawTag>) -> ForeignObject {
+    ForeignObject { handle: Handle(handle), kind: kind.into(), tags }
+}
+
+/// A drawing file in a test format: model space holds a definition NUMBER (handle 0x50) whose
+/// extension dictionary 0x60 owns the ACAD_FIELD dictionary 0x61, which owns the field 0x62 with
+/// a child field 0x63. Handles 0x61..0x63 stand for the file's own; a second, unrelated object
+/// 0x70 is not part of that tree. The bytes `plain` give the same definition without any of it.
+fn read(bytes: &[u8], _name: &str) -> Result<Drawing, String> {
+    let mut d = Drawing::new_imperial();
+    let text = Text {
+        insert: Vec3::ZERO,
+        align_pt: None,
+        height: 2.0,
+        value: "TEMPLATE".into(),
+        rotation: 0.0,
+        width_factor: 1.0,
+        oblique: 0.0,
+        style: "Standard".into(),
+        halign: Default::default(),
+        valign: Default::default(),
+    };
+    let def = Attrib {
+        tag: "NUMBER".into(),
+        text,
+        invisible: false,
+        constant: false,
+        prompt: String::new(),
+        props: Default::default(),
+        multiline: None,
+        modes: Default::default(),
+    };
+    d.model.push(Entity::new(Handle(0x50), EntityKind::AttDef(def)));
+    if bytes == b"plain" {
+        return Ok(d);
+    }
+    let f = Arc::make_mut(&mut d.foreign);
+    f.entities.insert(Handle(0x50), ForeignExtra { xdict: Some(Handle(0x60)), ..Default::default() });
+    f.objects.push(object(0x60, "DICTIONARY", vec![tag(5, "60"), tag(330, "50"), tag(3, "ACAD_FIELD"), tag(360, "61")]));
+    f.objects.push(object(0x61, "DICTIONARY", vec![tag(5, "61"), tag(330, "60"), tag(3, "TEXT"), tag(360, "62")]));
+    f.objects.push(object(0x62, "FIELD", vec![tag(5, "62"), tag(330, "61"), tag(1, "_text"), tag(360, "63")]));
+    f.objects.push(object(0x63, "FIELD", vec![tag(5, "63"), tag(330, "62"), tag(1, "AcVar")]));
+    f.objects.push(object(0x70, "XRECORD", vec![tag(5, "70"), tag(330, "0")]));
+    f.classes.push(vec![tag(1, "FIELD"), tag(2, "AcDbField")]);
+    Ok(d)
+}
+
+fn hooks() {
+    set_io(IoHooks { read, write: |_, _| Err("no writer in this test".into()), plot: None });
+}
+
+#[test]
+fn the_definition_of_an_inserted_file_keeps_its_field() {
+    hooks();
+    let dir = TempDir::new("insert-file-field");
+    let path = dir.file("border.dxf", "");
+    let mut s = Session::new();
+    s.execute("insert", &json!({ "name": "BORDER", "file": path, "at": [0, 0], "attribs": { "NUMBER": "A-1" } })).unwrap();
+    let d = s.doc().unwrap();
+    let def = d.block("BORDER").and_then(|b| b.entities.iter().find(|e| matches!(e.kind, EntityKind::AttDef(_)))).expect("the definition");
+    assert_ne!(def.handle, Handle(0x50), "the definition has a handle of this drawing");
+    let root = d.foreign.entities.get(&def.handle).and_then(|x| x.xdict).expect("the definition has an extension dictionary");
+    let tree = d.foreign.tree(root);
+    assert_eq!(tree.len(), 4, "the dictionaries and both fields, not the unrelated object");
+    assert!(tree.iter().all(|h| ![0x60, 0x61, 0x62, 0x63, 0x70].contains(&h.0)), "the copies have new handles");
+    // The root's owner is the definition, the others point at each other.
+    let owner =
+        |h: Handle| d.foreign.objects.iter().find(|o| o.handle == h).and_then(|o| o.tags.iter().find(|t| t.code == 330)).map(|t| t.value.clone());
+    assert_eq!(owner(tree[0]), Some(def.handle.hex()));
+    assert_eq!(owner(tree[1]), Some(tree[0].hex()));
+    assert_eq!(owner(tree[3]), Some(tree[2].hex()));
+    assert!(d.foreign.classes.iter().any(|c| c.iter().any(|t| t.code == 1 && t.value == "FIELD")), "the field class comes along");
+}
+
+#[test]
+fn a_definition_without_a_field_gets_none() {
+    hooks();
+    let dir = TempDir::new("insert-file-nofield");
+    let path = dir.file("plain.dxf", "plain");
+    let mut s = Session::new();
+    s.execute("insert", &json!({ "name": "PLAIN", "file": path, "at": [0, 0] })).unwrap();
+    let d = s.doc().unwrap();
+    assert!(d.block("PLAIN").is_some());
+    assert!(d.foreign.is_empty(), "nothing is kept for a drawing without fields");
+}