Skip to content

Commit 4044974

Browse files
committed
Metal: rebuild a row when its appearance changes, not only its contents
Addresses the review on #636. A line's `generation` moves when its contents change and at no other time, but a row is also drawn differently when it is selected, when a link on it is highlighted, when the modifier for link activation goes down, and when the blink phase turns over. Those arrive through the dirty range, which this branch stopped taking at face value — so the blinking row kept its geometry and stopped blinking, and a selection could be left unpainted. Each cached row now also carries the presentation it was built under: the selected columns on that row, the highlighted link range, whether the modifier is down, and the blink phase. The blink phase is compared only for rows that have something blinking on them — otherwise a single blinking cell would rebuild the whole screen twice a second, which is the cost this cache exists to avoid. Whether a line blinks is decided once per rebuild and kept, since only a change of contents can change it, and that moves the generation. Appearance that belongs to the view rather than to a row goes into `CacheSignature` instead: a colour revision bumped in `colorsChanged`, the anti-aliasing setting for custom glyphs, and the link highlight mode. A new palette rebuilds everything, which is what it should do. Three tests cover the three paths, and each fails without its fix: selecting a row rebuilds that row and only that row, toggling the blink phase rebuilds exactly the blinking row, and replacing a colour rebuilds every visible row. The row counters also leave `#if DEBUG` and lose their `debug` prefix — they are what the tests assert on, and a test that only compiles against a debug build is not a test of what ships. That is what kept the suite from running in release. It runs there now: release, upstream: 3.391 ms per frame, 48.0 rows rebuilt per frame release, this: 0.222 ms per frame, 2.0 rows rebuilt per frame
1 parent b52d654 commit 4044974

5 files changed

Lines changed: 181 additions & 26 deletions

File tree

