From a9aef11b4bdfd3478d746f71ff3cf82dfc6aefe6 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 8 Jul 2024 22:15:31 -0400 Subject: [PATCH 01/11] RefCountedSet: add some missing context delete callbacks --- src/terminal/ref_counted_set.zig | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/terminal/ref_counted_set.zig b/src/terminal/ref_counted_set.zig index 5fe5fa542..e19f8f70c 100644 --- a/src/terminal/ref_counted_set.zig +++ b/src/terminal/ref_counted_set.zig @@ -232,10 +232,16 @@ pub fn RefCountedSet( // we're reusing the existing value in the set. This allows // callers to clean up any resources associated with the value. if (comptime @hasDecl(Context, "deleted")) ctx.deleted(value); + items[id].meta.ref += 1; return id; } + // Notify the context that the value is "deleted" if we return an + // error. This allows callers to clean up any resources associated + // with the value. + errdefer if (comptime @hasDecl(Context, "deleted")) ctx.deleted(value); + // If the item doesn't exist, we need an available ID. if (self.next_id >= self.layout.cap) { // Arbitrarily chosen, threshold for rehashing. @@ -279,6 +285,8 @@ pub fn RefCountedSet( pub fn addWithIdContext(self: *Self, base: anytype, value: T, id: Id, ctx: Context) AddError!?Id { const items = self.items.ptr(base); + assert(id > 0); + if (id < self.next_id) { if (items[id].meta.ref == 0) { self.deleteItem(base, id, ctx); @@ -291,6 +299,11 @@ pub fn RefCountedSet( return if (added_id == id) null else added_id; } else if (ctx.eql(value, items[id].value)) { + // Notify the context that the value is "deleted" because + // we're reusing the existing value in the set. This allows + // callers to clean up any resources associated with the value. + if (comptime @hasDecl(Context, "deleted")) ctx.deleted(value); + items[id].meta.ref += 1; return null; @@ -501,6 +514,7 @@ pub fn RefCountedSet( // we're reusing the existing value in the set. This allows // callers to clean up any resources associated with the value. if (comptime @hasDecl(Context, "deleted")) ctx.deleted(value); + return id; } From 94f50be0fe25a89e10a11357ea42b03418b69d53 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 8 Jul 2024 22:16:13 -0400 Subject: [PATCH 02/11] Disable mouse scroll logging since it floods logs when trackpad scrolling --- src/Surface.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Surface.zig b/src/Surface.zig index 57f1138cb..ed417690b 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -1753,7 +1753,7 @@ pub fn scrollCallback( }; }; - log.info("scroll: delta_y={} delta_x={}", .{ y.delta, x.delta }); + // log.info("scroll: delta_y={} delta_x={}", .{ y.delta, x.delta }); { self.renderer_state.mutex.lock(); From 11c8bdc00e90ed7e2708143479b44650c769e7b6 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 8 Jul 2024 22:17:56 -0400 Subject: [PATCH 03/11] BitmapAllocator: slightly improve findFreeChunks mask calculation --- src/terminal/bitmap_allocator.zig | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/terminal/bitmap_allocator.zig b/src/terminal/bitmap_allocator.zig index a13236217..f96d39831 100644 --- a/src/terminal/bitmap_allocator.zig +++ b/src/terminal/bitmap_allocator.zig @@ -240,20 +240,28 @@ fn findFreeChunks(bitmaps: []u64, n: usize) ?usize { assert(n <= @bitSizeOf(u64)); for (bitmaps, 0..) |*bitmap, idx| { // Shift the bitmap to find `n` sequential free chunks. + // EXAMPLE: + // n = 4 + // shifted = 001111001011110010 + // & 000111100101111001 + // & 000011110010111100 + // & 000001111001011110 + // = 000001000000010000 + // ^ ^ + // In this example there are 2 places with at least 4 sequential 1s. var shifted: u64 = bitmap.*; for (1..n) |i| shifted &= bitmap.* >> @intCast(i); // If we have zero then we have no matches if (shifted == 0) continue; - // Trailing zeroes gets us the bit 1-indexed + // Trailing zeroes gets us the index of the first bit index with at + // least `n` sequential 1s. In the example above, that would be `4`. const bit = @ctz(shifted); // Calculate the mask so we can mark it as used - for (0..n) |i| { - const mask = @as(u64, 1) << @intCast(bit + i); - bitmap.* ^= mask; - } + const mask = (@as(u64, std.math.maxInt(u64)) >> @intCast(64 - n)) << @intCast(bit); + bitmap.* ^= mask; return (idx * 64) + bit; } From 29e6dcdee52b6b8cb9daa0f8ee4e4d46e21c5769 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 8 Jul 2024 22:20:31 -0400 Subject: [PATCH 04/11] terminal/page: add methods to get hyperlink and grapheme map capacities --- src/terminal/page.zig | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/terminal/page.zig b/src/terminal/page.zig index 5b7c4a008..dee90a785 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -942,7 +942,7 @@ pub const Page = struct { map.removeByPtr(entry.key_ptr); cell.hyperlink = false; - // Mark that we no longer have graphemes, also search the row + // Mark that we no longer have hyperlinks, also search the row // to make sure its state is correct. const cells = row.cells.ptr(self.memory)[0..self.size.cols]; for (cells) |c| if (c.hyperlink) return; @@ -992,6 +992,18 @@ pub const Page = struct { map.putAssumeCapacity(dst_offset, value); } + /// Returns the number of hyperlinks in the page. This isn't the byte + /// size but the total number of unique cells that have hyperlink data. + pub fn hyperlinkCount(self: *const Page) usize { + return self.hyperlink_map.map(self.memory).count(); + } + + /// Returns the hyperlink capacity for the page. This isn't the byte + /// size but the number of unique cells that can have hyperlink data. + pub fn hyperlinkCapacity(self: *const Page) usize { + return self.hyperlink_map.map(self.memory).capacity(); + } + /// Append a codepoint to the given cell as a grapheme. pub fn appendGrapheme(self: *Page, row: *Row, cell: *Cell, cp: u21) Allocator.Error!void { defer self.assertIntegrity(); @@ -1114,6 +1126,12 @@ pub const Page = struct { return self.grapheme_map.map(self.memory).count(); } + /// Returns the grapheme capacity for the page. This isn't the byte + /// size but the number of unique cells that can have grapheme data. + pub fn graphemeCapacity(self: *const Page) usize { + return self.grapheme_map.map(self.memory).capacity(); + } + /// Returns the bitset for the dirty bits on this page. /// /// The returned value is a DynamicBitSetUnmanaged but it is NOT From 0cee115ba3f8aade9f02062abcca2a4fd1b3ad8d Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 8 Jul 2024 22:22:18 -0400 Subject: [PATCH 05/11] terminal/page: use addWithIdContext when cloning hyperlink data --- src/terminal/page.zig | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/terminal/page.zig b/src/terminal/page.zig index dee90a785..49e1de475 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -684,11 +684,12 @@ pub const Page = struct { // add it, and migrate. const id = other.lookupHyperlink(src_cell).?; const other_link = other.hyperlink_set.get(other.memory, id); - const dst_id = try self.hyperlink_set.addContext( + const dst_id = try self.hyperlink_set.addWithIdContext( self.memory, try other_link.dupe(other, self), + id, .{ .page = self }, - ); + ) orelse id; try self.setHyperlink(dst_row, dst_cell, dst_id); } if (src_cell.style_id != style.default_id) { @@ -705,13 +706,11 @@ pub const Page = struct { // Slow path: Get the style from the other // page and add it to this page's style set. const other_style = other.styles.get(other.memory, src_cell.style_id); - if (try self.styles.addWithId( + dst_cell.style_id = try self.styles.addWithId( self.memory, other_style.*, src_cell.style_id, - )) |id| { - dst_cell.style_id = id; - } + ) orelse src_cell.style_id; } } } From 730185b212afc3fde1977ece3ec6c9845bee916e Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 8 Jul 2024 22:25:39 -0400 Subject: [PATCH 06/11] terminal: spacer heads and tails should be codepoint 0, not ' ' --- src/terminal/PageList.zig | 41 ++++++++++++++++++++++----------------- src/terminal/Terminal.zig | 36 +++++++++++++++++----------------- 2 files changed, 41 insertions(+), 36 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index c4a108aca..44f608121 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -5554,7 +5554,7 @@ test "PageList resize (no reflow) more cols with spacer head" { const rac = page.getRowAndCell(1, 0); rac.cell.* = .{ .content_tag = .codepoint, - .content = .{ .codepoint = ' ' }, + .content = .{ .codepoint = 0 }, .wide = .spacer_head, }; } @@ -5570,7 +5570,7 @@ test "PageList resize (no reflow) more cols with spacer head" { const rac = page.getRowAndCell(1, 1); rac.cell.* = .{ .content_tag = .codepoint, - .content = .{ .codepoint = ' ' }, + .content = .{ .codepoint = 0 }, .wide = .spacer_tail, }; } @@ -5593,7 +5593,7 @@ test "PageList resize (no reflow) more cols with spacer head" { } { const rac = page.getRowAndCell(1, 0); - try testing.expectEqual(@as(u21, ' '), rac.cell.content.codepoint); + try testing.expectEqual(@as(u21, 0), rac.cell.content.codepoint); try testing.expectEqual(pagepkg.Cell.Wide.narrow, rac.cell.wide); } { @@ -6465,12 +6465,13 @@ test "PageList resize reflow more cols unwrap wide spacer head" { const rac = page.getRowAndCell(1, 0); rac.cell.* = .{ .content_tag = .codepoint, - .content = .{ .codepoint = ' ' }, + .content = .{ .codepoint = 0 }, .wide = .spacer_head, }; } { const rac = page.getRowAndCell(0, 1); + rac.row.wrap_continuation = true; rac.cell.* = .{ .content_tag = .codepoint, .content = .{ .codepoint = 'πŸ˜€' }, @@ -6481,7 +6482,7 @@ test "PageList resize reflow more cols unwrap wide spacer head" { const rac = page.getRowAndCell(1, 1); rac.cell.* = .{ .content_tag = .codepoint, - .content = .{ .codepoint = ' ' }, + .content = .{ .codepoint = 0 }, .wide = .spacer_tail, }; } @@ -6509,7 +6510,7 @@ test "PageList resize reflow more cols unwrap wide spacer head" { } { const rac = page.getRowAndCell(2, 0); - try testing.expectEqual(@as(u21, ' '), rac.cell.content.codepoint); + try testing.expectEqual(@as(u21, 0), rac.cell.content.codepoint); try testing.expectEqual(pagepkg.Cell.Wide.spacer_tail, rac.cell.wide); } } @@ -6542,6 +6543,7 @@ test "PageList resize reflow more cols unwrap wide spacer head across two rows" } { const rac = page.getRowAndCell(0, 1); + rac.row.wrap_continuation = true; rac.row.wrap = true; rac.cell.* = .{ .content_tag = .codepoint, @@ -6552,12 +6554,13 @@ test "PageList resize reflow more cols unwrap wide spacer head across two rows" const rac = page.getRowAndCell(1, 1); rac.cell.* = .{ .content_tag = .codepoint, - .content = .{ .codepoint = ' ' }, + .content = .{ .codepoint = 0 }, .wide = .spacer_head, }; } { const rac = page.getRowAndCell(0, 2); + rac.row.wrap_continuation = true; rac.cell.* = .{ .content_tag = .codepoint, .content = .{ .codepoint = 'πŸ˜€' }, @@ -6568,7 +6571,7 @@ test "PageList resize reflow more cols unwrap wide spacer head across two rows" const rac = page.getRowAndCell(1, 2); rac.cell.* = .{ .content_tag = .codepoint, - .content = .{ .codepoint = ' ' }, + .content = .{ .codepoint = 0 }, .wide = .spacer_tail, }; } @@ -6601,7 +6604,7 @@ test "PageList resize reflow more cols unwrap wide spacer head across two rows" } { const rac = page.getRowAndCell(3, 0); - try testing.expectEqual(@as(u21, ' '), rac.cell.content.codepoint); + try testing.expectEqual(@as(u21, 0), rac.cell.content.codepoint); try testing.expectEqual(pagepkg.Cell.Wide.spacer_head, rac.cell.wide); } { @@ -6611,7 +6614,7 @@ test "PageList resize reflow more cols unwrap wide spacer head across two rows" } { const rac = page.getRowAndCell(1, 1); - try testing.expectEqual(@as(u21, ' '), rac.cell.content.codepoint); + try testing.expectEqual(@as(u21, 0), rac.cell.content.codepoint); try testing.expectEqual(pagepkg.Cell.Wide.spacer_tail, rac.cell.wide); } } @@ -6644,6 +6647,7 @@ test "PageList resize reflow more cols unwrap still requires wide spacer head" { } { const rac = page.getRowAndCell(0, 1); + rac.row.wrap_continuation = true; rac.cell.* = .{ .content_tag = .codepoint, .content = .{ .codepoint = 'πŸ˜€' }, @@ -6654,7 +6658,7 @@ test "PageList resize reflow more cols unwrap still requires wide spacer head" { const rac = page.getRowAndCell(1, 1); rac.cell.* = .{ .content_tag = .codepoint, - .content = .{ .codepoint = ' ' }, + .content = .{ .codepoint = 0 }, .wide = .spacer_tail, }; } @@ -6692,7 +6696,7 @@ test "PageList resize reflow more cols unwrap still requires wide spacer head" { } { const rac = page.getRowAndCell(1, 1); - try testing.expectEqual(@as(u21, ' '), rac.cell.content.codepoint); + try testing.expectEqual(@as(u21, 0), rac.cell.content.codepoint); try testing.expectEqual(pagepkg.Cell.Wide.spacer_tail, rac.cell.wide); } } @@ -7054,12 +7058,13 @@ test "PageList resize reflow less cols wraps spacer head" { const rac = page.getRowAndCell(3, 0); rac.cell.* = .{ .content_tag = .codepoint, - .content = .{ .codepoint = ' ' }, + .content = .{ .codepoint = 0 }, .wide = .spacer_head, }; } { const rac = page.getRowAndCell(0, 1); + rac.row.wrap_continuation = true; rac.cell.* = .{ .content_tag = .codepoint, .content = .{ .codepoint = 'πŸ˜€' }, @@ -7070,7 +7075,7 @@ test "PageList resize reflow less cols wraps spacer head" { const rac = page.getRowAndCell(1, 1); rac.cell.* = .{ .content_tag = .codepoint, - .content = .{ .codepoint = ' ' }, + .content = .{ .codepoint = 0 }, .wide = .spacer_tail, }; } @@ -7108,7 +7113,7 @@ test "PageList resize reflow less cols wraps spacer head" { } { const rac = page.getRowAndCell(1, 1); - try testing.expectEqual(@as(u21, ' '), rac.cell.content.codepoint); + try testing.expectEqual(@as(u21, 0), rac.cell.content.codepoint); try testing.expectEqual(pagepkg.Cell.Wide.spacer_tail, rac.cell.wide); } } @@ -7552,7 +7557,7 @@ test "PageList resize reflow less cols to eliminate a wide char" { const rac = page.getRowAndCell(1, 0); rac.cell.* = .{ .content_tag = .codepoint, - .content = .{ .codepoint = ' ' }, + .content = .{ .codepoint = 0 }, .wide = .spacer_tail, }; } @@ -7604,7 +7609,7 @@ test "PageList resize reflow less cols to wrap a wide char" { const rac = page.getRowAndCell(2, 0); rac.cell.* = .{ .content_tag = .codepoint, - .content = .{ .codepoint = ' ' }, + .content = .{ .codepoint = 0 }, .wide = .spacer_tail, }; } @@ -7637,7 +7642,7 @@ test "PageList resize reflow less cols to wrap a wide char" { } { const rac = page.getRowAndCell(1, 1); - try testing.expectEqual(@as(u21, ' '), rac.cell.content.codepoint); + try testing.expectEqual(@as(u21, 0), rac.cell.content.codepoint); try testing.expectEqual(pagepkg.Cell.Wide.spacer_tail, rac.cell.wide); } } diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index af903a71d..a25c46838 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -343,7 +343,7 @@ pub fn print(self: *Terminal, c: u21) !void { if (self.screen.cursor.x == right_limit - 1) { if (!self.modes.get(.wraparound)) return; self.printCell( - ' ', + 0, if (right_limit == self.cols) .spacer_head else .narrow, ); try self.printWrap(); @@ -353,7 +353,7 @@ pub fn print(self: *Terminal, c: u21) !void { // Write our spacer self.screen.cursorRight(1); - self.printCell(' ', .spacer_tail); + self.printCell(0, .spacer_tail); // Move the cursor again so we're beyond our spacer if (self.screen.cursor.x == right_limit - 1) { @@ -478,19 +478,19 @@ pub fn print(self: *Terminal, c: u21) !void { // We only create a spacer head if we're at the real edge // of the screen. Otherwise, we clear the space with a narrow. // This allows soft wrapping to work correctly. - self.printCell(' ', if (right_limit == self.cols) .spacer_head else .narrow); + self.printCell(0, if (right_limit == self.cols) .spacer_head else .narrow); try self.printWrap(); } self.screen.cursorMarkDirty(); self.printCell(c, .wide); self.screen.cursorRight(1); - self.printCell(' ', .spacer_tail); + self.printCell(0, .spacer_tail); } else { // This is pretty broken, terminals should never be only 1-wide. // We sould prevent this downstream. self.screen.cursorMarkDirty(); - self.printCell(' ', .narrow); + self.printCell(0, .narrow); }, else => unreachable, @@ -2777,7 +2777,7 @@ test "Terminal: print wide char in single-width terminal" { { const list_cell = t.screen.pages.getCell(.{ .screen = .{ .x = 0, .y = 0 } }).?; const cell = list_cell.cell; - try testing.expectEqual(@as(u21, ' '), cell.content.codepoint); + try testing.expectEqual(@as(u21, 0), cell.content.codepoint); try testing.expectEqual(Cell.Wide.narrow, cell.wide); } @@ -2928,7 +2928,7 @@ test "Terminal: print multicodepoint grapheme, disabled mode 2027" { { const list_cell = t.screen.pages.getCell(.{ .screen = .{ .x = 1, .y = 0 } }).?; const cell = list_cell.cell; - try testing.expectEqual(@as(u21, ' '), cell.content.codepoint); + try testing.expectEqual(@as(u21, 0), cell.content.codepoint); try testing.expect(!cell.hasGrapheme()); try testing.expectEqual(Cell.Wide.spacer_tail, cell.wide); try testing.expect(list_cell.page.data.lookupGrapheme(cell) == null); @@ -2945,7 +2945,7 @@ test "Terminal: print multicodepoint grapheme, disabled mode 2027" { { const list_cell = t.screen.pages.getCell(.{ .screen = .{ .x = 3, .y = 0 } }).?; const cell = list_cell.cell; - try testing.expectEqual(@as(u21, ' '), cell.content.codepoint); + try testing.expectEqual(@as(u21, 0), cell.content.codepoint); try testing.expect(!cell.hasGrapheme()); try testing.expectEqual(Cell.Wide.spacer_tail, cell.wide); try testing.expect(list_cell.page.data.lookupGrapheme(cell) == null); @@ -2961,7 +2961,7 @@ test "Terminal: print multicodepoint grapheme, disabled mode 2027" { { const list_cell = t.screen.pages.getCell(.{ .screen = .{ .x = 5, .y = 0 } }).?; const cell = list_cell.cell; - try testing.expectEqual(@as(u21, ' '), cell.content.codepoint); + try testing.expectEqual(@as(u21, 0), cell.content.codepoint); try testing.expect(!cell.hasGrapheme()); try testing.expectEqual(Cell.Wide.spacer_tail, cell.wide); try testing.expect(list_cell.page.data.lookupGrapheme(cell) == null); @@ -3091,7 +3091,7 @@ test "Terminal: print multicodepoint grapheme, mode 2027" { { const list_cell = t.screen.pages.getCell(.{ .screen = .{ .x = 1, .y = 0 } }).?; const cell = list_cell.cell; - try testing.expectEqual(@as(u21, ' '), cell.content.codepoint); + try testing.expectEqual(@as(u21, 0), cell.content.codepoint); try testing.expect(!cell.hasGrapheme()); try testing.expectEqual(Cell.Wide.spacer_tail, cell.wide); } @@ -3780,7 +3780,7 @@ test "Terminal: print wide char at right margin does not create spacer head" { { const list_cell = t.screen.pages.getCell(.{ .screen = .{ .x = 4, .y = 0 } }).?; const cell = list_cell.cell; - try testing.expectEqual(@as(u21, ' '), cell.content.codepoint); + try testing.expectEqual(@as(u21, 0), cell.content.codepoint); try testing.expectEqual(Cell.Wide.narrow, cell.wide); const row = list_cell.row; @@ -7431,8 +7431,8 @@ test "Terminal: deleteLines wide character spacer head" { defer testing.allocator.free(str); const unwrapped_str = try t.plainStringUnwrapped(testing.allocator); defer testing.allocator.free(unwrapped_str); - try testing.expectEqualStrings("BBBB \n\u{1F600}CCC", str); - try testing.expectEqualStrings("BBBB \n\u{1F600}CCC", unwrapped_str); + try testing.expectEqualStrings("BBBB\n\u{1F600}CCC", str); + try testing.expectEqualStrings("BBBB\n\u{1F600}CCC", unwrapped_str); } } @@ -7472,7 +7472,7 @@ test "Terminal: deleteLines wide character spacer head left scroll margin" { defer testing.allocator.free(str); const unwrapped_str = try t.plainStringUnwrapped(testing.allocator); defer testing.allocator.free(unwrapped_str); - try testing.expectEqualStrings("AABB \nBBCCC\n\u{1F600}", str); + try testing.expectEqualStrings("AABB\nBBCCC\n\u{1F600}", str); try testing.expectEqualStrings("AABB BBCCC\u{1F600}", unwrapped_str); } } @@ -7513,7 +7513,7 @@ test "Terminal: deleteLines wide character spacer head right scroll margin" { defer testing.allocator.free(str); const unwrapped_str = try t.plainStringUnwrapped(testing.allocator); defer testing.allocator.free(unwrapped_str); - try testing.expectEqualStrings("BBBBA\n\u{1F600}CC \n C", str); + try testing.expectEqualStrings("BBBBA\n\u{1F600}CC\n C", str); try testing.expectEqualStrings("BBBBA\u{1F600}CC C", unwrapped_str); } } @@ -7597,7 +7597,7 @@ test "Terminal: deleteLines wide character spacer head left (< 2) and right scro defer testing.allocator.free(str); const unwrapped_str = try t.plainStringUnwrapped(testing.allocator); defer testing.allocator.free(unwrapped_str); - try testing.expectEqualStrings("ABBBA\nB CC \n C", str); + try testing.expectEqualStrings("ABBBA\nB CC\n C", str); try testing.expectEqualStrings("ABBBAB CC C", unwrapped_str); } } @@ -7633,7 +7633,7 @@ test "Terminal: deleteLines wide characters split by left/right scroll region bo { const str = try t.plainString(testing.allocator); defer testing.allocator.free(str); - try testing.expectEqualStrings("A B A\n ", str); + try testing.expectEqualStrings("A B A", str); } } @@ -8600,7 +8600,7 @@ test "Terminal: deleteChars split wide character from end" { { const list_cell = t.screen.pages.getCell(.{ .screen = .{ .x = 1, .y = 0 } }).?; const cell = list_cell.cell; - try testing.expectEqual(@as(u21, ' '), cell.content.codepoint); + try testing.expectEqual(@as(u21, 0), cell.content.codepoint); try testing.expectEqual(Cell.Wide.spacer_tail, cell.wide); } } From 6f1a2d1e8e3f34d75121bc7749904547b714f023 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 8 Jul 2024 22:26:36 -0400 Subject: [PATCH 07/11] terminal/Screen: fix trailing blank cell handling for wrapped rows in dumpString --- src/terminal/Screen.zig | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index d99ca0b28..11d6b85f4 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2337,6 +2337,7 @@ pub fn dumpString( opts: DumpString, ) !void { var blank_rows: usize = 0; + var blank_cells: usize = 0; var iter = opts.tl.rowIterator(.right_down, opts.br); while (iter.next()) |row_offset| { @@ -2363,7 +2364,12 @@ pub fn dumpString( blank_rows += 1; } - var blank_cells: usize = 0; + if (!row.wrap_continuation or !opts.unwrap) { + // We should also reset blank cell counts at the start of each row + // unless we're unwrapping and this row is a wrap continuation. + blank_cells = 0; + } + for (cells) |*cell| { // Skip spacers switch (cell.wide) { @@ -2379,7 +2385,7 @@ pub fn dumpString( continue; } if (blank_cells > 0) { - for (0..blank_cells) |_| try writer.writeByte(' '); + try writer.writeByteNTimes(' ', blank_cells); blank_cells = 0; } From be99e50c50830dcb5d3b681ee18e3f9d4d975b39 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 8 Jul 2024 22:28:16 -0400 Subject: [PATCH 08/11] terminal/PageList: add method for logging debug diagrams --- src/terminal/PageList.zig | 207 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 207 insertions(+) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 44f608121..a4720370d 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -2548,6 +2548,213 @@ pub fn getCell(self: *const PageList, pt: point.Point) ?Cell { }; } +/// Log a debug diagram of the page list to the provided writer. +/// +/// EXAMPLE: +/// +/// +-----+ = PAGE 0 +/// ... | | +/// 50 | foo | +/// ... | | +/// +--------+ ACTIVE +/// 124 | | | 0 +/// 125 |Text | | 1 +/// : ^ : : = PIN 0 +/// 126 |Wrapp… | 2 +/// +-----+ : +/// +-----+ : = PAGE 1 +/// 0 …ed | | 3 +/// 1 | etc.| | 4 +/// +-----+ : +/// +--------+ +pub fn diagram(self: *const PageList, writer: anytype) !void { + const active_pin = self.getTopLeft(.active); + + var active = false; + var active_index: usize = 0; + + var page_index: usize = 0; + var cols: usize = 0; + + var it = self.pageIterator(.right_down, .{ .screen = .{} }, null); + while (it.next()) |chunk| : (page_index += 1) { + cols = chunk.page.data.size.cols; + + // Whether we've just skipped some number of rows and drawn + // an ellipsis row (this is reset when a row is not skipped). + var skipped = false; + + for (0..chunk.page.data.size.rows) |y| { + // Active header + if (!active and + chunk.page == active_pin.page and + active_pin.y == y) + { + active = true; + try writer.writeAll(" +-"); + try writer.writeByteNTimes('-', cols); + try writer.writeAll("--+ ACTIVE"); + try writer.writeByte('\n'); + } + + // Page header + if (y == 0) { + try writer.writeAll(" +"); + try writer.writeByteNTimes('-', cols); + try writer.writeByte('+'); + if (active) try writer.writeAll(" :"); + try writer.print(" = PAGE {}", .{page_index}); + try writer.writeByte('\n'); + } + + // Row contents + { + const row = chunk.page.data.getRow(y); + const cells = chunk.page.data.getCells(row)[0..cols]; + + var row_has_content = false; + + for (cells) |cell| { + if (cell.hasText()) { + row_has_content = true; + break; + } + } + + // We don't want to print this row's contents + // unless it has text or is in the active area. + if (!active and !row_has_content) { + // If we haven't, draw an ellipsis row. + if (!skipped) { + try writer.writeAll(" ... :"); + try writer.writeByteNTimes(' ', cols); + try writer.writeByte(':'); + if (active) try writer.writeAll(" :"); + try writer.writeByte('\n'); + } + skipped = true; + continue; + } + + skipped = false; + + // Left pad row number to 5 wide + const y_digits = if (y == 0) 0 else std.math.log10_int(y); + try writer.writeByteNTimes(' ', 4 - y_digits); + try writer.print("{} ", .{y}); + + // Left edge or wrap continuation marker + try writer.writeAll(if (row.wrap_continuation) "…" else "|"); + + // Row text + if (row_has_content) { + for (cells) |*cell| { + // Skip spacer tails, since wide cells are, well, wide. + if (cell.wide == .spacer_tail) continue; + + // Write non-printing bytes as base36, for convenience. + if (cell.codepoint() < ' ') { + try writer.writeByte("0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"[cell.codepoint()]); + continue; + } + try writer.print("{u}", .{cell.codepoint()}); + if (cell.hasGrapheme()) { + const grapheme = chunk.page.data.lookupGrapheme(cell).?; + for (grapheme) |cp| { + try writer.print("{u}", .{cp}); + } + } + } + } else { + try writer.writeByteNTimes(' ', cols); + } + + // Right edge or wrap marker + try writer.writeAll(if (row.wrap) "…" else "|"); + if (active) { + try writer.print(" | {}", .{active_index}); + active_index += 1; + } + + try writer.writeByte('\n'); + } + + // Tracked pin marker(s) + pins: { + // If we have more than 16 tracked pins in a row, oh well, + // don't wanna bother making this function allocating. + var pin_buf: [16]*Pin = undefined; + var pin_count: usize = 0; + var pin_it = self.tracked_pins.keyIterator(); + while (pin_it.next()) |p_ptr| { + const p = p_ptr.*; + if (p.page != chunk.page) continue; + if (p.y != y) continue; + pin_buf[pin_count] = p; + pin_count += 1; + if (pin_count >= pin_buf.len) return error.TooManyTrackedPinsInRow; + } + + if (pin_count == 0) break :pins; + + const pins = pin_buf[0..pin_count]; + std.mem.sort( + *Pin, + pins, + {}, + struct { + fn lt(_: void, a: *Pin, b: *Pin) bool { + return a.x < b.x; + } + }.lt, + ); + + try writer.writeAll(" :"); + var x: usize = 0; + + for (pins) |p| { + if (x > p.x) continue; + try writer.writeByteNTimes(' ', p.x - x); + try writer.writeByte('^'); + x = p.x + 1; + } + + try writer.writeByteNTimes(' ', cols - x); + try writer.writeByte(':'); + + if (active) try writer.writeAll(" :"); + + try writer.print(" = PIN{s}", .{if (pin_count > 1) "S" else ""}); + + x = pins[0].x; + for (pins, 0..) |p, i| { + if (p.x != x) try writer.writeByte(','); + try writer.print(" {}", .{i}); + } + + try writer.writeByte('\n'); + } + } + + // Page footer + { + try writer.writeAll(" +"); + try writer.writeByteNTimes('-', cols); + try writer.writeByte('+'); + if (active) try writer.writeAll(" :"); + try writer.writeByte('\n'); + } + } + + // Active footer + { + try writer.writeAll(" +-"); + try writer.writeByteNTimes('-', cols); + try writer.writeAll("--+"); + try writer.writeByte('\n'); + } +} + /// Direction that iterators can move. pub const Direction = enum { left_up, right_down }; From 10dbca946480c49f6f4e0bc55d4338291ed48557 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 8 Jul 2024 22:29:55 -0400 Subject: [PATCH 09/11] terminal/PageList: fix some incorrect test expectations --- src/terminal/PageList.zig | 143 +++++++++++++++++++++++++++++++++++--- 1 file changed, 133 insertions(+), 10 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index a4720370d..f31226577 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -6243,19 +6243,74 @@ test "PageList resize reflow more cols wrap across page boundary" { } } - // We expect one extra row since we unwrapped a row we need to resize - // to make our active area. - const end_rows = s.totalRows(); + // PageList.diagram -> + // + // +--+ = PAGE 0 + // ... : : + // +-----+ ACTIVE + // 15744 | | | 0 + // 15745 | | | 1 + // 15746 | | | 2 + // 15747 | | | 3 + // 15748 | | | 4 + // 15749 | | | 5 + // 15750 | | | 6 + // 15751 | | | 7 + // 15752 |01… | 8 + // +--+ : + // +--+ : = PAGE 1 + // 0 …01| | 9 + // +--+ : + // +-----+ + + // We expect one fewer rows since we unwrapped a row. + const end_rows = s.totalRows() - 1; // Resize try s.resize(.{ .cols = 4, .reflow = true }); try testing.expectEqual(@as(usize, 4), s.cols); try testing.expectEqual(@as(usize, end_rows), s.totalRows()); + // PageList.diagram -> + // + // +----+ = PAGE 0 + // ... : : + // +----+ + // +----+ = PAGE 1 + // ... : : + // +-------+ ACTIVE + // 6272 | | | 0 + // 6273 | | | 1 + // 6274 | | | 2 + // 6275 | | | 3 + // 6276 | | | 4 + // 6277 | | | 5 + // 6278 | | | 6 + // 6279 | | | 7 + // 6280 | | | 8 + // 6281 |0101| | 9 + // +----+ : + // +-------+ + { + // PAGE 1 ROW 6280, ACTIVE 8 + const p = s.pin(.{ .active = .{ .y = 8 } }).?; + const row = p.rowAndCell().row; + try testing.expect(!row.wrap); + try testing.expect(!row.wrap_continuation); + + const cells = p.cells(.all); + try testing.expect(!cells[0].hasText()); + try testing.expect(!cells[1].hasText()); + try testing.expect(!cells[2].hasText()); + try testing.expect(!cells[3].hasText()); + } + { + // PAGE 1 ROW 6281, ACTIVE 9 const p = s.pin(.{ .active = .{ .y = 9 } }).?; const row = p.rowAndCell().row; try testing.expect(!row.wrap); + try testing.expect(!row.wrap_continuation); const cells = p.cells(.all); try testing.expectEqual(@as(u21, 0), cells[0].content.codepoint); @@ -6322,9 +6377,8 @@ test "PageList resize reflow more cols wrap across page boundary cursor in secon defer s.untrackPin(p); try testing.expect(p.page == s.pages.last.?); - // We expect one extra row since we unwrapped a row we need to resize - // to make our active area. - const end_rows = s.totalRows(); + // We expect one fewer rows since we unwrapped a row. + const end_rows = s.totalRows() - 1; // Resize try s.resize(.{ .cols = 4, .reflow = true }); @@ -6408,6 +6462,27 @@ test "PageList resize reflow less cols wrap across page boundary cursor in secon try testing.expect(p.page == s.pages.last.?); try testing.expect(p.y == 0); + // PageList.diagram -> + // + // +-----+ = PAGE 0 + // ... : : + // +--------+ ACTIVE + // 7892 | | | 0 + // 7893 | | | 1 + // 7894 | | | 2 + // 7895 | | | 3 + // 7896 |01234… | 4 + // +-----+ : + // +-----+ : = PAGE 1 + // 0 …01234| | 5 + // : ^ : : = PIN 0 + // 1 | | | 6 + // 2 | | | 7 + // 3 | | | 8 + // 4 | | | 9 + // +-----+ : + // +--------+ + // Resize try s.resize(.{ .cols = 4, @@ -6416,14 +6491,47 @@ test "PageList resize reflow less cols wrap across page boundary cursor in secon }); try testing.expectEqual(@as(usize, 4), s.cols); + // PageList.diagram -> + // + // +----+ = PAGE 0 + // ... : : + // +-------+ ACTIVE + // 7892 | | | 0 + // 7893 | | | 1 + // 7894 | | | 2 + // 7895 | | | 3 + // 7896 |0123… | 4 + // 7897 …4012… | 5 + // : ^: : = PIN 0 + // 7898 …3400| | 6 + // 7899 | | | 7 + // 7900 | | | 8 + // 7901 | | | 9 + // +----+ : + // +-------+ + // Our cursor should remain on the same cell try testing.expectEqual(point.Point{ .active = .{ .x = 3, - .y = 6, + .y = 5, } }, s.pointFromPin(.active, p.*).?); { - const p2 = s.pin(.{ .active = .{ .y = 5 } }).?; + // PAGE 0 ROW 7895, ACTIVE 3 + const p2 = s.pin(.{ .active = .{ .y = 3 } }).?; + const row = p2.rowAndCell().row; + try testing.expect(!row.wrap); + try testing.expect(!row.wrap_continuation); + + const cells = p2.cells(.all); + try testing.expect(!cells[0].hasText()); + try testing.expect(!cells[1].hasText()); + try testing.expect(!cells[2].hasText()); + try testing.expect(!cells[3].hasText()); + } + { + // PAGE 0 ROW 7896, ACTIVE 4 + const p2 = s.pin(.{ .active = .{ .y = 4 } }).?; const row = p2.rowAndCell().row; try testing.expect(row.wrap); try testing.expect(!row.wrap_continuation); @@ -6435,7 +6543,8 @@ test "PageList resize reflow less cols wrap across page boundary cursor in secon try testing.expectEqual(@as(u21, 3), cells[3].content.codepoint); } { - const p2 = s.pin(.{ .active = .{ .y = 6 } }).?; + // PAGE 0 ROW 7897, ACTIVE 5 + const p2 = s.pin(.{ .active = .{ .y = 5 } }).?; const row = p2.rowAndCell().row; try testing.expect(row.wrap); try testing.expect(row.wrap_continuation); @@ -6447,7 +6556,8 @@ test "PageList resize reflow less cols wrap across page boundary cursor in secon try testing.expectEqual(@as(u21, 2), cells[3].content.codepoint); } { - const p2 = s.pin(.{ .active = .{ .y = 7 } }).?; + // PAGE 0 ROW 7898, ACTIVE 6 + const p2 = s.pin(.{ .active = .{ .y = 6 } }).?; const row = p2.rowAndCell().row; try testing.expect(!row.wrap); try testing.expect(row.wrap_continuation); @@ -6456,6 +6566,19 @@ test "PageList resize reflow less cols wrap across page boundary cursor in secon try testing.expectEqual(@as(u21, 3), cells[0].content.codepoint); try testing.expectEqual(@as(u21, 4), cells[1].content.codepoint); } + { + // PAGE 0 ROW 7899, ACTIVE 7 + const p2 = s.pin(.{ .active = .{ .y = 7 } }).?; + const row = p2.rowAndCell().row; + try testing.expect(!row.wrap); + try testing.expect(!row.wrap_continuation); + + const cells = p2.cells(.all); + try testing.expect(!cells[0].hasText()); + try testing.expect(!cells[1].hasText()); + try testing.expect(!cells[2].hasText()); + try testing.expect(!cells[3].hasText()); + } } test "PageList resize reflow more cols cursor in wrapped row" { const testing = std.testing; From 8589f2c0fb59a6d1412599eeb80c532bb4c85f66 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 8 Jul 2024 22:35:15 -0400 Subject: [PATCH 10/11] terminal/PageList: rework reflow logic to fix issues Reflow logic now lives inside of ReflowCursor. This fixes multiple issues with the previous implementation, and is more straightforward in how it works. The old impl resulted in fragmentation of pages, leading to unbounded memory growth when resizing repeatedly. Also improves the preserved_cursor logic in `resizeCols`. --- src/terminal/PageList.zig | 901 ++++++++++++++++++-------------------- src/terminal/page.zig | 27 ++ 2 files changed, 454 insertions(+), 474 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index f31226577..b4a984ae6 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -640,46 +640,59 @@ fn resizeCols( const preserved_cursor: ?struct { tracked_pin: *Pin, remaining_rows: usize, + wrapped_rows: usize, } = if (cursor) |c| cursor: { const p = self.pin(.{ .active = .{ .x = c.x, .y = c.y, } }) orelse break :cursor null; + const active_pin = self.pin(.{ .active = .{} }); + + // We count how many wraps the cursor had before it to begin with + // so that we can offset any additional wraps to avoid pushing the + // original row contents in to the scrollback. + const wrapped = wrapped: { + var wrapped: usize = 0; + + var row_it = p.rowIterator(.left_up, active_pin); + while (row_it.next()) |next| { + const row = next.rowAndCell().row; + if (row.wrap_continuation) wrapped += 1; + } + + break :wrapped wrapped; + }; + break :cursor .{ .tracked_pin = try self.trackPin(p), .remaining_rows = self.rows - c.y - 1, + .wrapped_rows = wrapped, }; } else null; defer if (preserved_cursor) |c| self.untrackPin(c.tracked_pin); - // Go page by page and shrink the columns on a per-page basis. - var it = self.pageIterator(.right_down, .{ .screen = .{} }, null); - while (it.next()) |chunk| { - // Fast-path: none of our rows are wrapped. In this case we can - // treat this like a no-reflow resize. This only applies if we - // are growing columns. - if (cols > self.cols) no_reflow: { - const page = &chunk.page.data; - const rows = page.rows.ptr(page.memory)[0..page.size.rows]; + const first = self.pages.first.?; + var it = self.rowIterator(.right_down, .{ .screen = .{} }, null); - // If our first row is a wrap continuation, then we have to - // reflow since we're continuing a wrapped line. - if (rows[0].wrap_continuation) break :no_reflow; + const dst_node = try self.createPage(try first.data.capacity.adjust(.{ .cols = cols })); + dst_node.data.size.rows = 1; - // If any row is soft-wrapped then we have to reflow - for (rows) |row| { - if (row.wrap) break :no_reflow; - } + // Set our new page as the only page. This orphans the existing pages + // in the list, but that's fine since we're gonna delete them anyway. + self.pages.first = dst_node; + self.pages.last = dst_node; - try self.resizeWithoutReflowGrowCols(cols, chunk); - continue; + var dst_cursor = ReflowCursor.init(dst_node); + + // Reflow all our rows. + while (it.next()) |row| { + try dst_cursor.reflowRow(self, row); + + // Once we're done reflowing a page, destroy it. + if (row.y == row.page.data.size.rows - 1) { + self.destroyPage(row.page); } - - // Note: we can do a fast-path here if all of our rows in this - // page already fit within the new capacity. In that case we can - // do a non-reflow resize. - try self.reflowPage(cols, chunk.page); } // If our total rows is less than our active rows, we need to grow. @@ -701,6 +714,8 @@ fn resizeCols( c.tracked_pin.*, ) orelse break :cursor; + const active_pin = self.pin(.{ .active = .{} }); + // We need to determine how many rows we wrapped from the original // and subtract that from the remaining rows we expect because if // we wrap down we don't want to push our original row contents into @@ -708,23 +723,25 @@ fn resizeCols( const wrapped = wrapped: { var wrapped: usize = 0; - var row_it = c.tracked_pin.rowIterator(.left_up, null); - _ = row_it.next(); // skip ourselves + var row_it = c.tracked_pin.rowIterator(.left_up, active_pin); while (row_it.next()) |next| { const row = next.rowAndCell().row; - if (!row.wrap) break; - wrapped += 1; + if (row.wrap_continuation) wrapped += 1; } break :wrapped wrapped; }; - // If we wrapped more than we expect, do nothing. - if (wrapped >= c.remaining_rows) break :cursor; - const desired = c.remaining_rows - wrapped; - const current = self.rows - (active_pt.active.y + 1); - if (current >= desired) break :cursor; - for (0..desired - current) |_| _ = try self.grow(); + const current = self.rows - active_pt.active.y - 1; + + var req_rows = c.remaining_rows; + req_rows -|= wrapped -| c.wrapped_rows; + req_rows -|= current; + + while (req_rows > 0) { + _ = try self.grow(); + req_rows -= 1; + } } // Update our cols @@ -739,22 +756,360 @@ const ReflowCursor = struct { x: size.CellCountInt, y: size.CellCountInt, pending_wrap: bool, + node: *List.Node, page: *pagepkg.Page, page_row: *pagepkg.Row, page_cell: *pagepkg.Cell, + new_rows: usize, - fn init(page: *pagepkg.Page) ReflowCursor { + fn init(node: *List.Node) ReflowCursor { + const page = &node.data; const rows = page.rows.ptr(page.memory); return .{ .x = 0, .y = 0, .pending_wrap = false, + .node = node, .page = page, .page_row = &rows[0], .page_cell = &rows[0].cells.ptr(page.memory)[0], + .new_rows = 0, }; } + /// Reflow the provided row in to this cursor. + fn reflowRow( + self: *ReflowCursor, + list: *PageList, + row: Pin, + ) !void { + const src_page = &row.page.data; + const src_row = row.rowAndCell().row; + const src_y = row.y; + + // Inherit increased styles or grapheme bytes from + // the src page we're reflowing from for new pages. + const cap = try src_page.capacity.adjust(.{ .cols = self.page.size.cols }); + + const cells = src_row.cells.ptr(src_page.memory)[0..src_page.size.cols]; + + var cols_len = src_page.size.cols; + + // If the row is wrapped, all empty cells are meaningful. + if (!src_row.wrap) { + while (cols_len > 0) { + if (!cells[cols_len - 1].isEmpty()) break; + cols_len -= 1; + } + + // If the row has a semantic prompt then the blank row is meaningful + // so we just consider pretend the first cell of the row isn't empty. + if (cols_len == 0 and src_row.semantic_prompt != .unknown) cols_len = 1; + } + + // Handle tracked pin adjustments. + { + var it = list.tracked_pins.keyIterator(); + while (it.next()) |p_ptr| { + const p = p_ptr.*; + if (&p.page.data != src_page or + p.y != src_y) continue; + + // If this pin is in the blanks on the right and past the end + // of the dst col width then we move it to the end of the dst + // col width instead. + if (p.x >= cols_len) { + p.x = @min(p.x, cap.cols - 1 - self.x); + } + + // We increase our col len to at least include this pin. + // This ensures that blank rows with pins are processed, + // so that the pins can be properly remapped. + cols_len = @max(cols_len, p.x + 1); + } + } + + // Defer processing of blank rows so that blank rows + // at the end of the page list are never written. + if (cols_len == 0) { + // If this blank row was a wrap continuation somehow + // then we won't need to write it since it should be + // a part of the previously written row. + if (!src_row.wrap_continuation) { + self.new_rows += 1; + } + return; + } + + // Our row isn't blank, write any new rows we deferred. + while (self.new_rows > 0) { + self.new_rows -= 1; + try self.cursorScrollOrNewPage(list, cap); + } + + self.copyRowMetadata(src_row); + + var x: usize = 0; + while (x < cols_len) { + if (self.pending_wrap) { + self.page_row.wrap = true; + try self.cursorScrollOrNewPage(list, cap); + self.copyRowMetadata(src_row); + self.page_row.wrap_continuation = true; + } + + // Move any tracked pins from the source. + { + var it = list.tracked_pins.keyIterator(); + while (it.next()) |p_ptr| { + const p = p_ptr.*; + if (&p.page.data != src_page or + p.y != src_y or + p.x != x) continue; + + p.page = self.node; + p.x = self.x; + p.y = self.y; + } + } + + const cell = &cells[x]; + x += 1; + + // std.log.warn("\nsrc_y={} src_x={} dst_y={} dst_x={} dst_cols={} cp={} wide={}", .{ + // src_y, + // x, + // self.y, + // self.x, + // self.page.size.cols, + // cell.content.codepoint, + // cell.wide, + // }); + + // Copy cell contents. + switch (cell.content_tag) { + .codepoint, + .codepoint_grapheme, + => switch (cell.wide) { + .narrow => self.page_cell.* = cell.*, + + .wide => if (self.page.size.cols > 1) { + if (self.x == self.page.size.cols - 1) { + // If there's a wide character in the last column of + // the reflowed page then we need to insert a spacer + // head and wrap before handling it. + self.page_cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = 0 }, + .wide = .spacer_head, + }; + + // Decrement the source position so that when we + // loop we'll process this source cell again. + x -= 1; + } else { + self.page_cell.* = cell.*; + } + } else { + // Edge case, when resizing to 1 column, wide + // characters are just destroyed and replaced + // with empty narrow cells. + self.page_cell.content.codepoint = 0; + self.page_cell.wide = .narrow; + self.cursorForward(); + // Skip spacer tail so it doesn't cause a wrap. + x += 1; + continue; + }, + + .spacer_tail => if (self.page.size.cols > 1) { + self.page_cell.* = cell.*; + } else { + // Edge case, when resizing to 1 column, wide + // characters are just destroyed and replaced + // with empty narrow cells, so we should just + // discard any spacer tails. + continue; + }, + + .spacer_head => { + // Spacer heads should be ignored. If we need a + // spacer head in our reflowed page, it is added + // when processing the wide cell it belongs to. + continue; + }, + }, + + .bg_color_palette, + .bg_color_rgb, + => { + // These are guaranteed to have no style or grapheme + // data associated with them so we can fast path them. + self.page_cell.* = cell.*; + self.cursorForward(); + continue; + }, + } + + // Prevent integrity checks from tripping + // while copying graphemes and hyperlinks. + if (comptime std.debug.runtime_safety) { + self.page_cell.style_id = stylepkg.default_id; + } + + // Copy grapheme data. + if (cell.content_tag == .codepoint_grapheme) { + // The tag is asserted to be .codepoint in setGraphemes. + self.page_cell.content_tag = .codepoint; + + // Copy the graphemes + const cps = src_page.lookupGrapheme(cell).?; + + // If our page can't support an additional cell with + // graphemes then we create a new page for this row. + if (self.page.graphemeCount() >= self.page.graphemeCapacity() - 1) { + try self.moveLastRowToNewPage(list, cap); + } else { + // Attempt to allocate the space that would be required for + // these graphemes, and if it's not available, create a new + // page for this row. + if (self.page.grapheme_alloc.alloc( + u21, + self.page.memory, + cps.len, + )) |slice| { + self.page.grapheme_alloc.free(self.page.memory, slice); + } else |_| { + try self.moveLastRowToNewPage(list, cap); + } + } + + self.page_row.grapheme = true; + + // This shouldn't fail since we made sure we have space above. + try self.page.setGraphemes(self.page_row, self.page_cell, cps); + } + + // Copy hyperlink data. + if (cell.hyperlink) { + const src_id = src_page.lookupHyperlink(cell).?; + const src_link = src_page.hyperlink_set.get(src_page.memory, src_id); + + // If our page can't support an additional cell with + // a hyperlink ID then we create a new page for this row. + if (self.page.hyperlinkCount() >= self.page.hyperlinkCapacity() - 1) { + try self.moveLastRowToNewPage(list, cap); + } + + const dst_id = self.page.hyperlink_set.addWithIdContext( + self.page.memory, + try src_link.dupe(src_page, self.page), + src_id, + .{ .page = self.page }, + ) catch id: { + // We have no space for this link, + // so make a new page for this row. + try self.moveLastRowToNewPage(list, cap); + + break :id try self.page.hyperlink_set.addContext( + self.page.memory, + try src_link.dupe(src_page, self.page), + .{ .page = self.page }, + ); + } orelse src_id; + + self.page_row.hyperlink = true; + + // We expect this to succeed due to the + // hyperlinkCapacity check we did before. + try self.page.setHyperlink( + self.page_row, + self.page_cell, + dst_id, + ); + } + + // Copy style data. + if (cell.hasStyling()) { + const style = src_page.styles.get( + src_page.memory, + cell.style_id, + ).*; + + const id = self.page.styles.addWithId( + self.page.memory, + style, + cell.style_id, + ) catch id: { + // We have no space for this style, + // so make a new page for this row. + try self.moveLastRowToNewPage(list, cap); + + break :id try self.page.styles.add( + self.page.memory, + style, + ); + } orelse cell.style_id; + + self.page_row.styled = true; + + self.page_cell.style_id = id; + } + + self.cursorForward(); + } + + // If the source row isn't wrapped then we should scroll afterwards. + if (!src_row.wrap) { + self.new_rows += 1; + } + } + + /// Create a new page in the provided list with the provided + /// capacity then clone the row currently being worked on to + /// it and delete it from the old page. Places cursor in the + /// same position it was in in the old row in the new one. + /// + /// Asserts that the cursor is on the final row of the page. + /// + /// Expects that the provided capacity is sufficient to copy + /// the row. + /// + /// If this is the only row in the page, the page is removed + /// from the list after cloning the row. + fn moveLastRowToNewPage( + self: *ReflowCursor, + list: *PageList, + cap: Capacity, + ) !void { + assert(self.y == self.page.size.rows - 1); + assert(!self.pending_wrap); + + const old_node = self.node; + const old_page = self.page; + const old_row = self.page_row; + const old_x = self.x; + + try self.cursorNewPage(list, cap); + + // Restore the x position of the cursor. + self.cursorAbsolute(old_x, 0); + + // We expect to have enough capacity to clone the row. + try self.page.cloneRowFrom(old_page, self.page_row, old_row); + + // Clear the row from the old page and truncate it. + old_page.clearCells(old_row, 0, self.page.size.cols); + old_page.size.rows -= 1; + + // If that was the last row in that page + // then we should remove it from the list. + if (old_page.size.rows == 0) { + list.pages.remove(old_node); + list.destroyPage(old_node); + } + } + /// True if this cursor is at the bottom of the page by capacity, /// i.e. we can't scroll anymore. fn bottom(self: *const ReflowCursor) bool { @@ -776,6 +1131,10 @@ const ReflowCursor = struct { self.cursorAbsolute(self.x, self.y + 1); } + /// Create a new row and move the cursor down. + /// + /// Asserts that the cursor is on the bottom row of the + /// page and that there is capacity to add a new one. fn cursorScroll(self: *ReflowCursor) void { // Scrolling requires that we're on the bottom of our page. // We also assert that we have capacity because reflow always @@ -796,6 +1155,40 @@ const ReflowCursor = struct { self.y += 1; } + /// Create a new page in the provided list with the provided + /// capacity and one row and move the cursor in to it at 0,0 + fn cursorNewPage( + self: *ReflowCursor, + list: *PageList, + cap: Capacity, + ) !void { + // Remember our new row count so we can restore it + // after reinitializing our cursor on the new page. + const new_rows = self.new_rows; + + const node = try list.createPage(cap); + node.data.size.rows = 1; + list.pages.insertAfter(self.node, node); + + self.* = ReflowCursor.init(node); + + self.new_rows = new_rows; + } + + /// Performs `cursorScroll` or `cursorNewPage` as necessary + /// depending on if the cursor is currently at the bottom. + fn cursorScrollOrNewPage( + self: *ReflowCursor, + list: *PageList, + cap: Capacity, + ) !void { + if (self.bottom()) { + try self.cursorNewPage(list, cap); + } else { + self.cursorScroll(); + } + } + fn cursorAbsolute( self: *ReflowCursor, x: size.CellCountInt, @@ -840,446 +1233,6 @@ const ReflowCursor = struct { } }; -/// Reflow the given page into the new capacity. The new capacity can have -/// any number of columns and rows. This will create as many pages as -/// necessary to fit the reflowed text and will remove the old page. -/// -/// Note a couple edge cases: -/// -/// 1. All initial rows that are wrap continuations are ignored. If you -/// want to reflow these lines you must reflow the page with the -/// initially wrapped line. -/// -/// 2. If the last row is wrapped then we will traverse forward to reflow -/// all the continuation rows. This follows from #1. -/// -/// Despite the edge cases above, this will only ever remove the initial -/// node, so that this can be called within a pageIterator. This is a weird -/// detail that will surely cause bugs one day so we should look into fixing -/// it. :) -/// -/// Conceptually, this is a simple process: we're effectively traversing -/// the old page and rewriting into the new page as if it were a text editor. -/// But, due to the edge cases, cursor tracking, and attempts at efficiency, -/// the code can be convoluted so this is going to be a heavily commented -/// function. -fn reflowPage( - self: *PageList, - cols: size.CellCountInt, - initial_node: *List.Node, -) !void { - // The cursor tracks where we are in the source page. - var src_node = initial_node; - var src_cursor = ReflowCursor.init(&src_node.data); - - // Skip initially reflowed lines - if (src_cursor.page_row.wrap_continuation) { - while (src_cursor.page_row.wrap_continuation) { - // If this entire page was continuations then we can remove it. - if (src_cursor.y == src_cursor.page.size.rows - 1) { - // If this is the last page, then we need to insert an empty - // page so that erasePage works. This is a rare scenario that - // can happen in no-scrollback pages where EVERY line is - // a continuation. - if (initial_node.prev == null and initial_node.next == null) { - const cap = try std_capacity.adjust(.{ .cols = cols }); - const node = try self.createPage(cap); - self.pages.insertAfter(initial_node, node); - } - - self.erasePage(initial_node); - return; - } - - src_cursor.cursorDown(); - } - } - - // This is set to true when we're in the middle of completing a wrap - // from the initial page. If this is true, the moment we see a non-wrapped - // row we are done. - var src_completing_wrap = false; - - // This is used to count blank lines so that we don't copy those. - var blank_lines: usize = 0; - - // This is set to true when we're wrapping a line that requires a new - // writer page. - var dst_wrap = false; - - // Our new capacity when growing columns may also shrink rows. So we - // need to do a loop in order to potentially make multiple pages. - dst_loop: while (true) { - // Our cap is based on the source page cap so we can inherit - // potentially increased styles/graphemes/etc. - const cap = try src_cursor.page.capacity.adjust(.{ .cols = cols }); - - // Create our new page and our cursor restarts at 0,0 in the new page. - // The new page always starts with a size of 1 because we know we have - // at least one row to copy from the src. - const dst_node = try self.createPage(cap); - defer dst_node.data.assertIntegrity(); - dst_node.data.size.rows = 1; - var dst_cursor = ReflowCursor.init(&dst_node.data); - dst_cursor.copyRowMetadata(src_cursor.page_row); - - // Set our wrap state - if (dst_wrap) { - dst_cursor.page_row.wrap_continuation = true; - dst_wrap = false; - } - - // Our new page goes before our src node. This will append it to any - // previous pages we've created. - self.pages.insertBefore(initial_node, dst_node); - - src_loop: while (true) { - // Continue traversing the source until we're out of space in our - // destination or we've copied all our intended rows. - const started_completing_wrap = src_completing_wrap; - for (src_cursor.y..src_cursor.page.size.rows) |src_y| { - // If we started completing a wrap and our flag is no longer true - // then we completed it and we can exit the loop. - if (started_completing_wrap and !src_completing_wrap) break; - - // We are previously wrapped if we're not on the first row and - // the previous row was wrapped OR if we're on the first row - // but we're not on our initial node it means the last row of - // our previous page was wrapped. - const prev_wrap = - (src_y > 0 and src_cursor.page_row.wrap) or - (src_y == 0 and src_node != initial_node); - src_cursor.cursorAbsolute(0, @intCast(src_y)); - - // Trim trailing empty cells if the row is not wrapped. If the - // row is wrapped then we don't trim trailing empty cells because - // the empty cells can be meaningful. - const trailing_empty = src_cursor.countTrailingEmptyCells(); - const cols_len = cols_len: { - var cols_len = src_cursor.page.size.cols - trailing_empty; - - if (cols_len > 0) { - // We want to update any tracked pins that are in our - // trailing empty cells to the last col. We don't - // want to wrap blanks. - var it = self.tracked_pins.keyIterator(); - while (it.next()) |p_ptr| { - const p = p_ptr.*; - if (&p.page.data != src_cursor.page or - p.y != src_cursor.y or - p.x < cols_len) continue; - if (p.x >= cap.cols) p.x = cap.cols - 1; - } - - break :cols_len cols_len; - } - - // If a tracked pin is in this row then we need to keep it - // even if it is empty, because it is somehow meaningful - // (usually the screen cursor), but we do trim the cells - // down to the desired size. - // - // The reason we do this logic is because if you do a scroll - // clear (i.e. move all active into scrollback and reset - // the screen), the cursor is on the top line again with - // an empty active. If you resize to a smaller col size we - // don't want to "pull down" all the scrollback again. The - // user expects we just shrink the active area. - var it = self.tracked_pins.keyIterator(); - while (it.next()) |p_ptr| { - const p = p_ptr.*; - if (&p.page.data != src_cursor.page or - p.y != src_cursor.y) continue; - - // If our tracked pin is outside our resized cols, we - // trim it to the last col, we don't want to wrap blanks. - if (p.x >= cap.cols) p.x = cap.cols - 1; - - // We increase our col len to at least include this pin - cols_len = @max(cols_len, p.x + 1); - } - - if (cols_len == 0) { - // If the row is empty, we don't copy it. We count it as a - // blank line and continue to the next row. - blank_lines += 1; - continue; - } - - break :cols_len cols_len; - }; - - // We have data, if we have blank lines we need to create them first. - if (blank_lines > 0) { - // This is a dumb edge caes where if we start with blank - // lines, we're off by one because our cursor is at 0 - // on the first blank line but if its in the middle we - // haven't scrolled yet. Don't worry, this is covered by - // unit tests so if we find a better way we can remove this. - const len = blank_lines - @intFromBool(blank_lines >= src_y); - for (0..len) |i| { - // If we're at the bottom we can't fit anymore into this page, - // so we need to reloop and create a new page. - if (dst_cursor.bottom()) { - blank_lines -= i; - continue :dst_loop; - } - - // TODO: cursor in here - dst_cursor.cursorScroll(); - } - } - - if (src_y > 0) { - // We're done with this row, if this row isn't wrapped, we can - // move our destination cursor to the next row. - // - // The blank_lines == 0 condition is because if we were prefixed - // with blank lines, we handled the scroll already above. - if (!prev_wrap) { - if (dst_cursor.bottom()) continue :dst_loop; - dst_cursor.cursorScroll(); - } - - dst_cursor.copyRowMetadata(src_cursor.page_row); - } - - // Reset our blank line count since handled it all above. - blank_lines = 0; - - for (src_cursor.x..cols_len) |src_x| { - assert(src_cursor.x == src_x); - - // std.log.warn("src_y={} src_x={} dst_y={} dst_x={} dst_cols={} cp={} wide={}", .{ - // src_cursor.y, - // src_cursor.x, - // dst_cursor.y, - // dst_cursor.x, - // dst_cursor.page.size.cols, - // src_cursor.page_cell.content.codepoint, - // src_cursor.page_cell.wide, - // }); - - if (cap.cols > 1) switch (src_cursor.page_cell.wide) { - .narrow => {}, - - .wide => if (!dst_cursor.pending_wrap and - dst_cursor.x == cap.cols - 1) - { - self.reflowUpdateCursor(&src_cursor, &dst_cursor, dst_node); - dst_cursor.page_cell.* = .{ - .content_tag = .codepoint, - .content = .{ .codepoint = 0 }, - .wide = .spacer_head, - }; - dst_cursor.cursorForward(); - assert(dst_cursor.pending_wrap); - }, - - .spacer_head => if (dst_cursor.pending_wrap or - dst_cursor.x != cap.cols - 1) - { - assert(src_cursor.x == src_cursor.page.size.cols - 1); - self.reflowUpdateCursor(&src_cursor, &dst_cursor, dst_node); - continue; - }, - - else => {}, - }; - - if (dst_cursor.pending_wrap) { - dst_cursor.page_row.wrap = true; - if (dst_cursor.bottom()) { - dst_wrap = true; - continue :dst_loop; - } - dst_cursor.cursorScroll(); - dst_cursor.page_row.wrap_continuation = true; - dst_cursor.copyRowMetadata(src_cursor.page_row); - } - - // A rare edge case. If we're resizing down to 1 column - // and the source is a non-narrow character, we reset the - // cell to a narrow blank and we skip to the next cell. - if (cap.cols == 1 and src_cursor.page_cell.wide != .narrow) { - switch (src_cursor.page_cell.wide) { - .narrow => unreachable, - - // Wide char, we delete it, reset it to narrow, - // and skip forward. - .wide => { - dst_cursor.page_cell.content.codepoint = 0; - dst_cursor.page_cell.wide = .narrow; - src_cursor.cursorForward(); - continue; - }, - - // Skip spacer tails since we should've already - // handled them in the previous cell. - .spacer_tail => {}, - - // TODO: test? - .spacer_head => {}, - } - } else { - switch (src_cursor.page_cell.content_tag) { - // These are guaranteed to have no styling data and no - // graphemes, a fast path. - .bg_color_palette, - .bg_color_rgb, - => { - assert(!src_cursor.page_cell.hasStyling()); - assert(!src_cursor.page_cell.hasGrapheme()); - dst_cursor.page_cell.* = src_cursor.page_cell.*; - }, - - .codepoint => { - dst_cursor.page_cell.* = src_cursor.page_cell.*; - }, - - .codepoint_grapheme => { - // We copy the cell like normal but we have to reset the - // tag because this is used for fast-path detection in - // appendGrapheme. - dst_cursor.page_cell.* = src_cursor.page_cell.*; - dst_cursor.page_cell.content_tag = .codepoint; - - // Unset the style ID so our integrity checks don't fire. - // We handle style fixups after this switch block. - if (comptime std.debug.runtime_safety) { - dst_cursor.page_cell.style_id = stylepkg.default_id; - } - - // Copy the graphemes - const src_cps = src_cursor.page.lookupGrapheme(src_cursor.page_cell).?; - for (src_cps) |cp| { - try dst_cursor.page.appendGrapheme( - dst_cursor.page_row, - dst_cursor.page_cell, - cp, - ); - } - }, - } - - // If the source cell has a hyperlink we need to copy it - if (src_cursor.page_cell.hyperlink) { - const src_page = src_cursor.page; - const dst_page = dst_cursor.page; - - // Pause integrity checks because setHyperlink - // calls them but we're not ready yet. - dst_page.pauseIntegrityChecks(true); - defer dst_page.pauseIntegrityChecks(false); - - const id = src_page.lookupHyperlink(src_cursor.page_cell).?; - const src_link = src_page.hyperlink_set.get(src_page.memory, id); - const dst_id = try dst_page.hyperlink_set.addContext( - dst_page.memory, - try src_link.dupe(src_page, dst_page), - .{ .page = dst_page }, - ); - try dst_page.setHyperlink( - dst_cursor.page_row, - dst_cursor.page_cell, - dst_id, - ); - } - - // If the source cell has a style, we need to copy it. - if (src_cursor.page_cell.style_id != stylepkg.default_id) { - const src_style = src_cursor.page.styles.get( - src_cursor.page.memory, - src_cursor.page_cell.style_id, - ).*; - if (try dst_cursor.page.styles.addWithId( - dst_cursor.page.memory, - src_style, - src_cursor.page_cell.style_id, - )) |id| { - dst_cursor.page_cell.style_id = id; - } - dst_cursor.page_row.styled = true; - } - } - - // If our original cursor was on this page, this x/y then - // we need to update to the new location. - self.reflowUpdateCursor(&src_cursor, &dst_cursor, dst_node); - - // Move both our cursors forward - src_cursor.cursorForward(); - dst_cursor.cursorForward(); - } else cursor: { - // We made it through all our source columns. As a final edge - // case, if our cursor is in one of the blanks, we update it - // to the edge of this page. - - // If we are in wrap completion mode and this row is not wrapped - // then we are done and we can gracefully exit our y loop. - if (src_completing_wrap and !src_cursor.page_row.wrap) { - assert(started_completing_wrap); - src_completing_wrap = false; - } - - // If we have no trailing empty cells, it can't be in the blanks. - if (trailing_empty == 0) break :cursor; - - // Update all our tracked pins - var it = self.tracked_pins.keyIterator(); - while (it.next()) |p_ptr| { - const p = p_ptr.*; - if (&p.page.data != src_cursor.page or - p.y != src_cursor.y or - p.x < cols_len) continue; - - p.page = dst_node; - p.y = dst_cursor.y; - } - } - } - - // If we're still in a wrapped line at the end of our page, - // we traverse forward and continue reflowing until we complete - // this entire line. - if (src_cursor.page_row.wrap) wrap: { - src_completing_wrap = true; - src_node = src_node.next orelse break :wrap; - src_cursor = ReflowCursor.init(&src_node.data); - continue :src_loop; - } - - // We are not on a wrapped line, we're truly done. - self.pages.remove(initial_node); - self.destroyPage(initial_node); - return; - } - } -} - -/// This updates the cursor offset if the cursor is exactly on the cell -/// we're currently reflowing. This can then be fixed up later to an exact -/// x/y (see resizeCols). -fn reflowUpdateCursor( - self: *const PageList, - src_cursor: *const ReflowCursor, - dst_cursor: *const ReflowCursor, - dst_node: *List.Node, -) void { - // Update all our tracked pins - var it = self.tracked_pins.keyIterator(); - while (it.next()) |p_ptr| { - const p = p_ptr.*; - if (&p.page.data != src_cursor.page or - p.y != src_cursor.y or - p.x != src_cursor.x) continue; - - p.page = dst_node; - p.x = dst_cursor.x; - p.y = dst_cursor.y; - } -} - fn resizeWithoutReflow(self: *PageList, opts: Resize) !void { // We only set the new min_max_size if we're not reflowing. If we are // reflowing, then resize handles this for us. diff --git a/src/terminal/page.zig b/src/terminal/page.zig index 49e1de475..bc7d3b740 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -1003,6 +1003,33 @@ pub const Page = struct { return self.hyperlink_map.map(self.memory).capacity(); } + /// Set the graphemes for the given cell. This asserts that the cell + /// has no graphemes set, and only contains a single codepoint. + pub fn setGraphemes(self: *Page, row: *Row, cell: *Cell, cps: []u21) Allocator.Error!void { + defer self.assertIntegrity(); + + assert(cell.hasText()); + assert(cell.content_tag == .codepoint); + + const cell_offset = getOffset(Cell, self.memory, cell); + var map = self.grapheme_map.map(self.memory); + + const slice = try self.grapheme_alloc.alloc(u21, self.memory, cps.len); + errdefer self.grapheme_alloc.free(self.memory, slice); + @memcpy(slice, cps); + + try map.putNoClobber(cell_offset, .{ + .offset = getOffset(u21, self.memory, @ptrCast(slice.ptr)), + .len = slice.len, + }); + errdefer map.remove(cell_offset); + + cell.content_tag = .codepoint_grapheme; + row.grapheme = true; + + return; + } + /// Append a codepoint to the given cell as a grapheme. pub fn appendGrapheme(self: *Page, row: *Row, cell: *Cell, cp: u21) Allocator.Error!void { defer self.assertIntegrity(); From 54034468b71b1b0100f72a809ecca079fc71e36e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 9 Jul 2024 09:07:28 -0700 Subject: [PATCH 11/11] terminal: remove errdefer deleted call for refcountedset --- src/terminal/ref_counted_set.zig | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/terminal/ref_counted_set.zig b/src/terminal/ref_counted_set.zig index e19f8f70c..d5c90bd6f 100644 --- a/src/terminal/ref_counted_set.zig +++ b/src/terminal/ref_counted_set.zig @@ -237,11 +237,6 @@ pub fn RefCountedSet( return id; } - // Notify the context that the value is "deleted" if we return an - // error. This allows callers to clean up any resources associated - // with the value. - errdefer if (comptime @hasDecl(Context, "deleted")) ctx.deleted(value); - // If the item doesn't exist, we need an available ID. if (self.next_id >= self.layout.cap) { // Arbitrarily chosen, threshold for rehashing.