Merge pull request #2037 from ghostty-org/extend

renderer: disable window-padding-color=extend in certain scenarios

pkg/opengl/Program.zig

@@ -104,6 +104,7 @@ pub fn setUniform(
 
     // Perform the correct call depending on the type of the value.
     switch (@TypeOf(value)) {
+        bool => glad.context.Uniform1i.?(loc, if (value) 1 else 0),
         comptime_int => glad.context.Uniform1i.?(loc, value),
         f32 => glad.context.Uniform1f.?(loc, value),
         @Vector(2, f32) => glad.context.Uniform2f.?(loc, value[0], value[1]),

src/config/Config.zig

@@ -674,6 +674,17 @@ keybind: Keybinds = .{},
 /// * `background` - The background color specified in `background`.
 /// * `extend` - Extend the background color of the nearest grid cell.
 ///
+/// The "extend" value will be disabled in certain scenarios. On primary
+/// screen applications (i.e. not something like Neovim), the color will not
+/// be extended vertically if any of the following are true:
+///
+/// * The nearest row has any cells that have the default backgroudn color.
+///   The thinking is that in this case, the default background color looks
+///   fine as a padding color.
+/// * The nearest row is a prompt row (requires shell integration). The
+///   thinking here is that prompts often contain powerline glyphs that
+///   do not look good extended.
+///
 /// The default value is "extend". This allows for smooth resizing of a
 /// terminal grid without having visible empty areas around the edge. The edge
 /// cells may appear slightly larger due to the extension.

src/renderer/Metal.zig

@@ -625,6 +625,8 @@ pub fn init(alloc: Allocator, options: renderer.Options) !Metal {
             .cell_size = undefined,
             .grid_size = undefined,
             .grid_padding = undefined,
+            .padding_extend_top = true,
+            .padding_extend_bottom = true,
             .min_contrast = options.config.min_contrast,
             .cursor_pos = .{ std.math.maxInt(u16), std.math.maxInt(u16) },
             .cursor_color = undefined,
@@ -872,6 +874,7 @@ pub fn updateFrame(
     const Critical = struct {
         bg: terminal.color.RGB,
         screen: terminal.Screen,
+        screen_type: terminal.ScreenType,
         mouse: renderer.State.Mouse,
         preedit: ?renderer.State.Preedit,
         cursor_style: ?renderer.CursorStyle,
@@ -1001,6 +1004,7 @@ pub fn updateFrame(
         break :critical .{
             .bg = self.background_color,
             .screen = screen_copy,
+            .screen_type = state.terminal.active_screen,
             .mouse = state.mouse,
             .preedit = preedit,
             .cursor_style = cursor_style,
@@ -1018,6 +1022,7 @@ pub fn updateFrame(
     try self.rebuildCells(
         critical.full_rebuild,
         &critical.screen,
+        critical.screen_type,
         critical.mouse,
         critical.preedit,
         critical.cursor_style,
@@ -1988,6 +1993,8 @@ pub fn setScreenSize(
             @floatFromInt(blank.bottom),
             @floatFromInt(blank.left),
         },
+        .padding_extend_top = old.padding_extend_top,
+        .padding_extend_bottom = old.padding_extend_bottom,
         .min_contrast = old.min_contrast,
         .cursor_pos = old.cursor_pos,
         .cursor_color = old.cursor_color,
@@ -2085,6 +2092,7 @@ fn rebuildCells(
     self: *Metal,
     rebuild: bool,
     screen: *terminal.Screen,
+    screen_type: terminal.ScreenType,
     mouse: renderer.State.Mouse,
     preedit: ?renderer.State.Preedit,
     cursor_style_: ?renderer.CursorStyle,
@@ -2126,8 +2134,14 @@ fn rebuildCells(
         };
     } else null;
 
-    // If we are doing a full rebuild, then we clear the entire cell buffer.
+    if (rebuild) {
-    if (rebuild) self.cells.reset();
+        // If we are doing a full rebuild, then we clear the entire cell buffer.
+        self.cells.reset();
+
+        // We also reset our padding extension depending on the screen type
+        self.uniforms.padding_extend_top = screen_type == .alternate;
+        self.uniforms.padding_extend_bottom = screen_type == .alternate;
+    }
 
     // Go row-by-row to build the cells. We go row by row because we do
     // font shaping by row. In the future, we will also do dirty tracking
@@ -2159,6 +2173,16 @@ fn rebuildCells(
             break :sel sel.containedRow(screen, pin) orelse null;
         };
 
+        // On primary screen, we still apply vertical padding extension
+        // under certain conditions we feel are safe. This helps make some
+        // scenarios look better while avoiding scenarios we know do NOT look
+        // good.
+        if (y == 0 and screen_type == .primary) {
+            self.uniforms.padding_extend_top = !row.neverExtendBg();
+        } else if (y == self.cells.size.rows - 1 and screen_type == .primary) {
+            self.uniforms.padding_extend_bottom = !row.neverExtendBg();
+        }
+
         // Split our row into runs and shape each one.
         var iter = self.font_shaper.runIterator(
             self.font_grid,

src/renderer/OpenGL.zig

@@ -53,12 +53,19 @@ grid_metrics: font.face.Metrics,
 /// Current screen size dimensions for this grid. This is set on the first
 /// resize event, and is not immediately available.
 screen_size: ?renderer.ScreenSize,
+grid_size: renderer.GridSize,
 
 /// The current set of cells to render. Each set of cells goes into
 /// a separate shader call.
 cells_bg: std.ArrayListUnmanaged(CellProgram.Cell),
 cells: std.ArrayListUnmanaged(CellProgram.Cell),
 
+/// The last viewport that we based our rebuild off of. If this changes,
+/// then we do a full rebuild of the cells. The pointer values in this pin
+/// are NOT SAFE to read because they may be modified, freed, etc from the
+/// termio thread. We treat the pointers as integers for comparison only.
+cells_viewport: ?terminal.Pin = null,
+
 /// The size of the cells list that was sent to the GPU. This is used
 /// to detect when the cells array was reallocated/resized and handle that
 /// accordingly.
@@ -120,6 +127,10 @@ draw_mutex: DrawMutex = drawMutexZero,
 /// terminal is in reversed mode.
 draw_background: terminal.color.RGB,
 
+/// Whether we're doing padding extension for vertical sides.
+padding_extend_top: bool = true,
+padding_extend_bottom: bool = true,
+
 /// The images that we may render.
 images: ImageMap = .{},
 image_placements: ImagePlacementList = .{},
@@ -394,6 +405,7 @@ pub fn init(alloc: Allocator, options: renderer.Options) !OpenGL {
         .cells = .{},
         .grid_metrics = grid.metrics,
         .screen_size = null,
+        .grid_size = .{},
         .gl_state = gl_state,
         .font_grid = grid,
         .font_shaper = shaper,
@@ -669,6 +681,12 @@ pub fn setFontGrid(self: *OpenGL, grid: *font.SharedGrid) void {
     self.font_shaper_cache.deinit(self.alloc);
     self.font_shaper_cache = font_shaper_cache;
 
+    // Update our grid size if we have a screen size. If we don't, its okay
+    // because this will get set when we get the screen size set.
+    if (self.screen_size) |size| {
+        self.grid_size = self.gridSize(size);
+    }
+
     // Defer our GPU updates
     self.deferred_font_size = .{ .metrics = grid.metrics };
 }
@@ -684,8 +702,10 @@ pub fn updateFrame(
 
     // Data we extract out of the critical area.
     const Critical = struct {
+        full_rebuild: bool,
         gl_bg: terminal.color.RGB,
         screen: terminal.Screen,
+        screen_type: terminal.ScreenType,
         mouse: renderer.State.Mouse,
         preedit: ?renderer.State.Preedit,
         cursor_style: ?renderer.CursorStyle,
@@ -715,6 +735,9 @@ pub fn updateFrame(
             self.foreground_color = bg;
         }
 
+        // Get the viewport pin so that we can compare it to the current.
+        const viewport_pin = state.terminal.screen.pages.pin(.{ .viewport = .{} }).?;
+
         // We used to share terminal state, but we've since learned through
         // analysis that it is faster to copy the terminal state than to
         // hold the lock wile rebuilding GPU cells.
@@ -758,9 +781,55 @@ pub fn updateFrame(
             try self.prepKittyGraphics(state.terminal);
         }
 
+        // If we have any terminal dirty flags set then we need to rebuild
+        // the entire screen. This can be optimized in the future.
+        const full_rebuild: bool = rebuild: {
+            {
+                const Int = @typeInfo(terminal.Terminal.Dirty).Struct.backing_integer.?;
+                const v: Int = @bitCast(state.terminal.flags.dirty);
+                if (v > 0) break :rebuild true;
+            }
+            {
+                const Int = @typeInfo(terminal.Screen.Dirty).Struct.backing_integer.?;
+                const v: Int = @bitCast(state.terminal.screen.dirty);
+                if (v > 0) break :rebuild true;
+            }
+
+            // If our viewport changed then we need to rebuild the entire
+            // screen because it means we scrolled. If we have no previous
+            // viewport then we must rebuild.
+            const prev_viewport = self.cells_viewport orelse break :rebuild true;
+            if (!prev_viewport.eql(viewport_pin)) break :rebuild true;
+
+            break :rebuild false;
+        };
+
+        // Reset the dirty flags in the terminal and screen. We assume
+        // that our rebuild will be successful since so we optimize for
+        // success and reset while we hold the lock. This is much easier
+        // than coordinating row by row or as changes are persisted.
+        state.terminal.flags.dirty = .{};
+        state.terminal.screen.dirty = .{};
+        {
+            var it = state.terminal.screen.pages.pageIterator(
+                .right_down,
+                .{ .screen = .{} },
+                null,
+            );
+            while (it.next()) |chunk| {
+                var dirty_set = chunk.page.data.dirtyBitSet();
+                dirty_set.unsetAll();
+            }
+        }
+
+        // Update our viewport pin for dirty tracking
+        self.cells_viewport = viewport_pin;
+
         break :critical .{
+            .full_rebuild = full_rebuild,
             .gl_bg = self.background_color,
             .screen = screen_copy,
+            .screen_type = state.terminal.active_screen,
             .mouse = state.mouse,
             .preedit = preedit,
             .cursor_style = cursor_style,
@@ -782,7 +851,9 @@ pub fn updateFrame(
 
         // Build our GPU cells
         try self.rebuildCells(
+            critical.full_rebuild,
             &critical.screen,
+            critical.screen_type,
             critical.mouse,
             critical.preedit,
             critical.cursor_style,
@@ -1089,7 +1160,9 @@ fn prepKittyImage(
 /// the renderer will do this when it needs more memory space.
 pub fn rebuildCells(
     self: *OpenGL,
+    rebuild: bool,
     screen: *terminal.Screen,
+    screen_type: terminal.ScreenType,
     mouse: renderer.State.Mouse,
     preedit: ?renderer.State.Preedit,
     cursor_style_: ?renderer.CursorStyle,
@@ -1135,6 +1208,12 @@ pub fn rebuildCells(
     // remains visible.
     var cursor_cell: ?CellProgram.Cell = null;
 
+    if (rebuild) {
+        // We also reset our padding extension depending on the screen type
+        self.padding_extend_top = screen_type == .alternate;
+        self.padding_extend_bottom = screen_type == .alternate;
+    }
+
     // Build each cell
     var row_it = screen.pages.rowIterator(.right_down, .{ .viewport = .{} }, null);
     var y: terminal.size.CellCountInt = 0;
@@ -1185,6 +1264,16 @@ pub fn rebuildCells(
             }
         };
 
+        // On primary screen, we still apply vertical padding extension
+        // under certain conditions we feel are safe. This helps make some
+        // scenarios look better while avoiding scenarios we know do NOT look
+        // good.
+        if (y == 0 and screen_type == .primary) {
+            self.padding_extend_top = !row.neverExtendBg();
+        } else if (y == self.grid_size.rows - 1 and screen_type == .primary) {
+            self.padding_extend_bottom = !row.neverExtendBg();
+        }
+
         // Split our row into runs and shape each one.
         var iter = self.font_shaper.runIterator(
             self.font_grid,
@@ -1787,13 +1876,11 @@ pub fn setScreenSize(
     // Store our screen size
     self.screen_size = dim;
     self.padding.explicit = pad;
-
+    self.grid_size = self.gridSize(dim);
-    // Recalculate the rows/columns.
-    const grid_size = self.gridSize(dim);
 
     log.debug("screen size screen={} grid={} cell={} padding={}", .{
         dim,
-        grid_size,
+        self.grid_size,
         renderer.CellSize{
             .width = self.grid_metrics.cell_width,
             .height = self.grid_metrics.cell_height,
@@ -1996,6 +2083,21 @@ fn drawCellProgram(
         self.deferred_config = null;
     }
 
+    // Apply our padding extension fields
+    {
+        const program = gl_state.cell_program;
+        const bind = try program.program.use();
+        defer bind.unbind();
+        try program.program.setUniform(
+            "padding_vertical_top",
+            self.padding_extend_top,
+        );
+        try program.program.setUniform(
+            "padding_vertical_bottom",
+            self.padding_extend_bottom,
+        );
+    }
+
     // Draw background images first
     try self.drawImages(
         gl_state,

src/renderer/metal/shaders.zig

@@ -124,6 +124,10 @@ pub const Uniforms = extern struct {
     /// top, right, bottom, left.
     grid_padding: [4]f32 align(16),
 
+    /// True if vertical padding gets the extended color of the nearest row.
+    padding_extend_top: bool align(1),
+    padding_extend_bottom: bool align(1),
+
     /// The minimum contrast ratio for text. The contrast ratio is calculated
     /// according to the WCAG 2.0 spec.
     min_contrast: f32 align(4),

src/renderer/shaders/cell.metal

@@ -5,6 +5,8 @@ struct Uniforms {
   float2 cell_size;
   ushort2 grid_size;
   float4 grid_padding;
+  bool padding_extend_top;
+  bool padding_extend_bottom;
   float min_contrast;
   ushort2 cursor_pos;
   uchar4 cursor_color;
@@ -101,11 +103,14 @@ vertex CellBgVertexOut cell_bg_vertex(unsigned int vid [[vertex_id]],
   cell_size_scaled.x = cell_size_scaled.x * input.cell_width;
 
   // If we're at the edge of the grid, we add our padding to the background
-  // to extend it. Note: grid_padding is top/right/bottom/left.
+  // to extend it. Note: grid_padding is top/right/bottom/left. We always
-  if (input.grid_pos.y == 0) {
+  // extend horiziontally because there is no downside but there are various
+  // heuristics to disable vertical extension.
+  if (input.grid_pos.y == 0 && uniforms.padding_extend_top) {
     cell_pos.y -= uniforms.grid_padding.r;
     cell_size_scaled.y += uniforms.grid_padding.r;
-  } else if (input.grid_pos.y == uniforms.grid_size.y - 1) {
+  } else if (input.grid_pos.y == uniforms.grid_size.y - 1 &&
+             uniforms.padding_extend_bottom) {
     cell_size_scaled.y += uniforms.grid_padding.b;
   }
   if (input.grid_pos.x == 0) {

src/renderer/shaders/cell.v.glsl

@@ -57,6 +57,8 @@ uniform sampler2D text_color;
 uniform vec2 cell_size;
 uniform vec2 grid_size;
 uniform vec4 grid_padding;
+uniform bool padding_vertical_top;
+uniform bool padding_vertical_bottom;
 uniform mat4 projection;
 uniform float min_contrast;
 
@@ -171,10 +173,10 @@ void main() {
     case MODE_BG:
         // If we're at the edge of the grid, we add our padding to the background
         // to extend it. Note: grid_padding is top/right/bottom/left.
-        if (grid_coord.y == 0) {
+        if (grid_coord.y == 0 && padding_vertical_top) {
             cell_pos.y -= grid_padding.r;
             cell_size_scaled.y += grid_padding.r;
-        } else if (grid_coord.y == grid_size.y - 1) {
+        } else if (grid_coord.y == grid_size.y - 1 && padding_vertical_bottom) {
             cell_size_scaled.y += grid_padding.b;
         }
         if (grid_coord.x == 0) {

src/terminal/PageList.zig

@@ -3254,6 +3254,35 @@ pub const Pin = struct {
         set.set(self.y);
     }
 
+    /// Returns true if the row of this pin should never have its background
+    /// color extended for filling padding space in the renderer. This is
+    /// a set of heuristics that help making our padding look better.
+    pub fn neverExtendBg(self: Pin) bool {
+        // Any semantic prompts should not have their background extended
+        // because prompts often contain special formatting (such as
+        // powerline) that looks bad when extended.
+        const rac = self.rowAndCell();
+        switch (rac.row.semantic_prompt) {
+            .prompt, .prompt_continuation, .input => return true,
+            .unknown, .command => {},
+        }
+
+        for (self.cells(.all)) |*cell| {
+            // If any cell has a default background color then we don't
+            // extend because the default background color probably looks
+            // good enough as an extension.
+            switch (cell.content_tag) {
+                .bg_color_palette, .bg_color_rgb => {},
+                .codepoint, .codepoint_grapheme => {
+                    const s = self.style(cell);
+                    if (s.bg_color == .none) return true;
+                },
+            }
+        }
+
+        return false;
+    }
+
     /// Iterators. These are the same as PageList iterator funcs but operate
     /// on pins rather than points. This is MUCH more efficient than calling
     /// pointFromPin and building up the iterator from points.