terminal/kitty: constrain placement offsets to cell bounds (#13891)

Clamp X and Y offsets when a placement is created and normalize them
again against current cell geometry when sizing and rendering. The
protocol requires offsets to remain within the first cell and not
enlarge explicit c/r rectangles:

https://sw.kovidgoyal.net/kitty/graphics-protocol/#controlling-displayed-image-layout

Previously, explicit placements were sized to the full c/r rectangle
before the offset was applied. This extended their far edge into
neighboring cells and let unbounded offsets reach renderer geometry.
This commit is contained in:
Mitchell Hashimoto
2026-08-18 10:31:53 -07:00
committed by GitHub
3 changed files with 148 additions and 23 deletions

View File

@@ -438,6 +438,7 @@ pub const State = struct {
// Calculate the dimensions of our image, taking in to
// account the rows / columns specified by the placement.
const dest_size = p.pixelSize(image.*, t);
const cell_offset = p.cellOffset(t);
const source = p.sourceRect(image.*);
@@ -453,8 +454,8 @@ pub const State = struct {
.z = p.z,
.width = dest_size.width,
.height = dest_size.height,
.cell_offset_x = p.x_offset,
.cell_offset_y = p.y_offset,
.cell_offset_x = cell_offset.x,
.cell_offset_y = cell_offset.y,
.source_x = source.x,
.source_y = source.y,
.source_width = source.width,

View File

@@ -259,17 +259,25 @@ fn display(
};
// Add the placement
const p: ImageStorage.Placement = .{
.location = location,
.x_offset = d.x_offset,
.y_offset = d.y_offset,
.source_x = d.x,
.source_y = d.y,
.source_width = d.width,
.source_height = d.height,
.columns = d.columns,
.rows = d.rows,
.z = d.z,
const p: ImageStorage.Placement = placement: {
var p: ImageStorage.Placement = .{
.location = location,
.x_offset = d.x_offset,
.y_offset = d.y_offset,
.source_x = d.x,
.source_y = d.y,
.source_width = d.width,
.source_height = d.height,
.columns = d.columns,
.rows = d.rows,
.z = d.z,
};
const cell_offset = p.cellOffset(terminal);
if (terminal.width_px / terminal.cols > 0) p.x_offset = cell_offset.x;
if (terminal.height_px / terminal.rows > 0) p.y_offset = cell_offset.y;
break :placement p;
};
storage.addPlacement(
io,
@@ -916,6 +924,36 @@ test "kittygfx delete then retransmit same id gets fresh generation" {
try testing.expect(gen2 > gen_delete);
}
test "kittygfx display clamps cell offsets" {
const testing = std.testing;
const alloc = testing.allocator;
const io = testing.io;
var t = try Terminal.init(io, alloc, .{ .rows = 5, .cols = 5 });
defer t.deinit(alloc);
t.width_px = 50; // 10 px per col
t.height_px = 100; // 20 px per row
const cmd = try command.Parser.parseString(
alloc,
"a=T,t=d,f=24,i=1,s=1,v=1,c=2,r=1,X=99,Y=99,C=1;AAAA",
);
defer cmd.deinit(alloc);
const resp = execute(io, alloc, &t, &cmd).?;
try testing.expect(resp.ok());
const storage = &t.screens.active.kitty_images;
var it = storage.placements.iterator();
const placement = it.next().?.value_ptr;
try testing.expectEqual(@as(u32, 9), placement.x_offset);
try testing.expectEqual(@as(u32, 19), placement.y_offset);
const actual = placement.pixelSize(storage.imageById(1).?, &t);
try testing.expectEqual(@as(u32, 11), actual.width);
try testing.expectEqual(@as(u32, 1), actual.height);
}
test "kittygfx placement bounds cursor movement for untrusted dimensions" {
const testing = std.testing;
const alloc = testing.allocator;

View File

@@ -921,6 +921,31 @@ pub const ImageStorage = struct {
};
}
/// Returns the placement offset clamped within the first cell. Pixel
/// geometry may temporarily be unavailable, in which case there is no
/// valid offset within the cell.
pub fn cellOffset(
self: Placement,
t: *const terminal.Terminal,
) struct {
x: u32,
y: u32,
} {
const cell_width: u32 = t.width_px / t.cols;
const cell_height: u32 = t.height_px / t.rows;
return .{
.x = if (cell_width > 0)
@min(self.x_offset, cell_width - 1)
else
0,
.y = if (cell_height > 0)
@min(self.y_offset, cell_height - 1)
else
0,
};
}
/// Returns the size of this placement's image in pixels,
/// taking into account the source rectangle, specified
/// rows/columns, and aspect ratio.
@@ -951,6 +976,7 @@ pub const ImageStorage = struct {
// count and the height by the row count, because it should be.
const cell_width: u32 = t.width_px / t.cols;
const cell_height: u32 = t.height_px / t.rows;
const cell_offset = self.cellOffset(t);
// If we have a specified cols AND rows then we calculate
// the width and height from them directly, we don't need
@@ -960,8 +986,8 @@ pub const ImageStorage = struct {
const calc_height = saturatingMul(cell_height, self.rows);
return .{
.width = calc_width,
.height = calc_height,
.width = calc_width -| cell_offset.x,
.height = calc_height -| cell_offset.y,
};
}
@@ -971,7 +997,10 @@ pub const ImageStorage = struct {
// If only the columns were specified, we determine
// the height of the image based on the aspect ratio.
if (self.columns > 0) {
const calc_width = saturatingMul(cell_width, self.columns);
const calc_width = saturatingMul(
cell_width,
self.columns,
) -| cell_offset.x;
const calc_height = scaleDimension(calc_width, height, width);
return .{
@@ -983,7 +1012,10 @@ pub const ImageStorage = struct {
// Otherwise, only the rows were specified, so we
// determine the width based on the aspect ratio.
{
const calc_height = saturatingMul(cell_height, self.rows);
const calc_height = saturatingMul(
cell_height,
self.rows,
) -| cell_offset.y;
const calc_width = scaleDimension(calc_height, width, height);
return .{
@@ -1011,15 +1043,16 @@ pub const ImageStorage = struct {
// Otherwise we calculate the pixel size, divide by
// cell size, and round up to the nearest integer.
const calc_size = self.pixelSize(image, t);
const cell_offset = self.cellOffset(t);
return .{
.cols = std.math.divCeil(
u32,
calc_size.width +| self.x_offset,
calc_size.width +| cell_offset.x,
t.width_px / t.cols,
) catch 0,
.rows = std.math.divCeil(
u32,
calc_size.height +| self.y_offset,
calc_size.height +| cell_offset.y,
t.height_px / t.rows,
) catch 0,
};
@@ -1760,6 +1793,59 @@ test "storage: delete images by range 4" {
try testing.expectEqual(tracked + 2, t.screens.active.pages.countTrackedPins());
}
test "storage: cell offsets stay within explicit destination rectangle" {
const testing = std.testing;
const alloc = testing.allocator;
const io = testing.io;
var t = try terminal.Terminal.init(io, alloc, .{ .cols = 5, .rows = 5 });
defer t.deinit(alloc);
t.width_px = 50; // 10 px per col
t.height_px = 100; // 20 px per row
// Explicit columns and rows describe the far cell boundaries. Offsets
// move the near edge inward without moving those far edges.
const placement: ImageStorage.Placement = .{
.location = .{ .virtual = {} },
.x_offset = 3,
.y_offset = 4,
.columns = 2,
.rows = 1,
};
const actual = placement.pixelSize(.{ .width = 4, .height = 3 }, &t);
try testing.expectEqual(@as(u32, 17), actual.width);
try testing.expectEqual(@as(u32, 16), actual.height);
try testing.expectEqual(@as(u32, 2), placement.gridSize(.{ .width = 4, .height = 3 }, &t).cols);
try testing.expectEqual(@as(u32, 1), placement.gridSize(.{ .width = 4, .height = 3 }, &t).rows);
}
test "storage: cell offsets clamp to cell bounds" {
const testing = std.testing;
const alloc = testing.allocator;
const io = testing.io;
var t = try terminal.Terminal.init(io, alloc, .{ .cols = 5, .rows = 5 });
defer t.deinit(alloc);
t.width_px = 50; // 10 px per col
t.height_px = 100; // 20 px per row
const placement: ImageStorage.Placement = .{
.location = .{ .virtual = {} },
.x_offset = std.math.maxInt(u32),
.y_offset = std.math.maxInt(u32),
.columns = 1,
.rows = 1,
};
const offset = placement.cellOffset(&t);
try testing.expectEqual(@as(u32, 9), offset.x);
try testing.expectEqual(@as(u32, 19), offset.y);
// Even hostile offsets leave one pixel inside the requested cell.
const actual = placement.pixelSize(.{ .width = 1, .height = 1 }, &t);
try testing.expectEqual(@as(u32, 1), actual.width);
try testing.expectEqual(@as(u32, 1), actual.height);
}
test "storage: aspect ratio calculation when only columns or rows specified" {
const testing = std.testing;
const alloc = testing.allocator;
@@ -1898,8 +1984,8 @@ test "storage: placement geometry handles untrusted dimensions" {
try testing.expectEqual(max, actual.height);
}
// Pixel offsets are protocol-controlled too. Include them without
// allowing the grid-size numerator to wrap.
// Pixel offsets are protocol-controlled too. Clamp them to the cell
// before including them in grid geometry.
t.width_px = 2;
t.height_px = 2;
{
@@ -1909,8 +1995,8 @@ test "storage: placement geometry handles untrusted dimensions" {
.y_offset = max,
};
const actual = placement.gridSize(.{ .width = 1, .height = 1 }, &t);
try testing.expectEqual(max, actual.cols);
try testing.expectEqual(max, actual.rows);
try testing.expectEqual(@as(u32, 1), actual.cols);
try testing.expectEqual(@as(u32, 1), actual.rows);
}
const pin = try trackPin(&t, .{ .x = 0, .y = 0 });