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,