Skip to content

terminal: add retainable Kitty image payloads#13285

Draft
rockorager wants to merge 3 commits into
ghostty-org:mainfrom
rockorager:ref-counted-image-storage
Draft

terminal: add retainable Kitty image payloads#13285
rockorager wants to merge 3 commits into
ghostty-org:mainfrom
rockorager:ref-counted-image-storage

Conversation

@rockorager

@rockorager rockorager commented Jul 10, 2026

Copy link
Copy Markdown
Member

Kitty image lookups currently return storage-owned pixel data. The data is
only valid until the next terminal mutation, so asynchronous consumers must
copy every image generation while holding the terminal state lock.

This is particularly costly for video workloads. The renderer copies the
entire frame and performs pixel conversion while terminal parsing is excluded,
then uploads the copied data after releasing the lock.

This series gives completed Kitty images an immutable, atomically
reference-counted payload. Consumers can retain a specific (image_id, generation) while terminal state is locked and safely use those bytes after
unlocking, even if storage replaces, deletes, evicts, or shuts down the image.

The first patch adds the ownership primitive and generation-checked retain
operation to terminal storage. The second exposes it through an additive C
API while leaving the existing borrowed API and ABI unchanged. The final
patch converts the in-tree renderer to retain Kitty payloads under the state
lock, then convert and upload them after unlocking. Generic and overlay
images continue to use renderer-owned storage.

Upload-failure paths release retained data and re-retain the exact generation
on a later update. Retry wakeups are coalesced so a persistent GPU failure
cannot create a hot loop.

The series is split as follows:

  1. terminal: add retainable Kitty image payloads
  2. terminal: expose retained Kitty images through C API
  3. renderer: retain kitty images for upload

Performance

I compared patch 2, where the ref-counted storage and C API are present but
the renderer still copies, against patch 3. This isolates the renderer
handoff from the prerequisite storage changes.

The workload was five interleaved runs per revision of
notcurses-demo -c x in a 1920x1080@60 Hz GPU-backed headless Sway session.
Both revisions were ReleaseFast builds. The compositor and Ghostty used the
Mesa iris driver on Intel Arc 130V/140V graphics. perf sampled userspace
CPU at 499 Hz, hardware counters ran for 17 seconds, and RSS was sampled every
20 ms.

Median of five runs Copying renderer Retained renderer Change
renderer-thread memcpy sample share 6.25% 0.18% -97.1%
whole-process memcpy sample share 23.56% 17.92% -23.9%
task-clock 9,182 ms 8,859 ms -3.5%
cycles 14.66 B 13.47 B -8.1%
instructions 18.65 B 17.41 B -6.6%
cache references 1.079 B 0.862 B -20.1%
cache misses 714.6 M 551.7 M -22.8%
median RSS 274,824 KiB 260,444 KiB -14,380 KiB (-5.2%)
peak RSS 283,740 KiB 269,696 KiB -14,044 KiB (-4.9%)

The I/O reader's unrelated memcpy share remained effectively unchanged
(17.0% before, 17.7% after). The renderer's median share fell from 6.25% to
0.18%, which attributes the whole-process reduction to the copy removed by
this series. Both binaries had the same roughly 224 MiB idle RSS floor; the
14 MiB difference appeared during the Kitty workload.

Visible throughput was noisy because unrelated build, browser, and agent
processes were active. Frames visible at approximately 14 seconds were:

copying:  173 159 147 158 117  (median 158)
retained: 197 179 151 152  86  (median 152)

The paired median was four more displayed frames and six fewer dropped frames
with retained payloads, but later runs degraded sharply. I do not consider
that sufficient evidence of a user-visible throughput change. The copy,
memory, and process-counter reductions were repeatable across the series.

Testing

  • zig build test --summary all: 3,036 passed, 33 skipped
  • ReleaseFast before/after benchmark builds: 205/205 build steps succeeded
  • downstream Monstar integration tests: 82 passed, 1 skipped

AI Disclosure: GPT 5.6 did the implementation, Fable 5 reviewed. A few minor findings which I (the human) disagreed with regarding retries.

@ghostty-bot ghostty-bot Bot added the vt Control sequence related label Jul 10, 2026

@mitchellh mitchellh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know its a draft but just marking this red too so it doesn't get merged.

Good ideas, but I think we should clean it up in various ways.

I don't have time for a full full review so high level: I think Image having data and ImagePayload also existing is a red flag. If we're going to introduce ref-counted images (a good thing!) I think we should just own it all the way and make that the primary interface to images.

The C API: We can just break compatibility at the moment, so we should move to a full ref counted/retained model.

The renderer: Generic having a render thread is a red flag, but haven't dove into that deeper. As I mentioned in Discord, would be curious if we can update RenderState to do something like an options struct to note what features we support (so a renderer can opt out of images if it wants) and then store the ref counted images directly on it some how. Since we have to traverse the images/placements in the lock anyways it'd be better to just store it on render state.

@rockorager

rockorager commented Jul 11, 2026

Copy link
Copy Markdown
Member Author

I agree, these also introduced retry machinery which I don't think belongs in this PR.

I'll do some cleanup and see if we can get this closer.

@rockorager rockorager force-pushed the ref-counted-image-storage branch from 0212b71 to 86c3ef6 Compare July 11, 2026 12:35
Asynchronous image consumers cannot safely keep borrowed Kitty image data
after terminal storage is unlocked. Replacement, deletion, eviction, or
shutdown may otherwise free the pixels.

Make Image itself the immutable, atomically reference-counted owner of
its metadata and pixels. Store image pointers directly and release the
storage reference on every removal path, while retained references remain
valid independently.

Reserve fallible insertion resources before eviction so failed additions
preserve caller and storage ownership. Keep ordinary lookups and the
existing C image lookup borrowed in this commit.
Borrowed Kitty image handles cannot safely cross into asynchronous render
or raster work because terminal mutation may invalidate them.

Make GhosttyKittyGraphicsImage an owned reference to Image itself. Image
lookup now retains the handle, retain increments the same reference count,
and free releases it without a wrapper allocation or caller-provided
allocator.

Keep image metadata and pixel getters read-only, with pixel pointers valid
for the lifetime of their image handle.
@rockorager rockorager force-pushed the ref-counted-image-storage branch from 86c3ef6 to 472367c Compare July 11, 2026 13:07
Kitty image preparation copied terminal-owned pixels and converted them
while the renderer state mutex excluded terminal parsing. Large images and
video frames therefore extended the critical section with a full copy and
format conversion.

Add an opt-in Kitty graphics snapshot to RenderState. Retain images and
copy placement geometry while the terminal is locked, then update renderer
image state under the draw mutex after releasing the terminal lock. Only
placed images are prepared for upload.

Track retained and renderer-owned pending data explicitly, and release the
retained image after synchronous texture creation consumes it. Failed
uploads remain pending for a later natural draw without a retry scheduler.
@rockorager rockorager force-pushed the ref-counted-image-storage branch from 472367c to 2a4db8d Compare July 11, 2026 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

vt Control sequence related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants