From cfedd477b2843c4624aac88b76ec24cd7ecd36d6 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Wed, 30 Apr 2025 14:02:46 -0600 Subject: [PATCH] font/freetype: introduce mutexes to ensure thread safety of Library and Face 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 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. --- src/font/CodepointResolver.zig | 6 +-- src/font/Collection.zig | 20 ++++---- src/font/DeferredFace.zig | 4 +- src/font/SharedGrid.zig | 2 +- src/font/SharedGridSet.zig | 2 +- src/font/face/coretext.zig | 17 ++++--- src/font/face/freetype.zig | 87 +++++++++++++++++++++++++--------- src/font/library.zig | 24 ++++++++-- src/font/opentype/svg.zig | 2 +- src/font/shaper/coretext.zig | 2 +- src/font/shaper/harfbuzz.zig | 2 +- 11 files changed, 115 insertions(+), 53 deletions(-) diff --git a/src/font/CodepointResolver.zig b/src/font/CodepointResolver.zig index 326ca0186..37093b59a 100644 --- a/src/font/CodepointResolver.zig +++ b/src/font/CodepointResolver.zig @@ -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(); diff --git a/src/font/Collection.zig b/src/font/Collection.zig index cfc633b04..66e13c109 100644 --- a/src/font/Collection.zig +++ b/src/font/Collection.zig @@ -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(); @@ -746,7 +746,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 +770,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 +801,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 +828,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 +851,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 +875,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 +898,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(); diff --git a/src/font/DeferredFace.zig b/src/font/DeferredFace.zig index 3ee104386..d61f5492f 100644 --- a/src/font/DeferredFace.zig +++ b/src/font/DeferredFace.zig @@ -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 @@ -437,7 +437,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 diff --git a/src/font/SharedGrid.zig b/src/font/SharedGrid.zig index 65c7ecd87..72e97fad8 100644 --- a/src/font/SharedGrid.zig +++ b/src/font/SharedGrid.zig @@ -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); diff --git a/src/font/SharedGridSet.zig b/src/font/SharedGridSet.zig index ca535eaf8..8ad30629e 100644 --- a/src/font/SharedGridSet.zig +++ b/src/font/SharedGridSet.zig @@ -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 .{ diff --git a/src/font/face/coretext.zig b/src/font/face/coretext.zig index 3749b4824..639eae43c 100644 --- a/src/font/face/coretext.zig +++ b/src/font/face/coretext.zig @@ -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 } }); diff --git a/src/font/face/freetype.zig b/src/font/face/freetype.zig index c2eab4599..bf86b88de 100644 --- a/src/font/face/freetype.zig +++ b/src/font/face/freetype.zig @@ -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.face.deinit(); + 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); diff --git a/src/font/library.zig b/src/font/library.zig index b00bbfce0..43aa101b7 100644 --- a/src/font/library.zig +++ b/src/font/library.zig @@ -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{}; } diff --git a/src/font/opentype/svg.zig b/src/font/opentype/svg.zig index 01d172d17..ff8eeed49 100644 --- a/src/font/opentype/svg.zig +++ b/src/font/opentype/svg.zig @@ -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 } }); diff --git a/src/font/shaper/coretext.zig b/src/font/shaper/coretext.zig index ec64fe6eb..f2ac5b85d 100644 --- a/src/font/shaper/coretext.zig +++ b/src/font/shaper/coretext.zig @@ -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(); diff --git a/src/font/shaper/harfbuzz.zig b/src/font/shaper/harfbuzz.zig index b284dc140..eb8130f79 100644 --- a/src/font/shaper/harfbuzz.zig +++ b/src/font/shaper/harfbuzz.zig @@ -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();