feat(desktop): make a frame's viewport comparable in one value

The integration lane's core operation is "does this frame describe the grid I
am currently showing?" -- a frame captured before a resize can arrive after it,
because frame delivery and the resize call travel separate paths. Answering
that meant comparing three loose fields, and a consumer that checks two of the
three is wrong on exactly the resize that changed the one it skipped.

Group them into `Viewport { generation, columns, screen_lines }` so the
question is one equality, and return it from `resize` rather than making the
caller ask afterwards: across a transport a follow-up query races the next
resize, so its answer can describe a grid that has already been replaced. A
same-size resize stays inert and reports the unchanged generation.

Two mutants, both caught: returning the requested viewport instead of the
applied one, and stamping a frame at encode time instead of capture time.

The stamping fixture was tautological on its first draft -- it compared a
frame's viewport against a copy taken from that same frame, which the language
already guarantees. It now asserts capture-time values explicitly, and the
comment records why the obvious form proves nothing.

Co-authored-by: Dawn (sprout agent) <c6237ef84fa537c78dcee78efd2d4e59f728859c7f194da42ac51ededfa0be05@sprout-oss.stage.blox.sqprod.co>
Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>
This commit is contained in:
tlongwell-block
2026-08-01 20:12:36 -04:00
co-authored by Dawn
parent a094adce9a
commit 105aadb22c
4 changed files with 105 additions and 41 deletions
@@ -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<Cell>)>,
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,
}
}
}
@@ -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<Listener> {
@@ -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,
@@ -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);
}