From 1c8992fdbe2680e64edc90571771e22346a73d0c Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sun, 11 Oct 2026 13:52:32 +0200 Subject: [PATCH] Charts: a missing varyColors varies single-series bar colours by point (#143) --- crates/pptx/src/read/chart.rs | 29 ++++++++++++++++++++++++++--- crates/render/src/chart.rs | 14 +++++++++++++- crates/render/src/tests.rs | 17 +++++++++++++++++ 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/crates/pptx/src/read/chart.rs b/crates/pptx/src/read/chart.rs index a9a960a3..e8239ce2 100644 --- a/crates/pptx/src/read/chart.rs +++ b/crates/pptx/src/read/chart.rs @@ -150,9 +150,8 @@ pub(crate) fn chart_from(root: &El) -> Option { "surfaceChart" | "surface3DChart" => ChartType::Surface, _ => continue, }; - if let Some(v) = g.child_val("varyColors") { - c.vary_colors = v == "1" || v == "true"; - } + // A missing means "vary by point" (the schema default is true), as PowerPoint draws it. + c.vary_colors = g.child_val("varyColors").is_none_or(|v| v == "1" || v == "true"); if let Some(v) = g.child_val("gapWidth").and_then(|v| v.parse::().ok()) { c.gap_width = (v / 100.0).clamp(0.0, 5.0); } @@ -225,3 +224,27 @@ pub(crate) fn chart_from(root: &El) -> Option { } Some(c) } + +#[cfg(test)] +mod tests { + use super::read_chart_xml; + + const NS: &str = "http://schemas.openxmlformats.org/drawingml/2006/chart"; + + fn xml(vary: &str) -> String { + let ser = r#""#; + let bar = format!(r#"{vary}{ser}"#); + format!(r#"{bar}"#) + } + + #[test] + fn missing_vary_colors_means_vary_by_point() { + assert!(read_chart_xml(xml("").as_bytes()).is_some_and(|c| c.vary_colors)); + } + + #[test] + fn explicit_vary_colors_is_respected() { + assert!(read_chart_xml(xml(r#""#).as_bytes()).is_some_and(|c| !c.vary_colors)); + assert!(read_chart_xml(xml(r#""#).as_bytes()).is_some_and(|c| c.vary_colors)); + } +} diff --git a/crates/render/src/chart.rs b/crates/render/src/chart.rs index a29b2023..ced1f77a 100644 --- a/crates/render/src/chart.rs +++ b/crates/render/src/chart.rs @@ -36,6 +36,18 @@ fn series_fill(ctx: &Ctx, c: &Chart, i: usize, s: Option<&deckcraft_model::chart } } +/// Fill of one bar: a `` fill wins, and a single-series chart that varies colours by point uses one accent per point +/// (not waterfalls, which are never read from a ``). +fn bar_fill(ctx: &Ctx, c: &Chart, si: usize, pi: usize, s: &deckcraft_model::chart::Series) -> Rgba { + if let Some((_, Fill::Solid { color: cc })) = s.point_fills.iter().find(|(k, _)| *k == pi) { + return ctx.color(cc, None); + } + if c.vary_colors && c.series.len() == 1 && s.fill.is_none() && c.kind != ChartType::Waterfall { + return series_color(ctx, c, pi); + } + series_fill(ctx, c, si, Some(s)) +} + fn label(ctx: &mut RenderContext, rctx: &Ctx, m: Affine, text: &str, r: Rect, size: f64, align: Align, anchor: Anchor, col: Rgba, bold: bool) { let mut body = TextBody::from_text(text); body.body.inset_l = Some(0.0); @@ -291,7 +303,7 @@ pub fn draw(ctx: &mut RenderContext, rctx: &Ctx, c: &Chart, m: Affine, w: f64, h Rect::new(plot.x0 + start, to_v(b), plot.x0 + start + bar, to_v(a)) }; ctx.set_transform(m); - ctx.set_paint(color(series_fill(rctx, c, si, Some(s)), 1.0)); + ctx.set_paint(color(bar_fill(rctx, c, si, i, s), 1.0)); ctx.fill_rect(&r.abs()); if c.data_labels { let t = fmt_num(s.values.get(i).copied().flatten().unwrap_or(0.0)); diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index ccb4472f..7e60e75d 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -523,3 +523,20 @@ fn hyperlinks_draw_in_the_hyperlink_colour_underlined() { let own = px(&run_deck(RunProps { link, fill: red.clone(), ..Default::default() })); assert_eq!(diff(&own, &px(&run_deck(RunProps { fill: red, underline: Some("sng".into()), ..Default::default() }))), 0); } + +#[test] +fn single_series_column_chart_varies_colours_by_point() { + let render = |vary: bool| { + let mut c = Chart::sample(ChartType::Column); + c.series.truncate(1); + c.vary_colors = vary; + let sh = Shape { + id: ShapeId(800), + xfrm: Some(Xfrm::new(50.0, 50.0, 400.0, 300.0)), + kind: ShapeKind::Chart(Box::new(c)), + ..Default::default() + }; + render_slide(&deck_with(vec![sh]), 0, &RenderOpts { scale: 1.0, ..Default::default() }).pixels + }; + assert_ne!(render(true), render(false)); +}