From 5cf0e24f430176fdac37dbd2a284faa7215f1dd7 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Tue, 13 Oct 2026 21:12:43 +0200 Subject: [PATCH] Charts: one combo plot-kind rule, no fixed_scale helper, ticks off the step grid (#1146) Co-Authored-By: Claude Sonnet 5.5 --- ROADMAP.md | 3 ++- crates/chart/src/axis.rs | 11 ++++------- crates/chart/src/render.rs | 20 +++++++++++--------- crates/chart/src/resolve.rs | 10 ++++++++-- crates/chart/src/tests.rs | 32 ++++++++++++++++++++++++++++++++ 5 files changed, 57 insertions(+), 19 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 070552d07..2e9336683 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: alpha** · next: beta, ~25 points of "ready for real work" and ~160–240 h away -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (rows auto-fit oversized fonts on open and whole-row/column formatting) · **Target:** Microsoft Excel (Microsoft 365) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-13 · **Change:** trivial (chart combo plot-kind rule, scale ticks) · **Target:** Microsoft Excel (Microsoft 365) GridCraft aims at full Microsoft Excel parity, and at being better in places: faster, open (XLSX is the native format), fully scriptable by agents (MCP, control channel, CLI), and on the web. @@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-13 | Charts: one combo plot-kind rule for resolving and drawing; ticks of a scale off the step grid keep their own values (#1146) | | 2026-10-12 | Rows auto-fit oversized fonts: opening a file and whole-row/column formatting grow non-custom rows to their tallest font (#833) | | 2026-10-12 | Number formats: parsing a format code is linear in its length (long runs of `[`, `,` or `/` no longer take quadratic time), and an unclosed `[` is a literal when splitting sections as well as when tokenizing (#810) | | 2026-10-12 | Number formats: a fraction denominator with 65,536 zeros no longer crashes; format codes are capped at 255 characters like Excel (longer codes display as General, `TEXT` gives `#VALUE!`) (#807) | diff --git a/crates/chart/src/axis.rs b/crates/chart/src/axis.rs index 8686bf7d9..46fc43f99 100644 --- a/crates/chart/src/axis.rs +++ b/crates/chart/src/axis.rs @@ -25,9 +25,11 @@ impl Scale { (0..=n) .map(|i| { let v = self.min + i as f64 * self.step; - // Snap away accumulated float error. + // Snap away accumulated float error, but only onto a multiple of the step that is + // really that close: a `min` off the step grid must keep its own ticks. let snapped = (v / self.step).round() * self.step; - if snapped.abs() < self.step * 1e-9 { 0.0 } else { snapped } + let v = if (snapped - v).abs() < self.step * 1e-9 { snapped } else { v }; + if v.abs() < self.step * 1e-9 { 0.0 } else { v } }) .collect() } @@ -107,11 +109,6 @@ pub fn nice_scale(lo: f64, hi: f64, force_zero: bool) -> Scale { Scale { min, max, step } } -/// Exact fixed scale (100% stacked axes). -pub(crate) fn fixed_scale(min: f64, max: f64, step: f64) -> Scale { - Scale { min, max, step } -} - /// A number format prepared once and reused for many labels. pub(crate) struct Fmt { general: bool, diff --git a/crates/chart/src/render.rs b/crates/chart/src/render.rs index 037b9099e..82769bcdc 100644 --- a/crates/chart/src/render.rs +++ b/crates/chart/src/render.rs @@ -5,7 +5,7 @@ use std::f32::consts::{PI, TAU}; use gridcraft_model::{Chart, ChartKind, LegendPos, LineDash, Theme, TrendlineKind}; -use crate::axis::{Fmt, Scale, clean, fixed_scale, nice_scale}; +use crate::axis::{Fmt, Scale, clean, nice_scale}; use crate::resolve::{MAX_POINTS, MAX_SERIES, MAX_TRENDLINES, series_color}; use crate::trend::{auto_name, fit_trendline, moving_average}; use crate::{ChartData, HAlign, Measure, Prim, Rgba, SeriesData, VAlign}; @@ -120,9 +120,10 @@ pub enum Swatch { Marker, } -/// The kind a series is drawn as (per-series kinds only apply to combo charts). -fn plot_kind(chart: &Chart, s: &SeriesData) -> ChartKind { - if chart.kind == ChartKind::Combo { if s.kind == ChartKind::Combo { ChartKind::Line } else { s.kind } } else { chart.kind } +/// The kind series `i` is drawn as (per-series kinds only apply to combo charts); the same rule +/// as `series_plot_kind`, applied to the already resolved series kind. +fn plot_kind(chart: &Chart, s: &SeriesData, i: usize) -> ChartKind { + crate::resolve::plot_kind(chart.kind, Some(s.kind), i) } /// Colour of point `j` for charts that colour by point (pie, doughnut, treemap, sunburst). @@ -163,8 +164,9 @@ pub fn legend_entries(chart: &Chart, data: &ChartData) -> Vec<(String, Rgba, Swa let series = data.series.iter().take(MAX_SERIES); let mut out: Vec<(String, Rgba, Swatch)> = series .clone() - .map(|s| { - let sw = match plot_kind(chart, s) { + .enumerate() + .map(|(i, s)| { + let sw = match plot_kind(chart, s, i) { ChartKind::Line | ChartKind::LineStacked | ChartKind::Radar | ChartKind::ScatterLines => Swatch::Line, ChartKind::LineMarkers => Swatch::LineMarker, ChartKind::Scatter | ChartKind::Bubble => Swatch::Marker, @@ -174,7 +176,7 @@ pub fn legend_entries(chart: &Chart, data: &ChartData) -> Vec<(String, Rgba, Swa }) .collect(); // Trendlines follow the series, named "Linear (Sales)" unless given a name. - for s in series.filter(|s| supports_trendline(plot_kind(chart, s))) { + for (_, s) in series.enumerate().filter(|(i, s)| supports_trendline(plot_kind(chart, s, *i))) { for t in s.trendlines.iter().take(MAX_TRENDLINES) { out.push((t.spec.name.clone().unwrap_or_else(|| auto_name(t.spec.kind, &s.name)), t.color, Swatch::Line)); } @@ -833,7 +835,7 @@ fn cartesian(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx, horizonta if ncat == 0 { return; } - let kinds: Vec = data.series.iter().map(|s| if horizontal { chart.kind } else { plot_kind(chart, s) }).collect(); + let kinds: Vec = data.series.iter().enumerate().map(|(i, s)| if horizontal { chart.kind } else { plot_kind(chart, s, i) }).collect(); let fams: Vec = kinds.iter().map(|k| family(*k)).collect(); let any_primary = data.series.iter().any(|s| !s.secondary); let sec: Vec = data.series.iter().map(|s| s.secondary && any_primary && !horizontal).collect(); @@ -915,7 +917,7 @@ fn cartesian(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx, horizonta })); let (lo, hi) = range.unwrap_or((0.0, 0.0)); let (scale, fmt) = if all100 { - (fixed_scale(if lo < -1e-9 { -1.0 } else { 0.0 }, if hi > 1e-9 || lo >= -1e-9 { 1.0 } else { 0.0 }, 0.2), Fmt::new("0%")) + (Scale { min: if lo < -1e-9 { -1.0 } else { 0.0 }, max: if hi > 1e-9 || lo >= -1e-9 { 1.0 } else { 0.0 }, step: 0.2 }, Fmt::new("0%")) } else { let code = members.first().and_then(|i| data.series.get(*i)).map(|s| s.number_format.as_str()).unwrap_or("General"); (nice_scale(lo, hi, force_zero), Fmt::new(code)) diff --git a/crates/chart/src/resolve.rs b/crates/chart/src/resolve.rs index c007e3e90..2faeeb356 100644 --- a/crates/chart/src/resolve.rs +++ b/crates/chart/src/resolve.rs @@ -170,7 +170,7 @@ fn series_name(wb: &Workbook, sheet: usize, name: &str) -> String { t.to_string() } -fn effective_kind(chart_kind: ChartKind, series_kind: Option, i: usize) -> ChartKind { +pub(crate) fn effective_kind(chart_kind: ChartKind, series_kind: Option, i: usize) -> ChartKind { match (chart_kind, series_kind) { (ChartKind::Combo, Some(k)) if k != ChartKind::Combo => k, (ChartKind::Combo, _) => { @@ -185,9 +185,15 @@ fn effective_kind(chart_kind: ChartKind, series_kind: Option, i: usiz } } +/// The kind series `i` is drawn as: its own kind in a combo chart (first series a column, the +/// rest lines when it has none), else the chart's. The one rule shared by resolving and drawing. +pub(crate) fn plot_kind(chart_kind: ChartKind, series_kind: Option, i: usize) -> ChartKind { + if chart_kind == ChartKind::Combo { effective_kind(chart_kind, series_kind, i) } else { chart_kind } +} + /// The kind series `i` of `chart` is drawn as: its own kind in a combo chart, else the chart's. pub fn series_plot_kind(chart: &Chart, i: usize) -> ChartKind { - if chart.kind == ChartKind::Combo { effective_kind(chart.kind, chart.series.get(i).and_then(|s| s.kind), i) } else { chart.kind } + plot_kind(chart.kind, chart.series.get(i).and_then(|s| s.kind), i) } /// Evaluates a chart's series formulas: names, values, categories (default "1", "2", …), X diff --git a/crates/chart/src/tests.rs b/crates/chart/src/tests.rs index 01f1aac49..4dcfcde84 100644 --- a/crates/chart/src/tests.rs +++ b/crates/chart/src/tests.rs @@ -1057,3 +1057,35 @@ fn area_charts_draw_source_value_labels_only_when_enabled() { assert!(first_label > last_poly, "{kind:?}: label drawn under a later area"); } } + +#[test] +fn drawing_and_resolving_agree_on_the_combo_plot_kind() { + // Resolved series carry `effective_kind`; drawing re-derives the kind from it. Both paths + // must give the same kind for every chart kind, series kind (or none) and series index. + let series_kinds = std::iter::once(None).chain(ALL_KINDS.iter().map(|k| Some(*k))); + for ck in ALL_KINDS { + for sk in series_kinds.clone() { + for i in 0..3 { + let mut c = chart(ck); + c.series = (0..3).map(|j| Series { kind: if j == i { sk } else { None }, ..Default::default() }).collect(); + let from_chart = series_plot_kind(&c, i); + let resolved = crate::resolve::effective_kind(ck, sk, i); + assert_eq!(crate::resolve::plot_kind(ck, Some(resolved), i), from_chart, "{ck:?} {sk:?} {i}"); + if ck != ChartKind::Combo { + assert_eq!(from_chart, ck); + } else { + assert_ne!(from_chart, ChartKind::Combo); + } + } + } + } +} + +#[test] +fn ticks_of_a_scale_off_the_step_grid_start_at_its_min() { + let s = Scale { min: 3.0, max: 7.0, step: 4.0 }; + assert_eq!(s.ticks(), vec![3.0, 7.0]); + // Aligned scales still snap float error away. + let s = Scale { min: 0.0, max: 1.0, step: 0.1 }; + assert!((s.ticks()[3] - 0.3).abs() < 1e-12 && s.ticks()[0] == 0.0); +}