Merge branch 'opentuah' of https://github.com/remorses/opentuah into opentuah

remorses · remorses · commit 8ff32c8d0b2a · 2026-02-13T23:31:14.000+01:00
diff --git a/packages/core/src/zig/buffer.zig b/packages/core/src/zig/buffer.zig
@@ -453,18 +453,21 @@ pub const OptimizedBuffer = struct {
 
             const span_start = index - @min(left, index - row_start);
             const span_end = index + @min(right, row_end - index);
-            const span_len = span_end - span_start + 1;
 
             var span_i: u32 = span_start;
-            while (span_i < span_start + span_len) : (span_i += 1) {
+            while (span_i <= span_end) : (span_i += 1) {
+                const span_char = self.buffer.char[span_i];
+                if (!(gp.isGraphemeChar(span_char) or gp.isContinuationChar(span_char))) continue;
+                if (gp.graphemeIdFromChar(span_char) != id) continue;
+
                 const span_link_id = ansi.TextAttributes.getLinkId(self.buffer.attributes[span_i]);
                 if (span_link_id != 0) {
                     self.link_tracker.removeCellRef(span_link_id);
                 }
-            }
 
-            @memset(self.buffer.char[span_start .. span_start + span_len], @intCast(DEFAULT_SPACE_CHAR));
-            @memset(self.buffer.attributes[span_start .. span_start + span_len], 0);
+                self.buffer.char[span_i] = @intCast(DEFAULT_SPACE_CHAR);
+                self.buffer.attributes[span_i] = 0;
+            }
         }
 
         if (gp.isGraphemeChar(cell.char)) {
diff --git a/packages/core/src/zig/grapheme.zig b/packages/core/src/zig/grapheme.zig
@@ -49,6 +49,7 @@ pub const GraphemePool = struct {
 
     allocator: std.mem.Allocator,
     classes: [MAX_CLASSES]ClassPool,
+    interned_live_ids: std.StringHashMapUnmanaged(IdPayload),
 
     const SlotHeader = extern struct {
         len: u16,
@@ -68,16 +69,81 @@ pub const GraphemePool = struct {
         while (i < MAX_CLASSES) : (i += 1) {
             classes[i] = ClassPool.init(allocator, CLASS_SIZES[i], slots_per_page[i]);
         }
-        return .{ .allocator = allocator, .classes = classes };
+        return .{ .allocator = allocator, .classes = classes, .interned_live_ids = .{} };
     }
 
     pub fn deinit(self: *GraphemePool) void {
+        var key_it = self.interned_live_ids.keyIterator();
+        while (key_it.next()) |key_ptr| {
+            self.allocator.free(@constCast(key_ptr.*));
+        }
+        self.interned_live_ids.deinit(self.allocator);
+
         var i: usize = 0;
         while (i < MAX_CLASSES) : (i += 1) {
             self.classes[i].deinit();
         }
     }
 
+    /// removeInternedLiveId removes an interned ID from the live set if it
+    /// matches the expected ID.
+    fn removeInternedLiveId(self: *GraphemePool, bytes: []const u8, expected_id: IdPayload) void {
+        const live_id = self.interned_live_ids.get(bytes) orelse return;
+        if (live_id != expected_id) return;
+        if (self.interned_live_ids.fetchRemove(bytes)) |removed| {
+            self.allocator.free(@constCast(removed.key));
+        }
+    }
+
+    /// lookupOrInvalidate checks if the given bytes are already interned and live, returning the existing ID if so.
+    fn lookupOrInvalidate(self: *GraphemePool, bytes: []const u8) ?IdPayload {
+        const live_id = self.interned_live_ids.get(bytes) orelse return null;
+
+        // Verify that the live ID is still valid and matches the bytes. If get
+        // fails, the ID is no longer valid, so remove it from the interned map.
+        const live_bytes = self.get(live_id) catch {
+            self.removeInternedLiveId(bytes, live_id);
+            return null;
+        };
+
+        // If the bytes don't match, this means the ID was recycled and now points
+        // to different data. Invalidate the interned ID.
+        if (!std.mem.eql(u8, live_bytes, bytes)) {
+            self.removeInternedLiveId(bytes, live_id);
+            return null;
+        }
+
+        // check refcount > 0 to ensure the ID is still live. If refcount is 0,
+        // the slot is free but hasn't been reused yet, so we can treat it as
+        // not found.
+        const live_refcount = self.getRefcount(live_id) catch {
+            self.removeInternedLiveId(bytes, live_id);
+            return null;
+        };
+        if (live_refcount == 0) {
+            self.removeInternedLiveId(bytes, live_id);
+            return null;
+        }
+
+        return live_id;
+    }
+
+    /// internLiveId interns the grapheme bytes.
+    fn internLiveId(self: *GraphemePool, id: IdPayload, bytes: []const u8) GraphemePoolError!void {
+        if (self.lookupOrInvalidate(bytes) != null) {
+            // Keep existing interned ID if it's still valid.
+            return;
+        }
+
+        const owned_key = self.allocator.dupe(u8, bytes) catch return GraphemePoolError.OutOfMemory;
+        errdefer self.allocator.free(owned_key);
+
+        if (self.interned_live_ids.fetchPut(self.allocator, owned_key, id) catch return GraphemePoolError.OutOfMemory) |replaced| {
+            // A previous key allocation was replaced.
+            self.allocator.free(@constCast(replaced.key));
+        }
+    }
+
     fn classForSize(size: usize) u32 {
         if (size <= 8) return 0;
         if (size <= 16) return 1;
@@ -94,6 +160,10 @@ pub const GraphemePool = struct {
     }
 
     pub fn alloc(self: *GraphemePool, bytes: []const u8) GraphemePoolError!IdPayload {
+        if (self.lookupOrInvalidate(bytes)) |live_id| {
+            return live_id;
+        }
+
         const class_id: u32 = classForSize(bytes.len);
         const slot_index = try self.classes[class_id].allocInternal(bytes, true);
         const generation = self.classes[class_id].getGeneration(slot_index);
@@ -116,14 +186,35 @@ pub const GraphemePool = struct {
         if (class_id >= MAX_CLASSES) return GraphemePoolError.InvalidId;
         const slot_index: u32 = id & SLOT_MASK;
         const generation: u32 = (id >> SLOT_BITS) & GENERATION_MASK;
+        const old_refcount = try self.classes[class_id].getRefcount(slot_index, generation);
         try self.classes[class_id].incref(slot_index, generation);
+
+        if (old_refcount == 0) {
+            const is_owned = try self.classes[class_id].isOwned(slot_index, generation);
+            if (is_owned) {
+                // This is a transition from 0 to 1 for owned bytes, so intern it.
+                const bytes = try self.classes[class_id].get(slot_index, generation);
+                try self.internLiveId(id, bytes);
+            }
+        }
     }
 
     pub fn decref(self: *GraphemePool, id: IdPayload) GraphemePoolError!void {
         const class_id: u32 = (id >> (GENERATION_BITS + SLOT_BITS)) & CLASS_MASK;
         if (class_id >= MAX_CLASSES) return GraphemePoolError.InvalidId;
         const slot_index: u32 = id & SLOT_MASK;
         const generation: u32 = (id >> SLOT_BITS) & GENERATION_MASK;
+
+        const old_refcount = try self.classes[class_id].getRefcount(slot_index, generation);
+        if (old_refcount == 1) {
+            const is_owned = try self.classes[class_id].isOwned(slot_index, generation);
+            if (is_owned) {
+                // This is a transition from 1 to 0 for owned bytes, remove map entry.
+                const bytes = try self.classes[class_id].get(slot_index, generation);
+                self.removeInternedLiveId(bytes, id);
+            }
+        }
+
         try self.classes[class_id].decref(slot_index, generation);
     }
 
@@ -135,6 +226,13 @@ pub const GraphemePool = struct {
         if (class_id >= MAX_CLASSES) return GraphemePoolError.InvalidId;
         const slot_index: u32 = id & SLOT_MASK;
         const generation: u32 = (id >> SLOT_BITS) & GENERATION_MASK;
+
+        const is_owned = try self.classes[class_id].isOwned(slot_index, generation);
+        if (is_owned) {
+            const bytes = try self.classes[class_id].get(slot_index, generation);
+            self.removeInternedLiveId(bytes, id);
+        }
+
         try self.classes[class_id].freeUnreferenced(slot_index, generation);
     }
 
@@ -313,6 +411,14 @@ pub const GraphemePool = struct {
             if (header_ptr.generation != expected_generation) return GraphemePoolError.WrongGeneration;
             return header_ptr.refcount;
         }
+
+        pub fn isOwned(self: *ClassPool, slot_index: u32, expected_generation: u32) GraphemePoolError!bool {
+            if (slot_index >= self.num_slots) return GraphemePoolError.InvalidId;
+            const p = self.slotPtr(slot_index);
+            const header_ptr = @as(*SlotHeader, @ptrCast(@alignCast(p)));
+            if (header_ptr.generation != expected_generation) return GraphemePoolError.WrongGeneration;
+            return header_ptr.is_owned == 1;
+        }
     };
 };
 
diff --git a/packages/core/src/zig/renderer.zig b/packages/core/src/zig/renderer.zig
@@ -740,19 +740,9 @@ pub const CliRenderer = struct {
                 }
                 runLength += 1;
 
-                // Update the current buffer with the new cell
-                self.currentRenderBuffer.setRaw(x, y, nextCell.?);
-
-                // If this is a grapheme start, also update all continuation cells
-                if (gp.isGraphemeChar(nextCell.?.char)) {
-                    const rightExtent = gp.charRightExtent(nextCell.?.char);
-                    var k: u32 = 1;
-                    while (k <= rightExtent and x + k < self.width) : (k += 1) {
-                        if (self.nextRenderBuffer.get(x + k, y)) |contCell| {
-                            self.currentRenderBuffer.setRaw(x + k, y, contCell);
-                        }
-                    }
-                }
+                // Update grapheme/link trackers (and continuation cells), so current buffer
+                // retains grapheme ownership after next buffer clear and IDs remain stable.
+                self.currentRenderBuffer.set(x, y, nextCell.?);
 
                 cellsUpdated += 1;
             }
diff --git a/packages/core/src/zig/tests/buffer_test.zig b/packages/core/src/zig/tests/buffer_test.zig
@@ -1016,6 +1016,50 @@ test "OptimizedBuffer - renderer two-buffer swap pattern should not leak" {
     }
 }
 
+test "OptimizedBuffer - set should not clear newly written adjacent grapheme continuation" {
+    var local_pool = gp.GraphemePool.initWithOptions(std.testing.allocator, .{});
+    defer local_pool.deinit();
+
+    var buf = try OptimizedBuffer.init(
+        std.testing.allocator,
+        8,
+        1,
+        .{ .pool = &local_pool, .id = "set-adjacent-grapheme" },
+    );
+    defer buf.deinit();
+
+    const bg = RGBA{ 0.0, 0.0, 0.0, 1.0 };
+    const fg = RGBA{ 1.0, 1.0, 1.0, 1.0 };
+    try buf.clear(bg, null);
+
+    const old_gid = try local_pool.alloc("🌟");
+    const old_start = gp.packGraphemeStart(old_gid & gp.GRAPHEME_ID_MASK, 2);
+    buf.set(3, 0, .{ .char = old_start, .fg = fg, .bg = bg, .attributes = 0 });
+
+    const new_gid = try local_pool.alloc("🔥");
+    const new_start = gp.packGraphemeStart(new_gid & gp.GRAPHEME_ID_MASK, 2);
+
+    // Simulate renderer's left-to-right in-place update:
+    // - x=2 writes a new grapheme (which writes continuation at x=3)
+    // - x=3 would be skipped by char-equality
+    // - x=4 overwrites an old continuation from the previous frame
+    // The overwrite at x=4 must not clear the new continuation at x=3.
+    buf.set(2, 0, .{ .char = new_start, .fg = fg, .bg = bg, .attributes = 0 });
+    buf.set(4, 0, .{ .char = ' ', .fg = fg, .bg = bg, .attributes = 0 });
+
+    const c2 = buf.get(2, 0).?;
+    const c3 = buf.get(3, 0).?;
+    const c4 = buf.get(4, 0).?;
+
+    try std.testing.expect(gp.isGraphemeChar(c2.char));
+    try std.testing.expect(gp.graphemeIdFromChar(c2.char) == (new_gid & gp.GRAPHEME_ID_MASK));
+
+    try std.testing.expect(gp.isContinuationChar(c3.char));
+    try std.testing.expect(gp.graphemeIdFromChar(c3.char) == (new_gid & gp.GRAPHEME_ID_MASK));
+
+    try std.testing.expect(c4.char == ' ');
+}
+
 test "OptimizedBuffer - sustained rendering should not leak" {
     const tiny_slots = [_]u32{ 2, 2, 2, 2, 2 };
     var local_pool = gp.GraphemePool.initWithOptions(std.testing.allocator, .{
diff --git a/packages/core/src/zig/tests/grapheme_test.zig b/packages/core/src/zig/tests/grapheme_test.zig
@@ -1037,6 +1037,27 @@ test "GraphemePool - allocUnowned large text" {
     try pool.decref(id);
 }
 
+test "GraphemePool - alloc does not reuse unowned IDs" {
+    var pool = GraphemePool.init(std.testing.allocator);
+    defer pool.deinit();
+
+    const external_text = "shared";
+
+    const unowned_id = try pool.allocUnowned(external_text);
+    try pool.incref(unowned_id);
+    defer pool.decref(unowned_id) catch {};
+
+    const owned_id = try pool.alloc(external_text);
+    try pool.incref(owned_id);
+    defer pool.decref(owned_id) catch {};
+
+    try std.testing.expect(owned_id != unowned_id);
+
+    const owned_bytes = try pool.get(owned_id);
+    try std.testing.expectEqualSlices(u8, external_text, owned_bytes);
+    try std.testing.expect(@intFromPtr(owned_bytes.ptr) != @intFromPtr(external_text.ptr));
+}
+
 test "GraphemeTracker - with unowned allocations" {
     var pool = GraphemePool.init(std.testing.allocator);
     defer pool.deinit();
@@ -1141,6 +1162,35 @@ test "GraphemePool - initWithOptions with small slots_per_page" {
     try pool.decref(id2);
 }
 
+test "GraphemePool - alloc reuses live ID for same bytes" {
+    const tiny_slots = [_]u32{ 1, 1, 1, 1, 1 };
+    var pool = gp.GraphemePool.initWithOptions(std.testing.allocator, .{
+        .slots_per_page = tiny_slots,
+    });
+    defer pool.deinit();
+
+    const grapheme = "👋";
+
+    const id1 = try pool.alloc(grapheme);
+    try pool.incref(id1);
+
+    const id2 = try pool.alloc(grapheme);
+    try std.testing.expectEqual(id1, id2);
+    try std.testing.expectEqual(@as(u32, 1), try pool.getRefcount(id1));
+
+    try pool.decref(id1);
+
+    const id3 = try pool.alloc(grapheme);
+    try pool.incref(id3);
+    defer pool.decref(id3) catch @panic("Failed to decref id3");
+
+    try std.testing.expect(id3 != id1);
+    try std.testing.expectEqualSlices(u8, grapheme, try pool.get(id3));
+
+    const id4 = try pool.alloc(grapheme);
+    try std.testing.expectEqual(id3, id4);
+}
+
 test "GraphemePool - small pool exhaustion and growth" {
     // Create a tiny pool that will need to grow
     const tiny_slots = [_]u32{ 1, 1, 1, 1, 1 }; // Only 1 slot per page initially
diff --git a/packages/core/src/zig/tests/renderer_test.zig b/packages/core/src/zig/tests/renderer_test.zig
@@ -571,6 +571,53 @@ test "renderer - grapheme pool refcounting with frame buffer fast path" {
     try std.testing.expect(rendered_cell != null);
 }
 
+test "renderer - unchanged grapheme should not churn IDs across frames" {
+    const pool = gp.initGlobalPool(std.testing.allocator);
+    defer gp.deinitGlobalPool();
+
+    var cli_renderer = try CliRenderer.create(
+        std.testing.allocator,
+        4,
+        1,
+        pool,
+        true,
+    );
+    defer cli_renderer.destroy();
+
+    const fg = RGBA{ 1.0, 1.0, 1.0, 1.0 };
+    const bg = RGBA{ 0.0, 0.0, 0.0, 1.0 };
+
+    const first_next_buffer = cli_renderer.getNextBuffer();
+    try first_next_buffer.drawText("👋", 0, 0, fg, bg, 0);
+    cli_renderer.render(false);
+
+    const first_output = cli_renderer.getLastOutputForTest();
+    try std.testing.expect(std.mem.indexOf(u8, first_output, "👋") != null);
+
+    const current_buffer = cli_renderer.getCurrentBuffer();
+    const first_cell = current_buffer.get(0, 0);
+    try std.testing.expect(first_cell != null);
+    try std.testing.expect(gp.isGraphemeChar(first_cell.?.char));
+    const first_gid = gp.graphemeIdFromChar(first_cell.?.char);
+
+    const second_next_buffer = cli_renderer.getNextBuffer();
+    try second_next_buffer.drawText("👋", 0, 0, fg, bg, 0);
+
+    const second_cell = second_next_buffer.get(0, 0);
+    try std.testing.expect(second_cell != null);
+    try std.testing.expect(gp.isGraphemeChar(second_cell.?.char));
+    const second_gid = gp.graphemeIdFromChar(second_cell.?.char);
+
+    // Same grapheme content in consecutive frames should keep a stable ID,
+    // otherwise diff/write treats unchanged cells as modified every frame.
+    try std.testing.expectEqual(first_gid, second_gid);
+
+    cli_renderer.render(false);
+
+    const second_output = cli_renderer.getLastOutputForTest();
+    try std.testing.expect(std.mem.indexOf(u8, second_output, "👋") == null);
+}
+
 test "renderer - hyperlinks enabled with OSC 8 output" {
     const pool = gp.initGlobalPool(std.testing.allocator);
     defer gp.deinitGlobalPool();
