screen-redraw: stop floating-pane drags from clobbering unrelated panes

window_pane_redraw_floating() (called by every floating-pane drag/resize
command) had two conflicting redraw paths: a precise damage-rectangle
path for the dragged pane, and an unconditional server_redraw_window()
added to fix border-status recomposition (aa6b52ef) that silently
defeated it, redrawing every pane - including retransmitting unrelated
images - on every drag tick.

Chasing the border-status bug back further: it was actually caused by
an upstream regression (824a0729) that split CLIENT_REDRAWWINDOW's
redraw flags into REDRAW_ALL vs. REDRAW_ALL & ~REDRAW_OVERLAY, silently
breaking every literal `flags == REDRAW_ALL` check downstream
(REDRAW_IS_ALL()), including the one that decides whether a pane's
border-status title should be forced to recompose even when its text
hasn't logically changed. Fix this properly instead of reaching for
server_redraw_window() again: decouple overlay-drawing from the shared
flags value (it's only ever read in the one place that repaints an
overlay) so redraw_screen() can pass a genuinely literal REDRAW_ALL for
a window redraw. This also fixes a second-client-attach bug where a new
client's pane titles never appeared until their text changed, since the
per-pane title cache is shared across clients.

Also fixed a real, independent bug in server-client.c found along the
way: `~c->flags & CLIENT_ALLREDRAWFLAGS` is a 6-bit mask, so `~x & MASK`
tests "any bit unset" (almost always true) rather than "no bits set" as
intended - causing damage to be composed and transmitted twice on every
floating-drag tick.

With server_redraw_window() gone, a second problem surfaced: deferred
client redraws (common under a busy tty) still escalated any pending
window damage to a full CLIENT_REDRAWWINDOW redraw, because
server_client_loop() frees w->damage unconditionally every pass with no
tracking of whether a deferred client actually got to consume it. Fix
this by giving each client a copy of missed damage (c->pending_damage,
reusing the existing merge/collapse/16-entry-cap logic damage lists
already have) keyed by the window's id, so a client can compose it
precisely on a later pass instead of resending everything. Window ids
are monotonic and never reused, so a stale id is enough to detect and
discard damage left over from a window the client has since switched
away from or that has been destroyed.

Verified with regress/image-border-status-wipe.sh,
regress/floating-pane-drag-scrollbar-strip.sh,
regress/image-movepane-drag-noflash.sh, and
regress/image-splitwindow-resize-noflash.sh (5 runs each), the full
regress suite (twice), an ASAN/UBSAN debug build, and manual repros for
a window switch and a window destroy mid-defer.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Michael Grant
2026-09-21 05:00:13 +01:00
parent efbd09e101
commit 729c361aad
4 changed files with 181 additions and 71 deletions

View File

