Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Toward enable should_render (fix pane render) #318

Merged
merged 11 commits into from
Apr 30, 2021
6 changes: 6 additions & 0 deletions src/client/panes/grid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
Expand Down
3 changes: 3 additions & 0 deletions src/client/panes/plugin_pane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ impl Pane for PluginPane {
fn position_and_size_override(&self) -> Option<PositionAndSize> {
self.position_and_size_override
}
fn contains_widechar(&self) -> bool {
false
}
fn should_render(&self) -> bool {
self.should_render
}
Expand Down
12 changes: 12 additions & 0 deletions src/client/panes/terminal_character.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use unicode_width::UnicodeWidthChar;

use crate::utils::logging::debug_log_to_file;
use ::std::fmt::{self, Debug, Display, Formatter};

Expand Down Expand Up @@ -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
}
}
22 changes: 10 additions & 12 deletions src/client/panes/terminal_pane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,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)
Expand Down Expand Up @@ -138,6 +139,9 @@ impl Pane for TerminalPane {
fn position_and_size_override(&self) -> Option<PositionAndSize> {
self.position_and_size_override
}
fn contains_widechar(&self) -> bool {
self.grid.contains_widechar()
}
fn should_render(&self) -> bool {
self.grid.should_render
}
Expand Down Expand Up @@ -166,14 +170,7 @@ impl Pane for TerminalPane {
self.max_width
}
fn render(&mut self) -> Option<String> {
// 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 || cfg!(test) {
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();
Expand Down Expand Up @@ -215,7 +212,7 @@ impl Pane for TerminalPane {
}
character_styles.clear();
}
self.grid.should_render = false;
self.set_should_render(false);
Some(vte_output)
} else {
None
Expand Down Expand Up @@ -274,15 +271,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);
}

fn active_at(&self) -> Instant {
Expand Down Expand Up @@ -337,6 +334,7 @@ impl TerminalPane {
let rows = self.get_rows();
let columns = self.get_columns();
self.grid.change_size(rows, columns);
self.set_should_render(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Elegant!

}
pub fn read_buffer_as_lines(&self) -> Vec<Vec<TerminalCharacter>> {
self.grid.as_character_lines()
Expand Down
16 changes: 16 additions & 0 deletions src/client/tab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ pub trait Pane {
fn position_and_size_override(&self) -> Option<PositionAndSize>;
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);
Expand Down Expand Up @@ -694,18 +697,31 @@ 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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. I would have gone with something on the state of Tab that bypasses the should_render of each pane, but the more I think about it, the more I like this solution.

for pane in self.panes.values_mut() {
pane.set_should_render(true);
}
}
pub fn is_sync_panes_active(&self) -> bool {
self.synchronize_is_active
}
pub fn toggle_sync_panes_is_active(&mut self) {
self.synchronize_is_active = !self.synchronize_is_active;
}
pub fn panes_contain_widechar(&self) -> bool {
self.panes.iter().any(|(_, 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.panes_contain_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,
Expand Down
14 changes: 9 additions & 5 deletions src/common/screen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,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;
}
Expand All @@ -169,8 +170,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;
}
Expand All @@ -181,9 +183,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();
Expand Down Expand Up @@ -227,6 +230,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();
}

Expand Down