Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ public enum ControlCommandExecutionPolicy: Sendable, Equatable {
// connection-owned shutdown path, which awaits asynchronous writers.
// Keep that wait off the main actor.
"debug.mobile.transport.disconnect",
// debug.surface.screenshot blocks its worker on the renderer's
// presented-frame acknowledgment, which is delivered on the main
// thread; running it on the main actor would deadlock for the full
// capture timeout. UI/model access inside the handler stays on main
// via v2MainSync and the main-thread presented callback.
"debug.surface.screenshot",
// Browser automation methods that wait on page JavaScript, WebKit
// cookies, or capture callbacks run on the socket worker: on the main
// actor they block SwiftUI updates for their full duration, and on a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ struct ControlCommandExecutionPolicyTests {
"workspace.remote.pty_bridge", "workspace.env", "sidebar.custom.reload",
"sidebar.custom.open",
"debug.sidebar.simulate_drag", "debug.mobile.transport.disconnect",
"debug.window.screenshot", "mobile.attach_ticket.create",
"debug.window.screenshot", "debug.surface.screenshot",
"mobile.attach_ticket.create",
"mobile.terminal.set_font", "mobile.task.models.list",
// JavaScript-evaluating browser methods block on page JS and must
// not hold the main actor (see socketWorkerMethods rationale).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@ extension TerminalSurface {
on: runtimeSurface,
callbackContext: callbackContext
)
installRenderPresentedObservation(
on: runtimeSurface,
callbackContext: callbackContext
)
}
#endif
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
public import Foundation
internal import GhosttyKit
internal import CmuxTerminalCore
internal import CmuxFoundation

/// One registered waiter for a tokened render-presented acknowledgment.
///
/// `expiresAt` bounds bookkeeping only: a waiter whose forced draw was skipped
/// by the renderer (unrealized renderer, zero-sized surface, size-discarded
/// layer assignment) never receives its callback, so stale entries are pruned
/// opportunistically on later registrations instead of leaking until surface
/// teardown. The caller's own timeout (the socket-worker callback awaiter)
/// remains the authoritative failure signal.
struct TerminalSurfacePresentedFrameWaiter {
let expiresAt: Date
let onPresented: @MainActor () -> Void
}

/// The libghostty render-presented callback (cmux fork C API). Ghostty invokes
/// it on the main thread, in the same dispatched block that assigned the
/// frame's IOSurface to the layer, so the layer's `contents` observed from the
/// waiter is at least as new as the acknowledged frame.
private let terminalGhosttyRenderPresentedCallback:
ghostty_render_presented_cb = { userdata, token in
guard let userdata else { return }
let userdataAddress = UInt(bitPattern: userdata)
MainActor.assumeIsolated {
guard let mainActorUserdata =
UnsafeMutableRawPointer(bitPattern: userdataAddress) else {
return
}
let context = Unmanaged<GhosttySurfaceCallbackContext>
.fromOpaque(mainActorUserdata)
.takeUnretainedValue()
guard let surface =
context.surfaceController as? TerminalSurface else {
return
}
surface.deliverRenderPresentedToken(token)
}
}

