Merge pull request #1043 from mitchellh/kitty-replace

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

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,16 +233,36 @@ 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 rgba = try rgbToRgba(alloc, data);
+                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);
@@ -146,11 +276,7 @@ pub const Image = union(enum) {
             rgba[rgba_i + 3] = 255;
         }
 
-                alloc.free(data);
-                p.data = rgba.ptr;
-                self.* = .{ .pending_rgba = p.* };
-            },
-        }
+        return rgba;
     }
 
     /// Upload the pending image to the GPU and change the state of this
@@ -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,20 +227,40 @@ 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 rgba = try rgbToRgba(alloc, data);
+                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);
@@ -144,11 +274,7 @@ pub const Image = union(enum) {
             rgba[rgba_i + 3] = 255;
         }
 
-                alloc.free(data);
-                p.data = rgba.ptr;
-                self.* = .{ .pending_rgba = p.* };
-            },
-        }
+        return rgba;
     }
 
     /// Upload the pending image to the GPU and change the state of this
@@ -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_rgba => .{ .internal = .rgba, .format = .rgba },
+            .pending_rgb, .replace_rgb => .{ .internal = .rgb, .format = .rgb },
+            .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,
         };
     }