mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-08-24 16:11:43 +00:00
terminal/kitty: fix point deletion calculations (d=p, d=c)
Fix d=p and d=c point deletion so only placements intersecting the target cell are removed. Previously, placements spanning multiple rows could be deleted from columns outside the target because the page-order comparison flattened row and column coordinates. Check the rectangle's column independently and use page order only for its row span, matching Kitty's implementation: https://github.com/kovidgoyal/kitty/blob/master/kitty/graphics.c NOTE: I did not look at Kitty's source prior to fixing this. I only referenced it after the fix to verify that the behavior matches. Spec: https://sw.kovidgoyal.net/kitty/graphics-protocol/#deleting-images
This commit is contained in:
@@ -682,6 +682,17 @@ pub const Image = struct {
|
||||
pub const Rect = struct {
|
||||
top_left: PageList.Pin,
|
||||
bottom_right: PageList.Pin,
|
||||
|
||||
/// Returns true if the grid cell is inside this rectangle. Pin.isBetween
|
||||
/// compares page order, so its interior rows intentionally do not constrain
|
||||
/// x. Check the column independently and use isBetween only for the row.
|
||||
pub fn contains(self: Rect, cell: PageList.Pin) bool {
|
||||
if (cell.x < self.top_left.x or cell.x > self.bottom_right.x) return false;
|
||||
|
||||
var row = cell;
|
||||
row.x = self.top_left.x;
|
||||
return row.isBetween(self.top_left, self.bottom_right);
|
||||
}
|
||||
};
|
||||
|
||||
/// Returns true if `path` is `dir` or is contained within it, requiring a
|
||||
|
||||
@@ -683,7 +683,7 @@ pub const ImageStorage = struct {
|
||||
while (it.next()) |entry| {
|
||||
const img = self.imageById(entry.key_ptr.image_id) orelse continue;
|
||||
const rect = entry.value_ptr.rect(img, t) orelse continue;
|
||||
if (target_pin.isBetween(rect.top_left, rect.bottom_right)) {
|
||||
if (rect.contains(target_pin)) {
|
||||
if (filter) |f| if (!f(filter_ctx, entry.value_ptr.*)) continue;
|
||||
entry.value_ptr.deinit(t.screens.active);
|
||||
self.placements.removeByPtr(entry.key_ptr);
|
||||
@@ -1346,6 +1346,61 @@ test "storage: delete intersecting cursor" {
|
||||
}) != null);
|
||||
}
|
||||
|
||||
test "storage: delete intersecting cursor checks interior row column" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
const io = testing.io;
|
||||
var t = try terminal.Terminal.init(io, alloc, .{ .rows = 100, .cols = 100 });
|
||||
defer t.deinit(alloc);
|
||||
t.width_px = 100;
|
||||
t.height_px = 100;
|
||||
|
||||
var s: ImageStorage = .{};
|
||||
defer s.deinit(alloc, t.screens.active);
|
||||
try s.addImage(io, alloc, .{ .id = 1, .width = 10, .height = 10 });
|
||||
try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } });
|
||||
try s.addPlacement(io, alloc, t.screens.active, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 20, .y = 0 }) } });
|
||||
|
||||
// This is inside the right placement and on an interior row shared by
|
||||
// both placements, but it is outside the left placement's columns.
|
||||
t.screens.active.cursorAbsolute(21, 5);
|
||||
s.delete(io, alloc, &t, .{ .intersect_cursor = false });
|
||||
|
||||
try testing.expectEqual(@as(usize, 1), s.placements.count());
|
||||
try testing.expect(s.placements.get(.{
|
||||
.image_id = 1,
|
||||
.placement_id = .{ .tag = .external, .id = 1 },
|
||||
}) != null);
|
||||
}
|
||||
|
||||
test "storage: delete intersecting cell checks interior row column" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
const io = testing.io;
|
||||
var t = try terminal.Terminal.init(io, alloc, .{ .rows = 100, .cols = 100 });
|
||||
defer t.deinit(alloc);
|
||||
t.width_px = 100;
|
||||
t.height_px = 100;
|
||||
|
||||
var s: ImageStorage = .{};
|
||||
defer s.deinit(alloc, t.screens.active);
|
||||
try s.addImage(io, alloc, .{ .id = 1, .width = 10, .height = 10 });
|
||||
try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } });
|
||||
try s.addPlacement(io, alloc, t.screens.active, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 20, .y = 0 }) } });
|
||||
|
||||
// Protocol coordinates are one-based, so this targets grid cell (21, 5).
|
||||
s.delete(io, alloc, &t, .{ .intersect_cell = .{
|
||||
.x = 22,
|
||||
.y = 6,
|
||||
} });
|
||||
|
||||
try testing.expectEqual(@as(usize, 1), s.placements.count());
|
||||
try testing.expect(s.placements.get(.{
|
||||
.image_id = 1,
|
||||
.placement_id = .{ .tag = .external, .id = 1 },
|
||||
}) != null);
|
||||
}
|
||||
|
||||
test "storage: delete intersecting cursor plus unused" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
Reference in New Issue
Block a user