Skip to content

fix(graphics): repair chafa rendering and mpv kitty streaming - #8

Merged
sanohiro merged 12 commits into
mainfrom
fix/kitty-chunked-base64
Apr 17, 2026
Merged

fix(graphics): repair chafa rendering and mpv kitty streaming#8
sanohiro merged 12 commits into
mainfrom
fix/kitty-chunked-base64

Conversation

@sanohiro

Copy link
Copy Markdown
Owner

Summary

Four graphics-protocol bugs found while debugging issue #7 (mplayer fbdev2 playback) by exercising chafa -f sixel, chafa -f kitty, and mpv --vo=kitty on bcon.

  • Kitty graphics: per-chunk base64 decode — chafa emits each chunk as a self-contained base64 unit with its own = padding. bcon was concatenating raw base64 across chunks and skipping = during decode, which leaked 2 leftover bits per chunk into the next, producing 600 extra bytes for a 640×480 RGBA image (2400 chunks × 2 bits). Now each chunk is decoded on arrival. Regression test added.
  • Kitty graphics: gate responses on client-provided id — the protocol says terminals must not emit responses unless the client set i= or I=. chafa sets neither, so bcon's OK reply was leaking into the shell input as visible text. Introduced Terminal::maybe_send_kitty_response to collapse seven nearly-identical call sites into one gated helper.
  • Grid: keep the cursor out of placed image areaplace_image advanced the cursor onto the last row occupied by the image (both the scroll and non-scroll paths clamped to rows - 1). The very next put_char then called remove_images_at_cell, discarding the placement we had just created — observed as the second chafa -f sixel run rendering nothing. Unified the branch so the cursor always lands one row past the image, scrolling additionally if needed.
  • Grid: dedupe fully-covered overlay placementsmpv --vo=kitty streams identically sized overlay frames (a=T,C=1) at the same cell without ever issuing a delete. bcon accumulated one placement and one ~3MB image per frame, stalling within ~4 seconds because the PTY read loop couldn't drain. place_image now returns the ids of any overlay placements fully covered by the new one at the same z-index, and the Kitty handler drops those ids from the image registry so GPU textures and RGBA buffers are reclaimed every frame.

Also marks sixel-decoded images with dirty_image_ids.push(img_id) so the renderer actually uploads the new texture — previously the sixel decode completed but no GPU upload ever ran, so the placement rendered as a solid black rectangle.

Test plan

  • cargo test --release terminal::kitty — new chunked-base64 regression test passes
  • chafa -f kitty test.png displays the image with no OK reply leaking into the shell
  • chafa -f sixel test.png displays the image; running it a second time still displays
  • mpv --no-audio --vo=kitty test.mp4 plays to completion without stalling (throughput is still heavy at full resolution; lower with --vf=scale=480:-2,fps=15 for smoother playback)
  • mpv --no-audio --vo=sixel test.mp4still broken (DCS parser leaks sixel bytes as printable text mid-stream). Static sixel and Kitty graphics video are unaffected. Tracked as follow-up.

Known follow-ups (not in this PR)

  • sixel streaming from mpv --vo=sixel breaks the DCS parser mid-playback
  • sixel palette index 255 collides with the decoder's "unpainted" sentinel — image pixels of color 255 render as transparent
  • Kitty graphics throughput optimization (SIMD rgb_to_rgba, faster base64, shared-memory t=s path) to make full-resolution video smoother

sanohiro and others added 12 commits April 15, 2026 23:34
Four related bugs surfaced while testing `chafa -f sixel`,
`chafa -f kitty`, and `mpv --vo=kitty` on bcon:

1. Kitty graphics: decode chunked base64 per chunk
   chafa emits each chunk as a self-contained base64 unit (often ending
   in `=` padding). bcon used to concatenate raw base64 across chunks
   and skip `=` during decode, which leaked the trailing 2 bits of each
   chunk's final group into the next chunk and produced 600 extra bytes
   for a 640x480 RGBA image (2400 chunks x 2 bits). The decoder now
   decodes each chunk on arrival and accumulates the decoded bytes.
   Added a regression test reproducing chafa-style chunking.

