Review fix-up: uniform, plot-clamped spline smoothing

smooth_run gives every segment the same number of sub-segments (cap / n, 1..100) so long
series are smoothed end to end within the point cap, and clamps the Catmull-Rom curve to
the plot box. List "smooth" in chart.addElement's params doc, drop the smooth_demo
example, and use the test sheet's own name in series refs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Brandon Thomas 2026-10-10 12:21:35 -04:00
parent 435a750bba
commit c975b5dc75
4 changed files with 54 additions and 96 deletions

View File

@ -546,19 +546,19 @@ fn decimate(pts: Vec<[f32; 2]>, max: usize) -> Vec<[f32; 2]> {
out
}
/// Draws a polyline split at gaps (`None`), decimated when long. When `smooth`, each run is
/// interpolated with a Catmull-Rom spline so the line curves through the data points.
fn draw_runs(ctx: &mut Ctx, pts: &[Option<[f32; 2]>], color: Rgba, width: f32, smooth: bool) {
/// Draws a polyline split at gaps (`None`), decimated when long. With `smooth` (the plot box),
/// each run is interpolated with a Catmull-Rom spline so the line curves through the data points.
fn draw_runs(ctx: &mut Ctx, pts: &[Option<[f32; 2]>], color: Rgba, width: f32, smooth: Option<Bx>) {
let mut run: Vec<[f32; 2]> = Vec::new();
for p in pts.iter().chain(std::iter::once(&None)) {
match p {
Some(p) => run.push(*p),
None => {
if run.len() >= 2 {
let r = if smooth {
smooth_run(decimate(std::mem::take(&mut run), MAX_LINE_PTS))
} else {
decimate(std::mem::take(&mut run), MAX_LINE_PTS)
let r = decimate(std::mem::take(&mut run), MAX_LINE_PTS);
let r = match smooth {
Some(plot) => smooth_run(r, plot),
None => r,
};
ctx.line(r, color, width);
} else {
@ -569,31 +569,33 @@ fn draw_runs(ctx: &mut Ctx, pts: &[Option<[f32; 2]>], color: Rgba, width: f32, s
}
}
/// Catmull-Rom spline through `pts`, splitting each segment into `SPLINE_STEPS` sub-segments.
/// Endpoints are duplicated so the curve passes through the first and last points.
fn smooth_run(pts: Vec<[f32; 2]>) -> Vec<[f32; 2]> {
const SPLINE_STEPS: usize = 100;
/// Catmull-Rom spline through `pts`: every segment gets the same number of sub-segments (up to
/// 100), chosen so the whole curve stays within `MAX_LINE_PTS * 2` points. Endpoints are
/// duplicated so the curve passes through the first and last points, and the curve is clamped
/// to the `plot` box (a spline overshoots its data near sharp turns).
fn smooth_run(pts: Vec<[f32; 2]>, plot: Bx) -> Vec<[f32; 2]> {
let n = pts.len();
if n < 3 {
return pts;
}
let cap = n.saturating_mul(SPLINE_STEPS).min(MAX_LINE_PTS * 2);
let mut out: Vec<[f32; 2]> = Vec::with_capacity(cap);
let cap = MAX_LINE_PTS * 2;
let steps = (cap / n).clamp(1, 100);
let mut out: Vec<[f32; 2]> = Vec::with_capacity(n.saturating_mul(steps).min(cap) + 1);
let at = |i: isize| -> [f32; 2] {
let j = i.clamp(0, n as isize - 1) as usize;
pts.get(j).copied().unwrap_or([0.0, 0.0])
};
let (x0, x1) = (plot.x.min(plot.right()), plot.x.max(plot.right()));
let (y0, y1) = (plot.y.min(plot.bottom()), plot.y.max(plot.bottom()));
let clip = |p: [f32; 2]| [p[0].clamp(x0, x1), p[1].clamp(y0, y1)];
for i in 0..n.saturating_sub(1) {
let (p0, p1, p2, p3) = (at(i as isize - 1), at(i as isize), at(i as isize + 1), at(i as isize + 2));
for s in 0..SPLINE_STEPS {
let t = s as f32 / SPLINE_STEPS as f32;
out.push(catmull(p0, p1, p2, p3, t));
if out.len() >= cap {
break;
}
for s in 0..steps {
let t = s as f32 / steps as f32;
out.push(clip(catmull(p0, p1, p2, p3, t)));
}
}
out.push(at(n as isize - 1));
out.push(clip(at(n as isize - 1)));
out
}
@ -882,7 +884,7 @@ fn cartesian(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx, horizonta
}
_ => {
let pts: Vec<Option<[f32; 2]>> = row.iter().enumerate().map(|(j, sp)| sp.map(|(_, t)| [center(j), vpos(t, &sc)])).collect();
draw_runs(ctx, &pts, s.color, 2.25, s.smooth);
draw_runs(ctx, &pts, s.color, 2.25, s.smooth.then_some(plot));
let k = kinds.get(i).copied().unwrap_or(ChartKind::Line);
if k == ChartKind::LineMarkers && cw >= 3.0 {
draw_markers(ctx, pts.iter().flatten().copied(), 3.5, s.color);
@ -960,7 +962,7 @@ fn scatter(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx) {
} else {
let lines = chart.kind == ChartKind::ScatterLines || s.kind == ChartKind::ScatterLines;
if lines {
draw_runs(ctx, &px, s.color, 2.0, s.smooth);
draw_runs(ctx, &px, s.color, 2.0, s.smooth.then_some(plot));
}
if !lines || px.len() <= 500 {
draw_markers(ctx, px.iter().flatten().copied(), 3.5, s.color);
@ -1514,3 +1516,29 @@ pub(crate) fn finalize(prims: Vec<Prim>, w: f32, h: f32) -> Vec<Prim> {
})
.collect()
}
#[cfg(test)]
mod smooth_tests {
use super::*;
#[test]
fn long_runs_are_smoothed_uniformly_within_the_cap() {
let plot = Bx { x: 0.0, y: 0.0, w: 3000.0, h: 100.0 };
let pts: Vec<[f32; 2]> = (0..1500).map(|i| [i as f32 * 2.0, if i % 2 == 0 { 20.0 } else { 80.0 }]).collect();
let out = smooth_run(pts, plot);
assert!(out.len() <= MAX_LINE_PTS * 2 + 1, "{}", out.len());
// Two sub-segments per segment all the way to the end, not 100 each and then a straight tail.
assert_eq!(out.len(), 1499 * 2 + 1);
assert_eq!(out.last().copied(), Some([2998.0, 80.0]));
}
#[test]
fn spline_overshoot_is_clamped_to_the_plot() {
let plot = Bx { x: 0.0, y: 10.0, w: 300.0, h: 100.0 };
// A step right at the plot edges: Catmull-Rom overshoots past both.
let pts = vec![[0.0, 110.0], [100.0, 110.0], [110.0, 10.0], [200.0, 10.0], [300.0, 10.0]];
let out = smooth_run(pts, plot);
assert!(out.len() > 5);
assert!(out.iter().all(|p| (0.0..=300.0).contains(&p[0]) && (10.0..=110.0).contains(&p[1])));
}
}

View File

@ -1,70 +0,0 @@
use gridcraft_engine::Session;
use serde_json::json;
fn main() -> Result<(), Box<dyn std::error::Error>> {
let mut session = Session::new();
session.new_workbook();
// Add data: months and sales figures with some curves
let months = ["Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"];
let sales = [45.0, 52.0, 48.0, 65.0, 58.0, 72.0, 68.0, 75.0, 70.0, 82.0, 78.0, 85.0];
let expenses = [30.0, 35.0, 32.0, 42.0, 38.0, 45.0, 43.0, 48.0, 46.0, 52.0, 50.0, 55.0];
// Headers
session.execute("cell.set", json!({"cell": "A1", "input": "Month"}))?;
session.execute("cell.set", json!({"cell": "B1", "input": "Sales"}))?;
session.execute("cell.set", json!({"cell": "C1", "input": "Expenses"}))?;
// Data
for (i, ((month, sale), expense)) in months.iter().zip(&sales).zip(&expenses).enumerate() {
let row = i + 2;
session.execute("cell.set", json!({"cell": format!("A{}", row), "input": month}))?;
session.execute("cell.set", json!({"cell": format!("B{}", row), "input": sale.to_string()}))?;
session.execute("cell.set", json!({"cell": format!("C{}", row), "input": expense.to_string()}))?;
}
// Create first chart (straight lines)
session.execute(
"insert.chart",
json!({
"type": "line",
"subtype": "markers",
"range": "A1:C13",
"title": "Straight Lines (default)",
"at": "E2"
}),
)?;
// Create second chart (smooth lines)
session.execute(
"insert.chart",
json!({
"type": "line",
"subtype": "markers",
"range": "A1:C13",
"title": "Smooth Curves",
"at": "E18"
}),
)?;
// Enable smoothing on the second chart
session.execute(
"chart.addElement",
json!({
"chart": 2,
"element": "smooth",
"on": true
}),
)?;
// Save the file
session.execute("file.saveAs", json!({"path": "smooth_demo.xlsx"}))?;
println!("Created smooth_demo.xlsx with two charts:");
println!(" - Chart 1: Straight lines (smooth=false)");
println!(" - Chart 2: Smooth curves (smooth=true)");
println!("\nTo view it:");
println!(" cargo run --release -p gridcraft -- smooth_demo.xlsx");
Ok(())
}

View File

@ -15,7 +15,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Add Chart Element",
["Chart Design", "Chart Layouts"],
None,
"{chart?, element: title|legend|dataLabels|gridlines|axisTitles, on?: bool, position?: bottom|top|left|right}",
"{chart?, element: title|legend|dataLabels|gridlines|axisTitles|smooth, on?: bool, position?: bottom|top|left|right}",
has_doc,
add_element
),

View File

@ -10,9 +10,9 @@ fn smooth_line_setting_round_trips() {
let mut wb = Workbook::default();
let mut s = Sheet::new("Data");
let base_series = Series {
name: Some("Sheet1!$A$1".into()),
categories: Some("Sheet1!$A$2:$A$5".into()),
values: "Sheet1!$B$2:$B$5".into(),
name: Some("Data!$A$1".into()),
categories: Some("Data!$A$2:$A$5".into()),
values: "Data!$B$2:$B$5".into(),
bubble_sizes: None,
color: Some(Color::Rgb(0x4472C4)),
secondary: false,