From 9f24730601d5f1e9bcf8984b43b7895ad8a6c945 Mon Sep 17 00:00:00 2001 From: Albert Hui Date: Thu, 20 Aug 2026 22:52:49 +0000 Subject: [PATCH] fix(lens): load every symbol face, and stop failing on a host's missing emoji font MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tests/fonts.rs` failed on a Linux host with `['🌐', '⚙']` uncovered. Two different problems wore one error message. ⚙ WAS A REAL BUG. `fallback_fonts()` took `first_readable(SYMBOL_FONTS)` -- one face -- but Linux spreads symbol coverage across several that do not subsume one another: Noto Symbols2 carries ◷ and not ⚙, Noto Symbols carries ⚙ and not 🌐, and DejaVu carries ⚙ where Noto is absent. A host with Symbols2 installed rendered ⚙ as tofu in the overlay while a face that covers it sat unread beside it. Every readable symbol face is now loaded, as SCRIPT_FONTS already did, and the candidate list gains Noto Symbols, DejaVu, and the platform emoji faces. That is a user-visible rendering fix, not a test fix. Anyone running the overlay on such a host was seeing the box. 🌐 WAS NOT A BUG. U+1F310 is astral and lives only in an emoji face; a host with none installed cannot render it whatever we ship. Failing on that made the test an audit of the developer's font packages rather than of the code -- and it is exactly the kind of red that gets waved off until it hides something real. The test now separates the two. Non-astral symbols are asserted unconditionally: an uncovered one means the chrome grew a glyph nothing carries, or SYMBOL_FONTS is missing a face, and both are ours to fix. Astral symbols are asserted only when an emoji face is actually loaded -- and when it is not, the skip is REPORTED BY NAME: no emoji face on this host — NOT checked: ['🌐'] (install a Noto Color Emoji / Segoe UI Emoji / Apple Color Emoji face to cover them) A silent skip would let the symbol set drift while the test still read green. PROVEN TO STILL FAIL. The first control I wrote was worthless: U+2C7F is covered by DejaVu, so the mutation passed and proved nothing. Probing the loaded stack for a genuinely uncovered codepoint gave U+A7CB; adding it to UI_SYMBOLS turns the test red and names the glyph, and removing it turns it green again. FOLLOW-UP after CI disagreed with the local run. Loading every symbol face made the stack non-empty on the CI runners, where it had been empty -- so the old `stack.is_empty()` skip stopped firing and the test began running there for the first time. It immediately reported the pillar labels and era kanji as tofu, because the ubuntu runner has no CJK face. Correct observation, wrong conclusion to draw from it: that is the runner's font packages, not this code. So the partition is by REQUIRED FACE CLASS rather than by astral-ness. A symbol is checked only when a face of its class actually parsed -- emoji for astral, CJK for Han, any face otherwise -- and an unchecked class is named in the output. `loaded()` now asks the PARSED faces, not the stack. NotoColorEmoji is a CBDT bitmap font; a parser that declines it would otherwise leave the key present, the glyph uncovered, and the test failing for a symbol nothing could render. And the skip is closed rather than merely reported: the Lens build job now installs fonts-noto-cjk, fonts-noto-core and fonts-noto-color-emoji, so CI is a host where every class CAN be checked. A skip that fires on every run is a check that never runs. --- .github/workflows/extras.yml | 7 +++ lens/src/fonts.rs | 40 ++++++++++++++--- lens/tests/fonts.rs | 86 +++++++++++++++++++++++++++++++----- 3 files changed, 116 insertions(+), 17 deletions(-) diff --git a/.github/workflows/extras.yml b/.github/workflows/extras.yml index 747c2e1..accb8a0 100644 --- a/.github/workflows/extras.yml +++ b/.github/workflows/extras.yml @@ -57,6 +57,13 @@ jobs: sudo apt-get install -y libxkbcommon-dev libgl1-mesa-dev libwayland-dev \ libx11-dev libxcursor-dev libxrandr-dev libxi-dev libxcb1-dev pkg-config \ mesa-vulkan-drivers + # Fonts, so tests/fonts.rs can actually CHECK rather than skip. That test + # only asserts a symbol class when a face of that class is loaded -- the + # alternative is auditing the runner's font packages instead of the code. + # Without these the runner has no CJK and no emoji face, so the pillar + # labels, era kanji and 🌐 go unchecked on every run, and a chrome that + # grew an uncoverable glyph would ship unnoticed. + sudo apt-get install -y fonts-noto-cjk fonts-noto-core fonts-noto-color-emoji - uses: dtolnay/rust-toolchain@1.96.0 with: components: clippy, rustfmt diff --git a/lens/src/fonts.rs b/lens/src/fonts.rs index 424987f..752f5bd 100644 --- a/lens/src/fonts.rs +++ b/lens/src/fonts.rs @@ -32,9 +32,31 @@ pub const CJK_FONTS: &[&str] = &[ /// Symbol faces covering the geometric / warning marks the CJK faces lack /// (◷ U+25F7, ⚠ U+26A0). pub const SYMBOL_FONTS: &[&str] = &[ - "/System/Library/Fonts/Apple Symbols.ttf", // macOS - "C:\\Windows\\Fonts\\seguisym.ttf", // Windows: Segoe UI Symbol - "/usr/share/fonts/truetype/noto/NotoSansSymbols2-Regular.ttf", // Linux + "/System/Library/Fonts/Apple Symbols.ttf", // macOS + "/System/Library/Fonts/Apple Color Emoji.ttc", // macOS: 🌐 and friends + "C:\\Windows\\Fonts\\seguisym.ttf", // Windows: Segoe UI Symbol + "C:\\Windows\\Fonts\\seguiemj.ttf", // Windows: Segoe UI Emoji + // Linux ships symbol coverage across SEVERAL faces, none of them complete: + // Symbols2 carries ◷, Symbols carries ⚙, and 🌐 (U+1F310) lives in the emoji + // face. DejaVu is the near-universal fallback and covers ⚙ where Noto is + // absent. Every readable one is loaded, not just the first -- see below. + "/usr/share/fonts/truetype/noto/NotoSansSymbols2-Regular.ttf", + "/usr/share/fonts/truetype/noto/NotoSansSymbols-Regular.ttf", + "/usr/share/fonts/truetype/noto/NotoColorEmoji.ttf", + "/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf", +]; + +/// One stable key per [`SYMBOL_FONTS`] entry, so a loaded face keeps a name in +/// the stack (egui registers fallbacks by key). Kept parallel to that list. +const SYMBOL_KEYS: &[&str] = &[ + "sym-apple", + "sym-apple-emoji", + "sym-segoe", + "sym-segoe-emoji", + "sym-noto2", + "sym-noto", + "sym-noto-emoji", + "sym-dejavu", ]; /// Per-script faces for scripts a single CJK face doesn't cover. Linux ships one @@ -71,8 +93,16 @@ pub fn fallback_fonts() -> Vec<(&'static str, Vec)> { if let Some(bytes) = first_readable(CJK_FONTS) { stack.push(("cjk", bytes)); } - if let Some(bytes) = first_readable(SYMBOL_FONTS) { - stack.push(("sym", bytes)); + // EVERY readable symbol face, not just the first. Symbol coverage is spread + // across faces that do not subsume one another -- Noto Symbols2 has ◷ but not + // ⚙, Noto Symbols has ⚙ but not 🌐, and 🌐 needs the emoji face. Taking only + // the first meant a host with Symbols2 installed rendered ⚙ and 🌐 as tofu + // while a perfectly good face sat unread beside it. Found by tests/fonts.rs + // on a Linux host that has exactly that combination. + for (i, path) in SYMBOL_FONTS.iter().enumerate() { + if let Ok(bytes) = std::fs::read(path) { + stack.push((SYMBOL_KEYS[i], bytes)); + } } // Per-script faces (Linux ships these separately; macOS/Windows cover them via // the CJK/pan-Unicode face above, so these paths simply won't exist there). diff --git a/lens/tests/fonts.rs b/lens/tests/fonts.rs index 47dfc80..fa3bd4e 100644 --- a/lens/tests/fonts.rs +++ b/lens/tests/fonts.rs @@ -2,12 +2,46 @@ //! the chrome renders, or those glyphs show as missing-glyph boxes (tofu). This //! reads the loaded fonts' cmaps and asserts coverage — the regression guard for //! "a redesign added a symbol no loaded font has" (e.g. ◷ / ⚠, absent from Arial -//! Unicode MS). Host-font-gated: skips cleanly when no system fallback is found. +//! Unicode MS). +//! +//! Two things can make a symbol uncoverable, and only one of them is a defect +//! here. A symbol no LOADED face carries is a real gap — either the chrome grew +//! a glyph nothing covers, or `SYMBOL_FONTS` is missing a face it should list. +//! An ASTRAL symbol on a host with no emoji face installed is neither: nothing +//! shipped can fix it, and failing on it would make this test an audit of the +//! developer's font packages rather than of the code. +//! +//! So the astral symbols are checked only when an emoji face is actually loaded, +//! and when it is not, the skip is REPORTED BY NAME. A silent skip would let the +//! set drift while the test still read green. #![allow(clippy::unwrap_used, clippy::expect_used)] use timeglyph_lens::fonts; use ttf_parser::Face; +/// Which class of face a symbol needs. A glyph is only CHECKABLE when a face of +/// its class is loaded: asserting a CJK ideograph against a host with no CJK +/// font tests the host's font packages, not this code. +#[derive(PartialEq)] +enum Needs { + /// Astral-plane (U+10000+): the emoji blocks, carried only by an emoji face. + Emoji, + /// Han ideographs — the 干支 pillar labels and era kanji. + Cjk, + /// Anything a general symbol or text face carries. + Any, +} + +fn needs(c: char) -> Needs { + if c.len_utf16() > 1 { + Needs::Emoji + } else if matches!(c, '\u{3400}'..='\u{4DBF}' | '\u{4E00}'..='\u{9FFF}') { + Needs::Cjk + } else { + Needs::Any + } +} + #[test] fn fallback_fonts_cover_every_ui_symbol() { let stack = fonts::fallback_fonts(); @@ -15,23 +49,51 @@ fn fallback_fonts_cover_every_ui_symbol() { eprintln!("no system fallback fonts on this host — skipping coverage check"); return; } - let faces: Vec = stack + // Key and face are kept TOGETHER on purpose. Asking "is an emoji face + // loaded?" of the stack while asking "is this covered?" of the parsed faces + // lets the two disagree: NotoColorEmoji is a CBDT bitmap font, and a parser + // that declines it would leave the key present, the glyph uncovered, and the + // test failing for a symbol nothing could have rendered. + let faces: Vec<(&str, Face)> = stack .iter() - .filter_map(|(_, bytes)| Face::parse(bytes, 0).ok()) + .filter_map(|(key, bytes)| Face::parse(bytes, 0).ok().map(|f| (*key, f))) .collect(); assert!( !faces.is_empty(), "loaded fallback bytes did not parse as fonts" ); + let covered = |c: char| faces.iter().any(|(_, f)| f.glyph_index(c).is_some()); + let loaded = |k: &str| faces.iter().any(|(key, _)| key.contains(k)); - let uncovered: Vec = fonts::UI_SYMBOLS - .iter() - .copied() - .filter(|&c| !faces.iter().any(|f| f.glyph_index(c).is_some())) - .collect(); + // (class, is the face for it loaded, what to call it when reporting a skip) + let classes = [ + (Needs::Any, true, "general"), + (Needs::Cjk, loaded("cjk"), "CJK"), + (Needs::Emoji, loaded("emoji"), "emoji"), + ]; - assert!( - uncovered.is_empty(), - "UI symbols with no glyph in any loaded fallback font (would render as tofu): {uncovered:?}" - ); + for (class, face_loaded, label) in classes { + let syms: Vec = fonts::UI_SYMBOLS + .iter() + .copied() + .filter(|&c| needs(c) == class) + .collect(); + if syms.is_empty() { + continue; + } + if !face_loaded { + // Named, never silent: a skip nobody can see is a hole the symbol + // set drifts into while the test still reads green. + eprintln!( + "no {label} face on this host — NOT checked: {syms:?} \ + (install one to cover them; CI runners and end-user machines differ here)" + ); + continue; + } + let missing: Vec = syms.into_iter().filter(|&c| !covered(c)).collect(); + assert!( + missing.is_empty(), + "a {label} face IS loaded but these would render as tofu: {missing:?}" + ); + } }