From 4dbcbfdaaa070e7f904f6113f2b2e8c52dc9b4c4 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 24 Oct 2026 19:37:44 -0700 Subject: [PATCH] COMPAREEXPORT: dimension block copies stay *D blocks, and image paths count in block reuse A dimension's block took a recolored copy named COMPARE__D1, and the DXF writer redraws a dimension whose block is not a *D block, so saving the snapshot replaced geometry read from a file. The copy now gets a free *D name. A named block both drawings define was reused when its images and data links had the same relative paths, even when they were relative to different folders. These paths are now taken from each drawing's folder before the blocks are compared. --- crates/engine/src/cmd/file/compare.rs | 13 ++- crates/engine/src/cmd/file/compare/export.rs | 18 +++- crates/engine/tests/compare_export.rs | 97 +++++++++++++++++++- 3 files changed, 118 insertions(+), 10 deletions(-) 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();