font/freetype: disable SVG glyphs, simplify color check (#6824)

We don't currently support rendering SVG glyphs so they should be
ignored when loading. Additionally, the check for whether a glyph is
colored has been simplified by just checking the pixel mode of the
rendered bitmap.

This commit also fixes a bug caused by calling the color check inside of
`renderGlyph`, which caused the bitmap to be freed creating a chance for
memory corruption and garbled glyphs. This bug was introduced by 40c1140
(#6602) and discussed in #6781.

Author: mitchellh

pkg/freetype/face.zig

@@ -278,7 +278,9 @@ pub const LoadFlags = packed struct {
     color: bool = false,
     compute_metrics: bool = false,
     bitmap_metrics_only: bool = false,
-    _padding2: u9 = 0,
+    _padding2: u1 = 0,
+    no_svg: bool = false,
+    _padding3: u7 = 0,
 
     test {
         // This must always be an i32 size so we can bitcast directly.

src/font/face/freetype.zig

@@ -270,25 +270,21 @@ pub const Face = struct {
 
     /// Returns true if the given glyph ID is colorized.
     pub fn isColorGlyph(self: *const Face, glyph_id: u32) bool {
-        // sbix table is always true for now
-        if (self.face.hasSBIX()) return true;
-
-        // CBDT/CBLC tables always imply colorized glyphs.
-        // These are used by Noto.
-        if (self.face.hasSfntTable(freetype.Tag.init("CBDT"))) return true;
-        if (self.face.hasSfntTable(freetype.Tag.init("CBLC"))) return true;
-
-        // Otherwise, load the glyph and see what format it is in.
+        // Load the glyph and see what pixel mode it renders with.
+        // All modes other than BGRA are non-color.
+        // If the glyph fails to load, just return false.
         self.face.loadGlyph(glyph_id, .{
             .render = true,
             .color = self.face.hasColor(),
+            // NO_SVG set to true because we don't currently support rendering
+            // SVG glyphs under FreeType, since that requires bundling another
+            // dependency to handle rendering the SVG.
+            .no_svg = true,
         }) catch return false;
 
-        // If the glyph is SVG we assume colorized
         const glyph = self.face.handle.*.glyph;
-        if (glyph.*.format == freetype.c.FT_GLYPH_FORMAT_SVG) return true;
 
-        return false;
+        return glyph.*.bitmap.pixel_mode == freetype.c.FT_PIXEL_MODE_BGRA;
     }
 
     /// Render a glyph using the glyph index. The rendered glyph is stored in the
@@ -321,6 +317,11 @@ pub const Face = struct {
             .force_autohint = !self.load_flags.@"force-autohint",
             .monochrome = !self.load_flags.monochrome,
             .no_autohint = !self.load_flags.autohint,
+
+            // NO_SVG set to true because we don't currently support rendering
+            // SVG glyphs under FreeType, since that requires bundling another
+            // dependency to handle rendering the SVG.
+            .no_svg = true,
         });
         const glyph = self.face.handle.*.glyph;
 
@@ -393,12 +394,13 @@ pub const Face = struct {
             const original_width = bitmap_original.width;
             const original_height = bitmap_original.rows;
             var result = bitmap_original;
-            // TODO: We are limiting this to only emoji. We can rework this after a future
-            // improvement (promised by Qwerasd) which implements more flexible resizing rules. For
-            // now, this will suffice
-            if (self.isColorGlyph(glyph_index) and opts.cell_width != null) {
+            // TODO: We are limiting this to only color glyphs, so mainly emoji.
+            // We can rework this after a future improvement (promised by Qwerasd)
+            // which implements more flexible resizing rules.
+            if (atlas.format != .grayscale and opts.cell_width != null) {
                 const cell_width = opts.cell_width orelse unreachable;
-                // If we have a cell_width, we constrain the glyph to fit within the cell
+                // If we have a cell_width, we constrain
+                // the glyph to fit within the cell(s).
                 result.width = metrics.cell_width * @as(u32, cell_width);
                 result.rows = (result.width * original_height) / original_width;
             } else {
@@ -743,7 +745,10 @@ pub const Face = struct {
             var c: u8 = ' ';
             while (c < 127) : (c += 1) {
                 if (face.getCharIndex(c)) |glyph_index| {
-                    if (face.loadGlyph(glyph_index, .{ .render = true })) {
+                    if (face.loadGlyph(glyph_index, .{
+                        .render = true,
+                        .no_svg = true,
+                    })) {
                         max = @max(
                             f26dot6ToF64(face.handle.*.glyph.*.advance.x),
                             max,
@@ -776,15 +781,21 @@ pub const Face = struct {
             break :heights .{
                 cap: {
                     if (face.getCharIndex('H')) |glyph_index| {
-                        if (face.loadGlyph(glyph_index, .{ .render = true })) {
+                        if (face.loadGlyph(glyph_index, .{
+                            .render = true,
+                            .no_svg = true,
+                        })) {
                             break :cap f26dot6ToF64(face.handle.*.glyph.*.metrics.height);
                         } else |_| {}
                     }
                     break :cap null;
                 },
                 ex: {
                     if (face.getCharIndex('x')) |glyph_index| {
-                        if (face.loadGlyph(glyph_index, .{ .render = true })) {
+                        if (face.loadGlyph(glyph_index, .{
+                            .render = true,
+                            .no_svg = true,
+                        })) {
                             break :ex f26dot6ToF64(face.handle.*.glyph.*.metrics.height);
                         } else |_| {}
                     }

src/font/shaper/coretext.zig

@@ -1015,25 +1015,35 @@ test "shape emoji width long" {
     var testdata = try testShaper(alloc);
     defer testdata.deinit();
 
-    var buf: [32]u8 = undefined;
-    var buf_idx: usize = 0;
-    buf_idx += try std.unicode.utf8Encode(0x1F9D4, buf[buf_idx..]); // man: beard
-    buf_idx += try std.unicode.utf8Encode(0x1F3FB, buf[buf_idx..]); // light skin tone (Fitz 1-2)
-    buf_idx += try std.unicode.utf8Encode(0x200D, buf[buf_idx..]); // ZWJ
-    buf_idx += try std.unicode.utf8Encode(0x2642, buf[buf_idx..]); // male sign
-    buf_idx += try std.unicode.utf8Encode(0xFE0F, buf[buf_idx..]); // emoji representation
-
-    // Make a screen with some data
+    // Make a screen and add a long emoji sequence to it.
     var screen = try terminal.Screen.init(alloc, 30, 3, 0);
     defer screen.deinit();
-    try screen.testWriteString(buf[0..buf_idx]);
+
+    var page = screen.pages.pages.first.?.data;
+    var row = page.getRow(1);
+    const cell = &row.cells.ptr(page.memory)[0];
+    cell.* = .{
+        .content_tag = .codepoint,
+        .content = .{ .codepoint = 0x1F9D4 }, // Person with beard
+    };
+    var graphemes = [_]u21{
+        0x1F3FB, // Light skin tone (Fitz 1-2)
+        0x200D, // ZWJ
+        0x2642, // Male sign
+        0xFE0F, // Emoji presentation selector
+    };
+    try page.setGraphemes(
+        row,
+        cell,
+        graphemes[0..],
+    );
 
     // Get our run iterator
     var shaper = &testdata.shaper;
     var it = shaper.runIterator(
         testdata.grid,
         &screen,
-        screen.pages.pin(.{ .screen = .{ .y = 0 } }).?,
+        screen.pages.pin(.{ .screen = .{ .y = 1 } }).?,
         null,
         null,
     );

src/font/shaper/harfbuzz.zig

@@ -540,25 +540,35 @@ test "shape emoji width long" {
     var testdata = try testShaper(alloc);
     defer testdata.deinit();
 
-    var buf: [32]u8 = undefined;
-    var buf_idx: usize = 0;
-    buf_idx += try std.unicode.utf8Encode(0x1F9D4, buf[buf_idx..]); // man: beard
-    buf_idx += try std.unicode.utf8Encode(0x1F3FB, buf[buf_idx..]); // light skin tone (Fitz 1-2)
-    buf_idx += try std.unicode.utf8Encode(0x200D, buf[buf_idx..]); // ZWJ
-    buf_idx += try std.unicode.utf8Encode(0x2642, buf[buf_idx..]); // male sign
-    buf_idx += try std.unicode.utf8Encode(0xFE0F, buf[buf_idx..]); // emoji representation
-
-    // Make a screen with some data
+    // Make a screen and add a long emoji sequence to it.
     var screen = try terminal.Screen.init(alloc, 30, 3, 0);
     defer screen.deinit();
-    try screen.testWriteString(buf[0..buf_idx]);
+
+    var page = screen.pages.pages.first.?.data;
+    var row = page.getRow(1);
+    const cell = &row.cells.ptr(page.memory)[0];
+    cell.* = .{
+        .content_tag = .codepoint,
+        .content = .{ .codepoint = 0x1F9D4 }, // Person with beard
+    };
+    var graphemes = [_]u21{
+        0x1F3FB, // Light skin tone (Fitz 1-2)
+        0x200D, // ZWJ
+        0x2642, // Male sign
+        0xFE0F, // Emoji presentation selector
+    };
+    try page.setGraphemes(
+        row,
+        cell,
+        graphemes[0..],
+    );
 
     // Get our run iterator
     var shaper = &testdata.shaper;
     var it = shaper.runIterator(
         testdata.grid,
         &screen,
-        screen.pages.pin(.{ .screen = .{ .y = 0 } }).?,
+        screen.pages.pin(.{ .screen = .{ .y = 1 } }).?,
         null,
         null,
     );

src/font/shaper/run.zig

@@ -360,11 +360,16 @@ pub const RunIterator = struct {
 
             // Find a font that supports this codepoint. If none support this
             // then the whole grapheme can't be rendered so we return null.
+            //
+            // We explicitly do not require the additional grapheme components
+            // to support the base presentation, since it is common for emoji
+            // fonts to support the base emoji with emoji presentation but not
+            // certain ZWJ-combined characters like the male and female signs.
             const idx = try self.grid.getIndex(
                 alloc,
                 cp,
                 style,
-                presentation,
+                null,
             ) orelse return null;
             candidates.appendAssumeCapacity(idx);
         }
@@ -375,7 +380,7 @@ pub const RunIterator = struct {
             for (cps) |cp| {
                 // Ignore Emoji ZWJs
                 if (cp == 0xFE0E or cp == 0xFE0F or cp == 0x200D) continue;
-                if (!self.grid.hasCodepoint(idx, cp, presentation)) break;
+                if (!self.grid.hasCodepoint(idx, cp, null)) break;
             } else {
                 // If the while completed, then we have a candidate that
                 // supports all of our codepoints.