Merge pull request #3956 from dotCooCoo/fix/recolor-group-keeps-swatch-links

Recolor: recolor.apply with a color group keeps art linked to the group's global swatches

crates/engine/src/cmd/recolor.rs

@@ -31,7 +31,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Recolor Artwork",
             ["Edit", "Edit Colors"],
             None,
-            "{map: rows [{from: [colour keys (recolor.colors) or \"#rrggbb\" (every colour shown as that hex)], to: colour (hex, key, {c,m,y,k} or {l,a,b}) | null, exclude?: false (the row's colours are kept)}] | {\"key or #rrggbb\": colour, …} (one row per colour), method?: \"exact\" (default)|\"preserveTints\"|\"scaleTints\"|\"tintsShades\"|\"hueShift\" (how a row's colours take its new colour, relative to the row's darkest, average or most saturated colour), limitTo?: swatch library id or name; or \"document\" (the document's swatches): new colours snap to its nearest colour, group?: colour group to rewrite, groupColors?: [colour] (what `group` is rewritten with; default: the rows' new colours, in order, excluded rows left out), rename?: the group's new name, includeImages?: true, includePatterns?: true} recolour the selection (gradients, text, meshes, image pixels and pattern tiles included; each new colour keeps the model of the one it replaces (model?: \"keep\" default | \"new\" (the new colour's own model, so a CMYK colour replacing RGB art gives CMYK art) | \"document\" (the document's colour mode)), except a library swatch's with limitTo: art takes the swatch itself, linked to a global or spot colour at its tints, and the swatch joins the document's swatches) and the group, as one undo step → {changed, swatches: [names added to the document's swatches], note?: set when a colour kept a model other than its new colour's (see model)}",
+            "{map: rows [{from: [colour keys (recolor.colors) or \"#rrggbb\" (every colour shown as that hex)], to: colour (hex, key, {c,m,y,k} or {l,a,b}) | null, exclude?: false (the row's colours are kept)}] | {\"key or #rrggbb\": colour, …} (one row per colour), method?: \"exact\" (default)|\"preserveTints\"|\"scaleTints\"|\"tintsShades\"|\"hueShift\" (how a row's colours take its new colour, relative to the row's darkest, average or most saturated colour), limitTo?: swatch library id or name; or \"document\" (the document's swatches): new colours snap to its nearest colour, group?: colour group to rewrite, groupColors?: [colour] (what `group` is rewritten with; default: the rows' new colours, in order, excluded rows left out), rename?: the group's new name, includeImages?: true, includePatterns?: true} recolour the selection (gradients, text, meshes, image pixels and pattern tiles included; each new colour keeps the model of the one it replaces (model?: \"keep\" default | \"new\" (the new colour's own model, so a CMYK colour replacing RGB art gives CMYK art) | \"document\" (the document's colour mode)), except a library swatch's with limitTo: art takes the swatch itself, linked to a global or spot colour at its tints, and the swatch joins the document's swatches; art linked to a global swatch of `group` that is rewritten with its row's new color stays linked to it, at its tint, unless the row converts the color into the art's model; other recolored art, and art in a row linked to a global swatch of `group` that is rewritten with another color, keeps its row's new color and loses its swatch link) and the group, as one undo step → {changed, swatches: [names added to the document's swatches], relinked?: with group, the other paints linked to its global swatches that took their new colors, note?: set when a colour kept a model other than its new colour's (see model)}",
             has_doc,
             apply
         ),
@@ -291,11 +291,23 @@ struct Link {
     color: Color,
 }
 
