Skip to content

Commit

Permalink
๐ŸŽจ Refactor CoreNode Renderability Logic ๐Ÿš€ (#476)
Browse files Browse the repository at this point in the history
### Whatโ€™s New? โœจ
This PR refactors Texture Throttling in the following ways:

- ๐Ÿ•’ Async RTT Texture Loading: When an RTT node is set, wait for the
texture to load asynchronously.
- ๐Ÿ—‘๏ธ Improved Texture Cleanup: When a texture is freed, its source
texture is now also marked as freed. Fixes a bug where textures weren't
returning after memory cleanup.
- ๐Ÿ”„ Auto-Trigger Texture Loading: Refactored isRenderable owner changes
to automatically call texture loading if the texture state is freed or
initial.

## How to Test? ๐Ÿงช

RTT Changes: Use `test=rtt-dimension`. ๐Ÿ–ผ๏ธ You should see race conditions
occurring prior to this PR.

Texture Freed Fix: Add a rocko.png to the
`test=texture-cleanup-critical` test and move it in and out of the
screen to validate. ๐ŸŽฏ (Not the prettiest, but it works!). Might create
an automated test for this in the future.

## Whatโ€™s Next? ๐Ÿš€
More Testing! ๐Ÿ› ๏ธ
  • Loading branch information
wouterlucas authored Jan 8, 2025
2 parents bd2186d + e31f190 commit 422d51e
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 25 deletions.
39 changes: 20 additions & 19 deletions src/core/CoreNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1017,7 +1017,7 @@ export class CoreNode extends EventEmitter {
if (this.updateType & UpdateType.RenderTexture && this.rtt) {
// Only the RTT node itself triggers `renderToTexture`
this.hasRTTupdates = true;
this.stage.renderer?.renderToTexture(this);
this.loadRenderTexture();
}

if (this.updateType & UpdateType.Global) {
Expand Down Expand Up @@ -1225,11 +1225,7 @@ export class CoreNode extends EventEmitter {
//check if CoreNode is renderable based on props
hasRenderableProperties(): boolean {
if (this.texture !== null) {
if (this.texture.state === 'loaded') {
return true;
}

return false;
return true;
}

if (!this.props.width || !this.props.height) {
Expand Down Expand Up @@ -1401,22 +1397,12 @@ export class CoreNode extends EventEmitter {
*/
updateIsRenderable() {
let newIsRenderable: boolean;
if (this.worldAlpha === 0 || !this.hasRenderableProperties()) {
if (this.worldAlpha === 0 || this.hasRenderableProperties() === false) {
newIsRenderable = false;
} else {
newIsRenderable = this.renderState > CoreNodeRenderState.OutOfBounds;
}

// If the texture is not loaded and the node is renderable, load the texture
// this only needs to happen once or until the texture is no longer loaded
if (
this.texture !== null &&
this.texture.state === 'freed' &&
this.renderState > CoreNodeRenderState.OutOfBounds
) {
this.stage.txManager.loadTexture(this.texture);
}

if (this.isRenderable !== newIsRenderable) {
this.isRenderable = newIsRenderable;
this.onChangeIsRenderable(newIsRenderable);
Expand Down Expand Up @@ -2045,10 +2031,25 @@ export class CoreNode extends EventEmitter {
height: this.height,
});

this.loadRenderTexture();
}

private loadRenderTexture() {
if (this.texture === null) {
return;
}

// If the texture is already loaded, render to it immediately
if (this.texture.state === 'loaded') {
this.stage.renderer?.renderToTexture(this);
return;
}

// call load immediately to ensure the texture is created
this.stage.txManager.loadTexture(this.texture, true);

this.stage.renderer?.renderToTexture(this); // Only this RTT node
this.texture.once('loaded', () => {
this.stage.renderer?.renderToTexture(this); // Only this RTT node
});
}

private cleanupRenderTexture() {
Expand Down
7 changes: 6 additions & 1 deletion src/core/Stage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,12 @@ export class Stage {
addQuads(node: CoreNode) {
assertTruthy(this.renderer);

if (node.isRenderable === true) {
// If the node is renderable and has a loaded texture, render it
if (
node.isRenderable === true &&
node.texture !== null &&
node.texture.state === 'loaded'
) {
node.renderQuads(this.renderer);
}

Expand Down
22 changes: 17 additions & 5 deletions src/core/textures/Texture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,12 @@ export interface TextureData {
premultiplyAlpha?: boolean | null;
}

export type TextureState = 'freed' | 'loading' | 'loaded' | 'failed';
export type TextureState =
| 'initial'
| 'freed'
| 'loading'
| 'loaded'
| 'failed';

export enum TextureType {
'generic' = 0,
Expand Down Expand Up @@ -155,11 +160,11 @@ export abstract class Texture extends EventEmitter {
readonly error: Error | null = null;

// aggregate state
public state: TextureState = 'freed';
public state: TextureState = 'initial';
// texture source state
private sourceState: TextureState = 'freed';
private sourceState: TextureState = 'initial';
// texture (gpu) state
private coreCtxState: TextureState = 'freed';
private coreCtxState: TextureState = 'initial';

readonly renderableOwners = new Set<unknown>();

Expand Down Expand Up @@ -195,13 +200,19 @@ export abstract class Texture extends EventEmitter {
*/
setRenderableOwner(owner: unknown, renderable: boolean): void {
const oldSize = this.renderableOwners.size;
if (renderable) {
if (renderable === true) {
this.renderableOwners.add(owner);
const newSize = this.renderableOwners.size;

if (newSize > oldSize && newSize === 1) {
(this.renderable as boolean) = true;
(this.lastRenderableChangeTime as number) = this.txManager.frameTime;
this.onChangeIsRenderable?.(true);

// Check if the texture needs to be added to the loading queue
if (this.state === 'freed' || this.state === 'initial') {
this.txManager.loadTexture(this);
}
}
} else {
this.renderableOwners.delete(owner);
Expand Down Expand Up @@ -249,6 +260,7 @@ export abstract class Texture extends EventEmitter {
this.ctxTexture?.free();
if (this.textureData !== null) {
this.textureData = null;
this.setSourceState('freed');
}
}

Expand Down

0 comments on commit 422d51e

Please sign in to comment.