renderer/metal,opengl: replace matching image IDs if transmit time differs

Fixes #1037

Renderers must convert the internal Kitty graphics state to a GPU
texture for rendering. For performance reasons, renderers cache the GPU
state by image ID. Unfortunately, this meant that if an image was
replaced with the same ID and was already cached, it would never be
updated on the GPU.

This PR adds the transmission time to the cache. If the transmission
time differs, we assume the image changed and replace the image.

src/renderer/Metal.zig

@@ -409,7 +409,7 @@ pub fn deinit(self: *Metal) void {
 
     {
         var it = self.images.iterator();
-        while (it.next()) |kv| kv.value_ptr.deinit(self.alloc);
+        while (it.next()) |kv| kv.value_ptr.image.deinit(self.alloc);
         self.images.deinit(self.alloc);
     }
     self.image_placements.deinit(self.alloc);
@@ -677,17 +677,20 @@ pub fn updateFrame(
     {
         var image_it = self.images.iterator();
         while (image_it.next()) |kv| {
-            switch (kv.value_ptr.*) {
+            switch (kv.value_ptr.image) {
                 .ready => {},
 
                 .pending_rgb,
                 .pending_rgba,
-                => try kv.value_ptr.upload(self.alloc, self.device),
+                .replace_rgb,
+                .replace_rgba,
+                => try kv.value_ptr.image.upload(self.alloc, self.device),
 
                 .unload_pending,
+                .unload_replace,
                 .unload_ready,
                 => {
-                    kv.value_ptr.deinit(self.alloc);
+                    kv.value_ptr.image.deinit(self.alloc);
                     self.images.removeByPtr(kv.key_ptr);
                 },
             }
@@ -952,7 +955,7 @@ fn drawImagePlacement(
     };
 
     // Get the texture
-    const texture = switch (image) {
+    const texture = switch (image.image) {
         .ready => |t| t,
         else => {
             log.warn("image not ready for placement image_id={}", .{p.image_id});
@@ -1121,7 +1124,7 @@ fn prepKittyGraphics(
         var it = self.images.iterator();
         while (it.next()) |kv| {
             if (storage.imageById(kv.key_ptr.*) == null) {
-                kv.value_ptr.markForUnload();
+                kv.value_ptr.image.markForUnload();
             }
         }
     }
@@ -1160,9 +1163,13 @@ fn prepKittyGraphics(
             break :offset_y @intCast(offset_pixels);
         } else 0;
 
-        // If we already know about this image then do nothing
+        // 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.
         const gop = try self.images.getOrPut(self.alloc, kv.key_ptr.image_id);
-        if (!gop.found_existing) {
+        if (!gop.found_existing or
+            gop.value_ptr.transmit_time.order(image.transmit_time) != .eq)
+        {
             // Copy the data into the pending state.
             const data = try self.alloc.dupe(u8, image.data);
             errdefer self.alloc.free(data);
@@ -1174,11 +1181,25 @@ fn prepKittyGraphics(
                 .data = data.ptr,
             };
 
-            gop.value_ptr.* = switch (image.format) {
+            const new_image: Image = switch (image.format) {
                 .rgb => .{ .pending_rgb = pending },
                 .rgba => .{ .pending_rgba = pending },
                 .png => unreachable, // should be decoded by now
             };
+
+            if (!gop.found_existing) {
+                gop.value_ptr.* = .{
+                    .image = new_image,
+                    .transmit_time = undefined,
+                };
+            } else {
+                try gop.value_ptr.image.markForReplace(
+                    self.alloc,
+                    new_image,
+                );
+            }
+
+            gop.value_ptr.transmit_time = image.transmit_time;
         }
 
         // Convert our screen point to a viewport point

src/renderer/OpenGL.zig

@@ -372,7 +372,7 @@ pub fn deinit(self: *OpenGL) void {
 
     {
         var it = self.images.iterator();
-        while (it.next()) |kv| kv.value_ptr.deinit(self.alloc);
+        while (it.next()) |kv| kv.value_ptr.image.deinit(self.alloc);
         self.images.deinit(self.alloc);
     }
     self.image_placements.deinit(self.alloc);
@@ -768,7 +768,7 @@ fn prepKittyGraphics(
         var it = self.images.iterator();
         while (it.next()) |kv| {
             if (storage.imageById(kv.key_ptr.*) == null) {
-                kv.value_ptr.markForUnload();
+                kv.value_ptr.image.markForUnload();
             }
         }
     }
@@ -807,9 +807,13 @@ fn prepKittyGraphics(
             break :offset_y @intCast(offset_pixels);
         } else 0;
 
-        // If we already know about this image then do nothing
+        // 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.
         const gop = try self.images.getOrPut(self.alloc, kv.key_ptr.image_id);
-        if (!gop.found_existing) {
+        if (!gop.found_existing or
+            gop.value_ptr.transmit_time.order(image.transmit_time) != .eq)
+        {
             // Copy the data into the pending state.
             const data = try self.alloc.dupe(u8, image.data);
             errdefer self.alloc.free(data);
@@ -821,11 +825,25 @@ fn prepKittyGraphics(
                 .data = data.ptr,
             };
 
-            gop.value_ptr.* = switch (image.format) {
+            const new_image: Image = switch (image.format) {
                 .rgb => .{ .pending_rgb = pending },
                 .rgba => .{ .pending_rgba = pending },
                 .png => unreachable, // should be decoded by now
             };
+
+            if (!gop.found_existing) {
+                gop.value_ptr.* = .{
+                    .image = new_image,
+                    .transmit_time = undefined,
+                };
+            } else {
+                try gop.value_ptr.image.markForReplace(
+                    self.alloc,
+                    new_image,
+                );
+            }
+
+            gop.value_ptr.transmit_time = image.transmit_time;
         }
 
         // Convert our screen point to a viewport point
@@ -1732,17 +1750,20 @@ pub fn drawFrame(self: *OpenGL, surface: *apprt.Surface) !void {
     {
         var image_it = self.images.iterator();
         while (image_it.next()) |kv| {
-            switch (kv.value_ptr.*) {
+            switch (kv.value_ptr.image) {
                 .ready => {},
 
                 .pending_rgb,
                 .pending_rgba,
-                => try kv.value_ptr.upload(self.alloc),
+                .replace_rgb,
+                .replace_rgba,
+                => try kv.value_ptr.image.upload(self.alloc),
 
                 .unload_pending,
+                .unload_replace,
                 .unload_ready,
                 => {
-                    kv.value_ptr.deinit(self.alloc);
+                    kv.value_ptr.image.deinit(self.alloc);
                     self.images.removeByPtr(kv.key_ptr);
                 },
             }
@@ -1873,7 +1894,7 @@ fn drawImages(
             continue;
         };
 
-        const texture = switch (image) {
+        const texture = switch (image.image) {
             .ready => |t| t,
             else => {
                 log.warn("image not ready for placement image_id={}", .{p.image_id});

src/renderer/metal/image.zig

@@ -33,7 +33,10 @@ pub const Placement = struct {
 };
 
 /// The map used for storing images.
-pub const ImageMap = std.AutoHashMapUnmanaged(u32, Image);
+pub const ImageMap = std.AutoHashMapUnmanaged(u32, struct {
+    image: Image,
+    transmit_time: std.time.Instant,
+});
 
 /// The state for a single image that is to be rendered. The image can be
 /// pending upload or ready to use with a texture.
@@ -47,12 +50,23 @@ pub const Image = union(enum) {
     pending_rgb: Pending,
     pending_rgba: Pending,
 
+    /// This is the same as the pending states but there is a texture
+    /// already allocated that we want to replace.
+    replace_rgb: Replace,
+    replace_rgba: Replace,
+
     /// The image is uploaded and ready to be used.
     ready: objc.Object, // MTLTexture
 
     /// The image is uploaded but is scheduled to be unloaded.
     unload_pending: []u8,
     unload_ready: objc.Object, // MTLTexture
+    unload_replace: struct { []u8, objc.Object },
+
+    pub const Replace = struct {
+        texture: objc.Object,
+        pending: Pending,
+    };
 
     /// Pending image data that needs to be uploaded to the GPU.
     pub const Pending = struct {
@@ -78,6 +92,21 @@ pub const Image = union(enum) {
             .pending_rgba => |p| alloc.free(p.dataSlice(4)),
             .unload_pending => |data| alloc.free(data),
 
+            .replace_rgb => |r| {
+                alloc.free(r.pending.dataSlice(3));
+                r.texture.msgSend(void, objc.sel("release"), .{});
+            },
+
+            .replace_rgba => |r| {
+                alloc.free(r.pending.dataSlice(4));
+                r.texture.msgSend(void, objc.sel("release"), .{});
+            },
+
+            .unload_replace => |r| {
+                alloc.free(r[0]);
+                r[1].msgSend(void, objc.sel("release"), .{});
+            },
+
             .ready,
             .unload_ready,
             => |obj| obj.msgSend(void, objc.sel("release"), .{}),
@@ -88,12 +117,93 @@ pub const Image = union(enum) {
     pub fn markForUnload(self: *Image) void {
         self.* = switch (self.*) {
             .unload_pending,
+            .unload_replace,
             .unload_ready,
             => return,
 
             .ready => |obj| .{ .unload_ready = obj },
             .pending_rgb => |p| .{ .unload_pending = p.dataSlice(3) },
             .pending_rgba => |p| .{ .unload_pending = p.dataSlice(4) },
+            .replace_rgb => |r| .{ .unload_replace = .{
+                r.pending.dataSlice(3), r.texture,
+            } },
+            .replace_rgba => |r| .{ .unload_replace = .{
+                r.pending.dataSlice(4), r.texture,
+            } },
+        };
+    }
+
+    /// Replace the currently pending image with a new one. This will
+    /// attempt to update the existing texture if it is already allocated.
+    /// If the texture is not allocated, this will act like a new upload.
+    ///
+    /// This function only marks the image for replace. The actual logic
+    /// to replace is done later.
+    pub fn markForReplace(self: *Image, alloc: Allocator, img: Image) !void {
+        assert(img.pending() != null);
+
+        // Get our existing texture. This switch statement will also handle
+        // scenarios where there is no existing texture and we can modify
+        // the self pointer directly.
+        const existing: objc.Object = switch (self.*) {
+            // For pending, we can free the old data and become pending ourselves.
+            .pending_rgb => |p| {
+                alloc.free(p.dataSlice(3));
+                self.* = img;
+                return;
+            },
+
+            .pending_rgba => |p| {
+                alloc.free(p.dataSlice(4));
+                self.* = img;
+                return;
+            },
+
+            // If we're marked for unload but we just have pending data,
+            // this behaves the same as a normal "pending": free the data,
+            // become new pending.
+            .unload_pending => |data| {
+                alloc.free(data);
+                self.* = img;
+                return;
+            },
+
+            .unload_replace => |r| existing: {
+                alloc.free(r[0]);
+                break :existing r[1];
+            },
+
+            // If we were already pending a replacement, then we free our
+            // existing pending data and use the same texture.
+            .replace_rgb => |r| existing: {
+                alloc.free(r.pending.dataSlice(3));
+                break :existing r.texture;
+            },
+
+            .replace_rgba => |r| existing: {
+                alloc.free(r.pending.dataSlice(4));
+                break :existing r.texture;
+            },
+
+            // For both ready and unload_ready, we need to replace the
+            // texture. We can't do that here, so we just mark ourselves
+            // for replacement.
+            .ready, .unload_ready => |tex| tex,
+        };
+
+        // We now have an existing texture, so set the proper replace key.
+        self.* = switch (img) {
+            .pending_rgb => |p| .{ .replace_rgb = .{
+                .texture = existing,
+                .pending = p,
+            } },
+
+            .pending_rgba => |p| .{ .replace_rgba = .{
+                .texture = existing,
+                .pending = p,
+            } },
+
+            else => unreachable,
         };
     }
 
@@ -123,36 +233,52 @@ pub const Image = union(enum) {
         switch (self.*) {
             .ready,
             .unload_pending,
+            .unload_replace,
             .unload_ready,
             => unreachable, // invalid
 
-            .pending_rgba => {}, // ready
+            .pending_rgba,
+            .replace_rgba,
+            => {}, // ready
 
             // RGB needs to be converted to RGBA because Metal textures
             // don't support RGB.
             .pending_rgb => |*p| {
                 // Note: this is the slowest possible way to do this...
                 const data = p.dataSlice(3);
-                const pixels = data.len / 3;
+                const rgba = try rgbToRgba(alloc, data);
-                var rgba = try alloc.alloc(u8, pixels * 4);
-                errdefer alloc.free(rgba);
-                var i: usize = 0;
-                while (i < pixels) : (i += 1) {
-                    const data_i = i * 3;
-                    const rgba_i = i * 4;
-                    rgba[rgba_i] = data[data_i];
-                    rgba[rgba_i + 1] = data[data_i + 1];
-                    rgba[rgba_i + 2] = data[data_i + 2];
-                    rgba[rgba_i + 3] = 255;
-                }
-
                 alloc.free(data);
                 p.data = rgba.ptr;
                 self.* = .{ .pending_rgba = p.* };
             },
+
+            .replace_rgb => |*r| {
+                const data = r.pending.dataSlice(3);
+                const rgba = try rgbToRgba(alloc, data);
+                alloc.free(data);
+                r.pending.data = rgba.ptr;
+                self.* = .{ .replace_rgba = r.* };
+            },
         }
     }
 
+    fn rgbToRgba(alloc: Allocator, data: []const u8) ![]u8 {
+        const pixels = data.len / 3;
+        var rgba = try alloc.alloc(u8, pixels * 4);
+        errdefer alloc.free(rgba);
+        var i: usize = 0;
+        while (i < pixels) : (i += 1) {
+            const data_i = i * 3;
+            const rgba_i = i * 4;
+            rgba[rgba_i] = data[data_i];
+            rgba[rgba_i + 1] = data[data_i + 1];
+            rgba[rgba_i + 2] = data[data_i + 2];
+            rgba[rgba_i + 3] = 255;
+        }
+
+        return rgba;
+    }
+
     /// Upload the pending image to the GPU and change the state of this
     /// image to ready.
     pub fn upload(
@@ -191,6 +317,10 @@ pub const Image = union(enum) {
         );
 
         // Uploaded. We can now clear our data and change our state.
+        //
+        // NOTE: For "replace_*" states, this will free the old texture.
+        // We don't currently actually replace the existing texture in-place
+        // but that is an optimization we can do later.
         self.deinit(alloc);
         self.* = .{ .ready = texture };
     }
@@ -200,6 +330,8 @@ pub const Image = union(enum) {
         return switch (self) {
             .pending_rgb => 3,
             .pending_rgba => 4,
+            .replace_rgb => 3,
+            .replace_rgba => 4,
             else => unreachable,
         };
     }
@@ -211,6 +343,10 @@ pub const Image = union(enum) {
             .pending_rgba,
             => |p| p,
 
+            .replace_rgb,
+            .replace_rgba,
+            => |r| r.pending,
+
             else => null,
         };
     }

src/renderer/opengl/image.zig

@@ -31,7 +31,10 @@ pub const Placement = struct {
 };
 
 /// The map used for storing images.
-pub const ImageMap = std.AutoHashMapUnmanaged(u32, Image);
+pub const ImageMap = std.AutoHashMapUnmanaged(u32, struct {
+    image: Image,
+    transmit_time: std.time.Instant,
+});
 
 /// The state for a single image that is to be rendered. The image can be
 /// pending upload or ready to use with a texture.
@@ -45,12 +48,23 @@ pub const Image = union(enum) {
     pending_rgb: Pending,
     pending_rgba: Pending,
 
+    /// This is the same as the pending states but there is a texture
+    /// already allocated that we want to replace.
+    replace_rgb: Replace,
+    replace_rgba: Replace,
+
     /// The image is uploaded and ready to be used.
     ready: gl.Texture,
 
     /// The image is uploaded but is scheduled to be unloaded.
     unload_pending: []u8,
     unload_ready: gl.Texture,
+    unload_replace: struct { []u8, gl.Texture },
+
+    pub const Replace = struct {
+        texture: gl.Texture,
+        pending: Pending,
+    };
 
     /// Pending image data that needs to be uploaded to the GPU.
     pub const Pending = struct {
@@ -76,6 +90,21 @@ pub const Image = union(enum) {
             .pending_rgba => |p| alloc.free(p.dataSlice(4)),
             .unload_pending => |data| alloc.free(data),
 
+            .replace_rgb => |r| {
+                alloc.free(r.pending.dataSlice(3));
+                r.texture.destroy();
+            },
+
+            .replace_rgba => |r| {
+                alloc.free(r.pending.dataSlice(4));
+                r.texture.destroy();
+            },
+
+            .unload_replace => |r| {
+                alloc.free(r[0]);
+                r[1].destroy();
+            },
+
             .ready,
             .unload_ready,
             => |tex| tex.destroy(),
@@ -86,12 +115,93 @@ pub const Image = union(enum) {
     pub fn markForUnload(self: *Image) void {
         self.* = switch (self.*) {
             .unload_pending,
+            .unload_replace,
             .unload_ready,
             => return,
 
             .ready => |obj| .{ .unload_ready = obj },
             .pending_rgb => |p| .{ .unload_pending = p.dataSlice(3) },
             .pending_rgba => |p| .{ .unload_pending = p.dataSlice(4) },
+            .replace_rgb => |r| .{ .unload_replace = .{
+                r.pending.dataSlice(3), r.texture,
+            } },
+            .replace_rgba => |r| .{ .unload_replace = .{
+                r.pending.dataSlice(4), r.texture,
+            } },
+        };
+    }
+
+    /// Replace the currently pending image with a new one. This will
+    /// attempt to update the existing texture if it is already allocated.
+    /// If the texture is not allocated, this will act like a new upload.
+    ///
+    /// This function only marks the image for replace. The actual logic
+    /// to replace is done later.
+    pub fn markForReplace(self: *Image, alloc: Allocator, img: Image) !void {
+        assert(img.pending() != null);
+
+        // Get our existing texture. This switch statement will also handle
+        // scenarios where there is no existing texture and we can modify
+        // the self pointer directly.
+        const existing: gl.Texture = switch (self.*) {
+            // For pending, we can free the old data and become pending ourselves.
+            .pending_rgb => |p| {
+                alloc.free(p.dataSlice(3));
+                self.* = img;
+                return;
+            },
+
+            .pending_rgba => |p| {
+                alloc.free(p.dataSlice(4));
+                self.* = img;
+                return;
+            },
+
+            // If we're marked for unload but we just have pending data,
+            // this behaves the same as a normal "pending": free the data,
+            // become new pending.
+            .unload_pending => |data| {
+                alloc.free(data);
+                self.* = img;
+                return;
+            },
+
+            .unload_replace => |r| existing: {
+                alloc.free(r[0]);
+                break :existing r[1];
+            },
+
+            // If we were already pending a replacement, then we free our
+            // existing pending data and use the same texture.
+            .replace_rgb => |r| existing: {
+                alloc.free(r.pending.dataSlice(3));
+                break :existing r.texture;
+            },
+
+            .replace_rgba => |r| existing: {
+                alloc.free(r.pending.dataSlice(4));
+                break :existing r.texture;
+            },
+
+            // For both ready and unload_ready, we need to replace the
+            // texture. We can't do that here, so we just mark ourselves
+            // for replacement.
+            .ready, .unload_ready => |tex| tex,
+        };
+
+        // We now have an existing texture, so set the proper replace key.
+        self.* = switch (img) {
+            .pending_rgb => |p| .{ .replace_rgb = .{
+                .texture = existing,
+                .pending = p,
+            } },
+
+            .pending_rgba => |p| .{ .replace_rgba = .{
+                .texture = existing,
+                .pending = p,
+            } },
+
+            else => unreachable,
         };
     }
 
@@ -117,40 +227,56 @@ pub const Image = union(enum) {
     /// Converts the image data to a format that can be uploaded to the GPU.
     /// If the data is already in a format that can be uploaded, this is a
     /// no-op.
-    fn convert(self: *Image, alloc: Allocator) !void {
+    pub fn convert(self: *Image, alloc: Allocator) !void {
         switch (self.*) {
             .ready,
             .unload_pending,
+            .unload_replace,
             .unload_ready,
             => unreachable, // invalid
 
-            .pending_rgba => {}, // ready
+            .pending_rgba,
+            .replace_rgba,
+            => {}, // ready
 
             // RGB needs to be converted to RGBA because Metal textures
             // don't support RGB.
             .pending_rgb => |*p| {
                 // Note: this is the slowest possible way to do this...
                 const data = p.dataSlice(3);
-                const pixels = data.len / 3;
+                const rgba = try rgbToRgba(alloc, data);
-                var rgba = try alloc.alloc(u8, pixels * 4);
-                errdefer alloc.free(rgba);
-                var i: usize = 0;
-                while (i < pixels) : (i += 1) {
-                    const data_i = i * 3;
-                    const rgba_i = i * 4;
-                    rgba[rgba_i] = data[data_i];
-                    rgba[rgba_i + 1] = data[data_i + 1];
-                    rgba[rgba_i + 2] = data[data_i + 2];
-                    rgba[rgba_i + 3] = 255;
-                }
-
                 alloc.free(data);
                 p.data = rgba.ptr;
                 self.* = .{ .pending_rgba = p.* };
             },
+
+            .replace_rgb => |*r| {
+                const data = r.pending.dataSlice(3);
+                const rgba = try rgbToRgba(alloc, data);
+                alloc.free(data);
+                r.pending.data = rgba.ptr;
+                self.* = .{ .replace_rgba = r.* };
+            },
         }
     }
 
+    fn rgbToRgba(alloc: Allocator, data: []const u8) ![]u8 {
+        const pixels = data.len / 3;
+        var rgba = try alloc.alloc(u8, pixels * 4);
+        errdefer alloc.free(rgba);
+        var i: usize = 0;
+        while (i < pixels) : (i += 1) {
+            const data_i = i * 3;
+            const rgba_i = i * 4;
+            rgba[rgba_i] = data[data_i];
+            rgba[rgba_i + 1] = data[data_i + 1];
+            rgba[rgba_i + 2] = data[data_i + 2];
+            rgba[rgba_i + 3] = 255;
+        }
+
+        return rgba;
+    }
+
     /// Upload the pending image to the GPU and change the state of this
     /// image to ready.
     pub fn upload(
@@ -168,8 +294,8 @@ pub const Image = union(enum) {
             internal: gl.Texture.InternalFormat,
             format: gl.Texture.Format,
         } = switch (self.*) {
-            .pending_rgb => .{ .internal = .rgb, .format = .rgb },
+            .pending_rgb, .replace_rgb => .{ .internal = .rgb, .format = .rgb },
-            .pending_rgba => .{ .internal = .rgba, .format = .rgba },
+            .pending_rgba, .replace_rgba => .{ .internal = .rgba, .format = .rgba },
             else => unreachable,
         };
 
@@ -203,6 +329,8 @@ pub const Image = union(enum) {
         return switch (self) {
             .pending_rgb => 3,
             .pending_rgba => 4,
+            .replace_rgb => 3,
+            .replace_rgba => 4,
             else => unreachable,
         };
     }
@@ -214,6 +342,10 @@ pub const Image = union(enum) {
             .pending_rgba,
             => |p| p,
 
+            .replace_rgb,
+            .replace_rgba,
+            => |r| r.pending,
+
             else => null,
         };
     }