From 2ed84e385cff2732a75282f3607e5fd297ac5dff Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Sun, 17 Dec 2023 21:46:20 +0000 Subject: [PATCH 01/29] refactor: Simplify transfer_rows_from_viewport_to_lines_above next_lines is always consolidated to a single Row, which immediately gets removed - we can remove some dead code as a result --- zellij-server/src/panes/grid.rs | 37 +++++++++++++-------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index b7ed874f70..17956a83ab 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -166,37 +166,28 @@ fn transfer_rows_from_viewport_to_lines_above( count: usize, max_viewport_width: usize, ) -> isize { - let mut next_lines: Vec = vec![]; let mut transferred_rows_count: isize = 0; for _ in 0..count { - if next_lines.is_empty() { - if !viewport.is_empty() { - let next_line = viewport.remove(0); - transferred_rows_count += - calculate_row_display_height(next_line.width(), max_viewport_width) as isize; - if !next_line.is_canonical { - let mut bottom_canonical_row_and_wraps_in_dst = - get_lines_above_bottom_canonical_row_and_wraps(lines_above); - next_lines.append(&mut bottom_canonical_row_and_wraps_in_dst); - } - next_lines.push(next_line); - next_lines = vec![Row::from_rows(next_lines)]; - } else { - break; // no more rows - } + let mut next_lines: Vec = vec![]; + if !viewport.is_empty() { + let next_line = viewport.remove(0); + transferred_rows_count += + calculate_row_display_height(next_line.width(), max_viewport_width) as isize; + if !next_line.is_canonical { + let mut bottom_canonical_row_and_wraps_in_dst = + get_lines_above_bottom_canonical_row_and_wraps(lines_above); + next_lines.append(&mut bottom_canonical_row_and_wraps_in_dst); + } + next_lines.push(next_line); + } else { + break; // no more rows } - let dropped_line_width = bounded_push(lines_above, sixel_grid, next_lines.remove(0)); + let dropped_line_width = bounded_push(lines_above, sixel_grid, Row::from_rows(next_lines)); if let Some(width) = dropped_line_width { transferred_rows_count -= calculate_row_display_height(width, max_viewport_width) as isize; } } - if !next_lines.is_empty() { - let excess_rows = Row::from_rows(next_lines).split_to_rows_of_length(max_viewport_width); - for row in excess_rows { - viewport.insert(0, row); - } - } transferred_rows_count } From c1d8b5d89595fe68ebfe51361262694d3791884d Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Sun, 17 Dec 2023 21:57:57 +0000 Subject: [PATCH 02/29] perf: Batch remove rows from the viewport for performance Given a 1MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~9s to ~3s --- zellij-server/src/panes/grid.rs | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 17956a83ab..941b5ebe6f 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -167,21 +167,17 @@ fn transfer_rows_from_viewport_to_lines_above( max_viewport_width: usize, ) -> isize { let mut transferred_rows_count: isize = 0; - for _ in 0..count { + let drained_lines = std::cmp::min(count, viewport.len()); + for next_line in viewport.drain(..drained_lines) { let mut next_lines: Vec = vec![]; - if !viewport.is_empty() { - let next_line = viewport.remove(0); - transferred_rows_count += - calculate_row_display_height(next_line.width(), max_viewport_width) as isize; - if !next_line.is_canonical { - let mut bottom_canonical_row_and_wraps_in_dst = - get_lines_above_bottom_canonical_row_and_wraps(lines_above); - next_lines.append(&mut bottom_canonical_row_and_wraps_in_dst); - } - next_lines.push(next_line); - } else { - break; // no more rows - } + transferred_rows_count += + calculate_row_display_height(next_line.width(), max_viewport_width) as isize; + if !next_line.is_canonical { + let mut bottom_canonical_row_and_wraps_in_dst = + get_lines_above_bottom_canonical_row_and_wraps(lines_above); + next_lines.append(&mut bottom_canonical_row_and_wraps_in_dst); + } + next_lines.push(next_line); let dropped_line_width = bounded_push(lines_above, sixel_grid, Row::from_rows(next_lines)); if let Some(width) = dropped_line_width { transferred_rows_count -= From 78fafb12ffc795468c554a86a802125ca67287fc Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Mon, 18 Dec 2023 00:42:12 +0000 Subject: [PATCH 03/29] perf: Optimize Row::drain_until by splitting chars in one step Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~23s to ~20s --- zellij-server/src/panes/grid.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 941b5ebe6f..4b950f9288 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -3366,19 +3366,20 @@ impl Row { self.width = None; } pub fn drain_until(&mut self, x: usize) -> VecDeque { - let mut drained_part: VecDeque = VecDeque::new(); let mut drained_part_len = 0; - while let Some(next_character) = self.columns.remove(0) { + let mut split_pos = 0; + for next_character in self.columns.iter() { // drained_part_len == 0 here is so that if the grid is resized // to a size of 1, we won't drop wide characters if drained_part_len + next_character.width <= x || drained_part_len == 0 { - drained_part.push_back(next_character); drained_part_len += next_character.width; + split_pos += 1 } else { - self.columns.push_front(next_character); // put it back break; } } + // Can't use split_off because it doesn't reduce capacity, causing OOM with long lines + let drained_part = self.columns.drain(..split_pos).collect(); self.width = None; drained_part } From cca9656df14ae40d41c76011a13fd53c598ec063 Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Wed, 20 Dec 2023 11:00:52 +0000 Subject: [PATCH 04/29] refactor: Simplify `if let` into a `.map` --- zellij-server/src/panes/grid.rs | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 4b950f9288..f8d9e1def1 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -850,25 +850,17 @@ impl Grid { self.viewport = new_viewport_rows; let mut new_cursor_y = self.canonical_line_y_coordinates(cursor_canonical_line_index); - let mut saved_cursor_y_coordinates = - if let Some(saved_cursor) = self.saved_cursor_position.as_ref() { - Some(self.canonical_line_y_coordinates(saved_cursor.y)) - } else { - None - }; + let mut saved_cursor_y_coordinates = self.saved_cursor_position.as_ref() + .map(|saved_cursor| self.canonical_line_y_coordinates(saved_cursor.y)); let new_cursor_x = (cursor_index_in_canonical_line / new_columns) + (cursor_index_in_canonical_line % new_columns); - let saved_cursor_x_coordinates = if let Some(saved_cursor_index_in_canonical_line) = - saved_cursor_index_in_canonical_line.as_ref() - { - Some( - (*saved_cursor_index_in_canonical_line / new_columns) - + (*saved_cursor_index_in_canonical_line % new_columns), - ) - } else { - None - }; + let saved_cursor_x_coordinates = saved_cursor_index_in_canonical_line.as_ref() + .map(|saved_cursor_index_in_canonical_line| + (*saved_cursor_index_in_canonical_line / new_columns) + + (*saved_cursor_index_in_canonical_line % new_columns) + ); + let current_viewport_row_count = self.viewport.len(); match current_viewport_row_count.cmp(&self.height) { Ordering::Less => { From 8ce6ce10b9cb282efbe9ffcbbb1643064b31f684 Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Sun, 24 Dec 2023 20:46:09 +0000 Subject: [PATCH 05/29] refactor: There are only new saved coordinates when there were old ones --- zellij-server/src/panes/grid.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index f8d9e1def1..c199169720 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -911,10 +911,9 @@ impl Grid { saved_cursor_position.x = saved_cursor_x_coordinates; saved_cursor_position.y = saved_cursor_y_coordinates; }, - _ => { - saved_cursor_position.x = new_cursor_x; - saved_cursor_position.y = new_cursor_y; - }, + _ => unreachable!("saved cursor {:?} {:?}", + saved_cursor_x_coordinates, + saved_cursor_y_coordinates), } }; } From 110bf46c2acd20c668fa3589da26b33cc05b365c Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Sun, 24 Dec 2023 20:50:32 +0000 Subject: [PATCH 06/29] refactor: Unify viewport transfer: use common variable names --- zellij-server/src/panes/grid.rs | 42 ++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index c199169720..8070bba225 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -918,6 +918,14 @@ impl Grid { }; } if new_rows != self.height { + let mut new_cursor_y = self.cursor.y; + let mut saved_cursor_y_coordinates = + self.saved_cursor_position.as_ref().map(|saved_cursor| saved_cursor.y); + + let new_cursor_x = self.cursor.x; + let mut saved_cursor_x_coordinates = + self.saved_cursor_position.as_ref().map(|saved_cursor| saved_cursor.x); + let current_viewport_row_count = self.viewport.len(); match current_viewport_row_count.cmp(&new_rows) { Ordering::Less => { @@ -930,23 +938,22 @@ impl Grid { new_columns, ); let rows_pulled = self.viewport.len() - current_viewport_row_count; - self.cursor.y += rows_pulled; - if let Some(saved_cursor_position) = self.saved_cursor_position.as_mut() { - saved_cursor_position.y += rows_pulled + new_cursor_y += rows_pulled; + if let Some(saved_cursor_y_coordinates) = saved_cursor_y_coordinates.as_mut() { + *saved_cursor_y_coordinates += rows_pulled; }; }, Ordering::Greater => { let row_count_to_transfer = current_viewport_row_count - new_rows; - if row_count_to_transfer > self.cursor.y { - self.cursor.y = 0; - if let Some(saved_cursor_position) = self.saved_cursor_position.as_mut() { - saved_cursor_position.y = 0 + if row_count_to_transfer > new_cursor_y { + new_cursor_y = 0; + if let Some(saved_cursor_y_coordinates) = saved_cursor_y_coordinates.as_mut() { + *saved_cursor_y_coordinates = 0 }; } else { - self.cursor.y -= row_count_to_transfer; - if let Some(saved_cursor_position) = self.saved_cursor_position.as_mut() { - saved_cursor_position.y = saved_cursor_position - .y + new_cursor_y -= row_count_to_transfer; + if let Some(saved_cursor_y_coordinates) = saved_cursor_y_coordinates.as_mut() { + *saved_cursor_y_coordinates = saved_cursor_y_coordinates .saturating_sub(row_count_to_transfer); }; } @@ -960,6 +967,19 @@ impl Grid { }, Ordering::Equal => {}, } + self.cursor.y = new_cursor_y; + self.cursor.x = new_cursor_x; + if let Some(saved_cursor_position) = self.saved_cursor_position.as_mut() { + match (saved_cursor_x_coordinates, saved_cursor_y_coordinates) { + (Some(saved_cursor_x_coordinates), Some(saved_cursor_y_coordinates)) => { + saved_cursor_position.x = saved_cursor_x_coordinates; + saved_cursor_position.y = saved_cursor_y_coordinates; + }, + _ => unreachable!("saved cursor {:?} {:?}", + saved_cursor_x_coordinates, + saved_cursor_y_coordinates), + } + }; } self.height = new_rows; self.width = new_columns; From 75143e6531b6ca2352f80667260a10d62feb5dc6 Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Mon, 25 Dec 2023 15:23:58 +0000 Subject: [PATCH 07/29] fix: Use same saved cursor logic in height resize as width See #2182 for original introduction that only added it in one branch, this fixes an issue where the saved cursor was incorrectly reset when the real cursor was --- zellij-server/src/panes/grid.rs | 16 +++---- zellij-server/src/panes/unit/grid_tests.rs | 42 +++++++++++++++++++ ...rid_tests__saved_cursor_across_resize.snap | 9 ++++ 3 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize.snap diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 8070bba225..82d08231a3 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -923,7 +923,7 @@ impl Grid { self.saved_cursor_position.as_ref().map(|saved_cursor| saved_cursor.y); let new_cursor_x = self.cursor.x; - let mut saved_cursor_x_coordinates = + let saved_cursor_x_coordinates = self.saved_cursor_position.as_ref().map(|saved_cursor| saved_cursor.x); let current_viewport_row_count = self.viewport.len(); @@ -947,15 +947,15 @@ impl Grid { let row_count_to_transfer = current_viewport_row_count - new_rows; if row_count_to_transfer > new_cursor_y { new_cursor_y = 0; - if let Some(saved_cursor_y_coordinates) = saved_cursor_y_coordinates.as_mut() { - *saved_cursor_y_coordinates = 0 - }; } else { new_cursor_y -= row_count_to_transfer; - if let Some(saved_cursor_y_coordinates) = saved_cursor_y_coordinates.as_mut() { - *saved_cursor_y_coordinates = saved_cursor_y_coordinates - .saturating_sub(row_count_to_transfer); - }; + } + if let Some(saved_cursor_y_coordinates) = saved_cursor_y_coordinates.as_mut() { + if row_count_to_transfer > *saved_cursor_y_coordinates { + *saved_cursor_y_coordinates = 0; + } else { + *saved_cursor_y_coordinates -= row_count_to_transfer; + } } transfer_rows_from_viewport_to_lines_above( &mut self.viewport, diff --git a/zellij-server/src/panes/unit/grid_tests.rs b/zellij-server/src/panes/unit/grid_tests.rs index 781904c365..01d6cc9402 100644 --- a/zellij-server/src/panes/unit/grid_tests.rs +++ b/zellij-server/src/panes/unit/grid_tests.rs @@ -2400,6 +2400,48 @@ pub fn scroll_up_increase_width_and_scroll_down() { assert_snapshot!(format!("{:?}", grid)); } +#[test] +fn saved_cursor_across_resize() { + let mut vte_parser = vte::Parser::new(); + let sixel_image_store = Rc::new(RefCell::new(SixelImageStore::default())); + let terminal_emulator_color_codes = Rc::new(RefCell::new(HashMap::new())); + let debug = false; + let arrow_fonts = true; + let styled_underlines = true; + let mut grid = Grid::new( + 4, + 20, + Rc::new(RefCell::new(Palette::default())), + terminal_emulator_color_codes, + Rc::new(RefCell::new(LinkHandler::new())), + Rc::new(RefCell::new(None)), + sixel_image_store, + Style::default(), + debug, + arrow_fonts, + styled_underlines, + ); + let mut parse = |s, grid: &mut Grid| + for b in Vec::from(s) { vte_parser.advance(&mut *grid, b) }; + let content = "\n +\rLine 1 >fill to 20_< +\rLine 2 >fill to 20_< +\rLine 3 >fill to 20_< +\rL\u{1b}[sine 4 >fill to 20_<"; + parse(content, &mut grid); + // Move real cursor position up three lines + let content = "\u{1b}[3A"; + parse(content, &mut grid); + // Truncate top of terminal, resetting cursor (but not saved cursor) + grid.change_size(3, 20); + // Wrap, resetting cursor again (but not saved cursor) + grid.change_size(3, 10); + // Restore saved cursor position and write ZZZ + let content = "\u{1b}[uZZZ"; + parse(content, &mut grid); + assert_snapshot!(format!("{:?}", grid)); +} + #[test] pub fn move_cursor_below_scroll_region() { let mut vte_parser = vte::Parser::new(); diff --git a/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize.snap b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize.snap new file mode 100644 index 0000000000..dcb20d3880 --- /dev/null +++ b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize.snap @@ -0,0 +1,9 @@ +--- +source: zellij-server/src/panes/./unit/grid_tests.rs +assertion_line: 2443 +expression: "format!(\"{:?}\", grid)" +--- +00 (W): ll to 20_< +01 (C): LZZZ 4 >fi +02 (W): ll to 20_< + From 37d0dab0b0778999dc7557fa7843a6f429e4bf65 Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Mon, 25 Dec 2023 18:17:59 +0000 Subject: [PATCH 08/29] fix: Correct saved+real cursor calculations when reflowing long lines --- zellij-server/src/panes/grid.rs | 14 ++++---- zellij-server/src/panes/unit/grid_tests.rs | 35 ++++++++++++++++++- ...__saved_cursor_across_resize_longline.snap | 10 ++++++ 3 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_longline.snap diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 82d08231a3..30896b7847 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -849,16 +849,18 @@ impl Grid { self.viewport = new_viewport_rows; - let mut new_cursor_y = self.canonical_line_y_coordinates(cursor_canonical_line_index); + let mut new_cursor_y = self.canonical_line_y_coordinates(cursor_canonical_line_index) + + (cursor_index_in_canonical_line / new_columns); let mut saved_cursor_y_coordinates = self.saved_cursor_position.as_ref() - .map(|saved_cursor| self.canonical_line_y_coordinates(saved_cursor.y)); + .map(|saved_cursor| + self.canonical_line_y_coordinates(saved_cursor.y) + + saved_cursor_index_in_canonical_line.as_ref().unwrap() / new_columns + ); - let new_cursor_x = (cursor_index_in_canonical_line / new_columns) - + (cursor_index_in_canonical_line % new_columns); + let new_cursor_x = cursor_index_in_canonical_line % new_columns; let saved_cursor_x_coordinates = saved_cursor_index_in_canonical_line.as_ref() .map(|saved_cursor_index_in_canonical_line| - (*saved_cursor_index_in_canonical_line / new_columns) - + (*saved_cursor_index_in_canonical_line % new_columns) + *saved_cursor_index_in_canonical_line % new_columns ); let current_viewport_row_count = self.viewport.len(); diff --git a/zellij-server/src/panes/unit/grid_tests.rs b/zellij-server/src/panes/unit/grid_tests.rs index 01d6cc9402..a17ac2bc1a 100644 --- a/zellij-server/src/panes/unit/grid_tests.rs +++ b/zellij-server/src/panes/unit/grid_tests.rs @@ -2423,7 +2423,7 @@ fn saved_cursor_across_resize() { ); let mut parse = |s, grid: &mut Grid| for b in Vec::from(s) { vte_parser.advance(&mut *grid, b) }; - let content = "\n + let content = " \rLine 1 >fill to 20_< \rLine 2 >fill to 20_< \rLine 3 >fill to 20_< @@ -2442,6 +2442,39 @@ fn saved_cursor_across_resize() { assert_snapshot!(format!("{:?}", grid)); } +#[test] +fn saved_cursor_across_resize_longline() { + let mut vte_parser = vte::Parser::new(); + let sixel_image_store = Rc::new(RefCell::new(SixelImageStore::default())); + let terminal_emulator_color_codes = Rc::new(RefCell::new(HashMap::new())); + let debug = false; + let arrow_fonts = true; + let styled_underlines = true; + let mut grid = Grid::new( + 4, + 20, + Rc::new(RefCell::new(Palette::default())), + terminal_emulator_color_codes, + Rc::new(RefCell::new(LinkHandler::new())), + Rc::new(RefCell::new(None)), + sixel_image_store, + Style::default(), + debug, + arrow_fonts, + styled_underlines, + ); + let mut parse = |s, grid: &mut Grid| + for b in Vec::from(s) { vte_parser.advance(&mut *grid, b) }; + let content = " +\rLine 1 >fill \u{1b}[sto 20_<"; + parse(content, &mut grid); + grid.change_size(4, 10); + // Write 'YY' at the end, restore to the saved cursor and overwrite 'to' with 'ZZ' + let content = "YY\u{1b}[uZZ\u{1b}[s"; + parse(content, &mut grid); + assert_snapshot!(format!("{:?}", grid)); +} + #[test] pub fn move_cursor_below_scroll_region() { let mut vte_parser = vte::Parser::new(); diff --git a/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_longline.snap b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_longline.snap new file mode 100644 index 0000000000..a183095da0 --- /dev/null +++ b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_longline.snap @@ -0,0 +1,10 @@ +--- +source: zellij-server/src/panes/./unit/grid_tests.rs +assertion_line: 2475 +expression: "format!(\"{:?}\", grid)" +--- +00 (C): +01 (C): Line 1 >fi +02 (W): ll ZZ 20_< +03 (C): YY + From 918b64296e03f56d558b88f86a1206ceeb339458 Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Mon, 25 Dec 2023 20:14:30 +0000 Subject: [PATCH 09/29] fix: Don't create canonical lines if cursor ends on EOL after resize Previously if a 20 character line were split into two 10 character lines, the cursor would be placed on the line after the two lines. New characters would then be treated as a new canonical line. This commit fixes this by biasing cursors to the end of the previous line. --- zellij-server/src/panes/grid.rs | 22 +++++++++++++++---- zellij-server/src/panes/unit/grid_tests.rs | 3 ++- ...__saved_cursor_across_resize_longline.snap | 2 +- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 30896b7847..2e9b9db677 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -857,11 +857,25 @@ impl Grid { + saved_cursor_index_in_canonical_line.as_ref().unwrap() / new_columns ); - let new_cursor_x = cursor_index_in_canonical_line % new_columns; + // A cursor at EOL has two equivalent positions - end of this line or beginning of + // next. If not already at the beginning of line, bias to EOL so add character logic + // doesn't create spurious canonical lines + let mut new_cursor_x = cursor_index_in_canonical_line % new_columns; + if self.cursor.x != 0 && new_cursor_x == 0 { + new_cursor_y -= 1; + new_cursor_x = new_columns + } let saved_cursor_x_coordinates = saved_cursor_index_in_canonical_line.as_ref() - .map(|saved_cursor_index_in_canonical_line| - *saved_cursor_index_in_canonical_line % new_columns - ); + .map(|saved_cursor_index_in_canonical_line| { + let x = self.saved_cursor_position.as_ref().unwrap().x; + let mut new_x = *saved_cursor_index_in_canonical_line % new_columns; + let new_y = saved_cursor_y_coordinates.as_mut().unwrap(); + if x != 0 && new_x == 0 { + *new_y -= 1; + new_x = new_columns + } + new_x + }); let current_viewport_row_count = self.viewport.len(); match current_viewport_row_count.cmp(&self.height) { diff --git a/zellij-server/src/panes/unit/grid_tests.rs b/zellij-server/src/panes/unit/grid_tests.rs index a17ac2bc1a..f940ddeaa9 100644 --- a/zellij-server/src/panes/unit/grid_tests.rs +++ b/zellij-server/src/panes/unit/grid_tests.rs @@ -2468,9 +2468,10 @@ fn saved_cursor_across_resize_longline() { let content = " \rLine 1 >fill \u{1b}[sto 20_<"; parse(content, &mut grid); + // Wrap each line halfway grid.change_size(4, 10); // Write 'YY' at the end, restore to the saved cursor and overwrite 'to' with 'ZZ' - let content = "YY\u{1b}[uZZ\u{1b}[s"; + let content = "YY\u{1b}[uZZ"; parse(content, &mut grid); assert_snapshot!(format!("{:?}", grid)); } diff --git a/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_longline.snap b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_longline.snap index a183095da0..1aedfaa9e7 100644 --- a/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_longline.snap +++ b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_longline.snap @@ -6,5 +6,5 @@ expression: "format!(\"{:?}\", grid)" 00 (C): 01 (C): Line 1 >fi 02 (W): ll ZZ 20_< -03 (C): YY +03 (W): YY From e2c9d6fdf7408865c5e2c78a68430aafb6d28e9c Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Mon, 25 Dec 2023 22:24:09 +0000 Subject: [PATCH 10/29] fix: for cursor index calculation in lines that are already wrapped --- zellij-server/src/panes/grid.rs | 9 ++--- zellij-server/src/panes/unit/grid_tests.rs | 40 ++++++++++++++++++- ...ts__saved_cursor_across_resize_rewrap.snap | 10 +++++ 3 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_rewrap.snap diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 2e9b9db677..13dd0affd7 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -623,8 +623,8 @@ impl Grid { cursor_canonical_line_index = i; } if i == self.cursor.y { - let line_wrap_position_in_line = self.cursor.y - cursor_canonical_line_index; - cursor_index_in_canonical_line = line_wrap_position_in_line + self.cursor.x; + let line_wraps = self.cursor.y - cursor_canonical_line_index; + cursor_index_in_canonical_line = (line_wraps * self.width) + self.cursor.x; break; } } @@ -639,10 +639,9 @@ impl Grid { cursor_canonical_line_index = i; } if i == saved_cursor_position.y { - let line_wrap_position_in_line = - saved_cursor_position.y - cursor_canonical_line_index; + let line_wraps = saved_cursor_position.y - cursor_canonical_line_index; cursor_index_in_canonical_line = - line_wrap_position_in_line + saved_cursor_position.x; + (line_wraps * self.width) + saved_cursor_position.x; break; } } diff --git a/zellij-server/src/panes/unit/grid_tests.rs b/zellij-server/src/panes/unit/grid_tests.rs index f940ddeaa9..7888e79be5 100644 --- a/zellij-server/src/panes/unit/grid_tests.rs +++ b/zellij-server/src/panes/unit/grid_tests.rs @@ -2468,14 +2468,50 @@ fn saved_cursor_across_resize_longline() { let content = " \rLine 1 >fill \u{1b}[sto 20_<"; parse(content, &mut grid); - // Wrap each line halfway + // Wrap each line precisely halfway grid.change_size(4, 10); - // Write 'YY' at the end, restore to the saved cursor and overwrite 'to' with 'ZZ' + // Write 'YY' at the end (ends up on a new wrapped line), restore to the saved cursor + // and overwrite 'to' with 'ZZ' let content = "YY\u{1b}[uZZ"; parse(content, &mut grid); assert_snapshot!(format!("{:?}", grid)); } +#[test] +fn saved_cursor_across_resize_rewrap() { + let mut vte_parser = vte::Parser::new(); + let sixel_image_store = Rc::new(RefCell::new(SixelImageStore::default())); + let terminal_emulator_color_codes = Rc::new(RefCell::new(HashMap::new())); + let debug = false; + let arrow_fonts = true; + let styled_underlines = true; + let mut grid = Grid::new( + 4, + 4*8, + Rc::new(RefCell::new(Palette::default())), + terminal_emulator_color_codes, + Rc::new(RefCell::new(LinkHandler::new())), + Rc::new(RefCell::new(None)), + sixel_image_store, + Style::default(), + debug, + arrow_fonts, + styled_underlines, + ); + let mut parse = |s, grid: &mut Grid| + for b in Vec::from(s) { vte_parser.advance(&mut *grid, b) }; + let content = " +\r12345678123456781234567\u{1b}[s812345678"; // 4*8 chars + parse(content, &mut grid); + // Wrap each line precisely halfway, then rewrap to halve them again + grid.change_size(4, 16); + grid.change_size(4, 8); + // Write 'Z' at the end of line 3 + let content = "\u{1b}[uZ"; + parse(content, &mut grid); + assert_snapshot!(format!("{:?}", grid)); +} + #[test] pub fn move_cursor_below_scroll_region() { let mut vte_parser = vte::Parser::new(); diff --git a/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_rewrap.snap b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_rewrap.snap new file mode 100644 index 0000000000..f250f8658f --- /dev/null +++ b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_rewrap.snap @@ -0,0 +1,10 @@ +--- +source: zellij-server/src/panes/./unit/grid_tests.rs +assertion_line: 2512 +expression: "format!(\"{:?}\", grid)" +--- +00 (C): 12345678 +01 (W): 12345678 +02 (W): 1234567Z +03 (W): 12345678 + From fba2574e8c2019061f3e736aac69eef518cc3e68 Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Tue, 26 Dec 2023 15:34:32 +0000 Subject: [PATCH 11/29] chore: test for real/saved cursor position being handled separately --- zellij-server/src/tab/unit/tab_integration_tests.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/zellij-server/src/tab/unit/tab_integration_tests.rs b/zellij-server/src/tab/unit/tab_integration_tests.rs index f1820b725c..1ee1a72701 100644 --- a/zellij-server/src/tab/unit/tab_integration_tests.rs +++ b/zellij-server/src/tab/unit/tab_integration_tests.rs @@ -2039,10 +2039,20 @@ fn save_cursor_position_across_resizes() { 1, Vec::from("\n\n\rI am some text\n\rI am another line of text\n\rLet's save the cursor position here \u{1b}[sI should be ovewritten".as_bytes()), ).unwrap(); - tab.resize_whole_tab(Size { cols: 100, rows: 3 }).unwrap(); + + // We check cursor and saved cursor are handled separately by: + // 1. moving real cursor up two lines + tab.handle_pty_bytes(1, Vec::from("\u{1b}[2A".as_bytes())); + // 2. resizing so real cursor gets lost above the viewport, which resets it to row 0 + // The saved cursor ends up on row 1, allowing detection if it (incorrectly) gets reset too + tab.resize_whole_tab(Size { cols: 35, rows: 4 }).unwrap(); + + // Now overwrite tab.handle_pty_bytes(1, Vec::from("\u{1b}[uthis overwrote me!".as_bytes())) .unwrap(); + tab.resize_whole_tab(Size { cols: 100, rows: 3 }).unwrap(); + tab.render(&mut output).unwrap(); let snapshot = take_snapshot( output.serialize().unwrap().get(&client_id).unwrap(), From 528ccbfe7b425972368e89759f2b6715036aa109 Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Wed, 27 Dec 2023 13:28:27 +0000 Subject: [PATCH 12/29] chore: Apply cargo format --- zellij-server/src/panes/grid.rs | 39 +++++++++++++--------- zellij-server/src/panes/unit/grid_tests.rs | 23 +++++++++---- 2 files changed, 39 insertions(+), 23 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 13dd0affd7..a600db59e9 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -850,11 +850,11 @@ impl Grid { let mut new_cursor_y = self.canonical_line_y_coordinates(cursor_canonical_line_index) + (cursor_index_in_canonical_line / new_columns); - let mut saved_cursor_y_coordinates = self.saved_cursor_position.as_ref() - .map(|saved_cursor| + let mut saved_cursor_y_coordinates = + self.saved_cursor_position.as_ref().map(|saved_cursor| { self.canonical_line_y_coordinates(saved_cursor.y) + saved_cursor_index_in_canonical_line.as_ref().unwrap() / new_columns - ); + }); // A cursor at EOL has two equivalent positions - end of this line or beginning of // next. If not already at the beginning of line, bias to EOL so add character logic @@ -864,8 +864,8 @@ impl Grid { new_cursor_y -= 1; new_cursor_x = new_columns } - let saved_cursor_x_coordinates = saved_cursor_index_in_canonical_line.as_ref() - .map(|saved_cursor_index_in_canonical_line| { + let saved_cursor_x_coordinates = saved_cursor_index_in_canonical_line.as_ref().map( + |saved_cursor_index_in_canonical_line| { let x = self.saved_cursor_position.as_ref().unwrap().x; let mut new_x = *saved_cursor_index_in_canonical_line % new_columns; let new_y = saved_cursor_y_coordinates.as_mut().unwrap(); @@ -874,7 +874,8 @@ impl Grid { new_x = new_columns } new_x - }); + }, + ); let current_viewport_row_count = self.viewport.len(); match current_viewport_row_count.cmp(&self.height) { @@ -926,20 +927,25 @@ impl Grid { saved_cursor_position.x = saved_cursor_x_coordinates; saved_cursor_position.y = saved_cursor_y_coordinates; }, - _ => unreachable!("saved cursor {:?} {:?}", - saved_cursor_x_coordinates, - saved_cursor_y_coordinates), + _ => unreachable!( + "saved cursor {:?} {:?}", + saved_cursor_x_coordinates, saved_cursor_y_coordinates + ), } }; } if new_rows != self.height { let mut new_cursor_y = self.cursor.y; - let mut saved_cursor_y_coordinates = - self.saved_cursor_position.as_ref().map(|saved_cursor| saved_cursor.y); + let mut saved_cursor_y_coordinates = self + .saved_cursor_position + .as_ref() + .map(|saved_cursor| saved_cursor.y); let new_cursor_x = self.cursor.x; - let saved_cursor_x_coordinates = - self.saved_cursor_position.as_ref().map(|saved_cursor| saved_cursor.x); + let saved_cursor_x_coordinates = self + .saved_cursor_position + .as_ref() + .map(|saved_cursor| saved_cursor.x); let current_viewport_row_count = self.viewport.len(); match current_viewport_row_count.cmp(&new_rows) { @@ -990,9 +996,10 @@ impl Grid { saved_cursor_position.x = saved_cursor_x_coordinates; saved_cursor_position.y = saved_cursor_y_coordinates; }, - _ => unreachable!("saved cursor {:?} {:?}", - saved_cursor_x_coordinates, - saved_cursor_y_coordinates), + _ => unreachable!( + "saved cursor {:?} {:?}", + saved_cursor_x_coordinates, saved_cursor_y_coordinates + ), } }; } diff --git a/zellij-server/src/panes/unit/grid_tests.rs b/zellij-server/src/panes/unit/grid_tests.rs index 7888e79be5..2459796b80 100644 --- a/zellij-server/src/panes/unit/grid_tests.rs +++ b/zellij-server/src/panes/unit/grid_tests.rs @@ -2421,8 +2421,11 @@ fn saved_cursor_across_resize() { arrow_fonts, styled_underlines, ); - let mut parse = |s, grid: &mut Grid| - for b in Vec::from(s) { vte_parser.advance(&mut *grid, b) }; + let mut parse = |s, grid: &mut Grid| { + for b in Vec::from(s) { + vte_parser.advance(&mut *grid, b) + } + }; let content = " \rLine 1 >fill to 20_< \rLine 2 >fill to 20_< @@ -2463,8 +2466,11 @@ fn saved_cursor_across_resize_longline() { arrow_fonts, styled_underlines, ); - let mut parse = |s, grid: &mut Grid| - for b in Vec::from(s) { vte_parser.advance(&mut *grid, b) }; + let mut parse = |s, grid: &mut Grid| { + for b in Vec::from(s) { + vte_parser.advance(&mut *grid, b) + } + }; let content = " \rLine 1 >fill \u{1b}[sto 20_<"; parse(content, &mut grid); @@ -2487,7 +2493,7 @@ fn saved_cursor_across_resize_rewrap() { let styled_underlines = true; let mut grid = Grid::new( 4, - 4*8, + 4 * 8, Rc::new(RefCell::new(Palette::default())), terminal_emulator_color_codes, Rc::new(RefCell::new(LinkHandler::new())), @@ -2498,8 +2504,11 @@ fn saved_cursor_across_resize_rewrap() { arrow_fonts, styled_underlines, ); - let mut parse = |s, grid: &mut Grid| - for b in Vec::from(s) { vte_parser.advance(&mut *grid, b) }; + let mut parse = |s, grid: &mut Grid| { + for b in Vec::from(s) { + vte_parser.advance(&mut *grid, b) + } + }; let content = " \r12345678123456781234567\u{1b}[s812345678"; // 4*8 chars parse(content, &mut grid); From c80072de8ede215beed7ea645aa594a7bcc6dbec Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Thu, 28 Dec 2023 21:05:18 +0000 Subject: [PATCH 13/29] refactor: Unify viewport transfer: transfer + cursor update together --- zellij-server/src/panes/grid.rs | 93 +++++++++++---------------------- 1 file changed, 30 insertions(+), 63 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index a600db59e9..d534e5d3e3 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -769,7 +769,7 @@ impl Grid { } self.selection.reset(); self.sixel_grid.character_cell_size_possibly_changed(); - if new_columns != self.width { + let cursors = if new_columns != self.width { self.horizontal_tabstops = create_horizontal_tabstops(new_columns); let mut cursor_canonical_line_index = self.cursor_canonical_line_index(); let cursor_index_in_canonical_line = self.cursor_index_in_canonical_line(); @@ -877,76 +877,42 @@ impl Grid { }, ); - let current_viewport_row_count = self.viewport.len(); - match current_viewport_row_count.cmp(&self.height) { - Ordering::Less => { - let row_count_to_transfer = self.height - current_viewport_row_count; - - transfer_rows_from_lines_above_to_viewport( - &mut self.lines_above, - &mut self.viewport, - &mut self.sixel_grid, - row_count_to_transfer, - new_columns, - ); - let rows_pulled = self.viewport.len() - current_viewport_row_count; - new_cursor_y += rows_pulled; - if let Some(saved_cursor_y_coordinates) = saved_cursor_y_coordinates.as_mut() { - *saved_cursor_y_coordinates += rows_pulled; - } - }, - Ordering::Greater => { - let row_count_to_transfer = current_viewport_row_count - self.height; - if row_count_to_transfer > new_cursor_y { - new_cursor_y = 0; - } else { - new_cursor_y -= row_count_to_transfer; - } - if let Some(saved_cursor_y_coordinates) = saved_cursor_y_coordinates.as_mut() { - if row_count_to_transfer > *saved_cursor_y_coordinates { - *saved_cursor_y_coordinates = 0; - } else { - *saved_cursor_y_coordinates -= row_count_to_transfer; - } - } - transfer_rows_from_viewport_to_lines_above( - &mut self.viewport, - &mut self.lines_above, - &mut self.sixel_grid, - row_count_to_transfer, - new_columns, - ); - }, - Ordering::Equal => {}, - } - self.cursor.y = new_cursor_y; - self.cursor.x = new_cursor_x; - if let Some(saved_cursor_position) = self.saved_cursor_position.as_mut() { - match (saved_cursor_x_coordinates, saved_cursor_y_coordinates) { - (Some(saved_cursor_x_coordinates), Some(saved_cursor_y_coordinates)) => { - saved_cursor_position.x = saved_cursor_x_coordinates; - saved_cursor_position.y = saved_cursor_y_coordinates; - }, - _ => unreachable!( - "saved cursor {:?} {:?}", - saved_cursor_x_coordinates, saved_cursor_y_coordinates - ), - } - }; - } - if new_rows != self.height { - let mut new_cursor_y = self.cursor.y; - let mut saved_cursor_y_coordinates = self + Some(( + new_cursor_y, + saved_cursor_y_coordinates, + new_cursor_x, + saved_cursor_x_coordinates, + )) + } else if new_rows != self.height { + let saved_cursor_y_coordinates = self .saved_cursor_position .as_ref() .map(|saved_cursor| saved_cursor.y); - - let new_cursor_x = self.cursor.x; let saved_cursor_x_coordinates = self .saved_cursor_position .as_ref() .map(|saved_cursor| saved_cursor.x); + Some(( + self.cursor.y, + saved_cursor_y_coordinates, + self.cursor.x, + saved_cursor_x_coordinates, + )) + } else { + None + }; + + if let Some(cursors) = cursors { + // At this point the x coordinates have been calculated, the y coordinates + // will be updated within this block + let ( + mut new_cursor_y, + mut saved_cursor_y_coordinates, + new_cursor_x, + saved_cursor_x_coordinates + ) = cursors; + let current_viewport_row_count = self.viewport.len(); match current_viewport_row_count.cmp(&new_rows) { Ordering::Less => { @@ -1003,6 +969,7 @@ impl Grid { } }; } + self.height = new_rows; self.width = new_columns; if self.scroll_region.is_some() { From 3ef9638d33f3ad2a079cbec7f9ce4fe50e4911dd Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Fri, 29 Dec 2023 14:30:02 +0000 Subject: [PATCH 14/29] perf: Reduce size of TerminalCharacter from 72 to 60 bytes With a 10MB single line catted into a fresh terminal, VmRSS goes from 964092 kB to 874020 kB (as reported by /proc//status) before/after this patch Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~15s to ~12.5s --- zellij-server/src/output/mod.rs | 16 ++--- zellij-server/src/panes/grid.rs | 64 ++++++++----------- zellij-server/src/panes/terminal_character.rs | 30 ++++++++- zellij-server/src/ui/boundaries.rs | 7 +- zellij-server/src/ui/pane_boundaries_frame.rs | 14 ++-- 5 files changed, 69 insertions(+), 62 deletions(-) diff --git a/zellij-server/src/output/mod.rs b/zellij-server/src/output/mod.rs index 3f99b4013d..b835cc090c 100644 --- a/zellij-server/src/output/mod.rs +++ b/zellij-server/src/output/mod.rs @@ -110,7 +110,7 @@ fn serialize_chunks_with_newlines( &mut vte_output, ) .with_context(err_context)?; - chunk_width += t_character.width; + chunk_width += t_character.width(); vte_output.push(t_character.character); } character_styles.clear(); @@ -151,7 +151,7 @@ fn serialize_chunks( &mut vte_output, ) .with_context(err_context)?; - chunk_width += t_character.width; + chunk_width += t_character.width(); vte_output.push(t_character.character); } character_styles.clear(); @@ -215,7 +215,7 @@ fn adjust_middle_segment_for_wide_chars( let mut pad_left_end_by = 0; let mut pad_right_start_by = 0; for (absolute_index, t_character) in terminal_characters.iter().enumerate() { - current_x += t_character.width; + current_x += t_character.width(); if current_x >= middle_start && absolute_middle_start_index.is_none() { if current_x > middle_start { pad_left_end_by = current_x - middle_start; @@ -802,7 +802,7 @@ impl CharacterChunk { pub fn width(&self) -> usize { let mut width = 0; for t_character in &self.terminal_characters { - width += t_character.width + width += t_character.width() } width } @@ -814,14 +814,14 @@ impl CharacterChunk { break; } let next_character = self.terminal_characters.remove(0); // TODO: consider copying self.terminal_characters into a VecDeque to make this process faster? - if drained_part_len + next_character.width <= x { + if drained_part_len + next_character.width() <= x { drained_part.push_back(next_character); - drained_part_len += next_character.width; + drained_part_len += next_character.width(); } else { if drained_part_len == x { self.terminal_characters.insert(0, next_character); // put it back - } else if next_character.width > 1 { - for _ in 1..next_character.width { + } else if next_character.width() > 1 { + for _ in 1..next_character.width() { self.terminal_characters.insert(0, EMPTY_TERMINAL_CHARACTER); drained_part.push_back(EMPTY_TERMINAL_CHARACTER); } diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index d534e5d3e3..dcf6a715ed 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -3,7 +3,6 @@ use std::borrow::Cow; use std::cell::RefCell; use std::collections::HashMap; use std::rc::Rc; -use unicode_width::UnicodeWidthChar; use zellij_utils::data::Style; use zellij_utils::errors::prelude::*; use zellij_utils::regex::Regex; @@ -416,11 +415,8 @@ impl Debug for Grid { for y in image_top_edge..image_bottom_edge { let row = buffer.get_mut(y).unwrap(); for x in image_left_edge..image_right_edge { - let fake_sixel_terminal_character = TerminalCharacter { - character: sixel_indication_character(x), - width: 1, - styles: Default::default(), - }; + let fake_sixel_terminal_character = + TerminalCharacter::new_singlewidth(sixel_indication_character(x)); row.add_character_at(fake_sixel_terminal_character, x); } } @@ -1349,7 +1345,7 @@ impl Grid { } } pub fn add_character(&mut self, terminal_character: TerminalCharacter) { - let character_width = terminal_character.width; + let character_width = terminal_character.width(); // Drop zero-width Unicode/UTF-8 codepoints, like for example Variation Selectors. // This breaks unicode grapheme segmentation, and is the reason why some characters // aren't displayed correctly. Refer to this issue for more information: @@ -1664,7 +1660,7 @@ impl Grid { for _ in 0..count { let deleted_character = current_row.delete_and_return_character(self.cursor.x); let excess_width = deleted_character - .map(|terminal_character| terminal_character.width) + .map(|terminal_character| terminal_character.width()) .unwrap_or(0) .saturating_sub(1); for _ in 0..excess_width { @@ -1795,7 +1791,7 @@ impl Grid { line_selection.push(terminal_character.character); } - terminal_col += terminal_character.width; + terminal_col += terminal_character.width(); } if row.is_canonical { @@ -2148,13 +2144,7 @@ impl Perform for Grid { fn print(&mut self, c: char) { let c = self.cursor.charsets[self.active_charset].map(c); - // apparently, building TerminalCharacter like this without a "new" method - // is a little faster - let terminal_character = TerminalCharacter { - character: c, - width: c.width().unwrap_or(0), - styles: self.cursor.pending_styles, - }; + let terminal_character = TerminalCharacter::new_styled(c, self.cursor.pending_styles); self.set_preceding_character(terminal_character); self.add_character(terminal_character); } @@ -3166,7 +3156,7 @@ impl Row { } else { let mut width = 0; for terminal_character in &self.columns { - width += terminal_character.width; + width += terminal_character.width(); } self.width = Some(width); width @@ -3175,15 +3165,15 @@ impl Row { pub fn width(&self) -> usize { let mut width = 0; for terminal_character in &self.columns { - width += terminal_character.width; + width += terminal_character.width(); } width } pub fn excess_width(&self) -> usize { let mut acc = 0; for terminal_character in &self.columns { - if terminal_character.width > 1 { - acc += terminal_character.width - 1; + if terminal_character.width() > 1 { + acc += terminal_character.width() - 1; } } acc @@ -3191,8 +3181,8 @@ impl Row { pub fn excess_width_until(&self, x: usize) -> usize { let mut acc = 0; for terminal_character in self.columns.iter().take(x) { - if terminal_character.width > 1 { - acc += terminal_character.width - 1; + if terminal_character.width() > 1 { + acc += terminal_character.width() - 1; } } acc @@ -3204,7 +3194,7 @@ impl Row { if i == absolute_index { break; } - if terminal_character.width > 1 { + if terminal_character.width() > 1 { absolute_index = absolute_index.saturating_sub(1); } } @@ -3217,10 +3207,10 @@ impl Row { let mut absolute_index = x; let mut position_inside_character = 0; for (i, terminal_character) in self.columns.iter().enumerate() { - accumulated_width += terminal_character.width; + accumulated_width += terminal_character.width(); absolute_index = i; if accumulated_width > x { - let character_start_position = accumulated_width - terminal_character.width; + let character_start_position = accumulated_width - terminal_character.width(); position_inside_character = x - character_start_position; break; } @@ -3233,7 +3223,7 @@ impl Row { // adding the character at the end of the current line self.columns.push_back(terminal_character); // this is unwrapped because this always happens after self.width_cached() - *self.width.as_mut().unwrap() += terminal_character.width; + *self.width.as_mut().unwrap() += terminal_character.width(); }, Ordering::Less => { // adding the character after the end of the current line @@ -3249,17 +3239,17 @@ impl Row { // we replace the character at its position let (absolute_x_index, position_inside_character) = self.absolute_character_index_and_position_in_char(x); - let character_width = terminal_character.width; + let character_width = terminal_character.width(); let replaced_character = std::mem::replace(&mut self.columns[absolute_x_index], terminal_character); - match character_width.cmp(&replaced_character.width) { + match character_width.cmp(&replaced_character.width()) { Ordering::Greater => { // the replaced character is narrower than the current character // (eg. we added a wide emoji in place of an English character) // we remove the character after it to make room let position_to_remove = absolute_x_index + 1; if let Some(removed) = self.columns.remove(position_to_remove) { - if removed.width > 1 { + if removed.width() > 1 { // the character we removed is a wide character itself, so we add // padding self.columns @@ -3308,7 +3298,7 @@ impl Row { self.columns.push_back(terminal_character); // this is much more performant than remove/insert if let Some(character) = self.columns.swap_remove_back(absolute_x_index) { - let excess_width = character.width.saturating_sub(terminal_character.width); + let excess_width = character.width().saturating_sub(terminal_character.width()); for _ in 0..excess_width { self.columns .insert(absolute_x_index, EMPTY_TERMINAL_CHARACTER); @@ -3339,8 +3329,8 @@ impl Row { if index == position { break; } - if terminal_character.width > 1 { - position = position.saturating_sub(terminal_character.width.saturating_sub(1)); + if terminal_character.width() > 1 { + position = position.saturating_sub(terminal_character.width().saturating_sub(1)); } } position @@ -3371,8 +3361,8 @@ impl Row { for next_character in self.columns.iter() { // drained_part_len == 0 here is so that if the grid is resized // to a size of 1, we won't drop wide characters - if drained_part_len + next_character.width <= x || drained_part_len == 0 { - drained_part_len += next_character.width; + if drained_part_len + next_character.width() <= x || drained_part_len == 0 { + drained_part_len += next_character.width(); split_pos += 1 } else { break; @@ -3388,7 +3378,7 @@ impl Row { let width_of_current_character = self .columns .get(to_position_accounting_for_widechars) - .map(|character| character.width) + .map(|character| character.width()) .unwrap_or(1); let mut replace_with = VecDeque::from(vec![terminal_character; to + width_of_current_character]); @@ -3423,13 +3413,13 @@ impl Row { let mut current_part: VecDeque = VecDeque::new(); let mut current_part_len = 0; for character in self.columns.drain(..) { - if current_part_len + character.width > max_row_length { + if current_part_len + character.width() > max_row_length { parts.push(Row::from_columns(current_part)); current_part = VecDeque::new(); current_part_len = 0; } current_part.push_back(character); - current_part_len += character.width; + current_part_len += character.width(); } if !current_part.is_empty() { parts.push(Row::from_columns(current_part)) diff --git a/zellij-server/src/panes/terminal_character.rs b/zellij-server/src/panes/terminal_character.rs index 50f3ec8e8d..38e98ba370 100644 --- a/zellij-server/src/panes/terminal_character.rs +++ b/zellij-server/src/panes/terminal_character.rs @@ -840,17 +840,41 @@ impl Cursor { pub struct TerminalCharacter { pub character: char, pub styles: CharacterStyles, - pub width: usize, + width: u8, } impl TerminalCharacter { + #[inline] pub fn new(character: char) -> Self { + Self::new_styled(character, CharacterStyles::default()) + } + + #[inline] + pub fn new_styled(character: char, styles: CharacterStyles) -> Self { TerminalCharacter { character, - styles: CharacterStyles::default(), - width: character.width().unwrap_or(0), + styles, + width: character.width().unwrap_or(0) as u8, } } + + #[inline] + pub fn new_singlewidth(character: char) -> Self { + Self::new_singlewidth_styled(character, CharacterStyles::default()) + } + + #[inline] + pub fn new_singlewidth_styled(character: char, styles: CharacterStyles) -> Self { + TerminalCharacter { + character, + styles, + width: 1, + } + } + + pub fn width(&self) -> usize { + self.width as usize + } } impl ::std::fmt::Debug for TerminalCharacter { diff --git a/zellij-server/src/ui/boundaries.rs b/zellij-server/src/ui/boundaries.rs index 29b614c102..fb33e4fba3 100644 --- a/zellij-server/src/ui/boundaries.rs +++ b/zellij-server/src/ui/boundaries.rs @@ -63,12 +63,11 @@ impl BoundarySymbol { self.boundary_type ) })?; - TerminalCharacter { + TerminalCharacter::new_singlewidth_styled( character, - width: 1, - styles: RESET_STYLES + RESET_STYLES .foreground(self.color.map(|palette_color| palette_color.into())), - } + ) }; Ok(tc) } diff --git a/zellij-server/src/ui/pane_boundaries_frame.rs b/zellij-server/src/ui/pane_boundaries_frame.rs index 745786de5f..2f73930473 100644 --- a/zellij-server/src/ui/pane_boundaries_frame.rs +++ b/zellij-server/src/ui/pane_boundaries_frame.rs @@ -25,11 +25,8 @@ fn foreground_color(characters: &str, color: Option) -> Vec) -> Vec Date: Fri, 29 Dec 2023 15:24:25 +0000 Subject: [PATCH 15/29] fix(build): Don't unconditionally rebuild zellij-utils --- xtask/src/build.rs | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/xtask/src/build.rs b/xtask/src/build.rs index 4babad0c3f..bc10362258 100644 --- a/xtask/src/build.rs +++ b/xtask/src/build.rs @@ -52,22 +52,34 @@ pub fn build(sh: &Shell, flags: flags::Build) -> anyhow::Result<()> { std::fs::create_dir_all(&prost_asset_dir).unwrap(); let mut prost = prost_build::Config::new(); + let last_generated = prost_asset_dir.join("generated_plugin_api.rs") + .metadata() + .and_then(|m| m.modified()); + let mut needs_regeneration = false; prost.out_dir(prost_asset_dir); prost.include_file("generated_plugin_api.rs"); let mut proto_files = vec![]; for entry in std::fs::read_dir(&protobuf_source_dir).unwrap() { let entry_path = entry.unwrap().path(); if entry_path.is_file() { - if let Some(extension) = entry_path.extension() { - if extension == "proto" { - proto_files.push(entry_path.display().to_string()) - } + if !entry_path.extension().map(|e| e == "proto").unwrap_or(false) { + continue + } + proto_files.push(entry_path.display().to_string()); + let modified = entry_path.metadata().and_then(|m| m.modified()); + needs_regeneration |= match (&last_generated, modified) { + (Ok(last_generated), Ok(modified)) => + modified >= *last_generated, + // Couldn't read some metadata, assume needs update + _ => true, } } } - prost - .compile_protos(&proto_files, &[protobuf_source_dir]) - .unwrap(); + if needs_regeneration { + prost + .compile_protos(&proto_files, &[protobuf_source_dir]) + .unwrap(); + } } let _pd = sh.push_dir(Path::new(crate_name)); From 846fda73c7d88dcfcdb6c57bfd8400a86493001c Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Fri, 29 Dec 2023 16:25:17 +0000 Subject: [PATCH 16/29] refactor: Remove Copy impl on TerminalCharacter --- zellij-server/src/output/mod.rs | 4 +- zellij-server/src/panes/grid.rs | 51 ++++++++++--------- zellij-server/src/panes/terminal_character.rs | 2 +- 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/zellij-server/src/output/mod.rs b/zellij-server/src/output/mod.rs index b835cc090c..de3f1fe775 100644 --- a/zellij-server/src/output/mod.rs +++ b/zellij-server/src/output/mod.rs @@ -815,8 +815,8 @@ impl CharacterChunk { } let next_character = self.terminal_characters.remove(0); // TODO: consider copying self.terminal_characters into a VecDeque to make this process faster? if drained_part_len + next_character.width() <= x { - drained_part.push_back(next_character); drained_part_len += next_character.width(); + drained_part.push_back(next_character); } else { if drained_part_len == x { self.terminal_characters.insert(0, next_character); // put it back @@ -963,7 +963,7 @@ impl OutputBuffer { row: &Row, viewport_width: usize, ) -> Vec { - let mut terminal_characters: Vec = row.columns.iter().copied().collect(); + let mut terminal_characters: Vec = row.columns.iter().cloned().collect(); // pad row let row_width = row.width(); if row_width < viewport_width { diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index dcf6a715ed..15288df9c1 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -987,7 +987,7 @@ impl Grid { .iter() .map(|r| { let excess_width = r.excess_width(); - let mut line: Vec = r.columns.iter().copied().collect(); + let mut line: Vec = r.columns.iter().cloned().collect(); // pad line line.resize( self.width.saturating_sub(excess_width), @@ -1205,7 +1205,7 @@ impl Grid { pad_character.styles = self.cursor.pending_styles; for _ in 0..count { self.viewport.remove(scroll_region_top); - let columns = VecDeque::from(vec![pad_character; self.width]); + let columns = VecDeque::from(vec![pad_character.clone(); self.width]); self.viewport .insert(scroll_region_bottom, Row::from_columns(columns).canonical()); } @@ -1220,7 +1220,7 @@ impl Grid { }; for _ in 0..self.height { - let columns = VecDeque::from(vec![character; self.width]); + let columns = VecDeque::from(vec![character.clone(); self.width]); self.viewport.push(Row::from_columns(columns).canonical()); } self.output_buffer.update_all_lines(); @@ -1367,7 +1367,7 @@ impl Grid { self.viewport .get(self.cursor.y) .and_then(|current_line| current_line.columns.get(absolute_x_in_line)) - .copied() + .cloned() } pub fn get_absolute_character_index(&self, x: usize, y: usize) -> usize { self.viewport.get(y).unwrap().absolute_character_index(x) @@ -1390,7 +1390,7 @@ impl Grid { pub fn clear_all_after_cursor(&mut self, replace_with: TerminalCharacter) { if let Some(cursor_row) = self.viewport.get_mut(self.cursor.y) { cursor_row.truncate(self.cursor.x); - let replace_with_columns = VecDeque::from(vec![replace_with; self.width]); + let replace_with_columns = VecDeque::from(vec![replace_with.clone(); self.width]); self.replace_characters_in_line_after_cursor(replace_with); for row in self.viewport.iter_mut().skip(self.cursor.y + 1) { row.replace_columns(replace_with_columns.clone()); @@ -1400,8 +1400,8 @@ impl Grid { } pub fn clear_all_before_cursor(&mut self, replace_with: TerminalCharacter) { if self.viewport.get(self.cursor.y).is_some() { + let replace_with_columns = VecDeque::from(vec![replace_with.clone(); self.width]); self.replace_characters_in_line_before_cursor(replace_with); - let replace_with_columns = VecDeque::from(vec![replace_with; self.width]); for row in self.viewport.iter_mut().take(self.cursor.y) { row.replace_columns(replace_with_columns.clone()); } @@ -1415,7 +1415,7 @@ impl Grid { } } pub fn clear_all(&mut self, replace_with: TerminalCharacter) { - let replace_with_columns = VecDeque::from(vec![replace_with; self.width]); + let replace_with_columns = VecDeque::from(vec![replace_with.clone(); self.width]); self.replace_characters_in_line_after_cursor(replace_with); for row in &mut self.viewport { row.replace_columns(replace_with_columns.clone()); @@ -1450,18 +1450,18 @@ impl Grid { fn pad_current_line_until(&mut self, position: usize, pad_character: TerminalCharacter) { if self.viewport.get(self.cursor.y).is_none() { - self.pad_lines_until(self.cursor.y, pad_character); + self.pad_lines_until(self.cursor.y, pad_character.clone()); } if let Some(current_row) = self.viewport.get_mut(self.cursor.y) { for _ in current_row.width()..position { - current_row.push(pad_character); + current_row.push(pad_character.clone()); } self.output_buffer.update_line(self.cursor.y); } } fn pad_lines_until(&mut self, position: usize, pad_character: TerminalCharacter) { for _ in self.viewport.len()..=position { - let columns = VecDeque::from(vec![pad_character; self.width]); + let columns = VecDeque::from(vec![pad_character.clone(); self.width]); self.viewport.push(Row::from_columns(columns).canonical()); self.output_buffer.update_line(self.viewport.len() - 1); } @@ -1480,13 +1480,13 @@ impl Grid { } else { self.cursor.y = std::cmp::min(self.height - 1, y + y_offset); } - self.pad_lines_until(self.cursor.y, pad_character); + self.pad_lines_until(self.cursor.y, pad_character.clone()); self.pad_current_line_until(self.cursor.x, pad_character); }, None => { self.cursor.x = std::cmp::min(self.width - 1, x); self.cursor.y = std::cmp::min(self.height - 1, y); - self.pad_lines_until(self.cursor.y, pad_character); + self.pad_lines_until(self.cursor.y, pad_character.clone()); self.pad_current_line_until(self.cursor.x, pad_character); }, } @@ -1586,7 +1586,7 @@ impl Grid { // region for _ in 0..count { self.viewport.remove(current_line_index); - let columns = VecDeque::from(vec![pad_character; self.width]); + let columns = VecDeque::from(vec![pad_character.clone(); self.width]); if self.viewport.len() > scroll_region_bottom { self.viewport .insert(scroll_region_bottom, Row::from_columns(columns).canonical()); @@ -1615,7 +1615,7 @@ impl Grid { if scroll_region_bottom < self.viewport.len() { self.viewport.remove(scroll_region_bottom); } - let columns = VecDeque::from(vec![pad_character; self.width]); + let columns = VecDeque::from(vec![pad_character.clone(); self.width]); self.viewport .insert(current_line_index, Row::from_columns(columns).canonical()); } @@ -1638,10 +1638,10 @@ impl Grid { let mut empty_character = EMPTY_TERMINAL_CHARACTER; empty_character.styles = empty_char_style; let pad_until = std::cmp::min(self.width, self.cursor.x + count); - self.pad_current_line_until(pad_until, empty_character); + self.pad_current_line_until(pad_until, empty_character.clone()); if let Some(current_row) = self.viewport.get_mut(self.cursor.y) { for i in 0..count { - current_row.replace_character_at(empty_character, self.cursor.x + i); + current_row.replace_character_at(empty_character.clone(), self.cursor.x + i); } self.output_buffer.update_line(self.cursor.y); } @@ -1664,9 +1664,9 @@ impl Grid { .unwrap_or(0) .saturating_sub(1); for _ in 0..excess_width { - current_row.insert_character_at(empty_character, self.cursor.x); + current_row.insert_character_at(empty_character.clone(), self.cursor.x); } - current_row.push(empty_character); + current_row.push(empty_character.clone()); } self.output_buffer.update_line(self.cursor.y); } @@ -2145,7 +2145,7 @@ impl Perform for Grid { let c = self.cursor.charsets[self.active_charset].map(c); let terminal_character = TerminalCharacter::new_styled(c, self.cursor.pending_styles); - self.set_preceding_character(terminal_character); + self.set_preceding_character(terminal_character.clone()); self.add_character(terminal_character); } @@ -2821,9 +2821,9 @@ impl Perform for Grid { self.add_character_at_cursor_position(pad_character, true); } } else if c == 'b' { - if let Some(c) = self.preceding_char { + if let Some(c) = self.preceding_char.clone() { for _ in 0..next_param_or(1) { - self.add_character(c); + self.add_character(c.clone()); } } } else if c == 'E' { @@ -3220,10 +3220,10 @@ impl Row { pub fn add_character_at(&mut self, terminal_character: TerminalCharacter, x: usize) { match self.width_cached().cmp(&x) { Ordering::Equal => { - // adding the character at the end of the current line - self.columns.push_back(terminal_character); // this is unwrapped because this always happens after self.width_cached() *self.width.as_mut().unwrap() += terminal_character.width(); + // adding the character at the end of the current line + self.columns.push_back(terminal_character); }, Ordering::Less => { // adding the character after the end of the current line @@ -3295,10 +3295,11 @@ impl Row { pub fn replace_character_at(&mut self, terminal_character: TerminalCharacter, x: usize) { let absolute_x_index = self.absolute_character_index(x); if absolute_x_index < self.columns.len() { + let terminal_character_width = terminal_character.width(); self.columns.push_back(terminal_character); // this is much more performant than remove/insert if let Some(character) = self.columns.swap_remove_back(absolute_x_index) { - let excess_width = character.width().saturating_sub(terminal_character.width()); + let excess_width = character.width().saturating_sub(terminal_character_width); for _ in 0..excess_width { self.columns .insert(absolute_x_index, EMPTY_TERMINAL_CHARACTER); @@ -3418,8 +3419,8 @@ impl Row { current_part = VecDeque::new(); current_part_len = 0; } - current_part.push_back(character); current_part_len += character.width(); + current_part.push_back(character); } if !current_part.is_empty() { parts.push(Row::from_columns(current_part)) diff --git a/zellij-server/src/panes/terminal_character.rs b/zellij-server/src/panes/terminal_character.rs index 38e98ba370..9e7a4e559a 100644 --- a/zellij-server/src/panes/terminal_character.rs +++ b/zellij-server/src/panes/terminal_character.rs @@ -836,7 +836,7 @@ impl Cursor { } } -#[derive(Clone, Copy, PartialEq)] +#[derive(Clone, PartialEq)] pub struct TerminalCharacter { pub character: char, pub styles: CharacterStyles, From afd2f6589e95930f8a22a4dc9d9b0b66d3e72efd Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Fri, 29 Dec 2023 17:23:42 +0000 Subject: [PATCH 17/29] perf: Rc styles to reduce TerminalCharacter from 60 to 24 bytes With a 10MB single line catted into a fresh terminal, VmRSS goes from 845156 kB to 478396 kB (as reported by /proc//status) before/after this patch Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~12.5s to ~7s --- zellij-server/src/output/mod.rs | 4 +- zellij-server/src/panes/grid.rs | 51 +++++----- zellij-server/src/panes/terminal_character.rs | 93 +++++++++++++++++-- zellij-server/src/panes/terminal_pane.rs | 6 +- zellij-server/src/ui/boundaries.rs | 3 +- zellij-server/src/ui/pane_boundaries_frame.rs | 4 +- 6 files changed, 123 insertions(+), 38 deletions(-) diff --git a/zellij-server/src/output/mod.rs b/zellij-server/src/output/mod.rs index de3f1fe775..a535787959 100644 --- a/zellij-server/src/output/mod.rs +++ b/zellij-server/src/output/mod.rs @@ -98,7 +98,7 @@ fn serialize_chunks_with_newlines( for t_character in character_chunk.terminal_characters.iter() { let current_character_styles = adjust_styles_for_possible_selection( character_chunk.selection_and_colors(), - t_character.styles, + *t_character.styles, character_chunk.y, chunk_width, ); @@ -139,7 +139,7 @@ fn serialize_chunks( for t_character in character_chunk.terminal_characters.iter() { let current_character_styles = adjust_styles_for_possible_selection( character_chunk.selection_and_colors(), - t_character.styles, + *t_character.styles, character_chunk.y, chunk_width, ); diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 15288df9c1..daace245ff 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -34,7 +34,7 @@ use crate::panes::link_handler::LinkHandler; use crate::panes::search::SearchResult; use crate::panes::selection::Selection; use crate::panes::terminal_character::{ - AnsiCode, CharacterStyles, CharsetIndex, Cursor, CursorShape, StandardCharset, + AnsiCode, CharsetIndex, Cursor, CursorShape, RcCharacterStyles, StandardCharset, TerminalCharacter, EMPTY_TERMINAL_CHARACTER, }; use crate::ui::components::UiComponentParser; @@ -513,7 +513,7 @@ impl Grid { pub fn update_line_for_rendering(&mut self, line_index: usize) { self.output_buffer.update_line(line_index); } - pub fn advance_to_next_tabstop(&mut self, styles: CharacterStyles) { + pub fn advance_to_next_tabstop(&mut self, styles: RcCharacterStyles) { let next_tabstop = self .horizontal_tabstops .iter() @@ -1186,7 +1186,7 @@ impl Grid { self.viewport.remove(scroll_region_bottom); } let mut pad_character = EMPTY_TERMINAL_CHARACTER; - pad_character.styles = self.cursor.pending_styles; + pad_character.styles = self.cursor.pending_styles.clone(); let columns = VecDeque::from(vec![pad_character; self.width]); self.viewport .insert(scroll_region_top, Row::from_columns(columns).canonical()); @@ -1202,7 +1202,7 @@ impl Grid { { self.pad_lines_until(scroll_region_bottom, EMPTY_TERMINAL_CHARACTER); let mut pad_character = EMPTY_TERMINAL_CHARACTER; - pad_character.styles = self.cursor.pending_styles; + pad_character.styles = self.cursor.pending_styles.clone(); for _ in 0..count { self.viewport.remove(scroll_region_top); let columns = VecDeque::from(vec![pad_character.clone(); self.width]); @@ -1246,14 +1246,14 @@ impl Grid { } let mut pad_character = EMPTY_TERMINAL_CHARACTER; - pad_character.styles = self.cursor.pending_styles; + pad_character.styles = self.cursor.pending_styles.clone(); let columns = VecDeque::from(vec![pad_character; self.width]); self.viewport.push(Row::from_columns(columns).canonical()); self.selection.move_up(1); } else { self.viewport.remove(scroll_region_top); let mut pad_character = EMPTY_TERMINAL_CHARACTER; - pad_character.styles = self.cursor.pending_styles; + pad_character.styles = self.cursor.pending_styles.clone(); let columns = VecDeque::from(vec![pad_character; self.width]); if self.viewport.len() >= scroll_region_bottom { self.viewport @@ -1562,7 +1562,7 @@ impl Grid { let bottom_line_index = bottom_line_index.unwrap_or(self.height); self.scroll_region = Some((top_line_index, bottom_line_index)); let mut pad_character = EMPTY_TERMINAL_CHARACTER; - pad_character.styles = self.cursor.pending_styles; + pad_character.styles = self.cursor.pending_styles.clone(); self.move_cursor_to(0, 0, pad_character); // DECSTBM moves the cursor to column 1 line 1 of the page } pub fn clear_scroll_region(&mut self) { @@ -1634,7 +1634,7 @@ impl Grid { let pad_character = EMPTY_TERMINAL_CHARACTER; self.pad_current_line_until(self.cursor.x, pad_character); } - pub fn replace_with_empty_chars(&mut self, count: usize, empty_char_style: CharacterStyles) { + pub fn replace_with_empty_chars(&mut self, count: usize, empty_char_style: RcCharacterStyles) { let mut empty_character = EMPTY_TERMINAL_CHARACTER; empty_character.styles = empty_char_style; let pad_until = std::cmp::min(self.width, self.cursor.x + count); @@ -1646,7 +1646,7 @@ impl Grid { self.output_buffer.update_line(self.cursor.y); } } - fn erase_characters(&mut self, count: usize, empty_char_style: CharacterStyles) { + fn erase_characters(&mut self, count: usize, empty_char_style: RcCharacterStyles) { let mut empty_character = EMPTY_TERMINAL_CHARACTER; empty_character.styles = empty_char_style; if let Some(current_row) = self.viewport.get_mut(self.cursor.y) { @@ -2144,7 +2144,7 @@ impl Perform for Grid { fn print(&mut self, c: char) { let c = self.cursor.charsets[self.active_charset].map(c); - let terminal_character = TerminalCharacter::new_styled(c, self.cursor.pending_styles); + let terminal_character = TerminalCharacter::new_styled(c, self.cursor.pending_styles.clone()); self.set_preceding_character(terminal_character.clone()); self.add_character(terminal_character); } @@ -2160,7 +2160,7 @@ impl Perform for Grid { }, 9 => { // tab - self.advance_to_next_tabstop(self.cursor.pending_styles); + self.advance_to_next_tabstop(self.cursor.pending_styles.clone()); }, 10 | 11 | 12 => { // 0a, newline @@ -2288,8 +2288,9 @@ impl Perform for Grid { if params.len() < 3 { return; } - self.cursor.pending_styles.link_anchor = - self.link_handler.borrow_mut().dispatch_osc8(params); + self.cursor.pending_styles.update(|styles| { + styles.link_anchor = self.link_handler.borrow_mut().dispatch_osc8(params) + }) }, // Get/set Foreground (b"10") or background (b"11") colors @@ -2442,7 +2443,9 @@ impl Perform for Grid { if intermediates.is_empty() { self.cursor .pending_styles - .add_style_from_ansi_params(&mut params_iter); + .update(|styles| { + styles.add_style_from_ansi_params(&mut params_iter) + }) } } else if c == 'C' || c == 'a' { // move cursor forward @@ -2453,7 +2456,9 @@ impl Perform for Grid { if let Some(clear_type) = params_iter.next().map(|param| param[0]) { let mut char_to_replace = EMPTY_TERMINAL_CHARACTER; if let Some(background_color) = self.cursor.pending_styles.background { - char_to_replace.styles.background = Some(background_color); + char_to_replace.styles.update(|styles| { + styles.background = Some(background_color) + }); } if clear_type == 0 { self.replace_characters_in_line_after_cursor(char_to_replace); @@ -2467,7 +2472,9 @@ impl Perform for Grid { // clear all (0 => below, 1 => above, 2 => all, 3 => saved) let mut char_to_replace = EMPTY_TERMINAL_CHARACTER; if let Some(background_color) = self.cursor.pending_styles.background { - char_to_replace.styles.background = Some(background_color); + char_to_replace.styles.update(|styles| { + styles.background = Some(background_color) + }); } if let Some(clear_type) = params_iter.next().map(|param| param[0]) { if clear_type == 0 { @@ -2729,13 +2736,13 @@ impl Perform for Grid { // delete lines if currently inside scroll region let line_count_to_delete = next_param_or(1); let mut pad_character = EMPTY_TERMINAL_CHARACTER; - pad_character.styles = self.cursor.pending_styles; + pad_character.styles = self.cursor.pending_styles.clone(); self.delete_lines_in_scroll_region(line_count_to_delete, pad_character); } else if c == 'L' { // insert blank lines if inside scroll region let line_count_to_add = next_param_or(1); let mut pad_character = EMPTY_TERMINAL_CHARACTER; - pad_character.styles = self.cursor.pending_styles; + pad_character.styles = self.cursor.pending_styles.clone(); self.add_empty_lines_in_scroll_region(line_count_to_add, pad_character); } else if c == 'G' || c == '`' { let column = next_param_or(1).saturating_sub(1); @@ -2756,11 +2763,11 @@ impl Perform for Grid { } else if c == 'P' { // erase characters let count = next_param_or(1); - self.erase_characters(count, self.cursor.pending_styles); + self.erase_characters(count, self.cursor.pending_styles.clone()); } else if c == 'X' { // erase characters and replace with empty characters of current style let count = next_param_or(1); - self.replace_with_empty_chars(count, self.cursor.pending_styles); + self.replace_with_empty_chars(count, self.cursor.pending_styles.clone()); } else if c == 'T' { /* * 124 54 T SD @@ -2817,7 +2824,7 @@ impl Perform for Grid { let count = next_param_or(1); for _ in 0..count { let mut pad_character = EMPTY_TERMINAL_CHARACTER; - pad_character.styles = self.cursor.pending_styles; + pad_character.styles = self.cursor.pending_styles.clone(); self.add_character_at_cursor_position(pad_character, true); } } else if c == 'b' { @@ -2839,7 +2846,7 @@ impl Perform for Grid { self.move_cursor_to_beginning_of_line(); } else if c == 'I' { for _ in 0..next_param_or(1) { - self.advance_to_next_tabstop(self.cursor.pending_styles); + self.advance_to_next_tabstop(self.cursor.pending_styles.clone()); } } else if c == 'q' { let first_intermediate_is_space = matches!(intermediates.get(0), Some(b' ')); diff --git a/zellij-server/src/panes/terminal_character.rs b/zellij-server/src/panes/terminal_character.rs index 9e7a4e559a..4f7d6a4556 100644 --- a/zellij-server/src/panes/terminal_character.rs +++ b/zellij-server/src/panes/terminal_character.rs @@ -1,6 +1,7 @@ use std::convert::From; use std::fmt::{self, Debug, Display, Formatter}; use std::ops::{Index, IndexMut}; +use std::rc::Rc; use unicode_width::UnicodeWidthChar; use unicode_width::UnicodeWidthStr; @@ -15,7 +16,7 @@ use crate::panes::alacritty_functions::parse_sgr_color; pub const EMPTY_TERMINAL_CHARACTER: TerminalCharacter = TerminalCharacter { character: ' ', width: 1, - styles: RESET_STYLES, + styles: RcCharacterStyles::Reset, }; pub const RESET_STYLES: CharacterStyles = CharacterStyles { @@ -35,6 +36,25 @@ pub const RESET_STYLES: CharacterStyles = CharacterStyles { styled_underlines_enabled: false, }; +// Have to manually write this out because Default::default +// is not a const fn +const DEFAULT_STYLES: CharacterStyles = CharacterStyles { + foreground: None, + background: None, + underline_color: None, + strike: None, + hidden: None, + reverse: None, + slow_blink: None, + fast_blink: None, + underline: None, + bold: None, + dim: None, + italic: None, + link_anchor: None, + styled_underlines_enabled: false, +}; + #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum AnsiCode { On, @@ -129,7 +149,54 @@ impl NamedColor { } } -#[derive(Clone, Copy, Debug, Default)] +#[derive(Clone, Debug, Default, PartialEq)] +pub enum RcCharacterStyles { + #[default] + Default, + Reset, + Rc(Rc), +} + +impl From for RcCharacterStyles { + fn from(styles: CharacterStyles) -> Self { + if styles == DEFAULT_STYLES { + RcCharacterStyles::Default + } else if styles == RESET_STYLES { + RcCharacterStyles::Reset + } else { + RcCharacterStyles::Rc(Rc::new(styles)) + } + } +} + +impl std::ops::Deref for RcCharacterStyles { + type Target = CharacterStyles; + + fn deref(&self) -> &Self::Target { + match self { + RcCharacterStyles::Default => &DEFAULT_STYLES, + RcCharacterStyles::Reset => &RESET_STYLES, + RcCharacterStyles::Rc(styles) => &*styles, + } + } +} + +impl Display for RcCharacterStyles { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + let styles: &CharacterStyles = &*self; + Display::fmt(&styles, f) + } +} + +impl RcCharacterStyles { + pub fn update(&mut self, f: impl FnOnce(&mut CharacterStyles)) { + let mut styles: CharacterStyles = **self; + f(&mut styles); + *self = styles.into(); + } +} + +#[derive(Clone, Copy, Debug)] pub struct CharacterStyles { pub foreground: Option, pub background: Option, @@ -147,6 +214,12 @@ pub struct CharacterStyles { pub styled_underlines_enabled: bool, } +impl Default for CharacterStyles { + fn default() -> Self { + DEFAULT_STYLES + } +} + impl PartialEq for CharacterStyles { fn eq(&self, other: &Self) -> bool { self.foreground == other.foreground @@ -813,7 +886,7 @@ impl CursorShape { pub struct Cursor { pub x: usize, pub y: usize, - pub pending_styles: CharacterStyles, + pub pending_styles: RcCharacterStyles, pub charsets: Charsets, shape: CursorShape, } @@ -823,7 +896,9 @@ impl Cursor { Cursor { x, y, - pending_styles: RESET_STYLES.enable_styled_underlines(styled_underlines), + pending_styles: RESET_STYLES + .enable_styled_underlines(styled_underlines) + .into(), charsets: Default::default(), shape: CursorShape::Initial, } @@ -839,18 +914,18 @@ impl Cursor { #[derive(Clone, PartialEq)] pub struct TerminalCharacter { pub character: char, - pub styles: CharacterStyles, + pub styles: RcCharacterStyles, width: u8, } impl TerminalCharacter { #[inline] pub fn new(character: char) -> Self { - Self::new_styled(character, CharacterStyles::default()) + Self::new_styled(character, Default::default()) } #[inline] - pub fn new_styled(character: char, styles: CharacterStyles) -> Self { + pub fn new_styled(character: char, styles: RcCharacterStyles) -> Self { TerminalCharacter { character, styles, @@ -860,11 +935,11 @@ impl TerminalCharacter { #[inline] pub fn new_singlewidth(character: char) -> Self { - Self::new_singlewidth_styled(character, CharacterStyles::default()) + Self::new_singlewidth_styled(character, Default::default()) } #[inline] - pub fn new_singlewidth_styled(character: char, styles: CharacterStyles) -> Self { + pub fn new_singlewidth_styled(character: char, styles: RcCharacterStyles) -> Self { TerminalCharacter { character, styles, diff --git a/zellij-server/src/panes/terminal_pane.rs b/zellij-server/src/panes/terminal_pane.rs index 1e550abd85..57c09a962e 100644 --- a/zellij-server/src/panes/terminal_pane.rs +++ b/zellij-server/src/panes/terminal_pane.rs @@ -427,8 +427,10 @@ impl Pane for TerminalPane { .grid .get_character_under_cursor() .unwrap_or(EMPTY_TERMINAL_CHARACTER); - character_under_cursor.styles.background = Some(cursor_color.into()); - character_under_cursor.styles.foreground = Some(text_color.into()); + character_under_cursor.styles.update(|styles| { + styles.background = Some(cursor_color.into()); + styles.foreground = Some(text_color.into()); + }); // we keep track of these so that we can clear them up later (see render function) self.fake_cursor_locations.insert((cursor_y, cursor_x)); let mut fake_cursor = format!( diff --git a/zellij-server/src/ui/boundaries.rs b/zellij-server/src/ui/boundaries.rs index fb33e4fba3..0df0e4eeae 100644 --- a/zellij-server/src/ui/boundaries.rs +++ b/zellij-server/src/ui/boundaries.rs @@ -66,7 +66,8 @@ impl BoundarySymbol { TerminalCharacter::new_singlewidth_styled( character, RESET_STYLES - .foreground(self.color.map(|palette_color| palette_color.into())), + .foreground(self.color.map(|palette_color| palette_color.into())) + .into(), ) }; Ok(tc) diff --git a/zellij-server/src/ui/pane_boundaries_frame.rs b/zellij-server/src/ui/pane_boundaries_frame.rs index 2f73930473..3eb80a0b31 100644 --- a/zellij-server/src/ui/pane_boundaries_frame.rs +++ b/zellij-server/src/ui/pane_boundaries_frame.rs @@ -26,7 +26,7 @@ fn foreground_color(characters: &str, color: Option) -> Vec) -> Vec Date: Fri, 29 Dec 2023 18:27:09 +0000 Subject: [PATCH 18/29] perf: Remove RcCharacterStyles::Default, allow enum niche optimisation This reduces TerminalCharacter from 24 to 16 bytes With a 10MB single line catted into a fresh terminal, VmRSS goes from 478396 kB to 398108 kB (as reported by /proc//status) before/after this patch Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~7s to ~4.75s --- zellij-server/src/output/mod.rs | 6 +- zellij-server/src/panes/terminal_character.rs | 49 +++++++++------- zellij-server/src/ui/pane_boundaries_frame.rs | 56 ++++++++----------- 3 files changed, 54 insertions(+), 57 deletions(-) diff --git a/zellij-server/src/output/mod.rs b/zellij-server/src/output/mod.rs index a535787959..fb92e1878d 100644 --- a/zellij-server/src/output/mod.rs +++ b/zellij-server/src/output/mod.rs @@ -6,7 +6,7 @@ use crate::panes::Row; use crate::{ panes::sixel::SixelImageStore, panes::terminal_character::{AnsiCode, CharacterStyles}, - panes::{LinkHandler, TerminalCharacter, EMPTY_TERMINAL_CHARACTER}, + panes::{LinkHandler, TerminalCharacter, DEFAULT_STYLES, EMPTY_TERMINAL_CHARACTER}, ClientId, }; use std::cell::RefCell; @@ -92,7 +92,7 @@ fn serialize_chunks_with_newlines( for character_chunk in character_chunks { let chunk_changed_colors = character_chunk.changed_colors(); let mut character_styles = - CharacterStyles::new().enable_styled_underlines(styled_underlines); + DEFAULT_STYLES.enable_styled_underlines(styled_underlines); vte_output.push_str("\n\r"); let mut chunk_width = character_chunk.x; for t_character in character_chunk.terminal_characters.iter() { @@ -132,7 +132,7 @@ fn serialize_chunks( for character_chunk in character_chunks { let chunk_changed_colors = character_chunk.changed_colors(); let mut character_styles = - CharacterStyles::new().enable_styled_underlines(styled_underlines); + DEFAULT_STYLES.enable_styled_underlines(styled_underlines); vte_goto_instruction(character_chunk.x, character_chunk.y, &mut vte_output) .with_context(err_context)?; let mut chunk_width = character_chunk.x; diff --git a/zellij-server/src/panes/terminal_character.rs b/zellij-server/src/panes/terminal_character.rs index 4f7d6a4556..035c14c7d9 100644 --- a/zellij-server/src/panes/terminal_character.rs +++ b/zellij-server/src/panes/terminal_character.rs @@ -36,9 +36,9 @@ pub const RESET_STYLES: CharacterStyles = CharacterStyles { styled_underlines_enabled: false, }; -// Have to manually write this out because Default::default -// is not a const fn -const DEFAULT_STYLES: CharacterStyles = CharacterStyles { +// Prefer to use RcCharacterStyles::default() where it makes sense +// as it will reduce memory usage +pub const DEFAULT_STYLES: CharacterStyles = CharacterStyles { foreground: None, background: None, underline_color: None, @@ -55,6 +55,11 @@ const DEFAULT_STYLES: CharacterStyles = CharacterStyles { styled_underlines_enabled: false, }; +thread_local! { + static RC_DEFAULT_STYLES: RcCharacterStyles = + RcCharacterStyles::Rc(Rc::new(DEFAULT_STYLES)); +} + #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum AnsiCode { On, @@ -149,19 +154,18 @@ impl NamedColor { } } -#[derive(Clone, Debug, Default, PartialEq)] +// This enum carefully only has two variants so +// enum niche optimisations can keep it to 8 bytes +#[derive(Clone, Debug, PartialEq)] pub enum RcCharacterStyles { - #[default] - Default, Reset, Rc(Rc), } +const _: [(); 8] = [(); std::mem::size_of::()]; impl From for RcCharacterStyles { fn from(styles: CharacterStyles) -> Self { - if styles == DEFAULT_STYLES { - RcCharacterStyles::Default - } else if styles == RESET_STYLES { + if styles == RESET_STYLES { RcCharacterStyles::Reset } else { RcCharacterStyles::Rc(Rc::new(styles)) @@ -169,12 +173,17 @@ impl From for RcCharacterStyles { } } +impl Default for RcCharacterStyles { + fn default() -> Self { + RC_DEFAULT_STYLES.with(|s| s.clone()) + } +} + impl std::ops::Deref for RcCharacterStyles { type Target = CharacterStyles; fn deref(&self) -> &Self::Target { match self { - RcCharacterStyles::Default => &DEFAULT_STYLES, RcCharacterStyles::Reset => &RESET_STYLES, RcCharacterStyles::Rc(styles) => &*styles, } @@ -189,6 +198,10 @@ impl Display for RcCharacterStyles { } impl RcCharacterStyles { + pub fn reset() -> Self { + Self::Reset + } + pub fn update(&mut self, f: impl FnOnce(&mut CharacterStyles)) { let mut styles: CharacterStyles = **self; f(&mut styles); @@ -214,12 +227,6 @@ pub struct CharacterStyles { pub styled_underlines_enabled: bool, } -impl Default for CharacterStyles { - fn default() -> Self { - DEFAULT_STYLES - } -} - impl PartialEq for CharacterStyles { fn eq(&self, other: &Self) -> bool { self.foreground == other.foreground @@ -239,9 +246,6 @@ impl PartialEq for CharacterStyles { } impl CharacterStyles { - pub fn new() -> Self { - Self::default() - } pub fn foreground(mut self, foreground_code: Option) -> Self { self.foreground = foreground_code; self @@ -329,7 +333,7 @@ impl CharacterStyles { // create diff from all changed styles let mut diff = - CharacterStyles::new().enable_styled_underlines(self.styled_underlines_enabled); + DEFAULT_STYLES.enable_styled_underlines(self.styled_underlines_enabled); if self.foreground != new_styles.foreground { diff.foreground = new_styles.foreground; @@ -388,7 +392,7 @@ impl CharacterStyles { } Some(diff) } - pub fn reset_all(&mut self) { + fn reset_all(&mut self) { self.foreground = Some(AnsiCode::Reset); self.background = Some(AnsiCode::Reset); self.underline_color = Some(AnsiCode::Reset); @@ -917,6 +921,9 @@ pub struct TerminalCharacter { pub styles: RcCharacterStyles, width: u8, } +// This size has significant memory and CPU implications for long lines, +// be careful about allowing it to grow +const _: [(); 16] = [(); std::mem::size_of::()]; impl TerminalCharacter { #[inline] diff --git a/zellij-server/src/ui/pane_boundaries_frame.rs b/zellij-server/src/ui/pane_boundaries_frame.rs index 3eb80a0b31..1c517415fd 100644 --- a/zellij-server/src/ui/pane_boundaries_frame.rs +++ b/zellij-server/src/ui/pane_boundaries_frame.rs @@ -1,5 +1,5 @@ use crate::output::CharacterChunk; -use crate::panes::{AnsiCode, CharacterStyles, TerminalCharacter, EMPTY_TERMINAL_CHARACTER}; +use crate::panes::{AnsiCode, RcCharacterStyles, TerminalCharacter, EMPTY_TERMINAL_CHARACTER}; use crate::ui::boundaries::boundary_type; use crate::ClientId; use zellij_utils::data::{client_id_to_colors, PaletteColor, Style}; @@ -11,22 +11,17 @@ use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; fn foreground_color(characters: &str, color: Option) -> Vec { let mut colored_string = Vec::new(); for character in characters.chars() { - let styles = match color { - Some(palette_color) => { - let mut styles = CharacterStyles::new(); - styles.reset_all(); - styles - .foreground(Some(AnsiCode::from(palette_color))) - .bold(Some(AnsiCode::On)) - }, - None => { - let mut styles = CharacterStyles::new(); - styles.reset_all(); - styles.bold(Some(AnsiCode::On)) - }, - }; - let terminal_character = - TerminalCharacter::new_styled(character, styles.into()); + let mut styles = RcCharacterStyles::reset(); + styles.update(|styles| { + styles.bold = Some(AnsiCode::On); + match color { + Some(palette_color) => { + styles.foreground = Some(AnsiCode::from(palette_color)); + }, + None => {}, + } + }); + let terminal_character = TerminalCharacter::new_styled(character, styles); colored_string.push(terminal_character); } colored_string @@ -35,22 +30,17 @@ fn foreground_color(characters: &str, color: Option) -> Vec) -> Vec { let mut colored_string = Vec::new(); for character in characters.chars() { - let styles = match color { - Some(palette_color) => { - let mut styles = CharacterStyles::new(); - styles.reset_all(); - styles - .background(Some(AnsiCode::from(palette_color))) - .bold(Some(AnsiCode::On)) - }, - None => { - let mut styles = CharacterStyles::new(); - styles.reset_all(); - styles - }, - }; - let terminal_character = - TerminalCharacter::new_styled(character, styles.into()); + let mut styles = RcCharacterStyles::reset(); + styles.update(|styles| { + match color { + Some(palette_color) => { + styles.background = Some(AnsiCode::from(palette_color)); + styles.bold(Some(AnsiCode::On)); + }, + None => {}, + } + }); + let terminal_character = TerminalCharacter::new_styled(character, styles); colored_string.push(terminal_character); } colored_string From d53b0e89c4a323c83c29f544ef2fed1478fb3f9c Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Fri, 29 Dec 2023 18:39:53 +0000 Subject: [PATCH 19/29] docs: link anchor omission from reset_all is deliberate reset_all is only used from ansi params, and ansi params don't control link anchor --- zellij-server/src/panes/terminal_character.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/zellij-server/src/panes/terminal_character.rs b/zellij-server/src/panes/terminal_character.rs index 035c14c7d9..3e5f761000 100644 --- a/zellij-server/src/panes/terminal_character.rs +++ b/zellij-server/src/panes/terminal_character.rs @@ -392,7 +392,7 @@ impl CharacterStyles { } Some(diff) } - fn reset_all(&mut self) { + fn reset_ansi(&mut self) { self.foreground = Some(AnsiCode::Reset); self.background = Some(AnsiCode::Reset); self.underline_color = Some(AnsiCode::Reset); @@ -405,11 +405,12 @@ impl CharacterStyles { self.reverse = Some(AnsiCode::Reset); self.hidden = Some(AnsiCode::Reset); self.strike = Some(AnsiCode::Reset); + // Deliberately don't end link anchor } pub fn add_style_from_ansi_params(&mut self, params: &mut ParamsIter) { while let Some(param) = params.next() { match param { - [] | [0] => self.reset_all(), + [] | [0] => self.reset_ansi(), [1] => *self = self.bold(Some(AnsiCode::On)), [2] => *self = self.dim(Some(AnsiCode::On)), [3] => *self = self.italic(Some(AnsiCode::On)), From a9a5a69371133da692a2da69728cd822feefacd3 Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Fri, 29 Dec 2023 18:41:05 +0000 Subject: [PATCH 20/29] fix: Remove no-op on variable that gets immediately dropped --- zellij-server/src/output/mod.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/zellij-server/src/output/mod.rs b/zellij-server/src/output/mod.rs index fb92e1878d..2f5238f791 100644 --- a/zellij-server/src/output/mod.rs +++ b/zellij-server/src/output/mod.rs @@ -113,7 +113,6 @@ fn serialize_chunks_with_newlines( chunk_width += t_character.width(); vte_output.push(t_character.character); } - character_styles.clear(); } Ok(vte_output) } @@ -154,7 +153,6 @@ fn serialize_chunks( chunk_width += t_character.width(); vte_output.push(t_character.character); } - character_styles.clear(); } if let Some(sixel_image_store) = sixel_image_store { if let Some(sixel_chunks) = sixel_chunks { From 3b55f63ee294066c2aa806da80ae31b534112e83 Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Fri, 29 Dec 2023 18:47:42 +0000 Subject: [PATCH 21/29] refactor: Simplify replace_character_at logic The original condition checked absolute_x_index was in bounds, then used the index to manipulate it. This is equivalent to getting a ref to the character at that position and manipulating directly --- zellij-server/src/panes/grid.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index daace245ff..659d92f1e3 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -3301,16 +3301,13 @@ impl Row { } pub fn replace_character_at(&mut self, terminal_character: TerminalCharacter, x: usize) { let absolute_x_index = self.absolute_character_index(x); - if absolute_x_index < self.columns.len() { + if let Some(character) = self.columns.get_mut(absolute_x_index) { let terminal_character_width = terminal_character.width(); - self.columns.push_back(terminal_character); - // this is much more performant than remove/insert - if let Some(character) = self.columns.swap_remove_back(absolute_x_index) { - let excess_width = character.width().saturating_sub(terminal_character_width); - for _ in 0..excess_width { - self.columns - .insert(absolute_x_index, EMPTY_TERMINAL_CHARACTER); - } + let character = std::mem::replace(character, terminal_character); + let excess_width = character.width().saturating_sub(terminal_character_width); + for _ in 0..excess_width { + self.columns + .insert(absolute_x_index, EMPTY_TERMINAL_CHARACTER); } } self.width = None; From a0731c2e2389d426ab026a2aa728b6dc4d20f2ef Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Fri, 29 Dec 2023 18:55:12 +0000 Subject: [PATCH 22/29] chore: Run xtask format --- xtask/src/build.rs | 14 ++++++++----- zellij-server/src/output/mod.rs | 6 ++---- zellij-server/src/panes/grid.rs | 21 +++++++++---------- zellij-server/src/panes/terminal_character.rs | 3 +-- zellij-server/src/ui/pane_boundaries_frame.rs | 14 ++++++------- 5 files changed, 28 insertions(+), 30 deletions(-) diff --git a/xtask/src/build.rs b/xtask/src/build.rs index bc10362258..67547a1c24 100644 --- a/xtask/src/build.rs +++ b/xtask/src/build.rs @@ -52,7 +52,8 @@ pub fn build(sh: &Shell, flags: flags::Build) -> anyhow::Result<()> { std::fs::create_dir_all(&prost_asset_dir).unwrap(); let mut prost = prost_build::Config::new(); - let last_generated = prost_asset_dir.join("generated_plugin_api.rs") + let last_generated = prost_asset_dir + .join("generated_plugin_api.rs") .metadata() .and_then(|m| m.modified()); let mut needs_regeneration = false; @@ -62,14 +63,17 @@ pub fn build(sh: &Shell, flags: flags::Build) -> anyhow::Result<()> { for entry in std::fs::read_dir(&protobuf_source_dir).unwrap() { let entry_path = entry.unwrap().path(); if entry_path.is_file() { - if !entry_path.extension().map(|e| e == "proto").unwrap_or(false) { - continue + if !entry_path + .extension() + .map(|e| e == "proto") + .unwrap_or(false) + { + continue; } proto_files.push(entry_path.display().to_string()); let modified = entry_path.metadata().and_then(|m| m.modified()); needs_regeneration |= match (&last_generated, modified) { - (Ok(last_generated), Ok(modified)) => - modified >= *last_generated, + (Ok(last_generated), Ok(modified)) => modified >= *last_generated, // Couldn't read some metadata, assume needs update _ => true, } diff --git a/zellij-server/src/output/mod.rs b/zellij-server/src/output/mod.rs index 2f5238f791..7ad7a38bc8 100644 --- a/zellij-server/src/output/mod.rs +++ b/zellij-server/src/output/mod.rs @@ -91,8 +91,7 @@ fn serialize_chunks_with_newlines( let link_handler = link_handler.map(|l_h| l_h.borrow()); for character_chunk in character_chunks { let chunk_changed_colors = character_chunk.changed_colors(); - let mut character_styles = - DEFAULT_STYLES.enable_styled_underlines(styled_underlines); + let mut character_styles = DEFAULT_STYLES.enable_styled_underlines(styled_underlines); vte_output.push_str("\n\r"); let mut chunk_width = character_chunk.x; for t_character in character_chunk.terminal_characters.iter() { @@ -130,8 +129,7 @@ fn serialize_chunks( let link_handler = link_handler.map(|l_h| l_h.borrow()); for character_chunk in character_chunks { let chunk_changed_colors = character_chunk.changed_colors(); - let mut character_styles = - DEFAULT_STYLES.enable_styled_underlines(styled_underlines); + let mut character_styles = DEFAULT_STYLES.enable_styled_underlines(styled_underlines); vte_goto_instruction(character_chunk.x, character_chunk.y, &mut vte_output) .with_context(err_context)?; let mut chunk_width = character_chunk.x; diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 659d92f1e3..377f05f6e8 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -906,7 +906,7 @@ impl Grid { mut new_cursor_y, mut saved_cursor_y_coordinates, new_cursor_x, - saved_cursor_x_coordinates + saved_cursor_x_coordinates, ) = cursors; let current_viewport_row_count = self.viewport.len(); @@ -2144,7 +2144,8 @@ impl Perform for Grid { fn print(&mut self, c: char) { let c = self.cursor.charsets[self.active_charset].map(c); - let terminal_character = TerminalCharacter::new_styled(c, self.cursor.pending_styles.clone()); + let terminal_character = + TerminalCharacter::new_styled(c, self.cursor.pending_styles.clone()); self.set_preceding_character(terminal_character.clone()); self.add_character(terminal_character); } @@ -2443,9 +2444,7 @@ impl Perform for Grid { if intermediates.is_empty() { self.cursor .pending_styles - .update(|styles| { - styles.add_style_from_ansi_params(&mut params_iter) - }) + .update(|styles| styles.add_style_from_ansi_params(&mut params_iter)) } } else if c == 'C' || c == 'a' { // move cursor forward @@ -2456,9 +2455,9 @@ impl Perform for Grid { if let Some(clear_type) = params_iter.next().map(|param| param[0]) { let mut char_to_replace = EMPTY_TERMINAL_CHARACTER; if let Some(background_color) = self.cursor.pending_styles.background { - char_to_replace.styles.update(|styles| { - styles.background = Some(background_color) - }); + char_to_replace + .styles + .update(|styles| styles.background = Some(background_color)); } if clear_type == 0 { self.replace_characters_in_line_after_cursor(char_to_replace); @@ -2472,9 +2471,9 @@ impl Perform for Grid { // clear all (0 => below, 1 => above, 2 => all, 3 => saved) let mut char_to_replace = EMPTY_TERMINAL_CHARACTER; if let Some(background_color) = self.cursor.pending_styles.background { - char_to_replace.styles.update(|styles| { - styles.background = Some(background_color) - }); + char_to_replace + .styles + .update(|styles| styles.background = Some(background_color)); } if let Some(clear_type) = params_iter.next().map(|param| param[0]) { if clear_type == 0 { diff --git a/zellij-server/src/panes/terminal_character.rs b/zellij-server/src/panes/terminal_character.rs index 3e5f761000..2bb1326496 100644 --- a/zellij-server/src/panes/terminal_character.rs +++ b/zellij-server/src/panes/terminal_character.rs @@ -332,8 +332,7 @@ impl CharacterStyles { } // create diff from all changed styles - let mut diff = - DEFAULT_STYLES.enable_styled_underlines(self.styled_underlines_enabled); + let mut diff = DEFAULT_STYLES.enable_styled_underlines(self.styled_underlines_enabled); if self.foreground != new_styles.foreground { diff.foreground = new_styles.foreground; diff --git a/zellij-server/src/ui/pane_boundaries_frame.rs b/zellij-server/src/ui/pane_boundaries_frame.rs index 1c517415fd..57cff1ce42 100644 --- a/zellij-server/src/ui/pane_boundaries_frame.rs +++ b/zellij-server/src/ui/pane_boundaries_frame.rs @@ -31,14 +31,12 @@ fn background_color(characters: &str, color: Option) -> Vec { - styles.background = Some(AnsiCode::from(palette_color)); - styles.bold(Some(AnsiCode::On)); - }, - None => {}, - } + styles.update(|styles| match color { + Some(palette_color) => { + styles.background = Some(AnsiCode::from(palette_color)); + styles.bold(Some(AnsiCode::On)); + }, + None => {}, }); let terminal_character = TerminalCharacter::new_styled(character, styles); colored_string.push(terminal_character); From 85195c5b12833dcdf2b49973f15516b185fa576d Mon Sep 17 00:00:00 2001 From: Aram Drevekenin Date: Thu, 28 Dec 2023 16:46:14 +0100 Subject: [PATCH 23/29] chore(repo): update issue templates --- .github/ISSUE_TEMPLATE/blank_issue.md | 10 ---------- .github/ISSUE_TEMPLATE/bug_report.md | 4 ++-- 2 files changed, 2 insertions(+), 12 deletions(-) delete mode 100644 .github/ISSUE_TEMPLATE/blank_issue.md diff --git a/.github/ISSUE_TEMPLATE/blank_issue.md b/.github/ISSUE_TEMPLATE/blank_issue.md deleted file mode 100644 index a08ad07cbf..0000000000 --- a/.github/ISSUE_TEMPLATE/blank_issue.md +++ /dev/null @@ -1,10 +0,0 @@ ---- -name: Blank Issue -about: Create a blank issue. -title: '' -labels: '' -assignees: '' - ---- - - diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 8ae1af7049..e84ba13cca 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -10,7 +10,7 @@ assignees: '' @@ -36,7 +36,7 @@ Please attach the files that were created in `/tmp/zellij-1000/zellij-log/` to t ## Further information Reproduction steps, noticeable behavior, related issues, etc -# 2. Issues with the Zellij UI / behavior +# 2. Issues with the Zellij UI / behavior / crash