From 19684cff26792cf1d919daadbc21cb98c0c50167 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Mon, 12 Oct 2026 00:29:58 -0400 Subject: [PATCH] layout: make Myanmar fallback tests deterministic (#807) tests_complex::myanmar_syllables_are_single_clusters and myanmar_in_a_latin_font_falls_back_to_a_regular_myanmar_face failed about 1 run in 4 of the full layout suite (cluster starts [0, 3, 6, 12, 18], "one face for the word") but passed alone. Root cause: a race in FontDb's system font fallback, not the tests. Tests share FontDb::global(), and layout asks for a fallback per character from several threads. A search marks a preferred family ("Padauk" for Myanmar) as tried before loading it; a second search racing it skipped the family as already tried, fell through to the coverage index, and loaded the first cataloged file in path order that maps the character (Padauk-Bold.ttf on its own). Characters of one syllable then fell back to different faces, so the shaper split the syllable and the word's faces differed. Separately, a search's answer was cached under the face count read after the search, so an answer chosen before another thread's load could be cached as current. Fix (crates/fonts): - System fallback searches run one at a time (a `searching` lock). A search that waited re-checks the loaded faces first, so it uses the family the previous search loaded instead of loading another file. - The fallback cache keys an answer by the face count it was chosen from, and doesn't cache an answer from before a concurrent load. Co-authored-by: Claude Opus 5.5 --- crates/fonts/src/fontdb.rs | 57 +++++++++++++++++++++++++++++--------- 1 file changed, 44 insertions(+), 13 deletions(-) diff --git a/crates/fonts/src/fontdb.rs b/crates/fonts/src/fontdb.rs index 2d0573a..dea01f8 100644 --- a/crates/fonts/src/fontdb.rs +++ b/crates/fonts/src/fontdb.rs @@ -306,6 +306,12 @@ pub struct FontDb { /// System fallback state: enabled, and characters no system font covers. #[cfg(not(target_arch = "wasm32"))] sys: Mutex, + /// Held through a system fallback search, so searches run one at a time. Without it a search + /// racing another that was still loading a preferred family ("Padauk" for Myanmar) skipped + /// that family as already tried and loaded whatever the coverage index found first (only + /// Padauk Bold), so which face a character fell back to depended on thread timing. + #[cfg(not(target_arch = "wasm32"))] + searching: Mutex<()>, /// The characters each installed font file maps, read the first time a fallback search /// needs that file. Held through a search, so two searches never read a file twice. #[cfg(not(target_arch = "wasm32"))] @@ -841,6 +847,8 @@ impl FontDb { #[cfg(not(target_arch = "wasm32"))] sys: Mutex::new(SysFallback { enabled: true, ..Default::default() }), #[cfg(not(target_arch = "wasm32"))] + searching: Mutex::new(()), + #[cfg(not(target_arch = "wasm32"))] coverage: Mutex::new(CoverageIndex::default()), #[cfg(not(target_arch = "wasm32"))] catalog_gen: AtomicU32::new(0), @@ -1276,9 +1284,13 @@ impl FontDb { return hit.clone(); } } - let found = self.search_fallback(c, exclude); - // Keyed by the face count after the search (a system fallback may have loaded fonts). - let n = self.read_faces().len(); + let (found, n) = self.search_fallback(c, exclude); + // Keyed by the face count the answer was chosen from (a system fallback may have loaded + // fonts, and another thread may have loaded more since: an answer from before that load + // is returned but not cached under the new count). + if n != self.read_faces().len() { + return found; + } let mut cache = self.fallbacks.write().unwrap_or_else(|e| e.into_inner()); if cache.faces != n || cache.map.len() >= FALLBACK_CACHE_MAX { cache.map.clear(); @@ -1288,17 +1300,36 @@ impl FontDb { found } - fn search_fallback(&self, c: char, exclude: u32) -> Option> { - self.loaded_fallback(c, exclude).or_else(|| { - #[cfg(not(target_arch = "wasm32"))] - if self.system_fallback(c) { - return self.loaded_fallback(c, exclude); - } - None - }) + /// The fallback face for `c`, and the number of loaded faces it was chosen from. + fn search_fallback(&self, c: char, exclude: u32) -> (Option>, usize) { + let (found, n) = self.loaded_fallback(c, exclude); + if found.is_some() { + return (found, n); + } + self.system_search(c, exclude) } - fn loaded_fallback(&self, c: char, exclude: u32) -> Option> { + /// [`Self::search_fallback`] when no loaded face covers `c`: load a system font that does, + /// one search at a time. + #[cfg(not(target_arch = "wasm32"))] + fn system_search(&self, c: char, exclude: u32) -> (Option>, usize) { + let _searching = self.searching.lock().unwrap_or_else(|e| e.into_inner()); + // A search that held the lock first may have loaded a font covering `c`: choose among the + // loaded faces again before loading more. + let (found, n) = self.loaded_fallback(c, exclude); + if found.is_none() && self.system_fallback(c) { + return self.loaded_fallback(c, exclude); + } + (found, n) + } + + #[cfg(target_arch = "wasm32")] + fn system_search(&self, _c: char, _exclude: u32) -> (Option>, usize) { + (None, self.read_faces().len()) + } + + /// The best loaded face covering `c`, and the number of loaded faces it was chosen from. + fn loaded_fallback(&self, c: char, exclude: u32) -> (Option>, usize) { let faces = self.read_faces(); let mut order: Vec<&Arc> = faces.iter().filter(|f| f.id != exclude).collect(); // Right-to-left letters prefer a font made for the script, so a word isn't split across @@ -1315,7 +1346,7 @@ impl FontDb { let complex = crate::needs_layout_tables(c); let shaping = |f: &FontFace| if complex { (!f.has_layout_tables(), crate::fallback_rank(c, &f.family)) } else { (false, 0) }; order.sort_by_key(|f| (rank(f), shaping(f), !f.family.eq_ignore_ascii_case(FALLBACK_FAMILY), f.italic, (f.weight - 400.0).abs() as i32)); - order.into_iter().find(|f| f.covers(c)).cloned() + (order.into_iter().find(|f| f.covers(c)).cloned(), faces.len()) } /// Load a system font covering `c` (preferred fallback families first, then any cataloged