From 7c1d73d8180b294f4ed289ec1edaa89343373cd2 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 31 Oct 2026 21:12:51 -0700 Subject: [PATCH 1/8] Symbols: test the Symbols panel, panel menu and Control bar entries to symbol editing mode Headless tests cover a double-click on a symbol in Thumbnail View and List View, the panel menu's Edit Symbol with and without a selected instance, the Control bar's Edit Symbol button, and the Control bar's Exit Symbol Editing Mode and Exit Isolation Mode buttons inside the symbol's art. The panel's module comment now says a double-click opens the symbol in symbol editing mode, and docs/mcp.md lists symbol.edit's ids parameter. --- crates/ui-egui/src/chrome/tests.rs | 57 +++++++++++++++ crates/ui-egui/src/panels/symbols.rs | 102 ++++++++++++++++++++++++++- docs/mcp.md | 2 +- 3 files changed, 159 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/chrome/tests.rs b/crates/ui-egui/src/chrome/tests.rs index 842ee1bc1..774de7540 100644 --- a/crates/ui-egui/src/chrome/tests.rs +++ b/crates/ui-egui/src/chrome/tests.rs @@ -1074,6 +1074,63 @@ fn the_control_bar_isolates_the_selected_object_and_exits_isolation_mode() { assert_eq!(isolation_button(&mut app, &ctx, vec![]), None, "the isolation bar ends pattern editing"); } +/// In symbol editing mode the Control bar's isolation button is Exit Symbol Editing Mode and +/// leaves the mode. With a group isolated inside the symbol's art it is Exit Isolation Mode and +/// goes back to the symbol's art. +#[test] +fn the_control_bar_exits_symbol_editing_mode() { + use vectorcraft_doc::NodeId; + let mut app = crate::VectorcraftApp::new(Session::new(), Default::default()); + app.run("file.new", json!({"width": 400, "height": 200})).unwrap(); + app.run("shape.rectangle", json!({"x": 10, "y": 10, "width": 30, "height": 20})).unwrap(); + app.run("symbol.new", json!({"name": "Box"})).unwrap(); + app.run("symbol.edit", json!({"name": "Box"})).unwrap(); + let layer = app.session.active().unwrap().doc.symbol_edit.as_ref().expect("symbol editing mode").layer; + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx); + ctx.enable_accesskit(); + // The isolation button's label (the one exit label the bar shows), and a click on it. + let label = |app: &mut crate::VectorcraftApp| { + let screen = egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1600.0, 300.0)); + let mut out = ctx.run_ui(egui::RawInput { screen_rect: Some(screen), ..Default::default() }, |ui| super::control_bar(app, ui)); + out.textures_delta.clear(); + let nodes = &out.platform_output.accesskit_update.as_ref().expect("an accessibility tree").nodes; + let exits: Vec = nodes + .iter() + .filter_map(|(_, n)| n.label().map(|l| l.trim().to_string())) + .filter(|l| l == "Exit Symbol Editing Mode" || l == "Exit Isolation Mode") + .collect(); + assert!(exits.len() <= 1, "{exits:?}"); + exits.into_iter().next() + }; + let click = |app: &mut crate::VectorcraftApp| { + let r = isolation_button(app, &ctx, vec![]).expect("the bar shows the isolation button"); + let press = + |pressed| egui::Event::PointerButton { pos: r.center(), button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }; + for e in [egui::Event::PointerMoved(r.center()), press(true), press(false)] { + isolation_button(app, &ctx, vec![e]); + } + }; + assert_eq!(label(&mut app).as_deref(), Some("Exit Symbol Editing Mode")); + // Two rectangles drawn into the symbol's art, grouped and isolated. + let a = app.run("shape.rectangle", json!({"x": 100, "y": 100, "width": 20, "height": 20})).unwrap()["id"].as_u64().unwrap(); + let b = app.run("shape.rectangle", json!({"x": 130, "y": 100, "width": 20, "height": 20})).unwrap()["id"].as_u64().unwrap(); + app.run("select.set", json!({"ids": [a, b]})).unwrap(); + let g = app.run("object.group", json!({})).unwrap()["id"].as_u64().unwrap(); + app.run("object.isolate", json!({"id": g})).unwrap(); + assert_eq!(label(&mut app).as_deref(), Some("Exit Isolation Mode")); + click(&mut app); + let st = app.session.active().unwrap(); + assert_eq!((st.isolation, st.doc.symbol_edit.is_some()), (Some(layer), true), "back to the symbol's art"); + assert_eq!(label(&mut app).as_deref(), Some("Exit Symbol Editing Mode")); + click(&mut app); + let st = app.session.active().unwrap(); + assert_eq!((st.isolation, st.doc.symbol_edit.is_some()), (None::, false), "symbol editing mode ends"); + // The symbol keeps the art drawn while editing. + let symbol = st.doc.symbols.iter().find(|s| s.name == "Box").expect("the symbol"); + assert!(symbol.art.children().is_some_and(|c| c.len() >= 2), "the edit reached the symbol"); +} + /// #4819: with Direct Selection, a selected blend shows the Control bar's and the Properties /// panel's Convert buttons, as a path selected whole does; with the Selection tool it doesn't. #[test] diff --git a/crates/ui-egui/src/panels/symbols.rs b/crates/ui-egui/src/panels/symbols.rs index 291257e97..1b2ff3559 100644 --- a/crates/ui-egui/src/panels/symbols.rs +++ b/crates/ui-egui/src/panels/symbols.rs @@ -1,5 +1,5 @@ //! Symbols panel: the document's symbols as rendered thumbnails or a list. Click selects a symbol -//! (it becomes the Symbol Sprayer's current symbol); double-click places an instance. Symbol +//! (it becomes the Symbol Sprayer's current symbol); double-click opens it in symbol editing mode. Symbol //! libraries open in library panels ([`SymbolLibraries`]). use std::cell::RefCell; @@ -979,6 +979,106 @@ mod tests { assert_eq!(editing_art(&app), Some([585.0, 35.0, 615.0, 65.0])); } + /// The symbol in symbol editing mode, if any. + fn editing(app: &VectorcraftApp) -> Option { + app.session.active()?.doc.symbol_edit.as_ref().map(|e| e.symbol.clone()) + } + + /// The first instance of symbol `name` in the active document. + fn instance_of(app: &VectorcraftApp, name: &str) -> Option { + let mut found = None; + for l in &app.session.active()?.doc.layers { + l.walk(&mut |n| { + if let NodeKind::SymbolInstance { symbol, .. } = &n.kind + && symbol == name + && found.is_none() + { + found = Some(n.id); + } + }); + } + found + } + + fn double_click(at: Pos2) -> Vec { + vec![egui::Event::PointerMoved(at), button(at, true), button(at, false), button(at, true), button(at, false)] + } + + /// A double-click on a symbol in the panel, in Thumbnail View and in List View, opens it in + /// symbol editing mode and places no instance; a single click only makes it current. + #[test] + fn double_clicking_a_symbol_in_the_panel_edits_it_in_either_view() { + let mut app = leaf_and_dot(); + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx); + // Thumbnail View: the first tile is Leaf's. + let tile = pos2(26.0, 26.0); + frame(&ctx, &mut app, vec![]); + frame(&ctx, &mut app, vec![egui::Event::PointerMoved(tile), button(tile, true), button(tile, false)]); + assert_eq!((editing(&app), current(&mut app)), (None, Some("Leaf".into())), "a click makes it current"); + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx); + frame(&ctx, &mut app, vec![]); + frame(&ctx, &mut app, double_click(tile)); + assert_eq!(editing(&app).as_deref(), Some("Leaf")); + assert_eq!(art(&app, "Leaf").0, 1, "no instance is placed"); + app.run("symbol.exitEditing", json!({})).unwrap(); + // List View: the row named Dot. + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx); + set_pstate(&ctx, "sym-list", true); + let raw = egui::RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(236.0, 400.0))), ..Default::default() }; + let mut out = ctx.run_ui(raw, |ui| show(&mut app, ui)); + out.textures_delta.clear(); + let labels = crate::tests_removeanchors::shapes_text(&out.shapes.iter().map(|c| c.shape.clone()).collect::>()); + let row = labels.iter().find(|(t, _)| t.trim() == "Dot").map(|(_, r)| r.center()).expect("a row named Dot"); + frame(&ctx, &mut app, double_click(row)); + assert_eq!(editing(&app).as_deref(), Some("Dot")); + assert_eq!(art(&app, "Dot").0, 0, "no instance is placed"); + } + + /// The panel menu's Edit Symbol edits the selected instance's symbol, and with no instance + /// selected the symbol current in the panel. + #[test] + fn the_panel_menus_edit_symbol_edits_the_instances_symbol_or_the_panels() { + let mut app = leaf_and_dot(); + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx); + click_menu(&ctx, &mut app, "Edit Symbol"); + assert_eq!(editing(&app).as_deref(), Some("Dot"), "no instance selected: the panel's symbol"); + app.run("symbol.exitEditing", json!({})).unwrap(); + let leaf = instance_of(&app, "Leaf").expect("a Leaf instance"); + app.run("select.set", json!({"ids": [leaf.0]})).unwrap(); + click_menu(&ctx, &mut app, "Edit Symbol"); + assert_eq!(editing(&app).as_deref(), Some("Leaf"), "the selected instance's symbol"); + assert_eq!(instance_of(&app, "Leaf"), Some(leaf), "the instance keeps its link"); + } + + /// With instances of one symbol selected, the Control bar's Edit Symbol opens that symbol in + /// symbol editing mode and keeps the instance linked. + #[test] + fn the_control_bars_edit_symbol_edits_the_selected_instances_symbol() { + let mut app = leaf_and_dot(); + let leaf = instance_of(&app, "Leaf").expect("a Leaf instance"); + app.run("select.set", json!({"ids": [leaf.0]})).unwrap(); + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx); + ctx.enable_accesskit(); + let bar = |app: &mut VectorcraftApp, events: Vec| { + let raw = egui::RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(800.0, 400.0))), events, ..Default::default() }; + let mut out = ctx.run_ui(raw, |ui| { + ui.horizontal(|ui| control_bar(app, ui)); + }); + out.textures_delta.clear(); + out + }; + let out = bar(&mut app, vec![]); + let at = clickable(&out).into_iter().find(|(l, _)| l == "Edit Symbol").expect("an Edit Symbol button").1; + bar(&mut app, vec![egui::Event::PointerMoved(at), button(at, true), button(at, false)]); + assert_eq!(editing(&app).as_deref(), Some("Leaf")); + assert_eq!(instance_of(&app, "Leaf"), Some(leaf), "the instance keeps its link"); + } + fn dialog_kind(app: &VectorcraftApp) -> Option<&str> { app.ui.dialog.as_ref().map(|d| d.kind.as_str()) } diff --git a/docs/mcp.md b/docs/mcp.md index 760ce7071..1846d5bc2 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1572,7 +1572,7 @@ already walks Image Trace. Short agent entry points: of `symbol.list` carry `nineSlice`: the guides or `null`; `type` is Symbol Type, `static` by default for the command and `dynamic` by default in the Symbol Options dialog), `symbol.place {name?, x?, y?}`, `symbol.breakLink {ids?}`, `symbol.replace`, - `symbol.edit {id?, name?, x?, y?, artboard?, viewCenter?}` (symbol editing mode: the symbol's art on a temporary + `symbol.edit {id?, ids?, name?, x?, y?, artboard?, viewCenter?}` (symbol editing mode: the symbol's art on a temporary isolated layer, where the instance `id` shows it or centered on (x, y), by default on the center of artboard `artboard`, the active one the app passes, or in a document without artboards on `viewCenter`, the view's center the app passes; every instance follows each edit and stays linked), `symbol.exitEditing` From 0af80963649c4e164c8a09a123f4456387037887 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 31 Oct 2026 22:12:19 -0700 Subject: [PATCH 2/8] Symbols: a refused switch to another symbol keeps the current edit, and isolation stays on the editing layer symbol.edit checks the new placement before it ends the symbol being edited, so a call with an out-of-range x or y fails and leaves the current edit open. While a symbol is edited, object.isolate refuses art outside the editing layer, and object.exitIsolation goes back to the editing layer only from a group inside it; from any other isolation it leaves symbol editing mode. --- crates/engine/src/cmd/object/arrange.rs | 8 +++++- crates/engine/src/cmd/symboledit.rs | 11 +++++++- crates/engine/src/tests/symboledit.rs | 35 +++++++++++++++++++++++++ docs/mcp.md | 7 ++--- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/object/arrange.rs b/crates/engine/src/cmd/object/arrange.rs index 69d2baa9c..2e7d2a8e4 100644 --- a/crates/engine/src/cmd/object/arrange.rs +++ b/crates/engine/src/cmd/object/arrange.rs @@ -315,6 +315,12 @@ pub(super) fn isolate(s: &mut Session, p: &Value) -> Result { if guides_locked && crate::is_guide_path(&st.doc, isolation) { return Err(EngineError::Other("guides are locked".into())); } + // While a symbol is edited, only its art on the editing layer is isolated. + if let Some(layer) = st.doc.symbol_edit.as_ref().map(|e| e.layer) + && !st.doc.ancestry(isolation).is_some_and(|a| a.contains(&layer)) + { + return Err(bad("object.isolate", "the object is outside the isolated group")); + } (isolation, st.isolation) }; if let Some(prev) = prev @@ -339,7 +345,7 @@ pub(super) fn exit_isolation(s: &mut Session, p: &Value) -> Result { // Done do. A group isolated inside the symbol's art goes back to the symbol's layer. let st = s.doc()?; if let Some(layer) = st.doc.symbol_edit.as_ref().map(|e| e.layer) - && let Some(inner) = st.isolation.filter(|i| *i != layer) + && let Some(inner) = st.isolation.filter(|i| *i != layer && st.doc.ancestry(*i).is_some_and(|a| a.contains(&layer))) { crate::cmd::settle_interaction(s)?; let st = s.doc_mut()?; diff --git a/crates/engine/src/cmd/symboledit.rs b/crates/engine/src/cmd/symboledit.rs index a89fbe8d6..0afdd7520 100644 --- a/crates/engine/src/cmd/symboledit.rs +++ b/crates/engine/src/cmd/symboledit.rs @@ -112,11 +112,20 @@ pub(crate) fn enter(s: &mut Session, p: &Value) -> Result { } }; // Editing another symbol (one from the Symbols panel, or through the API): the current edit - // ends first. + // ends first, once the new placement is known to be valid, so a refused switch keeps it open. if st.doc.symbol_edit.is_some() { if instance.is_some_and(|i| st.doc.symbol_edit.as_ref().is_some_and(|e| st.doc.ancestry(i).unwrap_or_default().contains(&e.layer))) { return Err(bad(C, "exit isolation mode first")); } + let mut trial = Document::clone(&st.doc); + let mut sel = Selection::default(); + begin(&mut trial, &mut sel, &name, xf, instance)?; + if trial.too_deep(Some(&st.doc)) { + return Err(EngineError::Other(crate::TOO_DEEP.into())); + } + if !crate::sanity::doc_sane(&trial, &sel) { + return Err(EngineError::Other("result would exceed the canvas (coordinates out of range)".into())); + } leave(s, &json!({}))?; } if let Some(i) = s.doc()?.isolation { diff --git a/crates/engine/src/tests/symboledit.rs b/crates/engine/src/tests/symboledit.rs index 491d6cd83..4087fb48b 100644 --- a/crates/engine/src/tests/symboledit.rs +++ b/crates/engine/src/tests/symboledit.rs @@ -372,6 +372,41 @@ fn other_editing_modes_and_symbol_editing_do_not_overlap() { assert_eq!(s.doc().unwrap().doc.layers.iter().filter(|l| l.name.as_deref() == Some("Box")).count(), 1); } +/// Editing another symbol at a place off the canvas fails and keeps the current edit open. +#[test] +fn a_switch_to_another_symbol_that_fails_keeps_the_current_edit() { + let (mut s, first, _) = two_instances(); + run(&mut s, "shape.rectangle", json!({"x": 10, "y": 200, "width": 20, "height": 20})); + run(&mut s, "symbol.new", json!({"name": "Dot"})); + let layer = NodeId(run(&mut s, "symbol.edit", json!({"id": first.0}))["layer"].as_u64().unwrap()); + let e = s.execute("symbol.edit", &json!({"name": "Dot", "x": 1e300})).unwrap_err().to_string(); + assert!(e.contains("exceed the canvas"), "{e}"); + let st = s.doc().unwrap(); + assert_eq!(st.doc.symbol_edit.as_ref().map(|e| (e.symbol.as_str(), e.layer)), Some(("Box", layer))); + assert_eq!((st.isolation, st.active_layer), (Some(layer), Some(layer))); + assert!(st.doc.node(layer).is_some()); +} + +/// While a symbol is edited, art outside the editing layer is not isolated, so leaving isolation +/// mode leaves symbol editing mode. +#[test] +fn art_outside_the_editing_layer_is_not_isolated_while_a_symbol_is_edited() { + let (mut s, first, _) = two_instances(); + let other = id(&run(&mut s, "shape.rectangle", json!({"x": 10, "y": 200, "width": 20, "height": 20}))); + run(&mut s, "select.set", json!({"ids": [first.0]})); + let layer = NodeId(run(&mut s, "symbol.edit", json!({"id": first.0}))["layer"].as_u64().unwrap()); + let e = s.execute("object.isolate", &json!({"id": other.0})).unwrap_err().to_string(); + assert!(e.contains("outside the isolated group"), "{e}"); + assert_eq!(s.doc().unwrap().isolation, Some(layer)); + // Isolation that names art outside the layer (a stale state) still leaves the mode. + s.doc_mut().unwrap().isolation = Some(other); + run(&mut s, "object.exitIsolation", json!({})); + let st = s.doc().unwrap(); + assert!(st.doc.symbol_edit.is_none() && st.doc.node(layer).is_none()); + assert_eq!(st.isolation, None); + assert_eq!(st.selection.objects, vec![first]); +} + #[test] fn renaming_deleting_and_saving_while_a_symbol_is_edited() { let (mut s, first, _) = two_instances(); diff --git a/docs/mcp.md b/docs/mcp.md index 1846d5bc2..bc762c434 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1575,7 +1575,8 @@ already walks Image Trace. Short agent entry points: `symbol.edit {id?, ids?, name?, x?, y?, artboard?, viewCenter?}` (symbol editing mode: the symbol's art on a temporary isolated layer, where the instance `id` shows it or centered on (x, y), by default on the center of artboard `artboard`, the active one the app passes, or in a document without artboards on `viewCenter`, the view's center the - app passes; every instance follows each edit and stays linked), `symbol.exitEditing` + app passes; every instance follows each edit and stays linked; while another symbol is edited, that edit ends first, + and a call that fails leaves it open), `symbol.exitEditing` (leaves the mode; without an edit it adds no undo step), `symbol.duplicate`, `symbol.selectInstances {name}`, `symbol.delete {name | names, expandInstances?}` (`names` deletes several in one undo step). `symbol.new` and `symbol.duplicate {name, newName?}` trim the white space around the @@ -2425,8 +2426,8 @@ and `layerRows`, the rows highlighted in the panel. Preview All Layers when the others are in Outline already), `layer.lockOthers` / `layer.unlockAll` (Alt-clicking an eye or a lock runs Hide or Lock Others for the row's layer, or Show or Unlock All when the others already are), `layer.pasteRemembersLayers`, `object.isolate {id?}` (a group, a layer or a single object, by default the first selected object; not type, not a - hidden or locked object or one in a hidden or locked group or layer, and not a guide path while Lock Guides is on) / - `object.exitIsolation`. + hidden or locked object or one in a hidden or locked group or layer, not a guide path while Lock Guides is on, and + while a symbol is edited only the editing layer or art on it) / `object.exitIsolation`. - **Dialogs and panel state** (control channel and the app's MCP): `ui.layerOptions {ids?}` and `ui.newLayer {sublayer?}` open Layer Options (dialog `layerOptions`: name, color, template, locked, visible, printable, preview, dimImages, dimPercent; OK is one undo step; Cmd/Ctrl+Alt+L runs `ui.newLayer` for a new layer), `ui.layersPanelOptions` opens Panel Options (`layersPanelOptions`: From 87dea538aee76b51d0a5e9500897444bcf7d562f Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 31 Oct 2026 22:45:54 -0700 Subject: [PATCH 3/8] 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(); From 52f6adfa3b9d280bbdbf39737a86172222963c76 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 31 Oct 2026 23:29:20 -0700 Subject: [PATCH 4/8] Symbols: the instance a symbol is edited from is not drawn in the view while it is edited Entering symbol editing mode from an instance puts a copy of the symbol's art on the editing layer in the instance's place, and the instance stays in the document. The view drew the instance dimmed under the copy, so art with transparency showed darker, and its edges showed a dimmed fringe. The renderer now leaves that instance out of the screen view while the editing layer is isolated, with its masks and effects, and in Outline view inside a clip group. Exports and thumbnails draw it as before. --- crates/render/src/renderer/nodes.rs | 18 +++++++-- crates/render/src/tests/isolation.rs | 59 ++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/crates/render/src/renderer/nodes.rs b/crates/render/src/renderer/nodes.rs index 97cd1e64e..b40e7154d 100644 --- a/crates/render/src/renderer/nodes.rs +++ b/crates/render/src/renderer/nodes.rs @@ -118,11 +118,19 @@ impl Renderer { self.path_of(f.symbols, &owner) } - /// Whether `a` is left out of this frame: hidden, a skipped template or guide, or culled. + /// Symbol editing mode on screen: `id` is the instance editing was entered from, which is not + /// drawn (with its masks and effects), since the editing layer holds a copy of its art in its + /// place. + fn edited_instance(f: &Frame, id: NodeId) -> bool { + f.opts.isolated.is_some() && f.doc.symbol_edit.as_ref().is_some_and(|e| e.instance == Some(id)) + } + + /// Whether `a` is left out of this frame: hidden, a skipped template or guide, the instance a + /// symbol is edited from, or culled. fn skipped(&mut self, f: &Frame, a: &Arc) -> bool { let skipped_view_item = (f.opts.skip_templates || f.opts.hide_templates) && matches!(a.kind, NodeKind::Layer { template: true, .. }) || (f.opts.skip_templates || f.opts.hide_guides) && matches!(a.kind, NodeKind::Path { guide: true, .. }); - if !a.visible || skipped_view_item || f.opts.hidden.contains(&a.id) { + if !a.visible || skipped_view_item || f.opts.hidden.contains(&a.id) || Self::edited_instance(f, a.id) { return true; } match self.bounds_of(f.symbols, a) { @@ -551,7 +559,11 @@ impl Renderer { /// `fx_owner`, so its path, its live-effect geometry and, through [`Self::draw_kept_node`], a /// live object's evaluation come from the caches. fn draw_outline_member(&mut self, ctx: &mut RenderContext, f: &Frame, c: &Arc) { - if !c.visible || f.opts.hidden.contains(&c.id) || f.opts.hide_guides && matches!(c.kind, NodeKind::Path { guide: true, .. }) { + if !c.visible + || f.opts.hidden.contains(&c.id) + || f.opts.hide_guides && matches!(c.kind, NodeKind::Path { guide: true, .. }) + || Self::edited_instance(f, c.id) + { return; } match self.bounds_of(f.symbols, c) { diff --git a/crates/render/src/tests/isolation.rs b/crates/render/src/tests/isolation.rs index ac71592c3..d88736785 100644 --- a/crates/render/src/tests/isolation.rs +++ b/crates/render/src/tests/isolation.rs @@ -417,6 +417,65 @@ fn isolation_mode_dims_the_art_around_the_isolated_group() { assert_eq!(isolated(NodeId(u64::MAX)).pixels, plain.pixels); } +/// Symbol editing mode entered from an instance of art at 50% opacity, plain, with a drop shadow, +/// or in a clip group in Outline view: the screen view draws the copy on the editing layer and not +/// the instance, so the art shows once, as it does with the instance removed. Without the editing +/// layer isolated (an export), the instance draws. +#[test] +fn symbol_editing_does_not_draw_the_instance_it_was_entered_from() { + use vectorcraft_doc::{Effect, Symbol, SymbolEdit}; + let editing = |effects: &[Effect], clipped: bool, with_instance: bool| { + let mut d = Document::new(100.0, 100.0); + let mut art = rect(&mut d, Rect::new(20.0, 20.0, 80.0, 80.0), S); + art.opacity = 0.5; + d.symbols.push(Symbol { dynamic: false, name: "Square".into(), art: Arc::new(art.clone()) }); + let mut inst = Node::new(d.alloc_id(), NodeKind::SymbolInstance { symbol: "Square".into(), xf: Affine::IDENTITY }); + inst.appearance.effects = effects.to_vec(); + let id = inst.id; + let l = d.layers[0].id; + let members: Vec = with_instance.then_some(inst).into_iter().collect(); + if clipped { + let clip = clip_path(&mut d, Rect::new(10.0, 10.0, 90.0, 90.0)); + let children = std::iter::once(clip).chain(members).map(Arc::new).collect(); + let g = Node::new(d.alloc_id(), NodeKind::Group { children, clip: true }); + d.insert(Some(l), usize::MAX, g).unwrap(); + } else { + for n in members { + d.insert(Some(l), usize::MAX, n).unwrap(); + } + } + let layer = d.add_layer(Some("Square")); + let copy = d.reid(&art); + d.insert(Some(layer), usize::MAX, copy).unwrap(); + d.symbol_edit = Some(SymbolEdit { + symbol: "Square".into(), + layer, + instance: Some(id), + xf: Affine::IDENTITY, + group: false, + changed: false, + back_isolation: None, + back_layer: None, + }); + (d, layer) + }; + let params = serde_json::json!({"x": 4, "y": 4, "blur": 1, "opacity": 100}); + let shadow = Effect { id: "stylize.dropShadow".into(), params, visible: true }; + for (effects, clipped) in [(vec![], false), (vec![shadow], false), (vec![], true)] { + let what = format!("{} effects, clipped {clipped}", effects.len()); + let (d, layer) = editing(&effects, clipped, true); + let (alone, alone_layer) = editing(&effects, clipped, false); + let opts = RenderOptions { outline: clipped, ..Default::default() }; + let screen = |d: &Document, layer| render_with(&mut Renderer::new(), d, &RenderOptions { isolated: Some(layer), ..opts.clone() }); + let shown = screen(&d, layer); + assert_eq!(shown.pixel(50, 50), screen(&alone, alone_layer).pixel(50, 50), "{what}: the art shows once"); + assert!(shown.pixels == screen(&alone, alone_layer).pixels, "{what}: the view draws as without the instance"); + // An export draws the instance and the copy. + let export = |d: &Document| render_with(&mut Renderer::new(), d, &opts).pixels; + assert!(export(&d) != export(&alone), "{what}: an export draws the instance"); + } +} + /// Deeply nested isolated blending groups (#3581): each level is drawn offscreen only while the /// offscreen pixels alive stay within a few frames' worth; deeper levels are drawn as layers. #[test] From 5c678ed52cbd6ce1920a6fba0d93b86b900a0bbb Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 1 Nov 2026 00:14:58 -0700 Subject: [PATCH 5/8] Symbols: instances in pattern tiles, graph designs and brushes follow an edit of their dynamic symbol --- crates/engine/src/cmd/swatch.rs | 2 +- crates/engine/src/cmd/symboledit.rs | 49 +++++++++++++++----- crates/engine/src/tests/symboledit.rs | 65 ++++++++++++++++++++++++++- 3 files changed, 104 insertions(+), 12 deletions(-) diff --git a/crates/engine/src/cmd/swatch.rs b/crates/engine/src/cmd/swatch.rs index 2de2d0581..80e8e2a12 100644 --- a/crates/engine/src/cmd/swatch.rs +++ b/crates/engine/src/cmd/swatch.rs @@ -601,7 +601,7 @@ pub(super) fn map_swatch_links(d: &mut Document, f: &mut LinkFn) -> usize { /// keeps the fields of `raw` that `base` leaves out, and drops the fields that `base` holds and /// `new` leaves out. An array takes the length of `new`, each element merged with the element at /// the same index in `raw`. -fn merge_stored(raw: Value, base: &Value, new: Value) -> Value { +pub(super) fn merge_stored(raw: Value, base: &Value, new: Value) -> Value { match (raw, new) { (Value::Object(mut raw), Value::Object(new)) => { let mut out = serde_json::Map::new(); diff --git a/crates/engine/src/cmd/symboledit.rs b/crates/engine/src/cmd/symboledit.rs index ead753827..4cb58788b 100644 --- a/crates/engine/src/cmd/symboledit.rs +++ b/crates/engine/src/cmd/symboledit.rs @@ -420,7 +420,8 @@ fn each_changed_instance(n: &mut Node, name: &str, f: &mut dyn FnMut(&mut Vec], bool), new: (&[Arc], bool)) { for id in super::brushsym::instances_of(d, Some(name)) { let Some(n) = d.node_mut(id) else { continue }; @@ -430,17 +431,45 @@ fn follow_objects(d: &mut Document, name: &str, old: (&[Arc], bool), new: 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; + let mut follow = |o: &mut Vec| { + follow_paths(o, old, new); + match &art { + Some(art) => vectorcraft_doc::symbol_overrides::retain_parts(o, art), + None => o.clear(), } - 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(), + }; + let symbols = d.symbols.iter_mut().filter(|s| s.name != name).map(|s| &mut s.art); + let trees = symbols.chain(d.graph_designs.iter_mut().map(|g| &mut g.art)); + let patterns = d.patterns.iter_mut().chain(d.pattern_edit.iter_mut().filter_map(|pe| pe.original.as_mut())); + for root in trees.chain(patterns.flat_map(|p| &mut p.art)) { + if holds_changed_instance(root, name) { + each_changed_instance(Arc::make_mut(root), name, &mut follow); + } + } + // The default brushes hold no instances, so only a stored library can change. A stored entry + // whose art holds no changed instance, or that does not parse, is stored again as it was read; + // a changed one is merged into its entry, which keeps the fields the brush definition does not read. + let Some(stored) = d.unknown.get(vectorcraft_brush::DOC_KEY).and_then(Value::as_array).cloned() else { return }; + let mut changed = false; + let lib: Vec = stored + .into_iter() + .map(|raw| { + let Ok(mut b) = vectorcraft_brush::Brush::from_json(raw.clone()) else { return raw }; + if !b.art().into_iter().any(|n| holds_changed_instance(n, name)) { + return raw; } - }); + let base = serde_json::to_value(&b).unwrap_or_default(); + for art in b.art_mut() { + if holds_changed_instance(art, name) { + each_changed_instance(art, name, &mut follow); + } + } + changed = true; + super::swatch::merge_stored(raw, &base, serde_json::to_value(&b).unwrap_or_default()) + }) + .collect(); + if changed { + d.unknown.insert(vectorcraft_brush::DOC_KEY.into(), Value::Array(lib)); } } diff --git a/crates/engine/src/tests/symboledit.rs b/crates/engine/src/tests/symboledit.rs index 0027ce8a1..9a3ce144c 100644 --- a/crates/engine/src/tests/symboledit.rs +++ b/crates/engine/src/tests/symboledit.rs @@ -1,7 +1,7 @@ //! Symbol editing mode (`symbol.edit`, `symbol.exitEditing`). use serde_json::{Value, json}; -use vectorcraft_doc::NodeKind; +use vectorcraft_doc::{Node, NodeKind}; use vectorcraft_geom::Rect; use crate::*; @@ -518,6 +518,69 @@ fn an_instance_inside_another_symbols_art_follows_an_edit_of_its_dynamic_symbol( assert_eq!(nested(&s), vec![(vec![0], "#ff0000".to_string())]); } +/// Instances of the edited dynamic symbol in a pattern tile, a graph design and an art brush keep +/// their appearance changes on the same objects when the edit reorders them, and drop the change +/// to an object the edit deleted. +#[test] +fn instances_in_pattern_tiles_graph_designs_and_brushes_follow_an_edit_of_their_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, "object.pattern.make", json!({"ids": [inst.0], "name": "Tile", "edit": false})); + run(&mut s, "select.set", json!({"ids": [inst.0]})); + run(&mut s, "graph.design", json!({"new": "Pairs"})); + run(&mut s, "brush.new", json!({"type": "art", "name": "Pair Band", "ids": [inst.0]})); + // A stored entry that does not parse, and a field the brush definition does not read, are kept. + let junk = json!({"name": "Unreadable", "kind": 7}); + let doc = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc); + let Some(Value::Array(entries)) = doc.unknown.get_mut(vectorcraft_brush::DOC_KEY) else { panic!("no stored brushes") }; + entries.iter_mut().filter(|e| e["name"] == "Pair Band").for_each(|e| e["note"] = json!("kept")); + entries.push(junk.clone()); + // The changes of each instance of "Pair" in `n`, by path and color. + fn changes(n: &Node, out: &mut Vec<(Vec, String)>) { + n.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()))); + } + }); + } + // The changes in the tile, the graph design and the brush. + let held = |s: &Session| -> [Vec<(Vec, String)>; 3] { + let d = &s.doc().unwrap().doc; + let mut out: [Vec<(Vec, String)>; 3] = Default::default(); + d.patterns.iter().filter(|p| p.name == "Tile").flat_map(|p| &p.art).for_each(|n| changes(n, &mut out[0])); + d.graph_designs.iter().for_each(|g| changes(&g.art, &mut out[1])); + let brush = vectorcraft_brush::find(d, "Pair Band").expect("the brush"); + brush.art().into_iter().for_each(|n| changes(n, &mut out[2])); + out + }; + let start = vec![(vec![0], "#00ff00".to_string()), (vec![1], "#ff0000".to_string())]; + assert_eq!(held(&s), [start.clone(), start.clone(), 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!({})); + let swapped = vec![(vec![1], "#00ff00".to_string()), (vec![0], "#ff0000".to_string())]; + assert_eq!(held(&s), [swapped.clone(), swapped.clone(), swapped]); + // 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!({})); + let left = vec![(vec![0], "#ff0000".to_string())]; + assert_eq!(held(&s), [left.clone(), left.clone(), left.clone()]); + run(&mut s, "symbol.exitEditing", json!({})); + assert_eq!(held(&s), [left.clone(), left.clone(), left]); + let stored = &s.doc().unwrap().doc.unknown[vectorcraft_brush::DOC_KEY]; + assert!(stored.as_array().is_some_and(|a| a.contains(&junk)), "{stored}"); + assert!(stored.as_array().is_some_and(|a| a.iter().any(|e| e["name"] == "Pair Band" && e["note"] == "kept")), "{stored}"); +} + #[test] fn undo_back_into_an_earlier_edit_leaves_it_to_its_own_isolation() { let (mut s, first, second) = two_instances(); From 294d13d857cc28cd1f55a0a7baf847440bdfcbfe Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 1 Nov 2026 00:37:35 -0700 Subject: [PATCH 6/8] Symbols: an instance in an opacity mask follows an edit of its dynamic symbol The appearance changes of the instances on the layers now follow the edited objects through the same pass as the instances in other symbols, graph designs, pattern tiles and brushes. That pass also reaches instances used as opacity-mask art, which keep their changes on the same objects when an edit reorders them and drop the change to an object the edit deletes. --- crates/engine/src/cmd/brushsym.rs | 2 +- crates/engine/src/cmd/brushsym/symbols.rs | 9 +---- crates/engine/src/cmd/symboledit.rs | 16 +++----- crates/engine/src/tests/symboledit.rs | 45 +++++++++++++++++++++++ 4 files changed, 52 insertions(+), 20 deletions(-) diff --git a/crates/engine/src/cmd/brushsym.rs b/crates/engine/src/cmd/brushsym.rs index 7386f7718..6f7661afc 100644 --- a/crates/engine/src/cmd/brushsym.rs +++ b/crates/engine/src/cmd/brushsym.rs @@ -35,7 +35,7 @@ use brushes::{ }; use dynamic::{symbol_instance_reset, symbol_instance_set_appearance}; use symbols::selected_instances; -pub(crate) use symbols::{fit_instances_of, instance_in_place, instances_of, natural_size, normalize, place_xf, set_natural_size, store_symbol}; +pub(crate) use symbols::{instance_in_place, natural_size, normalize, place_xf, set_natural_size, store_symbol}; use symbols::{ symbol_adjust, symbol_break_link, symbol_delete, symbol_duplicate, symbol_list, symbol_new, symbol_options, symbol_place, symbol_replace, symbol_select_instances, symbol_set_current, symbol_spray, symbol_unused, symbol_update, diff --git a/crates/engine/src/cmd/brushsym/symbols.rs b/crates/engine/src/cmd/brushsym/symbols.rs index 9bfbb902a..8eb21dd64 100644 --- a/crates/engine/src/cmd/brushsym/symbols.rs +++ b/crates/engine/src/cmd/brushsym/symbols.rs @@ -95,14 +95,7 @@ pub(crate) fn normalize(mut art: Node) -> Result<(Node, (f64, f64), Point)> { Ok((art, (w, h), c)) } -/// After the art of symbol `name` changed: each of its instances keeps its appearance changes to -/// the objects the art still has. -pub(crate) fn fit_instances_of(d: &mut Document, name: &str) { - let ids = instances_of(d, Some(name)); - fit_overrides(d, &ids); -} - -pub(crate) fn instances_of(d: &Document, name: Option<&str>) -> Vec { +fn instances_of(d: &Document, name: Option<&str>) -> Vec { let mut v = vec![]; d.walk(|n| { if let NodeKind::SymbolInstance { symbol, .. } = &n.kind diff --git a/crates/engine/src/cmd/symboledit.rs b/crates/engine/src/cmd/symboledit.rs index 4cb58788b..0c365919b 100644 --- a/crates/engine/src/cmd/symboledit.rs +++ b/crates/engine/src/cmd/symboledit.rs @@ -419,17 +419,12 @@ fn each_changed_instance(n: &mut Node, name: &str, f: &mut dyn FnMut(&mut Vec], 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; - } - 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()); let mut follow = |o: &mut Vec| { follow_paths(o, old, new); @@ -439,7 +434,7 @@ fn follow_objects(d: &mut Document, name: &str, old: (&[Arc], bool), new: } }; let symbols = d.symbols.iter_mut().filter(|s| s.name != name).map(|s| &mut s.art); - let trees = symbols.chain(d.graph_designs.iter_mut().map(|g| &mut g.art)); + let trees = d.layers.iter_mut().chain(symbols).chain(d.graph_designs.iter_mut().map(|g| &mut g.art)); let patterns = d.patterns.iter_mut().chain(d.pattern_edit.iter_mut().filter_map(|pe| pe.original.as_mut())); for root in trees.chain(patterns.flat_map(|p| &mut p.art)) { if holds_changed_instance(root, name) { @@ -522,7 +517,6 @@ pub(crate) fn sync(before: &Document, d: &mut Document) -> Result<()> { }; d.symbols[index].art = Arc::new(new_art); follow_objects(d, &se.symbol, (&old, old_group), (&art, group)); - super::brushsym::fit_instances_of(d, &se.symbol); if let Some(e) = d.symbol_edit.as_mut() { e.changed = true; e.group = group; diff --git a/crates/engine/src/tests/symboledit.rs b/crates/engine/src/tests/symboledit.rs index 9a3ce144c..8cd11aaa2 100644 --- a/crates/engine/src/tests/symboledit.rs +++ b/crates/engine/src/tests/symboledit.rs @@ -581,6 +581,51 @@ fn instances_in_pattern_tiles_graph_designs_and_brushes_follow_an_edit_of_their_ assert!(stored.as_array().is_some_and(|a| a.iter().any(|e| e["name"] == "Pair Band" && e["note"] == "kept")), "{stored}"); } +/// An instance of a dynamic symbol that is the opacity mask of an object on a layer 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_in_an_opacity_mask_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"})); + let masked = id(&run(&mut s, "shape.rectangle", json!({"x": 0, "y": 0, "width": 120, "height": 60}))); + run(&mut s, "select.set", json!({"ids": [inst.0]})); + run(&mut s, "object.arrange.bringToFront", json!({})); + run(&mut s, "transparency.makeOpacityMask", json!({"ids": [masked.0, inst.0]})); + // The changes of the instance of "Pair" in the masked object's mask, by path and color. + let held = |s: &Session| -> Vec<(Vec, String)> { + let d = &s.doc().unwrap().doc; + let mask = d.node(masked).and_then(|n| n.extras().mask.clone()).expect("the opacity mask"); + let mut out = vec![]; + vectorcraft_doc::swatches::walk_with_masks(&mask.art, &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 + }; + assert_eq!(held(&s), vec![(vec![0], "#00ff00".to_string()), (vec![1], "#ff0000".to_string())]); + 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!(held(&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!(held(&s), vec![(vec![0], "#ff0000".to_string())]); + run(&mut s, "symbol.exitEditing", json!({})); + assert_eq!(held(&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(); From 2b7b12df33334a51d81a131be07bbe363b24e7b9 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 1 Nov 2026 01:17:33 -0700 Subject: [PATCH 7/8] Symbols: the edited symbol takes the text wrap its area type gets after a wrap object moves Symbol editing copies the editing layer's art into the symbol before Text Wrap, list numbering and threaded re-flow update that layer, so moving a wrap object in the edit left the symbol, its other instances and the saved file with the old wrap. The edit now syncs the symbol again after those passes, and when that changes the symbol, inline graphics of it are refreshed and the threads holding them re-flow. --- crates/engine/src/session/edit.rs | 27 ++++++++++++++++- crates/engine/src/tests/symboledit.rs | 42 +++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/session/edit.rs b/crates/engine/src/session/edit.rs index fd2de6ec5..71f70d25a 100644 --- a/crates/engine/src/session/edit.rs +++ b/crates/engine/src/session/edit.rs @@ -39,10 +39,13 @@ impl Session { } // Symbol editing: the symbol follows its art on the editing layer, before // inline graphics follow their symbols. - let symbol_synced = + let mut symbol_synced = if st.doc.symbol_edit.is_some() { cmd::symboledit::sync(&before, Arc::make_mut(&mut st.doc)) } else { Ok(()) }; // Inline graphics in text follow their symbols. cmd::inline::refresh(&before, Arc::make_mut(&mut st.doc)); + // The document the passes below start from, for the symbol to follow what they + // change on its editing layer. + let synced = st.doc.symbol_edit.is_some().then(|| st.doc.clone()); // Text Wrap: area type follows its wrap objects; then threads re-flow. cmd::textwrap::refresh(&before, Arc::make_mut(&mut st.doc)); // Type in no thread numbers its list paragraphs from 1 (before the mask copies @@ -56,6 +59,28 @@ impl Session { if !st.doc.text_threads.is_empty() { cmd::threads::reflow(&before, Arc::make_mut(&mut st.doc)); } + // Symbol editing: the symbol also takes the text wrap, list numbering and + // re-flow those passes gave its editing layer; when that changes the symbol, its + // inline graphics follow it and the threads they are in re-flow. + let editing = st.doc.symbol_edit.as_ref().map(|e| e.layer); + if let Some(synced) = &synced + && symbol_synced.is_ok() + && editing.is_some() + && before.symbol_edit.as_ref().map(|e| e.layer) == editing + { + let art = + |d: &Document| d.symbol_edit.as_ref().and_then(|e| d.symbols.iter().find(|x| x.name == e.symbol)).map(|x| x.art.clone()); + let old = art(&st.doc); + symbol_synced = cmd::symboledit::sync(synced, Arc::make_mut(&mut st.doc)); + let new = art(&st.doc); + if symbol_synced.is_ok() && !matches!((&old, &new), (Some(a), Some(b)) if Arc::ptr_eq(a, b)) { + let resynced = st.doc.clone(); + cmd::inline::refresh(synced, Arc::make_mut(&mut st.doc)); + if !st.doc.text_threads.is_empty() { + cmd::threads::reflow(&resynced, Arc::make_mut(&mut st.doc)); + } + } + } // Asset Export: assets let go of deleted art. if !st.doc.assets.is_empty() { Arc::make_mut(&mut st.doc).prune_assets(); diff --git a/crates/engine/src/tests/symboledit.rs b/crates/engine/src/tests/symboledit.rs index 8cd11aaa2..450572b73 100644 --- a/crates/engine/src/tests/symboledit.rs +++ b/crates/engine/src/tests/symboledit.rs @@ -676,3 +676,45 @@ fn undoing_entry_with_a_group_isolated_inside_the_art_goes_back_to_before() { assert_eq!(st.isolation, None); assert_eq!(st.active_layer, before); } + +/// The text wrap of the area type in `n`. +fn wrap_in(n: &Node) -> Vec { + let mut wrap = None; + n.walk(&mut |n| { + if let NodeKind::Text(t) = &n.kind { + wrap = Some(t.wrap.clone()); + } + }); + wrap.expect("area type") +} + +#[test] +fn moving_a_wrap_object_in_the_edit_gives_the_symbol_the_new_text_wrap() { + let mut s = Session::new(); + run(&mut s, "file.new", json!({"width": 600, "height": 600})); + let text = "word ".repeat(200); + let t = id(&run(&mut s, "text.create", json!({"x": 0, "y": 0, "text": text, "size": 12, "area": {"width": 400, "height": 400}}))); + let r = id(&run(&mut s, "shape.rectangle", json!({"x": 0, "y": 50, "width": 200, "height": 100}))); + run(&mut s, "select.set", json!({"ids": [r.0]})); + run(&mut s, "object.textWrap.make", json!({})); + run(&mut s, "select.set", json!({"ids": [t.0, r.0]})); + let instance = id(&run(&mut s, "symbol.new", json!({"name": "Wrapped"}))); + let symbol_wrap = |s: &Session| wrap_in(&s.doc().unwrap().doc.symbols.iter().find(|x| x.name == "Wrapped").expect("the symbol").art); + let entered = symbol_wrap(&s); + assert!(!entered.is_empty()); + run(&mut s, "symbol.edit", json!({"id": instance.0})); + let (layer, rect) = { + let st = s.doc().unwrap(); + let layer = st.doc.symbol_edit.as_ref().unwrap().layer; + let rect = layer_art(&s).into_iter().find(|i| st.doc.node(*i).is_some_and(|n| n.extras().wrap.is_some())).expect("the wrap object"); + (layer, rect) + }; + run(&mut s, "select.set", json!({"ids": [rect.0]})); + run(&mut s, "object.move", json!({"dx": 150, "dy": 0})); + let on_layer = wrap_in(s.doc().unwrap().doc.node(layer).unwrap()); + assert_ne!(on_layer, entered, "the editing layer's text wraps around the moved object"); + assert_eq!(symbol_wrap(&s), on_layer); + // Leaving keeps it. + run(&mut s, "symbol.exitEditing", json!({})); + assert_eq!(symbol_wrap(&s), on_layer); +} From 4dd97bc850ec523181c5ab33a85fa8b7e2f6187d Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 1 Nov 2026 01:08:49 -0800 Subject: [PATCH 8/8] Docs: symbol editing mode keeps every instance in step Add the progress line for the symbol editing follow-ups next to the Symbol editing mode entry in docs/target-app-parity.md, with the tests that cover them. --- docs/target-app-parity.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 8be75c5c1..452b1544d 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -612,6 +612,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - Swatch and graphic style libraries, Libraries panel files, presets and plug-ins open wherever they are dropped on the window, as File › Open opens them. - **Symbol libraries** ([public source](https://helpx.adobe.com/illustrator/desktop/manage-objects/traces-mockups-symbols/create-or-import-symbol-libraries.html)): Window › Symbol Libraries and the Symbols panel's Symbol Libraries Menu open each library in a library panel of its own, as swatch and graphic style libraries do. The built-in library, Basic Shapes, is original art generated in code (`crates/engine/src/cmd/symbollib.rs`). A click on a symbol adds it to the document's Symbols as one undo step, with its Symbol Options (size, registration, export type, 9-slice guides) and every resource it uses (the symbols it nests, patterns, swatches, brushes, graphic styles, images), and makes it the current symbol; a symbol the document already has is not added again and another symbol of the same name keeps its name, the library's comes in numbered. Thumbnails, in the library panels and the Symbols panel, draw a symbol with the symbols it nests and the patterns and images it uses. Dragging a library symbol onto the canvas also places an instance there, in the same step ([placing from a library](https://helpx.adobe.com/illustrator/desktop/manage-objects/traces-mockups-symbols/create-and-place-symbols.html)). Save Symbols (the library menu) and Save Symbol Library… (the Symbols panel menu) write the document's symbols as a `.vectorcraft` document holding only them, into the user library folder (listed under User Defined) or a file; Other Library… reads the symbols of any document VectorCraft opens; a file it can't read is an error and opens no panel. Tests: `crates/engine/src/tests/symbollib.rs`, `crates/ui-egui/src/panels/tests_symbol_libraries.rs`, `panels::symbols::tests::thumbnails_show_the_symbols_a_symbol_nests`, `canvas::tests::a_library_symbol_dropped_on_the_canvas_is_added_and_placed_there_in_one_step`. - **Symbol editing mode** ([Edit symbols](https://helpx.adobe.com/illustrator/desktop/manage-objects/traces-mockups-symbols/edit-symbols.html), [Symbols panel options](https://helpx.adobe.com/illustrator/desktop/manage-objects/traces-mockups-symbols/symbols-panel-options.html)): a double-click on a symbol instance, the Control bar's Edit Symbol button, a double-click on a symbol in the Symbols panel and the panel menu's Edit Symbol (`symbol.edit`) open the symbol's art on a temporary isolated layer, where the instance shows it (from the panel: centered on the active artboard, or in the view in a document without artboards), with the rest of the document dimmed. Each edit there changes every instance at once; the instances keep their own transforms and stay linked (Break Link to Symbol still expands one), and a dynamic symbol's instances keep their appearance changes to the objects the art still has. Escape, the isolation bar's back button, the Control bar's Exit Symbol Editing Mode and a double-click outside the art leave the mode (`symbol.exitEditing`); leaving without an edit adds no undo step. Saving leaves the editing layer out. Tests: `crates/engine/src/tests/symboledit.rs`, `select::tests::a_double_click_on_a_symbol_instance_edits_its_symbol`, `panels::symbols::tests::edit_symbol_from_the_panel_centers_the_art_on_the_active_artboard_or_the_view`. + - Symbol editing mode keeps every instance in step: instances of a dynamic symbol inside other symbols' art, pattern tiles, graph designs, stored brushes and opacity masks keep their appearance changes on the objects an edit reorders and drop the changes to objects it deletes. The view does not draw the instance the edit was entered from. Area type in the symbol's art takes the text wrap that moving a wrap object gives it. Editing another symbol at a place that fails keeps the current edit open, and only the editing layer or art on it can be isolated. Tests: `crates/engine/src/tests/symboledit.rs`, `isolation::symbol_editing_does_not_draw_the_instance_it_was_entered_from`, `panels::symbols::tests::double_clicking_a_symbol_in_the_panel_edits_it_in_either_view`, `the_panel_menus_edit_symbol_edits_the_instances_symbol_or_the_panels`, `the_control_bars_edit_symbol_edits_the_selected_instances_symbol`, `chrome::tests::the_control_bar_exits_symbol_editing_mode`. - **Delete Symbol asks first** ([Symbols panel](https://helpx.adobe.com/illustrator/desktop/manage-objects/traces-mockups-symbols/symbols-panel-options.html), [symbol libraries](https://helpx.adobe.com/illustrator/desktop/manage-objects/traces-mockups-symbols/create-or-import-symbol-libraries.html), [Alt-click Delete](https://helpx.adobe.com/illustrator/using/default-keyboard-shortcuts.html)): the Symbols panel's Delete Symbol button and menu item ask "Delete the selected symbol?" with Yes and No for a symbol no instance uses. For a symbol with placed instances they ask with Expand Instances (the instances become ordinary art), Delete Instances and Cancel; either Instances button deletes the symbol in one undo step. Alt-click on the button deletes an unused symbol without asking and still asks about one with instances. A question left open while another document became active deletes nothing. `symbol.delete` itself does not ask. Tests: `panels::symbols::tests::delete_symbol_asks_first_and_alt_click_deletes_an_unused_symbol_at_once`, `the_in_use_question_expands_or_deletes_the_instances`, `a_stale_delete_symbol_question_deletes_nothing`, `the_delete_symbol_questions_show_their_buttons`. - **Symbols › Select All Unused** ([Symbols panel](https://helpx.adobe.com/illustrator/desktop/manage-objects/traces-mockups-symbols/symbols-panel-options.html), #4918): the Symbols panel menu's Select All Unused shows the symbols nothing places selected (`symbol.unused`): instances in the art (opacity masks included), graph designs, pattern tiles and brush art count, and so do instances in a used symbol's art. Delete Symbol, the button or the menu item, then asks "Delete the selected symbols?" once and deletes those still unused in one undo step (`symbol.delete {names}`); Alt-click on the button deletes them without asking, and a click on a symbol picks just it again. Tests: `tests::brushsym::symbol_unused_lists_the_symbols_nothing_places_and_delete_takes_several`, `panels::symbols::tests::select_all_unused_picks_the_unused_symbols_and_delete_deletes_them`, `dialogs::symbol_delete::tests::several_symbols_get_the_plural_question`. - **Brush libraries** ([public source](https://helpx.adobe.com/illustrator/desktop/paint-and-fill/apply-and-edit-strokes/create-new-brush-libraries.html), [import](https://helpx.adobe.com/illustrator/desktop/paint-and-fill/apply-and-edit-strokes/import-brushes-from-brush-libraries-or-other-files.html), [Brushes panel](https://helpx.adobe.com/illustrator/desktop/paint-and-fill/apply-and-edit-strokes/brushes-panel-overview.html)): Window › Brush Libraries, the Brushes panel's Brush Libraries Menu and its menu's Open Brush Library open each library in a library panel of its own, as swatch, graphic style and symbol libraries do. The built-in libraries (Art Ribbons, Calligraphic Nibs, Pattern Borders, Scatter Shapes) are original brushes generated in code (`crates/engine/src/cmd/brushlib.rs`); Striped Ribbon's art is filled with the library's own pattern and Sparkles scatters the library's own symbol. A click on a brush adds it to the document's Brushes as one undo step with every resource its art uses (patterns, symbols, swatches, graphic styles, images), makes it the current brush and, with paths selected, applies it to them in the same step; a brush the document already has is not added again and a different brush of the same name comes in numbered ("Wave 2"). Shift/Cmd-clicks select brushes for Add to Brushes; a brush dragged onto a path is added and applied to it, and one dragged onto the Brushes panel is added. Save Brushes… / the Brushes panel's Save Brush Library… write the document's brushes (or the selected ones) as a `.vectorcraft` library into the `Brushes` folder next to the preferences, listed under User Defined, or to a file; Other Library… reads the brushes of any document VectorCraft opens. Thumbnails, in the library panels, the Brushes panel and Brush Options, render brush art with the patterns and symbols it uses. Copying brushed art now carries the patterns and symbols its brush's art uses too. Tests: `crates/engine/src/tests/brushlib.rs`, `crates/ui-egui/src/panels/tests_brush_libraries.rs`, `panels::brushes::tests::library_brush_previews_show_the_patterns_their_art_uses`, `canvas::tests::a_library_brush_dropped_on_a_path_is_added_and_applied_in_one_step`.