PivotTables: the cell menu computes a report once for all its cells

The cell menu checked whether to offer Show Details by computing the
whole report again for each new active cell. pivot::detail_cells now
returns every value, subtotal and grand total cell of a report from one
computation, and the menu caches that set per document, revision, sheet
and PivotTable.

crates/engine/src/cmd/pivot.rs

@@ -2580,4 +2580,35 @@ mod tests {
         }
         assert_eq!(s.doc().unwrap().wb.sheets.len(), 2);
     }
+
+    #[test]
+    fn detail_cells_are_the_cells_show_details_takes() {
+        let mut s = sales();
+        s.execute("insert.pivotTable", json!({"rows": ["Region"], "columns": ["Product"], "values": ["Sales"]})).unwrap();
+        for layout in 0..2 {
+            if layout == 1 {
+                // A two-level row axis, with subtotals at the top of each group.
+                s.execute("pivot.moveField", json!({"field": "Product", "from": "columns", "to": "rows"})).unwrap();
+            }
+            let d = s.doc().unwrap();
+            let si = d.wb.active_sheet;
+            let pt = &d.wb.active().unwrap().pivots[0];
+            let r = pt.last_range.unwrap();
+            let cells = crate::pivot::detail_cells(&d.wb, pt, si).unwrap();
+            let mut values = 0;
+            for row in 0..r.height() {
+                for col in 0..r.width() {
+                    let one = crate::pivot::detail_records(&d.wb, pt, si, row, col).unwrap();
+                    assert_eq!(cells.contains(&(row, col)), one.is_some(), "layout {layout}, cell ({row}, {col})");
+                    values += usize::from(one.is_some());
+                }
+            }
+            assert!(values > 4, "layout {layout} has value cells");
+        }
+        // An out-of-date report answers as Show Details does.
+        s.execute("range.setValues", json!({"range": "Sheet1!A2", "values": [["Central"]]})).unwrap();
+        let d = s.doc().unwrap();
+        let pt = &d.wb.active().unwrap().pivots[0];
+        assert!(crate::pivot::detail_cells(&d.wb, pt, d.wb.active_sheet).unwrap_err().contains("out of date"));
+    }
 }

crates/engine/src/pivot.rs

