From 363a03a30dc270b50119fd188df3341cafd44e8d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 15 Mar 2023 21:11:52 -0700 Subject: [PATCH 1/2] macos: explicitly free surface resource on split/tab close We don't wait for Swift to garbage collect. We just free the expensive stuff (our surface) immediately. --- macos/Sources/Ghostty/Ghostty.SplitView.swift | 28 +++++++++++++++++++ macos/Sources/Ghostty/SurfaceView.swift | 10 +++++++ 2 files changed, 38 insertions(+) diff --git a/macos/Sources/Ghostty/Ghostty.SplitView.swift b/macos/Sources/Ghostty/Ghostty.SplitView.swift index 4cff3217c..52f8fdde6 100644 --- a/macos/Sources/Ghostty/Ghostty.SplitView.swift +++ b/macos/Sources/Ghostty/Ghostty.SplitView.swift @@ -45,6 +45,23 @@ extension Ghostty { } } + /// Close the surface associated with this node. This will likely deinitialize the + /// surface. At this point, the surface view in this node tree can never be used again. + func close() { + switch (self) { + case .noSplit(let leaf): + leaf.surface.close() + + case .horizontal(let container): + container.topLeft.close() + container.bottomRight.close() + + case .vertical(let container): + container.topLeft.close() + container.bottomRight.close() + } + } + class Leaf: ObservableObject { let app: ghostty_app_t @Published var surface: SurfaceView @@ -144,6 +161,11 @@ extension Ghostty { ) .onChange(of: requestClose) { value in guard value else { return } + + // Free any resources associated with this root, we're closing. + node.close() + + // Call our callback guard let onClose = self.onClose else { return } onClose() } @@ -289,6 +311,9 @@ extension Ghostty { .onChange(of: closeTopLeft) { value in guard value else { return } + // Close the top left and release all resources + container.topLeft.close() + // When closing the topLeft, our parent becomes the bottomRight. node = container.bottomRight TerminalSplitLeaf.moveFocus(node, previous: container.topLeft) @@ -307,6 +332,9 @@ extension Ghostty { .onChange(of: closeBottomRight) { value in guard value else { return } + // Close the node and release all resources + container.bottomRight.close() + // When closing the bottomRight, our parent becomes the topLeft. node = container.topLeft TerminalSplitLeaf.moveFocus(node, previous: container.bottomRight) diff --git a/macos/Sources/Ghostty/SurfaceView.swift b/macos/Sources/Ghostty/SurfaceView.swift index 659de9da7..5e0c2c67b 100644 --- a/macos/Sources/Ghostty/SurfaceView.swift +++ b/macos/Sources/Ghostty/SurfaceView.swift @@ -153,6 +153,16 @@ extension Ghostty { ghostty_surface_free(surface) } + /// Close the surface early. This will free the associated Ghostty surface and the view will + /// no longer render. The view can never be used again. This is a way for us to free the + /// Ghostty resources while references may still be held to this view. I've found that SwiftUI + /// tends to hold this view longer than it should so we free the expensive stuff explicitly. + func close() { + guard let surface = self.surface else { return } + ghostty_surface_free(surface) + self.surface = nil + } + func focusDidChange(_ focused: Bool) { guard let surface = self.surface else { return } ghostty_surface_set_focus(surface, focused) From c01e8337bd75642c19b7efc625d54a19842e282e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 15 Mar 2023 21:29:20 -0700 Subject: [PATCH 2/2] macos: reliable window focus tracking for surface --- macos/Sources/Ghostty/SurfaceView.swift | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/macos/Sources/Ghostty/SurfaceView.swift b/macos/Sources/Ghostty/SurfaceView.swift index 5e0c2c67b..7afc826b6 100644 --- a/macos/Sources/Ghostty/SurfaceView.swift +++ b/macos/Sources/Ghostty/SurfaceView.swift @@ -38,24 +38,42 @@ extension Ghostty { // remains the same, the surface that is being rendered remains the same. @ObservedObject var surfaceView: SurfaceView + // Maintain whether our view has focus or not @FocusState private var surfaceFocus: Bool - // https://nilcoalescing.com/blog/DetectFocusedWindowOnMacOS/ - @Environment(\.controlActiveState) var controlActiveState + // Maintain whether our window has focus (is key) or not + @State private var windowFocus: Bool = false // 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 && controlActiveState == .key } + private var hasFocus: Bool { surfaceFocus && windowFocus } var body: some View { // We use a GeometryReader to get the frame bounds so that our metal surface // is up to date. See TerminalSurfaceView for why we don't use the NSView // resize callback. GeometryReader { geo in + // We use these notifications to determine when the window our surface is + // attached to is or is not focused. + let pubBecomeKey = NotificationCenter.default.publisher(for: NSWindow.didBecomeKeyNotification) + let pubResign = NotificationCenter.default.publisher(for: NSWindow.didResignKeyNotification) + Surface(view: surfaceView, hasFocus: hasFocus, size: geo.size) .focused($surfaceFocus) .focusedValue(\.ghosttySurfaceTitle, surfaceView.title) .focusedValue(\.ghosttySurfaceView, surfaceView) + .onReceive(pubBecomeKey) { notification in + guard let window = notification.object as? NSWindow else { return } + guard let surfaceWindow = surfaceView.window else { return } + windowFocus = surfaceWindow == window + } + .onReceive(pubResign) { notification in + guard let window = notification.object as? NSWindow else { return } + guard let surfaceWindow = surfaceView.window else { return } + if (surfaceWindow == window) { + windowFocus = false + } + } } .ghosttySurfaceView(surfaceView) }