From c412b30cb5ffc1a208d4bb8b39c967cb00a5e6b4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 19 Jan 2026 10:08:51 -0800 Subject: [PATCH] terminal: splitForCapacity, manualStyleUpdate uses this --- src/terminal/PageList.zig | 2 +- src/terminal/Screen.zig | 189 ++++++++++++++++++++++++++++++++------ 2 files changed, 163 insertions(+), 28 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index c639f15cd..41f8d6533 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -2776,7 +2776,7 @@ pub fn split( // Ran into a bug that I can only explain via aliasing. If a tracked // pin is passed in, its possible Zig will alias the memory and then - // when we modify it later it updates our p here. Coyping the node + // when we modify it later it updates our p here. Copying the node // fixes this. const original_node = p.node; const page: *Page = &original_node.data; diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index d09477eda..5ea338b70 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -1791,7 +1791,10 @@ fn resizeInternal( /// /// If the style can't be set due to any internal errors (memory-related), /// then this will revert back to the existing style and return an error. -pub fn setAttribute(self: *Screen, attr: sgr.Attribute) !void { +pub fn setAttribute( + self: *Screen, + attr: sgr.Attribute, +) PageList.IncreaseCapacityError!void { // If we fail to set our style for any reason, we should revert // back to the old style. If we fail to do that, we revert back to // the default style. @@ -1995,13 +1998,22 @@ pub fn manualStyleUpdate(self: *Screen) PageList.IncreaseCapacityError!void { ) catch |err| id: { // Our style map is full or needs to be rehashed, so we need to // increase style capacity (or rehash). - const node = try self.increaseCapacity( + const node = self.increaseCapacity( self.cursor.page_pin.node, switch (err) { error.OutOfMemory => .styles, error.NeedsRehash => null, }, - ); + ) catch |increase_err| switch (increase_err) { + error.OutOfMemory => return error.OutOfMemory, + error.OutOfSpace => space: { + // Out of space, we need to split the page. Split wherever + // is using less capacity and hope that works. If it doesn't + // work, we tried. + try self.splitForCapacity(self.cursor.page_pin.*); + break :space self.cursor.page_pin.node; + }, + }; page = &node.data; break :id page.styles.add( @@ -2031,6 +2043,62 @@ pub fn manualStyleUpdate(self: *Screen) PageList.IncreaseCapacityError!void { self.cursor.style_id = id; } +/// Split at the given pin so that the pinned row moves to the page +/// with less used capacity after the split. +/// +/// The primary use case for this is to handle IncreaseCapacityError +/// OutOfSpace conditions where we need to split the page in order +/// to make room for more managed memory. +/// +/// If the caller cares about where the pin moves to, they should +/// setup a tracked pin before calling this and then check that. +/// In many calling cases, the input pin is tracked (e.g. the cursor +/// pin). +/// +/// If this returns OOM then its a system OOM. If this returns OutOfSpace +/// then it means the page can't be split further. +fn splitForCapacity( + self: *Screen, + pin: Pin, +) PageList.SplitError!void { + // Get our capacities. We include our target row because its + // capacity will be preserved. + const bytes_above = Page.layout(pin.node.data.exactRowCapacity( + 0, + pin.y + 1, + )).total_size; + const bytes_below = Page.layout(pin.node.data.exactRowCapacity( + pin.y, + pin.node.data.size.rows, + )).total_size; + + // We need to track the old cursor pin because if our split + // moves the cursor pin we need to update our accounting. + const old_cursor = self.cursor.page_pin.*; + + // If our bytes above are less than bytes below, we move the pin + // to split down one since splitting includes the pinned row in + // the new node. + try self.pages.split(if (bytes_above < bytes_below) + pin.down(1) orelse pin + else + pin); + + // Cursor didn't change nodes, we're done. + if (self.cursor.page_pin.node == old_cursor.node) return; + + // Cursor changed, we need to restore the old pin then use + // cursorChangePin to move to the new pin. The old node is guaranteed + // to still exist, just not the row. + // + // Note that page_row and all that will be invalid, it points to the + // new node, but at the time of writing this we don't need any of that + // to be right in cursorChangePin. + const new_cursor = self.cursor.page_pin.*; + self.cursor.page_pin.* = old_cursor; + self.cursorChangePin(new_cursor); +} + /// Append a grapheme to the given cell within the current cursor row. pub fn appendGrapheme( self: *Screen, @@ -9264,48 +9332,115 @@ test "Screen: cursorDown to page with insufficient capacity" { } } -test "Screen setAttribute returns OutOfSpace at max styles" { +test "Screen setAttribute increases capacity when style map is full" { + // Tests that setAttribute succeeds when the style map is full by + // increasing page capacity. When capacity is at max and increaseCapacity + // returns OutOfSpace, manualStyleUpdate will split the page instead. const testing = std.testing; const alloc = testing.allocator; - var s = try init(alloc, .{ .cols = 10, .rows = 10, .max_scrollback = 0 }); + // Use a small screen with multiple rows + var s = try init(alloc, .{ .cols = 10, .rows = 5, .max_scrollback = 10 }); defer s.deinit(); + // Write content to multiple rows + try s.testWriteString("line1\nline2\nline3\nline4\nline5"); + + // Get the page and fill its style map to capacity + const page = &s.cursor.page_pin.node.data; + const original_styles_capacity = page.capacity.styles; + + // Fill the style map to capacity + { + page.pauseIntegrityChecks(true); + defer page.pauseIntegrityChecks(false); + defer page.assertIntegrity(); + + var n: u24 = 1; + while (page.styles.add( + page.memory, + .{ .bg_color = .{ .rgb = @bitCast(n) } }, + )) |_| n += 1 else |_| {} + } + + // Now try to set a new unique attribute that would require a new style slot + // This should succeed by increasing capacity (or splitting if at max capacity) + try s.setAttribute(.bold); + + // The style should have been applied (bold flag set) + try testing.expect(s.cursor.style.flags.bold); + + // The cursor should have a valid non-default style_id + try testing.expect(s.cursor.style_id != style.default_id); + + // Either the capacity increased or the page was split/changed + const current_page = &s.cursor.page_pin.node.data; + const capacity_increased = current_page.capacity.styles > original_styles_capacity; + const page_changed = current_page != page; + try testing.expect(capacity_increased or page_changed); +} + +test "Screen setAttribute splits page on OutOfSpace at max styles" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ + .cols = 10, + .rows = 10, + .max_scrollback = 0, + }); + defer s.deinit(); + + // Write content to multiple rows so we have something to split + try s.testWriteString("line1\nline2\nline3\nline4\nline5"); + + // Remember the original node + const original_node = s.cursor.page_pin.node; + // Increase the page's style capacity to max by repeatedly calling increaseCapacity + // Use Screen.increaseCapacity to properly maintain cursor state const max_styles = std.math.maxInt(size.CellCountInt); while (s.cursor.page_pin.node.data.capacity.styles < max_styles) { - _ = s.pages.increaseCapacity( + _ = s.increaseCapacity( s.cursor.page_pin.node, .styles, ) catch break; } - // Get the page reference after increaseCapacity - const page = &s.cursor.page_pin.node.data; + // Get the page reference after increaseCapacity - cursor may have moved + var page = &s.cursor.page_pin.node.data; try testing.expectEqual(max_styles, page.capacity.styles); - // Fill up all style slots - page.pauseIntegrityChecks(true); - var n: u24 = 1; - while (page.styles.add( - page.memory, - .{ .bg_color = .{ .rgb = @bitCast(n) } }, - )) |_| n += 1 else |_| {} - page.pauseIntegrityChecks(false); + // Fill the style map to capacity + { + page.pauseIntegrityChecks(true); + defer page.pauseIntegrityChecks(false); + defer page.assertIntegrity(); - // Set a style that we can track as "prior" - // Use a style that's already in the page by reusing one we added - s.cursor.style = .{ .bg_color = .{ .rgb = @bitCast(@as(u24, 1)) } }; - const prior_id = page.styles.add(page.memory, s.cursor.style) catch unreachable; - s.cursor.style_id = prior_id; + var n: u24 = 1; + while (page.styles.add( + page.memory, + .{ .bg_color = .{ .rgb = @bitCast(n) } }, + )) |_| n += 1 else |_| {} + } + + // Track the node before setAttribute + const node_before_set = s.cursor.page_pin.node; // Now try to set a new unique attribute that would require a new style slot - // This should fail with OutOfSpace since we're at max capacity - const result = s.setAttribute(.bold); + // At max capacity, increaseCapacity will return OutOfSpace, triggering page split + try s.setAttribute(.bold); - // Should return OutOfSpace error - try testing.expectError(error.OutOfSpace, result); + // The style should have been applied (bold flag set) + try testing.expect(s.cursor.style.flags.bold); - // The cursor style_id should remain the prior one - try testing.expectEqual(prior_id, s.cursor.style_id); + // The cursor should have a valid non-default style_id + try testing.expect(s.cursor.style_id != style.default_id); + + // The page should have been split + const page_was_split = s.cursor.page_pin.node != node_before_set or + node_before_set.next != null or + node_before_set.prev != null or + s.cursor.page_pin.node != original_node; + try testing.expect(page_was_split); }