Graphs: swatch edits reach the kept black legend fill of graphs that draw it

Only a pie with Legends in Wedges picks its legend labels' fill, white on
dark wedges. Paint visits and swatch mappings now leave its default black
legend fill out only for such a pie; other graphs draw the kept fill, so
it is visited and mapped like the other label fills. A color mode change
still leaves the default black legend fill as the default, so turning on
Legends in Wedges later keeps the white labels on dark wedges.

crates/doc/src/graph.rs

@@ -548,6 +548,14 @@ impl GraphSpec {
         self.series_kinds.get(s).copied().flatten().filter(|k| k.combines_with(self.kind)).unwrap_or(self.kind)
     }
 
+    /// Does the graph pick the fill of its legend labels itself, white on a dark wedge and black on the others: a pie
+    /// with Legends in Wedges whose legend labels keep no fill of their own ([`LabelStyles::legend_fill_is_own`])? Only
+    /// then is a black legend fill it keeps the default, which edits that map the kept paints leave as it is; other
+    /// graphs draw the kept fill as it is.
+    pub fn picks_legend_fill(&self) -> bool {
+        self.kind == GraphKind::Pie && self.legend && self.pie_legend_in_wedges && !self.label_styles.legend_fill_is_own()
+    }
+
     /// The data with blank cells as `None`, every row as long as the longest (at most [`MAX_GRAPH_CATEGORIES`] rows
     /// of [`MAX_GRAPH_SERIES`] cells).
     pub fn cells(&self) -> Vec<Vec<Option<f64>>> {

crates/doc/src/swatches.rs

@@ -406,12 +406,13 @@ fn graph_paints_mut(g: &mut crate::graph::GraphSpec) -> impl Iterator<Item = &mu
         .chain(rows.flat_map(|r| [r.paint.fill.as_mut(), r.paint.stroke.as_mut()]).flatten())
 }
 
