From 91117d008ded130f2a9c371880e809e7f0b7fe63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aki=20Rodi=C4=87?= Date: Mon, 2 Mar 2026 05:18:15 +0100 Subject: [PATCH 1/2] WebGPURenderer: Fixes framebuffer caching with multiple references of different sizes (#32690) --- src/nodes/display/ViewportDepthTextureNode.js | 26 +- src/nodes/display/ViewportTextureNode.js | 25 +- src/renderers/common/Renderer.js | 36 ++- .../display/ViewportDepthTextureNode.tests.js | 132 ++++++++++ .../display/ViewportTextureNode.tests.js | 236 ++++++++++++++++++ test/unit/three.source.unit.js | 5 + 6 files changed, 425 insertions(+), 35 deletions(-) create mode 100644 test/unit/src/nodes/display/ViewportDepthTextureNode.tests.js create mode 100644 test/unit/src/nodes/display/ViewportTextureNode.tests.js diff --git a/src/nodes/display/ViewportDepthTextureNode.js b/src/nodes/display/ViewportDepthTextureNode.js index 3fa7ca9267b535..c849b59eeef58e 100644 --- a/src/nodes/display/ViewportDepthTextureNode.js +++ b/src/nodes/display/ViewportDepthTextureNode.js @@ -26,28 +26,23 @@ class ViewportDepthTextureNode extends ViewportTextureNode { * * @param {Node} [uvNode=screenUV] - The uv node. * @param {?Node} [levelNode=null] - The level node. + * @param {?DepthTexture} [depthTexture=null] - A depth texture. If not provided, uses a shared depth texture. */ - constructor( uvNode = screenUV, levelNode = null ) { + constructor( uvNode = screenUV, levelNode = null, depthTexture = null ) { - if ( _sharedDepthbuffer === null ) { + if ( depthTexture === null ) { - _sharedDepthbuffer = new DepthTexture(); + if ( _sharedDepthbuffer === null ) { - } + _sharedDepthbuffer = new DepthTexture(); - super( uvNode, levelNode, _sharedDepthbuffer ); + } - } + depthTexture = _sharedDepthbuffer; - /** - * Overwritten so the method always returns the unique shared - * depth texture. - * - * @return {DepthTexture} The shared depth texture. - */ - getTextureForReference() { + } - return _sharedDepthbuffer; + super( uvNode, levelNode, depthTexture ); } @@ -62,6 +57,7 @@ export default ViewportDepthTextureNode; * @function * @param {?Node} [uvNode=screenUV] - The uv node. * @param {?Node} [levelNode=null] - The level node. + * @param {?DepthTexture} [depthTexture=null] - A depth texture. If not provided, a depth texture is created automatically. * @returns {ViewportDepthTextureNode} */ -export const viewportDepthTexture = /*@__PURE__*/ nodeProxy( ViewportDepthTextureNode ).setParameterLength( 0, 2 ); +export const viewportDepthTexture = /*@__PURE__*/ nodeProxy( ViewportDepthTextureNode ).setParameterLength( 0, 3 ); diff --git a/src/nodes/display/ViewportTextureNode.js b/src/nodes/display/ViewportTextureNode.js index 7057a5c856e667..5aa2ebc9ebf151 100644 --- a/src/nodes/display/ViewportTextureNode.js +++ b/src/nodes/display/ViewportTextureNode.js @@ -99,12 +99,12 @@ class ViewportTextureNode extends TextureNode { } /** - * This methods returns a texture for the given render target reference. + * This methods returns a texture for the given render target or canvas target reference. * * To avoid rendering errors, `ViewportTextureNode` must use unique framebuffer textures * for different render contexts. * - * @param {?RenderTarget} [reference=null] - The render target reference. + * @param {?(RenderTarget|CanvasTarget)} [reference=null] - The render target or canvas target reference. * @return {Texture} The framebuffer texture. */ getTextureForReference( reference = null ) { @@ -144,9 +144,13 @@ class ViewportTextureNode extends TextureNode { updateReference( frame ) { - const renderTarget = frame.renderer.getRenderTarget(); + const renderer = frame.renderer; + const renderTarget = renderer.getRenderTarget(); + const canvasTarget = renderer.getCanvasTarget(); - this.value = this.getTextureForReference( renderTarget ); + const reference = renderTarget ? renderTarget : canvasTarget; + + this.value = this.getTextureForReference( reference ); return this.value; @@ -156,20 +160,27 @@ class ViewportTextureNode extends TextureNode { const renderer = frame.renderer; const renderTarget = renderer.getRenderTarget(); + const canvasTarget = renderer.getCanvasTarget(); - if ( renderTarget === null ) { + const reference = renderTarget ? renderTarget : canvasTarget; + + if ( reference === null ) { renderer.getDrawingBufferSize( _size ); + } else if ( reference.getDrawingBufferSize ) { + + reference.getDrawingBufferSize( _size ); + } else { - _size.set( renderTarget.width, renderTarget.height ); + _size.set( reference.width, reference.height ); } // - const framebufferTexture = this.getTextureForReference( renderTarget ); + const framebufferTexture = this.getTextureForReference( reference ); if ( framebufferTexture.image.width !== _size.width || framebufferTexture.image.height !== _size.height ) { diff --git a/src/renderers/common/Renderer.js b/src/renderers/common/Renderer.js index a497da24192255..7fc089726706c9 100644 --- a/src/renderers/common/Renderer.js +++ b/src/renderers/common/Renderer.js @@ -456,13 +456,12 @@ class Renderer { this._transparentSort = null; /** - * The framebuffer target. + * Cache of framebuffer targets per canvas target. * * @private - * @type {?RenderTarget} - * @default null + * @type {Map} */ - this._frameBufferTarget = null; + this._frameBufferTargets = new Map(); const alphaClear = this.alpha === true ? 0 : 1; @@ -1341,9 +1340,11 @@ class Renderer { const { width, height } = this.getDrawingBufferSize( _drawingBufferSize ); const { depth, stencil } = this; - let frameBufferTarget = this._frameBufferTarget; + const canvasTarget = this._canvasTarget; + + let frameBufferTarget = this._frameBufferTargets.get( canvasTarget ); - if ( frameBufferTarget === null ) { + if ( frameBufferTarget === undefined ) { frameBufferTarget = new RenderTarget( width, height, { depthBuffer: depth, @@ -1359,7 +1360,7 @@ class Renderer { frameBufferTarget.isPostProcessingRenderTarget = true; - this._frameBufferTarget = frameBufferTarget; + this._frameBufferTargets.set( canvasTarget, frameBufferTarget ); } @@ -1377,8 +1378,6 @@ class Renderer { } - const canvasTarget = this._canvasTarget; - frameBufferTarget.viewport.copy( canvasTarget._viewport ); frameBufferTarget.scissor.copy( canvasTarget._scissor ); frameBufferTarget.viewport.multiplyScalar( canvasTarget._pixelRatio ); @@ -2034,7 +2033,7 @@ class Renderer { * Defines the viewport. * * @param {number | Vector4} x - The horizontal coordinate for the upper left corner of the viewport origin in logical pixel unit. - * @param {number} y - The vertical coordinate for the upper left corner of the viewport origin in logical pixel unit. + * @param {number} y - The vertical coordinate for the upper left corner of the viewport origin in logical pixel unit. * @param {number} width - The width of the viewport in logical pixel unit. * @param {number} height - The height of the viewport in logical pixel unit. * @param {number} minDepth - The minimum depth value of the viewport. WebGPU only. @@ -2410,7 +2409,13 @@ class Renderer { this._renderContexts.dispose(); this._textures.dispose(); - if ( this._frameBufferTarget !== null ) this._frameBufferTarget.dispose(); + for ( const frameBufferTarget of this._frameBufferTargets.values() ) { + + frameBufferTarget.dispose(); + + } + + this._frameBufferTargets.clear(); Object.values( this.backend.timestampQueryPool ).forEach( queryPool => { @@ -2512,8 +2517,13 @@ class Renderer { this.setOutputRenderTarget( null ); this.setRenderTarget( null ); - this._frameBufferTarget.dispose(); - this._frameBufferTarget = null; + for ( const frameBufferTarget of this._frameBufferTargets.values() ) { + + frameBufferTarget.dispose(); + + } + + this._frameBufferTargets.clear(); } diff --git a/test/unit/src/nodes/display/ViewportDepthTextureNode.tests.js b/test/unit/src/nodes/display/ViewportDepthTextureNode.tests.js new file mode 100644 index 00000000000000..10b4491b1e7faf --- /dev/null +++ b/test/unit/src/nodes/display/ViewportDepthTextureNode.tests.js @@ -0,0 +1,132 @@ +import { viewportDepthTexture } from '../../../../../src/nodes/display/ViewportDepthTextureNode.js'; +import { DepthTexture } from '../../../../../src/textures/DepthTexture.js'; +import { RenderTarget } from '../../../../../src/core/RenderTarget.js'; + +export default QUnit.module( 'Nodes', () => { + + QUnit.module( 'Display', () => { + + QUnit.module( 'ViewportDepthTextureNode', () => { + + // DEPTH TEXTURE CACHING + QUnit.module( 'Depth Texture Caching', () => { + + QUnit.test( 'getTextureForReference returns shared buffer for null reference', ( assert ) => { + + const node = viewportDepthTexture(); + const texture = node.getTextureForReference( null ); + + assert.strictEqual( + texture, node.defaultFramebuffer, + 'null reference should return the shared depth buffer' + ); + + } ); + + QUnit.test( 'different references get independent cached depth textures', ( assert ) => { + + const node = viewportDepthTexture(); + + // Create mock canvas targets (simulating different canvases) + const canvasTarget1 = { isCanvasTarget: true, id: 1 }; + const canvasTarget2 = { isCanvasTarget: true, id: 2 }; + + // Get depth textures for each reference + const depthTex1 = node.getTextureForReference( canvasTarget1 ); + const depthTex2 = node.getTextureForReference( canvasTarget2 ); + + // CRITICAL: Different references must get different textures + assert.notStrictEqual( + depthTex1, depthTex2, + 'Different CanvasTarget references MUST get different cached depth textures' + ); + + // Both should be DepthTexture instances + assert.ok( + depthTex1 instanceof DepthTexture, + 'Cached texture for canvasTarget1 should be a DepthTexture' + ); + assert.ok( + depthTex2 instanceof DepthTexture, + 'Cached texture for canvasTarget2 should be a DepthTexture' + ); + + // Neither should be the shared buffer (they should be clones) + assert.notStrictEqual( + depthTex1, node.defaultFramebuffer, + 'Cached texture should not be the shared default buffer' + ); + assert.notStrictEqual( + depthTex2, node.defaultFramebuffer, + 'Cached texture should not be the shared default buffer' + ); + + } ); + + QUnit.test( 'same reference returns same cached depth texture', ( assert ) => { + + const node = viewportDepthTexture(); + + const canvasTarget = { isCanvasTarget: true }; + + // Get texture twice for same reference + const depthTex1 = node.getTextureForReference( canvasTarget ); + const depthTex2 = node.getTextureForReference( canvasTarget ); + + assert.strictEqual( + depthTex1, depthTex2, + 'Same reference should return the same cached depth texture' + ); + + } ); + + // Test with RenderTargets + QUnit.test( 'RenderTargets get independent cached depth textures', ( assert ) => { + + const node = viewportDepthTexture(); + + const renderTarget1 = new RenderTarget( 512, 512 ); + const renderTarget2 = new RenderTarget( 256, 256 ); + + const depthTex1 = node.getTextureForReference( renderTarget1 ); + const depthTex2 = node.getTextureForReference( renderTarget2 ); + + assert.notStrictEqual( + depthTex1, depthTex2, + 'Different RenderTargets MUST get different cached depth textures' + ); + + // Clean up + renderTarget1.dispose(); + renderTarget2.dispose(); + + } ); + + // Test mixed CanvasTarget and RenderTarget references + QUnit.test( 'CanvasTarget and RenderTarget get independent caches', ( assert ) => { + + const node = viewportDepthTexture(); + + const canvasTarget = { isCanvasTarget: true }; + const renderTarget = new RenderTarget( 512, 512 ); + + const canvasDepthTex = node.getTextureForReference( canvasTarget ); + const renderDepthTex = node.getTextureForReference( renderTarget ); + + assert.notStrictEqual( + canvasDepthTex, renderDepthTex, + 'CanvasTarget and RenderTarget MUST get different cached depth textures' + ); + + // Clean up + renderTarget.dispose(); + + } ); + + } ); + + } ); + + } ); + +} ); diff --git a/test/unit/src/nodes/display/ViewportTextureNode.tests.js b/test/unit/src/nodes/display/ViewportTextureNode.tests.js new file mode 100644 index 00000000000000..9b28ef4f0f4740 --- /dev/null +++ b/test/unit/src/nodes/display/ViewportTextureNode.tests.js @@ -0,0 +1,236 @@ +import { viewportTexture } from '../../../../../src/nodes/display/ViewportTextureNode.js'; +import { FramebufferTexture } from '../../../../../src/textures/FramebufferTexture.js'; +import { RenderTarget } from '../../../../../src/core/RenderTarget.js'; + +export default QUnit.module( 'Nodes', () => { + + QUnit.module( 'Display', () => { + + QUnit.module( 'ViewportTextureNode', () => { + + // CACHING BEHAVIOR + QUnit.module( 'Texture Caching', () => { + + QUnit.test( 'getTextureForReference returns defaultFramebuffer for null reference', ( assert ) => { + + const node = viewportTexture(); + const texture = node.getTextureForReference( null ); + + assert.strictEqual( + texture, node.defaultFramebuffer, + 'null reference should return defaultFramebuffer' + ); + + } ); + + QUnit.test( 'getTextureForReference caches textures per reference', ( assert ) => { + + const node = viewportTexture(); + + // Create mock references (using RenderTargets as they work as cache keys) + const renderTarget1 = new RenderTarget( 512, 512 ); + const renderTarget2 = new RenderTarget( 256, 256 ); + + // Get textures for each reference + const texture1a = node.getTextureForReference( renderTarget1 ); + const texture1b = node.getTextureForReference( renderTarget1 ); + const texture2 = node.getTextureForReference( renderTarget2 ); + + // Same reference should return same cached texture + assert.strictEqual( + texture1a, texture1b, + 'Same reference should return same cached texture' + ); + + // Different references should return different textures + assert.notStrictEqual( + texture1a, texture2, + 'Different references should return different cached textures' + ); + + // Both should be FramebufferTexture instances + assert.ok( + texture1a instanceof FramebufferTexture, + 'Cached texture should be FramebufferTexture instance' + ); + assert.ok( + texture2 instanceof FramebufferTexture, + 'Cached texture should be FramebufferTexture instance' + ); + + // Clean up + renderTarget1.dispose(); + renderTarget2.dispose(); + + } ); + + QUnit.test( 'cached textures are independent from defaultFramebuffer', ( assert ) => { + + const node = viewportTexture(); + + const renderTarget = new RenderTarget( 512, 512 ); + const cachedTexture = node.getTextureForReference( renderTarget ); + + assert.notStrictEqual( + cachedTexture, node.defaultFramebuffer, + 'Cached texture for a reference should not be the defaultFramebuffer' + ); + + // Clean up + renderTarget.dispose(); + + } ); + + QUnit.test( 'multiple render targets get independent caches', ( assert ) => { + + const node = viewportTexture(); + + // Create multiple render targets with different sizes + const targets = [ + new RenderTarget( 512, 512 ), + new RenderTarget( 256, 256 ), + new RenderTarget( 128, 128 ), + new RenderTarget( 1024, 1024 ) + ]; + + const textures = targets.map( target => node.getTextureForReference( target ) ); + + // All textures should be unique + for ( let i = 0; i < textures.length; i ++ ) { + + for ( let j = i + 1; j < textures.length; j ++ ) { + + assert.notStrictEqual( + textures[ i ], textures[ j ], + `Texture ${i} should be different from texture ${j}` + ); + + } + + } + + // Verify caching works for all + targets.forEach( ( target, index ) => { + + const retrieved = node.getTextureForReference( target ); + assert.strictEqual( + retrieved, textures[ index ], + `Re-retrieving texture ${index} should return cached version` + ); + + } ); + + // Clean up + targets.forEach( target => target.dispose() ); + + } ); + + } ); + + // REFERENCE PRIORITY + QUnit.module( 'Reference Priority', () => { + + QUnit.test( 'referenceNode delegation works correctly', ( assert ) => { + + // Create a parent node + const parentNode = viewportTexture(); + + // Create a child node that references the parent + const childNode = viewportTexture(); + childNode.referenceNode = parentNode; + + const renderTarget = new RenderTarget( 512, 512 ); + + // When childNode has a referenceNode, it should use parent's cache + const textureFromChild = childNode.getTextureForReference( renderTarget ); + const textureFromParent = parentNode.getTextureForReference( renderTarget ); + + assert.strictEqual( + textureFromChild, textureFromParent, + 'Child node with referenceNode should use parent cache' + ); + + // Clean up + renderTarget.dispose(); + + } ); + + // When rendering to a RenderTarget, it should take priority over CanvasTarget + QUnit.test( 'updateReference prioritizes renderTarget over canvasTarget', ( assert ) => { + + const node = viewportTexture(); + + // Create mock targets + const renderTarget = new RenderTarget( 512, 512 ); + const canvasTarget = { isCanvasTarget: true }; // Mock canvas target + + // Create mock renderer that returns both targets + const mockRenderer = { + getRenderTarget: () => renderTarget, + getCanvasTarget: () => canvasTarget + }; + + // Create mock frame object + const mockFrame = { + renderer: mockRenderer + }; + + // Call updateReference + node.updateReference( mockFrame ); + + // The node.value should be the texture for renderTarget, not canvasTarget + const expectedTexture = node.getTextureForReference( renderTarget ); + const canvasTexture = node.getTextureForReference( canvasTarget ); + + assert.strictEqual( + node.value, expectedTexture, + 'When both renderTarget and canvasTarget exist, renderTarget should be used as reference' + ); + + assert.notStrictEqual( + node.value, canvasTexture, + 'canvasTarget texture should NOT be used when renderTarget is available' + ); + + // Clean up + renderTarget.dispose(); + + } ); + + // Test the edge case: when only canvasTarget is available (renderTarget is null) + QUnit.test( 'updateReference uses canvasTarget when renderTarget is null', ( assert ) => { + + const node = viewportTexture(); + + const canvasTarget = { isCanvasTarget: true }; // Mock canvas target + + // Create mock renderer where renderTarget is null + const mockRenderer = { + getRenderTarget: () => null, + getCanvasTarget: () => canvasTarget + }; + + const mockFrame = { + renderer: mockRenderer + }; + + // Call updateReference + node.updateReference( mockFrame ); + + // The node.value should be the texture for canvasTarget + const expectedTexture = node.getTextureForReference( canvasTarget ); + + assert.strictEqual( + node.value, expectedTexture, + 'When renderTarget is null, canvasTarget should be used as reference' + ); + + } ); + + } ); + + } ); + + } ); + +} ); diff --git a/test/unit/three.source.unit.js b/test/unit/three.source.unit.js index 37ea95329d7bd2..96c8e2ca0c3602 100644 --- a/test/unit/three.source.unit.js +++ b/test/unit/three.source.unit.js @@ -285,3 +285,8 @@ import './src/textures/FramebufferTexture.tests.js'; import './src/textures/Source.tests.js'; import './src/textures/Texture.tests.js'; import './src/textures/VideoTexture.tests.js'; + + +//src/nodes/display +import './src/nodes/display/ViewportTextureNode.tests.js'; +import './src/nodes/display/ViewportDepthTextureNode.tests.js'; From 38331a28c54f35c8063263fd29b854eeaea668c7 Mon Sep 17 00:00:00 2001 From: sunag Date: Mon, 2 Mar 2026 01:38:44 -0300 Subject: [PATCH 2/2] WebGPURenderer: Add individual dispose for `CanvasTarget` caching (#33106) --- src/renderers/common/Renderer.js | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/renderers/common/Renderer.js b/src/renderers/common/Renderer.js index 7fc089726706c9..6372c866f6872c 100644 --- a/src/renderers/common/Renderer.js +++ b/src/renderers/common/Renderer.js @@ -1360,6 +1360,18 @@ class Renderer { frameBufferTarget.isPostProcessingRenderTarget = true; + const dispose = () => { + + canvasTarget.removeEventListener( 'dispose', dispose ); + + frameBufferTarget.dispose(); + + this._frameBufferTargets.delete( canvasTarget ); + + }; + + canvasTarget.addEventListener( 'dispose', dispose ); + this._frameBufferTargets.set( canvasTarget, frameBufferTarget ); } @@ -2409,14 +2421,12 @@ class Renderer { this._renderContexts.dispose(); this._textures.dispose(); - for ( const frameBufferTarget of this._frameBufferTargets.values() ) { + for ( const canvasTarget of this._frameBufferTargets.keys() ) { - frameBufferTarget.dispose(); + canvasTarget.dispose(); } - this._frameBufferTargets.clear(); - Object.values( this.backend.timestampQueryPool ).forEach( queryPool => { if ( queryPool !== null ) queryPool.dispose(); @@ -2517,14 +2527,12 @@ class Renderer { this.setOutputRenderTarget( null ); this.setRenderTarget( null ); - for ( const frameBufferTarget of this._frameBufferTargets.values() ) { + for ( const canvasTarget of this._frameBufferTargets.keys() ) { - frameBufferTarget.dispose(); + canvasTarget.dispose(); } - this._frameBufferTargets.clear(); - } /**