Sources/SwiftTerm/Apple/AppleTerminalView.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,7 @@ extension TerminalView {
568568
urlAttributes = [:]
569569
attributes = [:]
570570
clearCGColorCache()
571+
colorRevision &+= 1
571572

572573
#if os(macOS)
573574
if !isUsingMetalRenderer {

Sources/SwiftTerm/Apple/Metal/MetalTerminalRenderer.swift

Lines changed: 106 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,34 @@ struct RowDrawBuffers {
128128
var otherImageBuffers: [ImageDrawBuffer]
129129
}
130130

131+
/// What a row looks like beyond what its line holds.
132+
///
133+
/// A line's `generation` moves when its contents change, and nothing else moves
134+
/// it — but a row is also drawn differently when it is selected, when a link on
135+
/// it is highlighted, and when the blink phase turns over. Those arrive through
136+
/// the dirty range, which the cache no longer takes at face value, so they are
137+
/// compared here instead.
138+
struct RowPresentation: Equatable {
139+
let selection: Range<Int>?
140+
let linkHighlight: Range<Int>?
141+
let commandActive: Bool
142+
143+
/// The blink phase, and only for a row that has something blinking on it:
144+
/// otherwise one blinking cell anywhere would rebuild the whole screen
145+
/// twice a second, which is the cost this cache exists to avoid.
146+
let blinkVisible: Bool?
147+
}
148+
131149
struct RowCacheEntry {
132150
var lineRef: BufferLine
133151
var generation: UInt64
134152
var bidiParagraphRevision: Int
153+
var presentation: RowPresentation
154+
155+
/// Whether the line has any blinking cell. Kept from the build rather than
156+
/// asked again: the answer only changes when the contents do, and that
157+
/// moves the generation above.
158+
var hasBlink: Bool
135159
var data: RowDrawData?
136160
var buffers: RowDrawBuffers?
137161
}
@@ -198,6 +222,19 @@ struct KittyCacheStamp: Hashable {
198222
let nextPlacementId: UInt32
199223
}
200224

225+
extension LinkHighlightMode {
226+
/// A value a cache signature can hold. Not `Hashable` on the type itself:
227+
/// that is public API, and this is an implementation detail.
228+
var cacheStamp: Int {
229+
switch self {
230+
case .hover: return 0
231+
case .hoverWithModifier: return 1
232+
case .always: return 2
233+
case .alwaysWithModifier: return 3
234+
}
235+
}
236+
}
237+
201238
struct CacheSignature: Hashable {
202239
let scale: Double
203240
let cellWidth: Double
@@ -209,6 +246,14 @@ struct CacheSignature: Hashable {
209246
let fontName: String
210247
let fontSize: Double
211248
let isAltBuffer: Bool
249+
250+
/// Appearance that belongs to the view rather than to any line: the
251+
/// palette, whether custom glyphs are anti-aliased, and which links are
252+
/// underlined. None of these move a line's generation, and all of them
253+
/// change how every row is drawn.
254+
let colorRevision: Int
255+
let antiAliasCustomBlockGlyphs: Bool
256+
let linkHighlightMode: Int
212257
let kittyStamp: KittyCacheStamp
213258
let bidiHostPolicy: BidiHostPolicy
214259
}
@@ -257,13 +302,14 @@ final class MetalTerminalRenderer: NSObject, MTKViewDelegate {
257302
private let frameSemaphore = DispatchSemaphore(value: 1)
258303
private var pendingRedraw = false
259304
private let redrawLock = NSLock()
305+
/// How the last frame split between rebuilt and reused rows. Not behind
306+
/// `DEBUG`: this is what the row-cache tests assert on, and a test that can
307+
/// only run against a debug build is not a test of what ships.
308+
internal var rowsRebuiltLastFrame = 0
309+
internal var rowsReusedLastFrame = 0
260310
#if DEBUG
261311
private var debugFrameCount = 0
262312
private var debugLastLogTime = CFAbsoluteTimeGetCurrent()
263-
/// How the last frame split between rebuilt and reused rows. Internal so a
264-
/// test can measure the row cache without a window to draw into.
265-
internal var debugRowsRebuilt = 0
266-
internal var debugRowsCached = 0
267313
#endif
268314
#if DEBUG
269315
private var imageTextureFailures: Set<ObjectIdentifier> = []
@@ -436,9 +482,9 @@ final class MetalTerminalRenderer: NSObject, MTKViewDelegate {
436482
let now = CFAbsoluteTimeGetCurrent()
437483
let elapsed = now - debugLastLogTime
438484
if elapsed >= 1.0 {
439-
let totalRows = debugRowsRebuilt + debugRowsCached
485+
let totalRows = rowsRebuiltLastFrame + rowsReusedLastFrame
440486
let fps = Double(debugFrameCount) / elapsed
441-
print(String(format: "Metal FPS: %.1f (rows rebuilt: %d/%d)", fps, debugRowsRebuilt, totalRows))
487+
print(String(format: "Metal FPS: %.1f (rows rebuilt: %d/%d)", fps, rowsRebuiltLastFrame, totalRows))
442488
debugFrameCount = 0
443489
debugLastLogTime = now
444490
}
@@ -656,10 +702,8 @@ final class MetalTerminalRenderer: NSObject, MTKViewDelegate {
656702

657703
private func buildDrawDataPass(scale: CGFloat) -> DrawData {
658704
guard let terminalView = terminalView else {
659-
#if DEBUG
660-
debugRowsRebuilt = 0
661-
debugRowsCached = 0
662-
#endif
705+
rowsRebuiltLastFrame = 0
706+
rowsReusedLastFrame = 0
663707
return DrawData(rows: [],
664708
frame: nil,
665709
cursorColorVertices: [],
@@ -677,10 +721,8 @@ final class MetalTerminalRenderer: NSObject, MTKViewDelegate {
677721

678722
let rowInfo = visibleRowRange(buffer: buffer, cellHeight: cellHeight, terminalView: terminalView)
679723
guard let (firstRow, lastRow, visibleDisp) = rowInfo else {
680-
#if DEBUG
681-
debugRowsRebuilt = 0
682-
debugRowsCached = 0
683-
#endif
724+
rowsRebuiltLastFrame = 0
725+
rowsReusedLastFrame = 0
684726
return DrawData(rows: [],
685727
frame: nil,
686728
cursorColorVertices: [],
@@ -712,6 +754,9 @@ final class MetalTerminalRenderer: NSObject, MTKViewDelegate {
712754
fontName: terminalView.fontSet.normal.fontName,
713755
fontSize: Double(terminalView.fontSet.normal.pointSize),
714756
isAltBuffer: terminalView.terminal.isCurrentBufferAlternate,
757+
colorRevision: terminalView.colorRevision,
758+
antiAliasCustomBlockGlyphs: terminalView.antiAliasCustomBlockGlyphs,
759+
linkHighlightMode: terminalView.linkHighlightMode.cacheStamp,
715760
kittyStamp: kittyStamp,
716761
bidiHostPolicy: terminalView.bidiHostPolicy)
717762
let signatureChanged = signature != cacheSignature
@@ -770,9 +815,16 @@ final class MetalTerminalRenderer: NSObject, MTKViewDelegate {
770815
// Cache is valid only when the absolute row still maps to the same
771816
// BufferLine instance (scrolls rotate refs in the CircularList) and
772817
// that line has not been mutated since we cached its draw data.
818+
// Blink is asked of the cached answer where there is one: the
819+
// contents decide it, and unchanged contents cannot have changed
820+
// it. Without an entry the row is being built anyway.
821+
let hasBlink = entry.map { $0.hasBlink } ?? hasBlinkingCell(line: line, cols: buffer.cols)
822+
let rowPresentation = presentation(row: row, line: line, cols: buffer.cols,
823+
terminalView: terminalView, hasBlink: hasBlink)
773824
let cacheValid = entry?.lineRef === line
774825
&& entry?.generation == lineGeneration
775826
&& entry?.bidiParagraphRevision == bidiParagraphRevision
827+
&& entry?.presentation == rowPresentation
776828
// The dirty range is deliberately not consulted for rows that are
777829
// already cached and unchanged. A scroll marks every screen row
778830
// dirty — from the screen's point of view each one holds different
@@ -796,8 +848,14 @@ final class MetalTerminalRenderer: NSObject, MTKViewDelegate {
796848
scale: scale,
797849
virtualPlacementsByImageId: virtualPlacementsByImageId)
798850
let buffers = bufferingMode == .perRowPersistent ? makeRowBuffers(from: rowData) : nil
851+
let blinking = hasBlinkingCell(line: line, cols: buffer.cols)
799852
entry = RowCacheEntry(lineRef: line, generation: lineGeneration,
800853
bidiParagraphRevision: bidiParagraphRevision,
854+
presentation: presentation(row: row, line: line,
855+
cols: buffer.cols,
856+
terminalView: terminalView,
857+
hasBlink: blinking),
858+
hasBlink: blinking,
801859
data: rowData, buffers: buffers)
802860
rowCache[lineKey] = entry
803861
rowBuffers = buffers
@@ -814,6 +872,8 @@ final class MetalTerminalRenderer: NSObject, MTKViewDelegate {
814872
if cached.data == nil {
815873
entry = RowCacheEntry(lineRef: line, generation: lineGeneration,
816874
bidiParagraphRevision: bidiParagraphRevision,
875+
presentation: rowPresentation,
876+
hasBlink: cached.hasBlink,
817877
data: rowData, buffers: cached.buffers)
818878
rowCache[lineKey] = entry
819879
}
@@ -840,8 +900,14 @@ final class MetalTerminalRenderer: NSObject, MTKViewDelegate {
840900
scale: scale,
841901
virtualPlacementsByImageId: virtualPlacementsByImageId)
842902
let buffers = bufferingMode == .perRowPersistent ? makeRowBuffers(from: rowData) : nil
903+
let blinking = hasBlinkingCell(line: line, cols: buffer.cols)
843904
entry = RowCacheEntry(lineRef: line, generation: lineGeneration,
844905
bidiParagraphRevision: bidiParagraphRevision,
906+
presentation: presentation(row: row, line: line,
907+
cols: buffer.cols,
908+
terminalView: terminalView,
909+
hasBlink: blinking),
910+
hasBlink: blinking,
845911
data: rowData, buffers: buffers)
846912
rowCache[lineKey] = entry
847913
rowBuffers = buffers
@@ -877,10 +943,8 @@ final class MetalTerminalRenderer: NSObject, MTKViewDelegate {
877943
}
878944
}
879945
}
880-
#if DEBUG
881-
debugRowsRebuilt = rebuiltRows
882-
debugRowsCached = cachedRows
883-
#endif
946+
rowsRebuiltLastFrame = rebuiltRows
947+
rowsReusedLastFrame = cachedRows
884948

885949
let cursorData = buildCursorDrawData(scale: scale,
886950
cellWidth: cellWidth,
@@ -941,6 +1005,30 @@ final class MetalTerminalRenderer: NSObject, MTKViewDelegate {
9411005
#endif
9421006
}
9431007

1008+
/// What the row's appearance depends on right now, beyond its contents.
1009+
private func presentation(row: Int, line: BufferLine, cols: Int,
1010+
terminalView: TerminalView, hasBlink: Bool) -> RowPresentation {
1011+
var linkHighlight: Range<Int>?
1012+
if let highlights = terminalView.linkHighlightRange {
1013+
linkHighlight = highlights.first(where: { $0.row == row })?.range
1014+
}
1015+
1016+
return RowPresentation(
1017+
selection: terminalView.selectedColumnsRange(row: row, cols: cols),
1018+
linkHighlight: linkHighlight,
1019+
commandActive: terminalView.commandActive,
1020+
blinkVisible: hasBlink ? terminalView.textBlinkVisible : nil)
1021+
}
1022+
1023+
/// Whether anything on the line blinks. Walked once per rebuild, never per
1024+
/// frame — see [RowCacheEntry.hasBlink].
1025+
private func hasBlinkingCell(line: BufferLine, cols: Int) -> Bool {
1026+
for column in 0..<min(cols, line.count) where line[column].attribute.style.contains(.blink) {
1027+
return true
1028+
}
1029+
return false
1030+
}
1031+
9441032
/// Builds one row's geometry in the row's own coordinates — its baseline
9451033
/// sits at zero rather than at a screen position.
9461034
///

Sources/SwiftTerm/Mac/MacTerminalView.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,11 @@ open class TerminalView: NSView, NSTextInputClient, NSUserInterfaceValidations,
208208
var debug: TerminalDebugView?
209209
var pendingDisplay: Bool = false
210210
var textBlinkVisible = true
211+
212+
/// Bumped whenever the colours are replaced, so a cache keyed on
213+
/// appearance can tell that the same line now draws differently. See
214+
/// `colorsChanged` and `CacheSignature`.
215+
var colorRevision = 0
211216
var textBlinkTimer: Timer?
212217
var textBlinkObservers: [(NotificationCenter, NSObjectProtocol)] = []
213218
var textBlinkApplicationActive = true

Sources/SwiftTerm/iOS/iOSTerminalView.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,11 @@ open class TerminalView: UIScrollView, UITextInputTraits, UIKeyInput, UIScrollVi
201201
var debug: UIView?
202202
var pendingDisplay: Bool = false
203203
var textBlinkVisible = true
204+
205+
/// Bumped whenever the colours are replaced, so a cache keyed on
206+
/// appearance can tell that the same line now draws differently. See
207+
/// `colorsChanged` and `CacheSignature`.
208+
var colorRevision = 0
204209
var textBlinkTimer: Timer?
205210
var textBlinkObservers: [(NotificationCenter, NSObjectProtocol)] = []
206211
var textBlinkApplicationActive = true

Tests/SwiftTermTests/MetalRowCacheTests.swift

Lines changed: 64 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,10 @@ final class MetalRowCacheTests: XCTestCase {
5151
_ = renderer.buildDrawData(scale: 2)
5252

5353
XCTAssertLessThanOrEqual(
54-
renderer.debugRowsRebuilt, 3,
55-
"a one-line scroll rebuilt \(renderer.debugRowsRebuilt) rows of "
56-
+ "\(renderer.debugRowsRebuilt + renderer.debugRowsCached)")
57-
XCTAssertGreaterThan(renderer.debugRowsCached, 20)
54+
renderer.rowsRebuiltLastFrame, 3,
55+
"a one-line scroll rebuilt \(renderer.rowsRebuiltLastFrame) rows of "
56+
+ "\(renderer.rowsRebuiltLastFrame + renderer.rowsReusedLastFrame)")
57+
XCTAssertGreaterThan(renderer.rowsReusedLastFrame, 20)
5858
}
5959

6060
/// A frame in which nothing happened must rebuild nothing at all.
@@ -64,7 +64,7 @@ final class MetalRowCacheTests: XCTestCase {
6464
_ = renderer.buildDrawData(scale: 2)
6565

6666
_ = renderer.buildDrawData(scale: 2)
67-
XCTAssertEqual(renderer.debugRowsRebuilt, 0)
67+
XCTAssertEqual(renderer.rowsRebuiltLastFrame, 0)
6868
}
6969

7070
/// The row that changed is the row that is rebuilt — the cache must not
@@ -78,8 +78,64 @@ final class MetalRowCacheTests: XCTestCase {
7878
view.feed(text: "\roverwritten")
7979
_ = renderer.buildDrawData(scale: 2)
8080

81-
XCTAssertGreaterThanOrEqual(renderer.debugRowsRebuilt, 1)
82-
XCTAssertLessThanOrEqual(renderer.debugRowsRebuilt, 3)
81+
XCTAssertGreaterThanOrEqual(renderer.rowsRebuiltLastFrame, 1)
82+
XCTAssertLessThanOrEqual(renderer.rowsRebuiltLastFrame, 3)
83+
}
84+
85+
/// A row is drawn differently when it is selected, and selecting it does
86+
/// not touch the line's contents — so the generation the cache checks does
87+
/// not move. Before the presentation revision, the selected row kept its
88+
/// unselected geometry.
89+
func testSelectingARowRebuildsIt() throws {
90+
let (view, renderer) = try makeRenderer(rows: 24, cols: 80)
91+
feed(view, lines: 40)
92+
_ = renderer.buildDrawData(scale: 2)
93+
_ = renderer.buildDrawData(scale: 2)
94+
XCTAssertEqual(renderer.rowsRebuiltLastFrame, 0, "the second frame should have rebuilt nothing")
95+
96+
view.selection.setSelection(start: Position(col: 0, row: 20),
97+
end: Position(col: 10, row: 20))
98+
_ = renderer.buildDrawData(scale: 2)
99+
100+
XCTAssertGreaterThanOrEqual(renderer.rowsRebuiltLastFrame, 1,
101+
"the selected row has to be built again")
102+
XCTAssertLessThanOrEqual(renderer.rowsRebuiltLastFrame, 2,
103+
"and only that row: \(renderer.rowsRebuiltLastFrame) were")
104+
}
105+
106+
/// The blink phase turns over twice a second and changes nothing about the
107+
/// line's contents. A row with something blinking on it must follow; a row
108+
/// without must not be dragged along with it.
109+
func testBlinkRebuildsOnlyTheBlinkingRow() throws {
110+
let (view, renderer) = try makeRenderer(rows: 24, cols: 80)
111+
feed(view, lines: 20)
112+
view.feed(text: "\u{1b}[5mblinking\u{1b}[0m\r\n")
113+
feed(view, lines: 2)
114+
_ = renderer.buildDrawData(scale: 2)
115+
_ = renderer.buildDrawData(scale: 2)
116+
XCTAssertEqual(renderer.rowsRebuiltLastFrame, 0)
117+
118+
view.textBlinkVisible.toggle()
119+
_ = renderer.buildDrawData(scale: 2)
120+
121+
XCTAssertEqual(renderer.rowsRebuiltLastFrame, 1,
122+
"exactly the blinking row, not the whole screen")
123+
}
124+
125+
/// Replacing the palette changes how every row draws, and moves no line's
126+
/// generation either.
127+
func testChangingTheColoursRebuildsEverything() throws {
128+
let (view, renderer) = try makeRenderer(rows: 24, cols: 80)
129+
feed(view, lines: 40)
130+
_ = renderer.buildDrawData(scale: 2)
131+
_ = renderer.buildDrawData(scale: 2)
132+
XCTAssertEqual(renderer.rowsRebuiltLastFrame, 0)
133+
134+
view.nativeForegroundColor = TTColor.make(red: 0.9, green: 0.2, blue: 0.2, alpha: 1)
135+
_ = renderer.buildDrawData(scale: 2)
136+
137+
XCTAssertGreaterThan(renderer.rowsRebuiltLastFrame, 20,
138+
"a new palette means every visible row")
83139
}
84140

85141
/// What the change is for, in wall-clock terms.
@@ -96,7 +152,7 @@ final class MetalRowCacheTests: XCTestCase {
96152
view.feed(text: "\u{1b}[1;32mscrolled line \(i)\u{1b}[0m — "
97153
+ "\u{1b}[38;5;244mthe quick brown fox jumps over the lazy dog\u{1b}[0m\r\n")
98154
_ = renderer.buildDrawData(scale: 2)
99-
rebuilt += renderer.debugRowsRebuilt
155+
rebuilt += renderer.rowsRebuiltLastFrame
100156
}
101157
let each = Date().timeIntervalSince(started) / 200 * 1000
102158
print(String(format: "buildDrawData while scrolling: %.3f ms per frame, %.1f rows rebuilt per frame",

0 commit comments

Comments
 (0)