Clear the dirty region in the CoreGraphics renderer (fixes restricted-region scroll artifacts)#582
Merged
migueldeicaza merged 1 commit intoJul 1, 2026
Conversation
9fca016 to
f6c4776
Compare
…-region artifacts) Scrolling or editing inside a restricted DECSTBM scroll region (a fixed header/ footer, as in nano/vim/less/htop) left stale rows and a sub-cell "ghost" on the region's last row on the CoreGraphics renderer (Metal masks it by clearing the whole target each frame). The defect is in the renderer, not the scroll primitives: drawTerminalContents fills only cells that carry an explicit background — default-background cells rely on transparent backing-store pixels showing the layer's background color — and AppKit clears the backing store only on a full-view redraw. So a partial repaint (a restricted region, IL/DL) keeps stale pixels, and the sub-cell remainder below the band is only invalidated when rowEnd == rows-1. Fix in AppleTerminalView (macOS CG path only, Metal unaffected): - clear the invalidated region to transparent before the per-row paint (preserves a translucent background), and - extend a mid-screen region's invalidation down one cell so its bottom remainder is cleared too. This fixes every row-mutating primitive at the source. The scroll primitives then only need to flag the rows they changed, so refreshScrolledRegion flags just [scrollTop, scrollBottom] instead of the whole viewport, keeping the full-screen+scrollback cheap path. Verified in nano/vim/less/htop, translucency intact; the package builds cleanly.
f6c4776 to
d8cda94
Compare
Owner
|
Thank you, let me review this - it looks pretty clean. |
Owner
|
This looks good! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Scrolling or editing inside a restricted DECSTBM scroll region (a fixed header and/or footer, as in
nano,vim,less,htop) leaves stale rows and a sub-cell "ghost" on the region's last row on the CoreGraphics renderer (Metal disabled). The Metal path masks it by clearing the whole target each frame.Reproducible: open
nanoon a file with lines wider than the window and hold PageDown/PageUp; or dovimline edits in a fixed-footer layout.Root cause — the renderer, not the scroll primitives
The scroll/edit primitives report a correct dirty range. The CG renderer:
drawTerminalContentsfills background per-cell-run only, and only for cells that carry an explicit background; default-background cells rely on transparent backing-store pixels showing the layer'sbackgroundColor. AppKit clears the backing store only on a full-view redraw, so a partial repaint (a restricted region, IL/DL) keeps stale glyphs/backgrounds.rowEnd == rows-1— a restricted region ends atscrollBottom < rows-1, so the sliver below it is never repainted → the ghost.(There is no pixel blit; unchanged pixels persist via AppKit's dirty-rect coalescing over the layer backing store.)
Fix — macOS CG path only; Metal unaffected
In
AppleTerminalView:context.clear(dirtyRect)before the per-row paint — clears the invalidated region to transparent (a clear, not a fill, so a translucent background is preserved), giving the per-cell paint a clean slate.regiondown by one cell, so the bottom remainder is cleared too (previously onlyrowEnd == rows-1got this).This fixes every row-mutating primitive (
scroll,reverseIndex,cmdScrollUp/Down,cmdInsertLines/DeleteLines) at the source. With the renderer clearing any dirtied region, the primitives only need to flag the rows they changed — sorefreshScrolledRegionflags just[scrollTop, scrollBottom](keeping the full-screen + scrollback cheap path) instead of inflating to the whole viewport.The Metal renderer is untouched: it already
loadAction = .clears the whole target each frame, and theregionextension only feeds the non-MetalsetNeedsDisplay.Testing
Verified in
nano/vim/less/htopon the CG renderer — no stale rows or bottom ghost while paging or editing — with a translucent background intact, and normal full-screen output scrolling unaffected (cheap path preserved). The package builds cleanly.This supersedes an earlier per-primitive workaround on this branch (which inflated each scroll op's dirty range to the whole viewport) with the renderer-level root fix.