macos: use stable display UUID for quick terminal screen tracking (#9256)

mitchellh · web-flow · commit dffa4f4fc737 · 2025-10-17T21:14:56.000-07:00
NSScreen instances can be garbage collected at any time, even for
screens that remain connected, making NSMapTable with weak keys
unreliable for tracking per-screen state.

This changes the quick terminal to use CGDisplay UUIDs as stable
identifiers, keyed in a strong dictionary. Each entry stores the window
frame along with screen dimensions, scale factor, and last-seen
timestamp.

**This should make quick terminal size restore more stable than 1.2.2.**

Rules for pruning:
- Entries are invalidated when screens shrink or change scale
- Entries persist and update when screens grow (allowing cached state to
work with larger resolutions)
- Stale entries for disconnected screens expire after 14 days.
- Maximum of 10 screen entries to prevent unbounded growth
diff --git a/macos/Ghostty.xcodeproj/project.pbxproj b/macos/Ghostty.xcodeproj/project.pbxproj
@@ -96,6 +96,7 @@
 				Features/QuickTerminal/QuickTerminalController.swift,
 				Features/QuickTerminal/QuickTerminalPosition.swift,
 				Features/QuickTerminal/QuickTerminalScreen.swift,
+				Features/QuickTerminal/QuickTerminalScreenStateCache.swift,
 				Features/QuickTerminal/QuickTerminalSize.swift,
 				Features/QuickTerminal/QuickTerminalSpaceBehavior.swift,
 				Features/QuickTerminal/QuickTerminalWindow.swift,
diff --git a/macos/Sources/Features/QuickTerminal/QuickTerminalController.swift b/macos/Sources/Features/QuickTerminal/QuickTerminalController.swift
@@ -21,13 +21,8 @@ class QuickTerminalController: BaseTerminalController {
     // The active space when the quick terminal was last shown.
     private var previousActiveSpace: CGSSpace? = nil
 
-    /// The saved state when the quick terminal's surface tree becomes empty.
-    ///
-    /// This preserves the user's window size and position when all terminal surfaces
-    /// are closed (e.g., via the `exit` command). When a new surface is created,
-    /// the window will be restored to this frame, preventing SwiftUI from resetting
-    /// the window to its default minimum size.
-    private var lastClosedFrames: NSMapTable<NSScreen, LastClosedState>
+    /// Cache for per-screen window state.
+    private let screenStateCache = QuickTerminalScreenStateCache()
 
     /// Non-nil if we have hidden dock state.
     private var hiddenDock: HiddenDock? = nil
@@ -45,10 +40,6 @@ class QuickTerminalController: BaseTerminalController {
     ) {
         self.position = position
         self.derivedConfig = DerivedConfig(ghostty.config)
-        
-        // This is a weak to strong mapping, so that our keys being NSScreens
-        // can remove themselves when they disappear.
-        self.lastClosedFrames = .weakToStrongObjects()
 
         // Important detail here: we initialize with an empty surface tree so
         // that we don't start a terminal process. This gets started when the
@@ -379,17 +370,15 @@ class QuickTerminalController: BaseTerminalController {
     private func animateWindowIn(window: NSWindow, from position: QuickTerminalPosition) {
         guard let screen = derivedConfig.quickTerminalScreen.screen else { return }
         
-        // Grab our last closed frame to use, and clear our state since we're animating in.
-        // We only use the last closed frame if we're opening on the same screen.
-        let lastClosedFrame: NSRect? = lastClosedFrames.object(forKey: screen)?.frame
-        lastClosedFrames.removeObject(forKey: screen)
+        // Grab our last closed frame to use from the cache.
+        let closedFrame = screenStateCache.frame(for: screen)
 
         // Move our window off screen to the initial animation position.
         position.setInitial(
             in: window,
             on: screen,
             terminalSize: derivedConfig.quickTerminalSize,
-            closedFrame: lastClosedFrame)
+            closedFrame: closedFrame)
 
         // We need to set our window level to a high value. In testing, only
         // popUpMenu and above do what we want. This gets it above the menu bar
@@ -424,7 +413,7 @@ class QuickTerminalController: BaseTerminalController {
                 in: window.animator(),
                 on: screen,
                 terminalSize: derivedConfig.quickTerminalSize,
-                closedFrame: lastClosedFrame)
+                closedFrame: closedFrame)
         }, completionHandler: {
             // There is a very minor delay here so waiting at least an event loop tick
             // keeps us safe from the view not being on the window.
@@ -513,7 +502,7 @@ class QuickTerminalController: BaseTerminalController {
         // terminal is reactivated with a new surface. Without this, SwiftUI
         // would reset the window to its minimum content size.
         if window.frame.width > 0 && window.frame.height > 0, let screen = window.screen {
-            lastClosedFrames.setObject(.init(frame: window.frame), forKey: screen)
+            screenStateCache.save(frame: window.frame, for: screen)
         }
 
         // If we hid the dock then we unhide it.
@@ -598,7 +587,6 @@ class QuickTerminalController: BaseTerminalController {
         alert.alertStyle = .warning
         alert.beginSheetModal(for: window)
     }
-
     // MARK: First Responder
 
     @IBAction override func closeWindow(_ sender: Any) {
@@ -736,14 +724,6 @@ class QuickTerminalController: BaseTerminalController {
             hidden = false
         }
     }
-    
-    private class LastClosedState {
-        let frame: NSRect
-        
-        init(frame: NSRect) {
-            self.frame = frame
-        }
-    }
 }
 
 extension Notification.Name {
diff --git a/macos/Sources/Features/QuickTerminal/QuickTerminalScreenStateCache.swift b/macos/Sources/Features/QuickTerminal/QuickTerminalScreenStateCache.swift
@@ -0,0 +1,113 @@
+import Foundation
+import Cocoa
+
+/// Manages cached window state per screen for the quick terminal.
+///
+/// This cache tracks the last closed window frame for each screen, allowing the quick terminal
+/// to restore to its previous size and position when reopened. It uses stable display UUIDs
+/// to survive NSScreen garbage collection and automatically prunes stale entries.
+class QuickTerminalScreenStateCache {
+    /// The maximum number of saved screen states we retain. This is to avoid some kind of
+    /// pathological memory growth in case we get our screen state serializing wrong. I don't
+    /// know anyone with more than 10 screens, so let's just arbitrarily go with that.
+    private static let maxSavedScreens = 10
+    
+    /// Time-to-live for screen entries that are no longer present (14 days).
+    private static let screenStaleTTL: TimeInterval = 14 * 24 * 60 * 60
+    
+    /// Keyed by display UUID to survive NSScreen garbage collection.
+    private var stateByDisplay: [UUID: DisplayEntry] = [:]
+    
+    init() {
+        NotificationCenter.default.addObserver(
+            self,
+            selector: #selector(onScreensChanged(_:)),
+            name: NSApplication.didChangeScreenParametersNotification,
+            object: nil)
+    }
+    
+    deinit {
+        NotificationCenter.default.removeObserver(self)
+    }
+    
+    /// Save the window frame for a screen.
+    func save(frame: NSRect, for screen: NSScreen) {
+        guard let key = screen.displayUUID else { return }
+        let entry = DisplayEntry(
+            frame: frame,
+            screenSize: screen.frame.size,
+            scale: screen.backingScaleFactor,
+            lastSeen: Date()
+        )
+        stateByDisplay[key] = entry
+        pruneCapacity()
+    }
+    
+    /// Retrieve the last closed frame for a screen, if valid.
+    func frame(for screen: NSScreen) -> NSRect? {
+        guard let key = screen.displayUUID, var entry = stateByDisplay[key] else { return nil }
+        
+        // Drop on dimension/scale change that makes the entry invalid
+        if !entry.isValid(for: screen) {
+            stateByDisplay.removeValue(forKey: key)
+            return nil
+        }
+        
+        entry.lastSeen = Date()
+        stateByDisplay[key] = entry
+        return entry.frame
+    }
+    
+    @objc private func onScreensChanged(_ note: Notification) {
+        let screens = NSScreen.screens
+        let now = Date()
+        let currentIDs = Set(screens.compactMap { $0.displayUUID })
+        
+        for screen in screens {
+            guard let key = screen.displayUUID else { continue }
+            if var entry = stateByDisplay[key] {
+                // Drop on dimension/scale change that makes the entry invalid
+                if !entry.isValid(for: screen) {
+                    stateByDisplay.removeValue(forKey: key)
+                } else {
+                    // Update the screen size if it grew (keep entry valid for larger screens)
+                    entry.screenSize = screen.frame.size
+                    entry.lastSeen = now
+                    stateByDisplay[key] = entry
+                }
+            }
+        }
+        
+        // TTL prune for non-present screens
+        stateByDisplay = stateByDisplay.filter { key, entry in
+            currentIDs.contains(key) || now.timeIntervalSince(entry.lastSeen) < Self.screenStaleTTL
+        }
+        
+        pruneCapacity()
+    }
+    
+    private func pruneCapacity() {
+        guard stateByDisplay.count > Self.maxSavedScreens else { return }
+        let toRemove = stateByDisplay
+            .sorted { $0.value.lastSeen < $1.value.lastSeen }
+            .prefix(stateByDisplay.count - Self.maxSavedScreens)
+        for (key, _) in toRemove {
+            stateByDisplay.removeValue(forKey: key)
+        }
+    }
+    
+    private struct DisplayEntry {
+        var frame: NSRect
+        var screenSize: CGSize
+        var scale: CGFloat
+        var lastSeen: Date
+        
+        /// Returns true if this entry is still valid for the given screen.
+        /// Valid if the scale matches and the cached size is not larger than the current screen size.
+        /// This allows entries to persist when screens grow, but invalidates them when screens shrink.
+        func isValid(for screen: NSScreen) -> Bool {
+            guard scale == screen.backingScaleFactor else { return false }
+            return screenSize.width <= screen.frame.size.width && screenSize.height <= screen.frame.size.height
+        }
+    }
+}
diff --git a/macos/Sources/Helpers/Extensions/NSScreen+Extension.swift b/macos/Sources/Helpers/Extensions/NSScreen+Extension.swift
@@ -5,6 +5,13 @@ extension NSScreen {
     var displayID: UInt32? {
         deviceDescription[NSDeviceDescriptionKey("NSScreenNumber")] as? UInt32
     }
+    
+    /// The stable UUID for this display, suitable for tracking across reconnects and NSScreen garbage collection.
+    var displayUUID: UUID? {
+        guard let displayID = displayID else { return nil }
+        guard let cfuuid = CGDisplayCreateUUIDFromDisplayID(displayID)?.takeRetainedValue() else { return nil }
+        return UUID(cfuuid)
+    }
 
     // Returns true if the given screen has a visible dock. This isn't
     // point-in-time visible, this is true if the dock is always visible
diff --git a/macos/Sources/Helpers/Extensions/UUID+Extension.swift b/macos/Sources/Helpers/Extensions/UUID+Extension.swift
@@ -0,0 +1,9 @@
+import Foundation
+
+extension UUID {
+    /// Initialize a UUID from a CFUUID.
+    init?(_ cfuuid: CFUUID) {
+        guard let uuidString = CFUUIDCreateString(nil, cfuuid) as String? else { return nil }
+        self.init(uuidString: uuidString)
+    }
+}
