Merge pull request #358 from mitchellh/lig-flash

Ligature shaping fixes

src/font/shaper/harfbuzz.zig

@@ -229,14 +229,14 @@ test "run iterator: empty cells with background set" {
         // Make a screen with some data
         var screen = try terminal.Screen.init(alloc, 3, 5, 0);
         defer screen.deinit();
+        screen.cursor.pen.bg = try terminal.color.Name.cyan.default();
+        screen.cursor.pen.attrs.has_bg = true;
         try screen.testWriteString("A");
 
         // Get our first row
         const row = screen.getRow(.{ .active = 0 });
-        row.getCellPtr(1).bg = try terminal.color.Name.cyan.default();
+        row.getCellPtr(1).* = screen.cursor.pen;
-        row.getCellPtr(1).attrs.has_bg = true;
+        row.getCellPtr(2).* = screen.cursor.pen;
-        row.getCellPtr(2).fg = try terminal.color.Name.yellow.default();
-        row.getCellPtr(2).attrs.has_fg = true;
 
         // Get our run iterator
         var shaper = testdata.shaper;
@@ -760,6 +760,107 @@ test "shape cursor boundary and colored emoji" {
     }
 }
 
+test "shape cell attribute change" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    var testdata = try testShaper(alloc);
+    defer testdata.deinit();
+
+    // Plain >= should shape into 1 run
+    {
+        var screen = try terminal.Screen.init(alloc, 3, 10, 0);
+        defer screen.deinit();
+        try screen.testWriteString(">=");
+
+        var shaper = testdata.shaper;
+        var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null, null);
+        var count: usize = 0;
+        while (try it.next(alloc)) |run| {
+            count += 1;
+            _ = try shaper.shape(run);
+        }
+        try testing.expectEqual(@as(usize, 1), count);
+    }
+
+    // Bold vs regular should split
+    {
+        var screen = try terminal.Screen.init(alloc, 3, 10, 0);
+        defer screen.deinit();
+        try screen.testWriteString(">");
+        screen.cursor.pen.attrs.bold = true;
+        try screen.testWriteString("=");
+
+        var shaper = testdata.shaper;
+        var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null, null);
+        var count: usize = 0;
+        while (try it.next(alloc)) |run| {
+            count += 1;
+            _ = try shaper.shape(run);
+        }
+        try testing.expectEqual(@as(usize, 2), count);
+    }
+
+    // Changing fg color should split
+    {
+        var screen = try terminal.Screen.init(alloc, 3, 10, 0);
+        defer screen.deinit();
+        screen.cursor.pen.attrs.has_fg = true;
+        screen.cursor.pen.fg = .{ .r = 1, .g = 2, .b = 3 };
+        try screen.testWriteString(">");
+        screen.cursor.pen.fg = .{ .r = 3, .g = 2, .b = 1 };
+        try screen.testWriteString("=");
+
+        var shaper = testdata.shaper;
+        var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null, null);
+        var count: usize = 0;
+        while (try it.next(alloc)) |run| {
+            count += 1;
+            _ = try shaper.shape(run);
+        }
+        try testing.expectEqual(@as(usize, 2), count);
+    }
+
+    // Changing bg color should split
+    {
+        var screen = try terminal.Screen.init(alloc, 3, 10, 0);
+        defer screen.deinit();
+        screen.cursor.pen.attrs.has_bg = true;
+        screen.cursor.pen.bg = .{ .r = 1, .g = 2, .b = 3 };
+        try screen.testWriteString(">");
+        screen.cursor.pen.bg = .{ .r = 3, .g = 2, .b = 1 };
+        try screen.testWriteString("=");
+
+        var shaper = testdata.shaper;
+        var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null, null);
+        var count: usize = 0;
+        while (try it.next(alloc)) |run| {
+            count += 1;
+            _ = try shaper.shape(run);
+        }
+        try testing.expectEqual(@as(usize, 2), count);
+    }
+
+    // Same bg color should not split
+    {
+        var screen = try terminal.Screen.init(alloc, 3, 10, 0);
+        defer screen.deinit();
+        screen.cursor.pen.attrs.has_bg = true;
+        screen.cursor.pen.bg = .{ .r = 1, .g = 2, .b = 3 };
+        try screen.testWriteString(">");
+        try screen.testWriteString("=");
+
+        var shaper = testdata.shaper;
+        var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null, null);
+        var count: usize = 0;
+        while (try it.next(alloc)) |run| {
+            count += 1;
+            _ = try shaper.shape(run);
+        }
+        try testing.expectEqual(@as(usize, 1), count);
+    }
+}
+
 const TestShaper = struct {
     alloc: Allocator,
     shaper: Shaper,

src/font/shaper/run.zig

@@ -77,6 +77,20 @@ pub const RunIterator = struct {
             // If we're a spacer, then we ignore it
             if (cell.attrs.wide_spacer_tail) continue;
 
+            // If our cell attributes are changing, then we split the run.
+            // This prevents a single glyph for ">=" to be rendered with
+            // one color when the two components have different styling.
+            if (j > self.i) {
+                const prev_cell = self.row.getCell(j - 1);
+                const Attrs = @TypeOf(cell.attrs);
+                const Int = @typeInfo(Attrs).Struct.backing_integer.?;
+                const prev_attrs: Int = @bitCast(prev_cell.attrs.styleAttrs());
+                const attrs: Int = @bitCast(cell.attrs.styleAttrs());
+                if (prev_attrs != attrs) break;
+                if (cell.attrs.has_bg and !cell.bg.eql(prev_cell.bg)) break;
+                if (cell.attrs.has_fg and !cell.fg.eql(prev_cell.fg)) break;
+            }
+
             // Text runs break when font styles change so we need to get
             // the proper style.
             const style: font.Style = style: {

src/renderer/Metal.zig

@@ -1148,6 +1148,11 @@ fn rebuildCells(
             screen.viewportIsBottom() and
             y == screen.cursor.y;
 
+        // True if we want to do font shaping around the cursor. We want to
+        // do font shaping as long as the cursor is enabled.
+        const shape_cursor = screen.viewportIsBottom() and
+            y == screen.cursor.y;
+
         // If this is the row with our cursor, then we may have to modify
         // the cell with the cursor.
         const start_i: usize = self.cells.items.len;
@@ -1183,7 +1188,7 @@ fn rebuildCells(
             self.font_group,
             row,
             row_selection,
-            if (cursor_row) screen.cursor.x else null,
+            if (shape_cursor) screen.cursor.x else null,
         );
         while (try iter.next(self.alloc)) |run| {
             for (try self.font_shaper.shape(run)) |shaper_cell| {

src/renderer/OpenGL.zig

@@ -902,6 +902,11 @@ pub fn rebuildCells(
             screen.viewportIsBottom() and
             y == screen.cursor.y;
 
+        // True if we want to do font shaping around the cursor. We want to
+        // do font shaping as long as the cursor is enabled.
+        const shape_cursor = screen.viewportIsBottom() and
+            y == screen.cursor.y;
+
         // If this is the row with our cursor, then we may have to modify
         // the cell with the cursor.
         const start_i: usize = self.cells.items.len;
@@ -940,7 +945,7 @@ pub fn rebuildCells(
             self.font_group,
             row,
             selection,
-            if (cursor_row) screen.cursor.x else null,
+            if (shape_cursor) screen.cursor.x else null,
         );
         while (try iter.next(self.alloc)) |run| {
             for (try self.font_shaper.shape(run)) |shaper_cell| {

src/terminal/Screen.zig

@@ -218,6 +218,16 @@ pub const Cell = struct {
         /// also be true. The grapheme code points can be looked up in the
         /// screen grapheme map.
         grapheme: bool = false,
+
+        /// Returns only the attributes related to style.
+        pub fn styleAttrs(self: @This()) @This() {
+            var copy = self;
+            copy.wide = false;
+            copy.wide_spacer_tail = false;
+            copy.wide_spacer_head = false;
+            copy.grapheme = false;
+            return copy;
+        }
     } = .{},
 
     /// True if the cell should be skipped for drawing
@@ -2666,6 +2676,7 @@ pub fn testWriteString(self: *Screen, text: []const u8) !void {
         switch (width) {
             1 => {
                 const cell = row.getCellPtr(x);
+                cell.* = self.cursor.pen;
                 cell.char = @intCast(c);
 
                 grapheme.x = x;
@@ -2691,6 +2702,7 @@ pub fn testWriteString(self: *Screen, text: []const u8) !void {
 
                 {
                     const cell = row.getCellPtr(x);
+                    cell.* = self.cursor.pen;
                     cell.char = @intCast(c);
                     cell.attrs.wide = true;
 

src/terminal/color.zig

@@ -99,6 +99,10 @@ pub const RGB = struct {
     g: u8 = 0,
     b: u8 = 0,
 
+    pub fn eql(self: RGB, other: RGB) bool {
+        return self.r == other.r and self.g == other.g and self.b == other.b;
+    }
+
     test "size" {
         try std.testing.expectEqual(@as(usize, 24), @bitSizeOf(RGB));
         try std.testing.expectEqual(@as(usize, 3), @sizeOf(RGB));