From 118b51157a9415e69eb4f5eac322a35d166ce758 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 11 Feb 2024 09:16:41 -0800 Subject: [PATCH] macos: more robust surface focus state detection Fixes #1500 This overhauls how we do focus management for surfaces to make it more robust. This DID somehow all work before but was always brittle and was a sketchy play with SwiftUI/AppKit behavior across macOS versions. The new approach uses our window controller and terminal delegate system to disseminate focus information whenever any surface changes focus. This ensures that only ONE surface ever has focus in libghostty because the controller ensures it is widely distributed. --- .../Terminal/TerminalController.swift | 32 ++++++++++++++++++- macos/Sources/Ghostty/Ghostty.SplitNode.swift | 20 +++++++++++- macos/Sources/Ghostty/SurfaceView.swift | 12 +------ .../Sources/Ghostty/SurfaceView_AppKit.swift | 9 +++--- 4 files changed, 55 insertions(+), 18 deletions(-) diff --git a/macos/Sources/Features/Terminal/TerminalController.swift b/macos/Sources/Features/Terminal/TerminalController.swift index 2fabadd77..4c4429d04 100644 --- a/macos/Sources/Features/Terminal/TerminalController.swift +++ b/macos/Sources/Features/Terminal/TerminalController.swift @@ -14,7 +14,11 @@ class TerminalController: NSWindowController, NSWindowDelegate, let ghostty: Ghostty.App /// The currently focused surface. - var focusedSurface: Ghostty.SurfaceView? = nil + var focusedSurface: Ghostty.SurfaceView? = nil { + didSet { + syncFocusToSurfaceTree() + } + } /// The surface tree for this window. @Published var surfaceTree: Ghostty.SplitNode? = nil { @@ -23,6 +27,7 @@ class TerminalController: NSWindowController, NSWindowDelegate, // in the old tree have closed and then close the window. if (surfaceTree == nil) { oldValue?.close() + focusedSurface = nil lastSurfaceDidClose() } } @@ -181,6 +186,21 @@ class TerminalController: NSWindowController, NSWindowDelegate, } } + /// Update all surfaces with the focus state. This ensures that libghostty has an accurate view about + /// what surface is focused. This must be called whenever a surface OR window changes focus. + private func syncFocusToSurfaceTree() { + guard let tree = self.surfaceTree else { return } + + for leaf in tree { + // Our focus state requires that this window is key and our currently + // focused surface is the surface in this leaf. + let focused: Bool = (window?.isKeyWindow ?? false) && + focusedSurface != nil && + leaf.surface == focusedSurface! + leaf.surface.focusDidChange(focused) + } + } + //MARK: - NSWindowController override func windowWillLoad() { @@ -348,6 +368,16 @@ class TerminalController: NSWindowController, NSWindowDelegate, func windowDidBecomeKey(_ notification: Notification) { self.relabelTabs() self.fixTabBar() + + // Becoming/losing key means we have to notify our surface(s) that we have focus + // so things like cursors blink, pty events are sent, etc. + self.syncFocusToSurfaceTree() + } + + func windowDidResignKey(_ notification: Notification) { + // Becoming/losing key means we have to notify our surface(s) that we have focus + // so things like cursors blink, pty events are sent, etc. + self.syncFocusToSurfaceTree() } func windowDidMove(_ notification: Notification) { diff --git a/macos/Sources/Ghostty/Ghostty.SplitNode.swift b/macos/Sources/Ghostty/Ghostty.SplitNode.swift index 3d90502bd..5dc1ec492 100644 --- a/macos/Sources/Ghostty/Ghostty.SplitNode.swift +++ b/macos/Sources/Ghostty/Ghostty.SplitNode.swift @@ -11,7 +11,7 @@ extension Ghostty { /// "container" which has a recursive top/left SplitNode and bottom/right SplitNode. These /// values can further be split infinitely. /// - enum SplitNode: Equatable, Hashable, Codable { + enum SplitNode: Equatable, Hashable, Codable, Sequence { case leaf(Leaf) case split(Container) @@ -136,6 +136,24 @@ extension Ghostty { } } + // MARK: - Sequence + + func makeIterator() -> IndexingIterator<[Leaf]> { + return leaves().makeIterator() + } + + /// Return all the leaves in this split node. This isn't very efficient but our split trees are never super + /// deep so its not an issue. + private func leaves() -> [Leaf] { + switch (self) { + case .leaf(let leaf): + return [leaf] + + case .split(let container): + return container.topLeft.leaves() + container.bottomRight.leaves() + } + } + // MARK: - Equatable static func == (lhs: SplitNode, rhs: SplitNode) -> Bool { diff --git a/macos/Sources/Ghostty/SurfaceView.swift b/macos/Sources/Ghostty/SurfaceView.swift index be7cd9e6e..f4fefbaed 100644 --- a/macos/Sources/Ghostty/SurfaceView.swift +++ b/macos/Sources/Ghostty/SurfaceView.swift @@ -51,10 +51,6 @@ extension Ghostty { @EnvironmentObject private var ghostty: Ghostty.App - // This is true if the terminal is considered "focused". The terminal is focused if - // it is both individually focused and the containing window is key. - private var hasFocus: Bool { surfaceFocus && windowFocus } - var body: some View { let center = NotificationCenter.default @@ -72,7 +68,7 @@ extension Ghostty { let pubResign = center.publisher(for: NSWindow.didResignKeyNotification) #endif - Surface(view: surfaceView, hasFocus: hasFocus, size: geo.size) + Surface(view: surfaceView, size: geo.size) .focused($surfaceFocus) .focusedValue(\.ghosttySurfaceTitle, surfaceView.title) .focusedValue(\.ghosttySurfaceView, surfaceView) @@ -230,11 +226,6 @@ extension Ghostty { /// The view to render for the terminal surface. let view: SurfaceView - /// This should be set to true when the surface has focus. This is up to the parent because - /// focus is also defined by window focus. It is important this is set correctly since if it is - /// false then the surface will idle at almost 0% CPU. - let hasFocus: Bool - /// The size of the frame containing this view. We use this to update the the underlying /// surface. This does not actually SET the size of our frame, this only sets the size /// of our Metal surface for drawing. @@ -253,7 +244,6 @@ extension Ghostty { } func updateOSView(_ view: SurfaceView, context: Context) { - view.focusDidChange(hasFocus) view.sizeDidChange(size) } } diff --git a/macos/Sources/Ghostty/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/SurfaceView_AppKit.swift index f1f4f0f7e..591a6820d 100644 --- a/macos/Sources/Ghostty/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/SurfaceView_AppKit.swift @@ -188,6 +188,8 @@ extension Ghostty { func focusDidChange(_ focused: Bool) { guard let surface = self.surface else { return } + guard self.focused != focused else { return } + self.focused = focused ghostty_surface_set_focus(surface, focused) } @@ -358,7 +360,7 @@ extension Ghostty { override func becomeFirstResponder() -> Bool { let result = super.becomeFirstResponder() - if (result) { focused = true } + if (result) { focusDidChange(true) } return result } @@ -367,10 +369,7 @@ extension Ghostty { // We sometimes call this manually (see SplitView) as a way to force us to // yield our focus state. - if (result) { - focusDidChange(false) - focused = false - } + if (result) { focusDidChange(false) } return result }