mirror of
https://github.com/storytold/gridcraft.git
synced 2026-11-01 06:36:47 -06:00
Follow-up to #210, which the reviewer correctly rejected: the stated cause (a sparse column with a stray distant value) doesn't hold, since the scan already stops at the first empty cell. This is a different, narrower fix that doesn't change behaviour for any input — it never truncates a long fill, unlike #210's cap — only how fast a genuinely long, densely-populated run is scanned. grid.rs's fill-handle double-click called Sheet::value_ref once per row to find where the neighbouring column's data ends. Each call does a band lookup + row lookup + column lookup (three BTreeMap probes) via CellStore::get. For a real dense run (e.g. a filled column near the 1,048,576-row sheet limit) that's a lot of point lookups on the UI thread in one frame: ~59ms for a 1,000,000-row run in a release build (measured with the new fill_handle_perf example), likely worse under wasm where GridCraft actually runs in the browser. - crates/model/src/store.rs: CellStore::contiguous_value_run walks each band's row BTreeMap with `range` (one in-order pass per band) instead of one `get` per row, so a dense run costs roughly one tree-traversal step per row instead of one binary search per row. Deliberately narrower than `has`/`get`: a stored cell always decides the row on its own (even with an empty value), matching what `value_ref`'s callers actually test for, not `has`'s "formula counts regardless of its cached value" (used for Ctrl+arrow navigation). - crates/model/src/sheet.rs: Sheet::last_contiguous_nonempty layers `spill` back in wherever contiguous_value_run finds no stored cell, exactly where plain `value_ref` would fall through to it. - crates/ui-egui/src/grid.rs: the fill-handle double-click now calls this once instead of looping over value_ref. Result: 999,999-row dense run, 59.2ms -> 24.7ms (2.40x), same answer (asserted in the example). Four new tests in sheet.rs, including a 300-trial property test against a naive value_ref-loop reference (spills, formula-with-empty-value, gaps, blanks) — all pass, plus the full existing suite (`cargo test -p gridcraft-model -p gridcraft-ui-egui -p gridcraft-engine`; the only failures are #210-era gridcraft-engine CSV tests already failing on main, unrelated to this change). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>