@@ -1111,14 +1111,27 @@ redraw_free_damage(struct window *w)
#endif
}
/* Collapse all pending damage for a window into one rectangle - its union. */
/* Free all entries on an arbitrary damage list (used for pending_damage). */
static void
redraw_collapse_damage(struct window *w)
redraw_free_damage_list(struct redraw_damages *damage, u_int *count)
{
struct redraw_damage *rd, *rd1;
TAILQ_FOREACH_SAFE(rd, damage, entry, rd1) {
TAILQ_REMOVE(damage, rd, entry);
free(rd);
}
*count = 0;
}
/* Collapse a damage list into one rectangle - its union. */
static void
redraw_collapse_damage(struct redraw_damages *damage, u_int *count)
{
struct redraw_damage *rd, *rd1, *first;
u_int x0, y0, x1, y1;
first = TAILQ_FIRST(&w->damage);
first = TAILQ_FIRST(damage);
if (first == NULL)
return;
x0 = first->x;
@@ -1126,7 +1139,7 @@ redraw_collapse_damage(struct window *w)
x1 = first->x + first->sx;
y1 = first->y + first->sy;
TAILQ_FOREACH_SAFE(rd, &w->damage, entry, rd1) {
TAILQ_FOREACH_SAFE(rd, damage, entry, rd1) {
if (rd->x < x0)
x0 = rd->x;
if (rd->y < y0)
@@ -1137,7 +1150,7 @@ redraw_collapse_damage(struct window *w)
y1 = rd->y + rd->sy;
first->flags &= rd->flags;
if (rd != first) {
TAILQ_REMOVE(&w->damage, rd, entry);
TAILQ_REMOVE(damage, rd, entry);
free(rd);
}
}
@@ -1146,34 +1159,35 @@ redraw_collapse_damage(struct window *w)
first->y = y0;
first->sx = x1 - x0;
first->sy = y1 - y0;
w->damage_count = 1;
*count = 1;
}
/*
* Record a damaged window-coordinate rectangle. Clips it to the window,
* merges it with an existing rectangle where doing so does not make the
* result substantially larger than the two combined, and collapses the
* whole list to its union once it grows past a modest cap.
* Record a damaged window-coordinate rectangle on an arbitrary damage list,
* clipped to a bsx x bsy area. Merges it with an existing rectangle where
* doing so does not make the result substantially larger than the two
* combined, and collapses the whole list to its union once it grows past a
* modest cap.
*
* This only records damage - nothing consumes it yet.
*/
static void
redraw_damage_window_flags(struct window *w, u_int x, u_int y, u_int sx,
u_int sy, int flags)
redraw_add_damage(struct redraw_damages *damage, u_int *count, u_int bsx,
u_int bsy, u_int x, u_int y, u_int sx, u_int sy, int flags)
{
struct redraw_damage *rd;
u_int x0, y0, x1, y1, area, union_area;
if (x >= w->sx || y >= w->sy)
if (x >= bsx || y >= bsy)
return;
if (x + sx > w->sx)
sx = w->sx - x;
if (y + sy > w->sy)
sy = w->sy - y;
if (x + sx > bsx)
sx = bsx - x;
if (y + sy > bsy)
sy = bsy - y;
if (sx == 0 || sy == 0)
return;
TAILQ_FOREACH(rd, &w->damage, entry) {
TAILQ_FOREACH(rd, damage, entry) {
/* Skip unless overlapping or directly adjacent. */
if (x > rd->x + rd->sx || rd->x > x + sx ||
y > rd->y + rd->sy || rd->y > y + sy)
@@ -1203,11 +1217,23 @@ redraw_damage_window_flags(struct window *w, u_int x, u_int y, u_int sx,
rd->sx = sx;
rd->sy = sy;
rd->flags = flags;
TAILQ_INSERT_TAIL(&w->damage, rd, entry);
w->damage_count++;
TAILQ_INSERT_TAIL(damage, rd, entry);
(*count)++;
if (w->damage_count > REDRAW_DAMAGE_MAX)
redraw_collapse_damage(w);
if (*count > REDRAW_DAMAGE_MAX)
redraw_collapse_damage(damage, count);
}
/*
* Record a damaged window-coordinate rectangle. Clips it to the window.
* This only records damage - nothing consumes it yet.
*/
static void
redraw_damage_window_flags(struct window *w, u_int x, u_int y, u_int sx,
u_int sy, int flags)
{
redraw_add_damage(&w->damage, &w->damage_count, w->sx, w->sy, x, y,
sx, sy, flags);
}
void
@@ -1216,6 +1242,35 @@ redraw_damage_window(struct window *w, u_int x, u_int y, u_int sx, u_int sy)
redraw_damage_window_flags(w, x, y, sx, sy, 0);
}
/* Free a client's retained damage from a previously deferred redraw. */
void
redraw_free_pending_damage(struct client *c)
{
redraw_free_damage_list(&c->pending_damage, &c->pending_damage_count);
}
/*
* Copy a window's currently pending damage onto a client whose redraw was
* deferred this pass, so it survives server_client_loop()'s unconditional
* per-pass free of w->damage and can still be composed precisely on a later
* pass instead of escalating to a full-window redraw.
*/
void
redraw_defer_damage(struct client *c)
{
struct window *w = c->session->curw->window;
struct redraw_damage *rd;
if (c->pending_damage_id != w->id) {
redraw_free_pending_damage(c);
c->pending_damage_id = w->id;
}
TAILQ_FOREACH(rd, &w->damage, entry) {
redraw_add_damage(&c->pending_damage, &c->pending_damage_count,
w->sx, w->sy, rd->x, rd->y, rd->sx, rd->sy, rd->flags);
}
}
#ifdef ENABLE_IMAGES
/* Record damage caused by an image moving with a terminal scroll. */
void
@@ -2101,7 +2156,7 @@ redraw_draw(struct client *c, struct window_pane *wp, int flags)
}
}
}
if (c->overlay_draw != NULL && (flags & REDRAW_OVERLAY))
if (c->overlay_draw != NULL && (c->flags & CLIENT_REDRAWOVERLAY))
c->overlay_draw(c, c->overlay_data);
tty_reset(tty);
@@ -2150,10 +2205,16 @@ redraw_screen(struct client *c)
int flags = 0;
if (c->flags & CLIENT_REDRAWWINDOW) {
if (c->flags & CLIENT_REDRAWOVERLAY)
redraw_draw(c, NULL, REDRAW_ALL);
else
redraw_draw(c, NULL, REDRAW_ALL & ~REDRAW_OVERLAY);
/*
* Always pass the literal REDRAW_ALL here, even when no
* overlay is open - whether the overlay callback below
* actually fires is decided by CLIENT_REDRAWOVERLAY directly,
* not by this flags value. REDRAW_IS_ALL()/flags==REDRAW_ALL
* checks elsewhere (e.g. the PANE_NEWSTATUS force-refresh in
* the REDRAW_PANE_STATUS block below) rely on a real window
* redraw always being bit-exact REDRAW_ALL.
*/
redraw_draw(c, NULL, REDRAW_ALL);
} else {
if (c->flags & CLIENT_REDRAWBORDERS)
flags |= (REDRAW_PANE_BORDER|REDRAW_PANE_STATUS);
@@ -2362,6 +2423,35 @@ redraw_draw_damage_rect(struct redraw_draw_ctx *dctx, u_int x, u_int y,
}
}
/* Compose one damaged rectangle, clipped to what this client can see. */
static void
redraw_client_damage_rect(struct client *c, struct redraw_draw_ctx *dctx,
struct window *w, u_int ox, u_int oy, u_int sx, u_int sy,
struct redraw_damage *rd)
{
u_int x0, y0, x1, y1;
int skip_images = 0;
#ifdef ENABLE_IMAGES
if ((rd->flags & REDRAW_DAMAGE_SCROLL) &&
(image_backend_flags(&c->tty) & IMAGE_BACKEND_SCROLLS) &&
c->tty.image_scroll_window == w &&
c->tty.image_scroll_epoch == w->image_scroll_epoch &&
!c->tty.image_scroll_failed)
skip_images = 1;
#endif
x0 = (rd->x > ox) ? rd->x : ox;
y0 = (rd->y > oy) ? rd->y : oy;
x1 = (rd->x + rd->sx < ox + sx) ? rd->x + rd->sx : ox + sx;
y1 = (rd->y + rd->sy < oy + sy) ? rd->y + rd->sy : oy + sy;
if (x0 >= x1 || y0 >= y1)
return;
log_debug("%s: %s composing damage %u,%u %ux%u", __func__, c->name,
x0 - ox, y0 - oy, x1 - x0, y1 - y0);
redraw_draw_damage_rect(dctx, x0 - ox, y0 - oy, x1 - x0, y1 - y0,
skip_images);
}
/*
* Consume a client's window's pending damage by composing exactly the
* damaged cells, after clipping each rectangle to what this client can see
@@ -2370,6 +2460,12 @@ redraw_draw_damage_rect(struct redraw_draw_ctx *dctx, u_int x, u_int y,
* Unlike redraw_pane(), this does not redraw a whole pane's worth of cells
* for a small disturbance - only the cells within the (clipped) rectangle
* are touched, via redraw_draw_damage_rect().
*
* Also drains any damage this client missed on a previously deferred pass
* (see redraw_defer_damage()) - discarding it first instead if it was
* copied for a different window, or if CLIENT_REDRAWWINDOW is set (a full
* redraw already ran this pass, so composing old rectangles now would only
* retransmit content that redraw just drew).
*/
void
redraw_client_damage(struct client *c)
@@ -2377,11 +2473,12 @@ redraw_client_damage(struct client *c)
struct window *w = c->session->curw->window;
struct redraw_scene *scene;
struct redraw_draw_ctx dctx;
struct redraw_damage *rd;
u_int ox, oy, sx, sy, x0, y0, x1, y1;
int skip_images;
struct redraw_damage *rd, *rd1;
u_int ox, oy, sx, sy;
if (TAILQ_EMPTY(&w->damage))
if (c->pending_damage_id != w->id || (c->flags & CLIENT_REDRAWWINDOW))
redraw_free_pending_damage(c);
if (TAILQ_EMPTY(&w->damage) && TAILQ_EMPTY(&c->pending_damage))
return;
scene = redraw_get_scene(c);
@@ -2393,25 +2490,13 @@ redraw_client_damage(struct client *c)
tty_sync_start(&c->tty);
tty_update_mode(&c->tty, c->tty.mode & ~CURSOR_MODES, NULL);
TAILQ_FOREACH(rd, &w->damage, entry) {
skip_images = 0;
#ifdef ENABLE_IMAGES
if ((rd->flags & REDRAW_DAMAGE_SCROLL) &&
(image_backend_flags(&c->tty) & IMAGE_BACKEND_SCROLLS) &&
c->tty.image_scroll_window == w &&
c->tty.image_scroll_epoch == w->image_scroll_epoch &&
!c->tty.image_scroll_failed)
skip_images = 1;
#endif
x0 = (rd->x > ox) ? rd->x : ox;
y0 = (rd->y > oy) ? rd->y : oy;
x1 = (rd->x + rd->sx < ox + sx) ? rd->x + rd->sx : ox + sx;
y1 = (rd->y + rd->sy < oy + sy) ? rd->y + rd->sy : oy + sy;
if (x0 >= x1 || y0 >= y1)
continue;
log_debug("%s: %s composing damage %u,%u %ux%u", __func__,
c->name, x0 - ox, y0 - oy, x1 - x0, y1 - y0);
redraw_draw_damage_rect(&dctx, x0 - ox, y0 - oy, x1 - x0,
y1 - y0, skip_images);
TAILQ_FOREACH_SAFE(rd, &c->pending_damage, entry, rd1) {
redraw_client_damage_rect(c, &dctx, w, ox, oy, sx, sy, rd);
TAILQ_REMOVE(&c->pending_damage, rd, entry);
free(rd);
c->pending_damage_count--;
}
TAILQ_FOREACH(rd, &w->damage, entry)
redraw_client_damage_rect(c, &dctx, w, ox, oy, sx, sy, rd);
}

