From 27c540df709a5e0e646ee76751059c271ed7f849 Mon Sep 17 00:00:00 2001 From: Hisanari Kikuchi Date: Sun, 16 Aug 2026 12:23:22 +0900 Subject: [PATCH 1/2] feat: support multi-location placement fan-out for one transmitted image 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 --- docs/manual-testing.md | 7 + docs/spec/kitty-graphics.md | 56 +++-- docs/spec/renderer-placement.md | 164 +++++++++------ lua/blit/renderer.lua | 363 ++++++++++++++++++++++++-------- lua/blit/terminal.lua | 75 +++++-- tests/test_renderer.lua | 113 ++++++++-- tests/test_terminal_escape.lua | 91 ++++---- 7 files changed, 627 insertions(+), 242 deletions(-) diff --git a/docs/manual-testing.md b/docs/manual-testing.md index 240d906..1e98521 100644 --- a/docs/manual-testing.md +++ b/docs/manual-testing.md @@ -27,6 +27,13 @@ WezTerm (and Ghostty when available) before tagging a release. stuck at its old position overlapping buffer text (issue #18). - [ ] Scrolling the image fully out of view then back in re-displays it without a visible retransmission delay (cache hit). +- [ ] `show()` the same PNG path twice at two different buffer lines + without `clear()`-ing the first in between (issue #10, multi-location + fan-out). Both copies must render correctly and simultaneously — not + just the second one, and not the first one moved/disappeared. Then + `clear()` only the first handle: the second must remain visible, + unaffected. Finally `clear()` the second handle too and confirm no + stray pixels remain from either. - [ ] Scrolling so the image is cut off at the top or bottom of the window shows a **cropped** slice of the image (the still-visible portion, correctly sized to the remaining cell span) instead of a blank gap or diff --git a/docs/spec/kitty-graphics.md b/docs/spec/kitty-graphics.md index eda1c9f..ea1f53f 100644 --- a/docs/spec/kitty-graphics.md +++ b/docs/spec/kitty-graphics.md @@ -29,7 +29,7 @@ Every command is an APC (Application Program Command) escape sequence: | `i` | image id | one of blit's reserved range, see below | | `q` | quiet | `2` (suppress all responses) always, see "Response handling" below | | `m` | more chunks | `1` (more chunks follow) / `0` (last chunk) | -| `p` | placement id | always `1` (blit's single fixed placement id — see "Placement" below) | +| `p` | placement id | a per-handle id, distinct across every concurrently-live placement — see "Placement" below | | `c`, `r` | placement columns/rows | shrink to the visible cell span when a placement is partially clipped, see "Source-rectangle cropping" below | | `x`, `y` | source rectangle pixel offset (left/top) into the transmitted image | present only when a placement is partially clipped; see "Source-rectangle cropping" below | | `w`, `h` | source rectangle pixel size | present only when a placement is partially clipped; see "Source-rectangle cropping" below | @@ -53,23 +53,34 @@ Every command is an APC (Application Program Command) escape sequence: ## Placement -- `a=p,i=,p=1` redisplays an already-transmitted image without resending - pixel data. This is how blit satisfies the performance rule that - scroll/resize redraws must reuse the existing id rather than +- `a=p,i=,p=` redisplays an already-transmitted image + without resending pixel data. This is how blit satisfies the performance + rule that scroll/resize redraws must reuse the existing id rather than re-transmitting. -- `p=1` (blit's fixed placement id, `terminal.PLACEMENT_ID`) is **always** - sent, on every placement command — both the initial `a=T` transmit+display - and every later `a=p` reposition. If `p=` is omitted, the terminal creates - a brand-new placement on every call instead of moving the existing one; - since blit repositions on every debounced `WinScrolled`/`WinResized` - redraw, this silently accumulates stacked "ghost" placements at each prior - screen position — visible as partial/duplicated image fragments while - scrolling, until a `a=d,d=i` delete (see below) clears all of them at - once. Reusing the same `i=` **and** `p=` pair on every call makes each - `a=p` update that one placement in place instead. blit never needs more - than one placement id per image id: `renderer.lua`'s cache only ever marks - a given image id "active" for a single handle at a time, so a constant - `p=1` can never collide with a second live placement of the same id. +- `p=` (a caller-supplied placement id, `blit.terminal.PlacementOpts.placement_id` + in `terminal.lua`) is **always** sent, on every placement command — both + the initial `a=T` transmit+display and every later `a=p` reposition. If + `p=` is omitted, the terminal creates a brand-new placement on every call + instead of moving the existing one; since blit repositions on every + debounced `WinScrolled`/`WinResized` redraw, this silently accumulates + stacked "ghost" placements at each prior screen position — visible as + partial/duplicated image fragments while scrolling, until a `a=d,d=i` + delete (see below) clears all of them at once. Reusing the same `i=` + **and** `p=` pair on every call makes each `a=p` update that one placement + in place instead. +- **One image id can have several concurrent placements (issue #10, + "Multi-location placement fan-out").** Earlier versions of blit sent a + single fixed `p=1` on every call, relying on `renderer.lua`'s cache never + marking a given image id "active" for more than one handle at a time. That + constraint is gone: `renderer.lua` now allocates a distinct, never-reused + placement id per handle (`alloc_placement_id()`, an ever-incrementing + session-lifetime counter — see `docs/spec/renderer-placement.md`'s + "Transmission cache" section), so a second `show()` of the same + `(path, mtime)` while the first is still live reuses the SAME image id + under a DIFFERENT placement id — a second, independent placement — instead + of transmitting a redundant copy under a new image id. Each placement is + then addressed, repositioned, and torn down independently by its own + `(id, placement_id)` pair. - Optional placement keys, in the fixed order blit emits them: `p=` (always present when placing), `x=`, `y=`, `w=`, `h=` (source rectangle, only when cropped — see "Source-rectangle cropping" below), `c=`, `r=` (target cell @@ -144,6 +155,17 @@ clip-amount math (`compute_clip`) and the pixel-space conversion - `a=d,d=i,i=` deletes the visible placement(s) for one image id blit owns; `d=I` additionally frees the terminal's stored pixel data for that id. +- `p=`, when supplied alongside `d=i`, restricts the delete to + that ONE placement of the id rather than every placement blit has made for + it — required now that one id can have several concurrent placements (see + "Placement" above). `renderer.lua` always includes it except for the + whole-id teardown case: freeing a handle's placement while the id might + still be shared by a sibling handle (`terminal.build_delete(id, { + placement_id = ... })`), vs. freeing the id's stored data entirely once no + handle references it anymore (`terminal.build_delete(id, { free_data = + true })`, no `p=`, since every placement is being torn down together at + that point anyway) — see `docs/spec/renderer-placement.md`'s Lifecycle + section for the full decision. - blit **never** emits `d=a` (delete every image on the terminal, including ones placed by other plugins like image.nvim/snacks.image). Every deletion path in `terminal.lua` is scoped to a single caller-supplied id. diff --git a/docs/spec/renderer-placement.md b/docs/spec/renderer-placement.md index 270b665..1876a0b 100644 --- a/docs/spec/renderer-placement.md +++ b/docs/spec/renderer-placement.md @@ -35,19 +35,26 @@ overrides an intentional stretch/fit. One handle = one entry in z_index, visible, native_width, native_height }`, matching `AGENTS.md`'s "one image = one handle table" rule. -**Known limitation**: a handle is bound to exactly one `win` at creation -time, but the `virt_lines` extmark carrying its reserved blank rows is -buffer-scoped, not window-scoped — Neovim renders those reserved rows in -*every* window currently showing that buffer. If the same buffer is split -into a second window (`:split`/`:vsplit`), the non-anchor window displays -the reserved blank space with no image in it, for as long as it stays -open — `compute_placement` only ever computes visibility/position against -the one `handle.win` it was given. A real fix requires per-window -placement-id fan-out (a distinct `p=` for each window showing the buffer), -which conflicts with the one-handle-per-placement data model above and is -already tracked separately (see "Transmission cache" below and issue #10, -"Multi-location placement fan-out for a single transmitted image"). -Accepted for v0.x; revisit when #10 ships. +**Known limitation, narrower since issue #10**: a handle is still bound to +exactly one `win` at creation time, but the `virt_lines` extmark carrying +its reserved blank rows is buffer-scoped, not window-scoped — Neovim +renders those reserved rows in *every* window currently showing that +buffer. If the same buffer is split into a second window +(`:split`/`:vsplit`), the non-anchor window displays the reserved blank +space with no image in it, for as long as it stays open — +`compute_placement` only ever computes visibility/position against the one +`handle.win` it was given. Issue #10 ("Multi-location placement fan-out for +a single transmitted image") added the underlying primitive this needs — a +distinct `p=` per placement, so the same transmitted image id can carry a +second, independent placement without re-transmitting (see "Transmission +cache" below) — but it does not by itself detect a `:split` and create that +second placement automatically: `M.show()` still only ever creates one +handle bound to one `win` per call. A caller can work around the split case +today by calling `M.show()` a second time for the second window explicitly +(same `path`, so the same `(path, mtime)` cache key) — that second call now +fans out onto the first's image id instead of re-transmitting, per issue +#10 — but automatic per-window fan-out for a single `show()` call remains +unimplemented. Accepted for v0.x. ## Screen coordinate conversion @@ -263,12 +270,15 @@ single write is transient and does not survive past that write. Deletion (`a=d`) is not screen-position-dependent, so no cursor bracketing is needed there. -## Transmission cache: (path, mtime) keyed, no placement-id fan-out +## Transmission cache: (path, mtime) keyed, with placement-id fan-out Cache key: `path .. ":" .. mtime.sec .. "." .. mtime.nsec` (from -`vim.uv.fs_stat`). Each cache entry is a **list** of `{ id, active, lines, -columns, native_width, native_height }` entries for that exact file content -— a list, not a single entry, because: +`vim.uv.fs_stat`). Each cache entry is a **list** of `{ id, +active_placements, lines, columns, native_width, native_height }` entries +for that exact file content — a list, not a single entry, because a given +key can transiently have both an idle entry and a stale-but-still-active +one (e.g. mid-Ghostty-resize-recovery, see below) even though the common +case settles to exactly one entry per key: `native_width`/`native_height` (the PNG's native pixel dimensions, read once via `lua/blit/png.lua`'s IHDR reader when the file is actually read for @@ -277,23 +287,41 @@ every handle that reuses the entry — a cache hit never re-reads or re-parses the file. These are used solely by `pixel_crop` (see "Visibility policy" above), never for auto-sizing. -- `show()` on a cache hit with an **idle** (`active = false`) entry reuses - that id: no re-transmission, just an `a=p` placement (or nothing yet, if - not currently visible) — this is the AGENTS.md performance rule +`active_placements` counts how many currently-live handles reference this +entry's id — 0 means idle. `show()`'s `find_reusable_entry` tries an idle +entry first (see the Ghostty exception below), and only if none exists +falls back to any entry with `active_placements > 0`: + +- **Idle reuse**: no re-transmission, just an `a=p` placement (or nothing + yet, if not currently visible) — this is the AGENTS.md performance rule ("re-placement... must reuse its ID — never re-transmit"). -- `show()` on a cache hit where every existing entry is **active** (already - placed live somewhere else) transmits a fresh copy under a new id instead - of reusing/relocating the active one. blit does emit kitty's placement-id - key (`p=`, always `terminal.PLACEMENT_ID`, a fixed constant — see - `docs/spec/kitty-graphics.md`'s Placement section), but that fixed value - only prevents ghost placements when *repositioning* an id's one - placement; it is not a per-location identity. A given image id therefore - still has only one live placement at a time — reusing an active id for a - second simultaneous location would silently move the first location's - image instead of adding a second one. Supporting true multi-location - fan-out for one transmitted image (a distinct, allocated `p=` per - location) is deferred and is not needed for the common case (showing one - image once, or showing it again after it was cleared). +- **Active reuse, i.e. fan-out (issue #10, "Multi-location placement + fan-out for a single transmitted image")**: also no re-transmission. + Earlier versions of blit always sent a single fixed placement id + (`terminal.PLACEMENT_ID = 1`) and therefore could give a given image id + only one live placement at a time — a second concurrent `show()` of the + same file had to transmit a redundant copy under a brand-new id. Every + real placement command now carries a distinct, caller-allocated + `placement_id` (`alloc_placement_id()`, defined right below "Placement id + allocation" — an ever-incrementing, never-reused, session-lifetime + counter, since placement ids only need to be unique within one image id, + not terminal-wide the way image ids do) — see `docs/spec/kitty-graphics.md`'s + Placement section — so a second `show()` of an already-active entry + instead adds a second, independent placement of the SAME id: `id` + unchanged, `entry.active_placements` incremented, a fresh + `handle.placement_id` allocated, and only an `a=p` (or nothing, if not yet + visible) written — matching the idle-reuse case exactly except that the + entry was never idle to begin with. + +`destroy_handle` (see "Lifecycle" below) is the inverse: it decrements +`active_placements` and deletes only this handle's own placement +(`a=d,d=i,i=,p=`) — a sibling placement sharing the same +id, if any, is untouched. The terminal-side pixel data itself is only ever +freed (`d=I`, no `p=`) once `active_placements` reaches `0` **and** the +caller's intent was to free it (`VimLeavePre`'s full teardown, or a fatal +failure right after `M.show()` created the handle) — an everyday +`clear()`/`clear_all()` never frees data even when it drops the count to +`0`, keeping the entry idle-but-warm for a future `show()` instead. Cache entries are otherwise never evicted except at `VimLeavePre` (or the test-only `_reset()`) — an idle entry's id and terminal-side pixel data are @@ -304,7 +332,9 @@ long session that `show()`s many distinct files; unbounded growth is accepted for v0.x (mirrors that memo's own acceptance of the range being merely "negligible collision risk", not infinite). `alloc_id()` returns `nil, err` if the range is exhausted; `show()` propagates that as a normal -`nil, err_msg` failure. +`nil, err_msg` failure. Placement ids draw from the full unsigned 32-bit +`p=` space rather than a bounded pool (see "Placement id allocation" in +`renderer.lua`), so they have no equivalent exhaustion case in practice. **Ghostty exception: idle entries recorded against a stale terminal size are evicted eagerly, at reuse time.** Ghostty discards previously- @@ -315,7 +345,7 @@ all responses, `docs/spec/kitty-graphics.md`'s "Response handling"), and an id for a placement-only `a=p` then renders nothing (issue #24). Each cache entry therefore also records `vim.o.lines`/`vim.o.columns` (the whole Neovim grid size, which tracks the real terminal's size — not a per-window -size) at the moment of transmission. `acquire_idle_entry` compares an idle +size) at the moment of transmission. `find_reusable_entry` compares an idle entry's recorded size against the current size only when `caps.terminal == "ghostty"`; a mismatch means a resize happened at some point since transmission, so the entry is treated as dead: its id is freed back to the @@ -331,7 +361,7 @@ means a persistent autocmd outside the handle-gated `augroup` — directly conflicting with AGENTS.md's "no timers or autocmds active when zero images are displayed" performance rule. The size-comparison approach needs no autocmd at all: it only ever runs inside `show()`'s own -`acquire_idle_entry` call. **Accepted false negative**: if the terminal is +`find_reusable_entry` call. **Accepted false negative**: if the terminal is resized away and back to the *exact* original `lines`/`columns` before the next `show()`, the comparison can't tell that a resize happened in between, and a dead entry could still be handed out. This is deemed rare @@ -352,7 +382,7 @@ restore it, since nothing in `redraw_all()`'s normal `a=p`/`a=d` toggling ever re-transmits. `redraw_all()` now checks, for each handle it finds visible, whether `caps.terminal == "ghostty"` and that handle's own cache entry's recorded `vim.o.lines`/`vim.o.columns` differs from the current -values (the exact same signal `acquire_idle_entry` uses above, just read +values (the exact same signal `find_reusable_entry` uses above, just read instead of also gating reuse — see `ghostty_entry_stale()` in `renderer.lua`). @@ -363,9 +393,9 @@ this path beyond a naive "re-`a=T`, same id" attempt: will not restore a placement by re-`a=T`-ing under an id it already discarded the data for, even though the write itself reports success (`q=2` suppresses all responses, so blit has no way to detect this other - than the empirical result). `retransmit_and_place()` therefore frees the - stale id and hands the handle a *fresh* one via `alloc_id()`, exactly - mirroring how `acquire_idle_entry` above already treats a stale idle + than the empirical result). `retransmit_and_place_group()` therefore frees the + stale id and hands every handle sharing it a *fresh* one via `alloc_id()`, + exactly mirroring how `find_reusable_entry` above already treats a stale idle entry (free the old id, never reuse it) — it just also updates the now-live handle's `id` field and cache entry in place rather than waiting for a future `show()` call to do so. @@ -397,7 +427,7 @@ number of retries of its own (issue #37).** `ghostty_retransmit_pass()` can find a handle still `ghostty_entry_stale()` after it runs even though no further `WinResized`/`WinScrolled` restarted the timer: `compute_placement()` can come back `nil` for that one tick (a real drag-resize's tail end can -still race `vim.fn.screenpos()`), or `retransmit_and_place()`'s write itself +still race `vim.fn.screenpos()`), or `retransmit_and_place_group()`'s write itself can fail (`write_all()` exhausting its bounded EAGAIN retries — see `terminal.lua`). Before this was fixed, either case left the placement blank until the user happened to trigger another resize purely by luck — @@ -466,11 +496,17 @@ Two distinct kinds of state transition, kept separate: - **Destroy** (`BufWinLeave` for the specific `(buf, win)` pair, `WinClosed` for a closing window, `BufWipeout` for a wiped buffer, `M.clear()`/`M.clear_all()`, and `VimLeavePre`): removes the extmark, - removes the handle from `M._handles`, marks its cache entry idle, and - deletes its terminal-side placement. Only `VimLeavePre` (and - `M.clear`/`M.clear_all`'s eventual full-teardown path) also frees the - id back to the pool and the terminal's stored pixel data - (`d=I`) — everyday `clear()` keeps the cache warm. + removes the handle from `M._handles`, decrements its cache entry's + `active_placements`, and deletes only THIS handle's own placement + (`a=d,d=i,i=,p=`) — never the whole id, since a fan-out + sibling (issue #10) may still hold a live placement on it. Only once + `active_placements` reaches `0` **and** the caller's intent was to free + data (`VimLeavePre`'s full teardown, or a fatal failure right after + `M.show()` created the handle) does `destroy_handle` also free the id back + to the pool and the terminal's stored pixel data (`d=I`, no `p=`, since + every placement sharing the id is gone by then) — everyday `clear()` never + frees data, keeping the entry idle-but-warm instead, regardless of + whether it just dropped to `0`. **`a=d` is retried a bounded number of times after destroy too, not just on the redraw path above.** Unlike a still-tracked invisible handle, a @@ -478,21 +514,27 @@ Two distinct kinds of state transition, kept separate: WezTerm drops, there is no future `redraw_all()` pass left that would ever revisit it, and no guarantee a `WinScrolled`/`WinResized` event even fires again afterward to trigger one (issue #27; the repro is `clear_all()` - followed by *no* further input at all). `destroy_handle`'s `free_data = - false` path (the everyday teardown reasons above, not `VimLeavePre`) - therefore queues its id onto a small self-scheduled retry list - (`DESTROY_DELETE_RETRIES`, currently 3 extra attempts) that rides the same - debounce timer as `redraw_all`, resending a plain `a=d` each pass until - the budget runs out. Bounded, unlike the redraw path's resend-indefinitely - behavior, so a terminal that never honors the delete can't keep the - debounce timer (and therefore the "fully quiescent idle" guarantee) alive - forever. `VimLeavePre`'s `free_data = true` path is excluded: it frees the - id immediately, and Neovim is exiting right after, so a queued retry has - nothing meaningful left to protect and only risks racing a reused id - against a process that's already gone. If `acquire_idle_entry` reclaims a - still-queued id for a fresh placement before its retries are spent, the - queued entry is cancelled — otherwise a late retry could send `a=d` for an - id a brand new placement now legitimately owns. + followed by *no* further input at all). `destroy_handle`'s everyday- + teardown path (any call where the caller didn't request a full free, or + did but a fan-out sibling still shares the id — see above) therefore + queues its `(id, placement_id)` pair onto a small self-scheduled retry + list (`DESTROY_DELETE_RETRIES`, currently 3 extra attempts) that rides the + same debounce timer as `redraw_all`, resending the same scoped `a=d` each + pass until the budget runs out. Bounded, unlike the redraw path's resend- + indefinitely behavior, so a terminal that never honors the delete can't + keep the debounce timer (and therefore the "fully quiescent idle" + guarantee) alive forever. The full-free branch (id actually freed) is + excluded from queuing: for `VimLeavePre` specifically, Neovim is exiting + right after, so a queued retry has nothing meaningful left to protect and + only risks racing a reused id against a process that's already gone; for + a fatal `M.show()` failure, nothing else references the brand-new id + either. If `find_reusable_entry` reclaims a still-queued id for a fresh + placement before its retries are spent, the queued entries for that id + are cancelled (`cancel_pending_delete`) — though even without that, a late + retry naming the OLD `placement_id` could never hit the new placement's + DIFFERENT one, since placement ids are never reused (see "Placement id + allocation" in `renderer.lua`); the cancellation is belt-and-suspenders + against wasted escape-sequence bytes, not a correctness requirement. All autocmds live in one `augroup("blit", { clear = true })`; all extmarks in one `nvim_create_namespace("blit")`, both created lazily on first `show()` diff --git a/lua/blit/renderer.lua b/lua/blit/renderer.lua index d9618fa..34d3992 100644 --- a/lua/blit/renderer.lua +++ b/lua/blit/renderer.lua @@ -27,6 +27,10 @@ local NAMESPACE = "blit" ---@class blit.Handle ---@field id integer kitty image id (blit's reserved range) +---@field placement_id integer this handle's own kitty placement id (`p=`); +---never reused across handles within a session, so several handles can +---share one image {id} — each with its own live placement — without ever +---colliding (issue #10). See docs/spec/kitty-graphics.md's Placement section. ---@field buf integer ---@field win integer ---@field extmark_id integer @@ -94,9 +98,29 @@ local function free_id(id) used_ids[id] = nil end +-- Placement id allocation ------------------------------------------------------ +-- Unlike image ids, placement ids only need to be unique within one image +-- id, not terminal-wide — so a single ever-incrementing counter (never +-- freed/reused, unlike alloc_id's bounded pool) is sufficient: at any +-- realistic show()/clear() rate this session-lifetime counter would take +-- centuries to approach the 32-bit `p=` value space. Never reusing a +-- placement id number is also what makes a stray delayed retry from +-- destroy_handle's retry queue (see "Destroy-path delete retry queue" +-- below) provably harmless without needing its own cancellation bookkeeping +-- — it can never coincide with a later, still-live placement. + +local next_placement_id = 1 + +---@return integer +local function alloc_placement_id() + local pid = next_placement_id + next_placement_id = next_placement_id + 1 + return pid +end + -- Transmission cache ---------------------------------------------------------- ----@type table +---@type table local cache = {} ---@param path string @@ -114,7 +138,7 @@ M.cache_key = cache_key -- silently renders nothing (issue #24). Each cache entry therefore records -- the Neovim grid size (`vim.o.lines`/`vim.o.columns`, which tracks the -- real terminal's size, not just a per-window size) at transmit time; --- acquire_idle_entry rejects (and frees) an idle entry recorded against a +-- find_reusable_entry rejects (and frees) an idle entry recorded against a -- stale size on Ghostty, forcing a fresh transmit instead of trusting dead -- data. This is a lazy, reuse-time check rather than a `VimResized` -- listener specifically to avoid needing a persistent autocmd outside the @@ -125,13 +149,13 @@ M.cache_key = cache_key -- -- The same size-mismatch signal also drives redraw_all's still-visible- -- handle path below (issue #34): a handle that stays active/displayed --- across a Ghostty resize is not touched by acquire_idle_entry at all (it's +-- across a Ghostty resize is not touched by find_reusable_entry at all (it's -- never idle), so without this it stayed permanently blank once its data -- was discarded — see "Redraw" below and -- docs/spec/renderer-placement.md's Transmission cache section. -- -- This one fact — which terminal actually has this quirk — is centralized --- here rather than compared inline at each call site, so acquire_idle_entry, +-- here rather than compared inline at each call site, so find_reusable_entry, -- redraw_all's stale check, and its resize-race redraw guard all agree on -- the same definition as more terminals/quirks are added over time. ---@param terminal_name "kitty"|"wezterm"|"ghostty"|nil @@ -155,23 +179,45 @@ end local DESTROY_DELETE_RETRIES = 3 ----@type { id: integer, retries: integer }[] +---@type { id: integer, placement_id: integer, retries: integer }[] local pending_deletes = {} +-- Removes every still-pending retry for {id}, regardless of which +-- placement_id each was queued for. Reused-id-only reasoning, kept as +-- belt-and-suspenders even though placement ids are never reused (see +-- "Placement id allocation" above, which already makes a stray retry +-- harmless on its own): avoids wasting escape-sequence bytes resending +-- deletes for placements this reuse has nothing to do with. ---@param id integer local function cancel_pending_delete(id) - for i, pending in ipairs(pending_deletes) do - if pending.id == id then + local i = 1 + while i <= #pending_deletes do + if pending_deletes[i].id == id then table.remove(pending_deletes, i) - return + else + i = i + 1 end end end +-- Finds an existing cache entry this key's next show() can reuse instead of +-- transmitting fresh pixel data — either an IDLE entry (no handle currently +-- references its id) or, failing that, an ACTIVE one to fan out onto (issue +-- #10: a second, independent placement of the same id, via a fresh +-- placement id — no re-transmission needed). Idle entries are preferred +-- first only because that's also the only branch that needs the Ghostty +-- staleness check below: an idle entry has no live handle whose own +-- redraw-pass check (`ghostty_entry_stale`, see "Redraw" below) would ever +-- notice/self-heal a stale one, so staleness has to be caught here, at +-- reuse time, instead. An active entry always has at least one live handle +-- already doing that self-healing check every redraw pass, so reusing it +-- for fan-out even while transiently stale is safe — the settle timer will +-- catch up every placement sharing that id together (see +-- "retransmit_and_place_group" below). ---@param key string ---@param terminal_name "kitty"|"wezterm"|"ghostty"|nil ----@return { id: integer, active: boolean, lines: integer, columns: integer, native_width: integer, native_height: integer }? -local function acquire_idle_entry(key, terminal_name) +---@return { id: integer, active_placements: integer, lines: integer, columns: integer, native_width: integer, native_height: integer }? +local function find_reusable_entry(key, terminal_name) local entries = cache[key] if not entries then return nil @@ -179,7 +225,7 @@ local function acquire_idle_entry(key, terminal_name) local i = 1 while i <= #entries do local entry = entries[i] - if entry.active then + if entry.active_placements > 0 then i = i + 1 elseif discards_pixels_on_resize(terminal_name) @@ -188,15 +234,18 @@ local function acquire_idle_entry(key, terminal_name) free_id(entry.id) table.remove(entries, i) else - entry.active = true - -- The id may still have a bounded delete retry outstanding from a + -- The id may still have bounded delete retries outstanding from a -- prior destroy (see "Destroy-path delete retry queue" above); this - -- reuse legitimately reclaims it, so a late retry must not delete the - -- placement being made here. + -- reuse legitimately reclaims it. cancel_pending_delete(entry.id) return entry end end + for _, entry in ipairs(entries) do + if entry.active_placements > 0 then + return entry + end + end return nil end @@ -208,7 +257,7 @@ local function register_cache_entry(key, id, native_width, native_height) cache[key] = cache[key] or {} table.insert(cache[key], { id = id, - active = true, + active_placements = 1, lines = vim.o.lines, columns = vim.o.columns, native_width = native_width, @@ -218,7 +267,7 @@ end ---@param key string ---@param id integer ----@return { id: integer, active: boolean, lines: integer, columns: integer, native_width: integer, native_height: integer }? +---@return { id: integer, active_placements: integer, lines: integer, columns: integer, native_width: integer, native_height: integer }? local function find_cache_entry(key, id) local entries = cache[key] if not entries then @@ -232,21 +281,6 @@ local function find_cache_entry(key, id) return nil end ----@param key string ----@param id integer -local function mark_cache_entry_idle(key, id) - local entries = cache[key] - if not entries then - return - end - for _, entry in ipairs(entries) do - if entry.id == id then - entry.active = false - return - end - end -end - ---@param key string ---@param id integer local function drop_cache_entry(key, id) @@ -548,6 +582,7 @@ end ---@return blit.terminal.PlacementOpts local function placement_opts(handle, placement) return { + placement_id = handle.placement_id, columns = placement.cols, rows = placement.rows, z_index = handle.z_index, @@ -575,14 +610,18 @@ local function place_existing(handle, placement) return ok, err end +-- Scoped to this handle's own placement_id — with multi-location fan-out +-- (issue #10) an image id can have several concurrent placements, so an +-- unscoped `a=d,d=i,i=` would wipe out every sibling placement sharing +-- this id, not just this handle's own. ---@param handle blit.Handle local function hide_existing(handle) - M._write_fn({ terminal.build_delete(handle.id) }) + M._write_fn({ terminal.build_delete(handle.id, { placement_id = handle.placement_id }) }) handle.visible = false end -- On Ghostty, a still-visible handle's transmitted data can have been --- silently discarded by the same real terminal resize acquire_idle_entry +-- silently discarded by the same real terminal resize find_reusable_entry -- guards against for idle entries (issue #24/#34) — see the "Transmission -- cache" comment above. There is no response to detect this by, so the -- only signal available is the same one: the handle's cache entry was @@ -601,60 +640,133 @@ local function ghostty_entry_stale(handle, terminal_name) return entry.lines ~= vim.o.lines or entry.columns ~= vim.o.columns end --- Re-transmits a still-visible handle's pixel data under a *fresh* id and --- updates the handle/cache to point at it, freeing the old, now-dead one — --- confirmed via manual testing on a real Ghostty resize (issue #34) that --- simply re-`a=T`-ing under the SAME id Ghostty already discarded does NOT --- bring the placement back, even though the write itself reports success --- (`q=2` suppresses all responses, so blit has no way to detect this other --- than the empirical result). This mirrors `acquire_idle_entry`'s existing --- Ghostty eviction above exactly: that path never reuses a stale id either — --- it frees it and lets a fresh `alloc_id()` hand out a new one on the next --- transmit. This is the one place outside `M.show()` that transmits rather --- than reusing `a=p` — see AGENTS.md's performance rule on reuse-over- --- retransmit for why that's normally forbidden and the narrow, Ghostty-only --- exception carved out here. ----@param handle blit.Handle ----@param placement blit.PlacementResult +---@param id integer +---@return blit.Handle[] +local function handles_with_id(id) + local out = {} + for _, h in ipairs(M._handles) do + if h.id == id then + out[#out + 1] = h + end + end + return out +end + +-- Re-transmits a stale id's pixel data under a *fresh* id and migrates +-- EVERY handle currently sharing it (issue #10 fan-out means a stale id can +-- have several live placements, not just one), freeing the old, now-dead id +-- once every one of them has moved off it — confirmed via manual testing on +-- a real Ghostty resize (issue #34) that simply re-`a=T`-ing under the SAME +-- id Ghostty already discarded does NOT bring the placement back, even +-- though the write itself reports success (`q=2` suppresses all responses, +-- so blit has no way to detect this other than the empirical result). This +-- mirrors `find_reusable_entry`'s existing Ghostty eviction above exactly: +-- that path never reuses a stale id either — it frees it and lets a fresh +-- `alloc_id()` hand out a new one on the next transmit. This is the one +-- place outside `M.show()` that transmits rather than reusing `a=p` — see +-- AGENTS.md's performance rule on reuse-over-retransmit for why that's +-- normally forbidden and the narrow, Ghostty-only exception carved out +-- here. +-- +-- Migrating the group atomically (one delete+transmit for the shared id, +-- not one per handle) matters for correctness, not just efficiency: freeing +-- the old id's cache entry after only the FIRST sharing handle's retransmit +-- would leave every other handle still pointing at an id whose cache entry +-- (and therefore `ghostty_entry_stale`'s only signal) has already vanished +-- — they would never be recognized as needing a retry again, and this +-- pass's own `still_stale` bookkeeping would silently miss them too. +-- Handles with no current placement (scrolled off, wrong tab, etc.) are +-- still migrated to the new id — so a later redraw pass places them +-- correctly once they become visible again — but only the ones WITH a +-- current placement need an actual `a=p`/`a=T` written now; one of them +-- (`display_handle`) carries the transmit itself, the rest just add a +-- placement (`a=p`) against the data it just sent. +---@param handles blit.Handle[] every handle currently sharing one stale id ---@return boolean ok -local function retransmit_and_place(handle, placement) - local bytes = read_file(handle.path) +local function retransmit_and_place_group(handles) + local first = handles[1] + local old_id = first.id + + local bytes = read_file(first.path) if not bytes then - hide_existing(handle) + for _, h in ipairs(handles) do + hide_existing(h) + end return false end local new_id = alloc_id() if not new_id then - hide_existing(handle) + for _, h in ipairs(handles) do + hide_existing(h) + end return false end - local old_id = handle.id - local sequences = { - terminal.build_delete(old_id, { free_data = true }), - terminal.build_save_cursor(), - terminal.build_move_cursor(placement.screen_row, placement.screen_col), - } - vim.list_extend( - sequences, - terminal.build_transmit( - bytes, - { id = new_id, action = "T", placement = placement_opts(handle, placement) } + ---@type table + local placements = {} + local display_handle = nil + for _, h in ipairs(handles) do + placements[h] = compute_placement(h) + if placements[h] and not display_handle then + display_handle = h + end + end + + local sequences = { terminal.build_delete(old_id, { free_data = true }) } + + if display_handle then + vim.list_extend(sequences, { + terminal.build_save_cursor(), + terminal.build_move_cursor( + placements[display_handle].screen_row, + placements[display_handle].screen_col + ), + }) + vim.list_extend( + sequences, + terminal.build_transmit(bytes, { + id = new_id, + action = "T", + placement = placement_opts(display_handle, placements[display_handle]), + }) ) - ) - table.insert(sequences, terminal.build_restore_cursor()) + table.insert(sequences, terminal.build_restore_cursor()) + else + -- No handle sharing this id is currently visible; keep the data ready + -- (transmit-only) so whichever handle becomes visible next places + -- correctly against the new id without needing its own re-transmit. + vim.list_extend(sequences, terminal.build_transmit(bytes, { id = new_id, action = "t" })) + end + + for _, h in ipairs(handles) do + if h ~= display_handle and placements[h] then + vim.list_extend(sequences, { + terminal.build_save_cursor(), + terminal.build_move_cursor(placements[h].screen_row, placements[h].screen_col), + terminal.build_placement(new_id, placement_opts(h, placements[h])), + terminal.build_restore_cursor(), + }) + end + end local ok = M._write_fn(sequences) if ok then - handle.id = new_id - drop_cache_entry(handle.cache_key, old_id) + drop_cache_entry(first.cache_key, old_id) free_id(old_id) - register_cache_entry(handle.cache_key, new_id, handle.native_width, handle.native_height) + register_cache_entry(first.cache_key, new_id, first.native_width, first.native_height) + local new_entry = find_cache_entry(first.cache_key, new_id) + new_entry.active_placements = #handles + for _, h in ipairs(handles) do + h.id = new_id + h.visible = placements[h] ~= nil + end else free_id(new_id) + for _, h in ipairs(handles) do + h.visible = false + end end - handle.visible = ok and true or false return ok end @@ -699,8 +811,9 @@ local function arm_ghostty_retransmit_timer() end -- A handle can still be `ghostty_entry_stale()` after this pass runs: --- `retransmit_and_place()` itself can fail (e.g. `write_all()` exhausted its --- bounded EAGAIN retries), or `compute_placement()` can come back `nil` for +-- `retransmit_and_place_group()` itself can fail (e.g. `write_all()` +-- exhausted its bounded EAGAIN retries), or `compute_placement()` can come +-- back `nil` for -- this one settle-timer tick even though the handle's cache entry is still -- stale — a real drag-resize's tail end can still race `vim.fn.screenpos()` -- (see its pcall guard in `compute_placement`) at the exact moment the @@ -726,10 +839,32 @@ local ghostty_retransmit_retries_left = GHOSTTY_RETRANSMIT_RETRIES ghostty_retransmit_pass = function() local caps = M._detect_fn() local still_stale = false + -- One id can now have several sharing handles (fan-out); process each + -- stale id's whole group together via retransmit_and_place_group rather + -- than per-handle, and only once per id per pass. + local processed_ids = {} for _, handle in ipairs(M._handles) do - if ghostty_entry_stale(handle, caps.terminal) then - local placement = compute_placement(handle) - if not (placement and retransmit_and_place(handle, placement)) then + if not processed_ids[handle.id] and ghostty_entry_stale(handle, caps.terminal) then + processed_ids[handle.id] = true + local group = handles_with_id(handle.id) + local any_visible = false + for _, h in ipairs(group) do + if compute_placement(h) then + any_visible = true + break + end + end + if any_visible then + if not retransmit_and_place_group(group) then + still_stale = true + end + else + -- Mirrors the single-handle behavior this replaces: a stale group + -- with nothing currently visible is left alone rather than + -- transmitted speculatively, but still counts as "still stale" so + -- the bounded retry budget keeps checking back in case a + -- momentary screenpos() race (not genuine long-term invisibility) + -- clears up within the next pass or two. still_stale = true end end @@ -807,7 +942,9 @@ local function redraw_all() if #pending_deletes > 0 then local still_pending = {} for _, pending in ipairs(pending_deletes) do - M._write_fn({ terminal.build_delete(pending.id) }) + M._write_fn({ + terminal.build_delete(pending.id, { placement_id = pending.placement_id }), + }) pending.retries = pending.retries - 1 if pending.retries > 0 then table.insert(still_pending, pending) @@ -830,10 +967,23 @@ end -- Lifecycle: destroy ------------------------------------------------------ +-- Whether it's safe to fully free {handle}'s id (terminal-side pixel data +-- + the id itself) depends on whether any OTHER handle still shares it via +-- fan-out (issue #10), not just on the caller's free_data intent: freeing +-- data out from under a sibling placement that still needs it would blank +-- it. free_data therefore only ever triggers the full free once THIS +-- handle is the last one referencing the id (`remaining <= 0`); otherwise +-- only this handle's own placement is torn down (`d=i,p=`, +-- data untouched) and the id/cache entry are left alone for whichever +-- handle(s) still use it. Since VimLeavePre destroys every handle in one +-- pass, the LAST handle sharing an id to be destroyed always ends up taking +-- the full-free branch, so the id and its data are still guaranteed to be +-- fully released by the time VimLeavePre finishes — it just may not be +-- THIS particular call that does it. ---@param handle blit.Handle ---@param opts? { free_data?: boolean } local function destroy_handle(handle, opts) - local free_data = (opts and opts.free_data) or false + local free_data_requested = (opts and opts.free_data) or false if vim.api.nvim_buf_is_valid(handle.buf) then pcall(vim.api.nvim_buf_del_extmark, handle.buf, ensure_namespace(), handle.extmark_id) @@ -846,24 +996,47 @@ local function destroy_handle(handle, opts) end end - M._write_fn({ terminal.build_delete(handle.id, { free_data = free_data }) }) + local entry = find_cache_entry(handle.cache_key, handle.id) + local remaining = entry and math.max(0, entry.active_placements - 1) or 0 + if entry then + entry.active_placements = remaining + end - if free_data then + if free_data_requested and remaining <= 0 then + M._write_fn({ terminal.build_delete(handle.id, { free_data = true }) }) free_id(handle.id) drop_cache_entry(handle.cache_key, handle.id) - else - mark_cache_entry_idle(handle.cache_key, handle.id) - -- Only the free_data=false path (clear()/clear_all(), BufWinLeave, - -- WinClosed, BufWipeout) gets a retry: its id stays reserved and its - -- cache entry stays around for reuse, so a late retry always still - -- refers to either this same dead placement or nothing (cancelled via - -- cancel_pending_delete if acquire_idle_entry reclaims the id first). - -- VimLeavePre's free_data=true call frees the id immediately, so a - -- queued retry there could race a reused id after Neovim exits/the - -- process is gone anyway — not worth the risk for a shutdown path. - table.insert(pending_deletes, { id = handle.id, retries = DESTROY_DELETE_RETRIES }) - schedule_redraw() + return + end + + M._write_fn({ terminal.build_delete(handle.id, { placement_id = handle.placement_id }) }) + + if free_data_requested then + -- A sibling placement still shares this id; VimLeavePre's own remaining + -- iterations will eventually take the full-free branch above once the + -- last one goes. No retry queued here, matching the exclusion below: + -- Neovim is exiting right after, so a queued retry has nothing + -- meaningful left to protect. + return end + + -- Everyday teardown (clear()/clear_all(), BufWinLeave, WinClosed, + -- BufWipeout) never frees data: the id's cache entry stays around — + -- idle if this was the last handle sharing it, still active otherwise — + -- so a later show() of the same file (or a sibling fan-out placement) + -- stays cheap. A late retry always still refers to either this same dead + -- placement or nothing (cancelled via cancel_pending_delete if + -- find_reusable_entry reclaims the id first); it can never hit a + -- DIFFERENT live placement, since placement ids are never reused (see + -- "Placement id allocation" above). VimLeavePre's free_data=true path is + -- excluded above for the same reason as the full-free branch: the + -- process is exiting right after. + table.insert(pending_deletes, { + id = handle.id, + placement_id = handle.placement_id, + retries = DESTROY_DELETE_RETRIES, + }) + schedule_redraw() end local autocmds_ready = false @@ -1101,7 +1274,10 @@ function M.show(path, opts) debounce_ms = opts.debounce_ms or debounce_ms local key = cache_key(path, stat.mtime) - local entry = acquire_idle_entry(key, caps.terminal) + -- Either an idle entry (ordinary re-show of a cleared handle) or an + -- active one (issue #10 fan-out: a second, concurrent placement of the + -- same already-live id) — either way, no re-transmission needed. + local entry = find_reusable_entry(key, caps.terminal) local id, bytes, needs_transmit, native_width, native_height if entry then @@ -1109,6 +1285,7 @@ function M.show(path, opts) native_width = entry.native_width native_height = entry.native_height needs_transmit = false + entry.active_placements = entry.active_placements + 1 else local read_err bytes, read_err = read_file(path) @@ -1150,6 +1327,7 @@ function M.show(path, opts) ---@type blit.Handle local handle = { id = id, + placement_id = alloc_placement_id(), buf = buf, win = win, extmark_id = extmark_id, @@ -1246,6 +1424,7 @@ function M._reset() cache = {} used_ids = {} next_id = terminal.ID_RANGE_START + next_placement_id = 1 M._write_fn = function(sequences) return terminal.write(sequences) end diff --git a/lua/blit/terminal.lua b/lua/blit/terminal.lua index ccdc0ef..25f26bd 100644 --- a/lua/blit/terminal.lua +++ b/lua/blit/terminal.lua @@ -13,6 +13,7 @@ local APC_END = ESC .. "\\" ---@alias blit.terminal.Action "T"|"t" ---@class blit.terminal.PlacementOpts +---@field placement_id integer this placement's `p=` id — see below ---@field columns? integer ---@field rows? integer ---@field z_index? integer @@ -28,14 +29,25 @@ local APC_END = ESC .. "\\" ---@field quiet? 0|1|2 -- default 2 ---@field placement? blit.terminal.PlacementOpts --- Every blit placement uses this single fixed placement id. renderer.lua's --- cache never marks the same image id "active" for more than one handle at --- once, so a given id has at most one live placement at any time — reusing --- a constant placement id means a reposition (`a=p,i=,p=1,...`) UPDATES --- that placement in place. Omitting `p=` entirely (or varying it) makes the --- terminal create an additional, stacked placement instead of moving the --- existing one — see docs/spec/kitty-graphics.md's Placement section. -M.PLACEMENT_ID = 1 +-- Every real placement command carries a caller-supplied `p=` (placement +-- id), scoping it to one specific placement of one image id rather than +-- "the" placement — the same image id can now have several concurrent +-- placements (issue #10, "Multi-location placement fan-out"). Reusing the +-- SAME (id, placement_id) pair across calls makes a reposition +-- (`a=p,i=,p=,...`) UPDATE that one placement in place, exactly as +-- the old fixed `p=1` did; a DIFFERENT placement_id on the same id instead +-- adds a second, independent placement without re-sending pixel data. +-- Allocating distinct ids per placement — never reusing one across two +-- concurrently-live placements — is renderer.lua's responsibility (it owns +-- the handle table), per AGENTS.md's architecture; terminal.lua only knows +-- how to encode whatever id it's given. See +-- docs/spec/kitty-graphics.md's Placement section. + +---@param v any +---@return boolean +local function is_positive_integer(v) + return type(v) == "number" and v == math.floor(v) and v > 0 +end ---@param parts { [1]: string, [2]: string|integer }[] ---@return string @@ -125,6 +137,13 @@ function M.build_transmit(png_bytes, opts) end, "0, 1, or 2", }, + placement = { + opts.placement, + function(v) + return v == nil or is_positive_integer(v.placement_id) + end, + "a table with a positive integer placement_id, or nil", + }, }) local action = opts.action or "T" @@ -149,7 +168,7 @@ function M.build_transmit(png_bytes, opts) { "q", quiet }, } if opts.placement then - parts[#parts + 1] = { "p", M.PLACEMENT_ID } + parts[#parts + 1] = { "p", opts.placement.placement_id } end append_placement_parts(parts, opts.placement) parts[#parts + 1] = { "m", more } @@ -163,22 +182,44 @@ function M.build_transmit(png_bytes, opts) end ---@param id integer ----@param opts? blit.terminal.PlacementOpts +---@param opts blit.terminal.PlacementOpts ---@return string sequence function M.build_placement(id, opts) - vim.validate({ id = { id, M.is_valid_id, "a valid id in blit's reserved range" } }) - local parts = { { "a", "p" }, { "i", id }, { "p", M.PLACEMENT_ID } } + vim.validate({ + id = { id, M.is_valid_id, "a valid id in blit's reserved range" }, + opts = { opts, "table" }, + placement_id = { opts.placement_id, is_positive_integer, "a positive integer" }, + }) + local parts = { { "a", "p" }, { "i", id }, { "p", opts.placement_id } } append_placement_parts(parts, opts) return APC_START .. build_control(parts) .. APC_END end ---@param id integer ----@param opts? { free_data?: boolean } +---@param opts? { free_data?: boolean, placement_id?: integer } ---@return string sequence function M.build_delete(id, opts) - vim.validate({ id = { id, M.is_valid_id, "a valid id in blit's reserved range" } }) + vim.validate({ + id = { id, M.is_valid_id, "a valid id in blit's reserved range" }, + placement_id = { + opts and opts.placement_id, + function(v) + return v == nil or is_positive_integer(v) + end, + "a positive integer, or nil", + }, + }) local d = (opts and opts.free_data) and "I" or "i" local parts = { { "a", "d" }, { "d", d }, { "i", id } } + -- `p=` restricts the delete to one specific placement of this id instead + -- of every placement blit has made for it — required now that one id can + -- have several concurrent placements (issue #10). Omitted only for the + -- whole-id teardown case (free_data=true once no handle references the + -- id anymore — see renderer.lua's destroy_handle), where every placement + -- is being torn down together anyway. + if opts and opts.placement_id then + parts[#parts + 1] = { "p", opts.placement_id } + end return APC_START .. build_control(parts) .. APC_END end @@ -200,12 +241,6 @@ function M.build_restore_cursor() return ESC .. "8" end ----@param v any ----@return boolean -local function is_positive_integer(v) - return type(v) == "number" and v == math.floor(v) and v > 0 -end - ---@param row integer 1-indexed screen row ---@param col integer 1-indexed screen column ---@return string diff --git a/tests/test_renderer.lua b/tests/test_renderer.lua index 81d64d6..eb443ff 100644 --- a/tests/test_renderer.lua +++ b/tests/test_renderer.lua @@ -321,13 +321,71 @@ T["show"]["cache hit on idle entry places without retransmitting"] = function() MiniTest.expect.equality(all:find("a=p", 1, true) ~= nil, true) end -T["show"]["active cache entry forces a fresh id, never relocates"] = function() +T["show"]["active cache entry fans out onto the same id, no retransmit (issue #10)"] = function() + local buf, win = setup_floating(numbered_lines(10), 20, 10) + + local first = renderer.show(tmp_path, { width = 5, height = 3, buf = buf, win = win, lnum = 2 }) + MiniTest.expect.equality(first.visible, true) + + captured = {} + local second = renderer.show(tmp_path, { width = 5, height = 3, buf = buf, win = win, lnum = 4 }) + MiniTest.expect.equality(second.visible, true) + + -- Same image id (no re-transmission), but each placement carries its own + -- distinct placement_id so neither one ever displaces the other. + MiniTest.expect.equality(second.id, first.id) + MiniTest.expect.no_equality(second.placement_id, first.placement_id) + + local all = table.concat(captured[1], "") + MiniTest.expect.equality(all:find("a=T", 1, true), nil) + MiniTest.expect.equality(all:find("a=p", 1, true) ~= nil, true) + MiniTest.expect.equality(all:find(",p=" .. second.placement_id, 1, true) ~= nil, true) +end + +T["show"]["clearing one fanned-out handle leaves its sibling's placement alone (issue #10)"] = function() + local buf, win = setup_floating(numbered_lines(10), 20, 10) + + local first = renderer.show(tmp_path, { width = 5, height = 3, buf = buf, win = win, lnum = 2 }) + local second = renderer.show(tmp_path, { width = 5, height = 3, buf = buf, win = win, lnum = 4 }) + + captured = {} + renderer.clear(first) + + -- Scoped to first's own placement_id only: second's placement_id never + -- appears in the delete blit sends. + local all = table.concat(captured[1], "") + MiniTest.expect.equality( + all, + ESC .. "_Ga=d,d=i,i=" .. first.id .. ",p=" .. first.placement_id .. ESC .. "\\" + ) + MiniTest.expect.equality(all:find(",p=" .. second.placement_id, 1, true), nil) + MiniTest.expect.equality(second.visible, true) +end + +T["show"]["VimLeavePre frees data only once the last fanned-out handle is gone (issue #10)"] = function() local buf, win = setup_floating(numbered_lines(10), 20, 10) local first = renderer.show(tmp_path, { width = 5, height = 3, buf = buf, win = win, lnum = 2 }) local second = renderer.show(tmp_path, { width = 5, height = 3, buf = buf, win = win, lnum = 4 }) + MiniTest.expect.equality(first.id, second.id) - MiniTest.expect.no_equality(first.id, second.id) + captured = {} + vim.api.nvim_exec_autocmds("VimLeavePre", {}) + + local free_count, scoped_count = 0, 0 + for _, seq in ipairs(captured) do + local all = table.concat(seq, "") + if all:find("d=I", 1, true) then + free_count = free_count + 1 + elseif all:find("d=i", 1, true) then + scoped_count = scoped_count + 1 + end + end + -- One handle's teardown only removes its own placement (the id is still + -- shared); the other, destroyed once nothing references the id anymore, + -- is the one that actually frees the terminal-side pixel data. + MiniTest.expect.equality(free_count, 1) + MiniTest.expect.equality(scoped_count, 1) end T["show"]["ghostty: reuses idle cache entry when terminal size is unchanged"] = function() @@ -483,12 +541,19 @@ T["show"]["re-placing a later handle catches up an earlier handle's stale positi { width = 5, height = 3, buf = buf, win = win, lnum = 1, debounce_ms = 5 } ) MiniTest.expect.equality(b.visible, true) - MiniTest.expect.no_equality(a.id, b.id) + -- Same file, both still active: b fans out onto a's id (issue #10) rather + -- than getting a fresh one, but each keeps its own placement_id. + MiniTest.expect.equality(a.id, b.id) + MiniTest.expect.no_equality(a.placement_id, b.placement_id) local function a_was_replaced() for _, seq in ipairs(captured) do local all = table.concat(seq, "") - if all:find("a=p", 1, true) and all:find("i=" .. a.id, 1, true) then + if + all:find("a=p", 1, true) + and all:find("i=" .. a.id, 1, true) + and all:find(",p=" .. a.placement_id, 1, true) + then return true end end @@ -509,7 +574,10 @@ T["clear"]["deletes placement and extmark"] = function() renderer.clear(handle) local all = table.concat(captured[1], "") - MiniTest.expect.equality(all, ESC .. "_Ga=d,d=i,i=" .. handle.id .. ESC .. "\\") + MiniTest.expect.equality( + all, + ESC .. "_Ga=d,d=i,i=" .. handle.id .. ",p=" .. handle.placement_id .. ESC .. "\\" + ) local ns = vim.api.nvim_create_namespace("blit") local mark = vim.api.nvim_buf_get_extmark_by_id(buf, ns, handle.extmark_id, {}) @@ -537,7 +605,10 @@ T["clear"]["self-retries a=d a bounded number of times with no further events (i MiniTest.expect.equality(#captured, 4) for _, seq in ipairs(captured) do local all = table.concat(seq, "") - MiniTest.expect.equality(all, ESC .. "_Ga=d,d=i,i=" .. handle.id .. ESC .. "\\") + MiniTest.expect.equality( + all, + ESC .. "_Ga=d,d=i,i=" .. handle.id .. ",p=" .. handle.placement_id .. ESC .. "\\" + ) end -- The retry budget is bounded: nothing further is sent once it's spent. @@ -609,7 +680,10 @@ T["redraw"]["hides when the anchor line scrolls past the top edge (issue #28)"] MiniTest.expect.equality(handle.visible, false) local all = table.concat(captured[1], "") - MiniTest.expect.equality(all, ESC .. "_Ga=d,d=i,i=" .. handle.id .. ESC .. "\\") + MiniTest.expect.equality( + all, + ESC .. "_Ga=d,d=i,i=" .. handle.id .. ",p=" .. handle.placement_id .. ESC .. "\\" + ) end T["redraw"]["reissues a=d on every pass while invisible past the top edge (issue #28)"] = function() @@ -643,7 +717,10 @@ T["redraw"]["reissues a=d on every pass while invisible past the top edge (issue MiniTest.expect.equality(handle.visible, false) MiniTest.expect.equality(#captured, 1) local all = table.concat(captured[1], "") - MiniTest.expect.equality(all, ESC .. "_Ga=d,d=i,i=" .. handle.id .. ESC .. "\\") + MiniTest.expect.equality( + all, + ESC .. "_Ga=d,d=i,i=" .. handle.id .. ",p=" .. handle.placement_id .. ESC .. "\\" + ) end T["redraw"]["hides once the window shrinks below the reserved rows"] = function() @@ -667,7 +744,10 @@ T["redraw"]["hides once the window shrinks below the reserved rows"] = function( MiniTest.expect.equality(handle.visible, false) local all = table.concat(captured[1], "") - MiniTest.expect.equality(all, ESC .. "_Ga=d,d=i,i=" .. handle.id .. ESC .. "\\") + MiniTest.expect.equality( + all, + ESC .. "_Ga=d,d=i,i=" .. handle.id .. ",p=" .. handle.placement_id .. ESC .. "\\" + ) end T["redraw"]["reissues a=d on every pass while still invisible (issue #23)"] = function() @@ -702,7 +782,10 @@ T["redraw"]["reissues a=d on every pass while still invisible (issue #23)"] = fu MiniTest.expect.equality(handle.visible, false) MiniTest.expect.equality(#captured, 1) local all = table.concat(captured[1], "") - MiniTest.expect.equality(all, ESC .. "_Ga=d,d=i,i=" .. handle.id .. ESC .. "\\") + MiniTest.expect.equality( + all, + ESC .. "_Ga=d,d=i,i=" .. handle.id .. ",p=" .. handle.placement_id .. ESC .. "\\" + ) end T["redraw"]["hides on TabLeave and restores on TabEnter (issue #16)"] = function() @@ -721,7 +804,10 @@ T["redraw"]["hides on TabLeave and restores on TabEnter (issue #16)"] = function end) MiniTest.expect.equality(handle.visible, false) local hide_all = table.concat(captured[1], "") - MiniTest.expect.equality(hide_all, ESC .. "_Ga=d,d=i,i=" .. handle.id .. ESC .. "\\") + MiniTest.expect.equality( + hide_all, + ESC .. "_Ga=d,d=i,i=" .. handle.id .. ",p=" .. handle.placement_id .. ESC .. "\\" + ) captured = {} vim.cmd("tabclose") @@ -854,7 +940,10 @@ T["redraw"]["source-rect crop"]["hides once the reserved block has fully scrolle MiniTest.expect.equality(handle.visible, false) local all = table.concat(captured[1], "") - MiniTest.expect.equality(all, ESC .. "_Ga=d,d=i,i=" .. handle.id .. ESC .. "\\") + MiniTest.expect.equality( + all, + ESC .. "_Ga=d,d=i,i=" .. handle.id .. ",p=" .. handle.placement_id .. ESC .. "\\" + ) end T["redraw"]["source-rect crop"]["two handles anchored at the same lnum hide instead of risking a wrong crop"] = function() diff --git a/tests/test_terminal_escape.lua b/tests/test_terminal_escape.lua index 0a0c793..1d802ee 100644 --- a/tests/test_terminal_escape.lua +++ b/tests/test_terminal_escape.lua @@ -89,36 +89,47 @@ T["build_transmit"]["action t (transmit-only) differs only in a="] = function() MiniTest.expect.equality(display_seq:gsub("a=T", "a=t"), transmit_seq) end -T["build_transmit"]["combined transmit+placement carries the fixed placement id"] = function() +T["build_transmit"]["combined transmit+placement carries the caller-supplied placement id"] = function() local png_bytes = "hi" - local sequences = - terminal.build_transmit(png_bytes, { id = ID, placement = { columns = 10, rows = 5 } }) + local sequences = terminal.build_transmit( + png_bytes, + { id = ID, placement = { placement_id = 7, columns = 10, rows = 5 } } + ) local payload = vim.base64.encode(png_bytes) local expected = ESC .. "_Ga=T,f=100,t=d,i=" .. ID - .. ",q=2,p=" - .. terminal.PLACEMENT_ID - .. ",c=10,r=5,m=0;" + .. ",q=2,p=7,c=10,r=5,m=0;" .. payload .. ESC .. "\\" MiniTest.expect.equality(sequences, { expected }) end +T["build_transmit"]["placement without a placement_id is rejected"] = function() + local ok = pcall(terminal.build_transmit, "hi", { id = ID, placement = { columns = 10 } }) + MiniTest.expect.equality(ok, false) +end + T["build_transmit"]["combined transmit+placement carries crop keys before c=/r="] = function() local png_bytes = "hi" local sequences = terminal.build_transmit(png_bytes, { id = ID, - placement = { crop_x = 1, crop_y = 2, crop_w = 3, crop_h = 4, columns = 10, rows = 5 }, + placement = { + placement_id = 1, + crop_x = 1, + crop_y = 2, + crop_w = 3, + crop_h = 4, + columns = 10, + rows = 5, + }, }) local payload = vim.base64.encode(png_bytes) local expected = ESC .. "_Ga=T,f=100,t=d,i=" .. ID - .. ",q=2,p=" - .. terminal.PLACEMENT_ID - .. ",x=1,y=2,w=3,h=4,c=10,r=5,m=0;" + .. ",q=2,p=1,x=1,y=2,w=3,h=4,c=10,r=5,m=0;" .. payload .. ESC .. "\\" @@ -127,7 +138,7 @@ end T["build_transmit"]["deterministic across repeated calls"] = function() local png_bytes = "some bytes" - local opts = { id = ID, placement = { columns = 10, rows = 5 } } + local opts = { id = ID, placement = { placement_id = 1, columns = 10, rows = 5 } } local first = terminal.build_transmit(png_bytes, opts) local second = terminal.build_transmit(png_bytes, opts) MiniTest.expect.equality(first, second) @@ -136,61 +147,56 @@ end T["build_placement"] = MiniTest.new_set() T["build_placement"]["minimal"] = function() - local seq = terminal.build_placement(ID) - MiniTest.expect.equality( - seq, - ESC .. "_Ga=p,i=" .. ID .. ",p=" .. terminal.PLACEMENT_ID .. ESC .. "\\" - ) + local seq = terminal.build_placement(ID, { placement_id = 3 }) + MiniTest.expect.equality(seq, ESC .. "_Ga=p,i=" .. ID .. ",p=3" .. ESC .. "\\") +end + +T["build_placement"]["requires a positive integer placement_id"] = function() + local ok = pcall(terminal.build_placement, ID, {}) + MiniTest.expect.equality(ok, false) end T["build_placement"]["with options in canonical order"] = function() - local seq = - terminal.build_placement(ID, { columns = 10, rows = 5, z_index = 3, no_move_cursor = true }) - MiniTest.expect.equality( - seq, - ESC .. "_Ga=p,i=" .. ID .. ",p=" .. terminal.PLACEMENT_ID .. ",c=10,r=5,z=3,C=1" .. ESC .. "\\" + local seq = terminal.build_placement( + ID, + { placement_id = 3, columns = 10, rows = 5, z_index = 3, no_move_cursor = true } ) + MiniTest.expect.equality(seq, ESC .. "_Ga=p,i=" .. ID .. ",p=3,c=10,r=5,z=3,C=1" .. ESC .. "\\") end T["build_placement"]["source-rect crop keys precede target cell box, in fixed order"] = function() local seq = terminal.build_placement( ID, - { crop_x = 5, crop_y = 10, crop_w = 20, crop_h = 30, columns = 4, rows = 6 } + { placement_id = 3, crop_x = 5, crop_y = 10, crop_w = 20, crop_h = 30, columns = 4, rows = 6 } ) MiniTest.expect.equality( seq, - ESC - .. "_Ga=p,i=" - .. ID - .. ",p=" - .. terminal.PLACEMENT_ID - .. ",x=5,y=10,w=20,h=30,c=4,r=6" - .. ESC - .. "\\" + ESC .. "_Ga=p,i=" .. ID .. ",p=3,x=5,y=10,w=20,h=30,c=4,r=6" .. ESC .. "\\" ) end T["build_placement"]["crop_x/crop_y of 0 are emitted, not treated as absent"] = function() - local seq = terminal.build_placement(ID, { crop_x = 0, crop_y = 0, crop_w = 5, crop_h = 5 }) + local seq = terminal.build_placement( + ID, + { placement_id = 3, crop_x = 0, crop_y = 0, crop_w = 5, crop_h = 5 } + ) MiniTest.expect.equality(seq:find(",x=0,", 1, true) ~= nil, true) MiniTest.expect.equality(seq:find(",y=0,", 1, true) ~= nil, true) end T["build_placement"]["no crop keys when omitted, unchanged from existing placement bytes"] = function() - local seq = terminal.build_placement(ID, { columns = 10, rows = 5 }) + local seq = terminal.build_placement(ID, { placement_id = 3, columns = 10, rows = 5 }) MiniTest.expect.equality(seq:find(",x=", 1, true), nil) MiniTest.expect.equality(seq:find(",y=", 1, true), nil) MiniTest.expect.equality(seq:find(",w=", 1, true), nil) MiniTest.expect.equality(seq:find(",h=", 1, true), nil) end -T["build_placement"]["always uses the same fixed placement id across calls"] = function() - local first = terminal.build_placement(ID, { columns = 10, rows = 5 }) - local second = terminal.build_placement(ID, { columns = 12, rows = 6 }) - local function placement_id(sequence) - return sequence:match(",p=(%d+),") - end - MiniTest.expect.equality(placement_id(first), placement_id(second)) +T["build_placement"]["a different placement_id on the same image id fans out (issue #10)"] = function() + local first = terminal.build_placement(ID, { placement_id = 1, columns = 10, rows = 5 }) + local second = terminal.build_placement(ID, { placement_id = 2, columns = 10, rows = 5 }) + MiniTest.expect.equality(first:find(",p=1,", 1, true) ~= nil, true) + MiniTest.expect.equality(second:find(",p=2,", 1, true) ~= nil, true) end T["build_delete"] = MiniTest.new_set() @@ -205,8 +211,13 @@ T["build_delete"]["free_data uses d=I"] = function() MiniTest.expect.equality(seq, ESC .. "_Ga=d,d=I,i=" .. ID .. ESC .. "\\") end +T["build_delete"]["placement_id scopes the delete to one placement (issue #10)"] = function() + local seq = terminal.build_delete(ID, { placement_id = 5 }) + MiniTest.expect.equality(seq, ESC .. "_Ga=d,d=i,i=" .. ID .. ",p=5" .. ESC .. "\\") +end + T["build_delete"]["never emits delete-all"] = function() - for _, opts in ipairs({ nil, { free_data = true }, { free_data = false } }) do + for _, opts in ipairs({ nil, { free_data = true }, { free_data = false }, { placement_id = 5 } }) do local seq = terminal.build_delete(ID, opts) MiniTest.expect.equality(seq:find("d=a", 1, true), nil) end From e4d59d80c4510e0ad334c5643b52b1076feb55fe Mon Sep 17 00:00:00 2001 From: Hisanari Kikuchi Date: Sun, 16 Aug 2026 16:57:46 +0900 Subject: [PATCH 2/2] fix: retry destroy-path deletes for fan-out handles outside shutdown 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. --- docs/spec/renderer-placement.md | 17 ++++++--- lua/blit/renderer.lua | 51 ++++++++++++++++++--------- tests/test_renderer.lua | 62 +++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 22 deletions(-) diff --git a/docs/spec/renderer-placement.md b/docs/spec/renderer-placement.md index 1876a0b..7818eaf 100644 --- a/docs/spec/renderer-placement.md +++ b/docs/spec/renderer-placement.md @@ -524,11 +524,18 @@ Two distinct kinds of state transition, kept separate: indefinitely behavior, so a terminal that never honors the delete can't keep the debounce timer (and therefore the "fully quiescent idle" guarantee) alive forever. The full-free branch (id actually freed) is - excluded from queuing: for `VimLeavePre` specifically, Neovim is exiting - right after, so a queued retry has nothing meaningful left to protect and - only risks racing a reused id against a process that's already gone; for - a fatal `M.show()` failure, nothing else references the brand-new id - either. If `find_reusable_entry` reclaims a still-queued id for a fresh + always excluded from queuing — nothing else references that id anymore. + The fan-out-sibling-still-shares-the-id case is excluded from queuing + only when the caller is truly shutting down (`opts.shutting_down`, set by + `VimLeavePre` and the test-only `_reset()`): the process (or test run) is + exiting right after, so a queued retry has nothing meaningful left to + protect and only risks racing a reused id against a process that's + already gone. A fatal `M.show()` failure tearing down a fan-out handle + whose sibling is still live does NOT set `shutting_down` — the process + keeps running and no future `redraw_all()` pass will ever revisit this + specific `(id, placement_id)` again once the handle is gone, so it gets + the same bounded retry as everyday teardown instead (issue #10). If + `find_reusable_entry` reclaims a still-queued id for a fresh placement before its retries are spent, the queued entries for that id are cancelled (`cancel_pending_delete`) — though even without that, a late retry naming the OLD `placement_id` could never hit the new placement's diff --git a/lua/blit/renderer.lua b/lua/blit/renderer.lua index 34d3992..83f927d 100644 --- a/lua/blit/renderer.lua +++ b/lua/blit/renderer.lua @@ -980,10 +980,20 @@ end -- the full-free branch, so the id and its data are still guaranteed to be -- fully released by the time VimLeavePre finishes — it just may not be -- THIS particular call that does it. +-- +-- `shutting_down` is a separate axis from `free_data`: it's only true for +-- the true end-of-session call sites (`VimLeavePre`, the test-only +-- `_reset()`), where skipping the destroy-path delete retry queue below is +-- safe because nothing is left running to revisit a dropped delete anyway. +-- A `free_data = true, shutting_down = false` call (M.show()'s own failure +-- paths tearing down a fan-out handle whose sibling is still live) keeps +-- the retry: the process keeps running and no future redraw pass will ever +-- revisit this specific (id, placement_id) again once the handle is gone. ---@param handle blit.Handle ----@param opts? { free_data?: boolean } +---@param opts? { free_data?: boolean, shutting_down?: boolean } local function destroy_handle(handle, opts) local free_data_requested = (opts and opts.free_data) or false + local shutting_down = (opts and opts.shutting_down) or false if vim.api.nvim_buf_is_valid(handle.buf) then pcall(vim.api.nvim_buf_del_extmark, handle.buf, ensure_namespace(), handle.extmark_id) @@ -1011,26 +1021,33 @@ local function destroy_handle(handle, opts) M._write_fn({ terminal.build_delete(handle.id, { placement_id = handle.placement_id }) }) - if free_data_requested then + if free_data_requested and shutting_down then -- A sibling placement still shares this id; VimLeavePre's own remaining -- iterations will eventually take the full-free branch above once the - -- last one goes. No retry queued here, matching the exclusion below: - -- Neovim is exiting right after, so a queued retry has nothing - -- meaningful left to protect. + -- last one goes. No retry queued here: Neovim is exiting right after, + -- so a queued retry has nothing meaningful left to protect. This is + -- distinct from the free_data_requested-but-not-shutting_down case + -- below (a fatal M.show() failure tearing down a fan-out handle mid- + -- session) — there, the process keeps running and no future redraw + -- pass will ever revisit this specific (id, placement_id) again, so a + -- dropped delete would otherwise orphan the placement for as long as + -- the sibling stays alive (issue #10). return end -- Everyday teardown (clear()/clear_all(), BufWinLeave, WinClosed, - -- BufWipeout) never frees data: the id's cache entry stays around — - -- idle if this was the last handle sharing it, still active otherwise — - -- so a later show() of the same file (or a sibling fan-out placement) - -- stays cheap. A late retry always still refers to either this same dead - -- placement or nothing (cancelled via cancel_pending_delete if - -- find_reusable_entry reclaims the id first); it can never hit a - -- DIFFERENT live placement, since placement ids are never reused (see - -- "Placement id allocation" above). VimLeavePre's free_data=true path is - -- excluded above for the same reason as the full-free branch: the - -- process is exiting right after. + -- BufWipeout, or a fatal M.show() failure on a fan-out handle whose + -- sibling is still live) never frees data here: the id's cache entry + -- stays around — idle if this was the last handle sharing it, still + -- active otherwise — so a later show() of the same file (or a sibling + -- fan-out placement) stays cheap. A late retry always still refers to + -- either this same dead placement or nothing (cancelled via + -- cancel_pending_delete if find_reusable_entry reclaims the id first); it + -- can never hit a DIFFERENT live placement, since placement ids are never + -- reused (see "Placement id allocation" above). Only the true-shutdown + -- case above (VimLeavePre, `_reset()`) is excluded from queuing: the + -- process/test run is ending right after, so a queued retry has nothing + -- meaningful left to protect. table.insert(pending_deletes, { id = handle.id, placement_id = handle.placement_id, @@ -1093,7 +1110,7 @@ end local function on_vim_leave_pre() local handles = vim.list_extend({}, M._handles) for _, handle in ipairs(handles) do - destroy_handle(handle, { free_data = true }) + destroy_handle(handle, { free_data = true, shutting_down = true }) end maybe_teardown_autocmds() terminal.reset_writer() @@ -1412,7 +1429,7 @@ end function M._reset() local handles = vim.list_extend({}, M._handles) for _, handle in ipairs(handles) do - destroy_handle(handle, { free_data = true }) + destroy_handle(handle, { free_data = true, shutting_down = true }) end M._handles = {} -- Discard any still-outstanding destroy-path retries (see "Destroy-path diff --git a/tests/test_renderer.lua b/tests/test_renderer.lua index eb443ff..bc4adf0 100644 --- a/tests/test_renderer.lua +++ b/tests/test_renderer.lua @@ -1107,6 +1107,68 @@ T["redraw"]["ghostty: retries a failed retransmit without a further resize event MiniTest.expect.no_equality(handle.id, original_id) end +T["redraw"]["ghostty: migrates a whole fanned-out group sharing a stale id together (issue #10)"] = function() + renderer._detect_fn = function() + return { terminal = "ghostty", tmux = false, gui_embed = false, supported = true } + end + local buf, win = setup_floating(numbered_lines(10), 20, 10) + local first = renderer.show( + tmp_path, + { width = 5, height = 3, buf = buf, win = win, lnum = 2, debounce_ms = 5 } + ) + local second = renderer.show( + tmp_path, + { width = 5, height = 3, buf = buf, win = win, lnum = 4, debounce_ms = 5 } + ) + MiniTest.expect.equality(first.visible, true) + MiniTest.expect.equality(second.visible, true) + MiniTest.expect.equality(first.id, second.id) + local original_id = first.id + + local original_columns = vim.o.columns + vim.o.columns = original_columns + 1 + captured = {} + vim.api.nvim_exec_autocmds("WinResized", { pattern = tostring(win) }) + + vim.wait(1000, function() + return any_captured_has("a=T") + end) + vim.o.columns = original_columns + + -- Both siblings migrate onto the SAME fresh id, atomically: exactly one + -- a=T (the shared pixel data, sent once — not re-transmitted per + -- handle), exactly one a=p placing the other sibling onto that same new + -- id, and the old id freed exactly once, not per-handle. + MiniTest.expect.equality(first.visible, true) + MiniTest.expect.equality(second.visible, true) + MiniTest.expect.equality(first.id, second.id) + MiniTest.expect.no_equality(first.id, original_id) + + local transmit_count, free_count, sibling_placed = 0, 0, false + for _, seq in ipairs(captured) do + local all = table.concat(seq, "") + if all:find("a=T", 1, true) then + transmit_count = transmit_count + 1 + end + if all:find("d=I", 1, true) and all:find("i=" .. original_id, 1, true) then + free_count = free_count + 1 + end + if + all:find("a=p", 1, true) + and all:find("i=" .. first.id, 1, true) + and ( + all:find(",p=" .. second.placement_id, 1, true) ~= nil + or all:find(",p=" .. first.placement_id, 1, true) ~= nil + ) + then + sibling_placed = true + end + end + MiniTest.expect.equality(transmit_count, 1) + MiniTest.expect.equality(free_count, 1) + MiniTest.expect.equality(sibling_placed, true) +end + T["redraw"]["ghostty: does not retransmit a still-visible handle when size is unchanged"] = function() renderer._detect_fn = function() return { terminal = "ghostty", tmux = false, gui_embed = false, supported = true }