Repository navigation
Fix tmux resize/layout redraws in embedded terminal surfaces #3120
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 |
|---|---|---|
|
|
@@ -3562,6 +3562,14 @@ final class GhosttyMetalLayer: CAMetalLayer { | |
| private var drawableCount: Int = 0 | ||
| private var lastDrawableTime: CFTimeInterval = 0 | ||
|
|
||
| override var contents: Any? { | ||
| get { super.contents } | ||
| set { | ||
| guard shouldAccept(contents: newValue) else { return } | ||
| super.contents = newValue | ||
| } | ||
| } | ||
|
|
||
| func debugStats() -> (count: Int, last: CFTimeInterval) { | ||
| lock.lock() | ||
| defer { lock.unlock() } | ||
|
|
@@ -3575,6 +3583,23 @@ final class GhosttyMetalLayer: CAMetalLayer { | |
| lock.unlock() | ||
| return super.nextDrawable() | ||
| } | ||
|
|
||
| private func shouldAccept(contents newValue: Any?) -> Bool { | ||
| guard let newValue else { return true } | ||
| guard let obj = newValue as AnyObject? else { return true } | ||
| let cf = obj as CFTypeRef | ||
| guard CFGetTypeID(cf) == IOSurfaceGetTypeID() else { return true } | ||
|
|
||
| let surfaceRef = obj as! IOSurfaceRef | ||
| let scale = max(contentsScale, 1.0) | ||
| let expectedWidth = Int((bounds.width * scale).rounded(.toNearestOrAwayFromZero)) | ||
| let expectedHeight = Int((bounds.height * scale).rounded(.toNearestOrAwayFromZero)) | ||
| let actualWidth = Int(IOSurfaceGetWidth(surfaceRef)) | ||
| let actualHeight = Int(IOSurfaceGetHeight(surfaceRef)) | ||
|
|
||
| guard expectedWidth > 0, expectedHeight > 0 else { return true } | ||
| return abs(actualWidth - expectedWidth) <= 1 && abs(actualHeight - expectedHeight) <= 1 | ||
| } | ||
| } | ||
|
|
||
| final class TerminalSurfaceRegistry { | ||
|
|
@@ -4745,7 +4770,9 @@ final class TerminalSurface: Identifiable, ObservableObject { | |
| lastPixelHeight = hpx | ||
| } | ||
|
|
||
| // Let Ghostty continue rendering on its own wakeups for steady-state frames. | ||
| // Resize/reflow-heavy apps like tmux can leave stale pixels visible until a | ||
| // later wakeup if we don't ask Ghostty for a fresh frame immediately. | ||
| ghostty_surface_refresh(surface) | ||
| return true | ||
|
Comment on lines
+4773
to
4776
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.
The refresh nudge was added to fix stale tmux borders after resize, but because it sits after the combined if sizeChanged {
ghostty_surface_set_size(surface, wpx, hpx)
lastPixelWidth = wpx
lastPixelHeight = hpx
// Resize/reflow-heavy apps like tmux can leave stale pixels visible until a
// later wakeup if we don't ask Ghostty for a fresh frame immediately.
ghostty_surface_refresh(surface)
}
return true |
||
| } | ||
|
|
||
|
|
@@ -5465,6 +5492,7 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { | |
| private var pendingSurfaceSize: CGSize? | ||
| private var deferredSurfaceSizeRetryQueued = false | ||
| private var lastDrawableSize: CGSize = .zero | ||
| private var lastCommandRefreshAt: CFTimeInterval = 0 | ||
| private var isFindEscapeSuppressionArmed = false | ||
| #if DEBUG | ||
| private var lastSizeSkipSignature: String? | ||
|
|
@@ -5511,20 +5539,23 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { | |
| } | ||
|
|
||
| override func makeBackingLayer() -> CALayer { | ||
| let metalLayer = CAMetalLayer() | ||
| let metalLayer = GhosttyMetalLayer() | ||
| metalLayer.pixelFormat = .bgra8Unorm | ||
| metalLayer.isOpaque = false | ||
| // framebufferOnly=false lets the macOS compositor read the drawable | ||
| // when blending translucent or blurred window layers. This matches | ||
| // standalone Ghostty's SurfaceView and is required for background-opacity | ||
| // and background-blur to render correctly. | ||
| metalLayer.framebufferOnly = false | ||
| // Match Ghostty's resize behavior so old IOSurface contents stay pinned | ||
| // instead of stretching while the resized grid renders. | ||
| metalLayer.contentsGravity = .topLeft | ||
| return metalLayer | ||
| } | ||
|
|
||
| private func setup() { | ||
| // Only enable our instrumented CAMetalLayer in targeted debug/test scenarios. | ||
| // The lock in GhosttyMetalLayer.nextDrawable() adds overhead we don't want in normal runs. | ||
| // GhosttyMetalLayer validates IOSurface sizing during resize churn and also | ||
| // exposes lightweight drawable stats for debug tooling. | ||
| wantsLayer = true | ||
| layer?.masksToBounds = true | ||
| installEventMonitor() | ||
|
|
@@ -5988,6 +6019,46 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { | |
| return CGSize(width: pointsSize.width * scale, height: pointsSize.height * scale) | ||
| } | ||
|
|
||
| private func shouldRefreshAfterCommandKey(event: NSEvent) -> Bool { | ||
| let flags = event.modifierFlags.intersection(.deviceIndependentFlagsMask) | ||
| if flags.contains(.control) || flags.contains(.option) { | ||
| return true | ||
| } | ||
|
|
||
| switch Int(event.keyCode) { | ||
| case kVK_LeftArrow, | ||
| kVK_RightArrow, | ||
| kVK_UpArrow, | ||
| kVK_DownArrow, | ||
| kVK_Return, | ||
| kVK_Tab, | ||
| kVK_Escape, | ||
| kVK_Delete, | ||
| kVK_Home, | ||
| kVK_End, | ||
| kVK_PageUp, | ||
| kVK_PageDown: | ||
| return true | ||
| default: | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| private func refreshSurfaceAfterCommandIfNeeded(reason: String) { | ||
| guard let terminalSurface, | ||
| window != nil, | ||
| bounds.width > 0, | ||
| bounds.height > 0, | ||
| isVisibleInUI else { return } | ||
|
|
||
| let now = CACurrentMediaTime() | ||
| if now - lastCommandRefreshAt < 0.05 { | ||
|
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. P2: The 50ms throttle currently drops command-triggered refreshes outright. For multi-key tmux command sequences, this can skip the refresh on the actual layout/resize key and leave stale pixels until a later wakeup. Coalesce a trailing refresh instead of returning immediately. Prompt for AI agents |
||
| return | ||
| } | ||
| lastCommandRefreshAt = now | ||
| terminalSurface.forceRefresh(reason: reason) | ||
| } | ||
|
Comment on lines
+6047
to
+6060
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. Coalesce throttled command refreshes instead of dropping them. Line 6055 drops any command refresh inside the 50 ms window. In tmux prefix sequences, the prefix key can trigger the throttle, then the actual layout/resize key (for example an arrow/control-arrow) can arrive inside that window and never get the redraw this PR is trying to guarantee. Keep the throttle, but schedule one trailing refresh. Proposed fix- private var lastCommandRefreshAt: CFTimeInterval = 0
+ private var lastCommandRefreshAt: CFTimeInterval = 0
+ private var pendingCommandRefreshWorkItem: DispatchWorkItem? private func refreshSurfaceAfterCommandIfNeeded(reason: String) {
guard let terminalSurface,
window != nil,
bounds.width > 0,
bounds.height > 0,
isVisibleInUI else { return }
let now = CACurrentMediaTime()
- if now - lastCommandRefreshAt < 0.05 {
+ let minimumInterval: CFTimeInterval = 0.05
+ let elapsed = now - lastCommandRefreshAt
+ if elapsed < minimumInterval {
+ guard pendingCommandRefreshWorkItem == nil else {
+ return
+ }
+ let delay = minimumInterval - elapsed
+ let workItem = DispatchWorkItem { [weak self] in
+ guard let self else { return }
+ self.pendingCommandRefreshWorkItem = nil
+ self.refreshSurfaceAfterCommandIfNeeded(reason: reason)
+ }
+ pendingCommandRefreshWorkItem = workItem
+ DispatchQueue.main.asyncAfter(deadline: .now() + delay, execute: workItem)
return
}
+ pendingCommandRefreshWorkItem?.cancel()
+ pendingCommandRefreshWorkItem = nil
lastCommandRefreshAt = now
terminalSurface.forceRefresh(reason: reason)
}🤖 Prompt for AI Agents |
||
|
|
||
| // Convenience accessor for the ghostty surface | ||
| private var surface: ghostty_surface_t? { | ||
| terminalSurface?.surface | ||
|
|
@@ -6871,7 +6942,10 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { | |
| // If Ghostty handled the key (action/encoding), we're done. | ||
| // If not (e.g. `ignore` keybind), fall through to interpretKeyEvents | ||
| // so the IME gets a chance to process this event. | ||
| if handled { return } | ||
| if handled { | ||
| refreshSurfaceAfterCommandIfNeeded(reason: "keyDown.ctrlCommand") | ||
| return | ||
| } | ||
| } | ||
|
|
||
| let action = event.isARepeat ? GHOSTTY_ACTION_REPEAT : GHOSTTY_ACTION_PRESS | ||
|
|
@@ -7148,6 +7222,14 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { | |
| terminalSurface?.forceRefresh(reason: "keyDown.textInput") | ||
| #if DEBUG | ||
| refreshMs = (ProcessInfo.processInfo.systemUptime - refreshStart) * 1000.0 | ||
| #endif | ||
| } else if shouldRefreshAfterCommandKey(event: translationEvent) { | ||
| #if DEBUG | ||
| let refreshStart = ProcessInfo.processInfo.systemUptime | ||
| #endif | ||
| refreshSurfaceAfterCommandIfNeeded(reason: "keyDown.command") | ||
| #if DEBUG | ||
| refreshMs = (ProcessInfo.processInfo.systemUptime - refreshStart) * 1000.0 | ||
| #endif | ||
| } | ||
|
|
||
|
|
||
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.
bounds/contentsScaleincontentssettershouldAcceptreadsself.boundsandself.contentsScaleinside theCALayer.contentssetter. The whole point of this override is to intercept IOSurface assignments from the Ghostty renderer, which setscontentson the Metal render/commit thread — not on the main thread. Reading CALayer model-layer properties (bounds,contentsScale) from that thread without synchronization is an unsynchronized read that can race with main-thread writes during concurrent resizes, leading to a wrong size comparison or — in pathological cases — a Swift runtime crash. The existingNSLockinnextDrawable()shows the author is already aware of the threading context; the same care is needed here.A safe approach is to snapshot the expected size on the main thread and store it behind the existing lock so
shouldAcceptcan compare against a stable copy: