diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index b7ab72049..6a159d463 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -2042,7 +2042,11 @@ pub fn eraseRow( // 5 5 5 | 6 // 6 6 6 | 7 // 7 7 7 <' 4 - try page.data.cloneRowFrom(&next.data, &rows[page.data.size.rows - 1], &next_rows[0]); + try page.data.cloneRowFrom( + &next.data, + &rows[page.data.size.rows - 1], + &next_rows[0], + ); page = next; rows = next_rows; @@ -2101,7 +2105,9 @@ pub fn eraseRowBounded( var pin_it = self.tracked_pins.keyIterator(); while (pin_it.next()) |p_ptr| { const p = p_ptr.*; - if (p.page == page and p.y > pn.y and p.y < pn.y + limit) p.y -= 1; + if (p.page == page and + p.y >= pn.y and + p.y <= pn.y + limit) p.y -= 1; } return; @@ -2119,14 +2125,18 @@ pub fn eraseRowBounded( var pin_it = self.tracked_pins.keyIterator(); while (pin_it.next()) |p_ptr| { const p = p_ptr.*; - if (p.page == page and p.y > pn.y) p.y -= 1; + if (p.page == page and p.y >= pn.y) p.y -= 1; } } while (page.next) |next| { const next_rows = next.data.rows.ptr(next.data.memory.ptr); - try page.data.cloneRowFrom(&next.data, &rows[page.data.size.rows - 1], &next_rows[0]); + try page.data.cloneRowFrom( + &next.data, + &rows[page.data.size.rows - 1], + &next_rows[0], + ); page = next; rows = next_rows; @@ -4439,6 +4449,133 @@ test "PageList erase a one-row active" { } } +test "PageList eraseRowBounded less than full row" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 80, 10, null); + defer s.deinit(); + + // Pins + const p_top = try s.trackPin(s.pin(.{ .active = .{ .y = 5, .x = 0 } }).?); + defer s.untrackPin(p_top); + const p_bot = try s.trackPin(s.pin(.{ .active = .{ .y = 8, .x = 0 } }).?); + defer s.untrackPin(p_bot); + const p_out = try s.trackPin(s.pin(.{ .active = .{ .y = 9, .x = 0 } }).?); + defer s.untrackPin(p_out); + + // Erase only a few rows in our active + try s.eraseRowBounded(.{ .active = .{ .y = 5 } }, 3); + try testing.expectEqual(s.rows, s.totalRows()); + + try testing.expectEqual(s.pages.first.?, p_top.page); + try testing.expectEqual(@as(usize, 4), p_top.y); + try testing.expectEqual(@as(usize, 0), p_top.x); + + try testing.expectEqual(s.pages.first.?, p_bot.page); + try testing.expectEqual(@as(usize, 7), p_bot.y); + try testing.expectEqual(@as(usize, 0), p_bot.x); + + try testing.expectEqual(s.pages.first.?, p_out.page); + try testing.expectEqual(@as(usize, 9), p_out.y); + try testing.expectEqual(@as(usize, 0), p_out.x); +} + +test "PageList eraseRowBounded full rows single page" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 80, 10, null); + defer s.deinit(); + + // Pins + const p_in = try s.trackPin(s.pin(.{ .active = .{ .y = 7, .x = 0 } }).?); + defer s.untrackPin(p_in); + const p_out = try s.trackPin(s.pin(.{ .active = .{ .y = 9, .x = 0 } }).?); + defer s.untrackPin(p_out); + + // Erase only a few rows in our active + try s.eraseRowBounded(.{ .active = .{ .y = 5 } }, 10); + try testing.expectEqual(s.rows, s.totalRows()); + + // Our pin should move to the first page + try testing.expectEqual(s.pages.first.?, p_in.page); + try testing.expectEqual(@as(usize, 6), p_in.y); + try testing.expectEqual(@as(usize, 0), p_in.x); + + try testing.expectEqual(s.pages.first.?, p_out.page); + try testing.expectEqual(@as(usize, 8), p_out.y); + try testing.expectEqual(@as(usize, 0), p_out.x); +} + +test "PageList eraseRowBounded full rows two pages" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 80, 10, null); + defer s.deinit(); + + // Grow to two pages so our active area straddles + { + const page = &s.pages.last.?.data; + for (0..page.capacity.rows - page.size.rows) |_| _ = try s.grow(); + try s.growRows(5); + try testing.expectEqual(@as(usize, 2), s.totalPages()); + try testing.expectEqual(@as(usize, 5), s.pages.last.?.data.size.rows); + } + + // Pins + const p_first = try s.trackPin(s.pin(.{ .active = .{ .y = 4, .x = 0 } }).?); + defer s.untrackPin(p_first); + const p_first_out = try s.trackPin(s.pin(.{ .active = .{ .y = 3, .x = 0 } }).?); + defer s.untrackPin(p_first_out); + const p_in = try s.trackPin(s.pin(.{ .active = .{ .y = 8, .x = 0 } }).?); + defer s.untrackPin(p_in); + const p_out = try s.trackPin(s.pin(.{ .active = .{ .y = 9, .x = 0 } }).?); + defer s.untrackPin(p_out); + + { + try testing.expectEqual(s.pages.last.?.prev.?, p_first.page); + try testing.expectEqual(@as(usize, p_first.page.data.size.rows - 1), p_first.y); + try testing.expectEqual(@as(usize, 0), p_first.x); + + try testing.expectEqual(s.pages.last.?.prev.?, p_first_out.page); + try testing.expectEqual(@as(usize, p_first_out.page.data.size.rows - 2), p_first_out.y); + try testing.expectEqual(@as(usize, 0), p_first_out.x); + + try testing.expectEqual(s.pages.last.?, p_in.page); + try testing.expectEqual(@as(usize, 3), p_in.y); + try testing.expectEqual(@as(usize, 0), p_in.x); + + try testing.expectEqual(s.pages.last.?, p_out.page); + try testing.expectEqual(@as(usize, 4), p_out.y); + try testing.expectEqual(@as(usize, 0), p_out.x); + } + + // Erase only a few rows in our active + try s.eraseRowBounded(.{ .active = .{ .y = 4 } }, 4); + + // In page in first page is shifted + try testing.expectEqual(s.pages.last.?.prev.?, p_first.page); + try testing.expectEqual(@as(usize, p_first.page.data.size.rows - 2), p_first.y); + try testing.expectEqual(@as(usize, 0), p_first.x); + + // Out page in first page should not be shifted + try testing.expectEqual(s.pages.last.?.prev.?, p_first_out.page); + try testing.expectEqual(@as(usize, p_first_out.page.data.size.rows - 2), p_first_out.y); + try testing.expectEqual(@as(usize, 0), p_first_out.x); + + // In page is shifted + try testing.expectEqual(s.pages.last.?, p_in.page); + try testing.expectEqual(@as(usize, 2), p_in.y); + try testing.expectEqual(@as(usize, 0), p_in.x); + + // Out page is not shifted + try testing.expectEqual(s.pages.last.?, p_out.page); + try testing.expectEqual(@as(usize, 4), p_out.y); + try testing.expectEqual(@as(usize, 0), p_out.x); +} + test "PageList clone" { const testing = std.testing; const alloc = testing.allocator; diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index e49a5fcbe..a94a24c7a 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -470,12 +470,6 @@ pub fn cursorAbsolute(self: *Screen, x: size.CellCountInt, y: size.CellCountInt) assert(y < self.pages.rows); defer self.assertIntegrity(); - const pt: point.Point = self.pages.pointFromPin( - .active, - self.cursor.page_pin.*, - ) orelse unreachable; - std.log.warn("pt={} cur_y={} y={}", .{ pt, self.cursor.y, y }); - var page_pin = if (y < self.cursor.y) self.cursor.page_pin.up(self.cursor.y - y).? else if (y > self.cursor.y) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 52267fa13..2c17c051a 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -1095,19 +1095,25 @@ pub fn index(self: *Terminal) !void { // Otherwise use a fast path function from PageList to efficiently // scroll the contents of the scrolling region. - // eraseRow and eraseRowBounded will end up moving the cursor pin - // up by 1, so we save its current position and restore it after. - const cursor_x = self.screen.cursor.x; - const cursor_y = self.screen.cursor.y; - defer { - self.screen.cursorAbsolute(cursor_x, cursor_y); - } + // 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 + 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.