Skip to content

fix(textures): reversibly free textures on cleanup so they reload in-place#87

Merged
chiefcll merged 2 commits into
mainfrom
fix/texture-cleanup-reversible-free
Jun 9, 2026
Merged

fix(textures): reversibly free textures on cleanup so they reload in-place#87
chiefcll merged 2 commits into
mainfrom
fix/texture-cleanup-reversible-free

Conversation

@chiefcll

@chiefcll chiefcll commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

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 calls removeAllListeners()) + cache eviction.

A CoreNode only subscribes to its texture's loaded/freed events once, in loadTextureTask (on assignment/creation) — never on viewport re-entry. So when cleanup destroyed an off-screen texture the node still referenced:

  1. Node scrolls back in → setRenderableOwner(true)load() correctly re-triggers the reload.
  2. Texture reloads all the way to loaded.
  3. But its listeners were stripped by destroy(), so onTextureLoaded never runs → textureLoaded stays false → 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 added removeAllListeners() to Texture.destroy(). That change was correct for genuine teardown but severed the last surviving link for nodes still holding the reference.

The fix

  • New TextureMemoryManager.freeTexture() — reversible reclamation that calls texture.free() (releases the GPU resource, sets state freed, reclaims tracked memory) while keeping the Texture object, its listeners, and its cache entry intact.
  • cleanup() now calls freeTexture() instead of destroyTexture(). The freed → reload machinery already existed (ctxTexture getter auto-reload + loadTexture's freed → loading transition); the only thing that was broken was the notification back to the node.
  • destroyTexture() is retained for genuine teardown.
  • Works for both backends — WebGL and Canvas2D free() both set the source to freed and 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

  • Unit (TextureMemoryManager.test.ts): cleanup takes the free() path (not destroy()), and reclaims the freed memory back to baseline.
  • Visual regression (examples/tests/texture-free-reload.ts + certified snapshot): load → move off-screen → force cleanup(true) → move back on-screen → assert the texture reloads and re-displays. The certified baseline was generated via the Docker --ci flow. Verified locally that the buggy destroy path produces a blank poster.

322 unit tests pass; pnpm build clean.

🤖 Generated with Claude Code

chiefcll and others added 2 commits June 9, 2026 14:48
…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 chiefcll merged commit ebb2791 into main Jun 9, 2026
1 check passed
@chiefcll chiefcll deleted the fix/texture-cleanup-reversible-free branch June 9, 2026 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant