From f5b9bab01625f8e1a04cb1fd6bd1da655e5f11d0 Mon Sep 17 00:00:00 2001 From: moppeeer Date: Mon, 12 Oct 2026 10:03:50 +0200 Subject: [PATCH] fix(ui): Bounded font previews; a family missing from the read is read once - Previews were kept in the context's data for each sample text, size and colour, and never let go: every new text selection loaded textures for every visible row. They now live in a cache of at most 256 that drops the least recently used. - A family the background read didn't list (installed since) was read from disk on the UI thread every frame and never kept; it is now read once and stored with the others. - Return and Escape in a dialog's font menu are covered by #447's test on main; this branch's duplicate is dropped. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/font_menu.rs | 74 ++++++++++++++++++++++++-- crates/ui-egui/src/panels/mod.rs | 36 ------------- 2 files changed, 69 insertions(+), 41 deletions(-) diff --git a/crates/ui-egui/src/panels/font_menu.rs b/crates/ui-egui/src/panels/font_menu.rs index 7a50791c9..fa86f925f 100644 --- a/crates/ui-egui/src/panels/font_menu.rs +++ b/crates/ui-egui/src/panels/font_menu.rs @@ -226,16 +226,51 @@ fn preview( let ppp = ui.ctx().pixels_per_point(); let px = (size * ppp).round() as u32; let key = egui::Id::new(("font_preview", family, style, scope, text, px, color.to_array())); - if let Some(t) = ui.data(|d| d.get_temp::(key)) { + let cache_id = egui::Id::new("font_previews"); + if let Some(t) = ui.data_mut(|d| d.get_temp_mut_or_default::(cache_id).get(key)) { return t; } let img = designcraft_render::glyphs::text_line(fonts, family, style, text, px, color.to_array()); let ci = egui::ColorImage::from_rgba_premultiplied([img.width as usize, img.height as usize], &img.pixels); let t = ui.ctx().load_texture(format!("font_preview_{family}_{style}"), ci, egui::TextureOptions::LINEAR); - ui.data_mut(|d| d.insert_temp(key, t.clone())); + ui.data_mut(|d| d.get_temp_mut_or_default::(cache_id).insert(key, t.clone())); t } +/// Font menu previews kept for reuse, at most [`Previews::CAP`]: each sample text, size and colour +/// renders new ones, so the oldest are let go (their textures freed) as new ones come in. +#[derive(Clone, Default)] +struct Previews { + map: std::collections::HashMap, + order: std::collections::VecDeque, +} + +impl Previews { + /// About two screens of rows in each of two sample sizes. + const CAP: usize = 256; + + fn get(&mut self, key: egui::Id) -> Option { + let t = self.map.get(&key).cloned()?; + // Most recently used last. + if let Some(i) = self.order.iter().position(|k| *k == key) { + self.order.remove(i); + } + self.order.push_back(key); + Some(t) + } + + fn insert(&mut self, key: egui::Id, t: egui::TextureHandle) { + if self.map.insert(key, t).is_none() { + self.order.push_back(key); + } + while self.order.len() > Self::CAP { + if let Some(old) = self.order.pop_front() { + self.map.remove(&old); + } + } + } +} + /// A small toggle in the filter row (an icon), with its tooltip. fn toggle(ui: &mut Ui, on: bool, tip: &str, paint: impl FnOnce(&egui::Painter, Rect, Color32)) -> bool { let t = Tokens::get(ui.ctx()); @@ -389,10 +424,18 @@ pub fn font_menu_body(app: &mut DesignApp, ui: &mut Ui, menu: &[FamilyInfo], cur let mut loading = false; if filtered { let shared = kinds(ui.ctx(), &fonts, scope); - let k = lock(&shared); + let mut k = lock(&shared); loading = !k.done; - // A family the read didn't list (just activated) is read here, once the read is done. - let kind = |f: &str| k.of.get(f).copied().or_else(|| k.done.then(|| family_kind(&fonts, f))); + // A family the read didn't list (installed or activated since) is read here once, after the + // read is done, and kept. + if k.done { + let missing: Vec = families.iter().filter(|f| !k.of.contains_key(&f.family)).map(|f| f.family.clone()).collect(); + for f in missing { + let v = family_kind(&fonts, &f); + k.of.insert(f, v); + } + } + let kind = |f: &str| k.of.get(f).copied(); families.retain(|f| kind(&f.family).is_some_and(|(kind, added)| st.class.is_none_or(|c| kind.class == c) && (!st.only_recent || added))); if let Some(like) = &st.similar { let base = meta(ui, &fonts, scope, like).kind; @@ -620,3 +663,24 @@ pub fn font_menu_body(app: &mut DesignApp, ui: &mut Ui, menu: &[FamilyInfo], cur } pick } + +#[cfg(test)] +mod preview_cache_tests { + use super::Previews; + + #[test] + fn the_preview_cache_keeps_at_most_its_cap_and_drops_the_oldest() { + let ctx = egui::Context::default(); + let tex = |i: usize| ctx.load_texture(format!("t{i}"), egui::ColorImage::new([1, 1], vec![egui::Color32::WHITE]), Default::default()); + let mut c = Previews::default(); + let key = |i: usize| egui::Id::new(("p", i)); + for i in 0..Previews::CAP + 40 { + c.insert(key(i), tex(i)); + // The first one stays in use, so it is kept. + assert!(c.get(key(0)).is_some(), "{i}"); + } + assert_eq!((c.map.len(), c.order.len()), (Previews::CAP, Previews::CAP)); + assert!(c.get(key(1)).is_none(), "the oldest unused one was let go"); + assert!(c.get(key(Previews::CAP + 39)).is_some()); + } +} diff --git a/crates/ui-egui/src/panels/mod.rs b/crates/ui-egui/src/panels/mod.rs index 6ced52c72..bc023a28d 100644 --- a/crates/ui-egui/src/panels/mod.rs +++ b/crates/ui-egui/src/panels/mod.rs @@ -514,42 +514,6 @@ mod font_menu_ui_tests { assert!(shown.len() >= 2, "{shown:?}"); } - #[test] - fn return_and_escape_in_a_dialogs_font_menu_stay_in_the_menu() { - let paragraph = json!({"name": designcraft_doc::BASIC_PARAGRAPH, "section": "chars"}); - for (what, id, fields, shown) in [ - ("Paragraph Style Options", "paragraphStyleOptions", paragraph, designcraft_fonts::DEFAULT_FAMILY), - ("Find/Replace Font", "findFont", json!({}), "—"), - ] { - let app = crate::DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); - let mut h = test_window::open(app, vec2(1440.0, 900.0)); - h.state_mut().app.ui.dialog = Some(crate::dialogs::Dialog::new(id, fields)); - h.run_steps(4); - let dialog = |h: &Harness<'static, Window>| h.state().app.ui.dialog.as_ref().map(|d| d.id.clone()); - let combo = |h: &Harness<'static, Window>, family: &str| { - h.get_by(|n| n.role() == egui::accesskit::Role::ComboBox && n.value().as_deref() == Some(family)).rect().center() - }; - let at = combo(&h, shown); - click_at(&mut h, at); - assert!(h.ctx.text_edit_focused(), "{what}: the menu opens with its search focused"); - h.event(egui::Event::Text("source sans".into())); - h.run_steps(4); - h.key_press(egui::Key::ArrowDown); - h.run_steps(2); - h.key_press(egui::Key::Enter); - h.run_steps(4); - assert_eq!(dialog(&h).as_deref(), Some(id), "{what}: Return picked in the menu and left the dialog open"); - assert!(!h.ctx.text_edit_focused(), "{what}: the menu closed"); - let at = combo(&h, "Source Sans 3"); - click_at(&mut h, at); - assert!(h.ctx.text_edit_focused(), "{what}: the menu opened again"); - h.key_press(egui::Key::Escape); - h.run_steps(4); - assert!(!h.ctx.text_edit_focused(), "{what}: Escape closed the menu"); - assert_eq!(dialog(&h).as_deref(), Some(id), "{what}: Escape in the menu left the dialog open"); - } - } - #[test] fn typing_in_the_font_menu_searches_and_never_reaches_the_story() { let (mut h, story) = typing();