font: DeferredFace can no longer represent a loaded face

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

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 {
+) !Face {
-    // No-op if we already loaded
+    return switch (options.backend) {
-    if (self.face != null) return;
+        .fontconfig_freetype => try self.loadFontconfig(lib, size),
-
+        .coretext => try self.loadCoreText(lib, size),
-    switch (options.backend) {
+        .coretext_freetype => try self.loadCoreTextFreetype(lib, size),
-        .fontconfig_freetype => {
+        .web_canvas => try self.loadWebCanvas(size),
-            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;
-        },
 
         // 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 {
+) !Face {
-    assert(self.face == null);
     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 {
+) !Face {
-    assert(self.face == null);
     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 {
+) !Face {
-    assert(self.face == null);
     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 });
+    const face = try def.load(lib, .{ .points = 12 });
-    try testing.expect(def.hasCodepoint(' ', null));
+    try testing.expect(face.glyphIndex(' ') != null);
-    try testing.expect(def.face.?.glyphIndex(' ') != null);
 }

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 StyleArray = std.EnumArray(Style, std.ArrayListUnmanaged(GroupFace));
-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(),
-        }
-    }
-};
-
 /// 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 "<error>",
                 });
-                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);
+            const face = try d.load(self.lib, self.size);
-            item.* = .{ .loaded = d.face.? };
+            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).?;

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

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

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);