Skip to content

perf(datagrid): capture the column layout once per Size All Columns to Fit - #2463

Merged
datlechin merged 2 commits into
mainfrom
fix/datagrid-size-all-and-header-resize-zone
Aug 26, 2026
Merged

perf(datagrid): capture the column layout once per Size All Columns to Fit#2463
datlechin merged 2 commits into
mainfrom
fix/datagrid-size-all-and-header-resize-zone

Conversation

@datlechin

@datlechin datlechin commented Aug 26, 2026

Copy link
Copy Markdown
Member

Two data grid defects found while investigating #2446, neither of them the reported bug. #2446 itself is already fixed on main by #2455; this is the residue that fix did not cover.

Size All Columns to Fit was quadratic in the column count

sizeAllColumnsToFit writes one width per column, and every NSTableColumn.width write posts its own columnDidResizeNotification synchronously. Each one reaches the persistence half of tableViewColumnDidResize, which runs scheduleLayoutPersist()captureColumnLayout(), and that walks every attached column. So the menu item costs columns × columns: about 250,000 iterations on the 500-column result #2381 measured. #2455 made the constant worse by adding a full-body invalidation to the same re-entry.

The loop now runs with isRebuildingColumns set, so the run persists once at the end instead of once per column. Nothing is lost: the loop already calls markColumnWidthUserSized per column, so ownership is unaffected, and columnGeometryDidChange() sits ahead of that guard so the repaint #2455 added still happens. The flag is saved and restored rather than cleared, so a fit nested inside a rebuild leaves the rebuild's own suppression intact.

The header's leading edge showed a resize cursor with no divider there

SortableHeaderView.isInResizeZone filtered on resizingMask alone and never asked whether the result presents the column. NSTableHeaderView.headerRect(ofColumn:) returns a zero rect for a hidden column, so its trailing edge is x = 0, and DataGridColumnPool keeps every user-hidden column and every surplus slot of a previously wider result attached with userResizingMask set. Each of those reported a divider at x = 0.

The effect: run a 10-column query and then a 3-column one in the same tab, or just hide a column, and the leftmost 4 points of the header show the column-resize cursor over the row-number heading and swallow the clicks that land there. It also cost one headerRect(ofColumn:) per attached column on every mouse-moved event, surplus slots included.

presentsColumn is the question CLAUDE.md already names for "which columns is the user looking at" (#2381), and it is now asked here too.

CHANGELOG

main carried two entries for one fix: #2456 accidentally swept an uncommitted (#2446) line into its commit alongside #2455's own (#2449) entry. Merged into one, per the "two entries describing one change get merged" rule.

Verified

  • verify.sh build: PASS
  • verify.sh test SizeAllColumnsToFitTests SortableHeaderResizeZoneTests DataGridColumnGeometryRepaintTests TableViewCoordinatorLayoutTests SortableHeaderViewTests: PASS, 62 executed, 62 passed
  • verify.sh lint TablePro/Views/Results TableProTests/Views/Results: 0 violations
  • Reverting only the two source changes turns exactly the 4 regression tests red (theLeadingEdgeIsNotAResizeZone, surplusPoolSlotsAreNotResizeZones, fittingEveryColumnCapturesOnce, captureCountIsIndependentOfColumnCount) and leaves the 7 guard tests green.
  • Codex review and adversarial review both raised nothing against either change.

No UI automation: neither defect has a deterministic XCUITest. The cursor one is a NSCursor state over a 4-point strip, and CI's accessibility tree does not expose header dividers; the perf one is a call count. Both are covered by unit tests that fail without the fix.

No screenshots: neither change alters what the grid draws. The cursor fix changes which pointer appears over a 4-point strip of the header, which a screenshot does not capture.

Found while investigating #2446. That issue is fixed by #2455, not by this PR.

https://claude.ai/code/session_01MS7GCfhh6NmnPKajZ9wfpM

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
@datlechin
datlechin merged commit 4b7f61a into main Aug 26, 2026
1 check passed
@datlechin
datlechin deleted the fix/datagrid-size-all-and-header-resize-zone branch August 26, 2026 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant