Every redraw that replaces an image's placements (a pane revealed after
an overlapping floating pane closes, a full window redraw, etc.)
deleted all of an image's existing placements before creating their
replacements. That briefly left the image with none at all - and some
Kitty implementations free an image's underlying pixel data once it
has no placements left, so the replacement placements just placed
would then reference already-discarded data and render as nothing: an
empty hole where the image used to be. Not reproducible with SIXEL,
which has no separate placement/data lifecycle to race.
kitty_redraw_start() now only marks placements intersecting a redraw
area as stale instead of deleting them immediately; a new
kitty_redraw_finish() (called via image_redraw_finish() after a
redraw's replacement placements have already been created) deletes
whatever is still marked. The image now always has at least one live
placement throughout the transition.
The scroll-damage redraw path (redraw_draw_damage_rect()) always ran
its text-only pass even when imagescroll's trust skipped recompositing
the image on top, painting blank cells directly over image content
with nothing left to correct it. redraw_draw_pane_span() now consults
a new image_grid_next_span() to exclude any column range an image
occupies from that text pass.
The same path also never called the existing image_redraw_start()/
kitty_redraw_start() placement cleanup that the full-pane redraw path
already had, so every scroll of a Kitty-protocol image added a new set
of placements without deleting the stale ones, which the terminal then
rendered as an overlapping smudge. Added the missing call, skipped
when imagescroll is trusting the terminal not to need replacing.
Whether a terminal moves SIXEL/Kitty image content along with the rest of
a scrolling region turns out not to correlate with DECSLRM (margins)
support at all - direct testing (both a real -vv log of an actual native
scroll, and a raw, outside-tmux escape-sequence test mirroring the one
used to confirm Windows Terminal) showed mintty correctly scrolls text
within a margin-bounded region but drops sixel content placed there,
while WezTerm and Windows Terminal move it correctly. Since this is a
per-terminal capability with no relation to any other capability tmux
already tracks, it belongs in tty-features.c's table like margins/sixel/
kitty, not as a general server option.
New "imagescroll" terminal feature (tty-features.c), backed by a new
TERM_IMAGESCROLL flag (tmux.h). image_backend_flags() (image.c) now
checks tty->term->flags & TERM_IMAGESCROLL directly instead of the
removed image-region-scrolling option (options-table.c).
Flips the previous default while doing so: the option defaulted to on
(trust the terminal); nothing is granted this new feature by default.
Windows Terminal cannot be identified via XTVERSION at all (see the
WT_SESSION revert two commits back), so it gets the safe default here
too, same as any other unconfirmed terminal. Only WezTerm and ghostty are
granted imagescroll, both confirmed by direct testing - image content,
not just text, observed scrolling correctly within a margin-bounded
region on both.
regress/image-sixel-region-scroll.sh and image-kitty-region-scroll.sh
updated to grant/revoke the feature via terminal-features instead of
toggling the removed option; same two-phase check as before (granted - no
retransmit; not granted - always retransmit).
Verified: build clean; old option name now rejected ("invalid option");
new feature name accepted via terminal-features; both updated regress
tests pass (3x each); tty-margins-scrollbar.sh (unrelated but touches the
same tty-features.c) unaffected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
nicm is opposed to identifying a terminal via an environment variable, so
remove the WindowsTerminal tty_default_features() entry and the
WT_SESSION check in tty_term_create() entirely, pending some other
accepted identification mechanism for Windows Terminal (it cannot be
identified via the existing XTVERSION path at all - see the previous
commit's now-removed comment for why).
This leaves WezTerm and ghostty's margins grants in place - both are
identified via the existing, already-accepted XTVERSION mechanism, with
DECSLRM support confirmed directly in their own source, independent of
this change.
regress/tty-margins-wt-session.sh tested only the removed mechanism and
is replaced by regress/tty-margins-scrollbar.sh, which checks the same
underlying scrollbar/margins scroll-decision mechanism (still real, and
still what WezTerm's and ghostty's table entries rely on) via the
generic terminal-features option instead of simulating any one
terminal's identification handshake.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Brings in the WezTerm/ghostty/Windows Terminal DECSLRM (margins) coverage
from margins-terminal-detection (PR #5630), via redraw-damage-rectangles.
Two conflicts, both simple bit-flag/list collisions from independent work
on both sides adding something at the same slot:
- tmux.h: this branch's own TERM_KITTY/TERM_IMAGE_QUADRANTS/
TERM_IMAGE_SEXTANTS (0x100/0x200/0x400) collided with upstream's new
TERM_NOREPLACE, also assigned 0x100. Kept this branch's existing values
(already used throughout image.c/tty-features.c) and moved
TERM_NOREPLACE to the next free bit, 0x800 - confirmed no other code
references the literal 0x100 value for TERM_NOREPLACE, only the symbol.
- tty-features.c: this branch's own WezTerm entry (added "sixel") and the
incoming margins-terminal-detection change (added "margins") both
touched the same line - combined both.
Verified: build clean with --enable-images (image_support confirmed 1);
this branch's own image-kitty-region-scroll.sh/image-sixel-region-scroll.sh
plus this session's other prompt/cross-client/margins tests all pass (3x
each); full regress suite run pending/clean per standard practice for
this branch.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Brings in the WezTerm/ghostty/Windows Terminal DECSLRM (margins) coverage
from margins-terminal-detection (PR #5630), plus whatever independent
upstream progress that branch's master base had accumulated since this
branch last merged from master.
Conflict in screen-redraw.c's redraw_draw_pane_prompt(): this branch's own
earlier refactor (extracting redraw_make_pane_prompt(), reused by both the
normal redraw path and the Finding-2 damage-composition fix) collided with
an independent upstream fix on master (nicm, "Get x and y the right way
round, fixes crash when opening a mode prompt when the client is offset
into the window") that swapped tty_draw_line()'s px/py argument order in
this same function - this branch had diverged before that fix landed and
still had the pre-fix (buggy) order. Resolved by keeping this branch's
own refactor with the corrected argument order, verified against
tty_draw_line()'s signature (px, py - the prompt's screen is always
exactly one row, so py must be 0 and px must be the horizontal offset;
the pre-fix order passed the offset as the row instead, matching the
described crash). The other, structurally identical call site added this
session for Finding 2 (redraw_damage_draw_pane_prompt()) already had the
correct order and needed no change.
Verified: build clean; the new tty-margins-wt-session.sh and this
session's existing wide-character/prompt/cross-client tests all pass
(3x each); full regress suite passes clean twice, with only the same
pre-existing, unrelated failures already characterized this session
(prompt-words-history.sh, and untracked image-*-noflash scratch tests
from unrelated work, absent from this branch's own tracked suite).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Any pane that doesn't span the terminal's full width - because
pane-scrollbars is on (the scrollbar occupies a column) or the pane is one
of a side-by-side split - needs DECSLRM (left/right margin) support to use
the fast native-scroll path (tty_cmd_linefeed()/scrollup()/scrolldown()/
reverseindex(), tty.c: "(!tty_full_width(tty, ctx) && !tty_use_margin(tty))").
Without it, every single scroll falls back to tty_redraw_region()'s full
manual repaint of the whole region - confirmed via -vv log
("tty_redraw_region: ... large region redraw") - which is a real source of
flicker, and separately implicated in reports of image content not
surviving a scroll in this area of the tree.
The DECSLRM machinery itself (tty_margin_pane()/tty_margin()) is already
built and already called from all four native-scroll dispatch functions.
What was missing was terminal-capability coverage:
- WezTerm and ghostty are already correctly identified via the existing
XTVERSION mechanism (tty_keys_extended_device_attributes(),
tty-keys.c), and both genuinely implement DECSLRM - confirmed directly
in their own source (WezTerm: decslrm() in csi.rs, documented in their
escape-sequences reference, a changelog entry fixing a DECSLRM bug
confirming active use; ghostty: handled in dcs.zig/stream.zig,
explicitly emitted alongside DECSTBM in their own formatter.zig, with
its own terminfo entry for it). Neither's entry in
tty_default_features()'s table (tty-features.c) granted "margins".
This was a gap, not a detection problem, backed by source-level
evidence rather than guesswork - kitty and Rio show zero DECSLRM
references in their own source, so their omission is left as-is.
- Windows Terminal's DECSLRM support was independently confirmed by
direct, non-tmux escape-sequence testing, but it can never be
identified via XTVERSION - the tracking issue for that
(github.com/microsoft/terminal#18382) was explicitly closed
not_planned by the maintainer, calling XTVERSION "not extensible or
helpful for feature detection". Detected instead via the WT_SESSION
environment variable Windows Terminal sets for every child process -
a long-standing, stable signal already used for this exact purpose by
many other tools. Checked in tty_term_create() (tty-term.c) alongside
the existing COLORTERM-based RGB/256 detection, which already reads a
named variable out of the attaching client's own environment
(c->environ, populated via the MSG_IDENTIFY_ENVIRON handshake) the
same way. Deliberately grants only "margins" for now, not the full
modern-xterm feature bundle other entries get, since no other
capability has been verified for it.
New regress/tty-margins-wt-session.sh reproduces the scrollbar-pane
scenario with WT_SESSION injected into the attaching client's environment
and checks, via the -vv log, that no tty_redraw_region() fallback occurs -
plus a sanity phase confirming the same scenario does fall back without
WT_SESSION set, proving the test isn't accidentally trivial. Verified
failing 3/3 against the pre-fix code and passing 5/5 standalone; full
regress suite passes clean twice, with only pre-existing, unrelated
failures (check-names.sh - confirmed identical on unmodified master;
prompt-words-history.sh - already-known flake; four untracked
image-*-noflash scratch tests left over from unrelated work on another
branch, not part of this tree's tracked suite).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* refs/remotes/tmux-openbsd/master:
Get x and y the right way round, fixes crash when opening a mode prompt when the client is offset into the window; may be the crash reported by sthen@.
Reported: on Windows Terminal, a Kitty image placement is deleted (not
just left in the wrong place) by a plain newline-driven scroll-up or by
reverse-index scroll-down, but survives a delete-line/insert-line-based
scroll simulation of the same movement.
Root cause: image_backend_kitty unconditionally included
IMAGE_BACKEND_SCROLLS (image.c), so redraw_client_damage_rect()
(screen-redraw.c) always trusted that any terminal claiming the Kitty
graphics protocol also correctly relocates placements across a native
CSR-region scroll, and skipped re-sending the placement after one. There
was no way to turn this off, unlike the identical, already-acknowledged
problem for SIXEL - sixel-region-scrolling (added this branch, not yet
released anywhere) already carries the comment "there is no way to ask a
terminal whether it moves image content along with a scrolling region."
Windows Terminal's Kitty support does not move it - it drops it. DL/IL
never go through this scroll-specific "trust the terminal" shortcut,
which is why they were unaffected, matching the report.
Since the SIXEL option is new on this branch and unreleased, generalise
it instead of adding a near-duplicate: sixel-region-scrolling becomes
image-region-scrolling (still a server-scope flag, still on by default,
matching prior behaviour for terminals that do handle this correctly),
and image_backend_flags() now gates IMAGE_BACKEND_SCROLLS for both the
SIXEL and Kitty backends on it, rather than hardcoding it for Kitty.
Separately, tty_cmd_linefeed() was missing the redraw_image_scroll_result()
calls that tty_cmd_reverseindex()/tty_cmd_scrollup()/tty_cmd_scrolldown()
all already have on both their native and tty_redraw_region() fallback
branches - a plain newline-driven scroll never recorded whether the native
path was actually used, leaving the per-client scroll-result tracking
that redraw_client_damage_rect() reads keyed off whatever an unrelated
prior operation last left it at. Added the same two calls linefeed was
missing, matching the other three functions exactly.
regress/image-kitty-region-scroll.sh mirrors the existing
image-sixel-region-scroll.sh (renamed from its old option name here) for
the Kitty side: transmits a real Kitty placement, scrolls it with the
option on and off, and checks whether tmux's own re-placement command
(kitty_draw_rect()'s "a=p") gets reissued - proving tmux's decision to
skip or redraw is wired up correctly for Kitty too, not that any
particular terminal renders the result correctly (that needs a human, on
real terminals, per the existing SIXEL test's own caveat).
Both new/updated regress tests require --enable-images (this tree's build
was not actually configured with it - #{image_support} was silently 0 and
every image-prefixed test, including the four pre-existing ones already
in this tree, was trivially skipping rather than running; reconfigured
with --enable-images for this work, which also required libpng/zlib).
Verified: image-kitty-region-scroll.sh fails 3/3 against the pre-fix code
(the option does not exist) and passes 5/5 standalone; both region-scroll
tests pass in the full regress suite, run twice (this touches the scroll
path shared by every image-capable pane), with only the two pre-existing,
unrelated failures (image-support.sh, prompt-words-history.sh) and one
one-off flake (cursor-test1.sh, passed 3/3 standalone) - image-support.sh
verified to fail identically before and after this change.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* refs/remotes/tmux-openbsd/master:
Some terminals have extended DECSCUSR to allow it to reset to the default style and put that as the Se capability in their terminfo entry. Change so that if Ss and Se exist, tmux does not overwrite them with its own. GitHub issue 5476.
default style and put that as the Se capability in their terminfo entry.
Change so that if Ss and Se exist, tmux does not overwrite them with its
own. GitHub issue 5476.
redraw_damage_draw_pane_prompt() recomposes a pane's separately rendered
prompt (wp->prompt) over a damaged sub-range of a REDRAW_SPAN_PANE span,
reusing the (x, n) range that redraw_client_damage() already clipped and
grew against the pane's own *content* grid (wp->screen) via
redraw_damage_grow_span_clip(). That growing exists specifically to avoid
splitting a wide character at the range's edge - but the prompt is drawn
into its own, freshly allocated one-line screen with no relationship to
the content grid, so a range that's clean (or correctly grown) for the
content can still land mid-character in the prompt's own grid.
This is invisible whenever the pane's content is plain ASCII: the content
grid has no padding cells to find, so redraw_damage_grow_span_clip() never
grows the range at all, and the raw geometric range - however it landed -
is passed straight through to the prompt. If that range's end lands right
after a base cell whose padding half falls just outside it, tty_draw_line()
has no room left for that cell's second column
(tty_draw_line_get_empty()'s gc->data.width > nx check) and blanks it
entirely, even though the pane's own content never needed the fix at all.
Reproduced with a floating pane's CJK prompt straddling the boundary
between two tiled panes underneath it: a palette change (OSC 4) in one of
the tiled panes triggers a redraw of its own rectangle, which is occluded
by the floating pane but still geometrically overlaps its prompt row,
recomposing a partial range of the prompt that cuts through a character.
Fix: extract the grid-probing tail of redraw_span_cell_is_padding() into
redraw_screen_cell_is_padding(), usable against any screen, and add
redraw_damage_grow_screen_clip() - the same left/right one-step growth as
redraw_damage_grow_span_clip(), but against an explicit screen and
column origin. redraw_damage_draw_pane_prompt() now re-derives its own
(x0, x1) range from the caller's (x, n) by growing it against the
prompt's own screen before drawing, clamped to the span so it can't bleed
into a neighbouring one.
New regress/floating-pane-prompt-wide-character.sh constructs the exact
tiled-pane-boundary scenario above and checks the CJK prompt text is
intact after the trigger. Verified failing 3/3 against the pre-fix code
(a character is blanked) and passing 5/5 standalone plus 2/2 in the full
regress suite (twice, since this touches the same damage-composition path
as every other redraw) against the fix, with no other tests newly broken
(prompt-words-history.sh and the untracked image-support scratch tests
are pre-existing, unrelated failures).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
redraw_damage_refresh_status() force-regenerates a pane's border-status
title when a damage rectangle touches it (window_make_pane_status()'s
own content-diff cache can't tell "physically disturbed" from "never
changed"), guarded by the per-pane PANE_NEWSTATUS flag. But the
rendered content is per-client - window_make_pane_status() formats
pane-border-format using the requesting client's own context, so
fields like #{client_name} genuinely differ per client - while
wp->status_screen/PANE_NEWSTATUS are shared by every client viewing
the pane. With two clients attached to the same session, whichever
client's damage pass ran first rendered its own text and set the flag;
every other client's damage pass in that tick, or any later one, since
nothing else clears the flag on this path, found it already set and
skipped rendering - silently reusing the first client's text.
Fix: a per-pass serial (redraw_status_serial, bumped once per
redraw_client_damage() call - one call is one client's one redraw
pass) instead of a sticky flag. Deduplicates repeated calls within the
same client's same pass exactly as before, but forces a fresh,
correctly-client-formatted render whenever a different client or a
later pass touches the same pane's status - without needing to track
and later invalidate a client pointer with its own lifetime.
Also added a permanent log_debug() line for the actual regenerate,
since this class of bug (a damage pass silently trusting stale
per-pane state that should have been per-client) is otherwise
invisible to any external capture: an unrelated periodic per-client
status refresh reliably repaints each client's title correctly again
within the very same tick, before anything is ever flushed to either
terminal, so the wrong content this bug produces was never actually
observable in a capture-pane-based test - confirmed by direct
instrumentation while building the regression test below, which is
exactly why the fix is verified via this log rather than a capture.
regress/floating-pane-status-cross-client.sh attaches two clients to
one session, each with its own pane-border-format referencing
#{client_name}, and triggers a damage-only palette update (OSC 4) in
a tiled pane whose geometry overlaps a floating pane's own
border-status row - carefully picked so the trigger has no side
effect that would otherwise force a normal, already-correct, full
per-client status re-render in the same pass, which would mask the
result either way. Verified failing 3/3 against the pre-fix code
(neither client's damage pass regenerates at all - both silently
reuse whatever a much earlier full redraw left in the shared buffer)
and passing 5/5 against the fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* refs/remotes/tmux-openbsd/master:
Add a feature for mintty's application escape to avoid dodgy terminals whinging about not supporting it, GitHub issue 5626 from Pete Dietl.
Only rely on the previous line wrapping when the cursor is actually on it, GitHub issue 5625 from Ben Maurer.
Correctly write Z indexes to v2 layouts if zoomed, GitHub issue 5624.
Do not leak waiting clients (in wait-for) if they are killed, GitHub issue 5614.
`(~c->flags & CLIENT_ALLREDRAWFLAGS)` - for a multi-bit mask, ~x & MASK
means "at least one of these bits is unset" (almost always true), not
"none of these bits are set" as the comment and surrounding logic
clearly intend. Every floating-pane drag command unconditionally sets
CLIENT_REDRAWBORDERS (server_redraw_window_borders(), called from
cmd-resize-pane.c/cmd-join-pane.c/cmd-split-window.c) alongside
reporting window damage, so this fallback fired on every single drag
step: redraw_client_damage(c) ran here, then ran again a few lines
later at the CLIENT_ALLREDRAWFLAGS block for the exact same
rectangles. Confirmed via the server's own -vv log: a 6-step drag
produced 12 "composing damage" lines in matched pairs (identical
position and size, milliseconds apart) with no fix, 6 with it.
No memory-safety issue - redraw_client_damage() only reads w->damage,
never frees it - just wasted work rendering the same rectangles twice
per pass.
Fix: `(c->flags & CLIENT_ALLREDRAWFLAGS) == 0`, matching the comment's
actual intent.
regress/floating-pane-drag-no-double-composite.sh drags a floating
pane and asserts no two consecutive "composing damage" log lines share
the same position and size (comparing both together, since distinct
drag steps commonly share the same rectangle size and only the
position differs). Verified failing 3/3 against the pre-fix code (all
6 rectangles doubled each run) and passing 5/5 against the fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
server_client_key_callback()'s mouse-drag dispatch opens a
synchronized-output frame (tty_sync_start()) before running the drag
callback, on every single drag motion event - deliberately, so a fast-
path write and any later damage-composed correction land in one atomic
terminal update instead of two visible frames. But
server_client_check_redraw() later in the same pass checks
EVBUFFER_LENGTH(tty->out) != 0 to decide whether to defer this pass's
redraw, and nothing drains tty->out in between (the actual write
happens later, via libevent) - so the frame-open sequence just queued
(8 bytes: "\033[?2026h" on a synchronized-output-capable terminal)
makes that check see "outstanding output" and defer against itself,
escalating the drag's damage to a full-window redraw on every single
motion event. Confirmed via the server's own -vv log: a 6-step drag
produced five "redraw deferred (8 left)" lines, one per motion event,
with no fix.
Fix: record how much was already queued at the instant the sync frame
opened (tty->sync_offset), and have the redraw check discount
anything queued after that point - it's already part of the frame
this pass is committed to flushing, not a reason to defer. If the
buffer was genuinely non-empty before the frame opened, sync_offset
holds that real backlog and deferral still happens correctly.
regress/floating-pane-drag-sync-no-self-defer.sh drags a floating pane
on a synchronized-output-capable terminal and asserts the server log
never shows the self-inflicted 8-byte deferral. Verified failing 3/3
against the pre-fix code (5 occurrences per run, matching the manual
-vv repro) and passing 5/5 against the fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
window_copy_scroll() (called from both scrollbar-slider-drag paths -
window_copy_cmd_scroll_to_mouse() and copy-mode -S) calls
window_set_active_pane() to switch focus to the dragged pane, but was
the one caller in the whole codebase that did this without pairing it
with window_redraw_active_switch() first, and without falling back to
a full server_redraw_window() either. Every other window_set_active_
pane() caller does one or the other.
This used to be harmless because window_set_active_pane() itself did
an unconditional full redraw on every active-pane change - narrowing
that to borders/status-only for the non-zoomed case (this branch) made
pane *body* colours only repaint when something sets PANE_REDRAW,
which is exactly what window_redraw_active_switch() does by comparing
cached window-style/window-active-style colours. Without it, dragging
an inactive pane's scrollbar slider changes which pane is active
(borders and status update immediately) while both panes keep their
stale body colours until an unrelated redraw happens to touch them.
Fix: call window_redraw_active_switch() immediately before window_set_
active_pane(), matching the established pattern (e.g.
cmd-resize-pane.c's mouse-drag handler).
regress/window-copy-scrollbar-drag-focus-style.sh sets clearly
distinguishable window-active-style/window-style backgrounds, drags
the *inactive* pane's scrollbar slider, and checks - via an attached
client's own received bytes, since window-style is applied during
redraw composition rather than stored in the grid, so capture-pane
alone would not reflect it - that the newly active pane immediately
shows the active-style colour. Verified failing 3/3 against the
pre-fix code (showing both panes with swapped/stale colours, exactly
as reported) and passing 5/5 against the fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
screen_write_redraw_cb() passed wp->xoff/wp->yoff straight through as
u_int to redraw_damage_window(). Both are genuinely signed and can be
negative for a floating pane positioned partly off the window's left
or top edge (layout_floating_args_parse() explicitly allows -X/-Y
down to -sx/-sy). A negative value wraps to a huge u_int,
redraw_damage_window()'s first bounds check (x >= w->sx) rejects the
whole rectangle, and nothing gets redrawn - not even the pane's
visible portion.
This is broader than just the alternate-screen-exit case that first
surfaced it: screen_write_pane_is_obscured() routes any scrolling
output in such a pane through this same callback, so a partly
off-screen floating pane lost every scroll repaint, not just its
post-alternate-screen one.
Fixed at the call site (matching the existing correct reference
pattern in window_pane_damage_floating(), window.c): compute in signed
int, clip negative offsets to the window's own origin and shrink the
corresponding size to match, then convert to u_int only once the
rectangle is known to be sane. Left redraw_damage_window()'s own
signature alone - it has a second caller with an inclusive-bounds
convention that a signature change would need to reconcile, and the
bug is specific to this call site not clamping before converting.
regress/floating-pane-offscreen-alternate-redraw.sh creates a floating
pane with -X -5 (partly off the left edge), cycles it through the
alternate screen, and checks an attached client's own received bytes
(not capture-pane, which reads the grid directly and would pass
regardless of whether the client was ever told to redraw) show the
primary screen's content correctly restored in the pane's visible
columns. Verified failing 3/3 against the pre-fix code and passing
5/5 against the fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
redraw_damage_grow_span_clip() guards its left-edge growth against
walking onto an unrelated wide character's base cell (via
redraw_span_left_grow_ok(), commit 6f65c318), but the right edge was
left growing unconditionally, on the reasoning that "tty_draw_line()
already draws a wide character in full even when the requested range
clips off its trailing padding half" - that reasoning was wrong.
When the right edge lands cleanly on a fresh character's base cell (a
character fully outside the range), growing right pulls in only that
base column. tty_draw_line()'s tty_draw_line_get_empty() then sees
gc->data.width > nx for the truncated remainder and treats it as an
empty cell to clear via tty_draw_line_clear() - a different code path
than the leading-padding-clear the left-edge fix reasoned about, but
exactly as destructive: it blanks a character that was never inside
the damage rectangle at all.
The padding check the left-edge fix added doesn't actually care about
direction - "is the cell at this scene x-coordinate a padding cell" is
the same question whether asked of a range's start or its end - so
renamed redraw_span_left_grow_ok() to redraw_span_cell_is_padding()
and apply it symmetrically to both edges.
regress/floating-pane-drag-wide-character-right.sh mirrors the
existing left-edge test for this edge: constructs the exact column
parity needed (the vacated rectangle's right edge landing on a base
cell) deterministically rather than relying on luck, verified failing
3/3 against the pre-fix code and passing 5/5 against the fix. The
existing left-edge test continues to pass unchanged.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
redraw_damage_grow_span_clip() widens a composed damage rectangle's
clipped edge by one cell whenever it isn't already at the span's own
boundary, to pull a wide character's base half back into range when
the edge happens to land on its padding half. This was unconditional -
it never checked which half it was actually touching.
When the edge instead already lands cleanly on a fresh character's
base cell (nothing to pull in - that character is simply outside the
rectangle), growing left walks one cell further, into the *previous*,
unrelated character's padding half, and blanks it: tty_draw_line()
treats any leading padding cell in its draw range as proof the range
starts mid-character and clears it (the "If there is padding at the
start, we must have truncated a wide character" branch, tty-draw.c).
Net effect: redrawing a damage rectangle can destroy a wide character
sitting just outside it, on whichever side the edge's column parity
happens to be unlucky.
This surfaced via dragging a display-popup pane (now backed by a
floating pane upstream, since popups were folded into the general
floating-pane mechanism) away from wide-character content, but it is a
general bug in any damage-composed redraw, not popup-specific: an
identical drag against an ordinary floating pane reproduces it
whenever the parity lines up the same way, confirmed while building
the new regress test below. It only looked popup-specific because
display-popup's new floating-pane-backed drag moves the pane on the
very first motion event, reliably hitting the bad parity, whereas the
old (now-removed) popup.c's drag handler didn't move on the first
event and tended to land on the safe parity by chance.
Also confirmed this is not a tmux/terminal wide-character width
disagreement: utf8_width()'s only override table is emoji/regional-
indicator ranges (no CJK), so a codepoint like U+754C falls straight
through to wcwidth(); the -vv log's own "wcwidth(0754C) returned 2"
line during the repro confirms tmux and libc agree on width 2. The bug
is in the redraw-clipping logic, not the width calculation.
Fix: only grow the left edge when the cell actually there is a padding
cell (redraw_span_left_grow_ok()), for the span types that can contain
one - pane content, a pane's status line, and a menu, the only spans
drawn via tty_draw_line() against a real backing screen. Border and
scrollbar spans draw single synthesized cells directly and can never
split a wide character, so their unconditional growth is untouched.
The right edge doesn't need the same guard: tty_draw_line() already
draws a wide character in full even when the requested range clips off
its trailing padding half, so growing right is at worst redundant,
never destructive.
regress/floating-pane-drag-wide-character.sh reproduces this
deterministically with an ordinary floating pane (not a popup, since
the bug isn't popup-specific): it creates the pane, checks its real
resulting position (rather than hand-computing the border-framing
offset), and retries one column over if needed until the vacated
rectangle's left edge lands on a base cell - the bad-parity case every
earlier manual repro landed on only by chance. Verified failing 3/3
against the pre-fix code and passing 5/5 against the fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same category as the previous removals: its header comment describes
verifying redraw_damage_grow_span_clip()'s wide-character boundary
widening specifically through a dragged popup, whose drag mechanic no
longer exists as popup.c did it.
While isolating this, confirmed the underlying wide-character damage
restoration is NOT broken in general - an identical drag sequence
against a regular floating pane (new-pane -X -Y, not display-popup)
correctly restores the old rows. The failure is specific to a popup
created via the new display-popup-over-floating-pane path
(cmd_display_popup_exec, cmd-display-menu.c) not properly triggering
this fork's damage-rectangle restoration on the area it used to cover -
apparently a gap in how master's new popup-via-floating-pane code
integrates with this fork's damage system, neither of which existed
combined before today's merge. Worth investigating separately; not
something this merge's conflict resolutions caused or can fix inline.
Also removes the untracked regress/image-popup-drag-noflash.sh test
(uncommitted leftover from earlier session work), whose header
literally names the removed popup_handle_drag() (popup.c) as what it
guards against.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
display-popup is now a compatibility shim over a floating pane (upstream
removed the whole overlay/popup machinery in favour of floating panes -
see the preceding merge). These three tests moved/resized a popup with
hardcoded mouse coordinates tuned to the old popup implementation's
exact positioning and clamping math; under the new floating-pane-backed
implementation the popup's actual on-screen geometry shifts slightly,
so the hardcoded grab points miss the border entirely and the popup
never moves. This isn't a redraw regression: the underlying drag
mechanism (resize-pane -M / move-pane -M) is already covered by
floating-pane-drag-scrollbar-strip.sh and still works correctly for a
popup in popup-drag-wide-character.sh, which is unaffected and stays.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
display-popup is now a compatibility shim over a floating pane (upstream
removed the whole overlay/popup machinery in favour of floating panes -
see the preceding merge). These three tests moved/resized a popup with
hardcoded mouse coordinates tuned to the old popup implementation's
exact positioning and clamping math; under the new floating-pane-backed
implementation the popup's actual on-screen geometry shifts slightly,
so the hardcoded grab points miss the border entirely and the popup
never moves. This isn't a redraw regression: the underlying drag
mechanism (resize-pane -M / move-pane -M) is already covered by
floating-pane-drag-scrollbar-strip.sh and still works correctly for a
popup in popup-drag-wide-character.sh, which is unaffected and stays.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The test's session used the default $SHELL - the developer's own
interactive bash, whose PS1 embeds an OSC 0 title-setter for tmux/xterm
TERM types (common on this machine, not test-controlled). That redrew
the terminal title on every prompt, clobbering the test's own
OSC-2-set-title assertions regardless of how long it waited afterwards.
Use bash --noprofile --norc, matching the pattern already used by
style-trim.sh.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* refs/remotes/tmux-openbsd/master:
Do not do anything in session_detach and rely on the caller destroying the session, otherwise grouped sessions can reuse a session with no windows. From Jeong, Heon in GitHub issue 5620.
