Merge pull request #62 from justinjohn0306/fix/smooth-line-charts

Line charts: smooth (spline) curves (issue #52)

# Conflicts:
#	crates/xlsx/src/tests/mod.rs

crates/chart/src/lib.rs

@@ -138,6 +138,8 @@ pub struct SeriesData {
     pub kind: ChartKind,
     /// Plotted against the secondary (right) value axis.
     pub secondary: bool,
+    /// Draw line segments as a smoothed (spline) curve.
+    pub smooth: bool,
     /// Number format code of the source values ("General" by default).
     pub number_format: String,
 }
@@ -152,6 +154,7 @@ impl Default for SeriesData {
             color: [0x15, 0x60, 0x82, 0xFF],
             kind: ChartKind::ColumnClustered,
             secondary: false,
+            smooth: false,
             number_format: "General".into(),
         }
     }

crates/chart/src/render.rs

@@ -194,6 +194,7 @@ fn sanitize(d: &ChartData) -> ChartData {
                 color: s.color,
                 kind: s.kind,
                 secondary: s.secondary,
+                smooth: s.smooth,
                 number_format: s.number_format.clone(),
             })
             .collect(),
@@ -545,8 +546,9 @@ fn decimate(pts: Vec<[f32; 2]>, max: usize) -> Vec<[f32; 2]> {
     out
 }
 
-/// Draws a polyline split at gaps (`None`), decimated when long.
-fn draw_runs(ctx: &mut Ctx, pts: &[Option<[f32; 2]>], color: Rgba, width: f32) {
+/// 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 {
@@ -554,6 +556,10 @@ fn draw_runs(ctx: &mut Ctx, pts: &[Option<[f32; 2]>], color: Rgba, width: f32) {
             None => {
                 if run.len() >= 2 {
                     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 {
                     run.clear();
@@ -563,6 +569,48 @@ fn draw_runs(ctx: &mut Ctx, pts: &[Option<[f32; 2]>], color: Rgba, width: f32) {
     }
 }
 
+/// 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 = 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..steps {
+            let t = s as f32 / steps as f32;
+            out.push(clip(catmull(p0, p1, p2, p3, t)));
+        }
+    }
+    out.push(clip(at(n as isize - 1)));
+    out
+}
+
+/// One Catmull-Rom point between `p1` and `p2` (uniform parameterisation), tangent-scaled by 0.5.
+fn catmull(p0: [f32; 2], p1: [f32; 2], p2: [f32; 2], p3: [f32; 2], t: f32) -> [f32; 2] {
+    let t2 = t * t;
+    let t3 = t2 * t;
+    let mut o = [0.0f32; 2];
+    for k in 0..2 {
+        let (a0, a1, a2, a3) = (p0[k], p1[k], p2[k], p3[k]);
+        o[k] = 0.5 * ((2.0 * a1) + (-a0 + a2) * t + (2.0 * a0 - 5.0 * a1 + 4.0 * a2 - a3) * t2 + (-a0 + 3.0 * a1 - 3.0 * a2 + a3) * t3);
+    }
+    o
+}
+
 fn draw_markers(ctx: &mut Ctx, pts: impl Iterator<Item = [f32; 2]>, r: f32, color: Rgba) {
     let mut seen = HashSet::new();
     for p in pts {
@@ -836,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);
+                    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);
@@ -914,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);
+                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);
@@ -1468,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])));
+    }
+}

crates/chart/src/resolve.rs

@@ -166,7 +166,17 @@ pub fn resolve(wb: &Workbook, sheet: usize, chart: &Chart) -> ChartData {
             }
         }
         let sizes = s.bubble_sizes.as_deref().map(|f| nums(wb, sheet, f));
-        series.push(SeriesData { name, values, x, sizes, color, kind, secondary: s.secondary, number_format: ref_format(wb, sheet, &s.values) });
+        series.push(SeriesData {
+            name,
+            values,
+            x,
+            sizes,
+            color,
+            kind,
+            secondary: s.secondary,
+            smooth: s.smooth,
+            number_format: ref_format(wb, sheet, &s.values),
+        });
     }
     let n = series.iter().map(|s| s.values.len()).max().unwrap_or(0);
     let categories = categories.unwrap_or_else(|| (1..=n).map(|i| i.to_string()).collect());

