mirrors/foot
1
0
Fork 0
mirror of https://codeberg.org/dnkl/foot.git synced 2026-02-04 04:06:06 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions

term: scroll: only record scroll damage when viewport is at the bottom

Browse source 
We don’t need to record scroll damage if the viewport isn’t at the
bottom, since in this case, the renderer ignores the scroll damage
anyway.

This fixes a performance corner case, when the viewport is at the top
of the scrollback history.

When application scrolls the terminal contents, and the scrollback
history is full, and the viewport is at top of the history, then the
viewport needs to be moved (the scrollback history is a circular
buffer, and thus the top of the history “moves” when we’re scrolling
in new contents).

Moving the viewport typically results in another type of scroll
damage (DAMAGE_SCROLL_IN_VIEW, instead of the “normal” DAMAGE_SCROLL).

Thus, each application triggered scroll, will result in two scroll
damage records: one DAMAGE_SCROLL, and one
DAMAGE_SCROLL_IN_VIEW. These two are incompatible, meaning they can’t
be merged. What’s worse, it also means the DAMAGE_SCROLL records from
two application triggered scrolls cannot be merged (since there’s a
DAMAGE_SCROLL_IN_VIEW in between).

As a result, the renderer will not see one, or “a few” scroll damage
events, but a *ton*. _Each_ one typically a single line, or so. And
each one resulting in lots of traffic on the wayland socket, as we
create and destroy new buffer pools, when doing “shm scrolling”.

This eventually leads to the socket not being able to keep up, and the
socket is closed on us, forcing us to exit.

The fix is really simple: don’t record “normal” scroll damage when
scrolling, _unless_ the viewport is at the bottom (and thus “follows”
the application output).

As soon as the user scrolls up in the history, we’ll stop emitting
normal scroll damage records. This is just fine, since, as mentioned
above, the renderer ignores them when the viewport isn’t at the
bottom.

What if the viewport is moved back down again, before the next frame
has been rendered? Wont there be “missing” scroll damage records? No,
because moving the viewport results in scroll damage records by
itself.

Closes #1380
This commit is contained in:
Daniel Eklöf 2023-06-23 20:20:01 +02:00
parent 70ffc2632f
commit 8a3620bafa
No known key found for this signature in database
GPG key ID: 5BBD4992C116573F
2 changed files with 7 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files

3
CHANGELOG.md
View file

@ -89,10 +89,13 @@
opaque and non-opaque at runtime (using OSC-11).
* Regression: crash when resizing the window when `resize-delay-ms >
0` ([#1377][1377]).
* Crash when scrolling up while running something that generates a lot
of output (for example, `yes`) ([#1380][1380]).
[1317]: https://codeberg.org/dnkl/foot/issues/1317
[1355]: https://codeberg.org/dnkl/foot/issues/1355
[1377]: https://codeberg.org/dnkl/foot/issues/1377
[1380]: https://codeberg.org/dnkl/foot/issues/1380
### Security

8
terminal.c
View file

@ -2714,6 +2714,7 @@ term_scroll_partial(struct terminal *term, struct scroll_region region, int rows
term->grid->offset &= term->grid->num_rows - 1;
if (likely(view_follows)) {
term_damage_scroll(term, DAMAGE_SCROLL, region, rows);
selection_view_down(term, term->grid->offset);
term->grid->view = term->grid->offset;
} else if (unlikely(rows > view_sb_start_distance)) {
@ -2737,13 +2738,12 @@ term_scroll_partial(struct terminal *term, struct scroll_region region, int rows
erase_line(term, row);
}
term->grid->cur_row = grid_row(term->grid, term->grid->cursor.point.row);
#if defined(_DEBUG)
for (int r = 0; r < term->rows; r++)
xassert(grid_row(term->grid, r) != NULL);
#endif
term_damage_scroll(term, DAMAGE_SCROLL, region, rows);
term->grid->cur_row = grid_row(term->grid, term->grid->cursor.point.row);
}
void
@ -2800,6 +2800,7 @@ term_scroll_reverse_partial(struct terminal *term,
xassert(term->grid->offset < term->grid->num_rows);
if (view_follows) {
term_damage_scroll(term, DAMAGE_SCROLL_REVERSE, region, rows);
selection_view_up(term, term->grid->offset);
term->grid->view = term->grid->offset;
}
@ -2818,7 +2819,6 @@ term_scroll_reverse_partial(struct terminal *term,
erase_line(term, row);
}
term_damage_scroll(term, DAMAGE_SCROLL_REVERSE, region, rows);
term->grid->cur_row = grid_row(term->grid, term->grid->cursor.point.row);
#if defined(_DEBUG)