fix(kitty-graphics): stop inline images blanking in panes (re-emit eviction + large-frame compression)#1175
Closed
abhi-easygo wants to merge 1 commit into
Closed
Conversation
Two bugs in the virtual-placeholder re-emit path blanked inline images in herdr panes while the same images rendered fine under tmux and bare Ghostty. 1. Mid-frame eviction of a still-referenced image (ogulcancelik#1167). A source processed earlier in encode_graphics_update could delete a host image that a later same-frame placement still referenced, because the guard only checked already-recorded sources. With the frame-start data filter leaving the later placement's data empty, it then failed to re-upload and the placeholder cells stayed blank until the pane was recreated. Guard both deletion paths with a whole-frame reference set (frame_host_ids) and add an empty-data early-continue on the replaced-image branch. 2. Uncompressed re-emit inflated large frames ~96x. Stored images are decoded to raw RGBA and were re-transmitted uncompressed (f=32), turning a ~278 KB PNG into ~25.6 MB and overwhelming the outer terminal on large diagrams. Re-encode raw pixels back to PNG (f=100) via the png crate, the compact form bare Ghostty and tmux already deliver. Fixes ogulcancelik#1167.
Collaborator
|
Hi @abhi-easygo, thanks for your interest in contributing! New contributors need maintainer approval before opening PRs. This keeps review time focused on accepted work and avoids wasted effort. Herdr is opinionated about how it should look, feel, and work. Feature requests, ideas, questions, contribution proposals, and product-direction checks belong in Discussions, not issues. Next steps:
A discussion, issue, branch, or proposed implementation does not reserve the work and does not mean the PR path is approved. This PR will be closed automatically. See https://github.com/ogulcancelik/herdr/blob/master/CONTRIBUTING.md for more details. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two bugs in the
kitty_graphicsvirtual-placeholder re-emit path that caused inline images to go blank in herdr panes while the same images render correctly under tmux (allow-passthrough) and in bare Ghostty. Both live insrc/kitty_graphics.rs; the display-placement and delete paths and the FFI/vendored lib are untouched.Fixes #1167.
Bug 1 — mid-frame eviction of a still-referenced image (the #1167 blank-after-churn)
release_superseded_source_image(and the replaced-image branch inencode_graphics_update) could emita=d,d=Ion a host image mid-frame while a later placement in the same frame still referenced it. The guard only checked sources recorded so far (sources.values()), not all placements in the current frame. Combined with the frame-start data-filter (which leaves a later placement'sdataempty because the image was already uploaded), the later placement then couldn't re-upload (encode_upload_imagerefuses empty data →continue), leaving blank placeholder cells that persisted until the pane was recreated.Two content-hash-identical sources are enough to trigger it: when one moves to fresh content (superseding the shared image) and the other adopts the shared content in the same frame, the shared image gets deleted before the second placement claims it.
Fix: compute
frame_host_ids(host-ids referenced by all current-frame placements) up front, and never delete a host id still in that set. Applied torelease_superseded_source_image; the replaced-image branch (which deletes and re-uploads its own id) instead gets an empty-data early-continueso it can't delete-then-fail-to-upload.Bug 2 — uncompressed re-emit inflates large frames ~96×
herdr installs a PNG decoder into the vendored lib, so stored images are decoded to raw RGBA.
encode_upload_imagethen re-transmitted that raw RGBA uncompressed (f=32). A ~278 KB PNG diagram becomes ~25.6 MB of RGBA (~34 MB base64) on every fresh upload — small diagrams stay small and render, but large ones overwhelm the outer terminal's per-transmission handling and silently drop. tmux and bare Ghostty forward the original compact PNG untouched, so they never hit this.Fix: re-encode raw pixels back to PNG with the
pngcrate (already a direct dependency, used for decode) and upload withf=100— the exact compact form bare Ghostty/tmux already deliver. Non-RGBA formats and encode failures fall back to the existing raw path.Isolation evidence
Same Neovim + snacks.nvim image module + Ghostty for all three:
allow-passthrough on)kitty_graphics=true(before)The re-emit round-trip is the only path that touches image bytes, which is why both bugs are herdr-specific.
Verification
cargo build --release(zig@0.15): success.cargo test --bin herdr kitty_graphics: 23 passed, 0 failed, including:superseded_image_survives_when_later_placement_adopts_it_same_frame(new, Bug 1 regression)encode_upload_image_reencodes_rgba_as_png(new, Bug 2: assertsf=100control, nof=32/s=, payload < 1/10 raw size)shared_host_image_survives_...,replaced_image_content_deletes_..., etc.)Confirmed end-to-end on macOS 26 / Ghostty 1.3.1: a large flowchart that previously blanked in a herdr pane now renders, and small diagrams survive edit/reload/float-open-close/buffer-switch churn.
Notes