From e11e7a39d9d7b512fe97ee843f3c3da0bb741d46 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 29 Oct 2026 02:02:03 -0700 Subject: [PATCH] Recolor: count left-out art by the colors it shows A recolor of everything counted a locked or hidden object as left unchanged when the pass would only have linked it, or the tiles of a pattern it is filled with, to a swatch of the same color. With `group`, it also counted one as changed when the group's rewrite only removed its link to a dropped swatch, and as unchanged when the rewrite recolored a pattern it is filled with. The trial pass now leaves out changes to swatch links alone, patterns' tiles included, and the count compares the colors the object shows, its patterns' tiles included, before the recolor and after the rewrite. --- crates/engine/src/cmd/colorcmds.rs | 48 ++++++++++++++++-- crates/engine/src/cmd/recolor.rs | 37 +++++++++++--- crates/engine/src/tests/recolor.rs | 79 ++++++++++++++++++++++++++++++ 3 files changed, 155 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/cmd/colorcmds.rs b/crates/engine/src/cmd/colorcmds.rs index f068b41b5..3b0c3f788 100644 --- a/crates/engine/src/cmd/colorcmds.rs +++ b/crates/engine/src/cmd/colorcmds.rs @@ -218,10 +218,29 @@ impl<'a> Recolor<'a> { Self { links: self.links, ..Self::new(self.scope, self.f) } } - /// The entries of `objects` (ids, each with a flag) whose object this pass would change. The - /// document stays as it is. + /// The entries of `objects` (ids, each with a flag) whose object this pass would change, other + /// than by swatch links alone, its patterns' tiles included. The document stays as it is. pub(crate) fn would_change(mut self, d: &Document, objects: &[(NodeId, T)]) -> Vec<(NodeId, T)> { - objects.iter().copied().filter(|(id, _)| d.node(*id).is_some_and(|n| self.subtree(d, n).is_some())).collect() + let tried: Vec<(&Node, Node, T)> = + objects.iter().filter_map(|&(id, flag)| d.node(id).and_then(|n| Some((n, self.subtree(d, n)?, flag)))).collect(); + let same = self.link_only_copies(d); + tried.into_iter().filter_map(|(n, m, flag)| (without_links(m, &same) != without_links(n.clone(), &same)).then_some((n.id, flag))).collect() + } + + /// The patterns this pass has copied whose copy differs from the original only by swatch + /// links: the copy's name → the original's. + fn link_only_copies(&self, d: &Document) -> HashMap { + let none = HashMap::new(); + let art = |v: &[Arc]| v.iter().map(|n| without_links((**n).clone(), &none)).collect::>(); + let copies: HashMap<&str, &PatternDef> = self.new_patterns.iter().map(|p| (p.name.as_str(), p)).collect(); + self.patterns + .iter() + .filter_map(|(old, new)| { + let new = new.as_ref()?; + let copy = copies.get(new.as_str())?; + (art(&d.pattern(old)?.art) == art(©.art)).then(|| (new.clone(), old.clone())) + }) + .collect() } /// Recolour the objects `ids` and everything inside them (each once), add the new images and @@ -324,6 +343,29 @@ impl<'a> Recolor<'a> { } } +/// `n` with the swatch links of its paints and its children's removed, each color kept as it +/// shows, and each pattern named in `same` (a copy → its original) by its original's name. +fn without_links(mut n: Node, same: &HashMap) -> Node { + for p in vectorcraft_doc::swatches::node_paints_mut(&mut n) { + map_links(p, &|_, link, _| link.take().is_some()); + match p { + Paint::Gradient(g) => g.swatch = None, + Paint::Pattern { pattern, .. } => { + if let Some(original) = same.get(pattern.as_str()) { + *pattern = original.clone(); + } + } + _ => {} + } + } + if let Some(children) = n.children_mut() { + for c in children.iter_mut() { + *c = Arc::new(without_links((**c).clone(), same)); + } + } + n +} + /// What a pass over document `d` does to each colour the shared visitor meets. struct Visit<'p, 'a> { pass: &'p mut Recolor<'a>, diff --git a/crates/engine/src/cmd/recolor.rs b/crates/engine/src/cmd/recolor.rs index 4a29afd4b..f5fdeb122 100644 --- a/crates/engine/src/cmd/recolor.rs +++ b/crates/engine/src/cmd/recolor.rs @@ -116,6 +116,27 @@ fn used_colors(d: &Document, ids: &[NodeId], only: Option<&BTreeSet>) -> used.into_iter().map(|(_, u)| u).collect() } +/// The colors `n` shows, in paint order ([`node_colors`]), then the colors of the tiles of the +/// patterns it uses as fills or strokes. +fn shown_colors(d: &Document, n: &Node) -> Vec { + let mut colors = vec![]; + node_colors(n, &mut |c, _| colors.push(*c)); + let mut patterns: Vec<&str> = vec![]; + n.walk(&mut |m| { + for p in node_paints(m) { + if let Paint::Pattern { pattern, .. } = p + && !patterns.contains(&pattern.as_str()) + { + patterns.push(pattern); + } + } + }); + for def in patterns.iter().filter_map(|p| d.pattern(p)) { + def.art.iter().for_each(|a| node_colors(a, &mut |c, _| colors.push(*c))); + } + colors +} + /// The objects whose colors `recolor.colors` and `recolor.reduce` read, for [`used_colors`]: `ids` /// (each checked) or else the selection, with everything they hold; with nothing selected, the /// objects Select All takes and the objects inside them, leaving out every hidden or locked object @@ -501,18 +522,22 @@ fn apply(s: &mut Session, p: &Value) -> Result { 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, tint: &mut f32| map.map_link(c, link, tint); - // The objects left out whose colors the pass would change, as they are now. The trial - // pass does not count towards the note on color models. - let before: Vec<(Node, bool)> = Recolor::with_links(scope, &f, &lf) + // The objects left out whose colors the pass would change, with the colors they show now. + // The trial pass does not count towards the note on color models. + let before: Vec<(NodeId, Vec, bool)> = Recolor::with_links(scope, &f, &lf) .would_change(d, &left) .into_iter() - .filter_map(|(id, hidden)| Some((d.node(id)?.clone(), hidden))) + .filter_map(|(id, hidden)| Some((id, shown_colors(d, d.node(id)?), hidden))) .collect(); map.converted.set(false); 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()?; - // Rewriting the group's swatches can change art linked to them, left out or not. - let not_changed = NotChanged::of(before.iter().filter(|(n, _)| d.node(n.id) == Some(n)).map(|(_, hidden)| *hidden)); + // Rewriting the group's swatches can change the colors of art linked to them, left out or + // not, and of the patterns it uses. Art that only loses its link keeps its colors and is + // counted. + let not_changed = NotChanged::of( + before.iter().filter(|(id, colors, _)| d.node(*id).is_some_and(|now| shown_colors(d, now) == *colors)).map(|(_, _, hidden)| *hidden), + ); Ok((changed, edit, added, map.converted.get(), not_changed)) })?; let mut out = json!({ "changed": changed, "swatches": added }); diff --git a/crates/engine/src/tests/recolor.rs b/crates/engine/src/tests/recolor.rs index b9b4377e1..8e9c8da46 100644 --- a/crates/engine/src/tests/recolor.rs +++ b/crates/engine/src/tests/recolor.rs @@ -608,6 +608,85 @@ fn recolor_of_everything_counts_the_locked_and_hidden_objects_it_left_out() { } } +#[test] +fn recolor_of_everything_counts_locked_art_that_only_loses_its_group_swatch_link() { + let mut s = session(); + run(&mut s, "swatch.newGroup", json!({"name": "Brand", "colors": ["#2878f0", "#ff0000"]})); + let names: Vec = + s.doc().unwrap().doc.swatch_groups.iter().find(|g| g.name == "Brand").unwrap().swatches.iter().map(|w| w.name.clone()).collect(); + for name in &names { + run(&mut s, "swatch.edit", json!({"name": name, "global": true})); + } + let open = rect(&mut s, json!("#000000")); + let locked = rect(&mut s, json!("#000000")); + run(&mut s, "paint.setFill", json!({"ids": [open.0, locked.0], "swatch": names[1]})); + set_props(&mut s, &[locked], json!({"locked": true})); + select_all(&mut s); + // The group is rewritten with one color, so its second swatch goes and the locked art only + // loses its link to it. + let r = run(&mut s, "recolor.apply", json!({"map": {"#ff0000": "#00ff00"}, "group": "Brand", "groupColors": ["#123456"]})); + assert_eq!(fill(&s, open).to_hex(), "#00ff00"); + assert_eq!(fill(&s, locked).to_hex(), "#ff0000", "the locked art keeps its color"); + assert_eq!(r["notChanged"], json!({"locked": 1, "hidden": 0}), "{r}"); +} + +#[test] +fn recolor_of_everything_does_not_count_locked_art_whose_pattern_the_group_rewrite_recolors() { + let mut s = session(); + run(&mut s, "swatch.newGroup", json!({"name": "Brand", "colors": ["#2878f0", "#ff0000"]})); + let names: Vec = + s.doc().unwrap().doc.swatch_groups.iter().find(|g| g.name == "Brand").unwrap().swatches.iter().map(|w| w.name.clone()).collect(); + for name in &names { + run(&mut s, "swatch.edit", json!({"name": name, "global": true})); + } + // "Dots": a tile linked to the group's second swatch. + let tile = rect(&mut s, json!("#000000")); + run(&mut s, "paint.setFill", json!({"ids": [tile.0], "swatch": names[1]})); + run(&mut s, "select.set", json!({"ids": [tile.0]})); + run(&mut s, "object.pattern.make", json!({"name": "Dots", "width": 20, "height": 20})); + run(&mut s, "object.pattern.done", json!({})); + let tile_hex = |s: &Session| s.doc().unwrap().doc.pattern("Dots").unwrap().art[0].appearance.fill_paint().color().unwrap().to_hex(); + assert_eq!(tile_hex(&s), "#ff0000"); + let locked = rect(&mut s, json!("#000000")); + run(&mut s, "paint.setFill", json!({"ids": [locked.0], "swatch": "Dots"})); + set_props(&mut s, &[locked], json!({"locked": true})); + let open = rect(&mut s, json!("#000000")); + run(&mut s, "paint.setFill", json!({"ids": [open.0], "swatch": names[1]})); + select_all(&mut s); + // The group's second swatch is rewritten with the row's new color, and the tile follows it. + let r = run(&mut s, "recolor.apply", json!({"map": {"#ff0000": "#00ff00"}, "group": "Brand", "groupColors": ["#2878f0", "#00ff00"]})); + assert_eq!(tile_hex(&s), "#00ff00", "{r}"); + assert_eq!(r["notChanged"], json!({"locked": 0, "hidden": 0}), "{r}"); +} + +#[test] +fn recolor_of_everything_does_not_count_locked_art_that_would_only_take_a_swatch_link() { + let mut s = session(); + run(&mut s, "swatch.new", json!({"name": "Red", "color": "#ff0000", "global": true})); + // "Dots": an unlinked red tile. + let tile = rect(&mut s, json!("#ff0000")); + run(&mut s, "select.set", json!({"ids": [tile.0]})); + run(&mut s, "object.pattern.make", json!({"name": "Dots", "width": 20, "height": 20})); + run(&mut s, "object.pattern.done", json!({})); + let open = rect(&mut s, json!("#ff0000")); + let locked = rect(&mut s, json!("#ff0000")); + let dots = rect(&mut s, json!("#000000")); + run(&mut s, "paint.setFill", json!({"ids": [dots.0], "swatch": "Dots"})); + set_props(&mut s, &[locked, dots], json!({"locked": true})); + select_all(&mut s); + // The row keeps the color and snaps it to the document's global swatch of the same color, so + // the open art only takes the link. + let r = run(&mut s, "recolor.apply", json!({"map": {"#ff0000": "#ff0000"}, "limitTo": "document"})); + let link = |s: &Session, id: NodeId| match s.doc().unwrap().doc.node(id).unwrap().appearance.fill_paint() { + vectorcraft_color::Paint::Solid { swatch, .. } => swatch, + p => panic!("{p:?}"), + }; + assert!(link(&s, open).is_some(), "{r}"); + assert_eq!(fill(&s, locked).to_hex(), "#ff0000"); + assert_eq!(r["notChanged"], json!({"locked": 0, "hidden": 0}), "{r}"); + assert!(r.get("warning").is_none(), "{r}"); +} + #[test] fn locked_and_hidden_members_of_a_selected_group_are_recolored_and_not_counted() { let mut s = session();