-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Smooth iOS terminal scrollback with local prefetch #6067
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -540,6 +540,7 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |||||||||||||
| var terminalOutputQueuesBySurfaceID: [String: TerminalOutputDeliveryQueue] | ||||||||||||||
| var terminalScrollQueueTokensBySurfaceID: [String: UUID] | ||||||||||||||
| var terminalScrollQueuesBySurfaceID: [String: TerminalScrollDeliveryQueue] | ||||||||||||||
| var terminalScrollbackPrefetchStatesBySurfaceID: [String: TerminalScrollbackPrefetchState] | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift Keep new scroll-prefetch bookkeeping out of this over-budget store. These additions further grow As per coding guidelines, Also applies to: 670-670, 3083-3083, 4319-4337, 4371-4371 🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||||||||||
| private var rawTerminalInputBuffer: MobileTerminalInputSendBuffer | ||||||||||||||
| private var pairingAttemptID: UUID | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -666,6 +667,7 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |||||||||||||
| self.terminalOutputQueuesBySurfaceID = [:] | ||||||||||||||
| self.terminalScrollQueueTokensBySurfaceID = [:] | ||||||||||||||
| self.terminalScrollQueuesBySurfaceID = [:] | ||||||||||||||
| self.terminalScrollbackPrefetchStatesBySurfaceID = [:] | ||||||||||||||
| self.rawTerminalInputBuffer = MobileTerminalInputSendBuffer() | ||||||||||||||
| self.pairingAttemptID = UUID() | ||||||||||||||
| } | ||||||||||||||
|
|
@@ -3078,6 +3080,7 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |||||||||||||
| terminalOutputStreamTokensBySurfaceID = terminalOutputStreamTokensBySurfaceID.mapValues { _ in UUID() } | ||||||||||||||
| terminalScrollQueueTokensBySurfaceID = [:] | ||||||||||||||
| terminalScrollQueuesBySurfaceID = [:] | ||||||||||||||
| terminalScrollbackPrefetchStatesBySurfaceID = [:] | ||||||||||||||
| terminalOutputTransport = .rawBytes | ||||||||||||||
| supportedHostCapabilities = [] | ||||||||||||||
| terminalSubscriptionRefreshTask?.cancel() | ||||||||||||||
|
|
@@ -4313,6 +4316,26 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| func deliverAuthoritativeTerminalRenderGrid( | ||||||||||||||
| _ renderGrid: MobileTerminalRenderGridFrame, | ||||||||||||||
| expectedSurfaceID: String? = nil, | ||||||||||||||
| source: String | ||||||||||||||
| ) { | ||||||||||||||
| guard expectedSurfaceID == nil || renderGrid.surfaceID == expectedSurfaceID, | ||||||||||||||
| hasTerminalOutputSink(surfaceID: renderGrid.surfaceID) else { | ||||||||||||||
| return | ||||||||||||||
| } | ||||||||||||||
| if let deliveredSeq = deliveredTerminalByteEndSeqBySurfaceID[renderGrid.surfaceID], | ||||||||||||||
| deliveredSeq > renderGrid.stateSeq { | ||||||||||||||
|
Comment on lines
+4328
to
+4329
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
| MobileDebugLog.anchormux( | ||||||||||||||
| "sync.render_grid_stale source=\(source) surface=\(renderGrid.surfaceID) delivered=\(deliveredSeq) frame=\(renderGrid.stateSeq)" | ||||||||||||||
| ) | ||||||||||||||
| return | ||||||||||||||
| } | ||||||||||||||
| markTerminalBytesDelivered(surfaceID: renderGrid.surfaceID, endSeq: renderGrid.stateSeq) | ||||||||||||||
| deliverTerminalRenderGrid(renderGrid, surfaceID: renderGrid.surfaceID) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| private static func terminalSnapshotReplacementBytes(_ snapshotBytes: Data) -> Data { | ||||||||||||||
| var bytes = Data("\u{1B}c\u{1B}[H\u{1B}[2J\u{1B}[3J".utf8) | ||||||||||||||
| bytes.append(snapshotBytes) | ||||||||||||||
|
|
@@ -4345,6 +4368,7 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |||||||||||||
| terminalOutputQueuesBySurfaceID.removeValue(forKey: surfaceID) | ||||||||||||||
| terminalScrollQueueTokensBySurfaceID.removeValue(forKey: surfaceID) | ||||||||||||||
| terminalScrollQueuesBySurfaceID.removeValue(forKey: surfaceID) | ||||||||||||||
| terminalScrollbackPrefetchStatesBySurfaceID.removeValue(forKey: surfaceID) | ||||||||||||||
| deliveredTerminalByteEndSeqBySurfaceID.removeValue(forKey: surfaceID) | ||||||||||||||
| pendingTerminalByteEndSeqBySurfaceID.removeValue(forKey: surfaceID) | ||||||||||||||
| // Tell the Mac this device is no longer viewing the surface so it stops | ||||||||||||||
|
|
@@ -4533,18 +4557,10 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |||||||||||||
| hasTerminalOutputSink(surfaceID: renderGrid.surfaceID) else { | ||||||||||||||
| return | ||||||||||||||
| } | ||||||||||||||
| if let deliveredSeq = deliveredTerminalByteEndSeqBySurfaceID[renderGrid.surfaceID], | ||||||||||||||
| deliveredSeq > renderGrid.stateSeq { | ||||||||||||||
| MobileDebugLog.anchormux( | ||||||||||||||
| "sync.render_grid_stale surface=\(renderGrid.surfaceID) delivered=\(deliveredSeq) frame=\(renderGrid.stateSeq)" | ||||||||||||||
| ) | ||||||||||||||
| return | ||||||||||||||
| } | ||||||||||||||
| markTerminalBytesDelivered(surfaceID: renderGrid.surfaceID, endSeq: renderGrid.stateSeq) | ||||||||||||||
| #if DEBUG | ||||||||||||||
| mobileShellLog.info("CMUX_REPLAY live render_grid surface=\(renderGrid.surfaceID, privacy: .public) full=\(renderGrid.full, privacy: .public) spans=\(renderGrid.rowSpans.count, privacy: .public) cleared=\(renderGrid.clearedRows.count, privacy: .public) seq=\(renderGrid.stateSeq, privacy: .public) hasSink=true") | ||||||||||||||
| #endif | ||||||||||||||
| deliverTerminalRenderGrid(renderGrid, surfaceID: renderGrid.surfaceID) | ||||||||||||||
| deliverAuthoritativeTerminalRenderGrid(renderGrid, source: "event") | ||||||||||||||
|
Comment on lines
4560
to
+4563
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| private func handleNotificationDismissedEvent(_ event: MobileEventEnvelope) async { | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,11 +5,49 @@ struct TerminalScrollDelivery: Equatable, Sendable { | |
| var lines: Double | ||
| var col: Int | ||
| var row: Int | ||
| var maxScrollbackRows: Int? = nil | ||
|
|
||
| mutating func append(_ delivery: TerminalScrollDelivery) { | ||
| lines += delivery.lines | ||
| col = delivery.col | ||
| row = delivery.row | ||
| switch (maxScrollbackRows, delivery.maxScrollbackRows) { | ||
| case (.some(let current), .some(let incoming)): | ||
| maxScrollbackRows = max(current, incoming) | ||
| case (nil, .some(let incoming)): | ||
| maxScrollbackRows = incoming | ||
| case (.some, nil), (nil, nil): | ||
| break | ||
| } | ||
| } | ||
| } | ||
|
|
||
| struct TerminalScrollbackPrefetchState: Equatable, Sendable { | ||
| static let defaultWindowRows = 600 | ||
| static let defaultRefreshDistanceRows = 120.0 | ||
|
|
||
| var windowRows: Int | ||
| var refreshDistanceRows: Double | ||
| private var hasPrimedWindow = false | ||
| private var accumulatedRowsSincePrefetch = 0.0 | ||
|
|
||
| init( | ||
| windowRows: Int = Self.defaultWindowRows, | ||
| refreshDistanceRows: Double = Self.defaultRefreshDistanceRows | ||
| ) { | ||
| self.windowRows = max(0, windowRows) | ||
| self.refreshDistanceRows = max(1, refreshDistanceRows) | ||
| } | ||
|
Comment on lines
+34
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sanitize non-finite Line 39 clamps lower bound, but 💡 Proposed fix init(
windowRows: Int = Self.defaultWindowRows,
refreshDistanceRows: Double = Self.defaultRefreshDistanceRows
) {
self.windowRows = max(0, windowRows)
- self.refreshDistanceRows = max(1, refreshDistanceRows)
+ let sanitizedRefresh = refreshDistanceRows.isFinite ? refreshDistanceRows : Self.defaultRefreshDistanceRows
+ self.refreshDistanceRows = max(1, sanitizedRefresh)
}🤖 Prompt for AI Agents |
||
|
|
||
| mutating func rowsToPrefetch(forScrollLines lines: Double) -> Int? { | ||
| guard lines != 0, windowRows > 0 else { return nil } | ||
| accumulatedRowsSincePrefetch += abs(lines) | ||
| guard !hasPrimedWindow || accumulatedRowsSincePrefetch >= refreshDistanceRows else { | ||
| return nil | ||
| } | ||
| hasPrimedWindow = true | ||
| accumulatedRowsSincePrefetch = 0 | ||
| return windowRows | ||
|
Comment on lines
+42
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Preserve overflow distance after each prefetch trigger. On Line 49, resetting 💡 Proposed fix mutating func rowsToPrefetch(forScrollLines lines: Double) -> Int? {
guard lines != 0, windowRows > 0 else { return nil }
accumulatedRowsSincePrefetch += abs(lines)
guard !hasPrimedWindow || accumulatedRowsSincePrefetch >= refreshDistanceRows else {
return nil
}
+ let wasPrimed = hasPrimedWindow
hasPrimedWindow = true
- accumulatedRowsSincePrefetch = 0
+ accumulatedRowsSincePrefetch = wasPrimed
+ ? accumulatedRowsSincePrefetch.truncatingRemainder(dividingBy: refreshDistanceRows)
+ : 0
return windowRows
}🤖 Prompt for AI Agents |
||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,46 @@ import Testing | |
| #expect(queue.isIdle) | ||
| } | ||
|
|
||
| @Test func terminalScrollQueueCoalescesLargestScrollbackPrefetchWindow() throws { | ||
| var queue = TerminalScrollDeliveryQueue() | ||
| let inFlight = TerminalScrollDelivery(surfaceID: "surface", lines: 1, col: 1, row: 1) | ||
| let firstPending = TerminalScrollDelivery( | ||
| surfaceID: "surface", | ||
| lines: 2, | ||
| col: 2, | ||
| row: 2, | ||
| maxScrollbackRows: 240 | ||
| ) | ||
| let latestPending = TerminalScrollDelivery( | ||
| surfaceID: "surface", | ||
| lines: 3, | ||
| col: 3, | ||
| row: 3, | ||
| maxScrollbackRows: 600 | ||
| ) | ||
|
|
||
| #expect(queue.enqueue(inFlight) == inFlight) | ||
| #expect(queue.enqueue(firstPending) == nil) | ||
| #expect(queue.enqueue(latestPending) == nil) | ||
|
|
||
| let maybeCoalesced = queue.completeInFlight() | ||
| let coalesced = try #require(maybeCoalesced) | ||
| #expect(coalesced.lines == 5) | ||
| #expect(coalesced.col == 3) | ||
| #expect(coalesced.row == 3) | ||
| #expect(coalesced.maxScrollbackRows == 600) | ||
| } | ||
|
|
||
| @Test func terminalScrollbackPrefetchStatePrimesThenRefreshesByDistance() { | ||
| var state = TerminalScrollbackPrefetchState(windowRows: 600, refreshDistanceRows: 10) | ||
|
|
||
| #expect(state.rowsToPrefetch(forScrollLines: 0) == nil) | ||
| #expect(state.rowsToPrefetch(forScrollLines: 1) == 600) | ||
| #expect(state.rowsToPrefetch(forScrollLines: 4) == nil) | ||
| #expect(state.rowsToPrefetch(forScrollLines: -5.5) == nil) | ||
| #expect(state.rowsToPrefetch(forScrollLines: 0.5) == 600) | ||
| } | ||
|
Comment on lines
+70
to
+78
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win Add an overflow-carry regression case for prefetch cadence. This test covers prime + exact-threshold crossing, but it doesn’t lock behavior for a large single delta (e.g., crossing 🤖 Prompt for AI Agents |
||
|
|
||
| @Test func terminalScrollQueueResetDropsPendingWork() { | ||
| var queue = TerminalScrollDeliveryQueue() | ||
| let inFlight = TerminalScrollDelivery(surfaceID: "surface", lines: 1, col: 1, row: 1) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| #if canImport(UIKit) | ||
| import GhosttyKit | ||
| import UIKit | ||
|
|
||
| extension GhosttySurfaceView { | ||
| /// Apply the scroll to the phone's local Ghostty mirror immediately. On the | ||
| /// primary screen this consumes the preloaded local scrollback window, so a | ||
| /// drag/deceleration feels native while the Mac catches up. On alternate | ||
| /// screens libghostty turns this into mouse-wheel bytes; the mirror is | ||
| /// display-only and drops those bytes, so the authoritative Mac response | ||
| /// remains the visible update for TUIs. | ||
| func applyLocalScrollbackScroll(lines: Double, col: Int, row: Int) { | ||
| guard lines != 0, let surface else { return } | ||
| let displayScale = window?.windowScene?.screen.scale ?? traitCollection.displayScale | ||
| let scale = max(Double(displayScale), 1) | ||
| let size = ghostty_surface_size(surface) | ||
| let cellWidthPt = max(Double(size.cell_width_px) / scale, 1) | ||
| let cellHeightPt = max(Double(size.cell_height_px) / scale, 1) | ||
| let posX = (Double(max(0, col)) + 0.5) * cellWidthPt | ||
| let posY = (Double(max(0, row)) + 0.5) * cellHeightPt | ||
| ghostty_surface_mouse_pos(surface, posX, posY, GHOSTTY_MODS_NONE) | ||
| ghostty_surface_mouse_scroll(surface, 0, lines, 0) | ||
| drawForWakeup() | ||
| } | ||
| } | ||
| #endif |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| import CMUXMobileCore | ||
| import Foundation | ||
|
|
||
| extension TerminalController { | ||
| /// Scrollback rows included in a cold-attach render-grid replay snapshot. | ||
| /// Live render-grid events carry no scrollback; the phone keeps its own | ||
| /// bounded Ghostty scrollback mirror and scrolls that mirror locally while | ||
| /// the Mac remains authoritative. | ||
| nonisolated static let mobileReplayScrollbackLineBudget = 240 | ||
|
|
||
| /// Larger history window returned only on explicit mobile scroll prefetch | ||
| /// requests, keeping ordinary scroll RPCs small. | ||
| nonisolated static let mobileScrollPrefetchScrollbackLineBudget = 600 | ||
|
|
||
| func mobileTerminalRenderGridFrame( | ||
| terminalPanel: TerminalPanel, | ||
| surfaceID: UUID, | ||
| seq: UInt64, | ||
| scrollbackLines: Int = TerminalController.mobileReplayScrollbackLineBudget | ||
| ) -> MobileTerminalRenderGridFrame? { | ||
| guard surfaceID == terminalPanel.id else { return nil } | ||
| return terminalPanel.surface.mobileRenderGridFrame( | ||
| stateSeq: seq, | ||
| scrollbackLines: scrollbackLines | ||
| )?.frame | ||
| } | ||
|
|
||
| func mobileTerminalScrollResponsePayload( | ||
| workspaceID: UUID, | ||
| terminalPanel: TerminalPanel, | ||
| surfaceID: UUID, | ||
| params: [String: Any] | ||
| ) -> [String: Any] { | ||
| var payload: [String: Any] = [ | ||
| "workspace_id": workspaceID.uuidString, | ||
| "surface_id": surfaceID.uuidString, | ||
| ] | ||
| let scrollbackRows = mobileScrollPrefetchRows(params: params) | ||
| guard scrollbackRows > 0 else { return payload } | ||
| let stateSeq = MobileTerminalByteTee.shared.currentSequence(surfaceID: surfaceID) ?? 0 | ||
| guard let renderGrid = mobileTerminalRenderGridFrame( | ||
| terminalPanel: terminalPanel, | ||
| surfaceID: surfaceID, | ||
| seq: stateSeq, | ||
| scrollbackLines: scrollbackRows | ||
| ), | ||
| renderGrid.activeScreen == .primary, | ||
| let renderGridObject = try? renderGrid.jsonObject() else { | ||
| return payload | ||
| } | ||
| payload["columns"] = renderGrid.columns | ||
| payload["rows"] = renderGrid.rows | ||
| payload["render_grid"] = renderGridObject | ||
| payload["seq"] = renderGrid.stateSeq | ||
| return payload | ||
| } | ||
|
|
||
| private func mobileScrollPrefetchRows(params: [String: Any]) -> Int { | ||
| let requestedRows = (params["max_scrollback_rows"] as? NSNumber)?.intValue ?? 0 | ||
| return min( | ||
| max(0, requestedRows), | ||
| Self.mobileScrollPrefetchScrollbackLineBudget | ||
| ) | ||
|
Comment on lines
+59
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify existing TerminalController parsing convention and this callsite.
set -euo pipefail
echo "== v2 numeric helper definitions/usages in Sources/TerminalController* =="
rg -n --type=swift -C2 '\bv2(Int|StrictInt)\s*\(' Sources/TerminalController.swift Sources/TerminalController+*.swift
echo
echo "== current max_scrollback_rows parsing in mobile prefetch extension =="
rg -n --type=swift -C3 'max_scrollback_rows|NSNumber|intValue|mobileScrollPrefetchRows' Sources/TerminalController+MobileScrollPrefetch.swiftRepository: manaflow-ai/cmux Length of output: 9676 🏁 Script executed: rg -n --type=swift -B2 -A5 'func v2(Int|StrictInt)\(' Sources/TerminalController.swift | head -50Repository: manaflow-ai/cmux Length of output: 42 🏁 Script executed: rg -n --type=swift 'func v2Int|func v2StrictInt' Sources/TerminalController.swiftRepository: manaflow-ai/cmux Length of output: 42 🏁 Script executed: rg -n --type=swift 'v2Int|v2StrictInt' Sources/TerminalController.swift | grep 'func\|private' | head -20Repository: manaflow-ai/cmux Length of output: 42 🏁 Script executed: fd --type f --extension swift . Sources | xargs rg -l 'func v2Int|func v2StrictInt' | head -5Repository: manaflow-ai/cmux Length of output: 114 🏁 Script executed: rg -n --type=swift -A8 'func v2Int|func v2StrictInt' Sources/TerminalControllerV2ParamParsingSupport.swiftRepository: manaflow-ai/cmux Length of output: 1088 🏁 Script executed: cat -n Sources/TerminalControllerV2ParamParsingSupport.swift | sed -n '159,180p'Repository: manaflow-ai/cmux Length of output: 923 🏁 Script executed: cat -n Sources/TerminalController+MobileScrollPrefetch.swift | sed -n '58,65p'Repository: manaflow-ai/cmux Length of output: 409 Use Line 59 uses 🤖 Prompt for AI AgentsSource: Learnings |
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 22542
Move scroll-prefetch JSON decode off the main actor.
MobileTerminalReplayResponse.decode(data)executes on the main actor insideperformTerminalScroll(_:)within the scroll hot path. JSON decoding is CPU-bound work that can block frame rendering and scroll gesture responsiveness; detach it to a background task to avoid hitching.♻️ Suggested change
let data = try await client.sendRequest(request) guard let maxScrollbackRows = delivery.maxScrollbackRows, maxScrollbackRows > 0, remoteClient === client else { return } - guard let payload = try? MobileTerminalReplayResponse.decode(data), + let payload = await Task.detached(priority: .userInitiated) { + try? MobileTerminalReplayResponse.decode(data) + }.value + guard remoteClient === client, + let payload, let renderGrid = payload.renderGrid, renderGrid.surfaceID == delivery.surfaceID else { return }🤖 Prompt for AI Agents