From 8e3fba8a25376f4852dba1b0c8f1edda021d7354 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 16:36:02 +0100 Subject: [PATCH] Chart cache: drop closed documents once per frame, skip volatile series Move retain_docs out of the per-chart paint path into SheetApp::logic so closed documents are forgotten even when no chart is painted. Charts whose series call NOW, TODAY, RAND, RANDBETWEEN or RANDARRAY bypass the cache because their resolved values change between evaluations. Co-Authored-By: Claude Sonnet 5.5 --- crates/chart/src/lib.rs | 2 +- crates/chart/src/resolve.rs | 20 ++++++++++++++++++++ crates/chart/src/tests.rs | 29 +++++++++++++++++++++++++++++ crates/ui-egui/src/app.rs | 1 + crates/ui-egui/src/chartcache.rs | 31 +++++++++++++++++++++++++++++++ crates/ui-egui/src/chartview.rs | 5 ++++- 6 files changed, 86 insertions(+), 2 deletions(-) diff --git a/crates/chart/src/lib.rs b/crates/chart/src/lib.rs index 4b68b8d17..f4e2895fe 100644 --- a/crates/chart/src/lib.rs +++ b/crates/chart/src/lib.rs @@ -26,7 +26,7 @@ use serde::{Deserialize, Serialize}; pub use axis::{Scale, format_number, nice_scale}; pub use raster::rasterize; pub use render::{Swatch, legend_entries, render, supports_trendline}; -pub use resolve::{default_palette, resolve, series_color, series_plot_kind}; +pub use resolve::{default_palette, has_volatile_series, resolve, series_color, series_plot_kind}; pub use sparkline::render_sparkline; /// Straight (non-premultiplied) RGBA colour. diff --git a/crates/chart/src/resolve.rs b/crates/chart/src/resolve.rs index de3cdef1b..53dc177f1 100644 --- a/crates/chart/src/resolve.rs +++ b/crates/chart/src/resolve.rs @@ -104,6 +104,26 @@ fn eval_list_in(wb: &Workbook, sheet: usize, formula: &str, depth: u32) -> Vec bool { + const VOLATILE: [&str; 5] = ["NOW", "TODAY", "RAND", "RANDBETWEEN", "RANDARRAY"]; + let calls_volatile = |f: &str| { + // Formulas read from a file may carry the `_xlfn.` prefix of newer functions. + let up = f.to_ascii_uppercase().replace("_XLFN.", ""); + VOLATILE.iter().any(|name| { + up.match_indices(name).any(|(i, _)| { + let before = up[..i].chars().next_back(); + let after = up[i + name.len()..].trim_start(); + before.is_none_or(|c| !(c.is_alphanumeric() || c == '_' || c == '.')) && after.starts_with('(') + }) + }) + }; + ch.series.iter().any(|s| { + [s.name.as_deref(), s.categories.as_deref(), Some(s.values.as_str()), s.bubble_sizes.as_deref()].into_iter().flatten().any(calls_volatile) + }) +} + fn to_num(v: &Value) -> Option { match v { Value::Number(n) if n.is_finite() => Some(*n), diff --git a/crates/chart/src/tests.rs b/crates/chart/src/tests.rs index 6e423802c..2e08f3d68 100644 --- a/crates/chart/src/tests.rs +++ b/crates/chart/src/tests.rs @@ -1330,3 +1330,32 @@ fn a_series_named_by_a_name_on_a_sheet_with_a_bang_resolves() { let d = resolve(&wb, 0, &c); assert_eq!(d.series.first().map(|s| s.name.as_str()), Some("Takings")); } + +#[test] +fn volatile_series_formulas_are_detected() { + let with = |values: &str, name: Option<&str>| { + let mut c = chart(ChartKind::Line); + c.series.push(Series { + name: name.map(Into::into), + categories: None, + values: values.into(), + bubble_sizes: None, + color: None, + secondary: false, + kind: None, + smooth: false, + quartiles: Default::default(), + trendlines: vec![], + }); + c + }; + assert!(has_volatile_series(&with("=NOW()", None))); + assert!(has_volatile_series(&with("=SUM(A1:A3)*rand ()", None))); + assert!(has_volatile_series(&with("Sheet1!$B$2:$B$9", Some("=TODAY()")))); + assert!(has_volatile_series(&with("=RANDARRAY(5)", None))); + assert!(has_volatile_series(&with("=_xlfn.RANDARRAY(5)", None))); + assert!(!has_volatile_series(&with("Sheet1!$B$2:$B$9", Some("Sheet1!$B$1")))); + assert!(!has_volatile_series(&with("=MYRAND(A1)", None))); + assert!(!has_volatile_series(&with("=Sheet1!$NOW", None))); + assert!(!has_volatile_series(&chart(ChartKind::Line))); +} diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index b67e37d99..535c91a5c 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -115,6 +115,7 @@ impl SheetApp { // Floating pictures' textures go with their workbook, even when no sheet is painted. let open: Vec = self.session.documents().iter().map(|d| d.uid).collect(); floating_pictures::forget_closed(ctx, &open); + self.chart_cache.get_mut().unwrap_or_else(std::sync::PoisonError::into_inner).retain_docs(open.iter().copied()); self.apply_font_state(ctx); if let Some(answer) = self.desktop_theme.take_change() { self.desktop_dark = Some(answer); diff --git a/crates/ui-egui/src/chartcache.rs b/crates/ui-egui/src/chartcache.rs index 2eb8510ad..82e9a96a2 100644 --- a/crates/ui-egui/src/chartcache.rs +++ b/crates/ui-egui/src/chartcache.rs @@ -461,4 +461,35 @@ mod tests { assert_eq!(h.state().chart_cache.lock().unwrap().docs(), 1); assert_eq!(n, 0, "the remaining document's charts were cached too"); } + + #[test] + fn a_chart_with_a_volatile_series_is_rendered_on_every_frame() { + let mut h = harness(two_charts()); + { + let d = h.state_mut().session.active_mut().unwrap(); + let sh = Arc::make_mut(&mut d.wb).sheet_mut(0).unwrap(); + sh.charts[0].series[0].values = "=RAND()".into(); + } + h.run_steps(4); + let n = rendered_after(&mut h, |h| h.run_steps(6)); + assert!(n >= 6, "the volatile chart is resolved again on each frame, got {n}"); + assert!(n < 12, "the other chart still comes from the cache, got {n}"); + } + + #[test] + fn closed_documents_are_forgotten_without_painting_a_chart() { + let mut h = harness(two_charts()); + assert_eq!(h.state().chart_cache.lock().unwrap().docs(), 1); + exec(&mut h, "file.new", json!({})); + h.run_steps(4); + assert_eq!(h.state().session.documents().len(), 2); + // The charts' document is no longer shown, but still open. + assert_eq!(h.state().chart_cache.lock().unwrap().docs(), 1); + h.state_mut().session.set_active(0); + h.run_steps(2); + exec(&mut h, "file.close", json!({"force": true, "document": true})); + h.run_steps(2); + assert_eq!(h.state().session.documents().len(), 1); + assert_eq!(h.state().chart_cache.lock().unwrap().docs(), 0, "no chart is painted, yet the closed document's entries are gone"); + } } diff --git a/crates/ui-egui/src/chartview.rs b/crates/ui-egui/src/chartview.rs index 0db26f4da..c769dbc1c 100644 --- a/crates/ui-egui/src/chartview.rs +++ b/crates/ui-egui/src/chartview.rs @@ -258,8 +258,11 @@ fn chart_prims(app: &SheetApp, p: &Painter, z: f32, wb: &Arc, si: usiz let Some(d) = app.session.active().filter(|d| Arc::ptr_eq(&d.wb, wb)) else { return render_chart(p, z, wb, si, ch).into(); }; + // A series that calls NOW or RAND must be evaluated again on every paint. + if gridcraft_chart::has_volatile_series(ch) { + return render_chart(p, z, wb, si, ch).into(); + } let mut cache = app.chart_cache.lock().unwrap_or_else(std::sync::PoisonError::into_inner); - cache.retain_docs(app.session.documents().iter().map(|d| d.uid)); cache.get(d.uid, wb, si, ci, (z, p.ctx().pixels_per_point()), || render_chart(p, z, wb, si, ch)) }