fix(renderer): Don't force images to grid/cell sizes

This problem was introduced by f091a69 (PR #6675).

I've gone ahead and overhauled the placement positioning logic as well;
it was doing a lot of expensive calls before, I've significantly reduced
that.

Clipping partially off-screen images is now handled entirely by the
renderer, rather than while preparing the placement, and as such the
grid position passed to the image shader is now signed.
pull/7367/head
Qwerasd 2025-05-15 11:37:27 -06:00
parent 55c1ef779f
commit 709b0214a0
6 changed files with 174 additions and 177 deletions

View File

@ -1872,6 +1872,8 @@ fn prepKittyGraphics(
// points. This lets us determine offsets and containment of placements.
const top = t.screen.pages.getTopLeft(.viewport);
const bot = t.screen.pages.getBottomRight(.viewport).?;
const top_y = t.screen.pages.pointFromPin(.screen, top).?.screen.y;
const bot_y = t.screen.pages.pointFromPin(.screen, bot).?.screen.y;
// Go through the placements and ensure the image is loaded on the GPU.
var it = storage.placements.iterator();
@ -1903,7 +1905,7 @@ fn prepKittyGraphics(
continue;
};
try self.prepKittyPlacement(t, &top, &bot, &image, p);
try self.prepKittyPlacement(t, top_y, bot_y, &image, p);
}
// If we have virtual placements then we need to scan for placeholders.
@ -2009,8 +2011,8 @@ fn prepKittyVirtualPlacement(
fn prepKittyPlacement(
self: *Metal,
t: *terminal.Terminal,
top: *const terminal.Pin,
bot: *const terminal.Pin,
top_y: u32,
bot_y: u32,
image: *const terminal.kitty.graphics.Image,
p: *const terminal.kitty.graphics.ImageStorage.Placement,
) !void {
@ -2018,78 +2020,47 @@ fn prepKittyPlacement(
// a rect then its virtual or something so skip it.
const rect = p.rect(image.*, t) orelse return;
// This is expensive but necessary.
const img_top_y = t.screen.pages.pointFromPin(.screen, rect.top_left).?.screen.y;
const img_bot_y = t.screen.pages.pointFromPin(.screen, rect.bottom_right).?.screen.y;
// If the selection isn't within our viewport then skip it.
if (bot.before(rect.top_left)) return;
if (rect.bottom_right.before(top.*)) return;
// If the top left is outside the viewport we need to calc an offset
// so that we render (0, 0) with some offset for the texture.
const offset_y: u32 = if (rect.top_left.before(top.*)) offset_y: {
const vp_y = t.screen.pages.pointFromPin(.screen, top.*).?.screen.y;
const img_y = t.screen.pages.pointFromPin(.screen, rect.top_left).?.screen.y;
const offset_cells = vp_y - img_y;
const offset_pixels = offset_cells * self.grid_metrics.cell_height;
break :offset_y @intCast(offset_pixels);
} else 0;
// Get the grid size that respects aspect ratio
const grid_size = p.gridSize(image.*, t);
// If we specify `rows` then our offset above is in viewport space
// and not in the coordinate space of the source image. Without `rows`
// that's one and the same.
const source_offset_y: u32 = if (grid_size.rows > 0) source_offset_y: {
// Determine the scale factor to apply for this row height.
const image_height: f64 = @floatFromInt(image.height);
const viewport_height: f64 = @floatFromInt(grid_size.rows * self.grid_metrics.cell_height);
const scale: f64 = image_height / viewport_height;
// Apply the scale to the offset
const offset_y_f64: f64 = @floatFromInt(offset_y);
const source_offset_y_f64: f64 = offset_y_f64 * scale;
break :source_offset_y @intFromFloat(@round(source_offset_y_f64));
} else offset_y;
if (img_top_y > bot_y) return;
if (img_bot_y < top_y) return;
// We need to prep this image for upload if it isn't in the cache OR
// it is in the cache but the transmit time doesn't match meaning this
// image is different.
try self.prepKittyImage(image);
// Convert our screen point to a viewport point
const viewport: terminal.point.Point = t.screen.pages.pointFromPin(
.viewport,
rect.top_left,
) orelse .{ .viewport = .{} };
// Calculate the dimensions of our image, taking in to
// account the rows / columns specified by the placement.
const dest_size = p.calculatedSize(image.*, t);
// Calculate the source rectangle
const source_x = @min(image.width, p.source_x);
const source_y = @min(image.height, p.source_y + source_offset_y);
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, p.source_height)
@min(image.height - source_y, p.source_height)
else
image.height -| source_y;
image.height;
// Calculate the width/height of our image.
const dest_width = grid_size.cols * self.grid_metrics.cell_width;
const dest_height = if (grid_size.rows > 0) rows: {
// Clip to the viewport to handle scrolling. offset_y is already in
// viewport scale so we can subtract it directly.
break :rows (grid_size.rows * self.grid_metrics.cell_height) - offset_y;
} else source_height;
// Get the viewport-relative Y position of the placement.
const y_pos: i32 = @as(i32, @intCast(img_top_y)) - @as(i32, @intCast(top_y));
// Accumulate the placement
if (image.width > 0 and image.height > 0) {
if (dest_size.width > 0 and dest_size.height > 0) {
try self.image_placements.append(self.alloc, .{
.image_id = image.id,
.x = @intCast(rect.top_left.x),
.y = @intCast(viewport.viewport.y),
.y = y_pos,
.z = p.z,
.width = dest_width,
.height = dest_height,
.width = dest_size.width,
.height = dest_size.height,
.cell_offset_x = p.x_offset,
.cell_offset_y = p.y_offset,
.source_x = source_x,

View File

@ -913,6 +913,8 @@ fn prepKittyGraphics(
// points. This lets us determine offsets and containment of placements.
const top = t.screen.pages.getTopLeft(.viewport);
const bot = t.screen.pages.getBottomRight(.viewport).?;
const top_y = t.screen.pages.pointFromPin(.screen, top).?.screen.y;
const bot_y = t.screen.pages.pointFromPin(.screen, bot).?.screen.y;
// Go through the placements and ensure the image is loaded on the GPU.
var it = storage.placements.iterator();
@ -944,7 +946,7 @@ fn prepKittyGraphics(
continue;
};
try self.prepKittyPlacement(t, &top, &bot, &image, p);
try self.prepKittyPlacement(t, top_y, bot_y, &image, p);
}
// If we have virtual placements then we need to scan for placeholders.
@ -1050,8 +1052,8 @@ fn prepKittyVirtualPlacement(
fn prepKittyPlacement(
self: *OpenGL,
t: *terminal.Terminal,
top: *const terminal.Pin,
bot: *const terminal.Pin,
top_y: u32,
bot_y: u32,
image: *const terminal.kitty.graphics.Image,
p: *const terminal.kitty.graphics.ImageStorage.Placement,
) !void {
@ -1059,78 +1061,47 @@ fn prepKittyPlacement(
// a rect then its virtual or something so skip it.
const rect = p.rect(image.*, t) orelse return;
// This is expensive but necessary.
const img_top_y = t.screen.pages.pointFromPin(.screen, rect.top_left).?.screen.y;
const img_bot_y = t.screen.pages.pointFromPin(.screen, rect.bottom_right).?.screen.y;
// If the selection isn't within our viewport then skip it.
if (bot.before(rect.top_left)) return;
if (rect.bottom_right.before(top.*)) return;
// If the top left is outside the viewport we need to calc an offset
// so that we render (0, 0) with some offset for the texture.
const offset_y: u32 = if (rect.top_left.before(top.*)) offset_y: {
const vp_y = t.screen.pages.pointFromPin(.screen, top.*).?.screen.y;
const img_y = t.screen.pages.pointFromPin(.screen, rect.top_left).?.screen.y;
const offset_cells = vp_y - img_y;
const offset_pixels = offset_cells * self.grid_metrics.cell_height;
break :offset_y @intCast(offset_pixels);
} else 0;
// Get the grid size that respects aspect ratio
const grid_size = p.gridSize(image.*, t);
// If we specify `rows` then our offset above is in viewport space
// and not in the coordinate space of the source image. Without `rows`
// that's one and the same.
const source_offset_y: u32 = if (grid_size.rows > 0) source_offset_y: {
// Determine the scale factor to apply for this row height.
const image_height: f64 = @floatFromInt(image.height);
const viewport_height: f64 = @floatFromInt(grid_size.rows * self.grid_metrics.cell_height);
const scale: f64 = image_height / viewport_height;
// Apply the scale to the offset
const offset_y_f64: f64 = @floatFromInt(offset_y);
const source_offset_y_f64: f64 = offset_y_f64 * scale;
break :source_offset_y @intFromFloat(@round(source_offset_y_f64));
} else offset_y;
if (img_top_y > bot_y) return;
if (img_bot_y < top_y) return;
// We need to prep this image for upload if it isn't in the cache OR
// it is in the cache but the transmit time doesn't match meaning this
// image is different.
try self.prepKittyImage(image);
// Convert our screen point to a viewport point
const viewport: terminal.point.Point = t.screen.pages.pointFromPin(
.viewport,
rect.top_left,
) orelse .{ .viewport = .{} };
// Calculate the dimensions of our image, taking in to
// account the rows / columns specified by the placement.
const dest_size = p.calculatedSize(image.*, t);
// Calculate the source rectangle
const source_x = @min(image.width, p.source_x);
const source_y = @min(image.height, p.source_y + source_offset_y);
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, p.source_height)
@min(image.height - source_y, p.source_height)
else
image.height -| source_y;
image.height;
// Calculate the width/height of our image.
const dest_width = grid_size.cols * self.grid_metrics.cell_width;
const dest_height = if (grid_size.rows > 0) rows: {
// Clip to the viewport to handle scrolling. offset_y is already in
// viewport scale so we can subtract it directly.
break :rows (grid_size.rows * self.grid_metrics.cell_height) - offset_y;
} else source_height;
// Get the viewport-relative Y position of the placement.
const y_pos: i32 = @as(i32, @intCast(img_top_y)) - @as(i32, @intCast(top_y));
// Accumulate the placement
if (image.width > 0 and image.height > 0) {
if (dest_size.width > 0 and dest_size.height > 0) {
try self.image_placements.append(self.alloc, .{
.image_id = image.id,
.x = @intCast(rect.top_left.x),
.y = @intCast(viewport.viewport.y),
.y = y_pos,
.z = p.z,
.width = dest_width,
.height = dest_height,
.width = dest_size.width,
.height = dest_size.height,
.cell_offset_x = p.x_offset,
.cell_offset_y = p.y_offset,
.source_x = source_x,
@ -2511,8 +2482,8 @@ fn drawImages(
// Setup our data
try bind.vbo.setData(ImageProgram.Input{
.grid_col = @intCast(p.x),
.grid_row = @intCast(p.y),
.grid_col = p.x,
.grid_row = p.y,
.cell_offset_x = p.cell_offset_x,
.cell_offset_y = p.cell_offset_y,
.source_x = p.source_x,

View File

@ -13,16 +13,16 @@ pub const Placement = struct {
image_id: u32,
/// The grid x/y where this placement is located.
x: u32,
y: u32,
x: i32,
y: i32,
z: i32,
/// The width/height of the placed image.
width: u32,
height: u32,
/// The offset in pixels from the top left of the cell. This is
/// clamped to the size of a cell.
/// The offset in pixels from the top left of the cell.
/// This is clamped to the size of a cell.
cell_offset_x: u32,
cell_offset_y: u32,

View File

@ -11,8 +11,8 @@ vbo: gl.Buffer,
pub const Input = extern struct {
/// vec2 grid_coord
grid_col: u16,
grid_row: u16,
grid_col: i32,
grid_row: i32,
/// vec2 cell_offset
cell_offset_x: u32 = 0,
@ -66,8 +66,8 @@ pub fn init() !ImageProgram {
var vbobind = try vbo.bind(.array);
defer vbobind.unbind();
var offset: usize = 0;
try vbobind.attributeAdvanced(0, 2, gl.c.GL_UNSIGNED_SHORT, false, @sizeOf(Input), offset);
offset += 2 * @sizeOf(u16);
try vbobind.attributeAdvanced(0, 2, gl.c.GL_INT, false, @sizeOf(Input), offset);
offset += 2 * @sizeOf(i32);
try vbobind.attributeAdvanced(1, 2, gl.c.GL_UNSIGNED_INT, false, @sizeOf(Input), offset);
offset += 2 * @sizeOf(u32);
try vbobind.attributeAdvanced(2, 4, gl.c.GL_UNSIGNED_INT, false, @sizeOf(Input), offset);

View File

@ -11,8 +11,8 @@ pub const Placement = struct {
image_id: u32,
/// The grid x/y where this placement is located.
x: u32,
y: u32,
x: i32,
y: i32,
z: i32,
/// The width/height of the placed image.

View File

@ -658,6 +658,86 @@ pub const ImageStorage = struct {
}
}
/// Calculates the size of this placement's image in pixels,
/// taking in to account the specified rows and columns.
pub fn calculatedSize(
self: Placement,
image: Image,
t: *const terminal.Terminal,
) 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;
// 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
// re-scaled.
if (self.columns == 0 and self.rows == 0) return .{
.width = width,
.height = height,
};
// We calculate the size of a cell so that we can multiply
// it by the specified cols/rows to get the correct px size.
//
// We assume that the width is divided evenly by the column
// 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 width_f64: f64 = @floatFromInt(width);
const height_f64: f64 = @floatFromInt(height);
// If we have a specified cols AND rows then we calculate
// the width and height from them directly, we don't need
// to adjust for aspect ratio.
if (self.columns > 0 and self.rows > 0) {
const calc_width = cell_width * self.columns;
const calc_height = cell_height * self.rows;
return .{
.width = calc_width,
.height = calc_height,
};
}
// Either the columns or the rows were specified, but not both,
// so we need to calculate the other one based on the aspect ratio.
// If only the columns were specified, we determine
// the height of the image based on the aspect ratio.
if (self.columns > 0) {
const aspect = height_f64 / width_f64;
const calc_width: u32 = cell_width * self.columns;
const calc_height: u32 = @intFromFloat(@round(
@as(f64, @floatFromInt(calc_width)) * aspect,
));
return .{
.width = calc_width,
.height = calc_height,
};
}
// Otherwise, only the rows were specified, so we
// determine the width based on the aspect ratio.
{
const aspect = width_f64 / height_f64;
const calc_height: u32 = cell_height * self.rows;
const calc_width: u32 = @intFromFloat(@round(
@as(f64, @floatFromInt(calc_height)) * aspect,
));
return .{
.width = calc_width,
.height = calc_height,
};
}
}
/// Returns the size in grid cells that this placement takes up.
pub fn gridSize(
self: Placement,
@ -667,60 +747,29 @@ pub const ImageStorage = struct {
cols: u32,
rows: u32,
} {
// If we have a specified columns and rows then this is trivial.
if (self.columns > 0 and self.rows > 0) return .{
.cols = self.columns,
.rows = self.rows,
};
// Calculate our cell size.
const terminal_width_f64: f64 = @floatFromInt(t.width_px);
const terminal_height_f64: f64 = @floatFromInt(t.height_px);
const grid_columns_f64: f64 = @floatFromInt(t.cols);
const grid_rows_f64: f64 = @floatFromInt(t.rows);
const cell_width_f64 = terminal_width_f64 / grid_columns_f64;
const cell_height_f64 = terminal_height_f64 / grid_rows_f64;
// Our image width
const width_px = if (self.source_width > 0) self.source_width else image.width;
const height_px = if (self.source_height > 0) self.source_height else image.height;
// Calculate our image size in grid cells
const width_f64: f64 = @floatFromInt(width_px);
const height_f64: f64 = @floatFromInt(height_px);
// If only columns is specified, calculate rows based on aspect ratio
if (self.columns > 0 and self.rows == 0) {
const cols_f64: f64 = @floatFromInt(self.columns);
const cols_px = cols_f64 * cell_width_f64;
const aspect_ratio = height_f64 / width_f64;
const rows_px = cols_px * aspect_ratio;
const rows_cells = rows_px / cell_height_f64;
return .{
.cols = self.columns,
.rows = @intFromFloat(@ceil(rows_cells)),
};
}
// If only rows is specified, calculate columns based on aspect ratio
if (self.rows > 0 and self.columns == 0) {
const rows_f64: f64 = @floatFromInt(self.rows);
const rows_px = rows_f64 * cell_height_f64;
const aspect_ratio = width_f64 / height_f64;
const cols_px = rows_px * aspect_ratio;
const cols_cells = cols_px / cell_width_f64;
return .{
.cols = @intFromFloat(@ceil(cols_cells)),
.rows = self.rows,
};
}
const width_cells: u32 = @intFromFloat(@ceil(width_f64 / cell_width_f64));
const height_cells: u32 = @intFromFloat(@ceil(height_f64 / cell_height_f64));
// Otherwise we calculate the pixel size, divide by
// cell size, and round up to the nearest integer.
const calc_size = self.calculatedSize(image, t);
return .{
.cols = width_cells,
.rows = height_cells,
.cols = std.math.divCeil(
u32,
calc_size.width + self.x_offset,
t.width_px / t.cols,
) catch 0,
.rows = std.math.divCeil(
u32,
calc_size.height + self.y_offset,
t.height_px / t.rows,
) catch 0,
};
// NOTE: Above `divCeil`s can only fail if the cell size is 0,
// in such a case it seems safe to return 0 for this.
}
/// Returns a selection of the entire rectangle this placement
@ -1269,36 +1318,42 @@ test "storage: aspect ratio calculation when only columns or rows specified" {
var t = try terminal.Terminal.init(alloc, .{ .cols = 100, .rows = 100 });
defer t.deinit(alloc);
t.width_px = 100;
t.height_px = 100;
t.width_px = 1000; // 10 px per col
t.height_px = 2000; // 20 px per row
// Case 1: Only columns specified
{
const image = Image{ .id = 1, .width = 4, .height = 2 };
const image = Image{ .id = 1, .width = 16, .height = 9 };
var placement = ImageStorage.Placement{
.location = .{ .virtual = {} },
.columns = 6,
.columns = 10,
.rows = 0,
};
const grid_size = placement.gridSize(image, &t);
// 6 columns * (2/4) = 3 rows
try testing.expectEqual(@as(u32, 6), grid_size.cols);
try testing.expectEqual(@as(u32, 3), grid_size.rows);
// Image is 16x9, set to a width of 10 columns, at 10px per column
// that's 100px width. 100px * (9 / 16) = 56.25, which sould round
// to a height of 56px.
const calc_size = placement.calculatedSize(image, &t);
try testing.expectEqual(@as(u32, 100), calc_size.width);
try testing.expectEqual(@as(u32, 56), calc_size.height);
}
// Case 2: Only rows specified
{
const image = Image{ .id = 2, .width = 2, .height = 4 };
const image = Image{ .id = 2, .width = 16, .height = 9 };
var placement = ImageStorage.Placement{
.location = .{ .virtual = {} },
.columns = 0,
.rows = 6,
.rows = 5,
};
const grid_size = placement.gridSize(image, &t);
// 6 rows * (2/4) = 3 columns
try testing.expectEqual(@as(u32, 3), grid_size.cols);
try testing.expectEqual(@as(u32, 6), grid_size.rows);
// Image is 16x9, set to a height of 5 rows, at 20px per row that's
// 100px height. 100px * (16 / 9) = 177.77..., which should round to
// a width of 178px.
const calc_size = placement.calculatedSize(image, &t);
try testing.expectEqual(@as(u32, 178), calc_size.width);
try testing.expectEqual(@as(u32, 100), calc_size.height);
}
}