Big Cursor State Refactor

This makes a few major changes:

  - cursor style on terminal is single source of stylistic truth
  - cursor style is split between style and style request
  - cursor blinking is handled by the renderer thread
  - cursor style/visibility is no longer stored as persistent state on
    renderers
  - cursor style computation is extracted to be shared by all renderers
  - mode 12 "cursor_blinking" is now source of truth on whether blinking
    is enabled or not
  - CSI q and mode 12 are synced like xterm

10 files changed, 159 insertions, 133 deletions

diff --git a/src/Surface.zig b/src/Surface.zig
index 5531ac986..505f572f5 100644
--- a/src/Surface.zig
+++ b/src/Surface.zig
@@ -418,10 +418,6 @@ pub fn init(
         .renderer_thread = render_thread,
         .renderer_state = .{
             .mutex = mutex,
-            .cursor = .{
-                .style = .default,
-                .visible = true,
-            },
             .terminal = &self.io.terminal,
         },
         .renderer_thr = undefined,
diff --git a/src/config.zig b/src/config.zig
index 9e582305d..cc90a3c28 100644
--- a/src/config.zig
+++ b/src/config.zig
@@ -106,7 +106,7 @@ pub const Config = struct {
     /// In order to fix it, we probably would want to add something similar to Kitty's
     /// shell integration options (no-cursor). For more information see:
     /// https://sw.kovidgoyal.net/kitty/conf/#opt-kitty.shell_integration
-    @"cursor-style": CursorStyle = .bar,
+    @"cursor-style": terminal.Cursor.Style = .bar,
 
     /// Whether the cursor shall blink
     @"cursor-style-blink": bool = true,
@@ -1475,22 +1475,6 @@ pub const ShellIntegration = enum {
     zsh,
 };
 
-/// Available options for `cursor-style`. Blinking is configured with
-/// the `cursor-style-blink` option.
-pub const CursorStyle = enum {
-    bar,
-    block,
-    underline,
-
-    pub fn toTerminalCursorStyle(self: CursorStyle, blinks: bool) terminal.CursorStyle {
-        return switch (self) {
-            .bar => if (blinks) .blinking_bar else .steady_bar,
-            .block => if (blinks) .blinking_block else .steady_block,
-            .underline => if (blinks) .blinking_underline else .steady_underline,
-        };
-    }
-};
-
 // Wasm API.
 pub const Wasm = if (!builtin.target.isWasm()) struct {} else struct {
     const wasm = @import("os/wasm.zig");
diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig
index c0f5b3ac9..2586bdc0d 100644
--- a/src/renderer/Metal.zig
+++ b/src/renderer/Metal.zig
@@ -64,11 +64,6 @@ padding: renderer.Options.Padding,
 /// True if the window is focused
 focused: bool,
 
-/// Whether the cursor is visible or not. This is used to control cursor
-/// blinking.
-cursor_visible: bool,
-cursor_style: renderer.CursorStyle,
-
 /// The current set of cells to render. This is rebuilt on every frame
 /// but we keep this around so that we don't reallocate. Each set of
 /// cells goes into a separate shader.
@@ -108,7 +103,6 @@ pub const DerivedConfig = struct {
     font_thicken: bool,
     font_features: std.ArrayList([]const u8),
     cursor_color: ?terminal.color.RGB,
-    cursor_style: terminal.CursorStyle,
     cursor_text: ?terminal.color.RGB,
     background: terminal.color.RGB,
     background_opacity: f64,
@@ -137,7 +131,6 @@ pub const DerivedConfig = struct {
             else
                 null,
 
-            .cursor_style = config.@"cursor-style".toTerminalCursorStyle(config.@"cursor-style-blink"),
             .cursor_text = if (config.@"cursor-text") |txt|
                 txt.toTerminalRGB()
             else
@@ -252,8 +245,6 @@ pub fn init(alloc: Allocator, options: renderer.Options) !Metal {
         .screen_size = null,
         .padding = options.padding,
         .focused = true,
-        .cursor_visible = true,
-        .cursor_style = .box,
 
         // Render state
         .cells_bg = .{},
@@ -385,13 +376,6 @@ pub fn setFocus(self: *Metal, focus: bool) !void {
     self.focused = focus;
 }
 
-/// Called to toggle the blink state of the cursor
-///
-/// Must be called on the render thread.
-pub fn blinkCursor(self: *Metal, reset: bool) void {
-    self.cursor_visible = reset or !self.cursor_visible;
-}
-
 /// Set the new font size.
 ///
 /// Must be called on the render thread.
@@ -452,6 +436,7 @@ pub fn render(
     self: *Metal,
     surface: *apprt.Surface,
     state: *renderer.State,
+    cursor_blink_visible: bool,
 ) !void {
     _ = surface;
 
@@ -460,8 +445,8 @@ pub fn render(
         bg: terminal.color.RGB,
         selection: ?terminal.Selection,
         screen: terminal.Screen,
-        draw_cursor: bool,
         preedit: ?renderer.State.Preedit,
+        cursor_style: ?renderer.CursorStyle,
     };
 
     // Update all our data as tightly as possible within the mutex.
@@ -475,46 +460,6 @@ pub fn render(
             return;
         }
 
-        // If the terminal state isn't requesting any particular style,
-        // then use the configured style.
-        const selected_cursor_style = style: {
-            if (state.cursor.style != .default) break :style state.cursor.style;
-            if (self.config.cursor_style != .default) break :style self.config.cursor_style;
-            break :style .blinking_block;
-        };
-
-        self.cursor_visible = visible: {
-            // If the cursor is explicitly not visible in the state,
-            // then it is not visible.
-            if (!state.cursor.visible) break :visible false;
-
-            // If we are in preedit, then we always show the cursor
-            if (state.preedit != null) break :visible true;
-
-            // If the cursor isn't a blinking style, then never blink.
-            if (!selected_cursor_style.blinking()) break :visible true;
-
-            // If we're not focused, our cursor is always visible so that
-            // we can show the hollow box.
-            if (!self.focused) break :visible true;
-
-            // Otherwise, adhere to our current state.
-            break :visible self.cursor_visible;
-        };
-
-        // The cursor style only needs to be set if its visible.
-        if (self.cursor_visible) {
-            self.cursor_style = cursor_style: {
-                // If we have a dead key preedit then we always use a box style
-                if (state.preedit != null) break :cursor_style .box;
-
-                // If we aren't focused, we use a hollow box
-                if (!self.focused) break :cursor_style .box_hollow;
-
-                break :cursor_style renderer.CursorStyle.fromTerminal(selected_cursor_style) orelse .box;
-            };
-        }
-
         // Swap bg/fg if the terminal is reversed
         const bg = self.config.background;
         const fg = self.config.foreground;
@@ -550,7 +495,11 @@ pub fn render(
             null;
 
         // Whether to draw our cursor or not.
-        const draw_cursor = self.cursor_visible and state.terminal.screen.viewportIsBottom();
+        const cursor_style = renderer.cursorStyle(
+            state,
+            self.focused,
+            cursor_blink_visible,
+        );
 
         // If we have Kitty graphics data, we enter a SLOW SLOW SLOW path.
         // We only do this if the Kitty image state is dirty meaning only if
@@ -563,8 +512,8 @@ pub fn render(
             .bg = self.config.background,
             .selection = selection,
             .screen = screen_copy,
-            .draw_cursor = draw_cursor,
-            .preedit = if (draw_cursor) state.preedit else null,
+            .preedit = if (cursor_style != null) state.preedit else null,
+            .cursor_style = cursor_style,
         };
     };
     defer critical.screen.deinit();
@@ -577,8 +526,8 @@ pub fn render(
     try self.rebuildCells(
         critical.selection,
         &critical.screen,
-        critical.draw_cursor,
         critical.preedit,
+        critical.cursor_style,
     );
 
     // Get our drawable (CAMetalDrawable)
@@ -1114,8 +1063,8 @@ fn rebuildCells(
     self: *Metal,
     term_selection: ?terminal.Selection,
     screen: *terminal.Screen,
-    draw_cursor: bool,
     preedit: ?renderer.State.Preedit,
+    cursor_style_: ?renderer.CursorStyle,
 ) !void {
     // Bg cells at most will need space for the visible screen size
     self.cells_bg.clearRetainingCapacity();
@@ -1145,8 +1094,7 @@ fn rebuildCells(
         // True if this is the row with our cursor. There are a lot of conditions
         // here because the reasons we need to know this are primarily to invert.
         //
-        //   - If we aren't drawing the cursor (draw_cursor), then we don't need
-        //     to change our rendering.
+        //   - If we aren't drawing the cursor then we don't need to change our rendering.
         //   - If the cursor is not visible, then we don't need to change rendering.
         //   - If the cursor style is not a box, then we don't need to change
         //     rendering because it'll never fully overlap a glyph.
@@ -1157,11 +1105,12 @@ fn rebuildCells(
         //   - If this y doesn't match our cursor y then we don't need to
         //     change rendering.
         //
-        const cursor_row = draw_cursor and
-            self.cursor_visible and
-            self.cursor_style == .box and
-            screen.viewportIsBottom() and
-            y == screen.cursor.y;
+        const cursor_row = if (cursor_style_) |cursor_style|
+            cursor_style == .block and
+                screen.viewportIsBottom() and
+                y == screen.cursor.y
+        else
+            false;
 
         // True if we want to do font shaping around the cursor. We want to
         // do font shaping as long as the cursor is enabled.
@@ -1234,8 +1183,8 @@ fn rebuildCells(
     // Add the cursor at the end so that it overlays everything. If we have
     // a cursor cell then we invert the colors on that and add it in so
     // that we can always see it.
-    if (draw_cursor) {
-        const real_cursor_cell = self.addCursor(screen);
+    if (cursor_style_) |cursor_style| {
+        const real_cursor_cell = self.addCursor(screen, cursor_style);
 
         // If we have a preedit, we try to render the preedit text on top
         // of the cursor.
@@ -1452,7 +1401,11 @@ pub fn updateCell(
     return true;
 }
 
-fn addCursor(self: *Metal, screen: *terminal.Screen) ?*const mtl_shaders.Cell {
+fn addCursor(
+    self: *Metal,
+    screen: *terminal.Screen,
+    cursor_style: renderer.CursorStyle,
+) ?*const mtl_shaders.Cell {
     // Add the cursor
     const cell = screen.getCell(
         .active,
@@ -1466,9 +1419,9 @@ fn addCursor(self: *Metal, screen: *terminal.Screen) ?*const mtl_shaders.Cell {
         .b = 0xFF,
     };
 
-    const sprite: font.Sprite = switch (self.cursor_style) {
-        .box => .cursor_rect,
-        .box_hollow => .cursor_hollow_rect,
+    const sprite: font.Sprite = switch (cursor_style) {
+        .block => .cursor_rect,
+        .block_hollow => .cursor_hollow_rect,
         .bar => .cursor_bar,
     };
 
diff --git a/src/renderer/State.zig b/src/renderer/State.zig
index c216c7d9a..e791cfda4 100644
--- a/src/renderer/State.zig
+++ b/src/renderer/State.zig
@@ -11,9 +11,6 @@ const renderer = @import("../renderer.zig");
 /// state (i.e. the terminal, devmode, etc. values).
 mutex: *std.Thread.Mutex,
 
-/// Cursor configuration for rendering
-cursor: Cursor,
-
 /// The terminal data.
 terminal: *terminal.Terminal,
 
@@ -23,17 +20,6 @@ terminal: *terminal.Terminal,
 /// a future exercise.
 preedit: ?Preedit = null,
 
-pub const Cursor = struct {
-    /// Current cursor style. This can be set by escape sequences. To get
-    /// the default style, the config has to be referenced.
-    style: terminal.CursorStyle = .default,
-
-    /// Whether the cursor is visible at all. This should not be used for
-    /// "blink" settings, see "blink" for that. This is used to turn the
-    /// cursor ON or OFF.
-    visible: bool = true,
-};
-
 /// The pre-edit state. See Surface.preeditCallback for more information.
 pub const Preedit = struct {
     /// The codepoint to render as preedit text. We only support single
diff --git a/src/renderer/Thread.zig b/src/renderer/Thread.zig
index 41218791d..d193246be 100644
--- a/src/renderer/Thread.zig
+++ b/src/renderer/Thread.zig
@@ -48,6 +48,11 @@ cursor_h: xev.Timer,
 cursor_c: xev.Completion = .{},
 cursor_c_cancel: xev.Completion = .{},
 
+/// This is true when a blinking cursor should be visible and false
+/// when it should not be visible. This is toggled on a timer by the
+/// thread automatically.
+cursor_blink_visible: bool = false,
+
 /// The surface we're rendering to.
 surface: *apprt.Surface,
 
@@ -220,7 +225,7 @@ fn drainMailbox(self: *Thread) !void {
                     // If we're focused, we immediately show the cursor again
                     // and then restart the timer.
                     if (self.cursor_c.state() != .active) {
-                        self.renderer.blinkCursor(true);
+                        self.cursor_blink_visible = true;
                         self.cursor_h.run(
                             &self.loop,
                             &self.cursor_c,
@@ -234,7 +239,7 @@ fn drainMailbox(self: *Thread) !void {
             },
 
             .reset_cursor_blink => {
-                self.renderer.blinkCursor(true);
+                self.cursor_blink_visible = true;
                 if (self.cursor_c.state() == .active) {
                     self.cursor_h.reset(
                         &self.loop,
@@ -317,7 +322,11 @@ fn renderCallback(
         return .disarm;
     };
 
-    t.renderer.render(t.surface, t.state) catch |err|
+    t.renderer.render(
+        t.surface,
+        t.state,
+        t.cursor_blink_visible,
+    ) catch |err|
         log.warn("error rendering err={}", .{err});
 
     // If we're doing single-threaded GPU calls then we also wake up the
@@ -356,7 +365,7 @@ fn cursorTimerCallback(
         return .disarm;
     };
 
-    t.renderer.blinkCursor(false);
+    t.cursor_blink_visible = !t.cursor_blink_visible;
     t.wakeup.notify() catch {};
 
     t.cursor_h.run(&t.loop, &t.cursor_c, CURSOR_BLINK_INTERVAL, Thread, t, cursorTimerCallback);
diff --git a/src/renderer/cursor.zig b/src/renderer/cursor.zig
index df0ab1bc4..a8c89c2d4 100644
--- a/src/renderer/cursor.zig
+++ b/src/renderer/cursor.zig
@@ -1,19 +1,52 @@
+const std = @import("std");
 const terminal = @import("../terminal/main.zig");
+const State = @import("State.zig");
 
 /// Available cursor styles for drawing that renderers must support.
+/// This is a superset of terminal cursor styles since the renderer supports
+/// some additional cursor states such as the hollow block.
 pub const CursorStyle = enum {
-    box,
-    box_hollow,
+    block,
+    block_hollow,
     bar,
 
     /// Create a cursor style from the terminal style request.
-    pub fn fromTerminal(style: terminal.CursorStyle) ?CursorStyle {
+    pub fn fromTerminal(style: terminal.Cursor.Style) ?CursorStyle {
         return switch (style) {
-            .blinking_block, .steady_block => .box,
-            .blinking_bar, .steady_bar => .bar,
-            .blinking_underline, .steady_underline => null, // TODO
-            .default => .box,
-            else => null,
+            .bar => .bar,
+            .block => .block,
+            .underline => null, // TODO
         };
     }
 };
+
+/// Returns the cursor style to use for the current render state or null
+/// if a cursor should not be rendered at all.
+pub fn cursorStyle(
+    state: *State,
+    focused: bool,
+    blink_visible: bool,
+) ?CursorStyle {
+    // The cursor is only at the bottom of the viewport. If we aren't
+    // at the bottom, we never render the cursor.
+    if (!state.terminal.screen.viewportIsBottom()) return null;
+
+    // If we are in preedit, then we always show the cursor
+    if (state.preedit != null) return .block;
+
+    // If the cursor is explicitly not visible by terminal mode, then false.
+    if (!state.terminal.modes.get(.cursor_visible)) return null;
+
+    // If we're not focused, our cursor is always visible so that
+    // we can show the hollow box.
+    if (!focused) return .block_hollow;
+
+    // If the cursor is blinking and our blink state is not visible,
+    // then we don't show the cursor.
+    if (state.terminal.modes.get(.cursor_blinking) and !blink_visible) {
+        return null;
+    }
+
+    // Otherwise, we use whatever the terminal wants.
+    return CursorStyle.fromTerminal(state.terminal.screen.cursor.style);
+}
diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig
index 98b6edc4a..f289e096d 100644
--- a/src/terminal/Screen.zig
+++ b/src/terminal/Screen.zig
@@ -68,16 +68,26 @@ const log = std.log.scoped(.screen);
 
 /// Cursor represents the cursor state.
 pub const Cursor = struct {
-    // x, y where the cursor currently exists (0-indexed). This x/y is
-    // always the offset in the active area.
+    /// x, y where the cursor currently exists (0-indexed). This x/y is
+    /// always the offset in the active area.
     x: usize = 0,
     y: usize = 0,
 
-    // pen is the current cell styling to apply to new cells.
+    /// The visual style of the cursor. This defaults to block because
+    /// it has to default to something, but users of this struct are
+    /// encouraged to set their own default.
+    style: Style = .block,
+
+    /// pen is the current cell styling to apply to new cells.
     pen: Cell = .{ .char = 0 },
 
-    // The last column flag (LCF) used to do soft wrapping.
+    /// The last column flag (LCF) used to do soft wrapping.
     pending_wrap: bool = false,
+
+    /// The visual style of the cursor. Whether or not it blinks
+    /// is determined by mode 12 (modes.zig). This mode is synchronized
+    /// with CSI q, the same as xterm.
+    pub const Style = enum { bar, block, underline };
 };
 
 /// This is a single item within the storage buffer. We use a union to
diff --git a/src/terminal/main.zig b/src/terminal/main.zig
index 771fcab42..e1a6ee439 100644
--- a/src/terminal/main.zig
+++ b/src/terminal/main.zig
@@ -20,7 +20,8 @@ pub const Parser = @import("Parser.zig");
 pub const Selection = @import("Selection.zig");
 pub const Screen = @import("Screen.zig");
 pub const Stream = stream.Stream;
-pub const CursorStyle = ansi.CursorStyle;
+pub const Cursor = Screen.Cursor;
+pub const CursorStyleReq = ansi.CursorStyle;
 pub const DeviceAttributeReq = ansi.DeviceAttributeReq;
 pub const DeviceStatusReq = ansi.DeviceStatusReq;
 pub const Mode = modes.Mode;
diff --git a/src/terminal/modes.zig b/src/terminal/modes.zig
index 49b28d86f..2a2e89bf9 100644
--- a/src/terminal/modes.zig
+++ b/src/terminal/modes.zig
@@ -154,7 +154,8 @@ const entries: []const ModeEntry = &.{
     .{ .name = "origin", .value = 6 },
     .{ .name = "autowrap", .value = 7, .default = true },
     .{ .name = "mouse_event_x10", .value = 9 },
-    .{ .name = "cursor_visible", .value = 25 },
+    .{ .name = "cursor_blinking", .value = 12 },
+    .{ .name = "cursor_visible", .value = 25, .default = true },
     .{ .name = "enable_mode_3", .value = 40 },
     .{ .name = "keypad_keys", .value = 66 },
     .{ .name = "mouse_event_normal", .value = 1000 },
diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig
index 175e7fa0d..04e1d879f 100644
--- a/src/termio/Exec.zig
+++ b/src/termio/Exec.zig
@@ -63,6 +63,11 @@ surface_mailbox: apprt.surface.Mailbox,
 /// The cached grid size whenever a resize is called.
 grid_size: renderer.GridSize,
 
+/// The default cursor style. We need to know this so that we can set
+/// it when a CSI q with default is called.
+default_cursor_style: terminal.Cursor.Style,
+default_cursor_blink: bool,
+
 /// The data associated with the currently running thread.
 data: ?*EventData,
 
@@ -72,6 +77,8 @@ data: ?*EventData,
 pub const DerivedConfig = struct {
     palette: terminal.color.Palette,
     image_storage_limit: usize,
+    cursor_style: terminal.Cursor.Style,
+    cursor_blink: bool,
 
     pub fn init(
         alloc_gpa: Allocator,
@@ -82,6 +89,8 @@ pub const DerivedConfig = struct {
         return .{
             .palette = config.palette.value,
             .image_storage_limit = config.@"image-storage-limit",
+            .cursor_style = config.@"cursor-style",
+            .cursor_blink = config.@"cursor-style-blink",
         };
     }
 
@@ -126,6 +135,8 @@ pub fn init(alloc: Allocator, opts: termio.Options) !Exec {
         .renderer_mailbox = opts.renderer_mailbox,
         .surface_mailbox = opts.surface_mailbox,
         .grid_size = opts.grid_size,
+        .default_cursor_style = opts.config.cursor_style,
+        .default_cursor_blink = opts.config.cursor_blink,
         .data = null,
     };
 }
@@ -253,6 +264,10 @@ pub fn changeConfig(self: *Exec, config: *DerivedConfig) !void {
     // since we decode all palette colors to RGB on usage.
     self.terminal.color_palette = config.palette;
 
+    // Update our default cursor style
+    self.default_cursor_style = config.cursor_style;
+    self.default_cursor_blink = config.cursor_blink;
+
     // Set the image size limits
     try self.terminal.screen.kitty_images.setLimit(
         self.alloc,
@@ -468,6 +483,10 @@ const EventData = struct {
     /// this to determine if we need to default the window title.
     seen_title: bool = false,
 
+    /// The default cursor style used for CSI q.
+    default_cursor_style: terminal.Cursor.Style = .block,
+    default_cursor_blink: bool = true,
+
     pub fn deinit(self: *EventData, alloc: Allocator) void {
         // Clear our write pools. We know we aren't ever going to do
         // any more IO since we stop our data stream below so we can just
@@ -1339,9 +1358,6 @@ const StreamHandler = struct {
             // Origin resets cursor pos
             .origin => self.terminal.setCursorPos(1, 1),
 
-            // We need to update our renderer state for this mode
-            .cursor_visible => self.ev.renderer_state.cursor.visible = enabled,
-
             .alt_screen_save_cursor_clear_enter => {
                 const opts: terminal.Terminal.AlternateScreenOptions = .{
                     .cursor_save = true,
@@ -1462,9 +1478,46 @@ const StreamHandler = struct {
 
     pub fn setCursorStyle(
         self: *StreamHandler,
-        style: terminal.CursorStyle,
+        style: terminal.CursorStyleReq,
     ) !void {
-        self.ev.renderer_state.cursor.style = style;
+        switch (style) {
+            .default => {
+                self.terminal.screen.cursor.style = self.ev.default_cursor_style;
+                self.terminal.modes.set(.cursor_blinking, self.ev.default_cursor_blink);
+            },
+
+            .blinking_block => {
+                self.terminal.screen.cursor.style = .block;
+                self.terminal.modes.set(.cursor_blinking, true);
+            },
+
+            .steady_block => {
+                self.terminal.screen.cursor.style = .block;
+                self.terminal.modes.set(.cursor_blinking, false);
+            },
+
+            .blinking_underline => {
+                self.terminal.screen.cursor.style = .underline;
+                self.terminal.modes.set(.cursor_blinking, true);
+            },
+
+            .steady_underline => {
+                self.terminal.screen.cursor.style = .underline;
+                self.terminal.modes.set(.cursor_blinking, false);
+            },
+
+            .blinking_bar => {
+                self.terminal.screen.cursor.style = .bar;
+                self.terminal.modes.set(.cursor_blinking, true);
+            },
+
+            .steady_bar => {
+                self.terminal.screen.cursor.style = .bar;
+                self.terminal.modes.set(.cursor_blinking, false);
+            },
+
+            else => log.warn("unimplemented cursor style: {}", .{style}),
+        }
     }
 
     pub fn decaln(self: *StreamHandler) !void {