From d6063428bd9b58ca377b576eb0638be72afaa933 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Thu, 2 Oct 2025 14:06:58 -0600 Subject: [PATCH 1/2] font/coretext: tiny shaper improvements Reduce potential allocation while processing glyphs by ensuring capacity in the buffer ahead of time and also using CTRunGet*Ptr functions first and only allocating for those if that didn't work (it should almost always work in practice.) --- pkg/macos/text/run.zig | 40 ++++++++++++++++++++++++++++-------- src/font/shaper/coretext.zig | 13 ++++++++---- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/pkg/macos/text/run.zig b/pkg/macos/text/run.zig index 9d40de81f..2895bfe34 100644 --- a/pkg/macos/text/run.zig +++ b/pkg/macos/text/run.zig @@ -15,10 +15,13 @@ pub const Run = opaque { return @intCast(c.CTRunGetGlyphCount(@ptrCast(self))); } - pub fn getGlyphsPtr(self: *Run) []const graphics.Glyph { + pub fn getGlyphsPtr(self: *Run) ?[]const graphics.Glyph { const len = self.getGlyphCount(); if (len == 0) return &.{}; - const ptr = c.CTRunGetGlyphsPtr(@ptrCast(self)) orelse &.{}; + const ptr: [*c]const graphics.Glyph = @ptrCast( + c.CTRunGetGlyphsPtr(@ptrCast(self)), + ); + if (ptr == null) return null; return ptr[0..len]; } @@ -34,10 +37,13 @@ pub const Run = opaque { return ptr; } - pub fn getPositionsPtr(self: *Run) []const graphics.Point { + pub fn getPositionsPtr(self: *Run) ?[]const graphics.Point { const len = self.getGlyphCount(); if (len == 0) return &.{}; - const ptr = c.CTRunGetPositionsPtr(@ptrCast(self)) orelse &.{}; + const ptr: [*c]const graphics.Point = @ptrCast( + c.CTRunGetPositionsPtr(@ptrCast(self)), + ); + if (ptr == null) return null; return ptr[0..len]; } @@ -53,10 +59,13 @@ pub const Run = opaque { return ptr; } - pub fn getAdvancesPtr(self: *Run) []const graphics.Size { + pub fn getAdvancesPtr(self: *Run) ?[]const graphics.Size { const len = self.getGlyphCount(); if (len == 0) return &.{}; - const ptr = c.CTRunGetAdvancesPtr(@ptrCast(self)) orelse &.{}; + const ptr: [*c]const graphics.Size = @ptrCast( + c.CTRunGetAdvancesPtr(@ptrCast(self)), + ); + if (ptr == null) return null; return ptr[0..len]; } @@ -72,10 +81,13 @@ pub const Run = opaque { return ptr; } - pub fn getStringIndicesPtr(self: *Run) []const usize { + pub fn getStringIndicesPtr(self: *Run) ?[]const usize { const len = self.getGlyphCount(); if (len == 0) return &.{}; - const ptr = c.CTRunGetStringIndicesPtr(@ptrCast(self)) orelse &.{}; + const ptr: [*c]const usize = @ptrCast( + c.CTRunGetStringIndicesPtr(@ptrCast(self)), + ); + if (ptr == null) return null; return ptr[0..len]; } @@ -90,4 +102,16 @@ pub const Run = opaque { ); return ptr; } + + pub fn getStatus(self: *Run) Status { + return @bitCast(c.CTRunGetStatus(@ptrCast(self))); + } +}; + +/// https://developer.apple.com/documentation/coretext/ctrunstatus?language=objc +pub const Status = packed struct(u32) { + right_to_left: bool, + non_monotonic: bool, + has_non_identity_matrix: bool, + _pad: u29 = 0, }; diff --git a/src/font/shaper/coretext.zig b/src/font/shaper/coretext.zig index 285a5a6b9..b00610d2f 100644 --- a/src/font/shaper/coretext.zig +++ b/src/font/shaper/coretext.zig @@ -369,7 +369,12 @@ pub const Shaper = struct { x: f64 = 0, y: f64 = 0, } = .{}; + + // Clear our cell buf and make sure we have enough room for the whole + // line of glyphs, so that we can just assume capacity when appending + // instead of maybe allocating. self.cell_buf.clearRetainingCapacity(); + try self.cell_buf.ensureTotalCapacity(self.alloc, line.getGlyphCount()); // CoreText may generate multiple runs even though our input to // CoreText is already split into runs by our own run iterator. @@ -381,9 +386,9 @@ pub const Shaper = struct { const ctrun = runs.getValueAtIndex(macos.text.Run, i); // Get our glyphs and positions - const glyphs = try ctrun.getGlyphs(alloc); - const advances = try ctrun.getAdvances(alloc); - const indices = try ctrun.getStringIndices(alloc); + const glyphs = ctrun.getGlyphsPtr() orelse try ctrun.getGlyphs(alloc); + const advances = ctrun.getAdvancesPtr() orelse try ctrun.getAdvances(alloc); + const indices = ctrun.getStringIndicesPtr() orelse try ctrun.getStringIndices(alloc); assert(glyphs.len == advances.len); assert(glyphs.len == indices.len); @@ -406,7 +411,7 @@ pub const Shaper = struct { cell_offset = .{ .cluster = cluster }; } - try self.cell_buf.append(self.alloc, .{ + self.cell_buf.appendAssumeCapacity(.{ .x = @intCast(cluster), .x_offset = @intFromFloat(@round(cell_offset.x)), .y_offset = @intFromFloat(@round(cell_offset.y)), From efc6e0d6738d3ff070422db1512eb1b9f91b278f Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Thu, 2 Oct 2025 15:01:44 -0600 Subject: [PATCH 2/2] fix(font/coretext): always prevent shaper from emitting rtl The solution we had before worked in most cases but there were some which caused problems still. This is what HarfBuzz's CoreText shaper backend does, it uses a CTTypesetter with the forced embedding level attribute. This fixes the failure case I found that was causing non- monotonic outputs which can have all sorts of unexpected results, and causes a crash in Debug modes because we assert the monotonicity while rendering. --- pkg/macos/text.zig | 2 + pkg/macos/text/typesetter.zig | 36 +++++++++++++++++ src/font/shaper/coretext.zig | 73 ++++++++++++++++++++--------------- 3 files changed, 80 insertions(+), 31 deletions(-) create mode 100644 pkg/macos/text/typesetter.zig diff --git a/pkg/macos/text.zig b/pkg/macos/text.zig index 0589f8692..bfaa388b3 100644 --- a/pkg/macos/text.zig +++ b/pkg/macos/text.zig @@ -4,6 +4,7 @@ const font_descriptor = @import("text/font_descriptor.zig"); const font_manager = @import("text/font_manager.zig"); const frame = @import("text/frame.zig"); const framesetter = @import("text/framesetter.zig"); +const typesetter = @import("text/typesetter.zig"); const line = @import("text/line.zig"); const paragraph_style = @import("text/paragraph_style.zig"); const run = @import("text/run.zig"); @@ -23,6 +24,7 @@ pub const createFontDescriptorsFromData = font_manager.createFontDescriptorsFrom pub const createFontDescriptorFromData = font_manager.createFontDescriptorFromData; pub const Frame = frame.Frame; pub const Framesetter = framesetter.Framesetter; +pub const Typesetter = typesetter.Typesetter; pub const Line = line.Line; pub const ParagraphStyle = paragraph_style.ParagraphStyle; pub const ParagraphStyleSetting = paragraph_style.ParagraphStyleSetting; diff --git a/pkg/macos/text/typesetter.zig b/pkg/macos/text/typesetter.zig new file mode 100644 index 000000000..dc07df980 --- /dev/null +++ b/pkg/macos/text/typesetter.zig @@ -0,0 +1,36 @@ +const std = @import("std"); +const assert = std.debug.assert; +const Allocator = std.mem.Allocator; +const foundation = @import("../foundation.zig"); +const graphics = @import("../graphics.zig"); +const text = @import("../text.zig"); +const c = @import("c.zig").c; + +pub const Typesetter = opaque { + pub fn createWithAttributedStringAndOptions( + str: *foundation.AttributedString, + opts: *foundation.Dictionary, + ) Allocator.Error!*Typesetter { + return @as( + ?*Typesetter, + @ptrFromInt(@intFromPtr(c.CTTypesetterCreateWithAttributedStringAndOptions( + @ptrCast(str), + @ptrCast(opts), + ))), + ) orelse Allocator.Error.OutOfMemory; + } + + pub fn release(self: *Typesetter) void { + foundation.CFRelease(self); + } + + pub fn createLine( + self: *Typesetter, + range: foundation.c.CFRange, + ) *text.Line { + return @ptrFromInt(@intFromPtr(c.CTTypesetterCreateLine( + @ptrCast(self), + range, + ))); + } +}; diff --git a/src/font/shaper/coretext.zig b/src/font/shaper/coretext.zig index b00610d2f..f1368679d 100644 --- a/src/font/shaper/coretext.zig +++ b/src/font/shaper/coretext.zig @@ -52,10 +52,10 @@ pub const Shaper = struct { /// The shared memory used for shaping results. cell_buf: CellBuf, - /// The cached writing direction value for shaping. This isn't - /// configurable we just use this as a cache to avoid creating - /// and releasing many objects when shaping. - writing_direction: *macos.foundation.Array, + /// Cached attributes dict for creating CTTypesetter objects. + /// The values in this never change so we can avoid overhead + /// by just creating it once and saving it for re-use. + typesetter_attr_dict: *macos.foundation.Dictionary, /// List where we cache fonts, so we don't have to remake them for /// every single shaping operation. @@ -174,21 +174,28 @@ pub const Shaper = struct { // // See: https://github.com/mitchellh/ghostty/issues/1737 // See: https://github.com/mitchellh/ghostty/issues/1442 - const writing_direction = array: { - const dir: macos.text.WritingDirection = .lro; - const num = try macos.foundation.Number.create( - .int, - &@intFromEnum(dir), - ); + // + // We used to do this by setting the writing direction attribute + // on the attributed string we used, but it seems like that will + // still allow some weird results, for example a single space at + // the end of a line composed of RTL characters will be cause it + // to output a run containing just that space, BEFORE it outputs + // the rest of the line as a separate run, very weirdly with the + // "right to left" flag set in the single space run's run status... + // + // So instead what we do is use a CTTypesetter to create our line, + // using the kCTTypesetterOptionForcedEmbeddingLevel attribute to + // force CoreText not to try doing any sort of BiDi, instead just + // treat all text as embedding level 0 (left to right). + const typesetter_attr_dict = dict: { + const num = try macos.foundation.Number.create(.int, &0); defer num.release(); - - var arr_init = [_]*const macos.foundation.Number{num}; - break :array try macos.foundation.Array.create( - macos.foundation.Number, - &arr_init, + break :dict try macos.foundation.Dictionary.create( + &.{macos.c.kCTTypesetterOptionForcedEmbeddingLevel}, + &.{num}, ); }; - errdefer writing_direction.release(); + errdefer typesetter_attr_dict.release(); // Create the CF release thread. var cf_release_thread = try alloc.create(CFReleaseThread); @@ -210,7 +217,7 @@ pub const Shaper = struct { .run_state = run_state, .features = features, .features_no_default = features_no_default, - .writing_direction = writing_direction, + .typesetter_attr_dict = typesetter_attr_dict, .cached_fonts = .{}, .cached_font_grid = 0, .cf_release_pool = .{}, @@ -224,7 +231,7 @@ pub const Shaper = struct { self.run_state.deinit(self.alloc); self.features.release(); self.features_no_default.release(); - self.writing_direction.release(); + self.typesetter_attr_dict.release(); { for (self.cached_fonts.items) |ft| { @@ -346,8 +353,8 @@ pub const Shaper = struct { run.font_index, ); - // Make room for the attributed string and the CTLine. - try self.cf_release_pool.ensureUnusedCapacity(self.alloc, 3); + // Make room for the attributed string, CTTypesetter, and CTLine. + try self.cf_release_pool.ensureUnusedCapacity(self.alloc, 4); const str = macos.foundation.String.createWithCharactersNoCopy(state.unichars.items); self.cf_release_pool.appendAssumeCapacity(str); @@ -359,8 +366,17 @@ pub const Shaper = struct { ); self.cf_release_pool.appendAssumeCapacity(attr_str); - // We should always have one run because we do our own run splitting. - const line = try macos.text.Line.createWithAttributedString(attr_str); + // Create a typesetter from the attributed string and the cached + // attr dict. (See comment in init for more info on the attr dict.) + const typesetter = + try macos.text.Typesetter.createWithAttributedStringAndOptions( + attr_str, + self.typesetter_attr_dict, + ); + self.cf_release_pool.appendAssumeCapacity(typesetter); + + // Create a line from the typesetter + const line = typesetter.createLine(.{ .location = 0, .length = 0 }); self.cf_release_pool.appendAssumeCapacity(line); // This keeps track of the current offsets within a single cell. @@ -516,15 +532,10 @@ pub const Shaper = struct { // Get our font and use that get the attributes to set for the // attributed string so the whole string uses the same font. const attr_dict = dict: { - var keys = [_]?*const anyopaque{ - macos.text.StringAttribute.font.key(), - macos.text.StringAttribute.writing_direction.key(), - }; - var values = [_]?*const anyopaque{ - run_font, - self.writing_direction, - }; - break :dict try macos.foundation.Dictionary.create(&keys, &values); + break :dict try macos.foundation.Dictionary.create( + &.{macos.text.StringAttribute.font.key()}, + &.{run_font}, + ); }; self.cached_fonts.items[index_int] = attr_dict;