From 7afbc7ee14102206dfc342ec28779bf070a60eed Mon Sep 17 00:00:00 2001 From: Tim Culverhouse Date: Fri, 10 Jul 2026 11:04:53 -0500 Subject: [PATCH 1/3] terminal: make Kitty images retainable Asynchronous image consumers cannot safely keep borrowed Kitty image data after terminal storage is unlocked. Replacement, deletion, eviction, or shutdown may otherwise free the pixels. Make Image itself the immutable, atomically reference-counted owner of its metadata and pixels. Store image pointers directly and release the storage reference on every removal path, while retained references remain valid independently. Reserve fallible insertion resources before eviction so failed additions preserve caller and storage ownership. Keep ordinary lookups and the existing C image lookup borrowed in this commit. --- src/terminal/c/kitty_graphics.zig | 14 +- src/terminal/kitty/graphics_exec.zig | 11 +- src/terminal/kitty/graphics_image.zig | 125 +++++-- src/terminal/kitty/graphics_storage.zig | 449 ++++++++++++++++++------ src/terminal/kitty/graphics_unicode.zig | 19 +- 5 files changed, 473 insertions(+), 145 deletions(-) diff --git a/src/terminal/c/kitty_graphics.zig b/src/terminal/c/kitty_graphics.zig index 13a675ccc46..aa7aff5daf2 100644 --- a/src/terminal/c/kitty_graphics.zig +++ b/src/terminal/c/kitty_graphics.zig @@ -203,7 +203,7 @@ pub fn image_get_handle( if (comptime !build_options.kitty_graphics) return null; const storage = graphics_; - return storage.images.getPtr(image_id); + return storage.imageById(image_id); } pub fn image_get( @@ -423,7 +423,7 @@ pub fn placement_rect( const iter = iter_ orelse return .invalid_value; const entry = iter.entry orelse return .invalid_value; const r = entry.value_ptr.rect( - image.*, + image, wrapper.terminal, ) orelse return .no_value; @@ -449,7 +449,7 @@ pub fn placement_pixel_size( const image = image_ orelse return .invalid_value; const iter = iter_ orelse return .invalid_value; const entry = iter.entry orelse return .invalid_value; - const s = entry.value_ptr.pixelSize(image.*, wrapper.terminal); + const s = entry.value_ptr.pixelSize(image, wrapper.terminal); out_width.* = s.width; out_height.* = s.height; @@ -470,7 +470,7 @@ pub fn placement_grid_size( const image = image_ orelse return .invalid_value; const iter = iter_ orelse return .invalid_value; const entry = iter.entry orelse return .invalid_value; - const s = entry.value_ptr.gridSize(image.*, wrapper.terminal); + const s = entry.value_ptr.gridSize(image, wrapper.terminal); out_cols.* = s.cols; out_rows.* = s.rows; @@ -563,11 +563,11 @@ pub fn placement_render_info( const p = entry.value_ptr; - const ps = p.pixelSize(image.*, wrapper.terminal); + const ps = p.pixelSize(image, wrapper.terminal); out.pixel_width = ps.width; out.pixel_height = ps.height; - const gs = p.gridSize(image.*, wrapper.terminal); + const gs = p.gridSize(image, wrapper.terminal); out.grid_cols = gs.cols; out.grid_rows = gs.rows; @@ -630,7 +630,7 @@ fn computeViewportPos( // A placement is invisible if its bottom edge (row + height) // is above the viewport, or its top edge is at or below the // viewport's last row. - const grid_size = p.gridSize(image.*, t); + const grid_size = p.gridSize(image, t); const rows_i32: i32 = @intCast(grid_size.rows); const term_rows: i32 = @intCast(t.rows); const visible = vp_row + rows_i32 > 0 and vp_row < term_rows; diff --git a/src/terminal/kitty/graphics_exec.zig b/src/terminal/kitty/graphics_exec.zig index 6c72ec4e9ae..b1af6009249 100644 --- a/src/terminal/kitty/graphics_exec.zig +++ b/src/terminal/kitty/graphics_exec.zig @@ -149,7 +149,6 @@ fn transmit( encodeError(&result, err); return result; }; - errdefer load.image.deinit(alloc); // If we're also displaying, then do that now. This function does // both transmit and transmit and display. The display might also be @@ -200,7 +199,7 @@ fn display( // Verify the requested image exists if we have an ID const storage = &terminal.screens.active.kitty_images; - const img_: ?Image = if (d.image_id != 0) + const img_: ?*const Image = if (d.image_id != 0) storage.imageById(d.image_id) else storage.imageByNumber(d.image_number); @@ -302,7 +301,7 @@ fn loadAndAddImage( terminal: *Terminal, cmd: *const Command, ) !struct { - image: Image, + image: Image.Metadata, more: bool = false, display: ?command.Display = null, } { @@ -360,8 +359,8 @@ fn loadAndAddImage( // loading.debugDump() catch unreachable; // Validate and store our image - var img = try loading.complete(alloc); - errdefer img.deinit(alloc); + const img = try loading.complete(alloc); + errdefer img.release(); try storage.addImage(alloc, img); // Get our display settings @@ -371,7 +370,7 @@ fn loadAndAddImage( // won't be deinit because of "complete" above. loading.deinit(alloc); - return .{ .image = img, .display = display_ }; + return .{ .image = img.withoutData(), .display = display_ }; } const EncodeableError = Image.Error || Allocator.Error; diff --git a/src/terminal/kitty/graphics_image.zig b/src/terminal/kitty/graphics_image.zig index b6f44fac20b..0dcebc17340 100644 --- a/src/terminal/kitty/graphics_image.zig +++ b/src/terminal/kitty/graphics_image.zig @@ -31,7 +31,7 @@ const max_size = 400 * 1024 * 1024; // 400MB pub const LoadingImage = struct { /// The in-progress image. The first chunk must have all the metadata /// so this comes from that initially. - image: Image, + image: Image.Metadata, /// The data that is being built up. data: std.ArrayListUnmanaged(u8) = .{}, @@ -345,7 +345,6 @@ pub const LoadingImage = struct { } pub fn deinit(self: *LoadingImage, alloc: Allocator) void { - self.image.deinit(alloc); self.data.deinit(alloc); } @@ -376,7 +375,7 @@ pub const LoadingImage = struct { } /// Complete the chunked image, returning a completed image. - pub fn complete(self: *LoadingImage, alloc: Allocator) !Image { + pub fn complete(self: *LoadingImage, alloc: Allocator) !*Image { const img = &self.image; // Decompress the data if it is compressed. @@ -402,9 +401,9 @@ pub const LoadingImage = struct { } // Everything looks good, copy the image data over. - var result = self.image; - result.data = try self.data.toOwnedSlice(alloc); - errdefer result.deinit(alloc); + const data = try self.data.toOwnedSlice(alloc); + errdefer alloc.free(data); + const result = try Image.create(alloc, .fromMetadata(self.image, data)); self.image = .{}; return result; } @@ -504,14 +503,23 @@ pub const LoadingImage = struct { /// is completed, so `compression` is always `.none` and `format` is /// never `.png` for a stored image, and `data.len` always equals /// `width * height * bytes-per-pixel`. +/// +/// Images are pointer-owned and reference counted. They are immutable after +/// publication, except that ImageStorage stamps `generation` immediately +/// before publishing an image. The allocator interface is copied into the +/// image; any backing state used by that allocator must outlive every image +/// reference. References may be retained and released on different threads. pub const Image = struct { + refs: std.atomic.Value(usize) = .init(1), + allocator: Allocator, + id: u32 = 0, number: u32 = 0, width: u32 = 0, height: u32 = 0, format: command.Transmission.Format = .rgb, compression: command.Transmission.Compression = .none, - data: []const u8 = "", + data: []const u8, /// Unique, monotonically increasing stamp assigned each time an /// image is added to (or replaced in) an ImageStorage. A changed @@ -527,6 +535,43 @@ pub const Image = struct { /// IDs in the public range (which is bad!). implicit_id: bool = false, + pub const Metadata = struct { + id: u32 = 0, + number: u32 = 0, + width: u32 = 0, + height: u32 = 0, + format: command.Transmission.Format = .rgb, + compression: command.Transmission.Compression = .none, + generation: u64 = 0, + implicit_id: bool = false, + }; + + pub const Init = struct { + id: u32 = 0, + number: u32 = 0, + width: u32 = 0, + height: u32 = 0, + format: command.Transmission.Format = .rgb, + compression: command.Transmission.Compression = .none, + data: []const u8 = "", + generation: u64 = 0, + implicit_id: bool = false, + + fn fromMetadata(metadata: Metadata, data: []const u8) Init { + return .{ + .id = metadata.id, + .number = metadata.number, + .width = metadata.width, + .height = metadata.height, + .format = metadata.format, + .compression = metadata.compression, + .data = data, + .generation = metadata.generation, + .implicit_id = metadata.implicit_id, + }; + } + }; + pub const Error = error{ InvalidData, DecompressionFailed, @@ -540,15 +585,55 @@ pub const Image = struct { UnsupportedDepth, }; - pub fn deinit(self: *Image, alloc: Allocator) void { - if (self.data.len > 0) alloc.free(self.data); + /// Create an image that adopts `init.data`. On success the caller owns + /// one reference; on failure ownership of the data remains with the caller. + pub fn create(alloc: Allocator, init: Init) Allocator.Error!*Image { + const result = try alloc.create(Image); + result.* = .{ + .allocator = alloc, + .id = init.id, + .number = init.number, + .width = init.width, + .height = init.height, + .format = init.format, + .compression = init.compression, + .data = init.data, + .generation = init.generation, + .implicit_id = init.implicit_id, + }; + return result; + } + + pub fn retain(self: *const Image) *Image { + const result = @constCast(self); + const previous = result.refs.fetchAdd(1, .monotonic); + assert(previous > 0); + return result; + } + + pub fn release(self: *Image) void { + const previous = self.refs.fetchSub(1, .release); + assert(previous > 0); + if (previous == 1) { + _ = self.refs.load(.acquire); + const alloc = self.allocator; + if (self.data.len > 0) alloc.free(self.data); + alloc.destroy(self); + } } /// Mostly for logging - pub fn withoutData(self: *const Image) Image { - var copy = self.*; - copy.data = ""; - return copy; + pub fn withoutData(self: *const Image) Metadata { + return .{ + .id = self.id, + .number = self.number, + .width = self.width, + .height = self.height, + .format = self.format, + .compression = self.compression, + .generation = self.generation, + .implicit_id = self.implicit_id, + }; } }; @@ -641,7 +726,7 @@ test "image load: rgb, zlib compressed, direct" { var loading = try LoadingImage.init(alloc, &cmd, .direct); defer loading.deinit(alloc); var img = try loading.complete(alloc); - defer img.deinit(alloc); + defer img.release(); // should be decompressed try testing.expect(img.compression == .none); @@ -669,7 +754,7 @@ test "image load: rgb, not compressed, direct" { var loading = try LoadingImage.init(alloc, &cmd, .direct); defer loading.deinit(alloc); var img = try loading.complete(alloc); - defer img.deinit(alloc); + defer img.release(); // should be decompressed try testing.expect(img.compression == .none); @@ -708,7 +793,7 @@ test "image load: rgb, zlib compressed, direct, chunked" { // Complete var img = try loading.complete(alloc); - defer img.deinit(alloc); + defer img.release(); try testing.expect(img.compression == .none); } @@ -744,7 +829,7 @@ test "image load: rgb, zlib compressed, direct, chunked with zero initial chunk" // Complete var img = try loading.complete(alloc); - defer img.deinit(alloc); + defer img.release(); try testing.expect(img.compression == .none); } @@ -811,7 +896,7 @@ test "image load: rgb, not compressed, temporary file" { var loading = try LoadingImage.init(alloc, &cmd, .all); defer loading.deinit(alloc); var img = try loading.complete(alloc); - defer img.deinit(alloc); + defer img.release(); try testing.expect(img.compression == .none); // Temporary file should be gone @@ -848,7 +933,7 @@ test "image load: rgb, not compressed, regular file" { var loading = try LoadingImage.init(alloc, &cmd, .all); defer loading.deinit(alloc); var img = try loading.complete(alloc); - defer img.deinit(alloc); + defer img.release(); try testing.expect(img.compression == .none); try tmp_dir.dir.access(path, .{}); } @@ -885,7 +970,7 @@ test "image load: png, not compressed, regular file" { var loading = try LoadingImage.init(alloc, &cmd, .all); defer loading.deinit(alloc); var img = try loading.complete(alloc); - defer img.deinit(alloc); + defer img.release(); try testing.expect(img.compression == .none); try testing.expect(img.format == .rgba); try tmp_dir.dir.access(path, .{}); diff --git a/src/terminal/kitty/graphics_storage.zig b/src/terminal/kitty/graphics_storage.zig index ba82ea4024b..f90e4c548d6 100644 --- a/src/terminal/kitty/graphics_storage.zig +++ b/src/terminal/kitty/graphics_storage.zig @@ -67,7 +67,7 @@ const GenerationCounter = if (@bitSizeOf(usize) >= 64) struct { /// screen, alt screen) and contains all the transmitted images and /// placements. pub const ImageStorage = struct { - const ImageMap = std.AutoHashMapUnmanaged(u32, Image); + const ImageMap = std.AutoHashMapUnmanaged(u32, *Image); const PlacementMap = std.AutoHashMapUnmanaged(PlacementKey, Placement); /// Dirty is set to true if placements or images change. This is @@ -125,9 +125,11 @@ pub const ImageStorage = struct { /// The limits of what medium types are allowed for image loading. image_limits: LoadingImage.Limits = .direct, - /// The total bytes of image data that have been loaded and the limit. - /// If the limit is reached, the oldest images will be evicted to make - /// space. Unused images take priority. + /// The total bytes of image data currently owned by this storage and the + /// limit. If the limit is reached, the oldest images will be evicted to + /// make space. Unused images take priority. Externally retained images + /// no longer count after eviction, so actual process memory may + /// temporarily exceed this limit. total_bytes: usize = 0, total_limit: usize = 320 * 1000 * 1000, // 320MB @@ -139,7 +141,7 @@ pub const ImageStorage = struct { if (self.loading) |loading| loading.destroy(alloc); var it = self.images.iterator(); - while (it.next()) |kv| kv.value_ptr.deinit(alloc); + while (it.next()) |kv| kv.value_ptr.*.release(); self.images.deinit(alloc); self.clearPlacements(s); @@ -186,7 +188,7 @@ pub const ImageStorage = struct { if (limit < self.total_bytes) { const req_bytes = self.total_bytes - limit; log.info("evicting images to lower limit, evicting={}", .{req_bytes}); - if (!try self.evictImage(alloc, req_bytes)) { + if (!try self.evictImage(alloc, req_bytes, null)) { log.warn("failed to evict enough images for required bytes", .{}); } } @@ -194,47 +196,59 @@ pub const ImageStorage = struct { self.total_limit = limit; } - /// Add an already-loaded image to the storage. This will automatically - /// free any existing image with the same ID. - pub fn addImage(self: *ImageStorage, alloc: Allocator, img: Image) Allocator.Error!void { + /// Add an owned image reference to storage. Success transfers the caller's + /// reference to storage; failure leaves the caller's reference untouched. + /// Any existing image with the same ID is released on success. + pub fn addImage(self: *ImageStorage, alloc: Allocator, img: *Image) Allocator.Error!void { // If the image itself is over the limit, then error immediately if (img.data.len > self.total_limit) return error.OutOfMemory; - // If this would put us over the limit, then evict. - const total_bytes = self.total_bytes + img.data.len; + // Replacements only require enough quota for the difference between + // the old and new payloads. + const replacing = self.images.contains(img.id); + const replaced_len = if (self.images.get(img.id)) |stored| + stored.data.len + else + 0; + const total_bytes = self.total_bytes - replaced_len + img.data.len; + + // Reserve the map slot before eviction. After this succeeds, insertion + // cannot fail, so an allocation failure never + // leaves an evicted image or a half-initialized map entry behind. + if (!replacing) { + try self.images.ensureUnusedCapacity(alloc, 1); + } + + // If this would put us over the limit, then evict. evictImage performs + // all of its fallible allocation before mutating storage. if (total_bytes > self.total_limit) { const req_bytes = total_bytes - self.total_limit; log.info("evicting images to make space for {} bytes", .{req_bytes}); - if (!try self.evictImage(alloc, req_bytes)) { + // The replaced payload was already removed from total_bytes above, + // so it must not also count as bytes freed by eviction. Excluding + // it also preserves placements across same-ID replacement. + const exclude_id: ?u32 = if (replacing) img.id else null; + if (!try self.evictImage(alloc, req_bytes, exclude_id)) { log.warn("failed to evict enough images for required bytes", .{}); return error.OutOfMemory; } } - // Do the gop op first so if it fails we don't get a partial state - const gop = try self.images.getOrPut(alloc, img.id); + const gop = self.images.getOrPutAssumeCapacity(img.id); - log.debug("addImage image={}", .{img: { - var copy = img; - copy.data = ""; - break :img copy; - }}); + log.debug("addImage image={}", .{img.withoutData()}); // Write our new image if (gop.found_existing) { - self.total_bytes -= gop.value_ptr.data.len; - gop.value_ptr.deinit(alloc); + self.total_bytes -= gop.value_ptr.*.data.len; + gop.value_ptr.*.release(); } + // Stamp before publishing the pointer in the map. + self.markMutated(); + img.generation = self.generation; gop.value_ptr.* = img; self.total_bytes += img.data.len; - - // Stamp the stored image with a fresh generation. This gives - // every add/replace a unique stamp even when the same image ID - // is retransmitted with identical dimensions, so consumers - // (e.g. renderer texture caches) can detect content changes. - self.markMutated(); - gop.value_ptr.generation = self.generation; } /// Add a placement for a given image. The caller must verify in advance @@ -284,22 +298,26 @@ pub const ImageStorage = struct { self.placements.clearRetainingCapacity(); } - /// Get an image by its ID. If the image doesn't exist, null is returned. - pub fn imageById(self: *const ImageStorage, image_id: u32) ?Image { + /// Get a borrowed image by its ID. The returned data is only valid while + /// storage access is externally synchronized and the image remains in + /// this storage. This does not retain the image. + pub fn imageById(self: *const ImageStorage, image_id: u32) ?*const Image { return self.images.get(image_id); } - /// Get an image by its number. If the image doesn't exist, return null. - pub fn imageByNumber(self: *const ImageStorage, image_number: u32) ?Image { - var newest: ?Image = null; + /// Get the newest borrowed image by its number. This does not retain the + /// image; it has the same lifetime restrictions as imageById. + pub fn imageByNumber(self: *const ImageStorage, image_number: u32) ?*const Image { + var newest: ?*const Image = null; var it = self.images.iterator(); while (it.next()) |kv| { - if (kv.value_ptr.number == image_number) { + const image = kv.value_ptr.*; + if (image.number == image_number) { if (newest == null or - kv.value_ptr.generation > newest.?.generation) + image.generation > newest.?.generation) { - newest = kv.value_ptr.*; + newest = image; } } } @@ -307,6 +325,12 @@ pub const ImageStorage = struct { return newest; } + /// Return an owned handle to the image currently stored for the given ID. + pub fn retainImageById(self: *const ImageStorage, image_id: u32) ?*Image { + const image = self.images.get(image_id) orelse return null; + return image.retain(); + } + /// Delete placements, images. pub fn delete( self: *ImageStorage, @@ -551,7 +575,7 @@ pub const ImageStorage = struct { } /// Delete an image if it is unused. - fn deleteIfUnused(self: *ImageStorage, alloc: Allocator, image_id: u32) void { + fn deleteIfUnused(self: *ImageStorage, _: Allocator, image_id: u32) void { var it = self.placements.iterator(); while (it.next()) |kv| { if (kv.key_ptr.image_id == image_id) { @@ -561,8 +585,8 @@ pub const ImageStorage = struct { // If we get here, we can delete the image. if (self.images.getEntry(image_id)) |entry| { - self.total_bytes -= entry.value_ptr.data.len; - entry.value_ptr.deinit(alloc); + self.total_bytes -= entry.value_ptr.*.data.len; + entry.value_ptr.*.release(); self.images.removeByPtr(entry.key_ptr); } } @@ -599,7 +623,12 @@ pub const ImageStorage = struct { /// /// This will evict as many images as necessary to make space for /// req bytes. - fn evictImage(self: *ImageStorage, alloc: Allocator, req: usize) !bool { + fn evictImage( + self: *ImageStorage, + alloc: Allocator, + req: usize, + exclude_id: ?u32, + ) !bool { assert(req <= self.total_limit); // Ironically we allocate to evict. We should probably redesign the @@ -616,7 +645,8 @@ pub const ImageStorage = struct { var it = self.images.iterator(); while (it.next()) |kv| { - const img = kv.value_ptr; + const img = kv.value_ptr.*; + if (exclude_id == img.id) continue; // This is a huge waste. See comment above about redesigning // our data structures to avoid this. Eviction should be very @@ -686,16 +716,17 @@ pub const ImageStorage = struct { } if (self.images.getEntry(c.id)) |entry| { - log.info("evicting image id={} bytes={}", .{ c.id, entry.value_ptr.data.len }); + const data_len = entry.value_ptr.*.data.len; + log.info("evicting image id={} bytes={}", .{ c.id, data_len }); - evicted += entry.value_ptr.data.len; - self.total_bytes -= entry.value_ptr.data.len; + evicted += data_len; + self.total_bytes -= data_len; - entry.value_ptr.deinit(alloc); + entry.value_ptr.*.release(); self.images.removeByPtr(entry.key_ptr); any_evicted = true; - if (evicted > req) return true; + if (evicted >= req) return true; } } @@ -757,7 +788,7 @@ pub const ImageStorage = struct { /// rows/columns, and aspect ratio. pub fn pixelSize( self: Placement, - image: Image, + image: *const Image, t: *const terminal.Terminal, ) struct { width: u32, @@ -836,7 +867,7 @@ pub const ImageStorage = struct { /// Returns the size in grid cells that this placement takes up. pub fn gridSize( self: Placement, - image: Image, + image: *const Image, t: *const terminal.Terminal, ) struct { cols: u32, @@ -872,7 +903,7 @@ pub const ImageStorage = struct { /// doesn't have an associated rect (i.e. a virtual placement). pub fn rect( self: Placement, - image: Image, + image: *const Image, t: *const terminal.Terminal, ) ?Rect { const grid_size = self.gridSize(image, t); @@ -911,6 +942,17 @@ fn trackPin( }).?); } +/// Test-only convenience for constructing and transferring an image. +fn addTestImage( + storage: *ImageStorage, + alloc: Allocator, + init: Image.Init, +) Allocator.Error!void { + const image = try Image.create(alloc, init); + errdefer image.release(); + try storage.addImage(alloc, image); +} + test "storage: add placement with zero placement id" { const testing = std.testing; const alloc = testing.allocator; @@ -921,8 +963,8 @@ test "storage: add placement with zero placement id" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1, .width = 50, .height = 50 }); - try s.addImage(alloc, .{ .id = 2, .width = 25, .height = 25 }); + try addTestImage(&s, alloc, .{ .id = 1, .width = 50, .height = 50 }); + try addTestImage(&s, alloc, .{ .id = 2, .width = 25, .height = 25 }); try s.addPlacement(alloc, 1, 0, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); try s.addPlacement(alloc, 1, 0, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); @@ -949,9 +991,9 @@ test "storage: delete all placements and images" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1 }); - try s.addImage(alloc, .{ .id = 2 }); - try s.addImage(alloc, .{ .id = 3 }); + try addTestImage(&s, alloc, .{ .id = 1 }); + try addTestImage(&s, alloc, .{ .id = 2 }); + try addTestImage(&s, alloc, .{ .id = 3 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try s.addPlacement(alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); @@ -973,9 +1015,9 @@ test "storage: delete all placements and images preserves limit" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); s.total_limit = 5000; - try s.addImage(alloc, .{ .id = 1 }); - try s.addImage(alloc, .{ .id = 2 }); - try s.addImage(alloc, .{ .id = 3 }); + try addTestImage(&s, alloc, .{ .id = 1 }); + try addTestImage(&s, alloc, .{ .id = 2 }); + try addTestImage(&s, alloc, .{ .id = 3 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try s.addPlacement(alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); @@ -997,9 +1039,9 @@ test "storage: delete all placements" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1 }); - try s.addImage(alloc, .{ .id = 2 }); - try s.addImage(alloc, .{ .id = 3 }); + try addTestImage(&s, alloc, .{ .id = 1 }); + try addTestImage(&s, alloc, .{ .id = 2 }); + try addTestImage(&s, alloc, .{ .id = 3 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try s.addPlacement(alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); @@ -1020,9 +1062,9 @@ test "storage: delete all placements by image id" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1 }); - try s.addImage(alloc, .{ .id = 2 }); - try s.addImage(alloc, .{ .id = 3 }); + try addTestImage(&s, alloc, .{ .id = 1 }); + try addTestImage(&s, alloc, .{ .id = 2 }); + try addTestImage(&s, alloc, .{ .id = 3 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try s.addPlacement(alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); @@ -1043,9 +1085,9 @@ test "storage: delete all placements by image id and unused images" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1 }); - try s.addImage(alloc, .{ .id = 2 }); - try s.addImage(alloc, .{ .id = 3 }); + try addTestImage(&s, alloc, .{ .id = 1 }); + try addTestImage(&s, alloc, .{ .id = 2 }); + try addTestImage(&s, alloc, .{ .id = 3 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try s.addPlacement(alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); @@ -1066,9 +1108,9 @@ test "storage: delete placement by specific id" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1 }); - try s.addImage(alloc, .{ .id = 2 }); - try s.addImage(alloc, .{ .id = 3 }); + try addTestImage(&s, alloc, .{ .id = 1 }); + try addTestImage(&s, alloc, .{ .id = 2 }); + try addTestImage(&s, alloc, .{ .id = 3 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try s.addPlacement(alloc, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try s.addPlacement(alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); @@ -1096,8 +1138,8 @@ test "storage: delete intersecting cursor" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1, .width = 50, .height = 50 }); - try s.addImage(alloc, .{ .id = 2, .width = 25, .height = 25 }); + try addTestImage(&s, alloc, .{ .id = 1, .width = 50, .height = 50 }); + try addTestImage(&s, alloc, .{ .id = 2, .width = 25, .height = 25 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); try s.addPlacement(alloc, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); @@ -1128,8 +1170,8 @@ test "storage: delete intersecting cursor plus unused" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1, .width = 50, .height = 50 }); - try s.addImage(alloc, .{ .id = 2, .width = 25, .height = 25 }); + try addTestImage(&s, alloc, .{ .id = 1, .width = 50, .height = 50 }); + try addTestImage(&s, alloc, .{ .id = 2, .width = 25, .height = 25 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); try s.addPlacement(alloc, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); @@ -1160,8 +1202,8 @@ test "storage: delete intersecting cursor hits multiple" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1, .width = 50, .height = 50 }); - try s.addImage(alloc, .{ .id = 2, .width = 25, .height = 25 }); + try addTestImage(&s, alloc, .{ .id = 1, .width = 50, .height = 50 }); + try addTestImage(&s, alloc, .{ .id = 2, .width = 25, .height = 25 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); try s.addPlacement(alloc, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); @@ -1186,8 +1228,8 @@ test "storage: delete by column" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1, .width = 50, .height = 50 }); - try s.addImage(alloc, .{ .id = 2, .width = 25, .height = 25 }); + try addTestImage(&s, alloc, .{ .id = 1, .width = 50, .height = 50 }); + try addTestImage(&s, alloc, .{ .id = 2, .width = 25, .height = 25 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); try s.addPlacement(alloc, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); @@ -1218,7 +1260,7 @@ test "storage: delete by column 1x1" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1, .width = 1, .height = 1 }); + try addTestImage(&s, alloc, .{ .id = 1, .width = 1, .height = 1 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); try s.addPlacement(alloc, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 0 }) } }); try s.addPlacement(alloc, 1, 3, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 2, .y = 0 }) } }); @@ -1252,8 +1294,8 @@ test "storage: delete by row" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1, .width = 50, .height = 50 }); - try s.addImage(alloc, .{ .id = 2, .width = 25, .height = 25 }); + try addTestImage(&s, alloc, .{ .id = 1, .width = 50, .height = 50 }); + try addTestImage(&s, alloc, .{ .id = 2, .width = 25, .height = 25 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); try s.addPlacement(alloc, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); @@ -1284,7 +1326,7 @@ test "storage: delete by row 1x1" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1, .width = 1, .height = 1 }); + try addTestImage(&s, alloc, .{ .id = 1, .width = 1, .height = 1 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .y = 0 }) } }); try s.addPlacement(alloc, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .y = 1 }) } }); try s.addPlacement(alloc, 1, 3, .{ .location = .{ .pin = try trackPin(&t, .{ .y = 2 }) } }); @@ -1316,9 +1358,9 @@ test "storage: delete images by range 1" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1 }); - try s.addImage(alloc, .{ .id = 2 }); - try s.addImage(alloc, .{ .id = 3 }); + try addTestImage(&s, alloc, .{ .id = 1 }); + try addTestImage(&s, alloc, .{ .id = 2 }); + try addTestImage(&s, alloc, .{ .id = 3 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try s.addPlacement(alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try testing.expectEqual(@as(usize, 3), s.images.count()); @@ -1341,9 +1383,9 @@ test "storage: delete images by range 2" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1 }); - try s.addImage(alloc, .{ .id = 2 }); - try s.addImage(alloc, .{ .id = 3 }); + try addTestImage(&s, alloc, .{ .id = 1 }); + try addTestImage(&s, alloc, .{ .id = 2 }); + try addTestImage(&s, alloc, .{ .id = 3 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try s.addPlacement(alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try testing.expectEqual(@as(usize, 3), s.images.count()); @@ -1366,9 +1408,9 @@ test "storage: delete images by range 3" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1 }); - try s.addImage(alloc, .{ .id = 2 }); - try s.addImage(alloc, .{ .id = 3 }); + try addTestImage(&s, alloc, .{ .id = 1 }); + try addTestImage(&s, alloc, .{ .id = 2 }); + try addTestImage(&s, alloc, .{ .id = 3 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try s.addPlacement(alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try testing.expectEqual(@as(usize, 3), s.images.count()); @@ -1391,9 +1433,9 @@ test "storage: delete images by range 4" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1 }); - try s.addImage(alloc, .{ .id = 2 }); - try s.addImage(alloc, .{ .id = 3 }); + try addTestImage(&s, alloc, .{ .id = 1 }); + try addTestImage(&s, alloc, .{ .id = 2 }); + try addTestImage(&s, alloc, .{ .id = 3 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try s.addPlacement(alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try testing.expectEqual(@as(usize, 3), s.images.count()); @@ -1418,7 +1460,8 @@ test "storage: aspect ratio calculation when only columns or rows specified" { // Case 1: Only columns specified { - const image = Image{ .id = 1, .width = 16, .height = 9 }; + const image = try Image.create(alloc, .{ .id = 1, .width = 16, .height = 9 }); + defer image.release(); var placement = ImageStorage.Placement{ .location = .{ .virtual = {} }, .columns = 10, @@ -1436,7 +1479,8 @@ test "storage: aspect ratio calculation when only columns or rows specified" { // Case 2: Only rows specified { - const image = Image{ .id = 2, .width = 16, .height = 9 }; + const image = try Image.create(alloc, .{ .id = 2, .width = 16, .height = 9 }); + defer image.release(); var placement = ImageStorage.Placement{ .location = .{ .virtual = {} }, .columns = 0, @@ -1465,7 +1509,7 @@ test "storage: generation stamps on image add and replace" { // Fresh storage has generation zero (never mutated). try testing.expectEqual(@as(u64, 0), s.generation); - try s.addImage(alloc, .{ .id = 1, .width = 1, .height = 1 }); + try addTestImage(&s, alloc, .{ .id = 1, .width = 1, .height = 1 }); const gen1 = s.generation; try testing.expect(gen1 > 0); @@ -1473,7 +1517,7 @@ test "storage: generation stamps on image add and replace" { try testing.expectEqual(gen1, img1.generation); // A second image gets a strictly greater stamp. - try s.addImage(alloc, .{ .id = 2, .width = 1, .height = 1 }); + try addTestImage(&s, alloc, .{ .id = 2, .width = 1, .height = 1 }); const gen2 = s.generation; try testing.expect(gen2 > gen1); try testing.expectEqual(gen2, s.imageById(2).?.generation); @@ -1481,7 +1525,7 @@ test "storage: generation stamps on image add and replace" { // Retransmitting the same image ID (identical dimensions) gets a // fresh stamp: this is what makes same-sized retransmissions // detectable by renderers. - try s.addImage(alloc, .{ .id = 1, .width = 1, .height = 1 }); + try addTestImage(&s, alloc, .{ .id = 1, .width = 1, .height = 1 }); const gen3 = s.generation; try testing.expect(gen3 > gen2); try testing.expectEqual(gen3, s.imageById(1).?.generation); @@ -1490,6 +1534,203 @@ test "storage: generation stamps on image add and replace" { try testing.expectEqual(gen2, s.imageById(2).?.generation); } +test "storage: retained image shares storage object" { + const testing = std.testing; + const alloc = testing.allocator; + var t = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer t.deinit(alloc); + + var s: ImageStorage = .{}; + defer s.deinit(alloc, t.screens.active); + + const data = try alloc.dupe(u8, "pixels"); + try addTestImage(&s, alloc, .{ .id = 1, .width = 2, .height = 1, .data = data }); + const borrowed = s.imageById(1).?; + + const retained = borrowed.retain(); + defer retained.release(); + try testing.expectEqual(borrowed, retained); + try testing.expectEqual(borrowed.data.ptr, retained.data.ptr); + try testing.expectEqualSlices(u8, "pixels", retained.data); +} + +test "storage: retained image survives same-ID replacement" { + const testing = std.testing; + const alloc = testing.allocator; + var t = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer t.deinit(alloc); + + var s: ImageStorage = .{}; + defer s.deinit(alloc, t.screens.active); + + const old_data = try alloc.dupe(u8, "old"); + try addTestImage(&s, alloc, .{ .id = 1, .width = 1, .height = 1, .data = old_data }); + const old = s.imageById(1).?; + const retained = old.retain(); + defer retained.release(); + + const new_data = try alloc.dupe(u8, "new"); + try addTestImage(&s, alloc, .{ .id = 1, .width = 1, .height = 1, .data = new_data }); + + try testing.expectEqualSlices(u8, "old", retained.data); + try testing.expectEqualSlices(u8, "new", s.imageById(1).?.data); +} + +test "storage: replacement eviction excludes replaced image" { + const testing = std.testing; + const alloc = testing.allocator; + var t = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer t.deinit(alloc); + + var s: ImageStorage = .{ .total_limit = 10 }; + defer s.deinit(alloc, t.screens.active); + + const old_data = try alloc.dupe(u8, "123456"); + try addTestImage(&s, alloc, .{ .id = 1, .data = old_data }); + try s.addPlacement(alloc, 1, 1, .{ + .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) }, + }); + + const other_data = try alloc.dupe(u8, "1234"); + try addTestImage(&s, alloc, .{ .id = 2, .data = other_data }); + + const replacement_data = try alloc.dupe(u8, "12345678"); + try addTestImage(&s, alloc, .{ .id = 1, .data = replacement_data }); + + try testing.expectEqual(@as(usize, 8), s.total_bytes); + try testing.expect(s.imageById(1) != null); + try testing.expect(s.imageById(2) == null); + try testing.expectEqual(@as(usize, 1), s.placements.count()); +} + +test "storage: retained image survives explicit deletion" { + const testing = std.testing; + const alloc = testing.allocator; + var t = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer t.deinit(alloc); + + var s: ImageStorage = .{}; + defer s.deinit(alloc, t.screens.active); + + const data = try alloc.dupe(u8, "keep"); + try addTestImage(&s, alloc, .{ .id = 1, .width = 1, .height = 1, .data = data }); + const retained = s.imageById(1).?.retain(); + defer retained.release(); + + s.delete(alloc, &t, .{ .id = .{ .image_id = 1, .delete = true } }); + try testing.expect(s.imageById(1) == null); + try testing.expectEqual(@as(usize, 0), s.total_bytes); + try testing.expectEqualSlices(u8, "keep", retained.data); +} + +test "storage: retained image survives quota eviction" { + const testing = std.testing; + const alloc = testing.allocator; + var t = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer t.deinit(alloc); + + var s: ImageStorage = .{ .total_limit = 4 }; + defer s.deinit(alloc, t.screens.active); + + const old_data = try alloc.dupe(u8, "old!"); + try addTestImage(&s, alloc, .{ .id = 1, .width = 1, .height = 1, .data = old_data }); + const retained = s.imageById(1).?.retain(); + defer retained.release(); + + const new_data = try alloc.dupe(u8, "new!"); + try addTestImage(&s, alloc, .{ .id = 2, .width = 1, .height = 1, .data = new_data }); + + try testing.expect(s.imageById(1) == null); + try testing.expectEqual(@as(usize, 4), s.total_bytes); + try testing.expectEqualSlices(u8, "old!", retained.data); +} + +test "storage: retained image survives storage deinit until final release" { + const testing = std.testing; + var failing = testing.FailingAllocator.init(testing.allocator, .{}); + const alloc = failing.allocator(); + var t = try terminal.Terminal.init(testing.allocator, .{ .rows = 3, .cols = 3 }); + defer t.deinit(testing.allocator); + + var s: ImageStorage = .{}; + const data = try alloc.dupe(u8, "alive"); + try addTestImage(&s, alloc, .{ .id = 1, .width = 1, .height = 1, .data = data }); + const retained = s.imageById(1).?.retain(); + + s.deinit(alloc, t.screens.active); + try testing.expectEqualSlices(u8, "alive", retained.data); + + const deallocations = failing.deallocations; + retained.release(); + // The final release frees both the pixel allocation and Image header. + try testing.expectEqual(deallocations + 2, failing.deallocations); +} + +test "storage: failed insertion preserves caller and stored image ownership" { + const testing = std.testing; + const alloc = testing.allocator; + var t = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer t.deinit(alloc); + + // A map growth failure leaves the incoming allocation with the caller. + { + var s: ImageStorage = .{}; + defer s.deinit(alloc, t.screens.active); + var failing = testing.FailingAllocator.init(alloc, .{ .fail_index = 0 }); + const image = try Image.create(alloc, .{ .id = 1, .data = try alloc.dupe(u8, "incoming") }); + defer image.release(); + + try testing.expectError(error.OutOfMemory, s.addImage(failing.allocator(), image)); + try testing.expectEqual(@as(usize, 0), s.images.count()); + try testing.expectEqualSlices(u8, "incoming", image.data); + } + + // A quota failure during replacement leaves both the existing entry and + // incoming allocation valid and independently owned. + { + var s: ImageStorage = .{}; + defer s.deinit(alloc, t.screens.active); + const old_data = try alloc.dupe(u8, "stored"); + try addTestImage(&s, alloc, .{ .id = 1, .data = old_data }); + const old = s.imageById(1).?; + s.total_limit = old.data.len; + + const image = try Image.create(alloc, .{ .id = 1, .data = try alloc.dupe(u8, "incoming") }); + defer image.release(); + + try testing.expectError(error.OutOfMemory, s.addImage(alloc, image)); + try testing.expectEqual(old.generation, s.imageById(1).?.generation); + try testing.expectEqual(old.data.ptr, s.imageById(1).?.data.ptr); + try testing.expectEqualSlices(u8, "stored", s.imageById(1).?.data); + try testing.expectEqualSlices(u8, "incoming", image.data); + } +} + +test "storage: empty images allocate only an Image header" { + const testing = std.testing; + const alloc = testing.allocator; + var t = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer t.deinit(alloc); + + var s: ImageStorage = .{}; + defer s.deinit(alloc, t.screens.active); + try s.images.ensureUnusedCapacity(alloc, 1); + + var failing = testing.FailingAllocator.init(alloc, .{ .fail_index = 0 }); + try testing.expectError( + error.OutOfMemory, + addTestImage(&s, failing.allocator(), .{ .id = 1 }), + ); + try testing.expect(failing.has_induced_failure); + try addTestImage(&s, alloc, .{ .id = 1 }); + + const borrowed = s.imageById(1).?; + const retained = borrowed.retain(); + defer retained.release(); + try testing.expectEqual(@as(usize, 0), retained.data.len); + try testing.expectEqual(@as(usize, 2), retained.refs.load(.monotonic)); +} + test "storage: generation bumps on placement and delete" { const testing = std.testing; const alloc = testing.allocator; @@ -1498,7 +1739,7 @@ test "storage: generation bumps on placement and delete" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - try s.addImage(alloc, .{ .id = 1 }); + try addTestImage(&s, alloc, .{ .id = 1 }); const gen_add = s.generation; try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); @@ -1524,7 +1765,7 @@ test "storage: generation bumps when setLimit evicts or disables" { defer s.deinit(alloc, t.screens.active); const data = try alloc.dupe(u8, "1234"); - try s.addImage(alloc, .{ .id = 1, .width = 1, .height = 1, .data = data }); + try addTestImage(&s, alloc, .{ .id = 1, .width = 1, .height = 1, .data = data }); const gen_add = s.generation; // Lowering the limit evicts the image and must mark a mutation. @@ -1553,12 +1794,12 @@ test "storage: imageByNumber returns most recently transmitted" { // Two images sharing a number: the newest transmission wins, // regardless of insertion order or clock resolution. - try s.addImage(alloc, .{ .id = 1, .number = 7 }); - try s.addImage(alloc, .{ .id = 2, .number = 7 }); + try addTestImage(&s, alloc, .{ .id = 1, .number = 7 }); + try addTestImage(&s, alloc, .{ .id = 2, .number = 7 }); try testing.expectEqual(@as(u32, 2), s.imageByNumber(7).?.id); // Retransmit the first: it becomes the newest. - try s.addImage(alloc, .{ .id = 1, .number = 7 }); + try addTestImage(&s, alloc, .{ .id = 1, .number = 7 }); try testing.expectEqual(@as(u32, 1), s.imageByNumber(7).?.id); } @@ -1586,7 +1827,7 @@ test "storage: no-op delete does not mark a mutation" { try testing.expectEqual(@as(u64, 0), s.generation); // Same for a delete that matches nothing. - try s.addImage(alloc, .{ .id = 1 }); + try addTestImage(&s, alloc, .{ .id = 1 }); try s.addPlacement(alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); const gen = s.generation; s.dirty = false; diff --git a/src/terminal/kitty/graphics_unicode.zig b/src/terminal/kitty/graphics_unicode.zig index a223797baf8..320bcc09fe2 100644 --- a/src/terminal/kitty/graphics_unicode.zig +++ b/src/terminal/kitty/graphics_unicode.zig @@ -1182,7 +1182,8 @@ test "unicode render placement: dog 4x2" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - const image: Image = .{ .id = 1, .width = 500, .height = 306 }; + const image = try Image.create(alloc, .{ .id = 1, .width = 500, .height = 306 }); + errdefer image.release(); try s.addImage(alloc, image); try s.addPlacement(alloc, 1, 0, .{ .location = .{ .virtual = {} }, @@ -1201,7 +1202,7 @@ test "unicode render placement: dog 4x2" { .width = 4, .height = 1, }; - const rp = try p.renderPlacement(&s, &image, cell_width, cell_height); + const rp = try p.renderPlacement(&s, image, cell_width, cell_height); try testing.expectEqual(0, rp.offset_x); try testing.expectEqual(36, rp.offset_y); try testing.expectEqual(0, rp.source_x); @@ -1222,7 +1223,7 @@ test "unicode render placement: dog 4x2" { .width = 4, .height = 1, }; - const rp = try p.renderPlacement(&s, &image, cell_width, cell_height); + const rp = try p.renderPlacement(&s, image, cell_width, cell_height); try testing.expectEqual(0, rp.offset_x); try testing.expectEqual(0, rp.offset_y); try testing.expectEqual(0, rp.source_x); @@ -1249,7 +1250,8 @@ test "unicode render placement: dog 2x2 with blank cells" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - const image: Image = .{ .id = 1, .width = 500, .height = 306 }; + const image = try Image.create(alloc, .{ .id = 1, .width = 500, .height = 306 }); + errdefer image.release(); try s.addImage(alloc, image); try s.addPlacement(alloc, 1, 0, .{ .location = .{ .virtual = {} }, @@ -1268,7 +1270,7 @@ test "unicode render placement: dog 2x2 with blank cells" { .width = 4, .height = 1, }; - const rp = try p.renderPlacement(&s, &image, cell_width, cell_height); + const rp = try p.renderPlacement(&s, image, cell_width, cell_height); try testing.expectEqual(0, rp.offset_x); try testing.expectEqual(58, rp.offset_y); try testing.expectEqual(0, rp.source_x); @@ -1289,7 +1291,7 @@ test "unicode render placement: dog 2x2 with blank cells" { .width = 4, .height = 1, }; - const rp = try p.renderPlacement(&s, &image, cell_width, cell_height); + const rp = try p.renderPlacement(&s, image, cell_width, cell_height); try testing.expectEqual(0, rp.offset_x); try testing.expectEqual(0, rp.offset_y); try testing.expectEqual(0, rp.source_x); @@ -1315,7 +1317,8 @@ test "unicode render placement: dog 1x1" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); - const image: Image = .{ .id = 1, .width = 500, .height = 306 }; + const image = try Image.create(alloc, .{ .id = 1, .width = 500, .height = 306 }); + errdefer image.release(); try s.addImage(alloc, image); try s.addPlacement(alloc, 1, 0, .{ .location = .{ .virtual = {} }, @@ -1334,7 +1337,7 @@ test "unicode render placement: dog 1x1" { .width = 4, .height = 1, }; - const rp = try p.renderPlacement(&s, &image, cell_width, cell_height); + const rp = try p.renderPlacement(&s, image, cell_width, cell_height); try testing.expectEqual(0, rp.offset_x); try testing.expectEqual(29, rp.offset_y); try testing.expectEqual(0, rp.source_x); From 2d335823e6ac7d9f141b40c8bc76af996c49d363 Mon Sep 17 00:00:00 2001 From: Tim Culverhouse Date: Fri, 10 Jul 2026 11:26:25 -0500 Subject: [PATCH 2/3] terminal: expose retained Kitty images through C API Borrowed Kitty image handles cannot safely cross into asynchronous render or raster work because terminal mutation may invalidate them. Make GhosttyKittyGraphicsImage an owned reference to Image itself. Image lookup now retains the handle, retain increments the same reference count, and free releases it without a wrapper allocation or caller-provided allocator. Keep image metadata and pixel getters read-only, with pixel pointers valid for the lifetime of their image handle. --- example/c-vt-kitty-graphics/src/main.c | 1 + include/ghostty/vt/kitty_graphics.h | 51 ++++- include/ghostty/vt/types.h | 7 +- src/lib_vt.zig | 2 + src/terminal/c/kitty_graphics.zig | 252 ++++++++++++++++++++++++- src/terminal/c/main.zig | 2 + 6 files changed, 298 insertions(+), 17 deletions(-) diff --git a/example/c-vt-kitty-graphics/src/main.c b/example/c-vt-kitty-graphics/src/main.c index d0d891928d0..39994e19e54 100644 --- a/example/c-vt-kitty-graphics/src/main.c +++ b/example/c-vt-kitty-graphics/src/main.c @@ -216,6 +216,7 @@ int main() { &cols, &rows) == GHOSTTY_SUCCESS) { printf(" grid size: %u cols x %u rows\n", cols, rows); } + ghostty_kitty_graphics_image_free(image); } printf("Total placements: %d\n", placement_count); ghostty_kitty_graphics_placement_iterator_free(iter); diff --git a/include/ghostty/vt/kitty_graphics.h b/include/ghostty/vt/kitty_graphics.h index dda8b836e1e..1beaa0b02ce 100644 --- a/include/ghostty/vt/kitty_graphics.h +++ b/include/ghostty/vt/kitty_graphics.h @@ -66,8 +66,13 @@ extern "C" { * @ref GHOSTTY_KITTY_GRAPHICS_PLACEMENT_DATA_IMAGE_ID), call * ghostty_kitty_graphics_image() to get a @ref GhosttyKittyGraphicsImage * handle. From this handle, ghostty_kitty_graphics_image_get() provides - * the image dimensions, pixel format, compression, and a borrowed pointer - * to the raw pixel data. + * the image dimensions, pixel format, compression, and a pointer to the raw + * pixel data owned by and valid for the lifetime of the image handle. The + * pointer carries no additional ownership. + * + * GhosttyKittyGraphicsImage is an owned handle that remains valid across + * terminal mutations. Release every returned handle with + * ghostty_kitty_graphics_image_free(). * * ## Rendering Helpers * @@ -108,11 +113,13 @@ extern "C" { * * ## Lifetime and Thread Safety * - * All handles borrowed from the terminal (GhosttyKittyGraphics, - * GhosttyKittyGraphicsImage) are invalidated by any mutating terminal - * call. The placement iterator is independently owned and must be freed - * by the caller, but the data it yields is only valid while the - * underlying terminal is not mutated. + * GhosttyKittyGraphics is borrowed from the terminal and invalidated by any + * mutating terminal call. GhosttyKittyGraphicsImage is independently owned + * and keeps its immutable image data valid across terminal mutations. + * + * The placement iterator is independently owned and must be freed by the + * caller, but the data it yields is only valid while the underlying terminal + * is not mutated. * * ## Example * @@ -389,8 +396,8 @@ typedef enum GHOSTTY_ENUM_TYPED { GHOSTTY_KITTY_IMAGE_DATA_COMPRESSION = 6, /** - * Borrowed pointer to the raw pixel data. Valid as long as the - * underlying terminal is not mutated. + * Pointer to raw pixel data owned by the image handle. It remains valid until + * the image handle is freed and carries no additional ownership. * * The data is always fully decoded, uncompressed pixels in the * format reported by GHOSTTY_KITTY_IMAGE_DATA_FORMAT: zlib payloads @@ -502,7 +509,8 @@ GHOSTTY_API GhosttyResult ghostty_kitty_graphics_get( * * @param graphics The kitty graphics handle * @param image_id The image ID to look up - * @return An opaque image handle, or NULL if not found + * @return An owned image handle, or NULL if not found. The caller must free + * non-NULL handles with ghostty_kitty_graphics_image_free(). * * @ingroup kitty_graphics */ @@ -510,6 +518,29 @@ GHOSTTY_API GhosttyKittyGraphicsImage ghostty_kitty_graphics_image( GhosttyKittyGraphics graphics, uint32_t image_id); +/** + * Increment an image handle's reference count. + * + * @param image The image to retain, or NULL + * @return The same image handle, or NULL if image is NULL + * + * @ingroup kitty_graphics + */ +GHOSTTY_API GhosttyKittyGraphicsImage ghostty_kitty_graphics_image_retain( + GhosttyKittyGraphicsImage image); + +/** + * Release an owned Kitty graphics image handle. + * + * Passing NULL is allowed and is a no-op. + * + * @param image The owned image handle to free, or NULL + * + * @ingroup kitty_graphics + */ +GHOSTTY_API void ghostty_kitty_graphics_image_free( + GhosttyKittyGraphicsImage image); + /** * Get data from a Kitty graphics image. * diff --git a/include/ghostty/vt/types.h b/include/ghostty/vt/types.h index 214d282296c..142246a29e0 100644 --- a/include/ghostty/vt/types.h +++ b/include/ghostty/vt/types.h @@ -119,11 +119,10 @@ typedef struct GhosttyTrackedGridRefImpl* GhosttyTrackedGridRef; typedef struct GhosttyKittyGraphicsImpl* GhosttyKittyGraphics; /** - * Opaque handle to a Kitty graphics image. + * Owned handle to a Kitty graphics image. * - * Obtained via ghostty_kitty_graphics_image() with an image ID. The - * pointer is borrowed from the storage and remains valid until the next - * mutating terminal call. + * Images remain valid across terminal mutations. Every non-NULL handle must + * be released with ghostty_kitty_graphics_image_free(). * * @ingroup kitty_graphics */ diff --git a/src/lib_vt.zig b/src/lib_vt.zig index e01cdbb892c..958d2a5b913 100644 --- a/src/lib_vt.zig +++ b/src/lib_vt.zig @@ -291,6 +291,8 @@ comptime { @export(&c.terminal_point_from_grid_ref, .{ .name = "ghostty_terminal_point_from_grid_ref" }); @export(&c.kitty_graphics_get, .{ .name = "ghostty_kitty_graphics_get" }); @export(&c.kitty_graphics_image, .{ .name = "ghostty_kitty_graphics_image" }); + @export(&c.kitty_graphics_image_retain, .{ .name = "ghostty_kitty_graphics_image_retain" }); + @export(&c.kitty_graphics_image_free, .{ .name = "ghostty_kitty_graphics_image_free" }); @export(&c.kitty_graphics_image_get, .{ .name = "ghostty_kitty_graphics_image_get" }); @export(&c.kitty_graphics_image_get_multi, .{ .name = "ghostty_kitty_graphics_image_get_multi" }); @export(&c.kitty_graphics_placement_iterator_new, .{ .name = "ghostty_kitty_graphics_placement_iterator_new" }); diff --git a/src/terminal/c/kitty_graphics.zig b/src/terminal/c/kitty_graphics.zig index aa7aff5daf2..0fbe6cb4a51 100644 --- a/src/terminal/c/kitty_graphics.zig +++ b/src/terminal/c/kitty_graphics.zig @@ -20,9 +20,9 @@ else /// C: GhosttyKittyGraphicsImage pub const ImageHandle = if (build_options.kitty_graphics) - ?*const Image + ?*Image else - ?*const anyopaque; + ?*anyopaque; /// C: GhosttyKittyGraphicsPlacementIterator pub const PlacementIterator = if (build_options.kitty_graphics) @@ -203,7 +203,19 @@ pub fn image_get_handle( if (comptime !build_options.kitty_graphics) return null; const storage = graphics_; - return storage.imageById(image_id); + return storage.retainImageById(image_id); +} + +pub fn image_retain(image_: ImageHandle) callconv(lib.calling_conv) ImageHandle { + if (comptime !build_options.kitty_graphics) return null; + const image = image_ orelse return null; + return image.retain(); +} + +pub fn image_free(image_: ImageHandle) callconv(lib.calling_conv) void { + if (comptime !build_options.kitty_graphics) return; + const image = image_ orelse return; + image.release(); } pub fn image_get( @@ -956,6 +968,7 @@ test "image_get_handle and image_get with transmitted image" { )); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var id: u32 = undefined; @@ -983,6 +996,221 @@ test "image_get_handle and image_get with transmitted image" { try testing.expect(data_len > 0); } +test "image retain returns same owned handle and null is safe" { + if (comptime !build_options.kitty_graphics) return error.SkipZigTest; + + var t: terminal_c.Terminal = null; + try testing.expectEqual(Result.success, terminal_c.new( + &lib.alloc.test_allocator, + &t, + .{ .cols = 80, .rows = 24, .max_scrollback = 0 }, + )); + defer terminal_c.free(t); + + const transmit = "\x1b_Ga=t,t=d,f=24,i=1,s=1,v=2;////////\x1b\\"; + terminal_c.vt_write(t, transmit.ptr, transmit.len); + + var graphics: KittyGraphics = undefined; + try testing.expectEqual(Result.success, terminal_c.get( + t, + .kitty_graphics, + @ptrCast(&graphics), + )); + + const image = image_get_handle(graphics, 1); + defer image_free(image); + const retained = image_retain(image); + try testing.expectEqual(image, retained); + image_free(retained); + try testing.expect(image_retain(null) == null); + image_free(null); +} + +test "owned image survives same-ID replacement" { + if (comptime !build_options.kitty_graphics) return error.SkipZigTest; + + var t: terminal_c.Terminal = null; + try testing.expectEqual(Result.success, terminal_c.new( + &lib.alloc.test_allocator, + &t, + .{ .cols = 80, .rows = 24, .max_scrollback = 0 }, + )); + defer terminal_c.free(t); + + const transmit1 = "\x1b_Ga=t,t=d,f=24,i=1,s=1,v=2;////////\x1b\\"; + terminal_c.vt_write(t, transmit1.ptr, transmit1.len); + + var graphics: KittyGraphics = undefined; + try testing.expectEqual(Result.success, terminal_c.get( + t, + .kitty_graphics, + @ptrCast(&graphics), + )); + const view = image_get_handle(graphics, 1); + defer image_free(view); + var original_ptr: [*]const u8 = undefined; + try testing.expectEqual(Result.success, image_get( + view, + .data_ptr, + @ptrCast(&original_ptr), + )); + + const transmit2 = "\x1b_Ga=t,t=d,f=24,i=1,s=1,v=2;AAAAAAAA\x1b\\"; + terminal_c.vt_write(t, transmit2.ptr, transmit2.len); + + var retained_ptr: [*]const u8 = undefined; + var retained_len: usize = 0; + try testing.expectEqual(Result.success, image_get( + view, + .data_ptr, + @ptrCast(&retained_ptr), + )); + try testing.expectEqual(Result.success, image_get( + view, + .data_len, + @ptrCast(&retained_len), + )); + try testing.expectEqual(original_ptr, retained_ptr); + try testing.expectEqualSlices(u8, &([_]u8{0xFF} ** 6), retained_ptr[0..retained_len]); + + try testing.expectEqual(Result.success, terminal_c.get( + t, + .kitty_graphics, + @ptrCast(&graphics), + )); + const replacement = image_get_handle(graphics, 1); + defer image_free(replacement); + var replacement_ptr: [*]const u8 = undefined; + try testing.expectEqual(Result.success, image_get( + replacement, + .data_ptr, + @ptrCast(&replacement_ptr), + )); + try testing.expect(replacement_ptr != retained_ptr); + try testing.expectEqualSlices(u8, &([_]u8{0} ** 6), replacement_ptr[0..6]); +} + +test "owned image survives deletion and final free releases ownership" { + if (comptime !build_options.kitty_graphics) return error.SkipZigTest; + + var failing = testing.FailingAllocator.init(testing.allocator, .{}); + const alloc = failing.allocator(); + const c_alloc = CAllocator.fromZig(&alloc); + + var t: terminal_c.Terminal = null; + try testing.expectEqual(Result.success, terminal_c.new( + &c_alloc, + &t, + .{ .cols = 80, .rows = 24, .max_scrollback = 0 }, + )); + defer terminal_c.free(t); + + const transmit = "\x1b_Ga=t,t=d,f=24,i=1,s=1,v=2;////////\x1b\\"; + terminal_c.vt_write(t, transmit.ptr, transmit.len); + + var graphics: KittyGraphics = undefined; + try testing.expectEqual(Result.success, terminal_c.get( + t, + .kitty_graphics, + @ptrCast(&graphics), + )); + var view = image_get_handle(graphics, 1); + defer image_free(view); + + const delete = "\x1b_Ga=d,d=A\x1b\\"; + terminal_c.vt_write(t, delete.ptr, delete.len); + try testing.expectEqual(Result.success, terminal_c.get( + t, + .kitty_graphics, + @ptrCast(&graphics), + )); + try testing.expect(image_get_handle(graphics, 1) == null); + + var data_ptr: [*]const u8 = undefined; + var data_len: usize = 0; + try testing.expectEqual(Result.success, image_get( + view, + .data_ptr, + @ptrCast(&data_ptr), + )); + try testing.expectEqual(Result.success, image_get( + view, + .data_len, + @ptrCast(&data_len), + )); + try testing.expectEqualSlices(u8, &([_]u8{0xFF} ** 6), data_ptr[0..data_len]); + + const deallocations = failing.deallocations; + image_free(view); + view = null; + // Final release frees the pixel allocation and image owner. + try testing.expectEqual(deallocations + 2, failing.deallocations); +} + +test "owned image survives quota eviction" { + if (comptime !build_options.kitty_graphics) return error.SkipZigTest; + + var t: terminal_c.Terminal = null; + try testing.expectEqual(Result.success, terminal_c.new( + &lib.alloc.test_allocator, + &t, + .{ .cols = 80, .rows = 24, .max_scrollback = 0 }, + )); + defer terminal_c.free(t); + + var graphics: KittyGraphics = undefined; + try testing.expectEqual(Result.success, terminal_c.get( + t, + .kitty_graphics, + @ptrCast(&graphics), + )); + graphics.total_limit = 6; + + const transmit1 = "\x1b_Ga=t,t=d,f=24,i=1,s=1,v=2;////////\x1b\\"; + terminal_c.vt_write(t, transmit1.ptr, transmit1.len); + try testing.expectEqual(Result.success, terminal_c.get( + t, + .kitty_graphics, + @ptrCast(&graphics), + )); + + const view = image_get_handle(graphics, 1); + defer image_free(view); + + const transmit2 = "\x1b_Ga=t,t=d,f=24,i=2,s=1,v=2;AAAAAAAA\x1b\\"; + terminal_c.vt_write(t, transmit2.ptr, transmit2.len); + try testing.expectEqual(Result.success, terminal_c.get( + t, + .kitty_graphics, + @ptrCast(&graphics), + )); + try testing.expect(image_get_handle(graphics, 1) == null); + const current = image_get_handle(graphics, 2); + defer image_free(current); + try testing.expect(current != null); + + var data_ptr: [*]const u8 = undefined; + var data_len: usize = 0; + try testing.expectEqual(Result.success, image_get( + view, + .data_ptr, + @ptrCast(&data_ptr), + )); + try testing.expectEqual(Result.success, image_get( + view, + .data_len, + @ptrCast(&data_len), + )); + try testing.expectEqualSlices(u8, &([_]u8{0xFF} ** 6), data_ptr[0..data_len]); +} + +test "image handle kitty-disabled behavior" { + if (comptime build_options.kitty_graphics) return error.SkipZigTest; + + try testing.expect(image_retain(null) == null); + image_free(null); +} + test "placement_rect with transmit and display" { if (comptime !build_options.kitty_graphics) return error.SkipZigTest; @@ -1011,6 +1239,7 @@ test "placement_rect with transmit and display" { )); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var iter: PlacementIterator = null; @@ -1071,6 +1300,7 @@ test "placement_pixel_size with transmit and display" { )); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var iter: PlacementIterator = null; @@ -1125,6 +1355,7 @@ test "placement_grid_size with transmit and display" { )); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var iter: PlacementIterator = null; @@ -1178,6 +1409,7 @@ test "placement_viewport_pos with transmit and display" { )); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var iter: PlacementIterator = null; @@ -1224,6 +1456,7 @@ test "placement_viewport_pos fully off-screen above" { var graphics: KittyGraphics = undefined; try testing.expectEqual(Result.success, terminal_c.get(t, .kitty_graphics, @ptrCast(&graphics))); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var iter: PlacementIterator = null; @@ -1265,6 +1498,7 @@ test "placement_viewport_pos top off-screen" { var graphics: KittyGraphics = undefined; try testing.expectEqual(Result.success, terminal_c.get(t, .kitty_graphics, @ptrCast(&graphics))); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var iter: PlacementIterator = null; @@ -1305,6 +1539,7 @@ test "placement_viewport_pos bottom off-screen" { var graphics: KittyGraphics = undefined; try testing.expectEqual(Result.success, terminal_c.get(t, .kitty_graphics, @ptrCast(&graphics))); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var iter: PlacementIterator = null; @@ -1348,6 +1583,7 @@ test "placement_viewport_pos top and bottom off-screen" { var graphics: KittyGraphics = undefined; try testing.expectEqual(Result.success, terminal_c.get(t, .kitty_graphics, @ptrCast(&graphics))); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var iter: PlacementIterator = null; @@ -1391,6 +1627,7 @@ test "placement_source_rect defaults to full image" { var graphics: KittyGraphics = undefined; try testing.expectEqual(Result.success, terminal_c.get(t, .kitty_graphics, @ptrCast(&graphics))); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var iter: PlacementIterator = null; @@ -1435,6 +1672,7 @@ test "placement_source_rect with explicit source rect" { var graphics: KittyGraphics = undefined; try testing.expectEqual(Result.success, terminal_c.get(t, .kitty_graphics, @ptrCast(&graphics))); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var iter: PlacementIterator = null; @@ -1479,6 +1717,7 @@ test "placement_source_rect clamps to image bounds" { var graphics: KittyGraphics = undefined; try testing.expectEqual(Result.success, terminal_c.get(t, .kitty_graphics, @ptrCast(&graphics))); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var iter: PlacementIterator = null; @@ -1533,6 +1772,7 @@ test "placement_render_info returns all fields" { var graphics: KittyGraphics = undefined; try testing.expectEqual(Result.success, terminal_c.get(t, .kitty_graphics, @ptrCast(&graphics))); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var iter: PlacementIterator = null; @@ -1578,6 +1818,7 @@ test "placement_render_info off-screen sets viewport_visible false" { var graphics: KittyGraphics = undefined; try testing.expectEqual(Result.success, terminal_c.get(t, .kitty_graphics, @ptrCast(&graphics))); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var iter: PlacementIterator = null; @@ -1621,6 +1862,7 @@ test "image_get_multi success" { var graphics: KittyGraphics = undefined; try testing.expectEqual(Result.success, terminal_c.get(t, .kitty_graphics, @ptrCast(&graphics))); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var id: u32 = 0; @@ -1791,6 +2033,7 @@ test "image generation detects same-sized retransmission" { var len1: usize = 0; { const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); try testing.expectEqual(Result.success, image_get(img, .generation, @ptrCast(&gen1))); try testing.expectEqual(Result.success, image_get(img, .width, @ptrCast(&w1))); @@ -1808,6 +2051,7 @@ test "image generation detects same-sized retransmission" { { const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var gen2: u64 = 0; var w2: u32 = 0; @@ -1845,6 +2089,7 @@ test "image generation via image_get_multi" { var graphics: KittyGraphics = undefined; try testing.expectEqual(Result.success, terminal_c.get(t, .kitty_graphics, @ptrCast(&graphics))); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); var id: u32 = 0; @@ -1892,6 +2137,7 @@ test "image compression and format always report decoded data" { var graphics: KittyGraphics = undefined; try testing.expectEqual(Result.success, terminal_c.get(t, .kitty_graphics, @ptrCast(&graphics))); const img = image_get_handle(graphics, 1); + defer image_free(img); try testing.expect(img != null); // Compression must report NONE: the data was inflated at diff --git a/src/terminal/c/main.zig b/src/terminal/c/main.zig index 37dc57684aa..3c878bef85e 100644 --- a/src/terminal/c/main.zig +++ b/src/terminal/c/main.zig @@ -13,6 +13,8 @@ pub const grid_ref_tracked = @import("grid_ref_tracked.zig"); pub const kitty_graphics = @import("kitty_graphics.zig"); pub const kitty_graphics_get = kitty_graphics.get; pub const kitty_graphics_image = kitty_graphics.image_get_handle; +pub const kitty_graphics_image_retain = kitty_graphics.image_retain; +pub const kitty_graphics_image_free = kitty_graphics.image_free; pub const kitty_graphics_image_get = kitty_graphics.image_get; pub const kitty_graphics_image_get_multi = kitty_graphics.image_get_multi; pub const kitty_graphics_placement_iterator_new = kitty_graphics.placement_iterator_new; From 2a4db8d0ea927b1d6482c4fb7ecf0352eb7038b8 Mon Sep 17 00:00:00 2001 From: Tim Culverhouse Date: Fri, 10 Jul 2026 12:16:21 -0500 Subject: [PATCH 3/3] renderer: retain Kitty images for upload Kitty image preparation copied terminal-owned pixels and converted them while the renderer state mutex excluded terminal parsing. Large images and video frames therefore extended the critical section with a full copy and format conversion. Add an opt-in Kitty graphics snapshot to RenderState. Retain images and copy placement geometry while the terminal is locked, then update renderer image state under the draw mutex after releasing the terminal lock. Only placed images are prepared for upload. Track retained and renderer-owned pending data explicitly, and release the retained image after synchronous texture creation consumes it. Failed uploads remain pending for a later natural draw without a retry scheduler. --- src/renderer/Overlay.zig | 23 +- src/renderer/generic.zig | 52 +- src/renderer/image.zig | 1073 +++++++++++++++++++------------ src/renderer/metal/Texture.zig | 5 +- src/renderer/opengl/Texture.zig | 5 +- src/terminal/render.zig | 274 ++++++++ 6 files changed, 971 insertions(+), 461 deletions(-) diff --git a/src/renderer/Overlay.zig b/src/renderer/Overlay.zig index 62eb004ba64..5a036f08878 100644 --- a/src/renderer/Overlay.zig +++ b/src/renderer/Overlay.zig @@ -106,16 +106,33 @@ pub fn deinit(self: *Overlay, alloc: Allocator) void { self.surface.deinit(alloc); } -/// Returns a pending image that can be used to copy, convert, upload, etc. -pub fn pendingImage(self: *const Overlay) Image.Pending { +/// Returns a borrowed image source that the renderer copies before storing. +pub fn pendingImage(self: *const Overlay) Image.Source { return .{ .width = @intCast(self.surface.getWidth()), .height = @intCast(self.surface.getHeight()), .pixel_format = .rgba, - .data = @ptrCast(self.surface.image_surface_rgba.buf.ptr), + .data = std.mem.sliceAsBytes(self.surface.image_surface_rgba.buf), }; } +test "pending image exposes all RGBA bytes" { + const testing = std.testing; + const alloc = testing.allocator; + + var overlay = try Overlay.init(alloc, .{ + .screen = .{ .width = 7, .height = 5 }, + .cell = .{ .width = 1, .height = 1 }, + .padding = .{}, + }); + defer overlay.deinit(alloc); + + const source = overlay.pendingImage(); + try testing.expectEqual(@as(u32, 7), source.width); + try testing.expectEqual(@as(u32, 5), source.height); + try testing.expectEqual(@as(usize, 7 * 5 * 4), source.data.len); +} + /// Clear the overlay. pub fn reset(self: *Overlay) void { self.surface.paintPixel(.{ .rgba = .{ diff --git a/src/renderer/generic.zig b/src/renderer/generic.zig index 14b79036914..9982e504f33 100644 --- a/src/renderer/generic.zig +++ b/src/renderer/generic.zig @@ -220,7 +220,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { shaders: Shaders, /// The render state we update per loop. - terminal_state: terminal.RenderState = .empty, + terminal_state: terminal.RenderState = terminal.RenderState.init(.{ .kitty_graphics = true }), /// The number of frames since the last terminal state reset. /// We reset the terminal state after ~100,000 frames (about 10 to @@ -820,7 +820,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { self.images.deinit(self.alloc); - if (self.bg_image) |img| img.deinit(self.alloc); + if (self.bg_image) |*img| img.deinit(self.alloc); self.deinitShaders(); @@ -1143,7 +1143,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { const max_terminal_state_frame_count = 100_000; if (self.terminal_state_frame_count >= max_terminal_state_frame_count) { self.terminal_state.deinit(self.alloc); - self.terminal_state = .empty; + self.terminal_state = terminal.RenderState.init(.{ .kitty_graphics = true }); } self.terminal_state_frame_count += 1; @@ -1230,28 +1230,6 @@ pub fn Renderer(comptime GraphicsAPI: type) type { break :preedit try p.clone(arena_alloc); }; - // If we have Kitty graphics data, we enter a SLOW SLOW SLOW path. - // We only do this if the Kitty image state is dirty meaning only if - // it changes. - // - // If we have any virtual references, we must also rebuild our - // kitty state on every frame because any cell change can move - // an image. - if (self.images.kittyRequiresUpdate(state.terminal)) { - // We need to grab the draw mutex since this updates - // our image state that drawFrame uses. - self.draw_mutex.lock(); - defer self.draw_mutex.unlock(); - self.images.kittyUpdate( - self.alloc, - state.terminal, - .{ - .width = self.grid_metrics.cell_width, - .height = self.grid_metrics.cell_height, - }, - ); - } - // Get our OSC8 links we're hovering if we have a mouse. // This requires terminal state because of URLs. const links: terminal.RenderState.CellSet = osc8: { @@ -1293,6 +1271,14 @@ pub fn Renderer(comptime GraphicsAPI: type) type { // render state (e.g. rebuildCells). self.terminal_state.endUpdate(); + if (self.terminal_state.kitty.dirty) { + self.draw_mutex.lock(); + defer self.draw_mutex.unlock(); + if (self.images.kittyUpdate(self.alloc, &self.terminal_state)) { + self.terminal_state.kitty.dirty = false; + } + } + // Outside the critical area we can update our links to contain // our regex results. self.config.links.renderCellMap( @@ -1816,21 +1802,19 @@ pub fn Renderer(comptime GraphicsAPI: type) type { }, }; - const image: imagepkg.Image = .{ - .pending = .{ - .width = image_data.width, - .height = image_data.height, - .pixel_format = .rgba, - .data = image_data.data.ptr, - }, + var pending: imagepkg.Image.Pending = .{ + .width = image_data.width, + .height = image_data.height, + .pixel_format = .rgba, + .backing = .{ .renderer_owned = image_data.data }, }; // If we have an existing background image, replace it. // Otherwise, set this as our background image directly. if (self.bg_image) |*img| { - img.markForReplace(self.alloc, image); + img.markForReplace(self.alloc, &pending); } else { - self.bg_image = image; + self.bg_image = .{ .pending = pending.take() }; } } else { // If we don't have a background image path, mark our diff --git a/src/renderer/image.zig b/src/renderer/image.zig index f8269354a08..5a1872f10e4 100644 --- a/src/renderer/image.zig +++ b/src/renderer/image.zig @@ -7,7 +7,6 @@ const terminal = @import("../terminal/main.zig"); const Renderer = @import("../renderer.zig").Renderer; const GraphicsAPI = Renderer.API; const Texture = GraphicsAPI.Texture; -const CellSize = @import("size.zig").CellSize; const Overlay = @import("Overlay.zig"); const log = std.log.scoped(.renderer_image); @@ -28,11 +27,6 @@ pub const State = struct { kitty_bg_end: u32, kitty_text_end: u32, - /// True if there are any virtual placements. This needs to be known - /// because virtual placements need to be recalculated more often - /// on frame builds and are generally more expensive to handle. - kitty_virtual: bool, - /// Overlays overlay_placements: std.ArrayListUnmanaged(Placement), @@ -41,7 +35,6 @@ pub const State = struct { .kitty_placements = .empty, .kitty_bg_end = 0, .kitty_text_end = 0, - .kitty_virtual = false, .overlay_placements = .empty, }; @@ -58,9 +51,6 @@ pub const State = struct { /// Upload any images to the GPU that need to be uploaded, /// and remove any images that are no longer needed on the GPU. /// - /// If any uploads fail, they are ignored. The return value - /// can be used to detect if upload was a total success (true) - /// or not (false). pub fn upload( self: *State, alloc: Allocator, @@ -117,20 +107,15 @@ pub const State = struct { for (placements) |p| { // Look up the image - const image = self.images.get(p.image_id) orelse { + const image = self.images.getPtr(p.image_id) orelse { log.warn("image not found for placement image_id={}", .{p.image_id}); continue; }; // Get the texture - const texture = switch (image.image) { - .ready, - .unload_ready, - => |t| t, - else => { - log.warn("image not ready for placement image_id={}", .{p.image_id}); - continue; - }, + const texture = image.image.textureForDraw() orelse { + log.warn("image not ready for placement image_id={}", .{p.image_id}); + continue; }; // Create our vertex buffer, which is always exactly one item. @@ -229,304 +214,63 @@ pub const State = struct { }); } - /// Returns true if the Kitty graphics state requires an update based - /// on the terminal state and our internal state. - /// - /// This does not read/write state used by drawing. - pub fn kittyRequiresUpdate( - self: *const State, - t: *const terminal.Terminal, - ) bool { - // If the terminal kitty image state is dirty, we must update. - if (t.screens.active.kitty_images.dirty) return true; - - // If we have any virtual references, we must also rebuild our - // kitty state on every frame because any cell change can move - // an image. If the virtual placements were removed, this will - // be set to false on the next update. - if (self.kitty_virtual) return true; - - return false; - } - - /// Update the Kitty graphics state from the terminal. - /// - /// This reads/writes state used by drawing. + /// Update Kitty GPU state from the terminal-independent retained snapshot. pub fn kittyUpdate( self: *State, alloc: Allocator, - t: *const terminal.Terminal, - cell_size: CellSize, - ) void { - const storage = &t.screens.active.kitty_images; - defer storage.dirty = false; - - // We always clear our previous placements no matter what because - // we rebuild them from scratch. - self.kitty_placements.clearRetainingCapacity(); - self.kitty_virtual = false; - - // Go through our known images and if there are any that are no longer - // in use then mark them to be freed. - // - // This never conflicts with the below because a placement can't - // reference an image that doesn't exist. - { - var it = self.images.iterator(); - while (it.next()) |kv| { - switch (kv.key_ptr.*) { - // We're only looking at Kitty images - .kitty => |id| if (storage.imageById(id) == null) { - kv.value_ptr.image.markForUnload(); - }, - - .overlay => {}, - } - } - } + state: *const terminal.RenderState, + ) bool { + const snapshot = &state.kitty; - // The top-left and bottom-right corners of our viewport in screen - // points. This lets us determine offsets and containment of placements. - const top = t.screens.active.pages.getTopLeft(.viewport); - const bot = t.screens.active.pages.getBottomRight(.viewport).?; - const top_y = t.screens.active.pages.pointFromPin(.screen, top).?.screen.y; - const bot_y = t.screens.active.pages.pointFromPin(.screen, bot).?.screen.y; - - // Go through the placements and ensure the image is - // on the GPU or else is ready to be sent to the GPU. - var it = storage.placements.iterator(); - while (it.next()) |kv| { - const p = kv.value_ptr; - - // Special logic based on location - switch (p.location) { - .pin => {}, - .virtual => { - // We need to mark virtual placements on our renderer so that - // we know to rebuild in more scenarios since cell changes can - // now trigger placement changes. - self.kitty_virtual = true; - - // We also continue out because virtual placements are - // only triggered by the unicode placeholder, not by the - // placement itself. - continue; - }, - } + self.kitty_placements.ensureUnusedCapacity(alloc, snapshot.placements.items.len) catch { + return false; + }; - // Get the image for the placement - const image = storage.imageById(kv.key_ptr.image_id) orelse { - log.warn( - "missing image for placement, ignoring image_id={}", - .{kv.key_ptr.image_id}, - ); - continue; - }; + var cached = self.images.iterator(); + while (cached.next()) |entry| switch (entry.key_ptr.*) { + .kitty => |id| if (!snapshot.images.contains(id)) entry.value_ptr.image.markForUnload(), + .overlay => {}, + }; - self.prepKittyPlacement( - alloc, - t, - top_y, - bot_y, - &image, - p, - ) catch |err| { - // For errors we log and continue. We try to place - // other placements even if one fails. - log.warn("error preparing kitty placement err={}", .{err}); + var complete = true; + for (snapshot.placements.items) |placement| { + const image = snapshot.images.get(placement.image_id) orelse continue; + self.prepKittyImageRetained(alloc, image) catch |err| { + complete = false; + log.warn("error preparing kitty image err={}", .{err}); }; } - - // If we have virtual placements then we need to scan for placeholders. - if (self.kitty_virtual) { - var v_it = terminal.kitty.graphics.unicode.placementIterator(top, bot); - while (v_it.next()) |virtual_p| { - self.prepKittyVirtualPlacement( - alloc, - t, - &virtual_p, - cell_size, - ) catch |err| { - // For errors we log and continue. We try to place - // other placements even if one fails. - log.warn("error preparing kitty placement err={}", .{err}); - }; - } - } - - // Sort the placements by their Z value. - std.mem.sortUnstable( - Placement, - self.kitty_placements.items, - {}, - struct { - fn lessThan( - ctx: void, - lhs: Placement, - rhs: Placement, - ) bool { - _ = ctx; - return lhs.z < rhs.z or - (lhs.z == rhs.z and lhs.image_id.zLessThan(rhs.image_id)); - } - }.lessThan, - ); - - // Find our indices. The values are sorted by z so we can - // find the first placement out of bounds to find the limits. - const bg_limit = std.math.minInt(i32) / 2; - var bg_end: ?u32 = null; - var text_end: ?u32 = null; - for (self.kitty_placements.items, 0..) |p, i| { - if (bg_end == null and p.z >= bg_limit) bg_end = @intCast(i); - if (text_end == null and p.z >= 0) text_end = @intCast(i); - } - - // If we didn't see any images with a z > the bg limit, - // then our bg end is the end of our placement list. - self.kitty_bg_end = - bg_end orelse @intCast(self.kitty_placements.items.len); - // Same idea for the image_text_end. - self.kitty_text_end = - text_end orelse @intCast(self.kitty_placements.items.len); + self.kitty_placements.clearRetainingCapacity(); + for (snapshot.placements.items) |p| self.kitty_placements.appendAssumeCapacity(.{ + .image_id = .{ .kitty = p.image_id }, + .x = p.x, + .y = p.y, + .z = p.z, + .width = p.width, + .height = p.height, + .cell_offset_x = p.cell_offset_x, + .cell_offset_y = p.cell_offset_y, + .source_x = p.source_x, + .source_y = p.source_y, + .source_width = p.source_width, + .source_height = p.source_height, + }); + self.kitty_bg_end = snapshot.bg_end; + self.kitty_text_end = snapshot.text_end; + return complete; } const PrepImageError = error{ OutOfMemory, - ImageConversionError, }; - /// Get the viewport-relative position for this - /// placement and add it to the placements list. - fn prepKittyPlacement( - self: *State, - alloc: Allocator, - t: *const terminal.Terminal, - top_y: u32, - bot_y: u32, - image: *const terminal.kitty.graphics.Image, - p: *const terminal.kitty.graphics.ImageStorage.Placement, - ) PrepImageError!void { - // Get the rect for the placement. If this placement doesn't have - // 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.screens.active.pages.pointFromPin(.screen, rect.top_left).?.screen.y; - const img_bot_y = t.screens.active.pages.pointFromPin(.screen, rect.bottom_right).?.screen.y; - - // If the selection isn't within our viewport then skip it. - 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(alloc, image); - - // Calculate the dimensions of our image, taking in to - // 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; - - // 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 (dest_size.width > 0 and dest_size.height > 0) { - try self.kitty_placements.append(alloc, .{ - .image_id = .{ .kitty = image.id }, - .x = @intCast(rect.top_left.x), - .y = y_pos, - .z = p.z, - .width = dest_size.width, - .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, - }); - } - } - - fn prepKittyVirtualPlacement( - self: *State, - alloc: Allocator, - t: *const terminal.Terminal, - p: *const terminal.kitty.graphics.unicode.Placement, - cell_size: CellSize, - ) PrepImageError!void { - const storage = &t.screens.active.kitty_images; - const image = storage.imageById(p.image_id) orelse { - log.warn( - "missing image for virtual placement, ignoring image_id={}", - .{p.image_id}, - ); - return; - }; - - const rp = p.renderPlacement( - storage, - &image, - cell_size.width, - cell_size.height, - ) catch |err| { - log.warn("error rendering virtual placement err={}", .{err}); - return; - }; - - // If our placement is zero sized then we don't do anything. - if (rp.dest_width == 0 or rp.dest_height == 0) return; - - const viewport: terminal.point.Point = t.screens.active.pages.pointFromPin( - .viewport, - rp.top_left, - ) orelse { - // This is unreachable with virtual placements because we should - // only ever be looking at virtual placements that are in our - // viewport in the renderer and virtual placements only ever take - // up one row. - unreachable; - }; - - // Prepare the image for the GPU and store the placement. - try self.prepKittyImage(alloc, &image); - try self.kitty_placements.append(alloc, .{ - .image_id = .{ .kitty = image.id }, - .x = @intCast(rp.top_left.x), - .y = @intCast(viewport.viewport.y), - .z = -1, - .width = rp.dest_width, - .height = rp.dest_height, - .cell_offset_x = rp.offset_x, - .cell_offset_y = rp.offset_y, - .source_x = rp.source_x, - .source_y = rp.source_y, - .source_width = rp.source_width, - .source_height = rp.source_height, - }); - } - /// Prepare an image for upload to the GPU. fn prepImage( self: *State, alloc: Allocator, id: Id, generation: u64, - pending: Image.Pending, + source: Image.Source, ) PrepImageError!void { // If this image exists and its generation is the same it is the // identical image so we don't need to send it to the GPU. @@ -534,13 +278,14 @@ pub const State = struct { if (gop.found_existing and gop.value_ptr.generation == generation) { + gop.value_ptr.image.cancelUnload(); return; } // Copy the data so we own it. const data = if (alloc.dupe( u8, - pending.dataSlice(), + source.data, )) |v| v else |_| { if (!gop.found_existing) { // If this is a new entry we can just remove it since it @@ -554,69 +299,56 @@ pub const State = struct { return error.OutOfMemory; }; - // Note: we don't need to errdefer free the data because it is - // put into the map immediately below and our errdefer to - // handle our map state will fix this up. - // Store it in the map - const new_image: Image = .{ - .pending = .{ - .width = pending.width, - .height = pending.height, - .pixel_format = pending.pixel_format, - .data = data.ptr, - }, + var pending: Image.Pending = .{ + .width = source.width, + .height = source.height, + .pixel_format = source.pixel_format, + .backing = .{ .renderer_owned = data }, }; if (!gop.found_existing) { gop.value_ptr.* = .{ - .image = new_image, + .image = .{ .pending = pending.take() }, .generation = 0, }; } else { gop.value_ptr.image.markForReplace( alloc, - new_image, + &pending, ); } - - // If any error happens, we unload the image and it is invalid. - errdefer gop.value_ptr.image.markForUnload(); - - gop.value_ptr.image.prepForUpload(alloc) catch |err| { - log.warn("error preparing image for upload err={}", .{err}); - return error.ImageConversionError; - }; gop.value_ptr.generation = generation; } - /// Prepare the provided Kitty image for upload to the GPU by copying its - /// data with our allocator and setting it to the pending state. - fn prepKittyImage( + fn prepKittyImageRetained( self: *State, alloc: Allocator, image: *const terminal.kitty.graphics.Image, ) PrepImageError!void { - try self.prepImage( - alloc, - .{ .kitty = image.id }, - image.generation, - .{ - .width = image.width, - .height = image.height, - .pixel_format = switch (image.format) { - .gray => .gray, - .gray_alpha => .gray_alpha, - .rgb => .rgb, - .rgba => .rgba, - .png => unreachable, // should be decoded by now - }, - - // constCasts are always gross but this one is safe is because - // the data is only read from here and copied into its own - // buffer. - .data = @constCast(image.data.ptr), + const id: Id = .{ .kitty = image.id }; + if (self.images.getPtr(id)) |entry| { + if (entry.generation == image.generation) { + entry.image.cancelUnload(); + return; + } + } else try self.images.ensureUnusedCapacity(alloc, 1); + + const retained = image.retain(); + var pending: Image.Pending = .{ + .width = image.width, + .height = image.height, + .pixel_format = switch (image.format) { + .gray => .gray, + .gray_alpha => .gray_alpha, + .rgb => .rgb, + .rgba => .rgba, + .png => unreachable, }, - ); + .backing = .{ .kitty_retained = retained }, + }; + const gop = self.images.getOrPutAssumeCapacity(id); + if (gop.found_existing) gop.value_ptr.image.markForReplace(alloc, &pending) else gop.value_ptr.* = .{ .image = .{ .pending = pending.take() }, .generation = 0 }; + gop.value_ptr.generation = image.generation; } }; @@ -724,23 +456,79 @@ pub const Image = union(enum) { pending: Pending, }; + /// Non-owning source data used only while copying generic images into + /// renderer-owned pending storage. + pub const Source = struct { + height: u32, + width: u32, + pixel_format: Pending.PixelFormat, + data: []const u8, + }; + /// Pending image data that needs to be uploaded to the GPU. pub const Pending = struct { height: u32, width: u32, pixel_format: PixelFormat, - /// Data is always expected to be (width * height * bpp). - data: [*]u8, + /// Explicit ownership of the immutable CPU bytes. Retained Kitty data + /// must be released with the same allocator used by terminal storage. + backing: Backing, + + pub const Backing = union(enum) { + /// Moved-from sentinel. Live Pending values never use this. + none, - pub fn dataSlice(self: Pending) []u8 { - return self.data[0..self.len()]; + /// Mutable allocation owned by the renderer allocator. + renderer_owned: []u8, + + /// Immutable terminal image held through the core retain API. + kitty_retained: *terminal.kitty.graphics.Image, + + pub fn dataSlice(self: *const Backing) []const u8 { + return switch (self.*) { + .none => &.{}, + .renderer_owned => |data| data, + .kitty_retained => |retained| retained.data, + }; + } + + pub fn deinit(self: *Backing, alloc: Allocator) void { + switch (self.*) { + .none => {}, + .renderer_owned => |data| alloc.free(data), + .kitty_retained => self.kitty_retained.release(), + } + self.* = .none; + } + + pub fn isRetained(self: *const Backing) bool { + return self.* == .kitty_retained; + } + }; + + pub fn dataSlice(self: *const Pending) []const u8 { + const data = self.backing.dataSlice(); + assert(data.len == self.len()); + return data; } - pub fn len(self: Pending) usize { + pub fn len(self: *const Pending) usize { return self.width * self.height * self.pixel_format.bpp(); } + pub fn deinit(self: *Pending, alloc: Allocator) void { + self.backing.deinit(alloc); + } + + /// Explicitly move this ownership-bearing value and invalidate the + /// source so only the returned Pending may release its backing. + pub fn take(self: *Pending) Pending { + const result = self.*; + self.backing = .none; + return result; + } + pub const PixelFormat = enum { /// 1 byte per pixel grayscale. gray, @@ -769,72 +557,157 @@ pub const Image = union(enum) { }; }; - pub fn deinit(self: Image, alloc: Allocator) void { - switch (self) { - .pending, - .unload_pending, - => |p| alloc.free(p.dataSlice()), + pub fn deinit(self: *Image, alloc: Allocator) void { + switch (self.*) { + .pending => self.pending.deinit(alloc), + .unload_pending => self.unload_pending.deinit(alloc), - .replace, .unload_replace => |r| { - alloc.free(r.pending.dataSlice()); - r.texture.deinit(); + .replace => { + self.replace.pending.deinit(alloc); + self.replace.texture.deinit(); + }, + .unload_replace => { + self.unload_replace.pending.deinit(alloc); + self.unload_replace.texture.deinit(); }, - .ready, - .unload_ready, - => |t| t.deinit(), + .ready => self.ready.deinit(), + .unload_ready => self.unload_ready.deinit(), } + self.* = undefined; } /// Mark this image for unload whatever state it is in. pub fn markForUnload(self: *Image) void { - self.* = switch (self.*) { + switch (self.*) { .unload_pending, .unload_replace, .unload_ready, => return, - .ready => |t| .{ .unload_ready = t }, - .pending => |p| .{ .unload_pending = p }, - .replace => |r| .{ .unload_replace = r }, - }; + .ready => { + const texture = self.ready; + self.* = .{ .unload_ready = texture }; + }, + .pending => { + const pending = self.pending.take(); + self.* = .{ .unload_pending = pending }; + }, + .replace => { + const texture = self.replace.texture; + const pending = self.replace.pending.take(); + self.* = .{ .unload_replace = .{ + .texture = texture, + .pending = pending, + } }; + }, + } + } + + /// Cancel a pending unload without changing or duplicating ownership. + /// This is used when the same source generation becomes active again + /// before the renderer processes its unload transition. + fn cancelUnload(self: *Image) void { + switch (self.*) { + .pending, + .replace, + .ready, + => {}, + + .unload_pending => { + const pending = self.unload_pending.take(); + self.* = .{ .pending = pending }; + }, + .unload_ready => { + const texture = self.unload_ready; + self.* = .{ .ready = texture }; + }, + .unload_replace => { + const texture = self.unload_replace.texture; + const pending = self.unload_replace.pending.take(); + self.* = .{ .replace = .{ + .texture = texture, + .pending = pending, + } }; + }, + } } /// Mark the current image to be replaced with a pending one. This will /// attempt to update the existing texture if we have one, otherwise it /// will act like a new upload. - pub fn markForReplace(self: *Image, alloc: Allocator, img: Image) void { - assert(img.isPending()); - - // If we have pending data right now, free it. - if (self.getPending()) |p| { - alloc.free(p.dataSlice()); - } - // If we have an existing texture, use it in the replace. - if (self.getTexture()) |t| { - self.* = .{ .replace = .{ - .texture = t, - .pending = img.getPending().?, - } }; - return; + pub fn markForReplace( + self: *Image, + alloc: Allocator, + pending: *Pending, + ) void { + const replacement = pending.take(); + switch (self.*) { + .pending => { + self.pending.deinit(alloc); + self.* = .{ .pending = replacement }; + }, + .unload_pending => { + self.unload_pending.deinit(alloc); + self.* = .{ .pending = replacement }; + }, + .ready => { + const texture = self.ready; + self.* = .{ .replace = .{ + .texture = texture, + .pending = replacement, + } }; + }, + .unload_ready => { + const texture = self.unload_ready; + self.* = .{ .replace = .{ + .texture = texture, + .pending = replacement, + } }; + }, + .replace => { + self.replace.pending.deinit(alloc); + const texture = self.replace.texture; + self.* = .{ .replace = .{ + .texture = texture, + .pending = replacement, + } }; + }, + .unload_replace => { + self.unload_replace.pending.deinit(alloc); + const texture = self.unload_replace.texture; + self.* = .{ .replace = .{ + .texture = texture, + .pending = replacement, + } }; + }, } - // Otherwise we just become a pending image. - self.* = .{ .pending = img.getPending().? }; } /// Returns true if this image is pending upload. - pub fn isPending(self: Image) bool { - return self.getPending() != null; + pub fn isPending(self: *const Image) bool { + return self.getPendingPointerConst() != null; } /// Returns true if this image has an associated texture. - pub fn hasTexture(self: Image) bool { - return self.getTexture() != null; + pub fn hasTexture(self: *const Image) bool { + return switch (self.*) { + .ready, .unload_ready, .replace, .unload_replace => true, + .pending, .unload_pending => false, + }; + } + + fn textureForDraw(self: *const Image) ?Texture { + return switch (self.*) { + .ready => self.ready, + .unload_ready => self.unload_ready, + else => null, + }; } /// Returns true if this image is marked for unload. - pub fn isUnloading(self: Image) bool { - return switch (self) { + pub fn isUnloading(self: *const Image) bool { + return switch (self.*) { .unload_pending, .unload_replace, .unload_ready, @@ -866,8 +739,8 @@ pub const Image = union(enum) { .rgba => unreachable, .bgra => wuffs.swizzle.bgraToRgba(alloc, data), }; - alloc.free(data); - p.data = rgba.ptr; + p.backing.deinit(alloc); + p.backing = .{ .renderer_owned = rgba }; p.pixel_format = .rgba; } @@ -895,7 +768,7 @@ pub const Image = union(enum) { try self.prepForUpload(alloc); // Get our pending info - const p = self.getPending().?; + const p = self.getPendingPointerConst().?; // Create our texture const texture = Texture.init( @@ -915,40 +788,6 @@ pub const Image = union(enum) { self.* = .{ .ready = texture }; } - /// Returns any pending image data for this image that requires upload. - /// - /// If there is no pending data to upload, returns null. - fn getPending(self: Image) ?Pending { - return switch (self) { - .pending, - .unload_pending, - => |p| p, - - .replace, - .unload_replace, - => |r| r.pending, - - else => null, - }; - } - - /// Returns the texture for this image. - /// - /// If there is no texture for it yet, returns null. - fn getTexture(self: Image) ?Texture { - return switch (self) { - .ready, - .unload_ready, - => |t| t, - - .replace, - .unload_replace, - => |r| r.texture, - - else => null, - }; - } - // Same as getPending but returns a pointer instead of a copy. fn getPendingPointer(self: *Image) ?*Pending { return switch (self.*) { @@ -961,4 +800,394 @@ pub const Image = union(enum) { else => null, }; } + + fn getPendingPointerConst(self: *const Image) ?*const Pending { + return switch (self.*) { + .pending => &self.pending, + .unload_pending => &self.unload_pending, + + .replace => &self.replace.pending, + .unload_replace => &self.unload_replace.pending, + + else => null, + }; + } }; + +/// Test-only convenience for constructing and transferring a Kitty image. +fn addTestImage( + storage: *terminal.kitty.graphics.ImageStorage, + alloc: Allocator, + init: terminal.kitty.graphics.Image.Init, +) Allocator.Error!void { + const image = try terminal.kitty.graphics.Image.create(alloc, init); + errdefer image.release(); + try storage.addImage(alloc, image); +} + +test "renderer kitty snapshot transfers retained images without copying" { + const testing = std.testing; + const alloc = testing.allocator; + + var term = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer term.deinit(alloc); + term.width_px = 3; + term.height_px = 3; + const storage = &term.screens.active.kitty_images; + try addTestImage(storage, alloc, .{ + .id = 1, + .width = 1, + .height = 1, + .format = .rgba, + .data = try alloc.dupe(u8, &.{ 1, 2, 3, 4 }), + }); + try addTestImage(storage, alloc, .{ + .id = 2, + .width = 1, + .height = 1, + .format = .rgba, + .data = try alloc.dupe(u8, &.{ 5, 6, 7, 8 }), + }); + const pin = try term.screens.active.pages.trackPin( + term.screens.active.pages.pin(.{ .active = .{} }).?, + ); + try storage.addPlacement(alloc, 1, 1, .{ + .location = .{ .pin = pin }, + .columns = 1, + .rows = 1, + }); + + var render_state = terminal.RenderState.init(.{ .kitty_graphics = true }); + defer render_state.deinit(alloc); + try render_state.update(alloc, &term); + const snapshot_image = render_state.kitty.images.get(1).?; + try testing.expectEqual(@as(usize, 2), snapshot_image.refs.load(.monotonic)); + + var state: State = .empty; + defer state.deinit(alloc); + try testing.expect(state.kittyUpdate(alloc, &render_state)); + try testing.expect(state.images.getPtr(.{ .kitty = 2 }) == null); + const pending = state.images.getPtr(.{ .kitty = 1 }).?.image.getPendingPointerConst().?; + try testing.expectEqual(snapshot_image.data.ptr, pending.dataSlice().ptr); + try testing.expectEqual(@as(usize, 3), snapshot_image.refs.load(.monotonic)); + + try testing.expect(state.kittyUpdate(alloc, &render_state)); + try testing.expectEqual(@as(usize, 3), snapshot_image.refs.load(.monotonic)); +} + +test "renderer kitty prep retains payload without copy or duplicate retain" { + const testing = std.testing; + const alloc = testing.allocator; + + var term = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer term.deinit(alloc); + + var storage: terminal.kitty.graphics.ImageStorage = .{}; + defer storage.deinit(alloc, term.screens.active); + + try addTestImage(&storage, alloc, .{ + .id = 1, + .width = 1, + .height = 1, + .format = .rgb, + .data = try alloc.dupe(u8, &.{ 1, 2, 3 }), + }); + const borrowed = storage.imageById(1).?; + + var state: State = .empty; + defer state.deinit(alloc); + try state.prepKittyImageRetained(alloc, borrowed); + + const pending = state.images.getPtr(.{ .kitty = 1 }).?.image.getPendingPointerConst().?; + try testing.expectEqual(borrowed.data.ptr, pending.dataSlice().ptr); + const payload = pending.backing.kitty_retained; + try testing.expectEqual(@as(usize, 2), payload.refs.load(.monotonic)); + + var failing = testing.FailingAllocator.init(alloc, .{ .fail_index = 0 }); + try state.prepKittyImageRetained(failing.allocator(), borrowed); + try testing.expect(!failing.has_induced_failure); + try testing.expectEqual(@as(usize, 2), payload.refs.load(.monotonic)); +} + +test "renderer kitty same generation cancels pending unload without retaining again" { + const testing = std.testing; + const alloc = testing.allocator; + + var term = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer term.deinit(alloc); + + var storage: terminal.kitty.graphics.ImageStorage = .{}; + defer storage.deinit(alloc, term.screens.active); + + try addTestImage(&storage, alloc, .{ + .id = 1, + .width = 1, + .height = 1, + .format = .rgba, + .data = try alloc.dupe(u8, &.{ 1, 2, 3, 4 }), + }); + const borrowed = storage.imageById(1).?; + + var state: State = .empty; + defer state.deinit(alloc); + try state.prepKittyImageRetained(alloc, borrowed); + + const renderer_image = &state.images.getPtr(.{ .kitty = 1 }).?.image; + const pending_before = renderer_image.getPendingPointerConst().?; + const data_ptr = pending_before.dataSlice().ptr; + const payload = pending_before.backing.kitty_retained; + renderer_image.markForUnload(); + try testing.expect(renderer_image.isUnloading()); + + var failing = testing.FailingAllocator.init(alloc, .{ .fail_index = 0 }); + try state.prepKittyImageRetained(failing.allocator(), borrowed); + try testing.expect(!failing.has_induced_failure); + try testing.expect(!renderer_image.isUnloading()); + try testing.expect(renderer_image.isPending()); + try testing.expectEqual( + data_ptr, + renderer_image.getPendingPointerConst().?.dataSlice().ptr, + ); + try testing.expectEqual(@as(usize, 2), payload.refs.load(.monotonic)); +} + +test "renderer kitty retained pending survives replacement and releases old payload" { + const testing = std.testing; + const alloc = testing.allocator; + + var term = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer term.deinit(alloc); + + var storage: terminal.kitty.graphics.ImageStorage = .{}; + defer storage.deinit(alloc, term.screens.active); + + try addTestImage(&storage, alloc, .{ + .id = 1, + .width = 1, + .height = 1, + .format = .rgba, + .data = try alloc.dupe(u8, &.{ 1, 2, 3, 4 }), + }); + const original = storage.imageById(1).?; + + var state: State = .empty; + defer state.deinit(alloc); + try state.prepKittyImageRetained(alloc, original); + const old_pending = state.images.getPtr(.{ .kitty = 1 }).?.image.getPendingPointerConst().?; + try testing.expectEqualSlices(u8, &.{ 1, 2, 3, 4 }, old_pending.dataSlice()); + const old_observer = original.retain(); + defer old_observer.release(); + const old_payload = old_observer; + try testing.expectEqual(@as(usize, 3), old_payload.refs.load(.monotonic)); + + try addTestImage(&storage, alloc, .{ + .id = 1, + .width = 1, + .height = 1, + .format = .rgba, + .data = try alloc.dupe(u8, &.{ 5, 6, 7, 8 }), + }); + try testing.expectEqualSlices(u8, &.{ 1, 2, 3, 4 }, old_pending.dataSlice()); + try testing.expectEqual(@as(usize, 2), old_payload.refs.load(.monotonic)); + + const replacement = storage.imageById(1).?; + try state.prepKittyImageRetained(alloc, replacement); + try testing.expectEqual(@as(usize, 1), old_payload.refs.load(.monotonic)); + const new_pending = state.images.getPtr(.{ .kitty = 1 }).?.image.getPendingPointerConst().?; + try testing.expectEqual(replacement.data.ptr, new_pending.dataSlice().ptr); + try testing.expectEqualSlices(u8, &.{ 5, 6, 7, 8 }, new_pending.dataSlice()); +} + +test "renderer kitty retained pending survives deletion" { + const testing = std.testing; + const alloc = testing.allocator; + + var term = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer term.deinit(alloc); + + var storage: terminal.kitty.graphics.ImageStorage = .{}; + defer storage.deinit(alloc, term.screens.active); + + try addTestImage(&storage, alloc, .{ + .id = 1, + .width = 1, + .height = 1, + .format = .rgba, + .data = try alloc.dupe(u8, &.{ 1, 2, 3, 4 }), + }); + const borrowed = storage.imageById(1).?; + + var state: State = .empty; + defer state.deinit(alloc); + try state.prepKittyImageRetained(alloc, borrowed); + const pending = state.images.getPtr(.{ .kitty = 1 }).?.image.getPendingPointerConst().?; + + storage.delete(alloc, &term, .{ .id = .{ .image_id = 1, .delete = true } }); + try testing.expect(storage.imageById(1) == null); + try testing.expectEqualSlices(u8, &.{ 1, 2, 3, 4 }, pending.dataSlice()); +} + +test "renderer kitty retained pending survives eviction" { + const testing = std.testing; + const alloc = testing.allocator; + + var term = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer term.deinit(alloc); + + var storage: terminal.kitty.graphics.ImageStorage = .{ .total_limit = 4 }; + defer storage.deinit(alloc, term.screens.active); + + try addTestImage(&storage, alloc, .{ + .id = 1, + .width = 1, + .height = 1, + .format = .rgba, + .data = try alloc.dupe(u8, &.{ 1, 2, 3, 4 }), + }); + const borrowed = storage.imageById(1).?; + + var state: State = .empty; + defer state.deinit(alloc); + try state.prepKittyImageRetained(alloc, borrowed); + const pending = state.images.getPtr(.{ .kitty = 1 }).?.image.getPendingPointerConst().?; + + try addTestImage(&storage, alloc, .{ + .id = 2, + .width = 1, + .height = 1, + .format = .rgba, + .data = try alloc.dupe(u8, &.{ 5, 6, 7, 8 }), + }); + try testing.expect(storage.imageById(1) == null); + try testing.expectEqualSlices(u8, &.{ 1, 2, 3, 4 }, pending.dataSlice()); +} + +test "renderer kitty retained pending survives storage deinit" { + const testing = std.testing; + const alloc = testing.allocator; + + var term = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer term.deinit(alloc); + + var storage: terminal.kitty.graphics.ImageStorage = .{}; + var storage_live = true; + defer if (storage_live) storage.deinit(alloc, term.screens.active); + + try addTestImage(&storage, alloc, .{ + .id = 1, + .width = 1, + .height = 1, + .format = .rgba, + .data = try alloc.dupe(u8, &.{ 1, 2, 3, 4 }), + }); + const borrowed = storage.imageById(1).?; + + var state: State = .empty; + defer state.deinit(alloc); + try state.prepKittyImageRetained(alloc, borrowed); + const pending = state.images.getPtr(.{ .kitty = 1 }).?.image.getPendingPointerConst().?; + + storage.deinit(alloc, term.screens.active); + storage_live = false; + try testing.expectEqualSlices(u8, &.{ 1, 2, 3, 4 }, pending.dataSlice()); +} + +test "renderer kitty RGB conversion releases retained source" { + const testing = std.testing; + const alloc = testing.allocator; + + var term = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer term.deinit(alloc); + + var storage: terminal.kitty.graphics.ImageStorage = .{}; + defer storage.deinit(alloc, term.screens.active); + + try addTestImage(&storage, alloc, .{ + .id = 1, + .width = 1, + .height = 1, + .format = .rgb, + .data = try alloc.dupe(u8, &.{ 1, 2, 3 }), + }); + const borrowed = storage.imageById(1).?; + + var state: State = .empty; + defer state.deinit(alloc); + try state.prepKittyImageRetained(alloc, borrowed); + + const image = &state.images.getPtr(.{ .kitty = 1 }).?.image; + const retained = image.getPendingPointerConst().?; + const payload = retained.backing.kitty_retained; + try testing.expectEqual(@as(usize, 2), payload.refs.load(.monotonic)); + + try image.prepForUpload(alloc); + const converted = image.getPendingPointerConst().?; + try testing.expectEqual(Image.Pending.PixelFormat.rgba, converted.pixel_format); + try testing.expectEqualSlices(u8, &.{ 1, 2, 3, 255 }, converted.dataSlice()); + try testing.expect(!converted.backing.isRetained()); + try testing.expectEqual(@as(usize, 1), payload.refs.load(.monotonic)); +} + +test "renderer kitty RGBA remains retained without an intermediate copy" { + const testing = std.testing; + const alloc = testing.allocator; + + var term = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer term.deinit(alloc); + + var storage: terminal.kitty.graphics.ImageStorage = .{}; + defer storage.deinit(alloc, term.screens.active); + + try addTestImage(&storage, alloc, .{ + .id = 1, + .width = 1, + .height = 1, + .format = .rgba, + .data = try alloc.dupe(u8, &.{ 1, 2, 3, 4 }), + }); + const borrowed = storage.imageById(1).?; + + var state: State = .empty; + try state.prepKittyImageRetained(alloc, borrowed); + const image = &state.images.getPtr(.{ .kitty = 1 }).?.image; + const payload = image.getPendingPointerConst().?.backing.kitty_retained; + + try image.prepForUpload(alloc); + const pending = image.getPendingPointerConst().?; + try testing.expect(pending.backing.isRetained()); + try testing.expectEqual(borrowed.data.ptr, pending.dataSlice().ptr); + try testing.expectEqual(@as(usize, 2), payload.refs.load(.monotonic)); + + state.deinit(alloc); + try testing.expectEqual(@as(usize, 1), payload.refs.load(.monotonic)); +} + +test "renderer deinit and pending unload release retained Kitty payloads" { + const testing = std.testing; + const alloc = testing.allocator; + + var term = try terminal.Terminal.init(alloc, .{ .rows = 3, .cols = 3 }); + defer term.deinit(alloc); + + var storage: terminal.kitty.graphics.ImageStorage = .{}; + defer storage.deinit(alloc, term.screens.active); + + try addTestImage(&storage, alloc, .{ + .id = 1, + .width = 1, + .height = 1, + .format = .rgba, + .data = try alloc.dupe(u8, &.{ 1, 2, 3, 4 }), + }); + const borrowed = storage.imageById(1).?; + + var state: State = .empty; + try state.prepKittyImageRetained(alloc, borrowed); + const image = &state.images.getPtr(.{ .kitty = 1 }).?.image; + const payload = image.getPendingPointerConst().?.backing.kitty_retained; + image.markForUnload(); + try testing.expect(image.isUnloading()); + + state.deinit(alloc); + try testing.expectEqual(@as(usize, 1), payload.refs.load(.monotonic)); +} diff --git a/src/renderer/metal/Texture.zig b/src/renderer/metal/Texture.zig index 5042919ac3c..aaa9b23ae2a 100644 --- a/src/renderer/metal/Texture.zig +++ b/src/renderer/metal/Texture.zig @@ -36,7 +36,10 @@ pub const Error = error{ MetalFailed, }; -/// Initialize a texture +/// Initialize a texture. +/// +/// Metal's `replaceRegion:...withBytes:` synchronously copies `data`, so the +/// caller may release the CPU bytes after this returns. pub fn init( opts: Options, width: usize, diff --git a/src/renderer/opengl/Texture.zig b/src/renderer/opengl/Texture.zig index c37ec6866d0..4f7c33efa4f 100644 --- a/src/renderer/opengl/Texture.zig +++ b/src/renderer/opengl/Texture.zig @@ -38,7 +38,10 @@ pub const Error = error{ OpenGLFailed, }; -/// Initialize a texture +/// Initialize a texture. +/// +/// `image2D` synchronously consumes client memory. This renderer does not use +/// a pixel-unpack buffer, so `data` may be released after this returns. pub fn init( opts: Options, width: usize, diff --git a/src/terminal/render.zig b/src/terminal/render.zig index 01ded1d6bc5..5a89507f371 100644 --- a/src/terminal/render.zig +++ b/src/terminal/render.zig @@ -1,5 +1,6 @@ const std = @import("std"); const assert = @import("../quirks.zig").inlineAssert; +const build_options = @import("terminal_options"); const Allocator = std.mem.Allocator; const ArenaAllocator = std.heap.ArenaAllocator; const fastmem = @import("../fastmem.zig"); @@ -16,6 +17,7 @@ const Screen = @import("Screen.zig"); const ScreenSet = @import("ScreenSet.zig"); const Style = @import("style.zig").Style; const Terminal = @import("Terminal.zig"); +const kitty = @import("kitty.zig"); // Developer note: this is in src/terminal and not src/renderer because // the goal is that this remains generic to multiple renderers. This can @@ -70,6 +72,10 @@ const Terminal = @import("Terminal.zig"); /// waste, it is recommended that the caller `deinit` and start with an /// empty render state every so often. pub const RenderState = struct { + pub const Options = struct { kitty_graphics: bool = false }; + + options: Options, + kitty: KittySnapshot, /// The current screen dimensions. It is possible that these don't match /// the renderer's current dimensions in grid cells because resizing /// can happen asynchronously. For example, for Metal, our NSView resizes @@ -124,8 +130,46 @@ pub const RenderState = struct { /// beginUpdate. pending_styles: std.ArrayList(StyleRun) = .empty, + pub const KittySnapshot = if (build_options.kitty_graphics) struct { + images: std.AutoHashMapUnmanaged(u32, *kitty.graphics.Image) = .{}, + placements: std.ArrayListUnmanaged(Placement) = .{}, + generation: u64 = 0, + bg_end: u32 = 0, + text_end: u32 = 0, + virtual: bool = false, + dirty: bool = false, + + pub const Placement = struct { + image_id: u32, + x: i32, + y: i32, + z: i32, + width: u32, + height: u32, + cell_offset_x: u32, + cell_offset_y: u32, + source_x: u32, + source_y: u32, + source_width: u32, + source_height: u32, + }; + + fn deinit(self: *KittySnapshot, alloc: Allocator) void { + var it = self.images.valueIterator(); + while (it.next()) |image| image.*.release(); + self.images.deinit(alloc); + self.placements.deinit(alloc); + } + } else struct { + dirty: bool = false, + + fn deinit(_: *@This(), _: Allocator) void {} + }; + /// Initial state. pub const empty: RenderState = .{ + .options = .{}, + .kitty = .{}, .rows = 0, .cols = 0, .colors = .{ @@ -149,6 +193,12 @@ pub const RenderState = struct { .screen = .primary, }; + pub fn init(options: Options) RenderState { + var result: RenderState = .empty; + result.options = options; + return result; + } + /// The color state for the terminal. /// /// The background/foreground will be reversed if the terminal reverse @@ -295,6 +345,7 @@ pub const RenderState = struct { }; pub fn deinit(self: *RenderState, alloc: Allocator) void { + self.kitty.deinit(alloc); for ( self.row_data.items(.arena), self.row_data.items(.cells), @@ -353,6 +404,7 @@ pub const RenderState = struct { t: *Terminal, ) Allocator.Error!void { const s: *Screen = t.screens.active; + try self.updateKitty(alloc, t); const viewport_pin = s.pages.getTopLeft(.viewport); const redraw = redraw: { // If our screen key changed, we need to do a full rebuild @@ -704,6 +756,94 @@ pub const RenderState = struct { s.dirty = .{}; } + fn updateKitty(self: *RenderState, alloc: Allocator, t: *Terminal) Allocator.Error!void { + if (comptime !build_options.kitty_graphics) return; + if (!self.options.kitty_graphics) return; + const storage = &t.screens.active.kitty_images; + if (!storage.dirty and + !self.kitty.virtual and + self.kitty.generation == storage.generation) return; + + var next: KittySnapshot = .{ .generation = storage.generation }; + errdefer next.deinit(alloc); + var images = storage.images.iterator(); + while (images.next()) |entry| { + try next.images.ensureUnusedCapacity(alloc, 1); + const retained = entry.value_ptr.*.retain(); + next.images.putAssumeCapacity(entry.key_ptr.*, retained); + } + + const pages = &t.screens.active.pages; + const top = pages.getTopLeft(.viewport); + const bot = pages.getBottomRight(.viewport).?; + const top_y = pages.pointFromPin(.screen, top).?.screen.y; + const bot_y = pages.pointFromPin(.screen, bot).?.screen.y; + var placements = storage.placements.iterator(); + while (placements.next()) |entry| { + const p = entry.value_ptr; + if (p.location == .virtual) { + next.virtual = true; + continue; + } + const image = next.images.get(entry.key_ptr.image_id) orelse continue; + const rect = p.rect(image, t) orelse continue; + const image_top = pages.pointFromPin(.screen, rect.top_left).?.screen.y; + const image_bot = pages.pointFromPin(.screen, rect.bottom_right).?.screen.y; + if (image_top > bot_y or image_bot < top_y) continue; + const dest = p.pixelSize(image, t); + if (dest.width == 0 or dest.height == 0) continue; + const sx = @min(image.width, p.source_x); + const sy = @min(image.height, p.source_y); + try next.placements.append(alloc, .{ + .image_id = image.id, + .x = @intCast(rect.top_left.x), + .y = @as(i32, @intCast(image_top)) - @as(i32, @intCast(top_y)), + .z = p.z, + .width = dest.width, + .height = dest.height, + .cell_offset_x = p.x_offset, + .cell_offset_y = p.y_offset, + .source_x = sx, + .source_y = sy, + .source_width = if (p.source_width > 0) @min(image.width - sx, p.source_width) else image.width, + .source_height = if (p.source_height > 0) @min(image.height - sy, p.source_height) else image.height, + }); + } + const cell_width = if (pages.cols > 0) t.width_px / pages.cols else 0; + const cell_height = if (pages.rows > 0) t.height_px / pages.rows else 0; + if (next.virtual and cell_width > 0 and cell_height > 0) { + var vit = kitty.graphics.unicode.placementIterator(top, bot); + while (vit.next()) |vp| { + const image = next.images.get(vp.image_id) orelse continue; + const rp = vp.renderPlacement( + storage, + image, + cell_width, + cell_height, + ) catch continue; + if (rp.dest_width == 0 or rp.dest_height == 0) continue; + const viewport = pages.pointFromPin(.viewport, rp.top_left) orelse continue; + try next.placements.append(alloc, .{ .image_id = image.id, .x = @intCast(rp.top_left.x), .y = @intCast(viewport.viewport.y), .z = -1, .width = rp.dest_width, .height = rp.dest_height, .cell_offset_x = rp.offset_x, .cell_offset_y = rp.offset_y, .source_x = rp.source_x, .source_y = rp.source_y, .source_width = rp.source_width, .source_height = rp.source_height }); + } + } + std.mem.sortUnstable(KittySnapshot.Placement, next.placements.items, {}, struct { + fn lessThan(_: void, a: KittySnapshot.Placement, b: KittySnapshot.Placement) bool { + return a.z < b.z or (a.z == b.z and a.image_id < b.image_id); + } + }.lessThan); + const bg_limit = std.math.minInt(i32) / 2; + next.bg_end = @intCast(next.placements.items.len); + next.text_end = @intCast(next.placements.items.len); + for (next.placements.items, 0..) |p, i| { + if (next.bg_end == next.placements.items.len and p.z >= bg_limit) next.bg_end = @intCast(i); + if (next.text_end == next.placements.items.len and p.z >= 0) next.text_end = @intCast(i); + } + next.dirty = true; + self.kitty.deinit(alloc); + self.kitty = next; + storage.dirty = false; + } + /// Complete a prior `beginUpdate` call by performing any deferred /// work. At the time of writing, this denormalizes the pending /// style runs into the per-cell style data. @@ -2098,3 +2238,137 @@ test "dirty row resets highlights" { try testing.expectEqual(0, row_highlights[0].items.len); } } + +fn addKittyTestImage( + storage: *kitty.graphics.ImageStorage, + alloc: Allocator, + init_options: kitty.graphics.Image.Init, +) Allocator.Error!*const kitty.graphics.Image { + const image = try kitty.graphics.Image.create(alloc, init_options); + errdefer image.release(); + try storage.addImage(alloc, image); + return image; +} + +test "render state kitty opt-out does not consume storage" { + const testing = std.testing; + const alloc = testing.allocator; + + var t = try Terminal.init(alloc, .{ .cols = 3, .rows = 3 }); + defer t.deinit(alloc); + + const image = try addKittyTestImage(&t.screens.active.kitty_images, alloc, .{ + .id = 1, + .width = 1, + .height = 1, + .format = .rgba, + .data = try alloc.dupe(u8, &.{ 1, 2, 3, 4 }), + }); + + var state: RenderState = .empty; + defer state.deinit(alloc); + try state.update(alloc, &t); + + try testing.expect(t.screens.active.kitty_images.dirty); + try testing.expect(!state.kitty.dirty); + try testing.expectEqual(@as(usize, 0), state.kitty.images.count()); + try testing.expectEqual(@as(usize, 1), image.refs.load(.monotonic)); +} + +test "render state kitty snapshot retains images across terminal mutation" { + const testing = std.testing; + const alloc = testing.allocator; + + var t = try Terminal.init(alloc, .{ .cols = 3, .rows = 3 }); + defer t.deinit(alloc); + const storage = &t.screens.active.kitty_images; + + const old = try addKittyTestImage(storage, alloc, .{ + .id = 1, + .width = 1, + .height = 1, + .format = .rgba, + .data = try alloc.dupe(u8, &.{ 1, 2, 3, 4 }), + }); + + var state = RenderState.init(.{ .kitty_graphics = true }); + defer state.deinit(alloc); + try state.update(alloc, &t); + try testing.expect(state.kitty.dirty); + try testing.expect(!storage.dirty); + try testing.expectEqual(old, state.kitty.images.get(1).?); + + const observer = old.retain(); + defer observer.release(); + const replacement = try addKittyTestImage(storage, alloc, .{ + .id = 1, + .width = 1, + .height = 1, + .format = .rgba, + .data = try alloc.dupe(u8, &.{ 5, 6, 7, 8 }), + }); + + try testing.expectEqualSlices(u8, &.{ 1, 2, 3, 4 }, state.kitty.images.get(1).?.data); + try state.update(alloc, &t); + try testing.expectEqual(replacement, state.kitty.images.get(1).?); + try testing.expectEqualSlices(u8, &.{ 1, 2, 3, 4 }, observer.data); + + const generation = state.kitty.generation; + state.deinit(alloc); + state = RenderState.init(.{ .kitty_graphics = true }); + try state.update(alloc, &t); + try testing.expect(state.kitty.dirty); + try testing.expectEqual(generation, state.kitty.generation); + try testing.expectEqual(replacement, state.kitty.images.get(1).?); + + const current = replacement.retain(); + defer current.release(); + storage.delete(alloc, &t, .{ .id = .{ .image_id = 1, .delete = true } }); + try testing.expectEqualSlices(u8, &.{ 5, 6, 7, 8 }, state.kitty.images.get(1).?.data); + try state.update(alloc, &t); + try testing.expectEqual(@as(usize, 0), state.kitty.images.count()); + try testing.expectEqualSlices(u8, &.{ 5, 6, 7, 8 }, current.data); +} + +test "render state kitty snapshot computes placement geometry" { + const testing = std.testing; + const alloc = testing.allocator; + + var t = try Terminal.init(alloc, .{ .cols = 3, .rows = 3 }); + defer t.deinit(alloc); + t.width_px = 30; + t.height_px = 60; + const storage = &t.screens.active.kitty_images; + + _ = try addKittyTestImage(storage, alloc, .{ + .id = 1, + .width = 1, + .height = 2, + .format = .rgb, + .data = try alloc.dupe(u8, &.{ 1, 2, 3, 4, 5, 6 }), + }); + const pin = try t.screens.active.pages.trackPin( + t.screens.active.pages.pin(.{ .active = .{ .x = 1, .y = 1 } }).?, + ); + try storage.addPlacement(alloc, 1, 1, .{ + .location = .{ .pin = pin }, + .columns = 2, + .rows = 1, + .z = -1, + }); + + var state = RenderState.init(.{ .kitty_graphics = true }); + defer state.deinit(alloc); + try state.update(alloc, &t); + + 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.image_id); + try testing.expectEqual(@as(i32, 1), placement.x); + try testing.expectEqual(@as(i32, 1), placement.y); + try testing.expectEqual(@as(i32, -1), placement.z); + try testing.expectEqual(@as(u32, 20), placement.width); + try testing.expectEqual(@as(u32, 20), placement.height); + try testing.expectEqual(@as(u32, 1), placement.source_width); + try testing.expectEqual(@as(u32, 2), placement.source_height); +}