Skip to content

fix(graphics): evict GPU textures when their pixels are freed#1755

Open
cantona wants to merge 2 commits into
raphamorim:mainfrom
cantona:fix/gpu-texture-leak-sweep
Open

fix(graphics): evict GPU textures when their pixels are freed#1755
cantona wants to merge 2 commits into
raphamorim:mainfrom
cantona:fix/gpu-texture-leak-sweep

Conversation

@cantona

@cantona cantona commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

fix(graphics): evict GPU textures when their pixels are freed

Problem (fixes #1591)

The renderer keeps one GPU texture per standalone image in
Renderer::image_textures. That map is only ever pruned two ways:

  1. the byte-budget LRU in render_images (evicts the least-recently-drawn
    textures once the cache exceeds its budget), and
  2. an explicit kitty eviction (evict_image_texture).

Neither reacts to an image being freed. When a graphic goes away — an
atlas graphic's last referencing cell scrolls off the grid, or a kitty
image is deleted — its CPU-side pixels are dropped from
Sugarloaf::image_data, but the GPU texture stays resident for the life
of the window.

Because atlas ids are monotonic (every sixel/iTerm2 image gets a fresh
GraphicId), a long-running session that renders many images — shell
prompts with inline thumbnails, image-heavy TUIs, repeated icat — grows
image_textures without bound. The LRU only caps it at the byte budget;
below that ceiling, dead textures accumulate indefinitely.

Fix

Sweep image_textures at the start of prepare, keeping only keys that
still exist in image_data:

self.image_textures.retain(|id, _| image_data.contains_key(id));

image_data is the source of truth for what pixels exist:

  • A texture is only ever inserted for a key that already has an
    image_data entry (the upload path reads from image_data).
  • rio drops the image_data entry exactly when the graphic is freed
    (atlas graphic's last cell dropped, or kitty image deleted).

So a texture with no matching image_data key can never be drawn again —
it is safe to drop, and doing so is the only thing that reclaims its
VRAM. An image that is merely scrolled off-screen keeps its image_data
entry (so it can re-upload when scrolled back), and therefore keeps its
texture; only genuinely-freed textures are swept.

Cost

O(live GPU textures) per frame — and this sweep is exactly what keeps
that set small, so it is bounded by real on-screen/scrollback image
count, not by session history. No measurable per-frame overhead.

Testing

  • cargo fmt --all -- --check — clean
  • cargo clippy -p rioterm --all-features — no new warnings
  • cargo test --features wgpu — all suites pass (0 failed)

Note

This PR is independent of, but shares files with, my per-terminal image
key change (#1752). It is branched on top of it to avoid a textual
conflict in renderer/mod.rs; the fix itself does not depend on that
change and can be rebased onto main directly if #1752 is not taken.

cantona added 2 commits July 22, 2026 21:08
The window-level image stores (Sugarloaf::image_data and the renderer's
per-image GPU texture cache) were keyed by a bare numeric id: kitty
images by their protocol id verbatim, atlas graphics (sixel/iTerm2) by
their GraphicId shifted above the u32 range. One window hosts many
terminals (tabs and splits) that all share these stores, but both id
spaces restart per terminal -- a kitty client in each tab numbers its
images from 1, and each terminal's atlas GraphicIds start over too. So
two tabs displaying images collided: tab B's image id 1 overwrote tab
A's, and removals could free the wrong tab's texture.

Replace the flat u64 key with ImageKey { owner, source, id }, where
owner is the originating route id and source distinguishes the atlas
and kitty id spaces. Disjointness now comes from the source
discriminant rather than a numeric shift, so atlas keys carry the raw
GraphicId. The removal queue changes from Vec<u64> to
Vec<GraphicRemoval> (Atlas(GraphicId) | Kitty(u32)) so the frontend,
which owns the route id, builds the matching key; removals are applied
before inserts so an evict-then-retransmit of the same id in one batch
keeps the fresh pixels.
The per-image GPU texture cache was only ever pruned by the byte-budget
LRU or an explicit kitty eviction. A texture whose backing image_data
entry is gone -- the graphic was freed because an atlas graphic's last
referencing cell dropped, or a kitty image was deleted -- stayed
resident for the life of the window (upstream raphamorim#1591). Atlas ids are
monotonic, so a long session that renders many sixel/iTerm2 images grows
the texture cache without bound.

Sweep the texture map at the start of prepare against image_data, which
is the source of truth: a texture is only inserted for a key present in
image_data, and rio drops that entry when the graphic is freed. An
off-screen-but-scrollable image keeps its image_data entry and so keeps
its texture; only genuinely-freed textures are dropped. The sweep is
O(live textures) -- a set this eviction itself keeps small -- so it adds
no meaningful per-frame cost.
@cantona
cantona force-pushed the fix/gpu-texture-leak-sweep branch from 4ac295c to b5819b2 Compare July 23, 2026 09:06
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.

iTerm2 image protocol broken on 0.4.2 & 0.4.3

1 participant