From cd0b8da82becc73ecea3bb21bb99f67df52d0823 Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Thu, 19 Feb 2026 04:15:50 -0800 Subject: [PATCH] Fix split-close stale-frame stretch and add regression coverage --- Sources/GhosttyTerminalView.swift | 5 ++ Sources/TabManager.swift | 76 ++++++++++++++----- .../MenuKeyEquivalentRoutingUITests.swift | 44 +++++++++++ ghostty | 2 +- 4 files changed, 105 insertions(+), 22 deletions(-) diff --git a/Sources/GhosttyTerminalView.swift b/Sources/GhosttyTerminalView.swift index 5fe29ff4..a5bfca7e 100644 --- a/Sources/GhosttyTerminalView.swift +++ b/Sources/GhosttyTerminalView.swift @@ -3289,6 +3289,7 @@ final class GhosttySurfaceScrollView: NSView { let expectedWidthPx: Int let expectedHeightPx: Int let layerClass: String + let layerContentsGravity: String let layerContentsKey: String var isProbablyBlank: Bool { @@ -3350,6 +3351,7 @@ final class GhosttySurfaceScrollView: NSView { // Prefer the presentation layer to better match what the user sees on screen. let layer = modelLayer.presentation() ?? modelLayer let layerClass = String(describing: type(of: layer)) + let layerContentsGravity = layer.contentsGravity.rawValue let contentsKey = Self.contentsKey(for: layer) let presentationScale = max(1.0, layer.contentsScale) let expectedWidthPx = Int((layer.bounds.width * presentationScale).rounded(.toNearestOrAwayFromZero)) @@ -3370,6 +3372,7 @@ final class GhosttySurfaceScrollView: NSView { expectedWidthPx: expectedWidthPx, expectedHeightPx: expectedHeightPx, layerClass: layerClass, + layerContentsGravity: layerContentsGravity, layerContentsKey: contentsKey ) } @@ -3395,6 +3398,7 @@ final class GhosttySurfaceScrollView: NSView { expectedWidthPx: expectedWidthPx, expectedHeightPx: expectedHeightPx, layerClass: layerClass, + layerContentsGravity: layerContentsGravity, layerContentsKey: contentsKey ) } @@ -3480,6 +3484,7 @@ final class GhosttySurfaceScrollView: NSView { expectedWidthPx: expectedWidthPx, expectedHeightPx: expectedHeightPx, layerClass: layerClass, + layerContentsGravity: layerContentsGravity, layerContentsKey: contentsKey ) } diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index e33fb425..f3c43522 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -174,33 +174,37 @@ fileprivate func cmuxVsyncIOSurfaceTimelineCallback( for t in st.targets { guard let s = t.sample() else { continue } + let iosW = s.iosurfaceWidthPx + let iosH = s.iosurfaceHeightPx + let expW = s.expectedWidthPx + let expH = s.expectedHeightPx + let gravity = s.layerContentsGravity + let hasDimensions = iosW > 0 && iosH > 0 && expW > 0 && expH > 0 + let dw = hasDimensions ? abs(iosW - expW) : 0 + let dh = hasDimensions ? abs(iosH - expH) : 0 + let hasSizeMismatch = hasDimensions && (dw > 2 || dh > 2) + let stretchRisk = (gravity == CALayerContentsGravity.resize.rawValue) + // Ignore setup/warmup frames before the close action. We only care about // regressions that happen at/after the close mutation. if st.firstBlank == nil, st.framesWritten >= st.closeFrame, s.isProbablyBlank { st.firstBlank = (label: t.label, frame: st.framesWritten) } - let iosW = s.iosurfaceWidthPx - let iosH = s.iosurfaceHeightPx - let expW = s.expectedWidthPx - let expH = s.expectedHeightPx if st.firstSizeMismatch == nil, st.framesWritten >= st.closeFrame, - iosW > 0, iosH > 0, expW > 0, expH > 0 { - let dw = abs(iosW - expW) - let dh = abs(iosH - expH) - if dw > 2 || dh > 2 { - st.firstSizeMismatch = ( - label: t.label, - frame: st.framesWritten, - ios: "\(iosW)x\(iosH)", - expected: "\(expW)x\(expH)" - ) - } + stretchRisk, + hasSizeMismatch { + st.firstSizeMismatch = ( + label: t.label, + frame: st.framesWritten, + ios: "\(iosW)x\(iosH)", + expected: "\(expW)x\(expH)" + ) } if st.trace.count < 200 { - st.trace.append("\(st.framesWritten):\(t.label):blank=\(s.isProbablyBlank ? 1 : 0):ios=\(iosW)x\(iosH):exp=\(expW)x\(expH):key=\(s.layerContentsKey)") + st.trace.append("\(st.framesWritten):\(t.label):blank=\(s.isProbablyBlank ? 1 : 0):ios=\(iosW)x\(iosH):exp=\(expW)x\(expH):gravity=\(gravity):key=\(s.layerContentsKey)") } } @@ -1720,13 +1724,20 @@ class TabManager: ObservableObject { tab.closePanel(pid, force: true) } - // Create the 2x2 grid. The bug is order-dependent, so support multiple patterns. + // Create the repro layout. Most patterns use a 2x2 grid, but keep a single-split + // variant for the exact "close right in a horizontal pair" user report. let topLeftId = topLeftPanelId let topRight: TerminalPanel - let bottomLeft: TerminalPanel - let bottomRight: TerminalPanel + var bottomLeft: TerminalPanel? + var bottomRight: TerminalPanel? switch pattern { + case "close_right_single": + guard let tr = tab.newTerminalSplit(from: topLeftId, orientation: .horizontal) else { + writeSplitCloseRightTestData(["setupError": "Failed to split right from top-left (iteration \(i))"], at: path) + return + } + topRight = tr case "close_right_lrtd", "close_right_lrtd_bottom_first", "close_right_bottom_first", "close_right_lrtd_unfocused": // User repro: split left/right first, then split top/down in each column. guard let tr = tab.newTerminalSplit(from: topLeftId, orientation: .horizontal) else { @@ -1769,9 +1780,15 @@ class TabManager: ObservableObject { // Fill left panes with visible content. sendText(topLeftId, "printf '\\033[2J\\033[H'; for i in {1..200}; do echo CMUX_SPLIT_TOPLEFT_\(i); done; printf '\\033[HCMUX_MARKER_TOPLEFT\\n'\r") - sendText(bottomLeft.id, "printf '\\033[2J\\033[H'; for i in {1..200}; do echo CMUX_SPLIT_BOTTOMLEFT_\(i); done; printf '\\033[HCMUX_MARKER_BOTTOMLEFT\\n'\r") sendText(topRight.id, "printf '\\033[2J\\033[H'; for i in {1..200}; do echo CMUX_SPLIT_TOPRIGHT_\(i); done; printf '\\033[HCMUX_MARKER_TOPRIGHT\\n'\r") - sendText(bottomRight.id, "printf '\\033[2J\\033[H'; for i in {1..200}; do echo CMUX_SPLIT_BOTTOMRIGHT_\(i); done; printf '\\033[HCMUX_MARKER_BOTTOMRIGHT\\n'\r") + if let bottomLeft { + sendText(bottomLeft.id, "printf '\\033[2J\\033[H'; for i in {1..200}; do echo CMUX_SPLIT_BOTTOMLEFT_\(i); done; printf '\\033[HCMUX_MARKER_BOTTOMLEFT\\n'\r") + } + if let bottomRight { + sendText(bottomRight.id, "printf '\\033[2J\\033[H'; for i in {1..200}; do echo CMUX_SPLIT_BOTTOMRIGHT_\(i); done; printf '\\033[HCMUX_MARKER_BOTTOMRIGHT\\n'\r") + } + // Give shell output a moment to paint before we start the close timeline. + try? await Task.sleep(nanoseconds: 180_000_000) let desiredFrames = max(16, min(burstFrames, 60)) let closeFrame = min(6, max(1, desiredFrames / 4)) @@ -1781,7 +1798,16 @@ class TabManager: ObservableObject { var closeOrder = "" let actions: [(frame: Int, action: () -> Void)] = { switch pattern { + case "close_right_single": + closeOrder = "TR_ONLY" + return [ + (frame: closeFrame, action: { + tab.focusPanel(topRight.id) + tab.closePanel(topRight.id, force: true) + }), + ] case "close_bottom": + guard let bottomRight, let bottomLeft else { return [] } closeOrder = "BR_THEN_BL" return [ (frame: closeFrame, action: { @@ -1794,6 +1820,7 @@ class TabManager: ObservableObject { }), ] case "close_right_lrtd_bottom_first", "close_right_bottom_first": + guard let bottomRight else { return [] } closeOrder = "BR_THEN_TR" return [ (frame: closeFrame, action: { @@ -1806,6 +1833,7 @@ class TabManager: ObservableObject { }), ] case "close_right_lrtd_unfocused": + guard let bottomRight else { return [] } closeOrder = "TR_THEN_BR_UNFOCUSED" return [ (frame: closeFrame, action: { @@ -1816,6 +1844,7 @@ class TabManager: ObservableObject { }), ] default: + guard let bottomRight else { return [] } closeOrder = "TR_THEN_BR" return [ (frame: closeFrame, action: { @@ -1832,6 +1861,10 @@ class TabManager: ObservableObject { let targets: [(label: String, view: GhosttySurfaceScrollView)] = { switch pattern { + case "close_right_single": + return [ + ("TL", tab.terminalPanel(for: topLeftId)!.surface.hostedView), + ] case "close_bottom": return [ ("TL", tab.terminalPanel(for: topLeftId)!.surface.hostedView), @@ -1843,6 +1876,7 @@ class TabManager: ObservableObject { ("TL", tab.terminalPanel(for: topLeftId)!.surface.hostedView), ] default: + guard let bottomLeft else { return [] } return [ ("TL", tab.terminalPanel(for: topLeftId)!.surface.hostedView), ("BL", bottomLeft.surface.hostedView), diff --git a/cmuxUITests/MenuKeyEquivalentRoutingUITests.swift b/cmuxUITests/MenuKeyEquivalentRoutingUITests.swift index ef2a79f1..1f249d61 100644 --- a/cmuxUITests/MenuKeyEquivalentRoutingUITests.swift +++ b/cmuxUITests/MenuKeyEquivalentRoutingUITests.swift @@ -314,6 +314,50 @@ final class SplitCloseRightBlankRegressionUITests: XCTestCase { ) } + func testReproStretchAfterClosingSingleRightSplit() { + let app = XCUIApplication() + app.launchEnvironment["CMUX_UI_TEST_DIAGNOSTICS_PATH"] = diagnosticsPath + app.launchEnvironment["CMUX_UI_TEST_SPLIT_CLOSE_RIGHT_SETUP"] = "1" + app.launchEnvironment["CMUX_UI_TEST_SPLIT_CLOSE_RIGHT_PATH"] = dataPath + app.launchEnvironment["CMUX_UI_TEST_SPLIT_CLOSE_RIGHT_VISUAL"] = "1" + app.launchEnvironment["CMUX_UI_TEST_SPLIT_CLOSE_RIGHT_ITERATIONS"] = "16" + app.launchEnvironment["CMUX_UI_TEST_SPLIT_CLOSE_RIGHT_CLOSE_DELAY_MS"] = "0" + app.launchEnvironment["CMUX_UI_TEST_SPLIT_CLOSE_RIGHT_BURST_FRAMES"] = "36" + app.launchEnvironment["CMUX_UI_TEST_SPLIT_CLOSE_RIGHT_PATTERN"] = "close_right_single" + app.launch() + app.activate() + + XCTAssertTrue(waitForAnyData(timeout: 12.0), "Expected split-close-right test data to be written at \(dataPath)") + + let doneDeadline = Date().addingTimeInterval(90.0) + while Date() < doneDeadline { + if let data = loadData(), data["visualDone"] == "1" { + break + } + RunLoop.current.run(until: Date().addingTimeInterval(0.10)) + } + + guard let data = loadData() else { + XCTFail("Missing split-close-right data after waiting. path=\(dataPath)") + return + } + if let setupError = data["setupError"], !setupError.isEmpty { + XCTFail("Test setup failed: \(setupError)") + return + } + + let lastIter = Int(data["visualLastIteration"] ?? "") ?? 0 + XCTAssertGreaterThan(lastIter, 0, "Expected at least one visual iteration. data=\(data)") + + let sizeMismatchSeen = (data["sizeMismatchSeen"] ?? "") == "1" + let trace = data["timelineTrace"] ?? "" + + XCTAssertFalse( + sizeMismatchSeen, + "Transient IOSurface size mismatch detected (stretched text). at=\(data["sizeMismatchObservedAt"] ?? "") iter=\(data["sizeMismatchObservedIteration"] ?? "") trace=\(trace)" + ) + } + func testReproBlankAfterClosingBottomSplitsViaShortcuts() { let app = XCUIApplication() app.launchEnvironment["CMUX_UI_TEST_DIAGNOSTICS_PATH"] = diagnosticsPath diff --git a/ghostty b/ghostty index d641319c..80d3fa07 160000 --- a/ghostty +++ b/ghostty @@ -1 +1 @@ -Subproject commit d641319cd95604488e9cdbb508d5f218e91cd15a +Subproject commit 80d3fa07ff8ae86fe6089083371f71ac7634648f