terminal/kitty: clear placements on image retransmit (#13723)

Fixes #13719

The Kitty graphics protocol requires retransmitting data for a specific
image ID to delete the previous image and all of its placements.

Ghostty instead preserved the placement count and map when replacing
image data. Repeated `a=T` commands therefore added one anonymous
placement per frame and retained its tracked pin.

Spec:
https://sw.kovidgoyal.net/kitty/graphics-protocol/#display-images-on-screen

src/renderer/image.zig

@@ -976,7 +976,7 @@ pub const Image = union(enum) {
     }
 };
 
-test "kitty renderer ignores pending payloads and retains native placements" {
+test "kitty renderer ignores pending payloads and removes replaced placements" {
     const testing = std.testing;
     const alloc = testing.allocator;
     const io = testing.io;
@@ -990,6 +990,7 @@ test "kitty renderer ignores pending payloads and retains native placements" {
     defer state.deinit(alloc);
 
     const storage = &t.screens.active.kitty_images;
+    const tracked = t.screens.active.pages.countTrackedPins();
     const pending = try storage.addPendingImage(io, alloc, t.screens.active, .{
         .id = 1,
         .width = 1,
@@ -1020,8 +1021,8 @@ test "kitty renderer ignores pending payloads and retains native placements" {
         state.images.get(.{ .kitty = 1 }).?.generation,
     );
 
-    // A newer pending replacement clears the renderer placement and marks
-    // the copied texture data for unload without deleting the native pin.
+    // A newer pending replacement deletes the native placement and marks the
+    // copied texture data for unload.
     _ = try storage.addPendingImage(io, alloc, t.screens.active, .{
         .id = 1,
         .width = 1,
@@ -1030,7 +1031,8 @@ test "kitty renderer ignores pending payloads and retains native placements" {
         .data = .{ .pending = 4 },
     });
     state.kittyUpdate(alloc, &t, .{ .width = 10, .height = 10 });
-    try testing.expectEqual(@as(usize, 1), storage.placements.count());
+    try testing.expectEqual(@as(usize, 0), storage.placements.count());
+    try testing.expectEqual(tracked, t.screens.active.pages.countTrackedPins());
     try testing.expectEqual(@as(usize, 0), state.kitty_placements.items.len);
     try testing.expect(state.images.get(.{ .kitty = 1 }).?.image.isUnloading());
 }

src/terminal/kitty/graphics_exec.zig

@@ -636,6 +636,86 @@ test "kittygfx retransmit same id gets fresh image generation" {
     try testing.expectEqual(gen2, storage.generation);
 }
 
+test "kittygfx retransmit same id removes existing placements" {
+    const testing = std.testing;
+    const io = testing.io;
+    const alloc = testing.allocator;
+
+    var t = try Terminal.init(io, alloc, .{ .rows = 5, .cols = 5 });
+    defer t.deinit(alloc);
+    const storage = &t.screens.active.kitty_images;
+    const tracked = t.screens.active.pages.countTrackedPins();
+
+    // Transmit and display an image, then add anonymous and named placements.
+    // Multiple anonymous a=p placements for one image are explicitly valid.
+    {
+        const cmd = try command.Parser.parseString(
+            alloc,
+            "a=T,t=d,f=24,i=1,s=1,v=2,C=1;////////",
+        );
+        defer cmd.deinit(alloc);
+        const resp = execute(io, alloc, &t, &cmd).?;
+        try testing.expect(resp.ok());
+    }
+    {
+        const cmd = try command.Parser.parseString(alloc, "a=p,i=1,C=1");
+        defer cmd.deinit(alloc);
+        const resp = execute(io, alloc, &t, &cmd).?;
+        try testing.expect(resp.ok());
+    }
+    {
+        const cmd = try command.Parser.parseString(alloc, "a=p,i=1,p=7,C=1");
+        defer cmd.deinit(alloc);
+        const resp = execute(io, alloc, &t, &cmd).?;
+        try testing.expect(resp.ok());
+    }
+    try testing.expectEqual(@as(usize, 3), storage.placements.count());
+    try testing.expectEqual(
+        tracked + 3,
+        t.screens.active.pages.countTrackedPins(),
+    );
+
+    // Retransmitting replaces the image and must delete every old placement.
+    // Plain a=t creates no replacement placement of its own.
+    {
+        const cmd = try command.Parser.parseString(
+            alloc,
+            "a=t,t=d,f=24,i=1,s=1,v=2;AAAAAAAA",
+        );
+        defer cmd.deinit(alloc);
+        const resp = execute(io, alloc, &t, &cmd).?;
+        try testing.expect(resp.ok());
+    }
+    try testing.expectEqual(@as(usize, 0), storage.placements.count());
+    try testing.expectEqual(tracked, t.screens.active.pages.countTrackedPins());
+
+    // a=T creates one new placement after deleting the previous image and
+    // placements, so repeated redraws remain bounded at one placement.
+    {
+        const cmd = try command.Parser.parseString(
+            alloc,
+            "a=T,t=d,f=24,i=1,s=1,v=2,C=1;////////",
+        );
+        defer cmd.deinit(alloc);
+        const resp = execute(io, alloc, &t, &cmd).?;
+        try testing.expect(resp.ok());
+    }
+    {
+        const cmd = try command.Parser.parseString(
+            alloc,
+            "a=T,t=d,f=24,i=1,s=1,v=2,C=1;AAAAAAAA",
+        );
+        defer cmd.deinit(alloc);
+        const resp = execute(io, alloc, &t, &cmd).?;
+        try testing.expect(resp.ok());
+    }
+    try testing.expectEqual(@as(usize, 1), storage.placements.count());
+    try testing.expectEqual(
+        tracked + 1,
+        t.screens.active.pages.countTrackedPins(),
+    );
+}
+
 test "kittygfx delete then retransmit same id gets fresh generation" {
     const testing = std.testing;
     const io = testing.io;

src/terminal/kitty/graphics_storage.zig

@@ -246,7 +246,7 @@ pub const ImageStorage = struct {
         // Credit an existing image's reservation before calculating the
         // replacement size. In particular, a completed live transmission
         // replacing pending snapshot metadata must be able to reuse the
-        // reservation without evicting its own ID and placements.
+        // reservation without evicting its own ID.
         const old_len = if (self.images.get(img.id)) |old|
             old.data.len()
         else
@@ -274,16 +274,16 @@ pub const ImageStorage = struct {
 
         log.debug("addImage image={}", .{img.withoutData()});
 
-        // Write our new image, preserving placements across replacement.
-        const placement_count = if (gop.found_existing) count: {
+        // Retransmitting a specific image ID replaces the old image and all
+        // of its placements, as required by the Kitty graphics protocol.
+        if (gop.found_existing) {
+            self.removePlacementsByImageId(s, img.id);
             self.total_bytes -= gop.value_ptr.data.len();
-            const count = gop.value_ptr.metadata.placement_count;
             gop.value_ptr.deinit(alloc);
-            break :count count;
-        } else 0;
+        }
 
         gop.value_ptr.* = img;
-        gop.value_ptr.metadata.placement_count = placement_count;
+        gop.value_ptr.metadata.placement_count = 0;
         self.total_bytes += new_len;
 
         // Stamp the stored image with a fresh generation. This gives
@@ -404,6 +404,19 @@ pub const ImageStorage = struct {
         self.placements.clearRetainingCapacity();
     }
 
+    fn removePlacementsByImageId(
+        self: *ImageStorage,
+        s: *terminal.Screen,
+        image_id: u32,
+    ) void {
+        var it = self.placements.iterator();
+        while (it.next()) |entry| {
+            if (entry.key_ptr.image_id != image_id) continue;
+            entry.value_ptr.deinit(s);
+            self.removePlacementByPtr(entry.key_ptr);
+        }
+    }
+
     fn removePlacementByPtr(self: *ImageStorage, key: *PlacementKey) void {
         const img = self.images.getPtr(key.image_id).?;
         assert(img.metadata.placement_count > 0);
@@ -2217,7 +2230,7 @@ test "storage: stale pending completion loses to delete replacement and eviction
     defer alloc.free(evicted_data);
 }
 
-test "storage: replacement reuses pending reservation and preserves placements" {
+test "storage: replacement reuses pending reservation and removes placements" {
     const testing = std.testing;
     const io = testing.io;
     const alloc = testing.allocator;
@@ -2226,6 +2239,7 @@ test "storage: replacement reuses pending reservation and preserves placements"
 
     var s: ImageStorage = .{ .total_limit = 12 };
     defer s.deinit(alloc, t.screens.active);
+    const tracked = t.screens.active.pages.countTrackedPins();
 
     const pending = try s.addPendingImage(io, alloc, t.screens.active, .{
         .id = 1,
@@ -2251,7 +2265,12 @@ test "storage: replacement reuses pending reservation and preserves placements"
     });
     try testing.expect(s.images.contains(1));
     try testing.expect(s.images.contains(2));
-    try testing.expectEqual(@as(usize, 1), s.placements.count());
+    try testing.expectEqual(@as(usize, 0), s.placements.count());
+    try testing.expectEqual(tracked, t.screens.active.pages.countTrackedPins());
+    try testing.expectEqual(
+        @as(u30, 0),
+        s.imageById(1).?.metadata.placement_count,
+    );
     try testing.expectEqual(@as(usize, 12), s.total_bytes);
 
     const stale = try alloc.dupe(u8, "snapshot");
@@ -2259,18 +2278,17 @@ test "storage: replacement reuses pending reservation and preserves placements"
     try testing.expect(!stale_completed);
     defer alloc.free(stale);
 
-    // Growing the replacement requires eviction, but the replacement ID and
-    // its placement are excluded. The other image supplies the needed bytes.
+    // Growing the replacement requires eviction, but the replacement ID is
+    // excluded. The other image supplies the needed bytes.
     try s.addImage(io, alloc, t.screens.active, .{
         .id = 1,
         .data = .{ .complete = try alloc.dupe(u8, "1234567890") },
     });
     try testing.expect(s.images.contains(1));
     try testing.expect(!s.images.contains(2));
-    try testing.expectEqual(@as(usize, 1), s.placements.count());
+    try testing.expectEqual(@as(usize, 0), s.placements.count());
     try testing.expectEqual(@as(usize, 10), s.total_bytes);
 
-    // The placement remains a live reference across both replacements.
     s.delete(io, alloc, &t, .{ .id = .{ .delete = true, .image_id = 1 } });
     try testing.expectEqual(@as(usize, 0), s.images.count());
     try testing.expectEqual(@as(usize, 0), s.placements.count());