From 21c6026922c23c34d1201eec77db80732eabb8d1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 23 Feb 2024 13:17:52 -0800 Subject: [PATCH] terminal/new: pagelist doesn't actively maintain active offset --- src/terminal/new/PageList.zig | 40 +++++++++++++++++++++++++---------- src/terminal/new/Screen.zig | 19 +++++++---------- src/terminal/new/Terminal.zig | 11 +++++----- 3 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/terminal/new/PageList.zig b/src/terminal/new/PageList.zig index 2f3d407dc..11a6cf6eb 100644 --- a/src/terminal/new/PageList.zig +++ b/src/terminal/new/PageList.zig @@ -57,7 +57,6 @@ pages: List, /// - history: active row minus one /// viewport: Viewport, -active: RowOffset, /// The current desired screen dimensions. I say "desired" because individual /// pages may still be a different size and not yet reflowed since we lazily @@ -120,7 +119,6 @@ pub fn init( .page_pool = page_pool, .pages = page_list, .viewport = .{ .active = {} }, - .active = .{ .page = page }, }; } @@ -224,10 +222,30 @@ pub fn rowIterator( /// Get the top-left of the screen for the given tag. fn getTopLeft(self: *const PageList, tag: point.Tag) RowOffset { return switch (tag) { - .active => self.active, + // The full screen or history is always just the first page. .screen, .history => .{ .page = self.pages.first.? }, + .viewport => switch (self.viewport) { - .active => self.active, + // If the viewport is in the active area then its the same as active. + .active => self.getTopLeft(.active), + }, + + // The active area is calculated backwards from the last page. + // This makes getting the active top left slower but makes scrolling + // much faster because we don't need to update the top left. Under + // heavy load this makes a measurable difference. + .active => active: { + var page = self.pages.last.?; + var rem = self.rows; + while (rem > page.data.size.rows) { + rem -= page.data.size.rows; + page = page.prev.?; // assertion: we always have enough rows for active + } + + break :active .{ + .page = page, + .row_offset = page.data.size.rows - rem, + }; }, }; } @@ -311,12 +329,12 @@ test "PageList" { var s = try init(alloc, 80, 24, 1000); defer s.deinit(); - - // Viewport is setup try testing.expect(s.viewport == .active); - try testing.expect(s.active.page == s.pages.first); - try testing.expect(s.active.page.next == null); - try testing.expect(s.active.row_offset == 0); - try testing.expect(s.active.page.data.size.cols == 80); - try testing.expect(s.active.page.data.size.rows == 24); + try testing.expect(s.pages.first != null); + + // Active area should be the top + try testing.expectEqual(RowOffset{ + .page = s.pages.first.?, + .row_offset = 0, + }, s.getTopLeft(.active)); } diff --git a/src/terminal/new/Screen.zig b/src/terminal/new/Screen.zig index 7f6552675..12c0810b2 100644 --- a/src/terminal/new/Screen.zig +++ b/src/terminal/new/Screen.zig @@ -51,13 +51,17 @@ pub fn init( rows: size.CellCountInt, max_scrollback: usize, ) !Screen { - // Initialize our backing pages. This will initialize the viewport. + // Initialize our backing pages. var pages = try PageList.init(alloc, cols, rows, max_scrollback); errdefer pages.deinit(); - // The viewport is guaranteed to exist, so grab it so we can setup - // our initial cursor. - const page_offset = pages.rowOffset(.{ .active = .{ .x = 0, .y = 0 } }); + // The active area is guaranteed to be allocated and the first + // page in the list after init. This lets us quickly setup the cursor. + // This is MUCH faster than pages.rowOffset. + const page_offset: PageList.RowOffset = .{ + .page = pages.pages.first.?, + .row_offset = 0, + }; const page_rac = page_offset.rowAndCell(0); return .{ @@ -146,13 +150,6 @@ pub fn cursorDownScroll(self: *Screen) !void { self.cursor.page_offset = page_offset; self.cursor.page_row = page_rac.row; self.cursor.page_cell = page_rac.cell; - - // try self.pages.scrollActive(1); - // const page_offset = self.pages.active.forward(self.cursor.y).?; - // const page_rac = page_offset.rowAndCell(self.cursor.x); - // self.cursor.page_offset = page_offset; - // self.cursor.page_row = page_rac.row; - // self.cursor.page_cell = page_rac.cell; } /// Dump the screen to a string. The writer given should be buffered; diff --git a/src/terminal/new/Terminal.zig b/src/terminal/new/Terminal.zig index 6f5af80f9..a7ff10a44 100644 --- a/src/terminal/new/Terminal.zig +++ b/src/terminal/new/Terminal.zig @@ -461,12 +461,11 @@ test "Terminal: input that forces scroll" { for ("abcdef") |c| try t.print(c); try testing.expectEqual(@as(usize, 4), t.screen.cursor.y); try testing.expectEqual(@as(usize, 0), t.screen.cursor.x); - // TODO once viewport is moved - // { - // const str = try t.plainString(alloc); - // defer alloc.free(str); - // try testing.expectEqualStrings("b\nc\nd\ne\nf", str); - // } + { + const str = try t.plainString(alloc); + defer alloc.free(str); + try testing.expectEqualStrings("b\nc\nd\ne\nf", str); + } } test "Terminal: zero-width character at start" {