View File

@@ -316,6 +316,7 @@ server_client_create(int fd)
c->click_wp = -1;
TAILQ_INIT(&c->input_requests);
TAILQ_INIT(&c->pending_damage);
TAILQ_INSERT_TAIL(&clients, c, entry);
log_debug("new client %p", c);
@@ -548,6 +549,7 @@ server_client_lost(struct client *c)
status_free(c);
input_cancel_requests(c);
redraw_free_pending_damage(c);
free(c->title);
free(c->path);
@@ -1949,11 +1951,13 @@ server_client_loop(void)
/*
* Any windows will have been redrawn as part of clients, so clear
* their flags now. A client whose redraw was deferred this pass
* (waiting for outstanding tty output to drain) has already
* escalated to CLIENT_REDRAWWINDOW or CLIENT_REDRAWSCROLLBARS in
* server_client_check_redraw() to cover whatever it is about to
* lose here, so PANE_REDRAW/PANE_REDRAWSCROLLBAR and window damage
* can simply be cleared unconditionally.
* (waiting for outstanding tty output to drain) has already copied
* window damage onto its own pending_damage in
* server_client_check_redraw() (see redraw_defer_damage()) to
* compose precisely on a later pass, or escalated to
* CLIENT_REDRAWWINDOW/CLIENT_REDRAWSCROLLBARS for whole-pane needs
* that aren't rectangle-shaped, so PANE_REDRAW/PANE_REDRAWSCROLLBAR
* and window damage can simply be cleared unconditionally here.
*/
RB_FOREACH(w, windows, &windows) {
TAILQ_FOREACH(wp, &w->panes, entry) {
@@ -2516,6 +2520,8 @@ server_client_any_pane_redraw(struct client *c)
return (1);
if (!TAILQ_EMPTY(&w->damage))
return (1);
if (!TAILQ_EMPTY(&c->pending_damage))
return (1);
TAILQ_FOREACH(wp, &w->panes, entry) {
if (wp->flags & (PANE_REDRAW|PANE_REDRAWSCROLLBAR))
return (1);
@@ -2563,10 +2569,11 @@ server_client_check_redraw(struct client *c)
* consumed. We can just add a timer to get out of the event loop and
* end up back here. server_client_loop() clears PANE_REDRAW,
* PANE_REDRAWSCROLLBAR and window damage unconditionally every pass,
* so escalate to a coarser, persistent client flag that survives
* that clear and forces a full catch-up redraw once this client is
* unblocked, rather than trying to keep the fine-grained state
* around for a retry.
* so window damage is copied onto this client's own pending_damage
* (redraw_defer_damage()) to survive that and still be composed
* precisely on a later pass; a whole-pane need (PANE_REDRAW) or a
* pane's scrollbar isn't rectangle-shaped the same way, so those
* still escalate to a coarser, persistent client flag as before.
*/
n = EVBUFFER_LENGTH(tty->out);
if (n != 0 || (tty->flags & TTY_BLOCK)) {
@@ -2581,7 +2588,7 @@ server_client_check_redraw(struct client *c)
evtimer_add(&ev, &tv);
}
if (!TAILQ_EMPTY(&w->damage))
c->flags |= CLIENT_REDRAWWINDOW;
redraw_defer_damage(c);
TAILQ_FOREACH(wp, &w->panes, entry) {
if (wp->flags & PANE_REDRAW) {
c->flags |= CLIENT_REDRAWWINDOW;
@@ -2617,16 +2624,16 @@ server_client_check_redraw(struct client *c)
}
/*
* Window damage is also what makes server_client_any_pane_
* redraw() decide a redraw is needed at all, independently of
* any CLIENT_ALLREDRAWFLAGS bit. Every current damage source
* happens to set one of those flags too, so the block below
* always consumes it - but consume it here too in case that
* ever stops holding, since server_client_loop() clears
* window damage unconditionally every pass regardless of
* whether it was actually drawn.
* Window damage (and any damage this client missed on a
* previous deferred pass - see redraw_defer_damage()) is
* also what makes server_client_any_pane_redraw() decide a
* redraw is needed at all, independently of any
* CLIENT_ALLREDRAWFLAGS bit. Consume both here, since
* server_client_loop() clears window damage unconditionally
* every pass regardless of whether it was actually drawn.
*/
if (!TAILQ_EMPTY(&w->damage) && (~c->flags & CLIENT_ALLREDRAWFLAGS))
if ((!TAILQ_EMPTY(&w->damage) || !TAILQ_EMPTY(&c->pending_damage)) &&
!(c->flags & CLIENT_ALLREDRAWFLAGS))
redraw_client_damage(c);
}

19
tmux.h
View File

@@ -2317,6 +2317,23 @@ struct client {
struct redraw_scene *redraw_scene;
/*
* Damage this client missed because its redraw was deferred (pending
* tty output) on the pass it was reported - server_client_loop()
* frees w->damage unconditionally every pass regardless of whether
* every attached client got to consume it, so a deferred client's
* copy is kept here to compose precisely on a later pass instead of
* escalating to a full-window redraw. See redraw_defer_damage() and
* redraw_client_damage() (screen-redraw.c). pending_damage_id is the
* id of the window these rectangles were copied for (window ids are
* unique and never reused), used to discard them if the client's
* current window has since changed instead of composing them
* against the wrong window's scene.
*/
struct redraw_damages pending_damage;
u_int pending_damage_count;
u_int pending_damage_id;
struct event repeat_timer;
struct event click_timer;
@@ -3736,6 +3753,8 @@ void redraw_image_scroll_result(struct tty *, const struct tty_ctx *, int);
#endif
void redraw_free_damage(struct window *);
void redraw_client_damage(struct client *);
void redraw_defer_damage(struct client *);
void redraw_free_pending_damage(struct client *);
int redraw_get_status_border_cell_type(struct redraw_span **, u_int);
/* screen.c */

View File

@@ -3042,5 +3042,4 @@ window_pane_redraw_floating(struct window *w, struct window_pane *wp,
/* Session status formats may depend on the pane's new geometry. */
server_status_window(w);
server_redraw_window(w);
}