+/// A global swatch of the color group `recolor.apply` rewrites: its name, the color it is
+/// rewritten with and the color art linked to it shows afterwards.
+struct GroupSwatch {
+    name: String,
+    color: Color,
+    shown: Color,
+}
+
 /// A parsed map ready to recolour: the row of each current colour, and the swatch of each row's
 /// new colour.
 struct ColorMap {
     rows: Vec<RowMap>,
     links: Vec<Option<Link>>,
+    /// The new color of each of `rows`.
+    tos: Vec<Color>,
+    /// The global swatches of the color group being rewritten.
+    group: Vec<GroupSwatch>,
     keys: HashMap<ColorKey, usize>,
     hexes: HashMap<String, usize>,
     /// Set when a colour took a new colour of another model in its own model.
@@ -303,14 +315,17 @@ struct ColorMap {
 }
 
 impl ColorMap {
-    /// `links`: the swatch of each of `rows`' new colours (as many as `rows`, or none).
-    fn new(rows: &[Row], method: Method, own_model: bool, links: &[Option<Link>]) -> Self {
-        let mut out = Self { rows: vec![], links: vec![], keys: HashMap::new(), hexes: HashMap::new(), converted: Default::default() };
+    /// `links`: the swatch of each of `rows`' new colours (as many as `rows`, or none); `group`: the
+    /// global swatches of the color group being rewritten.
+    fn new(rows: &[Row], method: Method, own_model: bool, links: &[Option<Link>], group: Vec<GroupSwatch>) -> Self {
+        let mut out =
+            Self { rows: vec![], links: vec![], tos: vec![], group, keys: HashMap::new(), hexes: HashMap::new(), converted: Default::default() };
         for (n, r) in rows.iter().enumerate().filter(|(_, r)| !r.from.is_empty()) {
             let Some(to) = r.new_color() else { continue };
             let i = out.rows.len();
             let colors: Vec<Color> = r.from.iter().map(Match::color).collect();
             out.rows.push(RowMap::new(method, &colors, to).with_own_model(own_model));
+            out.tos.push(to);
             out.links.push(links.get(n).cloned().flatten());
             for m in &r.from {
                 match m {
@@ -337,11 +352,26 @@ impl ColorMap {
 
     /// Recolour a colour that can link to a swatch: a row whose new colour is a global or spot
     /// swatch links its colours to it (at their tints; shades and other hues stay unlinked), a
-    /// process swatch gives its colour in its own model; other rows map the colour, which loses its
-    /// link when it changes. True when anything changed.
+    /// process swatch gives its colour in its own model; a color linked to a global swatch of the
+    /// rewritten group that takes the row's new color keeps its link (at its tint in the row) when
+    /// the row gives it the new color in that color's model;
+    /// other rows map the colour, which loses its link when it changes or when the group's rewrite
+    /// would change it. True when anything changed.
     fn map_link(&self, c: &mut Color, link: &mut Option<String>, tint: &mut f32) -> bool {
         let Some(i) = self.row_of(*c) else { return false };
         let (Some(row), Some(l)) = (self.rows.get(i), self.links.get(i)) else { return false };
+        if l.is_none()
+            && let Some(g) = link.as_deref().and_then(|n| self.group.iter().find(|g| g.name == n))
+            && self.tos.get(i) == Some(&g.color)
+            && !row.converts(*c)
+            && let Some(t) = row.tint(*c)
+        {
+            let t = t.clamp(0.0, 1.0);
+            let shown = g.shown.tinted(t);
+            let changed = *tint != t || *c != shown;
+            (*c, *tint) = (shown, t);
+            return changed;
+        }
         let new = match (l, row.tint(*c)) {
             (Some(l), Some(t)) if l.global => {
                 let t = t.clamp(0.0, 1.0);
@@ -358,7 +388,10 @@ impl ColorMap {
                 row.map(*c)
             }
         };
-        if new == *c {
+        // A color its row leaves as it is still loses its link to a swatch of the rewritten group
+        // when the rewrite would give it another color.
+        let rewritten = link.as_deref().and_then(|n| self.group.iter().find(|g| g.name == n)).is_some_and(|g| g.shown.tinted(*tint) != new);
+        if new == *c && !rewritten {
             return false;
         }
         *c = new;
@@ -448,17 +481,31 @@ fn apply(s: &mut Session, p: &Value) -> Result<Value> {
                 Some(Link { name, global: w.global, color })
             })
             .collect();
-        let map = ColorMap::new(&rows, method, own_model, &links);
+        // The group's global swatches that its rewrite gives a color: art linked to one of them
+        // whose row's new color is that color stays linked.
+        let kept: Vec<GroupSwatch> = group
+            .and_then(|g| d.swatch_groups.iter().find(|x| x.name == g))
+            .map(|g| {
+                g.swatches
+                    .iter()
+                    .zip(&new)
+                    .filter(|(w, _)| w.global && !w.is_reserved())
+                    .map(|(w, c)| GroupSwatch { name: w.name.clone(), color: *c, shown: d.linked_color(*c, w.spot) })
+                    .collect()
+            })
+            .unwrap_or_default();
+        let map = ColorMap::new(&rows, method, own_model, &links, kept);
         let f = |c: Color| map.map(c);
         let lf = |c: &mut Color, link: &mut Option<String>, tint: &mut f32| map.map_link(c, link, tint);
         let changed = Recolor::with_links(scope, &f, &lf).run(d, &ids);
         let edit = group.map(|g| super::swatch::rewrite_group(d, g, &new, rename, C)).transpose()?;
         Ok((changed, edit, added, map.converted.get()))
     })?;
+    let mut out = json!({ "changed": changed, "swatches": added });
     if let Some(e) = edit {
         e.defaults(s);
+        out["relinked"] = json!(e.relinked());
     }
-    let mut out = json!({ "changed": changed, "swatches": added });
     if converted {
         out["note"] = json!(
             "some colours kept their own colour model, so the new colour was converted into it and may look different; \

crates/engine/src/cmd/swatch.rs

@@ -830,6 +830,11 @@ impl GroupEdit {
     pub(super) fn defaults(&self, s: &mut Session) {
         self.relinks.iter().for_each(|r| r.defaults(s));
     }
+
+    /// How many paints linked to the group's swatches the rewrite changed.
+    pub(super) fn relinked(&self) -> usize {
+        self.relinked
+    }
 }
 
 /// Rewrite colour group `group` with `colors`: its swatches take them in order (keeping their

crates/engine/src/tests_recolor.rs

@@ -379,6 +379,82 @@ fn edit_group_relinks_art_and_apply_rewrites_the_group_with_the_art() {
     assert_eq!(g.swatches.iter().map(|w| w.paint.color().unwrap()).collect::<Vec<_>>(), [Color::rgb8(1, 2, 3), Color::cmyk(0.0, 0.0, 0.0, 0.2)]);
 }
 
+/// Art linked to a global swatch of the group that `recolor.apply` rewrites stays linked when the
+/// swatch takes its row's new color: it shows that color at its tint and follows later edits.
+#[test]
+fn apply_with_a_group_keeps_art_linked_to_its_global_swatches() {
+    let mut s = session();
+    run(&mut s, "swatch.newGroup", json!({"name": "Brand", "colors": ["#2878f0"]}));
+    let name = s.doc().unwrap().doc.swatch_groups.iter().find(|g| g.name == "Brand").unwrap().swatches[0].name.clone();
+    run(&mut s, "swatch.edit", json!({"name": name, "global": true}));
+    let full = rect(&mut s, json!("#000000"));
+    let tint = rect(&mut s, json!("#000000"));
+    run(&mut s, "paint.setFill", json!({"ids": [full.0], "swatch": name}));
+    run(&mut s, "paint.setFill", json!({"ids": [tint.0], "swatch": name, "tint": 50}));
+    let link = |s: &Session, id: NodeId| match s.doc().unwrap().doc.node(id).unwrap().appearance.fill_paint() {
+        vectorcraft_color::Paint::Solid { color, swatch, tint } => (color, swatch, tint),
+        p => panic!("{p:?}"),
+    };
+    let swatch_hex = |s: &Session| s.doc().unwrap().doc.swatch(&name).unwrap().paint.color().unwrap().to_hex();
+    let orange = Color::from_hex("#ff6a00").unwrap();
+    select_all(&mut s);
+    let undo = s.doc().unwrap().history.undo.len();
+    // The 50% tint is not in the row: it follows the swatch.
+    let r = run(&mut s, "recolor.apply", json!({"map": {"#2878f0": "#ff6a00"}, "group": "Brand"}));
+    assert_eq!(r["relinked"], 1, "{r}");
+    assert_eq!(link(&s, full), (orange, Some(name.clone()), 1.0), "the recolored art stays linked");
+    let (c, w, t) = link(&s, tint);
+    assert!(w.as_deref() == Some(name.as_str()) && t == 0.5 && close(c, orange.tinted(0.5)), "{c:?} {w:?} {t}");
+    assert_eq!(swatch_hex(&s), "#ff6a00");
+    assert_eq!(s.doc().unwrap().history.undo.len(), undo + 1, "one undo step");
+    // A later edit of the swatch reaches the art.
+    let r = run(&mut s, "swatch.edit", json!({"name": name, "color": "#00ff00"}));
+    assert_eq!(r["relinked"], 2, "{r}");
+    assert_eq!(fill(&s, full).to_hex(), "#00ff00");
+    assert!(close(fill(&s, tint), Color::from_hex("#00ff00").unwrap().tinted(0.5)));
+    run(&mut s, "edit.undo", json!({}));
+    run(&mut s, "edit.undo", json!({}));
+    assert_eq!(swatch_hex(&s), "#2878f0");
+    assert_eq!(link(&s, full), (Color::from_hex("#2878f0").unwrap(), Some(name.clone()), 1.0), "undo restores the art");
+    // A row holding the tint too (as recolor.reduce gives it): the tint keeps its tint and its link.
+    select_all(&mut s);
+    let tint_key = vectorcraft_color::recolor::ColorKey::of(&fill(&s, tint)).to_string();
+    let rows = json!([{"from": ["rgb 40 120 240", tint_key], "to": "#ff6a00"}]);
+    run(&mut s, "recolor.apply", json!({"map": rows, "method": "scaleTints", "group": "Brand"}));
+    let (c, w, t) = link(&s, tint);
+    assert!(w.as_deref() == Some(name.as_str()) && (t - 0.5).abs() < 0.01 && close(c, orange.tinted(t)), "{c:?} {w:?} {t}");
+    assert_eq!(link(&s, full), (orange, Some(name.clone()), 1.0));
+    // A swatch rewritten with another color than its art's row: that art takes the row's color, unlinked.
+    run(&mut s, "edit.undo", json!({}));
+    select_all(&mut s);
+    run(&mut s, "recolor.apply", json!({"map": {"#2878f0": "#ff6a00"}, "group": "Brand", "groupColors": ["#123456"]}));
+    assert_eq!(link(&s, full), (orange, None, 1.0));
+    assert_eq!(swatch_hex(&s), "#123456");
+    // A row that leaves the art's color as it is while the swatch is rewritten with another color:
+    // the art keeps its color, unlinked.
+    run(&mut s, "edit.undo", json!({}));
+    select_all(&mut s);
+    let blue = Color::from_hex("#2878f0").unwrap();
+    run(&mut s, "recolor.apply", json!({"map": {"#2878f0": "#2878f0"}, "group": "Brand", "groupColors": ["#123456"]}));
+    assert_eq!(link(&s, full), (blue, None, 1.0));
+    assert_eq!(swatch_hex(&s), "#123456");
+    // A CMYK new color for RGB art keeps the art's model (model: "keep"), so the art takes the
+    // color in RGB, unlinked, while the swatch takes it in CMYK.
+    run(&mut s, "edit.undo", json!({}));
+    select_all(&mut s);
+    let r = run(&mut s, "recolor.apply", json!({"map": {"#2878f0": {"c": 0, "m": 60, "y": 100, "k": 0}}, "group": "Brand"}));
+    assert!(r["note"].is_string(), "{r}");
+    let (c, w, _) = link(&s, full);
+    assert!(matches!(c, Color::Rgb { .. }) && w.is_none(), "{c:?} {w:?}");
+    assert!(matches!(s.doc().unwrap().doc.swatch(&name).unwrap().paint.color(), Some(Color::Cmyk { .. })));
+    // Without a group the swatch is unchanged, so the art takes the new color unlinked.
+    run(&mut s, "edit.undo", json!({}));
+    select_all(&mut s);
+    run(&mut s, "recolor.apply", json!({"map": {"#2878f0": "#ff6a00"}}));
+    assert_eq!(link(&s, full), (orange, None, 1.0));
+    assert_eq!(swatch_hex(&s), "#2878f0");
+}
+
 #[test]
 fn edit_group_refuses_to_change_built_in_swatches() {
     let mut s = session();

docs/mcp.md

@@ -1692,7 +1692,12 @@ the others turn by the same hue). `limitTo` snaps new colours to a swatch librar
 `recolor.randomize {map, order?, saturationBrightness?, seed?}` shuffles or varies new colours; a row with
 `exclude: true` keeps its colours. `swatch.editGroup {group, colors, rename?}` rewrites a colour group in one undo
 step (art linked to its global swatches follows), and `recolor.apply` with `group` recolours the art and rewrites the
-group together.
+group together. Art linked to one of the group's global swatches stays linked when the swatch is rewritten with its
+row's new color (the default without `groupColors` when the rows follow the group's order), so a later `swatch.edit` still
+reaches it; the reply's `relinked` counts the other linked paints that took the group's new colors. Art in a row that
+is linked to a swatch rewritten with another color keeps its row's new color, even an unchanged one, and is no longer
+linked to the swatch, as does art whose color the row converts into the art's own color model. Without `group`,
+the swatch keeps its color and the recolored art is no longer linked to it.
 
 ```json
 {"name":"run_command","arguments":{"command":"recolor.reduce","params":{"colors":2,"preserve":{"grays":true}}}}