superrob1500
/
mirror-ghostty
mirror of https://github.com/ghostty-org/ghostty
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

font/freetype: introduce mutexes to ensure thread safety of Library and Face (#7238) 

tl;dr: FT_New_Face and FT_Done_Face require the Library to be locked for
thread safety, and FT_Load_Glyph and FT_Render_Glyph and friends need
the face to be locked for thread safety, since we're sharing faces
across threads.

For details see comments and FreeType docs @
-
https://freetype.org/freetype2/docs/reference/ft2-library_setup.html#ft_library
-
https://freetype.org/freetype2/docs/reference/ft2-face_creation.html#ft_face

---

This might resolve the issue discussed in #7016 but I wasn't able to
reliably reproduce it in a way I could debug, so someone who was
experiencing it should probably test this PR.

Additionally I can still semi-reliably produce a crash with the GTK
apprt by setting an `all:` keybind to adjust font sizes and changing the
font size rapidly with many surfaces open with emojis on them.
Unfortunately I can't really tell what the root cause is because the
debug info is broken in QEMU.

However, I do think this is a good idea for us to be thread safe with
this stuff even if it isn't related to that problem.
pull/7275/head
Mitchell Hashimoto 2025-05-06 07:18:11 -07:00 committed by GitHub
parent f73f7c805d 5319d38366
commit 702c3f58d9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 131 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/font/CodepointResolver.zig
View File

@ -380,7 +380,7 @@ test getIndex {
const testEmoji = font.embedded.emoji;
const testEmojiText = font.embedded.emoji_text;
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
var c = Collection.init();
@ -461,7 +461,7 @@ test "getIndex disabled font style" {
var atlas_grayscale = try font.Atlas.init(alloc, 512, .grayscale);
defer atlas_grayscale.deinit(alloc);
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
var c = Collection.init();
@ -513,7 +513,7 @@ test "getIndex box glyph" {
const testing = std.testing;
const alloc = testing.allocator;
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
const c = Collection.init();

35
src/font/Collection.zig
View File

@ -78,8 +78,8 @@ pub const AddError = Allocator.Error || error{
/// next in priority if others exist already, i.e. it'll be the _last_ to be
/// searched for a glyph in that list.
///
/// The collection takes ownership of the face. The face will be deallocated
/// when the collection is deallocated.
/// If no error is encountered then the collection takes ownership of the face,
/// in which case face will be deallocated when the collection is deallocated.
///
/// If a loaded face is added to the collection, it should be the same
/// size as all the other faces in the collection. This function will not
@ -700,7 +700,7 @@ test "add full" {
const alloc = testing.allocator;
const testFont = font.embedded.regular;
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
var c = init();
@ -714,15 +714,18 @@ test "add full" {
) });
}
try testing.expectError(error.CollectionFull, c.add(
alloc,
.regular,
.{ .loaded = try Face.init(
var face = try Face.init(
lib,
testFont,
.{ .size = .{ .points = 12 } },
) },
));
);
// We have to deinit it manually since the
// collection doesn't do it if adding fails.
defer face.deinit();
try testing.expectError(
error.CollectionFull,
c.add(alloc, .regular, .{ .loaded = face }),
);
}
test "add deferred without loading options" {
@ -746,7 +749,7 @@ test getFace {
const alloc = testing.allocator;
const testFont = font.embedded.regular;
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
var c = init();
@ -770,7 +773,7 @@ test getIndex {
const alloc = testing.allocator;
const testFont = font.embedded.regular;
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
var c = init();
@ -801,7 +804,7 @@ test completeStyles {
const alloc = testing.allocator;
const testFont = font.embedded.regular;
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
var c = init();
@ -828,7 +831,7 @@ test setSize {
const alloc = testing.allocator;
const testFont = font.embedded.regular;
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
var c = init();
@ -851,7 +854,7 @@ test hasCodepoint {
const alloc = testing.allocator;
const testFont = font.embedded.regular;
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
var c = init();
@ -875,7 +878,7 @@ test "hasCodepoint emoji default graphical" {
const alloc = testing.allocator;
const testEmoji = font.embedded.emoji;
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
var c = init();
@ -898,7 +901,7 @@ test "metrics" {
const alloc = testing.allocator;
const testFont = font.embedded.inconsolata;
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
var c = init();

10
src/font/DeferredFace.zig
View File

@ -407,7 +407,7 @@ test "fontconfig" {
const alloc = testing.allocator;
// Load freetype
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
// Get a deferred face from fontconfig
@ -425,7 +425,8 @@ test "fontconfig" {
try testing.expect(n.len > 0);
// Load it and verify it works
const face = try def.load(lib, .{ .size = .{ .points = 12 } });
var face = try def.load(lib, .{ .size = .{ .points = 12 } });
defer face.deinit();
try testing.expect(face.glyphIndex(' ') != null);
}
@ -437,7 +438,7 @@ test "coretext" {
const alloc = testing.allocator;
// Load freetype
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
// Get a deferred face from fontconfig
@ -456,6 +457,7 @@ test "coretext" {
try testing.expect(n.len > 0);
// Load it and verify it works
const face = try def.load(lib, .{ .size = .{ .points = 12 } });
var face = try def.load(lib, .{ .size = .{ .points = 12 } });
defer face.deinit();
try testing.expect(face.glyphIndex(' ') != null);
}

2
src/font/SharedGrid.zig
View File

@ -338,7 +338,7 @@ test getIndex {
const alloc = testing.allocator;
// const testEmoji = @import("test.zig").fontEmoji;
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
var grid = try testGrid(.normal, alloc, lib);

2
src/font/SharedGridSet.zig
View File

@ -50,7 +50,7 @@ pub const InitError = Library.InitError;
/// Initialize a new SharedGridSet.
pub fn init(alloc: Allocator) InitError!SharedGridSet {
var font_lib = try Library.init();
var font_lib = try Library.init(alloc);
errdefer font_lib.deinit();
return .{

17
src/font/face/coretext.zig
View File

@ -46,7 +46,11 @@ pub const Face = struct {
};
/// Initialize a CoreText-based font from a TTF/TTC in memory.
pub fn init(lib: font.Library, source: [:0]const u8, opts: font.face.Options) !Face {
pub fn init(
lib: font.Library,
source: [:0]const u8,
opts: font.face.Options,
) !Face {
_ = lib;
const data = try macos.foundation.Data.createWithBytesNoCopy(source);
@ -914,7 +918,7 @@ test "in-memory" {
var atlas = try font.Atlas.init(alloc, 512, .grayscale);
defer atlas.deinit(alloc);
var lib = try font.Library.init();
var lib = try font.Library.init(alloc);
defer lib.deinit();
var face = try Face.init(lib, testFont, .{ .size = .{ .points = 12 } });
@ -941,7 +945,7 @@ test "variable" {
var atlas = try font.Atlas.init(alloc, 512, .grayscale);
defer atlas.deinit(alloc);
var lib = try font.Library.init();
var lib = try font.Library.init(alloc);
defer lib.deinit();
var face = try Face.init(lib, testFont, .{ .size = .{ .points = 12 } });
@ -968,7 +972,7 @@ test "variable set variation" {
var atlas = try font.Atlas.init(alloc, 512, .grayscale);
defer atlas.deinit(alloc);
var lib = try font.Library.init();
var lib = try font.Library.init(alloc);
defer lib.deinit();
var face = try Face.init(lib, testFont, .{ .size = .{ .points = 12 } });
@ -996,7 +1000,7 @@ test "svg font table" {
const alloc = testing.allocator;
const testFont = font.embedded.julia_mono;
var lib = try font.Library.init();
var lib = try font.Library.init(alloc);
defer lib.deinit();
var face = try Face.init(lib, testFont, .{ .size = .{ .points = 12 } });
@ -1010,9 +1014,10 @@ test "svg font table" {
test "glyphIndex colored vs text" {
const testing = std.testing;
const alloc = testing.allocator;
const testFont = font.embedded.julia_mono;
var lib = try font.Library.init();
var lib = try font.Library.init(alloc);
defer lib.deinit();
var face = try Face.init(lib, testFont, .{ .size = .{ .points = 12 } });

85
src/font/face/freetype.zig
View File

@ -29,12 +29,20 @@ pub const Face = struct {
assert(font.face.FreetypeLoadFlags != void);
}
/// Our freetype library
lib: freetype.Library,
/// Our Library
lib: Library,
/// Our font face.
face: freetype.Face,
/// This mutex MUST be held while doing anything with the
/// glyph slot on the freetype face, because this struct
/// may be shared across multiple surfaces.
///
/// This means that anywhere where `self.face.loadGlyph`
/// is called, this mutex must be held.
ft_mutex: *std.Thread.Mutex,
/// Harfbuzz font corresponding to this face.
hb_font: harfbuzz.Font,
@ -59,30 +67,52 @@ pub const Face = struct {
};
/// Initialize a new font face with the given source in-memory.
pub fn initFile(lib: Library, path: [:0]const u8, index: i32, opts: font.face.Options) !Face {
pub fn initFile(
lib: Library,
path: [:0]const u8,
index: i32,
opts: font.face.Options,
) !Face {
lib.mutex.lock();
defer lib.mutex.unlock();
const face = try lib.lib.initFace(path, index);
errdefer face.deinit();
return try initFace(lib, face, opts);
}
/// Initialize a new font face with the given source in-memory.
pub fn init(lib: Library, source: [:0]const u8, opts: font.face.Options) !Face {
pub fn init(
lib: Library,
source: [:0]const u8,
opts: font.face.Options,
) !Face {
lib.mutex.lock();
defer lib.mutex.unlock();
const face = try lib.lib.initMemoryFace(source, 0);
errdefer face.deinit();
return try initFace(lib, face, opts);
}
fn initFace(lib: Library, face: freetype.Face, opts: font.face.Options) !Face {
fn initFace(
lib: Library,
face: freetype.Face,
opts: font.face.Options,
) !Face {
try face.selectCharmap(.unicode);
try setSize_(face, opts.size);
var hb_font = try harfbuzz.freetype.createFont(face.handle);
errdefer hb_font.destroy();
const ft_mutex = try lib.alloc.create(std.Thread.Mutex);
errdefer lib.alloc.destroy(ft_mutex);
ft_mutex.* = .{};
var result: Face = .{
.lib = lib.lib,
.lib = lib,
.face = face,
.hb_font = hb_font,
.ft_mutex = ft_mutex,
.load_flags = opts.freetype_load_flags,
};
result.quirks_disable_default_font_features = quirks.disableDefaultFontFeatures(&result);
@ -114,7 +144,13 @@ pub const Face = struct {
}
pub fn deinit(self: *Face) void {
self.lib.alloc.destroy(self.ft_mutex);
{
self.lib.mutex.lock();
defer self.lib.mutex.unlock();
self.face.deinit();
}
self.hb_font.destroy();
self.* = undefined;
}
@ -147,11 +183,7 @@ pub const Face = struct {
self.face.ref();
errdefer self.face.deinit();
var f = try initFace(
.{ .lib = self.lib },
self.face,
opts,
);
var f = try initFace(self.lib, self.face, opts);
errdefer f.deinit();
f.synthetic = self.synthetic;
f.synthetic.bold = true;
@ -166,11 +198,7 @@ pub const Face = struct {
self.face.ref();
errdefer self.face.deinit();
var f = try initFace(
.{ .lib = self.lib },
self.face,
opts,
);
var f = try initFace(self.lib, self.face, opts);
errdefer f.deinit();
f.synthetic = self.synthetic;
f.synthetic.italic = true;
@ -228,7 +256,7 @@ pub const Face = struct {
// first thing we have to do is get all the vars and put them into
// an array.
const mm = try self.face.getMMVar();
defer self.lib.doneMMVar(mm);
defer self.lib.lib.doneMMVar(mm);
// To avoid allocations, we cap the number of variation axes we can
// support. This is arbitrary but Firefox caps this at 16 so I
@ -270,6 +298,9 @@ pub const Face = struct {
/// Returns true if the given glyph ID is colorized.
pub fn isColorGlyph(self: *const Face, glyph_id: u32) bool {
self.ft_mutex.lock();
defer self.ft_mutex.unlock();
// Load the glyph and see what pixel mode it renders with.
// All modes other than BGRA are non-color.
// If the glyph fails to load, just return false.
@ -296,6 +327,9 @@ pub const Face = struct {
glyph_index: u32,
opts: font.face.RenderOptions,
) !Glyph {
self.ft_mutex.lock();
defer self.ft_mutex.unlock();
const metrics = opts.grid_metrics;
// If we have synthetic italic, then we apply a transformation matrix.
@ -741,6 +775,9 @@ pub const Face = struct {
// If we fail to load any visible ASCII we just use max_advance from
// the metrics provided by FreeType.
const cell_width: f64 = cell_width: {
self.ft_mutex.lock();
defer self.ft_mutex.unlock();
var max: f64 = 0.0;
var c: u8 = ' ';
while (c < 127) : (c += 1) {
@ -780,6 +817,8 @@ pub const Face = struct {
break :heights .{
cap: {
self.ft_mutex.lock();
defer self.ft_mutex.unlock();
if (face.getCharIndex('H')) |glyph_index| {
if (face.loadGlyph(glyph_index, .{
.render = true,
@ -791,6 +830,8 @@ pub const Face = struct {
break :cap null;
},
ex: {
self.ft_mutex.lock();
defer self.ft_mutex.unlock();
if (face.getCharIndex('x')) |glyph_index| {
if (face.loadGlyph(glyph_index, .{
.render = true,
@ -832,7 +873,7 @@ test {
const testFont = font.embedded.inconsolata;
const alloc = testing.allocator;
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
var atlas = try font.Atlas.init(alloc, 512, .grayscale);
@ -881,7 +922,7 @@ test "color emoji" {
const alloc = testing.allocator;
const testFont = font.embedded.emoji;
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
var atlas = try font.Atlas.init(alloc, 512, .rgba);
@ -936,7 +977,7 @@ test "mono to rgba" {
const alloc = testing.allocator;
const testFont = font.embedded.emoji;
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
var atlas = try font.Atlas.init(alloc, 512, .rgba);
@ -958,7 +999,7 @@ test "svg font table" {
const alloc = testing.allocator;
const testFont = font.embedded.julia_mono;
var lib = try font.Library.init();
var lib = try font.Library.init(alloc);
defer lib.deinit();
var face = try Face.init(lib, testFont, .{ .size = .{ .points = 12, .xdpi = 72, .ydpi = 72 } });
@ -995,7 +1036,7 @@ test "bitmap glyph" {
const alloc = testing.allocator;
const testFont = font.embedded.terminus_ttf;
var lib = try Library.init();
var lib = try Library.init(alloc);
defer lib.deinit();
var atlas = try font.Atlas.init(alloc, 512, .grayscale);

24
src/font/library.zig
View File

@ -1,5 +1,7 @@
//! A library represents the shared state that the underlying font
//! library implementation(s) require per-process.
const std = @import("std");
const Allocator = std.mem.Allocator;
const builtin = @import("builtin");
const options = @import("main.zig").options;
const freetype = @import("freetype");
@ -24,13 +26,26 @@ pub const Library = switch (options.backend) {
pub const FreetypeLibrary = struct {
lib: freetype.Library,
pub const InitError = freetype.Error;
alloc: Allocator,
pub fn init() InitError!Library {
return Library{ .lib = try freetype.Library.init() };
/// Mutex to be held any time the library is
/// being used to create or destroy a face.
mutex: *std.Thread.Mutex,
pub const InitError = freetype.Error || Allocator.Error;
pub fn init(alloc: Allocator) InitError!Library {
const lib = try freetype.Library.init();
errdefer lib.deinit();
const mutex = try alloc.create(std.Thread.Mutex);
mutex.* = .{};
return Library{ .lib = lib, .alloc = alloc, .mutex = mutex };
}
pub fn deinit(self: *Library) void {
self.alloc.destroy(self.mutex);
self.lib.deinit();
}
};
@ -38,7 +53,8 @@ pub const FreetypeLibrary = struct {
pub const NoopLibrary = struct {
pub const InitError = error{};
pub fn init() InitError!Library {
pub fn init(alloc: Allocator) InitError!Library {
_ = alloc;
return Library{};
}

2
src/font/opentype/svg.zig
View File

@ -99,7 +99,7 @@ test "SVG" {
const alloc = testing.allocator;
const testFont = font.embedded.julia_mono;
var lib = try font.Library.init();
var lib = try font.Library.init(alloc);
defer lib.deinit();
var face = try font.Face.init(lib, testFont, .{ .size = .{ .points = 12 } });

2
src/font/shaper/coretext.zig
View File

@ -1761,7 +1761,7 @@ fn testShaperWithFont(alloc: Allocator, font_req: TestFont) !TestShaper {
.nerd_font => font.embedded.nerd_font,
};
var lib = try Library.init();
var lib = try Library.init(alloc);
errdefer lib.deinit();
var c = Collection.init();

2
src/font/shaper/harfbuzz.zig
View File

@ -1220,7 +1220,7 @@ fn testShaperWithFont(alloc: Allocator, font_req: TestFont) !TestShaper {
.arabic => font.embedded.arabic,
};
var lib = try Library.init();
var lib = try Library.init(alloc);
errdefer lib.deinit();
var c = Collection.init();