Skip to content

feat(terminal): complete the kitty graphics protocol (unicode placeholders, animation, delete specifiers) - #331

Open
TranscriptionFactory wants to merge 11 commits into
zerx-lab:mainfrom
TranscriptionFactory:feat/kitty-graphics-protocol
Open

feat(terminal): complete the kitty graphics protocol (unicode placeholders, animation, delete specifiers)#331
TranscriptionFactory wants to merge 11 commits into
zerx-lab:mainfrom
TranscriptionFactory:feat/kitty-graphics-protocol

Conversation

@TranscriptionFactory

Copy link
Copy Markdown

补全 kitty graphics 协议:Unicode 占位符(U=1)、动画、全部删除指定符、规范回复。基于 main @ 5d87445,11 个 commit 零冲突,65 个新单测全绿。

Summary

Completes the kitty graphics protocol implementation. All 11 commits cherry-pick cleanly onto main @ 5d87445 with zero conflicts.

  • Unicode placeholders (U=1) — virtual placements rendered from U+10EEEE placeholder cells: 297-entry row/column diacritic table (verified against kitty's gen/rowcolumn-diacritics.txt), image ids from the cell fg color (indexed + 24-bit + third-diacritic high byte), consecutive cells batched into one source-rect draw per strip. Enables graphics through tmux passthrough, ratatui-image, timg -pk.
  • Animationa=f frame transmission (gap via z=), a=a play/stop and gap edits; iTerm-protocol GIFs also animate now (previously decoded, then dropped by the renderer). Frame compositing (x=/y=/c=, a=c) is deferred with spec-correct ENOTSUPP: replies. Frames are capped per image (1024 frames / 256 MB) so a runaway a=f stream cannot grow memory without bound.
  • Delete specifiers — the full set d=a/i/n/c/p/q/x/y/z/r plus the uppercase data-freeing forms. Fixes a latent bug where an unknown d= value fell through to DeleteAll; unknown specifiers now reply EINVAL:.
  • Spec-correct repliesENOENT:/EINVAL:/ENOTSUPP:/EBADF: error prefixes (previously Rust Debug strings), p=/I= echo, quiet-mode (q=1/q=2) semantics, and support queries (a=q) answered even when a block hasn't reached preexec (they were silently dropped by the header grid before, so probes at the prompt concluded graphics was unsupported).
  • Renderingdraw_image gains source-rect (UV sub-rect) sampling on both Metal and wgpu backends; existing callers unchanged. Animated grid images schedule repaints via repaint_after.
  • Correctness — fixes a release-mode hang/OOM: an extreme-aspect-ratio image drove the placement height to zero cells and 0usize - 1 wrapped to usize::MAX in the newline loop, turning it into an unbounded line-append triggerable by cat-ing a crafted file.
  • Feature gatingiterm_images moves from an unconditional build.rs cfg injection to a Cargo default feature; Windows behavior held exactly constant (no images).

Testing

  • 65 new unit tests (45 protocol tests in kitty_test.rs, 20 decoder tests in unicode_placeholder.rs) — all green on this branch against this repo's main; cargo check -p warp clean.
  • Development went through three independent adversarial review rounds; every confirmed finding is fixed in the fix(terminal): address wave-N kitty review findings commits (reply spec compliance, virtual-placement lifecycle on delete, memory bounds, render-pass early-outs).
  • Note: the Metal shader change (2 lines in shaders.metal + shader_types.h) is compile-verified in our fork's CI; the wgpu path is covered by the existing resources_tests.

Known limitations (documented in code)

  • Placement ids are not recoverable from placeholder cells (this codebase's Cell has no underline-color storage), so an image shown through several simultaneous virtual placements renders one of them deterministically.
  • Frame compositing and d=f/d=F frame deletion reply ENOTSUPP:; animation loop counts (v=) parse but play as infinite.
  • q=2 does not suppress query replies (deliberate, commented: a support probe should never go unanswered).

Kitty image rendering only worked because build.rs force-injected
`cargo:rustc-cfg=feature="iterm_images"` on every non-Windows target, so the
Cargo feature and the actual gate disagreed. Declare `iterm_images` as a real
default feature, drop the build.rs cfg injection, and make the platform policy
explicit at the flag registration site with
`cfg(all(not(windows), feature = "iterm_images"))` — Windows behavior is
unchanged (still no inline images).

The renderer gated image drawing on ITermImages alone in two duplicated
places; both now go through `terminal_images_enabled()`, which is true for
either protocol's flag since they share the same image map.

(cherry picked from commit 7cf4e57)
Animated inline images (e.g. a GIF sent over the iTerm protocol) decoded to
AnimatedBitmap and then hit a warn-and-drop arm in render_image, so nothing was
drawn at all. Resolve the current frame via AnimatedImage::get_current_frame,
draw it like a static image, and ask the presenter to repaint once the frame's
remaining delay elapses — the same pattern warpui_core's image element uses.

Playback phase comes from a module-level ANIMATION_EPOCH, shared by every
animated image; a GIF therefore joins mid-loop instead of at frame zero, and
the u32 millisecond cast wraps after ~50 days of uptime, matching the existing
behavior in crates/warpui_core/src/elements/image.rs.

(cherry picked from commit 1015f46)
scene::Image now carries a source_uv rect in unit UV space, and the new
Scene::draw_image_with_source lets callers sample a sub-rectangle of an
image asset instead of the whole thing. Both the Metal and wgpu image
pipelines map the unit-square vertex positions onto that sub-rectangle
when computing texture coordinates.

Existing draw_image behavior is unchanged: it passes the full-rect
source_uv (origin (0, 0), size (1, 1)), and icons likewise sample their
whole asset, so every current call site renders exactly as before.

(cherry picked from commit 66f5b53)
Fixes latent panics and spec violations in the kitty graphics protocol:
the image placement arms subtracted the cursor column from the terminal
width and computed `height_cells - 1` unchecked, so a cursor past the
last column or a zero-height placement underflowed in release-mode
wrapping/debug panic; DisplayStoredImage was also missing the
zero-image-size early return that the other arms have. Error replies
emitted Rust `Debug` strings instead of the protocol's ENOTSUPP/ENOENT/
EINVAL/EBADF codes, replies never echoed `p=<placement_id>`, and query
(`a=q`) responses were incorrectly silenced by quiet mode.

Next: broader kitty coverage (chunked transmission, deletion, z-index).
(cherry picked from commit bafabdb)
Replaces the two-case `d=` handling with the protocol's full set: a/i/n/c/p/q/
x/y/z/r, plus f reported as ENOTSUPP until animation frames are stored. The
non-positional specifiers are applied to every grid by the terminal model; the
positional ones are applied by the grid handler, which is the only place with a
cursor, a viewport and the block's own placement map.

Fixes a fallthrough bug where an unrecognized `d=` letter fell back to
`DeleteAction::default()`, i.e. DeleteAll, so a single typo erased every image
on screen. Unknown letters now parse to `DeleteAction::Unknown` and are answered
with an `EINVAL:` error reply, deleting nothing.

Supporting changes: `ImagePlacementData` records `width_cells` so a placement's
footprint can be tested against a cell, column or row; `ImageMap` gains the
matching query and bulk-eviction methods; `I=` (image number) is parsed and kept
on the stored metadata so `d=n` can resolve a number to an image id; `x=`/`y=`
are parsed as delete coordinates. Uppercase specifiers keep freeing the image
data, now including the positional forms.

(cherry picked from commit 3ea16e2)
Accept the kitty graphics protocol's U=1 placement mode instead of
rejecting it with EINVAL, and draw the images that its placeholder cells
stand in for.

A U=1 placement has no anchor in the grid, so it does not go through the
image map and must not move the cursor. It is recorded as a
VirtualPlacement on the image's own KittyImageMetadata, keyed by
placement id, which also means it is dropped automatically when the image
is. Its rows/cols stay unresolved and are re-resolved against the cell
size on every frame, so changing the font size re-tiles the image rather
than stretching it.

Placeholder cells (U+10EEEE) carry the image id in their foreground
colour and the image row/column as combining diacritics from kitty's
297-entry rowcolumn-diacritics table, reproduced here and verified
against kitty's source. A new render pass scans the visible cells --
rather than get_image_ids_in_range, which is empty under displayed-output
filtering -- and batches each horizontal strip of an image into a single
draw_image_with_source call. Cells that omit trailing diacritics inherit
the row and continue the column of the cell to their left, so both the
fully-specified and the shorthand encoding work.

Placeholder cells are skipped in both glyph paths, which keeps their
per-cell-unique grapheme strings out of CellGlyphCache and the text
layout cache.

Known limitation: this fork's Cell has nowhere to store an underline
colour, so a cell's placement id cannot be recovered. An image with
exactly one virtual placement uses that placement; images shown through
several simultaneous virtual placements are not distinguished.

(cherry picked from commit 6586a8f)
Omit p=0 from replies (not a valid placement id), answer support
queries before the block delegate so probes at the prompt are never
silently dropped, document the q=2 query deviation, and add regression
tests for the aspect-ratio underflow and pre-preexec query paths.

(cherry picked from commit fe91b5a)
Skip the per-cell scan (and its layer) when no virtual placements
exist, clamp client-supplied r=/c= like the anchored path to bound
resize cost, degrade to the lowest-numbered placement instead of
rendering nothing when an image has several, and mask the third
diacritic before shifting it into the image id.

(cherry picked from commit a3633cf)
…ontrol)

Implements `a=f` full-canvas frame transmission (gap from `z=`, frame
replacement via `r=`) and `a=a` playback control (`s=1` stops, `s=2`/`s=3`
play, `r=`+`z=` edits a frame's gap), with frames kept on the image's
metadata and delivered to the view as an `ImageType::AnimatedBitmap` so the
existing animated-image renderer plays them. A stopped animation is
delivered as its first frame alone.

Deferred, and answered with an `ENOTSUPP:` reply rather than silently
mis-rendered: frame compositing (nonzero frame `x=`/`y=`, `c=` base-frame
copy, a frame whose size differs from the canvas), editing frame 1's pixels,
the `a=c` compose action, and `d=f`/`d=F` frame deletion. Loop counts (`v=`)
are parsed and treated as infinite.

(cherry picked from commit 1007b11)
Lowercase deletes (d=a/i/n/z) now clear virtual (U=1) placements,
which previously survived everything short of freeing the image;
uppercase deletes evict every placement of a freed image instead of
leaving blank holes in other grids; replies echo I= and a message
carrying only I= now gets a reply naming the assigned id.

(cherry picked from commit 2c37f5f)
Cap animation frames per image (count + bytes, ENOTSUPP beyond), skip
asset rebuilds on no-op a=a messages, error on frame edits past the
end instead of appending, accept c=0 (no base frame), resolve I= to
the newest image for display/frame/control messages, ignore the I=0
unset default, restrict the orphan sweep to kitty placements, and
reject out-of-range third diacritics instead of masking them.

(cherry picked from commit 3080a67)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant