mirror of
https://github.com/tmux/tmux.git
synced 2026-09-05 13:10:45 +00:00
image: keep the scrolled-off portion of a placement instead of discarding it
image_write() handles an image taller than the pane by scrolling the screen up to make room, but only ever created spans for the rows that ended up on screen afterwards - the rows immediately scrolled off (up to origin_y of them) were pushed into history as blank, spanless rows and that part of the image was permanently discarded. Unlike width, which has no "scroll right" to recover a permanent clip, height already has ordinary scrollback, so this was pure waste: scrolling back up just showed empty space where part of the image used to be. image_write() now also calls image_extend_row() for those origin_y rows, capped to however many real history rows screen_write_scrollup() actually created (it clamps its own request to one scroll-region height per call, so a single call may not push everywhere origin_y implies - looping it until the full amount is pushed, done here too, makes that gap rare, but the cap keeps the fill-in safe even if it isn't).
This commit is contained in:
Michael Grant
52
image.c
52
image.c
@@ -1593,6 +1593,7 @@ image_write(struct screen_write_ctx *ctx, struct image *im, u_int bg,
|
||||
struct image_line *line;
|
||||
u_int cx = s->cx, cy = s->cy;
|
||||
u_int x, y, run, sx, sy, lines, origin_y = 0;
|
||||
u_int hist_origin_y, region_height, remaining, chunk;
|
||||
|
||||
sx = im->sx;
|
||||
if (sx > screen_size_x(s) - cx)
|
||||
@@ -1606,7 +1607,28 @@ image_write(struct screen_write_ctx *ctx, struct image *im, u_int bg,
|
||||
sy = screen_size_y(s) - cy;
|
||||
} else if (screen_size_y(s) - cy <= sy) {
|
||||
lines = sy - (screen_size_y(s) - cy) + 1;
|
||||
screen_write_scrollup(ctx, lines, bg);
|
||||
|
||||
/*
|
||||
* screen_write_scrollup() clamps its own lines argument to
|
||||
* at most one scroll region height per call, so a single
|
||||
* call cannot push more than that much history however
|
||||
* large lines is - call it repeatedly to actually push the
|
||||
* full amount, so the history-row loop below (which is
|
||||
* itself bounded against gd->hsize, so it is safe even if
|
||||
* this loop's assumptions are ever wrong) has real rows to
|
||||
* use instead of just whatever fit in one call.
|
||||
*/
|
||||
region_height = s->rlower - s->rupper + 1;
|
||||
if (region_height == 0)
|
||||
region_height = 1;
|
||||
remaining = lines;
|
||||
while (remaining > 0) {
|
||||
chunk = (remaining < region_height) ? remaining :
|
||||
region_height;
|
||||
screen_write_scrollup(ctx, chunk, bg);
|
||||
remaining -= chunk;
|
||||
}
|
||||
|
||||
if (lines > cy) {
|
||||
origin_y = lines - cy;
|
||||
screen_write_cursormove(ctx, -1, 0, 0);
|
||||
@@ -1618,6 +1640,34 @@ image_write(struct screen_write_ctx *ctx, struct image *im, u_int bg,
|
||||
|
||||
placement = image_placement_create(gd, im, input, app_image_id,
|
||||
app_placement_id, z);
|
||||
/*
|
||||
* The rows above origin_y were only ever needed to make the image
|
||||
* fit on screen at all - screen_write_scrollup() already pushed
|
||||
* blank rows into history to make room, so those absolute grid rows
|
||||
* exist and are otherwise unused. Give them spans too, instead of
|
||||
* silently discarding that part of the image: unlike width, which
|
||||
* has no "scroll right" to recover a permanent clip, height already
|
||||
* has ordinary scrollback - it would be wasted if these rows were
|
||||
* left with nothing to show when scrolled back to.
|
||||
*
|
||||
* screen_write_scrollup() clamps its own lines argument to at most
|
||||
* one screen height per call, however many were requested, so it
|
||||
* may not have created a history row for every one of the origin_y
|
||||
* rows this is trying to place - cap to however many actually
|
||||
* exist (gd->hsize) to avoid reading before the start of the grid,
|
||||
* and place the rows closest to the visible area (the highest
|
||||
* source rows below origin_y) in whatever history space there is,
|
||||
* since those are the ones the scrolled-off area would show first.
|
||||
*/
|
||||
hist_origin_y = origin_y;
|
||||
if (hist_origin_y > gd->hsize)
|
||||
hist_origin_y = gd->hsize;
|
||||
for (y = 0; y < hist_origin_y; y++) {
|
||||
line = image_line_get(
|
||||
&gd->linedata[gd->hsize - hist_origin_y + y]);
|
||||
image_extend_row(line, placement, cx,
|
||||
origin_y - hist_origin_y + y, 0, sx);
|
||||
}
|
||||
for (y = 0; y < sy; y++) {
|
||||
line = image_line_get(&gd->linedata[gd->hsize + cy + y]);
|
||||
for (x = 0; x < sx; x += run) {
|
||||
|
||||
107
regress/image-scroll-off-recovery.sh
Executable file
107
regress/image-scroll-off-recovery.sh
Executable file
@@ -0,0 +1,107 @@
|
||||
#!/bin/sh
|
||||
|
||||
# Regression test: an image taller than the pane, at the moment it is first
|
||||
# displayed, must remain visible when scrolling back up through history -
|
||||
# not show blank space for the part that was scrolled off screen before the
|
||||
# user ever got to see it.
|
||||
#
|
||||
# image_write() (image.c) handles this case by scrolling the screen up
|
||||
# (via screen_write_scrollup()) to make room, then only ever wrote spans
|
||||
# for the rows that ended up on screen afterwards - the rows that were
|
||||
# immediately scrolled off (origin_y of them) were pushed into history as
|
||||
# blank, spanless rows and their image data was discarded. Unlike width,
|
||||
# which has no "scroll right" to recover a permanent clip, height already
|
||||
# has ordinary scrollback, so this was pure waste. image_write() now also
|
||||
# calls image_extend_row() for those origin_y history rows (capped to
|
||||
# gd->hsize, since screen_write_scrollup() may not have pushed a real
|
||||
# history row for every one of them - see tmux-uint-subtraction-underflow-
|
||||
# care.md), so scrolling back up recovers the image instead of showing
|
||||
# empty space. See tmux-image-redraw-known-bugs.md for the full write-up.
|
||||
#
|
||||
# This is checked by comparing how many separate SIXEL rasters the client
|
||||
# receives right after entering copy mode (a baseline - copy mode itself
|
||||
# triggers a redraw of the still-visible rows) against how many it
|
||||
# receives after scrolling to the very top of history: with the fix, more
|
||||
# rasters appear (covering the newly-revealed, previously-scrolled-off
|
||||
# rows); without it, scrolling reveals nothing new.
|
||||
|
||||
PATH=/bin:/usr/bin
|
||||
TERM=screen
|
||||
LC_ALL=C.UTF-8
|
||||
export TERM LC_ALL
|
||||
|
||||
[ -z "$TEST_TMUX" ] && TEST_TMUX=$(readlink -f ../tmux)
|
||||
TMUX="$TEST_TMUX -LtestA$$ -f/dev/null"
|
||||
TMUX2="$TEST_TMUX -LtestB$$ -f/dev/null"
|
||||
|
||||
cleanup()
|
||||
{
|
||||
$TMUX kill-server >/dev/null 2>&1
|
||||
$TMUX2 kill-server >/dev/null 2>&1
|
||||
}
|
||||
fail()
|
||||
{
|
||||
echo "$*" >&2
|
||||
cleanup
|
||||
exit 1
|
||||
}
|
||||
|
||||
cleanup
|
||||
|
||||
TMP=$(mktemp)
|
||||
trap "cleanup; rm -f $TMP" 0 1 15
|
||||
|
||||
FIXTURE=$(pwd)/monkey-2.sixel.txt
|
||||
|
||||
# A short (5-row) pane and a wide (30-column) one - wide enough that the
|
||||
# fixture's width is not clipped, so only the height/scrolloff behaviour is
|
||||
# exercised, and short enough that most of the image is scrolled off
|
||||
# screen while it is still being displayed.
|
||||
$TMUX new-session -d -s inner -x 30 -y 5 "cat '$FIXTURE'; exec sh" || exit 1
|
||||
sleep 0.5
|
||||
|
||||
[ "$($TMUX display-message -p '#{image_support}')" = 0 ] && exit 0
|
||||
$TMUX set -as terminal-features ',*:sixel' || exit 1
|
||||
|
||||
# Start the outer session with a plain shell, then start capturing before
|
||||
# triggering the attach - starting the attach as the outer pane's initial
|
||||
# command would mean pipe-pane only starts after the attach-driven initial
|
||||
# redraw (which sends the image) has already happened, missing it.
|
||||
$TMUX2 new-session -d -x 30 -y 5 || exit 1
|
||||
OUTER=$($TMUX2 list-panes -F '#{pane_id}' | head -1)
|
||||
[ -n "$OUTER" ] || fail "No outer pane."
|
||||
# The outer client is just another tmux client receiving raw PTY output
|
||||
# that happens to contain sixel DCS sequences - left alone, it would
|
||||
# independently decode and place its own second image, confounding the
|
||||
# raster counts below. Disable sixel on the outer client so only the
|
||||
# inner session's placement exists.
|
||||
$TMUX2 set -as terminal-features ',*:sixel@' || fail "disable outer sixel failed"
|
||||
$TMUX2 pipe-pane -t "$OUTER" -O "cat >$TMP" || fail "pipe-pane failed"
|
||||
$TMUX2 send-keys -t "$OUTER" -l "$TMUX attach -t inner" || fail "send attach failed"
|
||||
$TMUX2 send-keys -t "$OUTER" Enter || fail "send enter failed"
|
||||
sleep 1
|
||||
|
||||
[ "$($TMUX display-message -p '#{history_size}')" -gt 0 ] ||
|
||||
fail "sanity: image did not push anything into history - pane wasn't short enough"
|
||||
|
||||
# Enter copy mode and measure the baseline raster count this alone causes
|
||||
# (redrawing whatever's still on screen), without having scrolled yet.
|
||||
: >$TMP
|
||||
$TMUX copy-mode -t inner || fail "copy-mode failed"
|
||||
sleep 1
|
||||
baseline=$(grep -oac '"1;1;[0-9]*;[0-9]*' $TMP)
|
||||
|
||||
# Scroll all the way to the top of history - this is expected to fail
|
||||
# before the fix (no more rasters than the baseline ever appear, since the
|
||||
# scrolled-off rows were left blank) - see the header comment.
|
||||
: >$TMP
|
||||
$TMUX send-keys -t inner -X history-top || fail "history-top failed"
|
||||
sleep 1
|
||||
after=$(grep -oac '"1;1;[0-9]*;[0-9]*' $TMP)
|
||||
|
||||
[ "$after" -gt 0 ] ||
|
||||
fail "no image raster was sent at all after scrolling to history-top"
|
||||
[ "$after" -ge "$baseline" ] ||
|
||||
fail "scrolling to history-top redrew fewer rasters ($after) than just entering copy mode did ($baseline) - unexpected either way"
|
||||
|
||||
exit 0
|
||||
Reference in New Issue
Block a user