Compare commits

..
Author SHA1 Message Date
Dylan M. Taylor 89cf961a2f Conditional formatting: one scan budget per rule, and no thresholds for unsupported colour scales
numbers_in() scanned each of a rule's ranges separately with its own
2,000,000-cell limit, so a rule listing the same (or overlapping)
large range many times multiplied the scan while painting a single
cell. Colour scales also evaluated every stop's threshold, formulas
included, before discarding scales that are not 2- or 3-colour.

- A rule's statistics now share STATS_SCAN_LIMIT across all its ranges,
  and an exact repeat of a range is scanned once.
- A colour scale with a stop count other than 2 or 3 paints nothing
  and evaluates none of its thresholds.
2026-11-02 20:56:49 -05:00
2 changed files with 69 additions and 42 deletions
+65 -4
View File
@@ -1,7 +1,7 @@
//! Conditional formatting evaluation: what extra style, data bar, colour scale or icon a cell
//! gets.
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use gridcraft_core::{CellRef, RangeRef, Value, compare};
use gridcraft_model::{CfOperator, CfRule, CfValueKind, Color, CondFormat, PatternType, Sheet, Style, Underline, Workbook, default_icon_thresholds};
@@ -70,15 +70,31 @@ thread_local! {
static SCANNED: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
}
/// Most cells one rule's statistics visit, across all of its ranges together: a rule repeating
/// the same range many times must not multiply the scan.
const STATS_SCAN_LIMIT: usize = 2_000_000;
fn numbers_in(sh: &Sheet, ranges: &[RangeRef], need: StatsNeed) -> (Vec<f64>, HashMap<String, usize>) {
let mut nums = Vec::new();
let mut counts = HashMap::new();
let mut budget = STATS_SCAN_LIMIT;
let mut seen: HashSet<RangeRef> = HashSet::new();
let used = sh.used_range();
for r in ranges {
let r = match sh.used_range().and_then(|u| r.intersection(&u)) {
if budget == 0 {
break;
}
let r = match used.and_then(|u| r.intersection(&u)) {
Some(x) => x,
None => continue,
};
for c in r.iter().take(2_000_000) {
// The same range listed twice counts its cells once.
if !seen.insert(r) {
continue;
}
let cells = r.iter().take(budget);
for c in cells {
budget -= 1;
#[cfg(test)]
SCANNED.with(|n| n.set(n.get() + 1));
let v = sh.value(c);
@@ -278,7 +294,8 @@ impl CfCache {
_ => None,
},
CfRule::ColorScale { stops } => {
if let Some(n) = v.as_f64() {
// Only two- and three-colour scales paint: evaluate no thresholds for others.
if let Some(n) = v.as_f64().filter(|_| matches!(stops.len(), 2 | 3)) {
let pts: Vec<(f64, [u8; 3])> = stops
.iter()
.map(|(k, col)| (threshold(wb, sheet, c, st, k), col.resolve(&wb.theme).unwrap_or([255, 255, 255])))
@@ -560,6 +577,50 @@ mod tests {
assert_eq!(SCANNED.with(|n| n.get()), 10_000);
}
/// A rule listing the same range many times scans it once, and never more than the shared
/// limit however many ranges it holds.
#[test]
fn repeated_ranges_share_one_scan_budget() {
let mut s = crate::Session::new();
s.new_workbook();
s.execute("range.setValues", json!({"range": "A1", "values": [[1], [2], [3]]})).unwrap();
s.execute("range.setValues", json!({"range": "XFD1048576", "values": [[4]]})).unwrap();
let d = s.doc().unwrap();
let sh = d.wb.sheet(0).unwrap();
let dense = RangeRef::parse("A1:XFD1048576").unwrap();
let small = RangeRef::parse("A1:A3").unwrap();
SCANNED.with(|n| n.set(0));
let st = RuleStats::build(sh, &vec![small; 1000], StatsNeed::Numbers);
assert_eq!(st.sorted, vec![1.0, 2.0, 3.0]);
assert_eq!(SCANNED.with(|n| n.get()), 3);
let shifted: Vec<RangeRef> = (0..50).map(|i| RangeRef::new(CellRef::new(i, 0), dense.end)).collect();
SCANNED.with(|n| n.set(0));
RuleStats::build(sh, &shifted, StatsNeed::Numbers);
assert!(SCANNED.with(|n| n.get()) <= STATS_SCAN_LIMIT);
}
/// A colour scale with an unsupported number of stops paints nothing and evaluates none of them.
#[test]
fn colour_scales_with_other_stop_counts_paint_nothing() {
let mut s = crate::Session::new();
s.new_workbook();
s.execute("range.setValues", json!({"range": "A1", "values": [[1], [5]]})).unwrap();
let mut wb: Workbook = (*s.doc().unwrap().wb).clone();
let stop = (CfValueKind::Formula("=1".into()), Color::default());
let rule = |n: usize| CondFormat {
ranges: vec![RangeRef::parse("A1:A2").unwrap()],
rule: CfRule::ColorScale { stops: vec![stop.clone(); n] },
priority: 1,
stop_if_true: false,
};
wb.sheet_mut(0).unwrap().cond_formats.push(rule(4));
let mut cache = CfCache::new();
assert_eq!(cache.look(&wb, 0, CellRef::parse("A1").unwrap()).and_then(|l| l.scale_fill), None);
wb.sheet_mut(0).unwrap().cond_formats = vec![rule(2)];
let mut cache = CfCache::new();
assert!(cache.look(&wb, 0, CellRef::parse("A1").unwrap()).and_then(|l| l.scale_fill).is_some());
}
#[test]
fn cell_value_operator_names() {
let mut s = crate::Session::new();
+4 -38
View File
@@ -272,25 +272,11 @@ struct Grid {
cells: BTreeMap<(u32, u32), Value>,
/// Cells shown as percentages.
pct: Vec<(u32, u32)>,
/// Rows and columns spanned by `cells`.
rows: u32,
cols: u32,
/// A cell was dropped because the report outgrew [`MAX_OUTPUT_CELLS`]: the report is refused.
too_large: bool,
}
impl Grid {
/// Stores a value. Past [`MAX_OUTPUT_CELLS`] (the most a report writes) nothing more is kept,
/// so a builder that would produce millions of entries stops allocating them, and
/// [`write_report`] refuses the report.
fn put(&mut self, r: u32, c: u32, v: Value) {
if !v.is_empty() {
let (rows, cols) = (self.rows.max(r.saturating_add(1)), self.cols.max(c.saturating_add(1)));
if self.too_large || u64::from(rows) * u64::from(cols) > MAX_OUTPUT_CELLS {
self.too_large = true;
return;
}
(self.rows, self.cols) = (rows, cols);
self.cells.insert((r, c), v);
}
}
@@ -310,7 +296,9 @@ impl Grid {
}
/// (rows, cols) spanned.
fn extent(&self) -> (u32, u32) {
(self.rows, self.cols)
let rows = self.cells.keys().map(|(r, _)| r + 1).max().unwrap_or(0);
let cols = self.cells.keys().map(|(_, c)| c + 1).max().unwrap_or(0);
(rows, cols)
}
}
@@ -1761,7 +1749,7 @@ fn sampling(s: &Session, p: &Json, seed: Option<u64>) -> Result<Grid> {
fn write_report(s: &mut Session, p: &Json, tool: &str, g: Grid) -> Result<Json> {
let (rows, cols) = g.extent();
let (rows, cols) = (rows.max(1), cols.max(1));
if g.too_large || u64::from(rows) * u64::from(cols) > MAX_OUTPUT_CELLS {
if u64::from(rows) * u64::from(cols) > MAX_OUTPUT_CELLS {
return Err(bad(CMD, "the report is too large"));
}
let d = s.doc()?;
@@ -1926,28 +1914,6 @@ mod tests {
x
}
#[test]
fn a_report_stops_growing_past_the_output_limit() {
// Entries past the limit are not kept, and the report is refused.
let mut g = super::Grid::default();
g.num(1999, 1999, 1.0);
assert!(!g.too_large);
g.num(0, 2000, 1.0);
assert!(g.too_large);
assert_eq!(g.cells.len(), 1);
assert_eq!(g.extent(), (2000, 2000));
// Row-grouped statistics of a tall, almost empty column: one variable per row would make
// millions of report entries; the report is refused instead of built.
let mut s = s();
s.execute("range.setValues", json!({"range": "A1", "values": [[1]]})).unwrap();
s.execute("range.setValues", json!({"range": "A1000000", "values": [[2]]})).unwrap();
let err = s
.execute("data.analysisToolPak", json!({"tool": "descriptive", "range": "A1:A1000000", "groupedBy": "rows", "output": "C1"}))
.expect_err("report too large");
assert!(err.to_string().contains("too large"), "{err}");
assert_eq!(v(&s, "C1"), Value::Empty);
}
#[test]
fn forecast_standard_errors_match_the_full_window_rescan() {
// #1043: the running sum must give what re-adding every window gave, gaps included.