extension TerminalSurface {
/// The lifetime bound for stale-waiter pruning. Comfortably above every
/// socket-side capture timeout so a pruned entry can never race a waiter
/// whose caller is still blocked.
private static let presentedFrameWaiterLifetime: TimeInterval = 60

/// Monotonic token source for tokened renders. An atomic (not main-actor
/// state) so nonisolated callers can mint tokens before hopping to main.
private static let presentedFrameTokenSource = AtomicUInt64Value(1)

/// Mints a process-unique nonzero token for one tokened render.
public static func makePresentedFrameToken() -> UInt64 {
presentedFrameTokenSource.wrappingIncrementRelaxed()
}

/// Installs the per-runtime-surface render-presented callback. Called once
/// directly after `ghostty_surface_new`, mirroring
/// `installFontSizeActionObservation`.
@MainActor
func installRenderPresentedObservation(
on runtimeSurface: ghostty_surface_t,
callbackContext: Unmanaged<GhosttySurfaceCallbackContext>
) {
precondition(
ghostty_surface_set_render_presented_callback(
runtimeSurface,
terminalGhosttyRenderPresentedCallback,
callbackContext.toOpaque()
),
"Each Ghostty surface installs one render-presented callback"
)
}

/// Requests one forced renderer-thread frame whose presentation is
/// acknowledged with `token`, invoking `onPresented` on the main actor
/// after the exact frame's IOSurface has been assigned to the layer.
///
/// Returns false without retaining `onPresented` when the surface has no
/// live runtime pointer or the runtime refused the request (no callback
/// installed, or another tokened draw is still pending). The caller owns
/// timeout handling; `cancelPresentedFrameWaiter` withdraws a waiter whose
/// caller gave up.
@MainActor
public func requestPresentedFrame(
token: UInt64,
onPresented: @escaping @MainActor () -> Void
) -> Bool {
guard let surface = liveSurfaceForGhosttyAccess(
reason: "renderer.requestPresentedFrame"
) else { return false }
prunePresentedFrameWaiters(now: Date())
pendingRenderPresentedWaiters[token] = TerminalSurfacePresentedFrameWaiter(
expiresAt: Date().addingTimeInterval(Self.presentedFrameWaiterLifetime),
onPresented: onPresented
)
guard ghostty_surface_request_render_with_token(surface, token) else {
pendingRenderPresentedWaiters.removeValue(forKey: token)
return false
}
return true
}

/// Withdraws a waiter whose caller timed out or was cancelled.
@MainActor
public func cancelPresentedFrameWaiter(token: UInt64) {
pendingRenderPresentedWaiters.removeValue(forKey: token)
}

/// Completes the waiter registered for `token`, if any. Tokens without a
/// waiter (already cancelled, or minted by another consumer of the tokened
/// render API) are ignored.
@MainActor
func deliverRenderPresentedToken(_ token: UInt64) {
guard let waiter =
pendingRenderPresentedWaiters.removeValue(forKey: token) else {
return
}
waiter.onPresented()
}

@MainActor
private func prunePresentedFrameWaiters(now: Date) {
guard !pendingRenderPresentedWaiters.isEmpty else { return }
pendingRenderPresentedWaiters = pendingRenderPresentedWaiters.filter {
$0.value.expiresAt > now
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,10 @@ extension TerminalSurface {
on: createdSurface,
callbackContext: surfaceCallbackContext
)
installRenderPresentedObservation(
on: createdSurface,
callbackContext: surfaceCallbackContext
)
if source == .scheduledRestore || source == .inputDemand {
requiresRestoreSpawnPacing = false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,14 @@ public final class TerminalSurface: Identifiable, ObservableObject {
/// state unless the workspace focus path requests it.
var desiredFocusState: Bool = false

/// Pending waiters for tokened render-presented acknowledgments, keyed by
/// the token passed to `ghostty_surface_request_render_with_token`. Filled
/// and drained on the main actor (the libghostty presented callback runs
/// on main, in the same block as the layer's IOSurface assignment).
/// See `TerminalSurface+PresentedFrameCapture.swift`.
var pendingRenderPresentedWaiters:
[UInt64: TerminalSurfacePresentedFrameWaiter] = [:]

/// Bumped after every completed runtime clipboard read.
public internal(set) var clipboardReadGeneration = 0
#if DEBUG
Expand Down
65 changes: 65 additions & 0 deletions Sources/GhosttyTerminalView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11308,6 +11308,71 @@ final class GhosttySurfaceScrollView: NSView {
)
}

/// One deep-copied presented terminal frame for the offscreen screenshot RPC.
struct DebugPresentedFrameImage {
/// Deep copy of the presented IOSurface pixels, tagged Display P3 (the
/// Ghostty Metal renderer draws every frame into a Display P3 BGRA8
/// target; see `ghostty/src/renderer/metal/Target.zig`).
let image: CGImage
/// The layer's contents scale (the native backing scale, 2 on Retina).
let backingScale: CGFloat
}

/// Deep-copies the renderer's presented IOSurface — the exact bytes the
/// compositor would show for the terminal grid area — into a CGImage
/// tagged with the renderer's true color space (Display P3), at native
/// backing resolution. Unlike `CGWindowListCreateImage`, this needs no
/// Screen Recording permission, no window-server compositing, and works
/// while the window is occluded, on another Space, or never ordered front.
func debugCopyPresentedFrameImage() -> DebugPresentedFrameImage? {
guard let modelLayer = surfaceView.layer else { return nil }
let layer = modelLayer.presentation() ?? modelLayer
guard let contents = layer.contents else { return nil }
Comment thread
cursor[bot] marked this conversation as resolved.
Comment on lines +11327 to +11330

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Read the acknowledged model-layer IOSurface.

Line 11329 prefers modelLayer.presentation(). That layer can still reference the preceding frame when the callback runs. The callback contract only guarantees that modelLayer.contents has the acknowledged IOSurface. Capture modelLayer.contents directly so the response matches the tokened render request.

Proposed fix
-        let layer = modelLayer.presentation() ?? modelLayer
-        guard let contents = layer.contents else { return nil }
+        guard let contents = modelLayer.contents else { return nil }
...
-            backingScale: max(1.0, layer.contentsScale)
+            backingScale: max(1.0, modelLayer.contentsScale)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func debugCopyPresentedFrameImage() -> DebugPresentedFrameImage? {
guard let modelLayer = surfaceView.layer else { return nil }
let layer = modelLayer.presentation() ?? modelLayer
guard let contents = layer.contents else { return nil }
func debugCopyPresentedFrameImage() -> DebugPresentedFrameImage? {
guard let modelLayer = surfaceView.layer else { return nil }
guard let contents = modelLayer.contents else { return nil }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Sources/GhosttyTerminalView.swift` around lines 11327 - 11330, Update
debugCopyPresentedFrameImage() to read contents directly from modelLayer and
remove the presentation-layer fallback, ensuring the returned image uses the
acknowledged IOSurface associated with the render request token.


let cf = contents as CFTypeRef
guard CFGetTypeID(cf) == IOSurfaceGetTypeID() else { return nil }
let surfaceRef = (contents as! IOSurfaceRef)

let width = Int(IOSurfaceGetWidth(surfaceRef))
let height = Int(IOSurfaceGetHeight(surfaceRef))
let bytesPerRow = Int(IOSurfaceGetBytesPerRow(surfaceRef))
guard width > 0, height > 0, bytesPerRow > 0 else { return nil }

IOSurfaceLock(surfaceRef, [.readOnly], nil)
defer { IOSurfaceUnlock(surfaceRef, [.readOnly], nil) }

let base = IOSurfaceGetBaseAddress(surfaceRef)
let size = bytesPerRow * height
let data = Data(bytes: base, count: size)
Comment on lines +11341 to +11346

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Handle IOSurfaceLock failure before reading pixels.

IOSurfaceLock can fail. The current path then reads the base address and unlocks an IOSurface that was not successfully locked. Return nil unless the lock succeeds and the base address exists.

Proposed fix
-        IOSurfaceLock(surfaceRef, [.readOnly], nil)
+        guard IOSurfaceLock(surfaceRef, [.readOnly], nil) == KERN_SUCCESS,
+              let base = IOSurfaceGetBaseAddress(surfaceRef) else {
+            return nil
+        }
         defer { IOSurfaceUnlock(surfaceRef, [.readOnly], nil) }
 
-        let base = IOSurfaceGetBaseAddress(surfaceRef)
         let size = bytesPerRow * height
#!/bin/bash
set -euo pipefail

sdk_root="$(xcrun --sdk macosx --show-sdk-path)"
fd -a 'IOSurface*.h' "$sdk_root" | while IFS= read -r header; do
  rg -n -C 3 'IOSurfaceLock|IOSurfaceGetBaseAddress|IOSurfaceUnlock' "$header"
done
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Sources/GhosttyTerminalView.swift` around lines 11326 - 11331, Update the
IOSurface pixel-reading flow around IOSurfaceLock to check its return status
before registering the IOSurfaceUnlock defer; return nil when locking fails.
Also validate that IOSurfaceGetBaseAddress returns a non-nil address before
constructing Data, returning nil otherwise.


guard let provider = CGDataProvider(data: data as CFData),
let colorSpace = CGColorSpace(name: CGColorSpace.displayP3) else {
return nil
}
let bitmapInfo = CGBitmapInfo.byteOrder32Little.union(
CGBitmapInfo(rawValue: CGImageAlphaInfo.premultipliedFirst.rawValue)
)

guard let image = CGImage(
width: width,
height: height,
bitsPerComponent: 8,
bitsPerPixel: 32,
bytesPerRow: bytesPerRow,
space: colorSpace,
bitmapInfo: bitmapInfo,
provider: provider,
decode: nil,
shouldInterpolate: false,
intent: .defaultIntent
) else { return nil }

return DebugPresentedFrameImage(
image: image,
backingScale: max(1.0, layer.contentsScale)
)
}

/// Sample the IOSurface backing the terminal layer (if any) to detect a transient blank frame
/// without using screenshots/screen recording permissions.
func debugSampleIOSurface(normalizedCrop: CGRect) -> DebugFrameSample? {
Expand Down
1 change: 1 addition & 0 deletions Sources/TerminalController+DebugMethodNames.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ extension TerminalController {
"debug.session_snapshot_benchmark",
"debug.session_snapshot_seed_scrollback",
"debug.window.screenshot",
"debug.surface.screenshot",
"debug.terminal.simulate_file_drop",
"debug.sidebar.simulate_drag",
"debug.mobile.transport.disconnect",
Expand Down
Loading