diff --git a/crates/engine/src/cmd/place/adopt.rs b/crates/engine/src/cmd/place/adopt.rs index 0a0f08d3c1..9cd28b7e25 100644 --- a/crates/engine/src/cmd/place/adopt.rs +++ b/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, 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( +/// (copied) or one that is the `same` (shared), else a free variant (`taken`, copied). +fn names( used: &[String], def: impl Fn(&str) -> Option, existing: impl Fn(&str) -> Option, + same: impl Fn(&T, &T) -> bool, taken: impl Fn(&str) -> bool, ) -> Renames { let mut out = Renames::new(); @@ -130,7 +131,7 @@ fn names( 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( 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::>(); + let sym: Vec = + shared(&symbols).into_iter().filter(|n| src.symbols.iter().any(|s| s.name == *n && uses_moved(&s.art, maps))).collect(); + let pat: Vec = + 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 = 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 { diff --git a/crates/engine/src/tests/place.rs b/crates/engine/src/tests/place.rs index 455471dbb1..ab80255376 100644 --- a/crates/engine/src/tests/place.rs +++ b/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.