From 964f2ce96a01b5d05719c08fb48101d7c61ccdf6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 6 Nov 2024 12:53:34 -0800 Subject: [PATCH 1/2] font/coretext: always score based on style string length This fixes an issue where for the regular style we were picking a suboptimal style because for some font faces we were choosing more bold faces (just as chance). This modifies our scoring to take the style length into account even for regular style. We already had this logic for explicit styles. --- src/font/discovery.zig | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/font/discovery.zig b/src/font/discovery.zig index aeaeb955e..67236d5c9 100644 --- a/src/font/discovery.zig +++ b/src/font/discovery.zig @@ -685,30 +685,29 @@ pub const CoreText = struct { break :style .unmatched; defer style.release(); + // Get our style string + var buf: [128]u8 = undefined; + const style_str = style.cstring(&buf, .utf8) orelse break :style .unmatched; + // If we have a specific desired style, attempt to search for that. if (desc.style) |desired_style| { - var buf: [128]u8 = undefined; - const style_str = style.cstring(&buf, .utf8) orelse break :style .unmatched; - // Matching style string gets highest score if (std.mem.eql(u8, desired_style, style_str)) break :style .match; - - // Otherwise the score is based on the length of the style string. - // Shorter styles are scored higher. - break :style @enumFromInt(100 -| style_str.len); + } else if (!desc.bold and !desc.italic) { + // If we do not, and we have no symbolic traits, then we try + // to find "regular" (or no style). If we have symbolic traits + // we do nothing but we can improve scoring by taking that into + // account, too. + if (std.mem.eql(u8, "Regular", style_str)) { + break :style .match; + } } - // If we do not, and we have no symbolic traits, then we try - // to find "regular" (or no style). If we have symbolic traits - // we do nothing but we can improve scoring by taking that into - // account, too. - if (!desc.bold and !desc.italic) { - var buf: [128]u8 = undefined; - const style_str = style.cstring(&buf, .utf8) orelse break :style .unmatched; - if (std.mem.eql(u8, "Regular", style_str)) break :style .match; - } - - break :style .unmatched; + // Otherwise the score is based on the length of the style string. + // Shorter styles are scored higher. This is a heuristic that + // if we don't have a desired style then shorter tends to be + // more often the "regular" style. + break :style @enumFromInt(100 -| style_str.len); }; score_acc.traits = traits: { From 94542b04f2f9e958f922be7e4400e1742e617a2d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 6 Nov 2024 12:56:12 -0800 Subject: [PATCH 2/2] font/coretext: do not set variation axes in discovery This was causing discovery to find some odd fonts under certain scenarios (namely: Recursive Mono). Due to our prior fix in e08eeb2b2ad810c4db22530a181858caee834b22 we no longer need to set variations here for them to stick. --- src/font/discovery.zig | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/font/discovery.zig b/src/font/discovery.zig index 67236d5c9..e73ea626f 100644 --- a/src/font/discovery.zig +++ b/src/font/discovery.zig @@ -235,21 +235,7 @@ pub const Descriptor = struct { ); } - // Build our descriptor from attrs - var desc = try macos.text.FontDescriptor.createWithAttributes(@ptrCast(attrs)); - errdefer desc.release(); - - // Variations are built by copying the descriptor. I don't know a way - // to set it on attrs directly. - for (self.variations) |v| { - const id = try macos.foundation.Number.create(.int, @ptrCast(&v.id)); - defer id.release(); - const next = try desc.createCopyWithVariation(id, v.value); - desc.release(); - desc = next; - } - - return desc; + return try macos.text.FontDescriptor.createWithAttributes(@ptrCast(attrs)); } };