From c2b0bb6395e79ab6055a0a11987d272dfb9bf826 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Fri, 7 Jun 2024 14:55:02 -0700
Subject: [PATCH] terminal: mark old/new rows as dirty when moving the cursor
 absolute

---
 src/terminal/Screen.zig   |  9 +++++++++
 src/terminal/Terminal.zig | 16 +++++++++++-----
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig
index 6490376b4..94053d21d 100644
--- a/src/terminal/Screen.zig
+++ b/src/terminal/Screen.zig
@@ -481,6 +481,10 @@ pub fn cursorAbsolute(self: *Screen, x: size.CellCountInt, y: size.CellCountInt)
     assert(y < self.pages.rows);
     defer self.assertIntegrity();
 
+    // Moving the cursor affects text run splitting (ligatures) so
+    // we must mark the old and new page dirty.
+    self.cursor.page_pin.markDirty();
+
     var page_pin = if (y < self.cursor.y)
         self.cursor.page_pin.up(self.cursor.y - y).?
     else if (y > self.cursor.y)
@@ -494,6 +498,11 @@ pub fn cursorAbsolute(self: *Screen, x: size.CellCountInt, y: size.CellCountInt)
     const page_rac = page_pin.rowAndCell();
     self.cursor.page_row = page_rac.row;
     self.cursor.page_cell = page_rac.cell;
+
+    // Mark the new page dirty. This might be the same page as the old
+    // but this is a fairly cheap operation and cursor movement isn't
+    // super common.
+    self.cursor.page_pin.markDirty();
 }
 
 /// Reloads the cursor pointer information into the screen. This is expensive
diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig
index 8dd08a8ba..6ef7f1ddb 100644
--- a/src/terminal/Terminal.zig
+++ b/src/terminal/Terminal.zig
@@ -4242,7 +4242,8 @@ test "Terminal: setTopAndBottomMargin top only" {
     t.clearDirty();
     t.scrollDown(1);
 
-    try testing.expect(!t.isDirty(.{ .active = .{ .x = 0, .y = 0 } }));
+    // This is dirty because the cursor moves from this row
+    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 } }));
@@ -4907,7 +4908,8 @@ test "Terminal: scrollUp top/bottom scroll region" {
     t.clearDirty();
     t.scrollUp(1);
 
-    try testing.expect(!t.isDirty(.{ .active = .{ .x = 0, .y = 0 } }));
+    // This is dirty because the cursor moves from this row
+    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 } }));
 
@@ -4985,7 +4987,8 @@ test "Terminal: scrollUp full top/bottom region" {
     t.clearDirty();
     t.scrollUp(4);
 
-    try testing.expect(!t.isDirty(.{ .active = .{ .x = 0, .y = 0 } }));
+    // This is dirty because the cursor moves from this row
+    try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 0 } }));
     try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 1 } }));
 
     {
@@ -5010,7 +5013,8 @@ test "Terminal: scrollUp full top/bottomleft/right scroll region" {
     t.clearDirty();
     t.scrollUp(4);
 
-    try testing.expect(!t.isDirty(.{ .active = .{ .x = 0, .y = 0 } }));
+    // This is dirty because the cursor moves from this row
+    try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 0 } }));
     for (1..5) |y| try testing.expect(t.isDirty(.{ .active = .{
         .x = 0,
         .y = @intCast(y),
@@ -5077,7 +5081,9 @@ test "Terminal: scrollDown outside of scroll region" {
     try testing.expectEqual(cursor.y, t.screen.cursor.y);
 
     try testing.expect(!t.isDirty(.{ .active = .{ .x = 0, .y = 0 } }));
-    try testing.expect(!t.isDirty(.{ .active = .{ .x = 0, .y = 1 } }));
+
+    // This is dirty because the cursor moves from this row
+    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 } }));