From 0a526c6f85ada7de1c9344e2c999d26383065b16 Mon Sep 17 00:00:00 2001 From: bherbruck Date: Mon, 12 Oct 2026 13:24:31 +0000 Subject: [PATCH] fix(i18n): keep saved language on font failure, tighten CJK font matching - Web: a failed CJK font fetch shows English for this session only; the saved language preference is kept (CjkFontLoader, persisted on save). - Web fetches the CJK font only when a CJK language (ja) is selected. - fonts.rs: directory scans match CJK-specific names only (no "noto" or "gothic"); CJK stays the last fallback after the Latin UI font. - Revert unrelated clippy rewrites in component_clone.rs, coplanar.rs, canvas.rs and issue_report.rs to match main. - Tests: default English, fallback order, CJK-only matching, fetch only for CJK, failed fetch keeps the saved language. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RjGw4gA5ZvKrU8MQP6sBgT --- apps/solvecraft-web/src/main.rs | 82 +++----- crates/engine/src/cmd/component_clone.rs | 20 +- crates/kernel/src/coplanar.rs | 2 +- crates/ui-egui/src/canvas.rs | 9 +- crates/ui-egui/src/fonts.rs | 238 +++++++++++++++++++++-- crates/ui-egui/src/issue_report.rs | 2 +- docs/localization.md | 4 +- 7 files changed, 275 insertions(+), 82 deletions(-) diff --git a/apps/solvecraft-web/src/main.rs b/apps/solvecraft-web/src/main.rs index 5fac03ba..20bee57f 100644 --- a/apps/solvecraft-web/src/main.rs +++ b/apps/solvecraft-web/src/main.rs @@ -48,14 +48,6 @@ mod web { const JAPANESE_FONT_PATH: &str = "./fonts/BIZUDPGothic-Regular.ttf"; - #[derive(Clone, Copy, Debug, PartialEq, Eq)] - enum FontStatus { - Unloaded, - Loading, - Installed, - Failed, - } - type FontLoadResult = Rc, String>>>>; struct Shell { @@ -70,7 +62,8 @@ mod web { rename_focus: bool, /// The start page's recent designs were filled from the stored ones. recents_synced: bool, - font_status: FontStatus, + /// On-demand CJK font: fetched only when a CJK language is selected. + font: solvecraft_ui_egui::fonts::CjkFontLoader, font_result: FontLoadResult, } @@ -319,51 +312,34 @@ mod web { files::autosave(&self.files, None); } - /// Check language preference and manage on-demand Japanese font loading. + /// Fetch and install the CJK font when a CJK language is selected. If that fails, this + /// session falls back to English (no tofu); the saved language preference is kept. fn tick_font(&mut self, ctx: &egui::Context) { - if self.font_status == FontStatus::Loading { - if let Some(res) = self.font_result.borrow_mut().take() { - match res { - Ok(bytes) => { - if solvecraft_ui_egui::fonts::install_cjk_font(ctx, bytes) { - log::info!("loaded web Japanese font from {JAPANESE_FONT_PATH}"); - self.font_status = FontStatus::Installed; - ctx.request_repaint(); - } else { - log::warn!("failed to install Japanese font: invalid font data; falling back to English"); - self.font_status = FontStatus::Failed; - self.app.preferences.language = "en".to_string(); - ctx.request_repaint(); - } - } - Err(err) => { - log::warn!("failed to fetch Japanese font from {JAPANESE_FONT_PATH}: {err}; falling back to English"); - self.font_status = FontStatus::Failed; - self.app.preferences.language = "en".to_string(); - ctx.request_repaint(); - } + let done = self.font_result.borrow_mut().take(); + if let Some(res) = done { + let installed = match res { + Ok(bytes) => solvecraft_ui_egui::fonts::install_cjk_font(ctx, bytes), + Err(err) => { + log::warn!("failed to fetch CJK font from {JAPANESE_FONT_PATH}: {err}"); + false } + }; + if installed { + log::info!("loaded web CJK font from {JAPANESE_FONT_PATH}"); + } else { + log::warn!("CJK font unavailable; showing English for this session"); } + self.font.finished(installed, &mut self.app.preferences.language); + ctx.request_repaint(); } - - if self.app.preferences.language == "ja" { - match self.font_status { - FontStatus::Unloaded => { - self.font_status = FontStatus::Loading; - let slot = self.font_result.clone(); - let ctx_clone = ctx.clone(); - wasm_bindgen_futures::spawn_local(async move { - let res = fetch_font_bytes(JAPANESE_FONT_PATH).await; - *slot.borrow_mut() = Some(res); - ctx_clone.request_repaint(); - }); - } - FontStatus::Failed => { - // Prevent tofu glyphs if font loading failed - self.app.preferences.language = "en".to_string(); - } - FontStatus::Loading | FontStatus::Installed => {} - } + if self.font.tick(&mut self.app.preferences.language) { + let slot = self.font_result.clone(); + let ctx = ctx.clone(); + wasm_bindgen_futures::spawn_local(async move { + let res = fetch_font_bytes(JAPANESE_FONT_PATH).await; + *slot.borrow_mut() = Some(res); + ctx.request_repaint(); + }); } } } @@ -386,7 +362,11 @@ mod web { self.recovery_prompt(&ctx); } fn save(&mut self, storage: &mut dyn eframe::Storage) { + // Persist the user's own language, not a session-only English fallback. + let saved = self.font.persisted_language(&self.app.preferences.language).to_owned(); + let live = std::mem::replace(&mut self.app.preferences.language, saved); storage.set_string(PREFS_KEY, self.app.prefs()); + self.app.preferences.language = live; } } @@ -475,7 +455,7 @@ mod web { renaming: None, rename_focus: false, recents_synced: false, - font_status: FontStatus::Unloaded, + font: Default::default(), font_result: Rc::new(std::cell::RefCell::new(None)), })) }), diff --git a/crates/engine/src/cmd/component_clone.rs b/crates/engine/src/cmd/component_clone.rs index f5e3ab5f..403b928f 100644 --- a/crates/engine/src/cmd/component_clone.rs +++ b/crates/engine/src/cmd/component_clone.rs @@ -515,16 +515,20 @@ pub(super) fn paste( | L::Body { body } | L::Vertex { body, .. } | L::Iso { body, .. } - | L::Spun { body, .. } - if !bodies.contains_key(body) => - { - notices.push(format!("{} retains external linked body {body}", feature.name)); + | L::Spun { body, .. } => { + if !bodies.contains_key(body) { + notices.push(format!("{} retains external linked body {body}", feature.name)); + } } - L::SketchCurve { sketch, .. } | L::SketchPoint { sketch, .. } if !features.contains_key(sketch) => { - notices.push(format!("{} retains external linked sketch {sketch}", feature.name)); + L::SketchCurve { sketch, .. } | L::SketchPoint { sketch, .. } => { + if !features.contains_key(sketch) { + notices.push(format!("{} retains external linked sketch {sketch}", feature.name)); + } } - L::OnSurface { sketch, body, .. } if !features.contains_key(sketch) || !bodies.contains_key(body) => { - notices.push(format!("{} retains external surface link", feature.name)); + L::OnSurface { sketch, body, .. } => { + if !features.contains_key(sketch) || !bodies.contains_key(body) { + notices.push(format!("{} retains external surface link", feature.name)); + } } L::Intersection { a, b } => { pending.push(a); diff --git a/crates/kernel/src/coplanar.rs b/crates/kernel/src/coplanar.rs index 10b73cd9..256ea52b 100644 --- a/crates/kernel/src/coplanar.rs +++ b/crates/kernel/src/coplanar.rs @@ -508,7 +508,7 @@ pub(crate) fn bore_union(a: &Body, b: &Body, accept: &dyn Fn(&Body) -> bool) -> let Some(radial) = (q - along).normalized() else { continue }; let on = along + radial * c.radius; let e = (c.radius * 0.05).max(size * 1e-4); - if !inside.contains(on + radial * e) || inside.contains(on - radial * e) { + if !(inside.contains(on + radial * e) && !inside.contains(on - radial * e)) { continue; } // The cylinder over the tool's length holds no material of either body. diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 9f4bbcae..a9c488a2 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -217,8 +217,13 @@ fn draw(app: &SolveApp, ui: &mut egui::Ui, painter: &egui::Painter, proj: &Proj, None }; let sign = match (to_span, &d.kind) { - (Some(span), _) if span < 0.0 => -1.0, - (Some(_), _) => 1.0, + (Some(span), _) => { + if span < 0.0 { + -1.0 + } else { + 1.0 + } + } (_, Kind::Extrude { direction: 1, .. }) => -1.0, _ => 1.0, }; diff --git a/crates/ui-egui/src/fonts.rs b/crates/ui-egui/src/fonts.rs index 936f207b..7e61c0d5 100644 --- a/crates/ui-egui/src/fonts.rs +++ b/crates/ui-egui/src/fonts.rs @@ -1,18 +1,142 @@ +//! System font loading and fallback setup for multi-language (CJK/Japanese) support. +//! +//! The UI's Latin font always comes first; a CJK font is only ever appended as a fallback, so +//! English text renders exactly as before and only glyphs the Latin font lacks use the CJK face. + use std::sync::Arc; +/// Name of the CJK fallback entry in [`egui::FontDefinitions`]. +pub const CJK_FALLBACK: &str = "cjk_fallback"; + +/// UI languages whose text needs the CJK font. The web app's bundled font covers Japanese only; +/// add Chinese or Korean here together with a font that covers them. +pub const CJK_LANGUAGES: &[&str] = &["ja"]; + +/// Whether the UI language needs a CJK font (the web app fetches one only then). +pub fn needs_cjk_font(language: &str) -> bool { + CJK_LANGUAGES.contains(&language) +} + +/// `fonts` with `bytes` appended as the last fallback of the Proportional and Monospace +/// families, after the Latin UI font. `None` when the bytes are not a font. +pub fn with_cjk_fallback(mut fonts: egui::FontDefinitions, bytes: Vec) -> Option { + ttf_parser::Face::parse(&bytes, 0).ok()?; + fonts.font_data.insert(CJK_FALLBACK.to_owned(), Arc::new(egui::FontData::from_owned(bytes))); + for family in [egui::FontFamily::Proportional, egui::FontFamily::Monospace] { + let list = fonts.families.entry(family).or_default(); + list.retain(|name| name != CJK_FALLBACK); + list.push(CJK_FALLBACK.to_owned()); + } + Some(fonts) +} + /// Install font bytes as fallback for Proportional and Monospace font families. /// Returns true if the font was parsed and installed successfully. pub fn install_cjk_font(ctx: &egui::Context, bytes: Vec) -> bool { - if ttf_parser::Face::parse(&bytes, 0).is_err() { - return false; + match with_cjk_fallback(egui::FontDefinitions::default(), bytes) { + Some(fonts) => { + ctx.set_fonts(fonts); + true + } + None => false, + } +} + +/// Lower-case file-name fragments that identify CJK fonts. Deliberately specific: generic +/// words such as "noto" or "gothic" also match Latin-only fonts (NotoSans-Regular, Century Gothic). +pub const CJK_FONT_KEYWORDS: &[&str] = &[ + "cjk", + "notosansjp", + "notoserifjp", + "bizud", + "biz-ud", + "yugoth", + "meiryo", + "msgothic", + "hiragino", + "pingfang", + "songti", + "arial unicode", + "ipag", + "ipaexg", + "takao", + "vlgothic", + "droidsansfallback", +]; + +/// Whether a font file name looks like a CJK font. +pub fn is_cjk_font_name(file_name: &str) -> bool { + let name = file_name.to_ascii_lowercase(); + CJK_FONT_KEYWORDS.iter().any(|k| name.contains(k)) +} + +/// Session state of the web app's on-demand CJK font. Pure state so it can be tested off the web: +/// the shell does the fetch when [`CjkFontLoader::tick`] asks for it. +#[derive(Debug, Default)] +pub struct CjkFontLoader { + status: CjkFontStatus, + /// The saved language while this session falls back to English after a failed load. + saved_language: Option, +} + +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub enum CjkFontStatus { + #[default] + Unloaded, + Loading, + Installed, + Failed, +} + +impl CjkFontLoader { + pub fn status(&self) -> CjkFontStatus { + self.status } - let mut fonts = egui::FontDefinitions::default(); - fonts.font_data.insert("cjk_fallback".to_owned(), Arc::new(egui::FontData::from_owned(bytes))); - fonts.families.entry(egui::FontFamily::Proportional).or_default().push("cjk_fallback".to_owned()); - fonts.families.entry(egui::FontFamily::Monospace).or_default().push("cjk_fallback".to_owned()); - ctx.set_fonts(fonts); - true + /// Called every frame with the live language. Returns true when the font fetch should start + /// now (only for a CJK language, only once). After a failed load a CJK language falls back to + /// English for this session; the saved preference is kept (see [`Self::persisted_language`]). + pub fn tick(&mut self, language: &mut String) -> bool { + if self.saved_language.is_some() && language != "en" { + // The user picked a language themselves since the fallback: that is the preference now. + self.saved_language = None; + } + if !needs_cjk_font(language) { + return false; + } + match self.status { + CjkFontStatus::Unloaded => { + self.status = CjkFontStatus::Loading; + true + } + CjkFontStatus::Failed => { + self.fall_back(language); + false + } + CjkFontStatus::Loading | CjkFontStatus::Installed => false, + } + } + + /// Record the outcome of the fetch and install. + pub fn finished(&mut self, installed: bool, language: &mut String) { + if installed { + self.status = CjkFontStatus::Installed; + } else { + self.status = CjkFontStatus::Failed; + if needs_cjk_font(language) { + self.fall_back(language); + } + } + } + + fn fall_back(&mut self, language: &mut String) { + self.saved_language = Some(std::mem::replace(language, "en".to_owned())); + } + + /// The language to write to storage: the user's own choice, not this session's fallback. + pub fn persisted_language<'a>(&'a self, live: &'a str) -> &'a str { + self.saved_language.as_deref().unwrap_or(live) + } } /// Candidate font paths to look for on different operating systems. @@ -72,7 +196,7 @@ fn try_read_font(path: &std::path::Path) -> Option> { /// Recursively search for matching CJK font files up to a bounded depth. #[cfg(not(target_arch = "wasm32"))] -fn scan_dir_for_font(dir: &std::path::Path, keywords: &[&str], depth_remaining: usize) -> Option> { +fn scan_dir_for_font(dir: &std::path::Path, depth_remaining: usize) -> Option> { if depth_remaining == 0 { return None; } @@ -85,7 +209,7 @@ fn scan_dir_for_font(dir: &std::path::Path, keywords: &[&str], depth_remaining: let name = p.file_name().map(|n| n.to_string_lossy().to_ascii_lowercase()).unwrap_or_default(); let is_font = name.ends_with(".ttc") || name.ends_with(".ttf") || name.ends_with(".otf"); if is_font - && keywords.iter().any(|k| name.contains(k)) + && is_cjk_font_name(&name) && let Some(bytes) = try_read_font(&p) { return Some(bytes); @@ -96,7 +220,7 @@ fn scan_dir_for_font(dir: &std::path::Path, keywords: &[&str], depth_remaining: } for sub in subdirs { - if let Some(bytes) = scan_dir_for_font(&sub, keywords, depth_remaining.saturating_sub(1)) { + if let Some(bytes) = scan_dir_for_font(&sub, depth_remaining.saturating_sub(1)) { return Some(bytes); } } @@ -139,7 +263,7 @@ fn find_system_cjk_font() -> Option> { } if let Ok(local) = std::env::var("LOCALAPPDATA") { let user_fonts = std::path::PathBuf::from(local).join("Microsoft").join("Windows").join("Fonts"); - if let Some(bytes) = scan_dir_for_font(&user_fonts, &["biz", "gothic", "noto", "yugoth"], 2) { + if let Some(bytes) = scan_dir_for_font(&user_fonts, 2) { return Some(bytes); } } @@ -148,9 +272,8 @@ fn find_system_cjk_font() -> Option> { #[cfg(target_os = "macos")] { let dirs = ["/System/Library/Fonts", "/System/Library/Fonts/Supplemental", "/Library/Fonts"]; - let keywords = ["hiragino", "pingfang", "arial unicode", "songti", "gothic"]; for d in dirs { - if let Some(bytes) = scan_dir_for_font(std::path::Path::new(d), &keywords, 2) { + if let Some(bytes) = scan_dir_for_font(std::path::Path::new(d), 2) { return Some(bytes); } } @@ -159,15 +282,14 @@ fn find_system_cjk_font() -> Option> { #[cfg(target_os = "linux")] { let search_dirs = [std::path::PathBuf::from("/usr/share/fonts"), std::path::PathBuf::from("/usr/local/share/fonts")]; - let keywords = ["notosanscjk", "noto", "cjk", "takao", "vlgothic", "ipa", "droid"]; for dir in &search_dirs { - if let Some(bytes) = scan_dir_for_font(dir, &keywords, 3) { + if let Some(bytes) = scan_dir_for_font(dir, 3) { return Some(bytes); } } if let Ok(home) = std::env::var("HOME") { let user_fonts = std::path::PathBuf::from(home).join(".local/share/fonts"); - if let Some(bytes) = scan_dir_for_font(&user_fonts, &keywords, 2) { + if let Some(bytes) = scan_dir_for_font(&user_fonts, 2) { return Some(bytes); } } @@ -188,3 +310,85 @@ pub fn setup_fonts(ctx: &egui::Context) { /// Use asynchronous loading through [`solvecraft_web`] or host scripts. #[cfg(target_arch = "wasm32")] pub fn setup_fonts(_ctx: &egui::Context) {} + +#[cfg(test)] +mod tests { + use super::*; + + fn latin_font_bytes() -> Vec { + let defaults = egui::FontDefinitions::default(); + let name = &defaults.families[&egui::FontFamily::Proportional][0]; + defaults.font_data[name].font.to_vec() + } + + #[test] + fn default_ui_language_is_english() { + assert_eq!(crate::prefs::Prefs::default().language, "en"); + assert!(!needs_cjk_font("en")); + } + + #[test] + fn cjk_font_is_a_fallback_after_the_latin_ui_font() { + let defaults = egui::FontDefinitions::default(); + let fonts = with_cjk_fallback(defaults.clone(), latin_font_bytes()).unwrap(); + for family in [egui::FontFamily::Proportional, egui::FontFamily::Monospace] { + let list = &fonts.families[&family]; + assert_eq!(list.first(), defaults.families[&family].first(), "the Latin UI font stays first"); + assert_eq!(list.last().map(String::as_str), Some(CJK_FALLBACK), "CJK is the last fallback"); + } + assert!(with_cjk_fallback(defaults, b"not a font".to_vec()).is_none()); + } + + #[test] + fn directory_scan_matches_cjk_fonts_only() { + for cjk in [ + "NotoSansCJK-Regular.ttc", + "NotoSansJP-Regular.ttf", + "BIZUDPGothic-Regular.ttf", + "YuGothM.ttc", + "meiryo.ttc", + "msgothic.ttc", + "Hiragino Sans W3.ttc", + "ipag.ttf", + "TakaoPGothic.ttf", + "DroidSansFallbackFull.ttf", + ] { + assert!(is_cjk_font_name(cjk), "{cjk} is a CJK font"); + } + for latin in ["NotoSans-Regular.ttf", "NotoSerif-Bold.ttf", "NotoMono-Regular.ttf", "GOTHIC.TTF", "DejaVuSans.ttf", "DroidSans.ttf"] { + assert!(!is_cjk_font_name(latin), "{latin} is not a CJK font"); + } + } + + #[test] + fn web_font_is_fetched_only_for_a_cjk_language() { + let mut loader = CjkFontLoader::default(); + for lang in ["en", "de"] { + let mut language = lang.to_owned(); + assert!(!loader.tick(&mut language), "no CJK font fetch for {lang}"); + } + let mut language = "ja".to_owned(); + assert!(loader.tick(&mut language), "Japanese starts the fetch"); + assert!(!loader.tick(&mut language), "the fetch starts once"); + loader.finished(true, &mut language); + assert_eq!(loader.status(), CjkFontStatus::Installed); + assert_eq!(language, "ja"); + } + + #[test] + fn failed_font_fetch_falls_back_for_the_session_without_changing_the_saved_language() { + let mut loader = CjkFontLoader::default(); + let mut language = "ja".to_owned(); + assert!(loader.tick(&mut language)); + loader.finished(false, &mut language); + assert_eq!(language, "en", "this session shows English instead of tofu"); + assert_eq!(loader.persisted_language(&language), "ja", "the saved preference stays Japanese"); + assert!(!loader.tick(&mut language)); + assert_eq!(loader.persisted_language(&language), "ja"); + + // Choosing a language afterwards makes that the saved preference. + language = "de".to_owned(); + assert!(!loader.tick(&mut language)); + assert_eq!(loader.persisted_language(&language), "de"); + } +} diff --git a/crates/ui-egui/src/issue_report.rs b/crates/ui-egui/src/issue_report.rs index 7e3359ee..8c1aa870 100644 --- a/crates/ui-egui/src/issue_report.rs +++ b/crates/ui-egui/src/issue_report.rs @@ -400,7 +400,7 @@ pub(crate) fn tick(app: &mut SolveApp, ctx: &egui::Context) { } pub(crate) fn receive(app: &mut SolveApp, token: u64, image: Arc) { - if app.issue_report.pending.as_ref().is_none_or(|p| p.token != token) { + if !app.issue_report.pending.as_ref().is_some_and(|p| p.token == token) { return; } let stale = app.issue_report.pending.as_ref().is_some_and(|p| p.signature != signature(app, app.dialog.as_ref())); diff --git a/docs/localization.md b/docs/localization.md index b0fb9c0d..df90c115 100644 --- a/docs/localization.md +++ b/docs/localization.md @@ -39,8 +39,8 @@ In the browser, `solvecraft-web` bundles a self-hosted Japanese font (`BIZUDPGot under SIL OFL 1.1) within the web distribution artifacts only. The font is loaded asynchronously from the same origin (`./fonts/BIZUDPGothic-Regular.ttf`) on-demand when the interface language is set to Japanese, without contacting any external CDNs or third-party servers. For users with an English interface, no font download -occurs. If font loading fails, the web interface gracefully falls back to English to prevent missing glyphs -(tofu). This bundled font specifically targets Japanese; Korean (Hangul) and Chinese scripts are not covered. +occurs. If font loading fails, the web interface shows English for that session only to prevent missing +glyphs (tofu); the saved language preference is kept, so the next visit tries again. This bundled font specifically targets Japanese; Korean (Hangul) and Chinese scripts are not covered. The current messages support named substitution, not locale-specific plural selection, number/date formatting, or right-to-left layout and shaping. Add those capabilities when