Skip to content

Fix stale rows when repainting in place while scrolled back - #620

Merged
migueldeicaza merged 2 commits into
migueldeicaza:mainfrom
evertjr:fix/stale-repaint-when-scrolled-back
Aug 7, 2026
Merged

Fix stale rows when repainting in place while scrolled back#620
migueldeicaza merged 2 commits into
migueldeicaza:mainfrom
evertjr:fix/stale-repaint-when-scrolled-back

Conversation

@evertjr

@evertjr evertjr commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Problem

The terminal's update range is recorded in buffer.y space, relative to yBase. updateDisplay turns it into an invalidation rect as if buffer row y were screen row y, but drawTerminalContents maps screen rects back to buffer rows through yDisp:

let firstRow = displayBuffer.yDisp+Int ((boundsMaxY-dirtyRect.maxY)/cellHeight)

Those agree only while the viewport is pinned to the bottom. Scrolled back by k rows, a change at buffer row y renders at screen row y + k while the invalidation still covers screen row y, so the rows that actually changed are never repainted. They stay stale until a resize or another scroll forces a full redraw.

Scrolling output masks it: scroll() ends with updateRange(scrollTop) and updateRange(scrollBottom), so on a full-height screen rows 0 and rows-1 are both dirty and the rowEnd == rows - 1 branch invalidates everything anyway. What stays exposed is in-place repainting, where the update range remains a strict subset. Agent TUIs that redraw a status block several times a second (Claude Code, Codex) hit this constantly: scroll up a few lines while one is working and the visible rows stop tracking the buffer.

The alternate screen is unaffected, it has no scrollback so yDisp == yBase always.

Fix

Invalidate the whole view when yDisp != yBase.

The pinned-to-bottom case keeps the existing partial rect untouched, including the restricted-region extension from #582. It is less blunt than it reads: drawTerminalContents still repaints only the rows intersecting the dirty rect, each read from its correct yDisp-relative line.

CoreGraphics path only. Metal builds its own metalDirtyRange a few lines below and is not affected.

Tests

Added a regression test: 20-row view, scrolled back 3 rows, cursor parked on row 2, then a write that neither scrolls nor moves the cursor. Update range is (2,2), and before the fix the invalidated rect is [256, 288] while the change renders at [224, 240]. Fails on main, passes with the fix. Rest of the suite is green.

The terminal's update range is recorded in buffer.y space, relative to
yBase. updateDisplay turns it into an invalidation rect as if buffer row
y were screen row y, but drawTerminalContents maps screen rects back to
buffer rows through yDisp.

Those agree only while the viewport is pinned to the bottom. Scrolled
back by k rows, a change at buffer row y renders at screen row y + k
while the invalidation still covers screen row y, so the rows that
actually changed are never repainted and keep stale content until a
resize or another scroll forces a full redraw.

Scrolling output masks it, since scroll() dirties both scrollTop and
scrollBottom and the rowEnd == rows - 1 branch then invalidates the whole
view anyway. What stays exposed is in-place repainting, where the update
range remains a strict subset: scroll up a few lines while a TUI redraws
a status block and the visible rows stop tracking the buffer.

Invalidate the whole view when yDisp != yBase. The pinned case keeps the
existing partial rect untouched, including the restricted-region
extension from migueldeicaza#582, and the draw still repaints only the rows
intersecting the dirty rect, each read from its correct yDisp-relative
line.
@migueldeicaza

Copy link
Copy Markdown
Owner

This looks good, just a couple of small edits coming in in a second

@migueldeicaza
migueldeicaza merged commit cc5ed6d into migueldeicaza:main Aug 7, 2026
1 check passed
@migueldeicaza

Copy link
Copy Markdown
Owner

Thank you for your contribution!

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.

2 participants