From a90ae1e9a975087dea5471240c54f57c1fbf94cb Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 29 Oct 2026 12:49:10 -0700 Subject: [PATCH] Recolor: count left-out art by the colors of nested pattern tiles too A left-out object whose pattern's tile uses another pattern showed the inner tile's colors without them being compared, so a group rewrite that recolored the inner tile still counted the object as left unchanged. The comparison now follows the patterns tiles use, each once. --- crates/engine/src/cmd/recolor.rs | 27 ++++++++++++++++-------- crates/engine/src/tests/recolor.rs | 33 ++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/cmd/recolor.rs b/crates/engine/src/cmd/recolor.rs index 0d07e2a2c6..a3ce988bbf 100644 --- a/crates/engine/src/cmd/recolor.rs +++ b/crates/engine/src/cmd/recolor.rs @@ -119,24 +119,35 @@ fn used_colors(d: &Document, ids: &[NodeId], only: Option<&BTreeSet>) -> } /// 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 { +/// patterns it uses as fills or strokes, and of the patterns those tiles use (each pattern once). +fn shown_colors<'a>(d: &'a Document, n: &'a Node) -> Vec { let mut colors = vec![]; node_colors(n, &mut |c, _| colors.push(*c)); let mut patterns: Vec<&str> = vec![]; + pattern_names(n, &mut patterns); + let mut next = 0; + while let Some(&name) = patterns.get(next) { + next += 1; + for a in d.pattern(name).map(|def| def.art.as_slice()).unwrap_or_default() { + node_colors(a, &mut |c, _| colors.push(*c)); + pattern_names(a, &mut patterns); + } + } + colors +} + +/// Adds the names of the patterns `n` and the objects inside it use as fills or strokes to `names`, +/// leaving out names already there. +fn pattern_names<'a>(n: &'a Node, names: &mut Vec<&'a str>) { n.walk(&mut |m| { for p in node_paints(m) { if let Paint::Pattern { pattern, .. } = p - && !patterns.contains(&pattern.as_str()) + && !names.contains(&pattern.as_str()) { - patterns.push(pattern); + names.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` diff --git a/crates/engine/src/tests/recolor.rs b/crates/engine/src/tests/recolor.rs index 17a30417e1..72f8f2f648 100644 --- a/crates/engine/src/tests/recolor.rs +++ b/crates/engine/src/tests/recolor.rs @@ -659,6 +659,39 @@ fn recolor_of_everything_does_not_count_locked_art_whose_pattern_the_group_rewri assert_eq!(r["notChanged"], json!({"locked": 0, "hidden": 0}), "{r}"); } +#[test] +fn recolor_of_everything_does_not_count_locked_art_whose_nested_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; "Rings": a tile filled with "Dots". + 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 outer = rect(&mut s, json!("#000000")); + run(&mut s, "paint.setFill", json!({"ids": [outer.0], "swatch": "Dots"})); + run(&mut s, "select.set", json!({"ids": [outer.0]})); + run(&mut s, "object.pattern.make", json!({"name": "Rings", "width": 40, "height": 40})); + 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(); + let locked = rect(&mut s, json!("#000000")); + run(&mut s, "paint.setFill", json!({"ids": [locked.0], "swatch": "Rings"})); + 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 rewrite recolors the inner tile, which the locked art shows through "Rings". + 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();