From 402b9227de1efab24de981ce0d71b4d378417490 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 5 Aug 2026 08:37:45 -0700 Subject: [PATCH] terminal/kitty: reclaim pruned placements Reclaim pin-backed Kitty graphics placements after their tracked screen content is pruned. Treat garbage pins as non-renderable until the next placement command sweeps them. Placements that scrolled beyond retained history previously remained in the placement map and tracked-pin set. Long-running graphics output could accumulate stale state, and remapped garbage pins could appear at an unrelated fallback location. Sweep garbage placements before growing the placement map, releasing each tracked pin while preserving virtual placements. Return no geometry or visible render position for garbage pins and cover both storage and C API behavior with regression tests. --- src/terminal/c/kitty_graphics.zig | 21 ++++++- src/terminal/kitty/graphics_storage.zig | 82 +++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 3 deletions(-) diff --git a/src/terminal/c/kitty_graphics.zig b/src/terminal/c/kitty_graphics.zig index c1c738a0b..4e36b88a8 100644 --- a/src/terminal/c/kitty_graphics.zig +++ b/src/terminal/c/kitty_graphics.zig @@ -595,9 +595,9 @@ pub fn placement_render_info( /// the placement's origin has scrolled above the top of the viewport. /// /// A placement is considered not visible if it is a virtual (unicode -/// placeholder) placement, or if it is fully off-screen (its bottom -/// edge is above the viewport or its top edge is at or below the -/// viewport's last row). +/// placeholder) placement, its tracked content has been pruned, or it is +/// fully off-screen (its bottom edge is above the viewport or its top edge is +/// at or below the viewport's last row). fn computeViewportPos( p: *const kitty_storage.ImageStorage.Placement, image: *const Image, @@ -609,6 +609,7 @@ fn computeViewportPos( .pin => |pin| pin, .virtual => return .{ .col = 0, .row = 0, .visible = false }, }; + if (pin.garbage) return .{ .col = 0, .row = 0, .visible = false }; // Convert both the placement's pin and the viewport's top-left // corner to screen-absolute coordinates so we can subtract them @@ -1621,6 +1622,20 @@ test "placement_render_info returns all fields" { try testing.expectEqual(0, ri.source_y); try testing.expectEqual(1, ri.source_width); try testing.expectEqual(2, ri.source_height); + + const entry = iter.?.entry.?; + const pin = switch (entry.value_ptr.location) { + .pin => |pin| pin, + .virtual => unreachable, + }; + pin.garbage = true; + + ri = .{}; + try testing.expectEqual(Result.success, placement_render_info(iter, img, t, &ri)); + try testing.expect(!ri.viewport_visible); + + var rect: selection_c.CSelection = undefined; + try testing.expectEqual(Result.no_value, placement_rect(iter, img, t, &rect)); } test "placement_render_info handles maximum grid dimensions" { diff --git a/src/terminal/kitty/graphics_storage.zig b/src/terminal/kitty/graphics_storage.zig index 6472d0efb..83bd5f9cb 100644 --- a/src/terminal/kitty/graphics_storage.zig +++ b/src/terminal/kitty/graphics_storage.zig @@ -319,6 +319,14 @@ pub const ImageStorage = struct { p, }); + // Tracked pins are marked garbage when their underlying history is + // pruned. Kitty removes placements once they scroll out of retained + // history, so reclaim those placements before growing the map for a + // new one. If allocation below fails, the sweep is still a content + // mutation and must be visible to consumers. + const removed_garbage = self.removeGarbagePlacements(s); + errdefer if (removed_garbage) self.markMutated(io); + // The important piece here is that the placement ID needs to // be marked internal if it is zero. This allows multiple placements // to be added for the same image. If it is non-zero, then it is @@ -345,6 +353,29 @@ pub const ImageStorage = struct { self.markMutated(io); } + /// Remove pin-backed placements whose tracked content has been pruned. + /// Virtual placements have no tracked screen location and are retained. + fn removeGarbagePlacements( + self: *ImageStorage, + s: *terminal.Screen, + ) bool { + var removed = false; + var it = self.placements.iterator(); + while (it.next()) |entry| { + const pin = switch (entry.value_ptr.location) { + .pin => |pin| pin, + .virtual => continue, + }; + if (!pin.garbage) continue; + + entry.value_ptr.deinit(s); + self.placements.removeByPtr(entry.key_ptr); + removed = true; + } + + return removed; + } + fn clearPlacements(self: *ImageStorage, s: *terminal.Screen) void { var it = self.placements.iterator(); while (it.next()) |entry| entry.value_ptr.deinit(s); @@ -990,6 +1021,7 @@ pub const ImageStorage = struct { .pin => |p| p, .virtual => return null, }; + if (pin.garbage) return null; // A zero pixel-sized placement can produce a zero grid size when // pixel geometry is unavailable. It occupies no rectangle. @@ -1093,6 +1125,43 @@ test "storage: replacing placement releases tracked pin" { ); } +test "storage: adding placement reclaims garbage placements" { + const testing = std.testing; + const alloc = testing.allocator; + const io = testing.io; + var t = try terminal.Terminal.init(io, alloc, .{ .cols = 3, .rows = 3 }); + defer t.deinit(alloc); + + var s: ImageStorage = .{}; + defer s.deinit(alloc, t.screens.active); + try s.addImage(io, alloc, .{ .id = 1 }); + + const tracked = t.screens.active.pages.countTrackedPins(); + const old_pin = try trackPin(&t, .{ .x = 0, .y = 0 }); + try s.addPlacement(io, alloc, t.screens.active, 1, 0, .{ + .location = .{ .pin = old_pin }, + }); + old_pin.garbage = true; + + const new_pin = try trackPin(&t, .{ .x = 1, .y = 1 }); + try s.addPlacement(io, alloc, t.screens.active, 1, 0, .{ + .location = .{ .pin = new_pin }, + }); + + try testing.expectEqual(@as(usize, 1), s.placements.count()); + try testing.expectEqual( + tracked + 1, + t.screens.active.pages.countTrackedPins(), + ); + try testing.expectEqual( + new_pin, + s.placements.get(.{ + .image_id = 1, + .placement_id = .{ .tag = .internal, .id = 1 }, + }).?.location.pin, + ); +} + test "storage: delete all placements and images" { const testing = std.testing; const alloc = testing.allocator; @@ -1719,6 +1788,19 @@ test "storage: placement geometry handles untrusted dimensions" { try testing.expectEqual(@as(size.CellCountInt, 1), rect.bottom_right.x); } + // A garbage pin represents content that has been pruned from retained + // history. Its fallback location must not make the placement visible. + pin.garbage = true; + { + const placement: ImageStorage.Placement = .{ + .location = .{ .pin = pin }, + .columns = 1, + .rows = 1, + }; + try testing.expect(placement.rect(.{ .width = 1, .height = 1 }, &t) == null); + } + pin.garbage = false; + // Terminals can temporarily have no pixel geometry. A placement whose // computed grid is empty has no rectangle, so rect must not subtract one // from a zero row count.