Merge pull request #1126 from dotCooCoo/fix/script-fallback-loaded-faces

crates/text/src/fonts.rs

@@ -325,6 +325,8 @@ pub fn add_source(src: &dyn FontSource) -> usize {
             n += 1;
         }
     }
+    // A new face can belong to one of a script's usual families.
+    family_faces_cache().write().unwrap_or_else(|e| e.into_inner()).clear();
     n
 }
 
@@ -333,7 +335,9 @@ pub fn add_font_data(data: Vec<u8>) -> Vec<FaceId> {
     let data = Arc::new(data);
     let names = read_faces_bytes(&data);
     let mut d = db().write().unwrap_or_else(|e| e.into_inner());
-    names.into_iter().map(|n| push(&mut d, info_from(n, FaceData::Shared(data.clone()), "user"))).collect()
+    let ids = names.into_iter().map(|n| push(&mut d, info_from(n, FaceData::Shared(data.clone()), "user"))).collect();
+    family_faces_cache().write().unwrap_or_else(|e| e.into_inner()).clear();
+    ids
 }
 
 /// Held for a whole system scan: a concurrent caller waits for the complete list instead of
@@ -645,9 +649,10 @@ pub fn set_cjk_locale(tag: &str) {
     let tag = tag.trim().to_ascii_lowercase();
     let requested = if tag.is_empty() || tag == "c" || tag == "posix" { None } else { Some(tag) };
     *REQUESTED_CJK_LOCALE.write().unwrap_or_else(|e| e.into_inner()) = requested;
-    // The fallback cache answers per character block and preferred face, so a change of language
-    // must drop it.
+    // The order of the CJK families follows the language, so a change of language drops the cached
+    // fallbacks.
     fallback_cache().write().unwrap_or_else(|e| e.into_inner()).clear();
+    family_faces_cache().write().unwrap_or_else(|e| e.into_inner()).clear();
 }
 
 fn requested_cjk_locale() -> Option<String> {
@@ -776,30 +781,52 @@ fn glyph_draws(font: &FontRef, gid: skrifa::GlyphId) -> bool {
     (0..strikes.len()).any(|i| strikes.get(i).is_some_and(|s| s.get(gid).is_some()))
 }
 
-/// Results of the expensive part of [`fallback_for`], per 256-character block and preferred face
-/// (so bold text gets the bold cut of the fallback family). `None` records that nothing installed
-/// covers the block, so the search over every font runs once per block, not once per character.
+/// Results of the search over every installed font in [`fallback_for`], per 256-character block and
+/// preferred face (so bold text gets the bold cut of the fallback family). `None` records that
+/// nothing installed covers the block, so the search over every font runs once per block, not once
+/// per character.
 fn fallback_cache() -> &'static RwLock<std::collections::HashMap<(u32, FaceId), Option<FaceId>>> {
     static CACHE: OnceLock<RwLock<std::collections::HashMap<(u32, FaceId), Option<FaceId>>>> = OnceLock::new();
     CACHE.get_or_init(Default::default)
 }
 
