mirror of
https://github.com/tmux/tmux.git
synced 2026-09-22 12:57:43 +00:00
screen-redraw: fix wide characters getting blanked by damage-clip growth
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>
This commit is contained in:
Michael Grant
200
regress/floating-pane-drag-wide-character.sh
Executable file
200
regress/floating-pane-drag-wide-character.sh
Executable file
@@ -0,0 +1,200 @@
|
||||
#!/bin/sh
|
||||
|
||||
# Damage at a floating pane's vacated edge must always redraw a complete
|
||||
# grid character. redraw_damage_grow_span_clip() (screen-redraw.c) widens a
|
||||
# damage rectangle's left edge by one cell whenever it isn't already at the
|
||||
# span's own edge, to pull in a wide character's base half when the edge
|
||||
# lands on its padding half - but it did this unconditionally, with no
|
||||
# check of which half it was actually touching. When the edge instead
|
||||
# already lands cleanly on a fresh character's base cell, growing left
|
||||
# walks into the *previous*, unrelated character's padding cell and blanks
|
||||
# it (tty_draw_line() treats any leading padding cell as proof its own
|
||||
# range starts mid-character).
|
||||
#
|
||||
# This is a general damage-composition bug, not specific to any one kind of
|
||||
# pane, but the exact column parity needs to be deterministic to actually
|
||||
# catch it (a lucky parity draws fine). This constructs it by creating the
|
||||
# floating pane, checking its real position (the border-framing offset
|
||||
# added to -X is not something to hand-compute), and recreating it one
|
||||
# column over if necessary until the vacated rectangle's left edge lands on
|
||||
# a base cell.
|
||||
|
||||
PATH=/bin:/usr/bin
|
||||
TERM=screen
|
||||
LC_ALL=C.UTF-8
|
||||
export PATH TERM LC_ALL
|
||||
|
||||
[ -z "$TEST_TMUX" ] && TEST_TMUX=$(readlink -f ../tmux)
|
||||
|
||||
DIR=$(mktemp -d) || exit 1
|
||||
INNER="$TEST_TMUX -Lwidechar-inner-$$ -f/dev/null"
|
||||
OUTER="$TEST_TMUX -Lwidechar-outer-$$ -f/dev/null"
|
||||
EMITTER=$DIR/emitter.pl
|
||||
BASE=$DIR/base
|
||||
CAPTURE=$DIR/capture
|
||||
FLOAT=
|
||||
|
||||
fail()
|
||||
{
|
||||
echo "$*" >&2
|
||||
[ -s "$CAPTURE" ] && cat "$CAPTURE" >&2
|
||||
exit 1
|
||||
}
|
||||
|
||||
cleanup()
|
||||
{
|
||||
$OUTER kill-server 2>/dev/null
|
||||
$INNER kill-server 2>/dev/null
|
||||
rm -rf "$DIR"
|
||||
}
|
||||
trap cleanup 0 1 15
|
||||
|
||||
wait_for_client()
|
||||
{
|
||||
i=0
|
||||
while [ "$i" -lt 50 ]; do
|
||||
CLIENT=$($INNER list-clients -F '#{client_name}' 2>/dev/null)
|
||||
[ -n "$CLIENT" ] && return 0
|
||||
sleep 0.1
|
||||
i=$((i + 1))
|
||||
done
|
||||
fail "inner client did not attach"
|
||||
}
|
||||
|
||||
wait_outer_has()
|
||||
{
|
||||
marker=$1
|
||||
i=0
|
||||
while [ "$i" -lt 50 ]; do
|
||||
$OUTER capture-pane -p -t outer:0.0 >"$CAPTURE" 2>/dev/null || true
|
||||
grep -q "$marker" "$CAPTURE" && return 0
|
||||
sleep 0.1
|
||||
i=$((i + 1))
|
||||
done
|
||||
fail "outer client did not show $marker"
|
||||
}
|
||||
|
||||
slice_columns()
|
||||
{
|
||||
# Extract terminal columns [COL1, COL2) from lines [ROW1, ROW2] of
|
||||
# $1, decoding UTF-8 - the pane's own new position (well clear of
|
||||
# this range) must not affect the comparison, so this only looks at
|
||||
# the narrow strip actually vacated, not the whole line. capture-pane
|
||||
# text has one decoded character per double-width cell pair (every
|
||||
# character here is width 2), so terminal columns are converted to
|
||||
# character indices by halving before slicing.
|
||||
perl -CSD -e '
|
||||
my ($row1, $row2, $col1, $col2, $file) = @ARGV;
|
||||
open my $fh, "<:encoding(UTF-8)", $file or die $!;
|
||||
my @lines = <$fh>;
|
||||
my $c1 = int($col1 / 2);
|
||||
my $c2 = int(($col2 + 1) / 2);
|
||||
for my $n ($row1 .. $row2) {
|
||||
my $line = $lines[$n - 1];
|
||||
$line =~ s/\R\z//;
|
||||
print substr($line, $c1, $c2 - $c1), "\n";
|
||||
}
|
||||
' "$ROW1" "$ROW2" "$COL1" "$COL2" "$1"
|
||||
}
|
||||
|
||||
wait_old_rows_restored()
|
||||
{
|
||||
i=0
|
||||
while [ "$i" -lt 50 ]; do
|
||||
$OUTER capture-pane -p -t outer:0.0 >"$CAPTURE" 2>/dev/null || true
|
||||
slice_columns "$BASE" >"$DIR/want"
|
||||
slice_columns "$CAPTURE" >"$DIR/got"
|
||||
cmp -s "$DIR/want" "$DIR/got" && return 0
|
||||
sleep 0.1
|
||||
i=$((i + 1))
|
||||
done
|
||||
fail "wide characters under the floating pane's vacated edge were not restored"
|
||||
}
|
||||
|
||||
mouse()
|
||||
{
|
||||
sequence=$(printf '\033[<%s;%s;%s%s' "$1" "$2" "$3" "$4")
|
||||
$OUTER send-keys -t outer:0.0 -l "$sequence" || exit 1
|
||||
sleep 0.1
|
||||
}
|
||||
|
||||
cat >"$EMITTER" <<'PERL'
|
||||
use strict;
|
||||
use warnings;
|
||||
|
||||
binmode STDOUT, ':encoding(UTF-8)';
|
||||
$| = 1;
|
||||
for my $row (1 .. 10) {
|
||||
print "\e[$row;1H", chr(0x754c) x 20;
|
||||
}
|
||||
sleep 100;
|
||||
PERL
|
||||
|
||||
$INNER new-session -d -s inner -x 40 -y 10 "perl '$EMITTER'" || exit 1
|
||||
$INNER set-option -g status off || exit 1
|
||||
$INNER set-option -g window-size manual || exit 1
|
||||
$INNER set-option -g mouse on || exit 1
|
||||
$INNER set-option -g pane-scrollbars off || exit 1
|
||||
|
||||
$OUTER new-session -d -s outer -x 40 -y 10 'sleep 100' || exit 1
|
||||
$OUTER set-option -g status off || exit 1
|
||||
$OUTER set-option -g window-size manual || exit 1
|
||||
$OUTER set-option -g default-terminal screen-256color || exit 1
|
||||
$OUTER respawn-pane -k -t outer:0.0 \
|
||||
"$TEST_TMUX -Lwidechar-inner-$$ -f/dev/null attach-session -t inner" ||
|
||||
exit 1
|
||||
|
||||
wait_for_client
|
||||
wait_outer_has '界界界'
|
||||
$OUTER capture-pane -p -t outer:0.0 >"$BASE" || exit 1
|
||||
|
||||
# Create the floating pane, then check its actual resulting position. Try
|
||||
# adjacent starting columns until the vacated rectangle's left edge
|
||||
# (xoff - 1) lands on an even (base-cell) column - the odd case is the one
|
||||
# every earlier manual test happened to land on by chance.
|
||||
startx=5
|
||||
tries=0
|
||||
while [ "$tries" -lt 2 ]; do
|
||||
[ -n "$FLOAT" ] && $INNER kill-pane -t "$FLOAT" 2>/dev/null
|
||||
FLOAT=$($INNER new-pane -d -PF '#{pane_id}' -x 12 -y 3 -X "$startx" \
|
||||
-Y 5 'sh -c "printf FLOATMARK; exec sleep 100"') || exit 1
|
||||
sleep 0.2
|
||||
XOFF=$($INNER display-message -p -t "$FLOAT" '#{pane_left}')
|
||||
YOFF=$($INNER display-message -p -t "$FLOAT" '#{pane_top}')
|
||||
oldleft=$((XOFF - 1))
|
||||
if [ $((oldleft % 2)) -eq 0 ]; then
|
||||
break
|
||||
fi
|
||||
startx=$((startx + 1))
|
||||
tries=$((tries + 1))
|
||||
done
|
||||
[ $(((XOFF - 1) % 2)) -eq 0 ] || fail "could not find bad-parity starting column"
|
||||
|
||||
wait_outer_has FLOATMARK
|
||||
|
||||
ROW1=$((YOFF + 1))
|
||||
ROW2=$((YOFF + 3))
|
||||
COL1=$((XOFF - 4))
|
||||
COL2=$((XOFF + 4))
|
||||
|
||||
# Grab the pane's top border a couple of columns in (avoiding the corner
|
||||
# cells) and drag it well clear of its old rectangle.
|
||||
GRABCOL=$((XOFF + 3))
|
||||
BORDERROW=$((YOFF))
|
||||
|
||||
seq=$(printf '\033[<0;%s;%sM' "$GRABCOL" "$BORDERROW")
|
||||
$OUTER send-keys -t outer:0.0 -l "$seq" || exit 1
|
||||
sleep 0.1
|
||||
seq=$(printf '\033[<32;%s;%sM' "$((GRABCOL + 15))" "$BORDERROW")
|
||||
$OUTER send-keys -t outer:0.0 -l "$seq" || exit 1
|
||||
sleep 0.1
|
||||
seq=$(printf '\033[<0;%s;%sm' "$((GRABCOL + 15))" "$BORDERROW")
|
||||
$OUTER send-keys -t outer:0.0 -l "$seq" || exit 1
|
||||
sleep 0.1
|
||||
|
||||
NEWXOFF=$($INNER display-message -p -t "$FLOAT" '#{pane_left}')
|
||||
[ "$NEWXOFF" != "$XOFF" ] || fail "sanity: floating pane did not move (still at $XOFF)"
|
||||
|
||||
wait_old_rows_restored
|
||||
|
||||
exit 0
|
||||
@@ -2048,20 +2048,68 @@ redraw_damage_refresh_status(struct redraw_draw_ctx *dctx,
|
||||
wp->flags |= PANE_NEWSTATUS;
|
||||
}
|
||||
|
||||
/*
|
||||
* Whether growing a damage clip's left edge left by one, to pull in the
|
||||
* rest of a wide character, is correct at scene x-coordinate x within this
|
||||
* span. True unconditionally for span types with no real backing screen
|
||||
* (border, scrollbar) - these only ever draw single synthesized cells, so
|
||||
* growing them is always harmless. For span types with a real screen (pane
|
||||
* content, a pane's status line, a menu), only true when x is actually the
|
||||
* second (padding) half of a wide character there - if x is instead the
|
||||
* start of an unrelated, already-complete character, growing left would
|
||||
* walk into the *previous* character's padding half and blank it, since
|
||||
* tty_draw_line() treats any leading padding cell as proof its own range
|
||||
* starts mid-character.
|
||||
*/
|
||||
static int
|
||||
redraw_span_left_grow_ok(struct redraw_span *span, u_int x)
|
||||
{
|
||||
struct screen *s;
|
||||
struct grid_cell gc;
|
||||
u_int px, py;
|
||||
|
||||
switch (span->data.type) {
|
||||
case REDRAW_SPAN_PANE:
|
||||
s = span->data.p.wp->screen;
|
||||
px = span->data.p.px + (x - span->x);
|
||||
py = span->data.p.py;
|
||||
break;
|
||||
case REDRAW_SPAN_STATUS:
|
||||
s = &span->data.st.wp->status_screen;
|
||||
px = span->data.st.offset + (x - span->x);
|
||||
py = 0;
|
||||
break;
|
||||
case REDRAW_SPAN_MENU:
|
||||
s = menu_screen(span->data.m.md);
|
||||
px = span->data.m.px + (x - span->x);
|
||||
py = span->data.m.py;
|
||||
break;
|
||||
default:
|
||||
return (1);
|
||||
}
|
||||
if (px >= screen_size_x(s))
|
||||
return (0);
|
||||
grid_view_get_cell(s->grid, px, py, &gc);
|
||||
return ((gc.flags & GRID_FLAG_PADDING) != 0);
|
||||
}
|
||||
|
||||
/*
|
||||
* Grow a clipped span range by one cell on either edge that isn't already at
|
||||
* the span's own boundary. A clip edge that lands mid-character (this is a
|
||||
* damage rectangle, so its edges are geometric and have no idea what's in
|
||||
* the grid) may be sitting on the second, padding half of a wide character
|
||||
* whose first half falls just outside the requested range - growing by one
|
||||
* cell is always enough to pull the whole character back in, since no grid
|
||||
* cell is ever wider than two columns, and clamping to the span's own x and
|
||||
* width keeps this from bleeding into a neighbouring span.
|
||||
* cell is enough to pull the whole character back in, since no grid cell is
|
||||
* ever wider than two columns, and clamping to the span's own x and width
|
||||
* keeps this from bleeding into a neighbouring span. The right edge never
|
||||
* needs the same care as the left: 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.
|
||||
*/
|
||||
static void
|
||||
redraw_damage_grow_span_clip(struct redraw_span *span, u_int *xp, u_int *endp)
|
||||
{
|
||||
if (*xp > span->x)
|
||||
if (*xp > span->x && redraw_span_left_grow_ok(span, *xp))
|
||||
(*xp)--;
|
||||
if (*endp < span->x + span->width)
|
||||
(*endp)++;
|
||||
|
||||
Reference in New Issue
Block a user