From fae63aa2bc97b895a90dc37cab77f844e16b1705 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 9 Aug 2026 14:25:29 +0800 Subject: [PATCH] fix(terminal): clear the other anchored state when the scrollback goes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removing the mark store took `marks().clear()` off `clear_scrollback`, which left that path with no anchored-state invalidation at all — and it never had any for the second store that needs it. Kitty image placements are anchored to an absolute scrollback row, so purging the history moves every anchor: the frame either paints over unrelated text or resolves past the viewport and disappears, with no redraw coming since the daemon does not replay out-of-band image frames. Clear the store, exactly as the reattach path already does. Route the purge through `Term::clear_screen(ClearMode::Saved)` while here. Reaching for `grid_mut().clear_history()` skipped the selection invalidation alacritty does there, so a selection anchored in the discarded rows survived, clamped onto the viewport, and copied whatever text had moved into it. Also collapse `Cut`, now a single-variant enum around `CursorCut`, and repoint the `memchr` comment at the scanner that still uses it. The image module's anchor note now records the alt-screen caveat: an anchor read off the alt grid resolves against the primary one, which wants a per-screen store to fix and is left alone here. --- Cargo.toml | 5 ++- src/terminal/images.rs | 8 ++++ src/terminal/remote.rs | 17 ++------ src/terminal/view.rs | 99 +++++++++++++++++++++++++++++++++++++++++- 4 files changed, 112 insertions(+), 17 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f58808f2..52d10607 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,8 +43,9 @@ log.workspace = true # Smart double-click selection patterns (URL/email/path). Already in the tree # transitively, so pinning it here adds no new native code. regex = "1" -# SIMD byte search for `MarkScanner`'s Text-state fast path (`terminal::marks`), -# which scans every output batch the client receives — the same skip-ahead the +# SIMD byte search for `ParkedCursorScanner`'s fast path +# (`terminal::parked_cursor`), which scans every output batch the client +# receives — the same skip-ahead the # tokenizers in `tty7-core` already use. Already in the tree via `tty7-core`, # so this pins no new code. memchr = "2" diff --git a/src/terminal/images.rs b/src/terminal/images.rs index ba9e6780..0cf21573 100644 --- a/src/terminal/images.rs +++ b/src/terminal/images.rs @@ -19,6 +19,14 @@ //! (unobservable) discard count, and a browser that redraws every frame corrects //! it on the next transmit anyway. //! +//! The anchor is read off whichever grid is active, and the alt screen has no +//! history of its own — so an image placed there records a small absolute row +//! that resolves against the primary grid once the app exits. A sender that +//! deletes its own images on the way out (the normal case) is unaffected; one +//! that dies without an `a=d` can leave a frame anchored over the primary +//! screen. Modelling that properly wants a per-screen store rather than one +//! keyed on the displayed grid. +//! //! GPUI's sprite atlas expects **BGRA** pixels (it swaps R↔B when caching an //! `image` crate `RgbaImage` — see `gpui::img`), so [`decode`] does the swap once //! at ingest; the placed [`RenderImage`] is uploaded verbatim thereafter. diff --git a/src/terminal/remote.rs b/src/terminal/remote.rs index bc4fc59f..93e12652 100644 --- a/src/terminal/remote.rs +++ b/src/terminal/remote.rs @@ -623,9 +623,8 @@ impl RemoteTerminal { // batch splits at each of them: advance the // emulator to the cut, act on the state that // sequence left behind, carry on. - let mut cuts: Vec<(usize, Cut)> = Vec::new(); - cursor_scan - .feed(&out_batch, |off, c| cuts.push((off, Cut::Cursor(c)))); + let mut cuts: Vec<(usize, CursorCut)> = Vec::new(); + cursor_scan.feed(&out_batch, |off, c| cuts.push((off, c))); { let t0 = trace.then(std::time::Instant::now); let mut term = term.lock(); @@ -640,11 +639,7 @@ impl RemoteTerminal { for (off, cut) in cuts { processor.advance(&mut *term, &out_batch[at..off]); at = off; - match cut { - Cut::Cursor(vis) => { - parked_cursor.apply(&mut term, vis); - } - } + parked_cursor.apply(&mut term, cut); } processor.advance(&mut *term, &out_batch[at..]); } @@ -1495,12 +1490,6 @@ impl RemoteTerminal { } } -/// Something in the pty stream the reader has to act on at the byte where it -/// appeared, rather than after the whole batch has been parsed. -enum Cut { - Cursor(CursorCut), -} - fn daemon_not_listening(err: &anyhow::Error) -> bool { err.chain().any(|cause| { cause.downcast_ref::().is_some_and(|io| { diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 34dc6542..a181bd48 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -2546,8 +2546,20 @@ impl TerminalView { } pub fn clear_scrollback(&mut self, cx: &mut Context) { + use alacritty_terminal::vte::ansi::{ClearMode, Handler as _}; + self.cancel_scroll_anim(); - self.terminal.term.lock().grid_mut().clear_history(); + // Go through `clear_screen` rather than `grid_mut().clear_history()`: + // it drops a selection anchored in the rows we are about to discard and + // clamps the vi cursor back into the grid, which purging the history + // behind the term's back would leave pointing at rows that no longer + // exist. + self.terminal.term.lock().clear_screen(ClearMode::Saved); + // Image placements are anchored in absolute scrollback rows, so the + // rows we just discarded moved every anchor. Drop them; the daemon does + // not replay out-of-band image frames, so a browser redraws on its next + // transmit (same reasoning as the reattach path in `adopt_relink`). + self.terminal.images().clear(); self.scroll_frac = 0.; self.terminal.write(vec![0x0c_u8]); cx.notify(); @@ -7117,6 +7129,91 @@ mod gpui_tests { ); } + /// A 1x1 red placement anchored at an absolute scrollback row, built the way + /// the decode worker hands one to the store. + fn placed_at(anchor_row: i64) -> crate::terminal::images::PlacedImage { + use tty7_core::core::kitty_graphics::{Image, WireFormat}; + + let mut img = Image { + id: 1, + number: 0, + placement: 0, + width: 1, + height: 1, + cols: 0, + rows: 0, + data: vec![0xff, 0x00, 0x00, 0xff], + format: WireFormat::Rgba, + compressed: false, + }; + let (data, width_px, height_px) = crate::terminal::images::decode(&mut img).unwrap(); + crate::terminal::images::PlacedImage { + data, + anchor_row, + anchor_col: 0, + width_px, + height_px, + cols: 0, + rows: 0, + id: 1, + placement: 0, + painted: std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)), + } + } + + #[gpui::test] + fn clearing_the_scrollback_drops_what_was_anchored_in_it(cx: &mut TestAppContext) { + let (window, mut daemon) = harness(cx); + + // Overflow the 24-row viewport so there is a scrollback to purge. + let mut out = Vec::new(); + for i in 0..60 { + out.extend_from_slice(format!("line {i}\r\n").as_bytes()); + } + DaemonMsg::Output(out).encode(&mut daemon).unwrap(); + for _ in 0..200 { + let filled = window + .update(cx, |view, _, _| { + view.terminal.term.lock().grid().history_size() > 0 + }) + .unwrap(); + if filled { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + + window + .update(cx, |view, _, cx| { + let history = view.terminal.term.lock().grid().history_size(); + assert!(history > 0, "the test needs a scrollback to clear"); + + // Both of these address rows `clear_history` is about to drop: + // an image anchored by absolute row, and a selection reaching up + // into the history. + view.terminal.images().place(placed_at(history as i64)); + view.terminal.term.lock().selection = Some(Selection::new( + SelectionType::Simple, + Point::new(Line(-1), Column(0)), + Side::Left, + )); + + view.clear_scrollback(cx); + + assert!( + view.terminal.images().snapshot().is_empty(), + "a stale anchor blits the frame over live output, or off-screen \ + entirely — the daemon never replays the frame to correct it" + ); + assert!( + view.terminal.term.lock().selection.is_none(), + "a selection left pointing at purged rows clamps onto the \ + viewport and copies whatever text moved into them" + ); + }) + .unwrap(); + } + #[gpui::test] fn a_stale_hover_row_does_not_index_the_shrunken_grid(cx: &mut TestAppContext) { let (window, _daemon) = harness(cx);