diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b1eed5..b03a884 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,33 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- Kitty temporary-file transfers require both an approved temporary directory + and the protocol filename marker. Shared-memory transfers validate the whole + requested range and copy through the kernel; `S` is a byte count independent + of `O`. Failed placements still enforce the image storage quota. +- PNG decoding expands packed grayscale, palette colors and transparency, and + checks decoded size before allocating pixel buffers. GPU uploads respect the + device dimension limit; deleting and recreating an image cannot reuse stale + texture contents. +- PTY exit waits for the reader's final output, with a two-second drain deadline + for descendants retaining the slave. User input and terminal replies share a + nonblocking byte budget; shutdown cancels writes waiting for PTY capacity. +- Search finds text across soft wraps and highlights every matching row. CSI + requests exceeding the parameter limit are ignored without changing accepted + parameter values. Panes sharing a redraw deadline emit one redraw notification. + +### Changed + +- Search runs on immutable snapshots in a worker with 35 ms debounce and + cancellation of superseded queries, moving history scans outside the UI + thread and terminal lock. +- Image storage uses ID/number/age indexes and caps image/frame metadata counts. + Static image placements reuse GPU uniform buffers and bind groups. + ## [0.2.9] - 2026-09-05 ### Fixed diff --git a/crates/noa-app/src/app.rs b/crates/noa-app/src/app.rs index f7d3fb1..92f71b4 100644 --- a/crates/noa-app/src/app.rs +++ b/crates/noa-app/src/app.rs @@ -280,6 +280,7 @@ pub struct App { next_path_probe_generation: u64, /// The open search prompt (Cmd+F), if any — see [`SearchPromptSession`]. search_prompt: Option, + search_worker: Option, /// Keyboard copy mode, bound to exactly one focused window/pane. copy_mode: Option, /// Physical presses consumed by copy mode whose matching Kitty release @@ -619,16 +620,23 @@ impl StartupTasks { })) .map_err(|e| format!("no compatible GPU adapter found ({e})"))?; crate::startup_trace::mark("gpu-adapter-ready"); - let (device, queue) = - pollster::block_on(adapter.request_device(&wgpu::DeviceDescriptor { + let (device, queue) = pollster::block_on( + adapter.request_device(&wgpu::DeviceDescriptor { label: Some("noa-device"), required_features: wgpu::Features::empty(), - required_limits: wgpu::Limits::default(), + required_limits: wgpu::Limits { + max_texture_dimension_2d: adapter + .limits() + .max_texture_dimension_2d + .min(noa_grid::kitty::MAX_IMAGE_DIM), + ..wgpu::Limits::default() + }, experimental_features: wgpu::ExperimentalFeatures::default(), memory_hints: wgpu::MemoryHints::default(), trace: wgpu::Trace::Off, - })) - .map_err(|e| format!("could not open a GPU device ({e})"))?; + }), + ) + .map_err(|e| format!("could not open a GPU device ({e})"))?; crate::startup_trace::mark("gpu-device-ready"); Ok(PrewarmedGpu { instance, @@ -794,6 +802,7 @@ impl App { path_probe_cache: HashMap::new(), next_path_probe_generation: 0, search_prompt: None, + search_worker: None, copy_mode: None, copy_mode_suppressed_releases: HashSet::new(), copy_mode_suppressed_repeats: HashSet::new(), diff --git a/crates/noa-app/src/app/event_loop.rs b/crates/noa-app/src/app/event_loop.rs index a0f9e3e..030d662 100644 --- a/crates/noa-app/src/app/event_loop.rs +++ b/crates/noa-app/src/app/event_loop.rs @@ -283,6 +283,13 @@ impl ApplicationHandler for App { title, text, } => self.show_file_preview(window_id, pane_id, title, text), + UserEvent::SearchUpdated(window_id, pane_id) => { + if let Some(window_id) = self.resolve_pane_window(window_id, pane_id) + && let Some(state) = self.windows.get(&window_id) + { + state.window.request_redraw(); + } + } UserEvent::Redraw(window_id, pane_id) => { #[cfg(target_os = "macos")] if let Some(panel) = &self.text_panel { diff --git a/crates/noa-app/src/app/input_ops/search.rs b/crates/noa-app/src/app/input_ops/search.rs index 0a62c6d..0031a75 100644 --- a/crates/noa-app/src/app/input_ops/search.rs +++ b/crates/noa-app/src/app/input_ops/search.rs @@ -1,6 +1,28 @@ use super::super::*; use super::ActiveOverlay; +fn navigate_search( + worker: Option<&crate::search_worker::SearchWorker>, + target: &Arc>, + terminal: &mut noa_grid::Terminal, + action: SearchAction, +) { + if worker + .is_some_and(|worker| worker.queue_navigation(target, terminal.screen_generation(), action)) + { + return; + } + match action { + SearchAction::FindNext => { + terminal.search_next(); + } + SearchAction::FindPrevious => { + terminal.search_previous(); + } + _ => unreachable!("only search navigation is handled here"), + } +} + impl App { pub(in crate::app) fn handle_search_action(&mut self, action: SearchAction) { let Some((window_id, pane_id)) = @@ -8,7 +30,7 @@ impl App { else { return; }; - let Some(terminal) = self + let Some(target) = self .windows .get(&window_id) .and_then(|state| state.surfaces.get(&pane_id)) @@ -17,7 +39,7 @@ impl App { return; }; - let mut terminal = terminal.lock(); + let mut terminal = target.lock(); match action { SearchAction::Find => { // Only one prompt is tracked app-wide; cmd+f while one is @@ -45,13 +67,15 @@ impl App { } return; } - SearchAction::FindNext => { - terminal.search_next(); + SearchAction::FindNext | SearchAction::FindPrevious => { + navigate_search(self.search_worker.as_ref(), &target, &mut terminal, action); } - SearchAction::FindPrevious => { - terminal.search_previous(); + SearchAction::Clear => { + if let Some(worker) = &self.search_worker { + worker.cancel(); + } + terminal.clear_search(); } - SearchAction::Clear => terminal.clear_search(), } drop(terminal); @@ -167,11 +191,33 @@ impl App { else { return; }; - { - let mut terminal = terminal.lock(); - match effect { - SearchPromptEffect::UpdateQuery(query) => terminal.set_search_query(query), - SearchPromptEffect::ClearQuery => terminal.clear_search(), + match effect { + SearchPromptEffect::UpdateQuery(query) => { + if self.search_worker.is_none() { + match crate::search_worker::SearchWorker::new() { + Ok(worker) => self.search_worker = Some(worker), + Err(err) => { + log::warn!("could not start search worker: {err}"); + return; + } + } + } + let proxy = self.proxy.clone(); + let screen_generation = terminal.lock().screen_generation(); + self.search_worker.as_ref().unwrap().submit( + Arc::downgrade(&terminal), + screen_generation, + query, + move || { + let _ = proxy.send_event(UserEvent::SearchUpdated(window_id, pane_id)); + }, + ); + } + SearchPromptEffect::ClearQuery => { + if let Some(worker) = &self.search_worker { + worker.cancel(); + } + terminal.lock().clear_search(); } } if let Some(state) = self.windows.get(&window_id) { @@ -187,6 +233,9 @@ impl App { let Some(session) = self.search_prompt.take() else { return; }; + if clear && let Some(worker) = &self.search_worker { + worker.cancel(); + } if clear && let Some(terminal) = self .windows @@ -201,3 +250,80 @@ impl App { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::search_worker::SearchWorker; + use noa_core::GridSize; + use noa_grid::Terminal; + use parking_lot::Mutex; + use std::time::Duration; + + #[test] + fn navigation_during_search_matches_synchronous_navigation() { + for (text, old_query, actions) in [ + ("foo foo", "", vec![SearchAction::FindNext]), + ("foo foo foo", "", vec![SearchAction::FindPrevious]), + ( + "foo foo foo", + "", + vec![ + SearchAction::FindNext, + SearchAction::FindNext, + SearchAction::FindPrevious, + ], + ), + ("foo far foo", "f", vec![SearchAction::FindNext]), + ("foo far foo", "f", vec![SearchAction::FindPrevious]), + ("absent", "", vec![SearchAction::FindNext]), + ] { + let make_terminal = || { + let mut terminal = Terminal::new(GridSize::new(20, 3)); + noa_vt::Stream::new().feed(text.as_bytes(), &mut terminal); + if !old_query.is_empty() { + terminal.set_search_query(old_query); + terminal.search_previous(); + } + terminal + }; + let mut expected = make_terminal(); + expected.set_search_query("foo"); + for action in &actions { + match action { + SearchAction::FindNext => { + expected.search_next(); + } + SearchAction::FindPrevious => { + expected.search_previous(); + } + _ => unreachable!(), + } + } + let terminal = Arc::new(Mutex::new(make_terminal())); + let worker = SearchWorker::new().unwrap(); + let mut guard = terminal.lock(); + let old_search = guard.active().search.clone(); + let (tx, rx) = crossbeam_channel::bounded(1); + worker.submit( + Arc::downgrade(&terminal), + guard.screen_generation(), + "foo".into(), + move || { + let _ = tx.send(()); + }, + ); + for action in actions { + navigate_search(Some(&worker), &terminal, &mut guard, action); + } + assert_eq!(guard.active().search, old_search); + drop(guard); + rx.recv_timeout(Duration::from_secs(2)).unwrap(); + assert_eq!( + terminal.lock().active().search, + expected.active().search, + "text={text:?}, old_query={old_query:?}" + ); + } + } +} diff --git a/crates/noa-app/src/app/input_ops/terminal.rs b/crates/noa-app/src/app/input_ops/terminal.rs index 7a4ee4d..f067b75 100644 --- a/crates/noa-app/src/app/input_ops/terminal.rs +++ b/crates/noa-app/src/app/input_ops/terminal.rs @@ -311,7 +311,7 @@ impl App { reserved, local.input_echo_seq.clone(), ); - match local.pty_writer.write_owned(stamped) { + match local.pty_writer.write_reserved(stamped) { Ok(()) => crate::io_thread::QueueInputResult::Queued, Err(_) => crate::io_thread::QueueInputResult::Disconnected, } diff --git a/crates/noa-app/src/app/lifecycle.rs b/crates/noa-app/src/app/lifecycle.rs index 2fa2a47..18382d6 100644 --- a/crates/noa-app/src/app/lifecycle.rs +++ b/crates/noa-app/src/app/lifecycle.rs @@ -809,7 +809,8 @@ impl App { let kitty_animation_flag = terminal.kitty_animation_flag(); let terminal = Arc::new(Mutex::new(terminal)); let (resize_tx, resize_rx) = crossbeam_channel::unbounded(); - let (pty_input_tx, pty_input_rx) = crate::io_thread::input_channel(); + let (pty_input_tx, pty_input_rx) = + crate::io_thread::input_channel_with_budget(pty.writer().budget()); let (auto_approve_feedback_tx, auto_approve_feedback_rx) = crossbeam_channel::unbounded(); let auto_approve_guards = Arc::new(Mutex::new( crate::auto_approve::AutoApproveInputGuards::default(), diff --git a/crates/noa-app/src/events.rs b/crates/noa-app/src/events.rs index b59da58..67d61ef 100644 --- a/crates/noa-app/src/events.rs +++ b/crates/noa-app/src/events.rs @@ -72,6 +72,7 @@ pub enum UserEvent { }, /// New terminal output is available; request a redraw. Redraw(WindowId, PaneId), + SearchUpdated(WindowId, PaneId), /// A hover-path existence probe finished on its worker thread /// (`App::hover_link_target` never stats the filesystem on the main /// thread — a network volume can block a metadata query indefinitely). @@ -133,11 +134,15 @@ pub enum UserEvent { /// `sendText`) is waiting on the main thread (noa-server spec DEC-C). The /// payload lives in `App::ipc_pending`, keyed by `request_id`, because /// `UserEvent` derives `Eq` and so cannot carry a reply channel directly. - IpcAction { request_id: u64 }, + IpcAction { + request_id: u64, + }, /// A remote discovery or create-pane worker completed. Panels and worker /// results remain in `App::remote_pending`; the Eq event carries only its /// monotonic lookup id and never carries the bearer token. - RemoteRequestCompleted { request_id: u64 }, + RemoteRequestCompleted { + request_id: u64, + }, /// Deferred focus restore after a macOS native-tab close. Calling /// `focus_window()` synchronously from `close_tab` collides with AppKit /// still transferring key/firstResponder to the sibling tab, leaving the @@ -145,7 +150,9 @@ pub enum UserEvent { /// winit's text-input view — so `keyDown:` never reaches winit and input /// goes dead. Posting through the proxy re-runs the focus on a fresh /// event-loop iteration, after AppKit has finished the tab teardown. - RestoreFocus { window_id: WindowId }, + RestoreFocus { + window_id: WindowId, + }, } /// Whether an AppleScript-driven spawn joins the focused window's tab group or diff --git a/crates/noa-app/src/io_thread.rs b/crates/noa-app/src/io_thread.rs index 4db0ad2..01171ee 100644 --- a/crates/noa-app/src/io_thread.rs +++ b/crates/noa-app/src/io_thread.rs @@ -62,7 +62,11 @@ use sidebar::*; use spawn::*; pub(crate) use auto_approve::{AutoApproveFeedback, AutoApprovePublish}; -pub(crate) use input_queue::{EchoStampedInput, PtyInputQueue, QueueInputResult, input_channel}; +#[cfg(test)] +pub(crate) use input_queue::input_channel; +pub(crate) use input_queue::{ + EchoStampedInput, PtyInputQueue, QueueInputResult, input_channel_with_budget, +}; 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 3c77169..546e617 100644 --- a/crates/noa-app/src/io_thread/input_queue.rs +++ b/crates/noa-app/src/io_thread/input_queue.rs @@ -1,8 +1,9 @@ //! Main-thread → io-thread pty input queueing: the bounded channel plus its //! ordered overflow buffer for bursts (huge pastes) the channel can't absorb. +use noa_pty::{BudgetedWrite, PtyWriteBudget}; use std::sync::Arc; -use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering}; +use std::sync::atomic::{AtomicU64, Ordering}; use crossbeam_channel::{Receiver, Sender, TrySendError}; use parking_lot::Mutex; @@ -15,57 +16,29 @@ pub(crate) const PTY_INPUT_QUEUE_CAPACITY: usize = 1024; /// channel and its overflow buffer. A message-count-only limit is insufficient: /// raw attach accepts 1 MiB messages, so 1024 channel slots could otherwise pin /// roughly 1 GiB before the overflow limit was even consulted. -pub(super) const PTY_INPUT_PENDING_BYTE_CAP: usize = 8 * 1024 * 1024; +#[cfg(test)] +pub(super) const PTY_INPUT_PENDING_BYTE_CAP: usize = noa_pty::WRITE_BYTE_CAP; /// Small frames are charged at least this much so container/allocation /// overhead is bounded along with payload bytes. -pub(super) const PTY_INPUT_PENDING_MIN_CHARGE: usize = 1024; +#[cfg(test)] +pub(super) const PTY_INPUT_PENDING_MIN_CHARGE: usize = noa_pty::WRITE_MIN_CHARGE; +#[cfg(test)] pub(crate) fn input_channel() -> (PtyInputQueue, Receiver) { + input_channel_with_budget(PtyWriteBudget::default()) +} + +pub(crate) fn input_channel_with_budget( + budget: PtyWriteBudget, +) -> (PtyInputQueue, Receiver) { let (tx, rx) = crossbeam_channel::bounded(PTY_INPUT_QUEUE_CAPACITY); - (PtyInputQueue::new(tx), rx) + (PtyInputQueue::new(tx, budget), rx) } /// Input plus a shared byte-budget reservation. The reservation follows the /// bytes through the channel, overflow queue, and PTY writer queue and is /// released only after the real PTY write completes (or the bytes are dropped). -pub(crate) struct QueuedPtyInput { - bytes: PtyInput, - pending_bytes: Arc, - charge: usize, -} - -impl QueuedPtyInput { - fn reserve(input: PtyInput, pending_bytes: Arc) -> Result { - let charge = input.len().max(PTY_INPUT_PENDING_MIN_CHARGE); - if pending_bytes - .fetch_update(Ordering::AcqRel, Ordering::Acquire, |current| { - current - .checked_add(charge) - .filter(|next| *next <= PTY_INPUT_PENDING_BYTE_CAP) - }) - .is_err() - { - return Err(input); - } - Ok(Self { - bytes: input, - pending_bytes, - charge, - }) - } -} - -impl AsRef<[u8]> for QueuedPtyInput { - fn as_ref(&self) -> &[u8] { - self.bytes.as_ref() - } -} - -impl Drop for QueuedPtyInput { - fn drop(&mut self) { - self.pending_bytes.fetch_sub(self.charge, Ordering::AcqRel); - } -} +pub(crate) type QueuedPtyInput = BudgetedWrite; /// 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 @@ -75,13 +48,13 @@ impl Drop for QueuedPtyInput { /// output — consume the debt, sending the actual echo through the normal /// redraw floor. pub(crate) struct EchoStampedInput { - input: QueuedPtyInput, + input: PtyInput, echo_seq: Arc, } impl EchoStampedInput { - pub(crate) fn new(input: QueuedPtyInput, echo_seq: Arc) -> Self { - Self { input, echo_seq } + pub(crate) fn new(input: QueuedPtyInput, echo_seq: Arc) -> BudgetedWrite { + input.map(|input| Self { input, echo_seq }) } } @@ -120,7 +93,7 @@ pub(crate) enum QueueInputResult { pub(crate) struct PtyInputQueue { tx: Sender, overflow: Arc>, - pending_bytes: Arc, + budget: PtyWriteBudget, } #[derive(Default)] @@ -145,11 +118,11 @@ impl InputOverflow { } impl PtyInputQueue { - fn new(tx: Sender) -> Self { + fn new(tx: Sender, budget: PtyWriteBudget) -> Self { PtyInputQueue { tx, overflow: Arc::new(Mutex::new(InputOverflow::default())), - pending_bytes: Arc::new(AtomicUsize::new(0)), + budget, } } @@ -163,12 +136,12 @@ impl PtyInputQueue { /// 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() + self.budget.reserve(input).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 { + let Ok(input) = self.budget.reserve(input) else { return QueueInputResult::Dropped; }; let mut overflow = self.overflow.lock(); diff --git a/crates/noa-app/src/io_thread/redraw.rs b/crates/noa-app/src/io_thread/redraw.rs index 6afcd1f..37f386d 100644 --- a/crates/noa-app/src/io_thread/redraw.rs +++ b/crates/noa-app/src/io_thread/redraw.rs @@ -141,7 +141,7 @@ fn instant_from_nanos(nanos: u64) -> Instant { /// Sentinel meaning "no redraw recorded yet" in [`RedrawFloor::last_redraw_at`]. /// Real timestamps are nudged to at least 1ns past the epoch (see -/// [`RedrawFloor::claim`]) so they never collide with it. +/// [`RedrawFloor::record`]) so they never collide with it. const NEVER: u64 = 0; /// A window's redraw-floor clock, shared by every pane's io thread in that @@ -187,29 +187,12 @@ impl RedrawFloor { Duration::from_nanos(self.min_interval_nanos.load(Ordering::Relaxed)) } - fn last_redraw(&self) -> Option { - match self.last_redraw_at.load(Ordering::Acquire) { - NEVER => None, - nanos => Some(instant_from_nanos(nanos)), - } - } - - /// Records `at` as a redraw and reports whether it is the most recent - /// one recorded so far. `fetch_max` makes this safe to call concurrently - /// from every pane's io thread in the window: only the caller whose - /// timestamp actually advances the clock gets `true` back, so panes that - /// raced to the same floor deadline converge on a single winner instead - /// of each sending its own wake. - fn claim(&self, at: Instant) -> bool { - let at_nanos = nanos_since_epoch(at).max(1); // never collide with NEVER (0) - self.last_redraw_at.fetch_max(at_nanos, Ordering::AcqRel) < at_nanos - } - /// Unconditionally record a redraw that is happening regardless of the /// floor (e.g. one triggered by an unrelated per-pane throttle), so the /// shared clock stays accurate for other panes in this window. pub(super) fn record(&self, at: Instant) { - let _ = self.claim(at); + self.last_redraw_at + .fetch_max(nanos_since_epoch(at).max(1), Ordering::AcqRel); } /// Decide whether a just-fed batch should trigger a redraw against this @@ -217,20 +200,57 @@ impl RedrawFloor { /// is recorded here so the next pane to ask — in this window, on any /// thread — sees it. pub(super) fn decide(&self, synchronized: bool, now: Instant) -> RedrawDecision { - let decision = - decide_redraw_floor(synchronized, self.last_redraw(), now, self.min_interval()); - if matches!(decision, RedrawDecision::Now) { - self.claim(now); + loop { + let previous = self.last_redraw_at.load(Ordering::Acquire); + let last = (previous != NEVER).then(|| instant_from_nanos(previous)); + let decision = decide_redraw_floor(synchronized, last, now, self.min_interval()); + if !matches!(decision, RedrawDecision::Now) { + return decision; + } + if self + .last_redraw_at + .compare_exchange( + previous, + nanos_since_epoch(now).max(1), + Ordering::AcqRel, + Ordering::Acquire, + ) + .is_ok() + { + return decision; + } } - decision } /// Attempt to fire an owed redraw deadline. Every pane suppressed within /// the same floor window computes the identical shared deadline (it's /// derived from this same clock), so without this guard they'd all fire - /// in the same tick; `claim` lets exactly one through. - pub(super) fn claim_deadline(&self, now: Instant) -> bool { - self.claim(now) + /// in the same tick. A loser keeps its paint debt until the next floor: + /// the winning event may already have snapshotted that loser's pane. + pub(super) fn claim_deadline(&self, deadline: Instant, now: Instant) -> RedrawDecision { + if now < deadline { + return RedrawDecision::Suppress { deadline }; + } + loop { + let previous = self.last_redraw_at.load(Ordering::Acquire); + if previous != NEVER && instant_from_nanos(previous) >= deadline { + return RedrawDecision::Suppress { + deadline: instant_from_nanos(previous) + self.min_interval(), + }; + } + if self + .last_redraw_at + .compare_exchange( + previous, + nanos_since_epoch(now).max(1), + Ordering::AcqRel, + Ordering::Acquire, + ) + .is_ok() + { + return RedrawDecision::Now; + } + } } /// [`RedrawFloor::decide`] for a batch that carries a user-input echo: diff --git a/crates/noa-app/src/io_thread/spawn.rs b/crates/noa-app/src/io_thread/spawn.rs index 6c47682..05567a8 100644 --- a/crates/noa-app/src/io_thread/spawn.rs +++ b/crates/noa-app/src/io_thread/spawn.rs @@ -372,7 +372,7 @@ pub fn spawn( // 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) { + if let Err(err) = writer.write_reserved(stamped) { log::warn!("failed to queue bytes to pty: {err}"); } did_work = true; @@ -659,7 +659,7 @@ pub fn spawn( deadline_elapsed = true; } let mut redraw_claimed = false; - if redraw_deadline.is_some_and(|deadline| now >= deadline) { + if let Some(deadline) = redraw_deadline.filter(|deadline| now >= *deadline) { // A withheld redraw (floor or synchronized-output cap) came // due — force the repaint so the stale frame can't persist. // Every pane suppressed within the same floor window shares @@ -668,9 +668,12 @@ pub fn spawn( // of every pane firing its own wake in the same tick. redraw_deadline = None; deadline_elapsed = true; - if redraw_floor.lock().claim_deadline(now) { - needs_redraw = true; - redraw_claimed = true; + match redraw_floor.lock().claim_deadline(deadline, now) { + RedrawDecision::Now => { + needs_redraw = true; + redraw_claimed = true; + } + RedrawDecision::Suppress { deadline } => redraw_deadline = Some(deadline), } } if auto_approve_rescan_at.is_some_and(|deadline| now >= deadline) { diff --git a/crates/noa-app/src/io_thread/tests.rs b/crates/noa-app/src/io_thread/tests.rs index 01413f9..5c2da40 100644 --- a/crates/noa-app/src/io_thread/tests.rs +++ b/crates/noa-app/src/io_thread/tests.rs @@ -1768,17 +1768,42 @@ fn redraw_floor_claim_deadline_lets_only_one_pane_through() { let pane_c = floor.clone(); let deadline = Instant::now(); - assert!(pane_a.claim_deadline(deadline), "first claim wins"); - assert!( - !pane_b.claim_deadline(deadline), - "same instant already claimed" + assert_eq!( + pane_a.claim_deadline(deadline, deadline), + RedrawDecision::Now ); - assert!( - !pane_c.claim_deadline(deadline), - "same instant already claimed" + let next = deadline + Duration::from_millis(10); + for (pane, delay) in [(pane_b.clone(), 1), (pane_c, 2)] { + assert_eq!( + pane.claim_deadline(deadline, deadline + Duration::from_micros(delay)), + RedrawDecision::Suppress { deadline: next } + ); + } + assert_eq!(pane_b.claim_deadline(next, next), RedrawDecision::Now); +} + +#[test] +fn concurrent_redraw_decisions_have_one_winner() { + let floor = RedrawFloor::new(Duration::from_millis(10)); + let barrier = Arc::new(std::sync::Barrier::new(8)); + let now = Instant::now(); + let workers: Vec<_> = (0..8) + .map(|_| { + let floor = floor.clone(); + let barrier = barrier.clone(); + std::thread::spawn(move || { + barrier.wait(); + floor.decide(false, now) + }) + }) + .collect(); + assert_eq!( + workers + .into_iter() + .filter_map(|w| (w.join().unwrap() == RedrawDecision::Now).then_some(())) + .count(), + 1 ); - // A genuinely later redraw can still be claimed afterward. - assert!(pane_b.claim_deadline(deadline + Duration::from_millis(1))); } // A user-input echo bypasses the redraw floor: even when the window painted diff --git a/crates/noa-app/src/lib.rs b/crates/noa-app/src/lib.rs index 126ec3a..7edcea3 100644 --- a/crates/noa-app/src/lib.rs +++ b/crates/noa-app/src/lib.rs @@ -38,6 +38,7 @@ mod remote_attach; mod scrollback_crypt; mod scrollback_persist; mod search_prompt; +mod search_worker; mod secure_input; mod session; pub mod session_overview; diff --git a/crates/noa-app/src/search_worker.rs b/crates/noa-app/src/search_worker.rs new file mode 100644 index 0000000..a3aa0b9 --- /dev/null +++ b/crates/noa-app/src/search_worker.rs @@ -0,0 +1,403 @@ +//! Latest-query search on immutable snapshots; no history scan on the UI thread. + +use noa_grid::Terminal; +use parking_lot::{Condvar, Mutex}; +use std::sync::{ + Arc, Weak, + atomic::{AtomicU64, Ordering}, +}; +use std::time::Duration; + +use crate::commands::SearchAction; + +const DEBOUNCE: Duration = Duration::from_millis(35); + +struct Job { + terminal: Weak>, + screen_generation: u64, + query: String, + generation: u64, + notify: Box, +} + +#[derive(Default)] +struct Pending { + job: Option, + navigation: Option, + shutdown: bool, +} + +struct PendingNavigation { + terminal: Weak>, + screen_generation: u64, + actions: Vec, +} + +#[derive(Default)] +struct Shared { + pending: Mutex, + ready: Condvar, + generation: AtomicU64, + #[cfg(test)] + after_snapshot: Mutex>>, +} + +pub(crate) struct SearchWorker { + shared: Arc, +} + +impl SearchWorker { + pub(crate) fn new() -> std::io::Result { + let shared = Arc::new(Shared::default()); + let work = shared.clone(); + std::thread::Builder::new() + .name("noa-search".into()) + .spawn(move || run(work))?; + Ok(Self { shared }) + } + + pub(crate) fn submit( + &self, + terminal: Weak>, + screen_generation: u64, + query: String, + notify: impl Fn() + Send + 'static, + ) { + let mut pending = self.shared.pending.lock(); + let generation = self.shared.generation.fetch_add(1, Ordering::AcqRel) + 1; + pending.navigation = Some(PendingNavigation { + terminal: terminal.clone(), + screen_generation, + actions: Vec::new(), + }); + pending.job = Some(Job { + terminal, + screen_generation, + query, + generation, + notify: Box::new(notify), + }); + self.shared.ready.notify_one(); + } + + /// Called under the target terminal lock, just like result publication. + /// The request stays available after the worker takes the debounced job. + pub(crate) fn queue_navigation( + &self, + terminal: &Arc>, + screen_generation: u64, + action: SearchAction, + ) -> bool { + debug_assert!(matches!( + action, + SearchAction::FindNext | SearchAction::FindPrevious + )); + let mut pending = self.shared.pending.lock(); + let Some(navigation) = &mut pending.navigation else { + return false; + }; + if !navigation.terminal.ptr_eq(&Arc::downgrade(terminal)) + || navigation.screen_generation != screen_generation + { + return false; + } + navigation.actions.push(action); + true + } + + pub(crate) fn cancel(&self) { + let mut pending = self.shared.pending.lock(); + self.shared.generation.fetch_add(1, Ordering::AcqRel); + pending.job = None; + pending.navigation = None; + self.shared.ready.notify_one(); + } +} + +impl Drop for SearchWorker { + fn drop(&mut self) { + let mut pending = self.shared.pending.lock(); + pending.shutdown = true; + pending.job = None; + pending.navigation = None; + self.shared.generation.fetch_add(1, Ordering::AcqRel); + self.shared.ready.notify_one(); + } +} + +fn run(shared: Arc) { + loop { + let job = { + let mut pending = shared.pending.lock(); + while pending.job.is_none() && !pending.shutdown { + shared.ready.wait(&mut pending); + } + if pending.shutdown { + return; + } + // Each edit restarts debounce; the slot retains at most one query. + while !shared.ready.wait_for(&mut pending, DEBOUNCE).timed_out() { + if pending.shutdown { + return; + } + } + let Some(job) = pending.job.take() else { + continue; + }; + job + }; + let cancelled = || shared.generation.load(Ordering::Acquire) != job.generation; + while !cancelled() { + let Some(terminal) = job.terminal.upgrade() else { + break; + }; + let (snapshot, space) = { + let terminal = terminal.lock(); + if cancelled() || terminal.screen_generation() != job.screen_generation { + break; + } + ( + terminal.active().search_snapshot(), + terminal.grid_coordinate_generation(), + ) + }; + #[cfg(test)] + if let Some(after_snapshot) = shared.after_snapshot.lock().take() { + after_snapshot(); + } + let Some(matches) = snapshot.find_matches(&job.query, cancelled) else { + break; + }; + // Allocate the result's shared backing outside the terminal lock. + let matches = Arc::from(matches.into_boxed_slice()); + let applied = { + let mut terminal = terminal.lock(); + // Serialize publication with submit/cancel so a stale result + // cannot race past a newer query or restore cleared highlights. + let mut pending = shared.pending.lock(); + if pending.shutdown + || cancelled() + || terminal.screen_generation() != job.screen_generation + { + break; + } + let applied = terminal.grid_coordinate_generation() == space + && terminal.apply_search_snapshot(&snapshot, job.query.clone(), matches); + if applied && let Some(navigation) = pending.navigation.take() { + for action in navigation.actions { + match action { + SearchAction::FindNext => { + terminal.search_next(); + } + SearchAction::FindPrevious => { + terminal.search_previous(); + } + _ => unreachable!("only search navigation is queued"), + } + } + } + applied + }; + if applied { + (job.notify)(); + break; + } + // Output changed during the scan. Wait briefly before retrying, + // keeping a busy producer from driving a search spin loop. + let mut pending = shared.pending.lock(); + if pending.shutdown || cancelled() { + break; + } + shared.ready.wait_for(&mut pending, DEBOUNCE); + } + let mut pending = shared.pending.lock(); + if !cancelled() { + pending.navigation = None; + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn pause_after_snapshot( + worker: &SearchWorker, + ) -> ( + crossbeam_channel::Receiver<()>, + crossbeam_channel::Sender<()>, + ) { + let (reached_tx, reached_rx) = crossbeam_channel::bounded(1); + let (resume_tx, resume_rx) = crossbeam_channel::bounded(1); + *worker.shared.after_snapshot.lock() = Some(Box::new(move || { + reached_tx.send(()).unwrap(); + resume_rx.recv_timeout(Duration::from_secs(2)).unwrap(); + })); + (reached_rx, resume_tx) + } + + #[test] + fn screen_switch_during_search_discards_the_query() { + for (setup, switch) in [ + ("", "\x1b[?1049h"), + ("\x1b[?1049h", "\x1b[?1049l"), + ("\x1b[?47h\x1b[?47l", "\x1b[?47h\x1b[?47l"), + ("\x1b[?1049h", "\x1b[?1049h"), + ("", "\x1bc"), + ] { + for during_scan in [false, true] { + let terminal = Arc::new(Mutex::new(Terminal::new(noa_core::GridSize::new(20, 3)))); + let worker = SearchWorker::new().unwrap(); + let barrier = during_scan.then(|| pause_after_snapshot(&worker)); + let mut guard = terminal.lock(); + noa_vt::Stream::new().feed(setup.as_bytes(), &mut *guard); + let (tx, rx) = crossbeam_channel::bounded(1); + worker.submit( + Arc::downgrade(&terminal), + guard.screen_generation(), + "old".into(), + move || { + let _ = tx.send(()); + }, + ); + if let Some((reached, _)) = &barrier { + drop(guard); + reached.recv_timeout(Duration::from_secs(2)).unwrap(); + guard = terminal.lock(); + } + assert!(worker.queue_navigation( + &terminal, + guard.screen_generation(), + SearchAction::FindNext + )); + noa_vt::Stream::new().feed(switch.as_bytes(), &mut *guard); + assert!(!worker.queue_navigation( + &terminal, + guard.screen_generation(), + SearchAction::FindNext + )); + drop(guard); + if let Some((_, resume)) = barrier { + resume.send(()).unwrap(); + } + assert_eq!( + rx.recv_timeout(Duration::from_secs(2)), + Err(crossbeam_channel::RecvTimeoutError::Disconnected), + "obsolete search must be discarded: setup={setup:?}, switch={switch:?}, during_scan={during_scan}", + ); + assert!(terminal.lock().active().search.query().is_empty()); + } + } + } + + #[test] + fn navigation_during_scan_survives_retry_after_output() { + let terminal = Arc::new(Mutex::new(Terminal::new(noa_core::GridSize::new(20, 3)))); + noa_vt::Stream::new().feed(b"foo foo", &mut *terminal.lock()); + let worker = SearchWorker::new().unwrap(); + let (reached, resume) = pause_after_snapshot(&worker); + let (tx, rx) = crossbeam_channel::bounded(1); + worker.submit( + Arc::downgrade(&terminal), + terminal.lock().screen_generation(), + "foo".into(), + move || { + tx.send(()).unwrap(); + }, + ); + reached.recv_timeout(Duration::from_secs(2)).unwrap(); + { + let mut guard = terminal.lock(); + noa_vt::Stream::new().feed(b" foo", &mut *guard); + assert!(worker.queue_navigation( + &terminal, + guard.screen_generation(), + SearchAction::FindNext + )); + assert!(worker.queue_navigation( + &terminal, + guard.screen_generation(), + SearchAction::FindNext + )); + assert!(guard.active().search.query().is_empty()); + } + resume.send(()).unwrap(); + rx.recv_timeout(Duration::from_secs(2)).unwrap(); + let guard = terminal.lock(); + assert_eq!(guard.active().search.matches().len(), 3); + assert_eq!(guard.active().search.active_index(), Some(1)); + assert!(!worker.queue_navigation( + &terminal, + guard.screen_generation(), + SearchAction::FindNext + )); + } + + #[test] + fn latest_query_wins_and_submission_does_not_lock_the_terminal() { + let terminal = Arc::new(Mutex::new(Terminal::new(noa_core::GridSize::new(20, 3)))); + noa_vt::Stream::new().feed(b"first latest latest", &mut *terminal.lock()); + let worker = SearchWorker::new().unwrap(); + let guard = terminal.lock(); + let (old_tx, old_rx) = crossbeam_channel::bounded(1); + worker.submit( + Arc::downgrade(&terminal), + guard.screen_generation(), + "first".into(), + move || { + let _ = old_tx.send(()); + }, + ); + assert!(worker.queue_navigation( + &terminal, + guard.screen_generation(), + SearchAction::FindNext + )); + let (tx, rx) = crossbeam_channel::bounded(1); + worker.submit( + Arc::downgrade(&terminal), + guard.screen_generation(), + "latest".into(), + move || { + let _ = tx.send(()); + }, + ); + drop(guard); + rx.recv_timeout(Duration::from_secs(2)).unwrap(); + assert!(old_rx.try_recv().is_err()); + let terminal = terminal.lock(); + assert_eq!(terminal.active().search.query(), "latest"); + assert_eq!(terminal.active().search.matches().len(), 2); + assert_eq!(terminal.active().search.active_index(), Some(1)); + } + + #[test] + fn cancellation_cannot_restore_a_cleared_query() { + let terminal = Arc::new(Mutex::new(Terminal::new(noa_core::GridSize::new(20, 3)))); + let worker = SearchWorker::new().unwrap(); + let guard = terminal.lock(); + let (tx, rx) = crossbeam_channel::bounded(1); + worker.submit( + Arc::downgrade(&terminal), + guard.screen_generation(), + "old".into(), + move || { + let _ = tx.send(()); + }, + ); + assert!(worker.queue_navigation( + &terminal, + guard.screen_generation(), + SearchAction::FindNext + )); + worker.cancel(); + assert!(!worker.queue_navigation( + &terminal, + guard.screen_generation(), + SearchAction::FindNext + )); + drop(guard); + assert!(rx.recv_timeout(Duration::from_millis(100)).is_err()); + assert!(terminal.lock().active().search.query().is_empty()); + } +} diff --git a/crates/noa-grid/examples/bench_search_images.rs b/crates/noa-grid/examples/bench_search_images.rs new file mode 100644 index 0000000..52c8fb4 --- /dev/null +++ b/crates/noa-grid/examples/bench_search_images.rs @@ -0,0 +1,68 @@ +//! Headless timings for image indexing and search lock work. +//! Run with --sync to measure a synchronous search on the same data. + +use noa_core::GridSize; +use noa_grid::{ImageStore, Terminal}; +use noa_vt::Stream; +use std::hint::black_box; +use std::time::Instant; + +fn summarize(label: &str, mut samples: Vec) { + samples.sort_by(f64::total_cmp); + println!( + "{label}: median_us={:.3} p95_us={:.3} p99_us={:.3}", + samples[samples.len() / 2], + samples[samples.len() * 95 / 100], + samples[samples.len() * 99 / 100], + ); +} + +fn main() { + let synchronous = std::env::args().any(|arg| arg == "--sync"); + for count in [1usize, 1024, 2048, 4096] { + let mut times = Vec::new(); + for _ in 0..31 { + let mut store = ImageStore::new(); + let start = Instant::now(); + for _ in 0..count { + store + .insert_rgba((4096 / count) as u32, 1, vec![1; 16384 / count]) + .unwrap(); + } + for id in (1..=count as u32).rev() { + black_box(store.get(id)); + } + times.push(start.elapsed().as_secs_f64() * 1e6); + black_box(store); + } + summarize(&format!("equal_16KiB_image_pixels count={count}"), times); + } + let mut terminal = Terminal::new(GridSize::new(120, 40)); + Stream::new().feed( + "ordinary search line abcdefghijklmnopqrstuvwxyz\r\n" + .repeat(20000) + .as_bytes(), + &mut terminal, + ); + let mut full = Vec::new(); + let mut held = Vec::new(); + for _ in 0..31 { + let start = Instant::now(); + if synchronous { + terminal.set_search_query("search"); + held.push(start.elapsed().as_secs_f64() * 1e6); + } else { + let snapshot = terminal.active().search_snapshot(); + let lock_us = start.elapsed().as_secs_f64() * 1e6; + let matches = snapshot.find_matches("search", || false).unwrap(); + let matches = std::sync::Arc::from(matches.into_boxed_slice()); + let apply = Instant::now(); + assert!(terminal.apply_search_snapshot(&snapshot, "search".into(), matches)); + held.push(lock_us + apply.elapsed().as_secs_f64() * 1e6); + } + full.push(start.elapsed().as_secs_f64() * 1e6); + } + summarize("search_20k_rows_total", full); + summarize("search_20k_rows_terminal_lock_work", held); + black_box(terminal); +} diff --git a/crates/noa-grid/src/kitty.rs b/crates/noa-grid/src/kitty.rs index 58f31f9..fe507db 100644 --- a/crates/noa-grid/src/kitty.rs +++ b/crates/noa-grid/src/kitty.rs @@ -10,23 +10,35 @@ //! Ghostty analog: `terminal/kitty/graphics_storage.zig` + //! `graphics_image.zig`. -use std::collections::HashSet; +use std::collections::{BTreeSet, HashMap, HashSet}; use std::sync::Arc; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use noa_vt::{KittyAction, KittyCompression, KittyFormat, KittyGraphicsCommand, KittyMedium}; use crate::osc::decode_base64_limited; +#[cfg(test)] +#[path = "kitty/regressions.rs"] +mod regressions; + /// Maximum width or height of a single image, in pixels (Ghostty parity). pub const MAX_IMAGE_DIM: u32 = 10_000; /// Total decoded-RGBA budget across all stored images (Kitty/Ghostty default). /// Configurable per terminal via [`ImageStore::set_byte_limit`]. pub const TOTAL_BYTES_LIMIT: usize = 320_000_000; +/// Bound metadata and allocation overhead independently of pixel bytes. +pub const MAX_STORED_IMAGES: usize = 4096; +const MAX_STORED_FRAMES: usize = 16384; /// Default per-frame gap (ms) applied when a frame declares none (`z=0`), /// matching kitty's animation default. const DEFAULT_FRAME_GAP_MS: i32 = 40; +fn next_image_epoch() -> u64 { + static EPOCH: AtomicU64 = AtomicU64::new(0); + EPOCH.fetch_add(1, Ordering::Relaxed) +} + /// A Kitty graphics error, rendered into a reply as `E:`. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum KittyError { @@ -140,7 +152,7 @@ impl KittyImage { self.anim.current = 1; } self.rgba = Arc::clone(&self.frames[self.anim.current - 1].rgba); - self.epoch = self.epoch.wrapping_add(1); + self.epoch = next_image_epoch(); } } @@ -180,14 +192,16 @@ pub enum TransmitStep { /// Screen-independent image storage with a global byte quota. pub struct ImageStore { - images: Vec, + images: HashMap, + order: BTreeSet<(u64, u32)>, + numbers: HashMap>, + total_frames: usize, total_bytes: usize, /// Configurable total-byte budget (`image-storage-limit`); doubles as the /// per-image / intermediate-decode ceiling so an inflating `o=z` stream or /// oversized frame can't exceed the whole terminal's budget. byte_limit: usize, next_auto_id: u32, - next_epoch: u64, next_seq: u64, transfer: Option, /// Mirrors [`Self::has_running_animation`], refreshed by @@ -200,11 +214,13 @@ pub struct ImageStore { impl Default for ImageStore { fn default() -> Self { ImageStore { - images: Vec::new(), + images: HashMap::new(), + order: BTreeSet::new(), + numbers: HashMap::new(), + total_frames: 0, total_bytes: 0, byte_limit: TOTAL_BYTES_LIMIT, next_auto_id: 1, - next_epoch: 0, next_seq: 0, transfer: None, animation_flag: Arc::new(AtomicBool::new(false)), @@ -226,36 +242,42 @@ impl ImageStore { /// A stored image by id. pub fn get(&self, id: u32) -> Option<&KittyImage> { - self.images.iter().find(|img| img.id == id) + self.images.get(&id) } /// The newest stored image carrying image number `number` (`I=`). pub fn get_by_number(&self, number: u32) -> Option<&KittyImage> { - self.images - .iter() - .filter(|img| img.number == number) - .max_by_key(|img| img.seq) + let (_, id) = self.numbers.get(&number)?.last()?; + self.get(*id) } /// All stored image ids carrying image number `number` (`I=`). pub fn ids_with_number(&self, number: u32) -> Vec { - self.images - .iter() - .filter(|img| img.number == number) - .map(|img| img.id) + self.numbers + .get(&number) + .into_iter() + .flatten() + .map(|(_, id)| *id) .collect() } /// All stored image ids (used by the quota sweep's "referenced" set). pub fn contains(&self, id: u32) -> bool { - self.images.iter().any(|img| img.id == id) + self.images.contains_key(&id) } /// Drop the image with `id` and its bytes. Returns whether anything changed. pub fn remove(&mut self, id: u32) -> bool { - if let Some(pos) = self.images.iter().position(|img| img.id == id) { - self.total_bytes -= self.images[pos].total_frame_bytes(); - self.images.remove(pos); + if let Some(image) = self.images.remove(&id) { + self.total_bytes -= image.total_frame_bytes(); + self.total_frames -= image.frames.len(); + self.order.remove(&(image.seq, id)); + if let Some(entries) = self.numbers.get_mut(&image.number) { + entries.remove(&(image.seq, id)); + if entries.is_empty() { + self.numbers.remove(&image.number); + } + } true } else { false @@ -265,6 +287,9 @@ impl ImageStore { /// Drop everything, including any in-flight chunked transfer. pub fn clear(&mut self) { self.images.clear(); + self.order.clear(); + self.numbers.clear(); + self.total_frames = 0; self.total_bytes = 0; self.transfer = None; } @@ -321,7 +346,7 @@ impl ImageStore { return Err(KittyError::TooBig); } let id = self.assign_auto_id(); - self.insert(id, 0, width, height, rgba); + self.insert(id, 0, width, height, rgba)?; Ok(id) } @@ -411,76 +436,51 @@ impl ImageStore { } /// Read a POSIX shared-memory payload (`t=s`): the base64 payload is the shm - /// object name (kitty convention: a leading-slash name from `shm_open`). The - /// object is `mmap`ped read-only, the requested byte range copied out, then - /// `shm_unlink`ed — the terminal owns unlinking after a successful read, per - /// the kitty spec. Honors `O=`/`S=` offset/size like the file medium. + /// object name (kitty convention: a leading-slash name from `shm_open`). + /// The kernel copies the requested range so a concurrent truncation cannot + /// turn an untrusted transfer into a SIGBUS in the terminal process. #[cfg(unix)] fn read_shared_memory(&self, cmd: &KittyGraphicsCommand) -> Result, KittyError> { use std::ffi::CString; + use std::os::fd::{AsRawFd, FromRawFd, OwnedFd}; let name = self.decode_base64(cmd)?; let cname = CString::new(name).map_err(|_| KittyError::Invalid)?; - // SAFETY: `cname` is a valid NUL-terminated C string for the duration of - // each call; all mapped pointers are checked before use and released on - // every exit path. - unsafe { - let fd = libc::shm_open(cname.as_ptr(), libc::O_RDONLY, 0); - if fd < 0 { - return Err(KittyError::NoEnt); - } - let mut st: libc::stat = std::mem::zeroed(); - if libc::fstat(fd, &mut st) != 0 { - libc::close(fd); + // SAFETY: cname is NUL terminated; successful shm_open transfers fd ownership. + let raw_fd = unsafe { libc::shm_open(cname.as_ptr(), libc::O_RDONLY, 0) }; + if raw_fd < 0 { + return Err(KittyError::NoEnt); + } + let fd = unsafe { OwnedFd::from_raw_fd(raw_fd) }; + let result = (|| { + let mut st: libc::stat = unsafe { std::mem::zeroed() }; + if unsafe { libc::fstat(fd.as_raw_fd(), &mut st) } != 0 { return Err(KittyError::NoEnt); } - // `fstat` on a POSIX shm object reports the size on Linux but returns - // 0 on macOS, so the byte count is taken from `S=` (the declared - // size) or computed from the raw format/dimensions, falling back to - // the stat size only when neither is available. - let stat_size = st.st_size.max(0) as u64; - let offset = cmd.file_offset as u64; - // The declared *data* size drives how much to read (the shm object - // may be page-rounded larger): `S=` wins, then the raw - // format/dimensions, then the stat size as a last resort. + let size = u64::try_from(st.st_size).map_err(|_| KittyError::NoData)?; + let offset = u64::from(cmd.file_offset); let want = if cmd.file_size != 0 { - (cmd.file_size as u64).saturating_sub(offset) - } else if let Some(e) = expected_raw_len(cmd) { - e as u64 + u64::from(cmd.file_size) + } else if let Some(expected) = expected_raw_len(cmd) { + expected as u64 } else { - stat_size.saturating_sub(offset) + size.saturating_sub(offset) }; - if want as usize > self.byte_limit { - libc::close(fd); - let _ = libc::shm_unlink(cname.as_ptr()); - return Err(KittyError::TooBig); - } - if want == 0 { - libc::close(fd); - let _ = libc::shm_unlink(cname.as_ptr()); + let end = offset.checked_add(want).ok_or(KittyError::TooBig)?; + if want == 0 || end > size { return Err(KittyError::NoData); } - let map_len = (offset + want) as usize; - let ptr = libc::mmap( - std::ptr::null_mut(), - map_len, - libc::PROT_READ, - libc::MAP_SHARED, - fd, - 0, - ); - libc::close(fd); - if ptr == libc::MAP_FAILED { - let _ = libc::shm_unlink(cname.as_ptr()); - return Err(KittyError::NoEnt); + let len = usize::try_from(want).map_err(|_| KittyError::TooBig)?; + if len > self.byte_limit { + return Err(KittyError::TooBig); } - let src = - std::slice::from_raw_parts((ptr as *const u8).add(offset as usize), want as usize); - let out = src.to_vec(); - libc::munmap(ptr, map_len); - let _ = libc::shm_unlink(cname.as_ptr()); - Ok(out) + read_shared_range(fd, offset, len) + })(); + // The protocol transfers ownership of the name once it has been opened. + unsafe { + libc::shm_unlink(cname.as_ptr()); } + result } #[cfg(not(unix))] @@ -499,13 +499,23 @@ impl ImageStore { return Err(KittyError::Invalid); } let canonical = std::fs::canonicalize(path).map_err(|_| KittyError::NoEnt)?; - let meta = std::fs::metadata(&canonical).map_err(|_| KittyError::NoEnt)?; - if !meta.is_file() { - return Err(KittyError::NoEnt); - } if cmd.medium == KittyMedium::TempFile && !is_temp_path(&canonical) { return Err(KittyError::Invalid); } + let mut options = std::fs::OpenOptions::new(); + options.read(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + // The path was resolved above. Do not follow a replacement symlink + // or block opening a FIFO before checking the opened object's type. + options.custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK); + } + let file = options.open(&canonical).map_err(|_| KittyError::NoEnt)?; + let meta = file.metadata().map_err(|_| KittyError::NoEnt)?; + if !meta.is_file() { + return Err(KittyError::NoEnt); + } let file_len = meta.len(); let offset = cmd.file_offset as u64; @@ -519,7 +529,7 @@ impl ImageStore { return Err(KittyError::TooBig); } - let bytes = read_file_range(&canonical, offset, want as usize)?; + let bytes = read_file_range(file, offset, want as usize)?; if cmd.medium == KittyMedium::TempFile { let _ = std::fs::remove_file(&canonical); } @@ -553,7 +563,7 @@ impl ImageStore { if cmd.action == KittyAction::TransmitFrame { return self.store_frame(cmd, raw); } - let (width, height, rgba) = decode_to_rgba(cmd, raw)?; + let (width, height, rgba) = decode_to_rgba(cmd, raw, self.byte_limit)?; if width == 0 || height == 0 || width > MAX_IMAGE_DIM || height > MAX_IMAGE_DIM { return Err(KittyError::TooBig); } @@ -567,7 +577,7 @@ impl ImageStore { } let id = self.assign_id(cmd); - self.insert(id, cmd.image_number, width, height, rgba); + self.insert(id, cmd.image_number, width, height, rgba)?; Ok(id) } @@ -590,47 +600,47 @@ impl ImageStore { } } - fn insert(&mut self, id: u32, number: u32, width: u32, height: u32, rgba: Vec) { + fn insert( + &mut self, + id: u32, + number: u32, + width: u32, + height: u32, + rgba: Vec, + ) -> Result<(), KittyError> { + if !self.contains(id) + && (self.images.len() >= MAX_STORED_IMAGES || self.total_frames >= MAX_STORED_FRAMES) + { + return Err(KittyError::TooBig); + } let seq = self.next_seq; self.next_seq += 1; let bytes = rgba.len(); let rgba: Arc<[u8]> = Arc::from(rgba); - if let Some(existing) = self.images.iter_mut().find(|img| img.id == id) { - // A re-transmit replaces the whole image, dropping any prior frames - // and resetting animation state. - self.total_bytes -= existing.total_frame_bytes(); - existing.epoch = existing.epoch.wrapping_add(1); - existing.number = number; - existing.width = width; - existing.height = height; - existing.rgba = Arc::clone(&rgba); - existing.frames = vec![KittyFrame { - rgba, - gap_ms: DEFAULT_FRAME_GAP_MS, - }]; - existing.anim = Anim::default(); - existing.seq = seq; - self.total_bytes += bytes; - } else { - let epoch = self.next_epoch; - self.next_epoch += 1; - self.images.push(KittyImage { + self.remove(id); + self.order.insert((seq, id)); + self.numbers.entry(number).or_default().insert((seq, id)); + self.images.insert( + id, + KittyImage { id, number, width, height, rgba: Arc::clone(&rgba), - epoch, + epoch: next_image_epoch(), seq, frames: vec![KittyFrame { rgba, gap_ms: DEFAULT_FRAME_GAP_MS, }], anim: Anim::default(), - }); - self.total_bytes += bytes; - } + }, + ); + self.total_bytes += bytes; + self.total_frames += 1; + Ok(()) } /// Evict images until the total byte budget is satisfied. Images whose id is @@ -640,17 +650,11 @@ impl ImageStore { let mut evicted_any = false; while self.total_bytes > self.byte_limit { let victim = self - .images + .order .iter() - .filter(|img| !referenced.contains(&img.id)) - .min_by_key(|img| img.seq) - .map(|img| img.id) - .or_else(|| { - self.images - .iter() - .min_by_key(|img| img.seq) - .map(|img| img.id) - }); + .find(|(_, id)| !referenced.contains(id)) + .or_else(|| self.order.first()) + .map(|(_, id)| *id); match victim { Some(id) => { self.remove(id); @@ -688,7 +692,10 @@ impl ImageStore { /// background color) at pixel offset `x=`/`y=` with mode `X=`, then appended /// as a new frame or written into frame `r=`. Returns the target image id. fn store_frame(&mut self, cmd: &KittyGraphicsCommand, raw: Vec) -> Result { - let (data_w, data_h, data) = decode_to_rgba(cmd, raw)?; + if cmd.rows == 0 && self.total_frames >= MAX_STORED_FRAMES { + return Err(KittyError::TooBig); + } + let (data_w, data_h, data) = decode_to_rgba(cmd, raw, self.byte_limit)?; let Some(target_id) = self.resolve_anim_target(cmd) else { return Err(KittyError::NoEnt); }; @@ -750,8 +757,7 @@ impl ImageStore { let img = self .images - .iter_mut() - .find(|i| i.id == target_id) + .get_mut(&target_id) .expect("target resolved above"); if edit_frame != 0 { let idx = edit_frame as usize; @@ -766,6 +772,7 @@ impl ImageStore { return Err(KittyError::TooBig); } img.frames.push(new_frame); + self.total_frames += 1; self.total_bytes += new_bytes; // Adding a second frame auto-starts looping playback, matching // kitty's default of animating as soon as frames exist. @@ -777,8 +784,7 @@ impl ImageStore { // Refresh so a re-uploaded texture reflects any edit to the shown frame. let img = self .images - .iter_mut() - .find(|i| i.id == target_id) + .get_mut(&target_id) .expect("target resolved above"); img.refresh_current(); Ok(target_id) @@ -792,8 +798,7 @@ impl ImageStore { }; let img = self .images - .iter_mut() - .find(|i| i.id == target_id) + .get_mut(&target_id) .expect("target resolved above"); // r= with z= edits that frame's gap without changing playback. @@ -846,8 +851,7 @@ impl ImageStore { } let img = self .images - .iter_mut() - .find(|i| i.id == target_id) + .get_mut(&target_id) .expect("target resolved above"); if dst_idx > img.frames.len() || src_idx > img.frames.len() { return Err(KittyError::Invalid); @@ -881,13 +885,14 @@ impl ImageStore { /// Delete an image's animation frames (`a=d,d=f`), keeping the root frame and /// resetting playback. Returns whether anything changed. pub fn delete_frames(&mut self, id: u32) -> bool { - let Some(img) = self.images.iter_mut().find(|i| i.id == id) else { + let Some(img) = self.images.get_mut(&id) else { return false; }; if img.frames.len() <= 1 { return false; } let dropped: usize = img.frames[1..].iter().map(|f| f.rgba.len()).sum(); + self.total_frames -= img.frames.len() - 1; img.frames.truncate(1); img.anim = Anim::default(); img.refresh_current(); @@ -901,7 +906,7 @@ impl ImageStore { pub fn advance_animations(&mut self, now_ms: u64) -> AnimationTick { let mut changed = false; let mut next_wake: Option = None; - for img in &mut self.images { + for img in self.images.values_mut() { if !img.anim.running || img.frames.len() < 2 { continue; } @@ -945,9 +950,11 @@ impl ImageStore { /// Whether any stored image is currently animating (>= 2 frames, running). pub fn has_running_animation(&self) -> bool { - self.images - .iter() - .any(|img| img.anim.running && img.frames.len() >= 2) + self.total_frames > self.images.len() + && self + .images + .values() + .any(|img| img.anim.running && img.frames.len() >= 2) } /// A cheap clone of the flag mirroring [`Self::has_running_animation`]. @@ -1073,23 +1080,24 @@ fn blend_pixel(canvas: &mut [u8], d: usize, src: &[u8], overwrite: bool) { /// Whether `path` sits in a location we accept for `t=t` (temp-file) media and /// may delete after reading. Mirrors Kitty's requirement. fn is_temp_path(path: &std::path::Path) -> bool { - let temp = std::fs::canonicalize(std::env::temp_dir()); - if let Ok(temp) = &temp - && path.starts_with(temp) - { - return true; - } - for prefix in ["/tmp", "/dev/shm", "/var/tmp"] { - if path.starts_with(prefix) { - return true; - } - } path.to_string_lossy().contains("tty-graphics-protocol") + && [ + std::env::temp_dir(), + "/tmp".into(), + "/dev/shm".into(), + "/var/tmp".into(), + ] + .iter() + .filter_map(|dir| std::fs::canonicalize(dir).ok()) + .any(|dir| path.starts_with(dir)) } -fn read_file_range(path: &std::path::Path, offset: u64, len: usize) -> Result, KittyError> { +fn read_file_range( + mut file: std::fs::File, + offset: u64, + len: usize, +) -> Result, KittyError> { use std::io::{Read, Seek, SeekFrom}; - let mut file = std::fs::File::open(path).map_err(|_| KittyError::NoEnt)?; if offset > 0 { file.seek(SeekFrom::Start(offset)) .map_err(|_| KittyError::Invalid)?; @@ -1099,6 +1107,69 @@ fn read_file_range(path: &std::path::Path, offset: u64, len: usize) -> Result Result, KittyError> { + // Reading through the kernel returns a short read if the object shrinks. + read_file_range(std::fs::File::from(fd), offset, len) +} + +#[cfg(target_os = "macos")] +fn read_shared_range( + fd: std::os::fd::OwnedFd, + offset: u64, + len: usize, +) -> Result, KittyError> { + use std::os::fd::AsRawFd; + unsafe extern "C" { + static mach_task_self_: libc::mach_port_t; + fn mach_vm_read_overwrite( + task: libc::mach_port_t, + address: u64, + size: u64, + data: u64, + out_size: *mut u64, + ) -> libc::kern_return_t; + } + let map_len = usize::try_from(offset) + .ok() + .and_then(|n| n.checked_add(len)) + .ok_or(KittyError::TooBig)?; + // macOS POSIX shm fds do not support read/pread. Copy through the kernel's + // VM API instead of dereferencing the mapping: inaccessible pages then + // produce an error, never a userspace SIGBUS. + unsafe { + let ptr = libc::mmap( + std::ptr::null_mut(), + map_len, + libc::PROT_READ, + libc::MAP_SHARED, + fd.as_raw_fd(), + 0, + ); + if ptr == libc::MAP_FAILED { + return Err(KittyError::NoData); + } + let mut out = vec![0; len]; + let mut copied = 0; + let status = mach_vm_read_overwrite( + mach_task_self_, + ptr as u64 + offset, + len as u64, + out.as_mut_ptr() as u64, + &mut copied, + ); + libc::munmap(ptr, map_len); + if status != libc::KERN_SUCCESS || copied != len as u64 { + return Err(KittyError::NoData); + } + Ok(out) + } +} + fn inflate_bounded(input: &[u8], limit: usize) -> Result, KittyError> { use std::io::Read; let mut decoder = flate2::read::ZlibDecoder::new(input); @@ -1123,7 +1194,12 @@ fn inflate_bounded(input: &[u8], limit: usize) -> Result, KittyError> { fn decode_to_rgba( cmd: &KittyGraphicsCommand, raw: Vec, + byte_limit: usize, ) -> Result<(u32, u32, Vec), KittyError> { + if cmd.format != KittyFormat::Png { + let (w, h) = raw_dimensions(cmd)?; + decoded_image_bytes(w, h, byte_limit)?; + } match cmd.format { KittyFormat::Rgba => { let (w, h) = raw_dimensions(cmd)?; @@ -1144,7 +1220,7 @@ fn decode_to_rgba( } Ok((w, h, rgba)) } - KittyFormat::Png => decode_png(&raw), + KittyFormat::Png => decode_png(&raw, byte_limit), } } @@ -1162,15 +1238,32 @@ fn raw_dimensions(cmd: &KittyGraphicsCommand) -> Result<(u32, u32), KittyError> Ok((cmd.width, cmd.height)) } -fn decode_png(bytes: &[u8]) -> Result<(u32, u32, Vec), KittyError> { - let decoder = png::Decoder::new(std::io::Cursor::new(bytes)); +fn decoded_image_bytes(width: u32, height: u32, limit: usize) -> Result { + if width == 0 || height == 0 || width > MAX_IMAGE_DIM || height > MAX_IMAGE_DIM { + return Err(KittyError::TooBig); + } + (width as usize) + .checked_mul(height as usize) + .and_then(|n| n.checked_mul(4)) + .filter(|&n| n <= limit) + .ok_or(KittyError::TooBig) +} + +fn decode_png(bytes: &[u8], byte_limit: usize) -> Result<(u32, u32, Vec), KittyError> { + let mut decoder = png::Decoder::new(std::io::Cursor::new(bytes)); + decoder.set_transformations(png::Transformations::EXPAND | png::Transformations::STRIP_16); + // Tiny images still need the decoder's fixed zlib workspace. + decoder.set_limits(png::Limits { + bytes: byte_limit.max(64 * 1024), + }); let mut reader = decoder.read_info().map_err(|_| KittyError::BadPng)?; let info = reader.info(); let (width, height) = (info.width, info.height); - if width == 0 || height == 0 || width > MAX_IMAGE_DIM || height > MAX_IMAGE_DIM { + decoded_image_bytes(width, height, byte_limit)?; + let buf_size = reader.output_buffer_size().ok_or(KittyError::TooBig)?; + if buf_size > byte_limit { return Err(KittyError::TooBig); } - let buf_size = reader.output_buffer_size().ok_or(KittyError::TooBig)?; let mut buf = vec![0u8; buf_size]; let frame = reader .next_frame(&mut buf) @@ -1181,9 +1274,7 @@ fn decode_png(bytes: &[u8]) -> Result<(u32, u32, Vec), KittyError> { Ok((width, height, rgba)) } -/// Normalize a decoded PNG frame to straight RGBA8. `png`'s transformations are -/// left off, so we expand grayscale/RGB/palette-expanded 8-bit output here; 16-bit -/// samples are truncated to the high byte. +/// Normalize the expanded PNG frame to straight RGBA8. fn normalize_to_rgba( buf: &[u8], width: u32, @@ -1192,8 +1283,7 @@ fn normalize_to_rgba( depth: png::BitDepth, ) -> Result, KittyError> { let pixels = (width as usize) * (height as usize); - // Only 8- and 16-bit outputs occur here; sub-byte depths are expanded by - // `png` to 8-bit for grayscale/indexed already when using `next_frame`. + // EXPAND resolves packed samples, palette colors and tRNS before this step. let sample_bytes = match depth { png::BitDepth::Sixteen => 2, _ => 1, @@ -1236,8 +1326,7 @@ fn normalize_to_rgba( } } png::ColorType::Indexed => { - // `next_frame` does not expand the palette; reject rather than - // mis-render (Kitty clients emit RGB/RGBA PNGs in practice). + // EXPAND must have resolved the palette before normalization. return Err(KittyError::BadPng); } } @@ -1442,7 +1531,7 @@ mod tests { store.transmit(&cmd); let e0 = store.get(4).unwrap().epoch; store.transmit(&cmd); - assert_eq!(store.get(4).unwrap().epoch, e0 + 1); + assert!(store.get(4).unwrap().epoch > e0); assert_eq!(store.len(), 1); } @@ -1477,7 +1566,7 @@ mod tests { // Manually evict one unreferenced oldest. let victim = store .images - .iter() + .values() .filter(|img| !referenced.contains(&img.id)) .min_by_key(|img| img.seq) .map(|img| img.id) diff --git a/crates/noa-grid/src/kitty/regressions.rs b/crates/noa-grid/src/kitty/regressions.rs new file mode 100644 index 0000000..1492b33 --- /dev/null +++ b/crates/noa-grid/src/kitty/regressions.rs @@ -0,0 +1,342 @@ +use super::*; + +fn command(ctrl: &str, bytes: &[u8]) -> KittyGraphicsCommand { + let mut full = format!("{ctrl};").into_bytes(); + crate::osc::encode_base64(bytes, &mut full); + noa_vt::kitty_graphics::parse(&full, false) +} + +fn transmit(store: &mut ImageStore, ctrl: &str, bytes: &[u8]) -> Result { + match store.transmit(&command(ctrl, bytes)) { + TransmitStep::Done(done) => done.result, + TransmitStep::NeedMore => panic!("expected a complete transfer"), + } +} + +#[test] +fn temporary_deletion_requires_both_directory_and_marker() { + let temp = std::fs::canonicalize(std::env::temp_dir()).unwrap(); + assert!(!is_temp_path(&temp.join("ordinary-file"))); + assert!(!is_temp_path(std::path::Path::new( + "/not-a-temp-dir/tty-graphics-protocol-image" + ))); + assert!(is_temp_path(&temp.join("tty-graphics-protocol-image"))); + if let Ok(tmp) = std::fs::canonicalize("/tmp") { + assert!(is_temp_path(&tmp.join("tty-graphics-protocol-image"))); + } +} + +#[test] +fn temporary_medium_preserves_an_ordinary_file_in_the_temp_directory() { + use std::io::Write; + let path = std::env::temp_dir().join(format!("noa-ordinary-image-{}", std::process::id())); + let mut file = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&path) + .unwrap(); + file.write_all(&[1, 2, 3, 4]).unwrap(); + let result = transmit( + &mut ImageStore::new(), + "a=t,t=t,f=32,s=1,v=1,i=1", + path.to_str().unwrap().as_bytes(), + ); + let remaining = std::fs::read(&path); + std::fs::remove_file(&path).unwrap(); + assert_eq!(result, Err(KittyError::Invalid)); + assert_eq!(remaining.unwrap(), [1, 2, 3, 4]); +} + +#[test] +fn failed_placements_still_obey_image_quota() { + let mut terminal = crate::Terminal::new(noa_core::GridSize::new(20, 4)); + terminal.set_pixel_metrics(10, 20, 200, 80); + terminal.set_kitty_image_limit(8); + for id in 1..=4 { + noa_vt::Handler::kitty_graphics( + &mut terminal, + command(&format!("a=T,f=32,s=1,v=1,i={id},x=1,w=0"), &[1, 2, 3, 4]), + ); + assert!(terminal.kitty_images.total_bytes() <= 8); + assert!(terminal.kitty_visible_placements().is_empty()); + assert!( + terminal + .pending_writes + .ends_with(b"EINVAL:invalid request\x1b\\") + ); + } +} + +fn png( + width: u32, + height: u32, + color: png::ColorType, + depth: png::BitDepth, + transparency: Option<&[u8]>, + data: &[u8], +) -> Vec { + let mut bytes = Vec::new(); + { + let mut encoder = png::Encoder::new(&mut bytes, width, height); + encoder.set_color(color); + encoder.set_depth(depth); + if let Some(trns) = transparency { + encoder.set_trns(trns); + } + encoder + .write_header() + .unwrap() + .write_image_data(data) + .unwrap(); + } + bytes +} + +#[test] +fn png_expands_packed_grayscale_and_transparency() { + let mut store = ImageStore::new(); + for (depth, packed, expected) in [ + ( + png::BitDepth::One, + vec![0xaa], + vec![255, 0, 255, 0, 255, 0, 255, 0], + ), + ( + png::BitDepth::Two, + vec![0x1b, 0x1b], + vec![0, 85, 170, 255, 0, 85, 170, 255], + ), + ( + png::BitDepth::Four, + vec![0x0f; 4], + vec![0, 255, 0, 255, 0, 255, 0, 255], + ), + ] { + let bytes = png(8, 1, png::ColorType::Grayscale, depth, None, &packed); + transmit(&mut store, "a=t,f=100,i=1", &bytes).unwrap(); + let actual: Vec<_> = store + .get(1) + .unwrap() + .rgba + .chunks_exact(4) + .map(|p| p[0]) + .collect(); + assert_eq!(actual, expected); + } + let bytes = png( + 1, + 1, + png::ColorType::Rgb, + png::BitDepth::Eight, + Some(&[0, 1, 0, 2, 0, 3]), + &[1, 2, 3], + ); + transmit(&mut store, "a=t,f=100,i=1", &bytes).unwrap(); + assert_eq!(&*store.get(1).unwrap().rgba, &[1, 2, 3, 0]); + let bytes = png( + 1, + 1, + png::ColorType::Grayscale, + png::BitDepth::Eight, + Some(&[0, 7]), + &[7], + ); + transmit(&mut store, "a=t,f=100,i=1", &bytes).unwrap(); + assert_eq!(&*store.get(1).unwrap().rgba, &[7, 7, 7, 0]); +} + +#[test] +fn png_budget_is_checked_before_pixel_decoding() { + let mut bytes = png( + 128, + 128, + png::ColorType::Rgba, + png::BitDepth::Eight, + None, + &vec![0; 128 * 128 * 4], + ); + // Leave a valid IHDR and IDAT header, but corrupt the compressed pixels. + // The dimensions alone must reject this image, before an IDAT decode error. + bytes.truncate(41); + let mut store = ImageStore::new(); + store.set_byte_limit(1024); + assert_eq!( + transmit(&mut store, "a=t,f=100,i=1", &bytes), + Err(KittyError::TooBig) + ); +} + +#[test] +fn png_expands_palette_colors_and_alpha() { + let mut bytes = Vec::new(); + { + let mut encoder = png::Encoder::new(&mut bytes, 2, 1); + encoder.set_color(png::ColorType::Indexed); + encoder.set_depth(png::BitDepth::One); + encoder.set_palette(&[10, 20, 30, 40, 50, 60][..]); + encoder.set_trns(&[0, 128][..]); + encoder + .write_header() + .unwrap() + .write_image_data(&[0x40]) + .unwrap(); + } + let mut store = ImageStore::new(); + transmit(&mut store, "a=t,f=100,i=1", &bytes).unwrap(); + assert_eq!( + &*store.get(1).unwrap().rgba, + &[10, 20, 30, 0, 40, 50, 60, 128] + ); +} + +#[test] +fn recreated_id_never_reuses_a_displayed_epoch() { + let mut store = ImageStore::new(); + transmit(&mut store, "a=t,f=32,s=1,v=1,i=1", &[1; 4]).unwrap(); + transmit(&mut store, "a=t,f=32,s=1,v=1,i=1", &[2; 4]).unwrap(); + let previous = store.get(1).unwrap().epoch; + store.remove(1); + transmit(&mut store, "a=t,f=32,s=1,v=1,i=1", &[3; 4]).unwrap(); + assert_ne!(store.get(1).unwrap().epoch, previous); + store.clear(); + transmit(&mut store, "a=t,f=32,s=1,v=1,i=1", &[4; 4]).unwrap(); + assert_ne!(store.get(1).unwrap().epoch, previous); +} + +#[test] +fn image_indices_track_retransmission_numbers_and_removal() { + let mut store = ImageStore::new(); + for id in 1..=3 { + transmit(&mut store, &format!("a=t,f=32,s=1,v=1,i={id},I=9"), &[0; 4]).unwrap(); + } + assert_eq!(store.get_by_number(9).unwrap().id, 3); + transmit(&mut store, "a=t,f=32,s=1,v=1,i=1,I=8", &[1; 4]).unwrap(); + assert_eq!(store.ids_with_number(9), vec![2, 3]); + store.remove(3); + assert_eq!(store.get_by_number(9).unwrap().id, 2); + store.set_byte_limit(4); + assert!(store.get_by_number(9).is_none()); + assert_eq!(store.get_by_number(8).unwrap().id, 1); + store.clear(); + assert!(store.get_by_number(8).is_none()); +} + +#[test] +fn tiny_images_and_frames_have_independent_metadata_caps() { + let mut store = ImageStore::new(); + for id in 1..=MAX_STORED_IMAGES { + transmit(&mut store, &format!("a=t,f=32,s=1,v=1,i={id}"), &[0; 4]).unwrap(); + } + assert_eq!( + transmit(&mut store, "a=t,f=32,s=1,v=1,i=9000", &[0; 4]), + Err(KittyError::TooBig) + ); + store.remove(1); + transmit(&mut store, "a=t,f=32,s=1,v=1,i=9000", &[0; 4]).unwrap(); + assert_eq!(store.len(), MAX_STORED_IMAGES); + assert!(!store.has_running_animation()); + store.clear(); + transmit(&mut store, "a=t,f=32,s=1,v=1,i=1", &[0; 4]).unwrap(); + for _ in 1..MAX_STORED_FRAMES { + transmit(&mut store, "a=f,f=32,s=1,v=1,i=1", &[0; 4]).unwrap(); + } + assert_eq!( + transmit(&mut store, "a=f,f=32,s=1,v=1,i=1", &[0; 4]), + Err(KittyError::TooBig) + ); + assert!(store.delete_frames(1)); + transmit(&mut store, "a=f,f=32,s=1,v=1,i=1", &[1; 4]).unwrap(); +} + +#[cfg(unix)] +struct SharedMemory { + name: std::ffi::CString, + fd: std::os::fd::OwnedFd, +} + +#[cfg(unix)] +impl SharedMemory { + fn new(len: usize) -> Self { + use std::os::fd::{AsRawFd, FromRawFd}; + static ID: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + let id = ID.fetch_add(1, Ordering::Relaxed); + let name = std::ffi::CString::new(format!("/noarg{}-{id}", std::process::id())).unwrap(); + // SAFETY: this test creates an exclusive object and only maps its own allocation. + unsafe { + let fd = libc::shm_open( + name.as_ptr(), + libc::O_CREAT | libc::O_EXCL | libc::O_RDWR, + 0o600, + ); + assert!(fd >= 0, "{}", std::io::Error::last_os_error()); + let shm = Self { + name, + fd: std::os::fd::OwnedFd::from_raw_fd(fd), + }; + assert_eq!(libc::ftruncate(shm.fd.as_raw_fd(), len as libc::off_t), 0); + let p = libc::mmap( + std::ptr::null_mut(), + len, + libc::PROT_READ | libc::PROT_WRITE, + libc::MAP_SHARED, + fd, + 0, + ); + assert_ne!(p, libc::MAP_FAILED); + std::ptr::write_bytes(p, 7, len); + assert_eq!(libc::munmap(p, len), 0); + shm + } + } +} + +#[cfg(unix)] +impl Drop for SharedMemory { + fn drop(&mut self) { + unsafe { + libc::shm_unlink(self.name.as_ptr()); + } + } +} + +#[test] +#[cfg(unix)] +fn shared_memory_size_is_a_length_not_an_end_offset() { + let shm = SharedMemory::new(16384); + let mut store = ImageStore::new(); + assert_eq!( + transmit( + &mut store, + "a=t,t=s,f=32,s=2,v=1,i=1,S=8,O=4", + shm.name.as_bytes() + ), + Ok(1) + ); + assert_eq!(&*store.get(1).unwrap().rgba, &[7; 8]); +} + +#[test] +#[cfg(unix)] +fn shared_memory_rejects_ranges_past_the_object() { + let shm = SharedMemory::new(16384); + assert_eq!( + transmit( + &mut ImageStore::new(), + "a=t,t=s,f=32,s=2,v=1,i=1,S=8,O=16380", + shm.name.as_bytes() + ), + Err(KittyError::NoData) + ); +} + +#[test] +#[cfg(all(unix, not(target_os = "macos")))] +fn shrinking_shared_memory_after_stat_returns_a_read_error() { + use std::os::fd::AsRawFd; + let shm = SharedMemory::new(4096); + assert_eq!(unsafe { libc::ftruncate(shm.fd.as_raw_fd(), 0) }, 0); + assert_eq!( + read_shared_range(shm.fd.try_clone().unwrap(), 0, 4096), + Err(KittyError::NoData) + ); +} diff --git a/crates/noa-grid/src/screen/text.rs b/crates/noa-grid/src/screen/text.rs index 3e2ff55..1bb8dc4 100644 --- a/crates/noa-grid/src/screen/text.rs +++ b/crates/noa-grid/src/screen/text.rs @@ -369,23 +369,76 @@ impl Screen { pub fn set_search_query(&mut self, query: impl Into) { let query = query.into(); let matches = self.compute_search_matches(&query); + let anchor = self.search_anchor(); + self.install_search_matches(query, matches, anchor); + } + + fn search_anchor(&self) -> SearchAnchor { // A fresh query anchors backward at the viewport bottom (activating // the bottom-most visible match rather than the oldest scrollback // row); an incremental edit anchors forward at the previous active // match so extending the query doesn't yank the viewport away. - let anchor = match self.search.active_match() { + match self.search.active_match() { Some(active) => SearchAnchor::Forward(active.start), None => SearchAnchor::Backward(SelectionPoint::new( self.cols.saturating_sub(1), self.visible_row_base() + (self.rows as usize).saturating_sub(1), )), - }; + } + } + + fn install_search_matches( + &mut self, + query: String, + matches: Vec, + anchor: SearchAnchor, + ) { self.search.set_query(query, matches, anchor); if let Some(active) = self.search.active_match() { self.reveal_search_match(active); } } + pub fn search_snapshot(&self) -> crate::search::SearchSnapshot { + crate::search::SearchSnapshot { + history: self.scrollback.search_snapshot(), + live: self.grid.iter().cloned().collect(), + history_len: self.scrollback_len(), + rows_evicted: self.rows_evicted, + coordinate_generation: self.coordinate_generation, + cols: self.cols, + anchor: self.search_anchor(), + } + } + + /// Do not attach stale coordinates to rows that changed during the scan. + pub fn apply_search_snapshot( + &mut self, + snapshot: &crate::search::SearchSnapshot, + query: String, + matches: std::sync::Arc<[SearchMatch]>, + ) -> bool { + if self.cols != snapshot.cols + || self.coordinate_generation != snapshot.coordinate_generation + || self.rows_evicted != snapshot.rows_evicted + || self.scrollback_len() != snapshot.history_len + || self.grid.len() != snapshot.live.len() + || self + .grid + .iter() + .zip(&snapshot.live) + .any(|(a, b)| a.wrapped != b.wrapped || a.cells != b.cells) + { + return false; + } + self.search + .set_shared_query(query, matches, snapshot.anchor); + if let Some(active) = self.search.active_match() { + self.reveal_search_match(active); + } + true + } + pub fn clear_search(&mut self) { self.search.clear(); } diff --git a/crates/noa-grid/src/scrollback.rs b/crates/noa-grid/src/scrollback.rs index 3681042..00be863 100644 --- a/crates/noa-grid/src/scrollback.rs +++ b/crates/noa-grid/src/scrollback.rs @@ -274,6 +274,7 @@ unsafe fn write_packed_word(dst: *mut PackedCell, word: u64) { } /// Append-only style pool for one page, with an interning lookup. +#[derive(Clone)] struct StyleTable { styles: Vec