Merge pull request #986 from mitchellh/preedit-alloc

remove length limit on preedit text, handle widths larger than screen

src/Surface.zig

@@ -629,6 +629,8 @@ pub fn deinit(self: *Surface) void {
         self.alloc.destroy(v);
     }
 
+    // Clean up our render state
+    if (self.renderer_state.preedit) |p| self.alloc.free(p.codepoints);
     self.alloc.destroy(self.renderer_state.mutex);
     self.config.deinit();
 
@@ -1087,7 +1089,10 @@ pub fn preeditCallback(self: *Surface, preedit_: ?[]const u8) !void {
     defer self.renderer_state.mutex.unlock();
 
     // We always clear our prior preedit
-    self.renderer_state.preedit = null;
+    if (self.renderer_state.preedit) |p| {
+        self.alloc.free(p.codepoints);
+        self.renderer_state.preedit = null;
+    }
 
     // If we have no text, we're done. We queue a render in case we cleared
     // a prior preedit (likely).
@@ -1101,7 +1106,9 @@ pub fn preeditCallback(self: *Surface, preedit_: ?[]const u8) !void {
     var it = view.iterator();
 
     // Allocate the codepoints slice
-    var preedit: renderer.State.Preedit = .{};
+    const Codepoint = renderer.State.Preedit.Codepoint;
+    var codepoints: std.ArrayListUnmanaged(Codepoint) = .{};
+    defer codepoints.deinit(self.alloc);
     while (it.nextCodepoint()) |cp| {
         const width = ziglyph.display_width.codePointWidth(cp, .half);
 
@@ -1110,21 +1117,18 @@ pub fn preeditCallback(self: *Surface, preedit_: ?[]const u8) !void {
         // Let's just ignore it.
         if (width <= 0) continue;
 
-        preedit.codepoints[preedit.len] = .{ .codepoint = cp, .wide = width >= 2 };
+        try codepoints.append(
-        preedit.len += 1;
+            self.alloc,
-
+            .{ .codepoint = cp, .wide = width >= 2 },
-        // This is a strange edge case. We have a generous buffer for
+        );
-        // preedit text but if we exceed it, we just truncate.
-        if (preedit.len >= preedit.codepoints.len) {
-            log.warn("preedit text is longer than our buffer, truncating", .{});
-            break;
-        }
     }
 
     // If we have no codepoints, then we're done.
-    if (preedit.len == 0) return;
+    if (codepoints.items.len == 0) return;
 
-    self.renderer_state.preedit = preedit;
+    self.renderer_state.preedit = .{
+        .codepoints = try codepoints.toOwnedSlice(self.alloc),
+    };
     try self.queueRender();
 }
 

src/renderer/Metal.zig

@@ -632,6 +632,14 @@ pub fn updateFrame(
             cursor_blink_visible,
         );
 
+        // Get our preedit state
+        const preedit: ?renderer.State.Preedit = preedit: {
+            if (cursor_style == null) break :preedit null;
+            const p = state.preedit orelse break :preedit null;
+            break :preedit try p.clone(self.alloc);
+        };
+        errdefer if (preedit) |p| p.deinit(self.alloc);
+
         // If we have Kitty graphics data, we enter a SLOW SLOW SLOW path.
         // We only do this if the Kitty image state is dirty meaning only if
         // it changes.
@@ -644,11 +652,14 @@ pub fn updateFrame(
             .selection = selection,
             .screen = screen_copy,
             .mouse = state.mouse,
-            .preedit = if (cursor_style != null) state.preedit else null,
+            .preedit = preedit,
             .cursor_style = cursor_style,
         };
     };
-    defer critical.screen.deinit();
+    defer {
+        critical.screen.deinit();
+        if (critical.preedit) |p| p.deinit(self.alloc);
+    }
 
     // Build our GPU cells
     try self.rebuildCells(
@@ -1423,10 +1434,13 @@ fn rebuildCells(
     const preedit_range: ?struct {
         y: usize,
         x: [2]usize,
+        cp_offset: usize,
     } = if (preedit) |preedit_v| preedit: {
+        const range = preedit_v.range(screen.cursor.x, screen.cols - 1);
         break :preedit .{
             .y = screen.cursor.y,
-            .x = preedit_v.range(screen.cursor.x, screen.cols - 1),
+            .x = .{ range.start, range.end },
+            .cp_offset = range.cp_offset,
         };
     } else null;
 
@@ -1572,7 +1586,7 @@ fn rebuildCells(
         if (preedit) |preedit_v| {
             const range = preedit_range.?;
             var x = range.x[0];
-            for (preedit_v.codepoints[0..preedit_v.len]) |cp| {
+            for (preedit_v.codepoints[range.cp_offset..]) |cp| {
                 self.addPreeditCell(cp, x, range.y) catch |err| {
                     log.warn("error building preedit cell, will be invalid x={} y={}, err={}", .{
                         x,

src/renderer/OpenGL.zig

@@ -697,6 +697,14 @@ pub fn updateFrame(
             cursor_blink_visible,
         );
 
+        // Get our preedit state
+        const preedit: ?renderer.State.Preedit = preedit: {
+            if (cursor_style == null) break :preedit null;
+            const p = state.preedit orelse break :preedit null;
+            break :preedit try p.clone(self.alloc);
+        };
+        errdefer if (preedit) |p| p.deinit(self.alloc);
+
         // If we have Kitty graphics data, we enter a SLOW SLOW SLOW path.
         // We only do this if the Kitty image state is dirty meaning only if
         // it changes.
@@ -709,11 +717,14 @@ pub fn updateFrame(
             .selection = selection,
             .screen = screen_copy,
             .mouse = state.mouse,
-            .preedit = if (cursor_style != null) state.preedit else null,
+            .preedit = preedit,
             .cursor_style = cursor_style,
         };
     };
-    defer critical.screen.deinit();
+    defer {
+        critical.screen.deinit();
+        if (critical.preedit) |p| p.deinit(self.alloc);
+    }
 
     // Grab our draw mutex if we have it and update our data
     {
@@ -943,10 +954,13 @@ pub fn rebuildCells(
     const preedit_range: ?struct {
         y: usize,
         x: [2]usize,
+        cp_offset: usize,
     } = if (preedit) |preedit_v| preedit: {
+        const range = preedit_v.range(screen.cursor.x, screen.cols - 1);
         break :preedit .{
             .y = screen.cursor.y,
-            .x = preedit_v.range(screen.cursor.x, screen.cols - 1),
+            .x = .{ range.start, range.end },
+            .cp_offset = range.cp_offset,
         };
     } else null;
 
@@ -1083,7 +1097,7 @@ pub fn rebuildCells(
         if (preedit) |preedit_v| {
             const range = preedit_range.?;
             var x = range.x[0];
-            for (preedit_v.codepoints[0..preedit_v.len]) |cp| {
+            for (preedit_v.codepoints[range.cp_offset..]) |cp| {
                 self.addPreeditCell(cp, x, range.y) catch |err| {
                     log.warn("error building preedit cell, will be invalid x={} y={}, err={}", .{
                         x,

src/renderer/State.zig

@@ -38,11 +38,8 @@ pub const Mouse = struct {
 
 /// The pre-edit state. See Surface.preeditCallback for more information.
 pub const Preedit = struct {
-    /// The codepoints to render as preedit text. We allow up to 16 codepoints
+    /// The codepoints to render as preedit text.
-    /// as a sort of arbitrary limit. If we experience a realisitic use case
+    codepoints: []const Codepoint = &.{},
-    /// where we need more please open an issue.
-    codepoints: [16]Codepoint = undefined,
-    len: u8 = 0,
 
     /// A single codepoint to render as preedit text.
     pub const Codepoint = struct {
@@ -50,21 +47,67 @@ pub const Preedit = struct {
         wide: bool = false,
     };
 
+    /// Deinit this preedit that was cre
+    pub fn deinit(self: *const Preedit, alloc: Allocator) void {
+        alloc.free(self.codepoints);
+    }
+
+    /// Allocate a copy of this preedit in the given allocator..
+    pub fn clone(self: *const Preedit, alloc: Allocator) !Preedit {
+        return .{
+            .codepoints = try alloc.dupe(Codepoint, self.codepoints),
+        };
+    }
+
     /// The width in cells of all codepoints in the preedit.
     pub fn width(self: *const Preedit) usize {
         var result: usize = 0;
-        for (self.codepoints[0..self.len]) |cp| {
+        for (self.codepoints) |cp| {
             result += if (cp.wide) 2 else 1;
         }
 
         return result;
     }
 
-    pub fn range(self: *const Preedit, start: usize, max: usize) [2]usize {
+    /// Range returns the start and end x position of the preedit text
+    /// along with any codepoint offset necessary to fit the preedit
+    /// into the available space.
+    pub fn range(self: *const Preedit, start: usize, max: usize) struct {
+        start: usize,
+        end: usize,
+        cp_offset: usize,
+    } {
+        // If our width is greater than the number of cells we have
+        // then we need to adjust our codepoint start to a point where
+        // our width would be less than the number of cells we have.
+        const w, const cp_offset = width: {
+            // max is inclusive, so we need to add 1 to it.
+            const max_width = max - start + 1;
+
+            // Rebuild our width in reverse order. This is because we want
+            // to offset by the end cells, not the start cells (if we have to).
+            var w: usize = 0;
+            for (0..self.codepoints.len) |i| {
+                const reverse_i = self.codepoints.len - i - 1;
+                const cp = self.codepoints[reverse_i];
+                w += if (cp.wide) 2 else 1;
+                if (w > max_width) {
+                    break :width .{ w, reverse_i };
+                }
+            }
+
+            // Width fit in the max width so no offset necessary.
+            break :width .{ w, 0 };
+        };
+
         // If our preedit goes off the end of the screen, we adjust it so
         // that it shifts left.
-        const end = start + self.width();
+        const end = start + w;
-        const offset = if (end > max) end - max else 0;
+        const start_offset = if (end > max) end - max else 0;
-        return .{ start -| offset, end -| offset };
+        return .{
+            .start = start -| start_offset,
+            .end = end -| start_offset,
+            .cp_offset = cp_offset,
+        };
     }
 };