FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Redraw damage rectangles by mgrant0 · Pull Request #5516 · tmux/tmux · GitHub

/ tmux Public

Redraw damage rectangles - #5516

Open
mgrant0 wants to merge 13 commits into
masterfrom
redraw-damage-rectangles
Open

Redraw damage rectangles#5516
mgrant0 wants to merge 13 commits into
masterfrom
redraw-damage-rectangles

Conversation

mgrant0 commented Aug 23, 2026

Copy link
Copy Markdown
Contributor
  1. layout — only mark scrollbar for redraw when the pane actually changed
  2. screen-redraw, screen-write — damage-rectangle tracking and composition
  3. window — floating-pane damage/redraw helpers
  4. server-client — consume window damage during the normal redraw pass
  5. window — redraw only borders/status on active-pane change
  6. cmd-join-pane, cmd-split-window, cmd-resize-pane — use shared floating-pane redraw
  7. popup — damage-based redraw during drag/resize

  wp->flags |= PANE_REDRAWSCROLLBAR was set unconditionally whenever a pane
  reserved a scrollbar, even if layout_fix_panes() left its geometry
  completely unchanged - forcing a needless scrollbar redraw on every layout
  pass. Move it inside the existing "did this pane's geometry actually
  change" check.
…ition

Introduce a per-window list of damaged rectangles (struct redraw_damage)
and redraw_damage_window() to record them, redraw_client_damage() to
consume them by composing exactly the damaged cells (via a new
redraw_draw_damage_rect(), which also force-refreshes any pane-status
span it touches, since window_make_pane_status()'s content-diff check
has no way to know the physical cells were disturbed by something else).

redraw_draw_span() now takes an explicit [clip_x, clip_x + clip_n) range
instead of always drawing a span's full width, so a damage rectangle can
redraw just the portion of a span it actually covers.

screen_write_redraw_cb() - the fallback when a write can't be applied
directly to the terminal - now reports damage for just the affected row
via this mechanism, instead of unconditionally flagging the whole pane
for a full redraw.
window_pane_redraw_floating() reports damage for only a floating pane's
old and new rectangle (via redraw_damage_window(), grown by one cell to
cover its border frame - see the "floating" case in screen-redraw.c),
instead of the caller falling back to a full window redraw. Any other
pane whose *scrollbar strip* - not its whole body - intersects either
rectangle still gets PANE_REDRAWSCROLLBAR directly, since scrollbars
aren't covered by the damage system.
redraw_client_damage() (added previously, unused until now) is called
from server_client_check_redraw()'s normal redraw pass, and
server_client_any_pane_redraw() now also checks for pending window
damage so a client with only damage (no PANE_REDRAW/PANE_REDRAWSCROLLBAR
flags) still gets its redraw pass run.

server_client_check_redraw() now returns whether the redraw was deferred
(waiting for outstanding tty output to drain) rather than performed. A
deferred redraw no longer escalates to a full CLIENT_REDRAWWINDOW to
avoid losing what was pending - server_client_loop() now only clears
PANE_REDRAW, PANE_REDRAWSCROLLBAR and window damage once every client
viewing a window actually drew this pass (tracked via a new per-window
redraw_deferred flag), otherwise they're left in place and retried in
their normal, narrowly-scoped form.

server_client_set_session() now redraws only if the client's session or
current window actually changed, not on every call (e.g. switch-client
-t= from clicking a pane name in the status line resolves here even
when nothing besides the active pane changed).

