From d57d1d2395c9f80ecdb7be4cd58858b299f95c42 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 2 Dec 2024 09:39:21 -0500 Subject: [PATCH 1/3] terminal: failing tracked pin test on fullReset --- src/terminal/PageList.zig | 6 +++++- src/terminal/Terminal.zig | 10 ++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 70f972ebe..3d67278ba 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -2356,7 +2356,11 @@ pub fn countTrackedPins(self: *const PageList) usize { /// Checks if a pin is valid for this pagelist. This is a very slow and /// expensive operation since we traverse the entire linked list in the /// worst case. Only for runtime safety/debug. -fn pinIsValid(self: *const PageList, p: Pin) bool { +pub fn pinIsValid(self: *const PageList, p: Pin) bool { + // This is very slow so we want to ensure we only ever + // call this during slow runtime safety builds. + comptime assert(build_config.slow_runtime_safety); + var it = self.pages.first; while (it) |node| : (it = node.next) { if (node != p.node) continue; diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index a2cf510b1..0a2914f10 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -10575,6 +10575,16 @@ test "Terminal: fullReset default modes" { try testing.expect(t.modes.get(.grapheme_cluster)); } +test "Terminal: fullReset tracked pins" { + var t = try init(testing.allocator, .{ .cols = 80, .rows = 80 }); + defer t.deinit(testing.allocator); + + // Create a tracked pin + const p = try t.screen.pages.trackPin(t.screen.cursor.page_pin.*); + t.fullReset(); + try testing.expect(t.screen.pages.pinIsValid(p.*)); +} + // https://github.com/mitchellh/ghostty/issues/272 // This is also tested in depth in screen resize tests but I want to keep // this test around to ensure we don't regress at multiple layers. From d7fcaefdf3622a3aefadbf788c062065576ff928 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 2 Dec 2024 17:25:17 -0500 Subject: [PATCH 2/3] terminal: PageList.reset --- src/terminal/PageList.zig | 134 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 3d67278ba..aec0af278 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -330,6 +330,77 @@ pub fn deinit(self: *PageList) void { } } +/// Reset the PageList back to an empty state. This is similar to +/// deinit and reinit but it importantly preserves the pointer +/// stability of tracked pins (they're moved to the top-left since +/// all contents are cleared). +/// +/// This can't fail because we always retain at least enough allocated +/// memory to fit the active area. +pub fn reset(self: *PageList) void { + // We need enough pages/nodes to keep our active area. This should + // never fail since we by definition have allocated a page already + // that fits our size but I'm not confident to make that assertion. + const cap = std_capacity.adjust( + .{ .cols = self.cols }, + ) catch @panic("reset: std_capacity.adjust failed"); + assert(cap.rows > 0); // adjust should never return 0 rows + + // The number of pages we need is the number of rows in the active + // area divided by the row capacity of a page. + const page_count = std.math.divCeil( + usize, + self.rows, + cap.rows, + ) catch unreachable; + + // Before resetting our pools we need to free any pages that + // are non-standard size since those were allocated outside + // the pool. + { + const page_alloc = self.pool.pages.arena.child_allocator; + var it = self.pages.first; + while (it) |node| : (it = node.next) { + if (node.data.memory.len > std_size) { + page_alloc.free(node.data.memory); + } + } + } + + // Reset our pools to free as much memory as possible while retaining + // the capacity for at least the minimum number of pages we need. + // The return value is whether memory was reclaimed or not, but in + // either case the pool is left in a valid state. + _ = self.pool.pages.reset(.{ + .retain_with_limit = page_count * PagePool.item_size, + }); + _ = self.pool.nodes.reset(.{ + .retain_with_limit = page_count * NodePool.item_size, + }); + + // Initialize our pages. This should not be able to fail since + // we retained the capacity for the minimum number of pages we need. + self.pages, self.page_size = initPages( + &self.pool, + self.cols, + self.rows, + ) catch @panic("initPages failed"); + + // Update all our tracked pins to point to our first page top-left + { + var it = self.tracked_pins.iterator(); + while (it.next()) |entry| { + const p: *Pin = entry.key_ptr.*; + p.node = self.pages.first.?; + p.x = 0; + p.y = 0; + } + } + + // Move our viewport back to the active area since everything is gone. + self.viewport = .active; +} + pub const Clone = struct { /// The top and bottom (inclusive) points of the region to clone. /// The x coordinate is ignored; the full row is always cloned. @@ -8195,3 +8266,66 @@ test "PageList resize reflow wrap moves kitty placeholder" { } try testing.expect(it.next() == null); } + +test "PageList reset" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 80, 24, null); + defer s.deinit(); + s.reset(); + try testing.expect(s.viewport == .active); + try testing.expect(s.pages.first != null); + try testing.expectEqual(@as(usize, s.rows), s.totalRows()); + + // Active area should be the top + try testing.expectEqual(Pin{ + .node = s.pages.first.?, + .y = 0, + .x = 0, + }, s.getTopLeft(.active)); +} + +test "PageList reset across two pages" { + const testing = std.testing; + const alloc = testing.allocator; + + // Find a cap that makes it so that rows don't fit on one page. + const rows = 100; + const cap = cap: { + var cap = try std_capacity.adjust(.{ .cols = 50 }); + while (cap.rows >= rows) cap = try std_capacity.adjust(.{ + .cols = cap.cols + 50, + }); + + break :cap cap; + }; + + // Init + var s = try init(alloc, cap.cols, rows, null); + defer s.deinit(); + s.reset(); + try testing.expect(s.viewport == .active); + try testing.expect(s.pages.first != null); + try testing.expectEqual(@as(usize, s.rows), s.totalRows()); +} + +test "PageList clears history" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 80, 24, null); + defer s.deinit(); + try s.growRows(30); + s.reset(); + try testing.expect(s.viewport == .active); + try testing.expect(s.pages.first != null); + try testing.expectEqual(@as(usize, s.rows), s.totalRows()); + + // Active area should be the top + try testing.expectEqual(Pin{ + .node = s.pages.first.?, + .y = 0, + .x = 0, + }, s.getTopLeft(.active)); +} From 212bd3d5fb5ced7866f459fdc80f757811f14b3e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 2 Dec 2024 17:43:56 -0500 Subject: [PATCH 3/3] terminal: fullReset uses the new screen reset methods --- src/terminal/Screen.zig | 48 +++++++++++++++++++++- src/terminal/Terminal.zig | 84 ++++++++++----------------------------- 2 files changed, 66 insertions(+), 66 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 4f3fe270e..d8787487f 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -83,8 +83,8 @@ pub const Dirty = packed struct { /// The cursor position and style. pub const Cursor = struct { // The x/y position within the viewport. - x: size.CellCountInt, - y: size.CellCountInt, + x: size.CellCountInt = 0, + y: size.CellCountInt = 0, /// The visual style of the cursor. This defaults to block because /// it has to default to something, but users of this struct are @@ -249,6 +249,50 @@ pub fn assertIntegrity(self: *const Screen) void { } } +/// Reset the screen according to the logic of a DEC RIS sequence. +/// +/// - Clears the screen and attempts to reclaim memory. +/// - Moves the cursor to the top-left. +/// - Clears any cursor state: style, hyperlink, etc. +/// - Resets the charset +/// - Clears the selection +/// - Deletes all Kitty graphics +/// - Resets Kitty Keyboard settings +/// - Disables protection mode +/// +pub fn reset(self: *Screen) void { + // Reset our pages + self.pages.reset(); + + // The above reset preserves tracked pins so we can still use + // our cursor pin, which should be at the top-left already. + const cursor_pin: *PageList.Pin = self.cursor.page_pin; + assert(cursor_pin.node == self.pages.pages.first.?); + assert(cursor_pin.x == 0); + assert(cursor_pin.y == 0); + const cursor_rac = cursor_pin.rowAndCell(); + self.cursor.deinit(self.alloc); + self.cursor = .{ + .page_pin = cursor_pin, + .page_row = cursor_rac.row, + .page_cell = cursor_rac.cell, + }; + + // Clear kitty graphics + self.kitty_images.delete( + self.alloc, + undefined, // All image deletion doesn't need the terminal + .{ .all = true }, + ); + + // Reset our basic state + self.saved_cursor = null; + self.charset = .{}; + self.kitty_keyboard = .{}; + self.protected_mode = .off; + self.clearSelection(); +} + /// Clone the screen. /// /// This will copy: diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 0a2914f10..a11028304 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -2627,82 +2627,38 @@ pub fn plainStringUnwrapped(self: *Terminal, alloc: Allocator) ![]const u8 { /// Full reset. /// -/// This will attempt to free the existing screen memory and allocate -/// new screens but if that fails this will reuse the existing memory -/// from the prior screens. In the latter case, memory may be wasted -/// (since its unused) but it isn't leaked. +/// This will attempt to free the existing screen memory but if that fails +/// this will reuse the existing memory. In the latter case, memory may +/// be wasted (since its unused) but it isn't leaked. pub fn fullReset(self: *Terminal) void { - // Attempt to initialize new screens. - var new_primary = Screen.init( - self.screen.alloc, - self.cols, - self.rows, - self.screen.pages.explicit_max_size, - ) catch |err| { - log.warn("failed to allocate new primary screen, reusing old memory err={}", .{err}); - self.fallbackReset(); - return; - }; - const new_secondary = Screen.init( - self.secondary_screen.alloc, - self.cols, - self.rows, - 0, - ) catch |err| { - log.warn("failed to allocate new secondary screen, reusing old memory err={}", .{err}); - new_primary.deinit(); - self.fallbackReset(); - return; - }; + // Reset our screens + self.screen.reset(); + self.secondary_screen.reset(); - // If we got here, both new screens were successfully allocated - // and we can deinitialize the old screens. - self.screen.deinit(); - self.secondary_screen.deinit(); + // Ensure we're back on primary screen + if (self.active_screen != .primary) { + const old = self.screen; + self.screen = self.secondary_screen; + self.secondary_screen = old; + self.active_screen = .primary; + } - // Replace with the newly allocated screens. - self.screen = new_primary; - self.secondary_screen = new_secondary; - - self.resetCommonState(); -} - -fn fallbackReset(self: *Terminal) void { - // Clear existing screens without reallocation - self.primaryScreen(.{ .clear_on_exit = true, .cursor_save = false }); - self.screen.clearSelection(); - self.eraseDisplay(.scrollback, false); - self.eraseDisplay(.complete, false); - self.screen.cursorAbsolute(0, 0); - self.resetCommonState(); -} - -fn resetCommonState(self: *Terminal) void { - // We set the saved cursor to null and then restore. This will force - // our cursor to go back to the default which will also move the cursor - // to the top-left. - self.screen.saved_cursor = null; - self.restoreCursor() catch |err| { - log.warn("restore cursor on primary screen failed err={}", .{err}); - }; - - self.screen.endHyperlink(); - self.screen.charset = .{}; + // Rest our basic state self.modes.reset(); self.flags = .{}; self.tabstops.reset(TABSTOP_INTERVAL); - self.screen.kitty_keyboard = .{}; - self.secondary_screen.kitty_keyboard = .{}; - self.screen.protected_mode = .off; + self.previous_char = null; + self.pwd.clearRetainingCapacity(); + self.status_display = .main; self.scrolling_region = .{ .top = 0, .bottom = self.rows - 1, .left = 0, .right = self.cols - 1, }; - self.previous_char = null; - self.pwd.clearRetainingCapacity(); - self.status_display = .main; + + // Always mark dirty so we redraw everything + self.flags.dirty.clear = true; } /// Returns true if the point is dirty, used for testing.