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;