Fix tmux resize/layout redraws in embedded terminal surfaces#3120
Fix tmux resize/layout redraws in embedded terminal surfaces#3120austinywang wants to merge 2 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR fixes tmux pane resize and layout rendering errors in cmux by introducing a custom Metal layer that validates and prevents stale IOSurface display, triggering immediate surface refresh on resize operations, and adding key-event-driven refresh behavior. A regression test validates pixel-level visual stability across tmux layout transformations. Changes
Sequence Diagram(s)sequenceDiagram
participant View as GhosttyNSView
participant Layer as GhosttyMetalLayer
participant Surface as TerminalSurface
participant Ghostty as Ghostty Engine
rect rgba(100, 200, 100, 0.5)
Note over View,Ghostty: Resize/Scale Path
View->>Surface: updateSize(newSize)
Surface->>Ghostty: ghostty_surface_refresh()
activate Ghostty
Ghostty->>Ghostty: Render frame to IOSurface
deactivate Ghostty
Ghostty-->>Layer: IOSurface contents
Layer->>Layer: Validate dimensions match scaled bounds
alt Dimensions valid
Layer->>View: Accept IOSurface
else Dimensions mismatch
Layer->>View: Reject (prevent stale content)
end
end
rect rgba(100, 150, 200, 0.5)
Note over View,Surface: Key Event Path
View->>View: keyDown event (cmd/nav/edit)
View->>Surface: forceRefresh()
Surface->>Ghostty: ghostty_surface_refresh()
activate Ghostty
Ghostty->>Layer: Updated IOSurface
deactivate Ghostty
Layer->>View: Render validated content
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR addresses tmux resize/layout visual stale-state bugs in the embedded terminal by: (1) overriding
Confidence Score: 3/5Needs the thread-safety fix in The P1 data race in the Sources/GhosttyTerminalView.swift — specifically the Important Files Changed
Sequence DiagramsequenceDiagram
participant Main as Main Thread
participant Render as Render/CA Thread
participant GML as GhosttyMetalLayer
participant Ghost as Ghostty Surface
Main->>GML: makeBackingLayer() → GhosttyMetalLayer (contentsGravity=.topLeft)
Main->>Ghost: updateSize() → ghostty_surface_set_size()
Main->>Ghost: ghostty_surface_refresh(surface) [NEW]
Render->>GML: nextDrawable() [lock acquired]
Render->>Ghost: render frame into IOSurface
Render->>GML: contents = newIOSurface [NEW override]
GML->>GML: shouldAccept(): reads self.bounds ⚠️ (no lock)
alt size matches (±1px)
GML->>GML: super.contents = newIOSurface
else size mismatch (stale resize frame)
GML->>GML: drop — return without setting
end
Note over Main,Ghost: keyDown ctrlCommand or tmux key
Main->>Main: refreshSurfaceAfterCommandIfNeeded() [NEW]
Main->>Ghost: forceRefresh(reason:) [rate-limited 50ms]
Reviews (1): Last reviewed commit: "Fix tmux resize/layout redraws in termin..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| 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 | ||
| } |
There was a problem hiding this comment.
bounds/contentsScale in contents setter
shouldAccept reads self.bounds and self.contentsScale inside the CALayer.contents setter. The whole point of this override is to intercept IOSurface assignments from the Ghostty renderer, which sets contents on 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 existing NSLock in nextDrawable() 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 shouldAccept can compare against a stable copy:
// store alongside drawableCount:
private var expectedPixelSize: CGSize = .zero
func updateExpectedSize(bounds: CGRect, scale: CGFloat) {
lock.lock()
defer { lock.unlock() }
let s = max(scale, 1.0)
expectedPixelSize = CGSize(
width: (bounds.width * s).rounded(.toNearestOrAwayFromZero),
height: (bounds.height * s).rounded(.toNearestOrAwayFromZero)
)
}
private func shouldAccept(contents newValue: Any?) -> Bool {
// ...
lock.lock()
let expected = expectedPixelSize
lock.unlock()
let ew = Int(expected.width), eh = Int(expected.height)
guard ew > 0, eh > 0 else { return true }
return abs(actualWidth - ew) <= 1 && abs(actualHeight - eh) <= 1
}| // 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 |
There was a problem hiding this comment.
ghostty_surface_refresh fires on scale-only changes
The refresh nudge was added to fix stale tmux borders after resize, but because it sits after the combined guard scaleChanged || sizeChanged check it also fires on content-scale-only changes — e.g., when the window moves between Retina and non-Retina displays. On a DPI-only change, ghostty_surface_set_size is skipped, so ghostty_surface_refresh is called immediately after ghostty_surface_set_content_scale before the renderer has had a chance to re-layout the grid at the new scale. This is likely harmless in practice but produces a spurious mid-transition frame and the comment only mentions the tmux resize scenario. Consider gating the call under the if sizeChanged block:
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![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 6047-6060: refreshSurfaceAfterCommandIfNeeded currently drops any
request that arrives within 50ms of the last call; instead implement a trailing
refresh so rapid calls are coalesced (throttle with a trailing invocation).
Modify refreshSurfaceAfterCommandIfNeeded to keep the existing 50ms early-return
but, when a call is suppressed, schedule a single delayed refresh (e.g., via
DispatchQueue.main.asyncAfter or a stored DispatchWorkItem) to run after the
remaining throttle interval and call terminalSurface.forceRefresh(reason:),
canceling/rescheduling that work item on subsequent suppressed calls; use
lastCommandRefreshAt and terminalSurface.forceRefresh(reason:) as the reference
points and store a small optional scheduledWorkItem property to manage
cancellation/rescheduling.
In `@tests/test_issue_3118_tmux_resize_layout.py`:
- Around line 17-23: The tests start per-test tmux servers running long-lived
sleep processes but never reliably kill them; add a helper function named
_kill_tmux_server(socket_name: str) that invokes tmux -L <socket_name>
kill-server (suppressing stdout/stderr and not raising on failure) and call this
helper from each test's teardown/finally path where tmux sessions are created
(reference the socket name variables used when launching tmux in the tests) so
the tmux server is always cleaned up even on failures; ensure the helper is
defined at module scope and invoked in both places that launch tmux sessions
(the two blocks indicated around lines ~349-405 as well as the initial case).
- Around line 253-259: Update the tmux options in the common list so the
display-time is a short finite timeout instead of 0: replace the entry that sets
"display-time 0" with one that sets "display-time 50" (use the same tmux
variable and session_ref references already in the list) to ensure transient
tmux messages expire before snapshots are taken.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 58673a75-be18-4c33-8791-bcbf28dc5f9d
📒 Files selected for processing (2)
Sources/GhosttyTerminalView.swifttests/test_issue_3118_tmux_resize_layout.py
| 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 { | ||
| return | ||
| } | ||
| lastCommandRefreshAt = now | ||
| terminalSurface.forceRefresh(reason: reason) | ||
| } |
There was a problem hiding this comment.
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
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyTerminalView.swift` around lines 6047 - 6060,
refreshSurfaceAfterCommandIfNeeded currently drops any request that arrives
within 50ms of the last call; instead implement a trailing refresh so rapid
calls are coalesced (throttle with a trailing invocation). Modify
refreshSurfaceAfterCommandIfNeeded to keep the existing 50ms early-return but,
when a call is suppressed, schedule a single delayed refresh (e.g., via
DispatchQueue.main.asyncAfter or a stored DispatchWorkItem) to run after the
remaining throttle interval and call terminalSurface.forceRefresh(reason:),
canceling/rescheduling that work item on subsequent suppressed calls; use
lastCommandRefreshAt and terminalSurface.forceRefresh(reason:) as the reference
points and store a small optional scheduledWorkItem property to manage
cancellation/rescheduling.
| import os | ||
| import shlex | ||
| import struct | ||
| import sys | ||
| import time | ||
| import zlib | ||
| from pathlib import Path |
There was a problem hiding this comment.
Clean up the per-test tmux servers.
Both cases launch tmux sessions whose panes run sleep 100000, but there is no teardown path. A failure before process/app cleanup can leave tmux servers and child sleeps behind.
Proposed fix
import os
import shlex
+import subprocess
import struct
import sys
import time
import zlib
@@
def _run_layout_roundtrip_case(c: cmux, token: str) -> None:
panel_id = _new_workspace_with_panel(c)
socket_name = f"cmux3118-layout-{token}"
session_name = f"cmux3118layout{token}"
- _launch_tmux(
- c,
- panel_id,
- socket_name=socket_name,
- session_name=session_name,
- pane_scripts=[_pane_script("A"), _pane_script("B"), _pane_script("C")],
- setup_commands=[
- f"tmux -L {shlex.quote(socket_name)} -f /dev/null split-window -h -t {shlex.quote(session_name)}:0 {shlex.quote(_pane_script('B'))}",
- f"tmux -L {shlex.quote(socket_name)} -f /dev/null split-window -v -t {shlex.quote(session_name)}:0.0 {shlex.quote(_pane_script('C'))}",
- f"tmux -L {shlex.quote(socket_name)} -f /dev/null select-layout -t {shlex.quote(session_name)}:0 tiled",
- f"tmux -L {shlex.quote(socket_name)} -f /dev/null select-pane -t {shlex.quote(session_name)}:0.0",
- ],
- )
-
- _wait_for_terminal_focus(c, panel_id)
- time.sleep(0.5)
-
- _assert_roundtrip_visual_stability(
- c,
- panel_id,
- label="tmux_layout",
- apply_change=lambda: _prefixed_shortcut(c, "opt+1"),
- apply_revert=lambda: _prefixed_shortcut(c, "opt+5"),
- )
+ try:
+ _launch_tmux(
+ c,
+ panel_id,
+ socket_name=socket_name,
+ session_name=session_name,
+ pane_scripts=[_pane_script("A"), _pane_script("B"), _pane_script("C")],
+ setup_commands=[
+ f"tmux -L {shlex.quote(socket_name)} -f /dev/null split-window -h -t {shlex.quote(session_name)}:0 {shlex.quote(_pane_script('B'))}",
+ f"tmux -L {shlex.quote(socket_name)} -f /dev/null split-window -v -t {shlex.quote(session_name)}:0.0 {shlex.quote(_pane_script('C'))}",
+ f"tmux -L {shlex.quote(socket_name)} -f /dev/null select-layout -t {shlex.quote(session_name)}:0 tiled",
+ f"tmux -L {shlex.quote(socket_name)} -f /dev/null select-pane -t {shlex.quote(session_name)}:0.0",
+ ],
+ )
+
+ _wait_for_terminal_focus(c, panel_id)
+ time.sleep(0.5)
+
+ _assert_roundtrip_visual_stability(
+ c,
+ panel_id,
+ label="tmux_layout",
+ apply_change=lambda: _prefixed_shortcut(c, "opt+1"),
+ apply_revert=lambda: _prefixed_shortcut(c, "opt+5"),
+ )
+ finally:
+ _kill_tmux_server(socket_name)
@@
def _run_resize_roundtrip_case(c: cmux, token: str) -> None:
panel_id = _new_workspace_with_panel(c)
socket_name = f"cmux3118-resize-{token}"
session_name = f"cmux3118resize{token}"
- _launch_tmux(
- c,
- panel_id,
- socket_name=socket_name,
- session_name=session_name,
- pane_scripts=[_pane_script("L"), _pane_script("R")],
- setup_commands=[
- f"tmux -L {shlex.quote(socket_name)} -f /dev/null split-window -h -t {shlex.quote(session_name)}:0 {shlex.quote(_pane_script('R'))}",
- f"tmux -L {shlex.quote(socket_name)} -f /dev/null select-pane -t {shlex.quote(session_name)}:0.1",
- ],
- )
-
- _wait_for_terminal_focus(c, panel_id)
- time.sleep(0.5)
-
- _assert_roundtrip_visual_stability(
- c,
- panel_id,
- label="tmux_resize",
- apply_change=lambda: _prefixed_shortcut(c, "ctrl+left"),
- apply_revert=lambda: _prefixed_shortcut(c, "ctrl+right"),
- )
+ try:
+ _launch_tmux(
+ c,
+ panel_id,
+ socket_name=socket_name,
+ session_name=session_name,
+ pane_scripts=[_pane_script("L"), _pane_script("R")],
+ setup_commands=[
+ f"tmux -L {shlex.quote(socket_name)} -f /dev/null split-window -h -t {shlex.quote(session_name)}:0 {shlex.quote(_pane_script('R'))}",
+ f"tmux -L {shlex.quote(socket_name)} -f /dev/null select-pane -t {shlex.quote(session_name)}:0.1",
+ ],
+ )
+
+ _wait_for_terminal_focus(c, panel_id)
+ time.sleep(0.5)
+
+ _assert_roundtrip_visual_stability(
+ c,
+ panel_id,
+ label="tmux_resize",
+ apply_change=lambda: _prefixed_shortcut(c, "ctrl+left"),
+ apply_revert=lambda: _prefixed_shortcut(c, "ctrl+right"),
+ )
+ finally:
+ _kill_tmux_server(socket_name)Add this helper outside the selected range:
def _kill_tmux_server(socket_name: str) -> None:
subprocess.run(
["tmux", "-L", socket_name, "kill-server"],
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
check=False,
)Also applies to: 349-405
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_issue_3118_tmux_resize_layout.py` around lines 17 - 23, The tests
start per-test tmux servers running long-lived sleep processes but never
reliably kill them; add a helper function named _kill_tmux_server(socket_name:
str) that invokes tmux -L <socket_name> kill-server (suppressing stdout/stderr
and not raising on failure) and call this helper from each test's
teardown/finally path where tmux sessions are created (reference the socket name
variables used when launching tmux in the tests) so the tmux server is always
cleaned up even on failures; ensure the helper is defined at module scope and
invoked in both places that launch tmux sessions (the two blocks indicated
around lines ~349-405 as well as the initial case).
| common = [ | ||
| f"{tmux} set-option -t {session_ref} status off", | ||
| f"{tmux} set-option -t {session_ref} display-time 0", | ||
| f"{tmux} set-option -t {session_ref} bell-action none", | ||
| f"{tmux} set-option -t {session_ref} pane-border-style fg=white", | ||
| f"{tmux} set-option -t {session_ref} pane-active-border-style fg=white", | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the test file to understand the context
head -n 300 tests/test_issue_3118_tmux_resize_layout.py | tail -n 100Repository: manaflow-ai/cmux
Length of output: 3468
🏁 Script executed:
# Check if the file exists and get its size
wc -l tests/test_issue_3118_tmux_resize_layout.pyRepository: manaflow-ai/cmux
Length of output: 107
🏁 Script executed:
# Look for the specific lines mentioned
sed -n '240,280p' tests/test_issue_3118_tmux_resize_layout.pyRepository: manaflow-ai/cmux
Length of output: 1436
🏁 Script executed:
# Search for settle delay or wait patterns in the test
rg -n "sleep|time|settle|wait" tests/test_issue_3118_tmux_resize_layout.py | head -20Repository: manaflow-ai/cmux
Length of output: 1158
🏁 Script executed:
# Look at the overall test structure - function names and key logic
rg -n "def |class |display.time|screenshot|snapshot|diff" tests/test_issue_3118_tmux_resize_layout.pyRepository: manaflow-ai/cmux
Length of output: 2468
🌐 Web query:
tmux display-time option meaning documentation
💡 Result:
The tmux display-time option sets the duration in milliseconds for which status line messages and other on-screen indicators (such as those from display-message or display-panes commands) are displayed. The default value is 750 milliseconds. If set to 0, messages and indicators remain displayed until a key is pressed. This is a session option, configurable with commands like set -g display-time 4000 in ~/.tmux.conf or via the command prompt.
Citations:
- 1: https://mytmux.com/configuration/options/
- 2: https://www.mintlify.com/tmux/tmux/reference/options-reference
- 3: https://www.unix.com/man_page/linux/1/tmux
- 4: https://linuxcommand.org/lc3_man_pages/tmux1.html
- 5: https://github.com/tmux/tmux/wiki/Getting-Started
- 6: https://github.com/tmux/tmux/blob/master/CHANGES
🏁 Script executed:
# Check tmux documentation or man page references in the repository
rg -i "display.time" --type py --type txtRepository: manaflow-ai/cmux
Length of output: 165
🏁 Script executed:
# Look at the test flow more carefully to understand when snapshots are taken
# relative to the settle delays and display-time
sed -n '274,340p' tests/test_issue_3118_tmux_resize_layout.pyRepository: manaflow-ai/cmux
Length of output: 2790
🏁 Script executed:
# Check what happens after the apply_change - how quickly are snapshots taken?
sed -n '300,330p' tests/test_issue_3118_tmux_resize_layout.pyRepository: manaflow-ai/cmux
Length of output: 1483
Use a finite display-time to prevent tmux messages from polluting snapshots.
Line 255 sets display-time 0, which makes tmux status messages and indicators persist on screen until a key is pressed. This can pollute the final snapshot after layout/resize operations and cause the pixel-diff assertions to fail incorrectly. Use a short finite timeout like 50ms, which will expire well within the existing settle delays (150ms before snapshots, 350ms after shortcuts) before any snapshot is captured.
Proposed fix
common = [
f"{tmux} set-option -t {session_ref} status off",
- f"{tmux} set-option -t {session_ref} display-time 0",
+ f"{tmux} set-option -t {session_ref} display-time 50",
f"{tmux} set-option -t {session_ref} bell-action none",
f"{tmux} set-option -t {session_ref} pane-border-style fg=white",
f"{tmux} set-option -t {session_ref} pane-active-border-style fg=white",
]📝 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.
| common = [ | |
| f"{tmux} set-option -t {session_ref} status off", | |
| f"{tmux} set-option -t {session_ref} display-time 0", | |
| f"{tmux} set-option -t {session_ref} bell-action none", | |
| f"{tmux} set-option -t {session_ref} pane-border-style fg=white", | |
| f"{tmux} set-option -t {session_ref} pane-active-border-style fg=white", | |
| ] | |
| common = [ | |
| f"{tmux} set-option -t {session_ref} status off", | |
| f"{tmux} set-option -t {session_ref} display-time 50", | |
| f"{tmux} set-option -t {session_ref} bell-action none", | |
| f"{tmux} set-option -t {session_ref} pane-border-style fg=white", | |
| f"{tmux} set-option -t {session_ref} pane-active-border-style fg=white", | |
| ] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_issue_3118_tmux_resize_layout.py` around lines 253 - 259, Update
the tmux options in the common list so the display-time is a short finite
timeout instead of 0: replace the entry that sets "display-time 0" with one that
sets "display-time 50" (use the same tmux variable and session_ref references
already in the list) to ensure transient tmux messages expire before snapshots
are taken.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/GhosttyTerminalView.swift">
<violation number="1" location="Sources/GhosttyTerminalView.swift:6055">
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.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| isVisibleInUI else { return } | ||
|
|
||
| let now = CACurrentMediaTime() | ||
| if now - lastCommandRefreshAt < 0.05 { |
There was a problem hiding this comment.
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
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/GhosttyTerminalView.swift, line 6055:
<comment>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.</comment>
<file context>
@@ -5988,6 +6019,46 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
+ isVisibleInUI else { return }
+
+ let now = CACurrentMediaTime()
+ if now - lastCommandRefreshAt < 0.05 {
+ return
+ }
</file context>
1 participant
Summary
Testing
Fixes #3118
Note
Medium Risk
Changes Metal layer content handling and forces extra
ghostty_surface_refresh/forceRefreshcalls during resize and certain key events, which could affect rendering correctness and performance in the embedded terminal path.Overview
Fixes tmux pane resize/layout redraw glitches in embedded terminal surfaces by forcing immediate Ghostty refreshes after real pixel-size changes and after non-text command/navigation key events (with a small time-based throttle).
Hardens the backing
CAMetalLayerby switching to a customGhosttyMetalLayerthat rejects wrong-sized IOSurfacecontentsupdates during resize churn and pins prior contents to.topLeftto avoid stretch artifacts.Adds an end-to-end regression test (
test_issue_3118_tmux_resize_layout.py) that drives tmux layout/resize round-trips and asserts visual stability by diffing captured panel PNGs.Reviewed by Cursor Bugbot for commit c3ebb52. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Bug Fixes
Tests