COMPAREEXPORT: a named block the other drawing defines differently is copied as PART_2

A block reference of the other drawing whose named block exists in this
drawing with other contents drew this drawing's definition in the red
group. The export now keeps this drawing's block only when both
drawings' blocks draw the same thing: the same objects, base point,
units, annotative and overlay flags and xref file, in the block and in
the blocks its references and multileaders use. Otherwise the other
drawing's block is copied under a free name (PART_2, PART_3, ...) and
its references point at the copy.

crates/engine/src/cmd/file/compare.rs

@@ -139,11 +139,19 @@ struct Signer<'a> {
     d: &'a Drawing,
     blocks: HashMap<String, u64>,
     work: usize,
+    /// When set, a block's signature also covers its base point, units, annotative and overlay
+    /// flags and xref file, with a relative xref path taken from the folder given, and a
+    /// multileader's signature covers its block's.
+    frames: Option<Option<&'a std::path::Path>>,
 }
 
 impl<'a> Signer<'a> {
     fn new(d: &'a Drawing) -> Self {
-        Signer { d, blocks: HashMap::new(), work: 0 }
+        Signer { d, blocks: HashMap::new(), work: 0, frames: None }
+    }
+    /// A signer whose block signatures also cover what changes how a reference draws the block.
+    fn with_frames(d: &'a Drawing, folder: Option<&'a std::path::Path>) -> Self {
+        Signer { frames: Some(folder), ..Signer::new(d) }
     }
     fn geometry(&mut self, k: &EntityKind, depth: usize) -> u64 {
         let mut h = std::collections::hash_map::DefaultHasher::new();
@@ -162,6 +170,12 @@ impl<'a> Signer<'a> {
         if let EntityKind::Insert(i) = k {
             self.block(&i.block, depth).hash(&mut h);
         }
+        if self.frames.is_some()
+            && let EntityKind::MLeader(m) = k
+            && let Some(b) = &m.block
+        {
+            self.block(&b.block, depth).hash(&mut h);
+        }
         h.finish()
     }
     fn block(&mut self, name: &str, depth: usize) -> u64 {
@@ -174,6 +188,13 @@ impl<'a> Signer<'a> {
         if depth < MAX_BLOCK_DEPTH
             && let Some(b) = self.d.block(name)
         {
+            if let Some(folder) = self.frames {
+                let mut xref = b.xref_path.clone();
+                if let Some(x) = xref.as_mut() {
+                    export::rebase_path(x, folder);
+                }
+                (format!("{:?}", b.base), b.units, b.annotative, b.xref_overlay, xref).hash(&mut h);
+            }
             for e in b.entities.iter() {
                 if self.work >= MAX_BLOCK_WORK {
                     break;

crates/engine/src/cmd/file/compare/export.rs

@@ -119,7 +119,7 @@ fn folder_of(path: Option<&str>) -> Option<PathBuf> {
 }
 
 /// `p` as an absolute path from `folder` when it is relative.
-fn rebase_path(p: &mut String, folder: Option<&Path>) {
+pub(super) fn rebase_path(p: &mut String, folder: Option<&Path>) {
     if let Some(folder) = folder
         && !p.trim().is_empty()
         && Path::new(p.as_str()).is_relative()
@@ -531,8 +531,11 @@ fn tinted_block(d: &mut Drawing, name: &str, color: Color, tinted: &mut Tinted,
 type Renames = HashMap<String, String>;
 
 /// Copy into `d` the blocks of `other` that `objs` refer to, directly or through other blocks.
-/// A named block `d` already has keeps `d`'s definition. An anonymous block (`*D3`, `*U7`…)
-/// whose name `d` already uses is copied under the next free name with the same prefix. Copied
+/// A named block `d` already has keeps `d`'s definition when it draws the same thing there (the
+/// same objects, base point, units, flags and xref file, in it and in the blocks it refers to);
+/// otherwise the other drawing's block is copied as `PART_2` (or `PART_3`…). An anonymous block
+/// (`*D3`, `*U7`…) whose name `d` already uses is copied under the next free name with the same
+/// prefix. Copied
 /// objects get new handles, and relative file paths are rebased on `folder`. The flag is true
 /// when more than [`MAX_BLOCKS`] blocks or [`MAX_COPIED`] objects were needed and the rest were
 /// left out.
@@ -547,6 +550,8 @@ fn copy_blocks<'a>(
     let mut seen = HashSet::new();
     let mut needed: Vec<Arc<Block>> = Vec::new();
     let mut left_out = false;
+    // `d`'s relative paths were made absolute from its own folder before the copy.
+    let (mut mine, mut theirs) = (super::Signer::with_frames(d, None), super::Signer::with_frames(other, folder));
     while let Some(n) = frontier.pop() {
         let key = n.to_ascii_uppercase();
         if seen.contains(&key) {
@@ -558,7 +563,14 @@ fn copy_blocks<'a>(
         }
         seen.insert(key);
         let Some(b) = other.block(&n) else { continue };
-        if d.block(&n).is_some() && !b.anonymous {
+        // Past the signers' work limit the signatures no longer cover every object, so the block
+        // is copied.
+        if !b.anonymous
+            && d.block(&n).is_some()
+            && mine.block(&n, 0) == theirs.block(&n, 0)
+            && mine.work < super::MAX_BLOCK_WORK
+            && theirs.work < super::MAX_BLOCK_WORK
+        {
             continue;
         }
         if tinted.copied.saturating_add(b.entities.len()) > MAX_COPIED {
@@ -577,8 +589,12 @@ fn copy_blocks<'a>(
         renames.insert(b.name.to_ascii_uppercase(), b.name.clone());
     }
     for b in needed.iter().filter(|b| d.block(&b.name).is_some()) {
-        let prefix: String = b.name.chars().take_while(|c| *c == '*' || c.is_ascii_alphabetic()).collect();
-        let free = (1..=taken.len() + 1).map(|i| format!("{prefix}{i}")).find(|n| !taken.contains(&n.to_ascii_uppercase()));
+        let free = if b.anonymous {
+            let prefix: String = b.name.chars().take_while(|c| *c == '*' || c.is_ascii_alphabetic()).collect();
+            (1..=taken.len() + 1).map(|i| format!("{prefix}{i}")).find(|n| !taken.contains(&n.to_ascii_uppercase()))
+        } else {
+            Some(free_name(&b.name, taken.len(), |n| taken.contains(&n.to_ascii_uppercase())))
+        };
         let Some(free) = free else { continue };
         taken.insert(free.to_ascii_uppercase());
         renames.insert(b.name.to_ascii_uppercase(), free);

crates/engine/tests/compare_export.rs

@@ -6,7 +6,7 @@ use std::sync::Arc;
 use cadcraft_engine::Session;
 use cadcraft_engine::doc::color::{Color, Rgb};
 use cadcraft_engine::doc::geom::Vec3;
-use cadcraft_engine::doc::{Block, CellStyle, Common, Drawing, Entity, EntityKind, Handle, Insert, Layer, Space};
+use cadcraft_engine::doc::{Block, CellStyle, Common, Drawing, Entity, EntityKind, Handle, Insert, Layer, MLeader, Space};
 use serde_json::json;
 
 fn insert(block: &str) -> Insert {
@@ -194,6 +194,101 @@ fn anonymous_blocks_of_the_other_drawing_keep_their_own_definition() {
     assert_eq!(ends(&s, "COMPARE_CURRENT_U1"), vec![1.0]);
 }
 
+#[test]
+fn a_named_block_defined_differently_in_the_other_drawing_shows_the_other_definition() {
+    // PART holds a line, a reference to SUB, which holds a line of length 2, and a multileader
+    // whose content is SYMBOL, which holds one line.
+    let blocks = |part_end: f64, part_base: f64, sub_base: f64, symbol_end: f64, first: u64| {
+        let mut part = Block::new("PART");
+        part.base = Vec3::new(part_base, 0.0, 0.0);
+        part.entities.push(Entity::new(Handle(first), line(part_end)));
+        part.entities.push(Entity::new(Handle(first + 1), EntityKind::Insert(insert("SUB"))));
+        let leader = MLeader {
+            leaders: Vec::new(),
+            landing: Vec3::ZERO,
+            dogleg: 0.0,
+            text: None,
+            style: "Standard".into(),
+            arrow_size: 1.0,
+            spline: false,
+            block: Some(insert("SYMBOL")),
+        };
+        part.entities.push(Entity::new(Handle(first + 2), EntityKind::MLeader(leader)));
+        let mut sub = Block::new("SUB");
+        sub.base = Vec3::new(sub_base, 0.0, 0.0);
+        sub.entities.push(Entity::new(Handle(first + 3), line(2.0)));
+        let mut symbol = Block::new("SYMBOL");
+        symbol.entities.push(Entity::new(Handle(first + 4), line(symbol_end)));
+        [part, sub, symbol]
+    };
+    let run = |theirs_end: f64, theirs_base: f64, theirs_sub_base: f64, theirs_symbol_end: f64| {
+        let mut s = Session::new();
+        for b in blocks(1.0, 0.0, 0.0, 3.0, 0x700) {
+            s.doc_mut().unwrap().blocks.insert(b.name.clone(), Arc::new(b));
+        }
+        s.doc_mut().unwrap().add(&Space::Model, Common::default(), EntityKind::Insert(insert("PART"))).unwrap();
+        let mut other = Drawing::new_imperial();
+        other.handseed = 0x9000;
+        for b in blocks(theirs_end, theirs_base, theirs_sub_base, theirs_symbol_end, 0x8000) {
+            other.blocks.insert(b.name.clone(), Arc::new(b));
+        }
+        // A second reference, so the other drawing has one that differs even when PART does not.
+        other.add(&Space::Model, Common::default(), EntityKind::Insert(Insert { insert: Vec3::new(9.0, 9.0, 0.0), ..insert("PART") })).unwrap();
+        s.open_drawing(other, "Other", None);
+        s.execute("document.switch", &json!({"index": 0})).unwrap();
+        s.execute("compare", &json!({"document": "Other"})).unwrap();
+        let r = s.execute("compareexport", &json!({})).unwrap();
+        let referenced: Vec<String> = members(&s, r["compared"].as_str().unwrap())
+            .iter()
+            .filter_map(|e| match &e.kind {
+                EntityKind::Insert(i) => Some(i.block.clone()),
+                _ => None,
+            })
+            .collect();
+        (s, referenced)
+    };
+    // The other drawing's PART draws a longer line: it is copied as PART_2 and shown in red.
+    let (s, referenced) = run(5.0, 0.0, 0.0, 3.0);
+    assert_eq!(ends(&s, "PART"), vec![1.0], "this drawing's definition is kept");
+    assert_eq!(ends(&s, "PART_2"), vec![5.0]);
+    assert_eq!(referenced, vec!["COMPARE_COMPARED_PART_2".to_string()]);
+    assert_eq!(ends(&s, "COMPARE_COMPARED_PART_2"), vec![5.0]);
+    let d = s.doc().unwrap();
+    assert!(d.block("SUB_2").is_none() && d.block("SYMBOL_2").is_none(), "SUB and SYMBOL are the same in both drawings");
+    // The blocks a copy of PART refers to.
+    let refers = |s: &Session| -> Vec<String> {
+        s.doc()
+            .unwrap()
+            .block("PART_2")
+            .unwrap()
+            .entities
+            .iter()
+            .flat_map(|e| match &e.kind {
+                EntityKind::Insert(i) => vec![i.block.clone()],
+                EntityKind::MLeader(m) => m.block.iter().map(|b| b.block.clone()).collect(),
+                _ => Vec::new(),
+            })
+            .collect()
+    };
+    // Another base point on PART, another base point on the SUB inside it, or another SYMBOL
+    // in its multileader: PART is copied too, with the blocks that differ.
+    let (s, referenced) = run(1.0, 3.0, 0.0, 3.0);
+    assert_eq!(s.doc().unwrap().block("PART_2").map(|b| b.base.x), Some(3.0));
+    assert_eq!(referenced, vec!["COMPARE_COMPARED_PART_2".to_string()]);
+    assert_eq!(refers(&s), ["SUB", "SYMBOL"]);
+    let (s, _) = run(1.0, 0.0, 4.0, 3.0);
+    assert_eq!(s.doc().unwrap().block("SUB_2").map(|b| b.base.x), Some(4.0));
+    assert_eq!(refers(&s), ["SUB_2", "SYMBOL"]);
+    let (s, _) = run(1.0, 0.0, 0.0, 6.0);
+    assert_eq!(ends(&s, "SYMBOL_2"), vec![6.0]);
+    assert_eq!(refers(&s), ["SUB", "SYMBOL_2"]);
+    // The same definitions in both drawings: the red reference uses this drawing's PART.
+    let (s, referenced) = run(1.0, 0.0, 0.0, 3.0);
+    assert!(s.doc().unwrap().block("PART_2").is_none());
+    assert_eq!(referenced, vec!["COMPARE_COMPARED_PART".to_string()]);
+    assert_eq!(ends(&s, "COMPARE_COMPARED_PART"), vec![1.0]);
+}
+
 #[test]
 fn block_contents_take_the_comparison_color() {
     let mut s = Session::new();