From 3347b4fa65c775dc28d54b20f17cd72c249a7d69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A4=D0=B5=D0=B4=D0=BE=D1=80=20=D0=91=D0=B0=D1=82=D0=BE?= =?UTF-8?q?=D0=BD=D0=BE=D0=B3=D0=BE=D0=B2?= Date: Wed, 15 Jul 2026 17:46:20 +0300 Subject: [PATCH] fix: prevent black Metal terminal frames --- Pine/TerminalSession.swift | 106 ++++++++++++++-- Pine/UITimings.swift | 8 ++ PineTests/TerminalMetalRendererTests.swift | 137 ++++++++++++++++++++- 3 files changed, 233 insertions(+), 18 deletions(-) diff --git a/Pine/TerminalSession.swift b/Pine/TerminalSession.swift index 47b585de..38674b30 100644 --- a/Pine/TerminalSession.swift +++ b/Pine/TerminalSession.swift @@ -7,6 +7,7 @@ import SwiftUI import SwiftTerm +import os /// Pine-specific terminal view wrapper. /// @@ -23,6 +24,13 @@ import SwiftTerm /// but deliberately does NOT zero `layer.contents` — see issue #1094. final class PineTerminalView: LocalProcessTerminalView { private var redrawBackgroundColor: CGColor? + private var initialMetalRedrawWorkItems: [DispatchWorkItem] = [] + + #if DEBUG + /// Test hook that observes every backend-aware redraw request without + /// replacing the production AppKit/Metal behavior. + var backendRedrawRequestObserver: (() -> Void)? + #endif /// Enables SwiftTerm's Metal renderer once the view lands in a window. /// @@ -42,8 +50,8 @@ final class PineTerminalView: LocalProcessTerminalView { /// maximize/restore) safe. On failure — a headless CI VM, an old GPU, a /// virtual display without a Metal device — /// `MTLCreateSystemDefaultDevice()` returns nil and - /// `MetalError.deviceUnavailable` is thrown; we swallow it and SwiftTerm - /// keeps using CoreGraphics with no behavioural change. + /// `MetalError.deviceUnavailable` is thrown; SwiftTerm keeps using + /// CoreGraphics and Pine records the concrete failure for diagnostics. func enableMetalRendererIfNeeded() { guard !Self.isMetalExplicitlyDisabled else { return } guard window != nil else { return } @@ -51,8 +59,9 @@ final class PineTerminalView: LocalProcessTerminalView { do { try setUseMetal(true) } catch { - // Metal unavailable (e.g. headless CI VM, old GPU). SwiftTerm - // keeps using CoreGraphics — no action needed. + Logger.terminal.error( + "SwiftTerm Metal renderer unavailable; falling back to CoreGraphics: \(String(describing: error), privacy: .public)" + ) } } @@ -69,12 +78,78 @@ final class PineTerminalView: LocalProcessTerminalView { override func viewDidMoveToWindow() { super.viewDidMoveToWindow() + guard window != nil else { + cancelInitialMetalRedrawRetries() + return + } // SwiftTerm's `setUseMetal(_:)` must be called only once the view is // in a window — it needs a Metal device, which is nil when headless // (GPURendering.md). Re-parenting (tab switch, pane split, drag-drop) // fires `viewDidMoveToWindow` again, but `enableMetalRendererIfNeeded` // is idempotent, so the second call is a cheap no-op. enableMetalRendererIfNeeded() + + // `MTKView` is on-demand (`isPaused = true`) and its CAMetalLayer may + // not have a drawable during the first display request. SwiftTerm + // 1.14.0 marks that frame pending, but the pending flag is consumed + // only by a successfully submitted command buffer — which does not + // exist in this bootstrap failure. Retry after every real attachment, + // including a detach/reattach within the same window: re-parenting + // replaces the CAMetalLayer presentation context and can reproduce + // the same drawable bootstrap race. The sequence is bounded and + // cancelled on the next detach (issue #1128). + if isUsingMetalRenderer { + scheduleInitialMetalRedrawRetries() + } + } + + /// Requests a display through the renderer that actually owns the pixels. + /// + /// SwiftTerm's CoreGraphics path draws in this outer NSView, so the + /// synchronous `setNeedsDisplay` + `displayIfNeeded` sequence remains the + /// right recovery operation. Under Metal, however, outer `draw(_:)` + /// returns immediately and a private nested `MTKView` owns presentation. + /// SwiftTerm 1.14.0 does not expose `requestMetalDisplay()` publicly, so + /// Pine uses two stable public entry points: + /// + /// - `selectionChanged(source:)` marks the full visible Metal range dirty + /// and queues a display, ensuring cached rows are rebuilt when needed. + /// - same-size `setFrameSize(_:)` is harmless (`processSizeChange` no-ops + /// when cols/rows are unchanged) and immediately calls SwiftTerm's + /// internal `requestMetalDisplay()`. + /// + /// Together they provide an immediate request plus a coalesced trailing + /// request. Replace this compatibility bridge once SwiftTerm exposes a + /// public backend-aware display API (issue #1128). + func requestRendererDisplay() { + #if DEBUG + backendRedrawRequestObserver?() + #endif + + if isUsingMetalRenderer { + selectionChanged(source: getTerminal()) + setFrameSize(frame.size) + } else { + setNeedsDisplay(bounds) + displayIfNeeded() + } + } + + private func scheduleInitialMetalRedrawRetries() { + cancelInitialMetalRedrawRetries() + initialMetalRedrawWorkItems = UITimings.Render.terminalFirstFrameRetryDelays.map { delay in + let workItem = DispatchWorkItem { [weak self] in + guard let self, self.window != nil, self.isUsingMetalRenderer else { return } + self.requestRendererDisplay() + } + DispatchQueue.main.asyncAfter(deadline: .now() + delay, execute: workItem) + return workItem + } + } + + private func cancelInitialMetalRedrawRetries() { + initialMetalRedrawWorkItems.forEach { $0.cancel() } + initialMetalRedrawWorkItems.removeAll() } func prepareLayerForRedraw(background: NSColor? = nil) { @@ -1293,22 +1368,23 @@ final class TerminalTab: Identifiable, Hashable { terminalView.terminate() } - /// Forces SwiftTerm to mark the entire visible buffer as dirty and the - /// view to redraw synchronously. + /// Forces SwiftTerm to mark the entire visible buffer as dirty and asks + /// the active renderer to present it immediately. /// /// Used after re-parenting the terminal view (tab switch, pane split, /// maximize/restore, drag-and-drop) and when the host window regains /// key focus. AppKit may have dropped the layer's backing store while /// the view was detached, leaving a black frame after re-attach. - /// `setNeedsDisplay(bounds)` + `displayIfNeeded()` is enough to - /// repaint from `displayBuffer`, since SwiftTerm's `draw(_:)` paints - /// the full visible buffer for any `dirtyRect` it receives. + /// On CoreGraphics, `setNeedsDisplay(bounds)` + `displayIfNeeded()` + /// repaints from `displayBuffer`. On Metal, the outer SwiftTerm view does + /// not draw; `PineTerminalView.requestRendererDisplay()` routes the + /// recovery request to the nested `MTKView` instead (issue #1128). /// `terminal.updateFullScreen()` additionally seeds the dirty range /// in case SwiftTerm's own throttled `updateDisplay` is the next path /// to fire (e.g. on incoming PTY data). /// - /// Safe to call when the view is detached from a window — AppKit - /// silently no-ops `displayIfNeeded()` in that state. + /// Safe to call when the view is detached from a window — AppKit and + /// SwiftTerm retain or safely coalesce the pending display request. func forceFullRedraw() { if let pineTerminalView = terminalView as? PineTerminalView { pineTerminalView.prepareLayerForRedraw(background: terminalView.nativeBackgroundColor) @@ -1317,8 +1393,12 @@ final class TerminalTab: Identifiable, Hashable { } let term = terminalView.getTerminal() term.updateFullScreen() - terminalView.setNeedsDisplay(terminalView.bounds) - terminalView.displayIfNeeded() + if let pineTerminalView = terminalView as? PineTerminalView { + pineTerminalView.requestRendererDisplay() + } else { + terminalView.setNeedsDisplay(terminalView.bounds) + terminalView.displayIfNeeded() + } } /// Sends the current PTY window size again via `TIOCSWINSZ`, which diff --git a/Pine/UITimings.swift b/Pine/UITimings.swift index 0a209335..66de1da9 100644 --- a/Pine/UITimings.swift +++ b/Pine/UITimings.swift @@ -121,5 +121,13 @@ nonisolated enum UITimings { /// the per-frame budget; trailing redraws coalesce via a /// scheduled work item to capture the final scroll position. static let minimapRedraw: TimeInterval = 0.025 + + /// Bounded retries for the first Metal terminal frame. A freshly + /// attached `CAMetalLayer` may legitimately have no drawable yet; + /// SwiftTerm 1.14.0 records that missed draw but only consumes the + /// pending flag after a *successful* command buffer completes. These + /// sparse retries bridge that bootstrap gap without creating a + /// continuous display loop or polling an occluded/detached view. + static let terminalFirstFrameRetryDelays: [TimeInterval] = [0, 0.05, 0.15, 0.35] } } diff --git a/PineTests/TerminalMetalRendererTests.swift b/PineTests/TerminalMetalRendererTests.swift index 0f1123b3..6c3053cc 100644 --- a/PineTests/TerminalMetalRendererTests.swift +++ b/PineTests/TerminalMetalRendererTests.swift @@ -7,16 +7,16 @@ import Testing import AppKit +import MetalKit import SwiftTerm @testable import Pine /// Tests for Pine's opt-in to SwiftTerm's Metal renderer. /// -/// The full Metal path needs a window + a Metal device, so it is exercised -/// manually / via UI tests. These unit tests pin the headless invariants -/// that must hold regardless of environment: the opt-out flag is honoured, -/// enabling without a window is a safe no-op, and the CoreGraphics-only -/// `prepareLayerForRedraw()` does not crash (#1108). +/// Hosted-window tests exercise the production Metal path when the runner has +/// a GPU; headless runners still pin the fallback invariants. This keeps the +/// suite portable while covering the first-frame recovery that UI tests miss +/// because they intentionally pass `--disable-metal` (#1108, #1128). @Suite("Terminal Metal Renderer Tests") @MainActor struct TerminalMetalRendererTests { @@ -66,4 +66,131 @@ struct TerminalMetalRendererTests { } #expect(view.isUsingMetalRenderer == false) } + + // MARK: - Backend-aware redraw (#1128) + + @Test("First-frame retry plan is bounded and ordered") + func firstFrameRetryPlanIsBoundedAndOrdered() { + let delays = UITimings.Render.terminalFirstFrameRetryDelays + #expect(delays.first == 0) + #expect(delays == delays.sorted()) + #expect(delays.count == 4) + #expect(delays.last == 0.35) + } + + @Test("forceFullRedraw routes through Pine's backend-aware display bridge") + func forceFullRedrawUsesBackendAwareBridge() { + let tab = TerminalTab(name: "redraw-test") + let view = tab.terminalView as? PineTerminalView + var redrawRequests = 0 + view?.backendRedrawRequestObserver = { redrawRequests += 1 } + + tab.forceFullRedraw() + + #expect(view != nil) + #expect(redrawRequests == 1) + } + + @Test("Metal redraw invalidates the nested MTKView") + func metalRedrawTargetsNestedView() async { + guard MTLCreateSystemDefaultDevice() != nil else { return } + let view = PineTerminalView(frame: NSRect(x: 0, y: 0, width: 800, height: 300)) + let window = makeWindow(containing: view) + + guard view.isUsingMetalRenderer, + let metalView = firstMetalView(in: view) else { + window.contentView = nil + return + } + + // Let the initial bounded retry batch drain, then isolate one request. + try? await Task.sleep(for: .milliseconds(450)) + metalView.needsDisplay = false + view.requestRendererDisplay() + + #expect(metalView.needsDisplay) + window.contentView = nil + } + + @Test("Initial Metal attachment schedules bounded redraw retries") + func initialAttachmentSchedulesRetries() async { + guard MTLCreateSystemDefaultDevice() != nil else { return } + let view = PineTerminalView(frame: NSRect(x: 0, y: 0, width: 800, height: 300)) + var redrawRequests = 0 + view.backendRedrawRequestObserver = { redrawRequests += 1 } + + let window = makeWindow(containing: view) + guard view.isUsingMetalRenderer else { + window.contentView = nil + return + } + + try? await Task.sleep(for: .milliseconds(450)) + + #expect(redrawRequests == UITimings.Render.terminalFirstFrameRetryDelays.count) + window.contentView = nil + } + + @Test("Detaching Metal view cancels pending first-frame retries") + func detachingCancelsRetries() async { + guard MTLCreateSystemDefaultDevice() != nil else { return } + let view = PineTerminalView(frame: NSRect(x: 0, y: 0, width: 800, height: 300)) + var redrawRequests = 0 + view.backendRedrawRequestObserver = { redrawRequests += 1 } + + let window = makeWindow(containing: view) + guard view.isUsingMetalRenderer else { + window.contentView = nil + return + } + window.contentView = nil + + try? await Task.sleep(for: .milliseconds(450)) + + #expect(redrawRequests == 0) + } + + @Test("Reattaching Metal view schedules a fresh bounded retry batch") + func reattachingSchedulesFreshRetries() async { + guard MTLCreateSystemDefaultDevice() != nil else { return } + let view = PineTerminalView(frame: NSRect(x: 0, y: 0, width: 800, height: 300)) + var redrawRequests = 0 + view.backendRedrawRequestObserver = { redrawRequests += 1 } + + let window = makeWindow(containing: view) + guard view.isUsingMetalRenderer else { + window.contentView = nil + return + } + + // Cancel the first batch before the main queue can deliver it, then + // reattach to the same window. A fresh batch is required because the + // CAMetalLayer can lose its drawable during ordinary tab re-parenting. + window.contentView = nil + window.contentView = view + + try? await Task.sleep(for: .milliseconds(450)) + + #expect(redrawRequests == UITimings.Render.terminalFirstFrameRetryDelays.count) + window.contentView = nil + } + + private func makeWindow(containing view: NSView) -> NSWindow { + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 800, height: 300), + styleMask: [.borderless], + backing: .buffered, + defer: false + ) + window.contentView = view + return window + } + + private func firstMetalView(in view: NSView) -> MTKView? { + if let metalView = view as? MTKView { return metalView } + for subview in view.subviews { + if let metalView = firstMetalView(in: subview) { return metalView } + } + return nil + } }