Done updating rendering performance for now. Further changes would require more fundamental work, I'm still not really happy with it as is though. Will probably take a few hours to clean the code though.

Mikayla Maki created

Change summary

crates/terminal/src/connection.rs                  | 52 +++++++----
crates/terminal/src/terminal.rs                    | 10 +
crates/terminal/src/terminal_element.rs            | 68 ++++++++--------
crates/terminal/src/tests/terminal_test_context.rs | 18 ++-
4 files changed, 84 insertions(+), 64 deletions(-)

Detailed changes

crates/terminal/src/connection.rs 🔗

@@ -7,11 +7,11 @@ use alacritty_terminal::{
     event_loop::{EventLoop, Msg, Notifier},
     grid::Scroll,
     index::{Direction, Point},
-    selection::{Selection, SelectionRange, SelectionType},
+    selection::{Selection, SelectionType},
     sync::FairMutex,
-    term::{cell::Cell, RenderableCursor, SizeInfo, TermMode},
+    term::{RenderableContent, SizeInfo, TermMode},
     tty::{self, setup_env},
-    Grid, Term,
+    Term,
 };
 use futures::{
     channel::mpsc::{unbounded, UnboundedSender},
@@ -54,7 +54,7 @@ pub enum TerminalConnection {
     Disconnected {
         directory: Option<PathBuf>,
         shell: Option<Shell>,
-        error: std::io::Error,
+        error: Option<std::io::Error>,
     },
 }
 
@@ -107,15 +107,23 @@ impl TerminalConnection {
                 return TerminalConnection::Disconnected {
                     directory: working_directory,
                     shell,
-                    error,
+                    error: Some(error),
                 };
             }
         };
 
         let shell_txt = {
-            let mut buf = [0; 1024];
-            let pw = alacritty_unix::get_pw_entry(&mut buf).unwrap();
-            pw.shell.to_string()
+            match shell {
+                Some(Shell::System) | None => {
+                    let mut buf = [0; 1024];
+                    let pw = alacritty_unix::get_pw_entry(&mut buf).unwrap();
+                    pw.shell.to_string()
+                }
+                Some(Shell::Program(program)) => program,
+                Some(Shell::WithArguments { program, args }) => {
+                    format!("{} {}", program, args.join(" "))
+                }
+            }
         };
 
         //And connect them together
@@ -292,21 +300,27 @@ impl Terminal {
         self.term.lock().selection = sel;
     }
 
-    pub fn grid(&self) -> Grid<Cell> {
-        let term = self.term.lock();
-        term.grid().clone() //TODO: BAD!!!!!!!!
-    }
+    pub fn render_lock<F, T>(&self, new_size: Option<SizeInfo>, f: F) -> T
+    where
+        F: FnOnce(RenderableContent) -> T,
+    {
+        if let Some(new_size) = new_size {
+            self.pty_tx.0.send(Msg::Resize(new_size)).ok(); //Give the PTY a chance to react to the new size
+                                                            //TODO: Is this bad for performance?
+        }
 
-    pub fn get_display_offset(&self) -> usize {
-        self.term.lock().renderable_content().display_offset
-    }
+        let mut term = self.term.lock(); //Lock
+
+        if let Some(new_size) = new_size {
+            term.resize(new_size); //Reflow
+        }
 
-    pub fn get_selection(&self) -> Option<SelectionRange> {
-        self.term.lock().renderable_content().selection //TODO: BAD!!!!!
+        let content = term.renderable_content();
+        f(content)
     }
 
-    pub fn get_cursor(&self) -> RenderableCursor {
-        self.term.lock().renderable_content().cursor
+    pub fn get_display_offset(&self) -> usize {
+        self.term.lock().renderable_content().display_offset
     }
 
     ///Scroll the terminal

crates/terminal/src/terminal.rs 🔗

@@ -364,10 +364,12 @@ fn get_wd_for_workspace(workspace: &Workspace, cx: &AppContext) -> Option<PathBu
         WorkingDirectory::CurrentProjectDirectory => current_project_directory(workspace, cx),
         WorkingDirectory::FirstProjectDirectory => first_project_directory(workspace, cx),
         WorkingDirectory::AlwaysHome => None,
-        WorkingDirectory::Always { directory } => shellexpand::full(&directory)
-            .ok()
-            .map(|dir| Path::new(&dir.to_string()).to_path_buf())
-            .filter(|dir| dir.is_dir()),
+        WorkingDirectory::Always { directory } => {
+            shellexpand::full(&directory) //TODO handle this better
+                .ok()
+                .map(|dir| Path::new(&dir.to_string()).to_path_buf())
+                .filter(|dir| dir.is_dir())
+        }
     };
     res.or_else(|| home_dir())
 }

crates/terminal/src/terminal_element.rs 🔗

@@ -9,7 +9,6 @@ use alacritty_terminal::{
         cell::{Cell, Flags},
         SizeInfo,
     },
