From cb43c581508d8d597a54dd963c3a1989c57d585e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 23 Feb 2022 18:59:06 +0100 Subject: [PATCH] commands: refactor scrollback up/down MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When moving the viewport in the scrollback (i.e. “scrolling”), we need to ensure the viewport is not moved past the beginning, or end, of the scrollback. This was previously accomplish by first limiting the number of lines to scroll to the number of visible rows (i.e the viewport _size_), and by adjusting the viewport after moving it, to ensure it doesn’t point into an uninitialized scrollback area etc. I.e. the implementation was _reactive_. This patch rewrites the logic to be _proactive_; we now calculate _where_ the beginning (or end) of the scrollback is, and then how many lines there is from there, to the viewport. This is our _maximum_ number of lines to scroll. When done correctly (which I hope this patch does), this allows us to remove _all_ checks after moving the viewport - we already _know_ it’s correct, and valid. As a bonus, we can remove the old limit, where scrolling was only allowed to be at most a single page. --- commands.c | 154 +++++++++++++++++++++-------------------------------- 1 file changed, 62 insertions(+), 92 deletions(-) diff --git a/commands.c b/commands.c index 4b0306aa..d6ec9fa0 100644 --- a/commands.c +++ b/commands.c @@ -18,68 +18,58 @@ cmd_scrollback_up(struct terminal *term, int rows) if (urls_mode_is_active(term)) return; - rows = min(rows, term->rows); - xassert(term->grid->offset >= 0); + const struct grid *grid = term->grid; + const int offset = grid->offset; + const int view = grid->view; + const int grid_rows = grid->num_rows; + const int screen_rows = term->rows; - int new_view = term->grid->view - rows; - while (new_view < 0) - new_view += term->grid->num_rows; - new_view %= term->grid->num_rows; + int scrollback_start = (offset + screen_rows) & (grid_rows - 1); - xassert(new_view >= 0); - xassert(new_view < term->grid->num_rows); - - /* Avoid scrolling in uninitialized rows */ - while (term->grid->rows[new_view] == NULL) - new_view = (new_view + 1) % term->grid->num_rows; - - if (new_view == term->grid->view) { - /* - * This happens when scrolling up in a newly opened terminal; - * every single line (except those already visible) are - * uninitialized, and the loop above will bring us back to - * where we started. - */ - return; + /* Part of the scrollback may be uninitialized */ + while (grid->rows[scrollback_start] == NULL) { + scrollback_start++; + scrollback_start &= grid_rows - 1; } - /* Don't scroll past scrollback history */ - int end = (term->grid->offset + term->rows - 1) % term->grid->num_rows; - if (end >= term->grid->offset) { - /* Not wrapped */ - if (new_view >= term->grid->offset && new_view <= end) - new_view = (end + 1) % term->grid->num_rows; + /* Number of rows to scroll, without going past the scrollback start */ + int max_rows = 0; + if (view + screen_rows >= grid_rows) { + /* View crosses scrollback wrap-around */ + xassert(scrollback_start <= view); + max_rows = view - scrollback_start; } else { - if (new_view >= term->grid->offset || new_view <= end) - new_view = (end + 1) % term->grid->num_rows; + if (scrollback_start <= view) + max_rows = view - scrollback_start; + else + max_rows = view + (grid_rows - scrollback_start); } - while (term->grid->rows[new_view] == NULL) - new_view = (new_view + 1) % term->grid->num_rows; + rows = min(rows, max_rows); + if (rows == 0) + return; + int new_view = (view + grid_rows) - rows; + new_view &= grid_rows - 1; + + xassert(new_view != view); + xassert(grid->rows[new_view] != NULL); #if defined(_DEBUG) for (int r = 0; r < term->rows; r++) - xassert(term->grid->rows[(new_view + r) % term->grid->num_rows] != NULL); + xassert(grid->rows[(new_view + r) & (grid->num_rows - 1)] != NULL); #endif LOG_DBG("scrollback UP: %d -> %d (offset = %d, end = %d, rows = %d)", - term->grid->view, new_view, term->grid->offset, end, term->grid->num_rows); - - if (new_view == term->grid->view) - return; - - int diff = -1; - if (new_view < term->grid->view) - diff = term->grid->view - new_view; - else - diff = (term->grid->num_rows - new_view) + term->grid->view; + view, new_view, offset, end, grid_rows); selection_view_up(term, new_view); term->grid->view = new_view; - if (diff >= 0 && diff < term->rows) { - term_damage_scroll(term, DAMAGE_SCROLL_REVERSE_IN_VIEW, (struct scroll_region){0, term->rows}, diff); - term_damage_rows_in_view(term, 0, diff - 1); + if (rows < term->rows) { + term_damage_scroll( + term, DAMAGE_SCROLL_REVERSE_IN_VIEW, + (struct scroll_region){0, term->rows}, rows); + term_damage_rows_in_view(term, 0, rows - 1); } else term_damage_view(term); @@ -95,65 +85,45 @@ cmd_scrollback_down(struct terminal *term, int rows) if (urls_mode_is_active(term)) return; - if (term->grid->view == term->grid->offset) + const struct grid *grid = term->grid; + const int offset = grid->offset; + const int view = grid->view; + const int grid_rows = grid->num_rows; + const int screen_rows = term->rows; + + const int scrollback_end = offset; + + /* Number of rows to scroll, without going past the scrollback end */ + int max_rows = 0; + if (view <= scrollback_end) + max_rows = scrollback_end - view; + else + max_rows = offset + (grid_rows - view); + + rows = min(rows, max_rows); + if (rows == 0) return; - rows = min(rows, term->rows); - xassert(term->grid->offset >= 0); - - int new_view = (term->grid->view + rows) % term->grid->num_rows; - xassert(new_view >= 0); - xassert(new_view < term->grid->num_rows); - - /* Prevent scrolling in uninitialized rows */ - bool all_initialized = false; - do { - all_initialized = true; - - for (int i = 0; i < term->rows; i++) { - int row_no = (new_view + i) % term->grid->num_rows; - if (term->grid->rows[row_no] == NULL) { - all_initialized = false; - new_view--; - break; - } - } - } while (!all_initialized); - - /* Don't scroll past scrollback history */ - int end = (term->grid->offset + term->rows - 1) % term->grid->num_rows; - if (end >= term->grid->offset) { - /* Not wrapped */ - if (new_view >= term->grid->offset && new_view <= end) - new_view = term->grid->offset; - } else { - if (new_view >= term->grid->offset || new_view <= end) - new_view = term->grid->offset; - } + int new_view = (view + rows) & (grid_rows - 1); + xassert(new_view != view); + xassert(grid->rows[new_view] != NULL); #if defined(_DEBUG) for (int r = 0; r < term->rows; r++) - xassert(term->grid->rows[(new_view + r) % term->grid->num_rows] != NULL); + xassert(grid->rows[(new_view + r) & (grid_rows - 1)] != NULL); #endif LOG_DBG("scrollback DOWN: %d -> %d (offset = %d, end = %d, rows = %d)", - term->grid->view, new_view, term->grid->offset, end, term->grid->num_rows); - - if (new_view == term->grid->view) - return; - - int diff = -1; - if (new_view > term->grid->view) - diff = new_view - term->grid->view; - else - diff = (term->grid->num_rows - term->grid->view) + new_view; + view, new_view, offset, end, grid_rows); selection_view_down(term, new_view); term->grid->view = new_view; - if (diff >= 0 && diff < term->rows) { - term_damage_scroll(term, DAMAGE_SCROLL_IN_VIEW, (struct scroll_region){0, term->rows}, diff); - term_damage_rows_in_view(term, term->rows - diff, term->rows - 1); + if (rows < term->rows) { + term_damage_scroll( + term, DAMAGE_SCROLL_IN_VIEW, + (struct scroll_region){0, term->rows}, rows); + term_damage_rows_in_view(term, term->rows - rows, screen_rows - 1); } else term_damage_view(term);