From e08eeb2b2ad810c4db22530a181858caee834b22 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 5 Nov 2024 16:13:53 -0800 Subject: [PATCH] coretext: set variations on deferred face load This commit makes CoreText behave a lot like FreeType where we set the variation axes on the deferred face load. This fixes a bug where the `slnt` variation axis could not be set with CoreText with the Monaspace Argon Variable font. This was a bug found in Discord. Specifically, with the Monaspace Argon Variable font, the `slnt` variation axis could not be set with CoreText. I'm not sure _exactly_ what causes this but I suspect it has to do with the `slnt` axis being a negative value. I'm not sure if this is a bug with CoreText or not. What was happening was that with CoreText, we set the variation axes during discovery and expect them to be preserved in the resulting discovered faces. That seems to be true with the `wght` axis but not the `slnt` axis for whatever reason. --- src/font/DeferredFace.zig | 48 ++++++++------------------------------ src/font/discovery.zig | 11 +++++++-- src/font/face/coretext.zig | 3 +++ 3 files changed, 22 insertions(+), 40 deletions(-) diff --git a/src/font/DeferredFace.zig b/src/font/DeferredFace.zig index db9e23623..3ee104386 100644 --- a/src/font/DeferredFace.zig +++ b/src/font/DeferredFace.zig @@ -57,6 +57,11 @@ pub const CoreText = struct { /// The initialized font font: *macos.text.Font, + /// Variations to apply to this font. We apply the variations to the + /// search descriptor but sometimes when the font collection is + /// made the variation axes are reset so we have to reapply them. + variations: []const font.face.Variation, + pub fn deinit(self: *CoreText) void { self.font.release(); self.* = undefined; @@ -194,7 +199,10 @@ fn loadCoreText( ) !Face { _ = lib; const ct = self.ct.?; - return try Face.initFontCopy(ct.font, opts); + var face = try Face.initFontCopy(ct.font, opts); + errdefer face.deinit(); + try face.setVariations(ct.variations, opts); + return face; } fn loadCoreTextFreetype( @@ -236,43 +244,7 @@ fn loadCoreTextFreetype( //std.log.warn("path={s}", .{path_slice}); var face = try Face.initFile(lib, buf[0..path_slice.len :0], 0, opts); errdefer face.deinit(); - - // If our ct font has variations, apply them to the face. - if (ct.font.copyAttribute(.variation)) |variations| vars: { - defer variations.release(); - if (variations.getCount() == 0) break :vars; - - // This configuration is just used for testing so we don't want to - // have to pass a full allocator through so use the stack. We - // shouldn't have a lot of variations and if we do we should use - // another mechanism. - // - // On macOS the default stack size for a thread is 512KB and the main - // thread gets megabytes so 16KB is a safe stack allocation. - var data: [1024 * 16]u8 = undefined; - var fba = std.heap.FixedBufferAllocator.init(&data); - const alloc = fba.allocator(); - - var face_vars = std.ArrayList(font.face.Variation).init(alloc); - const kav = try variations.getKeysAndValues(alloc); - for (kav.keys, kav.values) |key, value| { - const num: *const macos.foundation.Number = @ptrCast(key.?); - const val: *const macos.foundation.Number = @ptrCast(value.?); - - var num_i32: i32 = undefined; - if (!num.getValue(.sint32, &num_i32)) continue; - - var val_f64: f64 = undefined; - if (!val.getValue(.float64, &val_f64)) continue; - - try face_vars.append(.{ - .id = @bitCast(num_i32), - .value = val_f64, - }); - } - - try face.setVariations(face_vars.items, opts); - } + try face.setVariations(ct.variations, opts); return face; } diff --git a/src/font/discovery.zig b/src/font/discovery.zig index 3aa16eebf..aeaeb955e 100644 --- a/src/font/discovery.zig +++ b/src/font/discovery.zig @@ -79,7 +79,7 @@ pub const Descriptor = struct { // This is not correct, but we don't currently depend on the // hash value being different based on decimal values of variations. - autoHash(hasher, @as(u64, @intFromFloat(variation.value))); + autoHash(hasher, @as(i64, @intFromFloat(variation.value))); } } @@ -384,6 +384,7 @@ pub const CoreText = struct { return DiscoverIterator{ .alloc = alloc, .list = zig_list, + .variations = desc.variations, .i = 0, }; } @@ -420,6 +421,7 @@ pub const CoreText = struct { return DiscoverIterator{ .alloc = alloc, .list = list, + .variations = desc.variations, .i = 0, }; } @@ -443,6 +445,7 @@ pub const CoreText = struct { return DiscoverIterator{ .alloc = alloc, .list = list, + .variations = desc.variations, .i = 0, }; } @@ -721,6 +724,7 @@ pub const CoreText = struct { pub const DiscoverIterator = struct { alloc: Allocator, list: []const *macos.text.FontDescriptor, + variations: []const Variation, i: usize, pub fn deinit(self: *DiscoverIterator) void { @@ -756,7 +760,10 @@ pub const CoreText = struct { defer self.i += 1; return DeferredFace{ - .ct = .{ .font = font }, + .ct = .{ + .font = font, + .variations = self.variations, + }, }; } }; diff --git a/src/font/face/coretext.zig b/src/font/face/coretext.zig index 2403b3902..363dbacd8 100644 --- a/src/font/face/coretext.zig +++ b/src/font/face/coretext.zig @@ -229,6 +229,9 @@ pub const Face = struct { vs: []const font.face.Variation, opts: font.face.Options, ) !void { + // If we have no variations, we don't need to do anything. + if (vs.len == 0) return; + // Create a new font descriptor with all the variations set. var desc = self.font.copyDescriptor(); defer desc.release();