Skip to content
Merged
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
29 changes: 25 additions & 4 deletions Sources/GhosttyTerminalView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8573,6 +8573,23 @@ struct GhosttyTerminalView: NSViewRepresentable {
return !hostedViewHasSuperview
}

private static func synchronizePortalGeometry(
for host: HostContainerView,
coordinator: Coordinator
) {
let geometryRevision = host.geometryRevision
guard coordinator.lastSynchronizedHostGeometryRevision != geometryRevision else { return }
coordinator.lastSynchronizedHostGeometryRevision = geometryRevision
if host.inLiveResize || host.window?.inLiveResize == true {
TerminalWindowPortalRegistry.synchronizeForAnchor(host)
return
Comment on lines +8583 to +8585

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid immediate portal synchronization during live-resize churn

Line 8584 still calls the immediate synchronizeForAnchor path during live resize. That can re-enter AppKit layout/resize work and reintroduce the spinner-hang path this PR is targeting.

💡 Proposed fix
-        if host.inLiveResize || host.window?.inLiveResize == true {
-            TerminalWindowPortalRegistry.synchronizeForAnchor(host)
-            return
-        }
-        // Avoid synchronizing the terminal portal while AppKit is still inside
-        // the current layout turn. Re-entrant syncs here can wedge window resize
-        // handling and leave the app spinning on the wait cursor.
-        TerminalWindowPortalRegistry.scheduleExternalGeometrySynchronizeForAllWindows()
+        // Avoid re-entrant portal sync during AppKit layout/live-resize churn.
+        // Keep this deferred so geometry reconciliation happens after the current turn.
+        TerminalWindowPortalRegistry.scheduleExternalGeometrySynchronizeForAllWindows()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/GhosttyTerminalView.swift` around lines 8583 - 8585, The code
currently calls TerminalWindowPortalRegistry.synchronizeForAnchor(host)
immediately when host.inLiveResize or host.window?.inLiveResize == true, which
can re-enter AppKit during live-resize; change this to defer synchronization
instead of calling synchronizeForAnchor synchronously — e.g., skip the immediate
call when host.inLiveResize is true and schedule a deferred synchronization for
that host (using a main-queue async/next-runloop dispatch or a window
live-resize end observer) so
TerminalWindowPortalRegistry.synchronizeForAnchor(host) runs after live-resize
finishes.

}
// Avoid synchronizing the terminal portal while AppKit is still inside
// the current layout turn. Re-entrant syncs here can wedge window resize
// handling and leave the app spinning on the wait cursor.
TerminalWindowPortalRegistry.scheduleExternalGeometrySynchronizeForAllWindows()
Comment on lines +8582 to +8590

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid scheduling external sync for detached hosts

scheduleDeferredPortalGeometrySynchronize now always queues scheduleExternalGeometrySynchronizeForAllWindows() after a geometry revision bump, but unlike the previous synchronizeForAnchor(host) path it no longer short-circuits when host.window == nil. During SwiftUI reparent/detach churn (viewDidMoveToWindow/viewDidMoveToSuperview), this can trigger unnecessary full-portal syncs across every window, including reconcileGeometryNow()/refreshSurfaceNow() on unrelated terminals, which is a user-visible performance regression under window/workspace churn.

Useful? React with 👍 / 👎.

}

func makeNSView(context: Context) -> NSView {
let container = HostContainerView()
container.wantsLayer = false
Expand Down Expand Up @@ -8732,8 +8749,10 @@ struct GhosttyTerminalView: NSViewRepresentable {
hostedView.setActive(coordinator.desiredIsActive)
hostedView.setNotificationRing(visible: coordinator.desiredShowsUnreadNotificationRing)
}
TerminalWindowPortalRegistry.synchronizeForAnchor(host)
coordinator.lastSynchronizedHostGeometryRevision = host.geometryRevision
Self.synchronizePortalGeometry(
for: host,
coordinator: coordinator
)
}

if host.window != nil, hostOwnsPortalNow {
Expand Down Expand Up @@ -8768,8 +8787,10 @@ struct GhosttyTerminalView: NSViewRepresentable {
coordinator.lastBoundHostId = hostId
coordinator.lastSynchronizedHostGeometryRevision = geometryRevision
} else if coordinator.lastSynchronizedHostGeometryRevision != geometryRevision {
TerminalWindowPortalRegistry.synchronizeForAnchor(host)
coordinator.lastSynchronizedHostGeometryRevision = geometryRevision
Self.synchronizePortalGeometry(
for: host,
coordinator: coordinator
)
}
} else if hostOwnsPortalNow {
// Bind is deferred until host moves into a window. Update the
Expand Down
20 changes: 15 additions & 5 deletions Sources/TerminalWindowPortal.swift
Original file line number Diff line number Diff line change
Expand Up @@ -680,10 +680,18 @@ final class WindowTerminalPortal: NSObject {
private func scheduleExternalGeometrySynchronize() {
guard !hasExternalGeometrySyncScheduled else { return }
hasExternalGeometrySyncScheduled = true
let requiresSettledLayout = !(hostView.inLiveResize || window?.inLiveResize == true)
DispatchQueue.main.async { [weak self] in
guard let self else { return }
self.hasExternalGeometrySyncScheduled = false
self.synchronizeAllEntriesFromExternalGeometryChange()
let performSync = {
self.hasExternalGeometrySyncScheduled = false
self.synchronizeAllEntriesFromExternalGeometryChange()
}
if requiresSettledLayout {
DispatchQueue.main.async(execute: performSync)
} else {
performSync()
}
}
}

Expand Down Expand Up @@ -1785,9 +1793,11 @@ enum TerminalWindowPortalRegistry {
guard !Self.hasPendingExternalGeometrySyncForAllWindows else { return }
Self.hasPendingExternalGeometrySyncForAllWindows = true
DispatchQueue.main.async {
Self.hasPendingExternalGeometrySyncForAllWindows = false
for portal in Self.portalsByWindowId.values {
portal.synchronizeAllEntriesFromExternalGeometryChange()
DispatchQueue.main.async {
Self.hasPendingExternalGeometrySyncForAllWindows = false
for portal in Self.portalsByWindowId.values {
portal.synchronizeAllEntriesFromExternalGeometryChange()
}
}
}
}
Expand Down
83 changes: 83 additions & 0 deletions cmuxTests/CmuxWebViewKeyEquivalentTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13068,6 +13068,89 @@ final class TerminalWindowPortalLifecycleTests: XCTestCase {
"The scheduled external geometry sync should move the portal-hosted terminal to the anchor's new window position"
)
}

func testScheduledExternalGeometrySyncWaitsForQueuedLayoutShift() {
let window = NSWindow(
contentRect: NSRect(x: 0, y: 0, width: 700, height: 420),
styleMask: [.titled, .closable],
backing: .buffered,
defer: false
)
defer {
NotificationCenter.default.post(name: NSWindow.willCloseNotification, object: window)
window.orderOut(nil)
}

let surface = TerminalSurface(
tabId: UUID(),
context: GHOSTTY_SURFACE_CONTEXT_SPLIT,
configTemplate: nil,
workingDirectory: nil
)
guard let contentView = window.contentView else {
XCTFail("Expected content view")
return
}

let shiftedContainer = NSView(frame: NSRect(x: 40, y: 60, width: 260, height: 180))
contentView.addSubview(shiftedContainer)
let anchor = NSView(frame: NSRect(x: 0, y: 0, width: 260, height: 180))
shiftedContainer.addSubview(anchor)
let hosted = surface.hostedView
TerminalWindowPortalRegistry.bind(
hostedView: hosted,
to: anchor,
visibleInUI: true,
expectedSurfaceId: surface.id,
expectedGeneration: surface.portalBindingGeneration()
)
TerminalWindowPortalRegistry.synchronizeForAnchor(anchor)

let anchorCenter = NSPoint(x: anchor.bounds.midX, y: anchor.bounds.midY)
let originalWindowPoint = anchor.convert(anchorCenter, to: nil)
let originalAnchorFrameInWindow = anchor.convert(anchor.bounds, to: nil)
XCTAssertNotNil(
TerminalWindowPortalRegistry.terminalViewAtWindowPoint(originalWindowPoint, in: window),
"Initial hit-testing should resolve the portal-hosted terminal at its original window position"
)

TerminalWindowPortalRegistry.scheduleExternalGeometrySynchronizeForAllWindows()
DispatchQueue.main.async {
shiftedContainer.frame.origin.x += 72
contentView.layoutSubtreeIfNeeded()
window.displayIfNeeded()
}

RunLoop.current.run(until: Date().addingTimeInterval(0.05))

let shiftedAnchorFrameInWindow = anchor.convert(anchor.bounds, to: nil)
XCTAssertGreaterThan(
shiftedAnchorFrameInWindow.minX,
originalAnchorFrameInWindow.minX + 1,
"The queued layout shift should move the anchor to the right"
)
XCTAssertGreaterThan(
shiftedAnchorFrameInWindow.maxX,
originalAnchorFrameInWindow.maxX + 1,
"The shifted anchor should expose a new trailing region outside the stale portal frame"
)
let retiredStaleWindowPoint = NSPoint(
x: (originalAnchorFrameInWindow.minX + shiftedAnchorFrameInWindow.minX) / 2,
y: shiftedAnchorFrameInWindow.midY
)
let shiftedWindowPoint = NSPoint(
x: (originalAnchorFrameInWindow.maxX + shiftedAnchorFrameInWindow.maxX) / 2,
y: shiftedAnchorFrameInWindow.midY
)
XCTAssertNil(
TerminalWindowPortalRegistry.terminalViewAtWindowPoint(retiredStaleWindowPoint, in: window),
"The queued external sync should wait until the later layout shift settles, clearing the stale portal location"
)
XCTAssertNotNil(
TerminalWindowPortalRegistry.terminalViewAtWindowPoint(shiftedWindowPoint, in: window),
"The delayed external sync should move the portal-hosted terminal to the queued layout shift position"
)
}
}

@MainActor
Expand Down
Loading