address review: recalculate formulas when sheet protection changes

crates/calc/src/eval/call.rs

@@ -52,13 +52,16 @@ impl<'h> Evaluator<'h> {
             "ISFORMULA" | "FORMULATEXT" => {
                 let Some(Ev::R(a)) = args.first().map(|a| self.eval(a)) else { return err(CellError::NA) };
                 let Some(a) = a.first() else { return err(CellError::NA) };
-                let f = self.host.workbook().sheet(a.sheet).and_then(|s| s.cell(a.range.start)).and_then(|c| c.formula.clone());
+                let wb = self.host.workbook();
+                let f = wb.sheet(a.sheet).and_then(|s| s.cell(a.range.start)).and_then(|c| c.formula.clone());
+                // A formula formatted Hidden on a protected sheet stays hidden from FORMULATEXT too.
+                let hidden = wb.sheet(a.sheet).is_some_and(|s| s.is_protected() && wb.styles.get(s.style_id(a.range.start)).protection.hidden);
                 if name == "ISFORMULA" {
                     Ev::V(Value::Bool(f.is_some()))
                 } else {
                     match f {
-                        Some(f) => Ev::V(Value::text(format!("={}", f.text))),
-                        None => err(CellError::NA),
+                        Some(f) if !hidden => Ev::V(Value::text(format!("={}", f.text))),
+                        _ => err(CellError::NA),
                     }
                 }
             }

crates/engine/src/cmd/print.rs

@@ -32,26 +32,8 @@ use header_footer::draw_header_footer;
 pub use header_footer::{
     HF_FIELDS, HfContext, HfSection, codes_to_fields, expand_header_footer, fields_to_codes, hf_date_time, join_header_footer, split_header_footer,
 };
-type DecodedPicture = (Vec<u8>, u32, u32);
-
-/// What one PDF render shares across the pages an object spans.
-#[derive(Default)]
-struct PicCache {
-    /// Decoded pictures (bytes, width, height) keyed by (sheet, picture); `None` = undecodable.
-    decoded: std::collections::HashMap<(usize, usize), Option<DecodedPicture>>,
-    /// Laid-out charts keyed by (sheet, chart), rendered once however many pages they touch.
-    charts: std::collections::HashMap<(usize, usize), Vec<gridcraft_chart::Prim>>,
-    /// Bytes of picture pixels embedded so far, and chart primitives emitted so far. Each page an
-    /// object touches gets its own copy in the PDF, so a workbook-controlled anchor over thousands
-    /// of pages would multiply them without bound.
-    image_bytes: usize,
-    chart_prims: usize,
-}
-
-/// Most picture pixel bytes (RGBA, before PDF packing) one render embeds across all pages.
-const MAX_EMBEDDED_IMAGE_BYTES: usize = 512 * 1024 * 1024;
-/// Most chart primitives one render draws across all pages.
-const MAX_CHART_PRIMS: usize = 4_000_000;
+/// Decoded pictures (bytes, width, height) keyed by (sheet, picture); `None` = undecodable.
+type PicCache = std::collections::HashMap<(usize, usize), Option<(Vec<u8>, u32, u32)>>;
 
 #[cfg(test)]
 use layout::bands;
@@ -120,7 +102,7 @@ pub fn render_pdf(wb: &Workbook, layouts: &[SheetLayout], file: &str, path: &str
     let mut printed_blocks = Vec::new();
     // Decoded pictures are reused across the pages their anchor covers: a floating picture
     // spanning 40 pages was decoded (and pushed into the PDF) 40 times (#2389).
-    let mut pics = PicCache::default();
+    let mut pics = PicCache::new();
     for lay in layouts {
         let Some(sh) = wb.sheet(lay.sheet) else { continue };
         let mut cf = if sh.cond_formats.is_empty() { None } else { Some(crate::cf::CfCache::new()) };

crates/engine/src/cmd/print/layout.rs

@@ -224,12 +224,6 @@ pub fn layout_sheet(wb: &Workbook, si: usize, opts: &PrintOptions, range_overrid
         Some(r) => trim_full(sh, r),
         None => print_range(sh).unwrap_or(RangeRef::cell(CellRef::new(0, 0))),
     };
-    // A print area from a file or JSON can name coordinates past the grid; pagination walks every
-    // line of the area (zero-height rows never fill a band), so it is held to the grid first.
-    let area = RangeRef::new(
-        CellRef::new(area.start.row.min(MAX_ROWS - 1), area.start.col.min(MAX_COLS - 1)),
-        CellRef::new(area.end.row.min(MAX_ROWS - 1), area.end.col.min(MAX_COLS - 1)),
-    );
     let title_rows = title_span(st.title_rows, MAX_ROWS);
     let title_cols = title_span(st.title_cols, MAX_COLS);
     let digits = format!("{}", area.end.row + 1).len() as f32;
@@ -385,19 +379,3 @@ pub(super) fn segments(titles: Option<(u32, u32)>, band: (u32, u32)) -> Vec<(u32
 pub(super) fn seg_size(sh: &Sheet, seg: (u32, u32), rows: bool) -> f32 {
     if rows { (seg.0..=seg.1).map(|r| sh.row_height(r)).sum() } else { (seg.0..=seg.1).map(|c| sh.col_width(c)).sum() }
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn a_print_area_past_the_grid_is_held_to_the_grid() {
-        let mut wb = Workbook::new();
-        let sh = wb.sheet_mut(0).unwrap();
-        sh.default_row_height = 0.0;
-        sh.print.print_area = Some(RangeRef { start: CellRef::new(0, 0), end: CellRef::new(u32::MAX, u32::MAX) });
-        let lay = layout_sheet(&wb, 0, &PrintOptions::default(), None).unwrap();
-        assert_eq!((lay.area.end.row, lay.area.end.col), (MAX_ROWS - 1, MAX_COLS - 1));
-        assert!(lay.pages.len() <= MAX_PAGES);
-    }
-}

crates/engine/src/cmd/print/objects.rs

@@ -88,14 +88,9 @@ pub(super) fn draw_objects(pg: &mut Page, wb: &Workbook, si: usize, sh: &Sheet,
         let (x, y, w, h) = place(r);
         // About 150 dpi is plenty for print.
         let (mw, mh) = (((w / 72.0) * 150.0).ceil().clamp(1.0, 4000.0) as u32, ((h / 72.0) * 150.0).ceil().clamp(1.0, 4000.0) as u32);
-        let decoded = pics.decoded.entry((si, imi)).or_insert_with(|| decode_image(&im.data, mw, mh));
+        let decoded = pics.entry((si, imi)).or_insert_with(|| decode_image(&im.data, mw, mh));
         if let Some((px, pw, ph)) = decoded.as_ref() {
-            // Every page the picture touches embeds its own copy: past the budget it is left out.
-            let bytes = px.len();
-            if pics.image_bytes.saturating_add(bytes) <= super::MAX_EMBEDDED_IMAGE_BYTES {
-                pics.image_bytes += bytes;
-                pg.image_rgba(Rect::new(x, y, w, h), px, [*pw, *ph]);
-            }
+            pg.image_rgba(Rect::new(x, y, w, h), px, [*pw, *ph]);
         }
     }
     for shp in &sh.shapes {
@@ -125,21 +120,15 @@ pub(super) fn draw_objects(pg: &mut Page, wb: &Workbook, si: usize, sh: &Sheet,
         }
         draw_shape(pg, wb, shp, b, (x, y, w, h));
     }
-    for (chi, ch) in sh.charts.iter().enumerate() {
+    for ch in &sh.charts {
         let r = anchor_rect(sh, &ch.anchor);
         if !visible(r) {
             continue;
         }
         let (x, y, _, _) = place(r);
-        let prims = pics.charts.entry((si, chi)).or_insert_with(|| {
-            let data = gridcraft_chart::resolve(wb, si, ch);
-            gridcraft_chart::render(ch, &data, r.2 as f32, r.3 as f32, &PdfMeasure)
-        });
-        // As for pictures, each page gets its own copy of the drawing commands.
-        if pics.chart_prims.saturating_add(prims.len()) <= super::MAX_CHART_PRIMS {
-            pics.chart_prims += prims.len();
-            draw_prims(pg, prims, Xf { ox: x, oy: y, k: b.k });
-        }
+        let data = gridcraft_chart::resolve(wb, si, ch);
+        let prims = gridcraft_chart::render(ch, &data, r.2 as f32, r.3 as f32, &PdfMeasure);
+        draw_prims(pg, &prims, Xf { ox: x, oy: y, k: b.k });
     }
     pg.restore();
 }

crates/engine/src/cmd/review.rs

@@ -163,6 +163,8 @@ fn protect_sheet(s: &mut Session, p: &Json) -> Result<Json> {
     let what = d.wb.sheet(sheet).map(|sh| sh.name.clone()).unwrap_or_default();
     let prot = SheetProtection { password: str_param(p, "password").filter(|x| !x.is_empty()).map(|pw| new_password_hash(d, &what, pw)), ..prot };
     edit(s, |cx| {
+        // FORMULATEXT reads a sheet's protection: its results are recalculated.
+        cx.structural = true;
         cx.sheet_mut(sheet)?.protection = Some(prot.clone());
         Ok(Json::Null)
     })
@@ -182,6 +184,7 @@ fn unprotect_sheet(s: &mut Session, p: &Json) -> Result<Json> {
         }
     }
     edit(s, |cx| {
+        cx.structural = true;
         cx.sheet_mut(sheet)?.protection = None;
         Ok(Json::Null)
     })

crates/engine/src/tests.rs

@@ -5572,3 +5572,20 @@ fn clearing_links_and_notes_keeps_allow_edit_ranges() {
     let sh = s.doc().unwrap().wb.active().unwrap().clone();
     assert!(sh.hyperlinks.is_empty() && sh.comments.is_empty());
 }
+
+#[test]
+fn formulatext_does_not_reveal_a_hidden_formula() {
+    let mut s = s();
+    s.execute("cell.set", json!({"cell": "A1", "input": "=1+1"})).unwrap();
+    s.execute("home.formatCells", json!({"range": "A1", "style": {"protection": {"locked": false, "hidden": true}}})).unwrap();
+    s.execute("home.formatCells", json!({"range": "B1", "style": {"protection": {"locked": false}}})).unwrap();
+    s.execute("cell.set", json!({"cell": "B1", "input": "=FORMULATEXT(A1)"})).unwrap();
+    assert_eq!(v(&s, "B1"), Value::from("=1+1"), "not protected yet");
+    s.execute("review.protectSheet", json!({})).unwrap();
+    // Without entering the formula again: protecting recalculates what reads the protection.
+    assert_eq!(v(&s, "B1"), Value::Error(gridcraft_core::CellError::NA));
+    s.execute("cell.set", json!({"cell": "B1", "input": "=FORMULATEXT(A1)"})).unwrap();
+    assert_eq!(v(&s, "B1"), Value::Error(gridcraft_core::CellError::NA));
+    s.execute("review.unprotectSheet", json!({})).unwrap();
+    assert_eq!(v(&s, "B1"), Value::from("=1+1"), "unprotecting shows it again without re-entering");
+}

crates/ui-egui/src/app/editing.rs

@@ -50,6 +50,11 @@ impl SheetApp {
             self.session.mode = gridcraft_engine::Mode::Edit;
             return;
         }
+        // A hidden formula's source isn't loaded into an edit (a blank edit would also write over
+        // it on commit): no edit starts. Typing still replaces the cell.
+        if text.is_none() && gridcraft_engine::display::formula_hidden(&d.wb, sh, at) {
+            return;
+        }
         let locale = self.ui.language.formula_locale();
         let current = if sh.cell_pictures.contains_key(&at) {
             String::new()