diff --git a/app/src-tauri/src/main.rs b/app/src-tauri/src/main.rs index 7051c40..87289c6 100644 --- a/app/src-tauri/src/main.rs +++ b/app/src-tauri/src/main.rs @@ -26,12 +26,27 @@ /// their own init time, which happens inside the Tauri builder that /// function calls into, not at binary load. /// -/// A user who has already set this themselves is left alone. That includes -/// setting it to `0`, on the assumption WebKitGTK treats it as a boolean -/// rather than presence-only — not verified against WebKitGTK's own source, -/// so if it turns out to be presence-only, `=0` still reads as "set" here -/// and disables DMA-BUF the same as any other value, which is at least the -/// safe direction to be wrong in. +/// A user who has already set this themselves is left alone — with one +/// correction. The earlier version of this function left *any* pre-set value +/// alone, including `0`, on the assumption WebKitGTK reads the variable as a +/// boolean. WebKitGTK reads it as presence-only, so `WEBKIT_DISABLE_DMABUF_ +/// RENDERER=0` disabled DMA-BUF exactly like `=1` did, and there was no value +/// at all a user could set to get the accelerated path back: the escape hatch +/// the comment described did not exist. `0`, `false` and empty are now treated +/// as an explicit opt-out and the variable is *removed*, which is the only +/// thing WebKitGTK reads as "enabled". The default is unchanged — unset still +/// means disabled on Linux, so nobody who was not deliberately overriding this +/// sees any difference. +/// +/// That matters more than it looks, because the trade described above is not +/// the trade actually being made. `@xterm/addon-webgl` does not fall back to +/// the canvas renderer here: its constructor throws only when WebGL is +/// *absent*, and with DMA-BUF disabled WebGL is still present — served by +/// software rasterisation. So the addon loads happily and every terminal frame +/// is rendered on the CPU and copied, which is slower than the canvas renderer +/// this comment assumed it would degrade to, not faster. See +/// `terminal_gpu_rendering` in `AppSettings` for the switch that decides +/// whether the addon is loaded at all. /// /// This env var also leaks to whatever the app spawns afterwards — notably /// a cold-launched default browser via the `opener` plugin's `xdg-open` @@ -39,10 +54,77 @@ /// URL; most non-WebKitGTK browsers ignore the variable entirely), but /// worth knowing before chasing the "links don't open" half of triple-c#34 /// as a separate, unrelated cause. +#[cfg(target_os = "linux")] +const DMABUF_VAR: &str = "WEBKIT_DISABLE_DMABUF_RENDERER"; + +/// What to do with `WEBKIT_DISABLE_DMABUF_RENDERER`, given whatever it is +/// already set to. Split from the mutation so it can be tested without +/// touching process-wide environment state from a parallel test runner. +#[cfg(target_os = "linux")] +#[derive(Debug, PartialEq, Eq)] +enum DmabufAction { + /// Not set by the user — apply the workaround. + Disable, + /// Explicitly opted out. WebKitGTK reads presence, not value, so the only + /// way to express "enabled" is for the variable not to exist. + Remove, + /// Set to something meaning "disabled". Already what we want; leave it. + LeaveAlone, +} + +#[cfg(target_os = "linux")] +fn dmabuf_action(current: Option<&str>) -> DmabufAction { + match current { + None => DmabufAction::Disable, + Some(value) => match value.trim().to_ascii_lowercase().as_str() { + "" | "0" | "false" | "no" => DmabufAction::Remove, + _ => DmabufAction::LeaveAlone, + }, + } +} + #[cfg(target_os = "linux")] fn apply_webkit_wayland_workaround() { - if std::env::var_os("WEBKIT_DISABLE_DMABUF_RENDERER").is_none() { - std::env::set_var("WEBKIT_DISABLE_DMABUF_RENDERER", "1"); + let current = std::env::var(DMABUF_VAR).ok(); + match dmabuf_action(current.as_deref()) { + DmabufAction::Disable => std::env::set_var(DMABUF_VAR, "1"), + DmabufAction::Remove => std::env::remove_var(DMABUF_VAR), + DmabufAction::LeaveAlone => {} + } +} + +#[cfg(all(test, target_os = "linux"))] +mod tests { + use super::{dmabuf_action, DmabufAction}; + + #[test] + fn unset_gets_the_workaround() { + assert_eq!(dmabuf_action(None), DmabufAction::Disable); + } + + #[test] + fn falsey_values_opt_out_by_removing_the_variable() { + // The bug this replaces: these all previously read as "user set it, + // leave it alone", and WebKitGTK then disabled DMA-BUF anyway because + // it only checks presence. There was no way to ask for the GPU path. + for value in ["0", "false", "no", "", " 0 ", "FALSE", "No"] { + assert_eq!( + dmabuf_action(Some(value)), + DmabufAction::Remove, + "{value:?} should opt out" + ); + } + } + + #[test] + fn other_values_are_left_alone() { + for value in ["1", "true", "yes", "anything"] { + assert_eq!( + dmabuf_action(Some(value)), + DmabufAction::LeaveAlone, + "{value:?} should be left alone" + ); + } } } diff --git a/app/src-tauri/src/models/app_settings.rs b/app/src-tauri/src/models/app_settings.rs index a5bcfb8..26758f9 100644 --- a/app/src-tauri/src/models/app_settings.rs +++ b/app/src-tauri/src/models/app_settings.rs @@ -135,6 +135,26 @@ pub struct AppSettings { pub gateway: GatewaySettings, #[serde(default)] pub global_claude_code_settings: Option, + /// Whether the terminal loads `@xterm/addon-webgl`. + /// + /// `None` is "auto", and auto is not the same answer on every platform. + /// On Linux the app disables WebKitGTK's DMA-BUF renderer at startup (see + /// `apply_webkit_wayland_workaround` in `main.rs`, and triple-c#34), which + /// does not remove WebGL — it leaves it backed by software rasterisation. + /// The addon therefore loads successfully and then renders every frame on + /// the CPU, which is slower than the canvas renderer it would otherwise + /// have fallen back to. So auto means enabled on macOS and Windows, and + /// disabled on Linux. + /// + /// `Some(true)` / `Some(false)` force it either way on any platform. A + /// Linux user running X11, or one whose driver stack is unaffected, can + /// turn it back on; anyone seeing terminal lag can turn it off without + /// waiting for a release. Deliberately `Option` rather than `bool`: + /// the zero value has to mean "we choose", not "off", or every existing + /// settings file would silently pin the answer at whatever the default was + /// the day it was written. + #[serde(default)] + pub terminal_gpu_rendering: Option, } fn default_stt_model() -> String { @@ -226,6 +246,7 @@ impl Default for AppSettings { stt: SttSettings::default(), gateway: GatewaySettings::default(), global_claude_code_settings: None, + terminal_gpu_rendering: None, } } } diff --git a/app/src/components/settings/SettingsPanel.tsx b/app/src/components/settings/SettingsPanel.tsx index ea11494..6eba2cb 100644 --- a/app/src/components/settings/SettingsPanel.tsx +++ b/app/src/components/settings/SettingsPanel.tsx @@ -15,6 +15,8 @@ import type { EnvVar } from "../../lib/types"; import Tooltip from "../ui/Tooltip"; import AccordionSection from "../ui/AccordionSection"; import Toggle from "../ui/Toggle"; +import SegmentedControl from "../ui/SegmentedControl"; +import { resolveTerminalGpuRendering } from "../../lib/terminalRenderer"; import WebTerminalSettings from "./WebTerminalSettings"; import SttSettings from "./SttSettings"; import SharedAuthSettings from "./SharedAuthSettings"; @@ -67,6 +69,14 @@ export default function SettingsPanel() { } }; + const handleGpuRenderingChange = async (value: "auto" | "on" | "off") => { + if (!appSettings) return; + await saveSettings({ + ...appSettings, + terminal_gpu_rendering: value === "auto" ? null : value === "on", + }); + }; + const handleAutoCheckToggle = async () => { if (!appSettings) return; await saveSettings({ ...appSettings, auto_check_updates: !appSettings.auto_check_updates }); @@ -242,6 +252,45 @@ export default function SettingsPanel() { + +
+ + +

