terminal/kitty: fix various graphics deletion mismatches with spec

- Limit d=a/A to non-virtual placements that intersect the active screen.
- Keep unrelated unplaced image data when processing d=A.
- Make d=R delete matching unused images even when they have no
  placements, and default an omitted x bound to zero.
- Give ED2 a separate clear path that preserves scrollback references
  while reclaiming every unreferenced image.

References:
- Spec:
  https://sw.kovidgoyal.net/kitty/graphics-protocol/#deleting-images
- Delete reference implementation:
  0ecb10d158/kitty/graphics.c (L2114-L2363)
- ED2 reference implementation:
  0ecb10d158/kitty/screen.c (L2919-L2958)

src/terminal/Terminal.zig

@@ -3401,12 +3401,12 @@ pub fn eraseDisplay(
             self.screens.active.cursor.pending_wrap = false;
 
             if (comptime build_options.kitty_graphics) {
-                // Clear all Kitty graphics state for this screen
-                self.screens.active.kitty_images.delete(
+                // Clear only placements still visible after moving the active
+                // area into scrollback.
+                self.screens.active.kitty_images.clearScreen(
                     self.io(),
                     self.screens.active.alloc,
                     self,
-                    .{ .all = true },
                 );
             }
         },
@@ -3459,12 +3459,12 @@ pub fn eraseDisplay(
             self.screens.active.cursor.pending_wrap = false;
 
             if (comptime build_options.kitty_graphics) {
-                // Clear all Kitty graphics state for this screen
-                self.screens.active.kitty_images.delete(
+                // ED2 clears visible placements but preserves graphics that
+                // are wholly in scrollback.
+                self.screens.active.kitty_images.clearScreen(
                     self.io(),
                     self.screens.active.alloc,
                     self,
-                    .{ .all = true },
                 );
             }
 

src/terminal/kitty/graphics_command.zig

@@ -1094,7 +1094,7 @@ pub const Delete = struct {
                 },
 
                 'r', 'R' => blk: {
-                    const x = kv.get('x') orelse return error.InvalidFormat;
+                    const x = kv.get('x') orelse 0;
                     const y = kv.get('y') orelse return error.InvalidFormat;
                     if (x > y) return error.InvalidFormat;
                     break :blk .{
@@ -1594,5 +1594,14 @@ test "delete range command 5" {
 
     const input = "a=d,d=R,y=5";
     for (input) |c| try p.feed(c);
-    try testing.expectError(error.InvalidFormat, p.complete(alloc));
+    const command = try p.complete(alloc);
+    defer command.deinit(alloc);
+
+    try testing.expect(command.control == .delete);
+    const v = command.control.delete.action;
+    try testing.expect(v == .range);
+    const range = v.range;
+    try testing.expect(range.delete);
+    try testing.expectEqual(@as(u32, 0), range.first);
+    try testing.expectEqual(@as(u32, 5), range.last);
 }

src/terminal/kitty/graphics_storage.zig

@@ -447,6 +447,30 @@ pub const ImageStorage = struct {
         return newest;
     }
 
+    /// Clear placements intersecting the active screen, then reclaim every
+    /// image with no remaining placement. Unlike protocol d=A, a terminal
+    /// clear also reclaims images that were already unplaced.
+    pub fn clearScreen(
+        self: *ImageStorage,
+        io: std.Io,
+        alloc: Allocator,
+        t: *terminal.Terminal,
+    ) void {
+        const placements_before = self.placements.count();
+        const images_before = self.images.count();
+
+        // Delete unused placements and images
+        self.deleteVisiblePlacements(alloc, t, true);
+        var image_it = self.images.iterator();
+        while (image_it.next()) |entry| {
+            self.deleteIfUnused(alloc, entry.key_ptr.*);
+        }
+
+        // Mark mutated only if things changed.
+        if (self.placements.count() != placements_before or
+            self.images.count() != images_before) self.markMutated(io);
+    }
+
     /// Delete placements, images.
     pub fn delete(
         self: *ImageStorage,
@@ -457,37 +481,19 @@ pub const ImageStorage = struct {
     ) void {
         // Deletes only ever remove placements/images, so comparing counts
         // before and after tells us whether anything actually changed.
-        // Only then do we mark a mutation. This matters because a
-        // delete-all runs on every screen clear (e.g. `ESC [ 2 J`), and
-        // we don't want empty clears to dirty the image state or bump
-        // the generation.
+        // Only then do we mark a mutation, so a delete that matches nothing
+        // doesn't dirty the image state or bump the generation.
         const placements_before = self.placements.count();
         const images_before = self.images.count();
         defer if (self.placements.count() != placements_before or
             self.images.count() != images_before) self.markMutated(io);
 
         switch (cmd) {
-            .all => |delete_images| {
-                var it = self.placements.iterator();
-                while (it.next()) |entry| {
-                    // Skip virtual placements
-                    switch (entry.value_ptr.location) {
-                        .pin => {},
-                        .virtual => continue,
-                    }
-
-                    // Deinit the placement and remove it
-                    const image_id = entry.key_ptr.image_id;
-                    entry.value_ptr.deinit(t.screens.active);
-                    self.removePlacementByPtr(entry.key_ptr);
-                    if (delete_images) self.deleteIfUnused(alloc, image_id);
-                }
-
-                if (delete_images) {
-                    var image_it = self.images.iterator();
-                    while (image_it.next()) |kv| self.deleteIfUnused(alloc, kv.key_ptr.*);
-                }
-            },
+            .all => |delete_images| self.deleteVisiblePlacements(
+                alloc,
+                t,
+                delete_images,
+            ),
 
             .id => |v| self.deleteById(
                 alloc,
@@ -633,8 +639,9 @@ pub const ImageStorage = struct {
             },
 
             .range => |v| range: {
-                if (v.first <= 0 or v.last <= 0) {
-                    log.warn("delete range values must be greater than zero", .{});
+                // The lower bound defaults to zero when x is omitted.
+                if (v.last == 0) {
+                    log.warn("delete range upper bound must be greater than zero", .{});
                     break :range;
                 }
                 if (v.first > v.last) {
@@ -642,13 +649,24 @@ pub const ImageStorage = struct {
                     break :range;
                 }
 
-                var it = self.placements.iterator();
-                while (it.next()) |entry| {
-                    if (entry.key_ptr.image_id >= v.first and entry.key_ptr.image_id <= v.last) {
-                        const image_id = entry.key_ptr.image_id;
-                        entry.value_ptr.deinit(t.screens.active);
-                        self.removePlacementByPtr(entry.key_ptr);
-                        if (v.delete) self.deleteIfUnused(alloc, image_id);
+                // Remove matching placements in one pass.
+                var placement_it = self.placements.iterator();
+                while (placement_it.next()) |entry| {
+                    if (entry.key_ptr.image_id < v.first or
+                        entry.key_ptr.image_id > v.last) continue;
+
+                    entry.value_ptr.deinit(t.screens.active);
+                    self.removePlacementByPtr(entry.key_ptr);
+                }
+
+                // Uppercase deletion also frees matching images that are now
+                // unused, including images that had no placements initially.
+                if (!v.delete) break :range;
+                var image_it = self.images.iterator();
+                while (image_it.next()) |entry| {
+                    const image_id = entry.key_ptr.*;
+                    if (image_id >= v.first and image_id <= v.last) {
+                        self.deleteIfUnused(alloc, image_id);
                     }
                 }
             },
@@ -659,6 +677,43 @@ pub const ImageStorage = struct {
         }
     }
 
+    /// Delete only non-virtual placements that intersect the active screen.
+    /// The protocol defines d=a/A in terms of visible placements, and an
+    /// uppercase delete only frees data for images selected by those
+    /// placements.
+    fn deleteVisiblePlacements(
+        self: *ImageStorage,
+        alloc: Allocator,
+        t: *terminal.Terminal,
+        delete_unused: bool,
+    ) void {
+        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;
+
+            // Placements anchored in the active area are necessarily visible.
+            // Only compute their extent when the anchor is already in history
+            // and the bottom edge may still intersect the active area.
+            if (t.screens.active.pages.pointFromPin(
+                .active,
+                pin.*,
+            ) == null) {
+                const image = self.imageById(entry.key_ptr.image_id) orelse continue;
+                const rect = entry.value_ptr.rect(image, t) orelse continue;
+                if (t.screens.active.pages.pointFromPin(.active, rect.bottom_right) == null) continue;
+            }
+
+            const image_id = entry.key_ptr.image_id;
+            entry.value_ptr.deinit(t.screens.active);
+            self.removePlacementByPtr(entry.key_ptr);
+            if (delete_unused) self.deleteIfUnused(alloc, image_id);
+        }
+    }
+
     fn deleteById(
         self: *ImageStorage,
         alloc: Allocator,
@@ -668,22 +723,18 @@ pub const ImageStorage = struct {
         delete_unused: bool,
     ) void {
         // If no placement, we delete all placements with the ID
-        if (placement_id == 0) {
-            var it = self.placements.iterator();
-            while (it.next()) |entry| {
-                if (entry.key_ptr.image_id == image_id) {
-                    entry.value_ptr.deinit(s);
-                    self.removePlacementByPtr(entry.key_ptr);
-                }
-            }
-        } else {
-            if (self.placements.getEntry(.{
-                .image_id = image_id,
-                .placement_id = .{ .tag = .external, .id = placement_id },
-            })) |entry| {
-                entry.value_ptr.deinit(s);
-                self.removePlacementByPtr(entry.key_ptr);
-            }
+        if (placement_id == 0) self.removePlacementsByImageId(
+            s,
+            image_id,
+        ) else if (self.placements.getEntry(.{
+            .image_id = image_id,
+            .placement_id = .{
+                .tag = .external,
+                .id = placement_id,
+            },
+        })) |entry| {
+            entry.value_ptr.deinit(s);
+            self.removePlacementByPtr(entry.key_ptr);
         }
 
         // If this is specified, then we also delete the image
@@ -1236,7 +1287,7 @@ test "storage: placement count limit permits replacement" {
     try testing.expectEqual(@as(usize, 1), s.placements.count());
 }
 
-test "storage: delete all placements and images" {
+test "storage: delete all visible placements and matching images" {
     const testing = std.testing;
     const alloc = testing.allocator;
     const io = testing.io;
@@ -1255,7 +1306,8 @@ test "storage: delete all placements and images" {
     s.dirty = false;
     s.delete(io, alloc, &t, .{ .all = true });
     try testing.expect(s.dirty);
-    try testing.expectEqual(@as(usize, 0), s.images.count());
+    try testing.expectEqual(@as(usize, 1), s.images.count());
+    try testing.expect(s.images.contains(3));
     try testing.expectEqual(@as(usize, 0), s.placements.count());
     try testing.expectEqual(tracked, t.screens.active.pages.countTrackedPins());
 }
@@ -1280,12 +1332,98 @@ test "storage: delete all placements and images preserves limit" {
     s.dirty = false;
     s.delete(io, alloc, &t, .{ .all = true });
     try testing.expect(s.dirty);
-    try testing.expectEqual(@as(usize, 0), s.images.count());
+    try testing.expectEqual(@as(usize, 1), s.images.count());
+    try testing.expect(s.images.contains(3));
     try testing.expectEqual(@as(usize, 0), s.placements.count());
     try testing.expectEqual(@as(usize, 5000), s.total_limit);
     try testing.expectEqual(tracked, t.screens.active.pages.countTrackedPins());
 }
 
+test "storage: delete all visible placements preserves scrollback" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+    const io = testing.io;
+    var t = try terminal.Terminal.init(io, alloc, .{
+        .rows = 2,
+        .cols = 2,
+        .max_scrollback_bytes = 4096,
+    });
+    defer t.deinit(alloc);
+    t.width_px = 2;
+    t.height_px = 2;
+
+    var s: ImageStorage = .{};
+    defer s.deinit(alloc, t.screens.active);
+    try s.addImage(io, alloc, t.screens.active, .{ .id = 1, .width = 1, .height = 1 });
+    try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{
+        .location = .{ .pin = try trackPin(&t, .{ .y = 0 }) },
+    });
+    try t.scrollUp(1);
+
+    const history_key: ImageStorage.PlacementKey = .{
+        .image_id = 1,
+        .placement_id = .{ .tag = .external, .id = 1 },
+    };
+    try testing.expect(t.screens.active.pages.pointFromPin(
+        .active,
+        s.placements.get(history_key).?.location.pin.*,
+    ) == null);
+
+    try s.addPlacement(io, alloc, t.screens.active, 1, 2, .{
+        .location = .{ .pin = try trackPin(&t, .{ .y = 0 }) },
+    });
+
+    s.delete(io, alloc, &t, .{ .all = true });
+    try testing.expectEqual(@as(usize, 1), s.placements.count());
+    try testing.expect(s.placements.contains(history_key));
+    try testing.expectEqual(@as(usize, 1), s.images.count());
+    try testing.expect(s.images.contains(1));
+}
+
+test "storage: delete all includes placements spanning into active area" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+    const io = testing.io;
+    var t = try terminal.Terminal.init(io, alloc, .{
+        .rows = 2,
+        .cols = 2,
+        .max_scrollback_bytes = 4096,
+    });
+    defer t.deinit(alloc);
+    t.width_px = 2;
+    t.height_px = 2;
+
+    var s: ImageStorage = .{};
+    defer s.deinit(alloc, t.screens.active);
+    try s.addImage(io, alloc, t.screens.active, .{
+        .id = 1,
+        .width = 1,
+        .height = 2,
+    });
+    try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{
+        .location = .{ .pin = try trackPin(&t, .{ .y = 0 }) },
+    });
+    try t.scrollUp(1);
+
+    const placement = s.placements.get(.{
+        .image_id = 1,
+        .placement_id = .{ .tag = .external, .id = 1 },
+    }).?;
+    try testing.expect(t.screens.active.pages.pointFromPin(
+        .active,
+        placement.location.pin.*,
+    ) == null);
+    const rect = placement.rect(s.imageById(1).?, &t).?;
+    try testing.expect(t.screens.active.pages.pointFromPin(
+        .active,
+        rect.bottom_right,
+    ) != null);
+
+    s.delete(io, alloc, &t, .{ .all = true });
+    try testing.expectEqual(@as(usize, 0), s.placements.count());
+    try testing.expectEqual(@as(usize, 0), s.images.count());
+}
+
 test "storage: delete all placements" {
     const testing = std.testing;
     const alloc = testing.allocator;
@@ -1793,6 +1931,75 @@ test "storage: delete images by range 4" {
     try testing.expectEqual(tracked + 2, t.screens.active.pages.countTrackedPins());
 }
 
+test "storage: uppercase range deletes unplaced image data" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+    const io = testing.io;
+    var t = try terminal.Terminal.init(io, alloc, .{ .rows = 3, .cols = 3 });
+    defer t.deinit(alloc);
+
+    var s: ImageStorage = .{};
+    defer s.deinit(alloc, t.screens.active);
+    try s.addImage(io, alloc, t.screens.active, .{ .id = 1 });
+    try s.addImage(io, alloc, t.screens.active, .{ .id = 2 });
+    try s.addImage(io, alloc, t.screens.active, .{ .id = 3 });
+
+    s.delete(io, alloc, &t, .{
+        .range = .{
+            .delete = true,
+            // Zero is the default lower bound when x is omitted.
+            .first = 0,
+            .last = 2,
+        },
+    });
+    try testing.expectEqual(@as(usize, 1), s.images.count());
+    try testing.expect(s.images.contains(3));
+}
+
+test "storage: erase display preserves scrollback and reclaims unplaced images" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+    const io = testing.io;
+    var t = try terminal.Terminal.init(io, alloc, .{
+        .rows = 2,
+        .cols = 2,
+        .max_scrollback_bytes = 4096,
+    });
+    defer t.deinit(alloc);
+    t.width_px = 2;
+    t.height_px = 2;
+
+    const s = &t.screens.active.kitty_images;
+    try s.addImage(io, alloc, t.screens.active, .{ .id = 1, .width = 1, .height = 1 });
+    try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{
+        .location = .{ .pin = try trackPin(&t, .{ .y = 0 }) },
+    });
+    try t.scrollUp(1);
+
+    const history_key: ImageStorage.PlacementKey = .{
+        .image_id = 1,
+        .placement_id = .{ .tag = .external, .id = 1 },
+    };
+    try testing.expect(t.screens.active.pages.pointFromPin(
+        .active,
+        s.placements.get(history_key).?.location.pin.*,
+    ) == null);
+
+    try s.addImage(io, alloc, t.screens.active, .{ .id = 2, .width = 1, .height = 1 });
+    try s.addPlacement(io, alloc, t.screens.active, 2, 1, .{
+        .location = .{ .pin = try trackPin(&t, .{ .y = 0 }) },
+    });
+    try s.addImage(io, alloc, t.screens.active, .{ .id = 3, .width = 1, .height = 1 });
+
+    t.eraseDisplay(.complete, false);
+    try testing.expectEqual(@as(usize, 1), s.placements.count());
+    try testing.expect(s.placements.contains(history_key));
+    try testing.expectEqual(@as(usize, 1), s.images.count());
+    try testing.expect(s.images.contains(1));
+    try testing.expect(!s.images.contains(2));
+    try testing.expect(!s.images.contains(3));
+}
+
 test "storage: cell offsets stay within explicit destination rectangle" {
     const testing = std.testing;
     const alloc = testing.allocator;