Merge pull request #610 from mitchellh/coretext-scoring

coretext: score fonts to prefer bold over other bold styles

pkg/macos/build.zig

@@ -28,7 +28,7 @@ pub fn build(b: *std.Build) !void {
     lib.linkFramework("CoreFoundation");
     lib.linkFramework("CoreGraphics");
     lib.linkFramework("CoreText");
-    try apple_sdk.addPaths(b, lib);
+    if (!target.isNative()) try apple_sdk.addPaths(b, lib);
 
     b.installArtifact(lib);
 

pkg/macos/foundation/array.zig

@@ -1,6 +1,9 @@
 const std = @import("std");
 const Allocator = std.mem.Allocator;
+const base = @import("base.zig");
 const cftype = @import("type.zig");
+const ComparisonResult = base.ComparisonResult;
+const Range = base.Range;
 
 pub const Array = opaque {
     pub fn create(comptime T: type, values: []*const T) Allocator.Error!*Array {
@@ -38,6 +41,52 @@ pub const Array = opaque {
     extern "c" var kCFTypeArrayCallBacks: anyopaque;
 };
 
+pub const MutableArray = opaque {
+    pub fn createCopy(array: *Array) Allocator.Error!*MutableArray {
+        return CFArrayCreateMutableCopy(
+            null,
+            0,
+            array,
+        ) orelse error.OutOfMemory;
+    }
+
+    pub fn release(self: *MutableArray) void {
+        cftype.CFRelease(self);
+    }
+
+    pub fn sortValues(
+        self: *MutableArray,
+        comptime Elem: type,
+        comptime Context: type,
+        context: ?*Context,
+        comptime comparator: ?*const fn (
+            a: *const Elem,
+            b: *const Elem,
+            context: ?*Context,
+        ) callconv(.C) ComparisonResult,
+    ) void {
+        CFArraySortValues(
+            self,
+            Range.init(0, Array.CFArrayGetCount(@ptrCast(self))),
+            comparator,
+            context,
+        );
+    }
+
+    extern "c" fn CFArrayCreateMutableCopy(
+        allocator: ?*anyopaque,
+        capacity: usize,
+        array: *Array,
+    ) ?*MutableArray;
+
+    extern "c" fn CFArraySortValues(
+        array: *MutableArray,
+        range: Range,
+        comparator: ?*const anyopaque,
+        context: ?*anyopaque,
+    ) void;
+};
+
 test "array" {
     const testing = std.testing;
 
@@ -52,4 +101,43 @@ test "array" {
         const ch = arr.getValueAtIndex(u8, 0);
         try testing.expectEqual(@as(u8, 'h'), ch.*);
     }
+
+    // Can make it mutable
+    var mut = try MutableArray.createCopy(arr);
+    defer mut.release();
+}
+
+test "array sorting" {
+    const testing = std.testing;
+
+    const str = "hello";
+    var values = [_]*const u8{ &str[0], &str[1] };
+    const arr = try Array.create(u8, &values);
+    defer arr.release();
+    const mut = try MutableArray.createCopy(arr);
+    defer mut.release();
+
+    mut.sortValues(
+        u8,
+        void,
+        null,
+        struct {
+            fn compare(a: *const u8, b: *const u8, _: ?*void) callconv(.C) ComparisonResult {
+                if (a.* > b.*) return .greater;
+                if (a.* == b.*) return .equal;
+                return .less;
+            }
+        }.compare,
+    );
+
+    {
+        const mutarr: *Array = @ptrCast(mut);
+        const ch = mutarr.getValueAtIndex(u8, 0);
+        try testing.expectEqual(@as(u8, 'e'), ch.*);
+    }
+    {
+        const mutarr: *Array = @ptrCast(mut);
+        const ch = mutarr.getValueAtIndex(u8, 1);
+        try testing.expectEqual(@as(u8, 'h'), ch.*);
+    }
 }

pkg/macos/text/font_descriptor.zig

@@ -46,18 +46,22 @@ pub const FontDescriptor = opaque {
         ) orelse Allocator.Error.OutOfMemory;
     }
 
+    pub fn retain(self: *FontDescriptor) void {
+        _ = c.CFRetain(self);
+    }
+
     pub fn release(self: *FontDescriptor) void {
         c.CFRelease(self);
     }
 
-    pub fn copyAttribute(self: *FontDescriptor, comptime attr: FontAttribute) attr.Value() {
+    pub fn copyAttribute(self: *const FontDescriptor, comptime attr: FontAttribute) attr.Value() {
         return @ptrFromInt(@intFromPtr(c.CTFontDescriptorCopyAttribute(
             @ptrCast(self),
             @ptrCast(attr.key()),
         )));
     }
 
-    pub fn copyAttributes(self: *FontDescriptor) *foundation.Dictionary {
+    pub fn copyAttributes(self: *const FontDescriptor) *foundation.Dictionary {
         return @ptrFromInt(@intFromPtr(c.CTFontDescriptorCopyAttributes(
             @ptrCast(self),
         )));

src/Surface.zig

@@ -246,7 +246,7 @@ pub fn init(
             var name_buf: [256]u8 = undefined;
 
             if (config.@"font-family") |family| {
-                var disco_it = try disco.discover(.{
+                var disco_it = try disco.discover(alloc, .{
                     .family = family,
                     .style = config.@"font-style".nameValue(),
                     .size = font_size.points,
@@ -259,7 +259,7 @@ pub fn init(
                 } else log.warn("font-family not found: {s}", .{family});
             }
             if (config.@"font-family-bold") |family| {
-                var disco_it = try disco.discover(.{
+                var disco_it = try disco.discover(alloc, .{
                     .family = family,
                     .style = config.@"font-style-bold".nameValue(),
                     .size = font_size.points,
@@ -273,7 +273,7 @@ pub fn init(
                 } else log.warn("font-family-bold not found: {s}", .{family});
             }
             if (config.@"font-family-italic") |family| {
-                var disco_it = try disco.discover(.{
+                var disco_it = try disco.discover(alloc, .{
                     .family = family,
                     .style = config.@"font-style-italic".nameValue(),
                     .size = font_size.points,
@@ -287,7 +287,7 @@ pub fn init(
                 } else log.warn("font-family-italic not found: {s}", .{family});
             }
             if (config.@"font-family-bold-italic") |family| {
-                var disco_it = try disco.discover(.{
+                var disco_it = try disco.discover(alloc, .{
                     .family = family,
                     .style = config.@"font-style-bold-italic".nameValue(),
                     .size = font_size.points,

src/cli/list_fonts.zig

@@ -85,7 +85,7 @@ fn runArgs(alloc_gpa: Allocator, argsIter: anytype) !u8 {
     // Look up all available fonts
     var disco = font.Discover.init();
     defer disco.deinit();
-    var disco_it = try disco.discover(.{
+    var disco_it = try disco.discover(alloc, .{
         .family = config.family,
         .style = config.style,
         .bold = config.bold,

src/font/DeferredFace.zig

@@ -379,6 +379,7 @@ test "fontconfig" {
 
     const discovery = @import("main.zig").discovery;
     const testing = std.testing;
+    const alloc = testing.allocator;
 
     // Load freetype
     var lib = try Library.init();
@@ -387,7 +388,7 @@ test "fontconfig" {
     // Get a deferred face from fontconfig
     var def = def: {
         var fc = discovery.Fontconfig.init();
-        var it = try fc.discover(.{ .family = "monospace", .size = 12 });
+        var it = try fc.discover(alloc, .{ .family = "monospace", .size = 12 });
         defer it.deinit();
         break :def (try it.next()).?;
     };
@@ -408,6 +409,7 @@ test "coretext" {
 
     const discovery = @import("main.zig").discovery;
     const testing = std.testing;
+    const alloc = testing.allocator;
 
     // Load freetype
     var lib = try Library.init();
@@ -416,7 +418,7 @@ test "coretext" {
     // Get a deferred face from fontconfig
     var def = def: {
         var fc = discovery.CoreText.init();
-        var it = try fc.discover(.{ .family = "Monaco", .size = 12 });
+        var it = try fc.discover(alloc, .{ .family = "Monaco", .size = 12 });
         defer it.deinit();
         break :def (try it.next()).?;
     };

src/font/Group.zig

@@ -313,7 +313,7 @@ pub fn indexForCodepoint(
     // If we are regular, try looking for a fallback using discovery.
     if (style == .regular and font.Discover != void) {
         if (self.discover) |disco| discover: {
-            var disco_it = disco.discover(.{
+            var disco_it = disco.discover(self.alloc, .{
                 .codepoint = cp,
                 .size = self.size.points,
                 .bold = style == .bold or style == .bold_italic,
@@ -382,7 +382,7 @@ fn indexForCodepointOverride(self: *Group, cp: u32) !?FontIndex {
     const idx_: ?FontIndex = self.descriptor_cache.get(desc) orelse idx: {
         // Slow path: we have to find this descriptor and load the font
         const discover = self.discover orelse return null;
-        var disco_it = try discover.discover(desc);
+        var disco_it = try discover.discover(self.alloc, desc);
         defer disco_it.deinit();
 
         const face = (try disco_it.next()) orelse {
@@ -835,7 +835,7 @@ test "discover monospace with fontconfig and freetype" {
 
     // Search for fonts
     var fc = Discover.init();
-    var it = try fc.discover(.{ .family = "monospace", .size = 12 });
+    var it = try fc.discover(alloc, .{ .family = "monospace", .size = 12 });
     defer it.deinit();
 
     // Initialize the group with the deferred face

src/font/discovery.zig

@@ -1,5 +1,6 @@
 const std = @import("std");
 const builtin = @import("builtin");
+const Allocator = std.mem.Allocator;
 const assert = std.debug.assert;
 const fontconfig = @import("fontconfig");
 const macos = @import("macos");
@@ -233,7 +234,9 @@ pub const Fontconfig = struct {
 
     /// Discover fonts from a descriptor. This returns an iterator that can
     /// be used to build up the deferred fonts.
-    pub fn discover(self: *const Fontconfig, desc: Descriptor) !DiscoverIterator {
+    pub fn discover(self: *const Fontconfig, alloc: Allocator, desc: Descriptor) !DiscoverIterator {
+        _ = alloc;
+
         // Build our pattern that we'll search for
         const pat = desc.toFcPattern();
         errdefer pat.destroy();
@@ -307,7 +310,7 @@ pub const CoreText = struct {
 
     /// Discover fonts from a descriptor. This returns an iterator that can
     /// be used to build up the deferred fonts.
-    pub fn discover(self: *const CoreText, desc: Descriptor) !DiscoverIterator {
+    pub fn discover(self: *const CoreText, alloc: Allocator, desc: Descriptor) !DiscoverIterator {
         _ = self;
 
         // Build our pattern that we'll search for
@@ -323,25 +326,104 @@ pub const CoreText = struct {
         const set = try macos.text.FontCollection.createWithFontDescriptors(desc_arr);
         defer set.release();
         const list = set.createMatchingFontDescriptors();
-        errdefer list.release();
+        defer list.release();
+
+        // Sort our descriptors
+        const zig_list = try copyMatchingDescriptors(alloc, list);
+        errdefer alloc.free(zig_list);
+        sortMatchingDescriptors(&desc, zig_list);
 
         return DiscoverIterator{
-            .list = list,
+            .alloc = alloc,
+            .list = zig_list,
             .i = 0,
         };
     }
 
-    pub const DiscoverIterator = struct {
+    fn copyMatchingDescriptors(
+        alloc: Allocator,
         list: *macos.foundation.Array,
+    ) ![]*macos.text.FontDescriptor {
+        var result = try alloc.alloc(*macos.text.FontDescriptor, list.getCount());
+        errdefer alloc.free(result);
+        for (0..result.len) |i| {
+            result[i] = list.getValueAtIndex(macos.text.FontDescriptor, i);
+
+            // We need to retain becauseonce the list is freed it will
+            // release all its members.
+            result[i].retain();
+        }
+        return result;
+    }
+
+    fn sortMatchingDescriptors(
+        desc: *const Descriptor,
+        list: []*macos.text.FontDescriptor,
+    ) void {
+        var desc_mut = desc.*;
+        if (desc_mut.style == null) {
+            // If there is no explicit style set, we set a preferred
+            // based on the style bool attributes.
+            //
+            // TODO: doesn't handle i18n font names well, we should have
+            // another mechanism that uses the weight attribute if it exists.
+            // Wait for this to be a real problem.
+            desc_mut.style = if (desc_mut.bold and desc_mut.italic)
+                "Bold Italic"
+            else if (desc_mut.bold)
+                "Bold"
+            else if (desc_mut.italic)
+                "Italic"
+            else
+                null;
+        }
+
+        std.mem.sortUnstable(*macos.text.FontDescriptor, list, &desc_mut, struct {
+            fn lessThan(
+                desc_inner: *const Descriptor,
+                lhs: *macos.text.FontDescriptor,
+                rhs: *macos.text.FontDescriptor,
+            ) bool {
+                const lhs_score = score(desc_inner, lhs);
+                const rhs_score = score(desc_inner, rhs);
+                // Higher score is "less" (earlier)
+                return lhs_score > rhs_score;
+            }
+        }.lessThan);
+    }
+
+    fn score(desc: *const Descriptor, ct_desc: *const macos.text.FontDescriptor) i32 {
+        var score_acc: i32 = 0;
+
+        score_acc += if (desc.style) |desired_style| style: {
+            const style = ct_desc.copyAttribute(.style_name);
+            defer style.release();
+            var buf: [128]u8 = undefined;
+            const style_str = style.cstring(&buf, .utf8) orelse break :style 0;
+
+            // Matching style string gets highest score
+            if (std.mem.eql(u8, desired_style, style_str)) break :style 100;
+
+            // Otherwise the score is based on the length of the style string.
+            // Shorter styles are scored higher.
+            break :style -1 * @as(i32, @intCast(style_str.len));
+        } else 0;
+
+        return score_acc;
+    }
+
+    pub const DiscoverIterator = struct {
+        alloc: Allocator,
+        list: []const *macos.text.FontDescriptor,
         i: usize,
 
         pub fn deinit(self: *DiscoverIterator) void {
-            self.list.release();
+            self.alloc.free(self.list);
             self.* = undefined;
         }
 
         pub fn next(self: *DiscoverIterator) !?DeferredFace {
-            if (self.i >= self.list.getCount()) return null;
+            if (self.i >= self.list.len) return null;
 
             // Get our descriptor. We need to remove the character set
             // limitation because we may have used that to filter but we
@@ -349,7 +431,7 @@ pub const CoreText = struct {
             // available.
             //const desc = self.list.getValueAtIndex(macos.text.FontDescriptor, self.i);
             const desc = desc: {
-                const original = self.list.getValueAtIndex(macos.text.FontDescriptor, self.i);
+                const original = self.list[self.i];
 
                 // For some reason simply copying the attributes and recreating
                 // the descriptor removes the charset restriction. This is tested.
@@ -392,8 +474,11 @@ test "descriptor hash familiy names" {
 test "fontconfig" {
     if (options.backend != .fontconfig_freetype) return error.SkipZigTest;
 
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
     var fc = Fontconfig.init();
-    var it = try fc.discover(.{ .family = "monospace", .size = 12 });
+    var it = try fc.discover(alloc, .{ .family = "monospace", .size = 12 });
     defer it.deinit();
 }
 
@@ -401,9 +486,10 @@ test "fontconfig codepoint" {
     if (options.backend != .fontconfig_freetype) return error.SkipZigTest;
 
     const testing = std.testing;
+    const alloc = testing.allocator;
 
     var fc = Fontconfig.init();
-    var it = try fc.discover(.{ .codepoint = 'A', .size = 12 });
+    var it = try fc.discover(alloc, .{ .codepoint = 'A', .size = 12 });
     defer it.deinit();
 
     // The first result should have the codepoint. Later ones may not
@@ -420,10 +506,11 @@ test "coretext" {
         return error.SkipZigTest;
 
     const testing = std.testing;
+    const alloc = testing.allocator;
 
     var ct = CoreText.init();
     defer ct.deinit();
-    var it = try ct.discover(.{ .family = "Monaco", .size = 12 });
+    var it = try ct.discover(alloc, .{ .family = "Monaco", .size = 12 });
     defer it.deinit();
     var count: usize = 0;
     while (try it.next()) |_| {
@@ -437,10 +524,11 @@ test "coretext codepoint" {
         return error.SkipZigTest;
 
     const testing = std.testing;
+    const alloc = testing.allocator;
 
     var ct = CoreText.init();
     defer ct.deinit();
-    var it = try ct.discover(.{ .codepoint = 'A', .size = 12 });
+    var it = try ct.discover(alloc, .{ .codepoint = 'A', .size = 12 });
     defer it.deinit();
 
     // The first result should have the codepoint. Later ones may not

src/font/shaper/harfbuzz.zig

@@ -908,7 +908,7 @@ fn testShaper(alloc: Allocator) !TestShaper {
         // On CoreText we want to load Apple Emoji, we should have it.
         var disco = font.Discover.init();
         defer disco.deinit();
-        var disco_it = try disco.discover(.{
+        var disco_it = try disco.discover(alloc, .{
             .family = "Apple Color Emoji",
             .size = 12,
             .monospace = false,