From 568dd5826479d1adc33239dab1e0cd7c3245f8dd Mon Sep 17 00:00:00 2001 From: vangel76 Date: Tue, 13 Oct 2026 21:13:05 +0200 Subject: [PATCH] Charts: a huge trendline forecast no longer collapses the chart (#1148) Co-Authored-By: Claude Sonnet 5.5 --- ROADMAP.md | 1 + crates/chart/src/render.rs | 17 +++++++++--- crates/chart/src/tests.rs | 21 +++++++++++++++ crates/engine/src/cmd/extra.rs | 2 +- crates/model/src/features.rs | 28 ++++++++++++++++++++ crates/xlsx/src/chart.rs | 4 +-- crates/xlsx/src/tests/trendline_roundtrip.rs | 17 ++++++++++++ 7 files changed, 83 insertions(+), 7 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 070552d07..1d4ce08d0 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: a huge trendline forecast no longer collapses the chart: on a category axis it adds at most four times the data in empty slots (at least 32), the line is sampled over the plot only, and forecasts read from XLSX or set by command are clamped, NaN and infinity becoming 0 (#1148) | 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..ea77f226b 100644 --- a/crates/chart/src/render.rs +++ b/crates/chart/src/render.rs @@ -658,6 +658,13 @@ fn forecast(v: f64) -> f64 { if v.is_finite() && v > 0.0 { v.min(1e9) } else { 0.0 } } +/// Most empty category slots a forward forecast may add after `ncat` categories: four times the +/// data (at least 32), and never past `MAX_CATS` in total. A larger forecast would shrink every +/// column to nothing. +fn max_forward_slots(ncat: usize) -> f64 { + (ncat.saturating_mul(4).max(32)).min(MAX_CATS.saturating_sub(ncat)) as f64 +} + /// Longest forward forecast among `s`'s regression trendlines. fn max_forward(s: &SeriesData) -> f64 { s.trendlines.iter().filter(|t| !matches!(t.spec.kind, TrendlineKind::MovingAverage(_))).map(|t| forecast(t.spec.forward)).fold(0.0, f64::max) @@ -748,6 +755,7 @@ fn draw_trendlines( frac: &dyn Fn(f64, f64) -> (f64, f64), px: &dyn Fn((f64, f64)) -> [f32; 2], labels: &mut Vec, + window: (f64, f64), ) { let Some((xlo, xhi)) = value_range(pts.iter().map(|p| p.0)) else { return }; for t in s.trendlines.iter().take(MAX_TRENDLINES) { @@ -756,7 +764,8 @@ fn draw_trendlines( TrendlineKind::MovingAverage(p) => moving_average(pts, p).into_iter().map(|(x, y)| Some(frac(x, y))).collect(), _ => { let Some(fit) = fit_trendline(spec, pts) else { continue }; - let (lo, hi) = (xlo - forecast(spec.backward), xhi + forecast(spec.forward)); + // Sample only the part of the forecast that is on the plot. + let (lo, hi) = ((xlo - forecast(spec.backward)).max(window.0), (xhi + forecast(spec.forward)).min(window.1)); if spec.display_equation { labels.push(fit.equation()); } @@ -936,7 +945,7 @@ fn cartesian(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx, horizonta .filter(|(i, _)| kinds.get(*i).is_some_and(|k| supports_trendline(*k))) .map(|(_, s)| max_forward(s)) .fold(0.0, f64::max); - let ncat_slots = ncat.saturating_add(fwd.ceil() as usize).min(MAX_CATS.max(ncat)); + let ncat_slots = ncat.saturating_add(fwd.min(max_forward_slots(ncat)).ceil() as usize).min(MAX_CATS.max(ncat)); let labels: Vec = (0..ncat_slots).map(|j| if j < ncat { cat_label(data, j) } else { String::new() }).collect(); let plot = frame(ctx, area, CatAxis::Labels(&labels), &y1, y2.as_ref(), horizontal, chart.gridlines); if !plot.ok() { @@ -1102,7 +1111,7 @@ fn cartesian(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx, horizonta [plot.x + fx as f32 * plot.w, plot.bottom() - fy as f32 * plot.h] } }; - draw_trendlines(ctx, s, &pts, &frac, &px, &mut trend_labels); + draw_trendlines(ctx, s, &pts, &frac, &px, &mut trend_labels, (0.5, ncat_slots as f64 + 0.5)); } draw_trend_labels(ctx, plot, &trend_labels); } @@ -1196,7 +1205,7 @@ fn scatter(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx) { let pts: Vec<(f64, f64)> = pts.iter().flatten().copied().collect(); let frac = |x: f64, y: f64| ((x - xs.min) / (xs.max - xs.min), (y - ys.min) / (ys.max - ys.min)); let px = |(fx, fy): (f64, f64)| [plot.x + fx as f32 * plot.w, plot.bottom() - fy as f32 * plot.h]; - draw_trendlines(ctx, s, &pts, &frac, &px, &mut trend_labels); + draw_trendlines(ctx, s, &pts, &frac, &px, &mut trend_labels, (xs.min, xs.max)); } draw_trend_labels(ctx, plot, &trend_labels); } diff --git a/crates/chart/src/tests.rs b/crates/chart/src/tests.rs index 01f1aac49..a95368424 100644 --- a/crates/chart/src/tests.rs +++ b/crates/chart/src/tests.rs @@ -1057,3 +1057,24 @@ fn area_charts_draw_source_value_labels_only_when_enabled() { assert!(first_label > last_poly, "{kind:?}: label drawn under a later area"); } } + +#[test] +fn a_huge_trendline_forecast_does_not_collapse_the_columns() { + let m = ApproxMeasure; + let tc: Rgba = [1, 2, 3, 255]; + let col: Rgba = [10, 20, 30, 255]; + let mut s = series("Sales", &[1.0, 3.0, 2.0, 5.0, 4.0], col, ChartKind::ColumnClustered); + s.trendlines = vec![TrendlineData { spec: Trendline { forward: 1e6, backward: 1e6, ..Trendline::default() }, color: tc }]; + let d = ChartData { palette: vec![], categories: ["a", "b", "c", "d", "e"].map(String::from).to_vec(), series: vec![s] }; + let mut c = chart(ChartKind::ColumnClustered); + c.legend = LegendPos::None; + let prims = render(&c, &d, 600.0, 400.0, &m); + assert_sane(&prims, 600.0, 400.0); + let widths: Vec = + prims.iter().filter_map(|p| if let Prim::Rect { w, fill: Some(f), .. } = p { (*f == col).then_some(*w) } else { None }).collect(); + assert_eq!(widths.len(), 5); + assert!(widths.iter().all(|w| *w > 4.0), "columns stay visible: {widths:?}"); + let trend = lines_of(&prims, tc); + assert_eq!(trend.len(), 1); + assert!(trend[0].len() >= 20, "the trendline is sampled over the plot, not the forecast: {}", trend[0].len()); +} diff --git a/crates/engine/src/cmd/extra.rs b/crates/engine/src/cmd/extra.rs index 01e2ac6e6..360c8747a 100644 --- a/crates/engine/src/cmd/extra.rs +++ b/crates/engine/src/cmd/extra.rs @@ -351,7 +351,7 @@ fn add_element(s: &mut Session, p: &Json) -> Result { /// The trendline described by `chart.addElement`'s trendline parameters (Excel's Add Chart /// Element ▸ Trendline choices, plus the Format Trendline options). fn trendline_param(p: &Json) -> Result { - let num = |k: &str| f64_param(p, k).filter(|v| v.is_finite() && *v >= 0.0).unwrap_or(0.0); + let num = |k: &str| Trendline::clamp_forecast(f64_param(p, k).unwrap_or(0.0)); let mut t = Trendline { forward: num("forward"), backward: num("backward"), diff --git a/crates/model/src/features.rs b/crates/model/src/features.rs index b51f7bc98..73ce5e8d2 100644 --- a/crates/model/src/features.rs +++ b/crates/model/src/features.rs @@ -482,6 +482,18 @@ pub struct Trendline { pub dash: Option, } +impl Trendline { + /// Longest forward / backward forecast kept from a file or a command, in x units. Excel's own + /// limit is unconfirmed; the renderer additionally bounds the forecast by the plot's slot budget. + pub const MAX_FORECAST: f64 = 100_000.0; + + /// A forecast distance as stored: finite, not negative (NaN and infinity become 0) and at most + /// [`Self::MAX_FORECAST`]. + pub fn clamp_forecast(v: f64) -> f64 { + if v.is_finite() && v > 0.0 { v.min(Self::MAX_FORECAST) } else { 0.0 } + } +} + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct Chart { pub id: u32, @@ -1046,3 +1058,19 @@ impl Default for PivotTable { } } } + +#[cfg(test)] +mod trendline_forecast_tests { + use super::Trendline; + + #[test] + fn clamp_forecast_bounds_hostile_values() { + assert_eq!(Trendline::clamp_forecast(2.5), 2.5); + assert_eq!(Trendline::clamp_forecast(0.0), 0.0); + assert_eq!(Trendline::clamp_forecast(-4.0), 0.0); + assert_eq!(Trendline::clamp_forecast(f64::NAN), 0.0); + assert_eq!(Trendline::clamp_forecast(f64::INFINITY), 0.0); + assert_eq!(Trendline::clamp_forecast(f64::NEG_INFINITY), 0.0); + assert_eq!(Trendline::clamp_forecast(1e300), Trendline::MAX_FORECAST); + } +} diff --git a/crates/xlsx/src/chart.rs b/crates/xlsx/src/chart.rs index dee33c83d..837b31b27 100644 --- a/crates/xlsx/src/chart.rs +++ b/crates/xlsx/src/chart.rs @@ -151,8 +151,8 @@ fn read_trendline(t: &El) -> Option { Some(Trendline { kind, name: t.child("name").map(|n| n.text.clone()), - forward: f64_val(t, "forward").unwrap_or(0.0).max(0.0), - backward: f64_val(t, "backward").unwrap_or(0.0).max(0.0), + forward: Trendline::clamp_forecast(f64_val(t, "forward").unwrap_or(0.0)), + backward: Trendline::clamp_forecast(f64_val(t, "backward").unwrap_or(0.0)), intercept: f64_val(t, "intercept"), display_equation: bool_val(t, "dispEq"), display_r_squared: bool_val(t, "dispRSqr"), diff --git a/crates/xlsx/src/tests/trendline_roundtrip.rs b/crates/xlsx/src/tests/trendline_roundtrip.rs index d954c9bc6..92a1da18d 100644 --- a/crates/xlsx/src/tests/trendline_roundtrip.rs +++ b/crates/xlsx/src/tests/trendline_roundtrip.rs @@ -169,3 +169,20 @@ fn excel_style_trendlines_are_read() { assert_eq!(t[0].color, Some(Color::Theme(5, 0))); assert_eq!(t[1].kind, TrendlineKind::MovingAverage(2), "period clamps to 2..255"); } + +#[test] +fn a_huge_forecast_in_a_file_is_read_clamped() { + let t = |f: &str, b: &str| { + format!( + r#"Data!$B$2:$B$6"# + ) + }; + let read = |f: &str, b: &str| { + let root = crate::xml::parse(t(f, b).as_bytes()).unwrap(); + crate::chart::read_chart(&root).unwrap().series[0].trendlines[0].clone() + }; + let big = read("1000000", "1e300"); + assert_eq!((big.forward, big.backward), (Trendline::MAX_FORECAST, Trendline::MAX_FORECAST)); + let small = read("2.5", "-3"); + assert_eq!((small.forward, small.backward), (2.5, 0.0), "ordinary values are kept"); +}