diff --git a/crates/pstr-app/Cargo.toml b/crates/pstr-app/Cargo.toml index 63e7597..22b3eb9 100644 --- a/crates/pstr-app/Cargo.toml +++ b/crates/pstr-app/Cargo.toml @@ -25,6 +25,8 @@ tracing-subscriber.workspace = true eframe.workspace = true egui.workspace = true egui-phosphor.workspace = true +# The parser egui reads fonts with, to check a system font before handing it over. +skrifa.workspace = true image.workspace = true sha2.workspace = true parking_lot.workspace = true @@ -39,8 +41,6 @@ tao.workspace = true [build-dependencies] embed-resource.workspace = true -[dev-dependencies] -skrifa.workspace = true [target.'cfg(target_os = "linux")'.dependencies] dbus.workspace = true diff --git a/crates/pstr-app/src/theme.rs b/crates/pstr-app/src/theme.rs index ca4e779..a595d94 100644 --- a/crates/pstr-app/src/theme.rs +++ b/crates/pstr-app/src/theme.rs @@ -248,15 +248,39 @@ fn install_cjk_fallback(ctx: &egui::Context) { let Some((path, index)) = system_cjk_font() else { return; }; - let Ok(bytes) = std::fs::read(path) else { + let Ok(bytes) = std::fs::read(&path) else { + return; + }; + let Some(data) = cjk_font_data(bytes, index) else { + tracing::debug!("CJK fallback {}#{index} does not parse", path.display()); return; }; - let mut data = egui::FontData::from_owned(bytes); - data.index = index; ctx.set_fonts(font_definitions(Some(data))); ctx.request_repaint(); } +/// The fallback font, if egui can read it. +/// +/// egui panics on a font it cannot parse, and this one comes from whatever the +/// system has installed. So it is parsed here first, the same way egui does, +/// and a font that fails is left out rather than taking the app down. +fn cjk_font_data(bytes: Vec, index: u32) -> Option { + skrifa::FontRef::from_index(&bytes, index).ok()?; + let mut data = egui::FontData::from_owned(bytes); + data.index = index; + Some(data) +} + +/// The face within a font collection that fontconfig's `%{index}` names. +/// +/// Fontconfig packs a variable font's named instance into the high 16 bits — +/// Fedora's variable Noto Sans CJK answers 262144, instance 4 of face 0. The +/// instance is not a face, and passing it to egui as one stopped the app at +/// start. +fn fontconfig_face(index: u32) -> u32 { + index & 0xFFFF +} + #[cfg(target_os = "linux")] fn system_cjk_font() -> Option<(std::path::PathBuf, u32)> { let output = std::process::Command::new("fc-match") @@ -269,6 +293,7 @@ fn system_cjk_font() -> Option<(std::path::PathBuf, u32)> { let index = lines .next() .and_then(|value| value.parse().ok()) + .map(fontconfig_face) .unwrap_or(0); path.is_file().then_some((path, index)) } @@ -855,6 +880,20 @@ const RAMP_STOPS: usize = 64; mod tests { use super::*; + #[test] + fn a_variable_fonts_named_instance_is_not_taken_for_a_face() { + assert_eq!(fontconfig_face(262_144), 0); + assert_eq!(fontconfig_face(0x0002_0001), 1); + assert_eq!(fontconfig_face(2), 2); + } + + #[test] + fn a_fallback_font_egui_cannot_read_is_left_out() { + assert!(cjk_font_data(INTER.to_vec(), 0).is_some()); + assert!(cjk_font_data(INTER.to_vec(), 4).is_none()); + assert!(cjk_font_data(b"not a font".to_vec(), 0).is_none()); + } + #[test] fn the_bundled_inter_leaves_the_icon_codepoints_to_phosphor() { use skrifa::MetadataProvider as _; diff --git a/docs/BUGS.md b/docs/BUGS.md index 2347860..8f7607d 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -145,6 +145,27 @@ and [B44](#b44--r8-broke-the-app-under-instrumentation-in-five-places). ## Fixed +### B65 — the desktop app quit at start on a variable CJK font + +**Symptom.** On Secureblue (Fedora Atomic) the window showed for a moment and +the process panicked: `Error parsing "system-cjk-fallback" TTF/OTF font file: +Invalid index 262144 for font collection` +([#1](https://github.com/narrrl/proton-stream/issues/1)). + +**Cause.** The CJK fallback is whatever `fc-match "Noto Sans CJK JP"` names, +with the face taken from `%{index}`. For a variable font fontconfig packs the +named instance into that number's high 16 bits; Fedora ships Noto Sans CJK as a +variable font, so it answered 262144 — instance 4 of face 0. egui took it as a +face index, found no such face, and panics on a font it cannot parse. + +**Fix.** `theme.rs` keeps only the low 16 bits, the face. And the fallback is +parsed with skrifa, as egui parses it, before it is handed over; one that fails +is logged and left out, so no installed font can stop the app again. + +**Verified.** Workspace gate, with +`a_variable_fonts_named_instance_is_not_taken_for_a_face` and +`a_fallback_font_egui_cannot_read_is_left_out`. Not yet run on Secureblue. + ### B64 — Android offline downloads looked stuck **Symptom.** A queued episode, and more so a queued season, sat at a progress