A drag callback's mouse_drag_update() now opens a sync region itself
(tty_sync_start()) before its first write, so a fast-path write it makes
directly and a later correction arriving via redraw_client_damage() end
up in the same atomic terminal update instead of two visible frames.
window_set_active_pane() unconditionally called server_redraw_window(w)
on every active-pane change, redrawing every pane's content even though
only the previous and new active pane's border/status appearance
actually changed. Unzooming (which does change every pane's geometry)
still gets the full redraw; otherwise this now only redraws borders and
status.
…pane

redraw instead of full client redraw

The three interactive mouse-drag paths that move or resize a floating
pane (move-pane -M's Alt-drag, split-window/new-pane's interactive
resize, and resize-pane's own border drag) each unconditionally called
server_redraw_window(w), redrawing every pane in the window for a change
that only ever disturbs the floating pane's own old and new rectangle.
Switch all three to window_pane_redraw_floating().
…redraw

popup_handle_drag()'s MOVE and SIZE branches each called
server_redraw_client(c) unconditionally, redrawing the client's entire
window on every drag step. Report damage for just the popup's old and
new rectangle instead, via a new popup_damage() (translating from raw
client/tty coordinates into window coordinates), and set
CLIENT_REDRAWOVERLAY so the popup itself still redraws.
github-project-automation Bot moved this to Not Started in Open Issues & PRs Aug 23, 2026
nicm moved this from Not Started to In Progress in Open Issues & PRs Aug 24, 2026

Copy link
Copy Markdown

Thanks for working on this. I am exploring composite views of live panes that remain in their existing sessions, so the narrower redraws in this PR are useful for that work.

I tested the current head (f66eeef8a) and pushed four follow-up commits here:

redraw-damage-rectangles...darlingm:pr5516-regression-fixes

They add regression coverage for:

  • damage-only redraw passes
  • full and regional screen-write fallbacks
  • popup move and resize damage
  • damage shared by multiple clients

They also make two related changes.

First, the fallback added in f66eeef8a covers a case that already occurs, rather than only a possible future case. screen_write_redraw_cb() can record window damage without setting client redraw flags. More generally, damage belongs to the window while redraw flags belong to each client. The paired regression test reproduces this with a panned client. The follow-up keeps the same behavior but handles damage once after the flag-specific redraw work.

Second, the screen-write callback currently records only the cursor row even when the failed operation needs a full or regional redraw. The follow-up passes the affected row range to the callback. This fixes incomplete alternate-screen restoration and the other full and regional fallback paths.

All four tests pass on the follow-up branch.

I have not opened another PR yet. I can open a companion PR against redraw-damage-rectangles, or you can cherry-pick or fold the commits into this branch as you prefer.

Copy link
Copy Markdown

Separate integration note: the current conflict with master is in window_set_active_pane().

I think the combined logic should track whether unzoom actually occurred:

int		    did_unzoom = 0;

/* ... */

if ((w->flags & WINDOW_ZOOMED) && !window_pane_is_visible(wp)) {
	window_unzoom(w, 1);
	did_unzoom = 1;
}

/* ... */

if (did_unzoom)
	server_redraw_window(w);
else {
	server_redraw_window_borders(w);
	server_status_window(w);
}

Master preserves zoom when switching to a visible -A or modal floating pane. This PR needs a full redraw only when unzoom changed the pane geometry. Tracking the actual unzoom preserves both behaviors.

I left this resolution out of the follow-up branch so its diff remains limited to the tests and fixes above.

Copy link
Copy Markdown

I need to correct my earlier note. The first four follow-up commits were valid, but they were not complete. I had already reproduced five more regressions and failed to carry them into the branch or comment.

I updated the same companion branch:

redraw-damage-rectangles...darlingm:pr5516-regression-fixes

It now adds five more fix-and-test commits for:

  • switching windows within the same session without redrawing the client
  • damage edges splitting double-width characters
  • damage redraw replacing pane prompts with pane content
  • popup movement leaving old borders on bottom or multi-line top status lines
  • floating-pane geometry changes leaving status formats stale

Each new test passes current master (e5db7598b) and fails f66eeef8a plus the original four follow-up commits. The fixes are folded into the corresponding test commits. All nine focused tests pass on the updated branch.

I preserved the exact four-commit version I linked earlier as pr5516-regression-fixes-v1.

The branch still starts at f66eeef8a. It intentionally does not duplicate master’s existing b2de80387 synchronized-output fix or resolve the master conflict; those should come from rebasing onto master. This keeps the companion diff limited to PR 5516 review findings.

Two design concerns are not included in this tested patch set: window-global deferred damage can make fast clients repeat work while another client is blocked, and pane-global PANE_NEWSTATUS can cache client-specific status content. Those need a separate design decision rather than a narrow fix.

mgrant0 and others added 3 commits August 25, 2026 08:08
From github.com/darlingm/tmux, branch pr5516-regression-fixes. Adds 9
regression tests covering gaps found in the redraw-damage-rectangles
branch: screen-write full/region redraw fallback, same-session window
switches, wide-character clipping at damage edges, pane prompts and
status lines surviving damage, floating-pane status format refresh,
and multi-client damage delivery.

redraw-multiclient.sh is adapted here to use ASCII pane borders
(pane-border-lines simple) instead of darlingm's original UTF-8
borders: the original reliably "failed" under this test's nested
tmux-in-tmux harness (relaying through an outer tmux client) due to
that harness mis-rendering a cell that held a multi-byte UTF-8 border
character being overwritten by later plain content - confirmed to be
a nested-relay artifact, not a real bug, by replaying the identical
drag sequence against a real terminal (xterm), where it never
reproduces. ASCII borders avoid the artifact; the test still reliably
catches the real "damage consumed by only one client" bug it targets
(verified by reintroducing that bug and confirming the test fails).

The other 8 tests are added verbatim from darlingm's branch. Four of
them (popup-drag-status-line.sh, popup-drag-wide-character.sh,
popup-drag-pane-prompt.sh, switch-client-redraw.sh) currently FAIL on
this branch, since the source fixes they test for have not been
merged yet - only the tests are being added here.

Co-Authored-By: Michael K. Darling <darlingm@gmail.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fixes the four bugs caught by the regression tests added in aed1209
(popup-drag-status-line.sh, popup-drag-wide-character.sh,
popup-drag-pane-prompt.sh, switch-client-redraw.sh), based on fixes
from Michael K. Darling (github.com/darlingm/tmux, pr5516-regression-
fixes), reviewed and adapted:

- server-client.c: server_client_set_session()'s check for whether the
  client's window actually changed compared old->curw to s->curw, but
  when old == s these read the same, already-updated field, so a
  same-session window switch was never detected. Compare against the
  client's own cached redraw scene instead (redraw_client_has_window(),
  new in screen-redraw.c/tmux.h). Taken from darlingm as-is.

- popup.c: popup_damage() only translated a popup's client-coordinate
  rectangle into window coordinates, so a popup dragged across the
  status line never triggered a status-line redraw once it moved away -
  status_redraw()'s own "skip if content unchanged" optimization
  suppressed it, since only the popup moved, not the status content.
  Now detects overlap with the status line and forces a redraw via the
  existing (previously unused) CLIENT_REDRAWSTATUSALWAYS flag, and
  properly clips the reported rectangle to the pane area for
  status-at-top/bottom/off. Taken from darlingm as-is.

- screen-redraw.c: redraw_draw_damage_rect() clipped a span to a damage
  rectangle's raw geometric edges, which have no idea what's in the
  grid, so a clip edge could land mid-character and tear a wide
  character in half. Added redraw_damage_grow_span_clip(): widen the
  clip by one cell on each edge that isn't already at the span's own
  boundary. Reimplemented simpler than darlingm's version (which walked
  grid cells per span type via a switch and direct grid lookups) -
  since no grid cell is ever wider than two columns, an unconditional
  one-cell margin is always enough to pull a split character back in,
  with no need to inspect grid content at all.

