Merge PR #956: Charts: a series' own data-label setting decides whether its labels show (#868)

Conflicts: crates/model/src/chart.rs (kept series_data_labels and main's waterfall_value_range split; value_range stays where main has it)

ROADMAP.md

@@ -159,6 +159,7 @@ app's JSON control channel (`deckcraft --control PORT`).
 | 2026-10-17 | While a placeholder's text is being edited, a click on a shape in front of it selects that shape instead of moving the caret, as in PowerPoint (#931) |
 | 2026-10-16 | Symbol-font characters: a run's U+F0xx symbol code (from `<a:sym typeface="Symbol"/>`) now draws from the installed Symbol face even when it has no private-use mapping — Apple's Symbol on macOS maps the glyphs to real Unicode, so the code is remapped to that character (the Adobe Symbol encoding) instead of falling to a look-alike. Bullets and metafile text take the same remap |
 | 2026-10-15 | Native saves keep an imported chart's embedded workbook, drawings and relationships; headless PPTX → edit → native save/reopen → PPTX tests verify the payloads and stale-workbook rejection |
+| 2026-10-18 | Charts: a series' own data-label setting is honoured — numbers PowerPoint shows on a graph no longer disappear, and a bare `c:showVal` means on (#868) |
 | 2026-10-15 | PPTX trigger sequences keep separate On Click steps, including paragraph builds; synthetic import/edit/save/reopen and headless show regressions |
 | 2026-10-16 | Export: PNG with a transparent background — the slide background is left out, only the slide's content is drawn (Export… dialog, File › Export, web download, `file.export`/`file.render` `transparentBackground`, `deckcraft-cli render --transparent`) (#784) |
 | 2026-10-16 | Arrange ▸ Align: Align to Slide and Align Selected Objects, as in PowerPoint. One object aligns to the slide and several to each other until the other choice is ticked; the choice holds for that selection and also drives Distribute |

crates/pptx/src/read/chart.rs

@@ -214,7 +214,11 @@ fn chart_within(root: &El, mut budget: usize) -> Option<Chart> {
         if let Some(v) = g.child_val("holeSize").and_then(|v| v.parse::<f64>().ok()) {
             c.hole_size = (v / 100.0).clamp(0.0, 0.9);
         }
-        if g.path(&["dLbls", "showVal"]).and_then(|v| v.bool("val")) == Some(true) {
+        // `<c:showVal/>` without a val attribute means true, as in the schema's default.
+        // The setting belongs to this plot group: a group that asks for no labels keeps them
+        // off, even when another group of a combo chart asks for them (#868).
+        let group_labels = g.path(&["dLbls", "showVal"]).map(|v| v.bool("val").unwrap_or(true));
+        if group_labels == Some(true) {
             c.data_labels = true;
         }
         kinds.push(kind);
@@ -231,8 +235,12 @@ fn chart_within(root: &El, mut budget: usize) -> Option<Chart> {
                 secondary_axis: secondary,
                 ..Default::default()
             };
-            if let Some(d) = s.child("dLbls").and_then(|d| d.child("showVal")).and_then(|v| v.bool("val")) {
+            // `<c:showVal/>` without a val attribute means true, as in the schema's default.
+            if let Some(d) = s.child("dLbls").and_then(|d| d.child("showVal")).map(|v| v.bool("val").unwrap_or(true)) {
                 ser.data_labels = Some(d);
+            } else if let Some(false) = group_labels {
+                // The group asks for no labels and the series does not say otherwise.
+                ser.data_labels = Some(false);
             }
             for t in s.children_named("trendline").take(16) {
                 if let Some(t) = trendline(t) {

crates/pptx/src/write/chart.rs

@@ -867,7 +867,7 @@ pub fn build_chartex(c: &Chart) -> Vec<u8> {
     for (i, s) in c.series.iter().enumerate().take(if one { 1 } else { usize::MAX }) {
         w.open("cx:series", A::new().a("layoutId", layout).a("uniqueId", format!("{{DC00000{i}-0000-4000-8000-000000000000}}")));
         cx_text(&mut w, &s.name);
-        if s.data_labels.unwrap_or(c.data_labels) {
+        if c.series_data_labels(s) {
             w.open("cx:dataLabels", A::new().a("pos", if c.kind == ChartType::Funnel { "ctr" } else { "outEnd" }));
             w.empty("cx:visibility", A::new().a("seriesName", "0").a("categoryName", "0").a("value", "1"));
             w.close("cx:dataLabels");

crates/pptx/tests/chart_data_labels.rs

@@ -0,0 +1,154 @@
+//! Series-level data labels: `c:dLbls/c:showVal` on a series overrides the chart-wide setting,
+//! and a bare `<c:showVal/>` (no `val` attribute) means true, as the schema's default (#868).
+
+use std::io::{Cursor, Read, Write};
+use std::sync::Arc;
+
+use deckcraft_geom::Xfrm;
+use deckcraft_model::chart::{Chart, ChartType};
+use deckcraft_model::{LayoutType, Presentation, Shape, ShapeId, ShapeKind, defaults};
+
+/// A one-slide deck whose bar chart shows no labels anywhere.
+fn bar_chart_pptx() -> Vec<u8> {
+    let mut p = defaults::new_presentation(None);
+    p.slides.clear();
+    let layout = defaults::layout_of_kind(&p, LayoutType::Blank).unwrap_or_default();
+    let mut slide = defaults::new_slide(&mut p, layout).expect("slide");
+    let mut shape =
+        Shape { id: ShapeId(p.alloc_id().expect("id")), name: "Bars".into(), xfrm: Some(Xfrm::new(40.0, 40.0, 400.0, 300.0)), ..Default::default() };
+    let mut chart = Chart::sample(ChartType::Column);
+    chart.data_labels = false;
+    for s in &mut chart.series {
+        s.data_labels = None;
+    }
+    shape.kind = ShapeKind::Chart(Box::new(chart));
+    slide.shapes.push(shape);
+    p.slides.push(Arc::new(slide));
+    deckcraft_pptx::export(&p).expect("export")
+}
+
+/// Rewrite the chart part's XML through `f` and re-import the deck.
+fn reimport(bytes: &[u8], f: impl Fn(&str) -> String) -> Presentation {
+    let mut zip = zip::ZipArchive::new(Cursor::new(bytes)).expect("zip");
+    let mut out = zip::ZipWriter::new(Cursor::new(Vec::new()));
+    let opts = zip::write::SimpleFileOptions::default();
+    for i in 0..zip.len() {
+        let mut entry = zip.by_index(i).expect("entry");
+        let name = entry.name().to_string();
+        let mut data = vec![];
+        entry.read_to_end(&mut data).expect("read");
+        if name.starts_with("ppt/charts/chart") && name.ends_with(".xml") && !name.contains("_rels") {
+            let xml = String::from_utf8(data).expect("chart xml");
+            data = f(&xml).into_bytes();
+        }
+        out.start_file(&name, opts).expect("start");
+        out.write_all(&data).expect("write");
+    }
+    let patched = out.finish().expect("finish").into_inner();
+    deckcraft_pptx::import(&patched).expect("import")
+}
+
+fn chart_of(p: &Presentation) -> &Chart {
+    let s = p.slides.first().expect("slide");
+    for sh in &s.shapes {
+        if let ShapeKind::Chart(c) = &sh.kind {
+            return c;
+        }
+    }
+    panic!("no chart in the deck");
+}
+
+/// `<c:showVal/>` on the first series: that series shows labels although the chart-wide
+/// setting is off; the other series stay quiet.
+#[test]
+fn bare_series_showval_shows_labels() {
+    let p = reimport(&bar_chart_pptx(), |xml| xml.replacen("<c:cat>", r#"<c:dLbls><c:showVal/></c:dLbls><c:cat>"#, 1));
+    let c = chart_of(&p);
+    assert!(!c.data_labels, "the chart-wide setting stays off");
+    assert_eq!(c.series[0].data_labels, Some(true), "a bare showVal means true");
+    assert!(c.series_data_labels(&c.series[0]), "the first series' labels show");
+    for s in c.series.iter().skip(1) {
+        assert_eq!(s.data_labels, None, "other series keep the chart's setting");
+        assert!(!c.series_data_labels(s), "and stay quiet");
+    }
+}
+
+/// `val="0"` on the series hides its labels although the chart-wide setting is on.
+#[test]
+fn series_showval_false_hides_labels() {
+    let p = reimport(&bar_chart_pptx(), |xml| {
+        xml.replacen("<c:cat>", r#"<c:dLbls><c:showVal val="0"/></c:dLbls><c:cat>"#, 1).replacen(
+            "<c:gapWidth",
+            r#"<c:dLbls><c:showVal/></c:dLbls><c:gapWidth"#,
+            1,
+        )
+    });
+    let c = chart_of(&p);
+    assert!(c.data_labels, "the bare chart-wide showVal turns labels on");
+    assert_eq!(c.series[0].data_labels, Some(false), "the explicit series false is read");
+    assert!(!c.series_data_labels(&c.series[0]), "and hides that series' labels");
+    assert!(c.series_data_labels(&c.series[1]), "the next series takes the chart-wide on");
+}
+
+/// The series' label setting survives a save and load.
+#[test]
+fn series_label_settings_round_trip() {
+    let mut p = defaults::new_presentation(None);
+    p.slides.clear();
+    let layout = defaults::layout_of_kind(&p, LayoutType::Blank).unwrap_or_default();
+    let mut slide = defaults::new_slide(&mut p, layout).expect("slide");
+    let mut chart = Chart::sample(ChartType::Column);
+    chart.data_labels = true;
+    chart.series[0].data_labels = Some(false);
+    chart.series[1].data_labels = Some(true);
+    let mut shape =
+        Shape { id: ShapeId(p.alloc_id().expect("id")), name: "Bars".into(), xfrm: Some(Xfrm::new(40.0, 40.0, 400.0, 300.0)), ..Default::default() };
+    shape.kind = ShapeKind::Chart(Box::new(chart));
+    slide.shapes.push(shape);
+    p.slides.push(Arc::new(slide));
+
+    let bytes = deckcraft_pptx::export(&p).expect("export");
+    let back = deckcraft_pptx::import(&bytes).expect("import");
+    let c = chart_of(&back);
+    assert!(c.data_labels);
+    assert_eq!(c.series[0].data_labels, Some(false), "the explicit off survives");
+    assert_eq!(c.series[1].data_labels, Some(true), "the explicit on survives");
+    assert!(!c.series_data_labels(&c.series[0]));
+    assert!(c.series_data_labels(&c.series[1]));
+}
+
+/// A one-slide deck with a combo chart: a bar group and a line group.
+fn combo_chart_pptx() -> Vec<u8> {
+    let mut p = defaults::new_presentation(None);
+    p.slides.clear();
+    let layout = defaults::layout_of_kind(&p, LayoutType::Blank).unwrap_or_default();
+    let mut slide = defaults::new_slide(&mut p, layout).expect("slide");
+    let mut chart = Chart::sample(ChartType::Combo);
+    chart.series.truncate(2);
+    chart.series[1].kind = Some(ChartType::Line);
+    let mut shape =
+        Shape { id: ShapeId(p.alloc_id().expect("id")), name: "Bars".into(), xfrm: Some(Xfrm::new(40.0, 40.0, 400.0, 300.0)), ..Default::default() };
+    shape.kind = ShapeKind::Chart(Box::new(chart));
+    slide.shapes.push(shape);
+    p.slides.push(Arc::new(slide));
+    deckcraft_pptx::export(&p).expect("export")
+}
+
+/// A combo chart's groups keep their own label setting: the group that asks for no labels
+/// keeps them off, even when the other group asks for them (#868).
+#[test]
+fn a_combo_chart_keeps_each_groups_own_label_setting() {
+    let p = reimport(&combo_chart_pptx(), |xml| {
+        // The bar group asks for no labels, the line group for labels.
+        xml.replacen("<c:gapWidth", r#"<c:dLbls><c:showVal val="0"/></c:dLbls><c:gapWidth"#, 1).replacen(
+            "<c:marker val=\"1\"/>",
+            r#"<c:dLbls><c:showVal/></c:dLbls><c:marker val="1"/>"#,
+            1,
+        )
+    });
+    let c = chart_of(&p);
+    assert!(c.data_labels, "the line group's ask turns the chart-wide setting on");
+    assert_eq!(c.series[0].data_labels, Some(false), "the bar group asks for no labels");
+    assert!(!c.series_data_labels(&c.series[0]), "so the bars stay quiet");
+    assert!(c.series_data_labels(&c.series[1]), "the line takes the chart-wide on");
+}

crates/render/src/chart.rs

@@ -377,7 +377,7 @@ fn draw_inner(ctx: &mut RenderContext, rctx: &Ctx, c: &Chart, m: Affine, w: f64,
             ctx.set_paint(color(Rgba::WHITE, 1.0));
             ctx.set_stroke(kurbo::Stroke::new(1.0));
             ctx.stroke_path(&p);
-            if c.data_labels {
+            if c.series_data_labels(s) {
                 let mid = a0 + sweep / 2.0;
                 let lp = Point::new(cen.x + r * 0.65 * mid.cos(), cen.y + r * 0.65 * mid.sin());
                 label(
@@ -724,7 +724,7 @@ fn draw_inner(ctx: &mut RenderContext, rctx: &Ctx, c: &Chart, m: Affine, w: f64,
                         err_marks.push((mid, p0, p1, bar.min(base * 0.8) * 0.5, e.end_cap));
                     }
                 }
-                if c.data_labels {
+                if c.series_data_labels(s) {
                     let t = c.data_label(s, s.values.get(i).copied().flatten().unwrap_or(0.0));
                     let lr = if horiz {
                         Rect::new(r.x1 + 2.0, r.y0, r.x1 + 40.0, r.y1)

crates/render/src/chartex.rs

@@ -24,7 +24,7 @@ pub fn draw(ctx: &mut RenderContext, rctx: &Ctx, c: &Chart, m: Affine, area: Rec
         return;
     }
     let value_label = |v: f64| c.data_label(s, v);
-    let labels = c.data_labels || c.kind == ChartType::Funnel;
+    let labels = c.series_data_labels(s) || c.kind == ChartType::Funnel;
     match c.kind {
         ChartType::Funnel => funnel(ctx, rctx, c, m, area, base, text_col, &vals, &value_label),
         ChartType::Treemap if !c.category_levels.is_empty() => tree_treemap(ctx, rctx, c, m, area, base, labels.then_some(&value_label)),
@@ -107,7 +107,7 @@ fn region_tiles(ctx: &mut RenderContext, rctx: &Ctx, c: &Chart, m: Affine, area:
             ctx.fill_rect(&t);
             let dark = v.is_some_and(|v| (v - min) / span > 0.55);
             let mut text = name.clone();
-            if c.data_labels
+            if c.series_data_labels(s)
                 && let Some(v) = v
             {
                 text = format!(

crates/render/src/radar.rs

@@ -120,7 +120,7 @@ pub fn draw(ctx: &mut RenderContext, rctx: &Ctx, c: &Chart, m: Affine, area: Rec
                 }
             }
         }
-        if c.data_labels || s.data_labels == Some(true) {
+        if c.series_data_labels(s) {
             for (i, q) in pts.iter().enumerate() {
                 if let Some(v) = s.values.get(i).copied().flatten() {
                     let t = c.data_label(s, v);