From 175f51827ddd4539705145b31bca8477b388d877 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 12 Nov 2026 00:18:05 -0800 Subject: [PATCH] Symbols: redefining a dynamic symbol drops paints its new objects do not draw When a dynamic symbol is redefined or edited, each instance kept every appearance change whose path still named an object, even when that object is now one that draws no fill or stroke of its own (an image, a blend, a mesh) or a symbol instance given a gradient or pattern fill. symbol.instance.setAppearance refuses such a paint, but the change stayed on the instance and still counted as a color the document uses. Fitting the changes to the new art now also clears a fill or stroke the object at that path does not draw, and drops a change left empty. --- crates/engine/src/cmd/brushsym.rs | 2 +- crates/engine/src/cmd/brushsym/dynamic.rs | 43 +++++++++++++++++----- crates/engine/src/cmd/symboledit.rs | 8 ++-- crates/engine/src/tests/dynamic_symbols.rs | 24 ++++++++++++ docs/mcp.md | 5 ++- 5 files changed, 66 insertions(+), 16 deletions(-) diff --git a/crates/engine/src/cmd/brushsym.rs b/crates/engine/src/cmd/brushsym.rs index b4b231a22b..097be73686 100644 --- a/crates/engine/src/cmd/brushsym.rs +++ b/crates/engine/src/cmd/brushsym.rs @@ -36,7 +36,7 @@ use brushes::{ brush_apply, brush_delete, brush_duplicate, brush_freehand, brush_get, brush_list, brush_move, brush_new, brush_options, brush_remove, brush_selection_art, brush_set_current, brush_unused, expand_brush, }; -pub(crate) use dynamic::{edit_part, part_only, selected_part}; +pub(crate) use dynamic::{edit_part, fit_parts, part_only, selected_part}; use dynamic::{symbol_instance_reset, symbol_instance_select_part, symbol_instance_set_appearance}; use symbols::selected_instances; pub(crate) use symbols::{ diff --git a/crates/engine/src/cmd/brushsym/dynamic.rs b/crates/engine/src/cmd/brushsym/dynamic.rs index 888e450085..f5cef8351e 100644 --- a/crates/engine/src/cmd/brushsym/dynamic.rs +++ b/crates/engine/src/cmd/brushsym/dynamic.rs @@ -2,7 +2,7 @@ //! appearance of objects in the symbol's art and stays linked to the symbol //! (`symbol.instance.setAppearance`, `symbol.instance.reset`). The changes live on the instance //! ([`vectorcraft_doc::symbol_overrides`]); editing the symbol keeps those whose object is still -//! there ([`fit_overrides`]). +//! there and draws them ([`fit_overrides`]). use std::sync::Arc; @@ -27,7 +27,7 @@ pub(super) fn type_id(dynamic: bool) -> &'static str { } /// Fit the appearance changes of instances `ids` to their symbols: an instance of a static (or -/// missing) symbol keeps none, an instance of a dynamic one those naming an object of its art. +/// missing) symbol keeps none, an instance of a dynamic one those its art still draws ([`fit_parts`]). pub(super) fn fit_overrides(d: &mut Document, ids: &[NodeId]) { for id in ids { let art = match d.node(*id) { @@ -40,7 +40,7 @@ pub(super) fn fit_overrides(d: &mut Document, ids: &[NodeId]) { let Some(n) = d.node_mut(*id) else { continue }; let mut e = n.extras_mut(); match art { - Some(art) => symbol_overrides::retain_parts(&mut e.symbol_overrides, &art), + Some(art) => fit_parts(&mut e.symbol_overrides, &art), None => e.symbol_overrides.clear(), } } @@ -177,17 +177,42 @@ fn check_paint_kinds(p: &Value, art: &Node, path: &[u32], cmd: &str) -> Result<( fn solid_fill_check(p: &Value, art: &Node, path: &[u32], fill: Option<&vectorcraft_color::Paint>, cmd: &str) -> Result<()> { let asked = p.get("fill").is_some_and(|v| !v.is_null()); let is_instance = symbol_overrides::part(art, path).is_some_and(|n| matches!(n.kind, NodeKind::SymbolInstance { .. })); - let draws_nothing = |f: &vectorcraft_color::Paint| match f { - vectorcraft_color::Paint::Gradient(_) => true, - vectorcraft_color::Paint::Pattern { .. } => true, - vectorcraft_color::Paint::None | vectorcraft_color::Paint::Solid { .. } => false, - }; - if asked && is_instance && fill.is_some_and(draws_nothing) { + if asked && is_instance && fill.is_some_and(|f| !stains(f)) { return Err(bad(cmd, "fill on a symbol instance must be a solid color or none")); } Ok(()) } +/// Can a symbol instance's fill be `f`? An instance stains its art with a solid fill only. +fn stains(f: &vectorcraft_color::Paint) -> bool { + match f { + vectorcraft_color::Paint::Gradient(_) => false, + vectorcraft_color::Paint::Pattern { .. } => false, + vectorcraft_color::Paint::None | vectorcraft_color::Paint::Solid { .. } => true, + } +} + +/// Keep the appearance changes `overrides` that name an object of `art` (a symbol's new art), each +/// without a fill or stroke that object does not draw ([`paint_refusal`], [`stains`]); a change +/// left with nothing in it is dropped. +pub(crate) fn fit_parts(overrides: &mut Vec, art: &Node) { + symbol_overrides::retain_parts(overrides, art); + for o in overrides.iter_mut() { + let Some(n) = symbol_overrides::part(art, &o.path) else { continue }; + let instance = matches!(n.kind, NodeKind::SymbolInstance { .. }); + if paint_refusal(n, "fill", FIT).is_some() || (instance && o.fill.as_ref().is_some_and(|f| !stains(f))) { + o.fill = None; + } + if paint_refusal(n, "stroke", FIT).is_some() { + o.stroke = None; + } + } + overrides.retain(|o| !o.is_empty()); +} + +/// The command name [`fit_parts`] gives [`paint_refusal`], whose refusal it does not show. +const FIT: &str = "symbol.update"; + /// `fill` or `stroke` of `p` for override `o`: absent leaves it, null clears it, a colour string or /// an object of `paint.setFill`'s paint params sets it. fn paint_change(s: &Session, p: &Value, key: &str, cmd: &str) -> Result>> { diff --git a/crates/engine/src/cmd/symboledit.rs b/crates/engine/src/cmd/symboledit.rs index a5f677f5dd..606ab40fcf 100644 --- a/crates/engine/src/cmd/symboledit.rs +++ b/crates/engine/src/cmd/symboledit.rs @@ -19,7 +19,7 @@ use vectorcraft_doc::symbol_overrides::SymbolOverride; use vectorcraft_doc::{Document, Node, NodeId, NodeKind, Selection, SymbolEdit, SymbolRegistration}; use vectorcraft_geom::{Affine, Point}; -use super::brushsym::{natural_size, place_xf}; +use super::brushsym::{fit_parts, natural_size, place_xf}; use super::{EngineError, Result, Session, bad, number_or, object_param, ok, str_param}; /// The name of the undo step that enters symbol editing mode. @@ -425,8 +425,8 @@ fn each_changed_instance(n: &mut Node, name: &str, f: &mut dyn FnMut(&mut Vec], bool), new: let mut follow = |o: &mut Vec| { follow_paths(o, old, new); match &art { - Some(art) => vectorcraft_doc::symbol_overrides::retain_parts(o, art), + Some(art) => fit_parts(o, art), None => o.clear(), } }; diff --git a/crates/engine/src/tests/dynamic_symbols.rs b/crates/engine/src/tests/dynamic_symbols.rs index 278d979ef5..c5ef8a1949 100644 --- a/crates/engine/src/tests/dynamic_symbols.rs +++ b/crates/engine/src/tests/dynamic_symbols.rs @@ -432,6 +432,30 @@ fn redefining_the_symbol_keeps_the_changes_whose_objects_remain() { assert_eq!(overrides(&s, inst), 0); } +#[test] +fn redefining_the_symbol_drops_paints_its_new_objects_do_not_draw() { + let mut s = session(); + let inst = pair(&mut s, "dynamic"); + let p = json!({"id": inst.0, "path": [1], "fill": "#ff0000", "stroke": "#ff0000", "opacity": 50}); + s.execute("symbol.instance.setAppearance", &p).unwrap(); + // The second object is now an image, which draws no fill or stroke of its own. + let a = square(&mut s, 10.0); + place_image(&mut s); + let image = top(&s); + s.execute("symbol.update", &json!({"name": "Pair", "ids": [a.0, image.0]})).unwrap(); + let o = node(&s, inst).extras().symbol_overrides.clone(); + assert_eq!(o.len(), 1, "the opacity change stays: {o:?}"); + assert_eq!((&o[0].fill, &o[0].stroke, o[0].opacity), (&None, &None, Some(0.5))); + // A change of paint alone goes with its paint: the first object is now a blend. + s.execute("symbol.instance.setAppearance", &json!({"id": inst.0, "path": [0], "stroke": "#ff0000"})).unwrap(); + let (b, c) = (square(&mut s, 10.0), square(&mut s, 60.0)); + let blend = id(&s.execute("object.blend.make", &json!({"ids": [b.0, c.0], "steps": 1})).unwrap()); + let d = square(&mut s, 120.0); + s.execute("symbol.update", &json!({"name": "Pair", "ids": [blend.0, d.0]})).unwrap(); + let o = node(&s, inst).extras().symbol_overrides.clone(); + assert_eq!(o.iter().map(|o| o.path.clone()).collect::>(), [vec![1]], "{o:?}"); +} + #[test] fn replace_symbol_drops_the_changes_made_for_the_old_symbol() { let mut s = session(); diff --git a/docs/mcp.md b/docs/mcp.md index aeed51157a..c6a5e616d9 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -2575,8 +2575,9 @@ already walks Image Trace. Short agent entry points: locked or sits in a hidden or locked group or layer. Choosing another selection tool selects the instance whole again. `symbol.instance.reset {ids?}` drops the selected instances' changes (the Properties panel's Reset). Redefining the symbol (`symbol.update`) reshapes every instance and keeps each change whose path the - new art still has; `symbol.breakLink` bakes the changes into the expanded art; `symbol.edit` edits the symbol's own - art, and each edit there keeps the changes whose path the art still has; making the symbol static drops them, and + new art still has, less a fill or stroke the object now at that path does not draw (a change left empty is dropped); + `symbol.breakLink` bakes the changes into the expanded art; `symbol.edit` edits the symbol's own art, and each edit + there keeps the changes the same way; making the symbol static drops them, and so does `symbol.replace` with a different symbol (a replace with the instance's own symbol keeps them). ```json