2. Kitty graphics: gate responses on client-provided image id
   The protocol says terminals must not emit responses unless the
   client identified the command with `i=` or `I=`. chafa sets
   neither, so bcon's "OK" reply was leaking into the shell input as
   visible text (`^[_Gi=1;OK^[\` after every image). Introduced
   `Terminal::maybe_send_kitty_response` to gate all seven response
   sites in one place.

3. Grid: keep the cursor out of placed image area
   `place_image` advanced the cursor to the last row occupied by the
   image (clamped to `rows - 1` in both the scroll and non-scroll
   paths). The very next `put_char` on that row then triggered
   `remove_images_at_cell`, which discarded the placement we had just
   created — observed as the second `chafa -f sixel` run rendering
   nothing at all. Unified the branch so the cursor always lands one
   row past the bottom of the image, scrolling additionally if needed.

4. Grid: dedupe fully-covered overlay placements on replacement
   `mpv --vo=kitty` streams identically sized overlay frames
   (a=T,C=1) at the same cell position without ever issuing a delete.
   bcon accumulated one placement and one 3MB image per frame,
   stalling within ~4 seconds because the PTY read loop could not
   drain fast enough. `place_image` now returns the ids of any
   overlay placements fully covered by the new one (same z-index) and
   the Kitty handler drops those ids from the image registry so GPU
   textures and RGBA buffers are reclaimed every frame.

Also marks sixel-decoded images with `dirty_image_ids.push(img_id)`
so the renderer uploads the new texture — previously the sixel
decode completed but no GPU upload ever ran, leaving a black rect.

Known remaining issue: `mpv --vo=sixel` streaming still breaks the
DCS parser mid-playback (sixel data leaks as printable text); static
sixel and Kitty graphics video are unaffected. Tracking separately.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
vte 0.13's DCS state machine intermittently fails to enter
DcsPassthrough under streaming sixel load (mpv --vo=sixel),
causing entire frames of sixel data to leak through as printed
text and fill the screen with garbage characters.

Root cause: despite clean PTY data (no stray control bytes,
verified via raw PTY dump), vte occasionally processes ESC P q
sequences in Ground state instead of transitioning to
DcsPassthrough. The exact trigger is unclear but appears related
to read-boundary timing under high throughput.

Fix: handle DCS sixel entirely in bcon's own state machine
(extending the existing APC handler) instead of relying on vte.
When ESC P is detected, the new DcsParams/DcsData/DcsEscape
states route sixel bytes directly to SixelDecoder.push() without
creating a Performer or touching vte. Non-sixel DCS sequences
(DECRQSS, XTGETTCAP) are replayed to vte for standard handling.

Additional fixes in this commit:
- Sixel images placed at z=-1 so they survive text overwrites
  from mpv's interleaved status-line writes (remove_images_at_cell
  skips z<0 placements, matching xterm/foot behavior)
- Non-overlay placement dedup by (z, width, height) to prevent
  accumulation under streaming; reuses the first frame's position
  so cursor drift from status text doesn't shift later frames

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mpv slightly varies frame dimensions between sixel frames (e.g.
96x36 vs 99x36), so the exact size match prevented position reuse
and let cursor-polluted positions through. Match by z-index alone
for non-overlay placements.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sixel DCS started/complete, place_image, Kitty APC/decode, and
texture upload logs fire every frame during video streaming.
Under RUST_LOG=info these add measurable write(2) overhead per
frame. Move to trace! so they only appear with RUST_LOG=trace.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reduces read(2) syscall count by 16x during streaming sixel/kitty
video. A 315KB sixel frame previously needed ~77 reads; now ~5.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When in DcsData state, bypass the outer match/Option check per byte
and loop directly feeding bytes to SixelDecoder.push(). Only break
out when ESC or 0x9C is found. This eliminates ~315K match + enum
pattern-match operations per sixel frame.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
draw_sixel: single ensure_size per RLE run instead of per pixel,
skip pattern==0 (no-op), direct pixel buffer indexing without
set_pixel function call overhead.

finish (RGBA conversion): pre-build 256-entry RGBA LUT, then
direct slice write instead of per-pixel extend_from_slice.
Eliminates Vec capacity checks and branch per pixel.

These two paths dominate CPU time during sixel video streaming
(17s user vs kitty's 5s for same content).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
push, handle_normal, handle_color, handle_rle, handle_raster_attr,
draw_sixel, ensure_size — all marked #[inline(always)].
Called 315K+ times per frame, function call overhead dominates.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The z-index-only dedup with position reuse was designed for mpv
sixel streaming (z=-1) where cursor drift makes positions
unreliable. But it also fired for z=0 Kitty images (yazi), causing
old images to reuse stale positions and persist on screen when
navigating to different files.

Split the logic: z<0 uses aggressive dedup with position reuse
(sixel video), z>=0 uses conservative fully-covered dedup without
position reuse (Kitty/static sixel).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The aggressive z<0 dedup removed ALL z=-1 placements on every new
sixel image, breaking consecutive `chafa -f sixel` and `img2sixel`
calls: the second image superseded the first instead of stacking
below it. Now only supersede placements whose row ranges actually
overlap with the new image — mpv's same-position frames still
dedup, but vertically stacked images coexist.

Added test: sixel_consecutive_images_stack_vertically.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
yazi uses sixel (not Kitty) for image preview. With z=-1, text
writes couldn't remove the image (remove_images_at_cell skips
z<0), so images persisted when navigating to text files.

Changed sixel back to z=0 for both parser.rs and mod.rs paths.
mpv --vo=sixel works with --really-quiet (no status text to
trigger removal). Updated tests and removed debug logs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
New features:
- mpv --vo=kitty video playback at near-realtime speed
- mpv --vo=sixel video playback support
- Streaming sixel/kitty frame deduplication and position tracking

Bug fixes:
- Kitty graphics: decode chunked base64 per chunk (fixes chafa)
- Kitty graphics: gate responses on client-provided image id
- Sixel: bypass vte DCS parser for reliable streaming
- Grid: cursor lands below placed images, not inside
- Grid: overlay dedup for mpv --vo=kitty streaming
- Grid: sixel dedup with row-overlap check

Performance:
- PTY read buffer 4KB → 256KB
- Sixel decoder: single ensure_size per RLE run, RGBA LUT
- Per-frame logs downgraded to trace level
- Tight inner loop for DCS sixel data (no Performer overhead)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sanohiro
sanohiro merged commit c312ebd into main Apr 17, 2026
1 check passed
@sanohiro
sanohiro deleted the fix/kitty-chunked-base64 branch April 17, 2026 04:42
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