mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-08-25 00:21:46 +00:00
terminal/kitty: intersect source rectangles before sizing
Kitty graphics placements previously calculated pixel and grid geometry
from requested source dimensions before intersecting them with the image.
The renderer clamped explicit dimensions later but treated omitted
source dimensions as the full image.
This stretched clipped crops into incorrectly sized destinations and
exposed inconsistent geometry through storage, rendering, and
libghostty.
Resolve the source rectangle once against the image bounds and reuse it
for placement sizing, renderer preparation, and the C API. Add
regression tests for omitted and explicit dimensions and renderer
geometry.
Spec: https://sw.kovidgoyal.net/kitty/graphics-protocol/#controlling-displayed-image-layout
Reference: 0ecb10d158/kitty/graphics.c (L1225-L1240)
This commit is contained in:
@@ -439,17 +439,7 @@ pub const State = struct {
|
||||
// account the rows / columns specified by the placement.
|
||||
const dest_size = p.pixelSize(image.*, t);
|
||||
|
||||
// Calculate the source rectangle
|
||||
const source_x = @min(image.width, p.source_x);
|
||||
const source_y = @min(image.height, p.source_y);
|
||||
const source_width = if (p.source_width > 0)
|
||||
@min(image.width - source_x, p.source_width)
|
||||
else
|
||||
image.width;
|
||||
const source_height = if (p.source_height > 0)
|
||||
@min(image.height - source_y, p.source_height)
|
||||
else
|
||||
image.height;
|
||||
const source = p.sourceRect(image.*);
|
||||
|
||||
// Get the viewport-relative Y position of the placement.
|
||||
const y_pos: i32 = @as(i32, @intCast(img_top_y)) - @as(i32, @intCast(top_y));
|
||||
@@ -465,10 +455,10 @@ pub const State = struct {
|
||||
.height = dest_size.height,
|
||||
.cell_offset_x = p.x_offset,
|
||||
.cell_offset_y = p.y_offset,
|
||||
.source_x = source_x,
|
||||
.source_y = source_y,
|
||||
.source_width = source_width,
|
||||
.source_height = source_height,
|
||||
.source_x = source.x,
|
||||
.source_y = source.y,
|
||||
.source_width = source.width,
|
||||
.source_height = source.height,
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -1036,3 +1026,47 @@ test "kitty renderer ignores pending payloads and removes replaced placements" {
|
||||
try testing.expectEqual(@as(usize, 0), state.kitty_placements.items.len);
|
||||
try testing.expect(state.images.get(.{ .kitty = 1 }).?.image.isUnloading());
|
||||
}
|
||||
|
||||
test "kitty renderer uses the intersected source rectangle" {
|
||||
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);
|
||||
t.width_px = 30;
|
||||
t.height_px = 30;
|
||||
|
||||
var state: State = .empty;
|
||||
defer state.deinit(alloc);
|
||||
|
||||
const storage = &t.screens.active.kitty_images;
|
||||
const pixels = try alloc.alloc(u8, 4 * 3 * 3);
|
||||
@memset(pixels, 0);
|
||||
try storage.addImage(io, alloc, t.screens.active, .{
|
||||
.id = 1,
|
||||
.width = 4,
|
||||
.height = 3,
|
||||
.format = .rgb,
|
||||
.data = .{ .complete = pixels },
|
||||
});
|
||||
const pin = try t.screens.active.pages.trackPin(
|
||||
t.screens.active.cursor.page_pin.*,
|
||||
);
|
||||
try storage.addPlacement(io, alloc, t.screens.active, 1, 1, .{
|
||||
.location = .{ .pin = pin },
|
||||
.source_x = 3,
|
||||
.source_y = 1,
|
||||
});
|
||||
|
||||
state.kittyUpdate(alloc, &t, .{ .width = 10, .height = 10 });
|
||||
try testing.expectEqual(@as(usize, 1), state.kitty_placements.items.len);
|
||||
|
||||
const placement = state.kitty_placements.items[0];
|
||||
try testing.expectEqual(@as(u32, 1), placement.width);
|
||||
try testing.expectEqual(@as(u32, 2), placement.height);
|
||||
try testing.expectEqual(@as(u32, 3), placement.source_x);
|
||||
try testing.expectEqual(@as(u32, 1), placement.source_y);
|
||||
try testing.expectEqual(@as(u32, 1), placement.source_width);
|
||||
try testing.expectEqual(@as(u32, 2), placement.source_height);
|
||||
}
|
||||
|
||||
@@ -516,16 +516,12 @@ pub fn placement_source_rect(
|
||||
const entry = iter.entry orelse return .invalid_value;
|
||||
const p = entry.value_ptr;
|
||||
|
||||
// Apply "0 = full image dimension" convention, then clamp to image bounds.
|
||||
const x = @min(p.source_x, image.width);
|
||||
const y = @min(p.source_y, image.height);
|
||||
const w = @min(if (p.source_width > 0) p.source_width else image.width, image.width - x);
|
||||
const h = @min(if (p.source_height > 0) p.source_height else image.height, image.height - y);
|
||||
const source = p.sourceRect(image.*);
|
||||
|
||||
out_x.* = x;
|
||||
out_y.* = y;
|
||||
out_width.* = w;
|
||||
out_height.* = h;
|
||||
out_x.* = source.x;
|
||||
out_y.* = source.y;
|
||||
out_width.* = source.width;
|
||||
out_height.* = source.height;
|
||||
|
||||
return .success;
|
||||
}
|
||||
@@ -576,12 +572,11 @@ pub fn placement_render_info(
|
||||
out.viewport_row = vp.row;
|
||||
out.viewport_visible = vp.visible;
|
||||
|
||||
const x = @min(p.source_x, image.width);
|
||||
const y = @min(p.source_y, image.height);
|
||||
out.source_x = x;
|
||||
out.source_y = y;
|
||||
out.source_width = @min(if (p.source_width > 0) p.source_width else image.width, image.width - x);
|
||||
out.source_height = @min(if (p.source_height > 0) p.source_height else image.height, image.height - y);
|
||||
const source = p.sourceRect(image.*);
|
||||
out.source_x = source.x;
|
||||
out.source_y = source.y;
|
||||
out.source_width = source.width;
|
||||
out.source_height = source.height;
|
||||
|
||||
return .success;
|
||||
}
|
||||
@@ -1567,6 +1562,15 @@ test "placement_source_rect clamps to image bounds" {
|
||||
try testing.expectEqual(3, y);
|
||||
try testing.expectEqual(1, w);
|
||||
try testing.expectEqual(1, h);
|
||||
|
||||
var pixel_width: u32 = undefined;
|
||||
var pixel_height: u32 = undefined;
|
||||
try testing.expectEqual(
|
||||
Result.success,
|
||||
placement_pixel_size(iter, img, t, &pixel_width, &pixel_height),
|
||||
);
|
||||
try testing.expectEqual(1, pixel_width);
|
||||
try testing.expectEqual(1, pixel_height);
|
||||
}
|
||||
|
||||
test "placement_source_rect null args return invalid_value" {
|
||||
|
||||
@@ -894,6 +894,33 @@ pub const ImageStorage = struct {
|
||||
return std.math.cast(u32, rounded) orelse std.math.maxInt(u32);
|
||||
}
|
||||
|
||||
pub const SourceRect = struct {
|
||||
x: u32,
|
||||
y: u32,
|
||||
width: u32,
|
||||
height: u32,
|
||||
};
|
||||
|
||||
/// Returns the requested source rectangle intersected with the image.
|
||||
/// A zero width or height requests the full corresponding image
|
||||
/// dimension before intersection, as defined by the Kitty protocol.
|
||||
pub fn sourceRect(self: Placement, image: Image) SourceRect {
|
||||
const x = @min(self.source_x, image.width);
|
||||
const y = @min(self.source_y, image.height);
|
||||
return .{
|
||||
.x = x,
|
||||
.y = y,
|
||||
.width = @min(
|
||||
if (self.source_width > 0) self.source_width else image.width,
|
||||
image.width - x,
|
||||
),
|
||||
.height = @min(
|
||||
if (self.source_height > 0) self.source_height else image.height,
|
||||
image.height - y,
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
/// Returns the size of this placement's image in pixels,
|
||||
/// taking into account the source rectangle, specified
|
||||
/// rows/columns, and aspect ratio.
|
||||
@@ -905,9 +932,9 @@ pub const ImageStorage = struct {
|
||||
width: u32,
|
||||
height: u32,
|
||||
} {
|
||||
// Height / width of the image in px.
|
||||
const width = if (self.source_width > 0) self.source_width else image.width;
|
||||
const height = if (self.source_height > 0) self.source_height else image.height;
|
||||
const source = self.sourceRect(image);
|
||||
const width = source.width;
|
||||
const height = source.height;
|
||||
|
||||
// If we don't have any specified cols or rows then the placement
|
||||
// should be the native size of the image, and doesn't need to be
|
||||
@@ -1780,6 +1807,50 @@ test "storage: aspect ratio calculation when only columns or rows specified" {
|
||||
}
|
||||
}
|
||||
|
||||
test "storage: default source rectangle is intersected before sizing" {
|
||||
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);
|
||||
|
||||
const placement: ImageStorage.Placement = .{
|
||||
.location = .{ .virtual = {} },
|
||||
.source_x = 3,
|
||||
.source_y = 1,
|
||||
};
|
||||
const actual = placement.pixelSize(.{ .width = 4, .height = 3 }, &t);
|
||||
try testing.expectEqual(@as(u32, 1), actual.width);
|
||||
try testing.expectEqual(@as(u32, 2), actual.height);
|
||||
}
|
||||
|
||||
test "storage: explicit source rectangle is intersected before sizing" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
const io = testing.io;
|
||||
|
||||
var t = try terminal.Terminal.init(io, alloc, .{ .cols = 10, .rows = 10 });
|
||||
defer t.deinit(alloc);
|
||||
t.width_px = 100;
|
||||
t.height_px = 100;
|
||||
|
||||
// The requested 8x8 rectangle intersects this 10x10 image as 2x3.
|
||||
// With a two-column destination, the clipped 2:3 aspect ratio produces
|
||||
// a 20x30 pixel destination.
|
||||
const placement: ImageStorage.Placement = .{
|
||||
.location = .{ .virtual = {} },
|
||||
.source_x = 8,
|
||||
.source_y = 7,
|
||||
.source_width = 8,
|
||||
.source_height = 8,
|
||||
.columns = 2,
|
||||
};
|
||||
const actual = placement.pixelSize(.{ .width = 10, .height = 10 }, &t);
|
||||
try testing.expectEqual(@as(u32, 20), actual.width);
|
||||
try testing.expectEqual(@as(u32, 30), actual.height);
|
||||
}
|
||||
|
||||
test "storage: placement geometry handles untrusted dimensions" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
Reference in New Issue
Block a user