Merge pull request #1056 from mitchellh/ct-shaping

CoreText font shaper, fix glyph offset handling (in Harfbuzz, too)

src/font/face/coretext.zig

@@ -77,6 +77,7 @@ pub const Face = struct {
     pub fn initFont(ct_font: *macos.text.Font, opts: font.face.Options) !Face {
         var hb_font = try harfbuzz.coretext.createFont(ct_font);
         errdefer hb_font.destroy();
+        hb_font.setScale(opts.size.pixels(), opts.size.pixels());
 
         const traits = ct_font.getSymbolicTraits();
         const metrics = metrics: {

src/font/shape.zig

@@ -1,6 +1,7 @@
 const builtin = @import("builtin");
 const options = @import("main.zig").options;
 const harfbuzz = @import("shaper/harfbuzz.zig");
+const coretext = @import("shaper/coretext.zig");
 pub const web_canvas = @import("shaper/web_canvas.zig");
 pub usingnamespace @import("shaper/run.zig");
 
@@ -12,6 +13,10 @@ pub const Shaper = switch (options.backend) {
     .coretext,
     => harfbuzz.Shaper,
 
+    // Has missing features, can't be used yet. See the comments in
+    // the coretext.zig file for more details.
+    //.coretext => coretext.Shaper,
+
     .web_canvas => web_canvas.Shaper,
 };
 
@@ -24,6 +29,10 @@ pub const Cell = struct {
     /// caller has access to the original screen cell.
     x: u16,
 
+    /// An additional offset to apply to the rendering.
+    x_offset: i16 = 0,
+    y_offset: i16 = 0,
+
     /// The glyph index for this cell. The font index to use alongside
     /// this cell is available in the text run. This glyph index is only
     /// valid for a given GroupCache and FontIndex that was used to create

src/font/shaper/harfbuzz.zig

@@ -133,17 +133,43 @@ pub const Shaper = struct {
         // If it isn't true, I'd like to catch it and learn more.
         assert(info.len == pos.len);
 
+        // This keeps track of the current offsets within a single cell.
+        var cell_offset: struct {
+            cluster: u32 = 0,
+            x: i32 = 0,
+            y: i32 = 0,
+        } = .{};
+
         // Convert all our info/pos to cells and set it.
         self.cell_buf.clearRetainingCapacity();
         try self.cell_buf.ensureTotalCapacity(self.alloc, info.len);
-        for (info) |v| {
+        for (info, pos) |info_v, pos_v| {
+            if (info_v.cluster != cell_offset.cluster) cell_offset = .{
+                .cluster = info_v.cluster,
+            };
+
             self.cell_buf.appendAssumeCapacity(.{
-                .x = @intCast(v.cluster),
-                .glyph_index = v.codepoint,
+                .x = @intCast(info_v.cluster),
+                .x_offset = @intCast(cell_offset.x),
+                .y_offset = @intCast(cell_offset.y),
+                .glyph_index = info_v.codepoint,
             });
 
-            // log.warn("i={} info={} pos={} cell={}", .{ i, v, pos[i], self.cell_buf[i] });
+            if (font.options.backend.hasFreetype()) {
+                // Freetype returns 26.6 fixed point values, so we need to
+                // divide by 64 to get the actual value. I can't find any
+                // HB API to stop this.
+                cell_offset.x += pos_v.x_advance >> 6;
+                cell_offset.y += pos_v.y_advance >> 6;
+            } else {
+                cell_offset.x += pos_v.x_advance;
+                cell_offset.y += pos_v.y_advance;
+            }
+
+            // const i = self.cell_buf.items.len - 1;
+            // log.warn("i={} info={} pos={} cell={}", .{ i, info_v, pos_v, self.cell_buf.items[i] });
         }
+        //log.warn("----------------", .{});
 
         return self.cell_buf.items;
     }

src/font/shaper/run.zig

@@ -209,7 +209,10 @@ pub const RunIterator = struct {
             }
 
             // Add all the codepoints for our grapheme
-            try self.hooks.addCodepoint(cell.char, @intCast(cluster));
+            try self.hooks.addCodepoint(
+                if (cell.char == 0) ' ' else cell.char,
+                @intCast(cluster),
+            );
             if (cell.attrs.grapheme) {
                 var it = self.row.codepointIterator(j);
                 while (it.next()) |cp| {

src/renderer/Metal.zig

@@ -1787,15 +1787,6 @@ pub fn updateCell(
             .emoji => .fg_color,
         };
 
-        // If this glyph doesn't have an advance, then we assume it is
-        // connected to the previous glyph (perhaps an unsafe assumption...)
-        // and offset by the cell width.
-        // Related: https://github.com/mitchellh/ghostty/issues/1046
-        const extra_offset: i32 = if (glyph.advance_x == 0)
-            @intCast(self.grid_metrics.cell_width)
-        else
-            0;
-
         self.cells.appendAssumeCapacity(.{
             .mode = mode,
             .grid_pos = .{ @as(f32, @floatFromInt(x)), @as(f32, @floatFromInt(y)) },
@@ -1804,7 +1795,10 @@ pub fn updateCell(
             .bg_color = bg,
             .glyph_pos = .{ glyph.atlas_x, glyph.atlas_y },
             .glyph_size = .{ glyph.width, glyph.height },
-            .glyph_offset = .{ glyph.offset_x + extra_offset, glyph.offset_y },
+            .glyph_offset = .{
+                glyph.offset_x + shaper_cell.x_offset,
+                glyph.offset_y + shaper_cell.y_offset,
+            },
         });
     }
 

src/renderer/OpenGL.zig

@@ -1490,15 +1490,6 @@ pub fn updateCell(
             .emoji => .fg_color,
         };
 
-        // If this glyph doesn't have an advance, then we assume it is
-        // connected to the previous glyph (perhaps an unsafe assumption...)
-        // and offset by the cell width.
-        // Related: https://github.com/mitchellh/ghostty/issues/1046
-        const extra_offset: i32 = if (glyph.advance_x == 0)
-            @intCast(self.grid_metrics.cell_width)
-        else
-            0;
-
         self.cells.appendAssumeCapacity(.{
             .mode = mode,
             .grid_col = @intCast(x),
@@ -1508,8 +1499,8 @@ pub fn updateCell(
             .glyph_y = glyph.atlas_y,
             .glyph_width = glyph.width,
             .glyph_height = glyph.height,
-            .glyph_offset_x = glyph.offset_x + extra_offset,
-            .glyph_offset_y = glyph.offset_y,
+            .glyph_offset_x = glyph.offset_x + shaper_cell.x_offset,
+            .glyph_offset_y = glyph.offset_y + shaper_cell.y_offset,
             .r = colors.fg.r,
             .g = colors.fg.g,
             .b = colors.fg.b,