From 647fa1a2e59d5312383f348cd863e2ec0616b393 Mon Sep 17 00:00:00 2001 From: simota Date: Sat, 5 Sep 2026 14:33:11 +0900 Subject: [PATCH 1/2] fix(terminal): resolve image, PTY, and search defects Prevent unintended temporary-file deletion, unsafe shared-memory access, and image quota bypasses. Preserve PTY tail output and bound all pending writes so a child that stops reading cannot grow the reply queue forever. Correct PNG expansion, image epochs and dimensions, wrapped-line search, and CSI overflow handling. Move history searches outside the UI thread and terminal lock, and reduce redraw, image lookup and GPU resource work. Add regression coverage, repeatable performance probes, and operating limits for drain timeouts, write reservations and image metadata. Validation: cargo build --workspace --locked --offline; cargo test --workspace --locked --offline (2540 passed, 15 ignored); cargo fmt --all -- --check; git diff --check. --- CHANGELOG.md | 27 ++ crates/noa-app/src/app.rs | 19 +- crates/noa-app/src/app/event_loop.rs | 7 + crates/noa-app/src/app/input_ops/search.rs | 40 +- crates/noa-app/src/app/input_ops/terminal.rs | 2 +- crates/noa-app/src/app/lifecycle.rs | 3 +- crates/noa-app/src/events.rs | 13 +- crates/noa-app/src/io_thread.rs | 6 +- crates/noa-app/src/io_thread/input_queue.rs | 73 +--- crates/noa-app/src/io_thread/redraw.rs | 76 ++-- crates/noa-app/src/io_thread/spawn.rs | 13 +- crates/noa-app/src/io_thread/tests.rs | 43 +- crates/noa-app/src/lib.rs | 1 + crates/noa-app/src/search_worker.rs | 189 ++++++++ .../noa-grid/examples/bench_search_images.rs | 68 +++ crates/noa-grid/src/kitty.rs | 407 +++++++++++------- crates/noa-grid/src/kitty/regressions.rs | 342 +++++++++++++++ crates/noa-grid/src/screen/text.rs | 57 ++- crates/noa-grid/src/scrollback.rs | 80 +++- crates/noa-grid/src/search.rs | 200 ++++++++- crates/noa-grid/src/terminal.rs | 10 + .../noa-grid/src/terminal/kitty_graphics.rs | 6 +- crates/noa-pty/src/lib.rs | 2 +- crates/noa-pty/src/pty.rs | 5 +- crates/noa-pty/src/reader.rs | 213 ++++++++- crates/noa-pty/src/writer.rs | 310 +++++++++++-- crates/noa-render/src/image_layer.rs | 134 +++++- crates/noa-vt/src/parser.rs | 5 + crates/noa-vt/src/tests.rs | 24 ++ docs/performance-measurements.md | 46 ++ docs/review-fixes-2026-09-05.md | 64 +++ docs/specs/kitty-graphics.md | 24 +- 32 files changed, 2150 insertions(+), 359 deletions(-) create mode 100644 crates/noa-app/src/search_worker.rs create mode 100644 crates/noa-grid/examples/bench_search_images.rs create mode 100644 crates/noa-grid/src/kitty/regressions.rs create mode 100644 docs/review-fixes-2026-09-05.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b1eed5d..b03a8849 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 f7d3fb15..92f71b42 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 a0f9e3e0..030d6625 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 0a62c6d4..1af04066 100644 --- a/crates/noa-app/src/app/input_ops/search.rs +++ b/crates/noa-app/src/app/input_ops/search.rs @@ -51,7 +51,12 @@ impl App { SearchAction::FindPrevious => { terminal.search_previous(); } - SearchAction::Clear => terminal.clear_search(), + SearchAction::Clear => { + if let Some(worker) = &self.search_worker { + worker.cancel(); + } + terminal.clear_search(); + } } drop(terminal); @@ -167,11 +172,31 @@ 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(); + self.search_worker.as_ref().unwrap().submit( + Arc::downgrade(&terminal), + 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 +212,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 diff --git a/crates/noa-app/src/app/input_ops/terminal.rs b/crates/noa-app/src/app/input_ops/terminal.rs index 7a4ee4d7..f067b75c 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 2fa2a472..18382d63 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 b59da588..67d61efe 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 4db0ad2e..01171ee7 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 3c771693..546e6172 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 6afcd1f3..37f386d8 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 6c476828..05567a8d 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 01413f99..5c2da408 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 126ec3ae..7edcea31 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 00000000..51bf3b32 --- /dev/null +++ b/crates/noa-app/src/search_worker.rs @@ -0,0 +1,189 @@ +//! 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; + +const DEBOUNCE: Duration = Duration::from_millis(35); + +struct Job { + terminal: Weak>, + query: String, + generation: u64, + notify: Box, +} + +#[derive(Default)] +struct Pending { + job: Option, + shutdown: bool, +} + +#[derive(Default)] +struct Shared { + pending: Mutex, + ready: Condvar, + generation: AtomicU64, +} + +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>, + 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.job = Some(Job { + terminal, + query, + generation, + notify: Box::new(notify), + }); + self.shared.ready.notify_one(); + } + + pub(crate) fn cancel(&self) { + let mut pending = self.shared.pending.lock(); + self.shared.generation.fetch_add(1, Ordering::AcqRel); + pending.job = 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; + 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() { + break; + } + ( + terminal.active().search_snapshot(), + terminal.grid_coordinate_generation(), + ) + }; + 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 pending = shared.pending.lock(); + if pending.shutdown || cancelled() { + break; + } + terminal.grid_coordinate_generation() == space + && terminal.apply_search_snapshot(&snapshot, job.query.clone(), matches) + }; + 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); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[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", &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), "first".into(), move || { + let _ = old_tx.send(()); + }); + let (tx, rx) = crossbeam_channel::bounded(1); + worker.submit(Arc::downgrade(&terminal), "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(), 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), "old".into(), move || { + let _ = tx.send(()); + }); + worker.cancel(); + 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 00000000..52c8fb42 --- /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 58f31f95..fe507db7 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 00000000..1492b331 --- /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 3e2ff559..1bb8dc40 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 3681042a..00be863f 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