Skip to content

Commit

Permalink
Fix: destroying the render texture frambuffers correctly (#10437)
Browse files Browse the repository at this point in the history
* fix up so that gpu counter part of render target is destroyed

* fix lint
  • Loading branch information
GoodBoyDigital committed Apr 30, 2024
1 parent f06dd68 commit 952a66c
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 1 deletion.
30 changes: 30 additions & 0 deletions src/rendering/renderers/gl/renderTarget/GlRenderTargetAdaptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,36 @@ export class GlRenderTargetAdaptor implements RenderTargetAdaptor<GlRenderTarget
return glRenderTarget;
}

public destroyGpuRenderTarget(gpuRenderTarget: GlRenderTarget)
{
const gl = this._renderer.gl;

if (gpuRenderTarget.framebuffer)
{
gl.deleteFramebuffer(gpuRenderTarget.framebuffer);
gpuRenderTarget.framebuffer = null;
}

if (gpuRenderTarget.resolveTargetFramebuffer)
{
gl.deleteFramebuffer(gpuRenderTarget.resolveTargetFramebuffer);
gpuRenderTarget.resolveTargetFramebuffer = null;
}

if (gpuRenderTarget.depthStencilRenderBuffer)
{
gl.deleteRenderbuffer(gpuRenderTarget.depthStencilRenderBuffer);
gpuRenderTarget.depthStencilRenderBuffer = null;
}

gpuRenderTarget.msaaRenderBuffer.forEach((renderBuffer) =>
{
gl.deleteRenderbuffer(renderBuffer);
});

gpuRenderTarget.msaaRenderBuffer = null;
}

public clear(_renderTarget: RenderTarget, clear: CLEAR_OR_BOOL, clearColor?: RgbaArray)
{
if (!clear) return;
Expand Down
16 changes: 16 additions & 0 deletions src/rendering/renderers/gpu/renderTarget/GpuRenderTargetAdaptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,22 @@ export class GpuRenderTargetAdaptor implements RenderTargetAdaptor<GpuRenderTarg
return gpuRenderTarget;
}

public destroyGpuRenderTarget(gpuRenderTarget: GpuRenderTarget)
{
gpuRenderTarget.contexts.forEach((context) =>
{
context.unconfigure();
});

gpuRenderTarget.msaaTextures.forEach((texture) =>
{
texture.destroy();
});

gpuRenderTarget.msaaTextures.length = 0;
gpuRenderTarget.contexts.length = 0;
}

public ensureDepthStencilTexture(renderTarget: RenderTarget)
{
// TODO This function will be more useful once we cache the descriptors
Expand Down
12 changes: 12 additions & 0 deletions src/rendering/renderers/shared/renderTarget/RenderTarget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ export class RenderTarget
public isRoot = false;

private readonly _size = new Float32Array(2);
/** if true, then when the render target is destroyed, it will destroy all the textures that were created for it. */
private readonly _managedColorTextures: boolean = false;

/**
* @param [descriptor] - Options for creating a render target.
Expand All @@ -95,6 +97,8 @@ export class RenderTarget

if (typeof descriptor.colorTextures === 'number')
{
this._managedColorTextures = true;

for (let i = 0; i < descriptor.colorTextures; i++)
{
this.colorTextures.push(new TextureSource({
Expand Down Expand Up @@ -223,6 +227,14 @@ export class RenderTarget
{
this.colorTexture.source.off('resize', this.onSourceResize, this);

if (this._managedColorTextures)
{
this.colorTextures.forEach((texture) =>
{
texture.destroy();
});
}

if (this.depthStencilTexture)
{
this.depthStencilTexture.destroy();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,12 @@ export interface RenderTargetAdaptor<RENDER_TARGET extends GlRenderTarget | GpuR
/** the render target to resize */
renderTarget: RenderTarget
): void

/** destroys the gpu render target */
destroyGpuRenderTarget(
/** the render target to destroy */
gpuRenderTarget: RENDER_TARGET
): void
}

/**
Expand Down Expand Up @@ -494,9 +500,17 @@ export class RenderTargetSystem<RENDER_TARGET extends GlRenderTarget | GpuRender
}

// TODO add a test for this
renderSurface.on('destroy', () =>
renderSurface.once('destroy', () =>
{
renderTarget.destroy();

const gpuRenderTarget = this._gpuRenderTargetHash[renderTarget.uid];

if (gpuRenderTarget)
{
this._gpuRenderTargetHash[renderTarget.uid] = null;
this.adaptor.destroyGpuRenderTarget(gpuRenderTarget);
}
});
}

Expand Down
39 changes: 39 additions & 0 deletions tests/renderering/render-target/RenderTarget.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { DOMAdapter } from '../../../src/environment/adapter';
import { GlRenderTarget } from '../../../src/rendering/renderers/gl/GlRenderTarget';
import { isRenderingToScreen } from '../../../src/rendering/renderers/shared/renderTarget/isRenderingToScreen';
import { RenderTarget } from '../../../src/rendering/renderers/shared/renderTarget/RenderTarget';
import { RenderTexture } from '../../../src/rendering/renderers/shared/texture/RenderTexture';
import { CanvasSource } from '../../../src/rendering/renderers/shared/texture/sources/CanvasSource';
import { Texture } from '../../../src/rendering/renderers/shared/texture/Texture';
import { getWebGLRenderer } from '../../utils/getRenderer';

describe('isRenderingToScreen', () =>
{
Expand Down Expand Up @@ -104,4 +107,40 @@ describe('isRenderingToScreen', () =>
expect(renderTarget.colorTexture.source.resource.width).toEqual(width * 3);
expect(renderTarget.colorTexture.source.resource.height).toEqual(height * 3);
});

it('should destroy gpu counterpart correctly', async () =>
{
const renderer = await getWebGLRenderer();

const renderTexture = RenderTexture.create({ width: 100, height: 100 });

renderer.renderTarget.bind(renderTexture);

const renderTarget = renderer.renderTarget.getRenderTarget(renderTexture);

const glRenderTarget = renderer.renderTarget.getGpuRenderTarget(renderTarget);

renderTexture.destroy(true);

expect(glRenderTarget.framebuffer).toBeNull();
expect(glRenderTarget.resolveTargetFramebuffer).toBeNull();
expect(glRenderTarget.msaaRenderBuffer).toBeNull();

expect(glRenderTarget).toBeInstanceOf(GlRenderTarget);
});

it('should not destroy the original texture if the renderTarget is destroyed', async () =>
{
const renderer = await getWebGLRenderer();

const renderTexture = RenderTexture.create({ width: 100, height: 100 });

renderer.renderTarget.bind(renderTexture);

const renderTarget = renderer.renderTarget.getRenderTarget(renderTexture);

renderTarget.destroy();

expect(renderTexture.source.destroyed).toBe(false);
});
});

0 comments on commit 952a66c

Please sign in to comment.