mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-07-12 12:19:47 +00:00
terminal: renew generations for direct row shifts
Screen scroll fast paths and Terminal line insertion/deletion move Row values directly instead of using PageList erasure helpers. Their node generations therefore stayed valid after cached row coordinates changed. Expose the PageList layout invalidator to sibling terminal modules and renew each affected existing page once before full-row rotations or swaps. Keep partial-width cell moves on the content-only path and cover same-page, cross-page, and fresh-tail behavior.
This commit is contained in:
@@ -3116,6 +3116,8 @@ pub fn scrollClear(self: *PageList) Allocator.Error!void {
|
||||
/// the sole remaining page, or shortening the source page of a split. Without
|
||||
/// a new generation, a cached pointer, serial, and coordinate could still pass
|
||||
/// `nodeIsValid` while referring to a different row than it originally did.
|
||||
/// Screen and Terminal fast paths which manipulate Page rows directly must use
|
||||
/// this because they bypass PageList's own row-mutation helpers.
|
||||
///
|
||||
/// The caller must pass a node which is currently live in this PageList. Call
|
||||
/// this at the point the operation commits to changing the layout and before
|
||||
@@ -3142,7 +3144,7 @@ pub fn scrollClear(self: *PageList) Allocator.Error!void {
|
||||
/// adjust row or memory accounting, mark cells dirty, or notify incremental
|
||||
/// compression. The surrounding operation remains responsible for all such
|
||||
/// bookkeeping required by its layout change.
|
||||
fn invalidateNodeLayout(self: *PageList, node: *List.Node) void {
|
||||
pub fn invalidateNodeLayout(self: *PageList, node: *List.Node) void {
|
||||
node.serial = self.page_serial;
|
||||
self.page_serial += 1;
|
||||
}
|
||||
|
||||
@@ -960,8 +960,8 @@ pub fn cursorScrollAbove(self: *Screen) !void {
|
||||
// lot cheaper in 99% of cases.
|
||||
|
||||
const old_pin = self.cursor.page_pin.*;
|
||||
if (try self.pages.grow()) |_| {
|
||||
try self.cursorScrollAboveRotate();
|
||||
if (try self.pages.grow()) |new_node| {
|
||||
try self.cursorScrollAboveRotate(new_node);
|
||||
} else {
|
||||
// In this case, it means grow() didn't allocate a new page.
|
||||
|
||||
@@ -981,6 +981,8 @@ pub fn cursorScrollAbove(self: *Screen) !void {
|
||||
// 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);
|
||||
// Rotating this suffix changes which logical row its coordinates identify.
|
||||
self.pages.invalidateNodeLayout(pin.node);
|
||||
fastmem.rotateOnceR(Row, rows[pin.y..page.size.rows]);
|
||||
|
||||
// Mark the whole page as dirty.
|
||||
@@ -1014,7 +1016,7 @@ pub fn cursorScrollAbove(self: *Screen) !void {
|
||||
// 1 |5E00000000| | 4
|
||||
// +----------+ :
|
||||
// +-------------+
|
||||
try self.cursorScrollAboveRotate();
|
||||
try self.cursorScrollAboveRotate(null);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1029,7 +1031,10 @@ pub fn cursorScrollAbove(self: *Screen) !void {
|
||||
}
|
||||
}
|
||||
|
||||
fn cursorScrollAboveRotate(self: *Screen) !void {
|
||||
fn cursorScrollAboveRotate(
|
||||
self: *Screen,
|
||||
fresh_node: ?*PageList.List.Node,
|
||||
) !void {
|
||||
self.cursorChangePin(self.cursor.page_pin.down(1).?);
|
||||
|
||||
// Go through each of the pages following our pin, shift all rows
|
||||
@@ -1042,6 +1047,12 @@ fn cursorScrollAboveRotate(self: *Screen) !void {
|
||||
const prev_rows = prev_page.rows.ptr(prev_page.memory.ptr);
|
||||
const cur_rows = cur_page.rows.ptr(cur_page.memory.ptr);
|
||||
|
||||
// A newly allocated tail has no earlier references to invalidate.
|
||||
if (fresh_node == null or current != fresh_node.?) {
|
||||
// Rotating this page moves every cached row coordinate down by one.
|
||||
self.pages.invalidateNodeLayout(current);
|
||||
}
|
||||
|
||||
// Rotate the pages down: [ 0 1 2 3 ] => [ 3 0 1 2 ]
|
||||
fastmem.rotateOnceR(Row, cur_rows[0..cur_page.size.rows]);
|
||||
|
||||
@@ -1061,6 +1072,8 @@ fn cursorScrollAboveRotate(self: *Screen) !void {
|
||||
assert(current == self.cursor.page_pin.node);
|
||||
const cur_page = current.page();
|
||||
const cur_rows = cur_page.rows.ptr(cur_page.memory.ptr);
|
||||
// Rotating the cursor-page suffix changes its cached row coordinates.
|
||||
self.pages.invalidateNodeLayout(current);
|
||||
fastmem.rotateOnceR(Row, cur_rows[self.cursor.page_pin.y..cur_page.size.rows]);
|
||||
self.clearCells(
|
||||
cur_page,
|
||||
@@ -1145,6 +1158,8 @@ pub fn cursorScrollRegionUp(self: *Screen, limit: usize) !void {
|
||||
|
||||
// Rotate the region rows so the now-blank top row moves to the
|
||||
// bottom (the cursor row) and everything else shifts up by one.
|
||||
// Rotating the region changes which logical row its coordinates identify.
|
||||
self.pages.invalidateNodeLayout(pin.node);
|
||||
fastmem.rotateOnce(Row, rows);
|
||||
|
||||
// Mark the whole page as dirty.
|
||||
@@ -4919,6 +4934,22 @@ test "Screen: cursorScrollRegionUp simple" {
|
||||
}
|
||||
}
|
||||
|
||||
test "Screen: cursorScrollRegionUp renews page generation" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
var s = try init(alloc, .{ .cols = 5, .rows = 5, .max_scrollback = 0 });
|
||||
defer s.deinit();
|
||||
try s.testWriteString("1ABCD\n2EFGH\n3IJKL\n4MNOP\n5QRST");
|
||||
s.cursorAbsolute(0, 2);
|
||||
|
||||
const node = s.cursor.page_pin.node;
|
||||
const serial = node.serial;
|
||||
try s.cursorScrollRegionUp(2);
|
||||
|
||||
try testing.expect(!s.pages.nodeIsValid(node, serial));
|
||||
}
|
||||
|
||||
test "Screen: cursorScrollRegionUp moves selection" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
@@ -5281,7 +5312,10 @@ test "Screen: scroll above same page" {
|
||||
// +----------+ :
|
||||
// +-------------+
|
||||
|
||||
const node = s.cursor.page_pin.node;
|
||||
const serial = node.serial;
|
||||
try s.cursorScrollAbove();
|
||||
try testing.expect(!s.pages.nodeIsValid(node, serial));
|
||||
|
||||
// +----------+ = PAGE 0
|
||||
// 0 |1ABCD00000|
|
||||
@@ -5356,7 +5390,13 @@ test "Screen: scroll above same page but cursor on previous page" {
|
||||
// +----------+ :
|
||||
// +-------------+
|
||||
|
||||
const first_node = s.pages.pages.first.?;
|
||||
const second_node = first_node.next.?;
|
||||
const first_serial = first_node.serial;
|
||||
const second_serial = second_node.serial;
|
||||
try s.cursorScrollAbove();
|
||||
try testing.expect(!s.pages.nodeIsValid(first_node, first_serial));
|
||||
try testing.expect(!s.pages.nodeIsValid(second_node, second_serial));
|
||||
|
||||
// +----------+ = PAGE 0
|
||||
// ... : :
|
||||
@@ -5521,7 +5561,10 @@ test "Screen: scroll above creates new page" {
|
||||
// 4307 |3IJKL00000| | 2
|
||||
// +----------+ :
|
||||
// +-------------+
|
||||
const node = s.pages.pages.first.?;
|
||||
const serial = node.serial;
|
||||
try s.cursorScrollAbove();
|
||||
try testing.expect(!s.pages.nodeIsValid(node, serial));
|
||||
|
||||
// +----------+ = PAGE 0
|
||||
// ... : :
|
||||
|
||||
@@ -2429,6 +2429,21 @@ fn rowWillBeShifted(
|
||||
}
|
||||
}
|
||||
|
||||
/// Renew every live page generation in an inclusive range before a full-width
|
||||
/// line operation moves logical rows between their coordinates.
|
||||
fn invalidateFullWidthRowRange(
|
||||
self: *Terminal,
|
||||
first: *PageList.List.Node,
|
||||
last: *PageList.List.Node,
|
||||
) void {
|
||||
var node = first;
|
||||
while (true) : (node = node.next.?) {
|
||||
// Full-width line movement remaps cached row coordinates in this page.
|
||||
self.screens.active.pages.invalidateNodeLayout(node);
|
||||
if (node == last) break;
|
||||
}
|
||||
}
|
||||
|
||||
// TODO(qwerasd): `insertLines` and `deleteLines` are 99% identical,
|
||||
// the majority of their logic can (and should) be abstracted in to
|
||||
// a single shared helper function, probably on `Screen` not here.
|
||||
@@ -2504,6 +2519,12 @@ pub fn insertLines(self: *Terminal, count: usize) void {
|
||||
};
|
||||
defer self.screens.active.pages.untrackPin(cur_p);
|
||||
|
||||
// Partial-width margins edit cells in stable rows; full-width moves rows.
|
||||
if (!left_right) self.invalidateFullWidthRowRange(
|
||||
self.screens.active.cursor.page_pin.node,
|
||||
cur_p.node,
|
||||
);
|
||||
|
||||
// Our current y position relative to the cursor
|
||||
var y: usize = rem;
|
||||
|
||||
@@ -2699,6 +2720,12 @@ pub fn deleteLines(self: *Terminal, count: usize) void {
|
||||
};
|
||||
defer self.screens.active.pages.untrackPin(cur_p);
|
||||
|
||||
// Partial-width margins edit cells in stable rows; full-width moves rows.
|
||||
if (!left_right) self.invalidateFullWidthRowRange(
|
||||
cur_p.node,
|
||||
cur_p.down(rem - 1).?.node,
|
||||
);
|
||||
|
||||
// Our current y position relative to the cursor
|
||||
var y: usize = 0;
|
||||
|
||||
@@ -6867,8 +6894,11 @@ test "Terminal: insertLines simple" {
|
||||
try t.printString("GHI");
|
||||
t.setCursorPos(2, 2);
|
||||
|
||||
const node = t.screens.active.cursor.page_pin.node;
|
||||
const serial = node.serial;
|
||||
t.clearDirty();
|
||||
t.insertLines(1);
|
||||
try testing.expect(!t.screens.active.pages.nodeIsValid(node, serial));
|
||||
|
||||
try testing.expect(!t.isDirty(.{ .active = .{ .x = 0, .y = 0 } }));
|
||||
try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 1 } }));
|
||||
@@ -7046,11 +7076,15 @@ test "Terminal: insertLines across page boundary marks all shifted rows dirty" {
|
||||
try t.printString("5EEEE");
|
||||
|
||||
// Verify we now have a second page
|
||||
try testing.expect(first_page.next != null);
|
||||
const second_page = first_page.next.?;
|
||||
const first_serial = first_page.serial;
|
||||
const second_serial = second_page.serial;
|
||||
|
||||
t.setCursorPos(1, 1);
|
||||
t.clearDirty();
|
||||
t.insertLines(1);
|
||||
try testing.expect(!t.screens.active.pages.nodeIsValid(first_page, first_serial));
|
||||
try testing.expect(!t.screens.active.pages.nodeIsValid(second_page, second_serial));
|
||||
|
||||
try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 0 } }));
|
||||
try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 1 } }));
|
||||
@@ -9754,8 +9788,11 @@ test "Terminal: deleteLines simple" {
|
||||
try t.printString("GHI");
|
||||
t.setCursorPos(2, 2);
|
||||
|
||||
const node = t.screens.active.cursor.page_pin.node;
|
||||
const serial = node.serial;
|
||||
t.clearDirty();
|
||||
t.deleteLines(1);
|
||||
try testing.expect(!t.screens.active.pages.nodeIsValid(node, serial));
|
||||
|
||||
try testing.expect(!t.isDirty(.{ .active = .{ .x = 0, .y = 0 } }));
|
||||
try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 1 } }));
|
||||
@@ -9837,11 +9874,15 @@ test "Terminal: deleteLines across page boundary marks all shifted rows dirty" {
|
||||
try t.printString("5EEEE");
|
||||
|
||||
// Verify we now have a second page
|
||||
try testing.expect(first_page.next != null);
|
||||
const second_page = first_page.next.?;
|
||||
const first_serial = first_page.serial;
|
||||
const second_serial = second_page.serial;
|
||||
|
||||
t.setCursorPos(1, 1);
|
||||
t.clearDirty();
|
||||
t.deleteLines(1);
|
||||
try testing.expect(!t.screens.active.pages.nodeIsValid(first_page, first_serial));
|
||||
try testing.expect(!t.screens.active.pages.nodeIsValid(second_page, second_serial));
|
||||
|
||||
try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 0 } }));
|
||||
try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 1 } }));
|
||||
|
||||
Reference in New Issue
Block a user