Free font textures when a font asset is unloaded#8969
Conversation
Add Font.destroy() so unloading a font asset releases its texture VRAM and clears the textures from the resource loader cache. FontHandler passes the loader to the Font so it can clear the cache entries. Fixes #7033.
Build size reportThis PR changes the size of the minified bundles.
|
mvaligursky
left a comment
There was a problem hiding this comment.
🤖 Automated PR review — posted on my behalf by Claude Code (Opus 4.8). Not a human review. The points below are suggestions to weigh as possible improvements, not changes that necessarily need to be addressed.
Fix is correct for the standard load → unload → reload case (I confirmed texture.name reliably equals the load URL = cache key, since every texture parser sets name: url, and clearCache/getFromCache key on the same makeKey(url, type)). Three things to consider, in rough order of likelihood:
-
Active-use behavior change worth confirming. Previously the texture leak masked it; now
asset.unload()actually destroys the GPU textures. If aTextElement(orCanvasFontconsumer) still references the font when its asset is unloaded, it will hold destroyed textures and render with dead resources. This is the intended semantics of unload, but worth confirming the text element drops/handles its font reference gracefully on asset unload rather than erroring — the PR doesn't mention that path. -
Shared texture, no reference counting.
Font.destroy()unconditionally callstexture.destroy()andclearCache(...), and the loader cache (_cache[key]) isn't ref-counted. If the same texture URL is ever resolved for more than one liveFont(e.g. two font assets whose derived.pngURL collides, or any future path that shares a cached font texture), unloading one font destroys the texture out from under the others. Low probability since each font's atlas PNG is normally unique — but previously the leak hid this, so it's newly reachable. A guard ("only destroy if no other resource holds it") or accepting the limitation with a note would cover it. -
Coupling to the parser naming convention. Keying cache removal on
texture.nameworks only because all texture parsers happen to setname: url. That's true today across all six parsers, but it couplesFont.destroyto that convention; if a parser ever set a differentname, the cache entry would silently leak (texture still freed, but a reload returns a dead entry). Storing the actual loaded URLs on theFontat load time would decouple it. Minor / robustness only.
Add Font.destroy() so unloading a font asset releases its texture VRAM and clears the textures from the resource loader cache. FontHandler passes the loader to the Font so it can clear the cache entries. Fixes #7033. Co-authored-by: Martin Valigursky <mvaligursky@snapchat.com>
Unloading a font asset (
asset.unload()) did not release the GPU memory used by its textures. TheFontresource had nodestroy()method, so the asset system's genericresource.destroy?.()cleanup was a no-op and the textures stayed resident in VRAM. The textures were also left in the resource loader cache, so a subsequent load could return a stale entry.Fixes #7033.
Changes:
Font.destroy(), which destroys the font's textures and removes them from the resource loader cache.FontHandlernow gives the createdFonta reference to the resource loader so it can clear the cached texture entries on unload.API Changes:
Font#destroy(). It is called automatically when the owning font asset is unloaded; existing user workarounds that manually destroy the font's textures remain safe (double-destroy is a no-op).