diff options
| -rw-r--r-- | build.zig | 13 | ||||
| -rw-r--r-- | src/fonts.zig | 42 |
2 files changed, 49 insertions, 6 deletions
@@ -1091,6 +1091,19 @@ pub fn build(b: *std.Build) void { .link_libc = true, }) }); unit_step.dependOn(&b.addRunArtifact(nested_test).step); + // fonts.zig is the same shape once more, and it needs its own module + // for the reason spelled out above rather than as a convention: the + // core imports it behind `platform == .gui or .macos`, so on this build + // nothing analyses it and its tests would silently not exist. That is + // how a picker capped at 512 faces on a machine with a thousand of them + // went unnoticed. + const fonts_test = b.addTest(.{ .root_module = b.createModule(.{ + .target = target, + .optimize = optimize, + .root_source_file = b.path("src/fonts.zig"), + .link_libc = true, + }) }); + unit_step.dependOn(&b.addRunArtifact(fonts_test).step); // crt.zig is pure std — the mouse-mapping-vs-shader-formula test const crt_test = b.addTest(.{ .root_module = b.createModule(.{ .target = target, diff --git a/src/fonts.zig b/src/fonts.zig index 28789e3d..e4744daf 100644 --- a/src/fonts.zig +++ b/src/fonts.zig @@ -39,12 +39,20 @@ const home_dirs = switch (builtin.os.tag) { else => [_][]const u8{ ".local/share/fonts", ".fonts" }, }; -/// The same three safety rails look.find has, for the same reason: this walk -/// runs INSIDE the keystroke that asked for it, so it must end whatever it is +/// The same safety rails look.find has, for the same reason: this walk runs +/// INSIDE the keystroke that asked for it, so it must end whatever it is /// pointed at. A font tree is shallow and wide (one directory per family), so -/// the depth cap is lower than find's and the file cap is what a picker can -/// still be read as a list. -const max_fonts = 512; +/// the depth cap is lower than find's. +/// +/// `max_fonts` is a MEMORY bound and nothing else — the work is already bounded +/// by `max_steps`, since a face has to be walked past before it can be found. +/// It used to be 512, which is not a machine-sized number: a desktop with the +/// Noto and Nerd Font sets installed has nearly a thousand monospace faces, and +/// the picker silently served the first 512 the directories happened to hand +/// back. Sorting happens AFTER the cut, so the loss did not even look like +/// truncation — the list ran A to z with half the fonts missing out of the +/// middle of it, which is a far worse way to be wrong than a short list. +const max_fonts = 4096; const max_steps = 20_000; const max_depth = 8; @@ -96,7 +104,12 @@ pub fn fallbacks(arena: std.mem.Allocator) []const Font { /// chain. `wanted == null` means every monospace face; named scans accept /// proportional symbol/general faces and preserve the requested priority. fn scan(arena: std.mem.Allocator, wanted: ?[]const []const u8) []const Font { - var found: [max_fonts]Font = undefined; + // ...from the arena, not the stack: 4096 faces of {name, path} is 128 KiB, + // which is a fine thing to hand back to an arena that resets at the end of + // the keystroke and not a thing to put on a call stack. An allocator that + // cannot spare it answers with the embedded face, like every other failure + // in here. + const found = arena.alloc(Font, max_fonts) catch return &.{}; var found_len: usize = 0; // Zig 0.16 moved the filesystem behind std.Io; the blocking // single-threaded implementation is the synchronous walk a sans-IO core @@ -210,6 +223,23 @@ test "fallback discovery is unique and preserves candidate priority" { } } +test "the picker lists every monospace face rather than the first max_fonts" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const installed = list(arena_state.allocator(), null); + // A canary, and only meaningful on a machine that HAS fonts: reaching the + // cap means the list handed to the picker is a silent lie, sorted after + // the cut so it reads as complete while missing faces out of the middle. + // That shipped once at max_fonts = 512 on a desktop with ~1000 of them. + try std.testing.expect(installed.len < max_fonts); + // ...and what it does return is sorted, which is what makes the picker the + // same list twice running. + for (installed, 0..) |font, i| { + try std.testing.expect(font.name.len != 0); + if (i > 0) try std.testing.expect(!std.mem.lessThan(u8, font.name, installed[i - 1].name)); + } +} + /// Is every glyph in this font the same width? The terminal grid IS a /// monospace cell — one advance for every column, chosen once from 'M' — so a /// proportional font does not render badly in it, it renders as rubble: every |