From 0812d3aee5f99dfc9938f07ef140c7b43bf449d7 Mon Sep 17 00:00:00 2001 From: Aimar Mendez Date: Wed, 21 Oct 2026 19:03:36 +0100 Subject: [PATCH] PivotTable Fields pane: scan the source once per change, not every frame The pane ran `pivot.fields` on every frame while the active cell was in a PivotTable report. That command reads the whole source list and collects distinct labels for every field, so a 250,000 x 20 source made each frame cost ~2.4 s and the window looked frozen (#2113). Keep the answer in GridState, keyed by (doc uid, revision, pivot), the way Page Break Preview keeps its pages. Any undoable edit, undo or redo bumps the revision and drops it. Known limit: Calculate Now / Calculate Sheet don't bump the revision, so volatile formulas in the source can leave the tooltip samples stale until the next edit; the field list itself only changes through undoable edits. VERIFIED - New test pivot_fields_pane_reuses_its_field_list_until_the_workbook_changes fails without the change and passes with it. - Release build, 250,000 x 20 synthetic source, pane open (ui.inspect): main ~2,400 ms per frame throughout; this branch 2,528 ms for the first frame, then 0.4-0.9 ms. - `cargo xtask ci` passes all 8 steps on macOS with the separate clippy fix from #2141 applied locally (main fails clippy on macOS without it). NOT IN THIS CHANGE - The report is still empty after opening a file saved with refreshOnLoad="1" (second part of #2113). - `pivot.fields` itself is unchanged; the first frame still pays one scan. --- crates/ui-egui/src/grid.rs | 3 +++ crates/ui-egui/src/pivot_pane.rs | 13 +++++++++---- crates/ui-egui/tests/interaction.rs | 22 ++++++++++++++++++++++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 106bc6684..3a216d3a7 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -134,6 +134,9 @@ pub struct GridState { pub ink: Vec<[f32; 2]>, /// Page ranges for Page Break Preview, cached per (doc uid, revision, sheet). pub pages: Option<((u64, u64, usize), Vec<(RangeRef, u32)>)>, + /// The PivotTable Fields pane's `pivot.fields` answer, cached per (doc uid, revision, pivot): + /// it scans the whole source list, far too slow to repeat every frame (#2113). + pub pivot_fields: Option<((u64, u64, String), serde_json::Value)>, pub header_menu: Option<(Pos2, bool)>, /// Page Layout view pagination, cached per (doc uid, revision, sheet, rows, columns). pub paper: Option, diff --git a/crates/ui-egui/src/pivot_pane.rs b/crates/ui-egui/src/pivot_pane.rs index 55917ef17..9c381bb9a 100644 --- a/crates/ui-egui/src/pivot_pane.rs +++ b/crates/ui-egui/src/pivot_pane.rs @@ -21,10 +21,15 @@ pub fn show(app: &mut SheetApp, ui: &mut Ui) { let Some(name) = active_pivot(app) else { return }; let t = Tokens::get(ui.ctx()); let lang = i18n::current(ui.ctx()); - let info = match app.session.run("pivot.fields", json!({"pivot": name})) { - Ok(v) => v, - Err(_) => return, - }; + let Some(key) = app.session.active().map(|d| (d.uid, d.revision, name.clone())) else { return }; + if app.grid.pivot_fields.as_ref().is_none_or(|(k, _)| *k != key) { + let info = match app.session.run("pivot.fields", json!({"pivot": name})) { + Ok(v) => v, + Err(_) => return, + }; + app.grid.pivot_fields = Some((key, info)); + } + let Some((_, info)) = app.grid.pivot_fields.clone() else { return }; egui::Panel::right("pivot_pane").default_size(280.0).resizable(true).frame(egui::Frame::NONE.fill(t.ribbon).inner_margin(10)).show(ui, |ui| { ui.label(egui::RichText::new(lang.tr("PivotTable Fields")).font(theme::ui_bold(15.0))); ui.label(egui::RichText::new(lang.tr("Choose fields to add to report:")).small().color(t.text_dim)); diff --git a/crates/ui-egui/tests/interaction.rs b/crates/ui-egui/tests/interaction.rs index 02084ffb8..a3746a7e3 100644 --- a/crates/ui-egui/tests/interaction.rs +++ b/crates/ui-egui/tests/interaction.rs @@ -403,6 +403,28 @@ fn pivot_fields_pane_follows_the_interface_language() { assert!(h.query_by_label("Remove Field").is_none()); } +#[test] +fn pivot_fields_pane_reuses_its_field_list_until_the_workbook_changes() { + // #2113: the pane ran `pivot.fields`, a scan of the whole source list, on every frame. + let mut h = harness(blank()); + h.state_mut().run("range.setValues", json!({"range": "A1:B3", "values": [["Region", "Sales"], ["North", 10], ["South", 20]]})).unwrap(); + h.state_mut().run("insert.pivotTable", json!({"source": "Sheet1!$A$1:$B$3", "rows": ["Region"], "destination": "Sheet1!D1"})).unwrap(); + h.run_steps(2); + assert!(h.query_by_label("Sales").is_some(), "the pane lists the source fields"); + let (key, mut info) = h.state().grid.pivot_fields.clone().expect("the pane keeps its field list"); + info["fields"][1]["name"] = json!("Cached"); + h.state_mut().grid.pivot_fields = Some((key, info)); + h.run_steps(2); + assert!(h.query_by_label("Cached").is_some(), "repaints reuse the kept list instead of scanning the source again"); + h.state_mut().run("cell.set", json!({"cell": "B1", "input": "Revenue"})).unwrap(); + h.run_steps(2); + assert!(h.query_by_label("Cached").is_none(), "an edit drops the kept list"); + assert!(h.query_by_label("Revenue").is_some(), "the pane shows the edited source"); + h.state_mut().run("edit.undo", json!({})).unwrap(); + h.run_steps(2); + assert!(h.query_by_label("Sales").is_some(), "undo brings the previous field list back"); +} + #[test] fn format_cells_draws_its_labels_in_the_interface_language() { let mut h = harness(blank());