From 3354933fe3606b33aa2c47a77b50a253fb1975f8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 26 Feb 2024 13:54:26 -0800 Subject: [PATCH] terminal/new: more robust cell blanking in row --- src/terminal/new/Terminal.zig | 170 ++++++++++++++++------------------ 1 file changed, 79 insertions(+), 91 deletions(-) diff --git a/src/terminal/new/Terminal.zig b/src/terminal/new/Terminal.zig index d885cd841..e1e1ec83b 100644 --- a/src/terminal/new/Terminal.zig +++ b/src/terminal/new/Terminal.zig @@ -24,6 +24,7 @@ const size = @import("size.zig"); const pagepkg = @import("page.zig"); const style = @import("style.zig"); const Screen = @import("Screen.zig"); +const Page = pagepkg.Page; const Cell = pagepkg.Cell; const Row = pagepkg.Row; @@ -1103,58 +1104,14 @@ pub fn insertLines(self: *Terminal, count: usize) void { } // Inserted lines should keep our bg color - const blank_cell = self.blankCell(); for (0..adjusted_count) |i| { const row: *Row = @ptrCast(top + i); // Clear the src row. var page = &self.screen.cursor.page_offset.page.data; const cells = page.getCells(row); - - // If this row has graphemes, then we need go through a slow path - // and delete the cell graphemes. - if (row.grapheme) { - for (cells) |*cell| { - if (cell.hasGrapheme()) page.clearGrapheme(row, cell); - } - assert(!row.grapheme); - } - const cells_write = cells[self.scrolling_region.left .. self.scrolling_region.right + 1]; - - // Delete any styles. Note the `row.styled` check is VERY IMPORTANT - // for performance. Without this, every insert line is 4x slower. With - // this check, only styled rows are 4x slower. - if (row.styled) { - for (cells_write) |*cell| { - if (cell.style_id == style.default_id) continue; - - // Fast-path, the style ID matches, in this case we just update - // our own ref and continue. We never delete because our style - // is still active. - if (cell.style_id == self.screen.cursor.style_id) { - self.screen.cursor.style_ref.?.* -= 1; - continue; - } - - // Slow path: we need to lookup this style so we can decrement - // the ref count. Since we've already loaded everything, we also - // just go ahead and GC it if it reaches zero, too. - if (page.styles.lookupId(page.memory, cell.style_id)) |prev_style| { - // Below upsert can't fail because it should already be present - const md = page.styles.upsert(page.memory, prev_style.*) catch unreachable; - assert(md.ref > 0); - md.ref -= 1; - if (md.ref == 0) page.styles.remove(page.memory, cell.style_id); - } - } - - // If we have no left/right scroll region we can be sure that - // the row is no longer styled. - if (cells_write.len == self.cols) row.styled = false; - } - - @memset(cells_write, blank_cell); + self.blankCells(page, row, cells_write); } // Move the cursor to the left margin. But importantly this also @@ -1230,58 +1187,14 @@ pub fn deleteLines(self: *Terminal, count_req: usize) void { } const bottom: [*]Row = top + (rem - 1); - const blank_cell = self.blankCell(); while (@intFromPtr(y) <= @intFromPtr(bottom)) : (y += 1) { const row: *Row = @ptrCast(y); // Clear the src row. var page = &self.screen.cursor.page_offset.page.data; const cells = page.getCells(row); - - // If this row has graphemes, then we need go through a slow path - // and delete the cell graphemes. - if (row.grapheme) { - for (cells) |*cell| { - if (cell.hasGrapheme()) page.clearGrapheme(row, cell); - } - assert(!row.grapheme); - } - const cells_write = cells[self.scrolling_region.left .. self.scrolling_region.right + 1]; - - // Delete any styles. Note the `row.styled` check is VERY IMPORTANT - // for performance. Without this, every insert line is 4x slower. With - // this check, only styled rows are 4x slower. - if (row.styled) { - for (cells_write) |*cell| { - if (cell.style_id == style.default_id) continue; - - // Fast-path, the style ID matches, in this case we just update - // our own ref and continue. We never delete because our style - // is still active. - if (cell.style_id == self.screen.cursor.style_id) { - self.screen.cursor.style_ref.?.* -= 1; - continue; - } - - // Slow path: we need to lookup this style so we can decrement - // the ref count. Since we've already loaded everything, we also - // just go ahead and GC it if it reaches zero, too. - if (page.styles.lookupId(page.memory, cell.style_id)) |prev_style| { - // Below upsert can't fail because it should already be present - const md = page.styles.upsert(page.memory, prev_style.*) catch unreachable; - assert(md.ref > 0); - md.ref -= 1; - if (md.ref == 0) page.styles.remove(page.memory, cell.style_id); - } - } - - // If we have no left/right scroll region we can be sure that - // the row is no longer styled. - if (cells_write.len == self.cols) row.styled = false; - } - - @memset(cells_write, blank_cell); + self.blankCells(page, row, cells_write); } // Move the cursor to the left margin. But importantly this also @@ -1318,7 +1231,11 @@ pub fn eraseChars(self: *Terminal, count_req: usize) void { // Clear the cells const cells: [*]Cell = @ptrCast(self.screen.cursor.page_cell); - @memset(cells[0..end], self.blankCell()); + self.blankCells( + &self.screen.cursor.page_offset.page.data, + self.screen.cursor.page_row, + cells[0..end], + ); // This resets the soft-wrap of this line self.screen.cursor.page_row.wrap = false; @@ -1348,6 +1265,55 @@ pub fn eraseChars(self: *Terminal, count_req: usize) void { // } } +/// Blank the given cells. The cells must be long to the given row and page. +/// This will handle refcounted styles properly as well as graphemes. +fn blankCells( + self: *const Terminal, + page: *Page, + row: *Row, + cells: []Cell, +) void { + // If this row has graphemes, then we need go through a slow path + // and delete the cell graphemes. + if (row.grapheme) { + for (cells) |*cell| { + if (cell.hasGrapheme()) page.clearGrapheme(row, cell); + } + assert(!row.grapheme); + } + + if (row.styled) { + for (cells) |*cell| { + if (cell.style_id == style.default_id) continue; + + // Fast-path, the style ID matches, in this case we just update + // our own ref and continue. We never delete because our style + // is still active. + if (cell.style_id == self.screen.cursor.style_id) { + self.screen.cursor.style_ref.?.* -= 1; + continue; + } + + // Slow path: we need to lookup this style so we can decrement + // the ref count. Since we've already loaded everything, we also + // just go ahead and GC it if it reaches zero, too. + if (page.styles.lookupId(page.memory, cell.style_id)) |prev_style| { + // Below upsert can't fail because it should already be present + const md = page.styles.upsert(page.memory, prev_style.*) catch unreachable; + assert(md.ref > 0); + md.ref -= 1; + if (md.ref == 0) page.styles.remove(page.memory, cell.style_id); + } + } + + // If we have no left/right scroll region we can be sure that + // the row is no longer styled. + if (cells.len == self.cols) row.styled = false; + } + + @memset(cells, self.blankCell()); +} + /// Set a style attribute. pub fn setAttribute(self: *Terminal, attr: sgr.Attribute) !void { try self.screen.setAttribute(attr); @@ -3260,6 +3226,28 @@ test "Terminal: eraseChars preserves background sgr" { } } +test "Terminal: eraseChars handles refcounted styles" { + const alloc = testing.allocator; + var t = try init(alloc, 10, 10); + defer t.deinit(alloc); + + try t.setAttribute(.{ .bold = {} }); + try t.print('A'); + try t.print('B'); + try t.setAttribute(.{ .unset = {} }); + try t.print('C'); + + // verify we have styles in our style map + const page = t.screen.cursor.page_offset.page.data; + try testing.expectEqual(@as(usize, 1), page.styles.count(page.memory)); + + t.setCursorPos(1, 1); + t.eraseChars(2); + + // verify we have no styles in our style map + try testing.expectEqual(@as(usize, 0), page.styles.count(page.memory)); +} + test "Terminal: reverseIndex" { const alloc = testing.allocator; var t = try init(alloc, 2, 5);