fix(fonts): make a font cache invalidation reach the rayon workers - #416
Merged
Merged
Conversation
clear_all_media_caches ran on the calling thread. The font caches are thread-local, so it emptied main's copy and left every rayon worker -- the threads that actually render frames -- holding the old one. Under --watch, editing a custom font file kept serving the previous face and the previous glyph widths for the rest of the process. A rayon::broadcast would only reach the pool's current threads and would have to race with the work they are doing. A generation counter does not: FONT_GENERATION is global, each thread-local cache remembers the generation it was filled at, and it drops its contents on the first access after a bump. No thread has to be interrupted, and a worker spawned later starts on the current generation by construction. Both font caches were also missing from the function entirely -- CUSTOM_TYPEFACES predates it, and the fallback cache added in 5b81cd3 inherited the same gap. Both are GenerationCache now, and the fallback one is hoisted out of its function body so the two sit together. The registry had the same staleness one level down: register_custom_font_variant skipped a slot that was already filled, so even the global registry kept the previous file's bytes. That skip is not gratuitous -- one FontEntry can resolve to several files, and registering_the_same_weight_twice_keeps_the_first pins that two of them landing on the same weight must not fight. Both intents hold now: a variant records the generation it was registered in, so first-wins applies within a load and a later load supersedes it. load_custom_fonts opens a generation, which is what makes "a load" a thing the registry can see. It returns early on an empty list: a scenario with no custom fonts reloads constantly under --watch, and bumping there would clear every worker's cache for nothing.
LeadcodeDev
added a commit
that referenced
this pull request
Sep 29, 2026
…#447) FONT_GENERATION is one global counter and cargo runs tests in parallel, so a test reading it, calling something, and reading it again could have a sibling bump it in between. loading_an_empty_font_list_does_not_throw_ away_every_threads_cache failed that way on a full-workspace run while every scoped run of the same module stayed green, which is what made it look like someone else's regression. The five tests that read or move the counter now take one mutex. Six consecutive full runs of the crate pass where one in a handful failed before. My own defect, introduced with the counter in #416.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #323.
The defect
clear_all_media_cachesran on the calling thread. The font caches arethread-local, so it emptied main's copy and left every rayon worker — the
threads that actually render frames — holding the old one. Under
--watch,editing a custom font file kept serving the previous face and the previous glyph
widths for the rest of the process.
Not
rayon::broadcastA broadcast reaches the pool's current threads and has to race with the work
they are doing. A generation counter does not:
FONT_GENERATIONis global,each thread-local cache remembers the generation it was filled at, and drops its
contents on the first access after a bump.
No thread has to be interrupted, and a worker spawned later starts on the current
generation by construction. The clearing is lazy, which is the point —
a_generation_cache_holds_its_entries_until_the_generation_movespins thatsemantics on its own.
Two caches were missing from the function entirely
CUSTOM_TYPEFACESpredates it (already noted as F-RENDER-7), and the fallbackcache added in
5b81cd3inherited the same gap. Both areGenerationCachenow,and the fallback one is hoisted out of its function body so the two sit together.
The registry had the same staleness one level down
register_custom_font_variantskipped a slot that was already filled, so eventhe global registry kept the previous file's bytes.
That skip is not gratuitous. One
FontEntrycan resolve to several files, andregistering_the_same_weight_twice_keeps_the_firstpins that two of them landingon the same weight must not fight. My first attempt replaced unconditionally and
turned that test red — it is the test doing its job.
Both intents hold now: a variant records the generation it was registered in, so
first-wins applies within a load and a later load supersedes it.
load_custom_fontsopens a generation, which is what makes "a load" something theregistry can see. It returns early on an empty list: a scenario with no custom
fonts reloads constantly under
--watch, and bumping there would clear everyworker's cache for nothing.
Tests
a_rayon_worker_drops_its_own_copy_when_the_generation_movesa_generation_cache_holds_its_entries_until_the_generation_movesa_second_file_in_the_same_load_does_not_clobber_the_firstthe_next_load_supersedes_the_variant_the_previous_one_registeredloading_an_empty_font_list_does_not_throw_away_every_threads_cache--watchcostA first version of the rayon test passed trivially — the family it resolved was
never registered, so nothing was ever cached and the assertion proved nothing. It
now asserts the setup filled the caches before checking they emptied.
Both reverts bite:
Gate
cargo fmt --all --checkclean ·cargo clippy --workspace --all-targets --features rustmotion/studio -D warningsclean ·cargo test --workspace1794 passed.