diff --git a/crates/engine/src/cmd/file/compare.rs b/crates/engine/src/cmd/file/compare.rs index 58629f025..d4e2878e8 100644 --- a/crates/engine/src/cmd/file/compare.rs +++ b/crates/engine/src/cmd/file/compare.rs @@ -140,7 +140,8 @@ struct Signer<'a> { 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 the + /// flags and xref file, with relative xref, image and data link paths taken from the folder + /// given, and the /// signatures of a multileader and a dimension cover their blocks'. Each object in a block /// then also counts with its visibility, transparency, thickness and extrusion, and an /// anonymous block's signature leaves out its name. @@ -213,7 +214,15 @@ impl<'a> Signer<'a> { break; } self.work += 1; - self.geometry(&e.kind, depth + 1).hash(&mut h); + match self.frames { + // An image's file and a table's data link, from the folder given. + Some(folder) if matches!(e.kind, EntityKind::Image(_) | EntityKind::Table(_)) => { + let mut k = e.kind.clone(); + export::rebase(&mut k, folder); + self.geometry(&k, depth + 1).hash(&mut h); + } + _ => self.geometry(&e.kind, depth + 1).hash(&mut h), + } properties(e).hash(&mut h); if self.frames.is_some() { let c = &e.common; diff --git a/crates/engine/src/cmd/file/compare/export.rs b/crates/engine/src/cmd/file/compare/export.rs index 078006781..88a1c4350 100644 --- a/crates/engine/src/cmd/file/compare/export.rs +++ b/crates/engine/src/cmd/file/compare/export.rs @@ -5,8 +5,9 @@ //! only in this drawing or changed in it are drawn in the overlay's green, objects only in the //! other drawing or changed there in its red, and objects in both in gray (color 8). A block //! reference, dimension or multileader refers to a copy of its block whose objects all have that -//! color (`COMPARE_CURRENT_`, `COMPARE_COMPARED_`, `COMPARE_COMMON_`), and a -//! table or tolerance uses a recolored copy of its style; xrefs keep their own colors. The new +//! color (`COMPARE_CURRENT_`, `COMPARE_COMPARED_`, `COMPARE_COMMON_`; for a +//! dimension's `*D` block, another `*D` block), and a table or tolerance uses a recolored copy +//! of its style; xrefs keep their own colors. The new //! drawing has this drawing's layers, styles and blocks, plus the layers and styles of the other //! drawing that this one lacks and the blocks its copied objects refer to; an anonymous block of //! the other drawing whose name this one uses is copied under a free name. Relative xref, image @@ -130,7 +131,7 @@ pub(super) fn rebase_path(p: &mut String, folder: Option<&Path>) { } /// Rebase the file an object refers to: an image's file, a table's data link. -fn rebase(k: &mut EntityKind, folder: Option<&Path>) { +pub(super) fn rebase(k: &mut EntityKind, folder: Option<&Path>) { match k { EntityKind::Image(i) => rebase_path(&mut i.path, folder), EntityKind::Table(t) => { @@ -518,11 +519,18 @@ fn tinted_block(d: &mut Drawing, name: &str, color: Color, tinted: &mut Tinted, return None; } tinted.copied += b.entities.len(); - let new = free_name(&format!("COMPARE_{}_{}", tag(color), b.name.trim_start_matches('*')), d.blocks.len(), |n| d.block(n).is_some()); + // A dimension's block copy stays a `*D` block, so saving keeps its geometry instead of + // drawing the dimension again. + let dim_block = b.anonymous && b.name.get(..2).is_some_and(|p| p.eq_ignore_ascii_case("*D")); + let new = if dim_block { + (1..=d.blocks.len() + 1).map(|i| format!("*D{i}")).find(|n| d.block(n).is_none()).unwrap_or_else(|| format!("*D{}", d.blocks.len() + 2)) + } else { + free_name(&format!("COMPARE_{}_{}", tag(color), b.name.trim_start_matches('*')), d.blocks.len(), |n| d.block(n).is_some()) + }; tinted.names.insert(key, new.clone()); // Hold the name while the nested copies pick theirs. d.blocks.insert(new.clone(), Arc::new(Block::new(&new))); - let mut copy = Block { name: new.clone(), anonymous: false, entities: EntityStore::new(), ..(*b).clone() }; + let mut copy = Block { name: new.clone(), anonymous: dim_block, entities: EntityStore::new(), ..(*b).clone() }; for e in b.entities.iter() { let h = d.new_handle(); let mut e = Entity { handle: h, ..(**e).clone() }; diff --git a/crates/engine/tests/compare_export.rs b/crates/engine/tests/compare_export.rs index 643c8b86b..c001f6bd7 100644 --- a/crates/engine/tests/compare_export.rs +++ b/crates/engine/tests/compare_export.rs @@ -397,9 +397,7 @@ fn a_named_block_whose_dimension_block_has_another_generated_name_keeps_this_dra s.execute("document.switch", &json!({"index": 0})).unwrap(); s.execute("compare", &json!({"document": "Other"})).unwrap(); s.execute("compareexport", &json!({})).unwrap(); - let d = s.doc().unwrap(); - assert!(d.block("PART_2").is_none(), "PART draws the same thing in both drawings"); - assert!(d.block("*D3").is_none() && d.block("*D2").is_none(), "the other drawing's dimension block is not copied"); + assert!(s.doc().unwrap().block("PART_2").is_none(), "PART draws the same thing in both drawings"); } #[test] @@ -430,6 +428,99 @@ fn a_named_block_whose_line_is_hidden_or_thicker_in_the_other_drawing_shows_the_ assert!(!run(Common::default()), "the same line"); } +#[test] +fn a_named_block_with_the_same_relative_image_path_from_another_folder_shows_the_other_definition() { + // PART holds an image at `a.png` in both drawings; this drawing is unsaved, the other one is + // a file in another folder, so its `a.png` is another file. + let part = |first: u64| { + let at = serde_json::to_value(Vec3::ZERO).unwrap(); + let image = serde_json::from_value(json!({"insert": at, "u": at, "v": at, "size": [1.0, 1.0], "path": "a.png"})).unwrap(); + let mut part = Block::new("PART"); + part.entities.push(Entity::new(Handle(first), EntityKind::Image(image))); + part + }; + let dir = std::env::temp_dir().join("compareexport-image-folder"); + let mut s = Session::new(); + s.doc_mut().unwrap().blocks.insert("PART".into(), Arc::new(part(0x700))); + s.doc_mut().unwrap().add(&Space::Model, Common::default(), EntityKind::Insert(insert("PART"))).unwrap(); + let mut other = Drawing::new_imperial(); + other.handseed = 0x9000; + other.blocks.insert("PART".into(), Arc::new(part(0x8000))); + 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", Some(dir.join("other.dxf").to_string_lossy().to_string())); + s.execute("document.switch", &json!({"index": 0})).unwrap(); + s.execute("compare", &json!({"document": "Other"})).unwrap(); + s.execute("compareexport", &json!({})).unwrap(); + let d = s.doc().unwrap(); + let paths: Vec = d + .block("PART_2") + .map(|b| { + b.entities + .iter() + .filter_map(|e| match &e.kind { + EntityKind::Image(i) => Some(i.path.clone()), + _ => None, + }) + .collect() + }) + .unwrap_or_default(); + assert_eq!(paths.len(), 1, "the other drawing's PART is copied"); + assert_eq!(std::path::Path::new(&paths[0]).parent(), Some(std::path::absolute(&dir).unwrap().as_path())); +} + +#[test] +fn a_dimension_block_copy_keeps_its_geometry_when_the_snapshot_is_saved() { + use cadcraft_engine::doc::{DimKind, Dimension}; + // The dimension's block holds a line its parameters would not draw: a block read from a file. + let dim = Dimension { + kind: DimKind::Linear { rotation: 0.0 }, + defpt: Vec3::new(15.0, 5.0, 0.0), + text_mid: Vec3::new(7.0, 5.0, 0.0), + p13: Vec3::ZERO, + p14: Vec3::new(30.0, 0.0, 0.0), + p15: Vec3::ZERO, + p16: Vec3::ZERO, + text: String::new(), + style: "Standard".into(), + measurement: 30.0, + text_rotation: 0.0, + user_text_pos: false, + block: Some("*D1".into()), + overrides: Default::default(), + assoc: Vec::new(), + marks: Default::default(), + }; + let mut s = Session::new(); + let mut drawn = Block::new("*D1"); + drawn.entities.push(Entity::new(Handle(0x701), line(123.0))); + s.doc_mut().unwrap().blocks.insert("*D1".into(), Arc::new(drawn)); + s.doc_mut().unwrap().add(&Space::Model, Common::default(), EntityKind::Dimension(dim)).unwrap(); + s.open_drawing(Drawing::new_imperial(), "Other", None); + s.execute("document.switch", &json!({"index": 0})).unwrap(); + s.execute("compare", &json!({"document": "Other"})).unwrap(); + s.execute("compareexport", &json!({})).unwrap(); + let read = cadcraft_io::read_dxf(cadcraft_io::write_dxf(s.doc().unwrap()).as_bytes()).unwrap(); + let block = read + .model + .iter() + .find_map(|e| match &e.kind { + EntityKind::Dimension(dm) => dm.block.clone(), + _ => None, + }) + .unwrap(); + let ends: Vec = read + .block(&block) + .unwrap() + .entities + .iter() + .filter_map(|e| match &e.kind { + EntityKind::Line(l) => Some(l.b.x), + _ => None, + }) + .collect(); + assert_eq!(ends, [123.0], "the saved dimension block is the copy's"); +} + #[test] fn block_contents_take_the_comparison_color() { let mut s = Session::new();