mirror of
https://github.com/storytold/photocraft.git
synced 2026-10-24 09:57:08 -06:00
fix(type): key font menu caches on the shared engine's fonts (#4200)
The font menu's preview samples, family list and localized names were cached on fonts::generation(), which any FontDb moves, and the previews also on the served catalog's generation. Other engines registering fonts (tests running in parallel) re-rendered every visible sample each pass, and families covering Arabic asked for a repaint, so Harness::run never settled: options_font_family_preserves_peer_faces_and_batches_layer_flags and the font picker tests failed intermittently with "exceeded max_steps". - photocraft-text: fonts::shared_generation(), moved only by the shared engine's FontDb (FontDb::mark_shared from new_shared). - ui-egui: families(), localized_families() and the preview cache key on it; previews no longer key on the served catalog (a served family's sample appears when its fonts register, which moves the generation). Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -109,6 +109,16 @@ pub fn generation() -> u64 {
|
||||
GENERATION.load(Ordering::Relaxed)
|
||||
}
|
||||
|
||||
/// Bumped only by the [`FontDb`] of [`crate::shared`], so caches of the shared engine's fonts
|
||||
/// aren't invalidated by unrelated databases (a document's own engine, a test's throwaway one).
|
||||
static SHARED_GENERATION: AtomicU64 = AtomicU64::new(0);
|
||||
|
||||
/// Changes whenever font data is registered in the shared engine's [`FontDb`] ([`crate::shared`]).
|
||||
/// Font menus key their caches on this, not [`generation`].
|
||||
pub fn shared_generation() -> u64 {
|
||||
SHARED_GENERATION.load(Ordering::Relaxed)
|
||||
}
|
||||
|
||||
/// One face in the database (for font menus).
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
pub struct FaceInfo {
|
||||
@@ -145,6 +155,8 @@ pub struct FontDb {
|
||||
missing_glyph_protection: bool,
|
||||
face_cache: HashMap<String, Vec<FaceInfo>>,
|
||||
face_aliases: HashMap<(String, usize), String>,
|
||||
/// This is the shared engine's database: registrations also move [`shared_generation`].
|
||||
shared: bool,
|
||||
}
|
||||
|
||||
impl Default for FontDb {
|
||||
@@ -165,6 +177,7 @@ impl FontDb {
|
||||
missing_glyph_protection: true,
|
||||
face_cache: HashMap::new(),
|
||||
face_aliases: HashMap::new(),
|
||||
shared: false,
|
||||
};
|
||||
for (_, bytes) in BUNDLED {
|
||||
db.register_static_font(bytes.as_slice());
|
||||
@@ -246,9 +259,19 @@ impl FontDb {
|
||||
self.face_cache.clear();
|
||||
self.refresh_generics();
|
||||
GENERATION.fetch_add(1, Ordering::Relaxed);
|
||||
if self.shared {
|
||||
SHARED_GENERATION.fetch_add(1, Ordering::Relaxed);
|
||||
}
|
||||
names
|
||||
}
|
||||
|
||||
/// Marks this as the shared engine's database ([`crate::shared`]) and moves
|
||||
/// [`shared_generation`], since its fonts replace whatever caches saw before.
|
||||
pub(crate) fn mark_shared(&mut self) {
|
||||
self.shared = true;
|
||||
SHARED_GENERATION.fetch_add(1, Ordering::Relaxed);
|
||||
}
|
||||
|
||||
fn refresh_generics(&mut self) {
|
||||
let c = &mut self.fcx.collection;
|
||||
if let Some(inter) = c.family_id(DEFAULT_FAMILY) {
|
||||
|
||||
@@ -147,7 +147,9 @@ fn uses_system_fonts() -> bool {
|
||||
}
|
||||
|
||||
fn new_shared() -> std::sync::Mutex<TextEngine> {
|
||||
std::sync::Mutex::new(if uses_system_fonts() { TextEngine::with_system_fonts() } else { TextEngine::new() })
|
||||
let mut engine = if uses_system_fonts() { TextEngine::with_system_fonts() } else { TextEngine::new() };
|
||||
engine.fonts.mark_shared();
|
||||
std::sync::Mutex::new(engine)
|
||||
}
|
||||
|
||||
/// A process-wide engine for callers without their own (bundled fonts only on the web; bundled
|
||||
|
||||
@@ -47,6 +47,21 @@ fn registering_fonts_moves_the_generation() {
|
||||
assert!(fonts::generation() > g);
|
||||
}
|
||||
|
||||
/// Font menus cache on the shared engine's generation: other engines registering fonts (a test's
|
||||
/// throwaway one, in parallel) must not invalidate them, or the menus re-render every frame.
|
||||
#[test]
|
||||
fn only_the_shared_database_moves_the_shared_generation() {
|
||||
let _ = crate::shared();
|
||||
let g = fonts::shared_generation();
|
||||
TextEngine::new().fonts.register_font_data(fonts::INTER_REGULAR.to_vec());
|
||||
assert_eq!(fonts::shared_generation(), g, "an unrelated engine leaves it alone");
|
||||
let mut e = TextEngine::new();
|
||||
e.fonts.mark_shared();
|
||||
let g = fonts::shared_generation();
|
||||
e.fonts.register_font_data(fonts::INTER_REGULAR.to_vec());
|
||||
assert!(fonts::shared_generation() > g);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn literal_psd_tabs_shape_as_whitespace_without_shifting_text_offsets() {
|
||||
let mut e = TextEngine::new();
|
||||
|
||||
@@ -49,9 +49,12 @@ pub(crate) fn sample(preview: FontPreview) -> Option<(f32, [f32; 2])> {
|
||||
Some((size, box_for(size)))
|
||||
}
|
||||
|
||||
/// Family, display scale, the font database generations (so a family installed or fetched
|
||||
/// (served fonts) after its first miss gets a sample once it is available) and the point size.
|
||||
type Key = (String, u32, (u64, u64), u32);
|
||||
/// Family, display scale, the shared font database's generation (so a family installed or
|
||||
/// fetched (served fonts) after its first miss gets a sample once it is available) and the point
|
||||
/// size. Not the served catalog's generation: a listed family has no sample until its fonts are
|
||||
/// registered, which moves the database's generation, and keying on the catalog re-rendered
|
||||
/// every sample whenever it changed.
|
||||
type Key = (String, u32, u64, u32);
|
||||
#[derive(Clone)]
|
||||
struct Samples {
|
||||
latin: TextureHandle,
|
||||
@@ -111,7 +114,7 @@ fn images(engine: &mut photocraft_text::TextEngine, family: &str, scale: u32, si
|
||||
|
||||
fn key(ctx: &egui::Context, family: &str, size: f32) -> Key {
|
||||
let scale = ctx.pixels_per_point().ceil().clamp(1.0, 4.0) as u32;
|
||||
(family.to_owned(), scale, (photocraft_text::fonts::generation(), photocraft_text::served::generation()), size as u32)
|
||||
(family.to_owned(), scale, photocraft_text::fonts::shared_generation(), size as u32)
|
||||
}
|
||||
|
||||
/// Width a row reserves for its samples: wider once a painted row showed the family covers
|
||||
|
||||
@@ -985,11 +985,11 @@ fn draw_mask_overlay(app: &PhotocraftApp, painter: &egui::Painter, xf: &ViewXfor
|
||||
}
|
||||
|
||||
/// Font families: bundled, system, and those the host serves (`photocraft_text::served`, fetched
|
||||
/// when picked). Cached until fonts are registered or the served catalog changes.
|
||||
/// when picked). Cached until the shared engine registers fonts or the served catalog changes.
|
||||
pub fn families() -> Arc<Vec<String>> {
|
||||
type Cached = Option<((u64, u64), Arc<Vec<String>>)>;
|
||||
static FAMILIES: std::sync::Mutex<Cached> = std::sync::Mutex::new(None);
|
||||
let key = (photocraft_text::fonts::generation(), photocraft_text::served::generation());
|
||||
let key = (photocraft_text::fonts::shared_generation(), photocraft_text::served::generation());
|
||||
let mut cached = FAMILIES.lock().unwrap_or_else(PoisonError::into_inner);
|
||||
if let Some((k, v)) = cached.as_ref()
|
||||
&& *k == key
|
||||
@@ -1027,11 +1027,11 @@ pub fn sync_font_names(ctx: &egui::Context, show_in_english: bool) {
|
||||
});
|
||||
}
|
||||
|
||||
/// The fonts' localized family names, keyed by family. Cached until fonts are registered.
|
||||
/// The fonts' localized family names, keyed by family. Cached until the shared engine registers fonts.
|
||||
fn localized_families() -> Arc<HashMap<String, String>> {
|
||||
type Cached = Option<(u64, Arc<HashMap<String, String>>)>;
|
||||
static LOCALIZED: std::sync::Mutex<Cached> = std::sync::Mutex::new(None);
|
||||
let key = photocraft_text::fonts::generation();
|
||||
let key = photocraft_text::fonts::shared_generation();
|
||||
let mut cached = LOCALIZED.lock().unwrap_or_else(PoisonError::into_inner);
|
||||
if let Some((k, v)) = cached.as_ref()
|
||||
&& *k == key
|
||||
|
||||
@@ -1428,6 +1428,33 @@ mod multi_layer_formatting {
|
||||
assert_ne!(text(h.state(), ids[2]).antialias, AntiAlias::Sharp);
|
||||
}
|
||||
|
||||
/// Other engines registering fonts and the served catalog growing (tests running in parallel)
|
||||
/// used to move the key of the font menu's preview cache, so its samples re-rendered and asked
|
||||
/// for a repaint every pass and `Harness::run` never settled (a flaky `exceeded max_steps`).
|
||||
/// Here both happen on every pass.
|
||||
#[test]
|
||||
fn the_font_menu_settles_while_other_engines_register_fonts() {
|
||||
let (app, _) = fixture();
|
||||
let mut pass = 0u32;
|
||||
let mut h = Harness::builder().with_size(vec2(1100.0, 800.0)).build_ui_state(
|
||||
move |ui, app| {
|
||||
photocraft_text::TextEngine::new();
|
||||
// The served catalog is process-wide: names that sort last.
|
||||
photocraft_text::served::add_families([format!("zz Churn Test {pass:03}")]);
|
||||
pass = pass.saturating_add(1);
|
||||
if ui.ctx().fonts(|f| f.families().contains(&egui::FontFamily::Name("medium".into()))) {
|
||||
ui.horizontal(|ui| super::super::options_bar(app, ui));
|
||||
}
|
||||
},
|
||||
app,
|
||||
);
|
||||
PhotocraftApp::setup_context(&h.ctx, Default::default());
|
||||
h.run_steps(4);
|
||||
h.get_all_by_role(Role::ComboBox).next().unwrap().click();
|
||||
h.run();
|
||||
h.run();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn properties_edit_shown_metrics_and_paragraphs_with_a_non_type_primary() {
|
||||
let (mut app, ids) = fixture();
|
||||
|
||||
Reference in New Issue
Block a user