renderer/metal: swap to new cell contents map

src/renderer/Metal.zig

@@ -90,8 +90,8 @@ current_background_color: terminal.color.RGB,
 /// 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.
-cells_bg: std.ArrayListUnmanaged(mtl_shaders.CellBg),
-cells_text: std.ArrayListUnmanaged(mtl_shaders.CellText),
+// cells_bg: std.ArrayListUnmanaged(mtl_shaders.CellBg),
+// cells_text: std.ArrayListUnmanaged(mtl_shaders.CellText),
 cells: mtl_cell.Contents,
 
 /// The current GPU uniform values.
@@ -557,8 +557,6 @@ pub fn init(alloc: Allocator, options: renderer.Options) !Metal {
         .current_background_color = options.config.background,
 
         // Render state
-        .cells_bg = .{},
-        .cells_text = .{},
         .cells = .{},
         .uniforms = .{
             .projection_matrix = undefined,
@@ -584,8 +582,6 @@ pub fn deinit(self: *Metal) void {
     self.gpu_state.deinit();
 
     self.cells.deinit(self.alloc);
-    self.cells_bg.deinit(self.alloc);
-    self.cells_text.deinit(self.alloc);
 
     self.font_shaper.deinit();
 
@@ -776,13 +772,13 @@ pub fn updateFrame(
     }
 
     // Build our GPU cells (OLD)
-    try self.rebuildCells(
-        &critical.screen,
-        critical.mouse,
-        critical.preedit,
-        critical.cursor_style,
-        &critical.color_palette,
-    );
+    // try self.rebuildCells(
+    //     &critical.screen,
+    //     critical.mouse,
+    //     critical.preedit,
+    //     critical.cursor_style,
+    //     &critical.color_palette,
+    // );
 
     // Build our GPU cells
     try self.rebuildCells2(
@@ -834,8 +830,8 @@ pub fn drawFrame(self: *Metal, surface: *apprt.Surface) !void {
 
     // Setup our frame data
     try frame.uniforms.sync(self.gpu_state.device, &.{self.uniforms});
-    try frame.cells_bg.sync(self.gpu_state.device, self.cells_bg.items);
-    try frame.cells.sync(self.gpu_state.device, self.cells_text.items);
+    try frame.cells_bg.sync(self.gpu_state.device, self.cells.bgs.items);
+    try frame.cells.sync(self.gpu_state.device, self.cells.text.items);
 
     // If we have custom shaders, update the animation time.
     if (self.custom_shader_state) |*state| {
@@ -934,13 +930,13 @@ pub fn drawFrame(self: *Metal, surface: *apprt.Surface) !void {
         try self.drawImagePlacements(encoder, self.image_placements.items[0..self.image_bg_end]);
 
         // Then draw background cells
-        try self.drawCellBgs(encoder, frame, self.cells_bg.items.len);
+        try self.drawCellBgs(encoder, frame, self.cells.bgs.items.len);
 
         // Then draw images under text
         try self.drawImagePlacements(encoder, self.image_placements.items[self.image_bg_end..self.image_text_end]);
 
         // Then draw fg cells
-        try self.drawCellFgs(encoder, frame, self.cells_text.items.len);
+        try self.drawCellFgs(encoder, frame, self.cells.text.items.len);
 
         // Then draw remaining images
         try self.drawImagePlacements(encoder, self.image_placements.items[self.image_text_end..]);
@@ -1587,12 +1583,6 @@ pub fn setScreenSize(
         .min_contrast = old.min_contrast,
     };
 
-    // Reset our buffer sizes so that we free memory when the screen shrinks.
-    // This could be made more clever by only doing this when the screen
-    // shrinks but the performance cost really isn't that much.
-    self.cells_text.clearAndFree(self.alloc);
-    self.cells_bg.clearAndFree(self.alloc);
-
     // Reset our cell contents.
     try self.cells.resize(self.alloc, grid_size);
 
@@ -1892,8 +1882,8 @@ fn rebuildCells2(
     // 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: usize,
-        x: [2]usize,
+        y: terminal.size.CellCountInt,
+        x: [2]terminal.size.CellCountInt,
         cp_offset: usize,
     } = if (preedit) |preedit_v| preedit: {
         const range = preedit_v.range(screen.cursor.x, screen.pages.cols - 1);
@@ -1912,6 +1902,9 @@ fn rebuildCells2(
     while (row_it.next()) |row| {
         defer y += 1;
 
+        // If we're rebuilding a row, then we always clear the cells
+        self.cells.clear(y);
+
         // True if we want to do font shaping around the cursor. We want to
         // do font shaping as long as the cursor is enabled.
         const shape_cursor = screen.viewportIsBottom() and

src/renderer/metal/cell.zig

@@ -151,7 +151,7 @@ pub const Contents = struct {
                 const coord_: ?terminal.Coordinate = switch (entry.key) {
                     .bg => bg: {
                         _ = self.bgs.swapRemove(original_index);
-                        if (self.bgs.items.len == 0) break :bg null;
+                        if (self.bgs.items.len == original_index) break :bg null;
                         const new = self.bgs.items[original_index];
                         break :bg .{ .x = new.grid_pos[0], .y = new.grid_pos[1] };
                     },
@@ -161,7 +161,7 @@ pub const Contents = struct {
                     .strikethrough,
                     => text: {
                         _ = self.text.swapRemove(original_index);
-                        if (self.text.items.len == 0) break :text null;
+                        if (self.text.items.len == original_index) break :text null;
                         const new = self.text.items[original_index];
                         break :text .{ .x = new.grid_pos[0], .y = new.grid_pos[1] };
                     },
@@ -175,9 +175,19 @@ pub const Contents = struct {
                 // removing the last element in the array, then nothing
                 // is swapped in and nothing needs to be updated.
                 if (coord_) |coord| {
-                    const mapping = self.map[self.index(coord)].array.getPtr(entry.key);
-                    assert(mapping.set);
-                    mapping.index = original_index;
+                    const old_index = switch (entry.key) {
+                        .bg => self.bgs.items.len,
+                        .text, .underline, .strikethrough => self.text.items.len,
+                    };
+                    var old_it = self.map[self.index(coord)].array.iterator();
+                    while (old_it.next()) |old_entry| {
+                        if (old_entry.value.set and
+                            old_entry.value.index == old_index)
+                        {
+                            old_entry.value.index = original_index;
+                            break;
+                        }
+                    }
                 }
             }
         }
@@ -293,6 +303,38 @@ test "Contents clear retains other content" {
     try testing.expectEqual(cell2, c.get(.bg, .{ .x = 4, .y = 2 }).?);
 }
 
+test "Contents clear last added content" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    const rows = 10;
+    const cols = 10;
+
+    var c: Contents = .{};
+    try c.resize(alloc, .{ .rows = rows, .columns = cols });
+    defer c.deinit(alloc);
+
+    // Set some contents
+    const cell1: mtl_shaders.CellBg = .{
+        .mode = .rgb,
+        .grid_pos = .{ 4, 1 },
+        .cell_width = 1,
+        .color = .{ 0, 0, 0, 1 },
+    };
+    const cell2: mtl_shaders.CellBg = .{
+        .mode = .rgb,
+        .grid_pos = .{ 4, 2 },
+        .cell_width = 1,
+        .color = .{ 0, 0, 0, 1 },
+    };
+    try c.set(alloc, .bg, cell1);
+    try c.set(alloc, .bg, cell2);
+    c.clear(2);
+
+    // Row 2 should still be valid.
+    try testing.expectEqual(cell1, c.get(.bg, .{ .x = 4, .y = 1 }).?);
+}
+
 test "Contents.Map size" {
     // We want to be mindful of when this increases because it affects
     // renderer memory significantly.