fix: reserve the group rows monotonically, and bulk the ancestry compare
Copilot's fifth pass; both correct. **Inline group rows are back in `frameCost`.** Omitting them was the same failure I had just fixed for the header block: they take real table height, so any viewport containing one asked for more data rows than `#grid` could show, and the DOM takes its own scroll position while the client's cursor offsets come from the larger window. Copilot's own suggestion answers the oscillation objection — the reservation is a **high-water mark**, which cannot oscillate because it only rises, and it rises in the safe direction: over-reserving leaves blank space, under-reserving is the bug. It resets on a view change so one dense document does not make every later view pessimistic. **`same_prefix` is replaced by `AxisFrame::shares_ancestry`**, the bulk form, for the same reason `header_labels` and `groups` are bulk: called per (column, level) it scanned the frame twice apiece, quadratic in the columns drawn and growing with the horizontal offset, where the pre-anr9 widget's materialized lookup was O(1). `same_prefix` is **deleted** rather than kept beside it — two answers to one question is the thing this PR exists to remove, and keeping a per-position form that nothing calls would be an invitation. `shared_ancestry_widens_as_ the_level_rises` checks both arms, because a version comparing whole tuples passes the leaf arm and a version comparing only the outermost label passes the outer one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -301,12 +301,6 @@ impl AxisFrame {
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
/// The label tuple of `idx` down to and including `level`, clamped to
|
||||
/// the tuple's own length so a short tuple cannot panic.
|
||||
fn prefix(&self, idx: usize, level: usize) -> Option<&[String]> {
|
||||
self.item(idx).map(|v| &v[..(level + 1).min(v.len())])
|
||||
}
|
||||
|
||||
/// Should the header at `(idx, level)` be drawn, or blanked because it
|
||||
/// repeats the entry above it?
|
||||
///
|
||||
@@ -348,6 +342,31 @@ impl AxisFrame {
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// For each data item, whether it shares `with`'s ancestry down to each
|
||||
/// level: indexed `[data index][level]`.
|
||||
///
|
||||
/// It is the TUI's column-header underline: at the leaf level only the
|
||||
/// selected column matches, and at a higher level every column under the
|
||||
/// same ancestor does. There is no per-position form beside it, because
|
||||
/// one would be a second answer paid for per (column, level) — two frame
|
||||
/// scans apiece, quadratic in the columns drawn and growing with the
|
||||
/// horizontal offset.
|
||||
pub fn shares_ancestry(&self, with: usize) -> Vec<Vec<bool>> {
|
||||
let items: Vec<&Vec<String>> = self.items().collect();
|
||||
let levels = self.levels();
|
||||
let prefix = |i: usize, level: usize| items.get(i).map(|v| &v[..(level + 1).min(v.len())]);
|
||||
(0..items.len())
|
||||
.map(|idx| {
|
||||
(0..levels)
|
||||
.map(|level| match (prefix(idx, level), prefix(with, level)) {
|
||||
(Some(a), Some(b)) => a == b,
|
||||
_ => false,
|
||||
})
|
||||
.collect()
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// The group each data item sits under, in order — [`AxisFrame::group_of`]
|
||||
/// for every position, in one pass rather than a scan apiece.
|
||||
pub fn groups(&self) -> Vec<Option<&FrameGroup>> {
|
||||
@@ -362,15 +381,6 @@ impl AxisFrame {
|
||||
out
|
||||
}
|
||||
|
||||
/// Whether two data items sit under the same ancestor down to `level` —
|
||||
/// what the TUI underlines the selected column's ancestry with.
|
||||
pub fn same_prefix(&self, a: usize, b: usize, level: usize) -> bool {
|
||||
match (self.prefix(a, level), self.prefix(b, level)) {
|
||||
(Some(x), Some(y)) => x == y,
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// The nearest [`FrameEntry::Group`] before the data item at `idx`, or
|
||||
/// `None` when no header precedes it at all.
|
||||
///
|
||||
@@ -1240,6 +1250,26 @@ mod tests {
|
||||
assert!(group.collapsed, "the collapse state travels with the group");
|
||||
}
|
||||
|
||||
/// The underline rule: at the leaf level only the selected column, at a
|
||||
/// higher level every column under the same ancestor. Both levels are
|
||||
/// checked because a version that compared whole tuples would pass the
|
||||
/// leaf arm and light only one column at the outer level, and a version
|
||||
/// that compared only the outermost label would pass the outer arm and
|
||||
/// light a sibling at the leaf.
|
||||
#[test]
|
||||
fn shared_ancestry_widens_as_the_level_rises() {
|
||||
let frame = two_level_frame();
|
||||
let under_ay = frame.shares_ancestry(1); // [A,y]
|
||||
assert_eq!(
|
||||
under_ay,
|
||||
vec![
|
||||
vec![true, false], // [A,x] shares "A", not "A/x"
|
||||
vec![true, true], // itself
|
||||
vec![false, false], // [B,z] shares nothing
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
/// `body_from` starts at a data item, never at the header above it: a
|
||||
/// window opening mid-group must be able to *name* its group without
|
||||
/// painting that group's header as a row inside the window.
|
||||
|
||||
+9
-1
@@ -111,6 +111,10 @@ impl<'a> GridWidget<'a> {
|
||||
// a deep scroll offset, grows with the offset.
|
||||
let row_labels = rows.header_labels(row_offset);
|
||||
let col_groups = cols.groups();
|
||||
// The selected column's ancestry, per level, for every column — the
|
||||
// underline below. Asking `same_prefix` per (column, level) scans
|
||||
// the frame twice each time.
|
||||
let col_ancestry = cols.shares_ancestry(sel_col);
|
||||
// Transposed to level-major, because the column header is drawn one
|
||||
// line per level. `header_labels` answers per column.
|
||||
let col_lines: Vec<Vec<String>> = {
|
||||
@@ -222,7 +226,11 @@ impl<'a> GridWidget<'a> {
|
||||
// Underline columns that share the same ancestor group as
|
||||
// sel_col through level d. At the bottom level this matches
|
||||
// only sel_col; at higher levels it spans all sub-columns.
|
||||
let in_sel_group = cols.same_prefix(ci, sel_col, d);
|
||||
let in_sel_group = col_ancestry
|
||||
.get(ci)
|
||||
.and_then(|levels| levels.get(d))
|
||||
.copied()
|
||||
.unwrap_or(false);
|
||||
let styled = if in_sel_group {
|
||||
header_style.add_modifier(Modifier::UNDERLINED)
|
||||
} else {
|
||||
|
||||
+25
-12
@@ -116,8 +116,9 @@ function setShellStatus(text) {
|
||||
//
|
||||
// It is measured off the painted model rather than predicted, and defaults to
|
||||
// the old one-and-one before the first paint, which is the only moment there
|
||||
// is nothing to measure.
|
||||
let frameCost = { rows: 1, cols: 1 };
|
||||
// is nothing to measure. `groups` is kept as its own high-water mark — see
|
||||
// the tail of `repaint`.
|
||||
let frameCost = { view: null, rows: 1, cols: 1, groups: 0 };
|
||||
|
||||
/** The extent this window can show, in whole DATA cells. */
|
||||
function extent() {
|
||||
@@ -258,18 +259,30 @@ function repaint() {
|
||||
el.categories.replaceChildren(categoryList(model));
|
||||
|
||||
// What the frame costs, so the NEXT viewport announcement is honest: the
|
||||
// header lines, plus a group line if there is one, plus the row header's
|
||||
// width in columns.
|
||||
// header lines, a group line if there is one, the row header's width in
|
||||
// columns, and every inline group row.
|
||||
//
|
||||
// Inline group ROWS are deliberately not counted, even though they take
|
||||
// space. They depend on how many rows the window asked for, so counting
|
||||
// them would make this a feedback loop — a smaller window fetches fewer
|
||||
// rows, which crosses fewer groups, which asks for a larger window. The
|
||||
// three facts below depend only on the axes, so a repaint cannot chase
|
||||
// its own tail. The cost is a little over-fetch, which is the direction
|
||||
// ROW_PX already errs in.
|
||||
// The group rows are the awkward one, because they depend on how many
|
||||
// rows the window asked for: counted directly they are a feedback loop —
|
||||
// a smaller window crosses fewer groups and asks for a larger one, which
|
||||
// crosses more. So the reservation is a HIGH-WATER MARK, which cannot
|
||||
// oscillate because it only ever rises, and it rises in the safe
|
||||
// direction: over-reserving asks for fewer rows than fit and leaves
|
||||
// blank space, where under-reserving asks for more rows than the table
|
||||
// can show and is the off-screen-selection bug itself. It resets when
|
||||
// the view changes, so a dense document does not make every later view
|
||||
// pessimistic.
|
||||
if (model.view_name !== frameCost.view) {
|
||||
frameCost = { view: model.view_name, rows: 1, cols: 1, groups: 0 };
|
||||
}
|
||||
const groups = Math.max(
|
||||
frameCost.groups,
|
||||
model.rows.reduce((n, row) => n + (row.kind === "group" ? 1 : 0), 0),
|
||||
);
|
||||
const next = {
|
||||
rows: model.col_levels.length + (model.col_groups.length > 0 ? 1 : 0),
|
||||
view: model.view_name,
|
||||
groups,
|
||||
rows: model.col_levels.length + (model.col_groups.length > 0 ? 1 : 0) + groups,
|
||||
cols: model.row_levels,
|
||||
};
|
||||
if (next.rows !== frameCost.rows || next.cols !== frameCost.cols) {
|
||||
|
||||
Reference in New Issue
Block a user