Repository navigation
terminal: propagate NSWindow occlusion to Ghostty surface occlusion #7621
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| /// Two-axis occlusion state for a terminal surface. | ||
| /// | ||
| /// Ghostty should render only when the surface is both visible in the UI | ||
| /// (portal/canvas visibility) and its host window is visible according to | ||
| /// `NSWindow.occlusionState`. | ||
| public struct SurfaceOcclusionState: Equatable, Sendable { | ||
| /// Whether the portal or canvas currently considers the surface visible. | ||
| public var uiVisible: Bool | ||
|
|
||
| /// Whether the host window is currently visible to AppKit. | ||
| public var windowVisible: Bool | ||
|
|
||
| /// Creates occlusion state with both axes visible by default. | ||
| /// | ||
| /// - Parameters: | ||
| /// - uiVisible: The current portal or canvas visibility. | ||
| /// - windowVisible: The current host-window visibility. | ||
| public init(uiVisible: Bool = true, windowVisible: Bool = true) { | ||
| self.uiVisible = uiVisible | ||
| self.windowVisible = windowVisible | ||
| } | ||
|
|
||
| /// Whether Ghostty should treat the surface as visible. | ||
| public var effectiveVisible: Bool { | ||
| uiVisible && windowVisible | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import Testing | ||
| @testable import CmuxTerminal | ||
|
|
||
| @Suite struct SurfaceOcclusionStateTests { | ||
| @Test func defaultsAreVisibleOnBothAxes() { | ||
| let state = SurfaceOcclusionState() | ||
|
|
||
| #expect(state.uiVisible) | ||
| #expect(state.windowVisible) | ||
| #expect(state.effectiveVisible) | ||
| } | ||
|
|
||
| @Test(arguments: [ | ||
| (uiVisible: true, windowVisible: true, effectiveVisible: true), | ||
| (uiVisible: true, windowVisible: false, effectiveVisible: false), | ||
| (uiVisible: false, windowVisible: true, effectiveVisible: false), | ||
| (uiVisible: false, windowVisible: false, effectiveVisible: false) | ||
| ]) | ||
| func effectiveVisibilityIsTheAndOfBothAxes( | ||
| uiVisible: Bool, | ||
| windowVisible: Bool, | ||
| effectiveVisible: Bool | ||
| ) { | ||
| let state = SurfaceOcclusionState(uiVisible: uiVisible, windowVisible: windowVisible) | ||
|
|
||
| #expect(state.effectiveVisible == effectiveVisible) | ||
| } | ||
|
|
||
| @Test func uiVisibilityMustReturnBeforeWindowVisibilityCanRenderAgain() { | ||
| var state = SurfaceOcclusionState() | ||
|
|
||
| state.uiVisible = false | ||
| #expect(!state.effectiveVisible) | ||
|
|
||
| state.windowVisible = false | ||
| #expect(!state.effectiveVisible) | ||
|
|
||
| state.windowVisible = true | ||
| #expect(!state.effectiveVisible) | ||
|
|
||
| state.uiVisible = true | ||
| #expect(state.effectiveVisible) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3630,7 +3630,7 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { | |
| #endif | ||
| private var eventMonitor: Any? | ||
| private var trackingArea: NSTrackingArea? | ||
| private var windowObserver: NSObjectProtocol? | ||
| private var windowObserver: NSObjectProtocol?, occlusionObserver: NSObjectProtocol? | ||
| private var lastScrollEventTime: CFTimeInterval = 0 | ||
| private let scrollSpeedAccumulator = TerminalScrollSpeedAccumulator() | ||
| private var visibleInUI: Bool = true | ||
|
|
@@ -3869,6 +3869,11 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { | |
| surface.reconcileAttachedWindowIfNeeded(for: self) | ||
| } | ||
| surface.setKeyboardCopyModeActive(keyboardCopyModeActive) | ||
| // Seed the window-occlusion axis so a surface attached to a view already | ||
| // sitting in an occluded window does not keep rendering off-screen. | ||
| if let window { | ||
| surface.setWindowOcclusionVisible(window.occlusionState.contains(.visible)) | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Window occlusion seeded after createMedium Severity
Additional Locations (1)Reviewed by Cursor Bugbot for commit 00343ea. Configure here. |
||
| if !isAlreadyAttached { | ||
| updateSurfaceSize() | ||
| } | ||
|
|
@@ -3878,10 +3883,11 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { | |
|
|
||
| override func viewDidMoveToWindow() { | ||
| super.viewDidMoveToWindow() | ||
| if let windowObserver { | ||
| NotificationCenter.default.removeObserver(windowObserver) | ||
| self.windowObserver = nil | ||
| for observer in [windowObserver, occlusionObserver].compactMap({ $0 }) { | ||
| NotificationCenter.default.removeObserver(observer) | ||
| } | ||
| windowObserver = nil | ||
| occlusionObserver = nil | ||
| // Balance the cursor stack if the view is removed while hover is active | ||
| if wordPathHoverActive { | ||
| wordPathHoverActive = false | ||
|
|
@@ -3916,6 +3922,13 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { | |
| ) { [weak self] notification in | ||
| self?.windowDidChangeScreen(notification) | ||
| } | ||
| occlusionObserver = NotificationCenter.default.addObserver( | ||
| forName: NSWindow.didChangeOcclusionStateNotification, object: window, queue: .main | ||
| ) { [weak self, weak window] _ in | ||
| guard let self, let window, self.window === window else { return } | ||
| self.terminalSurface?.setWindowOcclusionVisible(window.occlusionState.contains(.visible)) | ||
| } | ||
| terminalSurface?.setWindowOcclusionVisible(window.occlusionState.contains(.visible)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 00343ea: |
||
|
|
||
| if let surface = terminalSurface?.surface, | ||
| let displayID = window.screen?.displayID, | ||
|
|
@@ -3953,11 +3966,6 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { | |
| ) | ||
| } | ||
|
|
||
| fileprivate func updateOcclusionState() { | ||
| // Intentionally no-op: we don't drive libghostty occlusion from AppKit occlusion state. | ||
| // This avoids transient clears during reparenting and keeps rendering logic minimal. | ||
| } | ||
|
|
||
| override func viewDidChangeBackingProperties() { | ||
| super.viewDidChangeBackingProperties() | ||
| if let window { | ||
|
|
@@ -7505,9 +7513,11 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { | |
| if let eventMonitor { | ||
| NSEvent.removeMonitor(eventMonitor) | ||
| } | ||
| if let windowObserver { | ||
| NotificationCenter.default.removeObserver(windowObserver) | ||
| for observer in [windowObserver, occlusionObserver].compactMap({ $0 }) { | ||
| NotificationCenter.default.removeObserver(observer) | ||
| } | ||
| windowObserver = nil | ||
| occlusionObserver = nil | ||
| if let trackingArea { | ||
| removeTrackingArea(trackingArea) | ||
| } | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new public
Sendablevalue model is used by nonisolatedTerminalSurfacemethods and main-actor lifecycle code, but it is not declarednonisolated. Under cmux's Swift 6 actor-isolation rule, pure Sendable models should opt out explicitly so later compiler or module isolation changes do not couple this runtime state to the main actor and produce isolation diagnostics in the setter paths.Rule Used: Flag new or materially worsened Swift 6 actor isol... (source)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Declining this one: the CmuxTerminal package compiles in plain Swift 6 language mode with no
defaultIsolation(MainActor.self)setting, so a top-levelpublic structhere is already nonisolated. Every sibling public Sendable value type in the package (TerminalSurfaceSpawnPolicy,TerminalSurfaceRuntimeFilesystem,TerminalSurfaceRegistryDiagnosticSnapshot) is declared without an explicitnonisolatedkeyword, so adding it only toSurfaceOcclusionStatewould deviate from the package's existing convention rather than follow it.