From a2909fb0df28b5d3bf9cde4cfad33588c44fd7cc Mon Sep 17 00:00:00 2001 From: grom Date: Sun, 18 Oct 2026 11:45:41 -0600 Subject: [PATCH] ui_text: a font file answers to a family of its own, so no sibling stands in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Naming a face to `cosmic-text` is a family query, and `fontdb` matches a family by exact name alone: two files whose name tables declare the same family are one family to the shaper, and whichever it reaches first answers for both. The player's AddOns folder is full of that — four addons ship the same `Myriad-Pro.ttf` and three the same `Hooge.ttf`, which is where our own substitution warning came from three times a session — and in one pair it is not even the same design: `pfUI-CustomMedia\fonts\NiceDamage.ttf` and `MikScrollingBattleText\Fonts\pepsi.ttf` both declare `! PEPSI !`, so one of them draws the other's letters at the other's widths. So each file is re-registered under `benilla:`, unique for the same reason `path_to_face` is keyed that way. The face's own axes are kept, and the shaper's per-character fallback is untouched — it walks the whole database and needs no family match — so the substitution warning now means only what is left: this face has no glyph for that character, and it says so. The test registers one real file at two paths and asserts each answers for itself; its second half is the behaviour that makes the rename necessary, `fontdb` answering with the first face for both. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn --- .../benilla-app/src/ui_text/engine/faces.rs | 132 ++++++++++++++++-- crates/benilla-app/src/ui_text/engine/mod.rs | 23 +-- 2 files changed, 138 insertions(+), 17 deletions(-) diff --git a/crates/benilla-app/src/ui_text/engine/faces.rs b/crates/benilla-app/src/ui_text/engine/faces.rs index 99884a79..d90f84b9 100644 --- a/crates/benilla-app/src/ui_text/engine/faces.rs +++ b/crates/benilla-app/src/ui_text/engine/faces.rs @@ -39,15 +39,42 @@ pub(super) fn hhea_ascent_ratio(bytes: &[u8]) -> Option { /// normal-weight, normal-style face and silently get another one when this face is bold or italic. pub(super) struct Registered { pub(super) id: fontdb::ID, + /// The family this face answers to: synthetic and unique to its path, see [`register_font`]. pub(super) family: String, + /// The family the file's own name table declares, for the load log; nothing queries it. + pub(super) declared: String, pub(super) weight: fontdb::Weight, pub(super) style: fontdb::Style, pub(super) stretch: fontdb::Stretch, } -/// Register a raw TTF into `font_system`'s database, named by what the loaded face says. +/// The family prefix a registered face answers to: a prefix no name table declares, with the font +/// path after it to make the rest unique. Matching is exact string equality, so even a file that +/// did declare this family would have to declare the whole path too. +const SYNTHETIC: &str = "benilla:"; + +/// Register a raw TTF into `font_system`'s database, under a family of its own. +/// +/// Naming a face to `cosmic-text` is a **family query** — `fontdb`'s own matching is exact string +/// equality against the name tables' family strings (`Database::query`) — so two files that declare +/// the same family are one family to the shaper, and whichever it matches first stands in for both. +/// That is not hypothetical in an addon folder: four of this player's addons ship the same +/// `Myriad-Pro.ttf` and three the same `Hooge.ttf`, which is what our substitution warning was +/// reporting three times a session; worse, `pfUI-CustomMedia\fonts\NiceDamage.ttf` and +/// `MikScrollingBattleText\Fonts\pepsi.ttf` are two *different* designs that both declare +/// `! PEPSI !`, where the stand-in draws the wrong letters at the wrong widths. +/// +/// So the face is re-registered under `benilla:`, which is unique for the same +/// reason `TextEngine::path_to_face` is keyed that way: one face per path. Its own axes are kept, +/// since the attrs that name this family carry them and `cosmic-text` matches on all three. The +/// shaper's per-character fallback is untouched — it walks the whole database and needs no family +/// match — so a character this face lacks is still drawn by one that has it. +/// +/// A font *collection* yields more faces than the one we take; those keep their declared family, +/// which now names no face we ever ask for, and stay reachable only as that fallback. pub(super) fn register_font( font_system: &mut FontSystem, + path: &str, bytes: Vec, ) -> anyhow::Result { let source = fontdb::Source::Binary( @@ -57,20 +84,109 @@ pub(super) fn register_font( let id = *ids .first() .ok_or_else(|| anyhow::anyhow!("font source produced no faces (not a valid TTF?)"))?; - let info = font_system - .db_mut() + let mut info = font_system + .db() .face(id) - .ok_or_else(|| anyhow::anyhow!("face {id:?} vanished right after loading"))?; - let family = info + .ok_or_else(|| anyhow::anyhow!("face {id:?} vanished right after loading"))? + .clone(); + let declared = info .families .first() .map(|(name, _)| name.clone()) .ok_or_else(|| anyhow::anyhow!("face {id:?} carries no family name"))?; + let family = format!("{SYNTHETIC}{}", path.to_ascii_lowercase()); + info.families = vec![(family.clone(), fontdb::Language::English_UnitedStates)]; + info.id = fontdb::ID::dummy(); + let (weight, style, stretch) = (info.weight, info.style, info.stretch); + // The swap is a removal and a push because `fontdb` hands out no mutable face: the id changes, + // which is safe only here, before anything has shaped with it. + let db = font_system.db_mut(); + db.remove_face(id); + let id = db.push_face_info(info); Ok(Registered { id, family, - weight: info.weight, - style: info.style, - stretch: info.stretch, + declared, + weight, + style, + stretch, }) } + +#[cfg(test)] +mod tests { + use super::*; + + /// MORPHEUS off the patch chain, standing in for an addon's own TTF: the fixture only has to + /// be one real file registered twice. + fn a_client_ttf() -> Option> { + let data = benilla_formats::wow_data()?; + let chain = benilla_formats::open_chain(&data).ok()?; + chain.read(CLIENT_FONTS[2]).ok() + } + + /// Two registrations of one file are two families, and each answers for itself — where the + /// name table's own family answers for whichever face `fontdb` reaches first, which is the + /// second half of this test and the whole reason for the rename. + #[test] + fn the_same_file_at_two_paths_is_two_families_and_each_answers_for_itself() { + let Some(bytes) = a_client_ttf() else { + eprintln!("skipping: no client install / patch chain"); + return; + }; + let pick = |fs: &FontSystem, family: &str, r: &Registered| { + fs.db().query(&fontdb::Query { + families: &[fontdb::Family::Name(family)], + weight: r.weight, + stretch: r.stretch, + style: r.style, + }) + }; + + let mut fs = super::super::client_font_system(); + let a = register_font( + &mut fs, + "Interface\\AddOns\\pfUI\\fonts\\Myriad-Pro.ttf", + bytes.clone(), + ) + .expect("the first registration"); + let b = register_font( + &mut fs, + "Interface\\AddOns\\TWThreat\\fonts\\Myriad-Pro.ttf", + bytes.clone(), + ) + .expect("the second registration"); + assert_eq!( + a.declared, b.declared, + "the fixture is the collision: one file's name table, at two paths" + ); + assert_ne!(a.family, b.family, "each path is its own family"); + assert_eq!(pick(&fs, &a.family, &a), Some(a.id)); + assert_eq!(pick(&fs, &b.family, &b), Some(b.id)); + assert_eq!( + fs.db().len(), + 2, + "and the renamed face replaces its own entry, it does not add one" + ); + + // Named by what the file declares, as before this rename: `fontdb` matches a family by + // name alone, so one of the two answers for both and the other is unreachable by name. + let mut plain = super::super::client_font_system(); + let ids: Vec = [&bytes, &bytes] + .iter() + .map(|b| { + *plain + .db_mut() + .load_font_source(fontdb::Source::Binary(std::sync::Arc::new((*b).clone()))) + .first() + .expect("a face per load") + }) + .collect(); + assert_ne!(ids[0], ids[1], "two entries, as ours are"); + assert_eq!( + pick(&plain, &a.declared, &a), + Some(ids[0]), + "the declared family answers with the first face, for both files" + ); + } +} diff --git a/crates/benilla-app/src/ui_text/engine/mod.rs b/crates/benilla-app/src/ui_text/engine/mod.rs index 91bcf277..3f87e323 100644 --- a/crates/benilla-app/src/ui_text/engine/mod.rs +++ b/crates/benilla-app/src/ui_text/engine/mod.rs @@ -168,7 +168,7 @@ impl TextEngine { } }; let ascent_ratio = hhea_ascent_ratio(&bytes).unwrap_or(0.794); - match register_font(&mut font_system, bytes) { + match register_font(&mut font_system, path, bytes) { Ok(r) => { path_to_face.insert(path.to_ascii_lowercase(), faces.len()); faces.push(Face { @@ -247,7 +247,7 @@ impl TextEngine { let source = self.source.as_ref()?; let bytes = read_font_bytes(source, path)?; let ascent_ratio = hhea_ascent_ratio(&bytes).unwrap_or(0.794); - let r = register_font(&mut self.font_system, bytes) + let r = register_font(&mut self.font_system, path, bytes) .inspect_err(|e| warn!("ui_text: failed to register {path}: {e:#}")) .ok()?; let index = self.faces.len(); @@ -261,7 +261,10 @@ impl TextEngine { ascent_ratio, }); self.path_to_face.insert(key.to_string(), index); - info!("ui_text: loaded font face {path}"); + info!( + "ui_text: loaded font face {path} — it declares family {:?}, registered as {:?}", + r.declared, self.faces[index].family + ); Some(index) } @@ -407,8 +410,10 @@ impl TextEngine { } } } - // Naming a face to `cosmic-text` is a query that takes the first face able to draw the - // character, so an unmatched family draws and measures in another face: warned once. + // The family named here is this face's own ([`faces::register_font`]), so a collision with + // another file's name table can no longer put a sibling in front. What is left is the + // shaper's per-character fallback: it walks the rest of the database for a face that has + // the codepoint, and that face's metrics are what this character draws and measures at. if let Some(got) = shaped_by.filter(|&got| got != face_id) { if self.substituted.insert(face) { let got_name = self @@ -418,9 +423,9 @@ impl TextEngine { .map_or_else(|| format!("{got:?}"), |i| i.post_script_name.clone()); let want = self.faces.get(face).map_or("?", |f| f.path.as_str()); warn!( - "ui_text: '{want}' loaded as family {family:?} (weight {}, style {style:?}) \ - but {ch:?} shaped in {got_name} instead — text in this face draws and \ - measures in the wrong one", + "ui_text: '{want}' (weight {}, style {style:?}) has no glyph for {ch:?} — the \ + shaper fell back to {got_name}, so this character draws and measures in \ + another face", weight.0 ); } @@ -611,7 +616,7 @@ pub(super) fn test_engine(dpi: f32) -> Option { continue; }; let ascent_ratio = hhea_ascent_ratio(&bytes).unwrap_or(0.794); - let r = register_font(&mut font_system, bytes).ok()?; + let r = register_font(&mut font_system, path, bytes).ok()?; path_to_face.insert(path.to_ascii_lowercase(), faces.len()); faces.push(Face { id: r.id,