From 1d7e87c88d4fbde38f1490add7c5168043ae7ace Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 17 Aug 2024 10:56:53 -0700 Subject: [PATCH 1/7] terminal: index from bottom row of scroll region always makes scrollback Ghostty previously incorrectly only created scrollback if the top/bot margins were the full height of the viewport. The actual xterm behavior is to create scrollback as long as the top margin is the top row and the cursor is on the bottom margin (wherever that may be). --- src/terminal/Terminal.zig | 146 ++++++++++++++++++++++++++------------ 1 file changed, 100 insertions(+), 46 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index bfe288d00..30c27762f 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -1109,63 +1109,98 @@ pub fn index(self: *Terminal) !void { // Scrolling dirties the images because it updates their placements pins. self.screen.kitty_images.dirty = true; - // If our scrolling region is the full screen, we create scrollback. - // Otherwise, we simply scroll the region. + // If our scrolling region is at the top, we create scrollback. if (self.scrolling_region.top == 0 and - self.scrolling_region.bottom == self.rows - 1 and self.scrolling_region.left == 0 and self.scrolling_region.right == self.cols - 1) { - try self.screen.cursorDownScroll(); - } else { - // Slow path for left and right scrolling region margins. - if (self.scrolling_region.left != 0 or - self.scrolling_region.right != self.cols - 1 or + // TODO: check if left/right need to be margin in xterm - // PERF(mitchellh): If we have an SGR background set then - // we need to preserve that background in our erased rows. - // scrollUp does that but eraseRowBounded below does not. - // However, scrollUp is WAY slower. We should optimize this - // case to work in the eraseRowBounded codepath and remove - // this check. - !self.screen.blankCell().isZero()) - { - self.scrollUp(1); + // If our scrolling region is the full screen, this is an + // easy and fast operation since we can just call grow. + if (self.scrolling_region.bottom == self.rows - 1) { + try self.screen.cursorDownScroll(); return; } - // Otherwise use a fast path function from PageList to efficiently - // scroll the contents of the scrolling region. + // Our scrolling region is partially down the screen. In this + // case we need to move the top of the scroll region into + // scrollback while keeping the bottom of the scroll region + // at the bottom of the screen. - // Preserve old cursor just for assertions - const old_cursor = self.screen.cursor; + // To do this today we break it down into a few operations: + // 1. Pretend we're at the bottom of the screen and scroll + // everything up. + // 2. Create a new scroll region from the bottom of the old + // scroll region to the bottom of the screen. + // 3. Use `insertLines` to push the scroll region down. + // 4. Reset the scroll region to the old scroll region. - try self.screen.pages.eraseRowBounded( - .{ .active = .{ .y = self.scrolling_region.top } }, - self.scrolling_region.bottom - self.scrolling_region.top, - ); + // Step 1 (from above) + const prev_x = self.screen.cursor.x; + self.screen.cursorAbsolute(prev_x, self.rows - 1); + try self.screen.cursorDownScroll(); - // eraseRow and eraseRowBounded will end up moving the cursor pin - // up by 1, so we need to move it back down. A `cursorReload` - // would be better option but this is more efficient and this is - // a super hot path so we do this instead. - if (comptime std.debug.runtime_safety) { - assert(self.screen.cursor.x == old_cursor.x); - assert(self.screen.cursor.y == old_cursor.y); - } - self.screen.cursor.y -= 1; - self.screen.cursorDown(1); + // Steps 2-4 (from above) + const old_top = self.scrolling_region.top; + self.scrolling_region.top = self.scrolling_region.bottom; + self.scrolling_region.bottom = self.rows - 1; + self.screen.cursorAbsolute(prev_x, self.scrolling_region.top); + self.insertLines(1); + self.scrolling_region.bottom = self.scrolling_region.top; + self.scrolling_region.top = old_top; + self.screen.cursorAbsolute(prev_x, self.scrolling_region.bottom); - // The operations above can prune our cursor style so we need to - // update. This should never fail because the above can only FREE - // memory. - self.screen.manualStyleUpdate() catch |err| { - std.log.warn("deleteLines manualStyleUpdate err={}", .{err}); - self.screen.cursor.style = .{}; - self.screen.manualStyleUpdate() catch unreachable; - }; + return; } + // Slow path for left and right scrolling region margins. + if (self.scrolling_region.left != 0 or + self.scrolling_region.right != self.cols - 1 or + + // PERF(mitchellh): If we have an SGR background set then + // we need to preserve that background in our erased rows. + // scrollUp does that but eraseRowBounded below does not. + // However, scrollUp is WAY slower. We should optimize this + // case to work in the eraseRowBounded codepath and remove + // this check. + !self.screen.blankCell().isZero()) + { + self.scrollUp(1); + return; + } + + // Otherwise use a fast path function from PageList to efficiently + // scroll the contents of the scrolling region. + + // Preserve old cursor just for assertions + const old_cursor = self.screen.cursor; + + try self.screen.pages.eraseRowBounded( + .{ .active = .{ .y = self.scrolling_region.top } }, + self.scrolling_region.bottom - self.scrolling_region.top, + ); + + // eraseRow and eraseRowBounded will end up moving the cursor pin + // up by 1, so we need to move it back down. A `cursorReload` + // would be better option but this is more efficient and this is + // a super hot path so we do this instead. + if (comptime std.debug.runtime_safety) { + assert(self.screen.cursor.x == old_cursor.x); + assert(self.screen.cursor.y == old_cursor.y); + } + self.screen.cursor.y -= 1; + self.screen.cursorDown(1); + + // The operations above can prune our cursor style so we need to + // update. This should never fail because the above can only FREE + // memory. + self.screen.manualStyleUpdate() catch |err| { + std.log.warn("deleteLines manualStyleUpdate err={}", .{err}); + self.screen.cursor.style = .{}; + self.screen.manualStyleUpdate() catch unreachable; + }; + return; } @@ -6438,7 +6473,7 @@ test "Terminal: index bottom of scroll region with hyperlinks" { test "Terminal: index bottom of scroll region clear hyperlinks" { const alloc = testing.allocator; - var t = try init(alloc, .{ .rows = 5, .cols = 5 }); + var t = try init(alloc, .{ .rows = 5, .cols = 5, .max_scrollback = 0 }); defer t.deinit(alloc); t.setTopAndBottomMargin(1, 2); @@ -6597,11 +6632,31 @@ test "Terminal: index inside left/right margin" { } } -test "Terminal: index bottom of scroll region" { +test "Terminal: index bottom of scroll region creates scrollback" { const alloc = testing.allocator; var t = try init(alloc, .{ .rows = 5, .cols = 5 }); defer t.deinit(alloc); + t.setTopAndBottomMargin(1, 3); + try t.printString("1\n2\n3"); + t.setCursorPos(4, 1); + try t.print('X'); + t.setCursorPos(3, 1); + try t.index(); + try t.print('Y'); + + { + const str = try t.screen.dumpStringAlloc(alloc, .{ .screen = .{} }); + defer testing.allocator.free(str); + try testing.expectEqualStrings("1\n2\n3\nY\nX", str); + } +} + +test "Terminal: index bottom of scroll region no scrollback" { + const alloc = testing.allocator; + var t = try init(alloc, .{ .rows = 5, .cols = 5, .max_scrollback = 0 }); + defer t.deinit(alloc); + t.setTopAndBottomMargin(1, 3); t.setCursorPos(4, 1); try t.print('B'); @@ -6614,7 +6669,6 @@ test "Terminal: index bottom of scroll region" { try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); - try testing.expect(!t.isDirty(.{ .active = .{ .x = 0, .y = 3 } })); { const str = try t.plainString(testing.allocator); From a125dc9682a529bf940018c6fff75cff7b7b5081 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 17 Aug 2024 20:01:47 -0700 Subject: [PATCH 2/7] terminal: add more tests for index, verified that l/r margin handling is good --- src/terminal/Terminal.zig | 48 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 30c27762f..5715bd43c 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -1114,8 +1114,6 @@ pub fn index(self: *Terminal) !void { self.scrolling_region.left == 0 and self.scrolling_region.right == self.cols - 1) { - // TODO: check if left/right need to be margin in xterm - // If our scrolling region is the full screen, this is an // easy and fast operation since we can just call grow. if (self.scrolling_region.bottom == self.rows - 1) { @@ -6645,6 +6643,11 @@ test "Terminal: index bottom of scroll region creates scrollback" { try t.index(); try t.print('Y'); + { + const str = try t.screen.dumpStringAlloc(alloc, .{ .viewport = .{} }); + defer testing.allocator.free(str); + try testing.expectEqualStrings("2\n3\nY\nX", str); + } { const str = try t.screen.dumpStringAlloc(alloc, .{ .screen = .{} }); defer testing.allocator.free(str); @@ -6677,6 +6680,47 @@ test "Terminal: index bottom of scroll region no scrollback" { } } +test "Terminal: index bottom of scroll region blank line preserves SGR" { + const alloc = testing.allocator; + var t = try init(alloc, .{ .rows = 5, .cols = 5 }); + defer t.deinit(alloc); + + t.setTopAndBottomMargin(1, 3); + try t.printString("1\n2\n3"); + t.setCursorPos(4, 1); + try t.print('X'); + t.setCursorPos(3, 1); + try t.setAttribute(.{ .direct_color_bg = .{ + .r = 0xFF, + .g = 0, + .b = 0, + } }); + try t.index(); + + { + const str = try t.screen.dumpStringAlloc(alloc, .{ .viewport = .{} }); + defer testing.allocator.free(str); + try testing.expectEqualStrings("2\n3\n\nX", str); + } + { + const str = try t.screen.dumpStringAlloc(alloc, .{ .screen = .{} }); + defer testing.allocator.free(str); + try testing.expectEqualStrings("1\n2\n3\n\nX", str); + } + for (0..t.cols) |x| { + const list_cell = t.screen.pages.getCell(.{ .active = .{ + .x = @intCast(x), + .y = 2, + } }).?; + try testing.expect(list_cell.cell.content_tag == .bg_color_rgb); + try testing.expectEqual(Cell.RGB{ + .r = 0xFF, + .g = 0, + .b = 0, + }, list_cell.cell.content.color_rgb); + } +} + test "Terminal: cursorUp basic" { const alloc = testing.allocator; var t = try init(alloc, .{ .rows = 5, .cols = 5 }); From 9898489e259e4f1de2d3e0be14cdf7d524374df6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 17 Aug 2024 21:57:07 -0700 Subject: [PATCH 3/7] terminal: add Screen.cursorScrollAbove and tests --- src/terminal/Screen.zig | 214 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 214 insertions(+) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 620267ad5..c0291b85d 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -5,6 +5,7 @@ const Allocator = std.mem.Allocator; const assert = std.debug.assert; const ansi = @import("ansi.zig"); const charsets = @import("charsets.zig"); +const fastmem = @import("../fastmem.zig"); const kitty = @import("kitty.zig"); const sgr = @import("sgr.zig"); const unicode = @import("../unicode/main.zig"); @@ -741,6 +742,100 @@ pub fn cursorDownScroll(self: *Screen) !void { } } +/// This scrolls the active area at and above the cursor. The lines below +/// the cursor are not scrolled. +pub fn cursorScrollAbove(self: *Screen) !void { + // If the cursor is on the bottom of the screen, its faster to use + // our specialized function for that case. + if (self.cursor.y == self.pages.rows - 1) { + return try self.cursorDownScroll(); + } + + defer self.assertIntegrity(); + + // Logic below assumes we always have at least one row that isn't moving + assert(self.cursor.y < self.pages.rows - 1); + + const old_pin = self.cursor.page_pin.*; + if (try self.pages.grow()) |new_page_node| { + // We allocated a new page and went to it. In this case, our new + // empty line is at the top of this page. + + // Prev is never null because pagelist asserts that we always have + // memory for at least two pages. This is an assertion. + assert(new_page_node.prev.? == old_pin.page); + const prev_page = &old_pin.page.data; + const new_page = &new_page_node.data; + + const prev_rows = prev_page.rows.ptr(prev_page.memory.ptr); + const new_rows = new_page.rows.ptr(new_page.memory.ptr); + const prev_last_row = &prev_rows[prev_page.size.rows - 1]; + + // First, copy the last row of the previous page to the top + // of our current page. + try new_page.cloneRowFrom( + prev_page, + &new_rows[0], + prev_last_row, + ); + + // Update our cursor metadata now. We call methods below that assert + // integrity in debug modes so we want to put ourselves in a + // consistent state first. + self.cursor.page_pin.* = self.cursor.page_pin.down(1).?; + self.cursorChangePin(self.cursor.page_pin.*); + const page_rac = self.cursor.page_pin.rowAndCell(); + self.cursor.page_row = page_rac.row; + self.cursor.page_cell = page_rac.cell; + + // Third, clear the last row of the previous page. + self.clearCells( + prev_page, + prev_last_row, + prev_page.getCells(prev_last_row), + ); + var dirty = prev_page.dirtyBitSet(); + dirty.set(prev_page.size.rows - 1); + } else { + // In this case, it means grow() didn't allocate a new page. This + // allows us to perform a fast path by rotating rows on the same page. + assert(old_pin.page == self.cursor.page_pin.page); + self.cursor.page_pin.* = self.cursor.page_pin.down(1).?; + + const pin = self.cursor.page_pin; + const page = &self.cursor.page_pin.page.data; + + // Rotate the rows so that the newly created empty row is at the + // beginning. e.g. [ 0 1 2 3 ] in to [ 3 0 1 2 ]. + var rows = page.rows.ptr(page.memory.ptr); + fastmem.rotateOnceR(Row, rows[pin.y..page.size.rows]); + + // Mark all our rotated rows as dirty. + var dirty = page.dirtyBitSet(); + dirty.setRangeValue(.{ .start = pin.y, .end = page.size.rows }, true); + + // Setup our cursor caches after the rotation so it points to the + // correct data + const page_rac = self.cursor.page_pin.rowAndCell(); + self.cursor.page_row = page_rac.row; + self.cursor.page_cell = page_rac.cell; + + // Note: we don't need to call cursorChangePin here because + // the pin page is the same so there is no accounting to do for + // styles or any of that. + } + + if (self.cursor.style_id != style.default_id) { + // The newly created line needs to be styled according to + // the bg color if it is set. + if (self.cursor.style.bgCell()) |blank_cell| { + const cell_current: [*]pagepkg.Cell = @ptrCast(self.cursor.page_cell); + const cells = cell_current - self.cursor.x; + @memset(cells[0..self.pages.cols], blank_cell); + } + } +} + /// Move the cursor down if we're not at the bottom of the screen. Otherwise /// scroll. Currently only used for testing. fn cursorDownOrScroll(self: *Screen) !void { @@ -3817,6 +3912,125 @@ test "Screen: scroll and clear ignore blank lines" { } } +test "Screen: scroll above same page" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 3, 10); + defer s.deinit(); + try s.setAttribute(.{ .direct_color_bg = .{ .r = 155 } }); + try s.testWriteString("1ABCD\n2EFGH\n3IJKL"); + s.cursorAbsolute(0, 1); + s.pages.clearDirty(); + try s.cursorScrollAbove(); + + { + const contents = try s.dumpStringAlloc(alloc, .{ .viewport = .{} }); + defer alloc.free(contents); + try testing.expectEqualStrings("2EFGH\n\n3IJKL", contents); + } + { + const list_cell = s.pages.getCell(.{ .active = .{ .x = 0, .y = 1 } }).?; + const cell = list_cell.cell; + try testing.expect(cell.content_tag == .bg_color_rgb); + try testing.expectEqual(Cell.RGB{ + .r = 155, + .g = 0, + .b = 0, + }, cell.content.color_rgb); + } + + // Only y=1,2 are dirty because they are the ones that CHANGED contents + // (not just scroll). + try testing.expect(!s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); +} + +test "Screen: scroll above creates new page" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 3, 10); + defer s.deinit(); + + // We need to get the cursor to a new page + const first_page_size = s.pages.pages.first.?.data.capacity.rows; + for (0..first_page_size - 3) |_| try s.testWriteString("\n"); + + try s.setAttribute(.{ .direct_color_bg = .{ .r = 155 } }); + try s.testWriteString("1ABCD\n2EFGH\n3IJKL"); + s.cursorAbsolute(0, 1); + s.pages.clearDirty(); + + // Ensure we're still on the first page + try testing.expect(s.cursor.page_pin.page == s.pages.pages.first.?); + try s.cursorScrollAbove(); + + // const stderr = std.io.getStdErr().writer(); + // try s.pages.diagram(stderr); + + { + const contents = try s.dumpStringAlloc(alloc, .{ .viewport = .{} }); + defer alloc.free(contents); + try testing.expectEqualStrings("2EFGH\n\n3IJKL", contents); + } + { + const list_cell = s.pages.getCell(.{ .active = .{ .x = 0, .y = 1 } }).?; + const cell = list_cell.cell; + try testing.expect(cell.content_tag == .bg_color_rgb); + try testing.expectEqual(Cell.RGB{ + .r = 155, + .g = 0, + .b = 0, + }, cell.content.color_rgb); + } + + // Only y=1 is dirty because they are the ones that CHANGED contents + try testing.expect(!s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); + try testing.expect(!s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); +} + +test "Screen: scroll above no scrollback bottom of page" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 3, 0); + defer s.deinit(); + + const first_page_size = s.pages.pages.first.?.data.capacity.rows; + for (0..first_page_size - 3) |_| try s.testWriteString("\n"); + + try s.setAttribute(.{ .direct_color_bg = .{ .r = 155 } }); + try s.testWriteString("1ABCD\n2EFGH\n3IJKL"); + s.cursorAbsolute(0, 1); + s.pages.clearDirty(); + try s.cursorScrollAbove(); + + { + const contents = try s.dumpStringAlloc(alloc, .{ .viewport = .{} }); + defer alloc.free(contents); + try testing.expectEqualStrings("2EFGH\n\n3IJKL", contents); + } + { + const list_cell = s.pages.getCell(.{ .active = .{ .x = 0, .y = 1 } }).?; + const cell = list_cell.cell; + try testing.expect(cell.content_tag == .bg_color_rgb); + try testing.expectEqual(Cell.RGB{ + .r = 155, + .g = 0, + .b = 0, + }, cell.content.color_rgb); + } + + // Only y=1,2 are dirty because they are the ones that CHANGED contents + // (not just scroll). + try testing.expect(!s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); +} + test "Screen: clone" { const testing = std.testing; const alloc = testing.allocator; From adb382f1c8253cdb13cc75f2b53dc976953ecb4b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 17 Aug 2024 22:00:52 -0700 Subject: [PATCH 4/7] terminal: call new method for scroll operation --- src/terminal/Terminal.zig | 36 +++++------------------------------- 1 file changed, 5 insertions(+), 31 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 5715bd43c..dd30d49b7 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -1125,30 +1125,7 @@ pub fn index(self: *Terminal) !void { // case we need to move the top of the scroll region into // scrollback while keeping the bottom of the scroll region // at the bottom of the screen. - - // To do this today we break it down into a few operations: - // 1. Pretend we're at the bottom of the screen and scroll - // everything up. - // 2. Create a new scroll region from the bottom of the old - // scroll region to the bottom of the screen. - // 3. Use `insertLines` to push the scroll region down. - // 4. Reset the scroll region to the old scroll region. - - // Step 1 (from above) - const prev_x = self.screen.cursor.x; - self.screen.cursorAbsolute(prev_x, self.rows - 1); - try self.screen.cursorDownScroll(); - - // Steps 2-4 (from above) - const old_top = self.scrolling_region.top; - self.scrolling_region.top = self.scrolling_region.bottom; - self.scrolling_region.bottom = self.rows - 1; - self.screen.cursorAbsolute(prev_x, self.scrolling_region.top); - self.insertLines(1); - self.scrolling_region.bottom = self.scrolling_region.top; - self.scrolling_region.top = old_top; - self.screen.cursorAbsolute(prev_x, self.scrolling_region.bottom); - + try self.screen.cursorScrollAbove(); return; } @@ -6474,7 +6451,8 @@ test "Terminal: index bottom of scroll region clear hyperlinks" { var t = try init(alloc, .{ .rows = 5, .cols = 5, .max_scrollback = 0 }); defer t.deinit(alloc); - t.setTopAndBottomMargin(1, 2); + t.setTopAndBottomMargin(2, 3); + t.setCursorPos(2, 1); try t.screen.startHyperlink("http://example.com", null); try t.print('A'); t.screen.endHyperlink(); @@ -6488,10 +6466,10 @@ test "Terminal: index bottom of scroll region clear hyperlinks" { { const str = try t.plainString(testing.allocator); defer testing.allocator.free(str); - try testing.expectEqualStrings("B\nC", str); + try testing.expectEqualStrings("\nB\nC", str); } - for (0..2) |y| { + for (1..3) |y| { const list_cell = t.screen.pages.getCell(.{ .viewport = .{ .x = 0, .y = @intCast(y), @@ -6669,10 +6647,6 @@ test "Terminal: index bottom of scroll region no scrollback" { try t.index(); try t.print('X'); - try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); - try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); - try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); - { const str = try t.plainString(testing.allocator); defer testing.allocator.free(str); From 1028fe1c56809296a85d975807b46a0c633bcddc Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 17 Aug 2024 22:02:48 -0700 Subject: [PATCH 5/7] terminal: only call new method --- src/terminal/Terminal.zig | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index dd30d49b7..667e28650 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -1114,17 +1114,6 @@ pub fn index(self: *Terminal) !void { self.scrolling_region.left == 0 and self.scrolling_region.right == self.cols - 1) { - // If our scrolling region is the full screen, this is an - // easy and fast operation since we can just call grow. - if (self.scrolling_region.bottom == self.rows - 1) { - try self.screen.cursorDownScroll(); - return; - } - - // Our scrolling region is partially down the screen. In this - // case we need to move the top of the scroll region into - // scrollback while keeping the bottom of the scroll region - // at the bottom of the screen. try self.screen.cursorScrollAbove(); return; } From 602fea52eca7de8d6b8305aa07e05000e86f6721 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 18 Aug 2024 10:14:35 -0700 Subject: [PATCH 6/7] terminal: cursorScrollAbove handles case of no new page, prev page --- src/terminal/Screen.zig | 255 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 231 insertions(+), 24 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index c0291b85d..7da1c2edc 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -797,32 +797,109 @@ pub fn cursorScrollAbove(self: *Screen) !void { var dirty = prev_page.dirtyBitSet(); dirty.set(prev_page.size.rows - 1); } else { - // In this case, it means grow() didn't allocate a new page. This - // allows us to perform a fast path by rotating rows on the same page. - assert(old_pin.page == self.cursor.page_pin.page); - self.cursor.page_pin.* = self.cursor.page_pin.down(1).?; + // In this case, it means grow() didn't allocate a new page. - const pin = self.cursor.page_pin; - const page = &self.cursor.page_pin.page.data; + if (self.cursor.page_pin.page == self.pages.pages.last) { + // If we're on the last page we can do a very fast path because + // all the rows we need to move around are within a single page. - // Rotate the rows so that the newly created empty row is at the - // beginning. e.g. [ 0 1 2 3 ] in to [ 3 0 1 2 ]. - var rows = page.rows.ptr(page.memory.ptr); - fastmem.rotateOnceR(Row, rows[pin.y..page.size.rows]); + assert(old_pin.page == self.cursor.page_pin.page); + self.cursor.page_pin.* = self.cursor.page_pin.down(1).?; - // Mark all our rotated rows as dirty. - var dirty = page.dirtyBitSet(); - dirty.setRangeValue(.{ .start = pin.y, .end = page.size.rows }, true); + const pin = self.cursor.page_pin; + const page = &self.cursor.page_pin.page.data; - // Setup our cursor caches after the rotation so it points to the - // correct data - const page_rac = self.cursor.page_pin.rowAndCell(); - self.cursor.page_row = page_rac.row; - self.cursor.page_cell = page_rac.cell; + // Rotate the rows so that the newly created empty row is at the + // beginning. e.g. [ 0 1 2 3 ] in to [ 3 0 1 2 ]. + var rows = page.rows.ptr(page.memory.ptr); + fastmem.rotateOnceR(Row, rows[pin.y..page.size.rows]); - // Note: we don't need to call cursorChangePin here because - // the pin page is the same so there is no accounting to do for - // styles or any of that. + // Mark all our rotated rows as dirty. + var dirty = page.dirtyBitSet(); + dirty.setRangeValue(.{ .start = pin.y, .end = page.size.rows }, true); + + // Setup our cursor caches after the rotation so it points to the + // correct data + const page_rac = self.cursor.page_pin.rowAndCell(); + self.cursor.page_row = page_rac.row; + self.cursor.page_cell = page_rac.cell; + + // Note: we don't need to call cursorChangePin here because + // the pin page is the same so there is no accounting to do for + // styles or any of that. + } else { + // We didn't grow pages but our cursor isn't on the last page. + // In this case we need to do more work because we need to copy + // elements between pages. + // + // An example scenario of this is shown below: + // + // +----------+ = PAGE 0 + // ... : : + // +-------------+ ACTIVE + // 4302 |1A00000000| | 0 + // 4303 |2B00000000| | 1 + // :^ : : = PIN 0 + // 4304 |3C00000000| | 2 + // +----------+ : + // +----------+ : = PAGE 1 + // 0 |4D00000000| | 3 + // 1 |5E00000000| | 4 + // +----------+ : + // +-------------+ + + self.cursor.page_pin.* = self.cursor.page_pin.down(1).?; + + // Go through each of the pages following our pin, shift all rows + // down by one, and copy the last row of the previous page. + var current = self.pages.pages.last.?; + while (current != self.cursor.page_pin.page) : (current = current.prev.?) { + const prev = current.prev.?; + const prev_page = &prev.data; + const cur_page = ¤t.data; + const prev_rows = prev_page.rows.ptr(prev_page.memory.ptr); + const cur_rows = cur_page.rows.ptr(cur_page.memory.ptr); + + // Rotate the pages down: [ 0 1 2 3 ] => [ 3 0 1 2 ] + fastmem.rotateOnceR(Row, cur_rows[0..cur_page.size.rows]); + + // Copy the last row of the previous page to the top of current. + try cur_page.cloneRowFrom( + prev_page, + &cur_rows[0], + &prev_rows[prev_page.size.rows - 1], + ); + + // All rows we rotated are dirty + var dirty = cur_page.dirtyBitSet(); + dirty.setRangeValue(.{ .start = 0, .end = cur_page.size.rows }, true); + } + + // Our current is our cursor page, we need to rotate down from + // our cursor and clear our row. + assert(current == self.cursor.page_pin.page); + const cur_page = ¤t.data; + const cur_rows = cur_page.rows.ptr(cur_page.memory.ptr); + fastmem.rotateOnceR(Row, cur_rows[self.cursor.page_pin.y..cur_page.size.rows]); + self.clearCells( + cur_page, + &cur_rows[0], + cur_page.getCells(&cur_rows[0]), + ); + + // Set all the rows we rotated and cleared dirty + var dirty = cur_page.dirtyBitSet(); + dirty.setRangeValue( + .{ .start = self.cursor.page_pin.y, .end = cur_page.size.rows }, + true, + ); + + // Setup cursor cache data after all the rotations so our + // row is valid. + const page_rac = self.cursor.page_pin.rowAndCell(); + self.cursor.page_row = page_rac.row; + self.cursor.page_cell = page_rac.cell; + } } if (self.cursor.style_id != style.default_id) { @@ -3947,6 +4024,139 @@ test "Screen: scroll above same page" { try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); } +test "Screen: scroll above same page but cursor on previous page" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 5, 10); + defer s.deinit(); + + // We need to get the cursor to a new page + const first_page_size = s.pages.pages.first.?.data.capacity.rows; + for (0..first_page_size - 3) |_| try s.testWriteString("\n"); + + try s.setAttribute(.{ .direct_color_bg = .{ .r = 155 } }); + try s.testWriteString("1A\n2B\n3C\n4D\n5E"); + s.cursorAbsolute(0, 1); + s.pages.clearDirty(); + + // Ensure we're still on the first page and have a second + try testing.expect(s.cursor.page_pin.page == s.pages.pages.first.?); + try testing.expect(s.pages.pages.first.?.next != null); + + // At this point: + // +----------+ = PAGE 0 + // ... : : + // +-------------+ ACTIVE + // 4303 |1A00000000| | 0 + // 4304 |2B00000000| | 1 + // :^ : : = PIN 0 + // +----------+ : + // +----------+ : = PAGE 1 + // 0 |3C00000000| | 2 + // 1 |4D00000000| | 3 + // 2 |5E00000000| | 4 + // +----------+ : + // +-------------+ + + try s.cursorScrollAbove(); + + { + const contents = try s.dumpStringAlloc(alloc, .{ .viewport = .{} }); + defer alloc.free(contents); + try testing.expectEqualStrings("2B\n\n3C\n4D\n5E", contents); + } + { + const list_cell = s.pages.getCell(.{ .active = .{ .x = 0, .y = 1 } }).?; + const cell = list_cell.cell; + try testing.expect(cell.content_tag == .bg_color_rgb); + try testing.expectEqual(Cell.RGB{ + .r = 155, + .g = 0, + .b = 0, + }, cell.content.color_rgb); + } + + try testing.expect(!s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); +} + +test "Screen: scroll above same page but cursor on previous page last row" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 5, 10); + defer s.deinit(); + + // We need to get the cursor to a new page + const first_page_size = s.pages.pages.first.?.data.capacity.rows; + for (0..first_page_size - 2) |_| try s.testWriteString("\n"); + + try s.setAttribute(.{ .direct_color_bg = .{ .r = 155 } }); + try s.testWriteString("1A\n2B\n3C\n4D\n5E"); + s.cursorAbsolute(0, 1); + s.pages.clearDirty(); + + // Ensure we're still on the first page and have a second + try testing.expect(s.cursor.page_pin.page == s.pages.pages.first.?); + try testing.expect(s.pages.pages.first.?.next != null); + + // At this point: + // +----------+ = PAGE 0 + // ... : : + // +-------------+ ACTIVE + // 4303 |1A00000000| | 0 + // 4304 |2B00000000| | 1 + // :^ : : = PIN 0 + // +----------+ : + // +----------+ : = PAGE 1 + // 0 |3C00000000| | 2 + // 1 |4D00000000| | 3 + // 2 |5E00000000| | 4 + // +----------+ : + // +-------------+ + + try s.cursorScrollAbove(); + + // +----------+ = PAGE 0 + // ... : : + // 4303 |1A00000000| + // +-------------+ ACTIVE + // 4304 |2B00000000| | 0 + // +----------+ : + // +----------+ : = PAGE 1 + // 0 | | | 1 + // :^ : : = PIN 0 + // 1 |3C00000000| | 2 + // 2 |4D00000000| | 3 + // 3 |5E00000000| | 4 + // +----------+ : + // +-------------+ + + // try s.pages.diagram(std.io.getStdErr().writer()); + + { + const contents = try s.dumpStringAlloc(alloc, .{ .viewport = .{} }); + defer alloc.free(contents); + try testing.expectEqualStrings("2B\n\n3C\n4D\n5E", contents); + } + { + const list_cell = s.pages.getCell(.{ .active = .{ .x = 0, .y = 1 } }).?; + const cell = list_cell.cell; + try testing.expect(cell.content_tag == .bg_color_rgb); + try testing.expectEqual(Cell.RGB{ + .r = 155, + .g = 0, + .b = 0, + }, cell.content.color_rgb); + } + + try testing.expect(!s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); +} + test "Screen: scroll above creates new page" { const testing = std.testing; const alloc = testing.allocator; @@ -3967,9 +4177,6 @@ test "Screen: scroll above creates new page" { try testing.expect(s.cursor.page_pin.page == s.pages.pages.first.?); try s.cursorScrollAbove(); - // const stderr = std.io.getStdErr().writer(); - // try s.pages.diagram(stderr); - { const contents = try s.dumpStringAlloc(alloc, .{ .viewport = .{} }); defer alloc.free(contents); From 994514981f62d02a94cbad80adafdf3e3d129f2a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 18 Aug 2024 10:20:23 -0700 Subject: [PATCH 7/7] terminal: handle case grow allocates but cursor is multiple pages back --- src/terminal/Screen.zig | 150 ++++++++++++++++------------------------ 1 file changed, 58 insertions(+), 92 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 7da1c2edc..54feb3c69 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -757,45 +757,8 @@ pub fn cursorScrollAbove(self: *Screen) !void { assert(self.cursor.y < self.pages.rows - 1); const old_pin = self.cursor.page_pin.*; - if (try self.pages.grow()) |new_page_node| { - // We allocated a new page and went to it. In this case, our new - // empty line is at the top of this page. - - // Prev is never null because pagelist asserts that we always have - // memory for at least two pages. This is an assertion. - assert(new_page_node.prev.? == old_pin.page); - const prev_page = &old_pin.page.data; - const new_page = &new_page_node.data; - - const prev_rows = prev_page.rows.ptr(prev_page.memory.ptr); - const new_rows = new_page.rows.ptr(new_page.memory.ptr); - const prev_last_row = &prev_rows[prev_page.size.rows - 1]; - - // First, copy the last row of the previous page to the top - // of our current page. - try new_page.cloneRowFrom( - prev_page, - &new_rows[0], - prev_last_row, - ); - - // Update our cursor metadata now. We call methods below that assert - // integrity in debug modes so we want to put ourselves in a - // consistent state first. - self.cursor.page_pin.* = self.cursor.page_pin.down(1).?; - self.cursorChangePin(self.cursor.page_pin.*); - const page_rac = self.cursor.page_pin.rowAndCell(); - self.cursor.page_row = page_rac.row; - self.cursor.page_cell = page_rac.cell; - - // Third, clear the last row of the previous page. - self.clearCells( - prev_page, - prev_last_row, - prev_page.getCells(prev_last_row), - ); - var dirty = prev_page.dirtyBitSet(); - dirty.set(prev_page.size.rows - 1); + if (try self.pages.grow()) |_| { + try self.cursorScrollAboveRotate(); } else { // In this case, it means grow() didn't allocate a new page. @@ -847,58 +810,7 @@ pub fn cursorScrollAbove(self: *Screen) !void { // 1 |5E00000000| | 4 // +----------+ : // +-------------+ - - self.cursor.page_pin.* = self.cursor.page_pin.down(1).?; - - // Go through each of the pages following our pin, shift all rows - // down by one, and copy the last row of the previous page. - var current = self.pages.pages.last.?; - while (current != self.cursor.page_pin.page) : (current = current.prev.?) { - const prev = current.prev.?; - const prev_page = &prev.data; - const cur_page = ¤t.data; - const prev_rows = prev_page.rows.ptr(prev_page.memory.ptr); - const cur_rows = cur_page.rows.ptr(cur_page.memory.ptr); - - // Rotate the pages down: [ 0 1 2 3 ] => [ 3 0 1 2 ] - fastmem.rotateOnceR(Row, cur_rows[0..cur_page.size.rows]); - - // Copy the last row of the previous page to the top of current. - try cur_page.cloneRowFrom( - prev_page, - &cur_rows[0], - &prev_rows[prev_page.size.rows - 1], - ); - - // All rows we rotated are dirty - var dirty = cur_page.dirtyBitSet(); - dirty.setRangeValue(.{ .start = 0, .end = cur_page.size.rows }, true); - } - - // Our current is our cursor page, we need to rotate down from - // our cursor and clear our row. - assert(current == self.cursor.page_pin.page); - const cur_page = ¤t.data; - const cur_rows = cur_page.rows.ptr(cur_page.memory.ptr); - fastmem.rotateOnceR(Row, cur_rows[self.cursor.page_pin.y..cur_page.size.rows]); - self.clearCells( - cur_page, - &cur_rows[0], - cur_page.getCells(&cur_rows[0]), - ); - - // Set all the rows we rotated and cleared dirty - var dirty = cur_page.dirtyBitSet(); - dirty.setRangeValue( - .{ .start = self.cursor.page_pin.y, .end = cur_page.size.rows }, - true, - ); - - // Setup cursor cache data after all the rotations so our - // row is valid. - const page_rac = self.cursor.page_pin.rowAndCell(); - self.cursor.page_row = page_rac.row; - self.cursor.page_cell = page_rac.cell; + try self.cursorScrollAboveRotate(); } } @@ -913,6 +825,60 @@ pub fn cursorScrollAbove(self: *Screen) !void { } } +fn cursorScrollAboveRotate(self: *Screen) !void { + self.cursor.page_pin.* = self.cursor.page_pin.down(1).?; + + // Go through each of the pages following our pin, shift all rows + // down by one, and copy the last row of the previous page. + var current = self.pages.pages.last.?; + while (current != self.cursor.page_pin.page) : (current = current.prev.?) { + const prev = current.prev.?; + const prev_page = &prev.data; + const cur_page = ¤t.data; + const prev_rows = prev_page.rows.ptr(prev_page.memory.ptr); + const cur_rows = cur_page.rows.ptr(cur_page.memory.ptr); + + // Rotate the pages down: [ 0 1 2 3 ] => [ 3 0 1 2 ] + fastmem.rotateOnceR(Row, cur_rows[0..cur_page.size.rows]); + + // Copy the last row of the previous page to the top of current. + try cur_page.cloneRowFrom( + prev_page, + &cur_rows[0], + &prev_rows[prev_page.size.rows - 1], + ); + + // All rows we rotated are dirty + var dirty = cur_page.dirtyBitSet(); + dirty.setRangeValue(.{ .start = 0, .end = cur_page.size.rows }, true); + } + + // Our current is our cursor page, we need to rotate down from + // our cursor and clear our row. + assert(current == self.cursor.page_pin.page); + const cur_page = ¤t.data; + const cur_rows = cur_page.rows.ptr(cur_page.memory.ptr); + fastmem.rotateOnceR(Row, cur_rows[self.cursor.page_pin.y..cur_page.size.rows]); + self.clearCells( + cur_page, + &cur_rows[0], + cur_page.getCells(&cur_rows[0]), + ); + + // Set all the rows we rotated and cleared dirty + var dirty = cur_page.dirtyBitSet(); + dirty.setRangeValue( + .{ .start = self.cursor.page_pin.y, .end = cur_page.size.rows }, + true, + ); + + // Setup cursor cache data after all the rotations so our + // row is valid. + const page_rac = self.cursor.page_pin.rowAndCell(); + self.cursor.page_row = page_rac.row; + self.cursor.page_cell = page_rac.cell; +} + /// Move the cursor down if we're not at the bottom of the screen. Otherwise /// scroll. Currently only used for testing. fn cursorDownOrScroll(self: *Screen) !void { @@ -4196,7 +4162,7 @@ test "Screen: scroll above creates new page" { // Only y=1 is dirty because they are the ones that CHANGED contents try testing.expect(!s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); - try testing.expect(!s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); } test "Screen: scroll above no scrollback bottom of page" {