From f82899bd58d26950cfc6e8d7111d1d2595fc85cf Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 20 Aug 2023 15:20:02 -0700 Subject: [PATCH] terminal/kitty-gfx: better memory ownership semantics around func calls --- src/terminal/kitty/graphics_command.zig | 10 ++++ src/terminal/kitty/graphics_exec.zig | 58 ++++++++++++++------ src/terminal/kitty/graphics_image.zig | 71 ++++++++++++++++--------- 3 files changed, 97 insertions(+), 42 deletions(-) diff --git a/src/terminal/kitty/graphics_command.zig b/src/terminal/kitty/graphics_command.zig index be06e5046..a52392276 100644 --- a/src/terminal/kitty/graphics_command.zig +++ b/src/terminal/kitty/graphics_command.zig @@ -284,6 +284,16 @@ pub const Command = struct { return result; } + /// Returns the transmission data if it has any. + pub fn transmission(self: Command) ?Transmission { + return switch (self.control) { + .query => |t| t, + .transmit => |t| t, + .transmit_and_display => |t| t.transmission, + else => null, + }; + } + pub fn deinit(self: Command, alloc: Allocator) void { if (self.data.len > 0) alloc.free(self.data); } diff --git a/src/terminal/kitty/graphics_exec.zig b/src/terminal/kitty/graphics_exec.zig index f21e82c18..aaf7a922c 100644 --- a/src/terminal/kitty/graphics_exec.zig +++ b/src/terminal/kitty/graphics_exec.zig @@ -27,6 +27,7 @@ pub fn execute( const resp_: ?Response = switch (cmd.control) { .query => query(alloc, cmd), + .transmit => transmit(alloc, cmd), else => .{ .message = "ERROR: unimplemented action" }, }; @@ -65,23 +66,46 @@ fn query(alloc: Allocator, cmd: *Command) Response { }; // Attempt to load the image. If we cannot, then set an appropriate error. - if (Image.load(alloc, t, cmd.data)) |img| { - // Tell the command we've consumed the data. - _ = cmd.toOwnedData(); - defer { - // We need a mutable reference to deinit the image. - var img_c = img; - img_c.deinit(alloc); - } - - // If the image is greater than a predetermined max size, then we - // error. The max size here is taken directly from Kitty. - } else |err| switch (err) { - error.InvalidData => result.message = "EINVAL: invalid data", - error.UnsupportedFormat => result.message = "EINVAL: unsupported format", - error.DimensionsRequired => result.message = "EINVAL: dimensions required", - error.DimensionsTooLarge => result.message = "EINVAL: dimensions too large", - } + var img = Image.load(alloc, cmd) catch |err| { + encodeError(&result, err); + return result; + }; + img.deinit(alloc); return result; } + +/// Transmit image data. +fn transmit(alloc: Allocator, cmd: *Command) Response { + const t = cmd.control.transmit; + var result: Response = .{ + .id = t.image_id, + .image_number = t.image_number, + .placement_id = t.placement_id, + }; + + // Load our image. This will also validate all the metadata. + var img = Image.load(alloc, cmd) catch |err| { + encodeError(&result, err); + return result; + }; + errdefer img.deinit(alloc); + + // Store our image + // TODO + img.deinit(alloc); + + return result; +} + +const EncodeableError = Image.Error; + +/// Encode an error code into a message for a response. +fn encodeError(r: *Response, err: EncodeableError) void { + switch (err) { + error.InvalidData => r.message = "EINVAL: invalid data", + error.UnsupportedFormat => r.message = "EINVAL: unsupported format", + 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 de564511c..006c4962c 100644 --- a/src/terminal/kitty/graphics_image.zig +++ b/src/terminal/kitty/graphics_image.zig @@ -19,15 +19,24 @@ pub const Image = struct { UnsupportedFormat, }; - /// Load an image from a transmission. The data will be owned by the - /// return value if it is successful. - pub fn load(alloc: Allocator, t: command.Transmission, data: []const u8) !Image { + /// 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; - return switch (t.format) { - .rgb => try loadPacked(3, t, data), - .rgba => try loadPacked(4, t, data), - else => error.UnsupportedFormat, + + 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, }; + + // 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. @@ -68,12 +77,16 @@ test "image load with invalid RGB data" { errdefer alloc.free(data); // _Gi=31,s=1,v=1,a=q,t=d,f=24;AAAA\ - var img = try Image.load(alloc, .{ - .format = .rgb, - .width = 1, - .height = 1, - .image_id = 31, - }, data); + var cmd: command.Command = .{ + .control = .{ .transmit = .{ + .format = .rgb, + .width = 1, + .height = 1, + .image_id = 31, + } }, + .data = data, + }; + var img = try Image.load(alloc, &cmd); defer img.deinit(alloc); } @@ -84,12 +97,16 @@ test "image load with image too wide" { var data = try alloc.dupe(u8, "AAAA"); defer alloc.free(data); - try testing.expectError(error.DimensionsTooLarge, Image.load(alloc, .{ - .format = .rgb, - .width = max_dimension + 1, - .height = 1, - .image_id = 31, - }, data)); + var cmd: command.Command = .{ + .control = .{ .transmit = .{ + .format = .rgb, + .width = max_dimension + 1, + .height = 1, + .image_id = 31, + } }, + .data = data, + }; + try testing.expectError(error.DimensionsTooLarge, Image.load(alloc, &cmd)); } test "image load with image too tall" { @@ -99,10 +116,14 @@ test "image load with image too tall" { var data = try alloc.dupe(u8, "AAAA"); defer alloc.free(data); - try testing.expectError(error.DimensionsTooLarge, Image.load(alloc, .{ - .format = .rgb, - .height = max_dimension + 1, - .width = 1, - .image_id = 31, - }, data)); + var cmd: command.Command = .{ + .control = .{ .transmit = .{ + .format = .rgb, + .height = max_dimension + 1, + .width = 1, + .image_id = 31, + } }, + .data = data, + }; + try testing.expectError(error.DimensionsTooLarge, Image.load(alloc, &cmd)); }