From e3c39ed502c62a907dff935089fe9e2747356da9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 20 Jan 2026 11:46:24 -0800 Subject: [PATCH 1/6] renderer: cell.Contents.resize errdefer handling is now safe --- src/renderer/cell.zig | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/renderer/cell.zig b/src/renderer/cell.zig index 9e5802ea5..5ea5b7ab0 100644 --- a/src/renderer/cell.zig +++ b/src/renderer/cell.zig @@ -90,7 +90,6 @@ pub const Contents = struct { const bg_cells = try alloc.alloc(shaderpkg.CellBg, cell_count); errdefer alloc.free(bg_cells); - @memset(bg_cells, .{ 0, 0, 0, 0 }); // The foreground lists can hold 3 types of items: @@ -106,32 +105,28 @@ pub const Contents = struct { // We have size.rows + 2 lists because indexes 0 and size.rows - 1 are // used for special lists containing the cursor cell which need to // be first and last in the buffer, respectively. - var fg_rows = try ArrayListCollection(shaderpkg.CellText).init( + var fg_rows: ArrayListCollection(shaderpkg.CellText) = try .init( alloc, size.rows + 2, size.columns * 3, ); errdefer fg_rows.deinit(alloc); - alloc.free(self.bg_cells); - self.fg_rows.deinit(alloc); - - self.bg_cells = bg_cells; - self.fg_rows = fg_rows; - // We don't need 3*cols worth of cells for the cursor lists, so we can // replace them with smaller lists. This is technically a tiny bit of // extra work but resize is not a hot function so it's worth it to not // waste the memory. - self.fg_rows.lists[0].deinit(alloc); - self.fg_rows.lists[0] = try std.ArrayListUnmanaged( - shaderpkg.CellText, - ).initCapacity(alloc, 1); + fg_rows.lists[0].deinit(alloc); + fg_rows.lists[0] = try .initCapacity(alloc, 1); + fg_rows.lists[size.rows + 1].deinit(alloc); + fg_rows.lists[size.rows + 1] = try .initCapacity(alloc, 1); - self.fg_rows.lists[size.rows + 1].deinit(alloc); - self.fg_rows.lists[size.rows + 1] = try std.ArrayListUnmanaged( - shaderpkg.CellText, - ).initCapacity(alloc, 1); + // Perform the swap, no going back from here. + errdefer comptime unreachable; + alloc.free(self.bg_cells); + self.fg_rows.deinit(alloc); + self.bg_cells = bg_cells; + self.fg_rows = fg_rows; } /// Reset the cell contents to an empty state without resizing. From e875b453b7342767f7b8315ba1628853e6fb4cb6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 20 Jan 2026 11:50:58 -0800 Subject: [PATCH 2/6] font/shaper: hook functions can't fail --- src/font/shaper/coretext.zig | 8 ++++---- src/font/shaper/harfbuzz.zig | 4 ++-- src/font/shaper/run.zig | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/font/shaper/coretext.zig b/src/font/shaper/coretext.zig index cc05022c4..ab3c6aaab 100644 --- a/src/font/shaper/coretext.zig +++ b/src/font/shaper/coretext.zig @@ -98,7 +98,7 @@ pub const Shaper = struct { self.unichars.deinit(alloc); } - fn reset(self: *RunState) !void { + fn reset(self: *RunState) void { self.codepoints.clearRetainingCapacity(); self.unichars.clearRetainingCapacity(); } @@ -644,8 +644,8 @@ pub const Shaper = struct { pub const RunIteratorHook = struct { shaper: *Shaper, - pub fn prepare(self: *RunIteratorHook) !void { - try self.shaper.run_state.reset(); + pub fn prepare(self: *RunIteratorHook) void { + self.shaper.run_state.reset(); // log.warn("----------- run reset -------------", .{}); } @@ -681,7 +681,7 @@ pub const Shaper = struct { }); } - pub fn finalize(self: RunIteratorHook) !void { + pub fn finalize(self: RunIteratorHook) void { _ = self; } }; diff --git a/src/font/shaper/harfbuzz.zig b/src/font/shaper/harfbuzz.zig index e4a9301e8..6dcc0f94e 100644 --- a/src/font/shaper/harfbuzz.zig +++ b/src/font/shaper/harfbuzz.zig @@ -175,7 +175,7 @@ pub const Shaper = struct { pub const RunIteratorHook = struct { shaper: *Shaper, - pub fn prepare(self: RunIteratorHook) !void { + pub fn prepare(self: RunIteratorHook) void { // Reset the buffer for our current run self.shaper.hb_buf.reset(); self.shaper.hb_buf.setContentType(.unicode); @@ -191,7 +191,7 @@ pub const Shaper = struct { self.shaper.hb_buf.add(cp, cluster); } - pub fn finalize(self: RunIteratorHook) !void { + pub fn finalize(self: RunIteratorHook) void { self.shaper.hb_buf.guessSegmentProperties(); } }; diff --git a/src/font/shaper/run.zig b/src/font/shaper/run.zig index 85c5c410b..45c5d38ca 100644 --- a/src/font/shaper/run.zig +++ b/src/font/shaper/run.zig @@ -73,7 +73,7 @@ pub const RunIterator = struct { var current_font: font.Collection.Index = .{}; // Allow the hook to prepare - try self.hooks.prepare(); + self.hooks.prepare(); // Initialize our hash for this run. var hasher = Hasher.init(0); @@ -283,7 +283,7 @@ pub const RunIterator = struct { } // Finalize our buffer - try self.hooks.finalize(); + self.hooks.finalize(); // Add our length to the hash as an additional mechanism to avoid collisions autoHash(&hasher, j - self.i); From 112363c4e174eb9335e8905f052c02c78dacc882 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 20 Jan 2026 11:57:18 -0800 Subject: [PATCH 3/6] font: Collection.getEntry explicit error set --- src/font/Collection.zig | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/font/Collection.zig b/src/font/Collection.zig index 412098f10..5d7bfa519 100644 --- a/src/font/Collection.zig +++ b/src/font/Collection.zig @@ -196,8 +196,18 @@ pub fn getFace(self: *Collection, index: Index) !*Face { return try self.getFaceFromEntry(try self.getEntry(index)); } +pub const EntryError = error{ + /// Index represents a special font (built-in) and these don't + /// have an associated face. This should be caught upstream and use + /// alternate logic. + SpecialHasNoFace, + + /// Invalid index. + IndexOutOfBounds, +}; + /// Get the unaliased entry from an index -pub fn getEntry(self: *Collection, index: Index) !*Entry { +pub fn getEntry(self: *Collection, index: Index) EntryError!*Entry { if (index.special() != null) return error.SpecialHasNoFace; const list = self.faces.getPtr(index.style); if (index.idx >= list.len) return error.IndexOutOfBounds; From db092ac3cec33998a4e3741dbe47775ae49b6a58 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 20 Jan 2026 12:05:35 -0800 Subject: [PATCH 4/6] renderer: extract rebuildRow to its own function --- src/renderer/generic.zig | 905 ++++++++++++++++++++------------------- 1 file changed, 465 insertions(+), 440 deletions(-) diff --git a/src/renderer/generic.zig b/src/renderer/generic.zig index f3c0556f5..bc81fa98e 100644 --- a/src/renderer/generic.zig +++ b/src/renderer/generic.zig @@ -2480,6 +2480,12 @@ pub fn Renderer(comptime GraphicsAPI: type) type { } } + const PreeditRange = struct { + y: terminal.size.CellCountInt, + x: [2]terminal.size.CellCountInt, + cp_offset: usize, + }; + /// Convert the terminal state to GPU cells stored in CPU memory. These /// are then synced to the GPU in the next frame. This only updates CPU /// memory and doesn't touch the GPU. @@ -2505,11 +2511,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { // Determine our x/y range for preedit. We don't want to render anything // here because we will render the preedit separately. - const preedit_range: ?struct { - y: terminal.size.CellCountInt, - x: [2]terminal.size.CellCountInt, - cp_offset: usize, - } = if (preedit) |preedit_v| preedit: { + const preedit_range: ?PreeditRange = if (preedit) |preedit_v| preedit: { // We base the preedit on the position of the cursor in the // viewport. If the cursor isn't visible in the viewport we // don't show it. @@ -2587,7 +2589,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { row_dirty[0..row_len], row_selection[0..row_len], row_highlights[0..row_len], - ) |y_usize, row, *cells, *dirty, selection, highlights| { + ) |y_usize, row, *cells, *dirty, selection, *highlights| { const y: terminal.size.CellCountInt = @intCast(y_usize); if (!rebuild) { @@ -2601,440 +2603,15 @@ pub fn Renderer(comptime GraphicsAPI: type) type { // Unmark the dirty state in our render state. dirty.* = false; - // If our viewport is wider than our cell contents buffer, - // we still only process cells up to the width of the buffer. - const cells_slice = cells.slice(); - const cells_len = @min(cells_slice.len, self.cells.size.columns); - const cells_raw = cells_slice.items(.raw); - const cells_style = cells_slice.items(.style); - - // 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. - switch (self.config.padding_color) { - // These already have the correct values set above. - .background, .@"extend-always" => {}, - - // Apply heuristics for padding extension. - .extend => if (y == 0) { - self.uniforms.padding_extend.up = !rowNeverExtendBg( - row, - cells_raw, - cells_style, - &state.colors.palette, - state.colors.background, - ); - } else if (y == self.cells.size.rows - 1) { - self.uniforms.padding_extend.down = !rowNeverExtendBg( - row, - cells_raw, - cells_style, - &state.colors.palette, - state.colors.background, - ); - }, - } - - // Iterator of runs for shaping. - var run_iter_opts: font.shape.RunOptions = .{ - .grid = self.font_grid, - .cells = cells_slice, - .selection = if (selection) |s| s else null, - - // We want to do font shaping as long as the cursor is - // visible on this viewport. - .cursor_x = cursor_x: { - const vp = state.cursor.viewport orelse break :cursor_x null; - if (vp.y != y) break :cursor_x null; - break :cursor_x vp.x; - }, - }; - run_iter_opts.applyBreakConfig(self.config.font_shaping_break); - var run_iter = self.font_shaper.runIterator(run_iter_opts); - var shaper_run: ?font.shape.TextRun = try run_iter.next(self.alloc); - var shaper_cells: ?[]const font.shape.Cell = null; - var shaper_cells_i: usize = 0; - - for ( - 0.., - cells_raw[0..cells_len], - cells_style[0..cells_len], - ) |x, *cell, *managed_style| { - // If this cell falls within our preedit range then we - // skip this because preedits are setup separately. - if (preedit_range) |range| preedit: { - // We're not on the preedit line, no actions necessary. - if (range.y != y) break :preedit; - // We're before the preedit range, no actions necessary. - if (x < range.x[0]) break :preedit; - // We're in the preedit range, skip this cell. - if (x <= range.x[1]) continue; - // After exiting the preedit range we need to catch - // the run position up because of the missed cells. - // In all other cases, no action is necessary. - if (x != range.x[1] + 1) break :preedit; - - // Step the run iterator until we find a run that ends - // after the current cell, which will be the soonest run - // that might contain glyphs for our cell. - while (shaper_run) |run| { - if (run.offset + run.cells > x) break; - shaper_run = try run_iter.next(self.alloc); - shaper_cells = null; - shaper_cells_i = 0; - } - - const run = shaper_run orelse break :preedit; - - // If we haven't shaped this run, do so now. - shaper_cells = shaper_cells orelse - // Try to read the cells from the shaping cache if we can. - self.font_shaper_cache.get(run) orelse - cache: { - // Otherwise we have to shape them. - const new_cells = try self.font_shaper.shape(run); - - // Try to cache them. If caching fails for any reason we - // continue because it is just a performance optimization, - // not a correctness issue. - self.font_shaper_cache.put( - self.alloc, - run, - new_cells, - ) catch |err| { - log.warn( - "error caching font shaping results err={}", - .{err}, - ); - }; - - // The cells we get from direct shaping are always owned - // by the shaper and valid until the next shaping call so - // we can safely use them. - break :cache new_cells; - }; - - // Advance our index until we reach or pass - // our current x position in the shaper cells. - const shaper_cells_unwrapped = shaper_cells.?; - while (run.offset + shaper_cells_unwrapped[shaper_cells_i].x < x) { - shaper_cells_i += 1; - } - } - - const wide = cell.wide; - const style: terminal.Style = if (cell.hasStyling()) - managed_style.* - else - .{}; - - // True if this cell is selected - const selected: enum { - false, - selection, - search, - search_selected, - } = selected: { - // Order below matters for precedence. - - // Selection should take the highest precedence. - const x_compare = if (wide == .spacer_tail) - x -| 1 - else - x; - if (selection) |sel| { - if (x_compare >= sel[0] and - x_compare <= sel[1]) break :selected .selection; - } - - // If we're highlighted, then we're selected. In the - // future we want to use a different style for this - // but this to get started. - for (highlights.items) |hl| { - if (x_compare >= hl.range[0] and - x_compare <= hl.range[1]) - { - const tag: HighlightTag = @enumFromInt(hl.tag); - break :selected switch (tag) { - .search_match => .search, - .search_match_selected => .search_selected, - }; - } - } - - break :selected .false; - }; - - // The `_style` suffixed values are the colors based on - // the cell style (SGR), before applying any additional - // configuration, inversions, selections, etc. - const bg_style = style.bg( - cell, - &state.colors.palette, - ); - const fg_style = style.fg(.{ - .default = state.colors.foreground, - .palette = &state.colors.palette, - .bold = self.config.bold_color, - }); - - // The final background color for the cell. - const bg = switch (selected) { - // If we have an explicit selection background color - // specified in the config, use that. - // - // If no configuration, then our selection background - // is our foreground color. - .selection => if (self.config.selection_background) |v| switch (v) { - .color => |color| color.toTerminalRGB(), - .@"cell-foreground" => if (style.flags.inverse) bg_style else fg_style, - .@"cell-background" => if (style.flags.inverse) fg_style else bg_style, - } else state.colors.foreground, - - .search => switch (self.config.search_background) { - .color => |color| color.toTerminalRGB(), - .@"cell-foreground" => if (style.flags.inverse) bg_style else fg_style, - .@"cell-background" => if (style.flags.inverse) fg_style else bg_style, - }, - - .search_selected => switch (self.config.search_selected_background) { - .color => |color| color.toTerminalRGB(), - .@"cell-foreground" => if (style.flags.inverse) bg_style else fg_style, - .@"cell-background" => if (style.flags.inverse) fg_style else bg_style, - }, - - // Not selected - .false => if (style.flags.inverse != isCovering(cell.codepoint())) - // Two cases cause us to invert (use the fg color as the bg) - // - The "inverse" style flag. - // - A "covering" glyph; we use fg for bg in that - // case to help make sure that padding extension - // works correctly. - // - // If one of these is true (but not the other) - // then we use the fg style color for the bg. - fg_style - else - // Otherwise they cancel out. - bg_style, - }; - - const fg = fg: { - // Our happy-path non-selection background color - // is our style or our configured defaults. - const final_bg = bg_style orelse state.colors.background; - - // Whether we need to use the bg color as our fg color: - // - Cell is selected, inverted, and set to cell-foreground - // - Cell is selected, not inverted, and set to cell-background - // - Cell is inverted and not selected - break :fg switch (selected) { - .selection => if (self.config.selection_foreground) |v| switch (v) { - .color => |color| color.toTerminalRGB(), - .@"cell-foreground" => if (style.flags.inverse) final_bg else fg_style, - .@"cell-background" => if (style.flags.inverse) fg_style else final_bg, - } else state.colors.background, - - .search => switch (self.config.search_foreground) { - .color => |color| color.toTerminalRGB(), - .@"cell-foreground" => if (style.flags.inverse) final_bg else fg_style, - .@"cell-background" => if (style.flags.inverse) fg_style else final_bg, - }, - - .search_selected => switch (self.config.search_selected_foreground) { - .color => |color| color.toTerminalRGB(), - .@"cell-foreground" => if (style.flags.inverse) final_bg else fg_style, - .@"cell-background" => if (style.flags.inverse) fg_style else final_bg, - }, - - .false => if (style.flags.inverse) - final_bg - else - fg_style, - }; - }; - - // Foreground alpha for this cell. - const alpha: u8 = if (style.flags.faint) self.config.faint_opacity else 255; - - // Set the cell's background color. - { - const rgb = bg orelse state.colors.background; - - // Determine our background alpha. If we have transparency configured - // then this is dynamic depending on some situations. This is all - // in an attempt to make transparency look the best for various - // situations. See inline comments. - const bg_alpha: u8 = bg_alpha: { - const default: u8 = 255; - - // Cells that are selected should be fully opaque. - if (selected != .false) break :bg_alpha default; - - // Cells that are reversed should be fully opaque. - if (style.flags.inverse) break :bg_alpha default; - - // If the user requested to have opacity on all cells, apply it. - if (self.config.background_opacity_cells and bg_style != null) { - var opacity: f64 = @floatFromInt(default); - opacity *= self.config.background_opacity; - break :bg_alpha @intFromFloat(opacity); - } - - // Cells that have an explicit bg color should be fully opaque. - if (bg_style != null) break :bg_alpha default; - - // Otherwise, we won't draw the bg for this cell, - // we'll let the already-drawn background color - // show through. - break :bg_alpha 0; - }; - - self.cells.bgCell(y, x).* = .{ - rgb.r, rgb.g, rgb.b, bg_alpha, - }; - } - - // If the invisible flag is set on this cell then we - // don't need to render any foreground elements, so - // we just skip all glyphs with this x coordinate. - // - // NOTE: This behavior matches xterm. Some other terminal - // emulators, e.g. Alacritty, still render text decorations - // and only make the text itself invisible. The decision - // has been made here to match xterm's behavior for this. - if (style.flags.invisible) { - continue; - } - - // Give links a single underline, unless they already have - // an underline, in which case use a double underline to - // distinguish them. - const underline: terminal.Attribute.Underline = underline: { - if (links.contains(.{ - .x = @intCast(x), - .y = @intCast(y), - })) { - break :underline if (style.flags.underline == .single) - .double - else - .single; - } - break :underline style.flags.underline; - }; - - // We draw underlines first so that they layer underneath text. - // This improves readability when a colored underline is used - // which intersects parts of the text (descenders). - if (underline != .none) self.addUnderline( - @intCast(x), - @intCast(y), - underline, - style.underlineColor(&state.colors.palette) orelse fg, - alpha, - ) catch |err| { - log.warn( - "error adding underline to cell, will be invalid x={} y={}, err={}", - .{ x, y, err }, - ); - }; - - if (style.flags.overline) self.addOverline(@intCast(x), @intCast(y), fg, alpha) catch |err| { - log.warn( - "error adding overline to cell, will be invalid x={} y={}, err={}", - .{ x, y, err }, - ); - }; - - // If we're at or past the end of our shaper run then - // we need to get the next run from the run iterator. - if (shaper_cells != null and shaper_cells_i >= shaper_cells.?.len) { - shaper_run = try run_iter.next(self.alloc); - shaper_cells = null; - shaper_cells_i = 0; - } - - if (shaper_run) |run| glyphs: { - // If we haven't shaped this run yet, do so. - shaper_cells = shaper_cells orelse - // Try to read the cells from the shaping cache if we can. - self.font_shaper_cache.get(run) orelse - cache: { - // Otherwise we have to shape them. - const new_cells = try self.font_shaper.shape(run); - - // Try to cache them. If caching fails for any reason we - // continue because it is just a performance optimization, - // not a correctness issue. - self.font_shaper_cache.put( - self.alloc, - run, - new_cells, - ) catch |err| { - log.warn( - "error caching font shaping results err={}", - .{err}, - ); - }; - - // The cells we get from direct shaping are always owned - // by the shaper and valid until the next shaping call so - // we can safely use them. - break :cache new_cells; - }; - - const shaped_cells = shaper_cells orelse break :glyphs; - - // If there are no shaper cells for this run, ignore it. - // This can occur for runs of empty cells, and is fine. - if (shaped_cells.len == 0) break :glyphs; - - // If we encounter a shaper cell to the left of the current - // cell then we have some problems. This logic relies on x - // position monotonically increasing. - assert(run.offset + shaped_cells[shaper_cells_i].x >= x); - - // NOTE: An assumption is made here that a single cell will never - // be present in more than one shaper run. If that assumption is - // violated, this logic breaks. - - while (shaper_cells_i < shaped_cells.len and - run.offset + shaped_cells[shaper_cells_i].x == x) : ({ - shaper_cells_i += 1; - }) { - self.addGlyph( - @intCast(x), - @intCast(y), - state.cols, - cells_raw, - shaped_cells[shaper_cells_i], - shaper_run.?, - fg, - alpha, - ) catch |err| { - log.warn( - "error adding glyph to cell, will be invalid x={} y={}, err={}", - .{ x, y, err }, - ); - }; - } - } - - // Finally, draw a strikethrough if necessary. - if (style.flags.strikethrough) self.addStrikethrough( - @intCast(x), - @intCast(y), - fg, - alpha, - ) catch |err| { - log.warn( - "error adding strikethrough to cell, will be invalid x={} y={}, err={}", - .{ x, y, err }, - ); - }; - } + try self.rebuildRow( + y, + row, + cells, + preedit_range, + selection, + highlights, + links, + ); } // Setup our cursor rendering information. @@ -3203,6 +2780,454 @@ pub fn Renderer(comptime GraphicsAPI: type) type { // }); } + fn rebuildRow( + self: *Self, + y: terminal.size.CellCountInt, + row: terminal.page.Row, + cells: *std.MultiArrayList(terminal.RenderState.Cell), + preedit_range: ?PreeditRange, + selection: ?[2]terminal.size.CellCountInt, + highlights: *const std.ArrayList(terminal.RenderState.Highlight), + links: *const terminal.RenderState.CellSet, + ) !void { + const state = &self.terminal_state; + + // If our viewport is wider than our cell contents buffer, + // we still only process cells up to the width of the buffer. + const cells_slice = cells.slice(); + const cells_len = @min(cells_slice.len, self.cells.size.columns); + const cells_raw = cells_slice.items(.raw); + const cells_style = cells_slice.items(.style); + + // 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. + switch (self.config.padding_color) { + // These already have the correct values set above. + .background, .@"extend-always" => {}, + + // Apply heuristics for padding extension. + .extend => if (y == 0) { + self.uniforms.padding_extend.up = !rowNeverExtendBg( + row, + cells_raw, + cells_style, + &state.colors.palette, + state.colors.background, + ); + } else if (y == self.cells.size.rows - 1) { + self.uniforms.padding_extend.down = !rowNeverExtendBg( + row, + cells_raw, + cells_style, + &state.colors.palette, + state.colors.background, + ); + }, + } + + // Iterator of runs for shaping. + var run_iter_opts: font.shape.RunOptions = .{ + .grid = self.font_grid, + .cells = cells_slice, + .selection = if (selection) |s| s else null, + + // We want to do font shaping as long as the cursor is + // visible on this viewport. + .cursor_x = cursor_x: { + const vp = state.cursor.viewport orelse break :cursor_x null; + if (vp.y != y) break :cursor_x null; + break :cursor_x vp.x; + }, + }; + run_iter_opts.applyBreakConfig(self.config.font_shaping_break); + var run_iter = self.font_shaper.runIterator(run_iter_opts); + var shaper_run: ?font.shape.TextRun = try run_iter.next(self.alloc); + var shaper_cells: ?[]const font.shape.Cell = null; + var shaper_cells_i: usize = 0; + + for ( + 0.., + cells_raw[0..cells_len], + cells_style[0..cells_len], + ) |x, *cell, *managed_style| { + // If this cell falls within our preedit range then we + // skip this because preedits are setup separately. + if (preedit_range) |range| preedit: { + // We're not on the preedit line, no actions necessary. + if (range.y != y) break :preedit; + // We're before the preedit range, no actions necessary. + if (x < range.x[0]) break :preedit; + // We're in the preedit range, skip this cell. + if (x <= range.x[1]) continue; + // After exiting the preedit range we need to catch + // the run position up because of the missed cells. + // In all other cases, no action is necessary. + if (x != range.x[1] + 1) break :preedit; + + // Step the run iterator until we find a run that ends + // after the current cell, which will be the soonest run + // that might contain glyphs for our cell. + while (shaper_run) |run| { + if (run.offset + run.cells > x) break; + shaper_run = try run_iter.next(self.alloc); + shaper_cells = null; + shaper_cells_i = 0; + } + + const run = shaper_run orelse break :preedit; + + // If we haven't shaped this run, do so now. + shaper_cells = shaper_cells orelse + // Try to read the cells from the shaping cache if we can. + self.font_shaper_cache.get(run) orelse + cache: { + // Otherwise we have to shape them. + const new_cells = try self.font_shaper.shape(run); + + // Try to cache them. If caching fails for any reason we + // continue because it is just a performance optimization, + // not a correctness issue. + self.font_shaper_cache.put( + self.alloc, + run, + new_cells, + ) catch |err| { + log.warn( + "error caching font shaping results err={}", + .{err}, + ); + }; + + // The cells we get from direct shaping are always owned + // by the shaper and valid until the next shaping call so + // we can safely use them. + break :cache new_cells; + }; + + // Advance our index until we reach or pass + // our current x position in the shaper cells. + const shaper_cells_unwrapped = shaper_cells.?; + while (run.offset + shaper_cells_unwrapped[shaper_cells_i].x < x) { + shaper_cells_i += 1; + } + } + + const wide = cell.wide; + const style: terminal.Style = if (cell.hasStyling()) + managed_style.* + else + .{}; + + // True if this cell is selected + const selected: enum { + false, + selection, + search, + search_selected, + } = selected: { + // Order below matters for precedence. + + // Selection should take the highest precedence. + const x_compare = if (wide == .spacer_tail) + x -| 1 + else + x; + if (selection) |sel| { + if (x_compare >= sel[0] and + x_compare <= sel[1]) break :selected .selection; + } + + // If we're highlighted, then we're selected. In the + // future we want to use a different style for this + // but this to get started. + for (highlights.items) |hl| { + if (x_compare >= hl.range[0] and + x_compare <= hl.range[1]) + { + const tag: HighlightTag = @enumFromInt(hl.tag); + break :selected switch (tag) { + .search_match => .search, + .search_match_selected => .search_selected, + }; + } + } + + break :selected .false; + }; + + // The `_style` suffixed values are the colors based on + // the cell style (SGR), before applying any additional + // configuration, inversions, selections, etc. + const bg_style = style.bg( + cell, + &state.colors.palette, + ); + const fg_style = style.fg(.{ + .default = state.colors.foreground, + .palette = &state.colors.palette, + .bold = self.config.bold_color, + }); + + // The final background color for the cell. + const bg = switch (selected) { + // If we have an explicit selection background color + // specified in the config, use that. + // + // If no configuration, then our selection background + // is our foreground color. + .selection => if (self.config.selection_background) |v| switch (v) { + .color => |color| color.toTerminalRGB(), + .@"cell-foreground" => if (style.flags.inverse) bg_style else fg_style, + .@"cell-background" => if (style.flags.inverse) fg_style else bg_style, + } else state.colors.foreground, + + .search => switch (self.config.search_background) { + .color => |color| color.toTerminalRGB(), + .@"cell-foreground" => if (style.flags.inverse) bg_style else fg_style, + .@"cell-background" => if (style.flags.inverse) fg_style else bg_style, + }, + + .search_selected => switch (self.config.search_selected_background) { + .color => |color| color.toTerminalRGB(), + .@"cell-foreground" => if (style.flags.inverse) bg_style else fg_style, + .@"cell-background" => if (style.flags.inverse) fg_style else bg_style, + }, + + // Not selected + .false => if (style.flags.inverse != isCovering(cell.codepoint())) + // Two cases cause us to invert (use the fg color as the bg) + // - The "inverse" style flag. + // - A "covering" glyph; we use fg for bg in that + // case to help make sure that padding extension + // works correctly. + // + // If one of these is true (but not the other) + // then we use the fg style color for the bg. + fg_style + else + // Otherwise they cancel out. + bg_style, + }; + + const fg = fg: { + // Our happy-path non-selection background color + // is our style or our configured defaults. + const final_bg = bg_style orelse state.colors.background; + + // Whether we need to use the bg color as our fg color: + // - Cell is selected, inverted, and set to cell-foreground + // - Cell is selected, not inverted, and set to cell-background + // - Cell is inverted and not selected + break :fg switch (selected) { + .selection => if (self.config.selection_foreground) |v| switch (v) { + .color => |color| color.toTerminalRGB(), + .@"cell-foreground" => if (style.flags.inverse) final_bg else fg_style, + .@"cell-background" => if (style.flags.inverse) fg_style else final_bg, + } else state.colors.background, + + .search => switch (self.config.search_foreground) { + .color => |color| color.toTerminalRGB(), + .@"cell-foreground" => if (style.flags.inverse) final_bg else fg_style, + .@"cell-background" => if (style.flags.inverse) fg_style else final_bg, + }, + + .search_selected => switch (self.config.search_selected_foreground) { + .color => |color| color.toTerminalRGB(), + .@"cell-foreground" => if (style.flags.inverse) final_bg else fg_style, + .@"cell-background" => if (style.flags.inverse) fg_style else final_bg, + }, + + .false => if (style.flags.inverse) + final_bg + else + fg_style, + }; + }; + + // Foreground alpha for this cell. + const alpha: u8 = if (style.flags.faint) self.config.faint_opacity else 255; + + // Set the cell's background color. + { + const rgb = bg orelse state.colors.background; + + // Determine our background alpha. If we have transparency configured + // then this is dynamic depending on some situations. This is all + // in an attempt to make transparency look the best for various + // situations. See inline comments. + const bg_alpha: u8 = bg_alpha: { + const default: u8 = 255; + + // Cells that are selected should be fully opaque. + if (selected != .false) break :bg_alpha default; + + // Cells that are reversed should be fully opaque. + if (style.flags.inverse) break :bg_alpha default; + + // If the user requested to have opacity on all cells, apply it. + if (self.config.background_opacity_cells and bg_style != null) { + var opacity: f64 = @floatFromInt(default); + opacity *= self.config.background_opacity; + break :bg_alpha @intFromFloat(opacity); + } + + // Cells that have an explicit bg color should be fully opaque. + if (bg_style != null) break :bg_alpha default; + + // Otherwise, we won't draw the bg for this cell, + // we'll let the already-drawn background color + // show through. + break :bg_alpha 0; + }; + + self.cells.bgCell(y, x).* = .{ + rgb.r, rgb.g, rgb.b, bg_alpha, + }; + } + + // If the invisible flag is set on this cell then we + // don't need to render any foreground elements, so + // we just skip all glyphs with this x coordinate. + // + // NOTE: This behavior matches xterm. Some other terminal + // emulators, e.g. Alacritty, still render text decorations + // and only make the text itself invisible. The decision + // has been made here to match xterm's behavior for this. + if (style.flags.invisible) { + continue; + } + + // Give links a single underline, unless they already have + // an underline, in which case use a double underline to + // distinguish them. + const underline: terminal.Attribute.Underline = underline: { + if (links.contains(.{ + .x = @intCast(x), + .y = @intCast(y), + })) { + break :underline if (style.flags.underline == .single) + .double + else + .single; + } + break :underline style.flags.underline; + }; + + // We draw underlines first so that they layer underneath text. + // This improves readability when a colored underline is used + // which intersects parts of the text (descenders). + if (underline != .none) self.addUnderline( + @intCast(x), + @intCast(y), + underline, + style.underlineColor(&state.colors.palette) orelse fg, + alpha, + ) catch |err| { + log.warn( + "error adding underline to cell, will be invalid x={} y={}, err={}", + .{ x, y, err }, + ); + }; + + if (style.flags.overline) self.addOverline(@intCast(x), @intCast(y), fg, alpha) catch |err| { + log.warn( + "error adding overline to cell, will be invalid x={} y={}, err={}", + .{ x, y, err }, + ); + }; + + // If we're at or past the end of our shaper run then + // we need to get the next run from the run iterator. + if (shaper_cells != null and shaper_cells_i >= shaper_cells.?.len) { + shaper_run = try run_iter.next(self.alloc); + shaper_cells = null; + shaper_cells_i = 0; + } + + if (shaper_run) |run| glyphs: { + // If we haven't shaped this run yet, do so. + shaper_cells = shaper_cells orelse + // Try to read the cells from the shaping cache if we can. + self.font_shaper_cache.get(run) orelse + cache: { + // Otherwise we have to shape them. + const new_cells = try self.font_shaper.shape(run); + + // Try to cache them. If caching fails for any reason we + // continue because it is just a performance optimization, + // not a correctness issue. + self.font_shaper_cache.put( + self.alloc, + run, + new_cells, + ) catch |err| { + log.warn( + "error caching font shaping results err={}", + .{err}, + ); + }; + + // The cells we get from direct shaping are always owned + // by the shaper and valid until the next shaping call so + // we can safely use them. + break :cache new_cells; + }; + + const shaped_cells = shaper_cells orelse break :glyphs; + + // If there are no shaper cells for this run, ignore it. + // This can occur for runs of empty cells, and is fine. + if (shaped_cells.len == 0) break :glyphs; + + // If we encounter a shaper cell to the left of the current + // cell then we have some problems. This logic relies on x + // position monotonically increasing. + assert(run.offset + shaped_cells[shaper_cells_i].x >= x); + + // NOTE: An assumption is made here that a single cell will never + // be present in more than one shaper run. If that assumption is + // violated, this logic breaks. + + while (shaper_cells_i < shaped_cells.len and + run.offset + shaped_cells[shaper_cells_i].x == x) : ({ + shaper_cells_i += 1; + }) { + self.addGlyph( + @intCast(x), + @intCast(y), + state.cols, + cells_raw, + shaped_cells[shaper_cells_i], + shaper_run.?, + fg, + alpha, + ) catch |err| { + log.warn( + "error adding glyph to cell, will be invalid x={} y={}, err={}", + .{ x, y, err }, + ); + }; + } + } + + // Finally, draw a strikethrough if necessary. + if (style.flags.strikethrough) self.addStrikethrough( + @intCast(x), + @intCast(y), + fg, + alpha, + ) catch |err| { + log.warn( + "error adding strikethrough to cell, will be invalid x={} y={}, err={}", + .{ x, y, err }, + ); + }; + } + } + /// Add an underline decoration to the specified cell fn addUnderline( self: *Self, From d235c490e9e68b6d90bb0016750b47c5e09b3be6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 20 Jan 2026 12:10:05 -0800 Subject: [PATCH 5/6] renderer: handle rebuildCells failures gracefully --- src/renderer/generic.zig | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/renderer/generic.zig b/src/renderer/generic.zig index bc81fa98e..17ca3f3cd 100644 --- a/src/renderer/generic.zig +++ b/src/renderer/generic.zig @@ -1282,6 +1282,9 @@ pub fn Renderer(comptime GraphicsAPI: type) type { } } + // From this point forward no more errors. + errdefer comptime unreachable; + // Reset our dirty state after updating. defer self.terminal_state.dirty = .false; @@ -1291,7 +1294,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { defer self.draw_mutex.unlock(); // Build our GPU cells - try self.rebuildCells( + self.rebuildCells( critical.preedit, renderer.cursorStyle(&self.terminal_state, .{ .preedit = critical.preedit != null, @@ -1299,7 +1302,13 @@ pub fn Renderer(comptime GraphicsAPI: type) type { .blink_visible = cursor_blink_visible, }), &critical.links, - ); + ) catch |err| { + // This means we weren't able to allocate our buffer + // to update the cells. In this case, we continue with + // our old buffer (frozen contents) and log it. + comptime assert(@TypeOf(err) == error{OutOfMemory}); + log.warn("error rebuilding GPU cells err={}", .{err}); + }; // The scrollbar is only emitted during draws so we also // check the scrollbar cache here and update if needed. @@ -2498,7 +2507,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { preedit: ?renderer.State.Preedit, cursor_style_: ?renderer.CursorStyle, links: *const terminal.RenderState.CellSet, - ) !void { + ) Allocator.Error!void { const state: *terminal.RenderState = &self.terminal_state; // const start = try std.time.Instant.now(); @@ -2566,6 +2575,10 @@ pub fn Renderer(comptime GraphicsAPI: type) type { } } + // From this point on we never fail. We produce some kind of + // working terminal state, even if incorrect. + errdefer comptime unreachable; + // Get our row data from our state const row_data = state.row_data.slice(); const row_raws = row_data.items(.raw); @@ -2603,7 +2616,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { // Unmark the dirty state in our render state. dirty.* = false; - try self.rebuildRow( + self.rebuildRow( y, row, cells, @@ -2611,7 +2624,14 @@ pub fn Renderer(comptime GraphicsAPI: type) type { selection, highlights, links, - ); + ) catch |err| { + // This should never happen except under exceptional + // scenarios. In this case, we don't want to corrupt + // our render state so just clear this row and keep + // trying to finish it out. + log.warn("error building row y={} err={}", .{ y, err }); + self.cells.clear(y); + }; } // Setup our cursor rendering information. From 6ec2bfe2887ab9f81175b9faf0b05e19379896ef Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 20 Jan 2026 12:27:28 -0800 Subject: [PATCH 6/6] renderer: kitty graphics prep can't fail (skip failed conversions) --- src/renderer/generic.zig | 100 +++++++++++++++++++++++++++------------ src/renderer/image.zig | 5 +- 2 files changed, 73 insertions(+), 32 deletions(-) diff --git a/src/renderer/generic.zig b/src/renderer/generic.zig index 17ca3f3cd..e75171721 100644 --- a/src/renderer/generic.zig +++ b/src/renderer/generic.zig @@ -1117,7 +1117,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { self: *Self, state: *renderer.State, cursor_blink_visible: bool, - ) !void { + ) Allocator.Error!void { // We fully deinit and reset the terminal state every so often // so that a particularly large terminal state doesn't cause // the renderer to hold on to retained memory. @@ -1193,7 +1193,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { if (state.terminal.screens.active.kitty_images.dirty or self.image_virtual) { - try self.prepKittyGraphics(state.terminal); + self.prepKittyGraphics(state.terminal); } // Get our OSC8 links we're hovering if we have a mouse. @@ -1712,7 +1712,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { fn prepKittyGraphics( self: *Self, t: *terminal.Terminal, - ) !void { + ) void { self.draw_mutex.lock(); defer self.draw_mutex.unlock(); @@ -1776,16 +1776,32 @@ pub fn Renderer(comptime GraphicsAPI: type) type { continue; }; - try self.prepKittyPlacement(t, top_y, bot_y, &image, p); + self.prepKittyPlacement( + t, + top_y, + bot_y, + &image, + p, + ) catch |err| { + // For errors we log and continue. We try to place + // other placements even if one fails. + log.warn("error preparing kitty placement err={}", .{err}); + }; } // If we have virtual placements then we need to scan for placeholders. if (self.image_virtual) { var v_it = terminal.kitty.graphics.unicode.placementIterator(top, bot); - while (v_it.next()) |virtual_p| try self.prepKittyVirtualPlacement( - t, - &virtual_p, - ); + while (v_it.next()) |virtual_p| { + self.prepKittyVirtualPlacement( + t, + &virtual_p, + ) catch |err| { + // For errors we log and continue. We try to place + // other placements even if one fails. + log.warn("error preparing kitty placement err={}", .{err}); + }; + } } // Sort the placements by their Z value. @@ -1833,7 +1849,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { self: *Self, t: *terminal.Terminal, p: *const terminal.kitty.graphics.unicode.Placement, - ) !void { + ) PrepKittyImageError!void { const storage = &t.screens.active.kitty_images; const image = storage.imageById(p.image_id) orelse { log.warn( @@ -1894,7 +1910,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { bot_y: u32, image: *const terminal.kitty.graphics.Image, p: *const terminal.kitty.graphics.ImageStorage.Placement, - ) !void { + ) PrepKittyImageError!void { // Get the rect for the placement. If this placement doesn't have // a rect then its virtual or something so skip it. const rect = p.rect(image.*, t) orelse return; @@ -1950,12 +1966,17 @@ pub fn Renderer(comptime GraphicsAPI: type) type { } } + const PrepKittyImageError = error{ + OutOfMemory, + ImageConversionError, + }; + /// Prepare the provided image for upload to the GPU by copying its /// data with our allocator and setting it to the pending state. fn prepKittyImage( self: *Self, image: *const terminal.kitty.graphics.Image, - ) !void { + ) PrepKittyImageError!void { // If this image exists and its transmit time is the same we assume // it is the identical image so we don't need to send it to the GPU. const gop = try self.images.getOrPut(self.alloc, image.id); @@ -1966,39 +1987,60 @@ pub fn Renderer(comptime GraphicsAPI: type) type { } // Copy the data into the pending state. - const data = try self.alloc.dupe(u8, image.data); - errdefer self.alloc.free(data); + const data = if (self.alloc.dupe( + u8, + image.data, + )) |v| v else |_| { + if (!gop.found_existing) { + // If this is a new entry we can just remove it since it + // was never sent to the GPU. + _ = self.images.remove(image.id); + } else { + // If this was an existing entry, it is invalid and + // we must unload it. + gop.value_ptr.image.markForUnload(); + } + + return error.OutOfMemory; + }; + // Note: we don't need to errdefer free the data because it is + // put into the map immediately below and our errdefer to + // handle our map state will fix this up. // Store it in the map - const pending: Image.Pending = .{ - .width = image.width, - .height = image.height, - .pixel_format = switch (image.format) { - .gray => .gray, - .gray_alpha => .gray_alpha, - .rgb => .rgb, - .rgba => .rgba, - .png => unreachable, // should be decoded by now + const new_image: Image = .{ + .pending = .{ + .width = image.width, + .height = image.height, + .pixel_format = switch (image.format) { + .gray => .gray, + .gray_alpha => .gray_alpha, + .rgb => .rgb, + .rgba => .rgba, + .png => unreachable, // should be decoded by now + }, + .data = data.ptr, }, - .data = data.ptr, }; - - const new_image: Image = .{ .pending = pending }; - if (!gop.found_existing) { gop.value_ptr.* = .{ .image = new_image, .transmit_time = undefined, }; } else { - try gop.value_ptr.image.markForReplace( + gop.value_ptr.image.markForReplace( self.alloc, new_image, ); } - try gop.value_ptr.image.prepForUpload(self.alloc); + // If any error happens, we unload the image and it is invalid. + errdefer gop.value_ptr.image.markForUnload(); + gop.value_ptr.image.prepForUpload(self.alloc) catch |err| { + log.warn("error preparing kitty image for upload err={}", .{err}); + return error.ImageConversionError; + }; gop.value_ptr.transmit_time = image.transmit_time; } @@ -2090,7 +2132,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { // If we have an existing background image, replace it. // Otherwise, set this as our background image directly. if (self.bg_image) |*img| { - try img.markForReplace(self.alloc, image); + img.markForReplace(self.alloc, image); } else { self.bg_image = image; } diff --git a/src/renderer/image.zig b/src/renderer/image.zig index 7089f5a8b..bf0f7b736 100644 --- a/src/renderer/image.zig +++ b/src/renderer/image.zig @@ -146,7 +146,7 @@ pub const Image = union(enum) { /// Mark the current image to be replaced with a pending one. This will /// attempt to update the existing texture if we have one, otherwise it /// will act like a new upload. - pub fn markForReplace(self: *Image, alloc: Allocator, img: Image) !void { + pub fn markForReplace(self: *Image, alloc: Allocator, img: Image) void { assert(img.isPending()); // If we have pending data right now, free it. @@ -216,9 +216,8 @@ pub const Image = union(enum) { /// Prepare the pending image data for upload to the GPU. /// This doesn't need GPU access so is safe to call any time. - pub fn prepForUpload(self: *Image, alloc: Allocator) !void { + pub fn prepForUpload(self: *Image, alloc: Allocator) wuffs.Error!void { assert(self.isPending()); - try self.convert(alloc); }