Merge pull request #7133 from dotCooCoo/fix/font-menu-scrollbar-test-flaky

Tests: the font menu scroll bar test lists fonts of its own

crates/text/src/fontdb.rs

@@ -105,6 +105,8 @@ pub struct FontDb {
     family_cache: Mutex<Option<Arc<[String]>>>,
     menu_cache: Mutex<Option<MenuFamilies>>,
     generation: AtomicU64,
+    /// This database's [`FontDb::id`].
+    id: u64,
     /// System fallback state: characters no system font covers.
     #[cfg(not(target_arch = "wasm32"))]
     sys: Mutex<SysFallback>,

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<String> {
         self.family_list().to_vec()

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<FontPick> {
-    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<Option<FontPick>> {
+    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<usize>, h: Listed<'_>, sample: Option<&str>, look: MenuLook) -> Option<FontPick> {
-    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.

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<String> = ["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();
 }