From 4a1e6de3034f0e953c679c1b2fa906830ee8b5fe Mon Sep 17 00:00:00 2001 From: vangel76 Date: Tue, 13 Oct 2026 21:21:24 +0200 Subject: [PATCH 1/6] Charts: work and memory are bounded by series x points; ranges are clipped before evaluation (#1147) A chart keeps at most 2,000,000 series x points (every series and the categories keep their first points), 16 trendlines per chart; columns and bars narrower than 0.25 px are merged per pixel column; series references are cut after the rows that can matter before calc evaluates them; the moving average is linear; fit_trendline allocates no Vec per point; render borrows clean data instead of cloning it. Co-Authored-By: Claude Sonnet 5.5 --- ROADMAP.md | 1 + crates/chart/src/render.rs | 105 +++++++++++++++- crates/chart/src/resolve.rs | 138 ++++++++++++++++++--- crates/chart/src/tests.rs | 2 + crates/chart/src/tests/budget.rs | 172 ++++++++++++++++++++++++++ crates/chart/src/trend.rs | 205 ++++++++++++++++++++++++++++++- 6 files changed, 602 insertions(+), 21 deletions(-) create mode 100644 crates/chart/src/tests/budget.rs diff --git a/ROADMAP.md b/ROADMAP.md index 070552d07..06b50e9cf 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -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: work and memory are bounded by series x points (2,000,000 per chart: every series keeps its first points; 16 trendlines per chart), hairline columns and bars are merged per pixel column, series ranges are clipped before they are evaluated, the moving average is linear and rendering clean data no longer copies it (#1147) | | 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/render.rs b/crates/chart/src/render.rs index 037b9099e..8df7c2800 100644 --- a/crates/chart/src/render.rs +++ b/crates/chart/src/render.rs @@ -1,12 +1,13 @@ //! Chart layout: title, legend, axes and every chart kind, drawn into [`Prim`]s. +use std::borrow::Cow; use std::collections::HashSet; 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::resolve::{MAX_POINTS, MAX_SERIES, MAX_TRENDLINES, series_color}; +use crate::resolve::{MAX_POINTS, MAX_SERIES, MAX_TRENDLINES, points_per_series, series_color}; use crate::trend::{auto_name, fit_trendline, moving_average}; use crate::{ChartData, HAlign, Measure, Prim, Rgba, SeriesData, VAlign}; @@ -30,6 +31,8 @@ const MAX_LINE_PTS: usize = 2000; const MAX_MARKERS: usize = 5000; /// Data labels are skipped when a series has more points than this. const MAX_LABELS: usize = 200; +/// Columns and bars narrower than this (in pixels) are merged per pixel column into one rectangle. +const MIN_BAR_PX: f32 = 0.25; #[derive(Clone, Copy, Debug)] struct Bx { @@ -188,7 +191,7 @@ fn sanitize_vals(v: &[Option]) -> Vec> { v.iter().take(MAX_POINTS).map(|x| x.filter(|n| n.is_finite()).map(clean)).collect() } -fn sanitize(d: &ChartData) -> ChartData { +pub(crate) fn sanitize(d: &ChartData) -> ChartData { ChartData { palette: d.palette.iter().take(MAX_POINTS).copied().collect(), categories: d.categories.iter().take(MAX_POINTS).cloned().collect(), @@ -212,6 +215,54 @@ fn sanitize(d: &ChartData) -> ChartData { } } +/// Whether `v` survives `sanitize_vals` unchanged. +fn is_clean(v: &Option) -> bool { + v.is_none_or(|n| n.is_finite() && clean(n) == n) +} + +/// Whether [`sanitize`] and [`apply_budget`] would change `d`. +fn needs_sanitizing(d: &ChartData) -> bool { + let cap = points_per_series(d.series.len()); + let long = |n: usize| n > cap; + let mut trendlines = 0usize; + d.palette.len() > MAX_POINTS + || long(d.categories.len()) + || d.series.len() > MAX_SERIES + || d.series.iter().any(|s| { + trendlines += s.trendlines.len(); + long(s.values.len()) + || !s.values.iter().all(is_clean) + || s.x.as_ref().is_some_and(|x| long(x.len()) || !x.iter().all(is_clean)) + || s.sizes.as_ref().is_some_and(|x| long(x.len()) || !x.iter().all(is_clean)) + || s.trendlines.len() > MAX_TRENDLINES + }) + || trendlines > MAX_TRENDLINES +} + +/// Enforces the chart's budget of points (`MAX_TOTAL_POINTS` = series x points: every series and +/// the category list keep their first `MAX_TOTAL_POINTS / series` points) and of trendlines +/// (`MAX_TRENDLINES` per chart, in series order). +fn apply_budget(mut d: ChartData) -> ChartData { + let cap = points_per_series(d.series.len()); + d.categories.truncate(cap); + let mut trendlines = MAX_TRENDLINES; + for s in &mut d.series { + s.values.truncate(cap); + for v in [&mut s.x, &mut s.sizes].into_iter().flatten() { + v.truncate(cap); + } + s.trendlines.truncate(trendlines); + trendlines = trendlines.saturating_sub(s.trendlines.len()); + } + d +} + +/// The data `render` draws: `d` itself when it is already clean and within budget (the usual +/// case: no copy), else a cleaned, truncated copy. +pub(crate) fn sanitized(d: &ChartData) -> Cow<'_, ChartData> { + if needs_sanitizing(d) { Cow::Owned(apply_budget(sanitize(d))) } else { Cow::Borrowed(d) } +} + fn chart_title(chart: &Chart, data: &ChartData) -> Option { match &chart.title { Some(t) => Some(t.trim().to_string()).filter(|t| !t.is_empty()), @@ -225,7 +276,7 @@ pub fn render(chart: &Chart, data: &ChartData, w: f32, h: f32, m: &dyn Measure) if !(w.is_finite() && h.is_finite()) || w < 1.0 || h < 1.0 || w > 1e7 || h > 1e7 { return vec![]; } - let data = sanitize(data); + let data = sanitized(data); let mut ctx = Ctx { out: Vec::new(), m }; ctx.out.push(Prim::Rect { x: 0.0, y: 0.0, w, h, fill: Some(WHITE), stroke: Some((GRID, 1.0)), radius: 0.0 }); let pad = (w.min(h) * 0.04).clamp(2.0, 10.0); @@ -828,6 +879,36 @@ fn is_line(f: Fam) -> bool { matches!(f, Fam::Line | Fam::LineStack) } +/// Bars that fall into one pixel column, merged: extent along the category axis (`a0..=a1`, the +/// bars' start positions) and along the value axis (`lo..hi`). +struct BarRun { + bucket: f32, + a0: f32, + a1: f32, + lo: f32, + hi: f32, +} + +impl BarRun { + fn new(pos: f32, lo: f32, hi: f32) -> Self { + BarRun { bucket: pos.floor(), a0: pos, a1: pos, lo, hi } + } + fn add(&mut self, pos: f32, lo: f32, hi: f32) { + self.a0 = self.a0.min(pos); + self.a1 = self.a1.max(pos); + self.lo = self.lo.min(lo); + self.hi = self.hi.max(hi); + } + fn draw(&self, ctx: &mut Ctx, bw: f32, horizontal: bool, color: Rgba) { + let along = self.a1 - self.a0 + bw; + if horizontal { + ctx.rect(self.lo, self.a0, self.hi - self.lo, along, color, None); + } else { + ctx.rect(self.a0, self.lo, along, self.hi - self.lo, color, None); + } + } +} + fn cartesian(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx, horizontal: bool) { let ncat = data.categories.len().max(data.series.iter().map(|s| s.values.len()).max().unwrap_or(0)).min(MAX_CATS); if ncat == 0 { @@ -1026,12 +1107,27 @@ fn cartesian(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx, horizonta } 1 => { let k = slot_of.get(i).copied().unwrap_or(0) as f32; + // Hairline bars are merged per pixel column (their envelope is what shows). + let cull = bw < MIN_BAR_PX && !labels_on; + let mut run: Option = None; for (j, sp) in row.iter().enumerate() { let Some((b, t)) = sp else { continue }; let (pb, pt) = (vpos(*b, &sc), vpos(*t, &sc)); let c = center(j); let lo = pb.min(pt); let len = (pb - pt).abs(); + if cull { + let pos = if horizontal { c + nslots * bw / 2.0 - (k + 1.0) * bw } else { c - nslots * bw / 2.0 + k * bw }; + match &mut run { + Some(m) if m.bucket == pos.floor() => m.add(pos, lo, lo + len), + _ => { + if let Some(m) = run.replace(BarRun::new(pos, lo, lo + len)) { + m.draw(ctx, bw, horizontal, s.color); + } + } + } + continue; + } if horizontal { let y = c + nslots * bw / 2.0 - (k + 1.0) * bw; ctx.rect(lo, y, len, bw, s.color, None); @@ -1064,6 +1160,9 @@ fn cartesian(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx, horizonta } } } + if let Some(m) = run { + m.draw(ctx, bw, horizontal, s.color); + } } _ => { let pts: Vec> = row.iter().enumerate().map(|(j, sp)| sp.map(|(_, t)| [center(j), vpos(t, &sc)])).collect(); diff --git a/crates/chart/src/resolve.rs b/crates/chart/src/resolve.rs index c007e3e90..c0dbaa173 100644 --- a/crates/chart/src/resolve.rs +++ b/crates/chart/src/resolve.rs @@ -11,8 +11,17 @@ use crate::{ChartData, Rgba, SeriesData, TrendlineData}; pub(crate) const MAX_POINTS: usize = 100_000; /// Cap on series per chart. pub(crate) const MAX_SERIES: usize = 255; -/// Cap on trendlines drawn per series. +/// Cap on trendlines per chart (not per series: each one is a fit over a series' points). pub(crate) const MAX_TRENDLINES: usize = 16; +/// Budget for series x points in one chart. When a chart would hold more, every series (and the +/// category list) keeps its first `MAX_TOTAL_POINTS / series` points; no series is dropped. +pub(crate) const MAX_TOTAL_POINTS: usize = 2_000_000; + +/// Points kept per series (and categories) in a chart of `series` series: `MAX_POINTS`, or less +/// when `series x MAX_POINTS` would pass `MAX_TOTAL_POINTS`. +pub(crate) fn points_per_series(series: usize) -> usize { + (MAX_TOTAL_POINTS / series.clamp(1, MAX_SERIES)).clamp(1, MAX_POINTS) +} /// Tints used for series beyond the six theme accents: lighter, darker, lighter still… const CYCLE_TINTS: [f64; 6] = [0.0, 0.4, -0.25, 0.6, -0.5, 0.2]; @@ -77,10 +86,34 @@ fn union_parts(f: &str) -> Option> { /// Evaluates a series formula to a flat list of values (row-major), capped. A union of /// references (`(A1:A3,A5)`, as PivotCharts use) is read part by part. fn eval_list(wb: &Workbook, sheet: usize, formula: &str) -> Vec { - eval_list_in(wb, sheet, formula, 0) + eval_list_in(wb, sheet, formula, MAX_POINTS, 0) } -fn eval_list_in(wb: &Workbook, sheet: usize, formula: &str, depth: u32) -> Vec { +/// calc reads a reference of up to this many cells at its full size and trims a larger one to the +/// sheet's used range (`FULL_SIZE_CELLS` in `gridcraft_calc::eval`). A clipped reference must stay +/// above it, or the clip would turn a trimmed read into a full-size one with trailing blanks. +const CALC_FULL_SIZE_CELLS: u64 = 16_384; + +/// `formula` as a plain reference (`Sheet!A1:A1000000`) cut after the rows that can still hold +/// one of the first `limit` points, so that calc never reads the rest; `None` when `formula` is +/// no reference or already small enough. Values are read row by row, and calc trims only the +/// bottom and right of a reference, so keeping `limit` rows keeps the first `limit` values. +fn clipped_reference(wb: &Workbook, sheet: usize, formula: &str, limit: usize) -> Option { + let (_, r) = parse_ref(wb, sheet, formula)?; + let (h, w) = (u64::from(r.height()), u64::from(r.width())); + if h.saturating_mul(w) <= CALC_FULL_SIZE_CELLS { + return None; + } + let keep = (limit as u64).max(CALC_FULL_SIZE_CELLS / w + 1); + if h <= keep { + return None; + } + let end = CellRef::new(r.start.row.saturating_add(u32::try_from(keep).ok()?).saturating_sub(1), r.end.col); + let prefix = formula.rfind('!').and_then(|i| formula.get(..=i)).unwrap_or(""); + Some(format!("{prefix}{}", RangeRef::new(r.start, end))) +} + +fn eval_list_in(wb: &Workbook, sheet: usize, formula: &str, limit: usize, depth: u32) -> Vec { let f = formula.trim(); if f.is_empty() { return vec![]; @@ -90,16 +123,17 @@ fn eval_list_in(wb: &Workbook, sheet: usize, formula: &str, depth: u32) -> Vec= MAX_POINTS { + if out.len() >= limit { break; } - let rest = MAX_POINTS - out.len(); - out.extend(eval_list_in(wb, sheet, p, depth + 1).into_iter().take(rest)); + let rest = limit - out.len(); + out.extend(eval_list_in(wb, sheet, p, rest, depth + 1).into_iter().take(rest)); } return out; } - match evaluate(wb, sheet, CellRef::default(), f) { - Value::Array(a) => a.iter().take(MAX_POINTS).cloned().collect(), + let clipped = clipped_reference(wb, sheet, f, limit); + match evaluate(wb, sheet, CellRef::default(), clipped.as_deref().unwrap_or(f)) { + Value::Array(a) => a.iter().take(limit).cloned().collect(), v => vec![v], } } @@ -111,8 +145,8 @@ fn to_num(v: &Value) -> Option { } } -fn nums(wb: &Workbook, sheet: usize, formula: &str) -> Vec> { - eval_list(wb, sheet, formula).iter().map(to_num).collect() +fn nums(wb: &Workbook, sheet: usize, formula: &str, limit: usize) -> Vec> { + eval_list_in(wb, sheet, formula, limit, 0).iter().map(to_num).collect() } /// Number format code of the first cell of a reference ("General" when not a reference). @@ -197,35 +231,41 @@ pub fn resolve(wb: &Workbook, sheet: usize, chart: &Chart) -> ChartData { let numeric_x = matches!(chart.kind, ChartKind::Scatter | ChartKind::ScatterLines | ChartKind::Bubble); let mut series = Vec::new(); let mut categories: Option> = None; + let limit = points_per_series(chart.series.len()); + let mut trend_left = MAX_TRENDLINES; for (i, s) in chart.series.iter().take(MAX_SERIES).enumerate() { - let values = nums(wb, sheet, &s.values); + let values = nums(wb, sheet, &s.values, limit); let name = s.name.as_deref().map(|n| series_name(wb, sheet, n)).filter(|n| !n.is_empty()).unwrap_or_else(|| format!("Series{}", i + 1)); let color = s.color.as_ref().and_then(|c| c.resolve(&wb.theme)).map(|[r, g, b]| [r, g, b, 0xFF]).unwrap_or_else(|| series_color(&wb.theme, i)); let kind = effective_kind(chart.kind, s.kind, i); let x = match (&s.categories, numeric_x || matches!(kind, ChartKind::Scatter | ChartKind::ScatterLines | ChartKind::Bubble)) { // X values with text in them (labels) are ignored: the points are numbered 1..n. - (Some(f), true) => Some(eval_list(wb, sheet, f)).filter(|xs| !xs.iter().any(Value::is_text)).map(|xs| xs.iter().map(to_num).collect()), + (Some(f), true) => { + Some(eval_list_in(wb, sheet, f, limit, 0)).filter(|xs| !xs.iter().any(Value::is_text)).map(|xs| xs.iter().map(to_num).collect()) + } _ => None, }; if categories.is_none() && let Some(f) = &s.categories { - let c = category_texts(wb, sheet, f); + let mut c = category_texts(wb, sheet, f); + c.truncate(limit); if !c.is_empty() { categories = Some(c); } } - let sizes = s.bubble_sizes.as_deref().map(|f| nums(wb, sheet, f)); + let sizes = s.bubble_sizes.as_deref().map(|f| nums(wb, sheet, f, limit)); let trendlines = s .trendlines .iter() - .take(MAX_TRENDLINES) + .take(trend_left) .map(|t| TrendlineData { spec: t.clone(), color: t.color.as_ref().and_then(|c| c.resolve(&wb.theme)).map(|[r, g, b]| [r, g, b, 0xFF]).unwrap_or(color), }) - .collect(); + .collect::>(); + trend_left = trend_left.saturating_sub(trendlines.len()); series.push(SeriesData { name, values, @@ -244,3 +284,69 @@ pub fn resolve(wb: &Workbook, sheet: usize, chart: &Chart) -> ChartData { let palette = default_palette(&wb.theme, n.min(PALETTE_CYCLE)); ChartData { categories, series, palette } } + +#[cfg(test)] +mod tests { + use gridcraft_model::Workbook; + + use super::*; + + fn book(rows: u32, cols: u32) -> Workbook { + let mut wb = Workbook::new(); + if let Some(s) = wb.sheet_mut(0) { + s.name = "My Sheet".into(); + for r in 0..rows { + for c in 0..cols { + s.set_value(CellRef::new(r, c), Value::Number(f64::from(r * 10 + c))); + } + } + } + wb + } + + #[test] + fn a_long_reference_is_clipped_before_it_is_evaluated() { + let wb = book(1, 1); + let rows = |f: &str, limit| { + let g = clipped_reference(&wb, 0, f, limit)?; + let (_, r) = parse_ref(&wb, 0, &g)?; + Some((g, u64::from(r.height()))) + }; + // 1,000,000 rows: at most `MAX_POINTS` of them are asked for. + assert_eq!(rows("'My Sheet'!$A$1:$A$1000000", MAX_POINTS), Some(("'My Sheet'!A1:A100000".to_string(), 100_000))); + assert_eq!(rows("A5:A1048576", MAX_POINTS).map(|r| r.1), Some(100_000)); + assert_eq!(rows("A:A", 50_000).map(|r| r.1), Some(50_000)); + // Never below what keeps calc trimming the clipped reference like the whole one. + assert_eq!(rows("A1:A1000000", 100).map(|r| r.1), Some(16_385)); + // Wide references keep whole rows. + assert_eq!(rows("A1:C1000000", MAX_POINTS).map(|r| r.1), Some(100_000)); + // Small references, single cells and anything that isn't a plain reference are left alone. + assert_eq!(rows("A1:A9000", MAX_POINTS), None); + assert_eq!(rows("A1:A20000", MAX_POINTS), None); + assert_eq!(rows("B2", MAX_POINTS), None); + assert_eq!(rows("SUM(A1:A1000000)", MAX_POINTS), None); + assert_eq!(rows("{1,2,3}", MAX_POINTS), None); + } + + #[test] + fn clipping_does_not_change_the_points() { + for (rows, cols) in [(10, 1), (30_000, 1), (30_000, 2), (3, 4)] { + let wb = book(rows, cols); + for f in ["'My Sheet'!A1:A1000000", "'My Sheet'!$A$3:$B$1048576", "A:A", "A1:C1000000", "(A1:A1000000,B1:B1000000)", "A2:A20000"] { + for limit in [100, 7_843, 25_000, MAX_POINTS] { + let clipped = eval_list_in(&wb, 0, f, limit, 0); + let whole: Vec = match evaluate(&wb, 0, CellRef::default(), f) { + Value::Array(a) => a.iter().take(limit).cloned().collect(), + v => vec![v], + }; + if f.starts_with('(') { + // A union is read part by part; each part keeps its own order. + assert!(clipped.len() <= limit); + continue; + } + assert_eq!(clipped, whole, "{f} limit {limit} on {rows}x{cols}"); + } + } + } + } +} diff --git a/crates/chart/src/tests.rs b/crates/chart/src/tests.rs index 01f1aac49..587ddd958 100644 --- a/crates/chart/src/tests.rs +++ b/crates/chart/src/tests.rs @@ -5,6 +5,8 @@ use gridcraft_model::{Anchor, Chart, ChartKind, Color, LegendPos, Series, Sparkl use crate::*; +mod budget; + const ALL_KINDS: [ChartKind; 25] = [ ChartKind::ColumnClustered, ChartKind::ColumnStacked, diff --git a/crates/chart/src/tests/budget.rs b/crates/chart/src/tests/budget.rs new file mode 100644 index 000000000..e151f635d --- /dev/null +++ b/crates/chart/src/tests/budget.rs @@ -0,0 +1,172 @@ +//! Work and memory are bounded by the product of the chart limits (series x points, trendlines +//! per chart, hairline bars), and clean data is rendered without a copy. + +use std::borrow::Cow; + +use gridcraft_core::{CellRef, Value}; +use gridcraft_model::{ChartKind, Series, Trendline, TrendlineKind, Workbook}; + +use super::{assert_sane, chart}; +use crate::render::{sanitize, sanitized}; +use crate::resolve::{MAX_POINTS, MAX_TOTAL_POINTS, MAX_TRENDLINES, points_per_series}; +use crate::*; + +fn data_of(series: usize, points: usize) -> ChartData { + ChartData { + palette: vec![], + categories: (0..points).map(|i| i.to_string()).collect(), + series: (0..series) + .map(|s| SeriesData { + name: format!("S{s}"), + values: (0..points).map(|i| Some(((i * 7 + s * 13) % 100 + 1) as f64)).collect(), + color: series_color(&Default::default(), s), + ..SeriesData::default() + }) + .collect(), + } +} + +fn series_rects(prims: &[Prim], color: Rgba) -> Vec<(f32, f32, f32, f32)> { + prims + .iter() + .filter_map(|p| match p { + Prim::Rect { x, y, w, h, fill: Some(f), .. } if *f == color => Some((*x, *y, *w, *h)), + _ => None, + }) + .collect() +} + +#[test] +fn the_point_budget_keeps_every_series_with_fewer_points() { + assert_eq!(points_per_series(1), MAX_POINTS); + assert_eq!(points_per_series(20), MAX_POINTS); + assert_eq!(points_per_series(21), MAX_TOTAL_POINTS / 21); + assert_eq!(points_per_series(255), MAX_TOTAL_POINTS / 255); + assert_eq!(points_per_series(0), MAX_POINTS); + + // 25 series x 100,000 points is 2.5 million: over budget. + let d = data_of(25, MAX_POINTS); + let kept = sanitized(&d); + assert_eq!(kept.series.len(), 25, "no series is dropped"); + assert!(kept.series.iter().all(|s| s.values.len() == MAX_TOTAL_POINTS / 25)); + assert_eq!(kept.categories.len(), MAX_TOTAL_POINTS / 25); + assert!(kept.series.iter().map(|s| s.values.len()).sum::() <= MAX_TOTAL_POINTS); + // The first points are the ones kept. + assert_eq!(kept.series[3].values[..5], d.series[3].values[..5]); + + // Drawing it stays bounded: hairline columns are merged per pixel column. + let mut c = chart(ChartKind::ColumnClustered); + c.data_labels = false; + c.legend = LegendPos::None; + let prims = render(&c, &d, 400.0, 300.0, &ApproxMeasure); + assert_sane(&prims, 400.0, 300.0); + assert!(prims.len() < 25 * 450 + 200, "{} primitives", prims.len()); +} + +#[test] +fn trendlines_are_limited_per_chart() { + let line = |kind| TrendlineData { spec: Trendline { kind, ..Trendline::default() }, color: [1, 2, 3, 255] }; + let mut d = data_of(3, 30); + for s in &mut d.series { + s.trendlines = vec![line(TrendlineKind::Linear); 10]; + } + let kept = sanitized(&d); + let counts: Vec = kept.series.iter().map(|s| s.trendlines.len()).collect(); + assert_eq!(counts, vec![10, MAX_TRENDLINES - 10, 0], "series order decides which trendlines stay"); + let entries = legend_entries(&chart(ChartKind::Line), &kept); + assert_eq!(entries.len(), 3 + MAX_TRENDLINES); +} + +#[test] +fn hairline_columns_are_merged_and_wide_ones_are_not() { + let mut c = chart(ChartKind::ColumnClustered); + c.data_labels = false; + c.legend = LegendPos::None; + + // Wide bars (>= 0.25 px): one rectangle per series and category, as before. + let d = data_of(3, 40); + let prims = render(&c, &d, 640.0, 400.0, &ApproxMeasure); + for s in &d.series { + assert_eq!(series_rects(&prims, s.color).len(), 40); + } + + // 4 series x 20,000 categories in 400 px: the bars are hairlines. + let d = data_of(4, 20_000); + let prims = render(&c, &d, 400.0, 300.0, &ApproxMeasure); + assert_sane(&prims, 400.0, 300.0); + for s in &d.series { + let n = series_rects(&prims, s.color).len(); + assert!((1..=420).contains(&n), "{n} rectangles for one series"); + } + + // The merged rectangles keep the envelope: alternating 1 and 10 draws the full-height bar. + let mut d = data_of(1, 20_000); + for (i, v) in d.series[0].values.iter_mut().enumerate() { + *v = Some(if i % 2 == 0 { 1.0 } else { 10.0 }); + } + let prims = render(&c, &d, 400.0, 300.0, &ApproxMeasure); + let rects = series_rects(&prims, d.series[0].color); + let tallest = rects.iter().map(|r| r.3).fold(0.0f32, f32::max); + assert!(rects.len() > 100 && tallest > 100.0); + assert!(rects.iter().all(|r| (r.3 - tallest).abs() < 0.01), "every pixel column shows the tall bar"); + // And together they cover the plot's width without a gap wider than a pixel. + let mut xs: Vec<(f32, f32)> = rects.iter().map(|r| (r.0, r.0 + r.2)).collect(); + xs.sort_by(|a, b| a.0.total_cmp(&b.0)); + assert!(xs.windows(2).all(|w| w[1].0 - w[0].1 < 1.0)); + + // Horizontal bars too. + let mut c = chart(ChartKind::BarClustered); + c.data_labels = false; + c.legend = LegendPos::None; + let prims = render(&c, &d, 400.0, 300.0, &ApproxMeasure); + let n = series_rects(&prims, d.series[0].color).len(); + assert!((1..=320).contains(&n), "{n} bars"); +} + +#[test] +fn clean_data_is_rendered_without_a_copy() { + let d = data_of(3, 50); + assert!(matches!(sanitized(&d), Cow::Borrowed(_))); + + // Dirty data takes the copying path and comes out as `sanitize` has always produced it. + let mut dirty = data_of(3, 50); + dirty.series[0].values[4] = Some(f64::NAN); + dirty.series[1].values[5] = Some(f64::INFINITY); + dirty.series[2].values[6] = Some(1e301); + dirty.series[2].x = Some(vec![Some(f64::NEG_INFINITY), Some(2.0)]); + dirty.series[1].sizes = Some(vec![Some(-1e305)]); + let got = sanitized(&dirty); + assert!(matches!(got, Cow::Owned(_))); + assert_eq!(*got, sanitize(&dirty)); + assert_eq!(got.series[0].values[4], None); + assert_eq!(got.series[2].values[6], Some(1e300)); + + // Over the series cap (within the point budget) the same. + let many = data_of(300, 3); + let got = sanitized(&many); + assert_eq!(got.series.len(), 255); + assert_eq!(*got, sanitize(&many)); +} + +#[test] +fn resolve_keeps_every_series_within_the_point_budget() { + // 40 series x 60,000 points is 2.4 million: over the 2 million budget. + let mut wb = Workbook::new(); + if let Some(s) = wb.sheet_mut(0) { + for r in 0..60_000u32 { + s.set_value(CellRef::new(r, 0), Value::Number(f64::from(r))); + } + } + let mut c = chart(ChartKind::ColumnClustered); + c.series = + (0..40).map(|_| Series { values: "Sheet1!$A$1:$A$1000000".into(), categories: Some("A1:A1000000".into()), ..Series::default() }).collect(); + let d = resolve(&wb, 0, &c); + assert_eq!(d.series.len(), 40); + assert!(d.series.iter().all(|s| s.values.len() == MAX_TOTAL_POINTS / 40)); + assert_eq!(d.categories.len(), MAX_TOTAL_POINTS / 40); + assert_eq!(d.series[7].values.first(), Some(&Some(0.0))); + // Trendlines: 16 per chart, in series order. + c.series = (0..4).map(|_| Series { values: "A1:A5".into(), trendlines: vec![Default::default(); 9], ..Series::default() }).collect(); + let counts: Vec = resolve(&wb, 0, &c).series.iter().map(|s| s.trendlines.len()).collect(); + assert_eq!(counts, vec![9, 7, 0, 0]); +} diff --git a/crates/chart/src/trend.rs b/crates/chart/src/trend.rs index d987ab58d..6b22ed7ad 100644 --- a/crates/chart/src/trend.rs +++ b/crates/chart/src/trend.rs @@ -72,7 +72,12 @@ pub(crate) fn fit_trendline(t: &Trendline, pts: &[(f64, f64)]) -> Option = (first..=d).map(|k| u.powi(k as i32)).collect(); + // `d` is at most 6, so the powers `first..=d` always fit the array (no allocation per point). + let mut buf = [0.0f64; 7]; + for (slot, k) in buf.iter_mut().zip(first..=d) { + *slot = u.powi(k as i32); + } + let pows = buf.get(..m)?; for (i, pi) in pows.iter().enumerate() { if let Some(v) = aty.get_mut(i) { *v += pi * (y - b0); @@ -258,10 +263,41 @@ fn trim_zeros(s: &str) -> String { /// Moving average of `period` consecutive points in plotted order: point `i` (from /// `period - 1`) is `(xᵢ, mean(yᵢ₋ₚ₊₁..=yᵢ))`. Empty when there are fewer points than `period`. +/// +/// Linear in the number of points: the points are cut into blocks of `period`, and a window is +/// the tail of one block plus the head of the next, taken from running sums that only ever add +/// (a running sum that also subtracts would lose small values after a huge one passed through). pub(crate) fn moving_average(pts: &[(f64, f64)], period: u32) -> Vec<(f64, f64)> { let p = period.clamp(2, 255) as usize; let pts: Vec<(f64, f64)> = pts.iter().copied().filter(|(x, y)| x.is_finite() && y.is_finite()).collect(); - pts.windows(p).filter_map(|w| w.last().map(|(x, _)| (*x, w.iter().map(|q| q.1).sum::() / p as f64))).collect() + let n = pts.len(); + if n < p { + return vec![]; + } + // `head[i]`: sum of the block's points up to and including `i`; `tail[i]`: from `i` to the block's end. + let mut head = vec![0.0f64; n]; + let mut tail = vec![0.0f64; n]; + for (b, block) in pts.chunks(p).enumerate() { + let (mut h, mut t) = (0.0f64, 0.0f64); + for k in 0..block.len() { + h += block.get(k).map_or(0.0, |q| q.1); + if let Some(v) = head.get_mut(b * p + k) { + *v = h; + } + let r = block.len() - 1 - k; + t += block.get(r).map_or(0.0, |q| q.1); + if let Some(v) = tail.get_mut(b * p + r) { + *v = t; + } + } + } + (p - 1..n) + .filter_map(|i| { + let s = i + 1 - p; + let sum = tail.get(s)? + if s.is_multiple_of(p) { 0.0 } else { *head.get(i)? }; + Some((pts.get(i)?.0, sum / p as f64)) + }) + .collect() } /// Excel's automatic trendline name for the legend, e.g. `Linear (Sales)`. @@ -276,3 +312,168 @@ pub(crate) fn auto_name(kind: TrendlineKind, series: &str) -> String { }; format!("{k} ({series})") } + +#[cfg(test)] +mod tests { + use super::*; + + /// xorshift64 for repeatable pseudo-random data. + struct Rng(u64); + + impl Rng { + fn next(&mut self) -> u64 { + self.0 ^= self.0 << 13; + self.0 ^= self.0 >> 7; + self.0 ^= self.0 << 17; + self.0 + } + fn unit(&mut self) -> f64 { + (self.next() % 1_000_000) as f64 / 1_000_000.0 - 0.5 + } + } + + /// The window-by-window moving average this module used to compute. + fn moving_average_naive(pts: &[(f64, f64)], period: u32) -> Vec<(f64, f64)> { + let p = period.clamp(2, 255) as usize; + let pts: Vec<(f64, f64)> = pts.iter().copied().filter(|(x, y)| x.is_finite() && y.is_finite()).collect(); + pts.windows(p).filter_map(|w| w.last().map(|(x, _)| (*x, w.iter().map(|q| q.1).sum::() / p as f64))).collect() + } + + #[test] + fn linear_moving_average_matches_the_window_sums() { + let mut r = Rng(0x9E37_79B9_7F4A_7C15); + for case in 0..60 { + let n = (r.next() % 700) as usize; + let pts: Vec<(f64, f64)> = (0..n) + .map(|i| { + let y = match case % 5 { + 0 => r.unit() * 100.0, + 1 => 1e9 + r.unit(), + // A huge value passing through must not leave rounding residue behind. + 2 => { + if r.next().is_multiple_of(50) { + 1e300 * r.unit().signum() + } else { + r.unit() + } + } + 3 => { + if i % 2 == 0 { + 1e15 + } else { + -1e15 + r.unit() + } + } + _ => { + if r.next().is_multiple_of(9) { + f64::NAN + } else { + r.unit() * 1e-3 + } + } + }; + (i as f64, y) + }) + .collect(); + for period in [0, 2, 3, 7, 50, 255, 400] { + let fast = moving_average(&pts, period); + let slow = moving_average_naive(&pts, period); + assert_eq!(fast.len(), slow.len(), "case {case} period {period}"); + let p = period.clamp(2, 255) as usize; + let finite: Vec<(f64, f64)> = pts.iter().copied().filter(|(x, y)| x.is_finite() && y.is_finite()).collect(); + for (i, ((fx, fy), (sx, sy))) in fast.iter().zip(&slow).enumerate() { + assert_eq!(fx, sx); + let mass: f64 = finite.iter().skip(i).take(p).map(|q| q.1.abs()).sum::() / p as f64; + assert!((fy - sy).abs() <= 1e-12 * mass, "case {case} period {period} point {i}: {fy} vs {sy}"); + } + } + } + } + + /// The trendline fit as it was written with a `Vec` of powers per data point. + fn fit_reference(t: &Trendline, pts: &[(f64, f64)]) -> Option { + let kind = t.kind; + let d = degree(kind)?; + let data: Vec<(f64, f64)> = pts + .iter() + .filter(|(x, y)| x.is_finite() && y.is_finite() && (!log_x(kind) || *x > 0.0) && (!log_y(kind) || *y > 0.0)) + .map(|&(x, y)| (if log_x(kind) { x.ln() } else { x }, if log_y(kind) { y.ln() } else { y })) + .collect(); + if data.len() < 2 || data.len() <= d { + return None; + } + let fixed = match kind { + TrendlineKind::Linear | TrendlineKind::Polynomial(_) => t.intercept, + TrendlineKind::Exponential => t.intercept.filter(|b| *b > 0.0).map(f64::ln), + _ => None, + } + .filter(|b| b.is_finite()); + let n = data.len() as f64; + let center = if fixed.is_some() { 0.0 } else { data.iter().map(|p| p.0).sum::() / n }; + let scale = data.iter().map(|p| (p.0 - center).abs()).fold(0.0f64, f64::max); + if !(scale.is_finite() && scale > 0.0) { + return None; + } + let first = usize::from(fixed.is_some()); + let b0 = fixed.unwrap_or(0.0); + let m = d + 1 - first; + let mut ata = vec![vec![0.0f64; m]; m]; + let mut aty = vec![0.0f64; m]; + for &(x, y) in &data { + let u = (x - center) / scale; + let pows: Vec = (first..=d).map(|k| u.powi(k as i32)).collect(); + for (i, pi) in pows.iter().enumerate() { + aty[i] += pi * (y - b0); + for (j, pj) in pows.iter().enumerate() { + ata[i][j] += pi * pj; + } + } + } + let sol = solve(ata, aty)?; + let mut a = vec![b0; d + 1]; + for (k, v) in sol.into_iter().enumerate() { + a[k + first] = v; + } + let coef = expand(&a, center, scale)?; + let fit = TrendFit { kind, coef, center, scale, a, r2: None }; + let mean = data.iter().map(|p| p.1).sum::() / n; + let (mut ss_res, mut ss_tot) = (0.0f64, 0.0f64); + for &(x, y) in &data { + ss_res += (y - fit.poly(x)).powi(2); + ss_tot += if fixed.is_some() { (y - b0).powi(2) } else { (y - mean).powi(2) }; + } + let r2 = (ss_tot > 0.0 && ss_tot.is_finite() && ss_res.is_finite()).then(|| 1.0 - ss_res / ss_tot); + Some(TrendFit { r2, ..fit }) + } + + #[test] + fn fit_without_a_vec_per_point_is_bit_identical() { + let mut r = Rng(0x1234_5678_9ABC_DEF1); + let kinds = [ + TrendlineKind::Linear, + TrendlineKind::Exponential, + TrendlineKind::Logarithmic, + TrendlineKind::Power, + TrendlineKind::Polynomial(2), + TrendlineKind::Polynomial(3), + TrendlineKind::Polynomial(6), + TrendlineKind::MovingAverage(3), + ]; + for case in 0..40 { + let n = 2 + (r.next() % 60) as usize; + let pts: Vec<(f64, f64)> = (0..n) + .map(|i| { + let x = i as f64 * (1.0 + r.unit()) + 1.0; + let y = if case % 4 == 0 { f64::NAN } else { (r.unit() + 0.6) * 10f64.powi((case % 7) - 2) }; + (x, if case % 4 == 0 && i % 3 != 0 { r.unit() + 1.0 } else { y }) + }) + .collect(); + for kind in kinds { + for intercept in [None, Some(0.5), Some(-1.0)] { + let t = Trendline { kind, intercept, ..Trendline::default() }; + assert_eq!(fit_trendline(&t, &pts), fit_reference(&t, &pts), "case {case} {kind:?} {intercept:?}"); + } + } + } + } +} From 95c3bb9b89e229759077aa025032396e951e30a8 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Thu, 29 Oct 2026 23:05:54 +0100 Subject: [PATCH 2/6] Fix clippy complexity after merging upstream/main --- crates/chart/src/render/cartesian.rs | 84 +++++++++++++++++++--------- 1 file changed, 58 insertions(+), 26 deletions(-) diff --git a/crates/chart/src/render/cartesian.rs b/crates/chart/src/render/cartesian.rs index f30283b58..78774b078 100644 --- a/crates/chart/src/render/cartesian.rs +++ b/crates/chart/src/render/cartesian.rs @@ -3,8 +3,8 @@ use super::legend::{cat_label, plot_kind}; use super::lines::{decimate, 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, + Chart, ChartData, ChartKind, Ctx, Fmt, HAlign, LABEL_SIZE, MAX_CATS, MAX_LABELS, MAX_LINE_PTS, MAX_MARKERS, MIN_BAR_PX, Prim, Rgba, Scale, + SeriesData, TEXT, VAlign, WHITE, clean, fixed_scale, nice_scale, }; // ---------------------------------------------------------------- cartesian (column/bar/line/area/combo) @@ -81,6 +81,58 @@ impl BarRun { } } +/// Whether bars of width `bw` are merged per pixel column (never when they carry data labels). +fn merge_hairlines(bw: f32, labels_on: bool) -> bool { + bw < MIN_BAR_PX && !labels_on +} + +/// Draws the bars of one series: a rectangle each, or, for hairline bars, one merged [`BarRun`] +/// per pixel column. +struct BarSink { + run: Option, + bw: f32, + horizontal: bool, + color: Rgba, + nslots: f32, + k: f32, +} + +impl BarSink { + fn new(bw: f32, horizontal: bool, color: Rgba, nslots: f32, k: f32) -> Self { + BarSink { run: None, bw, horizontal, color, nslots, k } + } + /// Start of this series' slot along the category axis, for the category centered at `c`. + fn pos(&self, c: f32) -> f32 { + if self.horizontal { c + self.nslots * self.bw / 2.0 - (self.k + 1.0) * self.bw } else { c - self.nslots * self.bw / 2.0 + self.k * self.bw } + } + /// Draws the bar at category center `c` spanning `lo..lo + len` on the value axis, or, when + /// `merge` is set, adds it to the current run. + fn bar(&mut self, ctx: &mut Ctx, merge: bool, c: f32, lo: f32, len: f32) { + let pos = self.pos(c); + if !merge { + if self.horizontal { + ctx.rect(lo, pos, len, self.bw, self.color, None); + } else { + ctx.rect(pos, lo, self.bw, len, self.color, None); + } + return; + } + match &mut self.run { + Some(m) if m.bucket == pos.floor() => m.add(pos, lo, lo + len), + run => { + if let Some(m) = run.replace(BarRun::new(pos, lo, lo + len)) { + m.draw(ctx, self.bw, self.horizontal, self.color); + } + } + } + } + fn finish(self, ctx: &mut Ctx) { + if let Some(m) = self.run { + m.draw(ctx, self.bw, self.horizontal, self.color); + } + } +} + fn series_spans(data: &ChartData, fams: &[Fam], sec: &[bool], ncat: usize) -> Vec>> { // Totals for 100% stacking, per axis and family (100% columns and 100% areas stack apart). let mut totals: Vec<((bool, Fam), Vec)> = Vec::new(); @@ -307,33 +359,15 @@ pub(super) fn cartesian(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx 1 => { let k = slot_of.get(i).copied().unwrap_or(0) as f32; // Hairline bars are merged per pixel column (their envelope is what shows). - let cull = bw < MIN_BAR_PX && !labels_on; - let mut run: Option = None; + let cull = merge_hairlines(bw, labels_on); + let mut sink = BarSink::new(bw, horizontal, s.color, nslots, k); for (j, sp) in row.iter().enumerate() { let Some((b, t)) = sp else { continue }; let (pb, pt) = (vpos(*b, &sc), vpos(*t, &sc)); let c = center(j); let lo = pb.min(pt); let len = (pb - pt).abs(); - if cull { - let pos = if horizontal { c + nslots * bw / 2.0 - (k + 1.0) * bw } else { c - nslots * bw / 2.0 + k * bw }; - match &mut run { - Some(m) if m.bucket == pos.floor() => m.add(pos, lo, lo + len), - _ => { - if let Some(m) = run.replace(BarRun::new(pos, lo, lo + len)) { - m.draw(ctx, bw, horizontal, s.color); - } - } - } - continue; - } - if horizontal { - let y = c + nslots * bw / 2.0 - (k + 1.0) * bw; - ctx.rect(lo, y, len, bw, s.color, None); - } else { - let x = c - nslots * bw / 2.0 + k * bw; - ctx.rect(x, lo, bw, len, s.color, None); - } + sink.bar(ctx, cull, c, lo, len); if labels_on && let Some(v) = s.values.get(j).copied().flatten() { let t = fmt.format(v, None); let stacked = fam != Fam::Col; @@ -359,9 +393,7 @@ pub(super) fn cartesian(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx } } } - if let Some(m) = run { - m.draw(ctx, bw, horizontal, s.color); - } + sink.finish(ctx); } _ => { let pts: Vec> = row.iter().enumerate().map(|(j, sp)| sp.map(|(_, t)| [center(j), vpos(t, &sc)])).collect(); From 494a7dbb5c0fe57d819b83f1981d62e15d4b14d4 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Fri, 30 Oct 2026 00:54:56 +0100 Subject: [PATCH 3/6] Keep clustered hairline bars of each series in their own strip of the pixel column Co-Authored-By: Claude Sonnet 5.5 --- crates/chart/src/render/cartesian.rs | 32 ++++++++++++++++++++------ crates/chart/src/tests/budget.rs | 34 ++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/crates/chart/src/render/cartesian.rs b/crates/chart/src/render/cartesian.rs index 78774b078..1d643c21f 100644 --- a/crates/chart/src/render/cartesian.rs +++ b/crates/chart/src/render/cartesian.rs @@ -59,24 +59,32 @@ struct BarRun { a1: f32, lo: f32, hi: f32, + /// More than one bar was merged. + merged: bool, } impl BarRun { fn new(pos: f32, lo: f32, hi: f32) -> Self { - BarRun { bucket: pos.floor(), a0: pos, a1: pos, lo, hi } + BarRun { bucket: pos.floor(), a0: pos, a1: pos, lo, hi, merged: false } } fn add(&mut self, pos: f32, lo: f32, hi: f32) { + self.merged = true; self.a0 = self.a0.min(pos); self.a1 = self.a1.max(pos); self.lo = self.lo.min(lo); self.hi = self.hi.max(hi); } - fn draw(&self, ctx: &mut Ctx, bw: f32, horizontal: bool, color: Rgba) { - let along = self.a1 - self.a0 + bw; + /// `band` is the part of the pixel column reserved for this series (`start`, `len`): in a + /// clustered chart a merged run stays inside it, so the other series keep a visible strip. + fn draw(&self, ctx: &mut Ctx, bw: f32, horizontal: bool, color: Rgba, band: Option<(f32, f32)>) { + let (a0, along) = match band { + Some((start, len)) if self.merged => (self.bucket + start, len), + _ => (self.a0, self.a1 - self.a0 + bw), + }; if horizontal { - ctx.rect(self.lo, self.a0, self.hi - self.lo, along, color, None); + ctx.rect(self.lo, a0, self.hi - self.lo, along, color, None); } else { - ctx.rect(self.a0, self.lo, along, self.hi - self.lo, color, None); + ctx.rect(a0, self.lo, along, self.hi - self.lo, color, None); } } } @@ -105,10 +113,19 @@ impl BarSink { fn pos(&self, c: f32) -> f32 { if self.horizontal { c + self.nslots * self.bw / 2.0 - (self.k + 1.0) * self.bw } else { c - self.nslots * self.bw / 2.0 + self.k * self.bw } } + /// This series' strip of a pixel column (offset, length) when the columns hold several + /// clustered series, in the direction the slots run. + fn band(&self) -> Option<(f32, f32)> { + (self.nslots > 1.0).then(|| { + let idx = if self.horizontal { self.nslots - 1.0 - self.k } else { self.k }; + (idx / self.nslots, 1.0 / self.nslots) + }) + } /// Draws the bar at category center `c` spanning `lo..lo + len` on the value axis, or, when /// `merge` is set, adds it to the current run. fn bar(&mut self, ctx: &mut Ctx, merge: bool, c: f32, lo: f32, len: f32) { let pos = self.pos(c); + let band = self.band(); if !merge { if self.horizontal { ctx.rect(lo, pos, len, self.bw, self.color, None); @@ -121,14 +138,15 @@ impl BarSink { Some(m) if m.bucket == pos.floor() => m.add(pos, lo, lo + len), run => { if let Some(m) = run.replace(BarRun::new(pos, lo, lo + len)) { - m.draw(ctx, self.bw, self.horizontal, self.color); + m.draw(ctx, self.bw, self.horizontal, self.color, band); } } } } fn finish(self, ctx: &mut Ctx) { + let band = self.band(); if let Some(m) = self.run { - m.draw(ctx, self.bw, self.horizontal, self.color); + m.draw(ctx, self.bw, self.horizontal, self.color, band); } } } diff --git a/crates/chart/src/tests/budget.rs b/crates/chart/src/tests/budget.rs index 1f2e35ab3..54d33aa7d 100644 --- a/crates/chart/src/tests/budget.rs +++ b/crates/chart/src/tests/budget.rs @@ -124,6 +124,40 @@ fn hairline_columns_are_merged_and_wide_ones_are_not() { assert!((1..=320).contains(&n), "{n} bars"); } +#[test] +fn clustered_hairline_series_keep_a_visible_strip() { + // Four series of similar height, 20,000 categories in 400 px: every pixel column holds a + // merged bar of each series, and they must not cover each other. + for kind in [ChartKind::ColumnClustered, ChartKind::BarClustered] { + let horizontal = kind == ChartKind::BarClustered; + let mut c = chart(kind); + c.data_labels = false; + c.legend = LegendPos::None; + let mut d = data_of(4, 20_000); + for (s, series) in d.series.iter_mut().enumerate() { + for (i, v) in series.values.iter_mut().enumerate() { + *v = Some(50.0 + ((i + s) % 3) as f64); + } + } + let prims = render(&c, &d, 400.0, 300.0, &ApproxMeasure); + let along = |r: &(f32, f32, f32, f32)| if horizontal { (r.1, r.1 + r.3) } else { (r.0, r.0 + r.2) }; + let mut strips: Vec<(f32, f32, usize)> = Vec::new(); + for (s, series) in d.series.iter().enumerate() { + strips.extend(series_rects(&prims, series.color).iter().map(|r| (along(r).0, along(r).1, s))); + } + strips.sort_by(|a, b| a.0.total_cmp(&b.0)); + assert!(strips.len() > 400); + // Rectangles of different series never overlap (more than rounding). + for w in strips.windows(2) { + assert!(w[0].2 == w[1].2 || w[1].0 >= w[0].1 - 1e-3, "series {} and {} overlap", w[0].2, w[1].2); + } + // And all four series show up in the same pixel column. + let col = strips[strips.len() / 2].0.floor(); + let seen: std::collections::BTreeSet = strips.iter().filter(|s| s.0.floor() == col).map(|s| s.2).collect(); + assert_eq!(seen.len(), 4, "series visible in pixel column {col}"); + } +} + #[test] fn clean_data_is_rendered_without_a_copy() { let d = data_of(3, 50); From bc1625248e325179f8da7d14f06b99b04c99a8d7 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 16:25:23 +0100 Subject: [PATCH 4/6] Chart: cap category levels in the clean-data check, recompute overflowing moving-average windows, keep disjoint stacked hairline spans apart Co-Authored-By: Claude Sonnet 5.5 --- crates/chart/src/render.rs | 2 ++ crates/chart/src/render/cartesian.rs | 6 +++++- crates/chart/src/tests/budget.rs | 27 +++++++++++++++++++++++++++ crates/chart/src/trend.rs | 15 ++++++++++++++- 4 files changed, 48 insertions(+), 2 deletions(-) diff --git a/crates/chart/src/render.rs b/crates/chart/src/render.rs index 3fa9f8431..3abf8bf14 100644 --- a/crates/chart/src/render.rs +++ b/crates/chart/src/render.rs @@ -141,6 +141,8 @@ fn needs_sanitizing(d: &ChartData) -> bool { let mut trendlines = 0usize; d.palette.len() > MAX_POINTS || long(d.categories.len()) + || d.category_levels.len() > 3 + || d.category_levels.iter().any(|l| l.len() > MAX_POINTS) || d.series.len() > MAX_SERIES || d.series.iter().any(|s| { trendlines += s.trendlines.len(); diff --git a/crates/chart/src/render/cartesian.rs b/crates/chart/src/render/cartesian.rs index 6689bf202..44c7a3dc9 100644 --- a/crates/chart/src/render/cartesian.rs +++ b/crates/chart/src/render/cartesian.rs @@ -67,6 +67,10 @@ impl BarRun { fn new(pos: f32, lo: f32, hi: f32) -> Self { BarRun { bucket: pos.floor(), a0: pos, a1: pos, lo, hi, merged: false } } + /// Whether `lo..hi` touches the run's value span; a disjoint bar would be joined by a filled gap. + fn touches(&self, lo: f32, hi: f32) -> bool { + lo <= self.hi && hi >= self.lo + } fn add(&mut self, pos: f32, lo: f32, hi: f32) { self.merged = true; self.a0 = self.a0.min(pos); @@ -135,7 +139,7 @@ impl BarSink { return; } match &mut self.run { - Some(m) if m.bucket == pos.floor() => m.add(pos, lo, lo + len), + Some(m) if m.bucket == pos.floor() && m.touches(lo, lo + len) => m.add(pos, lo, lo + len), run => { if let Some(m) = run.replace(BarRun::new(pos, lo, lo + len)) { m.draw(ctx, self.bw, self.horizontal, self.color, band); diff --git a/crates/chart/src/tests/budget.rs b/crates/chart/src/tests/budget.rs index 54d33aa7d..6c5a10879 100644 --- a/crates/chart/src/tests/budget.rs +++ b/crates/chart/src/tests/budget.rs @@ -158,6 +158,26 @@ fn clustered_hairline_series_keep_a_visible_strip() { } } +#[test] +fn stacked_hairline_bars_do_not_fill_the_gap_between_disjoint_spans() { + // Series 1 alternates +10 (stacked on the 100) and -10 (below zero): its two spans in one + // pixel column are 100..110 and -10..0, and the 100 units between them must stay empty. + let mut c = chart(ChartKind::ColumnStacked); + c.data_labels = false; + c.legend = LegendPos::None; + let mut d = data_of(2, 20_000); + for v in d.series[0].values.iter_mut() { + *v = Some(100.0); + } + for (i, v) in d.series[1].values.iter_mut().enumerate() { + *v = Some(if i % 2 == 0 { 10.0 } else { -10.0 }); + } + let prims = render(&c, &d, 400.0, 300.0, &ApproxMeasure); + let rects = series_rects(&prims, d.series[1].color); + let tallest = rects.iter().map(|r| r.3).fold(0.0f32, f32::max); + assert!(!rects.is_empty() && tallest < 60.0, "tallest merged bar is {tallest} px"); +} + #[test] fn clean_data_is_rendered_without_a_copy() { let d = data_of(3, 50); @@ -181,6 +201,13 @@ fn clean_data_is_rendered_without_a_copy() { let got = sanitized(&many); assert_eq!(got.series.len(), 255); assert_eq!(*got, sanitize(&many)); + + // Too many category levels take the copying path too. + let mut levels = data_of(3, 50); + levels.category_levels = vec![vec!["a".into()]; 5]; + let got = sanitized(&levels); + assert!(matches!(got, Cow::Owned(_))); + assert_eq!(got.category_levels.len(), 3); } #[test] diff --git a/crates/chart/src/trend.rs b/crates/chart/src/trend.rs index 6b22ed7ad..818c75796 100644 --- a/crates/chart/src/trend.rs +++ b/crates/chart/src/trend.rs @@ -294,7 +294,11 @@ pub(crate) fn moving_average(pts: &[(f64, f64)], period: u32) -> Vec<(f64, f64)> (p - 1..n) .filter_map(|i| { let s = i + 1 - p; - let sum = tail.get(s)? + if s.is_multiple_of(p) { 0.0 } else { *head.get(i)? }; + let mut sum = tail.get(s)? + if s.is_multiple_of(p) { 0.0 } else { *head.get(i)? }; + if !sum.is_finite() { + // A block sum overflowed although the window itself may not: sum it directly. + sum = pts.get(s..=i)?.iter().map(|q| q.1).sum(); + } Some((pts.get(i)?.0, sum / p as f64)) }) .collect() @@ -390,6 +394,15 @@ mod tests { } } + #[test] + fn moving_average_recomputes_a_window_whose_block_sums_overflow() { + let pts: Vec<(f64, f64)> = [0.0, -1e308, 1e308, 1e308, -1e308].iter().enumerate().map(|(i, y)| (i as f64, *y)).collect(); + let fast = moving_average(&pts, 4); + let slow = moving_average_naive(&pts, 4); + assert_eq!(fast, slow); + assert!(fast.iter().all(|q| q.1.is_finite())); + } + /// The trendline fit as it was written with a `Vec` of powers per data point. fn fit_reference(t: &Trendline, pts: &[(f64, f64)]) -> Option { let kind = t.kind; From 6a7cf9cf667956b486a719977106ff1f9152a7c4 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 17:31:27 +0100 Subject: [PATCH 5/6] Chart: bound the category axis by the point cap, not the raw cell count Read only the cells the category levels use and cut the category dimension at the per-series cap, so a multi-level axis keeps all its levels. Co-Authored-By: Claude Sonnet 5.5 --- crates/chart/src/resolve.rs | 36 +++++++++++++++++++++---- crates/chart/src/tests/budget.rs | 45 ++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/crates/chart/src/resolve.rs b/crates/chart/src/resolve.rs index 1ee635a14..bd6849435 100644 --- a/crates/chart/src/resolve.rs +++ b/crates/chart/src/resolve.rs @@ -189,8 +189,29 @@ fn value_text(v: &Value) -> String { /// taller than a row of values) groups its innermost labels, year beside month. Each outer level /// has one entry per category, non-empty where a group starts, as the cells are. fn category_table(wb: &Workbook, sheet: usize, formula: &str, values: &str, limit: usize) -> (Vec, Vec>) { - let vals = eval_list_in(wb, sheet, formula, limit, 0); let cells: Option<(usize, RangeRef)> = parse_ref(wb, sheet, formula); + let value_range = parse_ref(wb, sheet, values).map(|(_, r)| r); + // Bound the category dimension by `limit` and read only the cells the levels use (the + // innermost label line and up to three outer ones), so that every kept category keeps its + // cells in every level. Counting raw cells would cut a two-level axis in the middle. + let levels = cells.and_then(|(_, r)| category_levels(&r, value_range.as_ref())); + let (cells, read) = match (cells, levels) { + (Some((sh, r)), Some(lv)) => { + let cut = u32::try_from(limit.saturating_sub(1)).unwrap_or(u32::MAX); + let outer = lv.outer; + let end = if lv.by_columns { CellRef::new(r.end.row.min(r.start.row.saturating_add(cut)), r.end.col) } else { CellRef::new(r.end.row, r.end.col.min(r.start.col.saturating_add(cut))) }; + let start = if lv.by_columns { CellRef::new(r.start.row, r.start.col.max(end.col.saturating_sub(outer))) } else { CellRef::new(r.start.row.max(end.row.saturating_sub(outer)), r.start.col) }; + let r = RangeRef::new(start, end); + let prefix = formula.rfind('!').and_then(|i| formula.get(..=i)).unwrap_or(""); + let cap = (r.width() as usize).saturating_mul(r.height() as usize); + (Some((sh, r)), Some((format!("{prefix}{r}"), cap))) + } + (c, _) => (c, None), + }; + let vals = match &read { + Some((f, cap)) => eval_list_in(wb, sheet, f, *cap, 0), + None => eval_list_in(wb, sheet, formula, limit, 0), + }; let src = cells.and_then(|(sh, r)| wb.sheet(sh).map(|s| (s, r))); let texts: Vec = vals .iter() @@ -210,8 +231,7 @@ fn category_table(wb: &Workbook, sheet: usize, formula: &str, values: &str, limi }) .collect(); let Some((_, r)) = src else { return (texts, vec![]) }; - let value_range = parse_ref(wb, sheet, values).map(|(_, r)| r); - let Some(levels) = category_levels(&r, value_range.as_ref()) else { return (texts, vec![]) }; + let Some(levels) = levels else { return (texts, vec![]) }; let (w, h) = (r.width().max(1) as usize, r.height().max(1) as usize); if texts.len() != w.saturating_mul(h) { return (texts, vec![]); // truncated or not a plain block: a flat list @@ -252,7 +272,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, _) => { @@ -267,9 +287,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/budget.rs b/crates/chart/src/tests/budget.rs index 6c5a10879..fad349ba6 100644 --- a/crates/chart/src/tests/budget.rs +++ b/crates/chart/src/tests/budget.rs @@ -232,3 +232,48 @@ fn resolve_keeps_every_series_within_the_point_budget() { let counts: Vec = resolve(&wb, 0, &c).series.iter().map(|s| s.trendlines.len()).collect(); assert_eq!(counts, vec![9, 7, 0, 0]); } + +/// 100 series lower the per-series cap to 20,000, below the number of category cells of a +/// two-level axis (#1155). Each kept category keeps its inner label and its outer level. +fn two_level_chart(n: u32, vertical: bool) -> ChartData { + let at = |i: u32, level: u32| if vertical { CellRef::new(i, level) } else { CellRef::new(level, i) }; + let mut wb = Workbook::new(); + if let Some(s) = wb.sheet_mut(0) { + for i in 0..n { + s.set_value(at(i, 0), Value::Text(format!("Y{}", i / 3).into())); + s.set_value(at(i, 1), Value::Text(format!("C{i}").into())); + s.set_value(at(i, 2), Value::Number(f64::from(i))); + } + } + let cats = format!("{}:{}", at(0, 0), at(n - 1, 1)); + let values = format!("{}:{}", at(0, 2), at(n - 1, 2)); + let mut c = chart(ChartKind::ColumnClustered); + c.series = (0..100).map(|_| Series { values: values.clone(), categories: Some(cats.clone()), ..Series::default() }).collect(); + resolve(&wb, 0, &c) +} + +#[test] +fn multi_level_categories_keep_their_levels_under_the_point_cap() { + for vertical in [true, false] { + let d = two_level_chart(15_000, vertical); + assert_eq!(d.series[0].values.len(), 15_000, "vertical {vertical}"); + assert_eq!(d.categories.len(), 15_000, "vertical {vertical}"); + assert_eq!(d.categories[0], "C0"); + assert_eq!(d.categories[14_999], "C14999"); + assert_eq!(d.category_levels.len(), 1, "one outer level, vertical {vertical}"); + assert_eq!(d.category_levels[0].len(), 15_000); + assert_eq!(d.category_levels[0][3], "Y1"); + } +} + +#[test] +fn multi_level_categories_longer_than_the_cap_are_cut_together_with_their_levels() { + // 25,000 categories against a cap of 20,000. + let d = two_level_chart(25_000, true); + let cap = points_per_series(100); + assert_eq!(d.categories.len(), cap); + assert_eq!(d.category_levels.len(), 1); + assert_eq!(d.category_levels[0].len(), cap); + assert_eq!(d.categories[cap - 1], format!("C{}", cap - 1)); + assert_eq!(d.category_levels[0][cap - 3], format!("Y{}", (cap - 3) / 3)); +} From 9ba79c3e3850fc14e4ea12e1c2c76d3ef2952206 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 17:31:41 +0100 Subject: [PATCH 6/6] fmt --- crates/chart/src/resolve.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/chart/src/resolve.rs b/crates/chart/src/resolve.rs index bd6849435..11213a5e7 100644 --- a/crates/chart/src/resolve.rs +++ b/crates/chart/src/resolve.rs @@ -199,8 +199,16 @@ fn category_table(wb: &Workbook, sheet: usize, formula: &str, values: &str, limi (Some((sh, r)), Some(lv)) => { let cut = u32::try_from(limit.saturating_sub(1)).unwrap_or(u32::MAX); let outer = lv.outer; - let end = if lv.by_columns { CellRef::new(r.end.row.min(r.start.row.saturating_add(cut)), r.end.col) } else { CellRef::new(r.end.row, r.end.col.min(r.start.col.saturating_add(cut))) }; - let start = if lv.by_columns { CellRef::new(r.start.row, r.start.col.max(end.col.saturating_sub(outer))) } else { CellRef::new(r.start.row.max(end.row.saturating_sub(outer)), r.start.col) }; + let end = if lv.by_columns { + CellRef::new(r.end.row.min(r.start.row.saturating_add(cut)), r.end.col) + } else { + CellRef::new(r.end.row, r.end.col.min(r.start.col.saturating_add(cut))) + }; + let start = if lv.by_columns { + CellRef::new(r.start.row, r.start.col.max(end.col.saturating_sub(outer))) + } else { + CellRef::new(r.start.row.max(end.row.saturating_sub(outer)), r.start.col) + }; let r = RangeRef::new(start, end); let prefix = formula.rfind('!').and_then(|i| formula.get(..=i)).unwrap_or(""); let cap = (r.width() as usize).saturating_mul(r.height() as usize);