diff --git a/crates/engine/src/cmd/file/compare.rs b/crates/engine/src/cmd/file/compare.rs index 2003d2079..32e1486f6 100644 --- a/crates/engine/src/cmd/file/compare.rs +++ b/crates/engine/src/cmd/file/compare.rs @@ -139,11 +139,19 @@ struct Signer<'a> { d: &'a Drawing, blocks: HashMap, 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>, } 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; diff --git a/crates/engine/src/cmd/file/compare/export.rs b/crates/engine/src/cmd/file/compare/export.rs index 2ebae8b14..0e1aa4036 100644 --- a/crates/engine/src/cmd/file/compare/export.rs +++ b/crates/engine/src/cmd/file/compare/export.rs @@ -119,7 +119,7 @@ fn folder_of(path: Option<&str>) -> Option { } /// `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; /// 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> = 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); diff --git a/crates/engine/tests/compare_export.rs b/crates/engine/tests/compare_export.rs index 76e9ee9e8..1ec7278fd 100644 --- a/crates/engine/tests/compare_export.rs +++ b/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 = 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 { + 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();