crates/chart/src/tests.rs

@@ -60,6 +60,7 @@ fn series(name: &str, values: &[f64], color: Rgba, kind: ChartKind) -> SeriesDat
         color,
         kind,
         secondary: false,
+        smooth: false,
         number_format: "$#,##0".into(),
     }
 }
@@ -223,6 +224,7 @@ fn resolve_from_workbook() {
             color: None,
             secondary: false,
             kind: None,
+            smooth: false,
         },
         Series {
             name: None,
@@ -232,6 +234,7 @@ fn resolve_from_workbook() {
             color: Some(Color::rgb(1, 2, 3)),
             secondary: true,
             kind: None,
+            smooth: false,
         },
         Series {
             name: Some("Literal".into()),
@@ -241,6 +244,7 @@ fn resolve_from_workbook() {
             color: None,
             secondary: false,
             kind: None,
+            smooth: false,
         },
     ];
     let d = resolve(&wb, 0, &c);
@@ -269,8 +273,26 @@ fn resolve_from_workbook() {
     // Combo kinds and scatter X values.
     let mut c = chart(ChartKind::Combo);
     c.series = vec![
-        Series { name: None, categories: None, values: "Sales!B2:B4".into(), bubble_sizes: None, color: None, secondary: false, kind: None },
-        Series { name: None, categories: None, values: "Sales!B2:B4".into(), bubble_sizes: None, color: None, secondary: true, kind: None },
+        Series {
+            name: None,
+            categories: None,
+            values: "Sales!B2:B4".into(),
+            bubble_sizes: None,
+            color: None,
+            secondary: false,
+            kind: None,
+            smooth: false,
+        },
+        Series {
+            name: None,
+            categories: None,
+            values: "Sales!B2:B4".into(),
+            bubble_sizes: None,
+            color: None,
+            secondary: true,
+            kind: None,
+            smooth: false,
+        },
     ];
     let d = resolve(&wb, 0, &c);
     assert_eq!(d.series[0].kind, ChartKind::ColumnClustered);
@@ -284,6 +306,7 @@ fn resolve_from_workbook() {
         color: None,
         secondary: false,
         kind: None,
+        smooth: false,
     }];
     let d = resolve(&wb, 0, &c);
     assert_eq!(d.series[0].x.as_ref().map(|x| x.len()), Some(3));
@@ -299,6 +322,7 @@ fn resolve_from_workbook() {
         color: None,
         secondary: false,
         kind: None,
+        smooth: false,
     }];
     let d = resolve(&wb, 7, &c);
     assert_eq!(d.series.len(), 1);
@@ -473,8 +497,51 @@ fn thousands_of_points_are_decimated() {
     }
 }
 
+#[test]
+fn smooth_line_adds_interpolated_points_and_keeps_endpoints() {
+    let m = ApproxMeasure;
+    let cats = ["a", "b", "c", "d"].map(String::from).to_vec();
+    let line_pts = |smooth: bool, m: &ApproxMeasure| -> usize {
+        let mut s = series("L", &[1.0, 5.0, 2.0, 6.0], [10, 20, 30, 255], ChartKind::Line);
+        s.smooth = smooth;
+        let d = ChartData { categories: cats.clone(), series: vec![s] };
+        render(&chart(ChartKind::Line), &d, 600.0, 400.0, m)
+            .iter()
+            .filter_map(|p| match p {
+                Prim::Line { pts, .. } => Some(pts.len()),
+                _ => None,
+            })
+            .max()
+            .unwrap_or(0)
+    };
+    let straight = line_pts(false, &m);
+    let curved = line_pts(true, &m);
+    assert_eq!(straight, 4, "straight polyline keeps one point per datum");
+    assert!(curved > straight, "smooth line interpolates extra points: {curved} vs {straight}");
+    // Rendering either way must stay finite and inside the canvas.
+    let mut s = series("L", &[1.0, 5.0, 2.0, 6.0], [10, 20, 30, 255], ChartKind::Line);
+    s.smooth = true;
+    let d = ChartData { categories: cats, series: vec![s] };
+    assert_sane(&render(&chart(ChartKind::Line), &d, 600.0, 400.0, &m), 600.0, 400.0);
+}
+
+#[test]
+fn smooth_line_splits_at_gaps() {
+    let m = ApproxMeasure;
+    let mut s = series("L", &[1.0, 5.0, 2.0, 6.0], [10, 20, 30, 255], ChartKind::Line);
+    s.smooth = true;
+    s.values[1] = None; // gap between two data points
+    let d = ChartData { categories: vec!["a".into(), "b".into(), "c".into(), "d".into()], series: vec![s] };
+    let prims = render(&chart(ChartKind::Line), &d, 600.0, 400.0, &m);
+    // Two separate runs (before/after the gap) → at least two line primitives.
+    let lines = prims.iter().filter(|p| matches!(p, Prim::Line { .. })).count();
+    assert!(lines >= 2, "a gap must split the smoothed line into runs, got {lines}");
+    assert_sane(&prims, 600.0, 400.0);
+}
+
 /// Small deterministic PRNG (xorshift) for the fuzz test.
 struct Rng(u64);