- screen-redraw.c: redraw_draw_damage_rect() also never re-overlaid a
  pane's active in-pane prompt after drawing its underlying content, so
  damage crossing a prompt row erased it until an unrelated redraw
  restored it. Factored the existing full-redraw prompt-building code
  into a shared redraw_make_pane_prompt() helper and added
  redraw_damage_draw_pane_prompt(), which recomposes the prompt over
  the drawn range. Taken from darlingm as-is.

All 9 regression tests in regress/ now pass. redraw_damage_grow_span_clip
was verified independently by disabling it and confirming
popup-drag-wide-character.sh reproduces its original failure.

Co-Authored-By: Michael K. Darling <darlingm@gmail.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

mgrant0 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for your tests, I've included them all now. I ran things through claude this morning. There was an issue with redraw-multiclient.sh which was failing. Here is what it said and did:

Summary of the change to redraw-multiclient.sh for your reply:


Change: Switched the test to ASCII pane borders (pane-border-lines with plain ascii characters, e.g. -,|, and +) instead of the default UTF-8 box-drawing borders.

Why: The original test reliably "failed" — but I (Claude) tracked it down and it wasn't a real bug. This test nests a real tmux
client inside another tmux's pane to get a genuine terminal to capture from. That nested-tmux relay has a rendering
artifact: when a cell that previously held a multi-byte UTF-8 border character (┌, ─, etc.) gets overwritten later by plain
content, the outer tmux instance's own interpretation of that byte stream mis-renders it — independent of anything the inner
tmux actually sends (verified the inner server's escape sequences byte-for-byte correct in both the passing and failing
cases). Confirmed it's a harness artifact, not a tmux bug, by replaying the identical mouse-drag sequence against a real
terminal (xterm) instead of nested tmux — never reproduces there. ASCII borders sidestep the multi-byte-overwrite path
entirely, since +/-/| are single-byte.

