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

Fix(scroll): Prevent scroll buffer overwrite #655

Merged
merged 4 commits into from
Oct 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions zellij-server/src/panes/grid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ pub struct Grid {
pub pending_messages_to_pty: Vec<Vec<u8>>,
pub selection: Selection,
pub title: Option<String>,
pub is_scrolled: bool,
}

impl Debug for Grid {
Expand Down Expand Up @@ -405,6 +406,7 @@ impl Grid {
title_stack: vec![],
title: None,
changed_colors: None,
is_scrolled: false,
}
}
pub fn render_full_viewport(&mut self) {
Expand Down Expand Up @@ -537,6 +539,7 @@ impl Grid {
}
pub fn scroll_up_one_line(&mut self) {
if !self.lines_above.is_empty() && self.viewport.len() == self.height {
self.is_scrolled = true;
let line_to_push_down = self.viewport.pop().unwrap();
self.lines_below.insert(0, line_to_push_down);

Expand Down Expand Up @@ -572,6 +575,9 @@ impl Grid {
self.selection.move_up(1);
self.output_buffer.update_all_lines();
}
if self.lines_below.is_empty() {
self.is_scrolled = false;
}
}
fn force_change_size(&mut self, new_rows: usize, new_columns: usize) {
// this is an ugly hack - it's here because sometimes we need to change_size to the
Expand Down
3 changes: 3 additions & 0 deletions zellij-server/src/panes/plugin_pane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,9 @@ impl Pane for PluginPane {
fn clear_scroll(&mut self) {
//unimplemented!()
}
fn is_scrolled(&self) -> bool {
false
}

fn active_at(&self) -> Instant {
self.active_at
Expand Down
3 changes: 3 additions & 0 deletions zellij-server/src/panes/terminal_pane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,9 @@ impl Pane for TerminalPane {
self.grid.reset_viewport();
self.set_should_render(true);
}
fn is_scrolled(&self) -> bool {
self.grid.is_scrolled
}

fn active_at(&self) -> Instant {
self.active_at
Expand Down
19 changes: 17 additions & 2 deletions zellij-server/src/screen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::{
wasm_vm::PluginInstruction,
ServerInstruction, SessionState,
};
use zellij_tile::data::{Event, ModeInfo, Palette, PluginCapabilities, TabInfo};
use zellij_tile::data::{Event, InputMode, ModeInfo, Palette, PluginCapabilities, TabInfo};
use zellij_utils::{
errors::{ContextType, ScreenContext},
input::{get_mode_info, options::Options},
Expand Down Expand Up @@ -205,6 +205,12 @@ impl Screen {
new_tab.visible(true);

let old_active_index = self.active_tab_index.replace(new_tab_index);
if let Some(ref i) = old_active_index {
if let Some(t) = self.tabs.get_mut(i) {
t.is_active = false;
}
}
self.tabs.get_mut(&new_tab_index).unwrap().is_active = true;
self.tab_history.retain(|&e| e != Some(new_tab_pos));
self.tab_history.push(old_active_index);

Expand Down Expand Up @@ -264,6 +270,7 @@ impl Screen {
for t in self.tabs.values_mut() {
if t.index == self.active_tab_index.unwrap() {
t.set_force_render();
t.is_active = true;
t.visible(true);
}
if t.position > active_tab.position {
Expand Down Expand Up @@ -351,8 +358,9 @@ impl Screen {
self.draw_pane_frames,
);
tab.apply_layout(layout, new_pids, tab_index);
if let Some(active_tab) = self.get_active_tab() {
if let Some(active_tab) = self.get_active_tab_mut() {
active_tab.visible(false);
active_tab.is_active = false;
}
self.tab_history
.push(self.active_tab_index.replace(tab_index));
Expand Down Expand Up @@ -398,6 +406,13 @@ impl Screen {
self.update_tabs();
}
pub fn change_mode(&mut self, mode_info: ModeInfo) {
if self.mode_info.mode == InputMode::Scroll
&& (mode_info.mode == InputMode::Normal || mode_info.mode == InputMode::Locked)
{
self.get_active_tab_mut()
.unwrap()
.clear_active_terminal_scroll();
}
self.colors = mode_info.palette;
self.mode_info = mode_info;
for tab in self.tabs.values_mut() {
Expand Down
61 changes: 58 additions & 3 deletions zellij-server/src/tab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::sync::{mpsc::channel, Arc, RwLock};
use std::time::Instant;
use std::{
cmp::Reverse,
collections::{BTreeMap, HashSet},
collections::{BTreeMap, HashMap, HashSet},
};
use zellij_tile::data::{Event, InputMode, ModeInfo, Palette, PaletteColor};
use zellij_utils::input::layout::Direction;
Expand All @@ -39,6 +39,9 @@ const MIN_TERMINAL_WIDTH: usize = 5;

const RESIZE_PERCENT: f64 = 5.0;

const MAX_PENDING_VTE_EVENTS: usize = 7000;
const PENDING_EVENTS_SET_SIZE: usize = 200;

type BorderAndPaneIds = (usize, Vec<PaneId>);

fn split(direction: Direction, rect: &PaneGeom) -> Option<(PaneGeom, PaneGeom)> {
Expand Down Expand Up @@ -112,7 +115,9 @@ pub(crate) struct Tab {
session_state: Arc<RwLock<SessionState>>,
pub mode_info: ModeInfo,
pub colors: Palette,
pub is_active: bool,
draw_pane_frames: bool,
pending_vte_events: HashMap<RawFd, Vec<VteBytes>>,
}

#[derive(Clone, Debug, Default, Serialize, Deserialize)]
Expand Down Expand Up @@ -162,6 +167,7 @@ pub trait Pane {
fn scroll_up(&mut self, count: usize);
fn scroll_down(&mut self, count: usize);
fn clear_scroll(&mut self);
fn is_scrolled(&self) -> bool;
fn active_at(&self) -> Instant;
fn set_active_at(&mut self, instant: Instant);
fn set_frame(&mut self, frame: bool);
Expand Down Expand Up @@ -292,6 +298,8 @@ impl Tab {
colors,
session_state,
draw_pane_frames,
pending_vte_events: HashMap::new(),
is_active: true,
}
}

Expand Down Expand Up @@ -542,6 +550,44 @@ impl Tab {
self.panes.contains_key(&PaneId::Terminal(pid))
}
pub fn handle_pty_bytes(&mut self, pid: RawFd, bytes: VteBytes) {
if let Some(terminal_output) = self.panes.get_mut(&PaneId::Terminal(pid)) {
// If the pane is scrolled buffer the vte events
if terminal_output.is_scrolled() {
self.pending_vte_events.entry(pid).or_default().push(bytes);
if let Some(evs) = self.pending_vte_events.get(&pid) {
// Reset scroll and play the pending events if the buufer size exceeds the limit
if evs.len() >= MAX_PENDING_VTE_EVENTS {
terminal_output.clear_scroll();
self.play_pending_vte_events(pid);
}
}
return;
}
}
self.process_pty_bytes(pid, bytes);
}
fn play_pending_vte_events(&mut self, pid: RawFd) {
if self.pending_vte_events.get(&pid).is_some() {
if let Some(terminal_output) = self.panes.get_mut(&PaneId::Terminal(pid)) {
if terminal_output.is_scrolled() {
return;
}
}
let mut events = self.pending_vte_events.remove(&pid).unwrap();
while events.len() >= PENDING_EVENTS_SET_SIZE {
events
.drain(..PENDING_EVENTS_SET_SIZE)
.for_each(|bytes| self.process_pty_bytes(pid, bytes));
// Render at regular intervals
self.render();
}
events
.drain(..)
.for_each(|bytes| self.process_pty_bytes(pid, bytes));
self.render();
}
}
fn process_pty_bytes(&mut self, pid: RawFd, bytes: VteBytes) {
// if we don't have the terminal in self.terminals it's probably because
// of a race condition where the terminal was created in pty but has not
// yet been created in Screen. These events are currently not buffered, so
Expand All @@ -553,7 +599,6 @@ impl Tab {
for message in messages_to_pty {
self.write_to_pane_id(message, PaneId::Terminal(pid));
}
// self.render();
}
}
pub fn write_to_terminals_on_current_tab(&mut self, input_bytes: Vec<u8>) {
Expand Down Expand Up @@ -728,7 +773,10 @@ impl Tab {
}
}
pub fn render(&mut self) {
if self.active_terminal.is_none() || self.session_state.read().unwrap().clients.is_empty() {
if self.active_terminal.is_none()
|| self.session_state.read().unwrap().clients.is_empty()
|| !self.is_active
{
// 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
// or if there are no attached clients to this session
Expand Down Expand Up @@ -2218,6 +2266,7 @@ impl Tab {
.get_mut(&PaneId::Terminal(active_terminal_id))
.unwrap();
active_terminal.scroll_down(1);
self.play_pending_vte_events(active_terminal_id);
self.render();
}
}
Expand All @@ -2242,6 +2291,7 @@ impl Tab {
// prevent overflow when row == 0
let scroll_columns = active_terminal.rows().max(1) - 1;
active_terminal.scroll_down(scroll_columns);
self.play_pending_vte_events(active_terminal_id);
self.render();
}
}
Expand All @@ -2252,6 +2302,7 @@ impl Tab {
.get_mut(&PaneId::Terminal(active_terminal_id))
.unwrap();
active_terminal.clear_scroll();
self.play_pending_vte_events(active_terminal_id);
self.render();
}
}
Expand All @@ -2262,6 +2313,7 @@ impl Tab {
.get_mut(&PaneId::Terminal(active_terminal_id))
.unwrap();
active_terminal.clear_scroll();
self.play_pending_vte_events(active_terminal_id);
}
}
pub fn scroll_terminal_up(&mut self, point: &Position, lines: usize) {
Expand All @@ -2273,6 +2325,9 @@ impl Tab {
pub fn scroll_terminal_down(&mut self, point: &Position, lines: usize) {
if let Some(pane) = self.get_pane_at(point) {
pane.scroll_down(lines);
if let PaneId::Terminal(id) = pane.pid() {
self.play_pending_vte_events(id);
}
self.render();
}
}
Expand Down