@@ -816,11 +816,30 @@ pub const NOT_A_VALUE_CELL: &str = "select a value cell in a PivotTable";
 /// on the sheet is no longer what the source computes: the cell's records would be another cell's.
 pub fn detail_records(wb: &Workbook, pt: &PivotTable, pivot_sheet: usize, row: u32, col: u32) -> Result<Option<(Source, Vec<usize>)>, String> {
     let src = read_source(wb, &pt.source, pivot_sheet)?;
-    let mut probe = Probe { at: (row as usize, col as usize), hit: None, paths: Vec::new() };
+    let mut probe = Probe { at: Some((row as usize, col as usize)), hit: None, paths: Some(Vec::new()), cells: None };
     let out = compute_from(wb, pt, &src, Some(&mut probe))?;
-    // Excel reads its PivotTable cache; without one, the report written by the last refresh
-    // must still be what the source gives. Checked first: in a stale report even a cell that
-    // computes as a label may show a value.
+    // Checked first: in a stale report even a cell that computes as a label may show a value.
+    check_current(wb, pt, pivot_sheet, &out)?;
+    let Some((rp, cp)) = probe.hit else { return Ok(None) };
+    let records =
+        probe.paths.unwrap_or_default().into_iter().filter(|(_, r, c)| r.starts_with(&rp) && c.starts_with(&cp)).map(|(i, _, _)| i).collect();
+    Ok(Some((src, records)))
+}
+
+/// The cells of the report grid (relative to its top-left) where [`detail_records`] finds
+/// records: its value, subtotal and grand total cells, from one computation of the report. The
+/// same errors as [`detail_records`].
+pub fn detail_cells(wb: &Workbook, pt: &PivotTable, pivot_sheet: usize) -> Result<HashSet<(u32, u32)>, String> {
+    let src = read_source(wb, &pt.source, pivot_sheet)?;
+    let mut probe = Probe { at: None, hit: None, paths: None, cells: Some(HashSet::new()) };
+    let out = compute_from(wb, pt, &src, Some(&mut probe))?;
+    check_current(wb, pt, pivot_sheet, &out)?;
+    Ok(probe.cells.unwrap_or_default().into_iter().map(|(r, c)| (r as u32, c as u32)).collect())
+}
+
+/// An error when the report on the sheet is not what `out` computes. Excel reads its PivotTable
+/// cache; without one, the report written by the last refresh must still be what the source gives.
+fn check_current(wb: &Workbook, pt: &PivotTable, pivot_sheet: usize, out: &Output) -> Result<(), String> {
     let sh = wb.sheet(pivot_sheet).ok_or("There's no such sheet.")?;
     let current = pt.last_range.is_some_and(|r| {
         r.height() == out.height().max(1)
@@ -834,17 +853,17 @@ pub fn detail_records(wb: &Workbook, pt: &PivotTable, pivot_sheet: usize, row: u
     if !current {
         return Err("The PivotTable report is out of date. Refresh it first.".into());
     }
-    let Some((rp, cp)) = probe.hit else { return Ok(None) };
-    let records = probe.paths.into_iter().filter(|(_, r, c)| r.starts_with(&rp) && c.starts_with(&cp)).map(|(i, _, _)| i).collect();
-    Ok(Some((src, records)))
+    Ok(())
 }
 
-/// What [`detail_records`] asks of a report computation: the row and column item paths of the
-/// value cell at `at`, and the paths of every record the filters let through.
+/// What [`detail_records`] and [`detail_cells`] ask of a report computation: the row and column
+/// item paths of the value cell at `at`, the paths of every record the filters let through (when
+/// `paths` is set), and every value cell (when `cells` is set).
 struct Probe {
-    at: (usize, usize),
+    at: Option<(usize, usize)>,
     hit: Option<(Vec<u32>, Vec<u32>)>,
-    paths: Vec<(usize, Vec<u32>, Vec<u32>)>,
+    paths: Option<Vec<(usize, Vec<u32>, Vec<u32>)>>,
+    cells: Option<HashSet<(usize, usize)>>,
 }
 
 fn compute_from(wb: &Workbook, pt: &PivotTable, src: &Source, mut probe: Option<&mut Probe>) -> Result<Output, String> {
@@ -959,8 +978,8 @@ fn compute_from(wb: &Workbook, pt: &PivotTable, src: &Source, mut probe: Option<
         if !col_leaves.contains(&cp[..shown]) {
             col_leaves.insert(cp[..shown].to_vec());
         }
-        if let Some(pr) = probe.as_deref_mut() {
-            pr.paths.push((ri, rp.clone(), cp.clone()));
+        if let Some(paths) = probe.as_deref_mut().and_then(|pr| pr.paths.as_mut()) {
+            paths.push((ri, rp.clone(), cp.clone()));
         }
         row_set.insert(rp);
         col_set.insert(cp);
@@ -1326,10 +1345,13 @@ fn compute_from(wb: &Workbook, pt: &PivotTable, src: &Source, mut probe: Option<
                     _ => raw,
                 },
             };
-            if let Some(pr) = probe.as_deref_mut()
-                && pr.at == (r, l + j)
-            {
-                pr.hit = Some((rp.to_vec(), cp.to_vec()));
+            if let Some(pr) = probe.as_deref_mut() {
+                if pr.at == Some((r, l + j)) {
+                    pr.hit = Some((rp.to_vec(), cp.to_vec()));
+                }
+                if let Some(cells) = pr.cells.as_mut() {
+                    cells.insert((r, l + j));
+                }
             }
             g.set(r, l + j, shown, ck.1, 0, data_fmt.get(*vi).cloned().flatten());
         }

crates/ui-egui/src/grid.rs

@@ -163,9 +163,11 @@ pub struct GridState {
     /// 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)>,
-    /// Whether the cell menu offers Show Details, cached per (doc uid, revision, sheet, active
-    /// cell): finding out computes the whole PivotTable report.
-    pub show_details_item: Option<((u64, u64, usize, CellRef), bool)>,
+    /// The cells of a PivotTable report where the cell menu offers Show Details, relative to the
+    /// report's top-left and cached per (doc uid, revision, sheet, pivot): finding them computes
+    /// the whole report. `None` when the report can't be computed or is out of date: every cell
+    /// of it offers Show Details, and running it says why.
+    pub show_details_cells: Option<((u64, u64, usize, usize), Option<std::collections::HashSet<(u32, u32)>>)>,
     pub header_menu: Option<(Pos2, bool)>,
     /// Page Layout view pagination, cached per (doc uid, revision, sheet, rows, columns).
     pub paper: Option<crate::paper_view::PaperCache>,
@@ -3424,20 +3426,21 @@ fn context_menu(app: &mut SheetApp, ui: &mut egui::Ui) {
 fn show_details_offered(app: &mut SheetApp) -> bool {
     let Some(d) = app.session.active() else { return false };
     let (si, at) = (d.wb.active_sheet, d.selection.active);
-    let key = (d.uid, d.revision, si, at);
-    if let Some((k, on)) = app.grid.show_details_item
-        && k == key
+    let Some((pi, pt)) = d.wb.active().and_then(|sh| sh.pivots.iter().enumerate().find(|(_, pt)| pt.last_range.is_some_and(|r| r.contains(at))))
+    else {
+        return false;
+    };
+    let origin = pt.last_range.map(|r| r.start).unwrap_or_default();
+    let cell = (at.row.saturating_sub(origin.row), at.col.saturating_sub(origin.col));
+    let key = (d.uid, d.revision, si, pi);
+    if let Some((k, cells)) = &app.grid.show_details_cells
+        && *k == key
     {
-        return on;
+        return cells.as_ref().is_none_or(|c| c.contains(&cell));
     }
-    let on = d.wb.active().and_then(|sh| sh.pivots.iter().find(|pt| pt.last_range.is_some_and(|r| r.contains(at)))).is_some_and(|pt| {
-        let origin = pt.last_range.map(|r| r.start).unwrap_or_default();
-        !matches!(
-            gridcraft_engine::pivot::detail_records(&d.wb, pt, si, at.row.saturating_sub(origin.row), at.col.saturating_sub(origin.col)),
-            Ok(None)
-        )
-    });
-    app.grid.show_details_item = Some((key, on));
+    let cells = gridcraft_engine::pivot::detail_cells(&d.wb, pt, si).ok();
+    let on = cells.as_ref().is_none_or(|c| c.contains(&cell));
+    app.grid.show_details_cells = Some((key, cells));
     on
 }
 

crates/ui-egui/tests/it/pivot_show_details_menu.rs

@@ -4,7 +4,7 @@
 use egui::{Event, Modifiers, PointerButton, Pos2, vec2};
 use egui_kittest::kittest::Queryable as _;
 use gridcraft_engine::Session;
-use gridcraft_engine::core::CellRef;
+use gridcraft_engine::core::{CellRef, Value};
 use gridcraft_ui_egui::{SheetApp, grid::Geo};
 use serde_json::json;
 
@@ -90,6 +90,23 @@ fn labels_of_the_report_and_ordinary_cells_have_no_show_details_item() {
     }
 }
 
+#[test]
+fn the_cell_menu_computes_the_report_once_for_all_its_cells() {
+    let mut h = harness();
+    right_click(&mut h, "E4");
+    assert!(h.query_by_label("Show Details").is_some());
+    let escape = Event::Key { key: egui::Key::Escape, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE };
+    h.input_mut().events.push(escape);
+    h.run_steps(2);
+    // West goes away behind the session's back, so the revision stays the same. A menu that
+    // computed the report again for D4 would find it out of date and offer Show Details there.
+    let doc = h.state_mut().session.active_mut().unwrap();
+    let si = doc.wb.active_sheet;
+    std::sync::Arc::make_mut(&mut doc.wb).sheet_mut(si).unwrap().set_value(CellRef::parse("A3").unwrap(), Value::text("East"));
+    right_click(&mut h, "D4");
+    assert!(h.query_by_label("Show Details").is_none(), "D4 is a label of the report computed for E4");
+}
+
 #[test]
 fn show_details_in_an_out_of_date_report_asks_for_a_refresh() {
     let mut h = harness();