From 12e8d96b1a951d08914707248190d7df4c7a01e9 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Thu, 18 Jul 2024 17:39:57 -0400 Subject: [PATCH 1/2] shaper/coretext: reset font cache on grid change Not doing this caused issues with spacing of ligatures and multi-substitutions when changing the font size with cmd +/- --- src/font/shaper/coretext.zig | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/font/shaper/coretext.zig b/src/font/shaper/coretext.zig index 8ae98e1d5..1eb35fa2c 100644 --- a/src/font/shaper/coretext.zig +++ b/src/font/shaper/coretext.zig @@ -59,6 +59,10 @@ pub const Shaper = struct { /// attributed strings. cached_fonts: std.ArrayListUnmanaged(?*macos.foundation.Dictionary), + /// The grid that our cached fonts correspond to. + /// If the grid changes then we need to reset our cache. + cached_font_grid: ?*font.SharedGrid, + /// The list of CoreFoundation objects to release on the dedicated /// release thread. This is built up over the course of shaping and /// sent to the release thread when endFrame is called. @@ -241,6 +245,7 @@ pub const Shaper = struct { .features = feats, .writing_direction = writing_direction, .cached_fonts = .{}, + .cached_font_grid = null, .cf_release_pool = .{}, .cf_release_thread = cf_release_thread, .cf_release_thr = cf_release_thr, @@ -487,6 +492,24 @@ pub const Shaper = struct { grid: *font.SharedGrid, index: font.Collection.Index, ) !*macos.foundation.Dictionary { + // If this grid doesn't match the one we've cached fonts for, + // then we reset the cache list since it's no longer valid. + if (grid != self.cached_font_grid) { + // Put all the currently cached fonts in to + // the release pool before clearing the list. + try self.cf_release_pool.ensureUnusedCapacity( + self.alloc, + self.cached_fonts.items.len, + ); + for (self.cached_fonts.items) |ft| { + if (ft) |f| self.cf_release_pool.appendAssumeCapacity(f); + } + + self.cached_fonts.clearRetainingCapacity(); + + self.cached_font_grid = grid; + } + const index_int = index.int(); // The cached fonts are indexed directly by the font index, since From f4c26dfaf52576bcb5c36dec2decc470bc9f3bcc Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 18 Jul 2024 18:46:09 -0700 Subject: [PATCH 2/2] shaper/coretext: use pointer address for cache comparison --- src/font/shaper/coretext.zig | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/font/shaper/coretext.zig b/src/font/shaper/coretext.zig index 1eb35fa2c..3165a592c 100644 --- a/src/font/shaper/coretext.zig +++ b/src/font/shaper/coretext.zig @@ -61,7 +61,7 @@ pub const Shaper = struct { /// The grid that our cached fonts correspond to. /// If the grid changes then we need to reset our cache. - cached_font_grid: ?*font.SharedGrid, + cached_font_grid: usize, /// The list of CoreFoundation objects to release on the dedicated /// release thread. This is built up over the course of shaping and @@ -245,7 +245,7 @@ pub const Shaper = struct { .features = feats, .writing_direction = writing_direction, .cached_fonts = .{}, - .cached_font_grid = null, + .cached_font_grid = 0, .cf_release_pool = .{}, .cf_release_thread = cf_release_thread, .cf_release_thr = cf_release_thr, @@ -494,20 +494,26 @@ pub const Shaper = struct { ) !*macos.foundation.Dictionary { // If this grid doesn't match the one we've cached fonts for, // then we reset the cache list since it's no longer valid. - if (grid != self.cached_font_grid) { - // Put all the currently cached fonts in to - // the release pool before clearing the list. - try self.cf_release_pool.ensureUnusedCapacity( - self.alloc, - self.cached_fonts.items.len, - ); - for (self.cached_fonts.items) |ft| { - if (ft) |f| self.cf_release_pool.appendAssumeCapacity(f); + // We use an intFromPtr rather than direct pointer comparison + // because we don't want anyone to inadvertenly use the pointer. + const grid_id: usize = @intFromPtr(grid); + if (grid_id != self.cached_font_grid) { + if (self.cached_font_grid > 0) { + // Put all the currently cached fonts in to + // the release pool before clearing the list. + try self.cf_release_pool.ensureUnusedCapacity( + self.alloc, + self.cached_fonts.items.len, + ); + for (self.cached_fonts.items) |ft| { + if (ft) |f| { + self.cf_release_pool.appendAssumeCapacity(f); + } + } + self.cached_fonts.clearRetainingCapacity(); } - self.cached_fonts.clearRetainingCapacity(); - - self.cached_font_grid = grid; + self.cached_font_grid = grid_id; } const index_int = index.int();