Merge storytold/cadcraft#2049 (commits after the batch)

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

@@ -140,8 +140,11 @@ struct Signer<'a> {
     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 the
-    /// signatures of a multileader and a dimension cover their blocks'.
+    /// 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.
     frames: Option<Option<&'a std::path::Path>>,
 }
 
@@ -190,9 +193,14 @@ impl<'a> Signer<'a> {
             return *v;
         }
         let mut h = std::collections::hash_map::DefaultHasher::new();
-        key.hash(&mut h);
+        let block = self.d.block(name);
+        // With frames, an anonymous block (`*D3`, `*U7`) is signed by its contents only: the
+        // same block can have another generated name in another drawing.
+        if !(self.frames.is_some() && block.is_some_and(|b| b.anonymous)) {
+            key.hash(&mut h);
+        }
         if depth < MAX_BLOCK_DEPTH
-            && let Some(b) = self.d.block(name)
+            && let Some(b) = block
         {
             if let Some(folder) = self.frames {
                 let mut xref = b.xref_path.clone();
@@ -206,8 +214,20 @@ 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;
+                    format!("{:?}|{}|{}|{:?}", c.transparency, c.visible, (c.thickness * 1e6).round(), c.extrusion).hash(&mut h);
+                }
             }
         }
         let v = h.finish();

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_<name>`, `COMPARE_COMPARED_<name>`, `COMPARE_COMMON_<name>`), and a
-//! table or tolerance uses a recolored copy of its style; xrefs keep their own colors. The new
+//! color (`COMPARE_CURRENT_<name>`, `COMPARE_COMPARED_<name>`, `COMPARE_COMMON_<name>`; 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) => {
@@ -358,13 +359,21 @@ fn paint(d: &mut Drawing, e: &mut Entity, color: Color, tinted: &mut Tinted, dep
     };
     for a in attribs.into_iter().flatten() {
         a.props.color = Some(color);
+        if let Some(ml) = a.multiline.as_mut() {
+            ml.mtext.contents = without_color_codes(&ml.mtext.contents);
+        }
     }
-    // Colors an object carries apart from its own: inline MTEXT colors, a dimension style's
-    // DIMCLRD, DIMCLRE and DIMCLRT, gradient colors, multiline element colors, the style colors of
-    // a table or a tolerance (through a recolored copy of the style) and inline cell colors. Hatch
-    // backgrounds, multiline fills and table cell fills are left out.
+    // Colors an object carries apart from its own: inline MTEXT colors (in multiline attributes
+    // too), a dimension style's DIMCLRD, DIMCLRE and DIMCLRT, gradient colors, multiline element
+    // colors, the style colors of a table or a tolerance (through a recolored copy of the style)
+    // and inline cell colors. Hatch backgrounds, multiline fills and table cell fills are left out.
     match &mut e.kind {
         EntityKind::MText(t) => t.contents = without_color_codes(&t.contents),
+        EntityKind::AttDef(a) => {
+            if let Some(ml) = a.multiline.as_mut() {
+                ml.mtext.contents = without_color_codes(&ml.mtext.contents);
+            }
+        }
         EntityKind::MLeader(m) => {
             if let Some(t) = m.text.as_mut() {
                 t.contents = without_color_codes(&t.contents);
@@ -516,11 +525,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() };

crates/engine/tests/it/compare_export.rs

@@ -360,6 +360,175 @@ fn a_named_block_whose_dimension_draws_another_block_shows_the_other_definition(
     assert!(!d.blocks.keys().any(|n| dim_copy(n)), "{:?}", d.blocks.keys().collect::<Vec<_>>());
 }
 
+#[test]
+fn a_named_block_whose_dimension_block_has_another_generated_name_keeps_this_drawings_definition() {
+    use cadcraft_engine::doc::{DimKind, Dimension};
+    // PART holds the same dimension in both drawings; the block it draws holds the same line but
+    // is named `*D1` here and `*D3` in the other drawing.
+    let blocks = |drawn_name: &str, first: u64| {
+        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(drawn_name.into()),
+            overrides: Default::default(),
+            assoc: Vec::new(),
+            marks: Default::default(),
+        };
+        let mut part = Block::new("PART");
+        part.entities.push(Entity::new(Handle(first), EntityKind::Dimension(dim)));
+        let mut drawn = Block::new(drawn_name);
+        drawn.entities.push(Entity::new(Handle(first + 1), line(1.0)));
+        [part, drawn]
+    };
+    let mut s = Session::new();
+    for b in blocks("*D1", 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("*D3", 0x8000) {
+        other.blocks.insert(b.name.clone(), Arc::new(b));
+    }
+    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();
+    s.execute("compareexport", &json!({})).unwrap();
+    assert!(s.doc().unwrap().block("PART_2").is_none(), "PART draws the same thing in both drawings");
+}
+
+#[test]
+fn a_named_block_whose_line_is_hidden_or_thicker_in_the_other_drawing_shows_the_other_definition() {
+    let run = |theirs: Common| {
+        let part = |common: Common, first: u64| {
+            let mut part = Block::new("PART");
+            let mut e = Entity::new(Handle(first), line(1.0));
+            e.common = common;
+            part.entities.push(e);
+            part
+        };
+        let mut s = Session::new();
+        s.doc_mut().unwrap().blocks.insert("PART".into(), Arc::new(part(Common::default(), 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(theirs, 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", None);
+        s.execute("document.switch", &json!({"index": 0})).unwrap();
+        s.execute("compare", &json!({"document": "Other"})).unwrap();
+        s.execute("compareexport", &json!({})).unwrap();
+        s.doc().unwrap().block("PART_2").is_some()
+    };
+    assert!(run(Common { visible: false, ..Common::default() }), "a hidden line");
+    assert!(run(Common { thickness: 2.0, ..Common::default() }), "a line with thickness");
+    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<String> = 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<f64> = 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();
@@ -448,6 +617,51 @@ fn colors_carried_apart_from_the_object_color_are_replaced() {
     assert_eq!(d.dim_style("Standard").unwrap().dim_line_color, Color::Index(1));
 }
 
+#[test]
+fn inline_colors_of_multiline_attributes_are_replaced() {
+    use cadcraft_engine::doc::Attrib;
+    let attrib = |contents: &str| -> Attrib {
+        let at = serde_json::to_value(Vec3::ZERO).unwrap();
+        serde_json::from_value(json!({
+            "tag": "T",
+            "text": {"insert": at, "height": 1.0, "value": "x"},
+            "multiline": {"mtext": {"insert": at, "height": 1.0, "contents": contents}, "readAs": "x"},
+        }))
+        .unwrap()
+    };
+    let mut s = Session::new();
+    let mut part = Block::new("PART");
+    part.entities.push(Entity::new(Handle(0x700), EntityKind::AttDef(attrib("\\C1;x"))));
+    s.doc_mut().unwrap().blocks.insert("PART".into(), Arc::new(part));
+    let reference = Insert { attribs: vec![attrib("\\C3;x")], ..insert("PART") };
+    s.doc_mut().unwrap().add(&Space::Model, Common::default(), EntityKind::Insert(reference)).unwrap();
+    s.open_drawing(Drawing::new_imperial(), "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 multiline = |a: &Attrib| a.multiline.as_ref().map(|m| m.mtext.contents.clone());
+    let mut blocks = Vec::new();
+    for e in members(&s, r["current"].as_str().unwrap()) {
+        if let EntityKind::Insert(i) = &e.kind {
+            assert_eq!(i.attribs.iter().map(multiline).collect::<Vec<_>>(), [Some("x".to_string())], "the reference's attribute");
+            blocks.push(i.block.clone());
+        }
+    }
+    assert_eq!(blocks.len(), 1);
+    let d = s.doc().unwrap();
+    let defs: Vec<Option<String>> = d
+        .block(&blocks[0])
+        .unwrap()
+        .entities
+        .iter()
+        .filter_map(|e| match &e.kind {
+            EntityKind::AttDef(a) => Some(multiline(a)),
+            _ => None,
+        })
+        .collect();
+    assert_eq!(defs, [Some("x".to_string())], "the attribute definition in the recolored block");
+}
+
 #[test]
 fn edits_after_compare_count_and_objects_keep_their_layers() {
     let mut s = Session::new();