From bbcb2f96c8ad9ac62370c54ed11590e6de876bef Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 20 Aug 2023 19:28:39 -0700 Subject: [PATCH] terminal/kitty-gfx: huge progress on chunked transfers, lots of issues --- src/terminal/kitty/graphics_exec.zig | 62 ++++++++- src/terminal/kitty/graphics_image.zig | 165 ++++++++++++++++++------ src/terminal/kitty/graphics_storage.zig | 21 ++- src/termio/Exec.zig | 6 +- 4 files changed, 200 insertions(+), 54 deletions(-) diff --git a/src/terminal/kitty/graphics_exec.zig b/src/terminal/kitty/graphics_exec.zig index 005ee39a5..115e26e96 100644 --- a/src/terminal/kitty/graphics_exec.zig +++ b/src/terminal/kitty/graphics_exec.zig @@ -8,11 +8,14 @@ const command = @import("graphics_command.zig"); const image = @import("graphics_image.zig"); const Command = command.Command; const Response = command.Response; +const ChunkedImage = image.ChunkedImage; const Image = image.Image; +const log = std.log.scoped(.kitty_gfx); + // TODO: -// - image ids need to be assigned, can't just be zero // - delete +// - zlib deflate compression // (not exhaustive, almost every op is ignoring additional config) /// Execute a Kitty graphics command against the given terminal. This @@ -29,6 +32,7 @@ pub fn execute( cmd: *Command, ) ?Response { _ = buf; + log.debug("executing kitty graphics command: {}", .{cmd.control}); const resp_: ?Response = switch (cmd.control) { .query => query(alloc, cmd), @@ -45,6 +49,10 @@ pub fn execute( // Handle the quiet settings if (resp_) |resp| { + if (!resp.ok()) { + log.warn("erroneous kitty graphics response: {s}", .{resp.message}); + } + return switch (cmd.quiet) { .no => resp, .ok => if (resp.ok()) null else resp, @@ -111,6 +119,11 @@ fn transmit( // After the image is added, set the ID in case it changed result.id = img.id; + // If this is a transmit_and_display then the display part needs the image ID + if (cmd.control == .transmit_and_display) { + cmd.control.transmit_and_display.display.image_id = img.id; + } + return result; } @@ -173,6 +186,11 @@ fn transmitAndDisplay( ) Response { const resp = transmit(alloc, terminal, cmd); if (!resp.ok()) return resp; + + // If the transmission is chunked, we defer the display + const t = cmd.transmission().?; + if (t.more_chunks) return resp; + return display(alloc, terminal, cmd); } @@ -181,20 +199,51 @@ fn loadAndAddImage( terminal: *Terminal, cmd: *Command, ) !Image { - // Load the image - var img = try Image.load(alloc, cmd); + const t = cmd.transmission().?; + const storage = &terminal.screen.kitty_images; + + // Determine our image. This also handles chunking and early exit. + var img = if (storage.chunk) |chunk| img: { + try chunk.data.appendSlice(alloc, cmd.data); + _ = cmd.toOwnedData(); + + // If we have more then we're done + if (t.more_chunks) return chunk.image; + + // We have no more chunks. Complete and validate the image. + // At this point no matter what we want to clear out our chunked + // state. If we hit a validation error or something we don't want + // the chunked image hanging around in-memory. + defer { + chunk.destroy(alloc); + storage.chunk = null; + } + + break :img try chunk.complete(alloc); + } else try Image.load(alloc, cmd); errdefer img.deinit(alloc); // If the image has no ID, we assign one - const storage = &terminal.screen.kitty_images; if (img.id == 0) { img.id = storage.next_id; storage.next_id +%= 1; } - // Store our image - try storage.addImage(alloc, img); + // If this is chunked, this is the beginning of a new chunked transmission. + // (We checked for an in-progress chunk above.) + if (t.more_chunks) { + // We allocate the chunk on the heap because its rare and we + // don't want to always pay the memory cost to keep it around. + const chunk_ptr = try alloc.create(ChunkedImage); + errdefer alloc.destroy(chunk_ptr); + chunk_ptr.* = try ChunkedImage.init(alloc, img); + storage.chunk = chunk_ptr; + return img; + } + // Validate and store our image + try img.validate(); + try storage.addImage(alloc, img); return img; } @@ -206,6 +255,7 @@ fn encodeError(r: *Response, err: EncodeableError) void { error.OutOfMemory => r.message = "ENOMEM: out of memory", error.InvalidData => r.message = "EINVAL: invalid data", error.UnsupportedFormat => r.message = "EINVAL: unsupported format", + error.UnsupportedMedium => r.message = "EINVAL: unsupported medium", error.DimensionsRequired => r.message = "EINVAL: dimensions required", error.DimensionsTooLarge => r.message = "EINVAL: dimensions too large", } diff --git a/src/terminal/kitty/graphics_image.zig b/src/terminal/kitty/graphics_image.zig index 5ac7cf095..dbda94c3a 100644 --- a/src/terminal/kitty/graphics_image.zig +++ b/src/terminal/kitty/graphics_image.zig @@ -8,64 +8,151 @@ const command = @import("graphics_command.zig"); /// Maximum width or height of an image. Taken directly from Kitty. const max_dimension = 10000; +/// A chunked image is an image that is in-progress and being constructed +/// using chunks (the "m" parameter in the protocol). +pub const ChunkedImage = struct { + /// The in-progress image. The first chunk must have all the metadata + /// so this comes from that initially. + image: Image, + + /// The data that is being built up. + data: std.ArrayListUnmanaged(u8) = .{}, + + /// Initialize a chunked image from the first image part. + pub fn init(alloc: Allocator, image: Image) !ChunkedImage { + // Copy our initial set of data + var data = try std.ArrayListUnmanaged(u8).initCapacity(alloc, image.data.len * 2); + errdefer data.deinit(alloc); + try data.appendSlice(alloc, image.data); + + // Set data to empty so it doesn't get freed. + var result: ChunkedImage = .{ .image = image, .data = data }; + result.image.data = ""; + return result; + } + + pub fn deinit(self: *ChunkedImage, alloc: Allocator) void { + self.image.deinit(alloc); + self.data.deinit(alloc); + } + + pub fn destroy(self: *ChunkedImage, alloc: Allocator) void { + self.deinit(alloc); + alloc.destroy(self); + } + + /// Complete the chunked image, returning a completed image. + pub fn complete(self: *ChunkedImage, alloc: Allocator) !Image { + var result = self.image; + result.data = try self.data.toOwnedSlice(alloc); + self.image = .{}; + return result; + } +}; + +/// Image represents a single fully loaded image. pub const Image = struct { id: u32 = 0, number: u32 = 0, - data: []const u8, + width: u32 = 0, + height: u32 = 0, + format: Format = .rgb, + data: []const u8 = "", + + pub const Format = enum { rgb, rgba }; pub const Error = error{ InvalidData, DimensionsRequired, DimensionsTooLarge, UnsupportedFormat, + UnsupportedMedium, }; - /// Load an image from a transmission. The data in the command will be - /// owned by the image if successful. Note that you still must deinit - /// the command, all the state change will be done internally. - pub fn load(alloc: Allocator, cmd: *command.Command) !Image { - _ = alloc; - - const t = cmd.transmission().?; - const img = switch (t.format) { - .rgb => try loadPacked(3, t, cmd.data), - .rgba => try loadPacked(4, t, cmd.data), - else => return error.UnsupportedFormat, + /// Validate that the image appears valid. + pub fn validate(self: *const Image) !void { + const bpp: u32 = switch (self.format) { + .rgb => 3, + .rgba => 4, }; - // If we loaded an image successfully then we take ownership - // of the command data. - _ = cmd.toOwnedData(); - - return img; - } - - /// Load a package image format, i.e. RGB or RGBA. - fn loadPacked( - comptime bpp: comptime_int, - t: command.Transmission, - data: []const u8, - ) !Image { - if (t.width == 0 or t.height == 0) return error.DimensionsRequired; - if (t.width > max_dimension or t.height > max_dimension) return error.DimensionsTooLarge; + // Validate our dimensions. + if (self.width == 0 or self.height == 0) return error.DimensionsRequired; + if (self.width > max_dimension or self.height > max_dimension) return error.DimensionsTooLarge; // Data length must be what we expect // NOTE: we use a "<" check here because Kitty itself doesn't validate // this and if we validate exact data length then various Kitty // applications fail because the test that Kitty documents itself // uses an invalid value. - const expected_len = t.width * t.height * bpp; - if (data.len < expected_len) return error.InvalidData; + const expected_len = self.width * self.height * bpp; + std.log.warn( + "width={} height={} bpp={} expected_len={} actual_len={}", + .{ self.width, self.height, bpp, expected_len, self.data.len }, + ); + if (self.data.len < expected_len) return error.InvalidData; + } + /// Load an image from a transmission. The data in the command will be + /// owned by the image if successful. Note that you still must deinit + /// the command, all the state change will be done internally. + /// + /// If the command represents a chunked image then this image will + /// be incomplete. The caller is expected to inspect the command + /// and determine if it is a chunked image. + pub fn load(alloc: Allocator, cmd: *command.Command) !Image { + const t = cmd.transmission().?; + + // Load the data + const data = switch (t.medium) { + .direct => cmd.data, + else => { + std.log.warn("unimplemented medium={}", .{t.medium}); + return error.UnsupportedMedium; + }, + }; + + // If we loaded an image successfully then we take ownership + // of the command data and we need to make sure to clean up on error. + _ = cmd.toOwnedData(); + errdefer if (data.len > 0) alloc.free(data); + + const img = switch (t.format) { + .rgb, .rgba => try loadPacked(t, data), + else => return error.UnsupportedFormat, + }; + + return img; + } + + /// Load a package image format, i.e. RGB or RGBA. + fn loadPacked( + t: command.Transmission, + data: []const u8, + ) !Image { return Image{ .id = t.image_id, .number = t.image_number, + .width = t.width, + .height = t.height, + .format = switch (t.format) { + .rgb => .rgb, + .rgba => .rgba, + else => unreachable, + }, .data = data, }; } pub fn deinit(self: *Image, alloc: Allocator) void { - alloc.free(self.data); + if (self.data.len > 0) alloc.free(self.data); + } + + /// Mostly for logging + pub fn withoutData(self: *const Image) Image { + var copy = self.*; + copy.data = ""; + return copy; } }; @@ -75,9 +162,6 @@ test "image load with invalid RGB data" { const testing = std.testing; const alloc = testing.allocator; - var data = try alloc.dupe(u8, "AAAA"); - errdefer alloc.free(data); - // _Gi=31,s=1,v=1,a=q,t=d,f=24;AAAA\ var cmd: command.Command = .{ .control = .{ .transmit = .{ @@ -86,8 +170,9 @@ test "image load with invalid RGB data" { .height = 1, .image_id = 31, } }, - .data = data, + .data = try alloc.dupe(u8, "AAAA"), }; + defer cmd.deinit(alloc); var img = try Image.load(alloc, &cmd); defer img.deinit(alloc); } @@ -96,9 +181,6 @@ test "image load with image too wide" { const testing = std.testing; const alloc = testing.allocator; - var data = try alloc.dupe(u8, "AAAA"); - defer alloc.free(data); - var cmd: command.Command = .{ .control = .{ .transmit = .{ .format = .rgb, @@ -106,8 +188,9 @@ test "image load with image too wide" { .height = 1, .image_id = 31, } }, - .data = data, + .data = try alloc.dupe(u8, "AAAA"), }; + defer cmd.deinit(alloc); try testing.expectError(error.DimensionsTooLarge, Image.load(alloc, &cmd)); } @@ -115,9 +198,6 @@ test "image load with image too tall" { const testing = std.testing; const alloc = testing.allocator; - var data = try alloc.dupe(u8, "AAAA"); - defer alloc.free(data); - var cmd: command.Command = .{ .control = .{ .transmit = .{ .format = .rgb, @@ -125,7 +205,8 @@ test "image load with image too tall" { .width = 1, .image_id = 31, } }, - .data = data, + .data = try alloc.dupe(u8, "AAAA"), }; + defer cmd.deinit(alloc); try testing.expectError(error.DimensionsTooLarge, Image.load(alloc, &cmd)); } diff --git a/src/terminal/kitty/graphics_storage.zig b/src/terminal/kitty/graphics_storage.zig index a59303238..733b4c4e4 100644 --- a/src/terminal/kitty/graphics_storage.zig +++ b/src/terminal/kitty/graphics_storage.zig @@ -5,10 +5,13 @@ const ArenaAllocator = std.heap.ArenaAllocator; const point = @import("../point.zig"); const command = @import("graphics_command.zig"); +const ChunkedImage = @import("graphics_image.zig").ChunkedImage; const Image = @import("graphics_image.zig").Image; const Command = command.Command; const ScreenPoint = point.ScreenPoint; +const log = std.log.scoped(.kitty_gfx); + /// An image storage is associated with a terminal screen (i.e. main /// screen, alt screen) and contains all the transmitted images and /// placements. @@ -26,11 +29,16 @@ pub const ImageStorage = struct { /// The set of placements for loaded images. placements: PlacementMap = .{}, + /// Non-null if there is a chunked image in progress. + chunk: ?*ChunkedImage = null, + pub fn deinit(self: *ImageStorage, alloc: Allocator) void { + if (self.chunk) |chunk| chunk.destroy(alloc); + var it = self.images.iterator(); while (it.next()) |kv| kv.value_ptr.deinit(alloc); - self.images.deinit(alloc); + self.placements.deinit(alloc); } @@ -40,6 +48,12 @@ pub const ImageStorage = struct { // 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); + log.debug("addImage image={}", .{img: { + var copy = img; + copy.data = ""; + break :img copy; + }}); + // If the image has an image number, we need to invalidate the last // image with that same number. if (img.number > 0) { @@ -67,6 +81,11 @@ pub const ImageStorage = struct { p: Placement, ) !void { assert(self.images.get(image_id) != null); + log.debug("placement image_id={} placement_id={} placement={}\n", .{ + image_id, + placement_id, + p, + }); const key: PlacementKey = .{ .image_id = image_id, .placement_id = placement_id }; const gop = try self.placements.getOrPut(alloc, key); diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index d744514d0..330dc3d72 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -1084,15 +1084,11 @@ const StreamHandler = struct { var cmd = self.apc.end() orelse return; defer cmd.deinit(self.alloc); - log.warn("APC command: {}", .{cmd}); + // log.warn("APC command: {}", .{cmd}); switch (cmd) { .kitty => |*kitty_cmd| { var partial_buf: [512]u8 = undefined; if (self.terminal.kittyGraphics(self.alloc, &partial_buf, kitty_cmd)) |resp| { - if (!resp.ok()) { - log.warn("erroneous kitty graphics response: {s}", .{resp.message}); - } - var buf: [1024]u8 = undefined; var buf_stream = std.io.fixedBufferStream(&buf); try resp.encode(buf_stream.writer());