-/// A face that covers `c`, preferring `prefer`. Bundled faces and faces already loaded are tried
-/// first (cheap); then, for CJK characters, the platform's usual family for that language, in the
-/// preferred face's style; then any installed face that covers the character. `prefer` when
-/// nothing does (the glyph renders as the missing-glyph box).
+/// The results of [`family_faces`], per script and preferred face.
+fn family_faces_cache() -> &'static RwLock<std::collections::HashMap<(FallbackScript, FaceId), Arc<Vec<FaceId>>>> {
+    static CACHE: OnceLock<RwLock<std::collections::HashMap<(FallbackScript, FaceId), Arc<Vec<FaceId>>>>> = OnceLock::new();
+    CACHE.get_or_init(Default::default)
+}
+
+/// A face that covers `c`, preferring `prefer`. Bundled faces are tried first (cheap). For a
+/// character of a script with usual families (CJK, Arabic, Hebrew, Indic, Thai), the platform's
+/// usual families for that script are tried next, in the preferred face's style. Faces already
+/// loaded are tried after that (cheap), and any installed face that covers the character is tried
+/// last. `prefer` is returned when nothing covers it (the glyph renders as the missing-glyph box).
 pub fn fallback_for(c: char, prefer: FaceId) -> FaceId {
     let p = face(prefer);
     if p.has_char(c) || c.is_control() {
         return prefer;
     }
     let faces = all_faces();
-    // prefer a face of the same italic-ness that is bundled (cheap), then anything loaded already
+    // prefer a face of the same italic-ness that is bundled (cheap)
     for f in faces.iter().filter(|f| f.info.origin == "bundled") {
         if f.has_char(c) {
             return f.id;
         }
     }
+    // The usual families of the character's script come before the faces that are loaded already
+    // and before the search results of its block. Which faces are loaded depends on every earlier
+    // lookup, and Arial Unicode MS, the first face in the macOS scan, covers every script with
+    // usual families except Sinhala.
+    let script = script_of(c);
+    if script != FallbackScript::Other {
+        if cfg!(not(target_arch = "wasm32")) && !system_scanned() {
+            scan_system();
+        }
+        if let Some(&id) = family_faces(script, &p).iter().find(|&&id| face(id).has_char(c)) {
+            return id;
+        }
+    }
+    // Faces that are loaded already come next (cheap).
     for f in faces.iter().filter(|f| f.info.origin != "bundled" && f.bytes.get().is_some()) {
         if f.has_char(c) {
             return f.id;
@@ -821,9 +848,13 @@ pub fn fallback_for(c: char, prefer: FaceId) -> FaceId {
     found.unwrap_or(prefer)
 }
 
-/// The script-aware then exhaustive search behind [`fallback_for`].
-fn system_fallback(c: char, style: &str) -> Option<FaceId> {
-    let script = script_of(c);
+/// The script-aware part of [`fallback_for`]: the installed faces of the platform's usual families
+/// for `script`, in the order they are tried, each in the style of `prefer`. For CJK, the other CJK
+/// languages' families follow in the locale's order.
+fn family_faces(script: FallbackScript, prefer: &Face) -> Arc<Vec<FaceId>> {
+    if let Some(faces) = family_faces_cache().read().unwrap_or_else(|e| e.into_inner()).get(&(script, prefer.id)) {
+        return faces.clone();
+    }
     let order: Vec<FallbackScript> = match script {
         FallbackScript::Other => vec![],
         FallbackScript::Han => han_order().to_vec(),
@@ -834,17 +865,15 @@ fn system_fallback(c: char, style: &str) -> Option<FaceId> {
         }
         s => vec![s],
     };
-    for s in order {
-        for fam in families_for(s) {
-            if let Some(r) = resolve_in(fam, style)
-                && face(r.face).has_char(c)
-            {
-                return Some(r.face);
-            }
-        }
-    }
-    // Anything installed, in scan order; the style's weight and slant break ties among the faces
-    // of the first covering family.
+    let style = &prefer.info.style;
+    let faces: Arc<Vec<FaceId>> = Arc::new(order.into_iter().flat_map(families_for).filter_map(|fam| resolve_in(fam, style)).map(|r| r.face).collect());
+    family_faces_cache().write().unwrap_or_else(|e| e.into_inner()).insert((script, prefer.id), faces.clone());
+    faces
+}
+
+/// The exhaustive search behind [`fallback_for`]: anything installed, in scan order; the style's
+/// weight and slant break ties among the faces of the first covering family.
+fn system_fallback(c: char, style: &str) -> Option<FaceId> {
     let faces = all_faces();
     let first = faces.iter().find(|f| f.info.origin != "bundled" && file_covers(f, c))?;
     let r = resolve_in(&first.info.family, style)?;

crates/text/tests/script_fallback.rs

@@ -0,0 +1,36 @@
+//! Font fallback for characters of scripts with usual families gives the same face whatever earlier
+//! lookups read. Its own test binary, so no installed font has been read when it starts.
+
+use effectcraft_text::{fonts, variable};
+
+#[test]
+fn script_fallback_does_not_depend_on_earlier_lookups() {
+    fonts::scan_system();
+    let inter = fonts::resolve("Inter", "Regular").face;
+    let scripts = ['א', 'م', 'क', 'த', 'ก', 'あ', '水', '한'];
+    let before: Vec<_> = scripts.iter().map(|&c| fonts::fallback_for(c, inter)).collect();
+    // A character that no usual family covers keeps its face after a character of the same block
+    // that no face covers. On macOS only faces outside the Arabic list (.SF Arabic and .SF Arabic
+    // Rounded) cover U+08A4, and no face covers U+0870.
+    let first = fonts::fallback_for('\u{8a4}', inter);
+    fonts::fallback_for('\u{870}', inter);
+    let again = fonts::fallback_for('\u{8a4}', inter);
+    assert_eq!(again, first, "U+08A4: {} → {}", fonts::face(first).info.family, fonts::face(again).info.family);
+    // U+0590 is unassigned, in the Hebrew range. Armenian and Lao have no usual family; they share
+    // 256-character blocks with Hebrew and Thai. The variable font search reads installed faces in
+    // scan order. On macOS the first face in the scan, Arial Unicode MS, covers Armenian, Lao and
+    // the eight characters in `scripts`.
+    for c in ['\u{590}', 'Ա', 'ກ'] {
+        fonts::fallback_for(c, inter);
+    }
+    variable::find_variable_face();
+    let changed: Vec<String> = scripts
+        .iter()
+        .zip(&before)
+        .filter_map(|(&c, &id)| {
+            let after = fonts::fallback_for(c, inter);
+            (after != id).then(|| format!("{c}: {} → {}", fonts::face(id).info.family, fonts::face(after).info.family))
+        })
+        .collect();
+    assert!(changed.is_empty(), "{changed:?}");
+}