+ Takes effect when a terminal tab is next switched to. +

+
+ +
{appVersion && ( diff --git a/app/src/components/terminal/TerminalView.tsx b/app/src/components/terminal/TerminalView.tsx index 72d6ae0..eb658bd 100644 --- a/app/src/components/terminal/TerminalView.tsx +++ b/app/src/components/terminal/TerminalView.tsx @@ -28,6 +28,7 @@ import UrlToast, { URL_TOAST_SHORTCUT, } from "./UrlToast"; import { trimSelection } from "./trimSelection"; +import { resolveTerminalGpuRendering } from "../../lib/terminalRenderer"; import TerminalContextMenu from "./TerminalContextMenu"; interface Props { @@ -95,6 +96,7 @@ export default function TerminalView({ sessionId, active }: Props) { const webglRef = useRef(null); const detectorRef = useRef(null); const { sendInput, pasteImage, resize, onOutput, onExit } = useTerminal(); + const gpuRenderingSetting = useAppState(s => s.appSettings?.terminal_gpu_rendering ?? null); const setTerminalHasSelection = useAppState(s => s.setTerminalHasSelection); const setTerminalAtBottom = useAppState(s => s.setTerminalAtBottom); const setScrollActiveToBottom = useAppState(s => s.setScrollActiveToBottom); @@ -491,7 +493,11 @@ export default function TerminalView({ sessionId, active }: Props) { // Handle user input -> backend const inputDisposable = term.onData((data) => { - sendInput(sessionId, data); + // Ordered and coalesced by the queue in `useTerminal`; a rejection here + // means the session is gone, which the exit listener already reports. + sendInput(sessionId, data).catch((e) => + console.error("Failed to send terminal input:", e) + ); }); // Detect user-initiated scroll-up (mouse wheel) to pause auto-follow. @@ -684,7 +690,16 @@ export default function TerminalView({ sessionId, active }: Props) { const term = termRef.current; if (!term) return; - if (active) { + // Auto on macOS/Windows, off on Linux, overridable either way — see + // `resolveTerminalGpuRendering`. Loading the addon under a software-GL + // WebKitGTK is slower than xterm's canvas renderer, not faster. + const useGpu = resolveTerminalGpuRendering(gpuRenderingSetting, navigator.userAgent); + + // The renderer and the activation work are independent: a terminal with + // GPU rendering switched off still has to fit and take focus when its tab + // becomes active. Keeping these in one branch made "GPU off" silently mean + // "never re-fit, never focus". + if (active && useGpu) { // Attach WebGL renderer if (!webglRef.current) { try { @@ -699,19 +714,21 @@ export default function TerminalView({ sessionId, active }: Props) { // WebGL not available, canvas renderer is fine } } + } else if (webglRef.current) { + // Release the context — for inactive terminals, and when the setting + // turns GPU rendering off while this terminal is on screen. + try { webglRef.current.dispose(); } catch { /* ignore */ } + webglRef.current = null; + } + + if (active) { fitRef.current?.fit(); if (autoFollowRef.current) { term.scrollToBottom(); } term.focus(); - } else { - // Release WebGL context for inactive terminals - if (webglRef.current) { - try { webglRef.current.dispose(); } catch { /* ignore */ } - webglRef.current = null; - } } - }, [active]); + }, [active, gpuRenderingSetting]); // Auto-dismiss toast after 30 seconds — unless the user is standing in it. // A keyboard user who has just jumped into the toast is mid-decision, and diff --git a/app/src/hooks/useTerminal.test.ts b/app/src/hooks/useTerminal.test.ts new file mode 100644 index 0000000..f766476 --- /dev/null +++ b/app/src/hooks/useTerminal.test.ts @@ -0,0 +1,83 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// The queue lives at module scope in useTerminal, so the command layer is +// mocked and the hook's `sendInput` is exercised through `renderHook`. +const terminalInput = vi.fn<(sessionId: string, data: number[]) => Promise>(); + +vi.mock("../lib/tauri-commands", () => ({ + terminalInput: (sessionId: string, data: number[]) => terminalInput(sessionId, data), + openTerminalSession: vi.fn(), + closeTerminalSession: vi.fn(), + terminalResize: vi.fn(), + pasteImageToTerminal: vi.fn(), + updateProject: vi.fn(), +})); + +vi.mock("@tauri-apps/api/event", () => ({ listen: vi.fn() })); + +import { renderHook } from "@testing-library/react"; +import { useTerminal } from "./useTerminal"; + +const decode = (bytes: number[]) => new TextDecoder().decode(new Uint8Array(bytes)); + +describe("useTerminal input ordering", () => { + beforeEach(() => { + terminalInput.mockReset(); + }); + + it("preserves order even when the underlying invokes resolve out of order", async () => { + // Make the *first* call the slowest, which is exactly the race that put a + // backspace behind the characters typed after it. + const resolvers: Array<() => void> = []; + terminalInput.mockImplementation( + () => new Promise((resolve) => resolvers.push(resolve)), + ); + + const { result } = renderHook(() => useTerminal()); + + const first = result.current.sendInput("s1", "\x7f"); // backspace + const rest = ["a", "b", "c"].map((ch) => result.current.sendInput("s1", ch)); + + // Only one write may be in flight at a time. + expect(terminalInput).toHaveBeenCalledTimes(1); + expect(decode(terminalInput.mock.calls[0][1])).toBe("\x7f"); + + resolvers.shift()!(); + await first; + + // The three queued keystrokes coalesce into one ordered write. + expect(terminalInput).toHaveBeenCalledTimes(2); + expect(decode(terminalInput.mock.calls[1][1])).toBe("abc"); + + resolvers.shift()!(); + await Promise.all(rest); + + const sent = terminalInput.mock.calls.map((c) => decode(c[1])).join(""); + expect(sent).toBe("\x7fabc"); + }); + + it("settles each caller's promise and does not drop later writes on failure", async () => { + terminalInput.mockRejectedValueOnce(new Error("boom")).mockResolvedValue(undefined); + + const { result } = renderHook(() => useTerminal()); + + await expect(result.current.sendInput("s2", "x")).rejects.toThrow("boom"); + await expect(result.current.sendInput("s2", "y")).resolves.toBeUndefined(); + + expect(decode(terminalInput.mock.calls[1][1])).toBe("y"); + }); + + it("keeps separate sessions independent", async () => { + terminalInput.mockResolvedValue(undefined); + const { result } = renderHook(() => useTerminal()); + + await Promise.all([ + result.current.sendInput("a", "1"), + result.current.sendInput("b", "2"), + ]); + + const bySession = terminalInput.mock.calls.map((c) => [c[0], decode(c[1])]); + expect(bySession).toContainEqual(["a", "1"]); + expect(bySession).toContainEqual(["b", "2"]); + }); +}); diff --git a/app/src/hooks/useTerminal.ts b/app/src/hooks/useTerminal.ts index 0539263..5e2da3a 100644 --- a/app/src/hooks/useTerminal.ts +++ b/app/src/hooks/useTerminal.ts @@ -4,6 +4,86 @@ import { listen } from "@tauri-apps/api/event"; import { useAppState } from "../store/appState"; import * as commands from "../lib/tauri-commands"; +/** + * Per-session ordered write queue. + * + * Every keystroke used to be its own `invoke("terminal_input")`, and because + * that command is `async` on the Rust side Tauri spawns each one as an + * independent task. Those tasks then race for the session mutex in + * `ExecSessionManager::send_input`, so nothing preserved the order the bytes + * were typed in — the visible symptom was a backspace landing *after* the + * characters typed behind it. The serial writer task downstream cannot help, + * because the order is already lost by the time anything reaches the channel. + * + * The queue restores ordering the same way the web terminal gets it for free: + * one write in flight at a time, the next only after the previous resolves. + * Anything typed while a write is in flight coalesces into the next chunk, + * which also collapses a burst of typing into a couple of IPC round trips + * rather than one per key. Concatenating the byte arrays is safe — a PTY + * cannot tell one write of "ab" from writes of "a" then "b" — and each + * caller's promise still settles only when its own bytes have gone, so + * `await sendInput(...)` keeps the meaning it had. + * + * Module scope, not hook scope, because `useTerminal()` is called from several + * components (App for speech-to-text, TerminalView for typing and image paste, + * useProjectActions for tile commands). A per-hook queue would give each caller + * its own ordering and leave them racing against each other. + */ +type PendingWrite = { + bytes: number[]; + resolve: () => void; + reject: (reason: unknown) => void; +}; + +const inputQueues = new Map(); + +async function drainInputQueue(sessionId: string): Promise { + const q = inputQueues.get(sessionId); + if (!q || q.draining) return; + + q.draining = true; + try { + while (q.pending.length > 0) { + // Take everything queued so far as one batch, preserving order. + const batch = q.pending.splice(0, q.pending.length); + const bytes = batch.flatMap((w) => w.bytes); + try { + await commands.terminalInput(sessionId, bytes); + batch.forEach((w) => w.resolve()); + } catch (err) { + // Reject only the writes in this batch. Anything queued while it was + // in flight is still pending and gets its own attempt on the next lap. + batch.forEach((w) => w.reject(err)); + } + } + } finally { + q.draining = false; + // Drop the entry once idle so closed sessions do not accumulate. + if (q.pending.length === 0) inputQueues.delete(sessionId); + } +} + +function enqueueInput(sessionId: string, bytes: number[]): Promise { + return new Promise((resolve, reject) => { + let q = inputQueues.get(sessionId); + if (!q) { + q = { pending: [], draining: false }; + inputQueues.set(sessionId, q); + } + q.pending.push({ bytes, resolve, reject }); + void drainInputQueue(sessionId); + }); +} + +/** Drop any queued input for a session that is going away. */ +function discardInputQueue(sessionId: string): void { + const q = inputQueues.get(sessionId); + if (!q) return; + const dropped = q.pending.splice(0, q.pending.length); + dropped.forEach((w) => w.reject(new Error(`Session ${sessionId} closed`))); + if (!q.draining) inputQueues.delete(sessionId); +} + export function useTerminal() { const { sessions, activeSessionId, addSession, removeSession, setActiveSession } = useAppState( @@ -33,6 +113,7 @@ export function useTerminal() { const session = currentSessions.find((s) => s.id === sessionId); const project = session ? projects.find((p) => p.id === session.projectId) : undefined; + discardInputQueue(sessionId); await commands.closeTerminalSession(sessionId); removeSession(sessionId); @@ -54,7 +135,7 @@ export function useTerminal() { const sendInput = useCallback( async (sessionId: string, data: string) => { const bytes = Array.from(new TextEncoder().encode(data)); - await commands.terminalInput(sessionId, bytes); + await enqueueInput(sessionId, bytes); }, [], ); diff --git a/app/src/lib/terminalRenderer.test.ts b/app/src/lib/terminalRenderer.test.ts new file mode 100644 index 0000000..e0e3c19 --- /dev/null +++ b/app/src/lib/terminalRenderer.test.ts @@ -0,0 +1,39 @@ +import { describe, it, expect } from "vitest"; +import { isLinuxWebview, resolveTerminalGpuRendering } from "./terminalRenderer"; + +const LINUX = "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/605.1.15 Safari/605.1.15"; +const MAC = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 Safari/605.1.15"; +const WINDOWS = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 Chrome/120 Safari/537.36"; +const ANDROID = "Mozilla/5.0 (Linux; Android 14) AppleWebKit/537.36 Chrome/120 Mobile Safari/537.36"; + +describe("isLinuxWebview", () => { + it("recognises desktop Linux", () => { + expect(isLinuxWebview(LINUX)).toBe(true); + }); + + it("does not count Android as desktop Linux", () => { + expect(isLinuxWebview(ANDROID)).toBe(false); + }); + + it("rejects the other desktop platforms", () => { + expect(isLinuxWebview(MAC)).toBe(false); + expect(isLinuxWebview(WINDOWS)).toBe(false); + }); +}); + +describe("resolveTerminalGpuRendering", () => { + it("auto is off on Linux, where WebGL falls back to software rendering", () => { + expect(resolveTerminalGpuRendering(null, LINUX)).toBe(false); + expect(resolveTerminalGpuRendering(undefined, LINUX)).toBe(false); + }); + + it("auto is on elsewhere", () => { + expect(resolveTerminalGpuRendering(null, MAC)).toBe(true); + expect(resolveTerminalGpuRendering(null, WINDOWS)).toBe(true); + }); + + it("an explicit setting wins on every platform", () => { + expect(resolveTerminalGpuRendering(true, LINUX)).toBe(true); + expect(resolveTerminalGpuRendering(false, MAC)).toBe(false); + }); +}); diff --git a/app/src/lib/terminalRenderer.ts b/app/src/lib/terminalRenderer.ts new file mode 100644 index 0000000..f2416ef --- /dev/null +++ b/app/src/lib/terminalRenderer.ts @@ -0,0 +1,28 @@ +/** + * Decides whether the terminal loads `@xterm/addon-webgl`. + * + * Split out of `TerminalView` so it can be unit-tested without standing up a + * terminal, and so the platform rule lives in exactly one place. + */ + +/** True when the webview is running on Linux (WebKitGTK), excluding Android. */ +export function isLinuxWebview(userAgent: string): boolean { + return /\bLinux\b/.test(userAgent) && !/\bAndroid\b/.test(userAgent); +} + +/** + * Resolve the effective WebGL setting. + * + * `setting` is `AppSettings.terminal_gpu_rendering`: `true`/`false` force the + * answer, `null`/`undefined` mean auto. Auto is on everywhere except Linux — + * there the app disables WebKitGTK's DMA-BUF renderer at startup (triple-c#34), + * which leaves WebGL present but software-rasterised, so loading the addon is + * slower than the canvas renderer it would otherwise have fallen back to. + */ +export function resolveTerminalGpuRendering( + setting: boolean | null | undefined, + userAgent: string, +): boolean { + if (typeof setting === "boolean") return setting; + return !isLinuxWebview(userAgent); +} diff --git a/app/src/lib/types.ts b/app/src/lib/types.ts index ae2250c..1fab346 100644 --- a/app/src/lib/types.ts +++ b/app/src/lib/types.ts @@ -290,6 +290,12 @@ export interface AppSettings { stt: SttSettings; gateway: GatewaySettings; global_claude_code_settings: ClaudeCodeSettings | null; + /** Whether the terminal loads the WebGL renderer. `null` is auto: on + * everywhere except Linux, where the DMA-BUF workaround leaves WebGL + * backed by software rasterisation and the addon ends up slower than the + * canvas renderer it would otherwise fall back to. See + * `resolveTerminalGpuRendering` in `lib/terminalRenderer.ts`. */ + terminal_gpu_rendering: boolean | null; } /** What `preview_settings_import` returns before anything is applied —