Skip to content

fix(popup): stop repainting the whole popup on every child write - #233

Open
jiunbae wants to merge 3 commits into
Helvesec:mainfrom
Open330:fix/popup-paint-row-diff
Open

jiunbae wants to merge 3 commits into
Helvesec:mainfrom
Open330:fix/popup-paint-row-diff

Conversation

@jiunbae

@jiunbae jiunbae commented Oct 17, 2026 •

Copy link
Copy Markdown

This replaces #226, which I closed by mistake. That PR carried all of these fixes at once; they are now split into focused PRs (#233 popup paint, #234 popup background repaint, #235 destroy-unattached after switch; a renderer perf PR follows once #233 lands).

Problem

An updating display-popup is expensive and flickers. Three things combine:

  1. Each popup refresh first painted a rectangle of spaces over the whole popup, then painted the content rows over it. A terminal reading the stream incrementally (a PTY, SSH) can show that blank surface as a flash.
  2. Each refresh sent the full popup as a persistent overlay frame. A one-character clock or spinner update in a large popup resent every row. This is also the frame flood behind display-popup: large popup paint floods attach control backlog and detaches the client #216.
  3. On Unix the popup reader refreshed after every 8 KiB PTY read. A TUI frame that arrives in several writes was rendered once per read, and the intermediate states were sent too.

Change

Three commits, one for each cause:

  • fix(renderer): avoid blanking popups before painting content. Content rows are painted in their final state. Each surface row is padded after its text with the popup style, through the existing ANSI- and grapheme-aware truncator, so shorter and missing rows still erase old cells.
  • fix(renderer): send only changed popup rows on child-output refreshes. Popup reader refreshes are marked row-diffable. At the attach output boundary, after the generation checks, the frame is compared with the last full persistent frame actually emitted, and only changed rows are written. Identical frames write nothing. Only frames made entirely of self-contained rows at one column, in strictly increasing order, qualify. Anything else falls back to the full frame: moved or resized rows, nested menus, transient messages. Explicit client refreshes are never diffed. The cache always keeps the full frame, so refresh-client, resize, queue replacement and restore never depend on a chain of deltas.
  • fix(popup): batch Unix popup PTY reads into one refresh. After the first byte arrives, the reader waits a fixed 8 ms and drains up to 256 KiB before one refresh. An idle popup does not wake, and sustained output cannot extend the deadline. Windows keeps its existing reader.

Related to #216. This makes far fewer and much smaller overlay frames, but it does not change how the attach control backlog is charged or pruned, so I am not claiming it closes that issue.

Bordered popups (border edges use a different record form) still take the full-frame path here. A follow-up PR stacked on this one handles them.

Tests

  • renderer::overlay tests: no all-blank prepaint, each row positioned once, and a screen-model test that short Unicode/combining rows and missing rows erase old text.
  • pane_io::persistent_overlay::popup_delta_tests: replaying the delta over the previous frame gives the same screen as the full new frame, including shortened coloured wide-character rows and emptied rows. Identical frames give an empty delta, but the cache keeps the full frame. Restore, hidden overlay, moved rows, resized frames, overlapping drawing and explicit refreshes all fall back.
  • popup_reader_batches_split_output_without_waiting_for_eof (Unix only): two writes 1 ms apart come back as one batch, and an idle reader does not return.

Earlier Linux measurement replaying one recorded dashboard popup stream (140x40 client, borderless 99%-height popup, 6 s samples). It compares a build that already had the first commit against one with all three, so it measures the row diff and batching only. Terminal output went from 137,717 B / 663 row draws to 752 B / 4 with status off, and from 138,350 B to 1,385 B with status on. Daemon CPU differences were within one accounting tick.

Platforms tested

Windows 11 (x86_64, native, MSVC, toolchain from rust-toolchain.toml):

  • cargo build --workspace, cargo clippy --workspace --all-targets -- -D warnings and cargo fmt --all -- --check pass.
  • The tests added or changed here pass.
  • The full cargo test --workspace --all-targets is not green on Windows for unmodified main either (on my machine main itself fails a large set of real-daemon integration tests, and the rmux-server lib tests overflow their stack unless RUST_MIN_STACK is raised), so I did not use it as a gate here.

The Unix-only batching code and its test were not built or run on this machine; the Linux numbers above come from earlier work on this change.

🤖 Generated with Claude Code

An updating display-popup painted a rectangle of spaces over its whole
surface before repainting the content rows. Even a one-character TUI update
therefore emitted a complete blank popup followed by the real rows, and a
terminal consuming the stream incrementally (a PTY, SSH) can show that
blank intermediate surface as a flash.

Paint each content row directly in its final state instead. Surface rows
are padded after their text with the popup style, through the existing
ANSI- and grapheme-aware truncator, so shorter and missing rows still erase
stale cells. Static text rows use the existing fixed-width renderer for
every row. Borders and process SGR are unchanged.

Signed-off-by: Jiun Bae <jiunbae.623@gmail.com>
Every read from a popup's PTY re-rendered the whole popup and sent it to
the attach client as a full persistent overlay frame. A small clock or
spinner update in a large popup therefore repainted every row each time,
which both floods the attach control channel (see Helvesec#216) and gives
terminals many chances to show a partially drawn popup.

Mark the frames produced by popup reader refreshes as row-diffable. At
the attach output boundary, after the generation checks, compare such a
frame with the last full persistent frame that was actually emitted and
write only the rows that changed. Identical frames produce no output.

The comparison is deliberately conservative. Only frames made entirely
of self-contained rows (cursor save, SGR reset, absolute position, row,
SGR reset, cursor restore) at one column and strictly increasing rows
qualify; a changed row carries its padding, so shortened text still
erases old cells. Moved or resized rows, nested menus, transient
messages and any other layout fall back to the full frame. Explicit
client refreshes are never diffed, and the cache always keeps the full
new frame, so queue replacement, refresh-client, resize and restoring a
popup after other output never depend on a chain of deltas.

Signed-off-by: Jiun Bae <jiunbae.623@gmail.com>
The Unix popup reader refreshed the popup after every 8 KiB read. A TUI
frame that reaches the PTY in several writes was therefore rendered and
sent once per read, exposing intermediate states and multiplying the
overlay frames queued for the client.

After the first byte of a batch arrives, wait a fixed 8 ms and drain what
is readable, up to 256 KiB, before refreshing once. The window starts
only once data arrives, so an idle popup does not wake periodically, and
sustained output cannot extend the deadline: bytes beyond the limit are
left for the next batch. Windows keeps its existing blocking reader.

Signed-off-by: Jiun Bae <jiunbae.623@gmail.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant