| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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.
…dex to catch an unserviced edge-case.
|
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:
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. |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
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:
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. |
Sorry, something went wrong.
…ngles # Conflicts: # server-client.c # window.c
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>
|
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 Mechanical changes needed to support that:
Still verified to catch the real bug: reintroduced the actual "damage consumed by only one client" defect (made the damage |
Sorry, something went wrong.
|
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):
Took the approach, reimplemented simpler (1 of 4):
Took the approach, applied as-is with no changes (1 of 4):
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. |
Sorry, something went wrong.
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>
| Back | FazBrowse Home | New Git URL |
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.