From 4c94f573b867251204faa251335c212b3d7beb30 Mon Sep 17 00:00:00 2001 From: Hongjie Zhai Date: Thu, 22 Apr 2021 16:23:52 +0900 Subject: [PATCH 1/9] fix pane render when switch/resize tab --- src/client/panes/terminal_pane.rs | 11 ++++++----- src/client/tab.rs | 5 +++++ src/common/screen.rs | 14 +++++++++----- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/client/panes/terminal_pane.rs b/src/client/panes/terminal_pane.rs index 5d63a10093..adb884f215 100644 --- a/src/client/panes/terminal_pane.rs +++ b/src/client/panes/terminal_pane.rs @@ -81,6 +81,7 @@ impl Pane for TerminalPane { for byte in bytes.iter() { self.vte_parser.advance(&mut self.grid, *byte); } + self.set_should_render(true); } fn cursor_coordinates(&self) -> Option<(usize, usize)> { // (x, y) @@ -160,7 +161,7 @@ impl Pane for TerminalPane { // around // 2. When there are wide characters in a pane, since we don't yet handle them properly, // the spill over to the pane to the right - // if self.should_render || cfg!(test) { + // if self.should_render() || cfg!(test) { if true { let mut vte_output = String::new(); let buffer_lines = &self.read_buffer_as_lines(); @@ -203,7 +204,7 @@ impl Pane for TerminalPane { } character_styles.clear(); } - self.grid.should_render = false; + self.set_should_render(false); Some(vte_output) } else { None @@ -262,15 +263,15 @@ impl Pane for TerminalPane { } fn scroll_up(&mut self, count: usize) { self.grid.move_viewport_up(count); - self.grid.should_render = true; + self.set_should_render(true); } fn scroll_down(&mut self, count: usize) { self.grid.move_viewport_down(count); - self.grid.should_render = true; + self.set_should_render(true); } fn clear_scroll(&mut self) { self.grid.reset_viewport(); - self.grid.should_render = true; + self.set_should_render(true); } } diff --git a/src/client/tab.rs b/src/client/tab.rs index acccde3aca..6ba62e07d7 100644 --- a/src/client/tab.rs +++ b/src/client/tab.rs @@ -656,6 +656,11 @@ impl Tab { pub fn toggle_fullscreen_is_active(&mut self) { self.fullscreen_is_active = !self.fullscreen_is_active; } + pub fn set_force_render(&mut self) { + for (_, pane) in &mut self.panes { + pane.set_should_render(true); + } + } pub fn render(&mut self) { if self.active_terminal.is_none() { // we might not have an active terminal if we closed the last pane diff --git a/src/common/screen.rs b/src/common/screen.rs index a2f4157401..7d23ac33a8 100644 --- a/src/common/screen.rs +++ b/src/common/screen.rs @@ -150,8 +150,9 @@ impl Screen { let active_tab_pos = self.get_active_tab().unwrap().position; let new_tab_pos = (active_tab_pos + 1) % self.tabs.len(); - for tab in self.tabs.values() { + for tab in self.tabs.values_mut() { if tab.position == new_tab_pos { + tab.set_force_render(); self.active_tab_index = Some(tab.index); break; } @@ -168,8 +169,9 @@ impl Screen { } else { active_tab_pos - 1 }; - for tab in self.tabs.values() { + for tab in self.tabs.values_mut() { if tab.position == new_tab_pos { + tab.set_force_render(); self.active_tab_index = Some(tab.index); break; } @@ -180,9 +182,10 @@ impl Screen { pub fn go_to_tab(&mut self, mut tab_index: usize) { tab_index -= 1; - let active_tab = self.get_active_tab().unwrap(); - if let Some(t) = self.tabs.values().find(|t| t.position == tab_index) { - if t.index != active_tab.index { + let active_tab_index = self.get_active_tab().unwrap().index; + if let Some(t) = self.tabs.values_mut().find(|t| t.position == tab_index) { + if t.index != active_tab_index { + t.set_force_render(); self.active_tab_index = Some(t.index); self.update_tabs(); self.render(); @@ -226,6 +229,7 @@ impl Screen { for (_, tab) in self.tabs.iter_mut() { tab.resize_whole_tab(new_screen_size); } + let _ = self.get_active_tab_mut().map(|t| t.set_force_render()); self.render(); } From d949a99cf0115824d44114f01f926b900cfae01b Mon Sep 17 00:00:00 2001 From: Hongjie Zhai Date: Thu, 22 Apr 2021 16:49:25 +0900 Subject: [PATCH 2/9] render when reflow_lines --- src/client/panes/terminal_pane.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/client/panes/terminal_pane.rs b/src/client/panes/terminal_pane.rs index adb884f215..5297f8f781 100644 --- a/src/client/panes/terminal_pane.rs +++ b/src/client/panes/terminal_pane.rs @@ -161,8 +161,8 @@ impl Pane for TerminalPane { // around // 2. When there are wide characters in a pane, since we don't yet handle them properly, // the spill over to the pane to the right - // if self.should_render() || cfg!(test) { - if true { + if self.should_render() { + //if true { let mut vte_output = String::new(); let buffer_lines = &self.read_buffer_as_lines(); let display_cols = self.get_columns(); @@ -316,6 +316,7 @@ impl TerminalPane { let rows = self.get_rows(); let columns = self.get_columns(); self.grid.change_size(rows, columns); + self.set_should_render(true); } pub fn read_buffer_as_lines(&self) -> Vec> { self.grid.as_character_lines() From 8c474016b74087b3c3ab54ed7babce348b4d0a40 Mon Sep 17 00:00:00 2001 From: Hongjie Zhai Date: Thu, 22 Apr 2021 16:51:00 +0900 Subject: [PATCH 3/9] back to always render because widechar issue --- src/client/panes/terminal_pane.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/panes/terminal_pane.rs b/src/client/panes/terminal_pane.rs index 5297f8f781..51a63759bd 100644 --- a/src/client/panes/terminal_pane.rs +++ b/src/client/panes/terminal_pane.rs @@ -161,8 +161,8 @@ impl Pane for TerminalPane { // around // 2. When there are wide characters in a pane, since we don't yet handle them properly, // the spill over to the pane to the right - if self.should_render() { - //if true { + // if self.should_render() { + if true { let mut vte_output = String::new(); let buffer_lines = &self.read_buffer_as_lines(); let display_cols = self.get_columns(); From aa25ba26927169f7e3832ce9fdcd627c23e00ac1 Mon Sep 17 00:00:00 2001 From: Hongjie Zhai Date: Thu, 22 Apr 2021 16:58:36 +0900 Subject: [PATCH 4/9] fix clippy --- src/client/tab.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/tab.rs b/src/client/tab.rs index 6ba62e07d7..4aa6bcc712 100644 --- a/src/client/tab.rs +++ b/src/client/tab.rs @@ -657,7 +657,7 @@ impl Tab { self.fullscreen_is_active = !self.fullscreen_is_active; } pub fn set_force_render(&mut self) { - for (_, pane) in &mut self.panes { + for pane in self.panes.values_mut() { pane.set_should_render(true); } } From e1f7c725232f818b38ee87d57dc1718fc14788cd Mon Sep 17 00:00:00 2001 From: Hongjie Zhai Date: Wed, 28 Apr 2021 06:47:58 +0900 Subject: [PATCH 5/9] force_render when current grid contains widechar --- src/client/panes/grid.rs | 6 ++++++ src/client/panes/plugin_pane.rs | 3 +++ src/client/panes/terminal_character.rs | 12 ++++++++++++ src/client/panes/terminal_pane.rs | 7 +++++-- src/client/tab.rs | 11 +++++++++++ 5 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/client/panes/grid.rs b/src/client/panes/grid.rs index c6dd201e2f..0f36284b76 100644 --- a/src/client/panes/grid.rs +++ b/src/client/panes/grid.rs @@ -196,6 +196,9 @@ impl Grid { clear_viewport_before_rendering: false, } } + pub fn contains_widechar(&self) -> bool { + self.viewport.iter().any(|c| c.contains_widechar()) + } pub fn advance_to_next_tabstop(&mut self, styles: CharacterStyles) { let columns_until_next_tabstop = TABSTOP_WIDTH - (self.cursor.x % TABSTOP_WIDTH); let columns_until_screen_end = self.width - self.cursor.x; @@ -1156,6 +1159,9 @@ impl Row { self.is_canonical = true; self } + pub fn contains_widechar(&self) -> bool { + self.columns.iter().any(|c| c.is_widechar()) + } pub fn add_character_at(&mut self, terminal_character: TerminalCharacter, x: usize) { match self.columns.len().cmp(&x) { Ordering::Equal => self.columns.push(terminal_character), diff --git a/src/client/panes/plugin_pane.rs b/src/client/panes/plugin_pane.rs index 24daec08af..bb1db8ceb0 100644 --- a/src/client/panes/plugin_pane.rs +++ b/src/client/panes/plugin_pane.rs @@ -90,6 +90,9 @@ impl Pane for PluginPane { fn position_and_size_override(&self) -> Option { self.position_and_size_override } + fn contains_widechar(&self) -> bool { + false + } fn should_render(&self) -> bool { self.should_render } diff --git a/src/client/panes/terminal_character.rs b/src/client/panes/terminal_character.rs index 7569e9933b..94620bba7b 100644 --- a/src/client/panes/terminal_character.rs +++ b/src/client/panes/terminal_character.rs @@ -1,3 +1,5 @@ +use unicode_width::UnicodeWidthChar; + use crate::utils::logging::debug_log_to_file; use ::std::fmt::{self, Debug, Display, Formatter}; @@ -634,3 +636,13 @@ impl ::std::fmt::Debug for TerminalCharacter { write!(f, "{}", self.character) } } + +impl TerminalCharacter { + pub fn width(&self) -> usize { + self.character.width().unwrap_or(0) + } + + pub fn is_widechar(&self) -> bool { + self.width() > 1 + } +} diff --git a/src/client/panes/terminal_pane.rs b/src/client/panes/terminal_pane.rs index 51a63759bd..028eb6670b 100644 --- a/src/client/panes/terminal_pane.rs +++ b/src/client/panes/terminal_pane.rs @@ -133,6 +133,9 @@ impl Pane for TerminalPane { fn position_and_size_override(&self) -> Option { self.position_and_size_override } + fn contains_widechar(&self) -> bool { + self.grid.contains_widechar() + } fn should_render(&self) -> bool { self.grid.should_render } @@ -161,8 +164,8 @@ impl Pane for TerminalPane { // around // 2. When there are wide characters in a pane, since we don't yet handle them properly, // the spill over to the pane to the right - // if self.should_render() { - if true { + if self.should_render() { + //if true { let mut vte_output = String::new(); let buffer_lines = &self.read_buffer_as_lines(); let display_cols = self.get_columns(); diff --git a/src/client/tab.rs b/src/client/tab.rs index 4aa6bcc712..de9ed4d11d 100644 --- a/src/client/tab.rs +++ b/src/client/tab.rs @@ -100,6 +100,9 @@ pub trait Pane { fn position_and_size_override(&self) -> Option; fn should_render(&self) -> bool; fn set_should_render(&mut self, should_render: bool); + // FIXME: this method is used to trigger a force render to hide widechar problem + // it should be removed when we can handle widechars + fn contains_widechar(&self) -> bool; fn selectable(&self) -> bool; fn set_selectable(&mut self, selectable: bool); fn set_invisible_borders(&mut self, invisible_borders: bool); @@ -661,12 +664,20 @@ impl Tab { pane.set_should_render(true); } } + pub fn pane_contains_widechar(&self) -> bool { + self.panes.iter().all(|(_, p)| p.contains_widechar()) + } pub fn render(&mut self) { if self.active_terminal.is_none() { // we might not have an active terminal if we closed the last pane // in that case, we should not render as the app is exiting return; } + // if any pane contain widechar, all pane in the same row will messup. We should render them every time + // FIXME: remove this when we can handle widechars correctly + if self.pane_contains_widechar() { + self.set_force_render() + } let mut stdout = self.os_api.get_stdout_writer(); let mut boundaries = Boundaries::new( self.full_screen_ws.columns as u16, From 05e150aea347a759fdf59e8e9987f1781ce30120 Mon Sep 17 00:00:00 2001 From: Hongjie Zhai Date: Wed, 28 Apr 2021 06:56:10 +0900 Subject: [PATCH 6/9] bug fix --- src/client/tab.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/tab.rs b/src/client/tab.rs index de9ed4d11d..82db71b4e1 100644 --- a/src/client/tab.rs +++ b/src/client/tab.rs @@ -665,7 +665,7 @@ impl Tab { } } pub fn pane_contains_widechar(&self) -> bool { - self.panes.iter().all(|(_, p)| p.contains_widechar()) + self.panes.iter().any(|(_, p)| p.contains_widechar()) } pub fn render(&mut self) { if self.active_terminal.is_none() { From 98c5ec429ed31dca94972ec857f0e96f7ba2adbb Mon Sep 17 00:00:00 2001 From: Hongjie Zhai Date: Wed, 28 Apr 2021 07:04:59 +0900 Subject: [PATCH 7/9] pane_contains -> panes_contain --- src/client/tab.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/tab.rs b/src/client/tab.rs index 82db71b4e1..594efd7b3d 100644 --- a/src/client/tab.rs +++ b/src/client/tab.rs @@ -664,7 +664,7 @@ impl Tab { pane.set_should_render(true); } } - pub fn pane_contains_widechar(&self) -> bool { + pub fn panes_contain_widechar(&self) -> bool { self.panes.iter().any(|(_, p)| p.contains_widechar()) } pub fn render(&mut self) { @@ -675,7 +675,7 @@ impl Tab { } // if any pane contain widechar, all pane in the same row will messup. We should render them every time // FIXME: remove this when we can handle widechars correctly - if self.pane_contains_widechar() { + if self.panes_contain_widechar() { self.set_force_render() } let mut stdout = self.os_api.get_stdout_writer(); From 6bd2dcf5ae145fbaa6a01cdd4f360be86f905565 Mon Sep 17 00:00:00 2001 From: Hongjie Zhai Date: Thu, 29 Apr 2021 18:20:34 +0900 Subject: [PATCH 8/9] fix conflict --- src/client/tab.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/client/tab.rs b/src/client/tab.rs index 7d7dd5083c..7a37d70646 100644 --- a/src/client/tab.rs +++ b/src/client/tab.rs @@ -698,6 +698,7 @@ impl Tab { for pane in self.panes.values_mut() { pane.set_should_render(true); } + } pub fn is_sync_panes_active(&self) -> bool { self.synchronize_is_active } From 392a7d6c1cb63cfa91fd63e147ddcf325a8bcbe0 Mon Sep 17 00:00:00 2001 From: Aram Drevekenin Date: Fri, 30 Apr 2021 15:09:46 +0200 Subject: [PATCH 9/9] fix(terminal): bring back should_render --- src/client/panes/terminal_pane.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/client/panes/terminal_pane.rs b/src/client/panes/terminal_pane.rs index 898fc5a1c7..a5a957e673 100644 --- a/src/client/panes/terminal_pane.rs +++ b/src/client/panes/terminal_pane.rs @@ -170,14 +170,7 @@ impl Pane for TerminalPane { self.max_width } fn render(&mut self) -> Option { - // FIXME: - // the below conditional is commented out because it causes several bugs: - // 1. When panes are resized or tabs are switched the previous contents of the screen stick - // around - // 2. When there are wide characters in a pane, since we don't yet handle them properly, - // the spill over to the pane to the right - //if self.should_render() { - if true { + if self.should_render() { let mut vte_output = String::new(); let buffer_lines = &self.read_buffer_as_lines(); let display_cols = self.get_columns();