Improve handling of large output in embedded terminals (#32416)

#31922 made embedded terminals automatically grow to fit the content. We
since found some issues with large output which this PR addresses by:

- Only shaping / laying out lines that are visible in the viewport
(based on `window.content_mask`)
- Falling back to embedded scrolling after 1K lines. The perf fix above
actually makes it possible to handle a lot of lines, but:
- Alacrity uses a `u16` for rows internally, so we needed a limit to
prevent overflow.
- Scrolling through thousands of lines to get to the other side of a
terminal tool call isn't great UX, so we might as well set the limit
low.
- We can consider raising the limit when we make card headers sticky.

Release Notes:

- Agent: Improve handling of large terminal output

crates/assistant_tools/src/terminal_tool.rs

@@ -638,29 +638,36 @@ impl ToolCard for TerminalToolCard {
                             .bg(cx.theme().colors().editor_background)
                             .rounded_b_md()
                             .text_ui_sm(cx)
-                            .child(
+                            .child({
-                                ToolOutputPreview::new(
+                                let content_mode = terminal.read(cx).content_mode(window, cx);
-                                    terminal.clone().into_any_element(),
+
-                                    terminal.entity_id(),
+                                if content_mode.is_scrollable() {
-                                )
+                                    div().h_72().child(terminal.clone()).into_any_element()
-                                .with_total_lines(self.content_line_count)
+                                } else {
-                                .toggle_state(!terminal.read(cx).is_content_limited(window))
+                                    ToolOutputPreview::new(
-                                .on_toggle({
+                                        terminal.clone().into_any_element(),
-                                    let terminal = terminal.clone();
+                                        terminal.entity_id(),
-                                    move |is_expanded, _, cx| {
+                                    )
-                                        terminal.update(cx, |terminal, cx| {
+                                    .with_total_lines(self.content_line_count)
-                                            terminal.set_embedded_mode(
+                                    .toggle_state(!content_mode.is_limited())
-                                                if is_expanded {
+                                    .on_toggle({
-                                                    None
+                                        let terminal = terminal.clone();
-                                                } else {
+                                        move |is_expanded, _, cx| {
-                                                    Some(COLLAPSED_LINES)
+                                            terminal.update(cx, |terminal, cx| {
-                                                },
+                                                terminal.set_embedded_mode(
-                                                cx,
+                                                    if is_expanded {
-                                            );
+                                                        None
-                                        });
+                                                    } else {
-                                    }
+                                                        Some(COLLAPSED_LINES)
-                                }),
+                                                    },
-                            ),
+                                                    cx,
+                                                );
+                                            });
+                                        }
+                                    })
+                                    .into_any_element()
+                                }
+                            }),
                     )
                 },
             )

crates/repl/src/outputs/plain.rs

@@ -258,7 +258,7 @@ impl Render for TerminalOutput {
                 cell: ic.cell.clone(),
             });
         let (cells, rects) =
-            TerminalElement::layout_grid(grid, &text_style, text_system, None, window, cx);
+            TerminalElement::layout_grid(grid, 0, &text_style, text_system, None, window, cx);
 
         // lines are 0-indexed, so we must add 1 to get the number of lines
         let text_line_height = text_style.line_height_in_pixels(window.rem_size());

crates/terminal_view/src/terminal_element.rs