-/// The place in [`graph_paints`] of the legend fill graph `n` keeps when it is the default black and not the legend
-/// labels' own ([`crate::graph::LabelStyles::legend_fill_is_own`]).
+/// The place in [`graph_paints`] of the legend fill graph `n` keeps when it is the default black of a graph that picks
+/// its legend labels' fill ([`crate::graph::GraphSpec::picks_legend_fill`]). Other graphs draw the kept fill as it is,
+/// so it is visited and mapped.
 fn default_legend_fill(n: &Node) -> Option<usize> {
     let g = n.extras().graph.as_deref()?;
     let l = &g.label_styles;
-    if l.legend.is_none() || l.legend_fill_is_own() {
+    if l.legend.is_none() || !g.picks_legend_fill() {
         return None;
     }
     let series: usize = g.series_paints.iter().map(|s| usize::from(s.fill.is_some()) + usize::from(s.stroke.is_some())).sum();
@@ -764,26 +765,35 @@ mod tests {
 
     #[test]
     fn the_default_black_legend_fill_is_left_as_it_is() {
-        let (mut d, a, _) = doc();
-        let black = || crate::CharStyle { fill: Paint::solid(Color::BLACK), size: 11.0, ..Default::default() };
-        let spec = crate::graph::GraphSpec {
-            series_paints: vec![crate::SeriesPaint { fill: Some(Paint::solid(Color::BLACK)), ..Default::default() }],
-            label_styles: crate::graph::LabelStyles { values: Some(black()), legend: Some(black()), ..Default::default() },
-            ..Default::default()
-        };
-        let blacks = |d: &Document| {
-            let mut seen = 0;
-            d.visit_paints(&mut |p| seen += usize::from(*p == Paint::solid(Color::BLACK)));
-            seen
-        };
-        let before = blacks(&d);
-        d.node_mut(a).unwrap().extras_mut().graph = Some(Box::new(spec));
-        assert_eq!(blacks(&d) - before, 2, "the series fill and the value labels' fill are visited");
-        let mut link_black = |c: &mut Color, s: &mut Option<String>, _: &mut f32| *c == Color::BLACK && s.replace("Black".into()).is_none();
-        assert_eq!(d.map_solid_paints_in(&[a], &mut link_black), 2, "the series fill and the value labels' fill");
-        let styles = d.node(a).unwrap().extras().graph.clone().unwrap().label_styles;
-        assert_eq!(styles.legend.unwrap().fill, Paint::solid(Color::BLACK), "not linked");
-        assert_eq!(styles.values.unwrap().fill, linked(Color::BLACK, "Black"));
+        use crate::graph::GraphKind;
+        // Only a pie with Legends in Wedges picks its legend labels' fill; the other graphs draw the kept black.
+        for (kind, in_wedges, picks) in [(GraphKind::Pie, true, true), (GraphKind::Pie, false, false), (GraphKind::Column, true, false)] {
+            let (mut d, a, _) = doc();
+            let black = || crate::CharStyle { fill: Paint::solid(Color::BLACK), size: 11.0, ..Default::default() };
+            let spec = crate::graph::GraphSpec {
+                kind,
+                legend: true,
+                pie_legend_in_wedges: in_wedges,
+                series_paints: vec![crate::SeriesPaint { fill: Some(Paint::solid(Color::BLACK)), ..Default::default() }],
+                label_styles: crate::graph::LabelStyles { values: Some(black()), legend: Some(black()), ..Default::default() },
+                ..Default::default()
+            };
+            let blacks = |d: &Document| {
+                let mut seen = 0;
+                d.visit_paints(&mut |p| seen += usize::from(*p == Paint::solid(Color::BLACK)));
+                seen
+            };
+            let before = blacks(&d);
+            d.node_mut(a).unwrap().extras_mut().graph = Some(Box::new(spec));
+            let want = if picks { 2 } else { 3 };
+            assert_eq!(blacks(&d) - before, want, "{kind:?}, in wedges {in_wedges}: the series, value labels' and legend fills visited");
+            let mut link_black = |c: &mut Color, s: &mut Option<String>, _: &mut f32| *c == Color::BLACK && s.replace("Black".into()).is_none();
+            assert_eq!(d.map_solid_paints_in(&[a], &mut link_black), want, "{kind:?}, in wedges {in_wedges}");
+            let styles = d.node(a).unwrap().extras().graph.clone().unwrap().label_styles;
+            let legend = if picks { Paint::solid(Color::BLACK) } else { linked(Color::BLACK, "Black") };
+            assert_eq!(styles.legend.unwrap().fill, legend, "{kind:?}, in wedges {in_wedges}: linked unless the pie picks the fill");
+            assert_eq!(styles.values.unwrap().fill, linked(Color::BLACK, "Black"));
+        }
     }
 
     #[test]

crates/engine/src/cmd/graph/series.rs

@@ -886,9 +886,9 @@ fn run_paint_change(was: &Node, now: &Node, fill: bool) -> Option<(CharStyle, Ve
 }
 
 /// Does graph `g` pick the fill of its legend labels itself, white on a dark wedge and black on the others: a pie with
-/// Legends in Wedges whose legend labels keep no fill of their own ([`vectorcraft_doc::LabelStyles::legend_fill_is_own`])?
+/// Legends in Wedges whose legend labels keep no fill of their own ([`GraphSpec::picks_legend_fill`])?
 pub(super) fn picks_legend_fill(g: &GraphSpec) -> bool {
-    g.kind == GraphKind::Pie && g.legend && g.pie_legend_in_wedges && !g.label_styles.legend_fill_is_own()
+    g.picks_legend_fill()
 }
 
 /// The graph labels among `roots` and inside them, for an edit that reaches graph labels without naming them.

crates/engine/src/cmd/graph/tests.rs

@@ -6409,6 +6409,27 @@ fn a_color_mode_change_keeps_the_default_wedge_legend_fill() {
     assert!(spec(&s, id).label_styles.legend_fill_set, "the converted red is still the labels' own");
 }
 
+/// The default black legend fill of a graph that keeps its legend beside the marks stays the default through a color
+/// mode change, so turning on Legends in Wedges later still draws the labels on dark wedges white.
+#[test]
+fn a_color_mode_change_keeps_the_default_legend_fill_of_other_graphs() {
+    // A pie made here keeps its legend beside the pies.
+    for kind in ["column", "pie"] {
+        let mut s = Session::new();
+        s.execute("file.new", &json!({"width": 800, "height": 800})).unwrap();
+        let id = make(&mut s, kind, ",a,b\nP1,1,2");
+        s.execute("select.set", &json!({"ids": [id.0]})).unwrap();
+        s.execute("text.setStyle", &json!({"size": 11})).unwrap();
+        let black = Paint::solid(vectorcraft_color::Color::BLACK);
+        let before = spec(&s, id).label_styles;
+        assert_eq!(before.legend.as_ref().map(|st| st.fill.clone()), Some(black.clone()), "{kind}: {before:?}");
+        s.execute("file.documentColorMode", &json!({"mode": "cmyk"})).unwrap();
+        let after = spec(&s, id).label_styles;
+        assert_eq!(after.legend.as_ref().map(|st| st.fill.clone()), Some(black), "{kind}: {after:?}");
+        assert!(!after.legend_fill_set && !after.legend_fill_is_own(), "{kind}: still the default: {after:?}");
+    }
+}
+
 #[test]
 fn labels_are_told_apart_by_where_they_come_not_by_their_text() {
     let mut s = Session::new();