From 25abd1f4728f12f9c0a42a33ece650b3f88886a4 Mon Sep 17 00:00:00 2001 From: FractalEncrypt <59615752+FractalEncrypt@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:10:37 -0400 Subject: [PATCH] text: recheck concurrently loaded fallback faces A system fallback search can add no new face because another lookup loaded it first. Re-read the loaded faces regardless of the search's return value, so concurrent first use does not produce a missing glyph and different advances. Add an isolated generated-font regression with simultaneous cold lookups. The original code missed 14 of 16 lookups; the fixed regression passed twenty runs. Both previously failing Ruby tests pass unchanged. Validation: Rust 1.98.1 cargo xtask ci, all 10 gates passed; workspace tests 9,635 passed, 0 failed, 14 ignored. Default Rust toolchain unchanged. --- crates/text/src/fontdb.rs | 8 +++--- crates/text/src/tests_sysfonts.rs | 42 +++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/crates/text/src/fontdb.rs b/crates/text/src/fontdb.rs index 06e952e8d..a459e0491 100644 --- a/crates/text/src/fontdb.rs +++ b/crates/text/src/fontdb.rs @@ -2418,10 +2418,10 @@ impl FontDb { return Some(f); } #[cfg(not(target_arch = "wasm32"))] - if self.system_fallback(c) { - return self.loaded_fallback(c, exclude, weight, italic); - } - None + self.system_fallback(c); + // Another lookup may have loaded the face while this one searched. Loading no new + // faces does not mean no face covers the character (as with `face`'s catalog lookup). + self.loaded_fallback(c, exclude, weight, italic) } /// A face that covers `c`, for text drawn outside the canvas (the app's own UI): the loaded diff --git a/crates/text/src/tests_sysfonts.rs b/crates/text/src/tests_sysfonts.rs index 0752d849c..e75486233 100644 --- a/crates/text/src/tests_sysfonts.rs +++ b/crates/text/src/tests_sysfonts.rs @@ -8,6 +8,48 @@ use crate::fontdb::fonts_outside; const FAMILY: &str = "Sysfont Sans3"; +/// Concurrent first use must not report a missing glyph just because another lookup loaded it. +#[test] +fn concurrent_fallback_lookups_all_find_the_installed_face() { + const C: char = '\u{F0000}'; + let dir = std::env::temp_dir().join(format!("vc-sysfonts-{}-concurrent-fallback", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let base = named(&[(3, EN, 1, "Concurrent Sans"), (3, EN, 2, "Regular"), (3, EN, 6, "ConcurrentSans-Regular")]); + let font = skrifa::FontRef::new(&base).unwrap(); + let mut builder = write_fonts::FontBuilder::new(); + builder.add_raw(write_fonts::types::Tag::new(b"cmap"), crate::embed::cmap(&[(C as u32, 1)]).unwrap()); + for r in font.table_directory.table_records() { + let tag = write_fonts::types::Tag::new(&r.tag().to_be_bytes()); + if !builder.contains(tag) { + builder.add_raw(tag, font.table_data(r.tag()).unwrap().as_bytes()); + } + } + std::fs::write(dir.join("Concurrent.ttf"), builder.build()).unwrap(); + for _ in 0..4 { + let db = FontDb::with_font_dirs(vec![dir.clone()]); + assert_eq!(db.load_system_fonts(), 1); + let start = std::sync::Barrier::new(16); + let faces = std::thread::scope(|scope| { + let workers: Vec<_> = (0..16) + .map(|_| { + scope.spawn(|| { + start.wait(); + db.face_covering(C) + }) + }) + .collect(); + workers.into_iter().map(|worker| worker.join().unwrap()).collect::>() + }); + assert!( + faces.iter().all(|face| face.as_ref().is_some_and(|face| face.family == "Concurrent Sans")), + "{} of 16 lookups missed the installed face", + faces.iter().filter(|face| face.is_none()).count() + ); + } + std::fs::remove_file(dir.join("Concurrent.ttf")).unwrap(); + std::fs::remove_dir(dir).unwrap(); +} + /// A bundled Source Sans 3 file renamed [`FAMILY`] (as long as the original name). fn renamed(file: &str) -> Vec { renamed_to(file, FAMILY)