diff --git a/crates/noa-app/src/app.rs b/crates/noa-app/src/app.rs index 5273040..dcfffa5 100644 --- a/crates/noa-app/src/app.rs +++ b/crates/noa-app/src/app.rs @@ -20,7 +20,7 @@ use noa_grid::{ CursorStyle, PromptJump, Terminal, modes::{MouseFormat, MouseTracking}, }; -use noa_pty::{Pty, PtyConfig}; +use noa_pty::{Pty, PtyConfig, PtyWriter}; use noa_render::{ BackgroundImage, CardPipeline, CardStyle, CardTexturePlacement, CardTilePlacement, CommandPaletteSnapshot, FrameSnapshot, HoverLink, OverviewThumbnailResources, PaletteRow, @@ -764,28 +764,24 @@ impl App { .map(|state| state.window.clone()) } - fn app_cursor_keys(&self, window_id: WindowId) -> bool { + /// The three keyboard-encoding modes read on every key-encode pass, + /// returned `(app_cursor_keys, app_keypad, kitty_keyboard_flags)` under a + /// single terminal lock. One acquisition rather than three keeps the + /// key-input path off the io thread's output-batch lock longer than + /// necessary (input-latency under heavy pty output). + fn key_encode_modes(&self, window_id: WindowId) -> (bool, bool, u8) { self.windows .get(&window_id) .and_then(WindowState::focused_surface) - .map(|surface| surface.terminal.lock().modes.app_cursor_keys()) - .unwrap_or(false) - } - - fn app_keypad(&self, window_id: WindowId) -> bool { - self.windows - .get(&window_id) - .and_then(WindowState::focused_surface) - .map(|surface| surface.terminal.lock().modes.app_keypad()) - .unwrap_or(false) - } - - fn kitty_keyboard_flags(&self, window_id: WindowId) -> u8 { - self.windows - .get(&window_id) - .and_then(WindowState::focused_surface) - .map(|surface| surface.terminal.lock().kitty_keyboard_flags()) - .unwrap_or(0) + .map(|surface| { + let terminal = surface.terminal.lock(); + ( + terminal.modes.app_cursor_keys(), + terminal.modes.app_keypad(), + terminal.kitty_keyboard_flags(), + ) + }) + .unwrap_or((false, false, 0)) } fn focus_reporting(&self, window_id: WindowId) -> bool { diff --git a/crates/noa-app/src/app/event_loop.rs b/crates/noa-app/src/app/event_loop.rs index a80d313..24d8710 100644 --- a/crates/noa-app/src/app/event_loop.rs +++ b/crates/noa-app/src/app/event_loop.rs @@ -724,9 +724,7 @@ impl ApplicationHandler for App { ) { return; } - let app_cursor_keys = self.app_cursor_keys(window_id); - let app_keypad = self.app_keypad(window_id); - let kitty_flags = self.kitty_keyboard_flags(window_id); + let (app_cursor_keys, app_keypad, kitty_flags) = self.key_encode_modes(window_id); let unmodified_key = event.key_without_modifiers(); // On macOS, Option only acts as Alt when winit stripped its // composition per `macos-option-as-alt` — i.e. the delivered diff --git a/crates/noa-app/src/app/input_ops/terminal.rs b/crates/noa-app/src/app/input_ops/terminal.rs index e404893..4efe087 100644 --- a/crates/noa-app/src/app/input_ops/terminal.rs +++ b/crates/noa-app/src/app/input_ops/terminal.rs @@ -282,7 +282,30 @@ impl App { return crate::io_thread::QueueInputResult::Disconnected; }; match &surface.transport { - SurfaceTransport::Local(local) => local.pty_input_tx.queue(bytes), + // Reserve against the pane's shared byte budget, then write + // straight to the PTY writer thread — bypassing the io thread's + // output-batch loop so a keystroke isn't stuck behind a large + // pty-output batch. The reservation travels with the bytes and is + // released after the real write, preserving the overflow-cap + // accounting `PtyInputQueue::queue` would apply. + SurfaceTransport::Local(local) => match local.pty_input_tx.reserve(bytes) { + Some(reserved) => { + // The echo-repaint generation advances when the writer + // thread completes the real PTY write (the wrapper's + // Drop), not here: output the io thread was already + // parsing predates this input and must not consume the + // echo debt. + let stamped = crate::io_thread::EchoStampedInput::new( + reserved, + local.input_echo_seq.clone(), + ); + match local.pty_writer.write_owned(stamped) { + Ok(()) => crate::io_thread::QueueInputResult::Queued, + Err(_) => crate::io_thread::QueueInputResult::Disconnected, + } + } + None => crate::io_thread::QueueInputResult::Dropped, + }, SurfaceTransport::Remote(remote) => { if remote .connection diff --git a/crates/noa-app/src/app/lifecycle.rs b/crates/noa-app/src/app/lifecycle.rs index dc09b33..b145a65 100644 --- a/crates/noa-app/src/app/lifecycle.rs +++ b/crates/noa-app/src/app/lifecycle.rs @@ -270,10 +270,49 @@ impl App { let caps = surface.get_capabilities(&adapter); let alpha_blending = alpha_blending_mode(&self.config.font); + + // If the window landed on a scale factor the monitor probe didn't + // predict, the probe-sized surface would be the wrong physical + // size. Settle the scale-corrected size *before* the first + // `configure`/present so only one CAMetalLayer drawable generation + // is ever allocated: drawables are allocated lazily by the first + // `get_current_texture` (the present below), never by `configure` + // alone, so a second `request_inner_size`-driven generation would + // otherwise stay resident alongside the correct one. Deriving the + // corrected size needs the real window-scale metrics, so on this + // path the terminal font stack is joined up front; the common + // (matched-scale) path still presents the startup frame before the + // join to keep first paint early. + let font_px = font_pixel_size(self.runtime_font_size, window_scale_factor); + let mut terminal_stack_handle = Some(terminal_stack_handle); + let mut prebuilt_font: Option = None; + let mut configure_size = None; + if (window_scale_factor - monitor_scale_factor).abs() > f64::EPSILON { + let font = build_terminal_font( + terminal_stack_handle.take().expect("handle present"), + font_px, + font_config_from_noa_config(&self.config.font), + ); + let corrected = initial_window_logical_size( + font.metrics(), + initial_grid_size, + window_scale_factor, + self.padding, + ); + // macOS applies this synchronously and returns the new size, so + // the surface is configured at the corrected size on its first + // (and only) `configure`. A platform that applies it + // asynchronously returns `None`; the fall-through to + // `window.inner_size()` then keeps the pre-correction size and + // the later `Resized` reconfigures as before. + configure_size = window.request_inner_size(corrected); + prebuilt_font = Some(font); + } + let surface_config = build_surface_config( &caps, alpha_blending, - window.inner_size(), + configure_size.unwrap_or_else(|| window.inner_size()), self.config.background_opacity < 1.0, ); surface.configure(&device, &surface_config); @@ -340,35 +379,16 @@ impl App { // The window is on screen; now join the terminal font stack (the // worker has been running since the top of `crate::run`) and - // finish grid construction at the window's actual scale. - let font = { - let stack = match terminal_stack_handle.join() { - Ok(Ok(stack)) => stack, - Ok(Err(e)) => panic!("failed to load a system monospace font: {e:?}"), - Err(_) => panic!("failed to load a system monospace font: worker panicked"), - }; - let font = FontGrid::with_stack( - stack, - font_pixel_size(self.runtime_font_size, window_scale_factor), + // finish grid construction at the window's actual scale. The + // scale-mismatch path above already joined it to size the surface. + let font = match prebuilt_font { + Some(font) => font, + None => build_terminal_font( + terminal_stack_handle.take().expect("handle present"), + font_px, font_config_from_noa_config(&self.config.font), - ) - .expect("failed to load a system monospace font"); - crate::startup_trace::mark("font-ready"); - font + ), }; - // The probe metrics that sized the window were computed at the - // monitor's scale; if the window landed on a different scale the - // real metrics differ — correct the size now (the Resized event - // reconfigures the surface), matching the old inline-rebuild path. - if (window_scale_factor - monitor_scale_factor).abs() > f64::EPSILON { - let corrected = initial_window_logical_size( - font.metrics(), - initial_grid_size, - window_scale_factor, - self.padding, - ); - let _ = window.request_inner_size(corrected); - } self.gpu = Some(GpuState { instance, @@ -766,6 +786,10 @@ impl App { terminal.clone(), pty_input_tx.clone(), ); + // Main-thread writer clone taken before `pty` moves into the io thread, + // so keyboard/paste input writes straight to the writer thread. + let pty_writer = pty.writer(); + let input_echo_seq = Arc::new(AtomicU64::new(0)); let io_thread = crate::io_thread::spawn( pty, terminal.clone(), @@ -774,6 +798,7 @@ impl App { resize_rx, pty_input_rx, auto_approve_feedback_rx, + input_echo_seq.clone(), overview_publish, sidebar_publish, auto_approve, @@ -786,6 +811,8 @@ impl App { terminal, SurfaceTransport::Local(LocalSurfaceTransport { pty_input_tx, + pty_writer, + input_echo_seq, auto_approve_feedback_tx, resize_tx, io_thread: Some(io_thread), @@ -1443,6 +1470,25 @@ impl App { } } +/// Join the terminal font-discovery worker and build the primary [`FontGrid`] +/// at `px_size`. A join or parse failure is fatal — the terminal cannot render +/// without a monospace face — matching the old inline load. +fn build_terminal_font( + handle: std::thread::JoinHandle>, + px_size: f32, + font_cfg: noa_font::FontConfig, +) -> FontGrid { + let stack = match handle.join() { + Ok(Ok(stack)) => stack, + Ok(Err(e)) => panic!("failed to load a system monospace font: {e:?}"), + Err(_) => panic!("failed to load a system monospace font: worker panicked"), + }; + let font = FontGrid::with_stack(stack, px_size, font_cfg) + .expect("failed to load a system monospace font"); + crate::startup_trace::mark("font-ready"); + font +} + fn remote_status_preview(text: &str) -> Vec { vec![vec![crate::session_store::PreviewSpan { text: text.to_string(), diff --git a/crates/noa-app/src/app/state.rs b/crates/noa-app/src/app/state.rs index 5639e3c..5035d27 100644 --- a/crates/noa-app/src/app/state.rs +++ b/crates/noa-app/src/app/state.rs @@ -975,6 +975,22 @@ pub(super) enum SurfaceTransport { pub(super) struct LocalSurfaceTransport { pub(super) pty_input_tx: crate::io_thread::PtyInputQueue, + /// Main-thread handle to the PTY writer thread. Keyboard/paste/IME input + /// is written here directly (after reserving on `pty_input_tx`), skipping + /// the io thread's output-batch loop so a keystroke never waits out a + /// large pty-output batch (input-latency fix). The writer channel is MPSC: + /// the io thread's own DSR/DA replies share it, and independent-producer + /// interleaving is acceptable (same model as a separated termio writer). + pub(super) pty_writer: PtyWriter, + /// Shared with this pane's io thread: incremented when the writer thread + /// completes an input's real PTY write (`EchoStampedInput`'s Drop), so + /// the echo batch bypasses the redraw floor. A generation counter rather + /// than a bool: the io thread consumes only the generation it observed at + /// redraw-decision time, so an input landing between its load and consume + /// is never lost — and advancing at the real write (not queue time) keeps + /// output parsed before the write from consuming the debt. See + /// `io_thread::spawn`'s `input_echo_served`. + pub(super) input_echo_seq: Arc, pub(super) auto_approve_feedback_tx: Sender, pub(super) resize_tx: Sender, pub(super) io_thread: Option, diff --git a/crates/noa-app/src/io_thread.rs b/crates/noa-app/src/io_thread.rs index 10e8dcb..5a7462d 100644 --- a/crates/noa-app/src/io_thread.rs +++ b/crates/noa-app/src/io_thread.rs @@ -62,7 +62,7 @@ use sidebar::*; use spawn::*; pub(crate) use auto_approve::{AutoApproveFeedback, AutoApprovePublish}; -pub(crate) use input_queue::{PtyInputQueue, QueueInputResult, input_channel}; +pub(crate) use input_queue::{EchoStampedInput, PtyInputQueue, QueueInputResult, input_channel}; pub(crate) use ipc_tap::IpcOutputTap; pub(crate) use overview::{OverviewPublish, publish_overview_snapshot}; pub(crate) use raw_attach::RawAttachTap; diff --git a/crates/noa-app/src/io_thread/input_queue.rs b/crates/noa-app/src/io_thread/input_queue.rs index ca1b5e1..3c77169 100644 --- a/crates/noa-app/src/io_thread/input_queue.rs +++ b/crates/noa-app/src/io_thread/input_queue.rs @@ -2,7 +2,7 @@ //! ordered overflow buffer for bursts (huge pastes) the channel can't absorb. use std::sync::Arc; -use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering}; use crossbeam_channel::{Receiver, Sender, TrySendError}; use parking_lot::Mutex; @@ -67,6 +67,36 @@ impl Drop for QueuedPtyInput { } } +/// A reserved input that advances the pane's echo-repaint generation +/// (`input_echo_seq`) only when the writer thread drops it — i.e. after the +/// real PTY write completes (or the write is abandoned on a dead pane, where +/// the extra repaint is moot). Bumping at queue time instead would let an +/// output batch the io thread was already parsing — necessarily pre-echo +/// output — consume the debt, sending the actual echo through the normal +/// redraw floor. +pub(crate) struct EchoStampedInput { + input: QueuedPtyInput, + echo_seq: Arc, +} + +impl EchoStampedInput { + pub(crate) fn new(input: QueuedPtyInput, echo_seq: Arc) -> Self { + Self { input, echo_seq } + } +} + +impl AsRef<[u8]> for EchoStampedInput { + fn as_ref(&self) -> &[u8] { + self.input.as_ref() + } +} + +impl Drop for EchoStampedInput { + fn drop(&mut self) { + self.echo_seq.fetch_add(1, Ordering::Relaxed); + } +} + #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum QueueInputResult { Queued, @@ -123,6 +153,19 @@ impl PtyInputQueue { } } + /// Reserve `input` against this pane's shared byte budget without routing + /// it through the io thread's input channel, returning the reservation + /// wrapper for the caller to hand straight to the PTY writer (the + /// main-thread input fast path — keystrokes must not wait out the io + /// thread's output batch). `None` means the pane is at + /// [`PTY_INPUT_PENDING_BYTE_CAP`] and the input must be dropped, exactly + /// as [`queue`](Self::queue) would return [`QueueInputResult::Dropped`]. + /// The returned wrapper shares this pane's budget with `queue`, so both + /// paths are capped together. + pub(crate) fn reserve(&self, input: PtyInput) -> Option { + QueuedPtyInput::reserve(input, Arc::clone(&self.pending_bytes)).ok() + } + /// Queue `input` behind every byte accepted before it, blocking never. pub(crate) fn queue(&self, input: PtyInput) -> QueueInputResult { let Ok(input) = QueuedPtyInput::reserve(input, Arc::clone(&self.pending_bytes)) else { diff --git a/crates/noa-app/src/io_thread/spawn.rs b/crates/noa-app/src/io_thread/spawn.rs index 8465ba7..94512ae 100644 --- a/crates/noa-app/src/io_thread/spawn.rs +++ b/crates/noa-app/src/io_thread/spawn.rs @@ -7,6 +7,7 @@ //! crossbeam channels. use std::sync::Arc; +use std::sync::atomic::{AtomicU64, Ordering}; use std::time::{Duration, Instant}; use parking_lot::Mutex; @@ -30,7 +31,7 @@ use super::feed::{ PtyDrainTerminalEvent, capture_pty_bytes, drain_queued_pty_data, feed_terminal_batch, open_pty_capture, }; -use super::input_queue::QueuedPtyInput; +use super::input_queue::{EchoStampedInput, QueuedPtyInput}; use super::ipc_tap::{IpcOutputTap, flush_pending_ipc_output, force_ipc_output_refresh}; use super::overview::{OverviewPublish, flush_pending_overview_publish}; use super::raw_attach::RawAttachTap; @@ -202,6 +203,7 @@ pub fn spawn( resize_rx: Receiver, input_rx: Receiver, auto_approve_feedback_rx: Receiver, + input_echo_seq: Arc, overview: OverviewPublish, sidebar: SidebarPublish, auto_approve: AutoApprovePublish, @@ -263,15 +265,20 @@ pub fn spawn( // is owed and the select below blocks exactly as before. let mut redraw_deadline: Option = None; let mut auto_approve_rescan_at: Option = None; - // True while this pane owes a repaint for a user-input echo: set when - // input bytes are forwarded to the pty, cleared by the next redraw - // that actually fires. The next pty-output batch (the echo, when the - // program echoes at all) then bypasses the redraw floor via + // Generation of the last user input whose echo repaint this thread + // has served: the pane owes an echo repaint while `input_echo_seq` + // (bumped by the writer thread as it completes each real PTY input + // write — see `EchoStampedInput`) is ahead of it. + // The next pty-output batch (the echo, when the program echoes at + // all) then bypasses the redraw floor via // [`RedrawFloor::decide_input_echo`] instead of being withheld up to - // one floor interval behind another pane's recent paint. At most one - // bypass per input event, so a non-echoing program costs one extra - // repaint per keystroke at worst — bounded by typing speed. - let mut input_echo_pending = false; + // one floor interval behind another pane's recent paint. Consuming + // records only the generation observed at decision time, so an input + // that lands between the load and the consume stays owed — a shared + // bool cleared here would drop it. At most one bypass per input + // event, so a non-echoing program costs one extra repaint per + // keystroke at worst — bounded by typing speed. + let mut input_echo_served: u64 = 0; // Short-window pty traffic gauge for the hot-traffic spin gate: no // recent data (idle pane) or bulk-rate data (flood, whatever the // per-read chunk size) means every park below stays a plain block, @@ -333,10 +340,12 @@ pub fn spawn( String::from_utf8_lossy(bytes.as_ref()) ); } - if let Err(err) = writer.write_owned(bytes) { + // Stamped so the echo generation advances at the real + // PTY write, not at queue time (see `EchoStampedInput`). + let stamped = EchoStampedInput::new(bytes, input_echo_seq.clone()); + if let Err(err) = writer.write_owned(stamped) { log::warn!("failed to queue bytes to pty: {err}"); } - input_echo_pending = true; did_work = true; } Err(TryRecvError::Disconnected) => break, // main thread / App dropped @@ -460,22 +469,24 @@ pub fn spawn( // nothing — skip the redraw poke and floor bookkeeping // entirely instead of waking the main thread to snapshot // an unchanged frame while the next query of the burst is - // waiting on the same terminal lock. `input_echo_pending` + // waiting on the same terminal lock. The echo debt // stays armed: an echo can only arrive in a batch that // actually prints, which dirties the display. let redraw = if output.display_dirty { - Some(if input_echo_pending { + let echo_seq = input_echo_seq.load(Ordering::Relaxed); + let decision = if echo_seq != input_echo_served { redraw_floor .decide_input_echo(output.synchronized_output, Instant::now()) } else { redraw_floor.decide(output.synchronized_output, Instant::now()) - }) + }; + if matches!(decision, RedrawDecision::Now) { + input_echo_served = echo_seq; + } + Some(decision) } else { None }; - if matches!(redraw, Some(RedrawDecision::Now)) { - input_echo_pending = false; - } for text in output.pending_clipboard_writes { let _ = proxy.send_event(UserEvent::ClipboardWrite { window_id,