Repository navigation
feat: support multi-location placement fan-out for one transmitted image - #51
Merged
Merged
Conversation
Every real placement command now carries a distinct, never-reused placement id (`p=`) instead of the old fixed `terminal.PLACEMENT_ID = 1`. This lets one transmitted image id carry several concurrent placements: show()ing the same (path, mtime) a second time while the first is still live now fans out onto the existing id under a fresh placement id instead of transmitting a redundant copy under a new image id. - terminal.lua: build_transmit/build_placement/build_delete take a caller- supplied placement_id; build_delete's `p=` scopes a delete to one placement instead of the whole id. - renderer.lua: cache entries track active_placements (a count) instead of a single active boolean; find_reusable_entry (renamed from acquire_idle_entry) tries an idle entry first, then falls back to fanning out onto any active one. destroy_handle only frees an id's terminal-side data once the last handle referencing it is gone, regardless of the caller's free_data intent. The Ghostty resize-retransmit path now migrates every handle sharing a stale id together (retransmit_and_place_group), fixing a correctness gap the old per-handle version would have hit once ids could be shared. - docs/spec/kitty-graphics.md and docs/spec/renderer-placement.md updated to match; docs/manual-testing.md gets a fan-out checklist item. Closes #10
destroy_handle's fan-out-sibling-still-shares-the-id branch (issue #10) returned without queuing a delete retry whenever free_data was requested, even when the caller was M.show()'s own mid-session failure path rather than VimLeavePre. If the terminal dropped that one-shot scoped delete, the stray placement had no future redraw pass left to revisit it and stayed orphaned for as long as the sibling handle stayed alive. Add an explicit opts.shutting_down flag to destroy_handle, set only by on_vim_leave_pre and the test-only M._reset(). The fan-out branch now skips the retry queue only when shutting_down is true; M.show()'s failure paths get the same bounded pending_deletes retry every other placement-scoped delete in this file already has. Update docs/spec/renderer-placement.md's Lifecycle section, which documented the old reasoning, and add a regression test covering retransmit_and_place_group's multi-handle migration path (previously only exercised with a single handle). Found in code review of #10's multi-location placement fan-out.
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
The implementation of multi-location placement fan-out is well-designed and comprehensive. The code correctly handles:
- Placement ID allocation with an ever-incrementing counter that avoids reuse conflicts
- Cache entry management tracking active_placements to support fan-out
- Scoped delete operations using placement_id to prevent colliding with sibling placements
- Ghostty-specific group migration that atomically moves all handles sharing a stale ID
- Proper retry mechanisms for both destroy-path deletes and Ghostty retransmits
The extensive test coverage demonstrates the feature works correctly across various scenarios including fan-out, cache reuse, and terminal-specific behaviors. No defects blocking merge were identified.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
This was referenced Aug 22, 2026
hisanari-dev
added a commit
that referenced
this pull request
Oct 4, 2026
PR #51 (issue #10, multi-location placement fan-out) added the underlying primitive but never updated README/doc/blit.txt, despite docs/review-checklist.md's Tests & Docs category requiring vimdoc and README updates for user-facing behavior. Document the one concrete, user-actionable consequence: calling show() again with the same path reuses the already-transmitted image data and is the way to place the same image into a second window after :split/:vsplit today, since show() still binds one handle to one window and doesn't auto-detect splits.
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
1 participant
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.
📦 Pull Request
Description
Adds multi-location placement fan-out for a single transmitted image:
terminal.luanow carries a caller-suppliedp=(placement id) on every placement/transmit/delete command instead of a single fixed constant, andrenderer.luaallocates a distinct, never-reused placement id per handle so several handles can share one transmitted image id — each with its own live, independently addressable/deletable placement — without re-transmitting pixel data or clobbering a sibling's placement.Also includes a follow-up fix (from code review of this branch):
destroy_handle's fan-out-sibling-still-shares-the-id delete path now gets the same bounded delete retry as every other placement-scoped delete inrenderer.lua, except when the caller is truly shutting down (VimLeavePre/test_reset()). Previously a mid-sessionM.show()failure tearing down a fan-out handle skipped the retry, risking an orphaned placement if the terminal dropped that one delete.Related Issue
Closes #10
Type of Change
Checklist
makelocally (stylua --check,selene, tests) and it passes.docs/spec/memo if protocol or terminal-detection behavior changed.doc/blit.txtand/or README if this changes the public API.feat:,fix:, etc.).