From efa3c66ed16ea9251d5b2a74b606aabc248d2c00 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Sep 2026 13:58:07 -0700 Subject: [PATCH 1/4] terminal: PageList nodes are pooled individually from the gpa instead of an arena --- src/terminal/PageList.zig | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 6c4d892c1..39a48fb78 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -32,14 +32,17 @@ const log = std.log.scoped(.page_list); const native_freestanding = builtin.os.tag == .freestanding and !builtin.target.cpu.arch.isWasm(); -/// The number of PageList.Nodes we preheat the pool with. A node is -/// a very small struct so we can afford to preheat many, but the exact -/// number is uncertain. Any number too large is wasting memory, any number -/// too small will cause the pool to have to allocate more memory later. -/// This should be set to some reasonable minimum that we expect a terminal -/// window to scroll into quickly. +/// The number of pages we preheat the page pool with. For operating systems +/// that support it, pages are demand-paged (see PagePool) so this only +/// costs us address space. For other operating systems, we don't preheat. const page_preheat = 4; +/// The number of nodes we preheat the node pool with. Unlike pages, nodes +/// are ordinary heap memory so every idle preheated node costs real +/// memory. A new PageList needs exactly one node for its first page, so +/// we preheat that one and let the pool grow on demand. +const node_preheat = 1; + /// The list of pages in the screen. These are expected to be in order /// where the first page is the topmost page (scrollback) and the last is /// the bottommost page (the current active page). @@ -299,7 +302,14 @@ const Node = struct { }; /// The memory pool we get page nodes from. -const NodePool = std.heap.memory_pool.Managed(List.Node); +/// +/// We don't use a std memory pool here because it is backed by an arena +/// and we end up paying a lot of wasted memory for the growth factor when +/// in practice we don't usually use many nodes. +/// +/// We don't need the "untouched" property of our UntouchedPool but +/// this gives us a GPA-allocated pool so we reuse it here. +const NodePool = datastruct.UntouchedPool(List.Node, .of(List.Node)); /// The standard page capacity that we use as a starting point for /// all pages. This is chosen as a sane default that fits most terminal @@ -356,7 +366,7 @@ pub const MemoryPool = struct { page_alloc: Allocator, preheat: usize, ) Allocator.Error!MemoryPool { - var node_pool = try NodePool.initCapacity(gen_alloc, preheat); + var node_pool = try NodePool.initCapacity(gen_alloc, gen_alloc, node_preheat); errdefer node_pool.deinit(); var page_pool = try PagePool.initCapacity(gen_alloc, page_alloc, preheat); errdefer page_pool.deinit(); @@ -899,18 +909,19 @@ fn verifyIntegrity(self: *const PageList) IntegrityError!void { /// Release every page in the list during a teardown walk (deinit, /// reset, or an errdefer unwinding a partially built list): heap-owned -/// pages go back to the page allocator and pool-owned pages back to the -/// page pool. The nodes themselves are not touched; they live in the -/// node pool. +/// pages go back to the page allocator, pool-owned pages back to the +/// page pool, and the nodes back to the node pool's free list. fn releasePages(pool: *MemoryPool, list: List) void { const page_alloc = pool.pages.allocator; var it = list.first; - while (it) |node| : (it = node.next) { + while (it) |node| { + it = node.next; const page = node.restore(.discard); switch (node.owned) { .pool => releasePoolPage(pool, page), .heap => page_alloc.free(page.memory), } + pool.nodes.destroy(node); } } @@ -943,8 +954,7 @@ pub fn deinit(self: *PageList) void { // Always deallocate our hashmap. self.tracked_pins.deinit(self.pool.alloc); - // Release every page. We don't need to deallocate the list or - // nodes because they all reside in the pool. + // Release every page and node back to the pools, then free the pools. releasePages(&self.pool, self.pages); self.pool.deinit(); } From 35a6fb747f5d60ee6d92a7f5667474914a47cf21 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Sep 2026 13:59:43 -0700 Subject: [PATCH 2/4] terminal: PageList preheats only the viewport and cursor pins --- src/terminal/PageList.zig | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 39a48fb78..746a6cf8c 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -43,6 +43,11 @@ const page_preheat = 4; /// we preheat that one and let the pool grow on demand. const node_preheat = 1; +/// The number of pins we preheat the pin pool with: the viewport pin that +/// every PageList tracks and the cursor pin that every Screen tracks. +/// Selections, searches, and so on grow the pool on demand. +const pin_preheat = 2; + /// The list of pages in the screen. These are expected to be in order /// where the first page is the topmost page (scrollback) and the last is /// the bottommost page (the current active page). @@ -370,7 +375,7 @@ pub const MemoryPool = struct { errdefer node_pool.deinit(); var page_pool = try PagePool.initCapacity(gen_alloc, page_alloc, preheat); errdefer page_pool.deinit(); - var pin_pool = try PinPool.initCapacity(gen_alloc, 8); + var pin_pool = try PinPool.initCapacity(gen_alloc, pin_preheat); errdefer pin_pool.deinit(); return .{ .alloc = gen_alloc, @@ -664,11 +669,10 @@ pub fn init( try tw.check(.viewport_pin); const viewport_pin = try pool.pins.create(); viewport_pin.* = .{ .node = page_list.first.? }; - var tracked_pins: PinSet = .{}; - errdefer tracked_pins.deinit(pool.alloc); try tw.check(.viewport_pin_track); - try tracked_pins.putNoClobber(pool.alloc, viewport_pin, {}); + var tracked_pins = try initTrackedPins(pool.alloc, viewport_pin); + errdefer tracked_pins.deinit(pool.alloc); errdefer comptime unreachable; const result: PageList = .{ @@ -690,6 +694,17 @@ pub fn init( return result; } +/// Create the tracked pin set for a new PageList with the viewport pin +/// already tracked. The set is sized for exactly the viewport pin and the +/// cursor pin that every Screen tracks. +fn initTrackedPins(alloc: Allocator, viewport_pin: *Pin) Allocator.Error!PinSet { + var set: PinSet = .{}; + errdefer set.deinit(alloc); + try set.entries.setCapacity(alloc, pin_preheat); + set.putAssumeCapacityNoClobber(viewport_pin, {}); + return set; +} + const initPages_tw = tripwire.module(enum { page_node, page_buf_std, @@ -1098,9 +1113,8 @@ pub fn clone( // Create our viewport. In a clone, the viewport always goes // to the top. const viewport_pin = try pool.pins.create(); - var tracked_pins: PinSet = .{}; + var tracked_pins = try initTrackedPins(pool.alloc, viewport_pin); errdefer tracked_pins.deinit(pool.alloc); - try tracked_pins.putNoClobber(pool.alloc, viewport_pin, {}); // Our list of pages var page_list: List = .{}; @@ -7560,9 +7574,8 @@ pub const Builder = struct { viewport_pin.* = active_top; // Setup our one viewport tracked pin - var tracked_pins: PinSet = .{}; + var tracked_pins = try initTrackedPins(self.pool.alloc, viewport_pin); errdefer tracked_pins.deinit(self.pool.alloc); - try tracked_pins.putNoClobber(self.pool.alloc, viewport_pin, {}); // Initialize limits var limits: Limits = .init(self.options.cols, self.options.rows); From 105d0a5453654f15cbe9e902df9e72ed62fa5474 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Sep 2026 14:01:39 -0700 Subject: [PATCH 3/4] terminal: C terminal wrapper allocates the kitty temp dir path only when it is set --- src/terminal/c/terminal.zig | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/terminal/c/terminal.zig b/src/terminal/c/terminal.zig index 5bf2c750c..aed48e25f 100644 --- a/src/terminal/c/terminal.zig +++ b/src/terminal/c/terminal.zig @@ -105,9 +105,10 @@ const TerminalWrapper = struct { /// created by `new` or transferred from snapshot decoding until `free`. /// Freestanding owners contain no native allocation and expose failing I/O. io: Io, - /// We also need to store a temp dir path for some operations (e.g., kitty - /// graphics). This provides stable storage for the API calls. - tmp_dir_path: [max_path_bytes]u8, + /// Allocator-owned copy of the temporary directory path for some + /// operations (e.g. kitty graphics). This is only allocated once the + /// embedder sets the option. + tmp_dir_path: ?[]u8 = null, /// The terminfo name reported for XTGETTCAP "TN". The stream handler holds /// a slice into this. terminfo_name_buf: [Handler.max_terminfo_name_bytes]u8, @@ -728,7 +729,6 @@ fn wrap( wrapper.* = .{ .terminal = t, .io = io, - .tmp_dir_path = undefined, .terminfo_name_buf = undefined, .stream = Stream.init(.{ .allocator = alloc, @@ -1393,22 +1393,31 @@ fn setTyped( }, .kitty_image_medium_temp_file => { if (comptime !build_options.kitty_graphics) return .success; + const alloc = wrapper.terminal.gpa(); if (value) |v| { - if (v.len > wrapper.tmp_dir_path.len) return .out_of_memory; - @memcpy(wrapper.tmp_dir_path[0..v.len], v.ptr[0..v.len]); + if (v.len > max_path_bytes) return .out_of_memory; + const path = alloc.dupe(u8, v.ptr[0..v.len]) catch + return .out_of_memory; var it = wrapper.terminal.screens.all.iterator(); while (it.next()) |entry| { const screen = entry.value.*; screen.kitty_images.image_limits.temporary_file = .{ - .enabled = .{ .directory = wrapper.tmp_dir_path[0..v.len] }, + .enabled = .{ .directory = path }, }; } + + // Every screen points at the new copy now so the previous + // one can be released. + if (wrapper.tmp_dir_path) |old| alloc.free(old); + wrapper.tmp_dir_path = path; } else { var it = wrapper.terminal.screens.all.iterator(); while (it.next()) |entry| { const screen = entry.value.*; screen.kitty_images.image_limits.temporary_file = .disabled; } + if (wrapper.tmp_dir_path) |old| alloc.free(old); + wrapper.tmp_dir_path = null; } }, .apc_max_bytes => { @@ -1861,6 +1870,7 @@ pub fn free(terminal_: Terminal) callconv(lib.calling_conv) void { wrapper.searches.deinit(alloc); wrapper.stream.deinit(); t.deinit(alloc); + if (wrapper.tmp_dir_path) |path| alloc.free(path); wrapper.io.deinit(alloc); alloc.destroy(t); alloc.destroy(wrapper); From d1cd56a56c4244d9d9fadae028cbffc23a1d3d1a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Sep 2026 15:17:51 -0700 Subject: [PATCH 4/4] terminal: dynamic palette shares the built-in default instead of copying it per terminal --- src/terminal/Terminal.zig | 1 + src/terminal/c/terminal.zig | 5 +- src/terminal/color.zig | 167 ++++++++++++++++++++++++----- src/terminal/snapshot/terminal.zig | 22 ++-- src/termio/Termio.zig | 18 +++- 5 files changed, 169 insertions(+), 44 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 93cad0ca8..e9cc3b8e6 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -356,6 +356,7 @@ pub fn init( pub fn deinit(self: *Terminal, alloc: Allocator) void { self.tabstops.deinit(alloc); self.screens.deinit(alloc); + self.colors.palette.deinit(alloc); self.pwd.deinit(alloc); self.title.deinit(alloc); self.glyph_glossary.deinit(alloc); diff --git a/src/terminal/c/terminal.zig b/src/terminal/c/terminal.zig index aed48e25f..4f72bcbdd 100644 --- a/src/terminal/c/terminal.zig +++ b/src/terminal/c/terminal.zig @@ -1363,8 +1363,9 @@ fn setTyped( }, .color_palette => { wrapper.terminal.colors.palette.changeDefault( + wrapper.terminal.gpa(), if (value) |v| color.paletteZval(v) else color.default, - ); + ) catch return .out_of_memory; wrapper.terminal.flags.dirty.palette = true; }, .kitty_image_storage_limit => { @@ -1736,7 +1737,7 @@ fn getTyped( .color_background_default => out.* = (t.colors.background.default orelse return .no_value).cval(), .color_cursor_default => out.* = (t.colors.cursor.default orelse return .no_value).cval(), .color_palette => out.* = color.paletteCval(&t.colors.palette.current), - .color_palette_default => out.* = color.paletteCval(&t.colors.palette.original), + .color_palette_default => out.* = color.paletteCval(t.colors.palette.original), .kitty_image_storage_limit => { if (comptime !build_options.kitty_graphics) return .no_value; out.* = @intCast(t.screens.active.kitty_images.total_limit); diff --git a/src/terminal/color.zig b/src/terminal/color.zig index ce4433bbc..99d749890 100644 --- a/src/terminal/color.zig +++ b/src/terminal/color.zig @@ -2,6 +2,7 @@ const colorpkg = @This(); const std = @import("std"); const builtin = @import("builtin"); +const Allocator = std.mem.Allocator; const assert = @import("../quirks.zig").inlineAssert; const fraction = @import("fraction.zig"); const x11_color = @import("x11_color.zig"); @@ -322,22 +323,37 @@ pub const DynamicPalette = struct { /// The current palette including any user modifications. current: Palette, - /// The original/default palette values. - original: Palette, + /// The original/default palette values. This points at the shared + /// built-in default palette unless a different default was given + /// (see `changeDefault`), in which case it points at an + /// allocator-owned copy that `deinit` frees. Sharing the built-in + /// default saves a full palette copy for every terminal. + original: *const Palette, /// A bitset where each bit represents whether the corresponding /// palette index has been modified from its default value. mask: PaletteMask, - pub const default: DynamicPalette = .init(colorpkg.default); + /// A dynamic palette using the built-in default palette. This owns + /// no memory, but `deinit` is still safe to call on it. + pub const default: DynamicPalette = .{ + .current = colorpkg.default, + .original = &colorpkg.default, + .mask = .initEmpty(), + }; - /// Initialize a dynamic palette with a default palette. - pub fn init(def: Palette) DynamicPalette { - return .{ - .current = def, - .original = def, - .mask = .initEmpty(), - }; + /// Initialize a dynamic palette with a default palette. The result + /// must be released with `deinit`. No memory is allocated if `def` + /// is the built-in default palette. + pub fn init(alloc: Allocator, def: Palette) Allocator.Error!DynamicPalette { + var result: DynamicPalette = .default; + try result.changeDefault(alloc, def); + return result; + } + + pub fn deinit(self: *DynamicPalette, alloc: Allocator) void { + if (self.ownedOriginal()) |owned| alloc.destroy(owned); + self.* = undefined; } /// Set a custom color at the given palette index. @@ -354,16 +370,46 @@ pub const DynamicPalette = struct { /// Reset all colors to their original values. pub fn resetAll(self: *DynamicPalette) void { - self.* = .init(self.original); + self.current = self.original.*; + self.mask = .initEmpty(); } /// Change the default palette, but preserve the changed values. - pub fn changeDefault(self: *DynamicPalette, def: Palette) void { - self.original = def; + /// + /// The built-in default palette is shared rather than copied, so + /// changing back to it releases any owned copy (see `resetDefault`). + /// Any other default is copied into memory owned by this palette, + /// allocated on the first change and reused after that. On allocation + /// failure nothing changes. + pub fn changeDefault( + self: *DynamicPalette, + alloc: Allocator, + def: Palette, + ) Allocator.Error!void { + if (std.meta.eql(def, colorpkg.default)) return self.resetDefault(alloc); + const owned = self.ownedOriginal() orelse try alloc.create(Palette); + owned.* = def; + self.original = owned; + self.applyDefault(def); + } + + /// Change the default palette back to the built-in default palette, + /// preserving the changed values. This releases any owned copy and + /// never allocates, so it is safe to use as the fallback when + /// `changeDefault` fails. + pub fn resetDefault(self: *DynamicPalette, alloc: Allocator) void { + if (self.ownedOriginal()) |owned| alloc.destroy(owned); + self.original = &colorpkg.default; + self.applyDefault(colorpkg.default); + } + + /// Rebuild the current palette from a new default, preserving the + /// changed values. + fn applyDefault(self: *DynamicPalette, def: Palette) void { // Fast path, the palette is usually not changed. if (self.mask.count() == 0) { - self.current = self.original; + self.current = def; return; } @@ -374,6 +420,16 @@ pub const DynamicPalette = struct { while (it.next()) |idx| current[idx] = self.current[idx]; self.current = current; } + + /// The allocator-owned copy of the original palette, or null if the + /// original is the shared built-in default. + fn ownedOriginal(self: *const DynamicPalette) ?*Palette { + if (self.original == &colorpkg.default) return null; + + // Only the shared built-in default is ever const; anything else + // is a copy that we allocated ourselves. + return @constCast(self.original); + } }; /// RGB value that can be changed and reset. This can also be totally unset @@ -991,16 +1047,18 @@ test "RGB: encode" { test "DynamicPalette: init" { const testing = std.testing; - var p: DynamicPalette = .init(default); + var p: DynamicPalette = .default; + defer p.deinit(testing.allocator); try testing.expectEqual(default, p.current); - try testing.expectEqual(default, p.original); + try testing.expectEqual(default, p.original.*); + try testing.expectEqual(&default, p.original); try testing.expectEqual(@as(usize, 0), p.mask.count()); } test "DynamicPalette: set" { const testing = std.testing; - var p: DynamicPalette = .init(default); + var p: DynamicPalette = .default; const new_color = RGB{ .r = 255, .g = 0, .b = 0 }; p.set(0, new_color); @@ -1014,7 +1072,7 @@ test "DynamicPalette: set" { test "DynamicPalette: reset" { const testing = std.testing; - var p: DynamicPalette = .init(default); + var p: DynamicPalette = .default; const new_color = RGB{ .r = 255, .g = 0, .b = 0 }; p.set(0, new_color); @@ -1029,7 +1087,7 @@ test "DynamicPalette: reset" { test "DynamicPalette: resetAll" { const testing = std.testing; - var p: DynamicPalette = .init(default); + var p: DynamicPalette = .default; const new_color = RGB{ .r = 255, .g = 0, .b = 0 }; p.set(0, new_color); @@ -1039,27 +1097,79 @@ test "DynamicPalette: resetAll" { p.resetAll(); try testing.expectEqual(default, p.current); - try testing.expectEqual(default, p.original); + try testing.expectEqual(default, p.original.*); try testing.expectEqual(@as(usize, 0), p.mask.count()); } test "DynamicPalette: changeDefault with no changes" { const testing = std.testing; - var p: DynamicPalette = .init(default); + var p: DynamicPalette = .default; + defer p.deinit(testing.allocator); var new_palette = default; new_palette[0] = RGB{ .r = 100, .g = 100, .b = 100 }; - p.changeDefault(new_palette); - try testing.expectEqual(new_palette, p.original); + try p.changeDefault(testing.allocator, new_palette); + try testing.expectEqual(new_palette, p.original.*); try testing.expectEqual(new_palette, p.current); try testing.expectEqual(@as(usize, 0), p.mask.count()); } +test "DynamicPalette: changeDefault shares the built-in default" { + const testing = std.testing; + const alloc = testing.allocator; + + var new_palette = default; + new_palette[0] = RGB{ .r = 100, .g = 100, .b = 100 }; + + // A custom default is an owned copy. + var p: DynamicPalette = try .init(alloc, new_palette); + defer p.deinit(alloc); + try testing.expect(p.original != &default); + try testing.expectEqual(new_palette, p.original.*); + + // Changing the custom default reuses the copy. + const owned = p.original; + new_palette[1] = RGB{ .r = 101, .g = 101, .b = 101 }; + try p.changeDefault(alloc, new_palette); + try testing.expectEqual(owned, p.original); + try testing.expectEqual(new_palette, p.original.*); + + // Changing back to the built-in default releases the copy (the + // testing allocator would report the leak) and shares it again. + try p.changeDefault(alloc, default); + try testing.expectEqual(&default, p.original); + try testing.expectEqual(default, p.current); + + // resetDefault does the same directly, preserving changed values, + // and never allocates. + try p.changeDefault(alloc, new_palette); + p.set(2, RGB{ .r = 1, .g = 2, .b = 3 }); + var reset_failing: std.testing.FailingAllocator = .init(alloc, .{ + .fail_index = 0, + }); + p.resetDefault(reset_failing.allocator()); + try testing.expect(!reset_failing.has_induced_failure); + try testing.expectEqual(&default, p.original); + try testing.expectEqual(default[0], p.current[0]); + try testing.expectEqual(RGB{ .r = 1, .g = 2, .b = 3 }, p.current[2]); + try testing.expect(p.mask.isSet(2)); + + // The built-in default never allocates. + var failing: std.testing.FailingAllocator = .init(alloc, .{ + .fail_index = 0, + }); + var q: DynamicPalette = try .init(failing.allocator(), default); + defer q.deinit(failing.allocator()); + try testing.expectEqual(&default, q.original); + try testing.expect(!failing.has_induced_failure); +} + test "DynamicPalette: changeDefault preserves changes" { const testing = std.testing; - var p: DynamicPalette = .init(default); + var p: DynamicPalette = .default; + defer p.deinit(testing.allocator); const custom_color = RGB{ .r = 255, .g = 0, .b = 0 }; p.set(5, custom_color); @@ -1069,9 +1179,9 @@ test "DynamicPalette: changeDefault preserves changes" { new_palette[0] = RGB{ .r = 100, .g = 100, .b = 100 }; new_palette[5] = RGB{ .r = 50, .g = 50, .b = 50 }; - p.changeDefault(new_palette); + try p.changeDefault(testing.allocator, new_palette); - try testing.expectEqual(new_palette, p.original); + try testing.expectEqual(new_palette, p.original.*); try testing.expectEqual(new_palette[0], p.current[0]); try testing.expectEqual(custom_color, p.current[5]); try testing.expect(p.mask.isSet(5)); @@ -1081,7 +1191,8 @@ test "DynamicPalette: changeDefault preserves changes" { test "DynamicPalette: changeDefault with multiple changes" { const testing = std.testing; - var p: DynamicPalette = .init(default); + var p: DynamicPalette = .default; + defer p.deinit(testing.allocator); const red = RGB{ .r = 255, .g = 0, .b = 0 }; const green = RGB{ .r = 0, .g = 255, .b = 0 }; const blue = RGB{ .r = 0, .g = 0, .b = 255 }; @@ -1094,7 +1205,7 @@ test "DynamicPalette: changeDefault with multiple changes" { new_palette[0] = RGB{ .r = 50, .g = 50, .b = 50 }; new_palette[1] = RGB{ .r = 60, .g = 60, .b = 60 }; - p.changeDefault(new_palette); + try p.changeDefault(testing.allocator, new_palette); try testing.expectEqual(new_palette[0], p.current[0]); try testing.expectEqual(red, p.current[1]); diff --git a/src/terminal/snapshot/terminal.zig b/src/terminal/snapshot/terminal.zig index 11c9004bb..19305372a 100644 --- a/src/terminal/snapshot/terminal.zig +++ b/src/terminal/snapshot/terminal.zig @@ -761,7 +761,7 @@ pub const Header = struct { /// Allocator-owned semantic state decoded from one TERMINAL payload. /// -/// The palette and fixed header are values. `tabstops`, `pwd`, and `title` +/// The fixed header is a value. `tabstops`, `palette`, `pwd`, and `title` /// own allocations and are released by `deinit`. const DecodedPayload = struct { header: Header, @@ -772,6 +772,7 @@ const DecodedPayload = struct { fn deinit(self: *DecodedPayload, alloc: Allocator) void { self.tabstops.deinit(alloc); + self.palette.deinit(alloc); alloc.free(self.pwd); alloc.free(self.title); self.* = undefined; @@ -874,7 +875,8 @@ fn decodePayload( var override_mask: [32]u8 = undefined; try reader.readSliceAll(&override_mask); - var palette: terminal_color.DynamicPalette = .init(original); + var palette: terminal_color.DynamicPalette = try .init(alloc, original); + errdefer palette.deinit(alloc); for (0..256) |index| { const mask = @as(u8, 1) << @intCast(index % 8); if (override_mask[index / 8] & mask != 0) { @@ -992,6 +994,10 @@ pub fn decode( }); errdefer result.deinit(alloc); + // The terminal owns the decoded palette now, so the payload must not + // release it. + payload.palette = .default; + // Recreate the optional screen now so ScreenSet routing is complete before // its empty screens are replaced by the following SCREEN records. if (header.screen_count == 2) { @@ -1445,9 +1451,7 @@ test "TERMINAL payload round trip" { tabstops.set(test_header.columns - 1); // Use overrides at both ends of the palette to make ordering explicit. - var palette: terminal_color.DynamicPalette = .init( - terminal_color.default, - ); + var palette: terminal_color.DynamicPalette = .default; palette.set(0, .{ .r = 1, .g = 2, .b = 3 }); palette.set(255, .{ .r = 4, .g = 5, .b = 6 }); @@ -1512,9 +1516,7 @@ test "TERMINAL payload rejects noncanonical state" { 0, ); defer tabstops.deinit(testing.allocator); - var palette: terminal_color.DynamicPalette = .init( - terminal_color.default, - ); + var palette: terminal_color.DynamicPalette = .default; var encoded: [2048]u8 = undefined; var writer: std.Io.Writer = .fixed(&encoded); try testing.expectError( @@ -1557,9 +1559,7 @@ test "TERMINAL payload ignores tab-stop padding" { 0, ); defer tabstops.deinit(testing.allocator); - var palette: terminal_color.DynamicPalette = .init( - terminal_color.default, - ); + var palette: terminal_color.DynamicPalette = .default; palette.set(1, .{ .r = 1, .g = 2, .b = 3 }); var destination: std.Io.Writer.Allocating = .init(testing.allocator); diff --git a/src/termio/Termio.zig b/src/termio/Termio.zig index 7d9fd45ff..d08f2e5ff 100644 --- a/src/termio/Termio.zig +++ b/src/termio/Termio.zig @@ -270,7 +270,7 @@ pub fn init(self: *Termio, alloc: Allocator, opts: termio.Options) !void { const rgb = color.toTerminalRGB() orelse break :cursor .unset; break :cursor .init(rgb); }, - .palette = .init(opts.config.palette), + .palette = .default, }, .kitty_image_storage_limit = opts.config.image_storage_limit, .kitty_image_loading_limits = .allWithTempDir(global.tmpDirPath()), @@ -278,6 +278,10 @@ pub fn init(self: *Termio, alloc: Allocator, opts: termio.Options) !void { }); errdefer term.deinit(alloc); + // The default palette may be an allocator-owned copy, so it is set + // once the terminal owns its memory and can release it on deinit. + try term.colors.palette.changeDefault(alloc, opts.config.palette); + // Setup our terminal size in pixels for certain requests. term.width_px = term.cols * opts.size.cell.width; term.height_px = term.rows * opts.size.cell.height; @@ -463,8 +467,16 @@ pub fn changeConfig(self: *Termio, td: *ThreadData, config: *DerivedConfig) !voi // - command, working-directory: we never restart the underlying // process so we don't care or need to know about these. - // Update the default palette. - self.terminal.colors.palette.changeDefault(config.palette); + // Update the default palette. A config change must not fail here, so + // if we can't allocate the copy of the configured palette we fall back + // to the built-in default, which never allocates. + self.terminal.colors.palette.changeDefault( + self.alloc, + config.palette, + ) catch |err| { + log.warn("error changing default palette, using built-in default err={}", .{err}); + self.terminal.colors.palette.resetDefault(self.alloc); + }; self.terminal.flags.dirty.palette = true; // Update all our other colors