+
 impl Rng {
     fn next(&mut self) -> u64 {
         self.0 ^= self.0 << 13;
@@ -514,6 +581,7 @@ fn fuzz_random_data_and_sizes() {
                 color: [(r.next() % 256) as u8, 100, 50, 255],
                 kind: ALL_KINDS[(r.next() % ALL_KINDS.len() as u64) as usize],
                 secondary: r.next().is_multiple_of(3),
+                smooth: r.next().is_multiple_of(4),
                 number_format: ["General", "$#,##0", "0.00%", "yyyy-mm-dd", "#,##0.0;[Red]-#,##0.0", "@"][(r.next() % 6) as usize].into(),
             })
             .collect();

crates/engine/src/cmd/extra.rs

@@ -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
         ),
@@ -291,6 +291,11 @@ fn add_element(s: &mut Session, p: &Json) -> Result<Json> {
                 c.x_title = on.then(|| c.x_title.clone().unwrap_or_else(|| "Axis Title".into()));
                 c.y_title = on.then(|| c.y_title.clone().unwrap_or_else(|| "Axis Title".into()));
             }
+            "smooth" => {
+                for s in &mut c.series {
+                    s.smooth = on;
+                }
+            }
             other => return Err(bad("chart.addElement", format!("unknown element `{other}`"))),
         }
         Ok(())

crates/engine/src/cmd/insert.rs

@@ -509,6 +509,7 @@ pub fn series_from_range(sh: &Sheet, r: RangeRef, by_rows: bool) -> Vec<Series>
                 color: None,
                 secondary: false,
                 kind: None,
+                smooth: false,
             });
         }
     } else {
@@ -522,6 +523,7 @@ pub fn series_from_range(sh: &Sheet, r: RangeRef, by_rows: bool) -> Vec<Series>
                 color: None,
                 secondary: false,
                 kind: None,
+                smooth: false,
             });
         }
     }

crates/engine/src/sample.rs

@@ -108,6 +108,7 @@ fn sales(s: &mut Session) -> bool {
                 color: None,
                 secondary: false,
                 kind: None,
+                smooth: false,
             }];
         }
     }

crates/model/src/features.rs

@@ -352,6 +352,9 @@ pub struct Series {
     /// Combo charts: this series drawn as a line on the secondary axis.
     pub secondary: bool,
     pub kind: Option<ChartKind>,
+    /// Draw line segments as a smoothed (spline) curve.
+    #[serde(default)]
+    pub smooth: bool,
 }
 
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]

crates/xlsx/src/chart.rs

@@ -106,6 +106,7 @@ fn read_series(s: &El) -> Series {
         color,
         secondary: false,
         kind: None,
+        smooth: s.child("smooth").and_then(|e| e.attr("val")).and_then(|v| v.parse::<u8>().ok()).is_some_and(|v| v != 0),
     }
 }
 
