From eb2f3ba71a97c98a25b4727f4ff7eb05bffad8ff Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Fri, 13 Nov 2026 05:41:46 -0800 Subject: [PATCH] Tests: the font menu scroll bar test lists fonts of its own font_menu::tests::the_scroll_bar_leaves_the_stars_clear starred every family in the process-wide font database and listed that database, which tests running alongside add fonts and composite fonts to. In full parallel runs it sometimes failed with "8 families, no row stars drawn". With the four bundled families alone it never reached the scroll bar check, which ran only for seven or more families. - The menu's header takes the font database it lists and hands it to the rows and the name-width measurement. The app passes the process-wide database as before. - Each FontDb has an id of its own (FontDb::id), and the widest family name the menu keeps is stored with that id as well as the generation, so a width measured for one database is not used for another at the same generation. - The test lists a database of the bundled fonts, stars its four families, opens on Inter, and runs the frames the scroll bar takes to appear. It checks that the rows overflow the list's scroll area, that a row star is drawn inside that area, and that each star stays left of the scroll bar's column (#2277). With the list's scroll bar set to floating, the test fails with a star under the bar. Without the id in the widest name's key, each_font_database_has_its_own_widest_name fails. --- crates/text/src/fontdb.rs | 2 + crates/text/src/fontdb/db.rs | 10 ++++ crates/ui-egui/src/font_menu.rs | 66 ++++++++++++++---------- crates/ui-egui/src/font_menu/tests.rs | 74 ++++++++++++++++++--------- 4 files changed, 101 insertions(+), 51 deletions(-) diff --git a/crates/text/src/fontdb.rs b/crates/text/src/fontdb.rs index 9e18b2a86c..ace69a4148 100644 --- a/crates/text/src/fontdb.rs +++ b/crates/text/src/fontdb.rs @@ -105,6 +105,8 @@ pub struct FontDb { family_cache: Mutex>>, menu_cache: Mutex>, generation: AtomicU64, + /// This database's [`FontDb::id`]. + id: u64, /// System fallback state: characters no system font covers. #[cfg(not(target_arch = "wasm32"))] sys: Mutex, diff --git a/crates/text/src/fontdb/db.rs b/crates/text/src/fontdb/db.rs index 4ef17c40c6..0e28d57528 100644 --- a/crates/text/src/fontdb/db.rs +++ b/crates/text/src/fontdb/db.rs @@ -21,6 +21,9 @@ use super::{Arc, AtomicU64, FontDb, FontPaths, HashMap, Mutex, Ordering, Path, P #[cfg(not(target_arch = "wasm32"))] use super::{ScanStamp, modified}; +/// The next [`FontDb::id`]. +static NEXT_ID: AtomicU64 = AtomicU64::new(0); + impl FontDb { /// A database holding the bundled fonts, whose system font scan reads `font_dirs` (folders, /// read with their subfolders, and font files). @@ -64,6 +67,7 @@ impl FontDb { family_cache: Mutex::new(None), menu_cache: Mutex::new(None), generation: AtomicU64::new(0), + id: NEXT_ID.fetch_add(1, Ordering::Relaxed), #[cfg(not(target_arch = "wasm32"))] sys: Mutex::new(SysFallback { enabled: true, ..Default::default() }), #[cfg(not(target_arch = "wasm32"))] @@ -128,6 +132,12 @@ impl FontDb { self.generation.load(Ordering::Relaxed) } + /// A number no other database made in this process has: with [`FontDb::generation`], it tells + /// what a list built from a database was built from. + pub fn id(&self) -> u64 { + self.id + } + /// Family names available (loaded plus installed system fonts), sorted and deduplicated. pub fn families(&self) -> Vec { self.family_list().to_vec() diff --git a/crates/ui-egui/src/font_menu.rs b/crates/ui-egui/src/font_menu.rs index 4fbb7a7ffc..8e61f291e4 100644 --- a/crates/ui-egui/src/font_menu.rs +++ b/crates/ui-egui/src/font_menu.rs @@ -383,7 +383,7 @@ pub fn font_menu( |ui| { // Wider than a narrow field, so the filters keep their rows and long names show. ui.set_min_width(menu_width(ui, state, look)); - header(ui, state_id, state, current, look) + header(ui, vectorcraft_text::FontDb::global(), state_id, state, current, look) }, look.row, |h| h.rows.len(), @@ -970,6 +970,8 @@ pub(crate) fn take_raw_wheel(ctx: &egui::Context, raw: &mut egui::RawInput) { /// What the menu's header hands its rows. struct Listed<'s> { + /// The fonts listed: [`vectorcraft_text::FontDb::global`] outside the unit tests. + db: &'s vectorcraft_text::FontDb, st: &'s mut MenuState, /// The rows listed. rows: Arc<[Row]>, @@ -985,14 +987,28 @@ struct Listed<'s> { /// [`craft_ui::combo::Menu::show_rows_with_header`] lays them out. #[cfg(test)] fn list(ui: &mut Ui, state_id: egui::Id, st: &mut MenuState, current: &str, sample: Option<&str>, look: MenuLook) -> Option { - let h = header(ui, state_id, st, current, look); + list_of(ui, vectorcraft_text::FontDb::global(), state_id, st, current, sample, look).inner +} + +/// [`list`] of the families in `db`, with the rows' scroll area as laid out. +#[cfg(test)] +fn list_of( + ui: &mut Ui, + db: &vectorcraft_text::FontDb, + state_id: egui::Id, + st: &mut MenuState, + current: &str, + sample: Option<&str>, + look: MenuLook, +) -> egui::scroll_area::ScrollAreaOutput> { + let h = header(ui, db, state_id, st, current, look); let total = h.rows.len(); - egui::ScrollArea::vertical().max_height(ui.available_height()).show_rows(ui, look.row, total, |ui, range| rows(ui, range, h, sample, look)).inner + egui::ScrollArea::vertical().max_height(ui.available_height()).show_rows(ui, look.row, total, |ui, range| rows(ui, range, h, sample, look)) } /// The top of the open menu, which stays in place: the search field, the filters, and the rows -/// they list. -fn header<'s>(ui: &mut Ui, state_id: egui::Id, st: &'s mut MenuState, current: &str, look: MenuLook) -> Listed<'s> { +/// of `db` they list. +fn header<'s>(ui: &mut Ui, db: &'s vectorcraft_text::FontDb, state_id: egui::Id, st: &'s mut MenuState, current: &str, look: MenuLook) -> Listed<'s> { let t = Tokens::get(ui.ctx()); ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend); // Each time the menu opens: no query, the current family highlighted and in view (the @@ -1025,7 +1041,6 @@ fn header<'s>(ui: &mut Ui, state_id: egui::Id, st: &'s mut MenuState, current: & let query = widgets::search_field(ui, field, tl!("Search")).to_lowercase(); let refilter = filters(ui, st, &t); let enter = ui.input(|i| i.key_pressed(egui::Key::Enter)); - let db = vectorcraft_text::FontDb::global(); // The current family by its own name (a document may name it in Japanese: ヒラギノ角ゴシック), // as it was before any preview. let current = db.canonical(&st.current, "").0; @@ -1092,7 +1107,7 @@ fn header<'s>(ui: &mut Ui, state_id: egui::Id, st: &'s mut MenuState, current: & // The scroll bar beside the rows, not over them: a floating one covered the stars at their // right ends, so a favourite was hard to click (#2277). ui.spacing_mut().scroll = widgets::list_scroll_style(); - Listed { st, rows, current: current.to_string(), enter, keyed, opening } + Listed { db, st, rows, current: current.to_string(), enter, keyed, opening } } /// One of the filters' chips. @@ -1166,28 +1181,29 @@ fn filters(ui: &mut Ui, st: &mut MenuState, t: &Tokens) -> bool { /// [`WINDOW_SHARE`] of the window. The menu is never narrower than its field. fn menu_width(ui: &Ui, st: &MenuState, look: MenuLook) -> f32 { let sample = if look.samples { SAMPLE_W } else { 0.0 }; - let names = name_width(ui, look.english_names, &st.rows).min(NAME_MAX); + let names = name_width(ui, vectorcraft_text::FontDb::global(), look.english_names, &st.rows).min(NAME_MAX); let row = DISCLOSURE + names + NAME_GAP + sample + STAR + 2.0 + widgets::list_scroll_style().allocated_width(); // Rounding in the chips' widths must not wrap the widest row. let filters = if st.filters > 0.0 { st.filters.ceil() + 1.0 } else { 0.0 }; filters.max(row).min(ui.ctx().content_rect().width() * WINDOW_SHARE) } -/// The width of the widest name the menu lists after the disclosure triangle: the family names as -/// Show Font Names in English shows them, and the style names among `rows` after their indent. -fn name_width(ui: &Ui, english: bool, rows: &[Row]) -> f32 { +/// The width of the widest name the menu lists after the disclosure triangle: the family names of +/// `db` as Show Font Names in English shows them, and the style names among `rows` after their +/// indent. +fn name_width(ui: &Ui, db: &vectorcraft_text::FontDb, english: bool, rows: &[Row]) -> f32 { let font = egui::FontId::proportional(NAME_SIZE); let measure = |name: &str| ui.painter().layout_no_wrap(name.to_string(), font.clone(), Color32::WHITE).size().x; let styles = rows.iter().filter_map(Row::style).map(|s| STYLE_INDENT + measure(s)).fold(0.0, f32::max); - family_name_width(ui, english).max(styles) + family_name_width(ui, db, english).max(styles) } -/// What the widest family name was measured for: the font list's generation, Show Font Names in -/// English, and the number of fonts the interface draws with, which grows as faces for other -/// scripts are added to it. -fn family_name_key(ui: &Ui, english: bool) -> (u64, bool, usize) { +/// What the widest family name was measured for: the font database and its generation, Show Font +/// Names in English, and the number of fonts the interface draws with, which grows as faces for +/// other scripts are added to it. +fn family_name_key(ui: &Ui, db: &vectorcraft_text::FontDb, english: bool) -> (u64, u64, bool, usize) { let faces = ui.ctx().fonts(|f| f.definitions().font_data.len()); - (vectorcraft_text::FontDb::global().generation(), english, faces) + (db.id(), db.generation(), english, faces) } /// The id under which the widest family name's width is kept with its [`family_name_key`]. @@ -1195,16 +1211,15 @@ fn family_name_id() -> egui::Id { egui::Id::unique("font-menu-name-width") } -/// The width of the widest family name the menu lists, labeled as Show Font Names in English -/// (`english`) labels it; worked out again only when its [`family_name_key`] changes. -fn family_name_width(ui: &Ui, english: bool) -> f32 { - let key = family_name_key(ui, english); - if let Some((k, w)) = ui.data(|d| d.get_temp::<((u64, bool, usize), f32)>(family_name_id())) +/// The width of the widest family name the menu lists from `db`, labeled as Show Font Names in +/// English (`english`) labels it; worked out again only when its [`family_name_key`] changes. +fn family_name_width(ui: &Ui, db: &vectorcraft_text::FontDb, english: bool) -> f32 { + let key = family_name_key(ui, db, english); + if let Some((k, w)) = ui.data(|d| d.get_temp::<((u64, u64, bool, usize), f32)>(family_name_id())) && k == key { return w; } - let db = vectorcraft_text::FontDb::global(); let w = widest_label(ui, db.menu_family_list().iter().map(|f| db.family_display_name(f, english)).collect()); ui.data_mut(|d| d.insert_temp(family_name_id(), (key, w))); w @@ -1269,14 +1284,13 @@ fn heading_row(ui: &mut Ui, row_h: f32, label: &str, t: &Tokens) { /// The rows of the open menu in `range`, the ones in view: the menu's one scrolling part, under /// the search field and filters (#555). fn rows(ui: &mut Ui, range: std::ops::Range, h: Listed<'_>, sample: Option<&str>, look: MenuLook) -> Option { - let Listed { st, rows, current, enter, keyed, opening } = h; + let Listed { db, st, rows, current, enter, keyed, opening } = h; let current = current.as_str(); let row_h = look.row; let t = Tokens::get(ui.ctx()); let ink = sample_ink(look.on_white, t.text); - let db = vectorcraft_text::FontDb::global(); let favorite = |f: &str| look.favorites.iter().any(|x| x.eq_ignore_ascii_case(f)); - let widest = name_width(ui, look.english_names, &rows); + let widest = name_width(ui, db, look.english_names, &rows); if rows.is_empty() { // A filter or a font rescan can remove the row whose font was previewed. // Do not leave that font applied while the list has no candidate to highlight. diff --git a/crates/ui-egui/src/font_menu/tests.rs b/crates/ui-egui/src/font_menu/tests.rs index 88a524e15b..11a66cab00 100644 --- a/crates/ui-egui/src/font_menu/tests.rs +++ b/crates/ui-egui/src/font_menu/tests.rs @@ -21,40 +21,44 @@ fn the_thai_filter_lists_fonts_with_thai() { /// stay clear of it. #[test] fn the_scroll_bar_leaves_the_stars_clear() { - // Every family starred, so each row draws its star. Tests running alongside add fonts and - // composite fonts to the process-wide database, which the menu lists too: the ★ filter keeps - // the rows to the families starred here. - let all = vectorcraft_text::FontDb::global().families(); + // A database of the bundled fonts alone, each family starred, so each row draws its star: + // fonts that tests running alongside add to the process-wide database stay out of the rows. + let db = vectorcraft_text::FontDb::with_font_dirs(vec![]); + let favorites: Vec = ["Source Sans 3", "Source Serif 4", "Inter", "JetBrains Mono"].map(str::to_string).into(); let ctx = egui::Context::default(); + // Under the search field and the filters, room for fewer rows than the four families. let area = egui::Rect::from_min_size(egui::pos2(0.0, 0.0), egui::vec2(300.0, 160.0)); let mut st = super::MenuState { favorites_only: true, ..Default::default() }; - let look = super::MenuLook { samples: false, favorites: &all, ..Default::default() }; - let mut stars = vec![]; - for _ in 0..3 { + let look = super::MenuLook { samples: false, favorites: &favorites, ..Default::default() }; + let (mut list, mut stars) = (None, vec![]); + // Frames enough for the scroll bar to finish appearing. + for _ in 0..30 { let mut out = ctx.run_ui(egui::RawInput { screen_rect: Some(area), ..Default::default() }, |ui| { let mut child = ui.new_child(egui::UiBuilder::new().max_rect(area)); - super::list(&mut child, egui::Id::unique("fonts"), &mut st, "", None, look); + let shown = super::list_of(&mut child, &db, egui::Id::unique("fonts"), &mut st, "Inter", None, look); + list = Some((shown.inner_rect, shown.content_size)); }); out.textures_delta.clear(); stars = out .shapes .iter() .filter_map(|c| match &c.shape { - // The rows' stars, not the ★ filter button at the top left. - egui::Shape::Text(t) if t.galley.text() == "★" && t.pos.x > area.center().x => Some(t.visual_bounding_rect().right()), + egui::Shape::Text(t) if t.galley.text() == "★" => Some(t.visual_bounding_rect()), _ => None, }) .collect(); } - let families = st.rows.len(); - assert!(!stars.is_empty() || families == 0, "{families} families, no row stars drawn"); - // The rows don't fill the list without enough families to scroll, and then there is no bar. - if families as f32 * look.row > area.height() { - let bar = super::widgets::list_scroll_style(); - let column = bar.bar_width + bar.bar_inner_margin + bar.bar_outer_margin; - for right in stars { - assert!(right <= area.right() - column + 0.5, "a star at {right} runs under the scroll bar"); - } + let Some((inner, content)) = list else { panic!("the list was not drawn") }; + let families: Vec<&str> = st.rows.iter().filter_map(super::Row::family).collect(); + assert_eq!(families.len(), favorites.len(), "the starred families are listed: {families:?}"); + assert!(content.y > inner.height() + 0.5, "the rows, {} high, fill a list {} high", content.y, inner.height()); + // The rows' stars in the list, not the ★ filter above it. + let stars: Vec<_> = stars.into_iter().filter(|s| inner.contains(s.center())).collect(); + assert!(!stars.is_empty(), "no row star drawn in the list at {inner:?}"); + let bar = super::widgets::list_scroll_style(); + let column = bar.bar_width + bar.bar_inner_margin + bar.bar_outer_margin; + for star in stars { + assert!(star.right() <= area.right() - column + 0.5, "a star at {} runs under the scroll bar", star.right()); } } @@ -104,14 +108,33 @@ fn the_widest_name_is_measured_again_when_the_interface_fonts_change() { }; // A width no name has, kept as if measured for the fonts as they are. frame(&mut |ui| { - let key = super::family_name_key(ui, true); + let key = super::family_name_key(ui, vectorcraft_text::FontDb::global(), true); ui.data_mut(|d| d.insert_temp(super::family_name_id(), (key, -1.0_f32))); }); frame(&mut |ui| { let data = ui.ctx().fonts(|f| f.definitions().font_data.values().next().cloned()).expect("an interface font"); ui.ctx().add_font(egui::epaint::text::FontInsert { name: "font-menu-test-face".into(), data: (*data).clone(), families: vec![] }); }); - frame(&mut |ui| assert!(super::family_name_width(ui, true) >= 0.0, "measured again")); + frame(&mut |ui| assert!(super::family_name_width(ui, vectorcraft_text::FontDb::global(), true) >= 0.0, "measured again")); +} + +/// The widest family name measured for one font database is not taken for another one at the same +/// generation. +#[test] +fn each_font_database_has_its_own_widest_name() { + use vectorcraft_doc::CompositeFont; + let short = vectorcraft_text::FontDb::with_font_dirs(vec![]); + let long = vectorcraft_text::FontDb::with_font_dirs(vec![]); + assert!(short.set_composite_font(&CompositeFont::new("C", "Inter", "Regular"))); + assert!(long.set_composite_font(&CompositeFont::new("A composite font with a name longer than any family", "Inter", "Regular"))); + assert_eq!(short.generation(), long.generation()); + let ctx = egui::Context::default(); + let mut widths = (0.0, 0.0); + let mut out = ctx.run_ui(egui::RawInput::default(), |ui| { + widths = (super::family_name_width(ui, &short, true), super::family_name_width(ui, &long, true)); + }); + out.textures_delta.clear(); + assert!(widths.1 > widths.0 + 1.0, "the long name's database measured {} and the other {}", widths.1, widths.0); } /// Without a row of filters drawn yet, the menu still makes room for the widest name, its sample, @@ -125,7 +148,7 @@ fn the_menu_makes_room_for_the_widest_name_and_its_sample() { let mut out = ctx.run_ui(input, |ui| { let st = super::MenuState::default(); let look = super::MenuLook::default(); - let names = super::name_width(ui, look.english_names, &st.rows).min(NAME_MAX); + let names = super::name_width(ui, vectorcraft_text::FontDb::global(), look.english_names, &st.rows).min(NAME_MAX); assert!(names > 0.0, "a family is listed"); let width = super::menu_width(ui, &st, look); assert!(width >= DISCLOSURE + names + NAME_GAP + SAMPLE_W + STAR, "the menu is {width} wide for a name {names} wide"); @@ -143,14 +166,15 @@ fn only_listed_style_names_widen_the_names() { let ctx = egui::Context::default(); crate::theme::install_fonts(&ctx); let long = format!("SemiCondensed ExtraLight Italic {}", "W".repeat(80)); + let db = vectorcraft_text::FontDb::global(); let mut out = ctx.run_ui(egui::RawInput::default(), |ui| { - let families = super::name_width(ui, true, &[]); + let families = super::name_width(ui, db, true, &[]); let font = egui::FontId::proportional(super::NAME_SIZE); let style = STYLE_INDENT + ui.painter().layout_no_wrap(long.clone(), font, egui::Color32::WHITE).size().x; assert!(style > families, "the style name is wider than every family name"); let listed = [Row::Family("A".into()), Row::Style("A".into(), long.clone())]; - assert_eq!(super::name_width(ui, true, &listed), style); - assert!(super::name_width(ui, true, &listed[..1]) < style, "a style not listed is not measured"); + assert_eq!(super::name_width(ui, db, true, &listed), style); + assert!(super::name_width(ui, db, true, &listed[..1]) < style, "a style not listed is not measured"); }); out.textures_delta.clear(); }