From 87dea538aee76b51d0a5e9500897444bcf7d562f Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 31 Oct 2026 22:45:54 -0700 Subject: [PATCH] Symbols: instances inside other symbols' art follow an edit of their dynamic symbol Editing a dynamic symbol moved each placed instance's appearance changes with the objects they name, but an instance stored in the art of another symbol kept its changes at the old paths, so placing that symbol showed a change on the wrong object. Those instances, opacity masks included, now follow the edit too and keep only the changes to objects the art still has. --- crates/engine/src/cmd/symboledit.rs | 92 ++++++++++++++++++++------- crates/engine/src/tests/symboledit.rs | 44 +++++++++++++ 2 files changed, 114 insertions(+), 22 deletions(-) diff --git a/crates/engine/src/cmd/symboledit.rs b/crates/engine/src/cmd/symboledit.rs index 0afdd7520..ead753827 100644 --- a/crates/engine/src/cmd/symboledit.rs +++ b/crates/engine/src/cmd/symboledit.rs @@ -15,6 +15,7 @@ use std::collections::HashSet; use std::sync::Arc; use serde_json::{Value, json}; +use vectorcraft_doc::symbol_overrides::SymbolOverride; use vectorcraft_doc::{Document, Node, NodeId, NodeKind, Selection, SymbolEdit, SymbolRegistration}; use vectorcraft_geom::{Affine, Point}; @@ -363,34 +364,81 @@ fn path_of(nodes: &[Arc], id: NodeId, depth: usize) -> Option> { None } -/// Each instance's appearance changes to objects of symbol `name`'s art follow those objects -/// through an edit of the editing layer: `old` and `new` are the layer's objects before and after -/// with whether the symbol's art is a group of them (else it is the one object). A change to an -/// object the edit removed names no object afterwards. -fn follow_objects(d: &mut Document, name: &str, old: (&[Arc], bool), new: (&[Arc], bool)) { +/// Appearance changes to objects of a symbol's art follow those objects through an edit of the +/// editing layer: `old` and `new` are the layer's objects before and after with whether the +/// symbol's art is a group of them (else it is the one object). A change to an object the edit +/// removed is dropped. +fn follow_paths(overrides: &mut Vec, old: (&[Arc], bool), new: (&[Arc], bool)) { let depth = vectorcraft_doc::symbol_overrides::MAX_PATH_DEPTH + 1; + overrides.retain_mut(|o| { + // The art's own group stays the art's root. + if old.1 && o.path.is_empty() { + return true; + } + let on_layer: Vec = if old.1 { o.path.clone() } else { std::iter::once(0).chain(o.path.iter().copied()).collect() }; + let moved = at_path(old.0, &on_layer).and_then(|obj| path_of(new.0, obj.id, depth)).and_then(|p| match new.1 { + true => Some(p), + false => p.split_first().filter(|(first, _)| **first == 0).map(|(_, rest)| rest.to_vec()), + }); + match moved { + Some(p) => { + o.path = p; + true + } + None => false, + } + }); +} + +/// Whether `n` or its subtree (opacity masks included) holds an instance of symbol `name` with +/// appearance changes. +fn holds_changed_instance(n: &Node, name: &str) -> bool { + let mut found = false; + vectorcraft_doc::swatches::walk_with_masks(n, &mut |m| { + found |= matches!(&m.kind, NodeKind::SymbolInstance { symbol, .. } if symbol == name) && !m.extras().symbol_overrides.is_empty(); + }); + found +} + +/// Apply `f` to the appearance changes of each instance of symbol `name` in `n` and its subtree +/// (opacity masks included), copying only the shared nodes on the way to one. +fn each_changed_instance(n: &mut Node, name: &str, f: &mut dyn FnMut(&mut Vec)) { + if matches!(&n.kind, NodeKind::SymbolInstance { symbol, .. } if symbol == name) && !n.extras().symbol_overrides.is_empty() { + f(&mut n.extras_mut().symbol_overrides); + } + if let Some(mask) = n.extras_if_any().and_then(|e| e.mask.as_deref_mut()) + && holds_changed_instance(&mask.art, name) + { + each_changed_instance(Arc::make_mut(&mut mask.art), name, f); + } + for c in n.children_mut().into_iter().flatten() { + if holds_changed_instance(c, name) { + each_changed_instance(Arc::make_mut(c), name, f); + } + } +} + +/// Each instance's appearance changes to objects of symbol `name`'s art follow those objects +/// through an edit of the editing layer (see [`follow_paths`]). The instances in the art of the +/// document's other symbols follow too, and keep only the changes to objects the art still has. +fn follow_objects(d: &mut Document, name: &str, old: (&[Arc], bool), new: (&[Arc], bool)) { for id in super::brushsym::instances_of(d, Some(name)) { let Some(n) = d.node_mut(id) else { continue }; if n.extras().symbol_overrides.is_empty() { continue; } - let mut e = n.extras_mut(); - e.symbol_overrides.retain_mut(|o| { - // The art's own group stays the art's root. - if old.1 && o.path.is_empty() { - return true; - } - let on_layer: Vec = if old.1 { o.path.clone() } else { std::iter::once(0).chain(o.path.iter().copied()).collect() }; - let moved = at_path(old.0, &on_layer).and_then(|obj| path_of(new.0, obj.id, depth)).and_then(|p| match new.1 { - true => Some(p), - false => p.split_first().filter(|(first, _)| **first == 0).map(|(_, rest)| rest.to_vec()), - }); - match moved { - Some(p) => { - o.path = p; - true - } - None => false, + follow_paths(&mut n.extras_mut().symbol_overrides, old, new); + } + let art = d.symbols.iter().find(|s| s.name == name).filter(|s| s.dynamic).map(|s| s.art.clone()); + for sym in d.symbols.iter_mut().filter(|s| s.name != name) { + if !holds_changed_instance(&sym.art, name) { + continue; + } + each_changed_instance(Arc::make_mut(&mut sym.art), name, &mut |o| { + follow_paths(o, old, new); + match &art { + Some(art) => vectorcraft_doc::symbol_overrides::retain_parts(o, art), + None => o.clear(), } }); } diff --git a/crates/engine/src/tests/symboledit.rs b/crates/engine/src/tests/symboledit.rs index 4087fb48b..0027ce8a1 100644 --- a/crates/engine/src/tests/symboledit.rs +++ b/crates/engine/src/tests/symboledit.rs @@ -474,6 +474,50 @@ fn editing_a_dynamic_symbol_keeps_the_instances_changes_to_the_objects_still_the assert!(s.doc().unwrap().doc.node(inst).is_some_and(|n| matches!(n.kind, NodeKind::SymbolInstance { .. }))); } +/// An instance of the edited dynamic symbol inside another symbol's art keeps its appearance +/// changes on the same objects when the edit reorders them, and drops the change to an object the +/// edit deleted. +#[test] +fn an_instance_inside_another_symbols_art_follows_an_edit_of_its_dynamic_symbol() { + let mut s = Session::new(); + run(&mut s, "file.new", json!({"width": 400, "height": 300})); + let a = id(&run(&mut s, "shape.rectangle", json!({"x": 10, "y": 10, "width": 40, "height": 40}))); + let b = id(&run(&mut s, "shape.rectangle", json!({"x": 60, "y": 10, "width": 40, "height": 40}))); + let inst = id(&run(&mut s, "symbol.new", json!({"name": "Pair", "ids": [a.0, b.0], "type": "dynamic"}))); + run(&mut s, "symbol.instance.setAppearance", json!({"id": inst.0, "path": [0], "fill": "#00ff00"})); + run(&mut s, "symbol.instance.setAppearance", json!({"id": inst.0, "path": [1], "fill": "#ff0000"})); + run(&mut s, "symbol.new", json!({"name": "Outer", "ids": [inst.0]})); + // The changes of the instance of "Pair" in the art of "Outer", by path and color. + let nested = |s: &Session| -> Vec<(Vec, String)> { + let d = &s.doc().unwrap().doc; + let outer = d.symbols.iter().find(|x| x.name == "Outer").expect("Outer"); + let mut out = vec![]; + outer.art.walk(&mut |n| { + if matches!(&n.kind, NodeKind::SymbolInstance { symbol, .. } if symbol == "Pair") { + out.extend(n.extras().symbol_overrides.iter().map(|o| (o.path.clone(), o.fill.as_ref().and_then(|p| p.color()).unwrap().to_hex()))); + } + }); + out + }; + let start = vec![(vec![0], "#00ff00".to_string()), (vec![1], "#ff0000".to_string())]; + assert_eq!(nested(&s), start); + let other = id(&run(&mut s, "symbol.place", json!({"name": "Pair", "x": 200, "y": 200}))); + run(&mut s, "symbol.edit", json!({"id": other.0})); + let art = layer_art(&s); + assert_eq!(art.len(), 2); + // Sending the second object to the back: each change follows its object. + run(&mut s, "select.set", json!({"ids": [art[1].0]})); + run(&mut s, "object.arrange.sendToBack", json!({})); + assert_eq!(layer_art(&s), vec![art[1], art[0]]); + assert_eq!(nested(&s), vec![(vec![1], "#00ff00".to_string()), (vec![0], "#ff0000".to_string())]); + // Deleting the first object: the change to it goes. + run(&mut s, "select.set", json!({"ids": [art[0].0]})); + run(&mut s, "edit.clear", json!({})); + assert_eq!(nested(&s), vec![(vec![0], "#ff0000".to_string())]); + run(&mut s, "symbol.exitEditing", json!({})); + assert_eq!(nested(&s), vec![(vec![0], "#ff0000".to_string())]); +} + #[test] fn undo_back_into_an_earlier_edit_leaves_it_to_its_own_isolation() { let (mut s, first, second) = two_instances();