-    Grid,
 };
 use editor::{Cursor, CursorShape, HighlightedRange, HighlightedRangeLine};
 use gpui::{
@@ -282,41 +281,40 @@ impl Element for TerminalEl {
     ) -> (gpui::geometry::vector::Vector2F, Self::LayoutState) {
         let tcx = TerminalLayoutTheme::new(cx.global::<Settings>(), &cx.font_cache());
 
+        //This locks the terminal, so resize it first.
+        //Layout grid cells
+        let cur_size = make_new_size(constraint, &tcx.cell_width, &tcx.line_height);
+
         let terminal = self
             .connection
             .upgrade(cx)
             .unwrap()
             .read(cx)
-            .get_terminal() //TODO!
+            .get_terminal()
             .unwrap();
-        //This locks the terminal, so resize it first.
-        let cur_size = make_new_size(constraint, &tcx.cell_width, &tcx.line_height);
-        terminal.set_size(cur_size);
 
-        let grid = terminal.grid();
-        let selection = terminal.get_selection();
-        let display_offset = terminal.get_display_offset();
-        let cursor = terminal.get_cursor();
-        //Layout grid cells
-
-        let (cells, rects, highlights) = layout_grid(
-            grid.display_iter(),
-            &tcx.text_style,
-            tcx.terminal_theme,
-            cx.text_layout_cache,
-            self.modal,
-            selection,
-        );
-
-        //Layout cursor
-        let cursor = layout_cursor(
-            &grid,
-            cx.text_layout_cache,
-            &tcx,
-            cursor.point,
-            display_offset,
-            constraint,
-        );
+        let (cursor, cells, rects, highlights) = terminal.render_lock(Some(cur_size), |content| {
+            let (cells, rects, highlights) = layout_grid(
+                content.display_iter,
+                &tcx.text_style,
+                tcx.terminal_theme,
+                cx.text_layout_cache,
+                self.modal,
+                content.selection,
+            );
+
+            //Layout cursor
+            let cursor = layout_cursor(
+                // grid,
+                cx.text_layout_cache,
+                &tcx,
+                content.cursor.point,
+                content.display_offset,
+                constraint,
+            );
+
+            (cursor, cells, rects, highlights)
+        });
 
         //Select background color
         let background_color = if self.modal {
@@ -406,6 +404,7 @@ impl Element for TerminalEl {
                 }
             });
 
+            //Draw the text cells
             cx.paint_layer(clip_bounds, |cx| {
                 for cell in &layout.cells {
                     cell.paint(origin, layout, visible_bounds, cx);
@@ -484,15 +483,16 @@ impl Element for TerminalEl {
     }
 }
 
+///TODO: Fix cursor rendering with alacritty fork
 fn layout_cursor(
-    grid: &Grid<Cell>,
+    // grid: &Grid<Cell>,
     text_layout_cache: &TextLayoutCache,
     tcx: &TerminalLayoutTheme,
     cursor_point: Point,
     display_offset: usize,
     constraint: SizeConstraint,
 ) -> Option<Cursor> {
-    let cursor_text = layout_cursor_text(grid, text_layout_cache, tcx);
+    let cursor_text = layout_cursor_text(/*grid,*/ cursor_point, text_layout_cache, tcx);
     get_cursor_shape(
         cursor_point.line.0 as usize,
         cursor_point.column.0 as usize,
@@ -521,12 +521,12 @@ fn layout_cursor(
 }
 
 fn layout_cursor_text(
-    grid: &Grid<Cell>,
+    // grid: &Grid<Cell>,
+    _cursor_point: Point,
     text_layout_cache: &TextLayoutCache,
     tcx: &TerminalLayoutTheme,
 ) -> Line {
-    let cursor_point = grid.cursor.point;
-    let cursor_text = grid[cursor_point.line][cursor_point.column].c.to_string();
+    let cursor_text = " "; //grid[cursor_point.line][cursor_point.column].c.to_string();
 
     text_layout_cache.layout_str(
         &cursor_text,

crates/terminal/src/tests/terminal_test_context.rs 🔗

@@ -61,13 +61,17 @@ impl<'a> TerminalTestContext<'a> {
     }
 
     fn grid_as_str(connection: &TerminalConnection) -> String {
-        let grid = connection.get_terminal().unwrap().grid();
-        let lines = grid.display_iter().group_by(|i| i.point.line.0);
-        lines
-            .into_iter()
-            .map(|(_, line)| line.map(|i| i.c).collect::<String>())
-            .collect::<Vec<String>>()
-            .join("\n")
+        connection
+            .get_terminal()
+            .unwrap()
+            .render_lock(None, |content| {
+                let lines = content.display_iter.group_by(|i| i.point.line.0);
+                lines
+                    .into_iter()
+                    .map(|(_, line)| line.map(|i| i.c).collect::<String>())
+                    .collect::<Vec<String>>()
+                    .join("\n")
+            })
     }
 }