From 182a9d5c2e01ec9b155fc7edf547e8e6de2f2bd5 Mon Sep 17 00:00:00 2001 From: Nathan Lilienthal Date: Mon, 18 Nov 2019 16:15:25 -0500 Subject: [PATCH] Fix deletion of lines when clearing the screen Previously Alacritty would delete lines when clearing the screen, leading to a loss of data in the scrollback buffer. Instead of deleting these lines, they are now rotated outside of the visible region. This also fixes some issues with Alacritty only resetting lines partially when the background color of the template cell changed. Fixes #2199. --- CHANGELOG.md | 1 + alacritty_terminal/src/grid/mod.rs | 66 ++++++++++++++--- alacritty_terminal/src/grid/row.rs | 11 +-- alacritty_terminal/src/grid/storage.rs | 99 ++++++++++++++++++++++---- alacritty_terminal/src/term/mod.rs | 32 +++++---- 5 files changed, 170 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bedd1f..4596646 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - On Windows, query DirectWrite for recommended anti-aliasing settings +- Scroll lines out of the visible region instead of deleting them when clearing the screen ### Fixed diff --git a/alacritty_terminal/src/grid/mod.rs b/alacritty_terminal/src/grid/mod.rs index 3141208..09333b3 100644 --- a/alacritty_terminal/src/grid/mod.rs +++ b/alacritty_terminal/src/grid/mod.rs @@ -73,6 +73,32 @@ pub trait GridCell { } /// Represents the terminal display contents +/// +/// ```notrust +/// ┌─────────────────────────┐ <-- max_scroll_limit + lines +/// │ │ +/// │ UNINITIALIZED │ +/// │ │ +/// ├─────────────────────────┤ <-- raw.len() +/// │ │ +/// │ RESIZE BUFFER │ +/// │ │ +/// ├─────────────────────────┤ <-- scroll_limit + lines +/// │ │ +/// │ SCROLLUP REGION │ +/// │ │ +/// ├─────────────────────────┤v lines +/// │ │| +/// │ VISIBLE REGION │| +/// │ │| +/// ├─────────────────────────┤^ <-- display_offset +/// │ │ +/// │ SCROLLDOWN REGION │ +/// │ │ +/// └─────────────────────────┘ <-- zero +/// ^ +/// cols +/// ``` #[derive(Clone, Debug, Deserialize, Serialize)] pub struct Grid { /// Lines in the grid. Each row holds a list of cells corresponding to the @@ -82,9 +108,7 @@ pub struct Grid { /// Number of columns cols: index::Column, - /// Number of lines. - /// - /// Invariant: lines is equivalent to raw.len() + /// Number of visible lines. lines: index::Line, /// Offset of displayed area @@ -472,10 +496,10 @@ impl Grid { selection.rotate(*positions as isize); } - // // 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. + // 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 = *self.num_lines() - *region.end; for i in 0..fixed_lines { @@ -501,11 +525,30 @@ impl Grid { } } + pub fn clear_viewport(&mut self, template: &T) { + // Determine how many lines to scroll up by. + let end = Point { line: 0, col: self.num_cols() }; + let mut iter = self.iter_from(end); + while let Some(cell) = iter.prev() { + if !cell.is_empty() || iter.cur.line >= *self.lines { + break; + } + } + debug_assert!(iter.cur.line <= *self.lines); + let positions = self.lines - iter.cur.line; + let region = Line(0)..self.num_lines(); + + // Clear the viewport + self.scroll_up(®ion, positions, template); + + // Reset rotated lines + for i in positions.0..self.lines.0 { + self.raw[i].reset(&template); + } + } + // Completely reset the grid state pub fn reset(&mut self, template: &T) { - // Explicitly purge all lines from history - let shrinkage = self.raw.len() - self.lines.0; - self.raw.shrink_lines(shrinkage); self.clear_history(); // Reset all visible lines @@ -535,6 +578,9 @@ impl Grid { } pub fn clear_history(&mut self) { + // Explicitly purge all lines from history + let shrinkage = self.raw.len() - self.lines.0; + self.raw.shrink_lines(shrinkage); self.scroll_limit = 0; } diff --git a/alacritty_terminal/src/grid/row.rs b/alacritty_terminal/src/grid/row.rs index 058583f..daee440 100644 --- a/alacritty_terminal/src/grid/row.rs +++ b/alacritty_terminal/src/grid/row.rs @@ -90,12 +90,15 @@ impl Row { where T: GridCell, { - for item in &mut self.inner[..self.occ] { - *item = *template; - } - if template.is_empty() { + for item in &mut self.inner[..self.occ] { + *item = *template; + } self.occ = 0; + } else { + let len = self.inner.len(); + self.inner = vec![*template; len]; + self.occ = len; } } } diff --git a/alacritty_terminal/src/grid/storage.rs b/alacritty_terminal/src/grid/storage.rs index 10091f2..1a6d991 100644 --- a/alacritty_terminal/src/grid/storage.rs +++ b/alacritty_terminal/src/grid/storage.rs @@ -1,17 +1,4 @@ use std::cmp::Ordering; -/// Wrapper around Vec which supports fast indexing and rotation -/// -/// The rotation implemented by grid::Storage is a simple integer addition. -/// Compare with standard library rotation which requires rearranging items in -/// memory. -/// -/// As a consequence, the indexing operators need to be reimplemented for this -/// type to account for the 0th element not always being at the start of the -/// allocation. -/// -/// Because certain Vec operations are no longer valid on this type, no Deref -/// implementation is provided. Anything from Vec that should be exposed must be -/// done so manually. use std::ops::{Index, IndexMut}; use std::vec::Drain; @@ -23,10 +10,34 @@ use crate::index::Line; /// Maximum number of invisible lines before buffer is resized const TRUNCATE_STEP: usize = 100; +/// 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. +/// +/// 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. +/// +/// Because certain [`Vec`] operations are no longer valid on this type, no [`Deref`] +/// implementation is provided. Anything from [`Vec`] that should be exposed must be done so +/// manually. +/// +/// [`slice::rotate_left`]: https://doc.rust-lang.org/std/primitive.slice.html#method.rotate_left +/// [`Deref`]: std::ops::Deref +/// [`zero`]: #structfield.zero #[derive(Clone, Debug, Deserialize, Serialize)] pub struct Storage { inner: Vec>, + + /// Starting point for the storage of rows. + /// + /// This value represents the starting line offset within the ring buffer. The value of this + /// offset may be larger than the `len` itself, and will wrap around to the start to form the + /// ring buffer. It represents the bottommost line of the terminal. zero: usize, + + /// An index separating the visible and scrollback regions. visible_lines: Line, /// Total number of lines currently active in the terminal (scrollback + visible) @@ -326,6 +337,68 @@ mod test { fn set_wrap(&mut self, _wrap: bool) {} } + #[test] + fn with_capacity() { + let storage = Storage::with_capacity(Line(3), Row::new(Column(0), &' ')); + + assert_eq!(storage.inner.len(), 3); + assert_eq!(storage.len, 3); + assert_eq!(storage.zero, 0); + assert_eq!(storage.visible_lines, Line(2)); + } + + #[test] + fn indexing() { + let mut storage = Storage::with_capacity(Line(3), Row::new(Column(0), &' ')); + + storage[0] = Row::new(Column(1), &'0'); + storage[1] = Row::new(Column(1), &'1'); + storage[2] = Row::new(Column(1), &'2'); + + assert_eq!(storage[0], Row::new(Column(1), &'0')); + assert_eq!(storage[1], Row::new(Column(1), &'1')); + assert_eq!(storage[2], Row::new(Column(1), &'2')); + + storage.zero += 1; + + assert_eq!(storage[0], Row::new(Column(1), &'1')); + assert_eq!(storage[1], Row::new(Column(1), &'2')); + assert_eq!(storage[2], Row::new(Column(1), &'0')); + } + + #[test] + #[should_panic] + fn indexing_above_len() { + let mut storage = Storage::with_capacity(Line(3), Row::new(Column(0), &' ')); + storage.shrink_lines(2); + let _ = &storage[1]; + } + + #[test] + #[should_panic] + fn indexing_above_inner_len() { + let storage = Storage::with_capacity(Line(1), Row::new(Column(0), &' ')); + let _ = &storage[2]; + } + + #[test] + fn rotate() { + let mut storage = Storage::with_capacity(Line(3), Row::new(Column(0), &' ')); + storage.rotate(2); + assert_eq!(storage.zero, 2); + storage.shrink_lines(2); + assert_eq!(storage.len, 1); + assert_eq!(storage.inner.len(), 3); + assert_eq!(storage.zero, 2); + } + + #[test] + #[should_panic] + fn rotate_overflow() { + let mut storage = Storage::with_capacity(Line(3), Row::new(Column(0), &' ')); + storage.rotate(4); + } + /// Grow the buffer one line at the end of the buffer /// /// Before: diff --git a/alacritty_terminal/src/term/mod.rs b/alacritty_terminal/src/term/mod.rs index 8d69fb3..6a51ba3 100644 --- a/alacritty_terminal/src/term/mod.rs +++ b/alacritty_terminal/src/term/mod.rs @@ -701,7 +701,9 @@ pub struct Term { /// Mode flags mode: TermMode, - /// Scroll region + /// Scroll region. + /// + /// Range going from top to bottom of the terminal, indexed from the top of the viewport. scroll_region: Range, pub dirty: bool, @@ -1749,17 +1751,6 @@ impl ansi::Handler for Term { self.grid.selection = None; match mode { - ansi::ClearMode::Below => { - for cell in &mut self.grid[self.cursor.point.line][self.cursor.point.col..] { - cell.reset(&template); - } - if self.cursor.point.line < self.grid.num_lines() - 1 { - self.grid - .region_mut((self.cursor.point.line + 1)..) - .each(|cell| cell.reset(&template)); - } - }, - ansi::ClearMode::All => self.grid.region_mut(..).each(|c| c.reset(&template)), ansi::ClearMode::Above => { // If clearing more than one line if self.cursor.point.line > Line(1) { @@ -1774,6 +1765,23 @@ impl ansi::Handler for Term { cell.reset(&template); } }, + ansi::ClearMode::Below => { + for cell in &mut self.grid[self.cursor.point.line][self.cursor.point.col..] { + cell.reset(&template); + } + if self.cursor.point.line < self.grid.num_lines() - 1 { + self.grid + .region_mut((self.cursor.point.line + 1)..) + .each(|cell| cell.reset(&template)); + } + }, + ansi::ClearMode::All => { + if self.mode.contains(TermMode::ALT_SCREEN) { + self.grid.region_mut(..).each(|c| c.reset(&template)); + } else { + self.grid.clear_viewport(&template); + } + }, ansi::ClearMode::Saved => self.grid.clear_history(), } }