diff --git a/Sources/GhosttyTerminalView.swift b/Sources/GhosttyTerminalView.swift index 985de1b6f95..6cc520d1891 100644 --- a/Sources/GhosttyTerminalView.swift +++ b/Sources/GhosttyTerminalView.swift @@ -10263,6 +10263,13 @@ final class GhosttySurfaceScrollView: NSView { readySurfaceId == self.surfaceView.terminalSurface?.id else { return } + if self.surfaceView.isVisibleInUI && self.window != nil && !self.isHidden { + // A newly selected workspace starts visible from its first SwiftUI render, so it + // never passes through the normal setVisibleInUI(false -> true) refresh path. + // Reuse that redraw nudge when the runtime Ghostty surface becomes ready so the + // first Metal frame does not wait for a later tab switch or visibility churn. + self.refreshSurfaceNow(reason: "surfaceDidBecomeReady") + } // Session restore can request focus before the runtime surface exists. // Re-run the normal first-responder/focus path once the surface is live. guard self.isActive || self.surfaceView.desiredFocus || self.isSurfaceViewFirstResponder() else { diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index 966e9112970..d5c42156c1c 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -4932,6 +4932,10 @@ class TabManager: ObservableObject { // Route workspace reactivation through the normal focus machinery so panel-local // activation intents like browser find-field focus are restored on return. tab.focusPanel(panelId) + // Workspace selection can remount portal-hosted terminals/web views that were hidden + // while inactive. Kick the shared selection follow-up loop so the first visible frame + // is reconciled after attach instead of waiting for a second workspace switch. + tab.scheduleSelectionRenderFollowUp(panelId: panelId) } func completePendingWorkspaceUnfocus(reason: String) { diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index aa31e382af7..615df112739 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -7957,7 +7957,21 @@ final class Workspace: Identifiable, ObservableObject { private var layoutFollowUpAttemptScheduled = false private var layoutFollowUpAttemptVersion: Int = 0 private var layoutFollowUpStalledAttemptCount = 0 - private var portalRenderingEnabled = true + private enum LayoutFollowUpTargetUpdate { + case mergePendingTargets + case replaceSelectionTargets(panelId: UUID) + } + private enum PortalRenderingPhase { + case unmounted + case mounted + + var isMounted: Bool { + self == .mounted + } + } + + private var portalRenderingPhase: PortalRenderingPhase = .unmounted + private var portalRenderingEnabled: Bool { portalRenderingPhase.isMounted } private var isAttemptingLayoutFollowUp = false private var isNormalizingPinnedTabOrder = false private var pendingNonFocusSplitFocusReassert: PendingNonFocusSplitFocusReassert? @@ -10571,7 +10585,7 @@ final class Workspace: Identifiable, ObservableObject { /// Tear down all panels in this workspace, freeing their Ghostty surfaces. /// Called before TabManager removes the workspace so child processes receive SIGHUP even if ARC deallocation is delayed. func teardownAllPanels() { - portalRenderingEnabled = false + portalRenderingPhase = .unmounted clearLayoutFollowUp() hideAllTerminalPortalViews() hideAllBrowserPortalViews() @@ -11656,8 +11670,9 @@ final class Workspace: Identifiable, ObservableObject { } func setPortalRenderingEnabled(_ enabled: Bool, reason: String) { - let changed = portalRenderingEnabled != enabled - portalRenderingEnabled = enabled + let nextPhase: PortalRenderingPhase = enabled ? .mounted : .unmounted + let changed = portalRenderingPhase != nextPhase + portalRenderingPhase = nextPhase if enabled { if changed { beginEventDrivenLayoutFollowUp( @@ -11868,10 +11883,18 @@ final class Workspace: Identifiable, ObservableObject { browserPanelId: UUID? = nil, browserExitFocusPanelId: UUID? = nil, terminalFocusPanelId: UUID? = nil, - includeGeometry: Bool = false + includeGeometry: Bool = false, + targetUpdate: LayoutFollowUpTargetUpdate = .mergePendingTargets ) { guard portalRenderingEnabled else { return } layoutFollowUpReason = reason + if case .replaceSelectionTargets(let selectedPanelId) = targetUpdate { + layoutFollowUpBrowserPanelId = nil + layoutFollowUpTerminalFocusPanelId = nil + if layoutFollowUpBrowserExitFocusPanelId != selectedPanelId { + layoutFollowUpBrowserExitFocusPanelId = nil + } + } if let browserPanelId { layoutFollowUpBrowserPanelId = browserPanelId } @@ -11905,6 +11928,18 @@ final class Workspace: Identifiable, ObservableObject { scheduleLayoutFollowUpAttempt() } + func scheduleSelectionRenderFollowUp(panelId: UUID) { + let browserPanelId = browserPanel(for: panelId) != nil ? panelId : nil + let terminalPanelId = terminalPanel(for: panelId) != nil ? panelId : nil + beginEventDrivenLayoutFollowUp( + reason: "workspace.selection", + browserPanelId: browserPanelId, + terminalFocusPanelId: terminalPanelId, + includeGeometry: true, + targetUpdate: .replaceSelectionTargets(panelId: panelId) + ) + } + private func installLayoutFollowUpObservers() { guard layoutFollowUpTimeoutWorkItem == nil else { return } diff --git a/tests_v2/test_workspace_shortcut_initial_render.py b/tests_v2/test_workspace_shortcut_initial_render.py new file mode 100644 index 00000000000..eac7f6daf63 --- /dev/null +++ b/tests_v2/test_workspace_shortcut_initial_render.py @@ -0,0 +1,166 @@ +#!/usr/bin/env python3 +""" +Regression test: a Cmd+N-created workspace must present its first terminal frame +without requiring a tab/workspace switch. + +Issue 3068 reports that the newly selected workspace can stay visibly mounted but +blank until another selection change re-triggers portal visibility updates. This +test drives the real shortcut path, creates a burst of workspaces to amplify the +attach timing churn, and then verifies the final selected terminal presents at +least one frame without any follow-up input. +""" + +from __future__ import annotations + +import os +import sys +import time +from pathlib import Path +from typing import Callable, Optional + +sys.path.insert(0, str(Path(__file__).parent)) +from cmux import cmux, cmuxError + + +SOCKET_PATH = os.environ.get("CMUX_SOCKET_PATH") or os.environ.get("CMUX_SOCKET") or cmux.DEFAULT_SOCKET_PATH +WORKSPACE_BURST = 10 +FIRST_PRESENT_TIMEOUT_S = float(os.environ.get("CMUX_FIRST_PRESENT_TIMEOUT_S", "5.0")) + + +def _wait_for( + predicate: Callable[[], bool], + *, + timeout_s: float = 5.0, + cadence_s: float = 0.05, + label: str = "condition", +) -> None: + deadline = time.time() + timeout_s + while time.time() < deadline: + if predicate(): + return + time.sleep(cadence_s) + raise cmuxError(f"Timed out waiting for {label}") + + +def _workspace_ids(c: cmux) -> list[str]: + return [workspace_id for _index, workspace_id, _title, _selected in c.list_workspaces()] + + +def _selected_workspace_id(c: cmux) -> str: + return c.current_workspace() + + +def _focused_surface_id(c: cmux, workspace_id: str) -> str: + surfaces = c.list_surfaces(workspace_id) + if not surfaces: + raise cmuxError(f"Expected at least one surface in workspace {workspace_id}") + return next((sid for _idx, sid, focused in surfaces if focused), surfaces[0][1]) + + +def _surface_health_row(c: cmux, surface_id: str) -> Optional[dict]: + needle = surface_id.lower() + for row in c.surface_health(): + if str(row.get("surface_id") or "").lower() == needle: + return row + return None + + +def _wait_for_surface_mount(c: cmux, surface_id: str) -> dict: + last_row: dict | None = None + + def ready() -> bool: + nonlocal last_row + row = _surface_health_row(c, surface_id) + if row is None: + return False + last_row = row + width = float(((row.get("hosted_view_frame") or {}).get("width")) or 0.0) + height = float(((row.get("hosted_view_frame") or {}).get("height")) or 0.0) + return ( + row.get("mapped") is True + and row.get("workspace_selected") is True + and row.get("runtime_surface_ready") is True + and row.get("hosted_view_in_window") is True + and row.get("hosted_view_has_superview") is True + and row.get("hosted_view_visible_in_ui") is True + and row.get("hosted_view_hidden") is False + and width > 0.0 + and height > 0.0 + ) + + _wait_for(ready, timeout_s=5.0, label=f"mounted selected terminal {surface_id}") + assert last_row is not None + return last_row + + +def _wait_for_first_present(c: cmux, surface_id: str) -> dict: + last_stats: dict = {} + + def presented() -> bool: + nonlocal last_stats + last_stats = c.render_stats(surface_id) + return int(last_stats.get("presentCount") or 0) > 0 + + try: + _wait_for( + presented, + timeout_s=FIRST_PRESENT_TIMEOUT_S, + cadence_s=0.05, + label=f"first presented frame for {surface_id}", + ) + except Exception as exc: + raise cmuxError( + "Newly selected workspace never presented its first terminal frame before any " + "manual tab switch.\n" + f"surface_id={surface_id}\n" + f"render_stats={last_stats}\n" + f"surface_health={_surface_health_row(c, surface_id)}" + ) from exc + return last_stats + + +def _create_workspace_via_shortcut(c: cmux, expected_count: int, previous_workspace_id: str) -> str: + c.simulate_shortcut("cmd+n") + _wait_for( + lambda: len(_workspace_ids(c)) >= expected_count, + timeout_s=4.0, + label=f"workspace count >= {expected_count} after Cmd+N", + ) + _wait_for( + lambda: _selected_workspace_id(c) != previous_workspace_id, + timeout_s=4.0, + label="workspace selection after Cmd+N", + ) + return _selected_workspace_id(c) + + +def main() -> int: + with cmux(SOCKET_PATH) as c: + c.activate_app() + time.sleep(0.25) + + starting_workspaces = _workspace_ids(c) + if not starting_workspaces: + raise cmuxError("Expected at least one workspace before Cmd+N burst") + + selected_workspace_id = _selected_workspace_id(c) + expected_count = len(starting_workspaces) + + for _ in range(WORKSPACE_BURST): + expected_count += 1 + selected_workspace_id = _create_workspace_via_shortcut( + c, + expected_count=expected_count, + previous_workspace_id=selected_workspace_id, + ) + + surface_id = _focused_surface_id(c, selected_workspace_id) + _wait_for_surface_mount(c, surface_id) + _wait_for_first_present(c, surface_id) + + print("PASS: Cmd+N-created workspace presents its first terminal frame without a tab switch") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests_v2/test_workspace_switch_initial_render.py b/tests_v2/test_workspace_switch_initial_render.py new file mode 100644 index 00000000000..04160fc7655 --- /dev/null +++ b/tests_v2/test_workspace_switch_initial_render.py @@ -0,0 +1,148 @@ +#!/usr/bin/env python3 +""" +Regression test: switching to an already-existing workspace must render its first +visible frame without requiring a second switch away and back. + +Issue 3068's broader symptom family includes existing workspaces that stay blank +on the first return, then recover on a second workspace/tab switch. This test +warms two workspaces, repeatedly switches between them, and requires the selected +terminal surface to present a new frame on every first switch. +""" + +from __future__ import annotations + +import os +import sys +import time +from pathlib import Path +from typing import Callable, Optional + +sys.path.insert(0, str(Path(__file__).parent)) +from cmux import cmux, cmuxError + + +SOCKET_PATH = os.environ.get("CMUX_SOCKET_PATH") or os.environ.get("CMUX_SOCKET") or cmux.DEFAULT_SOCKET_PATH +SWITCH_CYCLES = 4 +PRESENT_ADVANCE_TIMEOUT_S = float(os.environ.get("CMUX_PRESENT_ADVANCE_TIMEOUT_S", "5.0")) + + +def _wait_for( + predicate: Callable[[], bool], + *, + timeout_s: float = 5.0, + cadence_s: float = 0.05, + label: str = "condition", +) -> None: + deadline = time.time() + timeout_s + while time.time() < deadline: + if predicate(): + return + time.sleep(cadence_s) + raise cmuxError(f"Timed out waiting for {label}") + + +def _focused_surface_id(c: cmux, workspace_id: str) -> str: + surfaces = c.list_surfaces(workspace_id) + if not surfaces: + raise cmuxError(f"Expected at least one surface in workspace {workspace_id}") + return next((sid for _idx, sid, focused in surfaces if focused), surfaces[0][1]) + + +def _surface_health_row(c: cmux, surface_id: str) -> Optional[dict]: + needle = surface_id.lower() + for row in c.surface_health(): + if str(row.get("surface_id") or "").lower() == needle: + return row + return None + + +def _wait_for_surface_mount(c: cmux, surface_id: str) -> dict: + last_row: dict | None = None + + def ready() -> bool: + nonlocal last_row + row = _surface_health_row(c, surface_id) + if row is None: + return False + last_row = row + width = float(((row.get("hosted_view_frame") or {}).get("width")) or 0.0) + height = float(((row.get("hosted_view_frame") or {}).get("height")) or 0.0) + return ( + row.get("mapped") is True + and row.get("workspace_selected") is True + and row.get("runtime_surface_ready") is True + and row.get("hosted_view_in_window") is True + and row.get("hosted_view_has_superview") is True + and row.get("hosted_view_visible_in_ui") is True + and row.get("hosted_view_hidden") is False + and width > 0.0 + and height > 0.0 + ) + + _wait_for(ready, timeout_s=5.0, label=f"mounted selected terminal {surface_id}") + assert last_row is not None + return last_row + + +def _wait_for_present_advance(c: cmux, surface_id: str, baseline_present: int, label: str) -> dict: + last_stats: dict = {} + + def presented() -> bool: + nonlocal last_stats + last_stats = c.render_stats(surface_id) + return int(last_stats.get("presentCount") or 0) > baseline_present + + try: + _wait_for(presented, timeout_s=PRESENT_ADVANCE_TIMEOUT_S, cadence_s=0.05, label=label) + except Exception as exc: + raise cmuxError( + "Selected workspace never presented a new terminal frame on its first return.\n" + f"label={label}\n" + f"surface_id={surface_id}\n" + f"baseline_present={baseline_present}\n" + f"render_stats={last_stats}\n" + f"surface_health={_surface_health_row(c, surface_id)}" + ) from exc + return last_stats + + +def _select_workspace(c: cmux, workspace_id: str) -> None: + c.select_workspace(workspace_id) + _wait_for(lambda: c.current_workspace() == workspace_id, timeout_s=4.0, label=f"workspace {workspace_id} selected") + + +def main() -> int: + with cmux(SOCKET_PATH) as c: + c.activate_app() + time.sleep(0.25) + + workspace_a = c.new_workspace() + workspace_b = c.new_workspace() + + _select_workspace(c, workspace_a) + surface_a = _focused_surface_id(c, workspace_a) + _wait_for_surface_mount(c, surface_a) + _wait_for_present_advance(c, surface_a, baseline_present=0, label="warm workspace A") + + _select_workspace(c, workspace_b) + surface_b = _focused_surface_id(c, workspace_b) + _wait_for_surface_mount(c, surface_b) + _wait_for_present_advance(c, surface_b, baseline_present=0, label="warm workspace B") + + for cycle in range(SWITCH_CYCLES): + baseline_a = int(c.render_stats(surface_a).get("presentCount") or 0) + _select_workspace(c, workspace_a) + _wait_for_surface_mount(c, surface_a) + _wait_for_present_advance(c, surface_a, baseline_a, label=f"switch to workspace A cycle {cycle}") + + baseline_b = int(c.render_stats(surface_b).get("presentCount") or 0) + _select_workspace(c, workspace_b) + _wait_for_surface_mount(c, surface_b) + _wait_for_present_advance(c, surface_b, baseline_b, label=f"switch to workspace B cycle {cycle}") + + print("PASS: existing workspaces render on the first switch back") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main())