From f6546156408fd4809689b3305358b47281e22b7b Mon Sep 17 00:00:00 2001 From: justinjohn0306 <34035011+justinjohn0306@users.noreply.github.com> Date: Thu, 8 Oct 2026 16:04:14 +0530 Subject: [PATCH 01/19] Fix: list and render installed system fonts (#4) The font dropdown showed a hardcoded 17-name list and cells always rendered in the built-in Calibri/role font, so a font installed on the machine could not be listed or chosen. Both trace to one constraint: egui panics when asked for a FontFamily::Name that was never bound to font data, so only the 8 built-in roles were ever bound. Scan the platform font folders for installed families (a bounded read of each file's name table via skrifa, already compiled transitively through epaint), bind the faces a workbook actually uses into egui as fnt:{family}:{b|n}{i|n} roles chained to the defaults, and offer the installed families in the font combos after the standard set. cell_family returns the installed role when it is bound, else the previous bucketing, so it never names an unbound family. The scan runs in the background and the first frame that rebinds skips painting, like the existing startup gate. Native only; wasm keeps the standard list. Incidental: newer local clippy flagged pre-existing lints in untouched crates (no toolchain is pinned); fixed behavior-preservingly so the gate passes. --- Cargo.lock | 2 + Cargo.toml | 4 + crates/core/src/date.rs | 2 +- crates/engine/src/cmd/file.rs | 2 +- crates/engine/src/cmd/spelling.rs | 26 +- crates/functions/src/math.rs | 2 +- crates/functions/src/stat.rs | 2 +- crates/functions/src/util.rs | 2 +- crates/numfmt/src/render.rs | 2 +- crates/ui-egui/Cargo.toml | 2 + crates/ui-egui/src/dialogs.rs | 4 +- crates/ui-egui/src/fonts.rs | 423 ++++++++++++++++++++++++++++++ crates/ui-egui/src/formula_bar.rs | 2 +- crates/ui-egui/src/grid.rs | 9 +- crates/ui-egui/src/lib.rs | 52 +++- crates/ui-egui/src/ribbon.rs | 6 +- crates/ui-egui/src/theme.rs | 71 ++++- 17 files changed, 580 insertions(+), 33 deletions(-) create mode 100644 crates/ui-egui/src/fonts.rs diff --git a/Cargo.lock b/Cargo.lock index f3b1e64..0855bf0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1522,12 +1522,14 @@ version = "0.1.0" dependencies = [ "egui", "egui_kittest", + "epaint_default_fonts", "gridcraft-chart", "gridcraft-engine", "image", "log", "serde", "serde_json", + "skrifa", "toml", ] diff --git a/Cargo.toml b/Cargo.toml index a7f99e0..e87627d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,6 +43,10 @@ eframe = { version = "0.36", default-features = false } egui_kittest = "0.36" rfd = "0.17" arboard = { version = "3.6", default-features = false } +# Font name-table reading. The same version epaint already depends on, so it adds no new build. +skrifa = "0.44" +# Real font bytes for tests (Ubuntu Light, compiled into epaint). +epaint_default_fonts = "0.36" [workspace.lints.rust] unsafe_code = "forbid" diff --git a/crates/core/src/date.rs b/crates/core/src/date.rs index 7fba23f..d27ee15 100644 --- a/crates/core/src/date.rs +++ b/crates/core/src/date.rs @@ -97,7 +97,7 @@ pub fn serial_from_ymd(sys: DateSystem, year: i64, month: i64, day: i64) -> Opti /// Date/time from a serial. `None` for negative or too-large serials. pub fn datetime_from_serial(sys: DateSystem, serial: f64) -> Option { - if !serial.is_finite() || serial < 0.0 || serial >= 2_958_466.0 { + if !(0.0..2_958_466.0).contains(&serial) { return None; } let mut day = serial.floor() as i64; diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 37a58dd..34b957c 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -48,7 +48,7 @@ fn open(s: &mut Session, p: &Json) -> Result { }; let (wb, warnings) = crate::io::open_bytes(&name, &bytes)?; let title = std::path::Path::new(&name).file_name().and_then(|n| n.to_str()).unwrap_or("Book").to_string(); - let path = str_param(p, "path").map(str::to_string).filter(|p| crate::io::FileKind::from_path(p) == Some(crate::io::FileKind::Xlsx) || true); + let path = str_param(p, "path").map(str::to_string); // Replace an untouched blank Book1 like Excel does. if s.documents().len() == 1 && s.active().is_some_and(|d| d.path.is_none() && !d.is_dirty() && d.undo.is_empty() && d.wb.sheets.iter().all(|sh| sh.cells.is_empty())) diff --git a/crates/engine/src/cmd/spelling.rs b/crates/engine/src/cmd/spelling.rs index 10892bc..e149120 100644 --- a/crates/engine/src/cmd/spelling.rs +++ b/crates/engine/src/cmd/spelling.rs @@ -21,17 +21,23 @@ pub fn specs() -> Vec { fn dictionary() -> &'static HashSet { static DICT: OnceLock> = OnceLock::new(); DICT.get_or_init(|| { - let mut set = HashSet::new(); - #[cfg(not(target_arch = "wasm32"))] - for path in ["/usr/share/dict/words", "/usr/share/dict/american-english", "/usr/share/dict/british-english"] { - if let Ok(text) = std::fs::read_to_string(path) { - for w in text.lines().take(2_000_000) { - set.insert(w.trim().to_lowercase()); - } - break; - } + #[cfg(target_arch = "wasm32")] + { + HashSet::new() + } + #[cfg(not(target_arch = "wasm32"))] + { + let mut set = HashSet::new(); + for path in ["/usr/share/dict/words", "/usr/share/dict/american-english", "/usr/share/dict/british-english"] { + if let Ok(text) = std::fs::read_to_string(path) { + for w in text.lines().take(2_000_000) { + set.insert(w.trim().to_lowercase()); + } + break; + } + } + set } - set }) } diff --git a/crates/functions/src/math.rs b/crates/functions/src/math.rs index 3a0bab3..654395f 100644 --- a/crates/functions/src/math.rs +++ b/crates/functions/src/math.rs @@ -487,7 +487,7 @@ fn integer_args(a: &[Arg]) -> R> { v.iter() .map(|x| { let t = x.trunc(); - if t < 0.0 || t >= 9.007_199_254_740_992e15 { Err(CellError::Num) } else { Ok(t as u64) } + if !(0.0..9.007_199_254_740_992e15).contains(&t) { Err(CellError::Num) } else { Ok(t as u64) } }) .collect() } diff --git a/crates/functions/src/stat.rs b/crates/functions/src/stat.rs index abdb4e7..51a5f32 100644 --- a/crates/functions/src/stat.rs +++ b/crates/functions/src/stat.rs @@ -1185,7 +1185,7 @@ fn binom_dist_range(a: &[Arg], _: C) -> R { let p = num(a, 1)?; let s1 = num(a, 2)?.trunc(); let s2 = if has(a, 3) { num(a, 3)?.trunc() } else { s1 }; - if n < 0.0 || n > 1e15 || !(0.0..=1.0).contains(&p) || s1 < 0.0 || s1 > n || s2 < s1 || s2 > n { + if !(0.0..=1e15).contains(&n) || !(0.0..=1.0).contains(&p) || s1 < 0.0 || s1 > n || s2 < s1 || s2 > n { return Err(CellError::Num); } if s2 - s1 > 10_000.0 { diff --git a/crates/functions/src/util.rs b/crates/functions/src/util.rs index fca5dc4..fbd6649 100644 --- a/crates/functions/src/util.rs +++ b/crates/functions/src/util.rs @@ -51,7 +51,7 @@ pub(crate) fn arg(args: &[Arg], i: usize) -> R<&Arg> { /// Whether argument `i` was supplied (present and not an empty literal like `F(1,)`). pub(crate) fn has(args: &[Arg], i: usize) -> bool { - matches!(args.get(i), Some(a) if !(matches!(a.value, Value::Empty) && !a.from_ref)) + matches!(args.get(i), Some(a) if !matches!(a.value, Value::Empty) || a.from_ref) } /// Number coercion for a scalar parameter: empty = 0, TRUE = 1, numeric text parses. diff --git a/crates/numfmt/src/render.rs b/crates/numfmt/src/render.rs index 363dec1..523b77d 100644 --- a/crates/numfmt/src/render.rs +++ b/crates/numfmt/src/render.rs @@ -58,7 +58,7 @@ pub(crate) fn render_general(sec: &Section, v: f64, width: usize) -> Out { /// Renders a date/time. `None` when the serial is outside the supported range. pub(crate) fn render_date(sec: &Section, v: f64, sys: DateSystem) -> Option { - if !v.is_finite() || v < 0.0 || v >= 2_958_466.0 { + if !(0.0..2_958_466.0).contains(&v) { return None; } let sub_digits = sec.toks.iter().filter_map(|t| if let Tok::Date(DatePart::SubSec(n)) = t { Some(*n) } else { None }).max().unwrap_or(0).min(3); diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index 01d7c00..283d53f 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -16,9 +16,11 @@ serde = { workspace = true } serde_json = { workspace = true } log = { workspace = true } image = { workspace = true } +skrifa = { workspace = true } [dev-dependencies] egui_kittest = { workspace = true, features = ["wgpu", "snapshot"] } +epaint_default_fonts = { workspace = true } [build-dependencies] serde_json = { workspace = true } diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index b6c186d..e921cdc 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1020,8 +1020,8 @@ fn format_cells(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dialog, confirm: } "Font" => { egui::ComboBox::from_label("Font").selected_text(st.font.name.clone()).show_ui(ui, |ui| { - for f in crate::ribbon::FONTS { - ui.selectable_value(&mut st.font.name, f.to_string(), *f); + for f in crate::fonts::families() { + ui.selectable_value(&mut st.font.name, f.clone(), f.clone()); } }); ui.add(egui::Slider::new(&mut st.font.size, 6.0..=72.0).text("Size")); diff --git a/crates/ui-egui/src/fonts.rs b/crates/ui-egui/src/fonts.rs new file mode 100644 index 0000000..940bca9 --- /dev/null +++ b/crates/ui-egui/src/fonts.rs @@ -0,0 +1,423 @@ +//! The fonts installed on the machine, so the font list can offer them and cells can render with +//! them (#4). +//! +//! egui has no font discovery of its own and panics when asked for a family that was never bound +//! to font data. So we do both halves: scan the platform's font folders for family names (a read +//! of each font file's `name` table — damaged or unreadable files yield nothing, never a panic), +//! and bind the files a workbook actually uses into the families +//! [`theme::font_definitions`](crate::theme::font_definitions) assembles. +//! +//! Native only: on wasm there are no system fonts, every function here reports "nothing", and the +//! font list is the standard [`crate::ribbon::FONTS`] set. + +use std::collections::HashSet; +use std::path::PathBuf; +use std::sync::OnceLock; + +/// A font file is skipped by the scan past this size: a catalog only needs each face's name, and a +/// damaged or enormous file must not turn startup into a long read. +#[cfg(not(target_arch = "wasm32"))] +const MAX_FONT_FILE: u64 = 64 << 20; + +/// One face of one installed font file. +#[derive(Clone, Debug)] +pub struct Face { + /// Typographic family name, e.g. `"Segoe UI"`. + pub family: String, + /// The file the face lives in. + pub path: PathBuf, + /// Face index within the file (collections hold several, e.g. a family's Regular and Bold). + pub index: u32, + /// Weight implied by the face's style name (400 = regular). + pub weight: f32, + /// Whether the style name reads as italic or oblique. + pub italic: bool, +} + +/// The font folders of the platform (the system's and the user's). Empty on wasm. +pub fn system_font_dirs() -> Vec { + #[cfg(target_arch = "wasm32")] + { + Vec::new() + } + #[cfg(not(target_arch = "wasm32"))] + { + let mut dirs: Vec = Vec::new(); + let home = std::env::var_os("HOME").or_else(|| std::env::var_os("USERPROFILE")).map(PathBuf::from); + if cfg!(target_os = "macos") { + dirs.extend(["/System/Library/Fonts", "/System/Library/Fonts/Supplemental", "/Library/Fonts"].map(Into::into)); + if let Some(h) = &home { + dirs.push(h.join("Library/Fonts")); + } + } else if cfg!(windows) { + let root = std::env::var_os("WINDIR").map(PathBuf::from).unwrap_or_else(|| PathBuf::from("C:\\Windows")); + dirs.push(root.join("Fonts")); + if let Some(l) = std::env::var_os("LOCALAPPDATA") { + dirs.push(PathBuf::from(l).join("Microsoft\\Windows\\Fonts")); + } + } else { + dirs.extend(["/usr/share/fonts", "/usr/local/share/fonts", "/usr/share/fonts/TTF"].map(Into::into)); + if let Some(h) = &home { + dirs.push(h.join(".fonts")); + dirs.push(h.join(".local/share/fonts")); + } + } + dirs + } +} + +/// Weight implied by a style name (`"Bold"` → 700, `"Light"` → 300). +#[cfg(not(target_arch = "wasm32"))] +fn style_weight(style: &str) -> f32 { + let s: String = style.chars().filter(|c| c.is_alphanumeric()).flat_map(|c| c.to_lowercase()).collect(); + const TABLE: &[(&str, f32)] = &[ + ("hairline", 100.0), + ("thin", 100.0), + ("extralight", 200.0), + ("ultralight", 200.0), + ("light", 300.0), + ("medium", 500.0), + ("demibold", 600.0), + ("semibold", 600.0), + ("extrabold", 800.0), + ("ultrabold", 800.0), + ("bold", 700.0), + ("black", 900.0), + ("heavy", 900.0), + ]; + TABLE.iter().find(|(k, _)| s.contains(k)).map(|(_, w)| *w).unwrap_or(400.0) +} + +/// Whether a style name reads as italic or oblique. +#[cfg(not(target_arch = "wasm32"))] +fn style_italic(style: &str) -> bool { + let s: String = style.chars().filter(|c| c.is_alphanumeric()).flat_map(|c| c.to_lowercase()).collect(); + s.contains("italic") || s.contains("oblique") +} + +/// Every face in `data` as `(index, family, style)`. Damaged data yields nothing, never a panic. +#[cfg(not(target_arch = "wasm32"))] +fn file_faces(data: &[u8]) -> Vec<(u32, String, String)> { + use skrifa::raw::FileRef; + use skrifa::{MetadataProvider, string::StringId}; + let count = match FileRef::new(data) { + Ok(FileRef::Font(_)) => 1, + Ok(FileRef::Collection(c)) => c.len(), + Err(_) => return Vec::new(), + }; + let name = |f: &skrifa::FontRef<'_>, ids: &[StringId]| -> Option { + ids.iter().find_map(|id| f.localized_strings(*id).english_or_first().map(|s| s.to_string()).filter(|s| !s.is_empty())) + }; + (0..count) + .filter_map(|i| { + let f = skrifa::FontRef::from_index(data, i).ok()?; + let family = name(&f, &[StringId::TYPOGRAPHIC_FAMILY_NAME, StringId::FAMILY_NAME])?; + let style = name(&f, &[StringId::TYPOGRAPHIC_SUBFAMILY_NAME, StringId::SUBFAMILY_NAME]).unwrap_or_else(|| "Regular".into()); + Some((i, family, style)) + }) + .collect() +} + +/// Catalog every face under `dirs`, recursing into subfolders. A folder reached twice (through a +/// symlink) is read once, so a link back to a parent ends the walk. +#[cfg(not(target_arch = "wasm32"))] +pub fn scan_dirs(dirs: &[PathBuf]) -> Vec { + let mut out: Vec = Vec::new(); + let mut stack: Vec = dirs.to_vec(); + let mut seen_dirs: HashSet = HashSet::new(); + while let Some(dir) = stack.pop() { + let key = std::fs::canonicalize(&dir).unwrap_or_else(|_| dir.clone()); + if !seen_dirs.insert(key) { + continue; + } + let Ok(entries) = std::fs::read_dir(&dir) else { continue }; + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + stack.push(path); + continue; + } + let is_font = + path.extension().and_then(|e| e.to_str()).is_some_and(|e| matches!(e.to_ascii_lowercase().as_str(), "ttf" | "otf" | "ttc" | "otc")); + if !is_font || std::fs::metadata(&path).map(|m| m.len() > MAX_FONT_FILE).unwrap_or(true) { + continue; + } + let Ok(data) = std::fs::read(&path) else { continue }; + for (index, family, style) in file_faces(&data) { + out.push(Face { family, path: path.clone(), index, weight: style_weight(&style), italic: style_italic(&style) }); + } + } + } + out +} + +#[cfg(target_arch = "wasm32")] +pub fn scan_dirs(_dirs: &[PathBuf]) -> Vec { + Vec::new() +} + +static CATALOG: OnceLock> = OnceLock::new(); + +/// The catalog of installed faces, scanned on first use. Blocks until the scan finishes. +pub fn catalog() -> &'static [Face] { + CATALOG.get_or_init(|| scan_dirs(&system_font_dirs())) +} + +/// The catalog if the scan has finished, else `None` — never blocks, for use while painting. +fn catalog_now() -> Option<&'static [Face]> { + CATALOG.get().map(Vec::as_slice) +} + +/// Start cataloging the installed fonts on a background thread, so the first frame doesn't wait for +/// it. A no-op on wasm and after the scan has started. +pub fn start_scan() { + static STARTED: OnceLock<()> = OnceLock::new(); + STARTED.get_or_init(|| { + #[cfg(not(target_arch = "wasm32"))] + if CATALOG.get().is_none() { + // A failed spawn leaves the scan to the first blocking lookup. + let _ = std::thread::Builder::new().name("font-scan".into()).spawn(|| { + catalog(); + }); + } + }); +} + +/// Whether the background scan has finished. +pub fn scan_done() -> bool { + CATALOG.get().is_some() +} + +/// The faces of one installed family, regular-weight first (the order roles pick from). +pub fn faces_of(family: &str) -> Vec { + let Some(cat) = catalog_now() else { return Vec::new() }; + let mut v: Vec = cat.iter().filter(|f| f.family.eq_ignore_ascii_case(family)).cloned().collect(); + v.sort_by(|a, b| a.weight.total_cmp(&b.weight).then(a.italic.cmp(&b.italic)).then(a.index.cmp(&b.index))); + v +} + +/// The subset of `names` that is installed, sorted and deduped. These are the families whose files +/// get bound for a workbook. +pub fn installed_among(names: &[String]) -> Vec { + let Some(cat) = catalog_now() else { return Vec::new() }; + let mut out: Vec = Vec::new(); + let mut seen: HashSet = HashSet::new(); + for wanted in names { + if !seen.insert(wanted.to_lowercase()) { + continue; + } + if let Some(f) = cat.iter().find(|f| f.family.eq_ignore_ascii_case(wanted)) { + out.push(f.family.clone()); + } + } + out.sort_by_key(|a| a.to_lowercase()); + out +} + +/// The font list a combo box shows: the standard [`crate::ribbon::FONTS`] names first (what Excel +/// offers, and what we substitute), then every other installed family A–Z. Never blocks: until the +/// background scan finishes it returns just the standard names. +pub fn families() -> Vec { + let mut out: Vec = crate::ribbon::FONTS.iter().map(|s| s.to_string()).collect(); + let Some(cat) = catalog_now() else { return out }; + let mut seen: HashSet = out.iter().map(|s| s.to_lowercase()).collect(); + let mut installed: Vec = cat.iter().map(|f| f.family.clone()).collect(); + installed.sort_by_key(|a| a.to_lowercase()); + installed.dedup_by(|a, b| a.eq_ignore_ascii_case(b)); + for f in installed { + if seen.insert(f.to_lowercase()) { + out.push(f); + } + } + out +} + +/// The egui family name for one role of one family: `fnt:{family}:{b|n}{i|n}`. +pub fn role_key(family: &str, bold: bool, italic: bool) -> String { + format!("fnt:{family}:{}{}", if bold { 'b' } else { 'n' }, if italic { 'i' } else { 'n' }) +} + +/// The face to use for a role, or `None` when the family has no faces. Missing bold/italic faces +/// fall back to the family's regular face, so every bound role has data behind it. +fn pick_face(faces: &[Face], bold: bool, italic: bool) -> Option<&Face> { + let regular = || faces.iter().find(|f| !f.italic && (f.weight - 400.0).abs() < 1.0); + let exact = |wanted_bold: bool, wanted_italic: bool| { + faces.iter().find(|f| f.italic == wanted_italic && if wanted_bold { f.weight >= 600.0 } else { f.weight < 600.0 }) + }; + (match (bold, italic) { + (true, true) => exact(true, true).or_else(|| exact(true, false)).or_else(|| exact(false, true)), + (true, false) => exact(true, false), + (false, true) => exact(false, true), + (false, false) => None, + }) + .or_else(regular) + .or_else(|| faces.first()) +} + +/// A stable key for a face's font data, unique per file/index within the process. +fn face_key(family: &str, face: &Face) -> String { + let file = face.path.file_name().and_then(|s| s.to_str()).unwrap_or("font"); + format!("fnt-src:{family}:{file}:{}", face.index) +} + +/// Bind one installed family's faces into `fonts`, one egui family per cell role, and return the +/// role keys that are now bound. A role with no exact face chains to the family's regular face, so +/// no role is ever left pointing at missing data (egui panics on that). A family with no faces +/// binds nothing. +/// +/// Each role's chain ends with egui's default proportional fonts, so glyphs the installed font +/// lacks (emoji, a stray script) still render. +pub fn bind_faces(fonts: &mut egui::FontDefinitions, family: &str, faces: &[Face]) -> Vec { + if faces.is_empty() { + return Vec::new(); + } + let base: Vec = fonts.families.get(&egui::FontFamily::Proportional).cloned().unwrap_or_default(); + let mut bound: Vec = Vec::new(); + for (bold, italic) in [(false, false), (true, false), (false, true), (true, true)] { + let Some(primary) = pick_face(faces, bold, italic) else { continue }; + let mut chain: Vec = Vec::new(); + for face in [Some(primary), pick_face(faces, false, false)] { + let Some(face) = face else { continue }; + let key = face_key(family, face); + if chain.contains(&key) { + continue; + } + let Ok(bytes) = std::fs::read(&face.path) else { continue }; + let mut fd = egui::FontData::from_owned(bytes); + fd.index = face.index; + fonts.font_data.entry(key.clone()).or_insert_with(|| std::sync::Arc::new(fd)); + chain.push(key); + } + if chain.is_empty() { + continue; + } + chain.extend(base.iter().cloned()); + let role = role_key(family, bold, italic); + fonts.families.insert(egui::FontFamily::Name(role.clone().into()), chain); + bound.push(role); + } + bound +} + +/// Bind every installed face of `family` (see [`bind_faces`]). +pub fn bind_family(fonts: &mut egui::FontDefinitions, family: &str) -> Vec { + bind_faces(fonts, family, &faces_of(family)) +} + +/// The role families currently bound to font data. `cell_family` consults this, so it never names a +/// family egui hasn't been given. +static BOUND: OnceLock>> = OnceLock::new(); + +fn bound() -> &'static std::sync::RwLock> { + BOUND.get_or_init(|| std::sync::RwLock::new(HashSet::new())) +} + +/// Record the role families a freshly built [`egui::FontDefinitions`] bound. +pub fn set_bound(keys: impl IntoIterator) { + *bound().write().unwrap_or_else(|e| e.into_inner()) = keys.into_iter().collect(); +} + +/// Is this exact role family bound to font data? +pub fn is_bound(role: &str) -> bool { + bound().read().unwrap_or_else(|e| e.into_inner()).contains(role) +} + +/// Serializes tests that mutate the process-wide bound-family registry. +#[cfg(test)] +pub(crate) static TEST_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn style_names_map_to_weight_and_slant() { + assert_eq!(style_weight("Bold"), 700.0); + assert_eq!(style_weight("SemiBold Italic"), 600.0); + assert_eq!(style_weight("Regular"), 400.0); + assert_eq!(style_weight("Light Oblique"), 300.0); + assert!(style_italic("Bold Italic") && style_italic("Oblique")); + assert!(!style_italic("Bold")); + // The weight table is checked in order, so "extralight" wins over "light". + assert_eq!(style_weight("ExtraLight"), 200.0); + } + + #[test] + fn role_keys_are_stable_and_distinct() { + assert_eq!(role_key("Segoe UI", false, false), "fnt:Segoe UI:nn"); + assert_eq!(role_key("Segoe UI", true, false), "fnt:Segoe UI:bn"); + assert_eq!(role_key("Segoe UI", false, true), "fnt:Segoe UI:ni"); + assert_eq!(role_key("Segoe UI", true, true), "fnt:Segoe UI:bi"); + } + + #[test] + fn nothing_is_bound_until_font_definitions_says_so() { + let _g = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + // A role key is only safe to name after font_definitions bound it. + let k = role_key("Some Installed Font", true, false); + assert!(!is_bound(&k)); + set_bound([k.clone()]); + assert!(is_bound(&k)); + set_bound([]); + assert!(!is_bound(&k)); + } + + #[test] + fn damaged_or_non_font_files_contribute_nothing() { + assert!(file_faces(b"ttcf\x00\x01\x00\x00\xff\xff\xff\xff").is_empty()); + assert!(file_faces(b"not a font at all").is_empty()); + assert!(file_faces(&[]).is_empty()); + } + + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn the_scan_catalogs_fonts_and_reads_each_folder_once() { + let dir = std::env::temp_dir().join(format!("gridcraft-fonts-scan-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("Sub")).expect("temp dir"); + std::fs::write(dir.join("Ubuntu-Light.ttf"), epaint_default_fonts::UBUNTU_LIGHT).expect("write"); + std::fs::write(dir.join("damaged.ttf"), b"ttcf\x00\x01\x00\x00\xff\xff\xff\xff").expect("write"); + std::fs::write(dir.join("readme.txt"), b"not a font").expect("write"); + // The same folder listed twice (once through `Sub/..`) is read once. + let faces = scan_dirs(&[dir.clone(), dir.join("Sub/..")]); + let families: Vec<&str> = faces.iter().map(|f| f.family.as_str()).collect(); + assert_eq!(families, ["Ubuntu"], "one face, family read from the name table"); + assert_eq!(faces[0].weight, 300.0, "“Light” reads as weight 300"); + assert!(!faces[0].italic); + let _ = std::fs::remove_dir_all(&dir); + } + + #[cfg(unix)] + #[test] + fn a_link_back_to_a_parent_folder_ends_the_scan() { + let dir = std::env::temp_dir().join(format!("gridcraft-fonts-loop-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("Sub")).expect("temp dir"); + std::fs::write(dir.join("Sub/Ubuntu-Light.ttf"), epaint_default_fonts::UBUNTU_LIGHT).expect("write"); + std::os::unix::fs::symlink(&dir, dir.join("Sub/loop")).expect("symlink"); + assert_eq!(scan_dirs(&[dir.clone()]).len(), 1, "the loop doesn't multiply faces"); + let _ = std::fs::remove_dir_all(&dir); + } + + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn binding_a_family_binds_all_four_roles_and_never_stores_missing_data() { + let dir = std::env::temp_dir().join(format!("gridcraft-fonts-bind-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("temp dir"); + let path = dir.join("Ubuntu-Light.ttf"); + std::fs::write(&path, epaint_default_fonts::UBUNTU_LIGHT).expect("write"); + let face = Face { family: "Ubuntu".into(), path, index: 0, weight: 300.0, italic: false }; + + let mut fonts = egui::FontDefinitions::default(); + let bound = bind_faces(&mut fonts, "Ubuntu", std::slice::from_ref(&face)); + assert_eq!(bound.len(), 4, "regular, bold, italic and bold-italic roles"); + for key in &bound { + let chain = fonts.families.get(&egui::FontFamily::Name(key.as_str().into())).expect("role family"); + assert!(chain.iter().all(|k| fonts.font_data.contains_key(k)), "every chained key has data"); + } + // A family with no faces binds nothing and never touches the map. + assert!(bind_faces(&mut fonts, "No Such Font", &[]).is_empty()); + let _ = std::fs::remove_dir_all(&dir); + } +} diff --git a/crates/ui-egui/src/formula_bar.rs b/crates/ui-egui/src/formula_bar.rs index 753585f..48141c3 100644 --- a/crates/ui-egui/src/formula_bar.rs +++ b/crates/ui-egui/src/formula_bar.rs @@ -166,7 +166,7 @@ pub fn editor_widget(app: &mut SheetApp, ui: &mut egui::Ui, id: egui::Id, font: ed.caret_to_end = false; ed.sync_caret = true; } - if !ed.autocomplete.is_empty() && (tab || (enter && false)) { + if !ed.autocomplete.is_empty() && tab { ed.accept_autocomplete(); } else if enter { action = Some((if shift { -1 } else { 1 }, 0, false, false)); diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 4b326cc..4faf43a 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -577,7 +577,7 @@ fn paint_quadrant( }; let indent = st.align.indent as f32 * 9.0 * z; let pad = 2.5 * z; - if st.align.wrap || st.align.h == HAlign::Justify || text.contains('\n') && st.align.wrap { + if st.align.wrap || st.align.h == HAlign::Justify { let wrap_w = (rect.width() - 2.0 * pad - indent).max(4.0); let mut job = egui::text::LayoutJob::simple(text.clone(), font.clone(), color, wrap_w); job.halign = match halign { @@ -1651,12 +1651,7 @@ fn filter_menu(app: &mut SheetApp, ui: &mut egui::Ui, geo: &Geo) { let Some(sh) = wb.sheet(si) else { return }; let _ = geo; // Tables get an autofilter on demand. - let table_range = sh - .tables - .iter() - .find(|t| t.range.start.row <= at.y as u32 || true) - .filter(|t| col >= t.range.start.col && col <= t.range.end.col) - .map(|t| t.range); + let table_range = sh.tables.first().filter(|t| col >= t.range.start.col && col <= t.range.end.col).map(|t| t.range); if sh.autofilter.is_none() && let Some(r) = table_range { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 8eb1eca..fd08ba3 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -11,6 +11,7 @@ pub mod control; pub mod credits; pub mod dialogs; pub mod editor; +pub mod fonts; pub mod formula_bar; pub mod grid; pub mod icons; @@ -104,6 +105,14 @@ pub struct SheetApp { pub perf: Perf, pub fonts_ready: bool, fonts_set: bool, + /// Workbook fonts bound into the current font set; a change rebinds (installed fonts only). + fonts_bound: Vec, + /// The (doc uid, revision) `fonts_bound` was computed for, so the style table is walked only + /// when the workbook changes. + fonts_seen: (u64, u64), + /// Set while a font-set change is pending activation: skip this frame's paint, like the + /// startup `fonts_ready` gate (egui applies new definitions at the next pass). + fonts_rebind: bool, pub name_box: Option, pub(crate) shots: control::Shots, /// Chart selected on the sheet (id). @@ -133,6 +142,9 @@ impl SheetApp { perf: Perf::default(), fonts_ready: false, fonts_set: false, + fonts_bound: Vec::new(), + fonts_seen: (0, 0), + fonts_rebind: false, name_box: None, shots: control::Shots::default(), selected_chart: None, @@ -140,12 +152,28 @@ impl SheetApp { } } - /// One-time context setup: fonts and visuals. + /// One-time context setup: start cataloging the machine's fonts, then install the fonts and + /// visuals. Workbook-specific fonts are bound later, once the scan is done (see [`logic`]). + /// + /// [`logic`]: SheetApp::logic pub fn setup_context(ctx: &egui::Context, dark: bool) { - ctx.set_fonts(theme::font_definitions()); + fonts::start_scan(); + ctx.set_fonts(theme::font_definitions(&[])); theme::apply(ctx, dark); } + /// The font names the active workbook uses: every style's font plus the theme's heading/body + /// fonts. These are the families [`fonts`] binds an installed file for. + pub fn used_fonts(&self) -> Vec { + let mut names: Vec = Vec::new(); + if let Some(d) = self.session.active() { + names.extend(d.wb.styles.iter().map(|(_, s)| s.font.name.clone())); + names.push(d.wb.theme.major_font.clone()); + names.push(d.wb.theme.minor_font.clone()); + } + names + } + /// Runs an engine command (or a UI command) and handles UI requests it makes. pub fn run(&mut self, id: &str, params: Json) -> Result { if let Some(r) = self.run_ui_command(id, ¶ms) { @@ -361,6 +389,24 @@ impl SheetApp { self.fonts_set = true; ctx.request_repaint(); } + } else if self.fonts_rebind { + // A font-set change made last frame is active now. + self.fonts_rebind = false; + } else if fonts::scan_done() { + // Bind the workbook's installed fonts once the background scan has finished. Adding + // fonts re-applies the font set, which egui activates at the next pass, so skip this + // frame's paint (same one-frame cost as startup). Only when the workbook changed. + let seen = self.session.active().map(|d| (d.uid, d.revision)).unwrap_or((0, 0)); + if seen != self.fonts_seen { + let used = self.used_fonts(); + self.fonts_seen = seen; + if used != self.fonts_bound { + ctx.set_fonts(theme::font_definitions(&used)); + self.fonts_bound = used; + self.fonts_rebind = true; + ctx.request_repaint(); + } + } } if ctx.global_style().visuals.dark_mode != self.ui.dark { theme::apply(ctx, self.ui.dark); @@ -394,7 +440,7 @@ impl SheetApp { /// Lays out the whole window. pub fn ui(&mut self, ui: &mut egui::Ui) { let ctx = ui.ctx().clone(); - if !self.fonts_ready { + if !self.fonts_ready || self.fonts_rebind { ctx.request_repaint(); return; } diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 47c5432..6abe3f3 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -688,9 +688,9 @@ fn font_combo(app: &mut SheetApp, ui: &mut Ui, st: &Style) { let mut name = st.font.name.clone(); let before = name.clone(); egui::ComboBox::from_id_salt("font_name").width(150.0).selected_text(egui::RichText::new(&name).font(theme::ui_font(12.5))).show_ui(ui, |ui| { - for f in FONTS { - let fam = theme::cell_family(f, false, false); - ui.selectable_value(&mut name, f.to_string(), egui::RichText::new(*f).font(egui::FontId::new(14.0, fam))); + for f in crate::fonts::families() { + let fam = theme::cell_family(&f, false, false); + ui.selectable_value(&mut name, f.clone(), egui::RichText::new(&f).font(egui::FontId::new(14.0, fam))); } }); if name != before { diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index 2dbd8df..1b51559 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -142,7 +142,16 @@ pub fn ui_bold(size: f32) -> FontId { } /// The font family to use for a cell font name and weight. +/// +/// When the name is an installed font whose faces were bound by [`font_definitions`], that family +/// is used; otherwise the name is bucketed to one of the built-in roles (mono/serif/cell). egui +/// panics on a family that was never bound, so the installed branch is gated on +/// [`crate::fonts::is_bound`] and never names a family egui hasn't been given. pub fn cell_family(name: &str, bold: bool, italic: bool) -> FontFamily { + let role = crate::fonts::role_key(name, bold, italic); + if crate::fonts::is_bound(&role) { + return FontFamily::Name(role.into()); + } let lower = name.to_ascii_lowercase(); if lower.contains("courier") || lower.contains("mono") || lower.contains("consolas") { return FontFamily::Name(MONO.into()); @@ -288,10 +297,16 @@ fn candidates(role: &str) -> Vec<(String, u32)> { } /// Builds the font set from system fonts (with egui's defaults as fallback). -pub fn font_definitions() -> FontDefinitions { +/// +/// `used` names the workbook's fonts (its styles' fonts and the theme's heading/body fonts). Any of +/// them that is installed is bound by name, so cells set in it render with it; the rest fall back +/// to the roles above. The set of bound role families is recorded in [`crate::fonts`] so +/// [`cell_family`] only ever names a family egui has data for. +pub fn font_definitions(used: &[String]) -> FontDefinitions { let mut fonts = FontDefinitions::default(); let base_prop: Vec = fonts.families.get(&FontFamily::Proportional).cloned().unwrap_or_default(); let base_mono: Vec = fonts.families.get(&FontFamily::Monospace).cloned().unwrap_or_default(); + let mut bound: Vec = Vec::new(); for role in [UI, UI_BOLD, CELL, CELL_BOLD, CELL_ITALIC, CELL_BOLD_ITALIC, SERIF, MONO] { let paths = candidates(role); let refs: Vec<(&str, u32)> = paths.iter().map(|(p, i)| (p.as_str(), *i)).collect(); @@ -308,11 +323,17 @@ pub fn font_definitions() -> FontDefinitions { chain.extend(if role == MONO { base_mono.clone() } else { base_prop.clone() }); chain.retain(|k| fonts.font_data.contains_key(k)); fonts.families.insert(FontFamily::Name(role.into()), chain); + bound.push(role.to_string()); + } + // Installed workbook fonts, so a cell's chosen font actually renders in it. + for family in crate::fonts::installed_among(used) { + bound.extend(crate::fonts::bind_family(&mut fonts, &family)); } // Default proportional text in widgets uses the UI font. if let Some(ui) = fonts.families.get(&FontFamily::Name(UI.into())).cloned() { fonts.families.insert(FontFamily::Proportional, ui); } + crate::fonts::set_bound(bound); fonts } @@ -351,3 +372,51 @@ pub fn apply(ctx: &egui::Context, dark: bool) { pub fn color32(rgb: [u8; 3]) -> Color32 { Color32::from_rgb(rgb[0], rgb[1], rgb[2]) } + +#[cfg(test)] +mod tests { + use super::*; + + /// The families [`font_definitions`] always binds, whatever is installed. + fn always_bound() -> Vec { + [UI, UI_BOLD, CELL, CELL_BOLD, CELL_ITALIC, CELL_BOLD_ITALIC, SERIF, MONO].iter().map(|s| s.to_string()).collect() + } + + #[test] + fn a_name_that_is_not_an_installed_font_gets_a_built_in_role() { + let _g = crate::fonts::TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + // Nothing installed: every name buckets to a role that font_definitions bound. + crate::fonts::set_bound(always_bound()); + for name in ["", "Calibri", "Courier New", "Times New Roman", "Grossly Missing Font", "\u{1F600}"] { + for (b, i) in [(false, false), (true, false), (false, true), (true, true)] { + let FontFamily::Name(fam) = cell_family(name, b, i) else { panic!("{name:?} should name a family") }; + assert!(crate::fonts::is_bound(&fam), "{name:?} {b} {i} → unbound {fam:?}"); + } + } + } + + #[test] + fn a_bound_installed_font_is_used_by_name() { + let _g = crate::fonts::TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let role = crate::fonts::role_key("Ubuntu", true, false); + crate::fonts::set_bound(always_bound().into_iter().chain([role.clone()])); + assert!(matches!(cell_family("Ubuntu", true, false), FontFamily::Name(f) if *f == *role)); + // The same family in a style that wasn't bound falls back to a built-in role that is. + let FontFamily::Name(fallback) = cell_family("Ubuntu", false, true) else { panic!("should name a family") }; + assert!(crate::fonts::is_bound(&fallback), "the fallback is a bound role"); + crate::fonts::set_bound(always_bound()); + } + + #[test] + fn arbitrary_names_never_yield_an_unbound_family() { + let _g = crate::fonts::TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + // A workbook can hold any font name; none may name a family egui has no data for. + crate::fonts::set_bound(always_bound()); + for seed in 0..500u32 { + let name: String = + (0..(seed % 40)).map(|k| char::from_u32(0x20 + (seed.wrapping_mul(31).wrapping_add(k) % 0x5F)).unwrap_or('x')).collect(); + let FontFamily::Name(fam) = cell_family(&name, seed % 2 == 0, seed % 3 == 0) else { continue }; + assert!(crate::fonts::is_bound(&fam), "{name:?} → unbound {fam:?}"); + } + } +} From 2ded7bf19333492e869442784475083e814c4c00 Mon Sep 17 00:00:00 2001 From: nihilist-narwhal <240798405+nihilist-narwhal@users.noreply.github.com> Date: Sat, 10 Oct 2026 01:39:01 -0500 Subject: [PATCH 02/19] XLSX: write the _xlfn. prefix for GROUPBY, PIVOTBY, ENCODEURL and the REGEX functions These were missing from the list of functions newer than the original file format, so they were saved without the prefix, and Excel shows #NAME? for such formulas. --- crates/xlsx/src/fmla.rs | 2 ++ crates/xlsx/src/tables.rs | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/crates/xlsx/src/fmla.rs b/crates/xlsx/src/fmla.rs index 9ac4ccf..c483414 100644 --- a/crates/xlsx/src/fmla.rs +++ b/crates/xlsx/src/fmla.rs @@ -132,6 +132,8 @@ mod tests { assert_eq!(to_file(&Formula::new("FILTER(A1:A3,B1:B3)")), "_xlfn._xlws.FILTER(A1:A3,B1:B3)"); assert_eq!(from_file("_xlfn.XLOOKUP(1,A:A,B:B)"), "XLOOKUP(1,A:A,B:B)"); assert_eq!(text_to_file("=IFS(A1>0,1)"), "_xlfn.IFS(A1>0,1)"); + assert_eq!(to_file(&Formula::new("REGEXTEST(A1,\"x\")&ENCODEURL(B1)")), "_xlfn.REGEXTEST(A1,\"x\")&_xlfn.ENCODEURL(B1)"); + assert_eq!(to_file(&Formula::new("GROUPBY(A1:A3,B1:B3,\"SUM\")")), "_xlfn.GROUPBY(A1:A3,B1:B3,\"SUM\")"); } #[test] diff --git a/crates/xlsx/src/tables.rs b/crates/xlsx/src/tables.rs index 2d63dec..fe7cbdc 100644 --- a/crates/xlsx/src/tables.rs +++ b/crates/xlsx/src/tables.rs @@ -133,6 +133,7 @@ const FUTURE_FUNCTIONS: &[&str] = &[ "DAYS", "DECIMAL", "DROP", + "ENCODEURL", "ERF.PRECISE", "ERFC.PRECISE", "EXPAND", @@ -157,6 +158,7 @@ const FUTURE_FUNCTIONS: &[&str] = &[ "GAMMA.INV", "GAMMALN.PRECISE", "GAUSS", + "GROUPBY", "HSTACK", "HYPGEOM.DIST", "IFNA", @@ -199,6 +201,7 @@ const FUTURE_FUNCTIONS: &[&str] = &[ "PERCENTRANK.INC", "PERMUTATIONA", "PHI", + "PIVOTBY", "POISSON.DIST", "QUARTILE.EXC", "QUARTILE.INC", @@ -206,6 +209,9 @@ const FUTURE_FUNCTIONS: &[&str] = &[ "RANK.AVG", "RANK.EQ", "REDUCE", + "REGEXEXTRACT", + "REGEXREPLACE", + "REGEXTEST", "RRI", "SCAN", "SEC", From 85ca8f6af57cca475886d6b9179abb4cfb057060 Mon Sep 17 00:00:00 2001 From: nihilist-narwhal <240798405+nihilist-narwhal@users.noreply.github.com> Date: Sat, 10 Oct 2026 02:54:04 -0500 Subject: [PATCH 03/19] INDEX: arrays of row and column numbers pick a value each INDEX took one row and one column number, so INDEX(A1:C3,{1;3},{1,3}) gave a single value and SUM(INDEX(A1:C3,{1,2,3},2)) summed one cell. In Excel, arrays of positions return an array of the values at each (broadcast) position. A position that picks a whole row or column gives #CALC! there, as arrays can't nest. --- crates/calc/src/eval.rs | 57 +++++++++++++++++++++++++++++++++------- crates/calc/src/tests.rs | 18 +++++++++++++ 2 files changed, 66 insertions(+), 9 deletions(-) diff --git a/crates/calc/src/eval.rs b/crates/calc/src/eval.rs index 3d4a743..9aa80fd 100644 --- a/crates/calc/src/eval.rs +++ b/crates/calc/src/eval.rs @@ -1085,18 +1085,57 @@ impl<'h> Evaluator<'h> { return err(CellError::Value); } let src = self.eval(&args[0]); - let row = match self.arg_num(args, 1, 0.0) { - Ok(n) => n.trunc() as i64, - Err(e) => return err(e), - }; - let col = match self.arg_num(args, 2, 0.0) { - Ok(n) => n.trunc() as i64, - Err(e) => return err(e), + let mut arg = |i: usize| match args.get(i) { + None | Some(Expr::Missing) => None, + Some(e) => Some(self.value(e)), }; + let (rows, cols) = (arg(1), arg(2)); let area_n = match self.arg_num(args, 3, 1.0) { Ok(n) => n.trunc() as i64, Err(e) => return err(e), }; + let two = args.len() == 2; + let num = |v: &Option| v.as_ref().map_or(Ok(0.0), |v| v.to_number()).map(|n| n.trunc() as i64); + // Arrays of positions pick a value for each: INDEX(A1:C10,{1;2},{1,3}) is 2×2. + if let (Some(Value::Array(_)), _) | (_, Some(Value::Array(_))) = (&rows, &cols) { + let as_array = |v: &Option| match v { + Some(Value::Array(a)) => Arc::clone(a), + Some(v) => Arc::new(Array::scalar(v.clone())), + None => Arc::new(Array::scalar(Value::Number(0.0))), + }; + let (ra, ca) = (as_array(&rows), as_array(&cols)); + let (h, w) = (ra.rows.max(ca.rows), ra.cols.max(ca.cols)); + if (h * w) as u64 > MAX_CELLS { + return err(CellError::Num); + } + let mut out = Vec::with_capacity(h * w); + for r in 0..h { + for c in 0..w { + let pos = (num(&Some(ra.get_broadcast(r, c))), num(&Some(ca.get_broadcast(r, c)))); + let v = match pos { + (Ok(row), Ok(col)) => { + let one = self.index_one(src.clone(), row, col, area_n, two); + match self.deref(one) { + Value::Array(a) if a.rows * a.cols > 1 => Value::Error(CellError::Calc), + v => v.scalar(), + } + } + (Err(e), _) | (_, Err(e)) => Value::Error(e), + }; + out.push(v); + } + } + return Ev::V(Array::new(h, w, out).map(Value::from).unwrap_or(Value::Error(CellError::Value))); + } + let (row, col) = match (num(&rows), num(&cols)) { + (Ok(r), Ok(c)) => (r, c), + (Err(e), _) | (_, Err(e)) => return err(e), + }; + self.index_one(src, row, col, area_n, two) + } + + /// INDEX for one row and column position (0 = the whole column or row). + fn index_one(&mut self, src: Ev, row: i64, col: i64, area_n: i64, two_args: bool) -> Ev { if row < 0 || col < 0 { return err(CellError::Value); } @@ -1105,7 +1144,7 @@ impl<'h> Evaluator<'h> { let Some(a) = areas.get((area_n - 1).max(0) as usize).copied() else { return err(CellError::Ref) }; let r = a.range; // A single row or column with one index picks along it. - let (row, col) = if args.len() == 2 && r.height() == 1 { (1, row) } else { (row, col) }; + let (row, col) = if two_args && r.height() == 1 { (1, row) } else { (row, col) }; if row as u64 > r.height() as u64 || col as u64 > r.width() as u64 { return err(CellError::Ref); } @@ -1114,7 +1153,7 @@ impl<'h> Evaluator<'h> { Ev::R(vec![Area { sheet: a.sheet, range: RangeRef::new(CellRef::new(r0, c0), CellRef::new(r1, c1)) }]) } Ev::V(Value::Array(arr)) => { - let (row, col) = if args.len() == 2 && arr.rows == 1 { (1, row) } else { (row, col) }; + let (row, col) = if two_args && arr.rows == 1 { (1, row) } else { (row, col) }; if row as usize > arr.rows || col as usize > arr.cols { return err(CellError::Ref); } diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index 2205a1e..50c4353 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -106,6 +106,24 @@ fn long_chain() { assert_eq!(t.num("A3000"), 3009.0); } +#[test] +fn index_with_arrays_of_positions() { + let mut t = T::new(); + for (i, row) in [[1, 2, 3], [4, 5, 6], [7, 8, 9]].iter().enumerate() { + for (j, v) in row.iter().enumerate() { + t.set(&format!("{}{}", ["A", "B", "C"][j], i + 1), &v.to_string()); + } + } + t.set("E1", "=INDEX(A1:C3,{1;3},{1,3})"); + assert_eq!([t.num("E1"), t.num("F1"), t.num("E2"), t.num("F2")], [1.0, 3.0, 7.0, 9.0]); + t.set("E4", "=SUM(INDEX(A1:C3,{1,2,3},2))"); + assert_eq!(t.num("E4"), 15.0); + t.set("E5", "=INDEX({10,20,30},{3,1})"); + assert_eq!([t.num("E5"), t.num("F5")], [30.0, 10.0]); + t.set("E7", "=INDEX(A1:C3,2,3)"); + assert_eq!(t.num("E7"), 6.0); +} + #[test] fn errors_and_cycles() { let mut t = T::new(); From 683ff6003c3f80975d383d732d80fb08b3ee5521 Mon Sep 17 00:00:00 2001 From: nihilist-narwhal <240798405+nihilist-narwhal@users.noreply.github.com> Date: Sat, 10 Oct 2026 02:31:01 -0500 Subject: [PATCH 04/19] Defined names with relative references A name whose references have no $ was evaluated as if they were absolute. In Excel such references are relative: defined from B2, =Sheet1!A2 means "the cell to the left", and the file stores it relative to A1 (Sheet1!XFD1, wrapping around the sheet's edge). Name formulas with relative references are now moved by the using cell's offset from A1, wrapping at the edges, and formulas.defineName takes relative references as relative to the active cell, as Excel's Name Manager does, and stores them relative to A1. --- crates/calc/src/eval.rs | 7 +++++++ crates/calc/src/tests.rs | 26 +++++++++++++++++++++++ crates/engine/src/cmd/formulas.rs | 34 ++++++++++++++++++++++++++++++- crates/formula/src/adjust.rs | 29 ++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 1 deletion(-) diff --git a/crates/calc/src/eval.rs b/crates/calc/src/eval.rs index 3d4a743..e5d621d 100644 --- a/crates/calc/src/eval.rs +++ b/crates/calc/src/eval.rs @@ -249,6 +249,13 @@ impl<'h> Evaluator<'h> { let scope = def.scope.unwrap_or(self.sheet); return match gridcraft_formula::parse(&text) { Ok(expr) => { + // Relative references in a name are relative to A1: used here, they move by + // this cell's offset from it. + let expr = if gridcraft_formula::adjust::has_relative_refs(&expr) { + gridcraft_formula::adjust::shift_relative_wrapping(expr, self.at.row as i64, self.at.col as i64) + } else { + expr + }; let saved = self.sheet; // Unqualified references in a name refer to the scope sheet. self.sheet = scope; diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index 2205a1e..bf64b5c 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -261,6 +261,32 @@ fn references_functions() { assert_eq!(t.num("B6"), 105.0); } +#[test] +fn relative_names() { + let mut t = T::new(); + // As files store them: relative to A1. `Left` is the cell to the left, `Above3` the three + // cells above, `Mixed` column A of the same row. + for (name, formula) in [("Left", "Sheet1!XFD1"), ("Above3", "Sheet1!A1048574:A1048576"), ("Mixed", "Sheet1!$A1")] { + t.wb.names.push(gridcraft_model::DefinedName { + name: name.into(), + scope: None, + formula: formula.into(), + comment: String::new(), + hidden: false, + }); + } + t.calc.rebuild(&t.wb); + for r in 1..=5 { + t.set(&format!("A{r}"), &(r * 10).to_string()); + } + t.set("B2", "=Left"); + assert_eq!(t.num("B2"), 20.0); + t.set("B4", "=Left+Mixed"); + assert_eq!(t.num("B4"), 80.0); + t.set("A6", "=SUM(Above3)"); + assert_eq!(t.num("A6"), 120.0); +} + #[test] fn spills() { let mut t = T::new(); diff --git a/crates/engine/src/cmd/formulas.rs b/crates/engine/src/cmd/formulas.rs index db8176b..df48a2c 100644 --- a/crates/engine/src/cmd/formulas.rs +++ b/crates/engine/src/cmd/formulas.rs @@ -253,7 +253,15 @@ fn define_name(s: &mut Session, p: &Json) -> Result { Some(sh) => Some(d.wb.sheet_index(sh).ok_or_else(|| bad("formulas.defineName", "no such sheet"))?), }; let refers = match str_param(p, "refersTo") { - Some(r) => r.trim_start_matches('=').to_string(), + // As in Excel, relative references are relative to the active cell; names keep them + // relative to A1, as files do. + Some(r) => match gridcraft_formula::parse(r.trim_start_matches('=')) { + Ok(e) if gridcraft_formula::adjust::has_relative_refs(&e) => { + let at = d.selection.active; + gridcraft_formula::print(&gridcraft_formula::adjust::shift_relative_wrapping(e, -(at.row as i64), -(at.col as i64))) + } + _ => r.trim_start_matches('=').to_string(), + }, None => { let sh = d.wb.active().ok_or(EngineError::NoDocument)?; let r = d.selection.current(); @@ -564,3 +572,27 @@ fn calc_options(s: &mut Session, p: &Json) -> Result { Ok(json!({"mode": format!("{:?}", cx.wb.calc.mode)})) }) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn names_with_relative_references() { + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [[1], [2], [3]]})).unwrap(); + // Defined from B2, `=Sheet1!A2` is "the cell to the left". + s.execute("selection.set", json!({"range": "B2"})).unwrap(); + let r = s.execute("formulas.defineName", json!({"name": "LeftCell", "refersTo": "=Sheet1!A2"})).unwrap(); + assert_eq!(r["refersTo"], "=Sheet1!XFD1"); + s.execute("cell.set", json!({"cell": "B1", "input": "=LeftCell"})).unwrap(); + s.execute("cell.set", json!({"cell": "B3", "input": "=LeftCell*10"})).unwrap(); + let v = |s: &mut Session, a: &str| s.execute("cell.get", json!({"cell": a})).unwrap()["value"].clone(); + assert_eq!(v(&mut s, "B1"), json!(1.0)); + assert_eq!(v(&mut s, "B3"), json!(30.0)); + // Absolute references are kept as they are. + let r = s.execute("formulas.defineName", json!({"name": "First", "refersTo": "=Sheet1!$A$1"})).unwrap(); + assert_eq!(r["refersTo"], "=Sheet1!$A$1"); + } +} diff --git a/crates/formula/src/adjust.rs b/crates/formula/src/adjust.rs index 7601674..a54eb0d 100644 --- a/crates/formula/src/adjust.rs +++ b/crates/formula/src/adjust.rs @@ -16,6 +16,35 @@ pub fn shift_relative(e: Expr, dr: i64, dc: i64) -> Expr { }) } +/// Defined names: relative parts move by `(dr, dc)`, wrapping around the sheet's edges. Files +/// store a name's relative references as seen from A1 (a name for "the cell to the left" defined +/// in B1 is `Sheet1!XFD1`), and a use in another cell moves them by that cell's offset from A1. +pub fn shift_relative_wrapping(e: Expr, dr: i64, dc: i64) -> Expr { + let wrap = |v: u32, abs: bool, d: i64, max: u32| if abs { v } else { (v as i64 + d).rem_euclid(max as i64) as u32 }; + let anchor = |a: Anchor| Anchor { row: wrap(a.row, a.row_abs, dr, MAX_ROWS), col: wrap(a.col, a.col_abs, dc, MAX_COLS), ..a }; + e.map(&mut |x| match x { + Expr::Ref(r) => { + let kind = match r.kind { + RefKind::Cell(a) => RefKind::Cell(anchor(a)), + RefKind::Range(a, b) => RefKind::Range(anchor(a), anchor(b)), + RefKind::Rows(r0, a0, r1, a1) => RefKind::Rows(wrap(r0, a0, dr, MAX_ROWS), a0, wrap(r1, a1, dr, MAX_ROWS), a1), + RefKind::Cols(c0, a0, c1, a1) => RefKind::Cols(wrap(c0, a0, dc, MAX_COLS), a0, wrap(c1, a1, dc, MAX_COLS), a1), + }; + Expr::Ref(Reference { sheet: r.sheet, kind }) + } + other => other, + }) +} + +/// Whether the formula has a relative (no `$`) row or column anywhere. +pub fn has_relative_refs(e: &Expr) -> bool { + e.references().iter().any(|r| match r.kind { + RefKind::Cell(a) => !a.row_abs || !a.col_abs, + RefKind::Range(a, b) => !a.row_abs || !a.col_abs || !b.row_abs || !b.col_abs, + RefKind::Rows(_, a0, _, a1) | RefKind::Cols(_, a0, _, a1) => !a0 || !a1, + }) +} + fn shift_anchor(a: &Anchor, dr: i64, dc: i64) -> Option { let row = if a.row_abs { a.row as i64 } else { a.row as i64 + dr }; let col = if a.col_abs { a.col as i64 } else { a.col as i64 + dc }; From 686501c7de991ccd3fa830c94281df6ab1047af4 Mon Sep 17 00:00:00 2001 From: nihilist-narwhal <240798405+nihilist-narwhal@users.noreply.github.com> Date: Sat, 10 Oct 2026 04:01:24 -0500 Subject: [PATCH 05/19] Data validation: relative formulas, checked with the value being entered Validation formulas were evaluated as written, at every cell of the range, and against the cell's old value. A Custom rule =A1>10 on A1:A9 rejected 20 typed into A1 (A1 was still empty) and checked A1, not A5, when typing into A5; a Whole number rule "<= B1" on C1:C9 compared C2 with B1. As in Excel, the formulas are now relative to the first cell of the range and see the value being entered. --- crates/engine/src/cmd/data.rs | 35 +++++++++++++++++++++++++++++++++-- crates/engine/src/tests.rs | 13 +++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index 32ec866..cb793e7 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -952,10 +952,26 @@ pub fn check_validation(wb: &Workbook, sheet: usize, at: CellRef, input: &str) - return None; } let v = gridcraft_core::parse::parse_input(input, wb.date_system).value; - let num = |f: &str| gridcraft_calc::evaluate(wb, sheet, at, f).to_number().ok(); + // The formulas are written for the first cell of the range and see the value being entered, + // as in Excel (a custom `=A1>10` on A1:A9 checks A5's new value at A5). + let first = dv.ranges.first().map(|r| r.start).unwrap_or(at); + let entered; + let wb = if sh.value(at) == v { + wb + } else { + let mut w = wb.clone(); + if let Some(sh) = w.sheet_mut(sheet) { + let style = sh.style_id(at); + sh.cells.set(at, Cell { value: v.clone(), style, ..Default::default() }); + } + entered = w; + &entered + }; + let eval = |f: &str| gridcraft_calc::evaluate(wb, sheet, at, &formula_at(f, first, at)); + let num = |f: &str| eval(f).to_number().ok(); let ok = match dv.kind { ValidationKind::List => list_items(wb, sheet, dv).iter().any(|x| x.eq_ignore_ascii_case(input)), - ValidationKind::Custom => gridcraft_calc::evaluate(wb, sheet, at, &dv.f1).to_bool().unwrap_or(false), + ValidationKind::Custom => eval(&dv.f1).to_bool().unwrap_or(false), ValidationKind::TextLength => { let n = input.chars().count() as f64; compare_op(dv.op, n, num(&dv.f1), dv.f2.as_deref().and_then(num)) @@ -982,6 +998,21 @@ pub fn check_validation(wb: &Workbook, sheet: usize, at: CellRef, input: &str) - Some((dv.clone(), msg)) } +/// A formula written for `first`, as seen from `at` (relative references shifted). +fn formula_at(f: &str, first: CellRef, at: CellRef) -> String { + if first == at { + return f.to_string(); + } + match gridcraft_formula::parse(f.trim_start_matches('=')) { + Ok(e) => gridcraft_formula::print(&gridcraft_formula::adjust::shift_relative( + e, + at.row as i64 - first.row as i64, + at.col as i64 - first.col as i64, + )), + Err(_) => f.to_string(), + } +} + fn compare_op(op: CfOperator, n: f64, a: Option, b: Option) -> bool { let Some(a) = a else { return false }; match op { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 575cc77..0322095 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1152,6 +1152,19 @@ fn ods_import_never_reuses_source_as_save_target() { std::fs::remove_dir_all(dir).unwrap(); } +#[test] +fn validation_formulas_are_relative_and_see_the_new_value() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "B1", "values": [[1], [100]]})).unwrap(); + s.execute("data.validation", json!({"range": "A1:A2", "type": "custom", "formula1": "=A1>10"})).unwrap(); + s.execute("data.validation", json!({"range": "C1:C2", "type": "whole", "operator": "lessOrEqual", "formula1": "=B1"})).unwrap(); + let ok = |s: &mut Session, cell: &str, input: &str| s.execute("data.validate", json!({"cell": cell, "input": input})).unwrap()["ok"] == true; + assert!(ok(&mut s, "A1", "20")); + assert!(!ok(&mut s, "A2", "5")); + assert!(ok(&mut s, "C2", "50")); + assert!(!ok(&mut s, "C1", "50")); +} + #[test] fn samples_build() { for (name, _) in crate::sample::SAMPLES { From 7a4aaebab0f60b3b5511aa14d1ca49743db676dc Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sun, 11 Oct 2026 20:58:49 +0200 Subject: [PATCH 06/19] Grid: merged ranges draw their right and bottom borders from the edge cells (#376) Excel stores a merged range's outline on the cells along each edge; the grid only used the anchor cell's borders, so a boxed merge showed only top and left. Co-Authored-By: Claude Opus 5.5 --- ROADMAP.md | 1 + crates/ui-egui/src/grid.rs | 62 +++++++++++++++++++++++++++++---- crates/ui-egui/tests/borders.rs | 33 ++++++++++++++++++ 3 files changed, 89 insertions(+), 7 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index a62418e..aef29c1 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-11 | Merged cells draw their whole outline: right and bottom edges come from the last column's and last row's cells (#376) | | 2026-10-11 | Legacy Excel 97-2003 `.xls` (BIFF8) import with formulas, number formats, merges, sizes and names (#121) | | 2026-10-11 | XLSX pass-through: custom XML, external links, connections, query tables, printer settings, form/ActiveX controls and OLE objects are kept on save (#330, #324) | | 2026-10-11 | Data › Forecast › Forecast Sheet: a new sheet with the history, FORECAST.ETS forecast, confidence bounds, statistics and a chart (#322) | diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index f6dc683..468b406 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -8,7 +8,7 @@ use std::sync::Arc; use egui::{Align2, Color32, CursorIcon, FontId, Painter, Pos2, Rect, Sense, Stroke, StrokeKind, pos2, vec2}; use gridcraft_engine::cf::{CfCache, CfLook}; use gridcraft_engine::core::{CellRef, MAX_COLS, MAX_ROWS, RangeRef, Value, col_to_letters}; -use gridcraft_engine::model::{BorderStyle, Color, HAlign, LineIndex, PatternType, Sheet, Style, Underline, VAlign, Workbook}; +use gridcraft_engine::model::{BorderLine, BorderStyle, Color, HAlign, LineIndex, PatternType, Sheet, Style, Underline, VAlign, Workbook}; use serde_json::json; use crate::SheetApp; @@ -1037,12 +1037,19 @@ pub(crate) fn paint_quadrant( // Borders (over fills and text). for (c, rect, lk, _) in &looks { let b = &lk.style.borders; - let edges = [ - (b.top, [rect.left_top(), rect.right_top()]), - (b.bottom, [rect.left_bottom(), rect.right_bottom()]), - (b.left, [rect.left_top(), rect.left_bottom()]), - (b.right, [rect.right_top(), rect.right_bottom()]), - ]; + let merge = merges.iter().find(|m| m.start == *c); + let edges = match merge { + // A merged area's outline lives on the cells along each of its edges (Excel writes the + // right edge on the last column's cells and the bottom edge on the last row's), so each + // edge is drawn cell by cell from the cell it belongs to. + Some(m) => merged_edges(wb, si, sh, geo, *m, *rect, (r0, r1), (c0, c1), cf), + None => vec![ + (b.top, [rect.left_top(), rect.right_top()]), + (b.bottom, [rect.left_bottom(), rect.right_bottom()]), + (b.left, [rect.left_top(), rect.left_bottom()]), + (b.right, [rect.right_top(), rect.right_bottom()]), + ], + }; for (line, pts) in edges { if line.is_none() { continue; @@ -1113,6 +1120,47 @@ pub fn filter_button_rect(geo: &Geo, sh: &Sheet, c: CellRef) -> Rect { Rect::from_min_size(pos2(rect.right() - s - 2.0, rect.bottom() - s - 2.0), vec2(s, s)) } +/// The outline segments of merged area `m` drawn at `rect`: the top and left edges come from the +/// first row's and first column's cells, the bottom and right edges from the last row's and last +/// column's cells, one segment per visible cell (limited to the visible `rows` × `cols`). +#[allow(clippy::too_many_arguments)] +fn merged_edges( + wb: &Workbook, + si: usize, + sh: &Sheet, + geo: &Geo, + m: RangeRef, + rect: Rect, + rows: (u32, u32), + cols: (u32, u32), + cf: &mut CfCache, +) -> Vec<(BorderLine, [Pos2; 2])> { + let mut out = Vec::new(); + let (cs, ce) = (m.start.col.max(cols.0), m.end.col.min(cols.1)); + for col in cs..=ce { + if sh.col_width(col) == 0.0 { + continue; + } + let (x0, x1) = (geo.x(sh, col), geo.x(sh, col.saturating_add(1))); + let top = look(wb, si, sh, CellRef::new(m.start.row, col), cf).0.style.borders.top; + let bottom = look(wb, si, sh, CellRef::new(m.end.row, col), cf).0.style.borders.bottom; + out.push((top, [pos2(x0, rect.top()), pos2(x1, rect.top())])); + out.push((bottom, [pos2(x0, rect.bottom()), pos2(x1, rect.bottom())])); + } + let (rs, re) = (m.start.row.max(rows.0), m.end.row.min(rows.1)); + for row in rs..=re { + if sh.row_height(row) == 0.0 { + continue; + } + let (y0, y1) = (geo.y(sh, row), geo.y(sh, row.saturating_add(1))); + let left = look(wb, si, sh, CellRef::new(row, m.start.col), cf).0.style.borders.left; + let right = look(wb, si, sh, CellRef::new(row, m.end.col), cf).0.style.borders.right; + out.push((left, [pos2(rect.left(), y0), pos2(rect.left(), y1)])); + out.push((right, [pos2(rect.right(), y0), pos2(rect.right(), y1)])); + } + out +} + pub(crate) fn paint_border(p: &Painter, pts: [Pos2; 2], style: BorderStyle, col: Color32) { let w = style.width(); let (a, b) = (pts[0], pts[1]); diff --git a/crates/ui-egui/tests/borders.rs b/crates/ui-egui/tests/borders.rs index af8a1a9..f248fc8 100644 --- a/crates/ui-egui/tests/borders.rs +++ b/crates/ui-egui/tests/borders.rs @@ -127,3 +127,36 @@ fn border_preview_uses_the_colored_draft_until_ok_and_cancel_discards_it() { h.state_mut().run("edit.undo", json!({})).unwrap(); assert_eq!(borders(&h, "A1"), original); } + +/// #376: a merged range's outline lives on the cells along each edge (the right edge on the last +/// column's cells, the bottom on the last row's), not on the anchor alone. All four sides are drawn. +#[test] +fn merged_range_draws_the_outline_from_its_edge_cells() { + let mut h = harness(); + h.state_mut().run("home.borders", json!({"range": "B2:E5", "preset": "thickOutside", "color": "#C81E1E"})).unwrap(); + h.state_mut().run("home.mergeCells", json!({"range": "B2:E5"})).unwrap(); + h.state_mut().run("selection.set", json!({"range": "H10"})).unwrap(); + h.run_steps(6); + assert!(borders(&h, "B2").right.is_none(), "the anchor carries only the top and left edges"); + let red = Color32::from_rgb(0xC8, 0x1E, 0x1E); + let segs: Vec<[egui::Pos2; 2]> = h + .output() + .shapes + .iter() + .filter_map(|shape| match &shape.shape { + Shape::LineSegment { points, stroke } if stroke.color == red => Some(*points), + _ => None, + }) + .collect(); + let vertical = |s: &[egui::Pos2; 2]| (s[0].x - s[1].x).abs() < 0.5; + let xs: Vec = segs.iter().filter(|s| vertical(s)).map(|s| s[0].x).collect(); + let ys: Vec = segs.iter().filter(|s| !vertical(s)).map(|s| s[0].y).collect(); + let (min_x, max_x) = xs.iter().fold((f32::MAX, f32::MIN), |(a, b), x| (a.min(*x), b.max(*x))); + let (min_y, max_y) = ys.iter().fold((f32::MAX, f32::MIN), |(a, b), y| (a.min(*y), b.max(*y))); + // Left and right edges (four rows each), top and bottom edges (four columns each). + assert_eq!(xs.iter().filter(|x| (**x - min_x).abs() < 0.5).count(), 4, "left edge"); + assert_eq!(xs.iter().filter(|x| (**x - max_x).abs() < 0.5).count(), 4, "right edge"); + assert_eq!(ys.iter().filter(|y| (**y - min_y).abs() < 0.5).count(), 4, "top edge"); + assert_eq!(ys.iter().filter(|y| (**y - max_y).abs() < 0.5).count(), 4, "bottom edge"); + assert!(max_x - min_x > 100.0 && max_y - min_y > 40.0, "the outline spans the whole merged area"); +} From 9d0053f3add9fc96e543d516f5847167fabf1ca9 Mon Sep 17 00:00:00 2001 From: nihilist-narwhal <240798405+nihilist-narwhal@users.noreply.github.com> Date: Sat, 10 Oct 2026 02:58:16 -0500 Subject: [PATCH 07/19] XLOOKUP: return a reference when the result range is one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit XLOOKUP returned only values, so it couldn't end a range or feed functions that need a reference: SUM(XLOOKUP("b",A1:A4,B1:B4): XLOOKUP("d",A1:A4,B1:B4)) and SUM(B1:XLOOKUP(…)) gave #VALUE!, and so did CELL("address",XLOOKUP(…)). A match on an empty cell gave 0, so ISBLANK(XLOOKUP(…)) was FALSE and "x"&XLOOKUP(…) gave "x0" (#374). In Excel XLOOKUP returns a reference to the matching row (or column) of its result range. The evaluator now does so, finding the position with XMATCH; arrays of lookup values, array arguments and errors still go through the library function. --- crates/calc/src/eval.rs | 52 ++++++++++++++++++++++++++++++++++++++++ crates/calc/src/tests.rs | 26 ++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/crates/calc/src/eval.rs b/crates/calc/src/eval.rs index 3d4a743..025608c 100644 --- a/crates/calc/src/eval.rs +++ b/crates/calc/src/eval.rs @@ -681,6 +681,7 @@ impl<'h> Evaluator<'h> { "OFFSET" => self.offset(args), "INDIRECT" => self.indirect(args), "INDEX" => self.index(args), + "XLOOKUP" => self.xlookup(args), "CELL" => self.cell_info(args), "TRIMRANGE" => self.trimrange(args), "SUBTOTAL" | "AGGREGATE" => self.subtotal(name, args), @@ -1137,6 +1138,57 @@ impl<'h> Evaluator<'h> { } } + /// XLOOKUP whose result range is a reference returns a reference, as in Excel, so that it can + /// end a range (`SUM(XLOOKUP(a,…):XLOOKUP(b,…))`) or feed CELL. Everything else (arrays of + /// lookup values, array arguments, errors) goes to the library function. + fn xlookup(&mut self, args: &[Expr]) -> Ev { + if (3..=6).contains(&args.len()) + && let (Ev::R(look), Ev::R(ret)) = (self.eval(&args[1]), self.eval(&args[2])) + && let ([l], [r]) = (look.as_slice(), ret.as_slice()) + && let Some(spec) = gridcraft_functions::lookup("XMATCH") + { + let key = self.value(&args[0]); + let (l, r) = (*l, *r); + let vertical = l.range.width() == 1 && l.range.height() > 1; + let horizontal = l.range.height() == 1 && l.range.width() > 1; + let same_len = if vertical { r.range.height() == l.range.height() } else { r.range.width() == l.range.width() }; + if !matches!(key, Value::Array(_)) && (vertical || horizontal) && same_len { + let mut xargs = vec![Arg { value: key, from_ref: false }, Arg { value: self.area_value(&l), from_ref: true }]; + for i in [4, 5] { + match args.get(i) { + Some(Expr::Missing) | None => break, + Some(e) => { + let value = self.value(e); + xargs.push(Arg { value, from_ref: false }); + } + } + } + let mut ctx = FnCtx { ev: self }; + return match gridcraft_functions::call(spec, &xargs, &mut ctx) { + Value::Number(p) if p >= 1.0 => { + let k = p as u32 - 1; + let range = if vertical { + RangeRef::new( + CellRef::new(r.range.start.row + k, r.range.start.col), + CellRef::new(r.range.start.row + k, r.range.end.col), + ) + } else { + RangeRef::new( + CellRef::new(r.range.start.row, r.range.start.col + k), + CellRef::new(r.range.end.row, r.range.start.col + k), + ) + }; + Ev::R(vec![Area { sheet: r.sheet, range }]) + } + Value::Error(CellError::NA) if args.get(3).is_some_and(|e| !matches!(e, Expr::Missing)) => self.eval(&args[3]), + Value::Error(e) => err(e), + _ => err(CellError::Value), + }; + } + } + self.builtin("XLOOKUP", args) + } + fn cell_info(&mut self, args: &[Expr]) -> Ev { let kind = match self.arg_val(args, 0).map(|v| v.scalar().to_text()) { Some(Ok(t)) => t.to_ascii_lowercase(), diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index 2205a1e..3b20027 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -94,6 +94,32 @@ fn chains_out_of_order() { assert_eq!(t.num("A5"), 5.0); } +#[test] +fn xlookup_returns_a_reference() { + let mut t = T::new(); + for (i, (k, v)) in [("a", 1), ("b", 2), ("c", 3), ("d", 4)].iter().enumerate() { + t.set(&format!("A{}", i + 1), k); + t.set(&format!("B{}", i + 1), &v.to_string()); + } + t.set("D1", "=SUM(XLOOKUP(\"b\",A1:A4,B1:B4):XLOOKUP(\"d\",A1:A4,B1:B4))"); + assert_eq!(t.num("D1"), 9.0); + t.set("D2", "=SUM(B1:XLOOKUP(\"c\",A1:A4,B1:B4))"); + assert_eq!(t.num("D2"), 6.0); + t.set("D3", "=CELL(\"address\",XLOOKUP(\"c\",A1:A4,B1:B4))"); + assert_eq!(t.get("D3"), Value::from("$B$3")); + // Not found: if_not_found, else #N/A, as before. + t.set("D4", "=XLOOKUP(\"z\",A1:A4,B1:B4,\"none\")"); + assert_eq!(t.get("D4"), Value::from("none")); + t.set("D5", "=XLOOKUP(\"z\",A1:A4,B1:B4)"); + assert_eq!(t.get("D5"), Value::Error(CellError::NA)); + // A match on an empty cell is a blank reference: ISBLANK is TRUE and & adds nothing (#374). + t.set("A5", "e"); + t.set("D6", "=ISBLANK(XLOOKUP(\"e\",A1:A5,B1:B5))"); + assert_eq!(t.get("D6"), Value::Bool(true)); + t.set("D7", "=\"x\"&XLOOKUP(\"e\",A1:A5,B1:B5)&\"y\""); + assert_eq!(t.get("D7"), Value::from("xy")); +} + #[test] fn long_chain() { let mut t = T::new(); From 3ae319154cb653341d30340d2d30d257380edb8f Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sun, 11 Oct 2026 21:01:12 +0200 Subject: [PATCH 08/19] Grid: render Shrink to fit (#375) Cells with shrinkToFit drew at full size: text spilled or was clipped and numbers showed ####. Too-wide content now gets a smaller font that fits the cell or merged area, matching the PDF/print path. Co-Authored-By: Claude Opus 5.5 --- ROADMAP.md | 1 + crates/ui-egui/src/grid.rs | 22 +++++++++- crates/ui-egui/tests/shrink_to_fit.rs | 61 +++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 crates/ui-egui/tests/shrink_to_fit.rs diff --git a/ROADMAP.md b/ROADMAP.md index a62418e..d642931 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-11 | Shrink to fit draws too-wide text and numbers in a smaller font inside the cell or merged area (#375) | | 2026-10-11 | Legacy Excel 97-2003 `.xls` (BIFF8) import with formulas, number formats, merges, sizes and names (#121) | | 2026-10-11 | XLSX pass-through: custom XML, external links, connections, query tables, printer settings, form/ActiveX controls and OLE objects are kept on save (#330, #324) | | 2026-10-11 | Data › Forecast › Forecast Sheet: a new sheet with the history, FORECAST.ETS forecast, confidence bounds, statistics and a chart (#322) | diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index f6dc683..2b018fe 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -934,10 +934,17 @@ pub(crate) fn paint_quadrant( continue; } let first_line = text.lines().next().unwrap_or("").to_string(); + // Shrink to fit: too-wide content is drawn in a smaller font so it fits the cell (or the + // merged area): it neither spills, nor is clipped, nor turns into ####. + let small = (st.align.shrink && !rotated && st.align.h != HAlign::Fill && fill_char.is_none()) + .then(|| shrink_font(p, &first_line, &font, avail_w - 2.0 * pad - indent)) + .flatten(); + let shrunk = small.is_some(); + let font = small.unwrap_or(font); let mut galley = p.layout_no_wrap(first_line.clone(), font.clone(), color); let mut w = galley.size().x; // Numbers and dates never overflow: they show #### when too narrow. - if (numeric || matches!(v, Value::Number(_))) && w > avail_w - 2.0 * pad && st.align.h != HAlign::Fill { + if !shrunk && (numeric || matches!(v, Value::Number(_))) && w > avail_w - 2.0 * pad && st.align.h != HAlign::Fill { let n = ((avail_w - 2.0 * pad) / char_w).floor().max(1.0) as usize; galley = p.layout_no_wrap("#".repeat(n), font.clone(), color); w = galley.size().x; @@ -1113,6 +1120,19 @@ pub fn filter_button_rect(geo: &Geo, sh: &Sheet, c: CellRef) -> Rect { Rect::from_min_size(pos2(rect.right() - s - 2.0, rect.bottom() - s - 2.0), vec2(s, s)) } +/// The smaller font that makes `text` fit in `room` points of width (Shrink to fit), or `None` +/// when it already fits at its own size. +fn shrink_font(p: &Painter, text: &str, font: &FontId, room: f32) -> Option { + let natural = p.layout_no_wrap(text.to_string(), font.clone(), Color32::PLACEHOLDER).size().x; + let room = room.max(1.0); + if natural.is_nan() || natural <= room { + return None; + } + // Glyph advances don't scale exactly linearly (rounding), so leave a little slack. + let size = (font.size * room / natural * 0.97).max(1.0); + Some(FontId::new(size, font.family.clone())) +} + pub(crate) fn paint_border(p: &Painter, pts: [Pos2; 2], style: BorderStyle, col: Color32) { let w = style.width(); let (a, b) = (pts[0], pts[1]); diff --git a/crates/ui-egui/tests/shrink_to_fit.rs b/crates/ui-egui/tests/shrink_to_fit.rs new file mode 100644 index 0000000..a9428fa --- /dev/null +++ b/crates/ui-egui/tests/shrink_to_fit.rs @@ -0,0 +1,61 @@ +//! Shrink to fit (#375): too-wide content is drawn in a smaller font inside its cell. + +use egui::Shape; +use gridcraft_engine::Session; +use gridcraft_engine::core::CellRef; +use gridcraft_ui_egui::{SheetApp, grid::Geo}; +use serde_json::json; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +fn harness() -> Harness { + let mut s = Session::new(); + s.new_workbook(); + let shrink = json!({"align": {"shrink": true}}); + s.execute("cell.set", json!({"cell": "A1", "input": "Executive Producer"})).unwrap(); + s.execute("home.formatCells", json!({"range": "A1", "style": shrink})).unwrap(); + s.execute("cell.set", json!({"cell": "A2", "input": "Control Producer"})).unwrap(); + s.execute("cell.set", json!({"cell": "A4", "input": "123456789012"})).unwrap(); + s.execute("home.formatCells", json!({"range": "A4", "style": {"num_fmt": "#,##0", "align": {"shrink": true}}})).unwrap(); + s.execute("cell.set", json!({"cell": "A5", "input": "123456789012"})).unwrap(); + s.execute("home.formatCells", json!({"range": "A5", "style": {"num_fmt": "#,##0"}})).unwrap(); + s.execute("selection.set", json!({"range": "H20"})).unwrap(); + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + app.logic(ui.ctx()); + app.ui(ui); + }, + SheetApp::new(s, Default::default()), + ); + h.run_steps(4); + h +} + +/// The painted text shape whose text is `text`: (font size, width, left x). +fn text_shape(h: &Harness, text: &str) -> Option<(f32, f32, f32)> { + h.output().shapes.iter().find_map(|s| match &s.shape { + Shape::Text(t) if t.galley.job.text == text => { + let size = t.galley.job.sections.first().map(|s| s.format.font_id.size)?; + Some((size, t.galley.size().x, t.pos.x)) + } + _ => None, + }) +} + +#[test] +fn shrink_to_fit_text_and_numbers_fit_their_cell() { + let h = harness(); + let app = h.state(); + let sheet = app.session.active().unwrap().wb.active().unwrap(); + let geo = Geo::new(sheet, app.grid.rect.unwrap(), app.view().scroll); + let a1 = geo.cell_rect(sheet, CellRef::parse("A1").unwrap()); + let (small, w, x) = text_shape(&h, "Executive Producer").expect("A1 is drawn"); + let (normal, cw, _) = text_shape(&h, "Control Producer").expect("A2 is drawn"); + assert!(small < normal, "A1 is drawn smaller ({small} vs {normal})"); + assert!(cw > a1.width(), "the control is wider than the column"); + assert!(x >= a1.left() && x + w <= a1.right(), "A1 fits inside column A"); + // A number shrinks instead of turning into ####; its unshrunk twin still shows ####. + let (num, nw, _) = text_shape(&h, "123,456,789,012").expect("A4 shows the number"); + assert!(num < normal && nw <= a1.width()); + assert!(h.output().shapes.iter().any(|s| matches!(&s.shape, Shape::Text(t) if t.galley.job.text.starts_with("##")))); +} From 806a39a5142904d7640b32c53b6ef333bd84c15a Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sun, 11 Oct 2026 21:02:20 +0200 Subject: [PATCH 09/19] Windows: compile DirectX 12 shaders with FXC, never a stray dxcompiler.dll (#339, #354) wgpu's default DX12 compiler (Auto) loads the first dxcompiler.dll on the DLL search path. GridCraft ships none, so another program's copy is used; without a matching dxil.dll its 'DXIL signing library not found' warning fails wgpu's internal compute pipelines, the device is lost and the app exits before the first frame. Pin FXC (part of Windows) unless WGPU_DX12_COMPILER asks for another compiler. Co-Authored-By: Claude Opus 5.5 --- README.md | 5 ++++ ROADMAP.md | 1 + apps/gridcraft/src/graphics.rs | 44 ++++++++++++++++++++++++++++++++-- docs/hardware-parity.md | 5 ++-- 4 files changed, 51 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 164c270..678495f 100644 --- a/README.md +++ b/README.md @@ -242,6 +242,11 @@ the first renderer fails to start, it tries the other one once. The log file rec (lines starting with `graphics:`). `WGPU_BACKEND` still overrides it (and then nothing else is tried), e.g. `$env:WGPU_BACKEND = "gl"` or `"dx12"`. +DirectX 12 shaders are compiled with FXC, which is part of Windows, so no `dxcompiler.dll` or +`dxil.dll` is needed, and a copy another program put on `PATH` can no longer stop the app at start +("Parent device is lost", #339). `$env:WGPU_DX12_COMPILER = "dxc"` (or `"auto"`) selects wgpu's DXC +handling again for troubleshooting. + ### macOS | Build | File | Notes | diff --git a/ROADMAP.md b/ROADMAP.md index a62418e..9589334 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-11 | Windows: DirectX 12 shaders compile with FXC, so a missing or stray `dxcompiler.dll` no longer stops the app at start (#339, #354) | | 2026-10-11 | Legacy Excel 97-2003 `.xls` (BIFF8) import with formulas, number formats, merges, sizes and names (#121) | | 2026-10-11 | XLSX pass-through: custom XML, external links, connections, query tables, printer settings, form/ActiveX controls and OLE objects are kept on save (#330, #324) | | 2026-10-11 | Data › Forecast › Forecast Sheet: a new sheet with the history, FORECAST.ETS forecast, confidence bounds, statistics and a chart (#322) | diff --git a/apps/gridcraft/src/graphics.rs b/apps/gridcraft/src/graphics.rs index 14344b4..83478ed 100644 --- a/apps/gridcraft/src/graphics.rs +++ b/apps/gridcraft/src/graphics.rs @@ -3,11 +3,15 @@ //! Vulkan is never initialized unless `WGPU_BACKEND` asks for it: enumerating Vulkan adapters //! crashes inside Intel's `igvk64.dll` on some UHD Graphics drivers (#120), and that access //! violation happens in the driver, where no Rust error handling can catch it. +//! +//! DirectX 12 shaders are compiled with FXC (`d3dcompiler_47.dll`, part of Windows), never with +//! whichever `dxcompiler.dll` the DLL search path happens to find (#339, #354): see +//! [`dx12_compiler`]. use eframe::{ NativeOptions, egui_wgpu::WgpuSetup, - wgpu::{Backends, DeviceType}, + wgpu::{Backends, DeviceType, Dx12Compiler}, }; /// What the start-up probe learned about one DirectX 12 adapter. @@ -59,9 +63,24 @@ pub fn fallback_backends(backend_override: Option, first: Backends) -> (!rest.is_empty()).then_some(rest) } -/// Restricts the wgpu instance eframe creates to `backends`. +/// The DirectX 12 shader compiler: `env_choice` (normally `Dx12Compiler::from_env()`, i.e. +/// `WGPU_DX12_COMPILER`, for troubleshooting) or else FXC. +/// +/// wgpu's default (`Auto`) loads the first `dxcompiler.dll` on the DLL search path. GridCraft ships +/// none, so that is another program's copy (a Windows SDK, an Edge or WebView folder, anything on +/// `PATH`). Without a matching `dxil.dll` beside it, DXC prints "DXIL signing library not found", +/// wgpu treats that warning as a compile error, its internal compute pipelines fail, the device is +/// lost and the app quits before the first frame ("Parent device is lost"). FXC is part of Windows +/// and needs no extra DLL; egui's shaders don't need anything newer. +pub fn dx12_compiler(env_choice: Option) -> Dx12Compiler { + env_choice.unwrap_or(Dx12Compiler::Fxc) +} + +/// Restricts the wgpu instance eframe creates to `backends`, and pins its DirectX 12 shader +/// compiler ([`dx12_compiler`]). pub fn configure_backends(options: &mut NativeOptions, backends: Backends) { if let WgpuSetup::CreateNew(create) = &mut options.wgpu_options.wgpu_setup { + create.instance_descriptor.backend_options.dx12.shader_compiler = dx12_compiler(Dx12Compiler::from_env()); // eframe's default includes Vulkan and GL. Creating the GL backend can crash inside AMD's // atio6axx.dll, and enumerating Vulkan adapters inside Intel's igvk64.dll (#120), before // adapter selection or any Rust error handling runs, so the window never appears. @@ -189,4 +208,25 @@ mod tests { // Both were already tried: nothing left to fall back to. assert_eq!(fallback_backends(None, Backends::DX12 | Backends::GL), None); } + + #[test] + fn dx12_compiles_shaders_with_fxc_unless_asked() { + // #339, #354: wgpu's `Auto` would load a stray dxcompiler.dll and lose the device. + assert!(matches!(dx12_compiler(None), Dx12Compiler::Fxc)); + assert!(matches!(dx12_compiler(Some(Dx12Compiler::Auto)), Dx12Compiler::Auto)); + assert!(matches!(dx12_compiler(Some(Dx12Compiler::default_dynamic_dxc())), Dx12Compiler::DynamicDxc { .. })); + // eframe's own default is `Auto`; the configured instance (first start and retry) uses FXC. + let WgpuSetup::CreateNew(eframe_default) = NativeOptions::default().wgpu_options.wgpu_setup else { + panic!("default native options must create a wgpu instance"); + }; + if Dx12Compiler::from_env().is_none() { + assert!(matches!(eframe_default.instance_descriptor.backend_options.dx12.shader_compiler, Dx12Compiler::Auto)); + let mut options = NativeOptions::default(); + configure_backends(&mut options, Backends::DX12); + let WgpuSetup::CreateNew(create) = options.wgpu_options.wgpu_setup else { + panic!("default native options must create a wgpu instance"); + }; + assert!(matches!(create.instance_descriptor.backend_options.dx12.shader_compiler, Dx12Compiler::Fxc)); + } + } } diff --git a/docs/hardware-parity.md b/docs/hardware-parity.md index f8ea38f..9d2c5e7 100644 --- a/docs/hardware-parity.md +++ b/docs/hardware-parity.md @@ -1,6 +1,6 @@ # Hardware parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** trivial (Windows renderer fallback, #120) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** trivial (Windows DirectX 12 shaders with FXC, #339) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 The hardware Excel uses, per platform, against GridCraft. Part of [target-app-parity.md](target-app-parity.md). **Hardware: ~40% ready, 25–40 h** (estimated). @@ -8,7 +8,7 @@ The hardware Excel uses, per platform, against GridCraft. Part of | Hardware feature | Excel | GridCraft (macOS · Windows · Linux · BSD · web) | Gap and estimate | |---|---|---|---| | Multi-core recalculation | Multi-threaded recalculation on all cores (Windows and Mac), configurable thread count | Single-threaded everywhere (no threading in `crates/calc`) | Parallel recalc of independent dependency chains on native builds; WASM stays single-threaded or uses workers later: 12–18 h | -| GPU rendering of the grid and charts | DirectX / Metal accelerated drawing | wgpu: Metal · DirectX 12 (default since 10-08; OpenGL when DirectX 12 has no GPU driver or fails to start; Vulkan only on request, so the Intel UHD `igvk64.dll` crash (#120) can't happen) · Vulkan/GL · Vulkan/GL · WebGPU or WebGL2 | At parity in kind. Driver problems: screen flashing on Windows 11 (#76): 2–4 h | +| GPU rendering of the grid and charts | DirectX / Metal accelerated drawing | wgpu: Metal · DirectX 12 (default since 10-08; OpenGL when DirectX 12 has no GPU driver or fails to start; Vulkan only on request, so the Intel UHD `igvk64.dll` crash (#120) can't happen; shaders compiled with FXC, so no DXC runtime is needed, #339) · Vulkan/GL · Vulkan/GL · WebGPU or WebGL2 | At parity in kind. Driver problems: screen flashing on Windows 11 (#76): 2–4 h | | Pen and touch (Draw tab) | Pen pressure and tilt, eraser end, touch mode, Ink to Shape, ink replay | Pen, highlighter, eraser, Ink to Shape with mouse/pen input; **no pressure or tilt**, no touch mode | Pressure-sensitive strokes where winit reports force; touch-friendly ribbon spacing: 4–6 h | | Trackpad and mouse | Smooth scrolling, pinch zoom, horizontal scroll, Magic Mouse | Smooth scrolling and zoom via egui | Verify pinch zoom on each platform: 1–2 h | | Multiple windows and monitors | One window per workbook, New Window for the same workbook, per-monitor DPI | One window; multiple workbooks switch inside it (#162; PR #171) | Native window per workbook, New Window, Arrange All across monitors: 6–8 h | @@ -20,5 +20,6 @@ The hardware Excel uses, per platform, against GridCraft. Part of | Date | Change | Summary | |---|---|---| +| 2026-10-11 | trivial | Windows: DirectX 12 shaders compiled with FXC, no DXC runtime needed (#339, #354) | | 2026-10-11 | trivial | Windows: OpenGL fallback when DirectX 12 has no GPU or fails to start; Vulkan never by default (#120) | | 2026-10-10 | major | Created: single-threaded calc, wgpu rendering, no pen pressure, single window | From 04001fd94394d7f83aaf318d08cc77ad78b460fa Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sun, 11 Oct 2026 21:11:23 +0200 Subject: [PATCH 10/19] View: restore and save the scroll position (sheetView topLeftCell) (#378) The model read topLeftCell but the grid always opened at A1 and never wrote the current scroll back. Sheets now open scrolled to their saved first visible cell; saves record the current one (and the active cell) without dirtying the workbook. Frozen panes use the pane's topLeftCell. Co-Authored-By: Claude Opus 5.5 --- ROADMAP.md | 1 + crates/engine/src/lib.rs | 29 ++++++++++++ crates/ui-egui/src/backstage.rs | 1 + crates/ui-egui/src/lib.rs | 46 +++++++++++++++++-- crates/ui-egui/tests/scroll_position.rs | 60 +++++++++++++++++++++++++ crates/xlsx/src/sheet_read.rs | 4 ++ crates/xlsx/src/sheet_write.rs | 6 ++- crates/xlsx/src/tests/roundtrip.rs | 25 +++++++++++ 8 files changed, 167 insertions(+), 5 deletions(-) create mode 100644 crates/ui-egui/tests/scroll_position.rs diff --git a/ROADMAP.md b/ROADMAP.md index a62418e..85fb5ae 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-11 | Workbooks reopen at their saved scroll position (`topLeftCell`, also with frozen panes) and save the current one back (#378) | | 2026-10-11 | Legacy Excel 97-2003 `.xls` (BIFF8) import with formulas, number formats, merges, sizes and names (#121) | | 2026-10-11 | XLSX pass-through: custom XML, external links, connections, query tables, printer settings, form/ActiveX controls and OLE objects are kept on save (#330, #324) | | 2026-10-11 | Data › Forecast › Forecast Sheet: a new sheet with the history, FORECAST.ETS forecast, confidence bounds, statistics and a chart (#322) | diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index c6f0fb9..8dc28d1 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -139,6 +139,35 @@ impl DocState { pub fn sheet_index(&self) -> usize { self.wb.active_sheet } + /// Records the view state that is saved with the file (`sheetView topLeftCell`, the active + /// cell): the first visible cell of the scrolling area per sheet index in `tops`, and the + /// active sheet's active cell. View state never makes the workbook dirty. + pub fn store_view(&mut self, tops: &[(usize, CellRef)]) { + let active = self.selection.active; + let si = self.wb.active_sheet; + let changed = tops.iter().any(|(i, c)| self.wb.sheets.get(*i).is_some_and(|sh| sh.view_top_left != *c)) + || self.wb.sheets.get(si).is_some_and(|sh| sh.view_active != active); + if !changed { + return; + } + let dirty = self.is_dirty(); + let wb = Arc::make_mut(&mut self.wb); + for (i, c) in tops { + if let Some(sh) = wb.sheets.get_mut(*i) + && sh.view_top_left != *c + { + Arc::make_mut(sh).view_top_left = *c; + } + } + if let Some(sh) = wb.sheets.get_mut(si) + && sh.view_active != active + { + Arc::make_mut(sh).view_active = active; + } + if !dirty { + self.saved = self.wb.clone(); + } + } pub fn display_title(&self) -> String { self.path .as_deref() diff --git a/crates/ui-egui/src/backstage.rs b/crates/ui-egui/src/backstage.rs index 32000f7..2c48b24 100644 --- a/crates/ui-egui/src/backstage.rs +++ b/crates/ui-egui/src/backstage.rs @@ -724,6 +724,7 @@ pub fn export_as(app: &mut SheetApp, fmt: &str) -> bool { } }; } + app.store_view_state(); match app.session.run("file.saveBytes", json!({"format": fmt})) { Ok(r) => { let bytes = r.get("base64").and_then(Json::as_str).and_then(gridcraft_engine::io::base64_decode); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index ea7404e..8268745 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -247,6 +247,9 @@ impl SheetApp { /// Runs an engine command (or a UI command) and handles UI requests it makes. pub fn run(&mut self, id: &str, params: Json) -> Result { + if matches!(id, "file.save" | "file.saveAs" | "file.saveBytes") { + self.store_view_state(); + } if id.starts_with("file.") || matches!( id, @@ -485,8 +488,9 @@ impl SheetApp { let path = if std::path::Path::new(&path).extension().is_none() { format!("{path}.xlsx") } else { path }; self.run_or_alert("file.save", json!({"path": path})); } - } else if let Some(dl) = &self.services.download - && let Ok(r) = self.session.run("file.saveBytes", json!({"format": "xlsx"})) + } else if self.services.download.is_some() + && let Ok(r) = self.run("file.saveBytes", json!({"format": "xlsx"})) + && let Some(dl) = &self.services.download && let Some(b) = r.get("base64").and_then(Json::as_str).and_then(gridcraft_engine::io::base64_decode) { let name = r.get("name").and_then(Json::as_str).unwrap_or("Book.xlsx").to_string(); @@ -532,12 +536,45 @@ impl SheetApp { } pub fn view(&self) -> SheetView { - self.view_key().and_then(|k| self.views.get(&k).copied()).unwrap_or_default() + self.view_key().and_then(|k| self.views.get(&k).copied()).unwrap_or_else(|| self.saved_view()) } pub fn view_mut(&mut self) -> Option<&mut SheetView> { let k = self.view_key()?; - Some(self.views.entry(k).or_default()) + let saved = self.saved_view(); + Some(self.views.entry(k).or_insert(saved)) + } + + /// The view a sheet opens with: scrolled to the file's saved first visible cell (`topLeftCell`). + fn saved_view(&self) -> SheetView { + let Some(sh) = self.session.active().and_then(|d| d.wb.active()) else { return SheetView::default() }; + if sh.view != gridcraft_engine::model::SheetViewMode::Normal { + return SheetView::default(); + } + let (fr, fc) = sh.freeze.unwrap_or((0, 0)); + let top = sh.view_top_left; + let y = sh.row_top(top.row.max(fr)) - sh.row_top(fr); + let x = sh.col_left(top.col.max(fc)) - sh.col_left(fc); + SheetView { scroll: egui::vec2(x.max(0.0) as f32, y.max(0.0) as f32) } + } + + /// Hands the active document's scroll positions (first visible cell per sheet) and active + /// cell to the engine so a save writes them (`sheetView topLeftCell`). Normal view only: + /// Page Layout view scrolls over pages, not cells. + pub fn store_view_state(&mut self) { + let Some(d) = self.session.active_mut() else { return }; + let mut tops = Vec::new(); + for (&(uid, si), v) in &self.views { + let Some(sh) = d.wb.sheets.get(si).filter(|sh| uid == d.uid && sh.view == gridcraft_engine::model::SheetViewMode::Normal) else { + continue; + }; + let (fr, fc) = sh.freeze.unwrap_or((0, 0)); + // A hair past the edge so a row that starts exactly at the scroll offset is the one found. + let row = sh.row_at(sh.row_top(fr) + v.scroll.y.max(0.0) as f64 + 0.01); + let col = sh.col_at(sh.col_left(fc) + v.scroll.x.max(0.0) as f64 + 0.01); + tops.push((si, CellRef::new(row, col))); + } + d.store_view(&tops); } /// Starts editing the active cell. `text` replaces the content (typing) or pre-fills it. @@ -769,6 +806,7 @@ impl SheetApp { && self.session.active().is_some_and(|d| d.is_dirty() && d.path.as_deref().is_some_and(|p| p.ends_with(".xlsx"))) { self.grid.last_autosave = now; + self.store_view_state(); let _ = self.session.run("file.save", json!({})); } // Window title. diff --git a/crates/ui-egui/tests/scroll_position.rs b/crates/ui-egui/tests/scroll_position.rs new file mode 100644 index 0000000..7945eb3 --- /dev/null +++ b/crates/ui-egui/tests/scroll_position.rs @@ -0,0 +1,60 @@ +//! The saved scroll position (`sheetView topLeftCell`) is restored on open and saved back (#378). + +use gridcraft_engine::core::{CellRef, Value}; +use gridcraft_engine::model::Workbook; +use gridcraft_engine::{DocState, Session}; +use gridcraft_ui_egui::{SheetApp, grid::Geo}; +use serde_json::json; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +fn harness(freeze: Option<(u32, u32)>) -> Harness { + let mut wb = Workbook::new(); + let sh = wb.sheet_mut(0).unwrap(); + for r in 0..60 { + sh.set_value(CellRef::new(r, 0), Value::text(format!("row {}", r + 1))); + } + sh.view_top_left = CellRef::parse("A31").unwrap(); + sh.view_active = CellRef::parse("A31").unwrap(); + sh.freeze = freeze; + let mut session = Session::new(); + session.add_document(DocState::new(wb, None, "Book1".into())); + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + app.logic(ui.ctx()); + app.ui(ui); + }, + SheetApp::new(session, Default::default()), + ); + h.run_steps(4); + h +} + +/// The first row painted at the top of the scrolling area. +fn first_scrolled_row(h: &Harness) -> u32 { + let app = h.state(); + let sheet = app.session.active().unwrap().wb.active().unwrap(); + let geo = Geo::new(sheet, app.grid.rect.unwrap(), app.view().scroll); + let (fr, _) = sheet.freeze.unwrap_or((0, 0)); + let top = geo.cells.top() + sheet.row_top(fr) as f32 * geo.z; + (fr..200).find(|r| geo.y(sheet, *r) <= top + 0.5 && geo.y(sheet, *r + 1) > top + 0.5).unwrap() +} + +#[test] +fn saved_scroll_position_is_restored_and_saved_back() { + for freeze in [None, Some((1, 0))] { + let mut h = harness(freeze); + assert_eq!(first_scrolled_row(&h), 30, "row 31 is the first visible row ({freeze:?})"); + // Scroll to row 51 and save: the file gets the new position, the document stays clean. + { + let app = h.state_mut(); + let sh = app.session.active().unwrap().wb.active().unwrap().clone(); + let fr = freeze.map_or(0, |f| f.0); + app.view_mut().unwrap().scroll.y = (sh.row_top(50) - sh.row_top(fr)) as f32; + app.run("file.saveBytes", json!({"format": "xlsx"})).unwrap(); + } + let d = h.state().session.active().unwrap(); + assert_eq!(d.wb.active().unwrap().view_top_left, CellRef::parse("A51").unwrap()); + assert!(!d.is_dirty(), "scrolling and saving view state does not dirty the workbook"); + } +} diff --git a/crates/xlsx/src/sheet_read.rs b/crates/xlsx/src/sheet_read.rs index 07ee340..8eac285 100644 --- a/crates/xlsx/src/sheet_read.rs +++ b/crates/xlsx/src/sheet_read.rs @@ -256,6 +256,10 @@ fn read_views(e: &El, sheet: &mut Sheet) { let rows = p.attr_f64("ySplit").unwrap_or(0.0).clamp(0.0, (MAX_ROWS - 1) as f64) as u32; if rows > 0 || cols > 0 { sheet.freeze = Some((rows, cols)); + // With frozen panes the scroll position is the scrolling pane's first cell. + if let Some(c) = p.attr("topLeftCell").and_then(CellRef::parse) { + sheet.view_top_left = c; + } } } active_pane = p.attr("activePane"); diff --git a/crates/xlsx/src/sheet_write.rs b/crates/xlsx/src/sheet_write.rs index fee804c..94f1663 100644 --- a/crates/xlsx/src/sheet_write.rs +++ b/crates/xlsx/src/sheet_write.rs @@ -182,7 +182,11 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> ( let _ = write!( s, " topLeftCell=\"{}\" activePane=\"{pane}\" state=\"frozen\"/>", - CellRef::new(rows.min(gridcraft_core::MAX_ROWS - 1), cols.min(gridcraft_core::MAX_COLS - 1)).a1() + CellRef::new( + rows.max(sheet.view_top_left.row).min(gridcraft_core::MAX_ROWS - 1), + cols.max(sheet.view_top_left.col).min(gridcraft_core::MAX_COLS - 1) + ) + .a1() ); let _ = write!(s, ""); } diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs index 2385b8e..a4f7b18 100644 --- a/crates/xlsx/src/tests/roundtrip.rs +++ b/crates/xlsx/src/tests/roundtrip.rs @@ -763,3 +763,28 @@ fn bad_comment_dates_are_left_out() { let c = &back.sheets[0].comments[&at("A1")]; assert!(c.threaded && c.dates.is_empty()); } + +/// #378: the saved scroll position survives a round trip, with and without frozen panes (where it +/// is the scrolling pane's `topLeftCell`). +#[test] +fn scroll_position_round_trips_with_and_without_frozen_panes() { + let mut wb = Workbook::new(); + wb.sheet_mut(0).unwrap().view_top_left = at("A31"); + let (back, _) = read_xlsx(&write_xlsx(&wb).unwrap()).unwrap(); + assert_eq!(back.sheet(0).unwrap().view_top_left, at("A31")); + + let s = wb.sheet_mut(0).unwrap(); + s.freeze = Some((1, 0)); + s.view_top_left = at("C40"); + let bytes = write_xlsx(&wb).unwrap(); + let (xml, _) = sheet1_xml(&bytes); + assert!(xml.contains(r#" Date: Sun, 11 Oct 2026 21:12:24 +0200 Subject: [PATCH 11/19] Review: Protect Workbook dialog with an Excel-compatible password (#345) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ribbon button toggled structure protection silently. It now opens a Protect Structure and Windows dialog with an optional password; when a password was set, unprotecting asks for it and really verifies it (SHA-512, spin count 100000, salted). The password hash is read from and written to XLSX workbookProtection (workbookAlgorithmName/HashValue/ SaltValue/SpinCount or the legacy workbookPassword); an unreadable one stays locked. File › Info uses the same flow. Co-Authored-By: Claude Opus 5.5 --- ROADMAP.md | 1 + crates/engine/src/cmd/review.rs | 65 ++++++++++++++++++++++++++---- crates/model/src/workbook.rs | 4 ++ crates/ui-egui/src/backstage.rs | 16 +++++++- crates/ui-egui/src/dialogs.rs | 17 ++++++++ crates/ui-egui/src/ribbon.rs | 2 +- crates/xlsx/src/read.rs | 1 + crates/xlsx/src/sheet_read.rs | 44 +++++++++++++------- crates/xlsx/src/sheet_write.rs | 35 +++++++++------- crates/xlsx/src/tests/fixtures.rs | 5 ++- crates/xlsx/src/tests/roundtrip.rs | 3 ++ crates/xlsx/src/write.rs | 3 +- 12 files changed, 156 insertions(+), 40 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index a62418e..b927fc4 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-11 | Review › Protect Workbook opens a dialog with an optional password (Excel-compatible hash, kept in XLSX `workbookProtection`); unprotecting verifies it (#345) | | 2026-10-11 | Legacy Excel 97-2003 `.xls` (BIFF8) import with formulas, number formats, merges, sizes and names (#121) | | 2026-10-11 | XLSX pass-through: custom XML, external links, connections, query tables, printer settings, form/ActiveX controls and OLE objects are kept on save (#330, #324) | | 2026-10-11 | Data › Forecast › Forecast Sheet: a new sheet with the history, FORECAST.ETS forecast, confidence bounds, statistics and a chart (#322) | diff --git a/crates/engine/src/cmd/review.rs b/crates/engine/src/cmd/review.rs index 70823cd..8931bf5 100644 --- a/crates/engine/src/cmd/review.rs +++ b/crates/engine/src/cmd/review.rs @@ -19,7 +19,15 @@ pub fn specs() -> Vec { protect_sheet ), cmd!("review.unprotectSheet", "Unprotect Sheet", ["Review", "Protect"], None, "{password?}", has_doc, unprotect_sheet), - cmd!("review.protectWorkbook", "Protect Workbook", ["Review", "Protect"], None, "{on?: bool}", has_doc, protect_workbook), + cmd!( + "review.protectWorkbook", + "Protect Workbook", + ["Review", "Protect"], + None, + "{on?: bool (default: toggle), password?} (structure protection; unprotecting needs the password when one was set)", + has_doc, + protect_workbook + ), cmd!(query "review.workbookStatistics", "Workbook Statistics", ["Review", "Proofing"], None, "{}", has_doc, stats), // Not journaled: the params can hold a password. CommandSpec { @@ -159,6 +167,15 @@ fn unprotect_sheet(s: &mut Session, p: &Json) -> Result { }) } +/// A new Excel-compatible (SHA-512, salted) hash of `pw`. The salt mixes `what` is protected +/// with the document, a counter and the clock, so equal passwords get different hashes. +fn new_password_hash(d: &DocState, what: &str, pw: &str) -> PasswordHash { + static SALT_COUNTER: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + let n = SALT_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let seed = format!("{what}\u{0}{n}\u{0}{}\u{0}{}", d.uid, gridcraft_calc::now_serial().to_bits()); + PasswordHash::new_sha512(pw, &gridcraft_model::password::salt_from(seed.as_bytes())) +} + fn range_text(r: &EditRange) -> String { r.ranges.iter().map(|x| x.a1()).collect::>().join(",") } @@ -166,7 +183,6 @@ fn range_text(r: &EditRange) -> String { /// Review › Allow Edit Ranges: list, add, modify or delete the ranges that stay editable on a /// protected sheet, or unlock a password range for this session. fn allow_edit_ranges(s: &mut Session, p: &Json) -> Result { - static SALT_COUNTER: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); let sheet = target_sheet(s, p)?; let action = str_param(p, "action").unwrap_or("list"); let title = str_param(p, "title").map(str::trim).filter(|t| !t.is_empty()); @@ -180,11 +196,7 @@ fn allow_edit_ranges(s: &mut Session, p: &Json) -> Result { .position(|r| r.title.eq_ignore_ascii_case(t)) .ok_or_else(|| bad("review.allowEditRanges", format!("no range titled `{t}`"))) }; - let hash = |pw: &str, title: &str| { - let n = SALT_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed); - let seed = format!("{sheet_name}\u{0}{title}\u{0}{n}\u{0}{}\u{0}{}", d.uid, gridcraft_calc::now_serial().to_bits()); - PasswordHash::new_sha512(pw, &gridcraft_model::password::salt_from(seed.as_bytes())) - }; + let hash = |pw: &str, title: &str| new_password_hash(d, &format!("{sheet_name}\u{0}{title}"), pw); if matches!(action, "add" | "modify" | "delete") && sh.is_protected() { return Err(EngineError::Other("Unprotect the sheet to change its edit ranges.".into())); } @@ -270,11 +282,28 @@ fn allow_edit_ranges(s: &mut Session, p: &Json) -> Result { )) } +/// Review › Protect Workbook: lock the workbook structure (sheets can't be added, moved, +/// renamed, hidden or deleted), optionally with a password that unprotecting must match. fn protect_workbook(s: &mut Session, p: &Json) -> Result { - let cur = s.doc()?.wb.protected_structure; + let d = s.doc()?; + let cur = d.wb.protected_structure; let on = bool_param(p, "on").unwrap_or(!cur); + if on == cur { + return Ok(json!({"on": on})); + } + let password = if on { + str_param(p, "password").filter(|pw| !pw.is_empty()).map(|pw| new_password_hash(d, "workbook", pw)) + } else { + if let Some(h) = &d.wb.structure_password + && !h.verify(str_param(p, "password").unwrap_or("")).map_err(EngineError::Other)? + { + return Err(EngineError::Other("The password you supplied is not correct.".into())); + } + None + }; edit(s, |cx| { cx.wb.protected_structure = on; + cx.wb.structure_password = password.clone(); Ok(json!({"on": on})) }) } @@ -355,6 +384,26 @@ mod tests { assert_eq!(at(&s), (0, "A1".to_string())); } + #[test] + fn protect_workbook_with_a_password_really_verifies() { + let mut s = book(); + s.execute("review.protectWorkbook", json!({"password": "pa55"})).unwrap(); + assert!(s.doc().unwrap().wb.protected_structure); + assert!(s.execute("home.insertSheet", json!({"name": "Three"})).is_err(), "the structure is locked"); + // A second Protect keeps it protected; unprotecting needs the right password. + assert_eq!(s.execute("review.protectWorkbook", json!({"on": true})).unwrap()["on"], true); + assert!(s.execute("review.protectWorkbook", json!({})).is_err()); + assert!(s.execute("review.protectWorkbook", json!({"on": false, "password": "pass"})).is_err()); + assert!(s.doc().unwrap().wb.protected_structure); + s.execute("review.protectWorkbook", json!({"on": false, "password": "pa55"})).unwrap(); + let wb = &s.doc().unwrap().wb; + assert!(!wb.protected_structure && wb.structure_password.is_none()); + // Without a password it toggles freely. + s.execute("review.protectWorkbook", json!({})).unwrap(); + s.execute("review.protectWorkbook", json!({})).unwrap(); + assert!(!s.doc().unwrap().wb.protected_structure); + } + #[test] fn convert_to_comments_turns_every_note_into_a_comment_and_undoes() { let mut s = book(); diff --git a/crates/model/src/workbook.rs b/crates/model/src/workbook.rs index 7e03cb3..6ea5f99 100644 --- a/crates/model/src/workbook.rs +++ b/crates/model/src/workbook.rs @@ -67,6 +67,9 @@ pub struct Workbook { pub props: DocProps, /// Workbook structure protection. pub protected_structure: bool, + /// Password of the structure protection (XLSX `workbookProtection`), never the password. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub structure_password: Option, /// Custom fill lists (Options › Custom Lists) beyond the built-in ones. pub custom_lists: Vec>, /// Named cell styles (Cell Styles gallery): name → style. @@ -95,6 +98,7 @@ impl Workbook { active_sheet: 0, props: DocProps::default(), protected_structure: false, + structure_password: None, custom_lists: vec![], cell_styles: vec![], preserved: None, diff --git a/crates/ui-egui/src/backstage.rs b/crates/ui-egui/src/backstage.rs index 32000f7..07e5705 100644 --- a/crates/ui-egui/src/backstage.rs +++ b/crates/ui-egui/src/backstage.rs @@ -422,6 +422,20 @@ fn open_page(app: &mut SheetApp, ui: &mut Ui, out: &mut Outcome) { recent_list(app, ui, out, usize::MAX); } +/// Review › Protect Workbook and File › Info › Protect Workbook: the protect dialog (with an +/// optional password) when the structure is open; when it's locked, the password prompt if one +/// was set, else unprotect right away. +pub fn protect_workbook(app: &mut SheetApp) { + let (on, password) = app.session.active().map(|d| (d.wb.protected_structure, d.wb.structure_password.is_some())).unwrap_or((false, false)); + if !on { + app.open_dialog("protectWorkbook", json!({})); + } else if password { + app.open_dialog("unprotectWorkbook", json!({})); + } else { + app.run_or_alert("review.protectWorkbook", json!({"on": false})); + } +} + fn info(app: &mut SheetApp, ui: &mut Ui, bs: &mut Backstage) { let t = Tokens::get(ui.ctx()); let lang = app.ui.language; @@ -451,7 +465,7 @@ fn info(app: &mut SheetApp, ui: &mut Ui, bs: &mut Backstage) { ui.add_space(18.0); ui.horizontal(|ui| { if square_button(ui, Icon::Lock, lang.tr("Protect Workbook")).clicked() { - app.run_or_alert("review.protectWorkbook", json!({"on": !protected})); + protect_workbook(app); } ui.add_space(12.0); ui.vertical(|ui| { diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 196d47a..0291ebb 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -219,6 +219,23 @@ impl Dialog { json!({"action": "add", "range": sel}), ) } + "protectWorkbook" => Dialog::form( + "protectWorkbook", + "Protect Structure and Windows", + "review.protectWorkbook", + vec![ + Password { key: "password", label: "Password (optional):" }, + Note("Protect workbook for: Structure (sheets can't be added, moved, renamed, hidden or deleted).".into()), + ], + json!({"on": true}), + ), + "unprotectWorkbook" => Dialog::form( + "unprotectWorkbook", + "Unprotect Workbook", + "review.protectWorkbook", + vec![Password { key: "password", label: "Password:" }], + json!({"on": false}), + ), "unprotectSheet" => Dialog::form( "unprotectSheet", "Unprotect Sheet", diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index ccc51eb..af602b2 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -1714,7 +1714,7 @@ fn review(app: &mut SheetApp, ui: &mut Ui) { } } if big_button(ui, Icon::Book, "Protect\nWorkbook", "Protect Workbook structure", false).clicked() { - act(app, "review.protectWorkbook", json!({})); + crate::backstage::protect_workbook(app); } if big_button(ui, Icon::Validation, "Allow Edit\nRanges", "Allow Edit Ranges", false).clicked() { app.open_dialog("allowEditRanges", json!({})); diff --git a/crates/xlsx/src/read.rs b/crates/xlsx/src/read.rs index 7c00d01..d1da00a 100644 --- a/crates/xlsx/src/read.rs +++ b/crates/xlsx/src/read.rs @@ -185,6 +185,7 @@ pub fn read_xlsx(bytes: &[u8]) -> Result<(Workbook, ReadReport), IoError> { } if let Some(p) = wb_xml.child("workbookProtection") { wb.protected_structure = p.flag("lockStructure", false); + wb.structure_password = if wb.protected_structure { crate::sheet_read::read_password(p, "workbook") } else { None }; } let active_tab = wb_xml.path(&["bookViews", "workbookView"]).and_then(|v| v.attr_u32("activeTab")).unwrap_or(0) as usize; diff --git a/crates/xlsx/src/sheet_read.rs b/crates/xlsx/src/sheet_read.rs index 07ee340..2c33d1f 100644 --- a/crates/xlsx/src/sheet_read.rs +++ b/crates/xlsx/src/sheet_read.rs @@ -208,6 +208,35 @@ pub fn read_sheet(cx: &mut Ctx<'_>, part: &str, name: &str) -> Result Option { + let attr = |base: &str| e.attr(&password_attr(prefix, base)); + if attr("algorithmName").is_some() || attr("hashValue").is_some() { + return Some(PasswordHash::Iterated { + algorithm: attr("algorithmName").unwrap_or("").to_string(), + hash: attr("hashValue").unwrap_or("").to_string(), + salt: attr("saltValue").unwrap_or("").to_string(), + spin_count: e.attr_u32(&password_attr(prefix, "spinCount")).unwrap_or(0), + }); + } + attr("password").map(|h| match u16::from_str_radix(h.trim(), 16) { + Ok(h) => PasswordHash::Legacy(h), + Err(_) => PasswordHash::Iterated { algorithm: String::new(), hash: String::new(), salt: String::new(), spin_count: 0 }, + }) +} + +/// `base` preceded by `prefix` in camel case: ("workbook", "hashValue") → `workbookHashValue`. +pub(crate) fn password_attr(prefix: &str, base: &str) -> String { + let mut chars = base.chars(); + match chars.next() { + Some(c) if !prefix.is_empty() => format!("{prefix}{}{}", c.to_ascii_uppercase(), chars.as_str()), + _ => base.to_string(), + } +} + /// Allow Edit Ranges. A range whose password can't be read keeps a hash that never matches /// (or an unsupported algorithm), so it stays locked rather than open. fn read_protected_ranges(e: &El, sheet: &mut Sheet) { @@ -216,20 +245,7 @@ fn read_protected_ranges(e: &El, sheet: &mut Sheet) { if ranges.is_empty() { continue; } - let password = if p.attr("algorithmName").is_some() || p.attr("hashValue").is_some() { - Some(PasswordHash::Iterated { - algorithm: p.attr("algorithmName").unwrap_or("").to_string(), - hash: p.attr("hashValue").unwrap_or("").to_string(), - salt: p.attr("saltValue").unwrap_or("").to_string(), - spin_count: p.attr_u32("spinCount").unwrap_or(0), - }) - } else { - p.attr("password").map(|h| match u16::from_str_radix(h.trim(), 16) { - Ok(h) => PasswordHash::Legacy(h), - // Unreadable: keep it locked. - Err(_) => PasswordHash::Iterated { algorithm: String::new(), hash: String::new(), salt: String::new(), spin_count: 0 }, - }) - }; + let password = read_password(p, ""); let title = p.attr("name").unwrap_or("").to_string(); sheet.edit_ranges.push(EditRange { title, ranges, password, security_descriptor: p.attr("securityDescriptor").map(str::to_string) }); } diff --git a/crates/xlsx/src/sheet_write.rs b/crates/xlsx/src/sheet_write.rs index fee804c..acc98f6 100644 --- a/crates/xlsx/src/sheet_write.rs +++ b/crates/xlsx/src/sheet_write.rs @@ -1118,26 +1118,33 @@ fn threaded_xml(sheet: &Sheet, ids: &BTreeMap>, persons: &m s } +/// A protection password hash as attributes, each name preceded by `prefix` (see +/// [`crate::sheet_read::read_password`]). +pub(crate) fn password_attrs(h: &gridcraft_model::PasswordHash, prefix: &str) -> String { + let name = |base: &str| crate::sheet_read::password_attr(prefix, base); + match h { + gridcraft_model::PasswordHash::Legacy(h) => format!(" {}=\"{h:04X}\"", name("password")), + gridcraft_model::PasswordHash::Iterated { algorithm, hash, salt, spin_count } => format!( + " {}=\"{}\" {}=\"{}\" {}=\"{}\" {}=\"{spin_count}\"", + name("algorithmName"), + esc_attr(algorithm), + name("hashValue"), + esc_attr(hash), + name("saltValue"), + esc_attr(salt), + name("spinCount") + ), + } +} + /// Allow Edit Ranges: `` (after `sheetProtection`). fn protected_ranges_xml(sheet: &Sheet) -> String { let mut s = String::new(); for r in sheet.edit_ranges.iter().filter(|r| !r.ranges.is_empty()) { let sqref: Vec = r.ranges.iter().map(|x| x.a1()).collect(); let _ = write!(s, " { - let _ = write!(s, " password=\"{h:04X}\""); - } - Some(gridcraft_model::PasswordHash::Iterated { algorithm, hash, salt, spin_count }) => { - let _ = write!( - s, - " algorithmName=\"{}\" hashValue=\"{}\" saltValue=\"{}\" spinCount=\"{spin_count}\"", - esc_attr(algorithm), - esc_attr(hash), - esc_attr(salt) - ); - } - None => {} + if let Some(h) = &r.password { + s.push_str(&password_attrs(h, "")); } if let Some(sd) = &r.security_descriptor { let _ = write!(s, " securityDescriptor=\"{}\"", esc_attr(sd)); diff --git a/crates/xlsx/src/tests/fixtures.rs b/crates/xlsx/src/tests/fixtures.rs index 5533e01..42adcb5 100644 --- a/crates/xlsx/src/tests/fixtures.rs +++ b/crates/xlsx/src/tests/fixtures.rs @@ -48,7 +48,7 @@ fn shared_formulas_and_value_types() { #[test] fn workbook_settings_names_and_states() { - let wb_extra = r#""#; + let wb_extra = r#""#; // Defined names must come after ; build the workbook by hand. let wbx = format!( r#"{wb_extra} @@ -69,6 +69,9 @@ fn workbook_settings_names_and_states() { assert_eq!(wb.sheets.len(), 3); assert_eq!(wb.date_system, DateSystem::D1904); assert!(wb.protected_structure); + // The legacy structure password ("password" hashes to 83AF) is kept and really verifies. + let pw = wb.structure_password.as_ref().unwrap(); + assert!(pw.verify("password").unwrap() && !pw.verify("Password").unwrap()); assert_eq!(wb.calc.mode, CalcMode::Manual); assert!(wb.calc.iterative); assert_eq!(wb.calc.max_iterations, 50); diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs index 2385b8e..4038908 100644 --- a/crates/xlsx/src/tests/roundtrip.rs +++ b/crates/xlsx/src/tests/roundtrip.rs @@ -22,6 +22,7 @@ fn sample() -> Workbook { wb.date_system = DateSystem::D1904; wb.calc = CalcSettings { mode: CalcMode::Manual, iterative: true, max_iterations: 42, max_change: 0.5, precision_as_displayed: true }; wb.protected_structure = true; + wb.structure_password = Some(gridcraft_model::PasswordHash::new_sha512("s3cret", b"0123456789abcdef")); wb.props = DocProps { title: "T & ".into(), subject: "S".into(), @@ -412,6 +413,8 @@ fn full_roundtrip() { assert_eq!(back.date_system, wb.date_system); assert_eq!(back.calc, wb.calc); assert!(back.protected_structure); + assert_eq!(back.structure_password, wb.structure_password); + assert!(back.structure_password.as_ref().unwrap().verify("s3cret").unwrap()); assert_eq!(back.props, wb.props); assert_eq!(back.theme, wb.theme); assert_eq!(back.names, wb.names); diff --git a/crates/xlsx/src/write.rs b/crates/xlsx/src/write.rs index dc2ddc4..6aae60e 100644 --- a/crates/xlsx/src/write.rs +++ b/crates/xlsx/src/write.rs @@ -268,7 +268,8 @@ pub fn write_xlsx(wb: &Workbook) -> Result, IoError> { let mut w = format!("{XML_DECL}"); w.push_str(if wb.date_system == gridcraft_core::DateSystem::D1904 { "" } else { "" }); if wb.protected_structure { - w.push_str(""); + let pw = wb.structure_password.as_ref().map(|h| crate::sheet_write::password_attrs(h, "workbook")).unwrap_or_default(); + let _ = write!(w, ""); } let _ = write!(w, "{sheets_xml}"); w.push_str(&crate::passthrough::write_workbook(wb, &mut wb_rels, &mut out)); From 1b04d06014179ed4a5286d88c70a88bb889c85a2 Mon Sep 17 00:00:00 2001 From: debarghaya <175645893+GhostMitra@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:51:23 +0530 Subject: [PATCH 12/19] Web & UI: resolve system theme preference and default web to system theme --- apps/gridcraft-web/src/web.rs | 1 + contributors/people.toml | 4 ++++ crates/ui-egui/src/theme.rs | 15 +++++++++++++++ crates/ui-egui/src/widgets.rs | 28 ++++++++++++++++++++++++++++ 4 files changed, 48 insertions(+) diff --git a/apps/gridcraft-web/src/web.rs b/apps/gridcraft-web/src/web.rs index f5744bd..30df447 100644 --- a/apps/gridcraft-web/src/web.rs +++ b/apps/gridcraft-web/src/web.rs @@ -38,6 +38,7 @@ pub fn start() { let _ = session.execute("file.new", serde_json::json!({"sample": "sales"})); } let mut app = SheetApp::new(session, services(inbox.clone(), cc.egui_ctx.clone())); + app.ui.system_theme = true; // Follow the browser's language, except a CJK one: the web build has no system // CJK fonts (none are bundled), so that interface would render as tofu. let browser = gridcraft_ui_egui::i18n::Language::system(); diff --git a/contributors/people.toml b/contributors/people.toml index 734bf81..6d4b23e 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -45,3 +45,7 @@ sync_github_name = false [people.Losminek] sync_github_name = true + +[people.GhostMitra] +sync_github_name = true + diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index 6812c51..5259de8 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -125,6 +125,21 @@ impl Tokens { } } +/// Resolves an [`egui::ThemePreference`] against the context's system theme. +pub fn resolve_theme(pref: egui::ThemePreference, ctx: &egui::Context) -> egui::Theme { + match pref { + egui::ThemePreference::System => { + if ctx.input(|i| i.raw.system_theme == Some(egui::Theme::Dark)) { + egui::Theme::Dark + } else { + egui::Theme::Light + } + } + egui::ThemePreference::Light => egui::Theme::Light, + egui::ThemePreference::Dark => egui::Theme::Dark, + } +} + pub const UI: &str = "ui"; pub const UI_BOLD: &str = "ui-bold"; pub const CELL: &str = "cell"; diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 7369892..9c82b98 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -236,3 +236,31 @@ pub fn toast(app: &mut SheetApp, ctx: &egui::Context) { ); ctx.request_repaint_after(std::time::Duration::from_millis(200)); } + +/// Turn a monochrome image (black on white) into an alpha mask tinted with the active text color. +pub fn raster_to_alpha_mask(pixels: &mut [egui::Color32]) { + for p in pixels.iter_mut() { + // Darkness of source becomes the opacity + let darkness = 255u8.saturating_sub(p.r()); + // Pure white glyph with variable alpha + *p = egui::Color32::from_rgba_unmultiplied(255, 255, 255, darkness); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_raster_to_alpha_mask() { + let mut pixels = vec![ + egui::Color32::WHITE, // r=255 -> darkness=0 -> alpha=0 + egui::Color32::BLACK, // r=0 -> darkness=255 -> alpha=255 + egui::Color32::from_rgb(100, 100, 100), // r=100 -> darkness=155 -> alpha=155 + ]; + raster_to_alpha_mask(&mut pixels); + assert_eq!(pixels[0], egui::Color32::from_rgba_unmultiplied(255, 255, 255, 0)); + assert_eq!(pixels[1], egui::Color32::from_rgba_unmultiplied(255, 255, 255, 255)); + assert_eq!(pixels[2], egui::Color32::from_rgba_unmultiplied(255, 255, 255, 155)); + } +} From cedfc5e44c01888342ba12249a04bc11b857b7e5 Mon Sep 17 00:00:00 2001 From: debarghaya <175645893+GhostMitra@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:54:49 +0530 Subject: [PATCH 13/19] Web: add favicon to web app index and disable dev incremental compilation --- Cargo.toml | 2 ++ apps/gridcraft-web/index.html | 1 + contributors/people.toml | 4 ++++ 3 files changed, 7 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index eab2853..0c00014 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,6 +64,8 @@ opt-level = 2 [profile.dev] opt-level = 1 +incremental = false + [profile.release] opt-level = 3 diff --git a/apps/gridcraft-web/index.html b/apps/gridcraft-web/index.html index 96d6347..5fdd1ea 100644 --- a/apps/gridcraft-web/index.html +++ b/apps/gridcraft-web/index.html @@ -4,6 +4,7 @@ GridCraft +