From 6aa72ee8d9908c05d4c850418d6983ee3890bc1c Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Tue, 27 Oct 2026 16:22:00 +0700 Subject: [PATCH 1/2] fix(graphics): move the cursor past an image as the declared terminal does A terminal that speaks kitty or sixel graphics moves the cursor past an image it places; termlens left it where the image began, and DESIGN.md said neither protocol moves it. The emulator now follows the terminal the test declared with graphics() and cell_size(): - kitty: onto the placement's last row, one column past its right edge, wrapping past the screen's edge and scrolling past the bottom margin, unless C=1, U=1 or P= says otherwise. The size in cells is c= and r=, or kitty's own arithmetic (update_dest_rect) over the source rectangle and the cell size; a=p finds its image's size by id in the payload log. - sixel: to the image's last text row, the column kept, as xterm does with sixel scrolling on; the height is the larger of the raster's and the painted one, measured before a payload past the capture bound is dropped. - Graphics::None, the default, moves nothing, as a terminal without the protocol ignores the escape. The facts the move needs travel beside the payload, not in it, so the JSON shape is unchanged. A kitty transmission naming no action is now a=t, transmit only, the protocol's default; it was reported as placed, which would also have moved the cursor for images kitty never shows. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- CHANGELOG.md | 11 ++ crates/termlens/src/emu/seq.rs | 25 +-- crates/termlens/src/emu/vt100.rs | 226 +++++++++++++++++++++++++- crates/termlens/src/graphics.rs | 255 ++++++++++++++++++++++++++++-- crates/termlens/src/terminal.rs | 25 ++- crates/termlens/tests/graphics.rs | 31 ++++ docs/DESIGN.md | 14 +- docs/LIMITATIONS.md | 13 ++ 8 files changed, 562 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c09a90b..5bd71e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,6 +101,17 @@ reads that marker. also write a character XML 1.0 forbids, a C0 control other than tab, LF and CR or U+FFFE/U+FFFF, as U+FFFD, so a window title holding `\x01` or a cell holding U+FFFF no longer makes an SVG no parser opens. +- A terminal declared to speak kitty or sixel graphics (`graphics()`) + moves the cursor past an image it places, as that terminal does (#588): + kitty to the placement's last row, one column past its right edge, + wrapping and scrolling as kitty does, unless `C=1`, `U=1` or `P=` says + otherwise; xterm's sixel, given a `cell_size()`, to the image's last text + row. The cursor stayed where the image began, so text an application + wrote after an image overwrote it, and `docs/DESIGN.md` claimed neither + protocol moves the cursor. The default, `Graphics::None`, still moves + nothing: a terminal without the protocol ignores the escape. A kitty + transmission that names no action is now reported as `a=t`, transmit + only, the protocol's default; it was reported as an image placed. - A chunked kitty upload (`m=1`) is no longer completed by an escape that cannot continue it (#587). A sixel sent mid-upload was built on top of it: one payload, a sixel carrying the kitty `s=`, `c=`/`r=`, `t=` and diff --git a/crates/termlens/src/emu/seq.rs b/crates/termlens/src/emu/seq.rs index 663f5c8..7e8e594 100644 --- a/crates/termlens/src/emu/seq.rs +++ b/crates/termlens/src/emu/seq.rs @@ -54,7 +54,7 @@ use std::sync::Arc; -use crate::graphics::{GraphicsBuilder, GraphicsCounts, GraphicsPayload}; +use crate::graphics::{GraphicsBuilder, GraphicsCounts, GraphicsPayload, Placement}; use crate::screen::{Clipboard, Link, MouseModes}; /// OSC strings are captured whole (titles must not truncate), but bounded: @@ -410,8 +410,9 @@ pub(crate) enum SeqEvent { Query(Query), /// An inline graphics payload completed. Carried out of the tracker /// rather than stored in it because the placement — where the cursor - /// stood — is a fact about the grid, which only the emulator holds. - Graphics(Box), + /// stood — is a fact about the grid, which only the emulator holds; so + /// is moving the cursor past the image, which the [`Placement`] sizes. + Graphics(Box, Placement), /// A tab-stop operation completed, and needs the cursor column to /// finish — carried out for the same reason a graphics placement is. Tabs(TabOp), @@ -1405,10 +1406,10 @@ impl SeqTracker { if key_is(keys, b"m", b"1") { return SeqEvent::None; } - return match self.building.finish() { - Some(payload) => { + return match self.building.finish_placed() { + Some((payload, placement)) => { self.counts.record(&payload); - SeqEvent::Graphics(Box::new(payload)) + SeqEvent::Graphics(Box::new(payload), placement) } None => SeqEvent::None, }; @@ -1439,10 +1440,10 @@ impl SeqTracker { self.dcs_body_full, self.capture, ); - return match self.building.finish() { - Some(payload) => { + return match self.building.finish_placed() { + Some((payload, placement)) => { self.counts.record(&payload); - SeqEvent::Graphics(Box::new(payload)) + SeqEvent::Graphics(Box::new(payload), placement) } None => SeqEvent::None, }; @@ -1892,7 +1893,7 @@ mod tests { let mut seen = None; for &byte in &wire { - if let SeqEvent::Graphics(payload) = tracker.step(byte) { + if let SeqEvent::Graphics(payload, _) = tracker.step(byte) { seen = Some(*payload); } } @@ -1911,7 +1912,7 @@ mod tests { let mut tracker = SeqTracker::new(crate::graphics::DEFAULT_CAPTURE, TEST_COLS); let mut out = Vec::new(); for &byte in bytes { - if let SeqEvent::Graphics(payload) = tracker.step(byte) { + if let SeqEvent::Graphics(payload, _) = tracker.step(byte) { out.push(*payload); } } @@ -2109,7 +2110,7 @@ mod tests { let mut tracker = SeqTracker::new(8, TEST_COLS); let mut seen = None; for &byte in b"\x1b_Ga=T,f=32,s=4,v=4;AAAABBBBCCCCDDDD\x1b\\" { - if let SeqEvent::Graphics(payload) = tracker.step(byte) { + if let SeqEvent::Graphics(payload, _) = tracker.step(byte) { seen = Some(*payload); } } diff --git a/crates/termlens/src/emu/vt100.rs b/crates/termlens/src/emu/vt100.rs index 6164d06..59e47f4 100644 --- a/crates/termlens/src/emu/vt100.rs +++ b/crates/termlens/src/emu/vt100.rs @@ -11,8 +11,12 @@ use super::seq::{SeqEvent, SeqTracker, TabOp}; use super::shadow::{AttrShadow, ColorNormalizer}; use super::unhandled::Unhandled; use super::{Emulator, FrameSpan, InputModes, ModeState, MouseEncoding, Processed, Stop}; -use crate::graphics::{GraphicsPayload, GraphicsSeen, HISTORY}; +use crate::graphics::{ + kitty_cells, GraphicsAction, GraphicsPayload, GraphicsProtocol, GraphicsSeen, Placement, + HISTORY, +}; use crate::screen::{Cell, Color, MouseMode, Screen, Style, TermState}; +use crate::Graphics; pub(crate) struct Vt100Emulator { /// Built with the callback set that records what vt100 could not @@ -56,6 +60,10 @@ pub(crate) struct Vt100Emulator { graphics_bytes: usize, /// The retention budget, from `TerminalBuilder::capture_graphics`. capture: usize, + /// The inline-graphics support the test declared, and its cell size: + /// what decides whether, and how far, an image moves the cursor (#588). + image_support: Graphics, + cell_size: Option<(u16, u16)>, /// Bytes rewritten on their way to the parsers and not yet handed over. /// Empty except while a DEC Special Graphics set is invoked: a byte the /// set redefines reaches the grid as the glyph it draws, so the stream @@ -64,6 +72,15 @@ pub(crate) struct Vt100Emulator { staged: Vec, } +/// How far past an image the declared terminal moves the cursor. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ImageStep { + /// kitty: the placement's size in cells. + Kitty { cols: u32, rows: u32 }, + /// xterm's sixel: the text rows the image covers. + Sixel { rows: u32 }, +} + impl Vt100Emulator { /// Close out the frame that just ended: how long it ran between the /// application's own markers, and how much it drew. @@ -109,10 +126,88 @@ impl Vt100Emulator { graphics: Arc::new(Vec::new()), graphics_bytes: 0, capture, + image_support: Graphics::None, + cell_size: None, staged: Vec::new(), } } + /// Simulate a terminal with `support` for inline graphics and cells of + /// `cell_size` pixels: an image it places then moves the cursor as that + /// terminal moves it. A terminal without the protocol ignores the + /// escape and moves nothing — the default (#588). + pub(crate) fn declare_graphics(&mut self, support: Graphics, cell_size: Option<(u16, u16)>) { + self.image_support = support; + self.cell_size = cell_size; + } + + /// How far the declared terminal moves the cursor past `payload`, or + /// `None` where it does not move it or the escape does not say. + /// + /// kitty moves it right by the placement's columns and down by its rows + /// less one — onto the image's last row, one column past it — for a + /// placement that asks (no `C=1`, `U=1` or `P=`). xterm, with sixel + /// scrolling on as it is by default, moves it down to the image's last + /// text row and leaves the column. Each ignores the other's escape. + fn image_step(&self, payload: &GraphicsPayload, placement: &Placement) -> Option { + match (self.image_support, payload.protocol()) { + (Graphics::Kitty, GraphicsProtocol::Kitty) => { + if !placement.moves { + return None; + } + let image = match payload.action() { + GraphicsAction::TransmitAndPlace => payload.size(), + // A placement of an image sent earlier: its size is the + // last transmission of that id still in the log. + GraphicsAction::Place => { + let id = payload.id()?; + self.graphics.iter().rev().find_map(|earlier| { + (earlier.protocol() == GraphicsProtocol::Kitty + && earlier.id() == Some(id)) + .then(|| earlier.size()) + .flatten() + }) + } + _ => return None, + }; + let (cols, rows) = kitty_cells(placement, image, self.cell_size)?; + Some(ImageStep::Kitty { cols, rows }) + } + (Graphics::Sixel, GraphicsProtocol::Sixel) => { + let (_, cell_height) = self.cell_size.filter(|&(w, h)| w > 0 && h > 0)?; + let rows = placement.height.div_ceil(u32::from(cell_height)); + (rows > 0).then_some(ImageStep::Sixel { rows }) + } + _ => None, + } + } + + /// Move the cursor past an image (#588): down to its last row — a line + /// feed at a time, so the bottom margin scrolls and the rows it pushes + /// out reach history as any other scroll does — then, for kitty, one + /// column past its right edge, or to the start of the next row when that + /// is past the screen's. No count scrolls further than the screen is + /// tall: past that every row it pushed out was blank. + fn move_past_image(&mut self, step: ImageStep) { + let (screen_rows, screen_cols) = size(self.parser.screen()); + let (_, col) = self.cursor_position(); + let (rows, across) = match step { + ImageStep::Kitty { cols, rows } => (rows, Some(u32::from(col).saturating_add(cols))), + ImageStep::Sixel { rows } => (rows, None), + }; + let wrap = across.is_some_and(|x| x >= u32::from(screen_cols)); + let down = (rows.saturating_sub(1) + u32::from(wrap)).min(u32::from(screen_rows)); + let mut bytes = vec![b'\n'; down as usize]; + match across { + Some(_) if wrap => bytes.push(b'\r'), + Some(x) if x > u32::from(col) => { + bytes.extend_from_slice(format!("\x1b[{}C", x - u32::from(col)).as_bytes()); + } + _ => {} + } + self.feed(&bytes); + } + /// Hand the grid the bytes the tracker has already scanned. /// /// The tracker runs a byte ahead of the parser so it can stop the read @@ -189,11 +284,14 @@ impl Vt100Emulator { /// File a completed payload, stamped with where it landed. /// - /// Neither protocol's escape moves the cursor, so the position once the - /// terminator has been consumed *is* the image's top-left corner — the - /// one fact about an image that lives in the grid rather than in the - /// payload, and the one an application gets wrong when a picture drifts - /// out from under its own labels. + /// The position once the terminator has been consumed *is* the image's + /// top-left corner: stamped before [`move_past_image`] moves the cursor + /// on, as the declared terminal does. It is the one fact about an image + /// that lives in the grid rather than in the payload, and the one an + /// application gets wrong when a picture drifts out from under its own + /// labels. + /// + /// [`move_past_image`]: Self::move_past_image fn record_graphics(&mut self, mut payload: GraphicsPayload) { payload.place(self.cursor_position()); let kept = payload.data().map_or(0, <[u8]>::len); @@ -416,10 +514,17 @@ impl Emulator for Vt100Emulator { let stop = match self.tracker.step(byte) { SeqEvent::SyncEnd => Some(Stop::FrameComplete(self.close_frame())), SeqEvent::Query(query) => Some(Stop::Query(query)), - SeqEvent::Graphics(payload) => { + SeqEvent::Graphics(payload, placement) => { self.feed_staged(&bytes[fed..=i]); fed = i + 1; + // Sized before the payload joins the log, so a placement + // finds the earlier transmission and not itself; moved + // after it is stamped, so `at` is the top-left corner. + let step = self.image_step(&payload, &placement); self.record_graphics(*payload); + if let Some(step) = step { + self.move_past_image(step); + } None } SeqEvent::SoftReset => { @@ -1475,6 +1580,113 @@ mod tests { assert_eq!(back.find("Y"), Some((0, 6))); } + /// An emulator simulating `support`, cells 10x20 pixels, fed `bytes`. + fn graphics_emu(support: Graphics, rows: u16, cols: u16, bytes: &[u8]) -> Vt100Emulator { + let mut emu = Vt100Emulator::new(rows, cols, 100, crate::graphics::DEFAULT_CAPTURE, false); + emu.declare_graphics(support, Some((10, 20))); + feed_all(&mut emu, bytes); + emu + } + + /// Where `X` landed. + fn x_at(emu: &Vt100Emulator) -> Option<(u16, u16)> { + emu.snapshot().find("X") + } + + #[test] + fn a_kitty_placement_moves_the_cursor_to_its_last_row_one_column_past_it() { + // kitty moves the cursor right by the columns and down by the rows + // less one (#588); `at` stays the top-left corner. + let placed = b"\x1b_Ga=T,f=24,s=1,v=1,c=3,r=2;ESIz\x1b\\X"; + let emu = graphics_emu(Graphics::Kitty, 6, 10, placed); + assert_eq!(x_at(&emu), Some((1, 3))); + assert_eq!(emu.snapshot().graphics().payloads()[0].at(), (0, 0)); + // A terminal without the protocol ignores the escape: nothing moves. + for support in [Graphics::None, Graphics::Sixel] { + assert_eq!( + x_at(&graphics_emu(support, 6, 10, placed)), + Some((0, 0)), + "{support:?}" + ); + } + // Asked not to (`C=1`), or only transmitted: no move either. A + // transmission naming no action is `a=t`. + for escape in [ + &b"\x1b_Ga=T,f=24,s=1,v=1,c=3,r=2,C=1;ESIz\x1b\\X"[..], + b"\x1b_Gf=24,s=1,v=1,c=3,r=2;ESIz\x1b\\X", + ] { + assert_eq!( + x_at(&graphics_emu(Graphics::Kitty, 6, 10, escape)), + Some((0, 0)) + ); + } + } + + #[test] + fn a_kitty_placement_wraps_and_scrolls_as_kitty_does() { + // Past the right edge: the start of the next row. + let emu = graphics_emu( + Graphics::Kitty, + 6, + 10, + b"\x1b[1;9H\x1b_Ga=T,f=24,s=1,v=1,c=3,r=1;ESIz\x1b\\X", + ); + assert_eq!(x_at(&emu), Some((1, 0))); + // Below the bottom margin: the screen scrolls, the rows pushed out + // go to history, and the cursor ends on the last row. + let emu = graphics_emu( + Graphics::Kitty, + 4, + 10, + b"top\r\n\r\n\r\n\x1b_Ga=T,f=24,s=1,v=1,c=2,r=3;ESIz\x1b\\X", + ); + let screen = emu.snapshot(); + assert_eq!(screen.find("X"), Some((3, 2))); + assert_eq!(screen.scrollback_text(), "top\n"); + } + + #[test] + fn a_kitty_placement_without_cells_is_sized_from_pixels_and_the_cell_size() { + // A 20x40 image in 10x20 cells: 2x2. `a=p` of it, by id, later. + let emu = graphics_emu( + Graphics::Kitty, + 6, + 10, + b"\x1b_Ga=t,f=24,s=20,v=40,i=7;ESIz\x1b\\\x1b_Ga=p,i=7\x1b\\X", + ); + assert_eq!(x_at(&emu), Some((1, 2))); + // Without a cell size there is nothing to size it by: no move. + let mut emu = Vt100Emulator::new(6, 10, 0, crate::graphics::DEFAULT_CAPTURE, false); + emu.declare_graphics(Graphics::Kitty, None); + feed_all(&mut emu, b"\x1b_Ga=T,f=24,s=20,v=40;ESIz\x1b\\X"); + assert_eq!(x_at(&emu), Some((0, 0))); + } + + #[test] + fn a_sixel_moves_the_cursor_to_its_last_text_row_and_keeps_the_column() { + // xterm, sixel scrolling on: 45 px over 20-px cells is three rows, + // so the cursor goes down two, in the column the image started at. + let sixel = b"\x1b[1;3H\x1bPq\"1;1;4;45#0;2;100;0;0~~~~-~~~~-~~~~\x1b\\X"; + assert_eq!( + x_at(&graphics_emu(Graphics::Sixel, 6, 10, sixel)), + Some((2, 2)) + ); + // An image shorter than a cell stays on its row. + let short = b"\x1bPq\"1;1;4;6#0;2;100;0;0~~~~\x1b\\X"; + assert_eq!( + x_at(&graphics_emu(Graphics::Sixel, 6, 10, short)), + Some((0, 0)) + ); + // kitty ignores sixel, and the default ignores both. + for support in [Graphics::None, Graphics::Kitty] { + assert_eq!( + x_at(&graphics_emu(support, 6, 10, sixel)), + Some((0, 2)), + "{support:?}" + ); + } + } + #[test] fn history_rows_keep_the_width_they_were_captured_at() { // Documented decision (`Terminal::resize`): history is not reflowed diff --git a/crates/termlens/src/graphics.rs b/crates/termlens/src/graphics.rs index e329ab4..1043548 100644 --- a/crates/termlens/src/graphics.rs +++ b/crates/termlens/src/graphics.rs @@ -330,7 +330,11 @@ impl GraphicsPayload { } /// Where the cursor stood when the payload completed, as `(row, col)` — - /// which for both protocols is the image's top-left corner. + /// which for both protocols is the image's top-left corner. A terminal + /// that speaks the protocol moves the cursor past the image afterwards, + /// and so does a termlens terminal declared to + /// ([`graphics`](crate::TerminalBuilder::graphics)); text that scrolls + /// later leaves this where the image was placed. #[must_use] pub fn at(&self) -> (u16, u16) { self.at @@ -989,6 +993,35 @@ impl GraphicsSeen { } } +/// Where an image lands, as far as its escape says: what the emulator needs +/// to move the cursor past it the way the terminal it simulates does (#588). +/// +/// Kept beside the payload rather than in it: the payload's fields are its +/// JSON, which a saved screen promises not to change. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub(crate) struct Placement { + /// kitty's `c=`, `r=`: the placement's size in cells, 0 when the escape + /// leaves it to the image's size, as kitty reads a missing key. + pub(crate) cols: u32, + pub(crate) rows: u32, + /// kitty's `w=`, `h=`, `x=`, `y=`: the source rectangle within the image, + /// 0 for the whole image. + pub(crate) src_width: u32, + pub(crate) src_height: u32, + pub(crate) src_x: u32, + pub(crate) src_y: u32, + /// kitty's `X=`, `Y=`: the offset within the first cell, in pixels. + pub(crate) cell_x: u32, + pub(crate) cell_y: u32, + /// kitty: whether a placement moves the cursor at all. `C=1` asks it not + /// to, and neither a Unicode-placeholder placement (`U=1`) nor one + /// relative to another image (`P=`) moves it. + pub(crate) moves: bool, + /// sixel: the image's height in pixels as xterm measures it, the larger + /// of the raster attributes' and the painted rows'. + pub(crate) height: u32, +} + /// A payload under construction, owned by the sequence tracker: kitty splits /// one image across escapes, so a payload is not complete until a kitty /// escape arrives without `m=1` — and it is [abandoned](Self::abandon) @@ -1009,6 +1042,7 @@ pub(crate) struct GraphicsBuilder { /// Set once any chunk arrived past the capture bound. The payload then /// reports no data at all rather than a prefix of one. dropped: bool, + placement: Placement, } impl Default for GraphicsBuilder { @@ -1026,6 +1060,7 @@ impl Default for GraphicsBuilder { bytes: 0, data: Vec::new(), dropped: false, + placement: Placement::default(), } } } @@ -1038,10 +1073,11 @@ impl GraphicsBuilder { if self.protocol.is_none() { self.protocol = Some(GraphicsProtocol::Kitty); self.action = match key(control, b"a") { - Some(b"t") => GraphicsAction::Transmit, + // `a=t`, transmit only, is the protocol's default: a + // transmission that names no action shows nothing (#588). + Some(b"t") | None => GraphicsAction::Transmit, Some(b"p") => GraphicsAction::Place, - // `a=T` is the default when a transmission names no action. - Some(b"T") | None => GraphicsAction::TransmitAndPlace, + Some(b"T") => GraphicsAction::TransmitAndPlace, Some(b"d") => GraphicsAction::Delete, Some(_) => GraphicsAction::Other, }; @@ -1068,6 +1104,19 @@ impl GraphicsBuilder { )), _ => None, }; + let value = |name: &[u8]| number(control, name).unwrap_or(0); + self.placement = Placement { + cols: value(b"c"), + rows: value(b"r"), + src_width: value(b"w"), + src_height: value(b"h"), + src_x: value(b"x"), + src_y: value(b"y"), + cell_x: value(b"X"), + cell_y: value(b"Y"), + moves: value(b"C") != 1 && value(b"U") != 1 && key(control, b"P").is_none(), + height: 0, + }; } } @@ -1092,6 +1141,12 @@ impl GraphicsBuilder { /// entirely rather than kept as a prefix that would decode into a /// plausible-looking wrong picture. pub(crate) fn chunk(&mut self, bytes: u64, data: &[u8], complete: bool, cap: usize) { + // Measured here, before a chunk past the capture bound is dropped: + // the cursor moves past an image whatever this test chose to keep. + if self.protocol == Some(GraphicsProtocol::Sixel) && self.chunks == 0 { + let declared = raster_size(data).map_or(0, |(_, height)| height); + self.placement.height = declared.max(sixel_painted_height(data)); + } self.chunks += 1; self.bytes += bytes; let fits = self.data.len().saturating_add(data.len()) <= cap; @@ -1103,10 +1158,17 @@ impl GraphicsBuilder { } } - /// Finish the payload. `at` is stamped later, by the emulator, once the - /// grid has caught up with the terminator. + /// [`finish_placed`](Self::finish_placed) without the placement. + #[cfg(test)] pub(crate) fn finish(&mut self) -> Option { + self.finish_placed().map(|(payload, _)| payload) + } + + /// Finish the payload, with where it lands. `at` is stamped later, by + /// the emulator, once the grid has caught up with the terminator. + pub(crate) fn finish_placed(&mut self) -> Option<(GraphicsPayload, Placement)> { let protocol = self.protocol.take()?; + let placement = self.placement; let mut size = self.size; if protocol == GraphicsProtocol::Sixel { size = raster_size(&self.data).or(size); @@ -1127,7 +1189,7 @@ impl GraphicsBuilder { .then(|| Arc::from(std::mem::take(&mut self.data).into_boxed_slice())), }; *self = Self::default(); - Some(payload) + Some((payload, placement)) } /// Drop a transmission that will never complete, handing back the wire @@ -1168,6 +1230,86 @@ fn raster_size(data: &[u8]) -> Option<(u32, u32)> { } } +/// The height in pixels the sixels in `data` paint, as xterm's decoder +/// measures it: down to the lowest pixel set. Bands of six rows are +/// separated by `-`, and each sixel character's bits are its band's rows, +/// the lowest bit the top one. +fn sixel_painted_height(data: &[u8]) -> u32 { + let mut band = 0u32; + let mut height = 0u32; + for &byte in data { + match byte { + b'-' => band = band.saturating_add(1), + 0x3f..=0x7e => { + let bits = u32::from(byte - 0x3f); + if bits != 0 { + let lowest = band.saturating_mul(6) + (u32::BITS - bits.leading_zeros()); + height = height.max(lowest); + } + } + _ => {} + } + } + height +} + +/// The cells a kitty placement covers, `(cols, rows)`, computed as kitty +/// computes them (`update_dest_rect` in kitty's `graphics.c`): `c=` and `r=` +/// when both are given; otherwise from the source rectangle — `w=`/`h=` or +/// the whole image, less `x=`/`y=` — and the cell size, keeping the image's +/// aspect when one of the two is given. `None` when that needs an image size +/// or a cell size nobody stated. +pub(crate) fn kitty_cells( + placement: &Placement, + image: Option<(u32, u32)>, + cell: Option<(u16, u16)>, +) -> Option<(u32, u32)> { + let (mut cols, mut rows) = (placement.cols, placement.rows); + if cols > 0 && rows > 0 { + return Some((cols, rows)); + } + let (cell_w, cell_h) = cell + .map(|(w, h)| (u32::from(w), u32::from(h))) + .filter(|&(w, h)| w > 0 && h > 0)?; + let (image_w, image_h) = image?; + let src_w = if placement.src_width > 0 { + placement.src_width + } else { + image_w + }; + let src_h = if placement.src_height > 0 { + placement.src_height + } else { + image_h + }; + let src_w = src_w.min(image_w - placement.src_x.min(image_w)); + let src_h = src_h.min(image_h - placement.src_y.min(image_h)); + let offset_x = placement.cell_x.min(cell_w - 1); + let offset_y = placement.cell_y.min(cell_h - 1); + let (auto_cols, auto_rows) = (cols == 0, rows == 0); + if auto_cols { + cols = if auto_rows { + (src_w + offset_x).div_ceil(cell_w) + } else if src_h == 0 { + 0 + } else { + let height = f64::from(cell_h * rows + offset_y); + (height * f64::from(src_w) / f64::from(src_h) / f64::from(cell_w)).ceil() as u32 + }; + } + if auto_rows { + rows = if auto_cols { + (src_h + offset_y).div_ceil(cell_h) + } else if src_w == 0 { + 0 + } else { + let width = f64::from(cell_w * cols + offset_x); + (width * f64::from(src_h) / f64::from(src_w) / f64::from(cell_h)).ceil() as u32 + }; + } + Some((cols, rows)) +} + /// The raw value of `name` in a kitty control block (`a=T,i=3,f=32`). fn key<'a>(control: &'a [u8], name: &[u8]) -> Option<&'a [u8]> { control.split(|&b| b == b',').find_map(|pair| { @@ -1213,13 +1355,108 @@ mod tests { #[test] fn the_protocols_defaults_are_the_protocols_defaults() { - // A transmission naming neither action nor format is `a=T,f=32`. + // A transmission naming neither action nor format is `a=t,f=32`: + // transmit only. It was reported as `a=T`, an image placed, which + // kitty never shows (#588). let payload = kitty_payload(b"s=1,v=1", b"AAAA"); - assert_eq!(payload.action(), GraphicsAction::TransmitAndPlace); + assert_eq!(payload.action(), GraphicsAction::Transmit); assert_eq!(payload.format(), GraphicsFormat::Rgba); assert!(!payload.compressed()); } + /// The placement facts the cursor move needs, read off the control + /// block of the first chunk (#588). + #[test] + fn a_kitty_placement_records_its_size_source_and_whether_it_moves() { + let mut builder = GraphicsBuilder::default(); + builder.kitty(b"a=T,f=24,s=8,v=4,c=3,w=6,h=2,x=1,y=1,X=4,Y=5"); + builder.chunk(4, b"AAAA", true, usize::MAX); + let (_, placement) = builder.finish_placed().expect("a payload"); + assert_eq!( + placement, + Placement { + cols: 3, + rows: 0, + src_width: 6, + src_height: 2, + src_x: 1, + src_y: 1, + cell_x: 4, + cell_y: 5, + moves: true, + height: 0, + } + ); + for control in [&b"a=T,C=1"[..], b"a=T,U=1", b"a=p,i=1,P=2"] { + let mut builder = GraphicsBuilder::default(); + builder.kitty(control); + builder.chunk(0, b"", true, usize::MAX); + let (_, placement) = builder.finish_placed().expect("a payload"); + assert!(!placement.moves, "{}", String::from_utf8_lossy(control)); + } + } + + /// kitty's own arithmetic (`update_dest_rect`): the cells given, or the + /// source rectangle over the cell size, rounded up, keeping the aspect + /// when only one side is given. + #[test] + fn kitty_sizes_a_placement_as_kitty_does() { + let cell = Some((10, 20)); + let given = |cols, rows| Placement { + cols, + rows, + ..Placement::default() + }; + // Both given: no image or cell size needed. + assert_eq!(kitty_cells(&given(3, 2), None, None), Some((3, 2))); + // Neither: the image over the cell, rounded up. + assert_eq!( + kitty_cells(&given(0, 0), Some((25, 41)), cell), + Some((3, 3)) + ); + // One given: the other keeps the aspect. 2 rows are 40 px tall, so a + // 20x10 image is 80 px wide there: 8 columns. + assert_eq!( + kitty_cells(&given(0, 2), Some((20, 10)), cell), + Some((8, 2)) + ); + assert_eq!( + kitty_cells(&given(4, 0), Some((20, 10)), cell), + Some((4, 1)) + ); + // A source rectangle, less its origin, and the offset in the cell. + let cropped = Placement { + src_width: 15, + src_height: 50, + src_x: 10, + cell_x: 6, + ..given(0, 0) + }; + // 15 wide but only 10 left past x=10; plus 6: 16 px, 2 columns. + assert_eq!(kitty_cells(&cropped, Some((20, 60)), cell), Some((2, 3))); + // Nothing to compute from: no move rather than a guess. + assert_eq!(kitty_cells(&given(0, 0), None, cell), None); + assert_eq!(kitty_cells(&given(0, 0), Some((20, 10)), None), None); + } + + /// xterm measures a sixel down to its lowest painted pixel; each sixel + /// character's bits are its band's six rows, the low bit the top one. + #[test] + fn a_sixel_is_as_tall_as_the_lowest_pixel_it_paints() { + assert_eq!(sixel_painted_height(b"#0~~~~"), 6); + assert_eq!(sixel_painted_height(b"#0~-~-@"), 13); + // `?` paints nothing; colour and raster parameters are not sixels. + assert_eq!(sixel_painted_height(b"\"1;1;4;45#0;2;100;0;0??"), 0); + // The builder takes the larger of the raster's and the painted one, + // from the data as it arrives — kept past the capture bound too. + let mut builder = GraphicsBuilder::default(); + builder.sixel(); + builder.chunk(8, b"\"1;1;4;45#0~-~", true, 0); + let (payload, placement) = builder.finish_placed().expect("a payload"); + assert_eq!(payload.data(), None, "dropped by the bound"); + assert_eq!(placement.height, 45); + } + #[test] fn a_delete_is_not_an_image() { let payload = kitty_payload(b"a=d,d=I,i=1,q=2", b""); diff --git a/crates/termlens/src/terminal.rs b/crates/termlens/src/terminal.rs index c5730ef..1e3822d 100644 --- a/crates/termlens/src/terminal.rs +++ b/crates/termlens/src/terminal.rs @@ -1787,7 +1787,12 @@ impl TerminalBuilder { /// Defaults to [`Graphics::None`] — nothing claimed. /// /// See [`Graphics`] for why declaring is not the harness lying, and pair - /// it with [`cell_size`](Self::cell_size). + /// it with [`cell_size`](Self::cell_size). A declared terminal also moves + /// the cursor past an image it places, as that terminal does — kitty to + /// the placement's last row, one column past it; xterm's sixel to the + /// image's last text row — so text the application writes next lands + /// where it would there (#588). `docs/LIMITATIONS.md` says what the move + /// cannot know. #[must_use] pub fn graphics(mut self, graphics: Graphics) -> Self { self.graphics = graphics; @@ -1958,13 +1963,17 @@ impl TerminalBuilder { // memory bound instead of on a queue-slot count. let queued_bytes = Arc::new(AtomicUsize::new(0)); let shared = Arc::new(Monitor::new(EmuState::new( - Box::new(Vt100Emulator::new( - self.rows, - self.cols, - self.scrollback, - self.capture_graphics, - self.scrollback_styles, - )), + Box::new({ + let mut emu = Vt100Emulator::new( + self.rows, + self.cols, + self.scrollback, + self.capture_graphics, + self.scrollback_styles, + ); + emu.declare_graphics(self.graphics, self.cell_size); + emu + }), Responder { respond: self.answer_queries, background: self.background, diff --git a/crates/termlens/tests/graphics.rs b/crates/termlens/tests/graphics.rs index 2fa39ae..db21dfb 100644 --- a/crates/termlens/tests/graphics.rs +++ b/crates/termlens/tests/graphics.rs @@ -302,3 +302,34 @@ mod decoded { Ok(()) } } + +/// A terminal declared to speak kitty graphics moves the cursor past a +/// placement as kitty does, onto the image's last row one column past it, +/// and the default terminal, which claims no graphics and so ignores the +/// escape, moves nothing (#588). The builder's `graphics` and `cell_size` +/// are what reach the emulator. +#[test] +#[cfg_attr( + windows, + ignore = "ConPTY does not forward kitty or sixel payloads (#149)" +)] +fn a_declared_kitty_terminal_moves_the_cursor_past_a_placement() -> termlens::Result<()> { + for (graphics, expected) in [ + (termlens::Graphics::Kitty, (1, 3)), + (termlens::Graphics::None, (0, 0)), + ] { + let mut t = util::spawn_emit( + Terminal::builder() + .size(20, 4) + .timeout(Duration::from_secs(30)) + .graphics(graphics) + .cell_size(10, 20), + &["--raw", r"\e_Ga=T,f=24,s=1,v=1,c=3,r=2;ESIz\e\\X", "--wait"], + )?; + t.wait_until(|s| s.contains("X"))?; + assert_eq!(t.screen().find("X"), Some(expected), "{graphics:?}"); + t.send(termlens::Key::Enter)?; + t.wait_exit()?; + } + Ok(()) +} diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 6df2ff6..872dea2 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -755,8 +755,18 @@ Rules: cursor position at the terminator. That is why `process` now feeds the parser in pieces: the tracker runs a byte ahead of the grid, and a placement read before the grid caught up would be the position from before - the whole read. Neither protocol's escape moves the cursor, so the position - after the terminator is the image's top-left corner. + the whole read. The position at the terminator is the image's top-left + corner, stamped before anything moves the cursor. A terminal that speaks + the protocol then does move it, and the emulator follows the one the test + declared (`graphics()`, with `cell_size()`; #588): kitty puts it on the + placement's last row, one column past its right edge, wrapping to the + next row past the screen's and scrolling past the bottom margin, unless + `C=1`, `U=1` or `P=` says otherwise; xterm, with sixel scrolling on as it + is by default, puts it on the image's last text row and leaves the + column. A terminal declared without the protocol ignores the escape and + moves nothing, which is the default. The cursor is moved with line feeds + and a cursor-forward fed to the grid, so a scroll it causes reaches + history like any other. Decoding sits behind the `decode` feature and runs on demand, never as the bytes arrive: a suite that only asks whether an image went out should pay diff --git a/docs/LIMITATIONS.md b/docs/LIMITATIONS.md index 08ed2e8..f2f990b 100644 --- a/docs/LIMITATIONS.md +++ b/docs/LIMITATIONS.md @@ -107,6 +107,19 @@ CHANGELOG entry. above 4096x4096: every size in a payload is chosen by the program under test, and a sixel `!n` repeat or a declared `65535x65535` would otherwise set the allocation directly. +- **The cursor moves past an image as the declared terminal moves it**, + and only as far as the escape says. A `Graphics::Kitty` terminal moves + it for a placement whose size in cells is known: from `c=` and `r=`, or + from the pixel size and `cell_size()`. A PNG (`f=100`) placed without + `c=` and `r=`, or a placement of an image whose transmission has left + the payload log, does not move it. Whether kitty would have loaded the + image at all is not knowable here, so a placement is taken to succeed. A + `Graphics::Sixel` terminal with a `cell_size()` moves it to the image's + last text row, as xterm does with sixel scrolling on. Sixel display mode + (`CSI ? 80 h`), xterm's extra scroll for a trailing `-` on the bottom row, + and its sixel-scrolls-right mode (`CSI ? 8452 h`) are not modelled. A + `Graphics::None` terminal, the default, moves nothing: it ignores both + escapes. - **Hyperlinks are captured, not attributed to cells.** `links()` reports every `OSC 8` span with its target, its `id`, and the text it wrapped, so "did it link the right place?" is assertable — but a `Cell` does not carry From 66d55bd0a2b617cbf7c55eda7bbb71425a49df49 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Tue, 27 Oct 2026 16:26:52 +0700 Subject: [PATCH 2/2] docs(graphics): say a transmission naming no action is a=t Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- crates/termlens/src/graphics.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/termlens/src/graphics.rs b/crates/termlens/src/graphics.rs index 1043548..a662556 100644 --- a/crates/termlens/src/graphics.rs +++ b/crates/termlens/src/graphics.rs @@ -88,7 +88,8 @@ impl fmt::Display for GraphicsProtocol { serde(rename_all = "snake_case") )] pub enum GraphicsAction { - /// `a=t`: transmit the data, place it later. + /// `a=t`: transmit the data, place it later. A transmission that names + /// no action is this one, the protocol's default. Transmit, /// `a=p`: place an image transmitted earlier. Place,