From 4975be29df848db9b339b5390290e4eb2ac8e140 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Thu, 10 Dec 2020 02:10:24 +0000 Subject: [PATCH] Fix scrolling region performance with fixed lines This resolves an issue with Alacritty's scrolling region performance when there's a number of fixed lines at the top of the screen. This affects commonly used applications like tmux or vim. Instead of using separate logic for when the scrolling region starts at the top of the screen without any fixed lines, the code should now try to figure out the target position of these fixed lines ahead of time, swap them into place and still perform the optimized implementation to move the grid. This comes with the small trade-off that since lines are swapped before rotating the screen without clearing or removing any lines during the rotation process, that the places the fixed lines have been swapped with will appear out of order when using scrolling regions in the primary screen buffer. Since the use of scrolling regions primarily affects the alternate screen and most terminals don't keep any history at all, this should however not cause any problems. --- CHANGELOG.md | 1 + alacritty_terminal/src/grid/mod.rs | 125 +++++++++++++++---------- alacritty_terminal/src/grid/storage.rs | 12 +-- 3 files changed, 84 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34dde984..32844199 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Preserve vi mode across terminal `reset` - Escapes `CSI Ps b` and `CSI Ps Z` with large parameters locking up Alacritty - Dimming colors which use the indexed `CSI 38 : 5 : Ps m` notation +- Performance of scrolling regions with offset from the bottom ### Removed diff --git a/alacritty_terminal/src/grid/mod.rs b/alacritty_terminal/src/grid/mod.rs index 21e7e2f9..1c23bfc8 100644 --- a/alacritty_terminal/src/grid/mod.rs +++ b/alacritty_terminal/src/grid/mod.rs @@ -223,25 +223,49 @@ impl Grid { T: ResetDiscriminant, D: PartialEq, { - // Whether or not there is a scrolling region active, as long as it - // starts at the top, we can do a full rotation which just involves - // changing the start index. - // - // To accommodate scroll regions, rows are reordered at the end. - if region.start == Line(0) && self.max_scroll_limit == 0 { - // Rotate the entire line buffer. If there's a scrolling region - // active, the bottom lines are restored in the next step. - self.raw.rotate_up(*positions); + let screen_lines = self.screen_lines().0; - // Now, restore any scroll region lines. - let lines = self.lines; - for i in IndexRange(region.end..lines) { - self.raw.swap_lines(i, i + positions); + // When rotating the entire region, just reset everything. + if positions >= region.end - region.start { + for i in region.start.0..region.end.0 { + let index = screen_lines - i - 1; + self.raw[index].reset(&self.cursor.template); } - // Finally, reset recycled lines. - for i in IndexRange(Line(0)..positions) { - self.raw[i].reset(&self.cursor.template); + return; + } + + // Which implementation we can use depends on the existence of a scrollback history. + // + // Since a scrollback history prevents us from rotating the entire buffer downwards, we + // instead have to rely on a slower, swap-based implementation. + if self.max_scroll_limit == 0 { + // Swap the lines fixed at the bottom to their target positions after rotation. + // + // Since we've made sure that the rotation will never rotate away the entire region, we + // know that the position of the fixed lines before the rotation must already be + // visible. + // + // We need to start from the top, to make sure the fixed lines aren't swapped with each + // other. + let fixed_lines = screen_lines - region.end.0; + for i in (0..fixed_lines).rev() { + self.raw.swap(i, i + positions.0); + } + + // Rotate the entire line buffer downward. + self.raw.rotate_down(*positions); + + // Ensure all new lines are fully cleared. + for i in 0..positions.0 { + let index = screen_lines - i - 1; + self.raw[index].reset(&self.cursor.template); + } + + // Swap the fixed lines at the top back into position. + for i in 0..region.start.0 { + let index = screen_lines - i - 1; + self.raw.swap(index, index - positions.0); } } else { // Subregion rotation. @@ -263,46 +287,51 @@ impl Grid { T: ResetDiscriminant, D: PartialEq, { - let num_lines = self.screen_lines().0; + let screen_lines = self.screen_lines().0; - if region.start == Line(0) { - // Update display offset when not pinned to active area. - if self.display_offset != 0 { - self.display_offset = min(self.display_offset + *positions, self.max_scroll_limit); + // When rotating the entire region with fixed lines at the top, just reset everything. + if positions >= region.end - region.start && region.start != Line(0) { + for i in region.start.0..region.end.0 { + let index = screen_lines - i - 1; + self.raw[index].reset(&self.cursor.template); } - self.increase_scroll_limit(*positions); + return; + } - // Rotate the entire line buffer. If there's a scrolling region - // active, the bottom lines are restored in the next step. - self.raw.rotate(-(*positions as isize)); + // Update display offset when not pinned to active area. + if self.display_offset != 0 { + self.display_offset = min(self.display_offset + *positions, self.max_scroll_limit); + } - // This next loop swaps "fixed" lines outside of a scroll region - // back into place after the rotation. The work is done in buffer- - // space rather than terminal-space to avoid redundant - // transformations. - let fixed_lines = num_lines - *region.end; + // Create scrollback for the new lines. + self.increase_scroll_limit(*positions); - for i in 0..fixed_lines { - self.raw.swap(i, i + *positions); - } + // Swap the lines fixed at the top to their target positions after rotation. + // + // Since we've made sure that the rotation will never rotate away the entire region, we + // know that the position of the fixed lines before the rotation must already be + // visible. + // + // We need to start from the bottom, to make sure the fixed lines aren't swapped with each + // other. + for i in (0..region.start.0).rev() { + let index = screen_lines - i - 1; + self.raw.swap(index, index - positions.0); + } - // Finally, reset recycled lines. - // - // Recycled lines are just above the end of the scrolling region. - for i in 0..*positions { - self.raw[i + fixed_lines].reset(&self.cursor.template); - } - } else { - // Subregion rotation. - for line in IndexRange(region.start..(region.end - positions)) { - self.raw.swap_lines(line, line + positions); - } + // Rotate the entire line buffer upward. + self.raw.rotate(-(positions.0 as isize)); - // Clear reused lines. - for line in IndexRange((region.end - positions)..region.end) { - self.raw[line].reset(&self.cursor.template); - } + // Ensure all new lines are fully cleared. + for i in 0..positions.0 { + self.raw[i].reset(&self.cursor.template); + } + + // Swap the fixed lines at the bottom back into position. + let fixed_lines = screen_lines - region.end.0; + for i in 0..fixed_lines { + self.raw.swap(i, i + positions.0); } } diff --git a/alacritty_terminal/src/grid/storage.rs b/alacritty_terminal/src/grid/storage.rs index dd8dbb22..96c578b3 100644 --- a/alacritty_terminal/src/grid/storage.rs +++ b/alacritty_terminal/src/grid/storage.rs @@ -12,9 +12,9 @@ const MAX_CACHE_SIZE: usize = 1_000; /// A ring buffer for optimizing indexing and rotation. /// -/// The [`Storage::rotate`] and [`Storage::rotate_up`] functions are fast modular additions on the -/// internal [`zero`] field. As compared with [`slice::rotate_left`] which must rearrange items in -/// memory. +/// The [`Storage::rotate`] and [`Storage::rotate_down`] functions are fast modular additions on +/// the internal [`zero`] field. As compared with [`slice::rotate_left`] which must rearrange items +/// in memory. /// /// As a consequence, both [`Index`] and [`IndexMut`] are reimplemented for this type to account /// for the zeroth element not always being at the start of the allocation. @@ -186,16 +186,16 @@ impl Storage { debug_assert!(count.abs() as usize <= self.inner.len()); let len = self.inner.len(); - self.zero = (self.zero as isize + count + len as isize) as usize % self.inner.len(); + self.zero = (self.zero as isize + count + len as isize) as usize % len; } - /// Rotate the grid up, moving all existing lines down in history. + /// Rotate all existing lines down in history. /// /// This is a faster, specialized version of [`rotate_left`]. /// /// [`rotate_left`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.rotate_left #[inline] - pub fn rotate_up(&mut self, count: usize) { + pub fn rotate_down(&mut self, count: usize) { self.zero = (self.zero + count) % self.inner.len(); }