Merge pull request #1150 from vangel76/fix/chart-combo-logic-cleanup

Charts: one combo plot-kind function, no dead scale helpers (#1146)
This commit is contained in:
FredApps authored and GitHub committed 2026-11-07 16:52:46 +01:00
commit 505cae34c8
7 files changed
+57 -19

No files matched your search

+1
View File
@@ -197,6 +197,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in
| 2026-10-13 | Conditional formatting: a rule gathers range statistics only when it reads them (top/bottom, above average, duplicates, scales, bars, icons) and borrows them instead of copying them per cell, so the per-frame lookup of an expression or cell-value rule no longer visits its whole range (#1098, part 1) |
| 2026-10-12 | Next/previous sheet navigation stops at the first/last visible worksheet instead of activating a hidden boundary sheet; the current selection is retained |
| 2026-10-21 | Macro engine ([macros.md](docs/macros.md)): typed, declarative macros saved in the workbook (a JSON field; hidden names in xlsx); declared inputs checked with messages that name the cells; tokens (`Input.col(2)`, `Row`, `Output`, `Name:Total`, `{{…}}` in formulas); one undo step with full rollback; a footprint dry run with ask, fail, overwrite, shift down or shift right; a policy that keeps macros away from files and the app, with approval for macros in opened files; Repeat, If and Stop steps; a debugger; a relative recorder; `macro.*` commands beside the unchanged Automate scripts |
| 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 | Security: the `--control` server requires an auth token (random, kept in a 0600 file or chosen with `GRIDCRAFT_CONTROL_TOKEN`) on each connection's first line and closes bad connections (#777) |
| 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) |
+4 -7
View File
@@ -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,
+1 -1
View File
@@ -4,7 +4,7 @@ use std::f32::consts::{PI, TAU};
use gridcraft_model::{Chart, ChartKind, LineDash, QuartileMethod, 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};
+3 -3
View File
@@ -4,7 +4,7 @@ use super::lines::{decimate_area, draw_markers, draw_runs};
use super::trend::{draw_trend_labels, draw_trendlines, max_backward, max_forward, max_forward_slots, supports_trendline};
use super::{
Chart, ChartData, ChartKind, Ctx, Fmt, HAlign, LABEL_SIZE, MAX_CATS, MAX_LABELS, MAX_LINE_PTS, MAX_MARKERS, Prim, Scale, SeriesData, TEXT,
VAlign, WHITE, clean, fixed_scale, nice_scale,
VAlign, WHITE, clean, nice_scale,
};
// ---------------------------------------------------------------- cartesian (column/bar/line/area/combo)
@@ -145,7 +145,7 @@ fn axis_scales(data: &ChartData, fams: &[Fam], sec: &[bool], spans: &[Vec<Option
}));
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))
@@ -187,7 +187,7 @@ pub(super) fn cartesian(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx
if ncat == 0 {
return;
}
let kinds: Vec<ChartKind> = data.series.iter().map(|s| if horizontal { chart.kind } else { plot_kind(chart, s) }).collect();
let kinds: Vec<ChartKind> = data.series.iter().enumerate().map(|(i, s)| if horizontal { chart.kind } else { plot_kind(chart, s, i) }).collect();
let fams: Vec<Fam> = kinds.iter().map(|k| family(*k)).collect();
let any_primary = data.series.iter().any(|s| !s.secondary);
let sec: Vec<bool> = data.series.iter().map(|s| s.secondary && any_primary && !horizontal).collect();
+8 -6
View File
@@ -15,9 +15,10 @@ pub enum Swatch {
Marker,
}
/// The kind a series is drawn as (per-series kinds only apply to combo charts).
pub(super) 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.
pub(super) 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).
@@ -60,8 +61,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,
@@ -71,7 +73,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));
}
+8 -2
View File
@@ -201,7 +201,7 @@ fn series_name(wb: &Workbook, sheet: usize, name: &str) -> String {
t.to_string()
}
fn effective_kind(chart_kind: ChartKind, series_kind: Option<ChartKind>, i: usize) -> ChartKind {
pub(crate) fn effective_kind(chart_kind: ChartKind, series_kind: Option<ChartKind>, i: usize) -> ChartKind {
match (chart_kind, series_kind) {
(ChartKind::Combo, Some(k)) if k != ChartKind::Combo => k,
(ChartKind::Combo, _) => {
@@ -216,9 +216,15 @@ fn effective_kind(chart_kind: ChartKind, series_kind: Option<ChartKind>, 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<ChartKind>, 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
+32
View File
@@ -1418,6 +1418,38 @@ fn area_charts_draw_source_value_labels_only_when_enabled() {
}
}
#[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);
}
#[test]
fn hundred_percent_stacked_areas_fill_each_category_to_its_total() {
let d = ChartData {