From 619d2ade3ee194551889fe6cb1ea2d28aada8cba Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 13 Aug 2023 08:01:33 -0700 Subject: [PATCH] only initialize font discovery mechanism once, cache on App Fontconfig in particular appears unsafe to initialize multiple times. Font discovery is a singleton object in an application and only ever accessed from the main thread so we can work around this by only initializing and caching the font discovery mechanism exactly once on the app singleton. --- src/App.zig | 21 +++++++++++++++++++++ src/Surface.zig | 31 +++++++++++++++++-------------- src/apprt/embedded.zig | 2 +- src/apprt/glfw.zig | 2 +- src/apprt/gtk.zig | 12 +++++++++++- src/font/discovery.zig | 2 +- 6 files changed, 52 insertions(+), 18 deletions(-) diff --git a/src/App.zig b/src/App.zig index 42c4183fe..32f890699 100644 --- a/src/App.zig +++ b/src/App.zig @@ -45,6 +45,11 @@ quit: bool, /// from source. This is null if we can't detect it. resources_dir: ?[]const u8 = null, +/// Font discovery mechanism. This is only safe to use from the main thread. +/// This is lazily initialized on the first call to fontDiscover so do +/// not access this directly. +font_discover: ?font.Discover = null, + /// Initialize the main app instance. This creates the main window, sets /// up the renderer state, compiles the shaders, etc. This is the primary /// "startup" logic. @@ -80,6 +85,8 @@ pub fn destroy(self: *App) void { self.surfaces.deinit(self.alloc); if (self.resources_dir) |dir| self.alloc.free(dir); + if (self.font_discover) |*v| v.deinit(); + self.alloc.destroy(self); } @@ -151,6 +158,20 @@ pub fn focusedSurface(self: *const App) ?*Surface { return surface; } +/// Initialize once and return the font discovery mechanism. This remains +/// initialized throughout the lifetime of the application because some +/// font discovery mechanisms (i.e. fontconfig) are unsafe to reinit. +pub fn fontDiscover(self: *App) !?font.Discover { + // If we're built without a font discovery mechanism, return null + if (comptime font.Discover == void) return null; + + // If we initialized, use it + if (self.font_discover) |v| return v; + + self.font_discover = font.Discover.init(); + return self.font_discover.?; +} + /// Drain the mailbox. fn drainMailbox(self: *App, rt_app: *apprt.App) !void { while (self.mailbox.pop()) |message| { diff --git a/src/Surface.zig b/src/Surface.zig index 56bab2ec5..c79facf22 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -177,8 +177,8 @@ pub fn init( self: *Surface, alloc: Allocator, config: *const configpkg.Config, + app: *App, app_mailbox: App.Mailbox, - app_resources_dir: ?[]const u8, rt_surface: *apprt.runtime.Surface, ) !void { // Initialize our renderer with our initialized surface. @@ -217,8 +217,11 @@ pub fn init( errdefer group.deinit(); // Search for fonts - if (font.Discover != void) { - var disco = font.Discover.init(); + if (font.Discover != void) discover: { + const disco = try app.fontDiscover() orelse { + log.warn("font discovery not available, cannot search for fonts", .{}); + break :discover; + }; group.discover = disco; if (config.@"font-family") |family| { @@ -320,16 +323,16 @@ pub fn init( // If we're on Mac, then we try to use the Apple Emoji font for Emoji. if (builtin.os.tag == .macos and font.Discover != void) { - var disco = font.Discover.init(); - defer disco.deinit(); - var disco_it = try disco.discover(.{ - .family = "Apple Color Emoji", - .size = font_size.points, - }); - defer disco_it.deinit(); - if (try disco_it.next()) |face| { - log.info("font emoji: {s}", .{try face.name()}); - try group.addFace(alloc, .regular, face); + if (try app.fontDiscover()) |disco| { + var disco_it = try disco.discover(.{ + .family = "Apple Color Emoji", + .size = font_size.points, + }); + defer disco_it.deinit(); + if (try disco_it.next()) |face| { + log.info("font emoji: {s}", .{try face.name()}); + try group.addFace(alloc, .regular, face); + } } } @@ -402,7 +405,7 @@ pub fn init( .screen_size = screen_size, .full_config = config, .config = try termio.Impl.DerivedConfig.init(alloc, config), - .resources_dir = app_resources_dir, + .resources_dir = app.resources_dir, .renderer_state = &self.renderer_state, .renderer_wakeup = render_thread.wakeup, .renderer_mailbox = render_thread.mailbox, diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index 892733d08..de257cdd4 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -196,8 +196,8 @@ pub const Surface = struct { try self.core_surface.init( app.core_app.alloc, &config, + app.core_app, .{ .rt_app = app, .mailbox = &app.core_app.mailbox }, - app.core_app.resources_dir, self, ); errdefer self.core_surface.deinit(); diff --git a/src/apprt/glfw.zig b/src/apprt/glfw.zig index a5cfcc12a..9f222b2c3 100644 --- a/src/apprt/glfw.zig +++ b/src/apprt/glfw.zig @@ -374,8 +374,8 @@ pub const Surface = struct { try self.core_surface.init( app.app.alloc, &config, + app.app, .{ .rt_app = app, .mailbox = &app.app.mailbox }, - app.app.resources_dir, self, ); errdefer self.core_surface.deinit(); diff --git a/src/apprt/gtk.zig b/src/apprt/gtk.zig index 90782765a..64ca8481e 100644 --- a/src/apprt/gtk.zig +++ b/src/apprt/gtk.zig @@ -789,8 +789,8 @@ pub const Surface = struct { try self.core_surface.init( self.app.core_app.alloc, &config, + self.app.core_app, .{ .rt_app = self.app, .mailbox = &self.app.core_app.mailbox }, - self.app.core_app.resources_dir, self, ); errdefer self.core_surface.deinit(); @@ -1203,6 +1203,14 @@ pub const Surface = struct { break :key input.Key.fromASCII(self.im_buf[0]) orelse physical_key; } else .invalid; + // log.debug("key pressed key={} physical_key={} composing={} text_len={} mods={}", .{ + // key, + // physical_key, + // self.im_composing, + // self.im_len, + // mods, + // }); + // If both keys are invalid then we won't call the key callback. But // if either one is valid, we want to give it a chance. if (key != .invalid or physical_key != .invalid) { @@ -1342,6 +1350,8 @@ pub const Surface = struct { if (str.len <= self.im_buf.len) { @memcpy(self.im_buf[0..str.len], str); self.im_len = @intCast(str.len); + + // log.debug("input commit: {x}", .{self.im_buf[0]}); } else { log.warn("not enough buffer space for input method commit", .{}); } diff --git a/src/font/discovery.zig b/src/font/discovery.zig index 2ef7080d8..548c23af1 100644 --- a/src/font/discovery.zig +++ b/src/font/discovery.zig @@ -168,7 +168,7 @@ pub const Fontconfig = struct { /// Discover fonts from a descriptor. This returns an iterator that can /// be used to build up the deferred fonts. - pub fn discover(self: *Fontconfig, desc: Descriptor) !DiscoverIterator { + pub fn discover(self: *const Fontconfig, desc: Descriptor) !DiscoverIterator { // Build our pattern that we'll search for const pat = desc.toFcPattern(); errdefer pat.destroy();