From 904414aa7d09077d8123c0c507fd431d8a602d5a Mon Sep 17 00:00:00 2001 From: Gert Pellin Date: Tue, 13 Oct 2026 21:59:06 +0200 Subject: [PATCH] Global swatch edits reach opacity-mask art (#1735) A mask's art lives outside the layer tree (in the masked object's extras), and the walks over linked colours skipped it: editing a global swatch recoloured everything but the mask, which kept showing (or hiding) as before, and a swatch used only in a mask counted as unused. The colour walks now go into opacity masks: map_solid_paints (swatch edits, renames, deletes, Merge and the like) and visit_paints (unused swatches). --- crates/doc/src/swatches.rs | 38 +++++++++++++++++++++++------ crates/engine/src/tests_swatches.rs | 30 +++++++++++++++++++++++ 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/crates/doc/src/swatches.rs b/crates/doc/src/swatches.rs index bda2f47a..689124b0 100644 --- a/crates/doc/src/swatches.rs +++ b/crates/doc/src/swatches.rs @@ -109,10 +109,11 @@ impl Document { } /// Visit every colour that can link to a swatch — solid paints and gradient stops of fills, - /// strokes and text runs — in the art, symbol definitions, pattern tiles, graphic styles and the - /// swatches themselves (tint swatches, gradient swatches' stops). `f(colour, link, tint)` may - /// change them and returns true when it did ([`Paint::map_links`]). Only nodes holding a changed - /// paint (and the paths to them) are copied. Returns the number of changed paints. + /// strokes and text runs — in the art (opacity masks included), symbol definitions, pattern + /// tiles, graphic styles and the swatches themselves (tint swatches, gradient swatches' stops). + /// `f(colour, link, tint)` may change them and returns true when it did + /// ([`Paint::map_links`]). Only nodes holding a changed paint (and the paths to them) are + /// copied. Returns the number of changed paints. pub fn map_solid_paints(&mut self, f: &mut LinkFn) -> usize { let mut n = map_trees(&mut self.layers, f); for s in &mut self.symbols { @@ -152,11 +153,12 @@ impl Document { } impl Document { - /// Visit every paint held by the art, symbol definitions, pattern tiles and graphic styles - /// (fills, strokes and text runs), and the colour of every gradient-mesh point as a solid paint. + /// Visit every paint held by the art (opacity masks included), symbol definitions, pattern + /// tiles and graphic styles (fills, strokes and text runs), and the colour of every + /// gradient-mesh point as a solid paint. pub fn visit_paints(&self, f: &mut dyn FnMut(&Paint)) { let mut tree = |n: &Node| { - n.walk(&mut |m| { + walk_with_masks(n, &mut |m| { node_paints(m).for_each(&mut *f); if let NodeKind::Mesh(mesh) = &m.kind { mesh.points.iter().for_each(|p| f(&Paint::solid(p.color))); @@ -280,9 +282,31 @@ fn map_node(n: &Node, f: &mut LinkFn) -> Option<(Node, usize)> { } } } + // Opacity mask art lives outside the tree, but its colours are the object's too (#1735). + if let Some(mask) = n.extras().mask.as_deref() + && let Some((new, k)) = map_node(&mask.art, f) + { + let m = out.get_or_insert_with(|| n.clone()); + if let Some(slot) = m.extras_if_any().and_then(|e| e.mask.as_deref_mut()) { + slot.art = Arc::new(new); + count += k; + } + } out.map(|m| (m, count)) } +/// Visit `n` and its subtree, the art of opacity masks included (mask art after the masked +/// object, before its children). +fn walk_with_masks<'a>(n: &'a Node, f: &mut dyn FnMut(&'a Node)) { + f(n); + if let Some(mask) = n.extras().mask.as_deref() { + walk_with_masks(&mask.art, f); + } + for c in n.children().into_iter().flatten() { + walk_with_masks(c, f); + } +} + fn map_tree(n: &mut Arc, f: &mut LinkFn) -> usize { match map_node(n, f) { Some((new, count)) => { diff --git a/crates/engine/src/tests_swatches.rs b/crates/engine/src/tests_swatches.rs index d22bf5eb..27b949aa 100644 --- a/crates/engine/src/tests_swatches.rs +++ b/crates/engine/src/tests_swatches.rs @@ -353,3 +353,33 @@ fn a_group_from_a_mesh_holds_its_point_colours() { let names = r["swatches"].as_array().unwrap(); assert!(names.contains(&json!("R=255 G=0 B=0")) && names.contains(&json!("R=255 G=255 B=255")), "{r}"); } + +/// #1735: a global swatch edit reaches opacity-mask art: the mask's colour (and with it what the +/// mask shows) follows the swatch, and a swatch used only in a mask isn't "unused". +#[test] +fn global_swatch_edits_reach_opacity_mask_art() { + let mut s = session(); + run(&mut s, "swatch.new", json!({"name": "MaskTone", "color": "#ffffff", "global": true})); + let obj = rect(&mut s); + run(&mut s, "paint.setFill", json!({"color": "#ff0000", "ids": [obj.0]})); + let m = rect(&mut s); + run(&mut s, "paint.setFill", json!({"swatch": "MaskTone", "ids": [m.0]})); + run(&mut s, "select.set", json!({"ids": [obj.0, m.0]})); + run(&mut s, "transparency.makeOpacityMask", json!({"clip": true, "invert": false})); + let mask_fill = |s: &Session| { + let n = doc(s).node(obj).unwrap(); + n.extras().mask.as_deref().unwrap().art.appearance.fill_paint() + }; + assert_eq!(mask_fill(&s).color(), Some(Color::rgb(1.0, 1.0, 1.0))); + assert!(!run(&mut s, "swatch.unused", json!({}))["names"].as_array().unwrap().iter().any(|n| n == "MaskTone"), "used by the mask"); + + let r = run(&mut s, "swatch.edit", json!({"name": "MaskTone", "color": "#000000"})); + assert_eq!(r["relinked"], 1, "{r}"); + let fill = mask_fill(&s); + assert_eq!(fill.color(), Some(Color::rgb(0.0, 0.0, 0.0))); + assert!(matches!(&fill, Paint::Solid { swatch: Some(n), .. } if n == "MaskTone"), "{fill:?}"); + + // Undo brings the mask's colour back with the swatch. + run(&mut s, "edit.undo", json!({})); + assert_eq!(mask_fill(&s).color(), Some(Color::rgb(1.0, 1.0, 1.0))); +}