mirror of
https://github.com/storytold/designcraft.git
synced 2026-10-29 20:56:47 -06:00
Merge #315: Retry missing font fallbacks after a font rescan
This commit is contained in:
@@ -31,6 +31,10 @@
|
||||
|
||||
DesignCraft aims at full Adobe InDesign parity — and to be better: faster, open (documented JSON format + IDML), scriptable by agents (MCP), and available on the web.
|
||||
|
||||
## Recent improvements (2026-10-08)
|
||||
|
||||
- QA1: Font rescans retry missing glyphs and fallback families after new fonts arrive; a controlled Linux regression covers older in-flight misses after a rescan.
|
||||
|
||||
## Status (2026-10-01)
|
||||
|
||||
**Working today**
|
||||
|
||||
@@ -357,17 +357,28 @@ pub struct FontDb {
|
||||
/// System fallback state: enabled, and characters no system font covers.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
sys: Mutex<SysFallback>,
|
||||
/// Pause one fallback after copying catalog paths, with no database locks held.
|
||||
#[cfg(all(test, target_os = "linux"))]
|
||||
fallback_snapshot_hook: Mutex<Option<FallbackSnapshotHook>>,
|
||||
}
|
||||
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
#[derive(Default)]
|
||||
struct SysFallback {
|
||||
enabled: bool,
|
||||
/// Rescan generation: an older in-flight lookup must not restore a cleared miss.
|
||||
generation: u64,
|
||||
misses: std::collections::HashSet<char>,
|
||||
/// CJK chain families already looked up in the catalog (loaded, or not installed).
|
||||
chain_tried: std::collections::HashSet<&'static str>,
|
||||
}
|
||||
|
||||
#[cfg(all(test, target_os = "linux"))]
|
||||
struct FallbackSnapshotHook {
|
||||
reached: std::sync::mpsc::SyncSender<()>,
|
||||
resume: Arc<std::sync::Barrier>,
|
||||
}
|
||||
|
||||
/// Families tried (when installed) for characters the loaded fonts lack: CJK, symbols, emoji.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
const SYSTEM_FALLBACKS: &[&str] = &[
|
||||
@@ -838,6 +849,8 @@ impl FontDb {
|
||||
cataloged: std::sync::OnceLock::new(),
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
sys: Mutex::new(SysFallback { enabled: true, ..Default::default() }),
|
||||
#[cfg(all(test, target_os = "linux"))]
|
||||
fallback_snapshot_hook: Mutex::new(None),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -964,7 +977,14 @@ impl FontDb {
|
||||
// The first scan, or a rescan once it is done (never both at once).
|
||||
let mut first = None;
|
||||
self.cataloged.get_or_init(|| first = Some(self.scan_font_dirs()));
|
||||
first.unwrap_or_else(|| self.scan_font_dirs())
|
||||
let count = first.unwrap_or_else(|| self.scan_font_dirs());
|
||||
// Retry characters and CJK chain families that newly installed fonts can now cover.
|
||||
// Reset outside scan_font_dirs: a chain lookup can hold sys during the initial scan.
|
||||
let mut sys = self.sys.lock().unwrap_or_else(|e| e.into_inner());
|
||||
sys.generation = sys.generation.wrapping_add(1);
|
||||
sys.misses.clear();
|
||||
sys.chain_tried.clear();
|
||||
count
|
||||
}
|
||||
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
@@ -1172,12 +1192,13 @@ impl FontDb {
|
||||
if c.is_control() || c.is_whitespace() {
|
||||
return false;
|
||||
}
|
||||
{
|
||||
let generation = {
|
||||
let sys = self.sys.lock().unwrap_or_else(|e| e.into_inner());
|
||||
if !sys.enabled || sys.misses.contains(&c) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
sys.generation
|
||||
};
|
||||
let covered = |db: &FontDb| db.read_faces().iter().any(|f| f.covers(c));
|
||||
let cataloged: Vec<String> = self.read_catalog().iter().map(|e| e.family.clone()).collect();
|
||||
for fam in SYSTEM_FALLBACKS {
|
||||
@@ -1191,6 +1212,14 @@ impl FontDb {
|
||||
let mut paths: Vec<std::path::PathBuf> = self.catalog.read().unwrap_or_else(|e| e.into_inner()).iter().map(|e| e.path.clone()).collect();
|
||||
paths.sort();
|
||||
paths.dedup();
|
||||
#[cfg(all(test, target_os = "linux"))]
|
||||
{
|
||||
let hook = self.fallback_snapshot_hook.lock().unwrap_or_else(|e| e.into_inner()).take();
|
||||
if let Some(hook) = hook {
|
||||
let _ = hook.reached.send(());
|
||||
hook.resume.wait();
|
||||
}
|
||||
}
|
||||
for p in paths {
|
||||
if std::fs::metadata(&p).map(|m| m.len() > 40 << 20).unwrap_or(true) {
|
||||
continue;
|
||||
@@ -1202,7 +1231,10 @@ impl FontDb {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
self.sys.lock().unwrap_or_else(|e| e.into_inner()).misses.insert(c);
|
||||
let mut sys = self.sys.lock().unwrap_or_else(|e| e.into_inner());
|
||||
if sys.generation == generation {
|
||||
sys.misses.insert(c);
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
|
||||
@@ -164,3 +164,82 @@ fn font_menus_group_installed_and_loaded_fonts_without_loading_them() {
|
||||
assert_eq!(infos.iter().filter(|i| i.family == "Groups Mincho").count(), 1);
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rescanning_retries_missing_glyphs_and_absent_fallback_families() {
|
||||
use crate::testing::font_with;
|
||||
let dir = font_dir("rescan-fallback-ttf");
|
||||
// Only synthetic TTF files in this folder are scanned. Keep the loaded Latin fallback
|
||||
// alone so optional craft-fonts cannot provide the characters used by this fixture.
|
||||
let db = FontDb::with_font_dirs(vec![dir.clone()]);
|
||||
db.faces.write().unwrap().retain(|f| f.family == FALLBACK_FAMILY);
|
||||
let c = '\u{1e900}';
|
||||
assert!(db.fallback_for(c, 0, None).is_none());
|
||||
// A loaded non-chain face supplies Hangul while all installed Korean chain faces are absent.
|
||||
db.add_font(font_with("Earlier Korean Fallback", &['가']).unwrap());
|
||||
let chain_family = "Noto Serif CJK KR";
|
||||
assert!(!db.has_family(chain_family));
|
||||
assert_eq!(db.fallback_for('가', 0, Some("ko")).unwrap().family, "Earlier Korean Fallback");
|
||||
let family = "Installed After Glyph Miss";
|
||||
std::fs::write(dir.join("Later.ttf"), font_with(family, &[c]).unwrap()).unwrap();
|
||||
std::fs::write(dir.join("LaterKorean.ttf"), font_with(chain_family, &['가']).unwrap()).unwrap();
|
||||
// Before an explicit rescan, both fallbacks still use the previous catalog.
|
||||
assert!(db.fallback_for(c, 0, None).is_none());
|
||||
assert_eq!(db.fallback_for('가', 0, Some("ko")).unwrap().family, "Earlier Korean Fallback");
|
||||
assert_eq!(db.load_system_fonts(), 4);
|
||||
assert!(db.has_family(family) && db.has_family(chain_family));
|
||||
assert_eq!(db.fallback_for(c, 0, None).unwrap().family, family);
|
||||
assert_eq!(db.fallback_for('가', 0, Some("ko")).unwrap().family, chain_family);
|
||||
std::fs::remove_dir_all(dir).unwrap();
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
#[test]
|
||||
fn a_rescan_does_not_cache_an_older_in_flight_glyph_miss() {
|
||||
use crate::testing::font_with;
|
||||
use std::sync::{Barrier, mpsc};
|
||||
use std::time::Duration;
|
||||
|
||||
let dir = font_dir("rescan-in-flight-ttf");
|
||||
let db = Arc::new(FontDb::with_font_dirs(vec![dir.clone()]));
|
||||
// Optional craft-fonts must not supply the test glyph from an already loaded face.
|
||||
db.faces.write().unwrap().retain(|f| f.family == FALLBACK_FAMILY);
|
||||
let c = '\u{1e900}';
|
||||
let family = "Installed During Glyph Lookup";
|
||||
let font = font_with(family, &[c]).unwrap();
|
||||
assert!(db.read_faces().iter().all(|f| !f.covers(c)));
|
||||
assert_eq!(db.load_system_fonts(), 2);
|
||||
|
||||
let deadline = Duration::from_secs(10);
|
||||
let (reached_tx, reached_rx) = mpsc::sync_channel(1);
|
||||
let resume = Arc::new(Barrier::new(2));
|
||||
*db.fallback_snapshot_hook.lock().unwrap() = Some(FallbackSnapshotHook { reached: reached_tx, resume: resume.clone() });
|
||||
let (lookup_tx, lookup_rx) = mpsc::sync_channel(1);
|
||||
let lookup_db = db.clone();
|
||||
let lookup = std::thread::spawn(move || {
|
||||
let face = lookup_db.fallback_for(c, 0, None).map(|f| f.family.clone());
|
||||
let _ = lookup_tx.send(face);
|
||||
});
|
||||
reached_rx.recv_timeout(deadline).expect("the lookup must reach its old catalog snapshot");
|
||||
|
||||
// Finish the rescan while the old lookup is paused. A timeout also detects accidentally
|
||||
// holding sys/catalog locks at the hook; release the lookup before reporting that failure.
|
||||
let (scan_tx, scan_rx) = mpsc::sync_channel(1);
|
||||
let scan_db = db.clone();
|
||||
let scan = std::thread::spawn(move || {
|
||||
let result = std::fs::write(dir.join("Later.ttf"), font).map(|()| scan_db.load_system_fonts());
|
||||
let _ = scan_tx.send(result);
|
||||
});
|
||||
let scanned = scan_rx.recv_timeout(deadline);
|
||||
resume.wait();
|
||||
let old_face = lookup_rx.recv_timeout(deadline).expect("the old lookup must finish after release");
|
||||
lookup.join().unwrap();
|
||||
assert_eq!(scanned.expect("the rescan must finish before releasing the old lookup").unwrap(), 3);
|
||||
scan.join().unwrap();
|
||||
assert!(old_face.is_none(), "the paused lookup only searched the old paths");
|
||||
assert!(db.has_family(family));
|
||||
assert!(!db.is_loaded(family), "a rescan catalogs the font without loading it");
|
||||
// The old miss must not hide the new catalog entry; no second rescan is performed.
|
||||
assert_eq!(db.fallback_for(c, 0, None).unwrap().family, family);
|
||||
std::fs::remove_dir_all(db.font_dirs.first().unwrap()).unwrap();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user