Merge pull request #888 from mitchellh/asian-input

Japanese input, handle multi-code point preedit states

src/Surface.zig

@@ -1025,25 +1025,50 @@ fn resize(self: *Surface, size: renderer.ScreenSize) !void {
 /// The core surface will NOT reset the preedit state on charCallback or
 /// keyCallback and we rely completely on the apprt implementation to track
 /// the preedit state correctly.
-pub fn preeditCallback(self: *Surface, preedit_: ?u21) !void {
+///
-    // log.debug("preedit cp={any}", .{preedit_});
+/// The preedit input must be UTF-8 encoded.
-
+pub fn preeditCallback(self: *Surface, preedit_: ?[]const u8) !void {
-    const preedit: ?renderer.State.Preedit = if (preedit_) |cp| preedit: {
-        const width = ziglyph.display_width.codePointWidth(cp, .half);
-
-        // This shouldn't ever happen in well-behaved programs because
-        // preedit text must be visible, but we want to protect against it
-        // at this point.
-        if (width <= 0) break :preedit null;
-
-        break :preedit .{
-            .codepoint = cp,
-            .wide = width >= 2,
-        };
-    } else null;
-
     self.renderer_state.mutex.lock();
     defer self.renderer_state.mutex.unlock();
+
+    // We always clear our prior preedit
+    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).
+    const text = preedit_ orelse {
+        try self.queueRender();
+        return;
+    };
+
+    // We convert the UTF-8 text to codepoints.
+    const view = try std.unicode.Utf8View.init(text);
+    var it = view.iterator();
+
+    // Allocate the codepoints slice
+    var preedit: renderer.State.Preedit = .{};
+    while (it.nextCodepoint()) |cp| {
+        const width = ziglyph.display_width.codePointWidth(cp, .half);
+
+        // I've never seen a preedit text with a zero-width character. In
+        // theory its possible but we can't really handle it right now.
+        // Let's just ignore it.
+        if (width <= 0) continue;
+
+        preedit.codepoints[preedit.len] = .{ .codepoint = cp, .wide = width >= 2 };
+        preedit.len += 1;
+
+        // 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;
+
     self.renderer_state.preedit = preedit;
     try self.queueRender();
 }
@@ -1055,7 +1080,7 @@ pub fn keyCallback(
     self: *Surface,
     event: input.KeyEvent,
 ) !bool {
-    // log.debug("keyCallback event={}", .{event});
+    // log.debug("text keyCallback event={}", .{event});
 
     // Setup our inspector event if we have an inspector.
     var insp_ev: ?inspector.key.Event = if (self.inspector != null) ev: {

src/apprt/embedded.zig

@@ -702,14 +702,7 @@ pub const Surface = struct {
             // If this is a dead key, then we're composing a character and
             // we need to set our proper preedit state.
             if (result.composing) {
-                const view = std.unicode.Utf8View.init(result.text) catch |err| {
+                self.core_surface.preeditCallback(result.text) catch |err| {
-                    log.warn("cannot build utf8 view over input: {}", .{err});
-                    return;
-                };
-                var it = view.iterator();
-
-                const cp: u21 = it.nextCodepoint() orelse 0;
-                self.core_surface.preeditCallback(cp) catch |err| {
                     log.err("error in preedit callback err={}", .{err});
                     return;
                 };

src/apprt/gtk/Surface.zig

@@ -894,14 +894,7 @@ fn keyEvent(
         // we need to set our proper preedit state.
         if (self.im_composing) preedit: {
             const text = self.im_buf[0..self.im_len];
-            const view = std.unicode.Utf8View.init(text) catch |err| {
+            self.core_surface.preeditCallback(text) catch |err| {
-                log.warn("cannot build utf8 view over input: {}", .{err});
-                break :preedit;
-            };
-            var it = view.iterator();
-
-            const cp: u21 = it.nextCodepoint() orelse 0;
-            self.core_surface.preeditCallback(cp) catch |err| {
                 log.err("error in preedit callback err={}", .{err});
                 break :preedit;
             };

src/input/KeyEncoder.zig

@@ -204,7 +204,16 @@ fn legacy(
         self.cursor_key_application,
         self.keypad_key_application,
         self.modify_other_keys_state_2,
-    )) |sequence| return copyToBuf(buf, sequence);
+    )) |sequence| pc_style: {
+        // If we're pressing enter and have UTF-8 text, we probably are
+        // clearing a dead key state. This happens specifically on macOS.
+        // We have a unit test for this.
+        if (self.event.key == .enter and self.event.utf8.len > 0) {
+            break :pc_style;
+        }
+
+        return copyToBuf(buf, sequence);
+    }
 
     // If we match a control sequence, we output that directly. For
     // ctrlSeq we have to use all mods because we want it to only
@@ -1168,6 +1177,20 @@ test "kitty: alternates omit control characters" {
     try testing.expectEqualStrings("\x1b[3~", actual);
 }
 
+test "legacy: enter with utf8 (dead key state)" {
+    var buf: [128]u8 = undefined;
+    var enc: KeyEncoder = .{
+        .event = .{
+            .key = .enter,
+            .utf8 = "A",
+            .unshifted_codepoint = 0x0D,
+        },
+    };
+
+    const actual = try enc.legacy(&buf);
+    try testing.expectEqualStrings("A", actual);
+}
+
 test "legacy: ctrl+alt+c" {
     var buf: [128]u8 = undefined;
     var enc: KeyEncoder = .{

src/renderer/Metal.zig

@@ -1095,6 +1095,18 @@ fn rebuildCells(
         (screen.rows * screen.cols * 2) + 1,
     );
 
+    // Determine our x/y range for preedit. We don't want to render anything
+    // here because we will render the preedit separately.
+    const preedit_range: ?struct {
+        y: usize,
+        x: [2]usize,
+    } = if (preedit) |preedit_v| preedit: {
+        break :preedit .{
+            .y = screen.cursor.y,
+            .x = preedit_v.range(screen.cursor.x, screen.cols - 1),
+        };
+    } else null;
+
     // This is the cell that has [mode == .fg] and is underneath our cursor.
     // We keep track of it so that we can invert the colors so the character
     // remains visible.
@@ -1175,6 +1187,18 @@ fn rebuildCells(
         );
         while (try iter.next(self.alloc)) |run| {
             for (try self.font_shaper.shape(run)) |shaper_cell| {
+                // If this cell falls within our preedit range then we skip it.
+                // We do this so we don't have conflicting data on the same
+                // cell.
+                if (preedit_range) |range| {
+                    if (range.y == y and
+                        shaper_cell.x >= range.x[0] and
+                        shaper_cell.x <= range.x[1])
+                    {
+                        continue;
+                    }
+                }
+
                 if (self.updateCell(
                     term_selection,
                     screen,
@@ -1202,30 +1226,29 @@ fn rebuildCells(
     // Add the cursor at the end so that it overlays everything. If we have
     // a cursor cell then we invert the colors on that and add it in so
     // that we can always see it.
-    if (cursor_style_) |cursor_style| {
+    if (cursor_style_) |cursor_style| cursor_style: {
-        const real_cursor_cell = self.addCursor(screen, cursor_style, preedit);
-
         // If we have a preedit, we try to render the preedit text on top
         // of the cursor.
-        if (preedit) |preedit_v| preedit: {
+        if (preedit) |preedit_v| {
-            if (preedit_v.codepoint > 0) {
+            const range = preedit_range.?;
-                // We try to base on the cursor cell but if its not there
+            var x = range.x[0];
-                // we use the actual cursor and if thats not there we give
+            for (preedit_v.codepoints[0..preedit_v.len]) |cp| {
-                // up on preedit rendering.
+                self.addPreeditCell(cp, x, range.y) catch |err| {
-                var cell: mtl_shaders.Cell = cursor_cell orelse
+                    log.warn("error building preedit cell, will be invalid x={} y={}, err={}", .{
-                    (real_cursor_cell orelse break :preedit).*;
+                        x,
-                cell.color = .{ 0, 0, 0, 255 };
+                        range.y,
-                cell.cell_width = if (preedit_v.wide) 2 else 1;
+                        err,
+                    });
+                };
 
-                // If preedit rendering succeeded then we don't want to
+                x += if (cp.wide) 2 else 1;
-                // re-render the underlying cell fg
-                if (self.updateCellChar(&cell, preedit_v.codepoint)) {
-                    cursor_cell = null;
-                    self.cells.appendAssumeCapacity(cell);
-                }
-            }
             }
 
+            // Preedit hides the cursor
+            break :cursor_style;
+        }
+
+        _ = self.addCursor(screen, cursor_style);
         if (cursor_cell) |*cell| {
             if (cell.mode == .fg) {
                 cell.color = if (self.config.cursor_text) |txt|
@@ -1428,18 +1451,10 @@ fn addCursor(
     self: *Metal,
     screen: *terminal.Screen,
     cursor_style: renderer.CursorStyle,
-    preedit: ?renderer.State.Preedit,
 ) ?*const mtl_shaders.Cell {
     // Add the cursor. We render the cursor over the wide character if
     // we're on the wide characer tail.
     const wide, const x = cell: {
-        // If we have preedit text, our width is based on that.
-        if (preedit) |p| {
-            if (p.codepoint > 0) {
-                break :cell .{ p.wide, screen.cursor.x };
-            }
-        }
-
         // The cursor goes over the screen cursor position.
         const cell = screen.getCell(
             .active,
@@ -1497,25 +1512,32 @@ fn addCursor(
     return &self.cells.items[self.cells.items.len - 1];
 }
 
-/// Updates cell with the the given character. This returns true if the
+fn addPreeditCell(
-/// cell was successfully updated.
+    self: *Metal,
-fn updateCellChar(self: *Metal, cell: *mtl_shaders.Cell, cp: u21) bool {
+    cp: renderer.State.Preedit.Codepoint,
-    // Get the font index for this codepoint
+    x: usize,
+    y: usize,
+) !void {
+    // Preedit is rendered inverted
+    const bg = self.foreground_color;
+    const fg = self.background_color;
+
+    // Get the font for this codepoint.
     const font_index = if (self.font_group.indexForCodepoint(
         self.alloc,
-        @intCast(cp),
+        @intCast(cp.codepoint),
         .regular,
         .text,
-    )) |index| index orelse return false else |_| return false;
+    )) |index| index orelse return else |_| return;
 
     // Get the font face so we can get the glyph
     const face = self.font_group.group.faceFromIndex(font_index) catch |err| {
         log.warn("error getting face for font_index={} err={}", .{ font_index, err });
-        return false;
+        return;
     };
 
     // Use the face to now get the glyph index
-    const glyph_index = face.glyphIndex(@intCast(cp)) orelse return false;
+    const glyph_index = face.glyphIndex(@intCast(cp.codepoint)) orelse return;
 
     // Render the glyph for our preedit text
     const glyph = self.font_group.renderGlyph(
@@ -1525,14 +1547,27 @@ fn updateCellChar(self: *Metal, cell: *mtl_shaders.Cell, cp: u21) bool {
         .{},
     ) catch |err| {
         log.warn("error rendering preedit glyph err={}", .{err});
-        return false;
+        return;
     };
 
-    // Update the cell glyph
+    // Add our opaque background cell
-    cell.glyph_pos = .{ glyph.atlas_x, glyph.atlas_y };
+    self.cells_bg.appendAssumeCapacity(.{
-    cell.glyph_size = .{ glyph.width, glyph.height };
+        .mode = .bg,
-    cell.glyph_offset = .{ glyph.offset_x, glyph.offset_y };
+        .grid_pos = .{ @as(f32, @floatFromInt(x)), @as(f32, @floatFromInt(y)) },
-    return true;
+        .cell_width = if (cp.wide) 2 else 1,
+        .color = .{ bg.r, bg.g, bg.b, 255 },
+    });
+
+    // Add our text
+    self.cells.appendAssumeCapacity(.{
+        .mode = .fg,
+        .grid_pos = .{ @as(f32, @floatFromInt(x)), @as(f32, @floatFromInt(y)) },
+        .cell_width = if (cp.wide) 2 else 1,
+        .color = .{ fg.r, fg.g, fg.b, 255 },
+        .glyph_pos = .{ glyph.atlas_x, glyph.atlas_y },
+        .glyph_size = .{ glyph.width, glyph.height },
+        .glyph_offset = .{ glyph.offset_x, glyph.offset_y },
+    });
 }
 
 /// Sync the atlas data to the given texture. This copies the bytes

src/renderer/OpenGL.zig

@@ -718,6 +718,18 @@ pub fn rebuildCells(
     // We've written no data to the GPU, refresh it all
     self.gl_cells_written = 0;
 
+    // Determine our x/y range for preedit. We don't want to render anything
+    // here because we will render the preedit separately.
+    const preedit_range: ?struct {
+        y: usize,
+        x: [2]usize,
+    } = if (preedit) |preedit_v| preedit: {
+        break :preedit .{
+            .y = screen.cursor.y,
+            .x = preedit_v.range(screen.cursor.x, screen.cols - 1),
+        };
+    } else null;
+
     // This is the cell that has [mode == .fg] and is underneath our cursor.
     // We keep track of it so that we can invert the colors so the character
     // remains visible.
@@ -789,6 +801,18 @@ pub fn rebuildCells(
         );
         while (try iter.next(self.alloc)) |run| {
             for (try self.font_shaper.shape(run)) |shaper_cell| {
+                // If this cell falls within our preedit range then we skip it.
+                // We do this so we don't have conflicting data on the same
+                // cell.
+                if (preedit_range) |range| {
+                    if (range.y == y and
+                        shaper_cell.x >= range.x[0] and
+                        shaper_cell.x <= range.x[1])
+                    {
+                        continue;
+                    }
+                }
+
                 if (self.updateCell(
                     term_selection,
                     screen,
@@ -816,33 +840,29 @@ pub fn rebuildCells(
     // Add the cursor at the end so that it overlays everything. If we have
     // a cursor cell then we invert the colors on that and add it in so
     // that we can always see it.
-    if (cursor_style_) |cursor_style| {
+    if (cursor_style_) |cursor_style| cursor_style: {
-        const real_cursor_cell = self.addCursor(screen, cursor_style, preedit);
-
         // If we have a preedit, we try to render the preedit text on top
         // of the cursor.
-        if (preedit) |preedit_v| preedit: {
+        if (preedit) |preedit_v| {
-            if (preedit_v.codepoint > 0) {
+            const range = preedit_range.?;
-                // We try to base on the cursor cell but if its not there
+            var x = range.x[0];
-                // we use the actual cursor and if thats not there we give
+            for (preedit_v.codepoints[0..preedit_v.len]) |cp| {
-                // up on preedit rendering.
+                self.addPreeditCell(cp, x, range.y) catch |err| {
-                var cell: GPUCell = cursor_cell orelse
+                    log.warn("error building preedit cell, will be invalid x={} y={}, err={}", .{
-                    (real_cursor_cell orelse break :preedit).*;
+                        x,
-                cell.fg_r = 0;
+                        range.y,
-                cell.fg_g = 0;
+                        err,
-                cell.fg_b = 0;
+                    });
-                cell.fg_a = 255;
+                };
-                cell.grid_width = if (preedit_v.wide) 2 else 1;
 
-                // If preedit rendering succeeded then we don't want to
+                x += if (cp.wide) 2 else 1;
-                // re-render the underlying cell fg
-                if (self.updateCellChar(&cell, preedit_v.codepoint)) {
-                    cursor_cell = null;
-                    self.cells.appendAssumeCapacity(cell);
-                }
-            }
             }
 
+            // Preedit hides the cursor
+            break :cursor_style;
+        }
+
+        _ = self.addCursor(screen, cursor_style);
         if (cursor_cell) |*cell| {
             if (cell.mode == .fg) {
                 if (self.config.cursor_text) |txt| {
@@ -868,22 +888,97 @@ pub fn rebuildCells(
     }
 }
 
+fn addPreeditCell(
+    self: *OpenGL,
+    cp: renderer.State.Preedit.Codepoint,
+    x: usize,
+    y: usize,
+) !void {
+    // Preedit is rendered inverted
+    const bg = self.foreground_color;
+    const fg = self.background_color;
+
+    // Get the font for this codepoint.
+    const font_index = if (self.font_group.indexForCodepoint(
+        self.alloc,
+        @intCast(cp.codepoint),
+        .regular,
+        .text,
+    )) |index| index orelse return else |_| return;
+
+    // Get the font face so we can get the glyph
+    const face = self.font_group.group.faceFromIndex(font_index) catch |err| {
+        log.warn("error getting face for font_index={} err={}", .{ font_index, err });
+        return;
+    };
+
+    // Use the face to now get the glyph index
+    const glyph_index = face.glyphIndex(@intCast(cp.codepoint)) orelse return;
+
+    // Render the glyph for our preedit text
+    const glyph = self.font_group.renderGlyph(
+        self.alloc,
+        font_index,
+        glyph_index,
+        .{},
+    ) catch |err| {
+        log.warn("error rendering preedit glyph err={}", .{err});
+        return;
+    };
+
+    // Add our opaque background cell
+    self.cells_bg.appendAssumeCapacity(.{
+        .mode = .bg,
+        .grid_col = @intCast(x),
+        .grid_row = @intCast(y),
+        .grid_width = if (cp.wide) 2 else 1,
+        .glyph_x = 0,
+        .glyph_y = 0,
+        .glyph_width = 0,
+        .glyph_height = 0,
+        .glyph_offset_x = 0,
+        .glyph_offset_y = 0,
+        .fg_r = 0,
+        .fg_g = 0,
+        .fg_b = 0,
+        .fg_a = 0,
+        .bg_r = bg.r,
+        .bg_g = bg.g,
+        .bg_b = bg.b,
+        .bg_a = 255,
+    });
+
+    // Add our text
+    self.cells.appendAssumeCapacity(.{
+        .mode = .fg,
+        .grid_col = @intCast(x),
+        .grid_row = @intCast(y),
+        .grid_width = if (cp.wide) 2 else 1,
+        .glyph_x = glyph.atlas_x,
+        .glyph_y = glyph.atlas_y,
+        .glyph_width = glyph.width,
+        .glyph_height = glyph.height,
+        .glyph_offset_x = glyph.offset_x,
+        .glyph_offset_y = glyph.offset_y,
+        .fg_r = fg.r,
+        .fg_g = fg.g,
+        .fg_b = fg.b,
+        .fg_a = 255,
+        .bg_r = 0,
+        .bg_g = 0,
+        .bg_b = 0,
+        .bg_a = 0,
+    });
+}
+
 fn addCursor(
     self: *OpenGL,
     screen: *terminal.Screen,
     cursor_style: renderer.CursorStyle,
-    preedit: ?renderer.State.Preedit,
 ) ?*const GPUCell {
     // Add the cursor. We render the cursor over the wide character if
     // we're on the wide characer tail.
     const wide, const x = cell: {
-        // If we have preedit text, our width is based on that.
-        if (preedit) |p| {
-            if (p.codepoint > 0) {
-                break :cell .{ p.wide, screen.cursor.x };
-            }
-        }
-
         // The cursor goes over the screen cursor position.
         const cell = screen.getCell(
             .active,
@@ -949,47 +1044,6 @@ fn addCursor(
     return &self.cells.items[self.cells.items.len - 1];
 }
 
-/// Updates cell with the the given character. This returns true if the
-/// cell was successfully updated.
-fn updateCellChar(self: *OpenGL, cell: *GPUCell, cp: u21) bool {
-    // Get the font index for this codepoint
-    const font_index = if (self.font_group.indexForCodepoint(
-        self.alloc,
-        @intCast(cp),
-        .regular,
-        .text,
-    )) |index| index orelse return false else |_| return false;
-
-    // Get the font face so we can get the glyph
-    const face = self.font_group.group.faceFromIndex(font_index) catch |err| {
-        log.warn("error getting face for font_index={} err={}", .{ font_index, err });
-        return false;
-    };
-
-    // Use the face to now get the glyph index
-    const glyph_index = face.glyphIndex(@intCast(cp)) orelse return false;
-
-    // Render the glyph for our preedit text
-    const glyph = self.font_group.renderGlyph(
-        self.alloc,
-        font_index,
-        glyph_index,
-        .{},
-    ) catch |err| {
-        log.warn("error rendering preedit glyph err={}", .{err});
-        return false;
-    };
-
-    // Update the cell glyph
-    cell.glyph_x = glyph.atlas_x;
-    cell.glyph_y = glyph.atlas_y;
-    cell.glyph_width = glyph.width;
-    cell.glyph_height = glyph.height;
-    cell.glyph_offset_x = glyph.offset_x;
-    cell.glyph_offset_y = glyph.offset_y;
-    return true;
-}
-
 /// Update a single cell. The bool returns whether the cell was updated
 /// or not. If the cell wasn't updated, a full refreshCells call is
 /// needed.

src/renderer/State.zig

@@ -27,14 +27,33 @@ preedit: ?Preedit = null,
 
 /// The pre-edit state. See Surface.preeditCallback for more information.
 pub const Preedit = struct {
-    /// The codepoint to render as preedit text. We only support single
+    /// The codepoints to render as preedit text. We allow up to 16 codepoints
-    /// codepoint for now. In theory this can be multiple codepoints but
+    /// as a sort of arbitrary limit. If we experience a realisitic use case
-    /// that is left as a future exercise.
+    /// where we need more please open an issue.
-    ///
+    codepoints: [16]Codepoint = undefined,
-    /// This can also be "0" in which case we can know we're in a preedit
+    len: u8 = 0,
-    /// mode but we don't have any preedit text to render.
-    codepoint: u21 = 0,
 
-    /// True if the preedit text should be rendered "wide" (two cells)
+    /// A single codepoint to render as preedit text.
+    pub const Codepoint = struct {
+        codepoint: u21,
         wide: bool = false,
+    };
+
+    /// 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| {
+            result += if (cp.wide) 2 else 1;
+        }
+
+        return result;
+    }
+
+    pub fn range(self: *const Preedit, start: usize, max: usize) [2]usize {
+        // If our preedit goes off the end of the screen, we adjust it so
+        // that it shifts left.
+        const end = start + self.width();
+        const offset = if (end > max) end - max else 0;
+        return .{ start -| offset, end -| offset };
+    }
 };