Merge pull request #444 from mitchellh/font-fallback

font fallback search must verify presentation, do not preload emoji

src/Surface.zig

@@ -239,7 +239,7 @@ pub fn init(
                 defer disco_it.deinit();
                 if (try disco_it.next()) |face| {
                     log.info("font regular: {s}", .{try face.name(&name_buf)});
-                    try group.addFace(.regular, .{ .deferred = face });
+                    _ = try group.addFace(.regular, .{ .deferred = face });
                 } else log.warn("font-family not found: {s}", .{family});
             }
             if (config.@"font-family-bold") |family| {
@@ -252,7 +252,7 @@ pub fn init(
                 defer disco_it.deinit();
                 if (try disco_it.next()) |face| {
                     log.info("font bold: {s}", .{try face.name(&name_buf)});
-                    try group.addFace(.bold, .{ .deferred = face });
+                    _ = try group.addFace(.bold, .{ .deferred = face });
                 } else log.warn("font-family-bold not found: {s}", .{family});
             }
             if (config.@"font-family-italic") |family| {
@@ -265,7 +265,7 @@ pub fn init(
                 defer disco_it.deinit();
                 if (try disco_it.next()) |face| {
                     log.info("font italic: {s}", .{try face.name(&name_buf)});
-                    try group.addFace(.italic, .{ .deferred = face });
+                    _ = try group.addFace(.italic, .{ .deferred = face });
                 } else log.warn("font-family-italic not found: {s}", .{family});
             }
             if (config.@"font-family-bold-italic") |family| {
@@ -279,17 +279,17 @@ pub fn init(
                 defer disco_it.deinit();
                 if (try disco_it.next()) |face| {
                     log.info("font bold+italic: {s}", .{try face.name(&name_buf)});
-                    try group.addFace(.bold_italic, .{ .deferred = face });
+                    _ = try group.addFace(.bold_italic, .{ .deferred = face });
                 } else log.warn("font-family-bold-italic not found: {s}", .{family});
             }
         }
 
         // Our built-in font will be used as a backup
-        try group.addFace(
+        _ = try group.addFace(
             .regular,
             .{ .loaded = try font.Face.init(font_lib, face_ttf, font_size) },
         );
-        try group.addFace(
+        _ = try group.addFace(
             .bold,
             .{ .loaded = try font.Face.init(font_lib, face_bold_ttf, font_size) },
         );
@@ -297,35 +297,6 @@ pub fn init(
         // Auto-italicize if we have to.
         try group.italicize();
 
-        // Emoji fallback. We don't include this on Mac since Mac is expected
-        // to always have the Apple Emoji available.
-        if (builtin.os.tag != .macos or font.Discover == void) {
-            try group.addFace(
-                .regular,
-                .{ .loaded = try font.Face.init(font_lib, face_emoji_ttf, font_size) },
-            );
-            try group.addFace(
-                .regular,
-                .{ .loaded = try font.Face.init(font_lib, face_emoji_text_ttf, font_size) },
-            );
-        }
-
-        // If we're on Mac, then we try to use the Apple Emoji font for Emoji.
-        if (builtin.os.tag == .macos and font.Discover != void) {
-            if (try app.fontDiscover()) |disco| {
-                var disco_it = try disco.discover(.{
-                    .family = "Apple Color Emoji",
-                    .size = font_size.points,
-                });
-                defer disco_it.deinit();
-                if (try disco_it.next()) |face| {
-                    var name_buf: [256]u8 = undefined;
-                    log.info("font emoji: {s}", .{try face.name(&name_buf)});
-                    try group.addFace(.regular, .{ .deferred = face });
-                }
-            }
-        }
-
         break :group group;
     });
     errdefer font_group.deinit(alloc);

src/font/DeferredFace.zig

@@ -254,6 +254,12 @@ pub fn hasCodepoint(self: DeferredFace, cp: u32, p: ?Presentation) bool {
         .coretext, .coretext_freetype => {
             // If we are using coretext, we check the loaded CT font.
             if (self.ct) |ct| {
+                if (p) |desired_p| {
+                    const traits = ct.font.getSymbolicTraits();
+                    const actual_p: Presentation = if (traits.color_glyphs) .emoji else .text;
+                    if (actual_p != desired_p) return false;
+                }
+
                 // Turn UTF-32 into UTF-16 for CT API
                 var unichars: [2]u16 = undefined;
                 const pair = macos.foundation.stringGetSurrogatePairForLongCharacter(cp, &unichars);

src/font/Group.zig

@@ -99,13 +99,15 @@ pub fn deinit(self: *Group) void {
 ///
 /// The group takes ownership of the face. The face will be deallocated when
 /// the group is deallocated.
-pub fn addFace(self: *Group, style: Style, face: GroupFace) !void {
+pub fn addFace(self: *Group, style: Style, face: GroupFace) !FontIndex {
     const list = self.faces.getPtr(style);
 
     // We have some special indexes so we must never pass those.
     if (list.items.len >= FontIndex.Special.start - 1) return error.GroupFull;
 
+    const idx = list.items.len;
     try list.append(self.alloc, face);
+    return .{ .style = style, .idx = @intCast(idx) };
 }
 
 /// Returns true if we have a face for the given style, though the face may
@@ -259,13 +261,17 @@ pub fn indexForCodepoint(
             defer disco_it.deinit();
 
             if (disco_it.next() catch break :discover) |face| {
-                var buf: [256]u8 = undefined;
+                // Discovery is supposed to only return faces that have our
-                log.info("found codepoint 0x{x} in fallback face={s}", .{
+                // codepoint but we can't search presentation in discovery so
-                    cp,
+                // we have to check it here.
-                    face.name(&buf) catch "<error>",
+                if (face.hasCodepoint(cp, p)) {
-                });
+                    var buf: [256]u8 = undefined;
-                self.addFace(style, .{ .deferred = face }) catch break :discover;
+                    log.info("found codepoint 0x{x} in fallback face={s}", .{
-                if (self.indexForCodepointExact(cp, style, p)) |value| return value;
+                        cp,
+                        face.name(&buf) catch "<error>",
+                    });
+                    return self.addFace(style, .{ .deferred = face }) catch break :discover;
+                }
             }
         }
     }
@@ -273,7 +279,7 @@ pub fn indexForCodepoint(
     // If this is already regular, we're done falling back.
     if (style == .regular and p == null) return null;
 
-    // For non-regular fonts, we fall back to regular.
+    // For non-regular fonts, we fall back to regular with no presentation
     return self.indexForCodepointExact(cp, .regular, null);
 }
 
@@ -501,13 +507,13 @@ test {
     var group = try init(alloc, lib, .{ .points = 12 });
     defer group.deinit();
 
-    try group.addFace(.regular, .{ .loaded = try Face.init(lib, testFont, .{ .points = 12 }) });
+    _ = try group.addFace(.regular, .{ .loaded = try Face.init(lib, testFont, .{ .points = 12 }) });
 
     if (font.options.backend != .coretext) {
         // Coretext doesn't support Noto's format
-        try group.addFace(.regular, .{ .loaded = try Face.init(lib, testEmoji, .{ .points = 12 }) });
+        _ = try group.addFace(.regular, .{ .loaded = try Face.init(lib, testEmoji, .{ .points = 12 }) });
     }
-    try group.addFace(.regular, .{ .loaded = try Face.init(lib, testEmojiText, .{ .points = 12 }) });
+    _ = try group.addFace(.regular, .{ .loaded = try Face.init(lib, testEmojiText, .{ .points = 12 }) });
 
     // Should find all visible ASCII
     var i: u32 = 32;
@@ -569,7 +575,7 @@ test "face count limit" {
     defer group.deinit();
 
     for (0..FontIndex.Special.start - 1) |_| {
-        try group.addFace(.regular, .{ .loaded = try Face.init(lib, testFont, .{ .points = 12 }) });
+        _ = try group.addFace(.regular, .{ .loaded = try Face.init(lib, testFont, .{ .points = 12 }) });
     }
 
     try testing.expectError(error.GroupFull, group.addFace(
@@ -624,7 +630,7 @@ test "resize" {
     var group = try init(alloc, lib, .{ .points = 12, .xdpi = 96, .ydpi = 96 });
     defer group.deinit();
 
-    try group.addFace(.regular, .{ .loaded = try Face.init(lib, testFont, .{ .points = 12, .xdpi = 96, .ydpi = 96 }) });
+    _ = try group.addFace(.regular, .{ .loaded = try Face.init(lib, testFont, .{ .points = 12, .xdpi = 96, .ydpi = 96 }) });
 
     // Load a letter
     {
@@ -677,7 +683,7 @@ test "discover monospace with fontconfig and freetype" {
     defer lib.deinit();
     var group = try init(alloc, lib, .{ .points = 12 });
     defer group.deinit();
-    try group.addFace(.regular, .{ .deferred = (try it.next()).? });
+    _ = try group.addFace(.regular, .{ .deferred = (try it.next()).? });
 
     // Should find all visible ASCII
     var atlas_greyscale = try font.Atlas.init(alloc, 512, .greyscale);
@@ -715,7 +721,7 @@ test "faceFromIndex returns pointer" {
     var group = try init(alloc, lib, .{ .points = 12, .xdpi = 96, .ydpi = 96 });
     defer group.deinit();
 
-    try group.addFace(.regular, .{ .loaded = try Face.init(lib, testFont, .{ .points = 12, .xdpi = 96, .ydpi = 96 }) });
+    _ = try group.addFace(.regular, .{ .loaded = try Face.init(lib, testFont, .{ .points = 12, .xdpi = 96, .ydpi = 96 }) });
 
     {
         const idx = group.indexForCodepoint('A', .regular, null).?;

src/font/GroupCache.zig

@@ -182,7 +182,7 @@ test {
     defer cache.deinit(alloc);
 
     // Setup group
-    try cache.group.addFace(
+    _ = try cache.group.addFace(
         .regular,
         .{ .loaded = try Face.init(lib, testFont, .{ .points = 12 }) },
     );
@@ -338,7 +338,7 @@ test "resize" {
     defer cache.deinit(alloc);
 
     // Setup group
-    try cache.group.addFace(
+    _ = try cache.group.addFace(
         .regular,
         .{ .loaded = try Face.init(lib, testFont, .{ .points = 12, .xdpi = 96, .ydpi = 96 }) },
     );

src/font/shaper/harfbuzz.zig

@@ -896,11 +896,11 @@ fn testShaper(alloc: Allocator) !TestShaper {
     errdefer cache_ptr.*.deinit(alloc);
 
     // Setup group
-    try cache_ptr.group.addFace(.regular, .{ .loaded = try Face.init(lib, testFont, .{ .points = 12 }) });
+    _ = try cache_ptr.group.addFace(.regular, .{ .loaded = try Face.init(lib, testFont, .{ .points = 12 }) });
 
     if (font.options.backend != .coretext) {
         // Coretext doesn't support Noto's format
-        try cache_ptr.group.addFace(.regular, .{ .loaded = try Face.init(lib, testEmoji, .{ .points = 12 }) });
+        _ = try cache_ptr.group.addFace(.regular, .{ .loaded = try Face.init(lib, testEmoji, .{ .points = 12 }) });
     } else {
         // On CoreText we want to load Apple Emoji, we should have it.
         var disco = font.Discover.init();
@@ -912,9 +912,9 @@ fn testShaper(alloc: Allocator) !TestShaper {
         defer disco_it.deinit();
         var face = (try disco_it.next()).?;
         errdefer face.deinit();
-        try cache_ptr.group.addFace(.regular, .{ .deferred = face });
+        _ = try cache_ptr.group.addFace(.regular, .{ .deferred = face });
     }
-    try cache_ptr.group.addFace(.regular, .{ .loaded = try Face.init(lib, testEmojiText, .{ .points = 12 }) });
+    _ = try cache_ptr.group.addFace(.regular, .{ .loaded = try Face.init(lib, testEmojiText, .{ .points = 12 }) });
 
     var cell_buf = try alloc.alloc(font.shape.Cell, 80);
     errdefer alloc.free(cell_buf);