perf(datagrid): draw the column separators instead of keeping one view per column (#2381)

[datlechin]

Opening the inline cell editor on a wide result took about a second. This fixes that, and carries PR #2385, which was merged but never reached main.

Why this carries #2385

#2382 was squash-merged to main at 18:30:28. #2385 was merged 40 seconds later into its stacked base branch, fix/datagrid-wide-result-windowing-2381, so main got the column-window arithmetic fix and none of the drawn-cells work. That work survives as 62bbc4439 and is included here unchanged, plus the fixes below. It cannot land on its own: three of its call sites were left broken, and it makes the separator cost worse by attaching every column.

Root cause

DataGridView.swift:78 set gridStyleMask = [.solidVerticalGridLineMask]. AppKit implements a vertical grid line as one separator NSView per column, held as a direct subview of the table view, and every layout pass re-sorts that whole subview list:

-[NSTableView layout]
  -[NSTableView _updateVerticalSeparator]
    -[NSTableView _updateSeparatorPositions]
      -[NSView sortSubviewsUsingFunction:context:]
        CFSortIndexes -> __CFSimpleMergeSort
          comparator -> -[NSArray containsObject:]     <- hottest leaf

An O(n) containsObject: inside an O(n log n) sort. Adding or removing any subview of the table view invalidates the ordering and forces a pass, and the inline editor adds one on the way in and removes it on the way out.

Measured in the app, with an override of NSTableView.layout counting passes and summing wall clock:

columns	grid lines	tableView.subviews	layout per edit
100	on	135	18 ms
100	off	34	0.03 ms
500	on	535	1,037 ms
500	off	34	0.06 ms

beginCellEdit itself is 3 to 6 ms in every configuration. The cost scales with attached columns, not rows.

The change

Only the vertical mask behaves this way. .solidHorizontalGridLineMask adds no subviews at all, because AppKit draws a horizontal separator inside NSTableRowView.drawSeparator(in:). So the grid clears gridStyleMask and draws the separators itself, and DataGridBodyChrome owns their geometry, thickness and colour exactly as SortableHeaderChrome does for the header.

Where they are drawn is not a free choice, and it was measured rather than assumed: a row view covers whatever the table view drew beneath it, so a line from drawGrid(inClipRect:) or drawBackground(inClipRect:) is invisible behind the rows and visible only past the last row.

	inside a row band	below the last row
drawBackground(inClipRect:)	covered	visible
drawGrid(inClipRect:)	covered	visible

So a row paints the separators crossing it, in a second pass after its cells so a modified or find-match tint cannot paint over one, and KeyHandlingTableView.drawBackground(inClipRect:) paints only the area below the last row. The separator stands at each presented column's leading edge, which is where AppKit put it and which keeps the row-number column's boundary; the boundary comes from presentsColumn and rect(ofColumn:), never from a fixed step.

Re-parenting the editor off the table view was measured as an alternative and rejected: it removes the edit-time cost but leaves 33 ms per scroll step at 500 columns, because the sort runs on every layout pass.

Three regressions from #2385, fixed here

Found by auditing the subsystem, all confirmed against the tree:

• Every in-cell editor was unreachable. Twelve guards asked view(atColumn:row:makeIfNecessary:false), which is always nil once cells are drawn, so the JSON, blob, PHP, date, enum, set, array, dropdown and type-picker editors all returned early. They now go through one presentsCell(row:tableColumnIndex:) instead of twelve copies of a guard.
• Seven repaint paths reached nothing. reloadData(forRowIndexes:columnIndexes:) rebuilds a cell view, and the row-number column is the only one that still has one. A committed cell edit, an undo, a display-setting change and the find-match highlight all stopped repainting. They go through repaintRows(_:), which reloads the row-number column and repaints the drawn cells.
• A long value froze the overlay. The editor and the viewer both handed the whole value to a wrapping TextKit 2 container, which lays the entire paragraph out before the overlay can appear: 206 ms for a 256 KB value, 816 ms for 1 MB, against 7 ms unwrapped. A cell holds one value, so both now behave like a field editor and scroll a long line. One definition, CellOverlayBase.applyCellTextLayout(to:), covers both.

Measured after the change

Same method as the measurement above: sandboxed Debug build, the same 500-column SQLite table, real CGEvent double clicks, sample at 1ms.

symbol	before	after
-[NSTableView _updateVerticalSeparator]	118 samples	0
-[NSView sortSubviewsUsingFunction:context:]	118 samples	0
CFSortIndexes	present	0
-[NSTableView layout]	120 samples	0

The whole path is gone, not reduced. tableView.subviews drops from 535 to 34 on a 500-column result.

The separators are drawn on the same pixels AppKit drew them on. Rasterising the same table in the same window before and after, the vertical transitions at y=300 are identical in both:

before: [742, 836, 838, 958, 960, 1442, 1444, 1926, 1928, 2410, 2412, 2894, 2896]
after:  [742, 836, 838, 958, 960, 1442, 1444, 1926, 1928, 2410, 2412, 2894, 2896]

The colour is tableView.gridColor, the same value AppKit was filling with, so dark mode and an appearance change follow it without a second spelling.

Verified

• verify.sh build PASS
• verify.sh test PASS, 52 cases across 7 suites
• verify.sh lint TablePro TableProTests PASS, 0 violations
• New: DataGridBodyChromeTests measures separator geometry against rect(ofColumn:) and rasterises a row against tableView.gridColor, so it cannot pass by agreeing with its own arithmetic. DrawnCellReachabilityTests covers the reachability guard, the repaint path and an out-of-range row. CellOverlayTextLayoutTests covers the overlay layout on TextKit 2.

One measured detail worth knowing when reading DataGridBodyChrome: rect(ofColumn:) includes the intercell spacing, so a column's separator sits inside the previous column's rect. columnIndexes(in:) returns both columns for that rect, so a partial repaint still redraws it, and the separators go down after the cells so a tint cannot paint over one. partialRepaintRedrawsTheSeparatorInsideIt pins that.

Two things left open, deliberately

• commitTypedCellEdit drops the whole display cache to repaint one cell, where applyDelta(.cellChanged) invalidates just that cell. Pre-existing, and narrowing it changes caching semantics, so it is reported rather than changed here.
• A 1MB single-line value now produces a document view around 5.6 million points wide. That is strictly better than the 816ms freeze it replaces, but the complete answer is routing a very large value to a full editor instead of the inline overlay, and there is no existing route for arbitrary text.

[chatgpt-codex-connector]

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

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated (UTC)
TablePro	🟢 Ready	View Preview	Aug 22, 2026, 8:50 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.