From 146b1f2a1bf7edf82afd53d5e852076e92e0d443 Mon Sep 17 00:00:00 2001 From: Pranav Mangal Date: Wed, 11 Dec 2024 23:10:03 +0530 Subject: [PATCH 1/3] Add delay before a title change to avoid flicker on macOS --- .../Features/Terminal/BaseTerminalController.swift | 11 +++++++++-- .../Features/Terminal/TerminalController.swift | 10 +++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/macos/Sources/Features/Terminal/BaseTerminalController.swift b/macos/Sources/Features/Terminal/BaseTerminalController.swift index 68c243004..5f4f357b6 100644 --- a/macos/Sources/Features/Terminal/BaseTerminalController.swift +++ b/macos/Sources/Features/Terminal/BaseTerminalController.swift @@ -53,6 +53,10 @@ class BaseTerminalController: NSWindowController, /// Fullscreen state management. private(set) var fullscreenStyle: FullscreenStyle? + + var titleChangeDelay: TimeInterval = 0.075 + + private var titleChangeTimer: Timer? /// Event monitor (see individual events for why) private var eventMonitor: Any? = nil @@ -260,9 +264,12 @@ class BaseTerminalController: NSWindowController, func titleDidChange(to: String) { guard let window else { return } - // Set the main window title - window.title = to + titleChangeTimer?.invalidate() + // Set the main window title after a small delay to prevent flicker + titleChangeTimer = Timer.scheduledTimer(withTimeInterval: titleChangeDelay, repeats: false) { _ in + window.title = to + } } func pwdDidChange(to: URL?) { diff --git a/macos/Sources/Features/Terminal/TerminalController.swift b/macos/Sources/Features/Terminal/TerminalController.swift index 67e7259f3..edb797890 100644 --- a/macos/Sources/Features/Terminal/TerminalController.swift +++ b/macos/Sources/Features/Terminal/TerminalController.swift @@ -26,6 +26,8 @@ class TerminalController: BaseTerminalController { /// The notification cancellable for focused surface property changes. private var surfaceAppearanceCancellables: Set = [] + + private var toolbarTitleChangeTimer: Timer? init(_ ghostty: Ghostty.App, withBaseConfig base: Ghostty.SurfaceConfiguration? = nil, @@ -546,7 +548,13 @@ class TerminalController: BaseTerminalController { // a custom view instead, we need to re-hide it. window.titleVisibility = .hidden } - toolbar.titleText = to + + toolbarTitleChangeTimer?.invalidate() + + // Set the toolbar title after a small delay to prevent flicker + toolbarTitleChangeTimer = Timer.scheduledTimer(withTimeInterval: titleChangeDelay, repeats: false) { _ in + toolbar.titleText = to + } } } From e35bd431f4245a42c35935d8e8c74c59dfd40b28 Mon Sep 17 00:00:00 2001 From: Pranav Mangal Date: Thu, 12 Dec 2024 14:58:42 +0530 Subject: [PATCH 2/3] Move title change timer to SurfaceView and call it from Ghostty.App instead of terminal controllers --- .../Terminal/BaseTerminalController.swift | 11 ++-------- .../Terminal/TerminalController.swift | 10 +--------- macos/Sources/Ghostty/Ghostty.App.swift | 20 ++++++++++++------- .../Sources/Ghostty/SurfaceView_AppKit.swift | 4 ++++ 4 files changed, 20 insertions(+), 25 deletions(-) diff --git a/macos/Sources/Features/Terminal/BaseTerminalController.swift b/macos/Sources/Features/Terminal/BaseTerminalController.swift index 5f4f357b6..68c243004 100644 --- a/macos/Sources/Features/Terminal/BaseTerminalController.swift +++ b/macos/Sources/Features/Terminal/BaseTerminalController.swift @@ -53,10 +53,6 @@ class BaseTerminalController: NSWindowController, /// Fullscreen state management. private(set) var fullscreenStyle: FullscreenStyle? - - var titleChangeDelay: TimeInterval = 0.075 - - private var titleChangeTimer: Timer? /// Event monitor (see individual events for why) private var eventMonitor: Any? = nil @@ -264,12 +260,9 @@ class BaseTerminalController: NSWindowController, func titleDidChange(to: String) { guard let window else { return } - titleChangeTimer?.invalidate() + // Set the main window title + window.title = to - // Set the main window title after a small delay to prevent flicker - titleChangeTimer = Timer.scheduledTimer(withTimeInterval: titleChangeDelay, repeats: false) { _ in - window.title = to - } } func pwdDidChange(to: URL?) { diff --git a/macos/Sources/Features/Terminal/TerminalController.swift b/macos/Sources/Features/Terminal/TerminalController.swift index edb797890..67e7259f3 100644 --- a/macos/Sources/Features/Terminal/TerminalController.swift +++ b/macos/Sources/Features/Terminal/TerminalController.swift @@ -26,8 +26,6 @@ class TerminalController: BaseTerminalController { /// The notification cancellable for focused surface property changes. private var surfaceAppearanceCancellables: Set = [] - - private var toolbarTitleChangeTimer: Timer? init(_ ghostty: Ghostty.App, withBaseConfig base: Ghostty.SurfaceConfiguration? = nil, @@ -548,13 +546,7 @@ class TerminalController: BaseTerminalController { // a custom view instead, we need to re-hide it. window.titleVisibility = .hidden } - - toolbarTitleChangeTimer?.invalidate() - - // Set the toolbar title after a small delay to prevent flicker - toolbarTitleChangeTimer = Timer.scheduledTimer(withTimeInterval: titleChangeDelay, repeats: false) { _ in - toolbar.titleText = to - } + toolbar.titleText = to } } diff --git a/macos/Sources/Ghostty/Ghostty.App.swift b/macos/Sources/Ghostty/Ghostty.App.swift index 9056e692a..c8e3cc476 100644 --- a/macos/Sources/Ghostty/Ghostty.App.swift +++ b/macos/Sources/Ghostty/Ghostty.App.swift @@ -947,15 +947,21 @@ extension Ghostty { guard let surface = target.target.surface else { return } guard let surfaceView = self.surfaceView(from: surface) else { return } guard let title = String(cString: v.title!, encoding: .utf8) else { return } - - // We must set this in a dispatchqueue to avoid a deadlock on startup on some - // versions of macOS. I unfortunately didn't document the exact versions so - // I don't know when its safe to remove this. - DispatchQueue.main.async { - surfaceView.title = title + + surfaceView.titleChangeTimer?.invalidate() + + surfaceView.titleChangeTimer = Timer.scheduledTimer( + withTimeInterval: surfaceView.titleChangeDelay, + repeats: false + ) { _ in + // We must set this in a dispatchqueue to avoid a deadlock on startup on some + // versions of macOS. I unfortunately didn't document the exact versions so + // I don't know when its safe to remove this. + DispatchQueue.main.async { + surfaceView.title = title + } } - default: assertionFailure() } diff --git a/macos/Sources/Ghostty/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/SurfaceView_AppKit.swift index 7e861a229..c426e9e07 100644 --- a/macos/Sources/Ghostty/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/SurfaceView_AppKit.swift @@ -13,6 +13,10 @@ extension Ghostty { // changed with escape codes. This is public because the callbacks go // to the app level and it is set from there. @Published var title: String = "👻" + + // A small delay that is introduced before a title change to avoid flickers + var titleChangeDelay: TimeInterval = 0.075 + var titleChangeTimer: Timer? // The current pwd of the surface as defined by the pty. This can be // changed with escape codes. From 4fdf5eb12b910cb92d34fdbddace1a0f996cb30b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 12 Dec 2024 16:43:10 -0800 Subject: [PATCH 3/3] macos: move title setting into a function to better encapsulate --- macos/Sources/Ghostty/Ghostty.App.swift | 20 ++++------------ .../Sources/Ghostty/SurfaceView_AppKit.swift | 23 +++++++++++++++---- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/macos/Sources/Ghostty/Ghostty.App.swift b/macos/Sources/Ghostty/Ghostty.App.swift index c8e3cc476..2d9822d6e 100644 --- a/macos/Sources/Ghostty/Ghostty.App.swift +++ b/macos/Sources/Ghostty/Ghostty.App.swift @@ -947,20 +947,7 @@ extension Ghostty { guard let surface = target.target.surface else { return } guard let surfaceView = self.surfaceView(from: surface) else { return } guard let title = String(cString: v.title!, encoding: .utf8) else { return } - - surfaceView.titleChangeTimer?.invalidate() - - surfaceView.titleChangeTimer = Timer.scheduledTimer( - withTimeInterval: surfaceView.titleChangeDelay, - repeats: false - ) { _ in - // We must set this in a dispatchqueue to avoid a deadlock on startup on some - // versions of macOS. I unfortunately didn't document the exact versions so - // I don't know when its safe to remove this. - DispatchQueue.main.async { - surfaceView.title = title - } - } + surfaceView.setTitle(title) default: assertionFailure() @@ -1095,7 +1082,10 @@ extension Ghostty { guard let surface = target.target.surface else { return } guard let surfaceView = self.surfaceView(from: surface) else { return } let backingSize = NSSize(width: Double(v.width), height: Double(v.height)) - surfaceView.cellSize = surfaceView.convertFromBacking(backingSize) + DispatchQueue.main.async { [weak surfaceView] in + guard let surfaceView else { return } + surfaceView.cellSize = surfaceView.convertFromBacking(backingSize) + } default: assertionFailure() diff --git a/macos/Sources/Ghostty/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/SurfaceView_AppKit.swift index c426e9e07..d06ab36eb 100644 --- a/macos/Sources/Ghostty/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/SurfaceView_AppKit.swift @@ -12,11 +12,7 @@ extension Ghostty { // The current title of the surface as defined by the pty. This can be // changed with escape codes. This is public because the callbacks go // to the app level and it is set from there. - @Published var title: String = "👻" - - // A small delay that is introduced before a title change to avoid flickers - var titleChangeDelay: TimeInterval = 0.075 - var titleChangeTimer: Timer? + @Published private(set) var title: String = "👻" // The current pwd of the surface as defined by the pty. This can be // changed with escape codes. @@ -114,6 +110,9 @@ extension Ghostty { // This is set to non-null during keyDown to accumulate insertText contents private var keyTextAccumulator: [String]? = nil + // A small delay that is introduced before a title change to avoid flickers + private var titleChangeTimer: Timer? + // We need to support being a first responder so that we can get input events override var acceptsFirstResponder: Bool { return true } @@ -343,6 +342,20 @@ extension Ghostty { NSCursor.setHiddenUntilMouseMoves(!visible) } + func setTitle(_ title: String) { + // This fixes an issue where very quick changes to the title could + // cause an unpleasant flickering. We set a timer so that we can + // coalesce rapid changes. The timer is short enough that it still + // feels "instant". + titleChangeTimer?.invalidate() + titleChangeTimer = Timer.scheduledTimer( + withTimeInterval: 0.075, + repeats: false + ) { [weak self] _ in + self?.title = title + } + } + // MARK: - Notifications @objc private func onUpdateRendererHealth(notification: SwiftUI.Notification) {