Skip to content

Commit 652d1cc

Browse files
authored
Refactor Texture Management and Garbage collection (#647)
This change improves the Texture Management and Garbage collection routines. To eliminate the "stuck texture" problem: ![IMG_9685](https://github.com/user-attachments/assets/4ce9f08b-2c00-47ec-a116-2c0f858d23f0) Fixes: - Auto retry mechanism in case of failure - Expose retry API on texture to trigger manually - Expose max retry count property on texture - Fix stuck texture failure case - Reset dimensions on texture re-initialise - Fix GC race condition on starved CPUs - Optimized renderability toggling for faster loading - Remove old fetching/fetched states - Extended Inspector to have more metrics surrounding texture loading for easy debugging
2 parents 5780615 + b763b51 commit 652d1cc

19 files changed

+791
-387
lines changed

src/core/CoreNode.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import type { TextureOptions } from './CoreTextureManager.js';
2727
import type { CoreRenderer } from './renderers/CoreRenderer.js';
2828
import type { Stage } from './Stage.js';
2929
import {
30+
TextureType,
3031
type Texture,
3132
type TextureFailedEventHandler,
3233
type TextureFreedEventHandler,
@@ -835,6 +836,7 @@ export class CoreNode extends EventEmitter {
835836

836837
texture.preventCleanup =
837838
this.props.textureOptions?.preventCleanup ?? false;
839+
838840
texture.on('loaded', this.onTextureLoaded);
839841
texture.on('failed', this.onTextureFailed);
840842
texture.on('freed', this.onTextureFreed);
@@ -866,7 +868,7 @@ export class CoreNode extends EventEmitter {
866868
this.texture.off('loaded', this.onTextureLoaded);
867869
this.texture.off('failed', this.onTextureFailed);
868870
this.texture.off('freed', this.onTextureFreed);
869-
this.texture.setRenderableOwner(this, false);
871+
this.texture.setRenderableOwner(this._id, false);
870872
}
871873
}
872874

@@ -908,6 +910,7 @@ export class CoreNode extends EventEmitter {
908910
// immediately set isRenderable to false, so that we handle the error
909911
// without waiting for the next frame loop
910912
this.isRenderable = false;
913+
this.updateTextureOwnership(false);
911914
this.setUpdateType(UpdateType.IsRenderable);
912915

913916
// If parent has a render texture, flag that we need to update
@@ -925,6 +928,7 @@ export class CoreNode extends EventEmitter {
925928
// immediately set isRenderable to false, so that we handle the error
926929
// without waiting for the next frame loop
927930
this.isRenderable = false;
931+
this.updateTextureOwnership(false);
928932
this.setUpdateType(UpdateType.IsRenderable);
929933

930934
// If parent has a render texture, flag that we need to update
@@ -1506,7 +1510,7 @@ export class CoreNode extends EventEmitter {
15061510
* Changes the renderable state of the node.
15071511
*/
15081512
updateTextureOwnership(isRenderable: boolean) {
1509-
this.texture?.setRenderableOwner(this, isRenderable);
1513+
this.texture?.setRenderableOwner(this._id, isRenderable);
15101514
}
15111515

15121516
/**
@@ -2333,6 +2337,7 @@ export class CoreNode extends EventEmitter {
23332337
sy: this.props.srcY,
23342338
sw: this.props.srcWidth,
23352339
sh: this.props.srcHeight,
2340+
maxRetryCount: this.props.textureOptions.maxRetryCount,
23362341
});
23372342
}
23382343

@@ -2418,13 +2423,12 @@ export class CoreNode extends EventEmitter {
24182423

24192424
const oldTexture = this.props.texture;
24202425
if (oldTexture) {
2421-
oldTexture.setRenderableOwner(this, false);
24222426
this.unloadTexture();
24232427
}
24242428

24252429
this.props.texture = value;
24262430
if (value !== null) {
2427-
value.setRenderableOwner(this, this.isRenderable);
2431+
value.setRenderableOwner(this._id, this.isRenderable);
24282432
this.loadTexture();
24292433
}
24302434

src/core/CoreTextureManager.ts

Lines changed: 59 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,20 @@ export interface TextureOptions {
138138
*/
139139
preventCleanup?: boolean;
140140

141+
/**
142+
* Number of times to retry loading a failed texture
143+
*
144+
* @remarks
145+
* When a texture fails to load, Lightning will retry up to this many times
146+
* before permanently giving up. Each retry will clear the texture ownership
147+
* and then re-establish it to trigger a new load attempt.
148+
*
149+
* Set to null to disable retries. Set to 0 to always try once and never retry.
150+
* This is typically only used on ImageTexture instances.
151+
*
152+
*/
153+
maxRetryCount?: number | null;
154+
141155
/**
142156
* Flip the texture horizontally when rendering
143157
*
@@ -329,27 +343,13 @@ export class CoreTextureManager extends EventEmitter {
329343
return texture as InstanceType<TextureMap[Type]>;
330344
}
331345

332-
orphanTexture(texture: Texture): void {
333-
// if it is part of the download or upload queue, remove it
334-
this.removeTextureFromQueue(texture);
335-
336-
if (texture.type === TextureType.subTexture) {
337-
// ignore subtextures
338-
return;
339-
}
340-
341-
this.stage.txMemManager.addToOrphanedTextures(texture);
342-
}
343-
344346
/**
345347
* Override loadTexture to use the batched approach.
346348
*
347349
* @param texture - The texture to load
348350
* @param immediate - Whether to prioritize the texture for immediate loading
349351
*/
350-
loadTexture(texture: Texture, priority?: boolean): void {
351-
this.stage.txMemManager.removeFromOrphanedTextures(texture);
352-
352+
async loadTexture(texture: Texture, priority?: boolean): Promise<void> {
353353
if (texture.type === TextureType.subTexture) {
354354
// ignore subtextures - they get loaded through their parent
355355
return;
@@ -360,7 +360,7 @@ export class CoreTextureManager extends EventEmitter {
360360
return;
361361
}
362362

363-
if (Texture.TRANSITIONAL_TEXTURE_STATES.includes(texture.state)) {
363+
if (texture.state === 'loading') {
364364
return;
365365
}
366366

@@ -370,46 +370,33 @@ export class CoreTextureManager extends EventEmitter {
370370
return;
371371
}
372372

373-
// If the texture failed to load, we need to re-download it.
374-
if (texture.state === 'failed') {
375-
texture.free();
376-
texture.freeTextureData();
377-
}
378-
379373
texture.setState('loading');
380374

381-
// Get the texture data
382-
texture
383-
.getTextureData()
384-
.then(() => {
385-
if (texture.state !== 'fetched') {
386-
texture.setState('failed');
387-
return;
388-
}
389-
390-
// For non-image textures, upload immediately
391-
if (texture.type !== TextureType.image) {
392-
this.uploadTexture(texture).catch((err) => {
393-
console.error('Failed to upload non-image texture:', err);
394-
texture.setState('failed');
395-
});
396-
} else {
397-
// For image textures, queue for throttled upload
398-
// If it's a priority texture, upload it immediately
399-
if (priority === true) {
400-
this.uploadTexture(texture).catch((err) => {
401-
console.error('Failed to upload priority texture:', err);
402-
texture.setState('failed');
403-
});
404-
} else {
405-
this.enqueueUploadTexture(texture);
406-
}
407-
}
408-
})
409-
.catch((err) => {
410-
console.error(err);
375+
// Get texture data - early return on failure
376+
const textureDataResult = await texture.getTextureData().catch((err) => {
377+
console.error(err);
378+
texture.setState('failed');
379+
return null;
380+
});
381+
382+
// Early return if texture data fetch failed
383+
if (textureDataResult === null || texture.state === 'failed') {
384+
return;
385+
}
386+
387+
// Handle non-image textures: upload immediately
388+
const shouldUploadImmediately =
389+
texture.type !== TextureType.image || priority === true;
390+
if (shouldUploadImmediately === true) {
391+
await this.uploadTexture(texture).catch((err) => {
392+
console.error(`Failed to upload texture:`, err);
411393
texture.setState('failed');
412394
});
395+
return;
396+
}
397+
398+
// Queue image textures for throttled upload
399+
this.enqueueUploadTexture(texture);
413400
}
414401

415402
/**
@@ -424,11 +411,30 @@ export class CoreTextureManager extends EventEmitter {
424411
this.stage.txMemManager.criticalCleanupRequested === true
425412
) {
426413
// we're at a critical memory threshold, don't upload textures
427-
texture.setState('failed');
414+
texture.setState('failed', new Error('Memory threshold exceeded'));
415+
return;
416+
}
417+
418+
if (texture.state === 'failed' || texture.state === 'freed') {
419+
// don't upload failed or freed textures
420+
return;
421+
}
422+
423+
if (texture.state === 'loaded') {
424+
// already loaded
425+
return;
426+
}
427+
428+
if (texture.textureData === null) {
429+
texture.setState(
430+
'failed',
431+
new Error('Texture data is null, cannot upload texture'),
432+
);
428433
return;
429434
}
430435

431436
const coreContext = texture.loadCtxTexture();
437+
432438
if (coreContext !== null && coreContext.state === 'loaded') {
433439
texture.setState('loaded');
434440
return;
@@ -529,18 +535,6 @@ export class CoreTextureManager extends EventEmitter {
529535
}
530536
}
531537

532-
/**
533-
* Remove texture from the upload queue
534-
*
535-
* @param texture - The texture to remove
536-
*/
537-
removeTextureFromQueue(texture: Texture): void {
538-
const uploadIndex = this.uploadTextureQueue.indexOf(texture);
539-
if (uploadIndex !== -1) {
540-
this.uploadTextureQueue.splice(uploadIndex, 1);
541-
}
542-
}
543-
544538
/**
545539
* Resolve a parent texture from the cache or fallback to the provided texture.
546540
*

src/core/Stage.ts

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,6 @@ export class Stage {
353353
* Create default PixelTexture
354354
*/
355355
createDefaultTexture() {
356-
console.log('Creating default texture');
357356
(this.defaultTexture as ColorTexture) = this.txManager.createTexture(
358357
'ColorTexture',
359358
{
@@ -367,7 +366,7 @@ export class Stage {
367366
// Mark the default texture as ALWAYS renderable
368367
// This prevents it from ever being cleaned up.
369368
// Fixes https://github.com/lightning-js/renderer/issues/262
370-
this.defaultTexture.setRenderableOwner(this, true);
369+
this.defaultTexture.setRenderableOwner('stage', true);
371370

372371
// When the default texture is loaded, request a render in case the
373372
// RAF is paused. Fixes: https://github.com/lightning-js/renderer/issues/123
@@ -422,16 +421,6 @@ export class Stage {
422421
// Reset render operations and clear the canvas
423422
renderer.reset();
424423

425-
// Check if we need to cleanup textures
426-
if (this.txMemManager.criticalCleanupRequested === true) {
427-
this.txMemManager.cleanup(false);
428-
429-
if (this.txMemManager.criticalCleanupRequested === true) {
430-
// If we still need to cleanup, request another but aggressive cleanup
431-
this.txMemManager.cleanup(true);
432-
}
433-
}
434-
435424
// If we have RTT nodes draw them first
436425
// So we can use them as textures in the main scene
437426
if (renderer.rttNodes.length > 0) {
@@ -451,6 +440,11 @@ export class Stage {
451440
if (renderRequested) {
452441
this.renderRequested = false;
453442
}
443+
444+
// Check if we need to cleanup textures
445+
if (this.txMemManager.criticalCleanupRequested === true) {
446+
this.txMemManager.cleanup();
447+
}
454448
}
455449

456450
/**
@@ -815,12 +809,12 @@ export class Stage {
815809
}
816810

817811
/**
818-
* Cleanup Orphaned Textures
812+
* Cleanup Unused Textures
819813
*
820814
* @remarks
821-
* This method is used to cleanup orphaned textures that are no longer in use.
815+
* This method is used to cleanup unused textures that are no longer in use.
822816
*/
823-
cleanup(aggressive: boolean) {
824-
this.txMemManager.cleanup(aggressive);
817+
cleanup() {
818+
this.txMemManager.cleanup();
825819
}
826820
}

0 commit comments

Comments
 (0)