diff --git a/crates/text/src/fontdb.rs b/crates/text/src/fontdb.rs index a1b232bc4..ff292f96c 100644 --- a/crates/text/src/fontdb.rs +++ b/crates/text/src/fontdb.rs @@ -929,6 +929,30 @@ pub(crate) fn alias_lists() -> u64 { ALIAS_LISTS.with(std::cell::Cell::get) } +#[cfg(test)] +thread_local! { + /// Tests: run on this thread each time it publishes faces to a database, to look the database + /// up at exactly that point (as another thread could). + static ON_FACES_PUBLISHED: std::cell::RefCell>> = const { std::cell::RefCell::new(None) }; +} + +/// Tests: run `hook` each time this thread publishes faces to a database. +#[cfg(test)] +pub(crate) fn on_faces_published(hook: Option>) { + ON_FACES_PUBLISHED.with(|h| *h.borrow_mut() = hook); +} + +#[cfg(test)] +fn faces_published() { + // Taken out while it runs, so a hook that adds faces itself doesn't run again. + if let Some(mut hook) = ON_FACES_PUBLISHED.with(|h| h.borrow_mut().take()) { + hook(); + ON_FACES_PUBLISHED.with(|h| { + h.borrow_mut().get_or_insert(hook); + }); + } +} + /// How a requested family and style resolved ([`FontDb::resolve`]). #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum FontMatch { @@ -1890,24 +1914,45 @@ impl FontDb { /// [`FontDb::add_font`] for the bytes of the file at `path` (when known). fn add_font_from(&self, bytes: Vec, path: Option<&Path>) -> usize { - let data = Arc::new(bytes); - let mut added = 0; - for (i, spec) in enumerate_faces(&data) { - // Another version of a family and style is kept: a document may name it (the first one - // loaded stays the one the family and style alone resolve to). - let version = skrifa::FontRef::from_index(&data, i).ok().and_then(|f| name(&f, &[StringId::VERSION_STRING])).unwrap_or_default(); - if self - .read_faces() - .iter() - .any(|f| f.family.eq_ignore_ascii_case(&spec.family) && f.style.eq_ignore_ascii_case(&spec.style) && f.version == version) - { - continue; - } - if let Some(f) = make_face(FontBytes::Owned(data.clone()), i, spec, path.map(Path::to_path_buf)) { - self.faces.write().unwrap_or_else(|e| e.into_inner()).push(Arc::new(f)); - added += 1; + self.add_fonts([(bytes, path.map(Path::to_path_buf))]) + } + + /// Add the faces of `files` (each file's bytes, and the file when known) all at once: a lookup + /// on another thread finds all of them or none, never a family with only some of its styles + /// loaded, whose nearest style it would take instead (Arial Bold Italic, the first of Arial's + /// files, for upright Regular text). Returns the number of faces added. + fn add_fonts(&self, files: impl IntoIterator, Option)>) -> usize { + // Another version of a family and style is kept: a document may name it (the first one + // loaded stays the one the family and style alone resolve to). + let same = |f: &FontFace, family: &str, style: &str, version: &str| { + f.family.eq_ignore_ascii_case(family) && f.style.eq_ignore_ascii_case(style) && f.version == version + }; + let mut made: Vec = Vec::new(); + for (bytes, path) in files { + let data = Arc::new(bytes); + for (i, spec) in enumerate_faces(&data) { + let version = skrifa::FontRef::from_index(&data, i).ok().and_then(|f| name(&f, &[StringId::VERSION_STRING])).unwrap_or_default(); + let present = |f: &FontFace| same(f, &spec.family, &spec.style, &version); + if self.read_faces().iter().any(|f| present(f)) || made.iter().any(present) { + continue; + } + if let Some(f) = make_face(FontBytes::Owned(data.clone()), i, spec, path.clone()) { + made.push(f); + } } } + let mut faces = self.faces.write().unwrap_or_else(|e| e.into_inner()); + let before = faces.len(); + for f in made { + // Another thread may have added the same face meanwhile. + if !faces.iter().any(|g| same(g, &f.family, &f.style, &f.version)) { + faces.push(Arc::new(f)); + } + } + let added = faces.len() - before; + drop(faces); + #[cfg(test)] + faces_published(); if added > 0 { self.changed(); } @@ -2183,20 +2228,17 @@ impl FontDb { self.postscript.read().unwrap_or_else(|e| e.into_inner()).get(&norm(name)).cloned() } - /// Load the files of the installed `family`. Returns whether any face was added. + /// Load the files of the installed `family`, adding all their faces at once + /// ([`Self::add_fonts`]). Returns whether the family is loaded now: by this call, or by another + /// thread meanwhile. #[cfg(not(target_arch = "wasm32"))] fn load_cataloged(&self, family: &str) -> bool { let mut paths: Vec = self.read_catalog().get(&family.to_ascii_lowercase()).map(|c| c.faces.iter().map(|cf| cf.path.clone()).collect()).unwrap_or_default(); paths.sort(); paths.dedup(); - let mut any = false; - for p in paths { - for data in font_files(&p) { - any |= self.add_font_from(data, Some(&p)) > 0; - } - } - any + let files = paths.into_iter().flat_map(|p| font_files(&p).into_iter().map(move |data| (data, Some(p.clone())))); + self.add_fonts(files) > 0 || self.is_loaded(family) } /// Resolve a family + style to a face, falling back to the closest style of the family, then to diff --git a/crates/text/src/tests_sysfonts.rs b/crates/text/src/tests_sysfonts.rs index e75486233..40cce9f06 100644 --- a/crates/text/src/tests_sysfonts.rs +++ b/crates/text/src/tests_sysfonts.rs @@ -194,6 +194,50 @@ fn rescanning_retries_characters_no_font_covered() { assert_eq!(db.face_covering(C).unwrap().family, "Retry Sans"); } +/// A lookup on another thread while an installed family loads finds the whole family or none of +/// it: never only the face of its first file, which falling back takes for text in the others' style +/// (Arial Bold Italic for upright Regular Arabic, the same text laid out again moments later in +/// Arial Regular). +#[test] +fn a_family_loading_on_another_thread_is_not_found_in_part() { + // A private-use character none of the bundled fonts has. + const C: char = '\u{F0000}'; + let dir = std::env::temp_dir().join(format!("vc-sysfonts-{}-whole-family", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + // [`FAMILY`] Bold and Regular, their character maps replaced by one holding only `C`. The Bold's + // file sorts first, so it is read first. + for (file, source) in [("Sysfont-Bold.ttf", "SourceSans3-Bold.ttf"), ("Sysfont-Regular.ttf", "SourceSans3-Regular.ttf")] { + let base = renamed(source); + 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(file), builder.build()).unwrap(); + } + let db = std::sync::Arc::new(FontDb::with_font_dirs(vec![dir.clone()])); + // Each time this thread publishes faces, another thread falls back for `C` right then. + let seen = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let (hook_db, hook_seen) = (db.clone(), seen.clone()); + crate::fontdb::on_faces_published(Some(Box::new(move || { + let db = hook_db.clone(); + let style = std::thread::spawn(move || db.fallback_for(C, 0).map(|f| f.style.clone())).join().unwrap(); + hook_seen.lock().unwrap().push(style); + }))); + let face = db.face(FAMILY, "Regular").map(|f| f.style.clone()); + crate::fontdb::on_faces_published(None); + assert_eq!(face.as_deref(), Some("Regular")); + let seen = seen.lock().unwrap().clone(); + assert!(!seen.is_empty()); + assert!(seen.iter().all(|s| s.as_deref() == Some("Regular")), "the lookups took the family's {seen:?} faces"); + std::fs::remove_dir_all(dir).unwrap(); +} + #[test] fn rescanning_finds_fonts_installed_since() { let dir = font_dir("rescan");