Calculate grid_width properly, use that instead of wide mask in shader

shaders/cell.v.glsl

@@ -11,7 +11,6 @@ const uint MODE_CURSOR_RECT = 3u;
 const uint MODE_CURSOR_RECT_HOLLOW = 4u;
 const uint MODE_CURSOR_BAR = 5u;
 const uint MODE_UNDERLINE = 6u;
-const uint MODE_WIDE_MASK = 128u; // 0b1000_0000
 
 // The grid coordinates (x, y) where x < columns and y < rows
 layout (location = 0) in vec2 grid_coord;
@@ -81,12 +80,9 @@ uniform float glyph_baseline;
  */
 
 void main() {
-    // Remove any masks from our mode
-    uint mode_unmasked = mode_in & ~MODE_WIDE_MASK;
-
     // We always forward our mode unmasked because the fragment
     // shader doesn't use any of the masks.
-    mode = mode_unmasked;
+    mode = mode_in;
 
     // Top-left cell coordinates converted to world space
     // Example: (1,0) with a 30 wide cell is converted to (30,0)
@@ -113,9 +109,7 @@ void main() {
 
     // Scaled for wide chars
     vec2 cell_size_scaled = cell_size;
-    if ((mode_in & MODE_WIDE_MASK) == MODE_WIDE_MASK) {
+    cell_size_scaled.x = cell_size_scaled.x * grid_width;
-        cell_size_scaled.x = cell_size_scaled.x * 2;
-    }
 
     switch (mode) {
     case MODE_BG:
@@ -136,7 +130,8 @@ void main() {
         // The "+ 3" here is to give some wiggle room for fonts that are
         // BARELY over it.
         vec2 glyph_size_downsampled = glyph_size;
-        if (glyph_size.y > cell_size.y + 2) {
+        if (glyph_size.y > cell_size.y + 2 ||
+            glyph_size.x > cell_size_scaled.x + 2) {
             glyph_size_downsampled.x = cell_size_scaled.x;
             glyph_size_downsampled.y = glyph_size.y * (glyph_size_downsampled.x / glyph_size.x);
             glyph_offset_calc.y = glyph_offset.y * (glyph_size_downsampled.x / glyph_size.x);

src/Grid.zig

@@ -78,7 +78,9 @@ pub const CursorStyle = enum(u8) {
 };
 
 /// The raw structure that maps directly to the buffer sent to the vertex shader.
-const GPUCell = struct {
+/// This must be "extern" so that the field order is not reordered by the
+/// Zig compiler.
+const GPUCell = extern struct {
     /// vec2 grid_coord
     grid_col: u16,
     grid_row: u16,
@@ -111,7 +113,7 @@ const GPUCell = struct {
     mode: GPUCellMode,
 
     /// The width in grid cells that a rendering takes.
-    grid_width: u16 = 1,
+    grid_width: u8,
 };
 
 const GPUCellMode = enum(u8) {
@@ -123,8 +125,6 @@ const GPUCellMode = enum(u8) {
     cursor_bar = 5,
     underline = 6,
 
-    wide_mask = 0b1000_0000,
-
     // Non-exhaustive because masks change it
     _,
 
@@ -228,7 +228,7 @@ pub fn init(
     offset += 4 * @sizeOf(u8);
     try vbobind.attributeIAdvanced(6, 1, gl.c.GL_UNSIGNED_BYTE, @sizeOf(GPUCell), offset);
     offset += 1 * @sizeOf(u8);
-    try vbobind.attributeAdvanced(7, 1, gl.c.GL_UNSIGNED_SHORT, false, @sizeOf(GPUCell), offset);
+    try vbobind.attributeIAdvanced(7, 1, gl.c.GL_UNSIGNED_BYTE, @sizeOf(GPUCell), offset);
     try vbobind.enableAttribArray(0);
     try vbobind.enableAttribArray(1);
     try vbobind.enableAttribArray(2);
@@ -413,7 +413,6 @@ fn addCursor(self: *Grid, term: *Terminal) void {
             GPUCellMode,
             @enumToInt(self.cursor_style),
         );
-        if (cell.attrs.wide) mode = mode.mask(.wide_mask);
 
         self.cells.appendAssumeCapacity(.{
             .mode = mode,
@@ -511,7 +510,6 @@ pub fn updateCell(
     // If the cell has a background, we always draw it.
     if (colors.bg) |rgb| {
         var mode: GPUCellMode = .bg;
-        if (cell.attrs.wide) mode = mode.mask(.wide_mask);
 
         self.cells.appendAssumeCapacity(.{
             .mode = mode,
@@ -549,9 +547,6 @@ pub fn updateCell(
         var mode: GPUCellMode = .fg;
         if (face.hasColor()) mode = .fg_color;
 
-        // If the cell is wide, we need to note that in the mode
-        if (cell.attrs.wide) mode = mode.mask(.wide_mask);
-
         self.cells.appendAssumeCapacity(.{
             .mode = mode,
             .grid_col = @intCast(u16, x),
@@ -575,11 +570,8 @@ pub fn updateCell(
     }
 
     if (cell.attrs.underline) {
-        var mode: GPUCellMode = .underline;
-        if (cell.attrs.wide) mode = mode.mask(.wide_mask);
-
         self.cells.appendAssumeCapacity(.{
-            .mode = mode,
+            .mode = .underline,
             .grid_col = @intCast(u16, x),
             .grid_row = @intCast(u16, y),
             .grid_width = shaper_cell.width,

src/font/Shaper.zig

@@ -54,8 +54,14 @@ pub fn runIterator(self: *Shaper, row: terminal.Screen.Row) RunIterator {
 ///
 /// If there is not enough space in the cell buffer, an error is returned.
 pub fn shape(self: *Shaper, run: TextRun) ![]Cell {
+    // TODO: we do not want to hardcode these
+    const hb_feats = &[_]harfbuzz.Feature{
+        harfbuzz.Feature.fromString("dlig").?,
+        harfbuzz.Feature.fromString("liga").?,
+    };
+
     const face = self.group.group.faceFromIndex(run.font_index);
-    harfbuzz.shape(face.hb_font, self.hb_buf, null);
+    harfbuzz.shape(face.hb_font, self.hb_buf, hb_feats);
 
     // If our buffer is empty, we short-circuit the rest of the work
     // return nothing.
@@ -69,7 +75,7 @@ pub fn shape(self: *Shaper, run: TextRun) ![]Cell {
 
     // Convert all our info/pos to cells and set it.
     if (info.len > self.cell_buf.len) return error.OutOfMemory;
-    //     log.debug("info={} pos={}", .{ info.len, pos.len });
+    //log.warn("info={} pos={} run={}", .{ info.len, pos.len, run });
 
     // x is the column that we currently occupy. We start at the offset.
     var x: u16 = run.offset;
@@ -80,7 +86,7 @@ pub fn shape(self: *Shaper, run: TextRun) ![]Cell {
         // to detect since we set the cluster number to the column it
         // originated.
         const cp_width = if (i == info.len - 1)
-            run.max_cluster - v.cluster
+            (run.max_cluster - v.cluster) + 1 // + 1 because we're zero indexed
         else width: {
             const next_cluster = info[i + 1].cluster;
             break :width next_cluster - v.cluster;
@@ -89,14 +95,14 @@ pub fn shape(self: *Shaper, run: TextRun) ![]Cell {
         self.cell_buf[i] = .{
             .x = x,
             .glyph_index = v.codepoint,
-            .width = if (cp_width > 2) 2 else @intCast(u8, cp_width),
+            .width = @intCast(u8, cp_width),
         };
 
         // Increase x by the amount of codepoints we replaced so that
         // we retain the grid.
         x += @intCast(u16, cp_width);
 
-        // log.debug("i={} info={} pos={} cell={}", .{ i, v, pos[i], self.cell_buf[i] });
+        //log.warn("i={} info={} pos={} cell={}", .{ i, v, pos[i], self.cell_buf[i] });
     }
 
     return self.cell_buf[0..info.len];
@@ -153,8 +159,15 @@ pub const RunIterator = struct {
         while (j < self.row.lenCells()) : (j += 1) {
             const cell = self.row.getCell(j);
 
-            // Ignore tailing wide spacers, this will get fixed up by the shaper
+            // Ignore empty cells
-            if (cell.empty() or cell.attrs.wide_spacer_tail) continue;
+            if (cell.empty()) continue;
+
+            // If we're a spacer, then we ignore it but increase the max cluster
+            // size so that the width calculation is correct.
+            if (cell.attrs.wide_spacer_tail) {
+                max_cluster = j;
+                continue;
+            }
 
             const style: Style = if (cell.attrs.bold)
                 .bold
@@ -232,6 +245,19 @@ test "run iterator" {
         try testing.expectEqual(@as(usize, 1), count);
     }
 
+    // Spaces should be part of a run
+    {
+        var screen = try terminal.Screen.init(alloc, 3, 10, 0);
+        defer screen.deinit(alloc);
+        screen.testWriteString("ABCD   EFG");
+
+        var shaper = testdata.shaper;
+        var it = shaper.runIterator(screen.getRow(.{ .screen = 0 }));
+        var count: usize = 0;
+        while (try it.next(alloc)) |_| count += 1;
+        try testing.expectEqual(@as(usize, 1), count);
+    }
+
     {
         // Make a screen with some data
         var screen = try terminal.Screen.init(alloc, 3, 5, 0);
@@ -282,6 +308,76 @@ test "shape" {
     try testing.expectEqual(@as(usize, 1), count);
 }
 
+test "shape inconsolata ligs" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    var testdata = try testShaper(alloc);
+    defer testdata.deinit();
+
+    {
+        var screen = try terminal.Screen.init(alloc, 3, 5, 0);
+        defer screen.deinit(alloc);
+        screen.testWriteString(">=");
+
+        var shaper = testdata.shaper;
+        var it = shaper.runIterator(screen.getRow(.{ .screen = 0 }));
+        var count: usize = 0;
+        while (try it.next(alloc)) |run| {
+            count += 1;
+
+            const cells = try shaper.shape(run);
+            try testing.expectEqual(@as(usize, 1), cells.len);
+            try testing.expectEqual(@as(u8, 2), cells[0].width);
+        }
+        try testing.expectEqual(@as(usize, 1), count);
+    }
+
+    {
+        var screen = try terminal.Screen.init(alloc, 3, 5, 0);
+        defer screen.deinit(alloc);
+        screen.testWriteString("===");
+
+        var shaper = testdata.shaper;
+        var it = shaper.runIterator(screen.getRow(.{ .screen = 0 }));
+        var count: usize = 0;
+        while (try it.next(alloc)) |run| {
+            count += 1;
+
+            const cells = try shaper.shape(run);
+            try testing.expectEqual(@as(usize, 1), cells.len);
+            try testing.expectEqual(@as(u8, 3), cells[0].width);
+        }
+        try testing.expectEqual(@as(usize, 1), count);
+    }
+}
+
+test "shape emoji width" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    var testdata = try testShaper(alloc);
+    defer testdata.deinit();
+
+    {
+        var screen = try terminal.Screen.init(alloc, 3, 5, 0);
+        defer screen.deinit(alloc);
+        screen.testWriteString("👍");
+
+        var shaper = testdata.shaper;
+        var it = shaper.runIterator(screen.getRow(.{ .screen = 0 }));
+        var count: usize = 0;
+        while (try it.next(alloc)) |run| {
+            count += 1;
+
+            const cells = try shaper.shape(run);
+            try testing.expectEqual(@as(usize, 1), cells.len);
+            try testing.expectEqual(@as(u8, 2), cells[0].width);
+        }
+        try testing.expectEqual(@as(usize, 1), count);
+    }
+}
+
 const TestShaper = struct {
     alloc: Allocator,
     shaper: Shaper,