diff --git a/examples/tests/texture-free-reload.ts b/examples/tests/texture-free-reload.ts new file mode 100644 index 0000000..96402c8 --- /dev/null +++ b/examples/tests/texture-free-reload.ts @@ -0,0 +1,136 @@ +import type { INode } from '@lightningjs/renderer'; +import type { ExampleSettings } from '../common/ExampleSettings.js'; +import { waitUntilIdle } from '../common/utils.js'; + +import rockoPng from '../assets/rocko.png'; + +/** + * Regression test: a texture that is freed by the memory manager while its node + * is out of the viewport must reload — and re-display — when the node scrolls + * back in. + * + * The bug this guards against: LRU cleanup used to *destroy* textures + * (`removeAllListeners` + cache evict) rather than reversibly *free* them. A + * node that kept its reference would then reload the texture to `loaded` but + * never be re-notified, so it stayed blank. The fix frees reversibly, keeping + * the node's subscription intact. See TextureMemoryManager.freeTexture. + * + * If the bug regresses, step 6 below never completes and the rocko image is + * missing from the snapshot. + */ + +const NODE = { x: 100, y: 100, w: 181, h: 218 }; + +function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +/** + * Poll `predicate` until it is true or the timeout elapses. `tick` runs each + * iteration to drive frames / re-request cleanup. + */ +async function waitFor( + predicate: () => boolean, + timeoutMs: number, + tick?: () => void, +): Promise { + const start = Date.now(); + while (predicate() === false) { + if (Date.now() - start > timeoutMs) { + return false; + } + if (tick !== undefined) { + tick(); + } + await delay(50); + } + return true; +} + +export async function automation(settings: ExampleSettings) { + await test(settings); + await waitUntilIdle(settings.renderer); + await settings.snapshot(); +} + +export default async function test({ renderer, testRoot }: ExampleSettings) { + renderer.createTextNode({ + fontFamily: 'Ubuntu', + text: 'Texture free + reload', + fontSize: 30, + color: 0xffffffff, + x: 20, + y: 20, + parent: testRoot, + }); + + const node: INode = renderer.createNode({ + ...NODE, + src: rockoPng, + parent: testRoot, + }); + + let loaded = false; + let freed = false; + node.on('loaded', () => { + loaded = true; + freed = false; + }); + node.on('freed', () => { + freed = true; + loaded = false; + }); + + // 1. Wait for the initial load. + if ((await waitFor(() => loaded, 10000)) === false) { + console.error('[texture-free-reload] texture never loaded'); + return false; + } + + // 2. Wait out the cleanup startup grace period (2s) so the memory manager is + // allowed to free the texture. + await delay(2200); + + // 3. Move the node far out of bounds and let a frame release its texture + // ownership (renderable -> false), which makes it eligible for cleanup. + node.x = -5000; + node.y = -5000; + renderer.rerender(); + await delay(100); + + // 4. Force an aggressive (full) cleanup until the texture is actually freed. + freed = false; + const wasFreed = await waitFor( + () => freed, + 5000, + () => { + renderer.stage.cleanup(true); + renderer.rerender(); + }, + ); + if (wasFreed === false) { + console.error('[texture-free-reload] texture never freed'); + return false; + } + + // 5. Scroll the node back into view. The still-subscribed node must be + // re-notified when the freed texture reloads. + loaded = false; + node.x = NODE.x; + node.y = NODE.y; + renderer.rerender(); + + // 6. Wait for the reload to complete. Before the fix this never fired. + const reloaded = await waitFor( + () => loaded, + 10000, + () => renderer.rerender(), + ); + if (reloaded === false) { + console.error('[texture-free-reload] freed texture failed to reload'); + return false; + } + + console.log('[texture-free-reload] freed texture reloaded and re-displayed'); + return true; +} diff --git a/src/core/TextureMemoryManager.test.ts b/src/core/TextureMemoryManager.test.ts index 5c685bc..bf6cd1c 100644 --- a/src/core/TextureMemoryManager.test.ts +++ b/src/core/TextureMemoryManager.test.ts @@ -1,10 +1,10 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { TextureMemoryManager, type TextureMemoryManagerSettings, } from './TextureMemoryManager.js'; import type { Stage } from './Stage.js'; -import type { Texture } from './textures/Texture.js'; +import { TextureType, type Texture } from './textures/Texture.js'; function makeSettings( overrides: Partial = {}, @@ -96,3 +96,52 @@ describe('TextureMemoryManager — out-of-memory event', () => { }); }); }); + +// A cleanable image texture: spies on free()/destroy() so we can assert which +// reclamation path cleanup() takes. memUsed starts at 0 — setTextureMemUse is +// what registers its size with the manager. +function cleanableTexture(): Texture & { + free: ReturnType; + destroy: ReturnType; +} { + return { + memUsed: 0, + state: 'loaded', + type: TextureType.image, + free: vi.fn(), + destroy: vi.fn(), + canBeCleanedUp: () => true, + } as unknown as Texture & { + free: ReturnType; + destroy: ReturnType; + }; +} + +describe('TextureMemoryManager — cleanup is reversible', () => { + it('frees textures rather than destroying them so they can reload', () => { + const { mgr } = makeManager({ criticalThreshold: 200e6 }); + const texture = cleanableTexture(); + mgr.setTextureMemUse(texture, 100e6); + + mgr.cleanup(true); + + // Reversible free path — keeps listeners + cache so a node still + // referencing the texture reloads (and is re-notified) on viewport + // re-entry. The terminal destroy path (removeAllListeners + cache evict) + // must NOT be taken. + expect(texture.free).toHaveBeenCalledTimes(1); + expect(texture.destroy).not.toHaveBeenCalled(); + }); + + it('reclaims the freed texture memory', () => { + const { mgr } = makeManager({ criticalThreshold: 200e6 }); + const texture = cleanableTexture(); + mgr.setTextureMemUse(texture, 100e6); + expect(mgr.getMemoryInfo().memUsed).toBe(126e6); // 26e6 baseline + 100e6 + + mgr.cleanup(true); + + expect(mgr.getMemoryInfo().memUsed).toBe(26e6); // back to baseline + expect(texture.memUsed).toBe(0); + }); +}); diff --git a/src/core/TextureMemoryManager.ts b/src/core/TextureMemoryManager.ts index 10444fc..86c6bab 100644 --- a/src/core/TextureMemoryManager.ts +++ b/src/core/TextureMemoryManager.ts @@ -166,6 +166,44 @@ export class TextureMemoryManager { return this.criticalThreshold > 0 && this.memUsed > this.criticalThreshold; } + /** + * Reversibly free a texture's GPU resources under memory pressure. + * + * @remarks + * Unlike {@link destroyTexture}, this keeps the `Texture` object, its event + * listeners, and its cache entry intact. It only releases the GPU-side + * resource and transitions the source to the `freed` state. A `CoreNode` that + * still references this texture will reload it — and be re-notified via its + * `loaded` listener — when it re-enters the viewport (see + * `Texture.setRenderableOwner` → `Texture.load`). + * + * This is the correct path for LRU/idle cleanup: destroying instead would + * sever the node's subscription (`removeAllListeners`) and evict the cache + * entry, leaving the node stuck on a texture that reloads to `loaded` but is + * never displayed. + * + * `texture.free()` reclaims tracked memory via `setTextureMemUse(0)` when a + * ctxTexture exists; the guard below keeps the accounting correct for any + * texture that entered `loadedTextures` without one. + * + * @param texture - The texture to free + */ + freeTexture(texture: Texture) { + if (this.debugLogging === true) { + console.log( + `[TextureMemoryManager] Freeing texture. State: ${texture.state}`, + ); + } + + texture.free(); + + if (this.loadedTextures.has(texture) === true) { + this.loadedTextures.delete(texture); + this.memUsed -= texture.memUsed; + texture.memUsed = 0; + } + } + /** * Destroy a texture and remove it from the memory manager * @@ -230,11 +268,12 @@ export class TextureMemoryManager { // Immediate cleanup if eligible if (isCleanableType && texture.canBeCleanedUp() === true) { - // Get memory before destroying + // Get memory before freeing const textureMemory = texture.memUsed; - // Destroy texture (which will null out the array position) - this.destroyTexture(texture); + // Reversibly free (keeps listeners + cache) so the texture reloads when + // its node re-enters the viewport. + this.freeTexture(texture); currentMemUsed -= textureMemory; } } diff --git a/visual-regression/certified-snapshots/chromium-ci/stress-animation-1.png b/visual-regression/certified-snapshots/chromium-ci/stress-animation-1.png new file mode 100644 index 0000000..34548d5 Binary files /dev/null and b/visual-regression/certified-snapshots/chromium-ci/stress-animation-1.png differ diff --git a/visual-regression/certified-snapshots/chromium-ci/texture-free-reload-1.png b/visual-regression/certified-snapshots/chromium-ci/texture-free-reload-1.png new file mode 100644 index 0000000..e4b65a0 Binary files /dev/null and b/visual-regression/certified-snapshots/chromium-ci/texture-free-reload-1.png differ