diff --git a/desktop/src-tauri/crates/buzz-terminal/src/damage.rs b/desktop/src-tauri/crates/buzz-terminal/src/damage.rs index 15097fbd4..86b97814c 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/damage.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/damage.rs @@ -93,16 +93,12 @@ pub struct Frame { pub cursor: CursorFrame, /// Whether the renderer should discard what it has and repaint. pub full: bool, - /// The viewport this frame describes. A change means the grid was resized + /// The grid this frame describes. A change means the terminal was resized /// and row indices refer to a different geometry than the previous frame's. /// Carried so the consumer can detect that from the frame itself instead of - /// trusting that no resize overtook it in flight. - pub generation: u64, - /// Grid dimensions this frame was captured at, so a full frame is - /// self-describing rather than only meaningful against a size the consumer - /// happens to remember. - pub columns: usize, - pub screen_lines: usize, + /// trusting that no resize overtook it in flight -- across a transport, a + /// frame captured before a resize can arrive after it. + pub viewport: crate::Viewport, } impl Frame { @@ -117,15 +113,13 @@ pub struct RawFrame { rows: Vec<(usize, Vec)>, cursor: CursorFrame, full: bool, - generation: u64, - columns: usize, - screen_lines: usize, + viewport: crate::Viewport, } /// Copy the damaged rows out of the terminal. **Runs under the lock; does no /// encoding.** Keep this function boring — everything added here is lock hold. pub fn capture(terminal: &mut crate::Terminal) -> RawFrame { - let generation = terminal.generation(); + let viewport = terminal.viewport(); let term = terminal.term_mut(); let columns = term.columns(); let screen_lines = term.screen_lines(); @@ -161,9 +155,7 @@ pub fn capture(terminal: &mut crate::Terminal) -> RawFrame { rows, cursor, full, - generation, - columns, - screen_lines, + viewport, } } @@ -205,9 +197,7 @@ impl Encoder { rows, cursor: raw.cursor, full: raw.full, - generation: raw.generation, - columns: raw.columns, - screen_lines: raw.screen_lines, + viewport: raw.viewport, } } } diff --git a/desktop/src-tauri/crates/buzz-terminal/src/lib.rs b/desktop/src-tauri/crates/buzz-terminal/src/lib.rs index 4dc529b6c..656d23d3f 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/lib.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/lib.rs @@ -19,6 +19,22 @@ pub use fences::{FenceStats, Fences}; pub use listener::{Action, Listener}; pub use shared::{AcquireMeter, AcquireStats, SharedTerminal}; +/// Which grid a frame or a resize refers to. +/// +/// Generation and dimensions travel together as one value because they answer +/// one question -- "is this the grid I am currently showing?" -- and a consumer +/// that compares them field by field can compare two of the three and be wrong +/// on a resize that changes only the one it skipped. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Viewport { + /// Advances on every *applied* resize. A same-size resize is inert and + /// does not advance it, so an unchanged `ResizeObserver` tick cannot look + /// like a discontinuity. + pub generation: u64, + pub columns: usize, + pub screen_lines: usize, +} + /// Terminal dimensions in cells, plus how much scrollback to retain. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct Size { @@ -109,12 +125,15 @@ impl Terminal { self.size } - /// Which viewport the grid currently has, incremented on every applied - /// resize. Stamped onto each [`damage::Frame`] so a consumer can tell that - /// a frame describes a *different* grid than the one it last drew, without - /// having to infer it from message ordering. - pub fn generation(&self) -> u64 { - self.generation + /// The grid as it stands now. Stamped onto each [`damage::Frame`] so a + /// consumer can tell that a frame describes a *different* grid than the one + /// it last drew, without having to infer it from message ordering. + pub fn viewport(&self) -> Viewport { + Viewport { + generation: self.generation, + columns: self.size.columns, + screen_lines: self.size.screen_lines, + } } /// Apply a new viewport. @@ -130,13 +149,21 @@ impl Terminal { /// keeps the encoder's per-line hashes from suppressing reflowed content. /// The generation bump is belt-and-braces on top of that: it lets the /// consumer *verify* it received the discontinuity rather than assume it. - pub fn resize(&mut self, size: Size) { + /// + /// Returns the viewport that is now in effect, which is not necessarily the + /// one requested: a same-size call is inert and returns the current + /// generation unchanged. Returning it here rather than making the caller + /// ask afterwards matters across a transport -- a follow-up query races the + /// next resize, so the answer could describe a grid that had already been + /// replaced by the time it was read. + pub fn resize(&mut self, size: Size) -> Viewport { if size == self.size { - return; + return self.viewport(); } self.term.resize(size); self.size = size; self.generation += 1; + self.viewport() } pub fn term(&self) -> &Term { diff --git a/desktop/src-tauri/crates/buzz-terminal/src/shared.rs b/desktop/src-tauri/crates/buzz-terminal/src/shared.rs index 918335eef..577908ee0 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/shared.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/shared.rs @@ -170,8 +170,8 @@ impl SharedTerminal { /// Apply a coalesced resize. Renderer plane: this competes with the /// renderer for the same lock and can hold it for milliseconds. - pub fn resize(&self, size: crate::Size) { - self.acquire(&self.renderer).resize(size); + pub fn resize(&self, size: crate::Size) -> crate::Viewport { + self.acquire(&self.renderer).resize(size) } /// Take the lock for something the methods above don't cover (input, diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/resize.rs b/desktop/src-tauri/crates/buzz-terminal/tests/resize.rs index f269e3d91..5b1dfbe8f 100644 --- a/desktop/src-tauri/crates/buzz-terminal/tests/resize.rs +++ b/desktop/src-tauri/crates/buzz-terminal/tests/resize.rs @@ -49,8 +49,8 @@ fn resize_forces_a_full_frame_at_the_new_width() { let first = shared.render(&mut encoder); assert!(first.full, "first frame after a fresh Term must be full"); - assert_eq!(first.columns, 40); - assert_eq!(first.generation, 0); + assert_eq!(first.viewport.columns, 40); + assert_eq!(first.viewport.generation, 0); // Nothing changed: dedup suppresses everything. Without this the next // assertion could pass simply because every frame is full. @@ -62,19 +62,24 @@ fn resize_forces_a_full_frame_at_the_new_width() { idle.rows.len() ); - shared.resize(size(20)); + let applied = shared.resize(size(20)); + assert_eq!( + applied.columns, 20, + "resize did not report the grid it applied" + ); + assert_eq!( + applied.generation, 1, + "generation must advance across a resize" + ); + let after = shared.render(&mut encoder); assert!( after.full, "a resize must invalidate the renderer's cached rows" ); assert_eq!( - after.columns, 20, - "frame does not describe the new viewport" - ); - assert_eq!( - after.generation, 1, - "generation must advance across a resize" + after.viewport, applied, + "frame's viewport disagrees with the one resize reported applying" ); assert_eq!(after.rows.len(), 10, "full frame must carry every line"); let row0: String = after.rows[0] @@ -98,12 +103,14 @@ fn identical_resize_is_inert() { shared.feed(b"hello"); shared.render(&mut encoder); - shared.resize(size(40)); - let after = shared.render(&mut encoder); + let applied = shared.resize(size(40)); assert_eq!( - after.generation, 0, + applied.generation, 0, "a same-size resize advanced the generation" ); + + let after = shared.render(&mut encoder); + assert_eq!(after.viewport, applied); assert!( !after.full, "a same-size resize forced a needless full repaint" @@ -138,7 +145,7 @@ fn full_frame_after_height_resize_republishes_unchanged_rows() { after.full, "a resize must invalidate the renderer's cached rows" ); - assert_eq!(after.screen_lines, 20); + assert_eq!(after.viewport.screen_lines, 20); assert_eq!( after.rows.len(), 20, @@ -147,3 +154,43 @@ fn full_frame_after_height_resize_republishes_unchanged_rows() { after.rows.len() ); } + +/// A frame is stamped with the grid it was **captured on**, and a later resize +/// does not retroactively re-label it. +/// +/// This is the cross-transport race in the integration lane: frame delivery and +/// the resize call are separate paths, so a generation-N frame can arrive after +/// generation N+1 has been applied. Rejecting it requires the stamp to be +/// capture-time truth. +/// +/// Note what is and is not proven here. That an owned `Frame` cannot mutate is +/// guaranteed by the language, so asserting it against a copy of itself would +/// be tautological. What this asserts is that `capture()` stamps the viewport +/// as it was **at capture**, against explicit expected values -- a `capture()` +/// that read the viewport a moment later, or a `Frame` that carried a handle +/// back to the terminal, would fail here. +#[test] +fn a_frame_is_stamped_with_the_grid_it_was_captured_on() { + let (shared, _actions) = shared(grid(40, 10)); + let mut encoder = Encoder::new(); + shared.feed(b"\x1b[2J\x1b[Hhello world"); + + let in_flight = shared.render(&mut encoder); + assert_eq!(in_flight.viewport.generation, 0); + assert_eq!(in_flight.viewport.columns, 40); + + let applied = shared.resize(grid(20, 10)); + assert_eq!(applied.generation, 1); + assert_eq!(applied.columns, 20); + + // The held frame still describes the pre-resize grid, so a consumer can + // compare the two and discard it rather than paint 40-column rows onto a + // 20-column grid. + assert_eq!( + in_flight.viewport.columns, 40, + "a frame captured before the resize describes the post-resize grid; \ + a stale frame arriving late would be indistinguishable from a fresh one" + ); + assert_eq!(in_flight.viewport.generation, 0); + assert_ne!(in_flight.viewport, applied); +}