@@ -349,7 +350,7 @@ fn series_xml(s: &Series, idx: usize, g: G, wb: &Workbook, si: usize) -> String
                 x.push_str(&num_data("bubbleSize", s.bubble_sizes.as_deref().or(Some(&s.values)), wb, si));
                 x.push_str("<c:bubble3D val=\"0\"/>");
             } else {
-                x.push_str("<c:smooth val=\"0\"/>");
+                let _ = write!(x, "<c:smooth val=\"{}\"/>", s.smooth as u8);
             }
         }
         _ => {
@@ -358,7 +359,7 @@ fn series_xml(s: &Series, idx: usize, g: G, wb: &Workbook, si: usize) -> String
             }
             x.push_str(&num_data("val", Some(&s.values), wb, si));
             if matches!(g, G::Line { .. }) {
-                x.push_str("<c:smooth val=\"0\"/>");
+                let _ = write!(x, "<c:smooth val=\"{}\"/>", s.smooth as u8);
             }
         }
     }

crates/xlsx/src/tests/mod.rs

@@ -6,6 +6,7 @@ mod malformed;
 mod ods_tests;
 mod pivot_tests;
 mod roundtrip;
+mod smooth_roundtrip;
 mod xlsb_tests;
 
 use std::io::Write;

crates/xlsx/src/tests/roundtrip.rs

@@ -287,6 +287,7 @@ fn sample() -> Workbook {
             color: Some(Color::Rgb(0xFF0000)),
             secondary: false,
             kind: None,
+            smooth: false,
         }],
         legend: LegendPos::Bottom,
         data_labels: true,
@@ -324,6 +325,7 @@ fn sample() -> Workbook {
         kind: Some(ChartKind::Line),
         secondary: true,
         color: None,
+        smooth: false,
         ..base_chart.series[0].clone()
     });
     s.charts.push(combo);

crates/xlsx/src/tests/smooth_roundtrip.rs

@@ -0,0 +1,62 @@
+//! Test that XLSX smooth line settings round-trip correctly.
+
+use gridcraft_model::{Anchor, Chart, ChartKind, Color, LegendPos, Series, Sheet, Workbook};
+use std::sync::Arc;
+
+use crate::{read_xlsx, write_xlsx};
+
+#[test]
+fn smooth_line_setting_round_trips() {
+    let mut wb = Workbook::default();
+    let mut s = Sheet::new("Data");
+    let base_series = Series {
+        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,
+        kind: None,
+        smooth: false,
+    };
+    s.charts.push(Chart {
+        id: 1,
+        kind: ChartKind::Line,
+        anchor: Anchor { cell: gridcraft_core::CellRef::new(0, 3), width: 480.0, height: 300.0, ..Anchor::default() },
+        title: Some("Straight Line".into()),
+        series: vec![base_series.clone()],
+        legend: LegendPos::Bottom,
+        data_labels: false,
+        gridlines: true,
+        style: 2,
+        x_title: None,
+        y_title: None,
+        source: None,
+        by_rows: false,
+    });
+    let mut smooth_series = base_series.clone();
+    smooth_series.smooth = true;
+    s.charts.push(Chart {
+        id: 2,
+        kind: ChartKind::ScatterLines,
+        anchor: Anchor { cell: gridcraft_core::CellRef::new(15, 3), width: 480.0, height: 300.0, ..Anchor::default() },
+        title: Some("Smooth Scatter".into()),
+        series: vec![smooth_series],
+        legend: LegendPos::Bottom,
+        data_labels: false,
+        gridlines: true,
+        style: 2,
+        x_title: None,
+        y_title: None,
+        source: None,
+        by_rows: false,
+    });
+    wb.sheets = vec![Arc::new(s)];
+    let bytes = write_xlsx(&wb).unwrap();
+    let (back, _rep) = read_xlsx(&bytes).unwrap();
+    assert_eq!(back.sheets.len(), 1);
+    let sh = &back.sheets[0];
+    assert_eq!(sh.charts.len(), 2);
+    assert!(!sh.charts[0].series[0].smooth, "straight line series should have smooth=false");
+    assert!(sh.charts[1].series[0].smooth, "smooth scatter series should have smooth=true");
+}