Mechanical changes needed to support that:

  • wait_outer_has ... '┌' → wait_outer_has ... '+'
  • assert_scene's frame-count check: grep -o '┌' | wc -l expecting 1 → grep -o '+' | wc -l expecting 4, since ASCII borders
    use the same + for every corner/junction (no distinct top-left glyph like ┌), so one rectangular frame always draws
    exactly 4 of them instead of 1.

Still verified to catch the real bug: reintroduced the actual "damage consumed by only one client" defect (made the damage
list get freed by whichever client draws first, instead of after all clients have had a turn) and confirmed the test fails
reliably against that, with the exact symptom the test's own comment describes — so it still does its job, just without the
nested-terminal false positive.

mgrant0 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

All 9 regression tests now pass. Summary of what I did with each of darlingm's 4 fixes:

Took essentially as-is (2 of 4):

  • switch-client-redraw fix (server-client.c + new redraw_client_has_window() in screen-redraw.c/tmux.h) — this is a real bug
    in code I wrote: old->curw != s->curw is always false when old == s, since both sides read the same already-mutated
    field. Darlingm's fix (compare against the client's own cached scene instead) is the correct, minimal way to sidestep that
    aliasing. No changes made.
  • popup-drag-status-line fix (popup.c + two small screen-redraw.c tweaks) — reuses a flag (CLIENT_REDRAWSTATUSALWAYS) that already existed in the codebase, unused, with a name that exactly matches what's needed here. The clip-to-pane-area logic correctly generalizes to status-at-top/bottom/off, which the original popup_damage() never handled for the bottom case. Took it as-is.

Took the approach, reimplemented simpler (1 of 4):

  • Wide-character clipping fix — darlingm's redraw_damage_expand_span() walks the grid cell-by-cell with a switch over span type (PANE/STATUS/MENU), fetching each type's own screen/offset. I replaced it with redraw_damage_grow_span_clip():
    unconditionally widen the clip by one cell on each edge, clamped to the span's own bounds — no grid lookup, no type
    switch, ~8 lines instead of ~50. This works because tmux's grid caps any character at 2 columns wide, so a flat ±1 margin
    is always sufficient to pull back a split character, regardless of what's actually there. I verified this isn't just
    theoretically sound — I disabled it, confirmed popup-drag-wide-character.sh fails with the exact original symptom (torn
    CJK glyphs), then re-enabled it and confirmed the fix.

Took the approach, applied as-is with no changes (1 of 4):

  • Pane-prompt-after-damage fix (redraw_make_pane_prompt() extracted as a shared helper, redraw_damage_draw_pane_prompt() added) — this is a legitimate dedup with correct logic (the row-selection math exactly mirrors the existing full-redraw path, and it correctly respects overlay clipping). Nothing to improve here.

All 4 previously-failing tests pass now (3/3 runs each), the other 5 still pass, and the two pre-existing floating-pane tests (floating-pane-drag-ghost.sh, floating-pane-drag-scrollbar-strip.sh) still pass.

Two more fixes based on Michael K. Darling's branch
(github.com/darlingm/tmux, pr5516-regression-fixes), taken as-is -
neither is caught by any test in regress/ yet, found by code review
rather than a failing test:

- screen_write_redraw_cb() (screen-write.c) reported damage for only a
  single row, using ttyctx->ocy as if every fallback redraw were a
  single-cell write. But it's also the callback for cases that can
  legitimately span many rows - a large scroll-region fallback
  (tty_redraw_region(), when tty_large_region() or the pane is
  obscured), a full reset, and entering/leaving the alternate screen.
  For those, only the top row of the affected area ever got marked as
  damaged, leaving the rest stale until an unrelated redraw happened to
  cover it. Changed the shared tty_ctx_redraw_cb typedef to carry
  (py, ny) - the actual row range - and updated every call site to pass
  the range it actually knows about, instead of hardcoding a single
  row.

- window_pane_redraw_floating() (window.c) never refreshed the status
  line after moving/resizing a floating pane, so a status format
  depending on that pane's geometry (e.g. #{pane_width}) could go
  stale until an unrelated status refresh happened. Added a
  server_status_window(w) call.

Also confirmed the window_pane_scrollbar_intersects() parameter
naming cleanup (loop -> wp) discussed earlier was already done in an
earlier "Cleanup." commit - nothing left to do there.

All 9 tests in regress/ plus the two pre-existing floating-pane tests
plus a further 19-test sweep of redraw/tty/input/sync-adjacent
regress tests pass.

Co-Authored-By: Michael K. Darling <darlingm@gmail.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL