mirror of
https://github.com/storytold/gridcraft.git
synced 2026-11-13 09:21:35 -06:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5.5
parent
ac0588dbaf
commit
8e3fba8a25
@@ -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.
|
||||
|
||||
@@ -104,6 +104,26 @@ fn eval_list_in(wb: &Workbook, sheet: usize, formula: &str, depth: u32) -> Vec<V
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether a series formula of `ch` calls a function whose result changes between evaluations
|
||||
/// (`NOW`, `TODAY`, `RAND`, `RANDBETWEEN`, `RANDARRAY`), so a result resolved earlier is stale.
|
||||
pub fn has_volatile_series(ch: &Chart) -> 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<f64> {
|
||||
match v {
|
||||
Value::Number(n) if n.is_finite() => Some(*n),
|
||||
|
||||
@@ -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)));
|
||||
}
|
||||
|
||||
@@ -115,6 +115,7 @@ impl SheetApp {
|
||||
// Floating pictures' textures go with their workbook, even when no sheet is painted.
|
||||
let open: Vec<u64> = 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);
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -258,8 +258,11 @@ fn chart_prims(app: &SheetApp, p: &Painter, z: f32, wb: &Arc<Workbook>, 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))
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user