fix(textures): reversibly free textures on cleanup so they reload in-place#87
Merged
Merged
Conversation
…place
LRU/idle texture cleanup destroyed textures (Texture.destroy() ->
removeAllListeners() + cache eviction) instead of reversibly freeing them.
A CoreNode that kept its texture reference would lose its 'loaded'/'freed'
subscription, so when it scrolled back into the viewport the texture
reloaded all the way to 'loaded' but the node was never re-notified --
leaving the node blank ("texture is loaded but nothing is displayed").
The latent design issue (cleanup using the terminal destroy path) dates to
the initial release, but it only became an observable regression in #56,
which added removeAllListeners() to Texture.destroy() -- correct for genuine
teardown, but it severed the last surviving link for nodes still holding the
reference.
Fix: add TextureMemoryManager.freeTexture(), a reversible reclamation that
releases the GPU resource and transitions the source to 'freed' while keeping
the Texture object, its listeners, and its cache entry intact. cleanup() now
uses freeTexture() instead of destroyTexture(); the 'freed' -> reload
machinery already existed and now correctly re-notifies the node. Frees are
kept in cache so re-requests reuse them (and avoid duplicate downloads).
destroyTexture() is retained for genuine teardown.
- Unit tests: cleanup takes the free() path (not destroy()) and reclaims memory.
- Visual regression: texture-free-reload exercises load -> offscreen ->
forced cleanup -> back onscreen -> assert reload + re-display.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
chiefcll
added a commit
that referenced
this pull request
Jun 10, 2026
destroy() severs all listeners, so calling destroyTexture on a texture that still has subscribers reintroduces the blank-poster bug from #87. Restrict it to the one caller that proves the orphan precondition (evictOrphanedTextures) and document freeTexture as the public, reversible path for memory pressure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
chiefcll
added a commit
that referenced
this pull request
Jun 10, 2026
* fix(textures): evict orphaned cached textures during cleanup Since #87, cleanup() only ever reversibly frees textures, so nothing evicts keyCache entries anymore — a slow leak of Texture JS objects in long-running apps that cycle many unique image URLs. Add an orphan-eviction sweep to TextureMemoryManager.cleanup(): a cached texture is destroyed and evicted only when nothing can ever re-display it — zero renderableOwners AND zero event listeners (every live referencer, CoreNode.loadTextureTask and SubTexture, subscribes via on()). This preserves the #87 invariant: a texture with any listener is never destroyed, since destroy()'s removeAllListeners() would sever the node's subscription and reintroduce the blank-poster bug. Evictable states are 'freed', plus 'initial'/'failed' orphans once the existing 2s startup grace period expires — the grace period covers the same-frame window where a new node's subscription microtask hasn't flushed yet. In-flight states are never evicted; 'loaded' orphans go through the pressure-driven free path first. Adds EventEmitter.hasListeners() as the liveness signal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(textures): make destroyTexture private destroy() severs all listeners, so calling destroyTexture on a texture that still has subscribers reintroduces the blank-poster bug from #87. Restrict it to the one caller that proves the orphan precondition (evictOrphanedTextures) and document freeTexture as the public, reversible path for memory pressure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jun 10, 2026
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.
What
Texture memory cleanup now reversibly frees textures instead of destroying them, so a node that keeps its texture reference re-displays correctly when it scrolls back into the viewport.
Why (the bug)
LRU/idle cleanup routed reclamation through the terminal path:
TextureMemoryManager.cleanup()→destroyTexture()→Texture.destroy()(which callsremoveAllListeners()) + cache eviction.A
CoreNodeonly subscribes to its texture'sloaded/freedevents once, inloadTextureTask(on assignment/creation) — never on viewport re-entry. So when cleanup destroyed an off-screen texture the node still referenced:setRenderableOwner(true)→load()correctly re-triggers the reload.loaded.destroy(), soonTextureLoadednever runs →textureLoadedstaysfalse→ the node is never put back in the render list.Result: "the texture is loaded but nothing is displayed" — a blank poster. It only self-corrected when the reactive layer happened to assign a new texture object (cache miss after eviction), which re-subscribed.
This was reported against an infinite/carousel virtual row: scroll down (textures freed under memory pressure), scroll back up, posters blank.
When it was introduced
The latent design smell — cleanup using the destroy path — dates to the initial release. It became an observable regression in #56 (
844fec62, "release shader value cache and subtexture listeners on destroy"), which addedremoveAllListeners()toTexture.destroy(). That change was correct for genuine teardown but severed the last surviving link for nodes still holding the reference.The fix
TextureMemoryManager.freeTexture()— reversible reclamation that callstexture.free()(releases the GPU resource, sets statefreed, reclaims tracked memory) while keeping theTextureobject, its listeners, and its cache entry intact.cleanup()now callsfreeTexture()instead ofdestroyTexture(). Thefreed → reloadmachinery already existed (ctxTexturegetter auto-reload +loadTexture'sfreed → loadingtransition); the only thing that was broken was the notification back to the node.destroyTexture()is retained for genuine teardown.free()both set the source tofreedand reclaim memory.Behavior note for reviewers
Freed textures are now kept in cache (correct LRU — re-requests reuse them and avoid duplicate downloads). If we ever want cache eviction under memory pressure, nodes would instead need to re-subscribe on viewport re-entry (more invasive). Flagging in case there was an intentional reason for the old eviction.
Tests
TextureMemoryManager.test.ts): cleanup takes thefree()path (notdestroy()), and reclaims the freed memory back to baseline.examples/tests/texture-free-reload.ts+ certified snapshot): load → move off-screen → forcecleanup(true)→ move back on-screen → assert the texture reloads and re-displays. The certified baseline was generated via the Docker--ciflow. Verified locally that the buggydestroypath produces a blank poster.322 unit tests pass;
pnpm buildclean.🤖 Generated with Claude Code