Compare commits

..
Author SHA1 Message Date
Dylan M. Taylor 4b0ab92e9d Merge branch 'main' into fix/sec-mode-frequency-linear 2026-11-03 17:34:27 -05:00
Dylan M. Taylor 55551913c8 address review: FREQUENCY keeps NaN data in the overflow bin and NaN bins empty 2026-11-03 21:27:00 +00:00
Dylan M. Taylor a305229287 PDF export: wrap text with running widths (#3208)
wrap_lines split an over-long word by re-measuring and copying the
rest of the word for every piece it cut, and only checked its line
limit once the word was done, so one long word in a narrow text box
cost word length x pieces. Each new word also re-measured the whole
line built so far.

Line and piece widths are now running sums of glyph widths (the same
sum text_width makes), so every character is measured once, and the
line limit is checked while a word is being split. The lines produced
are unchanged; a test compares them with the old wrapper.
2026-11-03 16:11:47 -05:00
Dylan M. Taylor 19c6f70e2a functions: MODE counts through an index, FREQUENCY bins by binary search
MODE/MODE.SNGL/MODE.MULT searched the list of distinct numbers seen so far
for every input, and FREQUENCY scanned the sorted bins for every data value,
so both were quadratic in computed arrays. MODE now keys counts by value
(-0 folded into 0, NaN never merged) and keeps first-occurrence order;
FREQUENCY finds the first bin at or above each value with partition_point
over the order it already sorts. Results are unchanged.
2026-11-02 22:18:10 -05:00
14 changed files with 197 additions and 126 deletions
+2 -9
View File
@@ -147,10 +147,10 @@ impl RangeRef {
RangeRef::new(CellRef::new(0, 0), CellRef::new(MAX_ROWS - 1, MAX_COLS - 1))
}
pub fn height(&self) -> u32 {
self.end.row.saturating_sub(self.start.row).saturating_add(1)
self.end.row - self.start.row + 1
}
pub fn width(&self) -> u32 {
self.end.col.saturating_sub(self.start.col).saturating_add(1)
self.end.col - self.start.col + 1
}
pub fn count(&self) -> u64 {
self.height() as u64 * self.width() as u64
@@ -300,13 +300,6 @@ pub fn split_sheet(reference: &str) -> (Option<String>, &str) {
mod tests {
use super::*;
#[test]
fn hostile_range_dimensions_saturate_instead_of_wrapping() {
let r = RangeRef { start: CellRef::new(0, 0), end: CellRef::new(u32::MAX, u32::MAX) };
assert_eq!((r.height(), r.width()), (u32::MAX, u32::MAX));
assert_eq!(r.count(), u64::from(u32::MAX) * u64::from(u32::MAX));
}
#[test]
fn range_parse_rejects_signed_rows() -> Result<(), std::io::Error> {
assert!(RangeRef::parse("+1:+3").is_none());
+1 -2
View File
@@ -184,8 +184,7 @@ pub(crate) fn grow_autofilter(sh: &mut Sheet, at: CellRef) {
pub(crate) fn apply_filter(wb: &mut Workbook, sheet: usize) {
let Some(sh) = wb.sheet(sheet) else { return };
let Some(af) = sh.autofilter.clone() else { return };
// An imported range can name rows past the grid; none of those exist to hide.
let r = RangeRef::new(af.range.start, CellRef::new(af.range.end.row.min(gridcraft_core::MAX_ROWS - 1), af.range.end.col));
let r = af.range;
let mut hide = Vec::new();
// Precompute per-criterion top10 threshold and average once, not per row.
let mut col_stats: std::collections::HashMap<usize, (Option<f64>, f64)> = std::collections::HashMap::new();
-7
View File
@@ -5,9 +5,6 @@ use super::{
// ---------------------------------------------------------------- tools
/// Most cells (rows times compared columns) one Remove Duplicates call reads.
const MAX_COMPARED_CELLS: u64 = 50_000_000;
pub(super) fn remove_duplicates(s: &mut Session, p: &Json) -> Result<Json> {
let (sheet, r) = data_target(s, p, "data.removeDuplicates")?;
let d = s.doc()?;
@@ -27,10 +24,6 @@ pub(super) fn remove_duplicates(s: &mut Session, p: &Json) -> Result<Json> {
if cols.is_empty() || cols.iter().any(|c| !(r.start.col..=r.end.col).contains(c)) {
return Err(bad("data.removeDuplicates", "`columns` must name at least one column of the range"));
}
// Rows are compared one by one, so a sparse sheet's enclosing rectangle is bounded like a list.
if r.count() > MAX_COMPARED_CELLS {
return Err(bad("data.removeDuplicates", "the range is too large"));
}
let start = r.start.row + header as u32;
let mut removed = 0;
edit(s, |cx| {
+1 -4
View File
@@ -289,14 +289,11 @@ pub(super) fn circle_invalid(s: &mut Session, _: &Json) -> Result<Json> {
let sheet = d.wb.active_sheet();
let Some(sh) = d.wb.sheet(sheet) else { return Err(EngineError::NoDocument) };
let mut bad_cells = Vec::new();
// One scan allowance for the whole command: duplicate or overlapping ranges don't multiply it.
let mut budget = 100_000usize;
for dv in &sh.validations {
for r in &dv.ranges {
let r = sh.used_range().and_then(|u| r.intersection(&u));
let Some(r) = r else { continue };
for c in r.iter().take(budget) {
budget -= 1;
for c in r.iter().take(100_000) {
let text = sh.input_text(c);
if !text.is_empty() && check_validation(&d.wb, sheet, c, &text).is_some() {
bad_cells.push(c.a1());
+1 -5
View File
@@ -130,11 +130,7 @@ pub(super) fn replace(s: &mut Session, p: &Json) -> Result<Json> {
let mut n = 0;
for si in sheets {
let cells: Vec<(CellRef, String)> =
// A hidden formula's source is not searched: a count or error would reveal its text.
cx.wb
.sheet(si)
.map(|sh| sh.cells.iter().filter(|(c, _)| !crate::display::formula_hidden(&cx.wb, sh, *c)).map(|(c, _)| (c, sh.input_text(c))).collect())
.unwrap_or_default();
cx.wb.sheet(si).map(|sh| sh.cells.iter().map(|(c, _)| (c, sh.input_text(c))).collect()).unwrap_or_default();
for (c, text) in cells {
if (!all && c != active) || selection.as_ref().is_some_and(|sel| !sel.contains(c)) {
continue;
+33 -31
View File
@@ -140,44 +140,46 @@ pub(super) fn autofit_cols(s: &mut Session, p: &Json) -> Result<Json> {
let measure = s.text_width.clone();
edit(s, |cx| {
let Some(sh) = cx.wb.sheet(si) else { return Ok(Json::Null) };
let mut widths = Vec::new();
let mut measured: std::collections::HashMap<StyleId, std::collections::HashMap<String, f32>> = Default::default();
let mut distinct = 0usize;
// Selected columns, then one pass over the populated cells (the store is row-oriented, so a
// scan per column would revisit every row for each selected column, empty or not).
let cols: std::collections::BTreeSet<u32> = lines.iter().flat_map(|&(a, b)| a..=b.min(a + 16383)).collect();
let mut maxes: std::collections::BTreeMap<u32, f32> = Default::default();
if let (Some(u), Some(&first), Some(&last)) = (sh.used_range(), cols.first(), cols.last()) {
let span = RangeRef::new(CellRef::new(u.start.row, first), CellRef::new(u.end.row, last));
for (cell, _) in sh.cells.iter_range(span) {
if !cols.contains(&cell.col) || sh.merge_at(cell).is_some_and(|m| !m.is_single()) {
continue;
}
let id = sh.style_id(cell);
let st = cx.wb.styles.get(id);
let text = crate::display::cell_text(&cx.wb, sh, cell);
for line in text.lines() {
let w = match &measure {
Some(m) => {
let known = measured.entry(id).or_default();
match known.get(line) {
Some(w) => *w,
None => {
let w = m(line, &st.font, distinct < EXACT_TEXT_WIDTHS);
known.insert(line.to_owned(), w);
distinct += 1;
w
let used = sh.used_range();
for (a, b) in lines {
for c in a..=b.min(a + 16383) {
let Some(u) = used else { continue };
let col = RangeRef::new(CellRef::new(u.start.row, c), CellRef::new(u.end.row, c));
let mut max: f32 = 0.0;
for (cell, _) in sh.cells.iter_range(col) {
if sh.merge_at(cell).is_some_and(|m| !m.is_single()) {
continue;
}
let id = sh.style_id(cell);
let st = cx.wb.styles.get(id);
let text = crate::display::cell_text(&cx.wb, sh, cell);
for line in text.lines() {
let w = match &measure {
Some(m) => {
let known = measured.entry(id).or_default();
match known.get(line) {
Some(w) => *w,
None => {
let w = m(line, &st.font, distinct < EXACT_TEXT_WIDTHS);
known.insert(line.to_owned(), w);
distinct += 1;
w
}
}
}
}
None => approx_text_width(line, st.font.size, st.font.bold),
};
let w = w + 6.0 + st.align.indent as f32 * 9.0 + sh.filter_button_space(cell);
let max = maxes.entry(cell.col).or_insert(0.0);
*max = max.max(w);
None => approx_text_width(line, st.font.size, st.font.bold),
};
max = max.max(w + 6.0 + st.align.indent as f32 * 9.0 + sh.filter_button_space(cell));
}
}
if max > 0.0 {
widths.push((c, max.clamp(8.0, 1785.0)));
}
}
}
let widths: Vec<(u32, f32)> = maxes.into_iter().filter(|&(_, m)| m > 0.0).map(|(c, m)| (c, m.clamp(8.0, 1785.0))).collect();
let sh = cx.sheet_mut(si)?;
for (c, w) in widths {
let e = sh.cols.entry(c).or_default();
+73
View File
@@ -695,6 +695,79 @@ fn bands_unit() {
assert!(w.len() > 2);
}
/// The wrapper `wrap_lines` replaced, re-measuring strings, as the reference for its lines.
fn wrap_lines_remeasuring(text: &str, font: Font, size: f32, width: f32) -> Vec<String> {
let mut out = Vec::new();
for para in text.split('\n') {
let mut line = String::new();
for word in para.split(' ') {
let cand = if line.is_empty() { word.to_string() } else { format!("{line} {word}") };
if line.is_empty() || gridcraft_pdf::text_width(font, size, &cand) <= width {
line = cand;
} else {
out.push(std::mem::take(&mut line));
line = word.to_string();
}
while gridcraft_pdf::text_width(font, size, &line) > width && line.chars().count() > 1 {
let chars: Vec<char> = line.chars().collect();
let (mut lo, mut hi) = (1usize, chars.len());
while lo < hi {
let mid = lo + (hi - lo).div_ceil(2);
let cut: String = chars[..mid].iter().collect();
if gridcraft_pdf::text_width(font, size, &cut) <= width {
lo = mid;
} else {
hi = mid - 1;
}
}
if lo == chars.len() {
break;
}
out.push(chars[..lo].iter().collect());
line = chars[lo..].iter().collect();
}
if out.len() > 2000 {
return out;
}
}
out.push(line);
}
out
}
#[test]
fn wrap_lines_matches_the_remeasuring_wrapper() {
let texts = [
"the quick brown fox jumps",
"Supercalifragilistic",
"a b c\n\nverylongwordthatneedsbreaking short\nend ",
"Wide WWWWWWWWWW words, iiiiiiii narrow ones; mixed MMMMiiiiMMMM",
"€ ü ß — unicode Ünïcödé text",
"",
" ",
];
for text in texts {
for font in [Font::Helvetica, Font::HelveticaBold, Font::Courier] {
for (size, width) in [(10.0, 50.0), (10.0, 20.0), (11.0, 1.0), (8.0, 300.0), (12.0, 0.0)] {
assert_eq!(wrap_lines(text, font, size, width), wrap_lines_remeasuring(text, font, size, width), "{text:?} {font:?} {size} {width}");
}
}
}
}
#[test]
fn wrapping_a_long_word_in_a_narrow_box_stops_at_the_line_limit() {
// One character per line: the old wrapper re-measured and copied the rest of the word for
// each of the 100,000 lines before checking the limit.
let word = "w".repeat(100_000);
let lines = wrap_lines(&word, Font::Helvetica, 10.0, 1.0);
assert!(lines.len() <= 2002, "{}", lines.len());
assert!(lines.iter().all(|l| l == "w"));
// A long paragraph in a very wide box is one line.
let para = vec!["word"; 50_000].join(" ");
assert_eq!(wrap_lines(&para, Font::Helvetica, 10.0, 1.0e9), vec![para.clone()]);
}
#[test]
fn center_across_spans_following_empty_center_across_cells() {
let mut styles = StyleTable::default();
+39 -24
View File
@@ -3,42 +3,57 @@ use super::{CellRef, Color, Font, HAlign, MAX_COLS, Page, Point, RangeRef, Sheet
const PAD_PX: f32 = 2.0;
/// Most lines [`wrap_lines`] returns (it may return one more, the rest of a paragraph).
const MAX_WRAP_LINES: usize = 2000;
/// Word-wraps `text` to `width` points.
///
/// Widths are kept as running sums of glyph widths (the sum `text_width` makes), so each
/// character is measured once: re-measuring the line for every word, or the rest of an
/// over-long word for every piece cut from it, made a long paragraph or word quadratic.
pub(super) fn wrap_lines(text: &str, font: Font, size: f32, width: f32) -> Vec<String> {
let units_of = |s: &str| -> u64 { s.chars().map(|c| u64::from(font.char_width(c))).sum() };
let fits = |units: u64| units as f32 * size / 1000.0 <= width;
let space = units_of(" ");
let mut out = Vec::new();
for para in text.split('\n') {
let mut line = String::new();
let mut line_units: u64 = 0;
for word in para.split(' ') {
let cand = if line.is_empty() { word.to_string() } else { format!("{line} {word}") };
if line.is_empty() || gridcraft_pdf::text_width(font, size, &cand) <= width {
line = cand;
let word_units = units_of(word);
if line.is_empty() {
line.push_str(word);
line_units = word_units;
} else if fits(line_units + space + word_units) {
line.push(' ');
line.push_str(word);
line_units += space + word_units;
} else {
out.push(std::mem::take(&mut line));
line = word.to_string();
line.push_str(word);
line_units = word_units;
}
// Break a single over-long word by characters. The cut that fits is found by binary
// search (each try measures one candidate prefix), so an over-long word costs
// O(n log n) measurements instead of re-measuring the whole cut per character for
// every emitted line (`a` × 100k pasted text would be cubic worst case).
while gridcraft_pdf::text_width(font, size, &line) > width && line.chars().count() > 1 {
let chars: Vec<char> = line.chars().collect();
let (mut lo, mut hi) = (1usize, chars.len());
while lo < hi {
let mid = lo + (hi - lo).div_ceil(2);
let cut: String = chars[..mid].iter().collect();
if gridcraft_pdf::text_width(font, size, &cut) <= width {
lo = mid;
} else {
hi = mid - 1;
// Break a single over-long word by characters: each piece is the longest that fits
// (at least one character).
if !fits(line_units) && line.chars().nth(1).is_some() {
let mut piece = String::new();
let mut units: u64 = 0;
for c in line.chars() {
let w = u64::from(font.char_width(c));
if !piece.is_empty() && !fits(units + w) {
out.push(std::mem::take(&mut piece));
units = 0;
if out.len() > MAX_WRAP_LINES {
return out;
}
}
piece.push(c);
units += w;
}
if lo == chars.len() {
break;
}
out.push(chars[..lo].iter().collect());
line = chars[lo..].iter().collect();
line = piece;
line_units = units;
}
if out.len() > 2000 {
if out.len() > MAX_WRAP_LINES {
return out;
}
}
@@ -83,9 +83,6 @@ pub(crate) fn criterion_matches(v: &Value, text: &str, crit: &Value) -> bool {
}
}
/// Most (list row, criteria row) pairs one Advanced Filter tests.
const MAX_FILTER_TESTS: u64 = 50_000_000;
pub(super) fn advanced_filter(s: &mut Session, p: &Json) -> Result<Json> {
const CMD: &str = "data.advancedFilter";
let d = s.doc()?;
@@ -136,10 +133,6 @@ pub(super) fn advanced_filter(s: &mut Session, p: &Json) -> Result<Json> {
rows.push(conds);
}
}
// Each list row is tried against every criteria row, so bound their product too.
if u64::from(list.height()) * rows.len() as u64 > MAX_FILTER_TESTS {
return Err(bad(CMD, "the list and criteria ranges together are too large"));
}
let matches_row = |row: u32| -> bool {
if rows.is_empty() {
return true;
-9
View File
@@ -297,12 +297,3 @@ fn consolidate_by_position_and_label() {
assert!(s.execute("data.consolidate", json!({"sources": []})).is_err());
assert!(s.execute("data.consolidate", json!({"sources": ["A1"], "function": "median"})).is_err());
}
#[test]
fn advanced_filter_refuses_a_list_times_criteria_product_over_the_work_limit() {
let mut s = s();
s.execute("range.setValues", json!({"range": "A1", "values": [["Name"], ["A"]]})).unwrap();
s.execute("range.setValues", json!({"range": "H1", "values": [["Name"], ["A"]]})).unwrap();
// A sparse million-row list against 9,999 blank criteria rows would be ~10 billion tests.
assert!(s.execute("data.advancedFilter", json!({"range": "A2:A1048576", "criteria": "H1:H10000"})).is_err());
}
+3 -12
View File
@@ -1280,18 +1280,9 @@ fn content_differs(b: Option<&Cell>, a: Option<&Cell>) -> bool {
/// Objects added, changed and removed between two versions of a sheet.
fn objects(b: &Sheet, a: &Sheet) -> (usize, usize, usize) {
fn count<T: PartialEq>(b: &[T], a: &[T], id: impl Fn(&T) -> u32) -> (usize, usize, usize) {
// Objects grouped by id, so each lookup is a map probe instead of a scan of the list.
fn group<'a, T>(v: &'a [T], id: &impl Fn(&T) -> u32) -> std::collections::HashMap<u32, Vec<&'a T>> {
let mut m: std::collections::HashMap<u32, Vec<&T>> = std::collections::HashMap::new();
for x in v {
m.entry(id(x)).or_default().push(x);
}
m
}
let (gb, ga) = (group(b, &id), group(a, &id));
let added = a.iter().filter(|x| !gb.contains_key(&id(x))).count();
let removed = b.iter().filter(|x| !ga.contains_key(&id(x))).count();
let changed = b.iter().filter(|x| ga.get(&id(x)).is_some_and(|ys| ys.iter().any(|y| *y != *x))).count();
let added = a.iter().filter(|x| !b.iter().any(|y| id(y) == id(x))).count();
let removed = b.iter().filter(|x| !a.iter().any(|y| id(y) == id(x))).count();
let changed = b.iter().filter(|x| a.iter().any(|y| id(y) == id(x) && y != *x)).count();
(added, changed, removed)
}
let parts = [count(&b.charts, &a.charts, |o| o.id), count(&b.images, &a.images, |o| o.id), count(&b.shapes, &a.shapes, |o| o.id)];
-11
View File
@@ -5572,14 +5572,3 @@ fn clearing_links_and_notes_keeps_allow_edit_ranges() {
let sh = s.doc().unwrap().wb.active().unwrap().clone();
assert!(sh.hyperlinks.is_empty() && sh.comments.is_empty());
}
#[test]
fn replace_does_not_search_hidden_formula_source() {
let mut s = s();
s.execute("cell.set", json!({"cell": "A1", "input": "=SEQUENCE(3)*7"})).unwrap();
s.execute("home.formatCells", json!({"range": "A1", "style": {"protection": {"locked": true, "hidden": true}}})).unwrap();
s.execute("review.protectSheet", json!({})).unwrap();
let hit = s.execute("edit.replace", json!({"what": "SEQUENCE", "with": "X"})).map_err(|e| e.to_string());
let miss = s.execute("edit.replace", json!({"what": "NOSUCHTEXT", "with": "X"})).map_err(|e| e.to_string());
assert_eq!(hit, miss);
}
+24 -5
View File
@@ -50,12 +50,27 @@ pub(super) fn median(v: Vec<f64>) -> R<f64> {
/// Values occurring at least twice, ordered by first occurrence, with their (equal, maximal) count.
fn modes(v: &[f64]) -> Vec<f64> {
// Index of each distinct number in `counts`, so counting is linear rather than a search of
// every distinct number so far. Keyed by bits with -0 folded into 0 (they compare equal); NaN
// never equals anything, so it is never merged.
let mut index: std::collections::HashMap<u64, usize> = std::collections::HashMap::new();
let mut counts: Vec<(f64, usize)> = Vec::new();
for &x in v {
if let Some(e) = counts.iter_mut().find(|e| e.0 == x) {
e.1 += 1;
} else {
if x.is_nan() {
counts.push((x, 1));
continue;
}
let key = if x == 0.0 { 0 } else { x.to_bits() };
match index.entry(key) {
std::collections::hash_map::Entry::Occupied(e) => {
if let Some(c) = counts.get_mut(*e.get()) {
c.1 += 1;
}
}
std::collections::hash_map::Entry::Vacant(e) => {
e.insert(counts.len());
counts.push((x, 1));
}
}
}
let best = counts.iter().map(|e| e.1).max().unwrap_or(0);
@@ -451,11 +466,15 @@ pub(super) fn frequency(a: &[Arg], _: C) -> R<Value> {
if bins.is_empty() {
return col_value(vec![data.len() as f64]);
}
let mut order: Vec<usize> = (0..bins.len()).collect();
// a NaN bin equals no value (and would break the ascending order searched below)
let mut order: Vec<usize> = (0..bins.len()).filter(|&i| bins.get(i).is_some_and(|b| !b.is_nan())).collect();
order.sort_by(|&i, &j| bins.get(i).copied().unwrap_or(0.0).total_cmp(&bins.get(j).copied().unwrap_or(0.0)));
let mut counts = vec![0.0; bins.len() + 1];
for x in data {
let slot = order.iter().find(|&&i| x <= bins.get(i).copied().unwrap_or(f64::INFINITY)).copied().unwrap_or(bins.len());
// `order` sorts the bins ascending, so the bins `x` exceeds come first: binary search for
// the first bin at or above `x` (the first of equal bins, as a linear scan would find).
let pos = if x.is_nan() { order.len() } else { order.partition_point(|&i| bins.get(i).copied().unwrap_or(f64::INFINITY) < x) };
let slot = order.get(pos).copied().unwrap_or(bins.len());
if let Some(c) = counts.get_mut(slot) {
*c += 1.0;
}
+20
View File
@@ -617,3 +617,23 @@ fn aggregate_helper() {
close(aggregate_values_k(9, &vals, &nv(2.0)), 9.0);
is_err(aggregate_values_k(14, &vals, &nv(10.0)), CellError::Num);
}
/// Security: FREQUENCY finds each value's bin by binary search, and MODE counts distinct numbers
/// through an index, so neither is quadratic in its inputs. Results are as before: unsorted and
/// duplicate bins keep their positions (a value goes to the first of equal bins), and tied modes
/// come in first-occurrence order.
#[test]
fn frequency_and_mode_scale_with_their_inputs() {
let f = ev("FREQUENCY", vec![c(&[1.0, 2.0, 2.0, 3.0, 5.0, 9.0, -1.0]), c(&[3.0, 2.0, 2.0, 0.0])]);
assert_eq!(rows_of(&f), vec![vec![nv(1.0)], vec![nv(3.0)], vec![nv(0.0)], vec![nv(1.0)], vec![nv(2.0)]]);
// NaN data goes to the overflow bin, and a NaN bin takes nothing
let f = ev("FREQUENCY", vec![c(&[f64::NAN, 0.5, 2.0]), c(&[f64::NAN, 1.0])]);
assert_eq!(rows_of(&f), vec![vec![nv(0.0)], vec![nv(1.0)], vec![nv(2.0)]]);
let data: Vec<f64> = (0..200_000).map(|i| f64::from(i) + 1e6).collect();
let bins: Vec<f64> = (0..200_000).map(f64::from).collect();
let f = ev("FREQUENCY", vec![c(&data), c(&bins)]);
assert_eq!(rows_of(&f).last(), Some(&vec![nv(200_000.0)]));
let distinct = ev("MODE", vec![c(&data)]);
is_err(distinct, CellError::NA);
assert_eq!(rows_of(&ev("MODE.MULT", vec![c(&[3.0, 0.0, 1.0, -0.0, 3.0, 1.0])])), vec![vec![nv(3.0)], vec![nv(0.0)], vec![nv(1.0)]]);
}