From d3aec2f7267a30bc08943303fc591009a97d5ce6 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Mon, 6 Apr 2026 22:19:12 -0700 Subject: [PATCH 1/6] Fix window restore after display collapse --- Sources/AppDelegate.swift | 438 ++++++++++++++++++++++-- Sources/Workspace.swift | 25 +- cmuxTests/SessionPersistenceTests.swift | 98 +++++- 3 files changed, 511 insertions(+), 50 deletions(-) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index 9d04fe352c0d..ce45d05f0d16 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -2210,9 +2210,31 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } struct PersistedWindowGeometry: Codable, Sendable { + struct StoredGeometry: Codable, Sendable { + let frame: SessionRectSnapshot + let display: SessionDisplaySnapshot? + } + let version: Int let frame: SessionRectSnapshot let display: SessionDisplaySnapshot? + let displayConfigurations: [String: StoredGeometry]? + + init( + version: Int, + frame: SessionRectSnapshot, + display: SessionDisplaySnapshot?, + displayConfigurations: [String: StoredGeometry]? = nil + ) { + self.version = version + self.frame = frame + self.display = display + self.displayConfigurations = displayConfigurations + } + + var storedGeometry: StoredGeometry { + StoredGeometry(frame: frame, display: display) + } } nonisolated static let persistedWindowGeometrySchemaVersion = 2 @@ -2220,6 +2242,14 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent private nonisolated static let legacyPersistedWindowGeometryDefaultsKeys = [ "cmux.session.lastWindowGeometry.v1" ] + private nonisolated static let minimumVisibleRestoredWindowWidth: CGFloat = 480 + private nonisolated static let minimumVisibleRestoredWindowHeight: CGFloat = 320 + private nonisolated static let defaultRestoredWindowSize = CGSize(width: 960, height: 720) + private nonisolated static let defaultRestoredWindowMargin: CGFloat = 80 + private nonisolated static let mainWindowMinimumContentSize = NSSize( + width: CGFloat(SessionPersistencePolicy.minimumWindowWidth), + height: CGFloat(SessionPersistencePolicy.minimumWindowHeight) + ) weak var tabManager: TabManager? weak var notificationStore: TerminalNotificationStore? @@ -2233,6 +2263,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent private var shortcutMonitor: Any? private var shortcutDefaultsObserver: NSObjectProtocol? private var menuBarVisibilityObserver: NSObjectProtocol? + private var screenParametersObserver: NSObjectProtocol? private var splitButtonTooltipRefreshScheduled = false private struct PendingConfiguredShortcutChord { let firstStroke: ShortcutStroke @@ -2584,6 +2615,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent titlebarAccessoryController.start() windowDecorationsController.start() installMainWindowKeyObserver() + installScreenParametersObserver() refreshGhosttyGotoSplitShortcuts() installGhosttyConfigObserver() installWindowResponderSwizzles() @@ -3738,13 +3770,40 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent return payload } + private func persistedWindowGeometryEntry( + defaults: UserDefaults = .standard, + availableDisplays: [SessionDisplayGeometry]? = nil, + matchingOnly: Bool = false + ) -> PersistedWindowGeometry.StoredGeometry? { + guard let payload = persistedWindowGeometry(defaults: defaults) else { + return nil + } + let fingerprint = Self.displayConfigurationFingerprint( + for: availableDisplays ?? currentDisplayGeometries().available + ) + return Self.persistedWindowGeometryEntry( + from: payload, + displayConfigurationFingerprint: fingerprint, + matchingOnly: matchingOnly + ) + } + private func persistWindowGeometry( frame: SessionRectSnapshot?, display: SessionDisplaySnapshot?, defaults: UserDefaults = .standard ) { Self.removeLegacyPersistedWindowGeometry(defaults: defaults) - guard let data = Self.encodedPersistedWindowGeometryData(frame: frame, display: display) else { + let existingDisplayConfigurations = persistedWindowGeometry(defaults: defaults)?.displayConfigurations + let displayConfigurationFingerprint = Self.displayConfigurationFingerprint( + for: currentDisplayGeometries().available + ) + guard let data = Self.encodedPersistedWindowGeometryData( + frame: frame, + display: display, + displayConfigurationFingerprint: displayConfigurationFingerprint, + existingDisplayConfigurations: existingDisplayConfigurations + ) else { return } defaults.set(data, forKey: Self.persistedWindowGeometryDefaultsKey) @@ -3752,13 +3811,23 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent private nonisolated static func encodedPersistedWindowGeometryData( frame: SessionRectSnapshot?, - display: SessionDisplaySnapshot? + display: SessionDisplaySnapshot?, + displayConfigurationFingerprint: String? = nil, + existingDisplayConfigurations: [String: PersistedWindowGeometry.StoredGeometry]? = nil ) -> Data? { guard let frame else { return nil } + var displayConfigurations = existingDisplayConfigurations ?? [:] + if let displayConfigurationFingerprint, !displayConfigurationFingerprint.isEmpty { + displayConfigurations[displayConfigurationFingerprint] = PersistedWindowGeometry.StoredGeometry( + frame: frame, + display: display + ) + } let payload = PersistedWindowGeometry( version: persistedWindowGeometrySchemaVersion, frame: frame, - display: display + display: display, + displayConfigurations: displayConfigurations.isEmpty ? nil : displayConfigurations ) return try? JSONEncoder().encode(payload) } @@ -3785,6 +3854,143 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent ) } + private func installScreenParametersObserver() { + guard screenParametersObserver == nil else { return } + screenParametersObserver = NotificationCenter.default.addObserver( + forName: NSApplication.didChangeScreenParametersNotification, + object: nil, + queue: .main + ) { [weak self] _ in + self?.handleScreenParametersDidChange() + } + } + + private func handleScreenParametersDidChange() { + let displays = currentDisplayGeometries() + let primaryWindow = preferredPrimaryMainWindowForGeometryRestore() + let matchingPersistedGeometry = persistedWindowGeometryEntry( + availableDisplays: displays.available, + matchingOnly: true + ) + + for context in mainWindowContexts.values { + guard let window = resolvedWindow(for: context), + shouldReconcileMainWindowFrameOnScreenParameterChange(window) else { + continue + } + + let resolvedFrame: CGRect? = { + if window === primaryWindow, let matchingPersistedGeometry { + return Self.resolvedWindowFrame( + from: matchingPersistedGeometry.frame, + display: matchingPersistedGeometry.display, + availableDisplays: displays.available, + fallbackDisplay: displays.fallback + ) + } + return Self.resolvedWindowFrame( + from: SessionRectSnapshot(window.frame), + display: nil, + availableDisplays: displays.available, + fallbackDisplay: displays.fallback + ) + }() + + guard let resolvedFrame else { continue } + guard !Self.rectApproximatelyEqual(window.frame, resolvedFrame) else { continue } + + applyValidatedMainWindowFrame(resolvedFrame, to: window, display: true) +#if DEBUG + dlog( + "window.frame.reconciled reason=screenParameters window={\(debugWindowToken(window))} " + + "frame={\(debugNSRectDescription(window.frame))}" + ) +#endif + } + + if let primaryWindow { + persistWindowGeometry(from: primaryWindow) + } + } + + private func preferredPrimaryMainWindowForGeometryRestore() -> NSWindow? { + if let keyWindow = NSApp.keyWindow, + let context = contextForMainTerminalWindow(keyWindow) { + return resolvedWindow(for: context) + } + if let mainWindow = NSApp.mainWindow, + let context = contextForMainTerminalWindow(mainWindow) { + return resolvedWindow(for: context) + } + if let activeManager = tabManager, + let context = mainWindowContexts.values.first(where: { $0.tabManager === activeManager }) { + return resolvedWindow(for: context) + } + return mainWindowContexts.values.compactMap { resolvedWindow(for: $0) }.first + } + + private func shouldReconcileMainWindowFrameOnScreenParameterChange(_ window: NSWindow) -> Bool { + guard !window.isMiniaturized else { return false } + guard !window.styleMask.contains(.fullScreen) else { return false } + return true + } + + private func enforceMainWindowMinimumSize(on window: NSWindow) { + let contentMinSize = Self.mainWindowMinimumContentSize + window.contentMinSize = contentMinSize + let minimumFrameSize = window.frameRect( + forContentRect: NSRect(origin: .zero, size: contentMinSize) + ).size + if window.minSize.width < minimumFrameSize.width || window.minSize.height < minimumFrameSize.height { + window.minSize = NSSize( + width: max(window.minSize.width, minimumFrameSize.width), + height: max(window.minSize.height, minimumFrameSize.height) + ) + } + } + + private func applyValidatedMainWindowFrame( + _ frame: CGRect, + to window: NSWindow, + display: Bool + ) { + enforceMainWindowMinimumSize(on: window) + + let minimumFrameSize = window.frameRect( + forContentRect: NSRect(origin: .zero, size: Self.mainWindowMinimumContentSize) + ).size + var targetFrame = frame.standardized + targetFrame.size.width = max(targetFrame.width, minimumFrameSize.width) + targetFrame.size.height = max(targetFrame.height, minimumFrameSize.height) + + if let screen = Self.screenForConstrainingFrame(targetFrame, currentWindow: window) { + targetFrame = window.constrainFrameRect(targetFrame, to: screen) + if targetFrame.width < minimumFrameSize.width || targetFrame.height < minimumFrameSize.height { + targetFrame.size.width = max(targetFrame.width, minimumFrameSize.width) + targetFrame.size.height = max(targetFrame.height, minimumFrameSize.height) + targetFrame = window.constrainFrameRect(targetFrame, to: screen) + } + } + + window.setFrame(targetFrame.integral, display: display, animate: false) + } + + private nonisolated static func screenForConstrainingFrame( + _ frame: CGRect, + currentWindow: NSWindow? + ) -> NSScreen? { + let bestScreen = NSScreen.screens.max { lhs, rhs in + intersectionArea(frame, lhs.visibleFrame) < intersectionArea(frame, rhs.visibleFrame) + } + if let bestScreen, intersectionArea(frame, bestScreen.visibleFrame) > 0 { + return bestScreen + } + if let currentScreen = currentWindow?.screen { + return currentScreen + } + return NSScreen.main ?? NSScreen.screens.first + } + private func currentDisplayGeometries() -> ( available: [SessionDisplayGeometry], fallback: SessionDisplayGeometry? @@ -3806,6 +4012,44 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent return (available, fallback) } + nonisolated static func persistedWindowGeometryEntry( + from payload: PersistedWindowGeometry, + displayConfigurationFingerprint: String?, + matchingOnly: Bool = false + ) -> PersistedWindowGeometry.StoredGeometry? { + if let displayConfigurationFingerprint, + let matching = payload.displayConfigurations?[displayConfigurationFingerprint] { + return matching + } + return matchingOnly ? nil : payload.storedGeometry + } + + nonisolated static func displayConfigurationFingerprint( + for displays: [SessionDisplayGeometry] + ) -> String? { + guard !displays.isEmpty else { return nil } + let components = displays.map { display in + [ + display.displayID.map(String.init) ?? "nil", + rectFingerprintComponent(display.frame), + rectFingerprintComponent(display.visibleFrame), + ].joined(separator: "@") + } + return components.sorted().joined(separator: "|") + } + + private nonisolated static func rectFingerprintComponent(_ rect: CGRect) -> String { + let standardized = rect.standardized + return [ + standardized.origin.x, + standardized.origin.y, + standardized.width, + standardized.height, + ] + .map { String(Int(($0 * 2).rounded())) } + .joined(separator: ",") + } + private func attemptStartupSessionRestoreIfNeeded(primaryWindow: NSWindow) { guard !didAttemptStartupSessionRestore else { return } didAttemptStartupSessionRestore = true @@ -3830,7 +4074,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent ) } else { let displays = currentDisplayGeometries() - let fallbackGeometry = persistedWindowGeometry() + let fallbackGeometry = persistedWindowGeometryEntry(availableDisplays: displays.available) if let restoredFrame = Self.resolvedStartupPrimaryWindowFrame( primarySnapshot: nil, fallbackFrame: fallbackGeometry?.frame, @@ -3838,7 +4082,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent availableDisplays: displays.available, fallbackDisplay: displays.fallback ) { - primaryWindow.setFrame(restoredFrame, display: true) + applyValidatedMainWindowFrame(restoredFrame, to: primaryWindow, display: true) } } @@ -3897,7 +4141,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent context.sidebarSelectionState.selection = snapshot.sidebar.selection.sidebarSelection if let restoredFrame = resolvedWindowFrame(from: snapshot), let window { - window.setFrame(restoredFrame, display: true) + applyValidatedMainWindowFrame(restoredFrame, to: window, display: true) #if DEBUG dlog( "session.restore.frameApplied window=\(context.windowId.uuidString.prefix(8)) " + @@ -3948,7 +4192,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent fallbackDisplay: SessionDisplayGeometry? ) -> CGRect? { guard let frameSnapshot else { return nil } - let frame = frameSnapshot.cgRect + let frame = frameSnapshot.cgRect.standardized guard frame.width.isFinite, frame.height.isFinite, frame.origin.x.isFinite, @@ -3958,18 +4202,42 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent let minWidth = CGFloat(SessionPersistencePolicy.minimumWindowWidth) let minHeight = CGFloat(SessionPersistencePolicy.minimumWindowHeight) - guard frame.width >= minWidth, - frame.height >= minHeight else { - return nil + guard !availableDisplays.isEmpty else { + return CGRect( + x: frame.minX, + y: frame.minY, + width: max(frame.width, minWidth), + height: max(frame.height, minHeight) + ) } - guard !availableDisplays.isEmpty else { return frame } + let minimumVisibleWidth = Self.minimumVisibleRestoredWindowWidth + let minimumVisibleHeight = max(Self.minimumVisibleRestoredWindowHeight, minHeight) + let fallbackTargetDisplay = fallbackDisplay ?? availableDisplays.first + + if frame.width < minWidth || frame.height < minHeight { + guard let fallbackTargetDisplay else { return nil } + return fallbackFrameForInvalidRestore( + frame, + in: fallbackTargetDisplay.visibleFrame, + minWidth: minWidth, + minHeight: minHeight + ) + } if let targetDisplay = display(for: displaySnapshot, in: availableDisplays) { if shouldPreserveExactFrame( frame: frame, displaySnapshot: displaySnapshot, targetDisplay: targetDisplay + ), + hasSufficientVisibleFrame( + frame, + in: [targetDisplay], + minWidth: minWidth, + minHeight: minHeight, + minimumVisibleWidth: minimumVisibleWidth, + minimumVisibleHeight: minimumVisibleHeight ) { return frame } @@ -3977,12 +4245,23 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent frame: frame, displaySnapshot: displaySnapshot, targetDisplay: targetDisplay, + fallbackDisplay: fallbackTargetDisplay, minWidth: minWidth, - minHeight: minHeight + minHeight: minHeight, + minimumVisibleWidth: minimumVisibleWidth, + minimumVisibleHeight: minimumVisibleHeight ) } - if let intersectingDisplay = availableDisplays.first(where: { $0.visibleFrame.intersects(frame) }) { + if hasSufficientVisibleFrame( + frame, + in: availableDisplays, + minWidth: minWidth, + minHeight: minHeight, + minimumVisibleWidth: minimumVisibleWidth, + minimumVisibleHeight: minimumVisibleHeight + ), + let intersectingDisplay = bestIntersectingDisplay(for: frame, in: availableDisplays) { return clampFrame( frame, within: intersectingDisplay.visibleFrame, @@ -3991,20 +4270,37 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent ) } - guard let fallbackDisplay else { return frame } - if let sourceReference = displaySnapshot?.visibleFrame?.cgRect ?? displaySnapshot?.frame?.cgRect { - return remappedFrame( + if let sourceReference = displaySnapshot?.visibleFrame?.cgRect ?? displaySnapshot?.frame?.cgRect, + let fallbackTargetDisplay { + let remapped = remappedFrame( frame, from: sourceReference, - to: fallbackDisplay.visibleFrame, + to: fallbackTargetDisplay.visibleFrame, + minWidth: minWidth, + minHeight: minHeight + ) + if hasSufficientVisibleFrame( + remapped, + in: [fallbackTargetDisplay], + minWidth: minWidth, + minHeight: minHeight, + minimumVisibleWidth: minimumVisibleWidth, + minimumVisibleHeight: minimumVisibleHeight + ) { + return remapped + } + return fallbackFrameForInvalidRestore( + frame, + in: fallbackTargetDisplay.visibleFrame, minWidth: minWidth, minHeight: minHeight ) } - return centeredFrame( + guard let fallbackTargetDisplay else { return nil } + return fallbackFrameForInvalidRestore( frame, - in: fallbackDisplay.visibleFrame, + in: fallbackTargetDisplay.visibleFrame, minWidth: minWidth, minHeight: minHeight ) @@ -4014,10 +4310,20 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent frame: CGRect, displaySnapshot: SessionDisplaySnapshot?, targetDisplay: SessionDisplayGeometry, + fallbackDisplay: SessionDisplayGeometry?, minWidth: CGFloat, - minHeight: CGFloat + minHeight: CGFloat, + minimumVisibleWidth: CGFloat, + minimumVisibleHeight: CGFloat ) -> CGRect { - if targetDisplay.visibleFrame.intersects(frame) { + if hasSufficientVisibleFrame( + frame, + in: [targetDisplay], + minWidth: minWidth, + minHeight: minHeight, + minimumVisibleWidth: minimumVisibleWidth, + minimumVisibleHeight: minimumVisibleHeight + ) { // Preserve the user's exact frame when enough of the top of the window // remains reachable on-screen; only clamp when the saved frame would // reopen with an inaccessible titlebar/top strip. @@ -4036,23 +4342,94 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } if let sourceReference = displaySnapshot?.visibleFrame?.cgRect ?? displaySnapshot?.frame?.cgRect { - return remappedFrame( + let remapped = remappedFrame( frame, from: sourceReference, to: targetDisplay.visibleFrame, minWidth: minWidth, minHeight: minHeight ) + if hasSufficientVisibleFrame( + remapped, + in: [targetDisplay], + minWidth: minWidth, + minHeight: minHeight, + minimumVisibleWidth: minimumVisibleWidth, + minimumVisibleHeight: minimumVisibleHeight + ) { + return remapped + } } - return centeredFrame( + let fallbackTargetDisplay = fallbackDisplay ?? targetDisplay + return fallbackFrameForInvalidRestore( frame, - in: targetDisplay.visibleFrame, + in: fallbackTargetDisplay.visibleFrame, minWidth: minWidth, minHeight: minHeight ) } + private nonisolated static func bestIntersectingDisplay( + for frame: CGRect, + in displays: [SessionDisplayGeometry] + ) -> SessionDisplayGeometry? { + let overlaps = displays.map { display -> (display: SessionDisplayGeometry, area: CGFloat) in + (display, intersectionArea(frame, display.visibleFrame)) + } + guard let bestOverlap = overlaps.max(by: { $0.area < $1.area }), + bestOverlap.area > 0 else { + return nil + } + return bestOverlap.display + } + + private nonisolated static func hasSufficientVisibleFrame( + _ frame: CGRect, + in displays: [SessionDisplayGeometry], + minWidth: CGFloat, + minHeight: CGFloat, + minimumVisibleWidth: CGFloat, + minimumVisibleHeight: CGFloat + ) -> Bool { + guard let visibleBounds = visibleIntersectionBounds(for: frame, in: displays) else { + return false + } + let requiredWidth = min(minimumVisibleWidth, max(frame.width, minWidth)) + let requiredHeight = min(minimumVisibleHeight, max(frame.height, minHeight)) + return visibleBounds.width >= requiredWidth && visibleBounds.height >= requiredHeight + } + + private nonisolated static func visibleIntersectionBounds( + for frame: CGRect, + in displays: [SessionDisplayGeometry] + ) -> CGRect? { + displays.reduce(nil) { partialResult, display in + let intersection = frame.intersection(display.visibleFrame) + guard !intersection.isNull else { return partialResult } + return partialResult?.union(intersection) ?? intersection + } + } + + private nonisolated static func fallbackFrameForInvalidRestore( + _ frame: CGRect, + in visibleFrame: CGRect, + minWidth: CGFloat, + minHeight: CGFloat + ) -> CGRect { + let preferredWidth = max(frame.width, max(Self.defaultRestoredWindowSize.width, minWidth)) + let preferredHeight = max(frame.height, max(Self.defaultRestoredWindowSize.height, minHeight)) + let maxWidth = max(visibleFrame.width - Self.defaultRestoredWindowMargin, minWidth) + let maxHeight = max(visibleFrame.height - Self.defaultRestoredWindowMargin, minHeight) + let defaultFrame = CGRect( + x: visibleFrame.midX - (min(preferredWidth, maxWidth) / 2), + y: visibleFrame.midY - (min(preferredHeight, maxHeight) / 2), + width: min(preferredWidth, maxWidth), + height: min(preferredHeight, maxHeight) + ) + return clampFrame(defaultFrame, within: visibleFrame, minWidth: minWidth, minHeight: minHeight) + } + private nonisolated static func shouldPreserveAccessibleFrame( frame: CGRect, targetDisplay: SessionDisplayGeometry, @@ -4432,10 +4809,16 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent return false } + let persistedDisplayConfigurations = persistedWindowGeometry()?.displayConfigurations + let persistedGeometryFingerprint = Self.displayConfigurationFingerprint( + for: currentDisplayGeometries().available + ) let persistedGeometryData = snapshot.windows.first.flatMap { primaryWindow in Self.encodedPersistedWindowGeometryData( frame: primaryWindow.frame, - display: primaryWindow.display + display: primaryWindow.display, + displayConfigurationFingerprint: persistedGeometryFingerprint, + existingDisplayConfigurations: persistedDisplayConfigurations ) } @@ -7088,9 +7471,10 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent window.titlebarAppearsTransparent = true window.isMovableByWindowBackground = false window.isMovable = false + enforceMainWindowMinimumSize(on: window) let restoredFrame = resolvedWindowFrame(from: sessionWindowSnapshot) if let restoredFrame { - window.setFrame(restoredFrame, display: false) + applyValidatedMainWindowFrame(restoredFrame, to: window, display: false) } else { window.center() // Cascade using the same algorithm as upstream Ghostty: seed from @@ -7141,7 +7525,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } } if let restoredFrame { - window.setFrame(restoredFrame, display: true) + applyValidatedMainWindowFrame(restoredFrame, to: window, display: true) #if DEBUG dlog( "session.restore.frameApplied window=\(windowId.uuidString.prefix(8)) " + diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 87ebe6372a2a..65473c8f51ed 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -6755,8 +6755,7 @@ final class Workspace: Identifiable, ObservableObject { private static func bonsplitAppearance(from config: GhosttyConfig) -> BonsplitConfiguration.Appearance { bonsplitAppearance( from: config.backgroundColor, - backgroundOpacity: config.backgroundOpacity, - tabTitleFontSize: config.surfaceTabBarFontSize + backgroundOpacity: config.backgroundOpacity ) } @@ -6777,11 +6776,9 @@ final class Workspace: Identifiable, ObservableObject { private static func bonsplitAppearance( from backgroundColor: NSColor, - backgroundOpacity: Double, - tabTitleFontSize: CGFloat = 11 + backgroundOpacity: Double ) -> BonsplitConfiguration.Appearance { BonsplitConfiguration.Appearance( - tabTitleFontSize: tabTitleFontSize, splitButtonTooltips: Self.currentSplitButtonTooltips(), enableAnimations: false, chromeColors: .init( @@ -6798,20 +6795,16 @@ final class Workspace: Identifiable, ObservableObject { backgroundColor: config.backgroundColor, backgroundOpacity: config.backgroundOpacity ) - let nextTabTitleFontSize = config.surfaceTabBarFontSize let currentAppearance = bonsplitController.configuration.appearance let currentBackgroundHex = currentAppearance.chromeColors.backgroundHex - let currentTabTitleFontSize = currentAppearance.tabTitleFontSize let backgroundChanged = currentBackgroundHex != nextHex - let fontSizeChanged = abs(currentTabTitleFontSize - nextTabTitleFontSize) > 0.0001 - let isNoOp = !backgroundChanged && !fontSizeChanged + let isNoOp = !backgroundChanged if GhosttyApp.shared.backgroundLogEnabled { GhosttyApp.shared.logBackground( "theme apply workspace=\(id.uuidString) reason=\(reason) " + "currentBg=\(currentBackgroundHex ?? "nil") nextBg=\(nextHex) " + - "currentTabFont=\(String(format: "%.3f", currentTabTitleFontSize)) " + - "nextTabFont=\(String(format: "%.3f", nextTabTitleFontSize)) noop=\(isNoOp)" + "noop=\(isNoOp)" ) } @@ -6820,15 +6813,11 @@ final class Workspace: Identifiable, ObservableObject { if backgroundChanged { bonsplitController.configuration.appearance.chromeColors.backgroundHex = nextHex } - if fontSizeChanged { - bonsplitController.configuration.appearance.tabTitleFontSize = nextTabTitleFontSize - } if GhosttyApp.shared.backgroundLogEnabled { GhosttyApp.shared.logBackground( "theme applied workspace=\(id.uuidString) reason=\(reason) " + - "resultingBg=\(bonsplitController.configuration.appearance.chromeColors.backgroundHex ?? "nil") " + - "resultingTabFont=\(String(format: "%.3f", bonsplitController.configuration.appearance.tabTitleFontSize))" + "resultingBg=\(bonsplitController.configuration.appearance.chromeColors.backgroundHex ?? "nil")" ) } } @@ -6884,11 +6873,9 @@ final class Workspace: Identifiable, ObservableObject { // and keep split entry instantaneous. // Use the cached Ghostty config so new workspaces inherit tab-strip sizing // without paying repeated parse costs on the workspace-creation hot path. - let initialSurfaceTabBarFontSize = GhosttyConfig.load().surfaceTabBarFontSize let appearance = Self.bonsplitAppearance( from: GhosttyApp.shared.defaultBackgroundColor, - backgroundOpacity: GhosttyApp.shared.defaultBackgroundOpacity, - tabTitleFontSize: initialSurfaceTabBarFontSize + backgroundOpacity: GhosttyApp.shared.defaultBackgroundOpacity ) let config = BonsplitConfiguration( allowSplits: true, diff --git a/cmuxTests/SessionPersistenceTests.swift b/cmuxTests/SessionPersistenceTests.swift index a8b58f7d3af0..d3688d92513b 100644 --- a/cmuxTests/SessionPersistenceTests.swift +++ b/cmuxTests/SessionPersistenceTests.swift @@ -739,10 +739,100 @@ final class SessionPersistenceTests: XCTestCase { XCTAssertNotNil(restored) guard let restored else { return } XCTAssertTrue(display.visibleFrame.contains(restored)) - XCTAssertEqual(restored.minX, 50, accuracy: 0.001) - XCTAssertEqual(restored.minY, 50, accuracy: 0.001) - XCTAssertEqual(restored.width, 900, accuracy: 0.001) - XCTAssertEqual(restored.height, 700, accuracy: 0.001) + XCTAssertEqual(restored.minX, 40, accuracy: 0.001) + XCTAssertEqual(restored.minY, 40, accuracy: 0.001) + XCTAssertEqual(restored.width, 920, accuracy: 0.001) + XCTAssertEqual(restored.height, 720, accuracy: 0.001) + } + + func testResolvedWindowFrameFallsBackToCenteredDefaultWhenOnlySliverIsVisible() { + let savedFrame = SessionRectSnapshot(x: -850, y: 60, width: 900, height: 700) + let display = AppDelegate.SessionDisplayGeometry( + displayID: 1, + frame: CGRect(x: 0, y: 0, width: 1_200, height: 800), + visibleFrame: CGRect(x: 0, y: 0, width: 1_200, height: 800) + ) + + let restored = AppDelegate.resolvedWindowFrame( + from: savedFrame, + display: nil, + availableDisplays: [display], + fallbackDisplay: display + ) + + XCTAssertNotNil(restored) + guard let restored else { return } + XCTAssertEqual(restored.minX, 120, accuracy: 0.001) + XCTAssertEqual(restored.minY, 40, accuracy: 0.001) + XCTAssertEqual(restored.width, 960, accuracy: 0.001) + XCTAssertEqual(restored.height, 720, accuracy: 0.001) + } + + func testResolvedWindowFrameFallsBackToCenteredDefaultWhenSavedFrameIsBelowMinimumSize() { + let savedFrame = SessionRectSnapshot(x: 0, y: 0, width: 50, height: 240) + let display = AppDelegate.SessionDisplayGeometry( + displayID: 1, + frame: CGRect(x: 0, y: 0, width: 1_200, height: 800), + visibleFrame: CGRect(x: 0, y: 0, width: 1_200, height: 800) + ) + + let restored = AppDelegate.resolvedWindowFrame( + from: savedFrame, + display: nil, + availableDisplays: [display], + fallbackDisplay: display + ) + + XCTAssertNotNil(restored) + guard let restored else { return } + XCTAssertEqual(restored.minX, 120, accuracy: 0.001) + XCTAssertEqual(restored.minY, 40, accuracy: 0.001) + XCTAssertEqual(restored.width, 960, accuracy: 0.001) + XCTAssertEqual(restored.height, 720, accuracy: 0.001) + } + + func testPersistedWindowGeometryEntryPrefersMatchingDisplayConfiguration() throws { + let builtInDisplay = AppDelegate.SessionDisplayGeometry( + displayID: 1, + frame: CGRect(x: 0, y: 0, width: 1_510, height: 982), + visibleFrame: CGRect(x: 0, y: 0, width: 1_510, height: 944) + ) + let builtInFingerprint = try XCTUnwrap( + AppDelegate.displayConfigurationFingerprint(for: [builtInDisplay]) + ) + let builtInGeometry = AppDelegate.PersistedWindowGeometry.StoredGeometry( + frame: SessionRectSnapshot(x: 180, y: 120, width: 1_100, height: 760), + display: SessionDisplaySnapshot( + displayID: 1, + frame: SessionRectSnapshot(x: 0, y: 0, width: 1_510, height: 982), + visibleFrame: SessionRectSnapshot(x: 0, y: 0, width: 1_510, height: 944) + ) + ) + let fallbackGeometry = SessionRectSnapshot(x: 2_240, y: 140, width: 1_400, height: 900) + let payload = AppDelegate.PersistedWindowGeometry( + version: AppDelegate.persistedWindowGeometrySchemaVersion, + frame: fallbackGeometry, + display: nil, + displayConfigurations: [builtInFingerprint: builtInGeometry] + ) + + let resolved = AppDelegate.persistedWindowGeometryEntry( + from: payload, + displayConfigurationFingerprint: builtInFingerprint + ) + + let resolvedGeometry = try XCTUnwrap(resolved) + XCTAssertEqual(resolvedGeometry.frame.x, builtInGeometry.frame.x, accuracy: 0.001) + XCTAssertEqual(resolvedGeometry.frame.y, builtInGeometry.frame.y, accuracy: 0.001) + XCTAssertEqual(resolvedGeometry.frame.width, builtInGeometry.frame.width, accuracy: 0.001) + XCTAssertEqual(resolvedGeometry.frame.height, builtInGeometry.frame.height, accuracy: 0.001) + XCTAssertNil( + AppDelegate.persistedWindowGeometryEntry( + from: payload, + displayConfigurationFingerprint: "missing-fingerprint", + matchingOnly: true + ) + ) } func testResolvedWindowFramePreservesExactGeometryWhenDisplayIsUnchanged() { From 0ab22569b670eb6a0d97e4a6d72fc466b1fd1ade Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Tue, 7 Apr 2026 02:19:26 -0700 Subject: [PATCH 2/6] Address review feedback on window restore fix Lost-update race: persistWindowGeometry and saveSessionSnapshot both wrote the displayConfigurations map with their own copies, racing on UserDefaults. Both writers now go through sessionPersistenceQueue and do read-merge-encode-write inside the queue (writePersistedWindowGeometry). Per-display visibility: hasSufficientVisibleFrame previously unioned intersections into a bbox, which falsely passed two disjoint slivers across two displays. It now checks each display individually so a window is only "reachable" if at least one display has a usable chunk. LRU cap: bound the per-display geometry map at 8 entries so users cycling through many docks/displays don't accumulate UserDefaults indefinitely. The just-written fingerprint is always preserved. Gate post-reconcile persist: handleScreenParametersDidChange now only persists the primary window's frame after the new display set if it actually reconciled the primary (skipping miniaturized/fullscreen windows), so a stale off-screen frame can't get baked into the new fingerprint. Restore tab title font size plumbing: the prior commit removed the surfaceTabBarFontSize wiring in Workspace.swift, silently reverting PR #2645. The bonsplit API still exposes tabTitleFontSize, so the plumbing is restored. Add regression tests for the per-display visibility check and the LRU eviction so the contracts are pinned. Co-Authored-By: Claude Opus 4.6 --- Sources/AppDelegate.swift | 183 ++++++++++++++++++------ Sources/Workspace.swift | 25 +++- cmuxTests/SessionPersistenceTests.swift | 111 ++++++++++++++ 3 files changed, 271 insertions(+), 48 deletions(-) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index ce45d05f0d16..998c0dccf426 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -2246,6 +2246,10 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent private nonisolated static let minimumVisibleRestoredWindowHeight: CGFloat = 320 private nonisolated static let defaultRestoredWindowSize = CGSize(width: 960, height: 720) private nonisolated static let defaultRestoredWindowMargin: CGFloat = 80 + // Cap how many per-display-configuration window geometries we keep around + // in UserDefaults so users that frequently dock/undock or rotate through + // conference-room displays don't accumulate entries indefinitely. + nonisolated static let maxStoredDisplayConfigurations = 8 private nonisolated static let mainWindowMinimumContentSize = NSSize( width: CGFloat(SessionPersistencePolicy.minimumWindowWidth), height: CGFloat(SessionPersistencePolicy.minimumWindowHeight) @@ -3793,12 +3797,40 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent display: SessionDisplaySnapshot?, defaults: UserDefaults = .standard ) { - Self.removeLegacyPersistedWindowGeometry(defaults: defaults) - let existingDisplayConfigurations = persistedWindowGeometry(defaults: defaults)?.displayConfigurations + guard let frame else { return } + // Snapshot the current display fingerprint on the main actor while + // NSScreen is safe to read, then dispatch the read-merge-encode-write + // onto sessionPersistenceQueue so it serializes with saveSessionSnapshot's + // background writes (avoids the lost-update race where two writers each + // read the old displayConfigurations map and clobber the other's entry). let displayConfigurationFingerprint = Self.displayConfigurationFingerprint( for: currentDisplayGeometries().available ) - guard let data = Self.encodedPersistedWindowGeometryData( + sessionPersistenceQueue.async { + Self.writePersistedWindowGeometry( + frame: frame, + display: display, + displayConfigurationFingerprint: displayConfigurationFingerprint, + defaults: defaults + ) + } + } + + /// Read-merge-encode-write of the per-display window geometry payload. + /// Always called on `sessionPersistenceQueue` so reads and writes against + /// the `displayConfigurations` map are serialized. + nonisolated static func writePersistedWindowGeometry( + frame: SessionRectSnapshot, + display: SessionDisplaySnapshot?, + displayConfigurationFingerprint: String?, + defaults: UserDefaults = .standard + ) { + removeLegacyPersistedWindowGeometry(defaults: defaults) + let existingDisplayConfigurations = defaults + .data(forKey: persistedWindowGeometryDefaultsKey) + .flatMap { decodedPersistedWindowGeometryData($0) }? + .displayConfigurations + guard let data = encodedPersistedWindowGeometryData( frame: frame, display: display, displayConfigurationFingerprint: displayConfigurationFingerprint, @@ -3806,7 +3838,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent ) else { return } - defaults.set(data, forKey: Self.persistedWindowGeometryDefaultsKey) + defaults.set(data, forKey: persistedWindowGeometryDefaultsKey) } private nonisolated static func encodedPersistedWindowGeometryData( @@ -3816,22 +3848,52 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent existingDisplayConfigurations: [String: PersistedWindowGeometry.StoredGeometry]? = nil ) -> Data? { guard let frame else { return nil } - var displayConfigurations = existingDisplayConfigurations ?? [:] - if let displayConfigurationFingerprint, !displayConfigurationFingerprint.isEmpty { - displayConfigurations[displayConfigurationFingerprint] = PersistedWindowGeometry.StoredGeometry( - frame: frame, - display: display - ) - } + let displayConfigurations = mergedDisplayConfigurations( + existing: existingDisplayConfigurations, + fingerprint: displayConfigurationFingerprint, + frame: frame, + display: display + ) let payload = PersistedWindowGeometry( version: persistedWindowGeometrySchemaVersion, frame: frame, display: display, - displayConfigurations: displayConfigurations.isEmpty ? nil : displayConfigurations + displayConfigurations: displayConfigurations ) return try? JSONEncoder().encode(payload) } + /// Merge a freshly-captured per-display-configuration entry into the + /// existing map and evict the oldest extras when we exceed the LRU cap. + /// The `fingerprint` entry (if any) is always preserved as the most + /// recent — only other entries are eligible for eviction. + nonisolated static func mergedDisplayConfigurations( + existing: [String: PersistedWindowGeometry.StoredGeometry]?, + fingerprint: String?, + frame: SessionRectSnapshot, + display: SessionDisplaySnapshot? + ) -> [String: PersistedWindowGeometry.StoredGeometry]? { + var merged = existing ?? [:] + if let fingerprint, !fingerprint.isEmpty { + merged[fingerprint] = PersistedWindowGeometry.StoredGeometry( + frame: frame, + display: display + ) + } + while merged.count > maxStoredDisplayConfigurations { + // Evict any entry other than the just-written fingerprint. We + // don't track true LRU order, so picking an arbitrary stale key + // is acceptable for an eight-slot bound. + let evictionKey = merged.keys.first { $0 != fingerprint } + if let evictionKey { + merged.removeValue(forKey: evictionKey) + } else { + break + } + } + return merged.isEmpty ? nil : merged + } + nonisolated static func decodedPersistedWindowGeometryData(_ data: Data) -> PersistedWindowGeometry? { guard let payload = try? JSONDecoder().decode(PersistedWindowGeometry.self, from: data), payload.version == persistedWindowGeometrySchemaVersion else { @@ -3872,6 +3934,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent availableDisplays: displays.available, matchingOnly: true ) + var didReconcilePrimary = false for context in mainWindowContexts.values { guard let window = resolvedWindow(for: context), @@ -3897,6 +3960,18 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent }() guard let resolvedFrame else { continue } + + // Mark the primary window as reconciled whenever we computed a + // valid frame for it (even if no setFrame was needed). This is + // intentional: it means the primary's current frame is already + // usable on the new display set, and persisting it for the new + // fingerprint is desired. We do NOT mark it reconciled when the + // primary is miniaturized/fullscreen above (that path skips this + // loop entirely), so we won't persist a stale off-screen frame. + if window === primaryWindow { + didReconcilePrimary = true + } + guard !Self.rectApproximatelyEqual(window.frame, resolvedFrame) else { continue } applyValidatedMainWindowFrame(resolvedFrame, to: window, display: true) @@ -3908,7 +3983,12 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent #endif } - if let primaryWindow { + // Only persist if the primary window passed the reconcile guard (not + // miniaturized/fullscreen). Persisting a miniaturized window's frame + // here would write the pre-miniaturization frame from the *old* + // display into the *new* display-config entry, recreating the sliver + // bug on next launch. + if didReconcilePrimary, let primaryWindow { persistWindowGeometry(from: primaryWindow) } } @@ -4384,7 +4464,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent return bestOverlap.display } - private nonisolated static func hasSufficientVisibleFrame( + nonisolated static func hasSufficientVisibleFrame( _ frame: CGRect, in displays: [SessionDisplayGeometry], minWidth: CGFloat, @@ -4392,22 +4472,21 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent minimumVisibleWidth: CGFloat, minimumVisibleHeight: CGFloat ) -> Bool { - guard let visibleBounds = visibleIntersectionBounds(for: frame, in: displays) else { - return false - } + // A frame is "sufficiently visible" only if at least one individual + // display contains an intersection meeting the minimum visible + // width/height. Checking per-display (rather than the union of + // intersections) is important for two reasons: + // - Disjoint slivers on two displays would otherwise produce a wide + // bounding box that falsely passed the union check, even though + // neither display had enough of the window to be reachable. + // - Legitimately spanning windows still pass: at least one display + // has a meaningful chunk of the window above the threshold. let requiredWidth = min(minimumVisibleWidth, max(frame.width, minWidth)) let requiredHeight = min(minimumVisibleHeight, max(frame.height, minHeight)) - return visibleBounds.width >= requiredWidth && visibleBounds.height >= requiredHeight - } - - private nonisolated static func visibleIntersectionBounds( - for frame: CGRect, - in displays: [SessionDisplayGeometry] - ) -> CGRect? { - displays.reduce(nil) { partialResult, display in + return displays.contains { display in let intersection = frame.intersection(display.visibleFrame) - guard !intersection.isNull else { return partialResult } - return partialResult?.union(intersection) ?? intersection + guard !intersection.isNull else { return false } + return intersection.width >= requiredWidth && intersection.height >= requiredHeight } } @@ -4803,22 +4882,26 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent persistSessionSnapshot( nil, removeWhenEmpty: removeWhenEmpty, - persistedGeometryData: nil, + pendingGeometryWrite: nil, synchronously: writeSynchronously ) return false } - let persistedDisplayConfigurations = persistedWindowGeometry()?.displayConfigurations + // Capture only the inputs to the geometry write here on the main actor + // (NSScreen reads). The actual read-merge-encode-write of the + // displayConfigurations map happens later inside persistSessionSnapshot's + // writeBlock on sessionPersistenceQueue, so it serializes with + // persistWindowGeometry's writes and avoids lost updates. let persistedGeometryFingerprint = Self.displayConfigurationFingerprint( for: currentDisplayGeometries().available ) - let persistedGeometryData = snapshot.windows.first.flatMap { primaryWindow in - Self.encodedPersistedWindowGeometryData( - frame: primaryWindow.frame, + let pendingGeometryWrite: PendingGeometryWrite? = snapshot.windows.first.flatMap { primaryWindow in + guard let frame = primaryWindow.frame else { return nil } + return PendingGeometryWrite( + frame: frame, display: primaryWindow.display, - displayConfigurationFingerprint: persistedGeometryFingerprint, - existingDisplayConfigurations: persistedDisplayConfigurations + displayConfigurationFingerprint: persistedGeometryFingerprint ) } @@ -4828,12 +4911,22 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent persistSessionSnapshot( snapshot, removeWhenEmpty: false, - persistedGeometryData: persistedGeometryData, + pendingGeometryWrite: pendingGeometryWrite, synchronously: writeSynchronously ) return true } + /// Inputs to a deferred per-display window geometry write. Captured on + /// the main actor and merged into UserDefaults later, on + /// `sessionPersistenceQueue`, so the read-merge-encode happens at write + /// time and never races with concurrent writers. + struct PendingGeometryWrite: Sendable { + let frame: SessionRectSnapshot + let display: SessionDisplaySnapshot? + let displayConfigurationFingerprint: String? + } + nonisolated static func shouldPersistSnapshotOnWindowUnregister(isTerminatingApp: Bool) -> Bool { !isTerminatingApp } @@ -5013,18 +5106,24 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent private func persistSessionSnapshot( _ snapshot: AppSessionSnapshot?, removeWhenEmpty: Bool, - persistedGeometryData: Data?, + pendingGeometryWrite: PendingGeometryWrite?, synchronously: Bool ) { - guard snapshot != nil || removeWhenEmpty || persistedGeometryData != nil else { return } + guard snapshot != nil || removeWhenEmpty || pendingGeometryWrite != nil else { return } let writeBlock = { - Self.removeLegacyPersistedWindowGeometry() - if let persistedGeometryData { - UserDefaults.standard.set( - persistedGeometryData, - forKey: Self.persistedWindowGeometryDefaultsKey + if let pendingGeometryWrite { + // Read existing displayConfigurations from UserDefaults *here*, + // inside the queue, then merge and write back. Doing the + // read-merge-encode at write time prevents the lost-update + // race with persistWindowGeometry's queue writes. + Self.writePersistedWindowGeometry( + frame: pendingGeometryWrite.frame, + display: pendingGeometryWrite.display, + displayConfigurationFingerprint: pendingGeometryWrite.displayConfigurationFingerprint ) + } else { + Self.removeLegacyPersistedWindowGeometry() } if let snapshot { _ = SessionPersistenceStore.save(snapshot) diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 65473c8f51ed..87ebe6372a2a 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -6755,7 +6755,8 @@ final class Workspace: Identifiable, ObservableObject { private static func bonsplitAppearance(from config: GhosttyConfig) -> BonsplitConfiguration.Appearance { bonsplitAppearance( from: config.backgroundColor, - backgroundOpacity: config.backgroundOpacity + backgroundOpacity: config.backgroundOpacity, + tabTitleFontSize: config.surfaceTabBarFontSize ) } @@ -6776,9 +6777,11 @@ final class Workspace: Identifiable, ObservableObject { private static func bonsplitAppearance( from backgroundColor: NSColor, - backgroundOpacity: Double + backgroundOpacity: Double, + tabTitleFontSize: CGFloat = 11 ) -> BonsplitConfiguration.Appearance { BonsplitConfiguration.Appearance( + tabTitleFontSize: tabTitleFontSize, splitButtonTooltips: Self.currentSplitButtonTooltips(), enableAnimations: false, chromeColors: .init( @@ -6795,16 +6798,20 @@ final class Workspace: Identifiable, ObservableObject { backgroundColor: config.backgroundColor, backgroundOpacity: config.backgroundOpacity ) + let nextTabTitleFontSize = config.surfaceTabBarFontSize let currentAppearance = bonsplitController.configuration.appearance let currentBackgroundHex = currentAppearance.chromeColors.backgroundHex + let currentTabTitleFontSize = currentAppearance.tabTitleFontSize let backgroundChanged = currentBackgroundHex != nextHex - let isNoOp = !backgroundChanged + let fontSizeChanged = abs(currentTabTitleFontSize - nextTabTitleFontSize) > 0.0001 + let isNoOp = !backgroundChanged && !fontSizeChanged if GhosttyApp.shared.backgroundLogEnabled { GhosttyApp.shared.logBackground( "theme apply workspace=\(id.uuidString) reason=\(reason) " + "currentBg=\(currentBackgroundHex ?? "nil") nextBg=\(nextHex) " + - "noop=\(isNoOp)" + "currentTabFont=\(String(format: "%.3f", currentTabTitleFontSize)) " + + "nextTabFont=\(String(format: "%.3f", nextTabTitleFontSize)) noop=\(isNoOp)" ) } @@ -6813,11 +6820,15 @@ final class Workspace: Identifiable, ObservableObject { if backgroundChanged { bonsplitController.configuration.appearance.chromeColors.backgroundHex = nextHex } + if fontSizeChanged { + bonsplitController.configuration.appearance.tabTitleFontSize = nextTabTitleFontSize + } if GhosttyApp.shared.backgroundLogEnabled { GhosttyApp.shared.logBackground( "theme applied workspace=\(id.uuidString) reason=\(reason) " + - "resultingBg=\(bonsplitController.configuration.appearance.chromeColors.backgroundHex ?? "nil")" + "resultingBg=\(bonsplitController.configuration.appearance.chromeColors.backgroundHex ?? "nil") " + + "resultingTabFont=\(String(format: "%.3f", bonsplitController.configuration.appearance.tabTitleFontSize))" ) } } @@ -6873,9 +6884,11 @@ final class Workspace: Identifiable, ObservableObject { // and keep split entry instantaneous. // Use the cached Ghostty config so new workspaces inherit tab-strip sizing // without paying repeated parse costs on the workspace-creation hot path. + let initialSurfaceTabBarFontSize = GhosttyConfig.load().surfaceTabBarFontSize let appearance = Self.bonsplitAppearance( from: GhosttyApp.shared.defaultBackgroundColor, - backgroundOpacity: GhosttyApp.shared.defaultBackgroundOpacity + backgroundOpacity: GhosttyApp.shared.defaultBackgroundOpacity, + tabTitleFontSize: initialSurfaceTabBarFontSize ) let config = BonsplitConfiguration( allowSplits: true, diff --git a/cmuxTests/SessionPersistenceTests.swift b/cmuxTests/SessionPersistenceTests.swift index d3688d92513b..5723c0bddf9e 100644 --- a/cmuxTests/SessionPersistenceTests.swift +++ b/cmuxTests/SessionPersistenceTests.swift @@ -791,6 +791,117 @@ final class SessionPersistenceTests: XCTestCase { XCTAssertEqual(restored.height, 720, accuracy: 0.001) } + func testHasSufficientVisibleFrameRejectsDisjointSliversAcrossDisplays() { + // Two narrow strips, one on each display. Neither display individually + // sees enough of the window to be reachable, even though the bounding + // box of the two slivers is "wide". + let leftDisplay = AppDelegate.SessionDisplayGeometry( + displayID: 1, + frame: CGRect(x: 0, y: 0, width: 1_000, height: 800), + visibleFrame: CGRect(x: 0, y: 0, width: 1_000, height: 800) + ) + let rightDisplay = AppDelegate.SessionDisplayGeometry( + displayID: 2, + frame: CGRect(x: 2_000, y: 0, width: 1_000, height: 800), + visibleFrame: CGRect(x: 2_000, y: 0, width: 1_000, height: 800) + ) + // Frame straddles a 1000-wide gap between the two displays, leaving a + // 50pt strip on each side. + let stradlingFrame = CGRect(x: 950, y: 200, width: 1_100, height: 600) + + XCTAssertFalse( + AppDelegate.hasSufficientVisibleFrame( + stradlingFrame, + in: [leftDisplay, rightDisplay], + minWidth: 400, + minHeight: 300, + minimumVisibleWidth: 480, + minimumVisibleHeight: 320 + ) + ) + } + + func testHasSufficientVisibleFramePreservesSpanningWindow() { + // A genuinely spanning window across two side-by-side displays. At + // least one display sees a wide chunk of the window, so we should + // preserve it instead of forcing a centered fallback. + let leftDisplay = AppDelegate.SessionDisplayGeometry( + displayID: 1, + frame: CGRect(x: 0, y: 0, width: 1_000, height: 800), + visibleFrame: CGRect(x: 0, y: 0, width: 1_000, height: 800) + ) + let rightDisplay = AppDelegate.SessionDisplayGeometry( + displayID: 2, + frame: CGRect(x: 1_000, y: 0, width: 1_000, height: 800), + visibleFrame: CGRect(x: 1_000, y: 0, width: 1_000, height: 800) + ) + let spanningFrame = CGRect(x: 200, y: 100, width: 1_600, height: 600) + + XCTAssertTrue( + AppDelegate.hasSufficientVisibleFrame( + spanningFrame, + in: [leftDisplay, rightDisplay], + minWidth: 400, + minHeight: 300, + minimumVisibleWidth: 480, + minimumVisibleHeight: 320 + ) + ) + } + + func testMergedDisplayConfigurationsEvictsOldEntriesPastCap() { + // Pre-populate the map with one more than the cap, all under + // distinct fingerprints. The newly-written fingerprint must survive; + // the map size must be capped to maxStoredDisplayConfigurations. + let cap = AppDelegate.maxStoredDisplayConfigurations + var existing: [String: AppDelegate.PersistedWindowGeometry.StoredGeometry] = [:] + for index in 0.. Date: Wed, 8 Apr 2026 19:03:30 -0700 Subject: [PATCH 3/6] Fix remaining window restore review follow-ups --- Sources/AppDelegate.swift | 24 ++++++++----- cmuxTests/SessionPersistenceTests.swift | 46 +++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index 998c0dccf426..8fe41a39a010 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -4340,14 +4340,22 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent minHeight: minHeight, minimumVisibleWidth: minimumVisibleWidth, minimumVisibleHeight: minimumVisibleHeight - ), - let intersectingDisplay = bestIntersectingDisplay(for: frame, in: availableDisplays) { - return clampFrame( - frame, - within: intersectingDisplay.visibleFrame, - minWidth: minWidth, - minHeight: minHeight - ) + ) { + // When we have no originating display snapshot, the current frame + // is already usable on the active display set. Preserve it as-is + // so legitimate spanning windows are not collapsed into a single + // display during live screen-parameter changes. + if displaySnapshot == nil { + return frame + } + if let intersectingDisplay = bestIntersectingDisplay(for: frame, in: availableDisplays) { + return clampFrame( + frame, + within: intersectingDisplay.visibleFrame, + minWidth: minWidth, + minHeight: minHeight + ) + } } if let sourceReference = displaySnapshot?.visibleFrame?.cgRect ?? displaySnapshot?.frame?.cgRect, diff --git a/cmuxTests/SessionPersistenceTests.swift b/cmuxTests/SessionPersistenceTests.swift index 5723c0bddf9e..c78fb8ce7838 100644 --- a/cmuxTests/SessionPersistenceTests.swift +++ b/cmuxTests/SessionPersistenceTests.swift @@ -849,7 +849,7 @@ final class SessionPersistenceTests: XCTestCase { ) } - func testMergedDisplayConfigurationsEvictsOldEntriesPastCap() { + func testMergedDisplayConfigurationsEvictsOldEntriesPastCap() throws { // Pre-populate the map with one more than the cap, all under // distinct fingerprints. The newly-written fingerprint must survive; // the map size must be capped to maxStoredDisplayConfigurations. @@ -872,8 +872,11 @@ final class SessionPersistenceTests: XCTestCase { let resolved = merged ?? [:] XCTAssertLessThanOrEqual(resolved.count, cap) - XCTAssertNotNil(resolved[newFingerprint], "newest fingerprint must survive eviction") - XCTAssertEqual(resolved[newFingerprint]?.frame.width, 1_200, accuracy: 0.001) + let newestGeometry = try XCTUnwrap( + resolved[newFingerprint], + "newest fingerprint must survive eviction" + ) + XCTAssertEqual(newestGeometry.frame.width, 1_200, accuracy: 0.001) } func testMergedDisplayConfigurationsKeepsExistingEntriesUnderCap() throws { @@ -944,6 +947,43 @@ final class SessionPersistenceTests: XCTestCase { matchingOnly: true ) ) + let fallbackResolved = try XCTUnwrap( + AppDelegate.persistedWindowGeometryEntry( + from: payload, + displayConfigurationFingerprint: "missing-fingerprint" + ) + ) + XCTAssertEqual(fallbackResolved.frame.x, fallbackGeometry.x, accuracy: 0.001) + XCTAssertEqual(fallbackResolved.frame.y, fallbackGeometry.y, accuracy: 0.001) + XCTAssertEqual(fallbackResolved.frame.width, fallbackGeometry.width, accuracy: 0.001) + XCTAssertEqual(fallbackResolved.frame.height, fallbackGeometry.height, accuracy: 0.001) + } + + func testResolvedWindowFramePreservesVisibleSpanningGeometryWithoutDisplaySnapshot() throws { + let leftDisplay = AppDelegate.SessionDisplayGeometry( + displayID: 1, + frame: CGRect(x: 0, y: 0, width: 1_000, height: 800), + visibleFrame: CGRect(x: 0, y: 0, width: 1_000, height: 800) + ) + let rightDisplay = AppDelegate.SessionDisplayGeometry( + displayID: 2, + frame: CGRect(x: 1_000, y: 0, width: 1_000, height: 800), + visibleFrame: CGRect(x: 1_000, y: 0, width: 1_000, height: 800) + ) + let savedFrame = SessionRectSnapshot(x: 200, y: 100, width: 1_600, height: 600) + + let restored = AppDelegate.resolvedWindowFrame( + from: savedFrame, + display: nil, + availableDisplays: [leftDisplay, rightDisplay], + fallbackDisplay: leftDisplay + ) + + let restoredFrame = try XCTUnwrap(restored) + XCTAssertEqual(restoredFrame.minX, 200, accuracy: 0.001) + XCTAssertEqual(restoredFrame.minY, 100, accuracy: 0.001) + XCTAssertEqual(restoredFrame.width, 1_600, accuracy: 0.001) + XCTAssertEqual(restoredFrame.height, 600, accuracy: 0.001) } func testResolvedWindowFramePreservesExactGeometryWhenDisplayIsUnchanged() { From 1a1045a25c26166136196c573902f96861fcca77 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Thu, 9 Apr 2026 14:25:54 -0700 Subject: [PATCH 4/6] Address window restore PR review comments --- Sources/AppDelegate.swift | 117 +++++++++++++++++++++--- Sources/Workspace.swift | 78 ++++++++-------- cmuxTests/SessionPersistenceTests.swift | 89 ++++++++++++++++-- 3 files changed, 226 insertions(+), 58 deletions(-) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index 8fe41a39a010..e8eb2a78c060 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -2213,6 +2213,17 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent struct StoredGeometry: Codable, Sendable { let frame: SessionRectSnapshot let display: SessionDisplaySnapshot? + let lastUsedAt: Date? + + init( + frame: SessionRectSnapshot, + display: SessionDisplaySnapshot?, + lastUsedAt: Date? = nil + ) { + self.frame = frame + self.display = display + self.lastUsedAt = lastUsedAt + } } let version: Int @@ -2414,10 +2425,15 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent private var sessionAutosaveTimer: DispatchSourceTimer? private var sessionAutosaveTickInFlight = false private var sessionAutosaveDeferredRetryPending = false - private let sessionPersistenceQueue = DispatchQueue( - label: "com.cmuxterm.app.sessionPersistence", - qos: .utility - ) + private nonisolated static let sessionPersistenceQueueSpecificKey = DispatchSpecificKey() + private let sessionPersistenceQueue: DispatchQueue = { + let queue = DispatchQueue( + label: "com.cmuxterm.app.sessionPersistence", + qos: .utility + ) + queue.setSpecific(key: AppDelegate.sessionPersistenceQueueSpecificKey, value: 1) + return queue + }() private nonisolated static let launchServicesRegistrationQueue = DispatchQueue( label: "com.cmuxterm.app.launchServicesRegistration", qos: .utility @@ -3864,27 +3880,39 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } /// Merge a freshly-captured per-display-configuration entry into the - /// existing map and evict the oldest extras when we exceed the LRU cap. - /// The `fingerprint` entry (if any) is always preserved as the most - /// recent — only other entries are eligible for eviction. + /// existing map and evict the least-recently-used extras when we exceed + /// the cap. The `fingerprint` entry (if any) is always refreshed as the + /// most recent and never evicted by the same merge. nonisolated static func mergedDisplayConfigurations( existing: [String: PersistedWindowGeometry.StoredGeometry]?, fingerprint: String?, frame: SessionRectSnapshot, - display: SessionDisplaySnapshot? + display: SessionDisplaySnapshot?, + now: Date = Date() ) -> [String: PersistedWindowGeometry.StoredGeometry]? { var merged = existing ?? [:] if let fingerprint, !fingerprint.isEmpty { merged[fingerprint] = PersistedWindowGeometry.StoredGeometry( frame: frame, - display: display + display: display, + lastUsedAt: now ) } while merged.count > maxStoredDisplayConfigurations { - // Evict any entry other than the just-written fingerprint. We - // don't track true LRU order, so picking an arbitrary stale key - // is acceptable for an eight-slot bound. - let evictionKey = merged.keys.first { $0 != fingerprint } + let evictionKey = merged + .filter { entry in + guard let fingerprint else { return true } + return entry.key != fingerprint + } + .min { lhs, rhs in + let lhsLastUsedAt = lhs.value.lastUsedAt ?? .distantPast + let rhsLastUsedAt = rhs.value.lastUsedAt ?? .distantPast + if lhsLastUsedAt == rhsLastUsedAt { + return lhs.key < rhs.key + } + return lhsLastUsedAt < rhsLastUsedAt + }? + .key if let evictionKey { merged.removeValue(forKey: evictionKey) } else { @@ -4043,7 +4071,18 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent targetFrame.size.width = max(targetFrame.width, minimumFrameSize.width) targetFrame.size.height = max(targetFrame.height, minimumFrameSize.height) - if let screen = Self.screenForConstrainingFrame(targetFrame, currentWindow: window) { + let availableDisplays = currentDisplayGeometries().available + let shouldPreserveSpanningFrame = Self.shouldPreserveSpanningFrame( + targetFrame, + availableDisplays: availableDisplays, + minWidth: minimumFrameSize.width, + minHeight: minimumFrameSize.height, + minimumVisibleWidth: Self.minimumVisibleRestoredWindowWidth, + minimumVisibleHeight: max(Self.minimumVisibleRestoredWindowHeight, minimumFrameSize.height) + ) + + if !shouldPreserveSpanningFrame, + let screen = Self.screenForConstrainingFrame(targetFrame, currentWindow: window) { targetFrame = window.constrainFrameRect(targetFrame, to: screen) if targetFrame.width < minimumFrameSize.width || targetFrame.height < minimumFrameSize.height { targetFrame.size.width = max(targetFrame.width, minimumFrameSize.width) @@ -4358,6 +4397,19 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } } + if displaySnapshot == nil, + frame.width >= minWidth, + frame.height >= minHeight, + !intersectsAnyDisplay(frame, in: availableDisplays), + let fallbackTargetDisplay { + return centeredFrame( + frame, + in: fallbackTargetDisplay.visibleFrame, + minWidth: minWidth, + minHeight: minHeight + ) + } + if let sourceReference = displaySnapshot?.visibleFrame?.cgRect ?? displaySnapshot?.frame?.cgRect, let fallbackTargetDisplay { let remapped = remappedFrame( @@ -4472,6 +4524,37 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent return bestOverlap.display } + private nonisolated static func intersectsAnyDisplay( + _ frame: CGRect, + in displays: [SessionDisplayGeometry] + ) -> Bool { + displays.contains { display in + intersectionArea(frame, display.visibleFrame) > 0 + } + } + + nonisolated static func shouldPreserveSpanningFrame( + _ frame: CGRect, + availableDisplays: [SessionDisplayGeometry], + minWidth: CGFloat, + minHeight: CGFloat, + minimumVisibleWidth: CGFloat, + minimumVisibleHeight: CGFloat + ) -> Bool { + let intersectingDisplays = availableDisplays.filter { display in + intersectionArea(frame, display.visibleFrame) > 0 + } + guard intersectingDisplays.count > 1 else { return false } + return hasSufficientVisibleFrame( + frame, + in: intersectingDisplays, + minWidth: minWidth, + minHeight: minHeight, + minimumVisibleWidth: minimumVisibleWidth, + minimumVisibleHeight: minimumVisibleHeight + ) + } + nonisolated static func hasSufficientVisibleFrame( _ frame: CGRect, in displays: [SessionDisplayGeometry], @@ -5141,7 +5224,11 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } if synchronously { - writeBlock() + if DispatchQueue.getSpecific(key: Self.sessionPersistenceQueueSpecificKey) != nil { + writeBlock() + } else { + sessionPersistenceQueue.sync(execute: writeBlock) + } } else { sessionPersistenceQueue.async(execute: writeBlock) } diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 87ebe6372a2a..bc7223bdc2cc 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -6794,24 +6794,51 @@ final class Workspace: Identifiable, ObservableObject { } func applyGhosttyChrome(from config: GhosttyConfig, reason: String = "unspecified") { - let nextHex = Self.bonsplitChromeHex( + applyGhosttyChrome( backgroundColor: config.backgroundColor, - backgroundOpacity: config.backgroundOpacity + backgroundOpacity: config.backgroundOpacity, + tabTitleFontSize: config.surfaceTabBarFontSize, + reason: reason + ) + } + + func applyGhosttyChrome(backgroundColor: NSColor, backgroundOpacity: Double, reason: String = "unspecified") { + applyGhosttyChrome( + backgroundColor: backgroundColor, + backgroundOpacity: backgroundOpacity, + tabTitleFontSize: nil, + reason: reason + ) + } + + private func applyGhosttyChrome( + backgroundColor: NSColor, + backgroundOpacity: Double, + tabTitleFontSize: CGFloat?, + reason: String + ) { + let nextHex = Self.bonsplitChromeHex( + backgroundColor: backgroundColor, + backgroundOpacity: backgroundOpacity ) - let nextTabTitleFontSize = config.surfaceTabBarFontSize let currentAppearance = bonsplitController.configuration.appearance let currentBackgroundHex = currentAppearance.chromeColors.backgroundHex let currentTabTitleFontSize = currentAppearance.tabTitleFontSize let backgroundChanged = currentBackgroundHex != nextHex - let fontSizeChanged = abs(currentTabTitleFontSize - nextTabTitleFontSize) > 0.0001 + let fontSizeChanged = tabTitleFontSize.map { + abs(currentTabTitleFontSize - $0) > 0.0001 + } ?? false let isNoOp = !backgroundChanged && !fontSizeChanged if GhosttyApp.shared.backgroundLogEnabled { + let tabFontLog = tabTitleFontSize.map { nextTabTitleFontSize in + " currentTabFont=\(String(format: "%.3f", currentTabTitleFontSize)) " + + "nextTabFont=\(String(format: "%.3f", nextTabTitleFontSize))" + } ?? "" GhosttyApp.shared.logBackground( "theme apply workspace=\(id.uuidString) reason=\(reason) " + - "currentBg=\(currentBackgroundHex ?? "nil") nextBg=\(nextHex) " + - "currentTabFont=\(String(format: "%.3f", currentTabTitleFontSize)) " + - "nextTabFont=\(String(format: "%.3f", nextTabTitleFontSize)) noop=\(isNoOp)" + "currentBg=\(currentBackgroundHex ?? "nil") nextBg=\(nextHex)" + + "\(tabFontLog) noop=\(isNoOp)" ) } @@ -6820,41 +6847,18 @@ final class Workspace: Identifiable, ObservableObject { if backgroundChanged { bonsplitController.configuration.appearance.chromeColors.backgroundHex = nextHex } - if fontSizeChanged { - bonsplitController.configuration.appearance.tabTitleFontSize = nextTabTitleFontSize + if let tabTitleFontSize, fontSizeChanged { + bonsplitController.configuration.appearance.tabTitleFontSize = tabTitleFontSize } if GhosttyApp.shared.backgroundLogEnabled { + let resultingTabFontLog = tabTitleFontSize.map { _ in + " resultingTabFont=\(String(format: "%.3f", bonsplitController.configuration.appearance.tabTitleFontSize))" + } ?? "" GhosttyApp.shared.logBackground( "theme applied workspace=\(id.uuidString) reason=\(reason) " + - "resultingBg=\(bonsplitController.configuration.appearance.chromeColors.backgroundHex ?? "nil") " + - "resultingTabFont=\(String(format: "%.3f", bonsplitController.configuration.appearance.tabTitleFontSize))" - ) - } - } - - func applyGhosttyChrome(backgroundColor: NSColor, backgroundOpacity: Double, reason: String = "unspecified") { - let nextHex = Self.bonsplitChromeHex( - backgroundColor: backgroundColor, - backgroundOpacity: backgroundOpacity - ) - let currentChromeColors = bonsplitController.configuration.appearance.chromeColors - let isNoOp = currentChromeColors.backgroundHex == nextHex - - if GhosttyApp.shared.backgroundLogEnabled { - let currentBackgroundHex = currentChromeColors.backgroundHex ?? "nil" - GhosttyApp.shared.logBackground( - "theme apply workspace=\(id.uuidString) reason=\(reason) currentBg=\(currentBackgroundHex) nextBg=\(nextHex) noop=\(isNoOp)" - ) - } - - if isNoOp { - return - } - bonsplitController.configuration.appearance.chromeColors.backgroundHex = nextHex - if GhosttyApp.shared.backgroundLogEnabled { - GhosttyApp.shared.logBackground( - "theme applied workspace=\(id.uuidString) reason=\(reason) resultingBg=\(bonsplitController.configuration.appearance.chromeColors.backgroundHex ?? "nil")" + "resultingBg=\(bonsplitController.configuration.appearance.chromeColors.backgroundHex ?? "nil")" + + "\(resultingTabFontLog)" ) } } diff --git a/cmuxTests/SessionPersistenceTests.swift b/cmuxTests/SessionPersistenceTests.swift index c78fb8ce7838..8750cf1ca1df 100644 --- a/cmuxTests/SessionPersistenceTests.swift +++ b/cmuxTests/SessionPersistenceTests.swift @@ -745,6 +745,29 @@ final class SessionPersistenceTests: XCTestCase { XCTAssertEqual(restored.height, 720, accuracy: 0.001) } + func testResolvedWindowFrameCentersOffscreenValidWindowWithoutInflatingSize() { + let savedFrame = SessionRectSnapshot(x: 4_000, y: 4_000, width: 500, height: 400) + let display = AppDelegate.SessionDisplayGeometry( + displayID: 1, + frame: CGRect(x: 0, y: 0, width: 1_000, height: 800), + visibleFrame: CGRect(x: 0, y: 0, width: 1_000, height: 800) + ) + + let restored = AppDelegate.resolvedWindowFrame( + from: savedFrame, + display: nil, + availableDisplays: [display], + fallbackDisplay: display + ) + + XCTAssertNotNil(restored) + guard let restored else { return } + XCTAssertEqual(restored.minX, 250, accuracy: 0.001) + XCTAssertEqual(restored.minY, 200, accuracy: 0.001) + XCTAssertEqual(restored.width, 500, accuracy: 0.001) + XCTAssertEqual(restored.height, 400, accuracy: 0.001) + } + func testResolvedWindowFrameFallsBackToCenteredDefaultWhenOnlySliverIsVisible() { let savedFrame = SessionRectSnapshot(x: -850, y: 60, width: 900, height: 700) let display = AppDelegate.SessionDisplayGeometry( @@ -807,11 +830,11 @@ final class SessionPersistenceTests: XCTestCase { ) // Frame straddles a 1000-wide gap between the two displays, leaving a // 50pt strip on each side. - let stradlingFrame = CGRect(x: 950, y: 200, width: 1_100, height: 600) + let straddlingFrame = CGRect(x: 950, y: 200, width: 1_100, height: 600) XCTAssertFalse( AppDelegate.hasSufficientVisibleFrame( - stradlingFrame, + straddlingFrame, in: [leftDisplay, rightDisplay], minWidth: 400, minHeight: 300, @@ -849,16 +872,42 @@ final class SessionPersistenceTests: XCTestCase { ) } + func testShouldPreserveSpanningFrameAcrossMultipleDisplays() { + let leftDisplay = AppDelegate.SessionDisplayGeometry( + displayID: 1, + frame: CGRect(x: 0, y: 0, width: 1_000, height: 800), + visibleFrame: CGRect(x: 0, y: 0, width: 1_000, height: 800) + ) + let rightDisplay = AppDelegate.SessionDisplayGeometry( + displayID: 2, + frame: CGRect(x: 1_000, y: 0, width: 1_000, height: 800), + visibleFrame: CGRect(x: 1_000, y: 0, width: 1_000, height: 800) + ) + let spanningFrame = CGRect(x: 200, y: 100, width: 1_600, height: 600) + + XCTAssertTrue( + AppDelegate.shouldPreserveSpanningFrame( + spanningFrame, + availableDisplays: [leftDisplay, rightDisplay], + minWidth: 400, + minHeight: 300, + minimumVisibleWidth: 480, + minimumVisibleHeight: 320 + ) + ) + } + func testMergedDisplayConfigurationsEvictsOldEntriesPastCap() throws { - // Pre-populate the map with one more than the cap, all under + // Pre-populate the map with exactly the cap entries, all under // distinct fingerprints. The newly-written fingerprint must survive; - // the map size must be capped to maxStoredDisplayConfigurations. + // the least-recently-used entry should be evicted to preserve the cap. let cap = AppDelegate.maxStoredDisplayConfigurations var existing: [String: AppDelegate.PersistedWindowGeometry.StoredGeometry] = [:] for index in 0.. Date: Thu, 9 Apr 2026 14:59:26 -0700 Subject: [PATCH 5/6] Fix window restore review regressions --- Sources/AppDelegate.swift | 79 +++++++++----- cmuxTests/SessionPersistenceTests.swift | 135 +++++++++++++++++++++++- 2 files changed, 183 insertions(+), 31 deletions(-) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index e8eb2a78c060..fc1b09c11d76 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -3963,29 +3963,21 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent matchingOnly: true ) var didReconcilePrimary = false + let contexts = Array(mainWindowContexts.values) - for context in mainWindowContexts.values { + for context in contexts { guard let window = resolvedWindow(for: context), shouldReconcileMainWindowFrameOnScreenParameterChange(window) else { continue } - let resolvedFrame: CGRect? = { - if window === primaryWindow, let matchingPersistedGeometry { - return Self.resolvedWindowFrame( - from: matchingPersistedGeometry.frame, - display: matchingPersistedGeometry.display, - availableDisplays: displays.available, - fallbackDisplay: displays.fallback - ) - } - return Self.resolvedWindowFrame( - from: SessionRectSnapshot(window.frame), - display: nil, - availableDisplays: displays.available, - fallbackDisplay: displays.fallback - ) - }() + let resolvedFrame = Self.resolvedWindowFrameForScreenParameterChange( + currentFrame: window.frame, + currentDisplaySnapshot: displaySnapshot(for: window), + matchingPersistedGeometry: window === primaryWindow ? matchingPersistedGeometry : nil, + availableDisplays: displays.available, + fallbackDisplay: displays.fallback + ) guard let resolvedFrame else { continue } @@ -4080,8 +4072,15 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent minimumVisibleWidth: Self.minimumVisibleRestoredWindowWidth, minimumVisibleHeight: max(Self.minimumVisibleRestoredWindowHeight, minimumFrameSize.height) ) + let shouldPreserveAccessibleSingleDisplayFrame = !shouldPreserveSpanningFrame + && ( + Self.bestIntersectingDisplay(for: targetFrame, in: availableDisplays).map { + Self.shouldPreserveAccessibleFrame(frame: targetFrame, targetDisplay: $0) + } ?? false + ) if !shouldPreserveSpanningFrame, + !shouldPreserveAccessibleSingleDisplayFrame, let screen = Self.screenForConstrainingFrame(targetFrame, currentWindow: window) { targetFrame = window.constrainFrameRect(targetFrame, to: screen) if targetFrame.width < minimumFrameSize.width || targetFrame.height < minimumFrameSize.height { @@ -4304,6 +4303,36 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent ) } + nonisolated static func resolvedWindowFrameForScreenParameterChange( + currentFrame: CGRect, + currentDisplaySnapshot: SessionDisplaySnapshot?, + matchingPersistedGeometry: PersistedWindowGeometry.StoredGeometry?, + availableDisplays: [SessionDisplayGeometry], + fallbackDisplay: SessionDisplayGeometry? + ) -> CGRect? { + let liveResolved = resolvedWindowFrame( + from: SessionRectSnapshot(currentFrame), + display: currentDisplaySnapshot, + availableDisplays: availableDisplays, + fallbackDisplay: fallbackDisplay + ) + + // Prefer the live frame whenever the window still belongs to a current + // display. That avoids snapping a visible window back to older persisted + // geometry during didChangeScreenParameters. + guard currentDisplaySnapshot == nil, + let matchingPersistedGeometry else { + return liveResolved + } + + return resolvedWindowFrame( + from: matchingPersistedGeometry.frame, + display: matchingPersistedGeometry.display, + availableDisplays: availableDisplays, + fallbackDisplay: fallbackDisplay + ) ?? liveResolved + } + nonisolated static func resolvedWindowFrame( from frameSnapshot: SessionRectSnapshot?, display displaySnapshot: SessionDisplaySnapshot?, @@ -4456,6 +4485,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent minimumVisibleWidth: CGFloat, minimumVisibleHeight: CGFloat ) -> CGRect { + if shouldPreserveAccessibleFrame( + frame: frame, + targetDisplay: targetDisplay + ) { + return frame + } + if hasSufficientVisibleFrame( frame, in: [targetDisplay], @@ -4464,15 +4500,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent minimumVisibleWidth: minimumVisibleWidth, minimumVisibleHeight: minimumVisibleHeight ) { - // Preserve the user's exact frame when enough of the top of the window - // remains reachable on-screen; only clamp when the saved frame would - // reopen with an inaccessible titlebar/top strip. - if shouldPreserveAccessibleFrame( - frame: frame, - targetDisplay: targetDisplay - ) { - return frame - } return clampFrame( frame, within: targetDisplay.visibleFrame, diff --git a/cmuxTests/SessionPersistenceTests.swift b/cmuxTests/SessionPersistenceTests.swift index 8750cf1ca1df..325ed54c41b9 100644 --- a/cmuxTests/SessionPersistenceTests.swift +++ b/cmuxTests/SessionPersistenceTests.swift @@ -739,10 +739,66 @@ final class SessionPersistenceTests: XCTestCase { XCTAssertNotNil(restored) guard let restored else { return } XCTAssertTrue(display.visibleFrame.contains(restored)) - XCTAssertEqual(restored.minX, 40, accuracy: 0.001) - XCTAssertEqual(restored.minY, 40, accuracy: 0.001) - XCTAssertEqual(restored.width, 920, accuracy: 0.001) - XCTAssertEqual(restored.height, 720, accuracy: 0.001) + XCTAssertEqual(restored.minX, 50, accuracy: 0.001) + XCTAssertEqual(restored.minY, 50, accuracy: 0.001) + XCTAssertEqual(restored.width, 900, accuracy: 0.001) + XCTAssertEqual(restored.height, 700, accuracy: 0.001) + } + + func testResolvedWindowFramePreservesReachableSameDisplayFrameBelowVisibilityThreshold() { + let display = AppDelegate.SessionDisplayGeometry( + displayID: 1, + frame: CGRect(x: 0, y: 0, width: 1_000, height: 800), + visibleFrame: CGRect(x: 0, y: 0, width: 1_000, height: 800) + ) + let displaySnapshot = SessionDisplaySnapshot( + displayID: 1, + frame: SessionRectSnapshot(x: 0, y: 0, width: 1_000, height: 800), + visibleFrame: SessionRectSnapshot(x: 0, y: 0, width: 1_000, height: 800) + ) + let savedFrame = SessionRectSnapshot(x: -380, y: 120, width: 500, height: 400) + + let restored = AppDelegate.resolvedWindowFrame( + from: savedFrame, + display: displaySnapshot, + availableDisplays: [display], + fallbackDisplay: display + ) + + XCTAssertNotNil(restored) + guard let restored else { return } + XCTAssertEqual(restored.minX, -380, accuracy: 0.001) + XCTAssertEqual(restored.minY, 120, accuracy: 0.001) + XCTAssertEqual(restored.width, 500, accuracy: 0.001) + XCTAssertEqual(restored.height, 400, accuracy: 0.001) + } + + func testResolvedWindowFrameKeepsSmallReachableWindowOnCurrentDisplay() { + let display = AppDelegate.SessionDisplayGeometry( + displayID: 1, + frame: CGRect(x: 0, y: 0, width: 1_000, height: 800), + visibleFrame: CGRect(x: 0, y: 50, width: 1_000, height: 750) + ) + let displaySnapshot = SessionDisplaySnapshot( + displayID: 1, + frame: SessionRectSnapshot(x: 0, y: 0, width: 1_000, height: 800), + visibleFrame: SessionRectSnapshot(x: 0, y: 50, width: 1_000, height: 750) + ) + let savedFrame = SessionRectSnapshot(x: 120, y: 20, width: 420, height: 300) + + let restored = AppDelegate.resolvedWindowFrame( + from: savedFrame, + display: displaySnapshot, + availableDisplays: [display], + fallbackDisplay: display + ) + + XCTAssertNotNil(restored) + guard let restored else { return } + XCTAssertEqual(restored.minX, 120, accuracy: 0.001) + XCTAssertEqual(restored.minY, 20, accuracy: 0.001) + XCTAssertEqual(restored.width, 420, accuracy: 0.001) + XCTAssertEqual(restored.height, 300, accuracy: 0.001) } func testResolvedWindowFrameCentersOffscreenValidWindowWithoutInflatingSize() { @@ -768,6 +824,71 @@ final class SessionPersistenceTests: XCTestCase { XCTAssertEqual(restored.height, 400, accuracy: 0.001) } + func testResolvedWindowFrameForScreenParameterChangePrefersLivePrimaryGeometry() { + let display = AppDelegate.SessionDisplayGeometry( + displayID: 1, + frame: CGRect(x: 0, y: 0, width: 1_400, height: 900), + visibleFrame: CGRect(x: 0, y: 0, width: 1_400, height: 900) + ) + let currentFrame = CGRect(x: 260, y: 180, width: 840, height: 620) + let liveDisplaySnapshot = SessionDisplaySnapshot( + displayID: 1, + frame: SessionRectSnapshot(x: 0, y: 0, width: 1_400, height: 900), + visibleFrame: SessionRectSnapshot(x: 0, y: 0, width: 1_400, height: 900) + ) + let matchingPersistedGeometry = AppDelegate.PersistedWindowGeometry.StoredGeometry( + frame: SessionRectSnapshot(x: 80, y: 60, width: 1_000, height: 700), + display: liveDisplaySnapshot + ) + + let restored = AppDelegate.resolvedWindowFrameForScreenParameterChange( + currentFrame: currentFrame, + currentDisplaySnapshot: liveDisplaySnapshot, + matchingPersistedGeometry: matchingPersistedGeometry, + availableDisplays: [display], + fallbackDisplay: display + ) + + XCTAssertNotNil(restored) + guard let restored else { return } + XCTAssertEqual(restored.minX, currentFrame.minX, accuracy: 0.001) + XCTAssertEqual(restored.minY, currentFrame.minY, accuracy: 0.001) + XCTAssertEqual(restored.width, currentFrame.width, accuracy: 0.001) + XCTAssertEqual(restored.height, currentFrame.height, accuracy: 0.001) + } + + func testResolvedWindowFrameForScreenParameterChangeFallsBackToMatchingPrimaryGeometryWhenOffscreen() { + let display = AppDelegate.SessionDisplayGeometry( + displayID: 1, + frame: CGRect(x: 0, y: 0, width: 1_400, height: 900), + visibleFrame: CGRect(x: 0, y: 0, width: 1_400, height: 900) + ) + let displaySnapshot = SessionDisplaySnapshot( + displayID: 1, + frame: SessionRectSnapshot(x: 0, y: 0, width: 1_400, height: 900), + visibleFrame: SessionRectSnapshot(x: 0, y: 0, width: 1_400, height: 900) + ) + let matchingPersistedGeometry = AppDelegate.PersistedWindowGeometry.StoredGeometry( + frame: SessionRectSnapshot(x: 120, y: 100, width: 960, height: 720), + display: displaySnapshot + ) + + let restored = AppDelegate.resolvedWindowFrameForScreenParameterChange( + currentFrame: CGRect(x: 4_000, y: 4_000, width: 900, height: 700), + currentDisplaySnapshot: nil, + matchingPersistedGeometry: matchingPersistedGeometry, + availableDisplays: [display], + fallbackDisplay: display + ) + + XCTAssertNotNil(restored) + guard let restored else { return } + XCTAssertEqual(restored.minX, 120, accuracy: 0.001) + XCTAssertEqual(restored.minY, 100, accuracy: 0.001) + XCTAssertEqual(restored.width, 960, accuracy: 0.001) + XCTAssertEqual(restored.height, 720, accuracy: 0.001) + } + func testResolvedWindowFrameFallsBackToCenteredDefaultWhenOnlySliverIsVisible() { let savedFrame = SessionRectSnapshot(x: -850, y: 60, width: 900, height: 700) let display = AppDelegate.SessionDisplayGeometry( @@ -921,7 +1042,11 @@ final class SessionPersistenceTests: XCTestCase { ) let resolved = merged ?? [:] - XCTAssertLessThanOrEqual(resolved.count, cap) + XCTAssertEqual( + resolved.count, + cap, + "Merging at capacity should evict exactly one least-recently-used entry" + ) XCTAssertNil(resolved["existing-0"]) let newestGeometry = try XCTUnwrap( resolved[newFingerprint], From 8feca448a621fd825b8dbf667650f3c292da6ec4 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Thu, 9 Apr 2026 17:21:44 -0700 Subject: [PATCH 6/6] Restore saved geometry for sliver-visible windows --- Sources/AppDelegate.swift | 27 ++++++++-- cmuxTests/SessionPersistenceTests.swift | 65 +++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index fc1b09c11d76..8ace276a77c1 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -4310,17 +4310,36 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent availableDisplays: [SessionDisplayGeometry], fallbackDisplay: SessionDisplayGeometry? ) -> CGRect? { + let minWidth = CGFloat(SessionPersistencePolicy.minimumWindowWidth) + let minHeight = CGFloat(SessionPersistencePolicy.minimumWindowHeight) + let minimumVisibleWidth = Self.minimumVisibleRestoredWindowWidth + let minimumVisibleHeight = max(Self.minimumVisibleRestoredWindowHeight, minHeight) let liveResolved = resolvedWindowFrame( from: SessionRectSnapshot(currentFrame), display: currentDisplaySnapshot, availableDisplays: availableDisplays, fallbackDisplay: fallbackDisplay ) + let liveFrameIsUsable = hasSufficientVisibleFrame( + currentFrame, + in: availableDisplays, + minWidth: minWidth, + minHeight: minHeight, + minimumVisibleWidth: minimumVisibleWidth, + minimumVisibleHeight: minimumVisibleHeight + ) || display( + for: currentDisplaySnapshot, + in: availableDisplays + ).map { + shouldPreserveAccessibleFrame(frame: currentFrame, targetDisplay: $0) + } ?? false - // Prefer the live frame whenever the window still belongs to a current - // display. That avoids snapping a visible window back to older persisted - // geometry during didChangeScreenParameters. - guard currentDisplaySnapshot == nil, + // Prefer the live frame only when it is genuinely usable on the current + // display set. A sliver intersection is not enough: in dock/undock + // flows AppKit can leave a thin strip on-screen, and re-saving that + // clamped sliver would overwrite the display-specific geometry we want + // to restore. + guard !liveFrameIsUsable, let matchingPersistedGeometry else { return liveResolved } diff --git a/cmuxTests/SessionPersistenceTests.swift b/cmuxTests/SessionPersistenceTests.swift index 325ed54c41b9..2cf8adcdbd7d 100644 --- a/cmuxTests/SessionPersistenceTests.swift +++ b/cmuxTests/SessionPersistenceTests.swift @@ -889,6 +889,71 @@ final class SessionPersistenceTests: XCTestCase { XCTAssertEqual(restored.height, 720, accuracy: 0.001) } + func testResolvedWindowFrameForScreenParameterChangeFallsBackToMatchingPrimaryGeometryWhenCurrentFrameIsOnlySliverVisible() { + let display = AppDelegate.SessionDisplayGeometry( + displayID: 1, + frame: CGRect(x: 0, y: 0, width: 1_400, height: 900), + visibleFrame: CGRect(x: 0, y: 0, width: 1_400, height: 900) + ) + let displaySnapshot = SessionDisplaySnapshot( + displayID: 1, + frame: SessionRectSnapshot(x: 0, y: 0, width: 1_400, height: 900), + visibleFrame: SessionRectSnapshot(x: 0, y: 0, width: 1_400, height: 900) + ) + let matchingPersistedGeometry = AppDelegate.PersistedWindowGeometry.StoredGeometry( + frame: SessionRectSnapshot(x: 180, y: 120, width: 980, height: 740), + display: displaySnapshot + ) + + let restored = AppDelegate.resolvedWindowFrameForScreenParameterChange( + currentFrame: CGRect(x: -930, y: 100, width: 980, height: 740), + currentDisplaySnapshot: displaySnapshot, + matchingPersistedGeometry: matchingPersistedGeometry, + availableDisplays: [display], + fallbackDisplay: display + ) + + XCTAssertNotNil(restored) + guard let restored else { return } + XCTAssertEqual(restored.minX, 180, accuracy: 0.001) + XCTAssertEqual(restored.minY, 120, accuracy: 0.001) + XCTAssertEqual(restored.width, 980, accuracy: 0.001) + XCTAssertEqual(restored.height, 740, accuracy: 0.001) + } + + func testResolvedWindowFrameForScreenParameterChangePreservesAccessibleCurrentFrame() { + let display = AppDelegate.SessionDisplayGeometry( + displayID: 1, + frame: CGRect(x: 0, y: 0, width: 1_000, height: 800), + visibleFrame: CGRect(x: 0, y: 0, width: 1_000, height: 800) + ) + let displaySnapshot = SessionDisplaySnapshot( + displayID: 1, + frame: SessionRectSnapshot(x: 0, y: 0, width: 1_000, height: 800), + visibleFrame: SessionRectSnapshot(x: 0, y: 0, width: 1_000, height: 800) + ) + let currentFrame = CGRect(x: -380, y: 120, width: 500, height: 400) + let matchingPersistedGeometry = AppDelegate.PersistedWindowGeometry.StoredGeometry( + frame: SessionRectSnapshot(x: 160, y: 100, width: 900, height: 700), + display: displaySnapshot + ) + + let restored = AppDelegate.resolvedWindowFrameForScreenParameterChange( + currentFrame: currentFrame, + currentDisplaySnapshot: displaySnapshot, + matchingPersistedGeometry: matchingPersistedGeometry, + availableDisplays: [display], + fallbackDisplay: display + ) + + XCTAssertNotNil(restored) + guard let restored else { return } + XCTAssertEqual(restored.minX, currentFrame.minX, accuracy: 0.001) + XCTAssertEqual(restored.minY, currentFrame.minY, accuracy: 0.001) + XCTAssertEqual(restored.width, currentFrame.width, accuracy: 0.001) + XCTAssertEqual(restored.height, currentFrame.height, accuracy: 0.001) + } + func testResolvedWindowFrameFallsBackToCenteredDefaultWhenOnlySliverIsVisible() { let savedFrame = SessionRectSnapshot(x: -850, y: 60, width: 900, height: 700) let display = AppDelegate.SessionDisplayGeometry(