@@ -2,7 +2,7 @@ use editor::{CursorLayout, HighlightedRange, HighlightedRangeLine};
 use gpui::{
     AnyElement, App, AvailableSpace, Bounds, ContentMask, Context, DispatchPhase, Element,
     ElementId, Entity, FocusHandle, Font, FontStyle, FontWeight, GlobalElementId, HighlightStyle,
-    Hitbox, Hsla, InputHandler, InteractiveElement, Interactivity, IntoElement, LayoutId,
+    Hitbox, Hsla, InputHandler, InteractiveElement, Interactivity, IntoElement, LayoutId, Length,
     ModifiersChangedEvent, MouseButton, MouseMoveEvent, Pixels, Point, ShapedLine,
     StatefulInteractiveElement, StrikethroughStyle, Styled, TextRun, TextStyle, UTF16Selection,
     UnderlineStyle, WeakEntity, WhiteSpace, Window, WindowTextSystem, div, fill, point, px,
@@ -32,7 +32,7 @@ use workspace::Workspace;
 use std::mem;
 use std::{fmt::Debug, ops::RangeInclusive, rc::Rc};
 
-use crate::{BlockContext, BlockProperties, TerminalMode, TerminalView};
+use crate::{BlockContext, BlockProperties, ContentMode, TerminalMode, TerminalView};
 
 /// The information generated during layout that is necessary for painting.
 pub struct LayoutState {
@@ -49,6 +49,7 @@ pub struct LayoutState {
     gutter: Pixels,
     block_below_cursor_element: Option<AnyElement>,
     base_text_style: TextStyle,
+    content_mode: ContentMode,
 }
 
 /// Helper struct for converting data between Alacritty's cursor points, and displayed cursor points.
@@ -202,6 +203,7 @@ impl TerminalElement {
 
     pub fn layout_grid(
         grid: impl Iterator<Item = IndexedCell>,
+        start_line_offset: i32,
         text_style: &TextStyle,
         // terminal_theme: &TerminalStyle,
         text_system: &WindowTextSystem,
@@ -218,6 +220,8 @@ impl TerminalElement {
 
         let linegroups = grid.into_iter().chunk_by(|i| i.point.line);
         for (line_index, (_, line)) in linegroups.into_iter().enumerate() {
+            let alac_line = start_line_offset + line_index as i32;
+
             for cell in line {
                 let mut fg = cell.fg;
                 let mut bg = cell.bg;
@@ -245,7 +249,7 @@ impl TerminalElement {
                                         || {
                                             Some(LayoutRect::new(
                                                 AlacPoint::new(
-                                                    line_index as i32,
+                                                    alac_line,
                                                     cell.point.column.0 as i32,
                                                 ),
                                                 1,
@@ -260,10 +264,7 @@ impl TerminalElement {
                                         rects.push(cur_rect.take().unwrap());
                                     }
                                     cur_rect = Some(LayoutRect::new(
-                                        AlacPoint::new(
+                                        AlacPoint::new(alac_line, cell.point.column.0 as i32),
-                                            line_index as i32,
-                                            cell.point.column.0 as i32,
-                                        ),
                                         1,
                                         convert_color(&bg, theme),
                                     ));
@@ -272,7 +273,7 @@ impl TerminalElement {
                             None => {
                                 cur_alac_color = Some(bg);
                                 cur_rect = Some(LayoutRect::new(
-                                    AlacPoint::new(line_index as i32, cell.point.column.0 as i32),
+                                    AlacPoint::new(alac_line, cell.point.column.0 as i32),
                                     1,
                                     convert_color(&bg, theme),
                                 ));
@@ -295,7 +296,7 @@ impl TerminalElement {
                         );
 
                         cells.push(LayoutCell::new(
-                            AlacPoint::new(line_index as i32, cell.point.column.0 as i32),
+                            AlacPoint::new(alac_line, cell.point.column.0 as i32),
                             layout_cell,
                         ))
                     };
@@ -430,7 +431,13 @@ impl TerminalElement {
         }
     }
 
-    fn register_mouse_listeners(&mut self, mode: TermMode, hitbox: &Hitbox, window: &mut Window) {
+    fn register_mouse_listeners(
+        &mut self,
+        mode: TermMode,
+        hitbox: &Hitbox,
+        content_mode: &ContentMode,
+        window: &mut Window,
+    ) {
         let focus = self.focus.clone();
         let terminal = self.terminal.clone();
         let terminal_view = self.terminal_view.clone();
@@ -512,14 +519,18 @@ impl TerminalElement {
             ),
         );
 
-        if !matches!(self.mode, TerminalMode::Embedded { .. }) {
+        if content_mode.is_scrollable() {
             self.interactivity.on_scroll_wheel({
                 let terminal_view = self.terminal_view.downgrade();
-                move |e, _window, cx| {
+                move |e, window, cx| {
                     terminal_view
                         .update(cx, |terminal_view, cx| {
-                            terminal_view.scroll_wheel(e, cx);
+                            if matches!(terminal_view.mode, TerminalMode::Standalone)
-                            cx.notify();
+                                || terminal_view.focus_handle.is_focused(window)
+                            {
+                                terminal_view.scroll_wheel(e, cx);
+                                cx.notify();
+                            }
                         })
                         .ok();
                 }
@@ -605,6 +616,32 @@ impl Element for TerminalElement {
         window: &mut Window,
         cx: &mut App,
     ) -> (LayoutId, Self::RequestLayoutState) {
+        let height: Length = match self.terminal_view.read(cx).content_mode(window, cx) {
+            ContentMode::Inline {
+                displayed_lines,
+                total_lines: _,
+            } => {
+                let rem_size = window.rem_size();
+                let line_height = window.text_style().font_size.to_pixels(rem_size)
+                    * TerminalSettings::get_global(cx)
+                        .line_height
+                        .value()
+                        .to_pixels(rem_size)
+                        .0;
+                (displayed_lines * line_height).into()
+            }
+            ContentMode::Scrollable => {
+                if let TerminalMode::Embedded { .. } = &self.mode {
+                    let term = self.terminal.read(cx);
+                    if !term.scrolled_to_top() && !term.scrolled_to_bottom() && self.focused {
+                        self.interactivity.occlude_mouse();
+                    }
+                }
+
+                relative(1.).into()
+            }
+        };
+
         let layout_id = self.interactivity.request_layout(
             global_id,
             inspector_id,
@@ -612,29 +649,7 @@ impl Element for TerminalElement {
             cx,
             |mut style, window, cx| {
                 style.size.width = relative(1.).into();
-
+                style.size.height = height;
-                match &self.mode {
-                    TerminalMode::Scrollable => {
-                        style.size.height = relative(1.).into();
-                    }
-                    TerminalMode::Embedded { max_lines } => {
-                        let rem_size = window.rem_size();
-                        let line_height = window.text_style().font_size.to_pixels(rem_size)
-                            * TerminalSettings::get_global(cx)
-                                .line_height
-                                .value()
-                                .to_pixels(rem_size)
-                                .0;
-
-                        let mut line_count = self.terminal.read(cx).total_lines();
-                        if !self.focused {
-                            if let Some(max_lines) = max_lines {
-                                line_count = line_count.min(*max_lines);
-                            }
-                        }
-                        style.size.height = (line_count * line_height).into();
-                    }
-                }
 
                 window.request_layout(style, None, cx)
             },
@@ -693,7 +708,7 @@ impl Element for TerminalElement {
                     TerminalMode::Embedded { .. } => {
                         window.text_style().font_size.to_pixels(window.rem_size())
                     }
-                    TerminalMode::Scrollable => terminal_settings
+                    TerminalMode::Standalone => terminal_settings
                         .font_size
                         .map_or(buffer_font_size, |size| theme::adjusted_font_size(size, cx)),
                 };
@@ -733,7 +748,7 @@ impl Element for TerminalElement {
                 let player_color = theme.players().local();
                 let match_color = theme.colors().search_match_background;
                 let gutter;
-                let dimensions = {
+                let (dimensions, line_height_px) = {
                     let rem_size = window.rem_size();
                     let font_pixels = text_style.font_size.to_pixels(rem_size);
                     // TODO: line_height should be an f32 not an AbsoluteLength.
@@ -759,7 +774,10 @@ impl Element for TerminalElement {
                     let mut origin = bounds.origin;
                     origin.x += gutter;
 
-                    TerminalBounds::new(line_height, cell_width, Bounds { origin, size })
+                    (
+                        TerminalBounds::new(line_height, cell_width, Bounds { origin, size }),
+                        line_height,
+                    )
                 };
 
                 let search_matches = self.terminal.read(cx).matches.clone();
@@ -827,16 +845,42 @@ impl Element for TerminalElement {
 
                 // then have that representation be converted to the appropriate highlight data structure
 
-                let (cells, rects) = TerminalElement::layout_grid(
+                let content_mode = self.terminal_view.read(cx).content_mode(window, cx);
-                    cells.iter().cloned(),
+                let (cells, rects) = match content_mode {
-                    &text_style,
+                    ContentMode::Scrollable => TerminalElement::layout_grid(
-                    window.text_system(),
+                        cells.iter().cloned(),
-                    last_hovered_word
+                        0,
-                        .as_ref()
+                        &text_style,
-                        .map(|last_hovered_word| (link_style, &last_hovered_word.word_match)),
+                        window.text_system(),
-                    window,
+                        last_hovered_word
-                    cx,
+                            .as_ref()
-                );
+                            .map(|last_hovered_word| (link_style, &last_hovered_word.word_match)),
+                        window,
+                        cx,
+                    ),
+                    ContentMode::Inline { .. } => {
+                        let intersection = window.content_mask().bounds.intersect(&bounds);
+                        let start_row = (intersection.top() - bounds.top()) / line_height_px;
+                        let end_row = start_row + intersection.size.height / line_height_px;
+                        let line_range = (start_row as i32)..=(end_row as i32);
+
+                        TerminalElement::layout_grid(
+                            cells
+                                .iter()
+                                .skip_while(|i| &i.point.line < line_range.start())
+                                .take_while(|i| &i.point.line <= line_range.end())
+                                .cloned(),
+                            *line_range.start(),
+                            &text_style,
+                            window.text_system(),
+                            last_hovered_word.as_ref().map(|last_hovered_word| {
+                                (link_style, &last_hovered_word.word_match)
+                            }),
+                            window,
+                            cx,
+                        )
+                    }
+                };
 
                 // Layout cursor. Rectangle is used for IME, so we should lay it out even
                 // if we don't end up showing it.
@@ -932,6 +976,7 @@ impl Element for TerminalElement {
                     gutter,
                     block_below_cursor_element,
                     base_text_style: text_style,
+                    content_mode,
                 }
             },
         )
@@ -969,7 +1014,12 @@ impl Element for TerminalElement {
                 workspace: self.workspace.clone(),
             };
 
-            self.register_mouse_listeners(layout.mode, &layout.hitbox, window);
+            self.register_mouse_listeners(
+                layout.mode,
+                &layout.hitbox,
+                &layout.content_mode,
+                window,
+            );
             if window.modifiers().secondary()
                 && bounds.contains(&window.mouse_position())
                 && self.terminal_view.read(cx).hover.is_some()

crates/terminal_view/src/terminal_view.rs

@@ -140,12 +140,37 @@ pub struct TerminalView {
 #[derive(Default, Clone)]
 pub enum TerminalMode {
     #[default]
-    Scrollable,
+    Standalone,
     Embedded {
-        max_lines: Option<usize>,
+        max_lines_when_unfocused: Option<usize>,
     },
 }
 
+#[derive(Clone)]
+pub enum ContentMode {
+    Scrollable,
+    Inline {
+        displayed_lines: usize,
+        total_lines: usize,
+    },
+}
+
+impl ContentMode {
+    pub fn is_limited(&self) -> bool {
+        match self {
+            ContentMode::Scrollable => false,
+            ContentMode::Inline {
+                displayed_lines,
+                total_lines,
+            } => displayed_lines < total_lines,
+        }
+    }
+
+    pub fn is_scrollable(&self) -> bool {
+        matches!(self, ContentMode::Scrollable)
+    }
+}
+
 #[derive(Debug)]
 struct HoverTarget {
     tooltip: String,
@@ -223,7 +248,7 @@ impl TerminalView {
             blink_epoch: 0,
             hover: None,
             hover_tooltip_update: Task::ready(()),
-            mode: TerminalMode::Scrollable,
+            mode: TerminalMode::Standalone,
             workspace_id,
             show_breadcrumbs: TerminalSettings::get_global(cx).toolbar.breadcrumbs,
             block_below_cursor: None,
@@ -245,16 +270,46 @@ impl TerminalView {
     }
 
     /// Enable 'embedded' mode where the terminal displays the full content with an optional limit of lines.
-    pub fn set_embedded_mode(&mut self, max_lines: Option<usize>, cx: &mut Context<Self>) {
+    pub fn set_embedded_mode(
-        self.mode = TerminalMode::Embedded { max_lines };
+        &mut self,
+        max_lines_when_unfocused: Option<usize>,
+        cx: &mut Context<Self>,
+    ) {
+        self.mode = TerminalMode::Embedded {
+            max_lines_when_unfocused,
+        };
         cx.notify();
     }
 
-    pub fn is_content_limited(&self, window: &Window) -> bool {
+    const MAX_EMBEDDED_LINES: usize = 1_000;
+
+    /// Returns the current `ContentMode` depending on the set `TerminalMode` and the current number of lines
+    ///
+    /// Note: Even in embedded mode, the terminal will fallback to scrollable when its content exceeds `MAX_EMBEDDED_LINES`
+    pub fn content_mode(&self, window: &Window, cx: &App) -> ContentMode {
         match &self.mode {
-            TerminalMode::Scrollable => false,
+            TerminalMode::Standalone => ContentMode::Scrollable,
-            TerminalMode::Embedded { max_lines } => {
+            TerminalMode::Embedded {
-                !self.focus_handle.is_focused(window) && max_lines.is_some()
+                max_lines_when_unfocused,
+            } => {
+                let total_lines = self.terminal.read(cx).total_lines();
+
+                if total_lines > Self::MAX_EMBEDDED_LINES {
+                    ContentMode::Scrollable
+                } else {
+                    let mut displayed_lines = total_lines;
+
+                    if !self.focus_handle.is_focused(window) {
+                        if let Some(max_lines) = max_lines_when_unfocused {
+                            displayed_lines = displayed_lines.min(*max_lines)
+                        }
+                    }
+
+                    ContentMode::Inline {
+                        displayed_lines,
+                        total_lines,
+                    }
+                }
             }
         }
     }
@@ -840,10 +895,10 @@ impl TerminalView {
         }))
     }
 
-    fn render_scrollbar(&self, cx: &mut Context<Self>) -> Option<Stateful<Div>> {
+    fn render_scrollbar(&self, window: &Window, cx: &mut Context<Self>) -> Option<Stateful<Div>> {
         if !Self::should_show_scrollbar(cx)
             || !(self.show_scrollbar || self.scrollbar_state.is_dragging())
-            || matches!(self.mode, TerminalMode::Embedded { .. })
+            || !self.content_mode(window, cx).is_scrollable()
         {
             return None;
         }
@@ -1493,7 +1548,7 @@ impl Render for TerminalView {
                         self.block_below_cursor.clone(),
                         self.mode.clone(),
                     ))
-                    .when_some(self.render_scrollbar(cx), |div, scrollbar| {
+                    .when_some(self.render_scrollbar(window, cx), |div, scrollbar| {
                         div.child(scrollbar)
                     }),
             )