From 4f26eb203bd2f6d568cbe103f2a690bd711f37c9 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Tue, 7 May 2024 20:46:20 -0400 Subject: [PATCH] renderer/Metal: cell Contents cleanup Cleaned up naming, some logic changes, added comments. --- src/renderer/Metal.zig | 9 +- src/renderer/metal/cell.zig | 173 +++++++++++++++++++++++------------- 2 files changed, 114 insertions(+), 68 deletions(-) diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig index e57b2add6..b8a513d6c 100644 --- a/src/renderer/Metal.zig +++ b/src/renderer/Metal.zig @@ -566,9 +566,6 @@ pub fn init(alloc: Allocator, options: renderer.Options) !Metal { }; }; - var cells = try mtl_cell.Contents.init(alloc); - errdefer cells.deinit(alloc); - const display_link: ?DisplayLink = switch (builtin.os.tag) { .macos => if (options.config.vsync) try macos.video.DisplayLink.createWithActiveCGDisplays() @@ -592,7 +589,7 @@ pub fn init(alloc: Allocator, options: renderer.Options) !Metal { .current_background_color = options.config.background, // Render state - .cells = cells, + .cells = .{}, .uniforms = .{ .projection_matrix = undefined, .cell_size = undefined, @@ -1037,8 +1034,8 @@ pub fn drawFrame(self: *Metal, surface: *apprt.Surface) !void { // Setup our frame data try frame.uniforms.sync(self.gpu_state.device, &.{self.uniforms}); - const bg_count = try frame.cells_bg.syncFromArrayLists(self.gpu_state.device, self.cells.bgs.pools); - const fg_count = try frame.cells.syncFromArrayLists(self.gpu_state.device, self.cells.text.pools); + const bg_count = try frame.cells_bg.syncFromArrayLists(self.gpu_state.device, self.cells.bg_rows.lists); + const fg_count = try frame.cells.syncFromArrayLists(self.gpu_state.device, self.cells.fg_rows.lists); // If we have custom shaders, update the animation time. if (self.custom_shader_state) |*state| { diff --git a/src/renderer/metal/cell.zig b/src/renderer/metal/cell.zig index 42a693208..a6925645f 100644 --- a/src/renderer/metal/cell.zig +++ b/src/renderer/metal/cell.zig @@ -26,54 +26,41 @@ pub const Key = enum { } }; -/// A collection of ArrayLists with methods for bulk operations. -fn PooledArrayList(comptime T: type) type { +/// A pool of ArrayLists with methods for bulk operations. +fn ArrayListPool(comptime T: type) type { return struct { - pools: []std.ArrayListUnmanaged(T), + const Self = ArrayListPool(T); + const ArrayListT = std.ArrayListUnmanaged(T); - pub fn init(alloc: Allocator, pool_count: usize) !PooledArrayList(T) { - var self: PooledArrayList(T) = .{ - .pools = try alloc.alloc(std.ArrayListUnmanaged(T), pool_count), + // An array containing the lists that belong to this pool. + lists: []ArrayListT = &[_]ArrayListT{}, + + // The pool will be initialized with empty ArrayLists. + pub fn init(alloc: Allocator, list_count: usize, initial_capacity: usize) !Self { + const self: Self = .{ + .lists = try alloc.alloc(ArrayListT, list_count), }; - for (self.pools) |*list| { - list.* = .{}; + for (self.lists) |*list| { + list.* = try ArrayListT.initCapacity(alloc, initial_capacity); } - self.reset(); - return self; } - pub fn deinit(self: *PooledArrayList(T), alloc: Allocator) void { - for (self.pools) |*list| { + pub fn deinit(self: *Self, alloc: Allocator) void { + for (self.lists) |*list| { list.deinit(alloc); } - alloc.free(self.pools); + alloc.free(self.lists); } - /// Reset all pools to an empty state without freeing or resizing. - pub fn reset(self: *PooledArrayList(T)) void { - for (self.pools) |*list| { + /// Clear all lists in the pool. + pub fn reset(self: *Self) void { + for (self.lists) |*list| { list.clearRetainingCapacity(); } } - - /// Change the pool count and clear the contents of all pools. - pub fn resize(self: *PooledArrayList(T), alloc: Allocator, pool_count: u16) !void { - const pools = try alloc.alloc(std.ArrayListUnmanaged(T), pool_count); - errdefer alloc.free(pools); - - alloc.free(self.pools); - - self.pools = pools; - - for (self.pools) |*list| { - list.* = .{}; - } - - self.reset(); - } }; } @@ -83,25 +70,54 @@ fn PooledArrayList(comptime T: type) type { /// clearing of data from the GPU buffers, to allow for row-wise dirty /// tracking to eliminate the overhead of rebuilding the GPU buffers /// each frame. +/// +/// Must be initialized by resizing before calling any operations. pub const Contents = struct { - size: renderer.GridSize, + size: renderer.GridSize = .{ .rows = 0, .columns = 0 }, - bgs: PooledArrayList(mtl_shaders.CellBg), - text: PooledArrayList(mtl_shaders.CellText), + /// The ArrayListPool which holds all of the background cells. When sized + /// with Contents.resize the individual ArrayLists SHOULD be given enough + /// capacity that appendAssumeCapacity may be used, since it should be + /// impossible for a row to have more background cells than columns. + /// + /// HOWEVER, the initial capacity can be exceeded due to multi-glyph + /// composites each adding a background cell for the same position. + /// This should probably be considered a bug, but for now it means + /// that sometimes allocations might happen, so appendAssumeCapacity + /// MUST NOT be used. + /// + /// Rows are indexed as Contents.bg_rows[y]. + /// + /// Must be initialized by calling resize on the Contents struct before + /// calling any operations. + bg_rows: ArrayListPool(mtl_shaders.CellBg) = .{}, - pub fn init(alloc: Allocator) !Contents { - const result: Contents = .{ - .size = .{ .rows = 0, .columns = 0 }, - .bgs = try PooledArrayList(mtl_shaders.CellBg).init(alloc, 0), - .text = try PooledArrayList(mtl_shaders.CellText).init(alloc, 0), - }; - - return result; - } + /// The ArrayListPool which holds all of the foreground cells. When sized + /// with Contents.resize the individual ArrayLists are given enough room + /// that they can hold a single row with #cols glyphs, underlines, and + /// strikethroughs; however, appendAssumeCapacity MUST NOT be used since + /// it is possible to exceed this with combining glyphs that add a glyph + /// but take up no column since they combine with the previous one, as + /// well as with fonts that perform multi-substitutions for glyphs, which + /// can result in a similar situation where multiple glyphs reside in the + /// same column. + /// + /// Allocations should nevertheless be exceedingly rare since hitting the + /// initial capacity of a list would require a row filled with underlined + /// struck through characters, at least one of which is a multi-glyph + /// composite. + /// + /// Rows are indexed as Contents.fg_rows[y + 1], because the first list in + /// the pool is reserved for the cursor, which must be the first item in + /// the buffer. + /// + /// Must be initialized by calling resize on the Contents struct before + /// calling any operations. + fg_rows: ArrayListPool(mtl_shaders.CellText) = .{}, pub fn deinit(self: *Contents, alloc: Allocator) void { - self.bgs.deinit(alloc); - self.text.deinit(alloc); + self.bg_rows.deinit(alloc); + self.fg_rows.deinit(alloc); } /// Resize the cell contents for the given grid size. This will @@ -112,25 +128,58 @@ pub const Contents = struct { size: renderer.GridSize, ) !void { self.size = size; - try self.bgs.resize(alloc, size.rows); - try self.text.resize(alloc, size.rows + 1); - // Make sure we don't have to allocate for the cursor cell. - try self.text.pools[0].ensureTotalCapacity(alloc, 1); + // When we create our bg_rows pool, we give the lists an initial + // capacity of size.columns. This is to account for the usual case + // where you have a row with normal text and background colors. + // This can be exceeded due to multi-glyph composites each adding + // a background cell for the same position. This should probably be + // considered a bug, but for now it means that sometimes allocations + // might happen, and appendAssumeCapacity MUST NOT be used. + var bg_rows = try ArrayListPool(mtl_shaders.CellBg).init(alloc, size.rows, size.columns); + errdefer bg_rows.deinit(alloc); + + // The foreground lists can hold 3 types of items: + // - Glyphs + // - Underlines + // - Strikethroughs + // So we give them an initial capacity of size.columns * 3, which will + // avoid any further allocations in the vast majority of cases. Sadly + // we can not assume capacity though, since with combining glyphs that + // form a single grapheme, and multi-substitutions in fonts, the number + // of glyphs in a row is theoretically unlimited. + // + // We have size.rows + 1 lists because index 0 is used for a special + // list containing the cursor cell which needs to be first in the buffer. + var fg_rows = try ArrayListPool(mtl_shaders.CellText).init(alloc, size.rows + 1, size.columns * 3); + errdefer fg_rows.deinit(alloc); + + self.bg_rows.deinit(alloc); + self.fg_rows.deinit(alloc); + + self.bg_rows = bg_rows; + self.fg_rows = fg_rows; + + // We don't need 3*cols worth of cells for the cursor list, so we can + // replace it with a smaller list. 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(mtl_shaders.CellText).initCapacity(alloc, 1); } /// Reset the cell contents to an empty state without resizing. pub fn reset(self: *Contents) void { - self.bgs.reset(); - self.text.reset(); + self.bg_rows.reset(); + self.fg_rows.reset(); } /// Set the cursor value. If the value is null then the cursor is hidden. pub fn setCursor(self: *Contents, v: ?mtl_shaders.CellText) void { - self.text.pools[0].clearRetainingCapacity(); + self.fg_rows.lists[0].clearRetainingCapacity(); if (v) |cell| { - self.text.pools[0].appendAssumeCapacity(cell); + self.fg_rows.lists[0].appendAssumeCapacity(cell); } } @@ -147,25 +196,25 @@ pub const Contents = struct { switch (key) { .bg - => try self.bgs.pools[y].append(alloc, cell), + => try self.bg_rows.lists[y].append(alloc, cell), .text, .underline, .strikethrough - // We have a special pool containing the cursor cell at the start - // of our text pool list, so we need to add 1 to the y to get the + // We have a special list containing the cursor cell at the start + // of our fg row pool, so we need to add 1 to the y to get the // correct index. - => try self.text.pools[y + 1].append(alloc, cell), + => try self.fg_rows.lists[y + 1].append(alloc, cell), } } /// Clear all of the cell contents for a given row. pub fn clear(self: *Contents, y: terminal.size.CellCountInt) void { - self.bgs.pools[y].clearRetainingCapacity(); - // We have a special pool containing the cursor cell at the start - // of our text pool list, so we need to add 1 to the y to get the + self.bg_rows.lists[y].clearRetainingCapacity(); + // We have a special list containing the cursor cell at the start + // of our fg row pool, so we need to add 1 to the y to get the // correct index. - self.text.pools[y + 1].clearRetainingCapacity(); + self.fg_rows.lists[y + 1].clearRetainingCapacity(); } };