Place: placing the same file again shares the symbols and patterns it brought

Symbol and pattern art copied in by a placement takes ids of the
document's own, so the next placement of the same file compared it with
the file's art, found the ids different and added Star 2, Dots 2 and so
on. Definitions are now compared by what their art draws, whatever its
ids. A symbol, pattern or brush whose art uses an image, symbol, pattern,
brush or swatch that comes in under another name is copied under a free
name instead of shared.

crates/engine/src/cmd/place/adopt.rs

@@ -10,7 +10,7 @@ use std::sync::Arc;
 
 use vectorcraft_brush::{Brush, BrushKind};
 use vectorcraft_color::Paint;
-use vectorcraft_doc::{AppearanceItem, Document, Node, NodeKind};
+use vectorcraft_doc::{AppearanceItem, Document, Node, NodeKind, PatternDef, Symbol};
 
 use super::unique_name;
 
@@ -118,11 +118,12 @@ impl Used {
 type Renames = BTreeMap<String, (Option<String>, bool)>;
 
 /// Names in `dst` for the `used` definitions of `src` (`def`): the same name when `dst` has none
-/// (copied) or an equal one (shared), else a free variant (`taken`, copied).
-fn names<T: PartialEq>(
+/// (copied) or one that is the `same` (shared), else a free variant (`taken`, copied).
+fn names<T>(
     used: &[String],
     def: impl Fn(&str) -> Option<T>,
     existing: impl Fn(&str) -> Option<T>,
+    same: impl Fn(&T, &T) -> bool,
     taken: impl Fn(&str) -> bool,
 ) -> Renames {
     let mut out = Renames::new();
@@ -130,7 +131,7 @@ fn names<T: PartialEq>(
         let Some(d) = def(name) else { continue };
         let entry = match existing(name) {
             None => (Some(name.clone()), true),
-            Some(e) if e == d => (Some(name.clone()), false),
+            Some(e) if same(&e, &d) => (Some(name.clone()), false),
             Some(_) => {
                 (Some(unique_name(name, |c| taken(c) || used.iter().any(|u| u == c) || out.values().any(|(n, _)| n.as_deref() == Some(c)))), true)
             }
@@ -140,6 +141,65 @@ fn names<T: PartialEq>(
     out
 }
 
+/// `n` as `dst` would copy it in ([`Document::reid`]), with the ids a document of its own gives it:
+/// art copied in once, which differs from the file's only in its ids, compares equal to the file's.
+fn canon(dst: &Document, n: &Node) -> Node {
+    let mut own = Document::new(0.0, 0.0);
+    // Pixels the art carries take a key `dst` doesn't use for other pixels, as they do in `dst`.
+    let mut keys = vec![];
+    vectorcraft_doc::swatches::walk_with_masks(n, &mut |c| {
+        if let NodeKind::Image(im) = &c.kind
+            && im.pixels.is_some()
+        {
+            keys.push(im.key.clone());
+        }
+    });
+    for (k, blob) in &dst.images {
+        if keys.iter().any(|key| k == key || k.strip_prefix(key.as_str()).is_some_and(|rest| rest.starts_with('-'))) {
+            own.images.insert(k.clone(), blob.clone());
+        }
+    }
+    own.reid(n)
+}
+
+/// Do symbols `a` and `b` draw the same, whatever ids their art has ([`canon`])?
+fn same_symbol(dst: &Document, a: &Symbol, b: &Symbol) -> bool {
+    let own = |s: &Symbol| Symbol { art: Arc::new(canon(dst, &s.art)), ..s.clone() };
+    own(a) == own(b)
+}
+
+/// Do patterns `a` and `b` draw the same, whatever ids their art has ([`canon`])?
+fn same_pattern(dst: &Document, a: &PatternDef, b: &PatternDef) -> bool {
+    let bare = |p: &PatternDef| PatternDef { art: vec![], ..p.clone() };
+    bare(a) == bare(b) && a.art.len() == b.art.len() && a.art.iter().zip(&b.art).all(|(x, y)| canon(dst, x) == canon(dst, y))
+}
+
+/// Does art `a` use a resource that `m` (images, symbols, patterns, brushes, swatches) gives
+/// another name, or a swatch whose link `m` drops?
+fn uses_moved(a: &Node, m: [&Renames; 5]) -> bool {
+    let moved = |map: &Renames, k: &str| map.get(k).is_some_and(|(new, _)| new.as_deref() != Some(k));
+    let (mut a, mut any) = (a.clone(), false);
+    refs(&mut a, &mut |r| {
+        any |= match r {
+            Ref::Image(k) => moved(m[0], k),
+            Ref::Symbol(k) => moved(m[1], k),
+            Ref::Pattern(k) => moved(m[2], k),
+            Ref::Brush(k) => moved(m[3], k),
+            Ref::Swatch(link) => link.as_deref().is_some_and(|k| moved(m[4], k)),
+        }
+    });
+    any
+}
+
+/// Copy each of `shared` in `map` under a free name, as [`names`] does for a definition `dst`
+/// holds differently.
+fn unshare(map: &mut Renames, shared: &[String], used: &[String], taken: impl Fn(&str) -> bool) {
+    for name in shared {
+        let free = unique_name(name, |c| taken(c) || used.iter().any(|u| u == c) || map.values().any(|(n, _)| n.as_deref() == Some(c)));
+        map.insert(name.clone(), (Some(free), true));
+    }
+}
+
 /// The renamed resources whose definitions are copied: (name in `src`, name in `dst`).
 fn copies(m: &Renames) -> Vec<(&str, &str)> {
     m.iter().filter(|(_, (_, copy))| *copy).filter_map(|(old, (new, _))| Some((old.as_str(), new.as_deref()?))).collect()
@@ -194,14 +254,17 @@ pub(crate) fn adopt(dst: &mut Document, src: &Document, nodes: &mut [Node]) {
         }
     }
 
-    let images = names(&used.images, |k| src.images.get(k), |k| dst.images.get(k), |c| dst.images.contains_key(c));
-    let symbols = names(
+    let images = names(&used.images, |k| src.images.get(k), |k| dst.images.get(k), |a, b| a == b, |c| dst.images.contains_key(c));
+    // Symbol and pattern art copied in takes ids of `dst`, so their art is compared by what it
+    // draws: placing the same file again shares what the first placement brought.
+    let mut symbols = names(
         &used.symbols,
         |n| src.symbols.iter().find(|s| s.name == n),
         |n| dst.symbols.iter().find(|s| s.name == n),
+        |a, b| same_symbol(dst, a, b),
         |c| dst.symbols.iter().any(|s| s.name == c),
     );
-    let patterns = names(&used.patterns, |n| src.pattern(n), |n| dst.pattern(n), |c| dst.pattern(c).is_some());
+    let mut patterns = names(&used.patterns, |n| src.pattern(n), |n| dst.pattern(n), |a, b| same_pattern(dst, a, b), |c| dst.pattern(c).is_some());
     // A swatch the document defines differently isn't renamed (that would add look-alike swatches):
     // the colours linked to it keep their look, unlinked.
     let swatches: Renames = used
@@ -236,12 +299,33 @@ pub(crate) fn adopt(dst: &mut Document, src: &Document, nodes: &mut [Node]) {
         any
     };
     let mut dst_brushes = vectorcraft_brush::library(dst);
-    let brushes = names(
+    let mut brushes = names(
         &used.brushes,
         |n| src_brushes.iter().find(|b| b.name == n).map(|b| (b, moves_art(b))),
         |n| dst_brushes.iter().find(|b| b.name == n).map(|b| (b, false)),
+        |a, b| a == b,
         |c| dst_brushes.iter().any(|b| b.name == c),
     );
+    // Likewise a symbol, pattern or brush whose art uses a resource that moves isn't the
+    // document's of that name: it is copied under a free name, which can move what uses it in turn.
+    loop {
+        let maps = [&images, &symbols, &patterns, &brushes, &swatches];
+        let shared = |map: &Renames| map.iter().filter(|(_, (_, copy))| !*copy).map(|(k, _)| k.clone()).collect::<Vec<_>>();
+        let sym: Vec<String> =
+            shared(&symbols).into_iter().filter(|n| src.symbols.iter().any(|s| s.name == *n && uses_moved(&s.art, maps))).collect();
+        let pat: Vec<String> =
+            shared(&patterns).into_iter().filter(|n| src.pattern(n).is_some_and(|p| p.art.iter().any(|a| uses_moved(a, maps)))).collect();
+        let bru: Vec<String> = shared(&brushes)
+            .into_iter()
+            .filter(|n| src_brushes.iter().find(|b| b.name == *n).is_some_and(|b| brush_art(&mut b.clone()).into_iter().any(|a| uses_moved(a, maps))))
+            .collect();
+        if sym.is_empty() && pat.is_empty() && bru.is_empty() {
+            break;
+        }
+        unshare(&mut symbols, &sym, &used.symbols, |c| dst.symbols.iter().any(|s| s.name == c));
+        unshare(&mut patterns, &pat, &used.patterns, |c| dst.pattern(c).is_some());
+        unshare(&mut brushes, &bru, &used.brushes, |c| dst_brushes.iter().any(|b| b.name == c));
+    }
 
     let mut rename = |r: Ref| {
         let (map, k) = match r {

crates/engine/src/tests/place.rs

@@ -689,6 +689,74 @@ fn placed_art_brings_its_symbols_and_patterns_with_ids_of_the_documents_own() {
     assert_eq!(unique.len(), ids.len(), "{ids:?}");
 }
 
+/// Placing the same file again shares the symbols and patterns the first placement brought: their
+/// art differs from the file's only in its ids, so no `Star 2` or `Dots 2` joins the document.
+#[test]
+fn placing_the_same_art_again_shares_its_symbols_and_patterns() {
+    let mut src = session();
+    src.execute("shape.ellipse", &json!({"x": 50, "y": 10, "width": 20, "height": 20})).unwrap();
+    src.execute("symbol.new", &json!({"name": "Star"})).unwrap();
+    src.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 8, "height": 8})).unwrap();
+    let tile = src.doc().unwrap().selection.objects[0];
+    src.execute("object.pattern.make", &json!({"ids": [tile.0], "edit": false, "name": "Dots"})).unwrap();
+    src.execute("paint.setFill", &json!({"ids": [tile.0], "swatch": "Dots"})).unwrap();
+    let bytes = serialize(&mut src, "vectorcraft");
+    let mut s = session();
+    let counts = |s: &Session| {
+        let doc = &s.doc().unwrap().doc;
+        (doc.symbols.len(), doc.patterns.len())
+    };
+    place(&mut s, "logo.vectorcraft", &bytes, json!({}));
+    let once = counts(&s);
+    for _ in 0..2 {
+        place(&mut s, "logo.vectorcraft", &bytes, json!({}));
+    }
+    assert_eq!(counts(&s), once);
+    let doc = &s.doc().unwrap().doc;
+    assert!(!doc.symbols.iter().any(|x| x.name == "Star 2") && doc.pattern("Dots 2").is_none());
+}
+
+/// A symbol whose art matches the document's own but uses a pattern the document defines
+/// differently is not shared: it comes in under a free name, using the pattern it brings.
+#[test]
+fn placed_art_does_not_share_a_symbol_whose_pattern_moves() {
+    let mut src = session();
+    src.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 8, "height": 8})).unwrap();
+    let tile = src.doc().unwrap().selection.objects[0];
+    src.execute("object.pattern.make", &json!({"ids": [tile.0], "edit": false, "name": "Dots"})).unwrap();
+    src.execute("paint.setFill", &json!({"ids": [tile.0], "swatch": "Dots"})).unwrap();
+    src.execute("symbol.new", &json!({"name": "Star", "ids": [tile.0]})).unwrap();
+    let bytes = serialize(&mut src, "vectorcraft");
+    let mut s = session();
+    place(&mut s, "logo.vectorcraft", &bytes, json!({}));
+    // The document's "Dots" changes; its "Star" still matches the file's.
+    let st = s.doc_mut().unwrap();
+    let d = Arc::make_mut(&mut st.doc);
+    let dots = d.pattern_mut("Dots").unwrap();
+    dots.tile = Rect::new(0.0, 0.0, 50.0, 50.0);
+    let r = place(&mut s, "logo.vectorcraft", &bytes, json!({}));
+    let mut symbol = None;
+    node(&s, &r["ids"][0]).walk(&mut |n| {
+        if let NodeKind::SymbolInstance { symbol: name, .. } = &n.kind {
+            symbol = Some(name.clone());
+        }
+    });
+    assert_eq!(symbol.as_deref(), Some("Star 2"));
+    let doc = &s.doc().unwrap().doc;
+    let star2 = doc.symbols.iter().find(|x| x.name == "Star 2").unwrap();
+    let mut fills = vec![];
+    star2.art.walk(&mut |n| {
+        fills.extend(n.appearance.items.iter().filter_map(|i| match i {
+            AppearanceItem::Fill(f) => match &f.paint {
+                Paint::Pattern { pattern, .. } => Some(pattern.clone()),
+                _ => None,
+            },
+            _ => None,
+        }))
+    });
+    assert_eq!(fills, ["Dots 2"]);
+}
+
 #[test]
 fn placed_art_brings_its_brushes_renaming_on_conflict() {
     // The file: a path with its own brush, and one with a brush the defaults hold.