Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
13 changes: 7 additions & 6 deletions Sources/GhosttySurfaceScrollView+WorkDiagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ import Foundation

extension GhosttySurfaceScrollView {
/// Returns whether revealing a portal needs the fallback synchronous refresh.
/// A renderer that already presented a frame is already paintable; asking
/// Ghostty to refresh it again during workspace selection can block the main
/// thread while a remote surface drains its current frame.
static func shouldScheduleVisibilityRevealRefresh(hasPresentedFrame: Bool) -> Bool {
!hasPresentedFrame
/// A frame may have been presented before the pane was hidden, while the
/// renderer is no longer presented when the workspace becomes visible again.
/// Use current renderer health so that stale warm-frame state cannot suppress
/// the recovery redraw.
static func shouldScheduleVisibilityRevealRefresh(rendererPresented: Bool) -> Bool {
!rendererPresented
}

/// Request an immediate terminal redraw after geometry updates so stale IOSurface
Expand All @@ -26,7 +27,7 @@ extension GhosttySurfaceScrollView {
let transition = self.pendingVisibilityRefreshTransition
self.pendingVisibilityRefreshTransition = .unknown
guard self.isVisibleInUI else { return }
guard self.surfaceView.terminalSurface?.hasPresentedFrame != true else { return }
guard self.surfaceView.terminalSurface?.isRendererPresented != true else { return }
self.refreshSurfaceNow(reason: "setVisibleInUI.deferred", transition: transition)
}
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/GhosttyTerminalView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12343,7 +12343,7 @@ final class GhosttySurfaceScrollView: NSView {
// from inside SwiftUI update/layout (updateNSView, viewDidMoveToWindow, the
// geometry-callback rebind), where a synchronous display can wedge the main
// thread in Metal against the still-open window transaction.
if GhosttySurfaceScrollView.shouldScheduleVisibilityRevealRefresh(hasPresentedFrame: surfaceView.terminalSurface?.hasPresentedFrame == true) { scheduleVisibilityRevealRefresh(transition: terminalWorkTransition == .unknown ? .reveal : terminalWorkTransition) }
if GhosttySurfaceScrollView.shouldScheduleVisibilityRevealRefresh(rendererPresented: surfaceView.terminalSurface?.isRendererPresented == true) { scheduleVisibilityRevealRefresh(transition: terminalWorkTransition == .unknown ? .reveal : terminalWorkTransition) }
scheduleAutomaticFirstResponderApply(reason: "setVisibleInUI")
}
}
Expand Down
6 changes: 5 additions & 1 deletion cmuxTests/GhosttyTerminalViewVisibilityPolicyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,11 @@ struct GhosttyTerminalViewVisibilityPolicyTests {
)
)
}
@Test func warmRendererRevealDoesNotScheduleBlockingFallbackRefresh() { #expect(!GhosttySurfaceScrollView.shouldScheduleVisibilityRevealRefresh(hasPresentedFrame: true)); #expect(GhosttySurfaceScrollView.shouldScheduleVisibilityRevealRefresh(hasPresentedFrame: false)) }
@Test("A previously painted pane still needs a reveal fallback after renderer loss")
func hiddenRendererRevealSchedulesFallbackRefresh() {
#expect(GhosttySurfaceScrollView.shouldScheduleVisibilityRevealRefresh(rendererPresented: false))
#expect(!GhosttySurfaceScrollView.shouldScheduleVisibilityRevealRefresh(rendererPresented: true))
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}
@Test func immediateStateUpdateAllowedWhenUnboundAndNotAttachedAnywhere() {
#expect(
GhosttyTerminalView.shouldApplyImmediateHostedStateUpdate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,60 @@ import CmuxTerminalCore
#endif

extension TerminalNotificationDirectInteractionTests {
func testPresentedRendererRevealSkipsDeferredRefresh() throws {
#if DEBUG
let window = makeWindow()
defer { window.orderOut(nil) }

guard let contentView = window.contentView else {
XCTFail("Expected content view")
return
}

let livePortalWorkspace = try makeAuthorizedPortalTabId()
defer { livePortalWorkspace.tearDown() }

let surface = TerminalSurface(
tabId: livePortalWorkspace.id,
context: GHOSTTY_SURFACE_CONTEXT_SPLIT,
configTemplate: nil,
workingDirectory: nil
)
let hostedView = surface.hostedView
defer { surface.releaseHostedSurfaceForTesting() }
hostedView.frame = contentView.bounds
hostedView.autoresizingMask = [.width, .height]
contentView.addSubview(hostedView)
hostedView.setVisibleInUI(true)

window.makeKeyAndOrderFront(nil)
window.displayIfNeeded()
contentView.layoutSubtreeIfNeeded()
hostedView.layoutSubtreeIfNeeded()
waitForRuntimeSurface(surface, file: #filePath, line: #line)
guard surface.surface != nil else { return }
XCTAssertTrue(
waitUntil(timeout: 5.0) { surface.isRendererPresented },
"Expected the visible renderer to present before testing the reveal policy"
)

surface.resetDebugForceRefreshCount()
if GhosttySurfaceScrollView.shouldScheduleVisibilityRevealRefresh(
rendererPresented: surface.isRendererPresented
) {
hostedView.scheduleVisibilityRevealRefresh(transition: .reveal)
}
XCTAssertFalse(
hostedView.hasVisibilityRevealRefreshScheduled,
"A currently presented renderer must not enter the deferred reveal path"
)
drainMainQueue()
XCTAssertEqual(surface.debugForceRefreshCount(), 0)
#else
throw XCTSkip("Debug-only regression test")
#endif
}

func testVisibilityRestoreRefreshesSurfaceWhileTerminalIsInactive() throws {
#if DEBUG
try assertInactiveVisibilityRestoreRefreshCount(
Expand All @@ -22,22 +76,23 @@ extension TerminalNotificationDirectInteractionTests {
#endif
}

func testWarmVisibilityRestoreSkipsRefreshWhileTerminalIsInactive() throws {
func testWarmVisibilityRestoreRefreshesAfterRendererLossWhileTerminalIsInactive() throws {
#if DEBUG
try assertInactiveVisibilityRestoreRefreshCount(
presentedFrameBeforeReveal: true,
expected: 0,
"A renderer that already presented a frame keeps it across the hide; revealing it must not force a blocking redraw"
expected: 1,
"A historical frame must not suppress the reveal redraw after the renderer is no longer presented"
)
#else
throw XCTSkip("Debug-only regression test")
#endif
}

#if DEBUG
/// Whether a reveal forces a redraw depends on whether the renderer has
/// presented a frame (#14044). The test pins that state while the portal
/// is hidden instead of inheriting whatever the GPU presented during setup.
/// The reveal fallback depends on whether the renderer is currently
/// presented, rather than on whether it presented a historical frame
/// (#14044). The test pins that historical state while the portal is hidden
/// instead of inheriting whatever the GPU presented during setup.
private func assertInactiveVisibilityRestoreRefreshCount(
presentedFrameBeforeReveal: Bool,
expected: Int,
Expand Down Expand Up @@ -83,25 +138,9 @@ extension TerminalNotificationDirectInteractionTests {
surface.setRendererPresentedFrameForTesting(presentedFrameBeforeReveal)
surface.resetDebugForceRefreshCount()
hostedView.setVisibleInUI(true)
if expected == 0 {
// The deferred refresh re-checks the presented frame, so a wrongly
// scheduled one would not show up in the refresh count below.
XCTAssertFalse(
hostedView.hasVisibilityRevealRefreshScheduled,
"A warm reveal must not schedule a deferred refresh",
file: file,
line: line
)
}
drainMainQueue()
if expected > 0 {
// The reveal redraw runs on a later main-queue turn; wait for it.
_ = waitUntil(timeout: 2.0) { surface.debugForceRefreshCount() >= expected }
} else {
// Give a wrongly scheduled deferred redraw the same turns to land.
RunLoop.current.run(until: Date().addingTimeInterval(0.1))
drainMainQueue()
}
// The reveal redraw runs on a later main-queue turn; wait for it.
_ = waitUntil(timeout: 2.0) { surface.debugForceRefreshCount() >= expected }

XCTAssertEqual(surface.debugForceRefreshCount(), expected, message, file: file, line: line)
}
Expand Down
Loading