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.
pull/9002/head
Qwerasd 2025-10-02 15:01:44 -06:00
parent d6063428bd
commit efc6e0d673
3 changed files with 80 additions and 31 deletions

View File

@ -4,6 +4,7 @@ const font_descriptor = @import("text/font_descriptor.zig");
const font_manager = @import("text/font_manager.zig"); const font_manager = @import("text/font_manager.zig");
const frame = @import("text/frame.zig"); const frame = @import("text/frame.zig");
const framesetter = @import("text/framesetter.zig"); const framesetter = @import("text/framesetter.zig");
const typesetter = @import("text/typesetter.zig");
const line = @import("text/line.zig"); const line = @import("text/line.zig");
const paragraph_style = @import("text/paragraph_style.zig"); const paragraph_style = @import("text/paragraph_style.zig");
const run = @import("text/run.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 createFontDescriptorFromData = font_manager.createFontDescriptorFromData;
pub const Frame = frame.Frame; pub const Frame = frame.Frame;
pub const Framesetter = framesetter.Framesetter; pub const Framesetter = framesetter.Framesetter;
pub const Typesetter = typesetter.Typesetter;
pub const Line = line.Line; pub const Line = line.Line;
pub const ParagraphStyle = paragraph_style.ParagraphStyle; pub const ParagraphStyle = paragraph_style.ParagraphStyle;
pub const ParagraphStyleSetting = paragraph_style.ParagraphStyleSetting; pub const ParagraphStyleSetting = paragraph_style.ParagraphStyleSetting;

View File

@ -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,
)));
}
};

View File

@ -52,10 +52,10 @@ pub const Shaper = struct {
/// The shared memory used for shaping results. /// The shared memory used for shaping results.
cell_buf: CellBuf, cell_buf: CellBuf,
/// The cached writing direction value for shaping. This isn't /// Cached attributes dict for creating CTTypesetter objects.
/// configurable we just use this as a cache to avoid creating /// The values in this never change so we can avoid overhead
/// and releasing many objects when shaping. /// by just creating it once and saving it for re-use.
writing_direction: *macos.foundation.Array, typesetter_attr_dict: *macos.foundation.Dictionary,
/// List where we cache fonts, so we don't have to remake them for /// List where we cache fonts, so we don't have to remake them for
/// every single shaping operation. /// 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/1737
// See: https://github.com/mitchellh/ghostty/issues/1442 // See: https://github.com/mitchellh/ghostty/issues/1442
const writing_direction = array: { //
const dir: macos.text.WritingDirection = .lro; // We used to do this by setting the writing direction attribute
const num = try macos.foundation.Number.create( // on the attributed string we used, but it seems like that will
.int, // still allow some weird results, for example a single space at
&@intFromEnum(dir), // 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(); defer num.release();
break :dict try macos.foundation.Dictionary.create(
var arr_init = [_]*const macos.foundation.Number{num}; &.{macos.c.kCTTypesetterOptionForcedEmbeddingLevel},
break :array try macos.foundation.Array.create( &.{num},
macos.foundation.Number,
&arr_init,
); );
}; };
errdefer writing_direction.release(); errdefer typesetter_attr_dict.release();
// Create the CF release thread. // Create the CF release thread.
var cf_release_thread = try alloc.create(CFReleaseThread); var cf_release_thread = try alloc.create(CFReleaseThread);
@ -210,7 +217,7 @@ pub const Shaper = struct {
.run_state = run_state, .run_state = run_state,
.features = features, .features = features,
.features_no_default = features_no_default, .features_no_default = features_no_default,
.writing_direction = writing_direction, .typesetter_attr_dict = typesetter_attr_dict,
.cached_fonts = .{}, .cached_fonts = .{},
.cached_font_grid = 0, .cached_font_grid = 0,
.cf_release_pool = .{}, .cf_release_pool = .{},
@ -224,7 +231,7 @@ pub const Shaper = struct {
self.run_state.deinit(self.alloc); self.run_state.deinit(self.alloc);
self.features.release(); self.features.release();
self.features_no_default.release(); self.features_no_default.release();
self.writing_direction.release(); self.typesetter_attr_dict.release();
{ {
for (self.cached_fonts.items) |ft| { for (self.cached_fonts.items) |ft| {
@ -346,8 +353,8 @@ pub const Shaper = struct {
run.font_index, run.font_index,
); );
// Make room for the attributed string and the CTLine. // Make room for the attributed string, CTTypesetter, and CTLine.
try self.cf_release_pool.ensureUnusedCapacity(self.alloc, 3); try self.cf_release_pool.ensureUnusedCapacity(self.alloc, 4);
const str = macos.foundation.String.createWithCharactersNoCopy(state.unichars.items); const str = macos.foundation.String.createWithCharactersNoCopy(state.unichars.items);
self.cf_release_pool.appendAssumeCapacity(str); self.cf_release_pool.appendAssumeCapacity(str);
@ -359,8 +366,17 @@ pub const Shaper = struct {
); );
self.cf_release_pool.appendAssumeCapacity(attr_str); self.cf_release_pool.appendAssumeCapacity(attr_str);
// We should always have one run because we do our own run splitting. // Create a typesetter from the attributed string and the cached
const line = try macos.text.Line.createWithAttributedString(attr_str); // 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); self.cf_release_pool.appendAssumeCapacity(line);
// This keeps track of the current offsets within a single cell. // 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 // Get our font and use that get the attributes to set for the
// attributed string so the whole string uses the same font. // attributed string so the whole string uses the same font.
const attr_dict = dict: { const attr_dict = dict: {
var keys = [_]?*const anyopaque{ break :dict try macos.foundation.Dictionary.create(
macos.text.StringAttribute.font.key(), &.{macos.text.StringAttribute.font.key()},
macos.text.StringAttribute.writing_direction.key(), &.{run_font},
}; );
var values = [_]?*const anyopaque{
run_font,
self.writing_direction,
};
break :dict try macos.foundation.Dictionary.create(&keys, &values);
}; };
self.cached_fonts.items[index_int] = attr_dict; self.cached_fonts.items[index_int] = attr_dict;