font: clean up Collection api somewhat

Move size adjustment logic out of `Entry`, I understand the impulse to
put it there but it results in passing a lot of stuff around which isn't
great.

Rework `add(...)` in to `add(...)` and `addDeferred(...)`, faces are
passed directly now instead of passing an entry, and an options struct
is used instead of positional arguments for things like style, fallback,
and size adjustment.

Change size adjustment test back to a half pixel tolerance instead of 5%
because the previous commit (allowing fractional pixel sizes) fixed the
root cause of large differences.

src/font/CodepointResolver.zig

@@ -190,7 +190,10 @@ pub fn getIndex(
                 // Discovery is supposed to only return faces that have our
                 // codepoint but we can't search presentation in discovery so
                 // we have to check it here.
-                const face: Collection.Entry = .init(.{ .fallback_deferred = deferred_face });
+                const face: Collection.Entry = .{
+                    .face = .{ .deferred = deferred_face },
+                    .fallback = true,
+                };
                 if (!face.hasCodepoint(cp, p_mode)) {
                     deferred_face.deinit();
                     continue;
@@ -201,7 +204,11 @@ pub fn getIndex(
                     cp,
                     deferred_face.name(&buf) catch "<error>",
                 });
-                return self.collection.add(alloc, style, face) catch {
+                return self.collection.addDeferred(alloc, deferred_face, .{
+                    .style = style,
+                    .fallback = true,
+                    .size_adjustment = font.default_fallback_adjustment,
+                }) catch {
                     deferred_face.deinit();
                     break :discover;
                 };
@@ -263,11 +270,11 @@ fn getIndexCodepointOverride(
 
         // Add the font to our list of fonts so we can get an index for it,
         // and ensure the index is stored in the descriptor cache for next time.
-        const idx = try self.collection.add(
+        const idx = try self.collection.addDeferred(alloc, face, .{
-            alloc,
+            .style = .regular,
-            .regular,
+            .fallback = false,
-            .init(.{ .deferred = face }),
+            .size_adjustment = font.default_fallback_adjustment,
-        );
+        });
         try self.descriptor_cache.put(alloc, desc, idx);
 
         break :idx idx;
@@ -388,32 +395,36 @@ test getIndex {
 
     {
         errdefer c.deinit(alloc);
-        _ = try c.add(alloc, .regular, .init(.{ .loaded = try .init(
+        _ = try c.add(alloc, try .init(
             lib,
             testFont,
             .{ .size = .{ .points = 12, .xdpi = 96, .ydpi = 96 } },
-        ) }));
+        ), .{
+            .style = .regular,
+            .fallback = false,
+            .size_adjustment = .none,
+        });
         if (comptime !font.options.backend.hasCoretext()) {
             // Coretext doesn't support Noto's format
-            _ = try c.add(
+            _ = try c.add(alloc, try .init(
-                alloc,
-                .regular,
-                .init(.{ .loaded = try .init(
                 lib,
                 testEmoji,
                 .{ .size = .{ .points = 12 } },
-                ) }),
+            ), .{
-            );
+                .style = .regular,
+                .fallback = false,
+                .size_adjustment = .none,
+            });
         }
-        _ = try c.add(
+        _ = try c.add(alloc, try .init(
-            alloc,
-            .regular,
-            .init(.{ .loaded = try .init(
             lib,
             testEmojiText,
             .{ .size = .{ .points = 12 } },
-            ) }),
+        ), .{
-        );
+            .style = .regular,
+            .fallback = false,
+            .size_adjustment = .none,
+        });
     }
 
     var r: CodepointResolver = .{ .collection = c };
@@ -467,21 +478,33 @@ test "getIndex disabled font style" {
     var c = Collection.init();
     c.load_options = .{ .library = lib };
 
-    _ = try c.add(alloc, .regular, .init(.{ .loaded = try .init(
+    _ = try c.add(alloc, try .init(
         lib,
         testFont,
         .{ .size = .{ .points = 12, .xdpi = 96, .ydpi = 96 } },
-    ) }));
+    ), .{
-    _ = try c.add(alloc, .bold, .init(.{ .loaded = try .init(
+        .style = .regular,
+        .fallback = false,
+        .size_adjustment = .none,
+    });
+    _ = try c.add(alloc, try .init(
         lib,
         testFont,
         .{ .size = .{ .points = 12, .xdpi = 96, .ydpi = 96 } },
-    ) }));
+    ), .{
-    _ = try c.add(alloc, .italic, .init(.{ .loaded = try .init(
+        .style = .bold,
+        .fallback = false,
+        .size_adjustment = .none,
+    });
+    _ = try c.add(alloc, try .init(
         lib,
         testFont,
         .{ .size = .{ .points = 12, .xdpi = 96, .ydpi = 96 } },
-    ) }));
+    ), .{
+        .style = .italic,
+        .fallback = false,
+        .size_adjustment = .none,
+    });
 
     var r: CodepointResolver = .{ .collection = c };
     defer r.deinit(alloc);
@@ -522,6 +545,7 @@ test "getIndex box glyph" {
         .collection = c,
         .sprite = .{
             .metrics = font.Metrics.calc(.{
+                .px_per_em = 30.0,
                 .cell_width = 18.0,
                 .ascent = 30.0,
                 .descent = -6.0,

src/font/Metrics.zig

@@ -55,6 +55,11 @@ const Minimums = struct {
 /// Metrics extracted from a font face, based on
 /// the metadata tables and glyph measurements.
 pub const FaceMetrics = struct {
+    /// Pixels per em, dividing the other values in this struct by this should
+    /// yield sizes in ems, to allow comparing metrics from faces of different
+    /// sizes.
+    px_per_em: f64,
+
     /// The minimum cell width that can contain any glyph in the ASCII range.
     ///
     /// Determined by measuring all printable glyphs in the ASCII range.

src/font/SharedGrid.zig

@@ -376,11 +376,15 @@ fn testGrid(mode: TestMode, alloc: Allocator, lib: Library) !SharedGrid {
 
     switch (mode) {
         .normal => {
-            _ = try c.add(alloc, .regular, .init(.{ .loaded = try .init(
+            _ = try c.add(alloc, try .init(
                 lib,
                 testFont,
                 .{ .size = .{ .points = 12, .xdpi = 96, .ydpi = 96 } },
-            ) }));
+            ), .{
+                .style = .regular,
+                .fallback = false,
+                .size_adjustment = .none,
+            });
         },
     }
 

src/font/SharedGridSet.zig

@@ -200,11 +200,12 @@ fn collection(
                             try face.name(&name_buf),
                         });
 
-                        _ = try c.add(
+                        _ = try c.addDeferred(self.alloc, face, .{
-                            self.alloc,
+                            .style = style,
-                            style,
+                            .fallback = false,
-                            .init(.{ .deferred = face }),
+                            // No size adjustment for primary fonts.
-                        );
+                            .size_adjustment = .none,
+                        });
 
                         continue;
                     }
@@ -230,11 +231,12 @@ fn collection(
                             try face.name(&name_buf),
                         });
 
-                        _ = try c.add(
+                        _ = try c.addDeferred(self.alloc, face, .{
-                            self.alloc,
+                            .style = style,
-                            style,
+                            .fallback = false,
-                            .init(.{ .deferred = face }),
+                            // No size adjustment for primary fonts.
-                        );
+                            .size_adjustment = .none,
+                        });
 
                         continue;
                     }
@@ -257,59 +259,77 @@ fn collection(
     // Our built-in font will be used as a backup
     _ = try c.add(
         self.alloc,
-        .regular,
+        try .init(
-        .init(.{ .fallback_loaded = try .init(
             self.font_lib,
             font.embedded.variable,
             load_options.faceOptions(),
-        ) }),
+        ),
+        .{
+            .style = .regular,
+            .fallback = true,
+            .size_adjustment = font.default_fallback_adjustment,
+        },
     );
     try (try c.getFace(try c.add(
         self.alloc,
-        .bold,
+        try .init(
-        .init(.{ .fallback_loaded = try .init(
             self.font_lib,
             font.embedded.variable,
             load_options.faceOptions(),
-        ) }),
+        ),
+        .{
+            .style = .bold,
+            .fallback = true,
+            .size_adjustment = font.default_fallback_adjustment,
+        },
     ))).setVariations(
         &.{.{ .id = .init("wght"), .value = 700 }},
         load_options.faceOptions(),
     );
     _ = try c.add(
         self.alloc,
-        .italic,
+        try .init(
-        .init(.{ .fallback_loaded = try .init(
             self.font_lib,
             font.embedded.variable_italic,
             load_options.faceOptions(),
-        ) }),
+        ),
+        .{
+            .style = .italic,
+            .fallback = true,
+            .size_adjustment = font.default_fallback_adjustment,
+        },
     );
     try (try c.getFace(try c.add(
         self.alloc,
-        .bold_italic,
+        try .init(
-        .init(.{ .fallback_loaded = try .init(
             self.font_lib,
             font.embedded.variable_italic,
             load_options.faceOptions(),
-        ) }),
+        ),
+        .{
+            .style = .bold_italic,
+            .fallback = true,
+            .size_adjustment = font.default_fallback_adjustment,
+        },
     ))).setVariations(
         &.{.{ .id = .init("wght"), .value = 700 }},
         load_options.faceOptions(),
     );
 
     // Nerd-font symbols fallback.
-    // For proper icon scaling, this should be loaded at the same point
-    // size as the primary font and not undergo size normalization,
-    // hence we use the em size as scale reference.
     _ = try c.add(
         self.alloc,
-        .regular,
+        try .init(
-        .initWithScaleReference(.{ .fallback_loaded = try .init(
             self.font_lib,
             font.embedded.symbols_nerd_font,
             load_options.faceOptions(),
-        ) }, .none),
+        ),
+        .{
+            .style = .regular,
+            .fallback = true,
+            // No size adjustment for the symbols font.
+            .size_adjustment = .none,
+        },
     );
 
     // On macOS, always search for and add the Apple Emoji font
@@ -324,11 +344,12 @@ fn collection(
         });
         defer disco_it.deinit();
         if (try disco_it.next()) |face| {
-            _ = try c.add(
+            _ = try c.addDeferred(self.alloc, face, .{
-                self.alloc,
+                .style = .regular,
-                .regular,
+                .fallback = true,
-                .init(.{ .fallback_deferred = face }),
+                // No size adjustment for emojis.
-            );
+                .size_adjustment = .none,
+            });
         }
     }
 
@@ -337,21 +358,31 @@ fn collection(
     if (comptime !builtin.target.os.tag.isDarwin() or Discover == void) {
         _ = try c.add(
             self.alloc,
-            .regular,
+            try .init(
-            .init(.{ .fallback_loaded = try .init(
                 self.font_lib,
                 font.embedded.emoji,
                 load_options.faceOptions(),
-            ) }),
+            ),
+            .{
+                .style = .regular,
+                .fallback = true,
+                // No size adjustment for emojis.
+                .size_adjustment = .none,
+            },
         );
         _ = try c.add(
             self.alloc,
-            .regular,
+            try .init(
-            .init(.{ .fallback_loaded = try .init(
                 self.font_lib,
                 font.embedded.emoji_text,
                 load_options.faceOptions(),
-            ) }),
+            ),
+            .{
+                .style = .regular,
+                .fallback = true,
+                // No size adjustment for emojis.
+                .size_adjustment = .none,
+            },
         );
     }
 

src/font/face/coretext.zig

@@ -241,10 +241,14 @@ pub const Face = struct {
             desc = next;
         }
 
+        // Put our current size in the opts so that we don't change size.
+        var new_opts = opts;
+        new_opts.size = self.size;
+
         // Initialize a font based on these attributes.
         const ct_font = try self.font.copyWithAttributes(0, null, desc);
         errdefer ct_font.release();
-        const face = try initFont(ct_font, opts);
+        const face = try initFont(ct_font, new_opts);
         self.deinit();
         self.* = face;
     }
@@ -843,14 +847,20 @@ pub const Face = struct {
         };
 
         return .{
+            .px_per_em = px_per_em,
+
             .cell_width = cell_width,
+
             .ascent = ascent,
             .descent = descent,
             .line_gap = line_gap,
+
             .underline_position = underline_position,
             .underline_thickness = underline_thickness,
+
             .strikethrough_position = strikethrough_position,
             .strikethrough_thickness = strikethrough_thickness,
+
             .cap_height = cap_height,
             .ex_height = ex_height,
             .ic_width = ic_width,

src/font/face/freetype.zig

@@ -933,6 +933,8 @@ pub const Face = struct {
         };
 
         return .{
+            .px_per_em = px_per_em,
+
             .cell_width = cell_width,
 
             .ascent = ascent,

src/font/main.zig

@@ -174,6 +174,11 @@ pub const Presentation = enum(u1) {
 /// A FontIndex that can be used to use the sprite font directly.
 pub const sprite_index = Collection.Index.initSpecial(.sprite);
 
+/// The default font size adjustment we use when loading fallback fonts.
+///
+/// TODO: Add user configuration for this instead of hard-coding it.
+pub const default_fallback_adjustment: Collection.SizeAdjustment = .ic_width;
+
 test {
     // For non-wasm we want to test everything we can
     if (!comptime builtin.target.cpu.arch.isWasm()) {

src/font/shaper/coretext.zig

@@ -1779,19 +1779,27 @@ fn testShaperWithFont(alloc: Allocator, font_req: TestFont) !TestShaper {
     c.load_options = .{ .library = lib };
 
     // Setup group
-    _ = try c.add(alloc, .regular, .init(.{ .loaded = try .init(
+    _ = try c.add(alloc, try .init(
         lib,
         testFont,
         .{ .size = .{ .points = 12 } },
-    ) }));
+    ), .{
+        .style = .regular,
+        .fallback = false,
+        .size_adjustment = .none,
+    });
 
     if (font.options.backend != .coretext) {
         // Coretext doesn't support Noto's format
-        _ = try c.add(alloc, .regular, .init(.{ .loaded = try .init(
+        _ = try c.add(alloc, try .init(
             lib,
             testEmoji,
             .{ .size = .{ .points = 12 } },
-        ) }));
+        ), .{
+            .style = .regular,
+            .fallback = false,
+            .size_adjustment = .none,
+        });
     } else {
         // On CoreText we want to load Apple Emoji, we should have it.
         var disco = font.Discover.init();
@@ -1804,13 +1812,21 @@ fn testShaperWithFont(alloc: Allocator, font_req: TestFont) !TestShaper {
         defer disco_it.deinit();
         var face = (try disco_it.next()).?;
         errdefer face.deinit();
-        _ = try c.add(alloc, .regular, .init(.{ .deferred = face }));
+        _ = try c.addDeferred(alloc, face, .{
+            .style = .regular,
+            .fallback = false,
+            .size_adjustment = .none,
+        });
     }
-    _ = try c.add(alloc, .regular, .init(.{ .loaded = try .init(
+    _ = try c.add(alloc, try .init(
         lib,
         testEmojiText,
         .{ .size = .{ .points = 12 } },
-    ) }));
+    ), .{
+        .style = .regular,
+        .fallback = false,
+        .size_adjustment = .none,
+    });
 
     const grid_ptr = try alloc.create(SharedGrid);
     errdefer alloc.destroy(grid_ptr);

src/font/shaper/harfbuzz.zig

@@ -1242,19 +1242,27 @@ fn testShaperWithFont(alloc: Allocator, font_req: TestFont) !TestShaper {
     c.load_options = .{ .library = lib };
 
     // Setup group
-    _ = try c.add(alloc, .regular, .init(.{ .loaded = try .init(
+    _ = try c.add(alloc, try .init(
         lib,
         testFont,
         .{ .size = .{ .points = 12 } },
-    ) }));
+    ), .{
+        .style = .regular,
+        .fallback = false,
+        .size_adjustment = .none,
+    });
 
     if (comptime !font.options.backend.hasCoretext()) {
         // Coretext doesn't support Noto's format
-        _ = try c.add(alloc, .regular, .init(.{ .loaded = try .init(
+        _ = try c.add(alloc, try .init(
             lib,
             testEmoji,
             .{ .size = .{ .points = 12 } },
-        ) }));
+        ), .{
+            .style = .regular,
+            .fallback = false,
+            .size_adjustment = .none,
+        });
     } else {
         // On CoreText we want to load Apple Emoji, we should have it.
         var disco = font.Discover.init();
@@ -1267,13 +1275,21 @@ fn testShaperWithFont(alloc: Allocator, font_req: TestFont) !TestShaper {
         defer disco_it.deinit();
         var face = (try disco_it.next()).?;
         errdefer face.deinit();
-        _ = try c.add(alloc, .regular, .init(.{ .deferred = face }));
+        _ = try c.addDeferred(alloc, face, .{
+            .style = .regular,
+            .fallback = false,
+            .size_adjustment = .none,
+        });
     }
-    _ = try c.add(alloc, .regular, .init(.{ .loaded = try .init(
+    _ = try c.add(alloc, try .init(
         lib,
         testEmojiText,
         .{ .size = .{ .points = 12 } },
-    ) }));
+    ), .{
+        .style = .regular,
+        .fallback = false,
+        .size_adjustment = .none,
+    });
 
     const grid_ptr = try alloc.create(SharedGrid);
     errdefer alloc.destroy(grid_ptr);

src/font/sprite/Face.zig

@@ -389,6 +389,9 @@ fn testDrawRanges(
     const alloc = testing.allocator;
 
     const metrics: font.Metrics = .calc(.{
+        // Fudged number, not used in anything we care about here.
+        .px_per_em = 16,
+
         .cell_width = @floatFromInt(width),
         .ascent = @floatFromInt(ascent),
         .descent = -@as(f64, @floatFromInt(descent)),