Skip to content

Commit 3a3a5e7

Browse files
committed
Fix stale rows when repainting in place while scrolled back
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 #582, and the draw still repaints only the rows intersecting the dirty rect, each read from its correct yDisp-relative line.
1 parent 25ba06f commit 3a3a5e7

2 files changed

Lines changed: 110 additions & 16 deletions

File tree

Sources/SwiftTerm/Apple/AppleTerminalView.swift

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1844,25 +1844,41 @@ extension TerminalView {
18441844

18451845
#if os(macOS)
18461846
let baseLine = frame.height
1847-
var region = CGRect (x: 0,
1847+
var region: CGRect
1848+
// `rowStart`/`rowEnd` come from the terminal's update range, which is recorded
1849+
// in `buffer.y` space (relative to `yBase`, the live screen). The rect below
1850+
// maps them to the screen as if row `y` were screen row `y`, but
1851+
// drawTerminalContents maps screen rects back to buffer rows via `yDisp`.
1852+
// Those two agree only while the viewport is pinned to the bottom. Once the
1853+
// user scrolls back by `k` rows, the cells that changed are drawn at screen
1854+
// row `y - k` while the invalidation still covers screen row `y`, so the rows
1855+
// that actually changed are never repainted and keep stale pixels until
1856+
// something forces a full redraw. Invalidate everything in that case; the
1857+
// draw still only repaints rows intersecting the dirty rect and reads each
1858+
// from its correct `yDisp`-relative buffer line.
1859+
if terminal.displayBuffer.yDisp != terminal.displayBuffer.yBase {
1860+
region = CGRect (x: 0, y: 0, width: frame.width, height: frame.height)
1861+
} else {
1862+
region = CGRect (x: 0,
18481863
y: baseLine - (cellDimension.height + CGFloat(rowEnd) * cellDimension.height),
18491864
width: frame.width,
18501865
height: CGFloat(rowEnd-rowStart + 1) * cellDimension.height)
1851-
1852-
// If we are the last line, we should also queue a refresh for the "remaining" bits at the
1853-
// end which can be redrawn by large unicode
1854-
if rowEnd == terminal.rows - 1 {
1855-
let oh = region.height
1856-
let oy = region.origin.y
1857-
region = CGRect (x: 0, y: 0, width: frame.width, height: oh + oy)
1858-
} else {
1859-
// Region ends mid-screen (a restricted DECSTBM region): extend the
1860-
// invalidation down by one cell so the sub-cell remainder just below the
1861-
// band's bottom row (descenders / tall unicode) is cleared too. Previously
1862-
// only rowEnd == rows-1 got this, leaving a one-row ghost below the region.
1863-
let extra = cellDimension.height
1864-
let newY = max (0, region.origin.y - extra)
1865-
region = CGRect (x: 0, y: newY, width: frame.width, height: region.maxY - newY)
1866+
1867+
// If we are the last line, we should also queue a refresh for the "remaining" bits at the
1868+
// end which can be redrawn by large unicode
1869+
if rowEnd == terminal.rows - 1 {
1870+
let oh = region.height
1871+
let oy = region.origin.y
1872+
region = CGRect (x: 0, y: 0, width: frame.width, height: oh + oy)
1873+
} else {
1874+
// Region ends mid-screen (a restricted DECSTBM region): extend the
1875+
// invalidation down by one cell so the sub-cell remainder just below the
1876+
// band's bottom row (descenders / tall unicode) is cleared too. Previously
1877+
// only rowEnd == rows-1 got this, leaving a one-row ghost below the region.
1878+
let extra = cellDimension.height
1879+
let newY = max (0, region.origin.y - extra)
1880+
region = CGRect (x: 0, y: newY, width: frame.width, height: region.maxY - newY)
1881+
}
18661882
}
18671883
#if canImport(MetalKit)
18681884
if metalView != nil {
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import Testing
2+
@testable import SwiftTerm
3+
4+
#if os(macOS)
5+
import AppKit
6+
7+
private final class ScrollbackRepaintDelegate: TerminalViewDelegate {
8+
func sizeChanged(source: TerminalView, newCols: Int, newRows: Int) {}
9+
func setTerminalTitle(source: TerminalView, title: String) {}
10+
func hostCurrentDirectoryUpdate(source: TerminalView, directory: String?) {}
11+
func send(source: TerminalView, data: ArraySlice<UInt8>) {}
12+
func scrolled(source: TerminalView, position: Double) {}
13+
func rangeChanged(source: TerminalView, startY: Int, endY: Int) {}
14+
}
15+
16+
/// Records the rects `updateDisplay` asks AppKit to repaint.
17+
private final class InvalidationCapturingTerminalView: TerminalView {
18+
var invalidated: [NSRect] = []
19+
20+
override func setNeedsDisplay(_ invalidRect: NSRect) {
21+
invalidated.append(invalidRect)
22+
super.setNeedsDisplay(invalidRect)
23+
}
24+
}
25+
26+
struct ScrollbackRepaintTests {
27+
private let esc = "\u{1b}"
28+
29+
/// The update range is recorded in `buffer.y` space (relative to `yBase`),
30+
/// but the draw maps screen rects back to buffer rows through `yDisp`. When
31+
/// the viewport is scrolled back the two disagree by `yBase - yDisp`, so a
32+
/// partial invalidation lands on the wrong screen rows and the cells that
33+
/// changed keep stale content.
34+
///
35+
/// Scrolling output hides this, since `scroll()` dirties both `scrollTop`
36+
/// and `scrollBottom` and the whole view gets invalidated anyway. The case
37+
/// that stays exposed is an in-place repaint, so the write below neither
38+
/// scrolls nor moves the cursor between rows.
39+
@Test func inPlaceRepaintWhileScrolledBackInvalidatesRenderedRow() {
40+
let view = InvalidationCapturingTerminalView(frame: CGRect(x: 0, y: 0, width: 640, height: 320))
41+
view.terminalDelegate = ScrollbackRepaintDelegate()
42+
let terminal: Terminal = view.terminal
43+
let cellHeight = view.cellDimension.height
44+
45+
for i in 0..<(terminal.rows * 3) {
46+
terminal.feed(text: "line \(i)\r\n")
47+
}
48+
view.updateDisplay()
49+
50+
let scrolledBackBy = 3
51+
let maxScrollback = max(0, terminal.buffer.lines.count - terminal.buffer.rows)
52+
view.scrollTo(row: maxScrollback - scrolledBackBy)
53+
#expect(terminal.buffer.yBase - terminal.buffer.yDisp == scrolledBackBy)
54+
55+
// Park the cursor on the target row and flush, so the repaint below
56+
// dirties only that row instead of also dirtying the row it came from.
57+
let targetRow = 2
58+
terminal.feed(text: "\(esc)[\(targetRow + 1);1H")
59+
view.updateDisplay()
60+
terminal.clearUpdateRange()
61+
view.invalidated.removeAll()
62+
63+
terminal.feed(text: "XXXX")
64+
let updateRange = terminal.getUpdateRange()
65+
#expect(updateRange?.startY == targetRow)
66+
#expect(updateRange?.endY == targetRow)
67+
view.updateDisplay()
68+
69+
// The change is on buffer row `targetRow` of the live screen, which is
70+
// drawn `yBase - yDisp` rows further down the viewport.
71+
let renderedRow = targetRow + (terminal.buffer.yBase - terminal.buffer.yDisp)
72+
let rowBottom = view.frame.height - CGFloat(renderedRow + 1) * cellHeight
73+
let rowTop = rowBottom + cellHeight
74+
let repainted = view.invalidated.contains { $0.minY <= rowBottom && $0.maxY >= rowTop }
75+
#expect(repainted, "invalidated \(view.invalidated), change renders at [\(rowBottom), \(rowTop)]")
76+
}
77+
}
78+
#endif

0 commit comments

Comments
 (0)