From 167bf6f0980c312fdff0d4eedb6ad3ecdfa39d26 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 25 Aug 2023 13:28:46 -0700 Subject: [PATCH] font: DeferredFace can no longer represent a loaded face --- src/Surface.zig | 18 +++---- src/font/DeferredFace.zig | 102 ++++++----------------------------- src/font/Group.zig | 48 +++++++++-------- src/font/GroupCache.zig | 4 +- src/font/discovery.zig | 11 +--- src/font/shaper/harfbuzz.zig | 7 +-- 6 files changed, 58 insertions(+), 132 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index 7f5582292..066e5464a 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -230,7 +230,7 @@ pub fn init( defer disco_it.deinit(); if (try disco_it.next()) |face| { log.info("font regular: {s}", .{try face.name()}); - try group.addFace(.regular, face); + try group.addFace(.regular, .{ .deferred = face }); } else log.warn("font-family not found: {s}", .{family}); } if (config.@"font-family-bold") |family| { @@ -242,7 +242,7 @@ pub fn init( defer disco_it.deinit(); if (try disco_it.next()) |face| { log.info("font bold: {s}", .{try face.name()}); - try group.addFace(.bold, face); + try group.addFace(.bold, .{ .deferred = face }); } else log.warn("font-family-bold not found: {s}", .{family}); } if (config.@"font-family-italic") |family| { @@ -254,7 +254,7 @@ pub fn init( defer disco_it.deinit(); if (try disco_it.next()) |face| { log.info("font italic: {s}", .{try face.name()}); - try group.addFace(.italic, face); + try group.addFace(.italic, .{ .deferred = face }); } else log.warn("font-family-italic not found: {s}", .{family}); } if (config.@"font-family-bold-italic") |family| { @@ -267,7 +267,7 @@ pub fn init( defer disco_it.deinit(); if (try disco_it.next()) |face| { log.info("font bold+italic: {s}", .{try face.name()}); - try group.addFace(.bold_italic, face); + try group.addFace(.bold_italic, .{ .deferred = face }); } else log.warn("font-family-bold-italic not found: {s}", .{family}); } } @@ -275,11 +275,11 @@ pub fn init( // Our built-in font will be used as a backup try group.addFace( .regular, - font.DeferredFace.initLoaded(try font.Face.init(font_lib, face_ttf, font_size)), + .{ .loaded = try font.Face.init(font_lib, face_ttf, font_size) }, ); try group.addFace( .bold, - font.DeferredFace.initLoaded(try font.Face.init(font_lib, face_bold_ttf, font_size)), + .{ .loaded = try font.Face.init(font_lib, face_bold_ttf, font_size) }, ); // Auto-italicize if we have to. @@ -290,11 +290,11 @@ pub fn init( if (builtin.os.tag != .macos or font.Discover == void) { try group.addFace( .regular, - font.DeferredFace.initLoaded(try font.Face.init(font_lib, face_emoji_ttf, font_size)), + .{ .loaded = try font.Face.init(font_lib, face_emoji_ttf, font_size) }, ); try group.addFace( .regular, - font.DeferredFace.initLoaded(try font.Face.init(font_lib, face_emoji_text_ttf, font_size)), + .{ .loaded = try font.Face.init(font_lib, face_emoji_text_ttf, font_size) }, ); } @@ -308,7 +308,7 @@ pub fn init( defer disco_it.deinit(); if (try disco_it.next()) |face| { log.info("font emoji: {s}", .{try face.name()}); - try group.addFace(.regular, face); + try group.addFace(.regular, .{ .deferred = face }); } } } diff --git a/src/font/DeferredFace.zig b/src/font/DeferredFace.zig index 4ebfe5ad7..27e0c040b 100644 --- a/src/font/DeferredFace.zig +++ b/src/font/DeferredFace.zig @@ -30,9 +30,6 @@ const FaceState = switch (options.backend) { .web_canvas => WebCanvas, }; -/// The loaded face (once loaded). -face: ?Face = null, - /// Fontconfig fc: if (options.backend == .fontconfig_freetype) ?Fontconfig else void = if (options.backend == .fontconfig_freetype) null else {}, @@ -91,14 +88,7 @@ pub const WebCanvas = struct { } }; -/// Initialize a deferred face that is already pre-loaded. The deferred face -/// takes ownership over the loaded face, deinit will deinit the loaded face. -pub fn initLoaded(face: Face) DeferredFace { - return .{ .face = face }; -} - pub fn deinit(self: *DeferredFace) void { - if (self.face) |*face| face.deinit(); switch (options.backend) { .fontconfig_freetype => if (self.fc) |*fc| fc.deinit(), .coretext, .coretext_freetype => if (self.ct) |*ct| ct.deinit(), @@ -108,11 +98,6 @@ pub fn deinit(self: *DeferredFace) void { self.* = undefined; } -/// Returns true if the face has been loaded. -pub inline fn loaded(self: DeferredFace) bool { - return self.face != null; -} - /// Returns the name of this face. The memory is always owned by the /// face so it doesn't have to be freed. pub fn name(self: DeferredFace) ![:0]const u8 { @@ -152,69 +137,48 @@ pub fn load( self: *DeferredFace, lib: Library, size: font.face.DesiredSize, -) !void { - // No-op if we already loaded - if (self.face != null) return; - - switch (options.backend) { - .fontconfig_freetype => { - try self.loadFontconfig(lib, size); - return; - }, - - .coretext => { - try self.loadCoreText(lib, size); - return; - }, - - .coretext_freetype => { - try self.loadCoreTextFreetype(lib, size); - return; - }, - - .web_canvas => { - try self.loadWebCanvas(size); - return; - }, +) !Face { + return switch (options.backend) { + .fontconfig_freetype => try self.loadFontconfig(lib, size), + .coretext => try self.loadCoreText(lib, size), + .coretext_freetype => try self.loadCoreTextFreetype(lib, size), + .web_canvas => try self.loadWebCanvas(size), // Unreachable because we must be already loaded or have the // proper configuration for one of the other deferred mechanisms. .freetype => unreachable, - } + }; } fn loadFontconfig( self: *DeferredFace, lib: Library, size: font.face.DesiredSize, -) !void { - assert(self.face == null); +) !Face { const fc = self.fc.?; // Filename and index for our face so we can load it const filename = (try fc.pattern.get(.file, 0)).string; const face_index = (try fc.pattern.get(.index, 0)).integer; - self.face = try Face.initFile(lib, filename, face_index, size); + return try Face.initFile(lib, filename, face_index, size); } fn loadCoreText( self: *DeferredFace, lib: Library, size: font.face.DesiredSize, -) !void { +) !Face { _ = lib; - assert(self.face == null); const ct = self.ct.?; - self.face = try Face.initFontCopy(ct.font, size); + return try Face.initFontCopy(ct.font, size); } fn loadCoreTextFreetype( self: *DeferredFace, lib: Library, size: font.face.DesiredSize, -) !void { - assert(self.face == null); +) !Face { const ct = self.ct.?; // Get the URL for the font so we can get the filepath @@ -246,16 +210,15 @@ fn loadCoreTextFreetype( // TODO: face index 0 is not correct long term and we should switch // to using CoreText for rendering, too. //std.log.warn("path={s}", .{path_slice}); - self.face = try Face.initFile(lib, buf[0..path_slice.len :0], 0, size); + return try Face.initFile(lib, buf[0..path_slice.len :0], 0, size); } fn loadWebCanvas( self: *DeferredFace, size: font.face.DesiredSize, -) !void { - assert(self.face == null); +) !Face { const wc = self.wc.?; - self.face = try Face.initNamed(wc.alloc, wc.font_str, size, wc.presentation); + return try Face.initNamed(wc.alloc, wc.font_str, size, wc.presentation); } /// Returns true if this face can satisfy the given codepoint and @@ -266,12 +229,6 @@ fn loadWebCanvas( /// discovery mechanism (i.e. fontconfig). If no discovery is used, /// the face is always expected to be loaded. pub fn hasCodepoint(self: DeferredFace, cp: u32, p: ?Presentation) bool { - // If we have the face, use the face. - if (self.face) |face| { - if (p) |desired| if (face.presentation != desired) return false; - return face.glyphIndex(cp) != null; - } - switch (options.backend) { .fontconfig_freetype => { // If we are using fontconfig, use the fontconfig metadata to @@ -388,30 +345,8 @@ pub const Wasm = struct { return; }; } - - /// Caller should not free this, the face is owned by the deferred face. - export fn deferred_face_face(self: *DeferredFace) ?*Face { - assert(self.loaded()); - return &self.face.?; - } }; -test "preloaded" { - const testing = std.testing; - const testFont = @import("test.zig").fontRegular; - - var lib = try Library.init(); - defer lib.deinit(); - - var face = try Face.init(lib, testFont, .{ .points = 12 }); - errdefer face.deinit(); - - var def = initLoaded(face); - defer def.deinit(); - - try testing.expect(def.hasCodepoint(' ', null)); -} - test "fontconfig" { if (options.backend != .fontconfig_freetype) return error.SkipZigTest; @@ -430,7 +365,6 @@ test "fontconfig" { break :def (try it.next()).?; }; defer def.deinit(); - try testing.expect(!def.loaded()); // Verify we can get the name const n = try def.name(); @@ -460,7 +394,6 @@ test "coretext" { break :def (try it.next()).?; }; defer def.deinit(); - try testing.expect(!def.loaded()); try testing.expect(def.hasCodepoint(' ', null)); // Verify we can get the name @@ -468,7 +401,6 @@ test "coretext" { try testing.expect(n.len > 0); // Load it and verify it works - try def.load(lib, .{ .points = 12 }); - try testing.expect(def.hasCodepoint(' ', null)); - try testing.expect(def.face.?.glyphIndex(' ') != null); + const face = try def.load(lib, .{ .points = 12 }); + try testing.expect(face.glyphIndex(' ') != null); } diff --git a/src/font/Group.zig b/src/font/Group.zig index 298e77086..a80fc016e 100644 --- a/src/font/Group.zig +++ b/src/font/Group.zig @@ -31,19 +31,7 @@ const log = std.log.scoped(.font_group); // usually only one font group for the entire process so this isn't the // most important memory efficiency we can look for. This is totally opaque // to the user so we can change this later. -const StyleArray = std.EnumArray(Style, std.ArrayListUnmanaged(StyleElem)); -const StyleElem = union(enum) { - deferred: DeferredFace, // Not loaded - loaded: Face, // Loaded - - pub fn deinit(self: *StyleElem) void { - switch (self.*) { - .deferred => |*v| v.deinit(), - .loaded => |*v| v.deinit(), - } - } -}; - +const StyleArray = std.EnumArray(Style, std.ArrayListUnmanaged(GroupFace)); /// The allocator for this group alloc: Allocator, @@ -67,6 +55,19 @@ discover: ?*font.Discover = null, /// terminal rendering will look wrong. sprite: ?font.sprite.Face = null, +/// A face in a group can be deferred or loaded. +pub const GroupFace = union(enum) { + deferred: DeferredFace, // Not loaded + loaded: Face, // Loaded + + pub fn deinit(self: *GroupFace) void { + switch (self.*) { + .deferred => |*v| v.deinit(), + .loaded => |*v| v.deinit(), + } + } +}; + pub fn init( alloc: Allocator, lib: Library, @@ -98,13 +99,13 @@ pub fn deinit(self: *Group) void { /// /// The group takes ownership of the face. The face will be deallocated when /// the group is deallocated. -pub fn addFace(self: *Group, style: Style, face: DeferredFace) !void { +pub fn addFace(self: *Group, style: Style, face: GroupFace) !void { const list = self.faces.getPtr(style); // We have some special indexes so we must never pass those. if (list.items.len >= FontIndex.Special.start - 1) return error.GroupFull; - try list.append(self.alloc, .{ .deferred = face }); + try list.append(self.alloc, face); } /// Returns true if we have a face for the given style, though the face may @@ -262,7 +263,7 @@ pub fn indexForCodepoint( cp, face.name() catch "", }); - self.addFace(style, face) catch break :discover; + self.addFace(style, .{ .deferred = face }) catch break :discover; if (self.indexForCodepointExact(cp, style, p)) |value| return value; } } @@ -317,8 +318,9 @@ pub fn faceFromIndex(self: *Group, index: FontIndex) !*Face { const item = &list.items[@intCast(index.idx)]; return switch (item.*) { .deferred => |*d| deferred: { - try d.load(self.lib, self.size); - item.* = .{ .loaded = d.face.? }; + const face = try d.load(self.lib, self.size); + d.deinit(); + item.* = .{ .loaded = face }; break :deferred &item.loaded; }, @@ -483,13 +485,13 @@ test { var group = try init(alloc, lib, .{ .points = 12 }); defer group.deinit(); - try group.addFace(.regular, DeferredFace.initLoaded(try Face.init(lib, testFont, .{ .points = 12 }))); + try group.addFace(.regular, .{ .loaded = try Face.init(lib, testFont, .{ .points = 12 }) }); if (font.options.backend != .coretext) { // Coretext doesn't support Noto's format - try group.addFace(.regular, DeferredFace.initLoaded(try Face.init(lib, testEmoji, .{ .points = 12 }))); + try group.addFace(.regular, .{ .loaded = try Face.init(lib, testEmoji, .{ .points = 12 }) }); } - try group.addFace(.regular, DeferredFace.initLoaded(try Face.init(lib, testEmojiText, .{ .points = 12 }))); + try group.addFace(.regular, .{ .loaded = try Face.init(lib, testEmojiText, .{ .points = 12 }) }); // Should find all visible ASCII var i: u32 = 32; @@ -582,7 +584,7 @@ test "resize" { var group = try init(alloc, lib, .{ .points = 12, .xdpi = 96, .ydpi = 96 }); defer group.deinit(); - try group.addFace(.regular, DeferredFace.initLoaded(try Face.init(lib, testFont, .{ .points = 12, .xdpi = 96, .ydpi = 96 }))); + try group.addFace(.regular, .{ .loaded = try Face.init(lib, testFont, .{ .points = 12, .xdpi = 96, .ydpi = 96 }) }); // Load a letter { @@ -673,7 +675,7 @@ test "faceFromIndex returns pointer" { var group = try init(alloc, lib, .{ .points = 12, .xdpi = 96, .ydpi = 96 }); defer group.deinit(); - try group.addFace(.regular, DeferredFace.initLoaded(try Face.init(lib, testFont, .{ .points = 12, .xdpi = 96, .ydpi = 96 }))); + try group.addFace(.regular, .{ .loaded = try Face.init(lib, testFont, .{ .points = 12, .xdpi = 96, .ydpi = 96 }) }); { const idx = group.indexForCodepoint('A', .regular, null).?; diff --git a/src/font/GroupCache.zig b/src/font/GroupCache.zig index dc4d1df2c..b869e2b5f 100644 --- a/src/font/GroupCache.zig +++ b/src/font/GroupCache.zig @@ -184,7 +184,7 @@ test { // Setup group try cache.group.addFace( .regular, - DeferredFace.initLoaded(try Face.init(lib, testFont, .{ .points = 12 })), + .{ .loaded = try Face.init(lib, testFont, .{ .points = 12 }) }, ); var group = cache.group; @@ -340,7 +340,7 @@ test "resize" { // Setup group try cache.group.addFace( .regular, - DeferredFace.initLoaded(try Face.init(lib, testFont, .{ .points = 12, .xdpi = 96, .ydpi = 96 })), + .{ .loaded = try Face.init(lib, testFont, .{ .points = 12, .xdpi = 96, .ydpi = 96 }) }, ); // Load a letter diff --git a/src/font/discovery.zig b/src/font/discovery.zig index 548c23af1..f105b630c 100644 --- a/src/font/discovery.zig +++ b/src/font/discovery.zig @@ -301,7 +301,6 @@ pub const CoreText = struct { defer self.i += 1; return DeferredFace{ - .face = null, .ct = .{ .font = font }, }; } @@ -311,14 +310,9 @@ pub const CoreText = struct { test "fontconfig" { if (options.backend != .fontconfig_freetype) return error.SkipZigTest; - const testing = std.testing; - var fc = Fontconfig.init(); var it = try fc.discover(.{ .family = "monospace", .size = 12 }); defer it.deinit(); - while (try it.next()) |face| { - try testing.expect(!face.loaded()); - } } test "fontconfig codepoint" { @@ -333,7 +327,6 @@ test "fontconfig codepoint" { // The first result should have the codepoint. Later ones may not // because fontconfig returns all fonts sorted. const face = (try it.next()).?; - try testing.expect(!face.loaded()); try testing.expect(face.hasCodepoint('A', null)); // Should have other codepoints too @@ -351,9 +344,8 @@ test "coretext" { var it = try ct.discover(.{ .family = "Monaco", .size = 12 }); defer it.deinit(); var count: usize = 0; - while (try it.next()) |face| { + while (try it.next()) |_| { count += 1; - try testing.expect(!face.loaded()); } try testing.expect(count > 0); } @@ -372,7 +364,6 @@ test "coretext codepoint" { // The first result should have the codepoint. Later ones may not // because fontconfig returns all fonts sorted. const face = (try it.next()).?; - try testing.expect(!face.loaded()); try testing.expect(face.hasCodepoint('A', null)); // Should have other codepoints too diff --git a/src/font/shaper/harfbuzz.zig b/src/font/shaper/harfbuzz.zig index 590cbbe0c..fe41ae519 100644 --- a/src/font/shaper/harfbuzz.zig +++ b/src/font/shaper/harfbuzz.zig @@ -795,12 +795,13 @@ fn testShaper(alloc: Allocator) !TestShaper { errdefer cache_ptr.*.deinit(alloc); // Setup group - try cache_ptr.group.addFace(.regular, DeferredFace.initLoaded(try Face.init(lib, testFont, .{ .points = 12 }))); + try cache_ptr.group.addFace(.regular, .{ .loaded = try Face.init(lib, testFont, .{ .points = 12 }) }); + if (font.options.backend != .coretext) { // Coretext doesn't support Noto's format - try cache_ptr.group.addFace(.regular, DeferredFace.initLoaded(try Face.init(lib, testEmoji, .{ .points = 12 }))); + try cache_ptr.group.addFace(.regular, .{ .loaded = try Face.init(lib, testEmoji, .{ .points = 12 }) }); } - try cache_ptr.group.addFace(.regular, DeferredFace.initLoaded(try Face.init(lib, testEmojiText, .{ .points = 12 }))); + try cache_ptr.group.addFace(.regular, .{ .loaded = try Face.init(lib, testEmojiText, .{ .points = 12 }) }); var cell_buf = try alloc.alloc(font.shape.Cell, 80); errdefer alloc.free(cell_buf);