Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebGLRenderer: Support render target textures in copyTextureToTexture(). #29662

Merged
merged 3 commits into from
Oct 19, 2024

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Oct 15, 2024

Related #29612

Description

This PR makes sure texture and depthTexture can be copied between render targets via copyTextureToTexture(). The idea is to use blitFramebuffer() since it offers a wider range of supported formats compared to copyTexSubImage2D().

To make this change possible, it was necessary to save the reference of a render target texture to its render target in textureProperties.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Oct 15, 2024

I have implemented a similar change for the WebGL backend of WebGPURenderer here: b19b10c. It's part of #29636 so I can verify the code via the TRAA implementation.

Copy link

github-actions bot commented Oct 15, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 690.67
171.1
691.82
171.3
+1.15 kB
+199 B
WebGPU 816.43
220.08
816.43
220.08
+0 B
+0 B
WebGPU Nodes 815.94
219.95
815.94
219.95
+0 B
+0 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 463.31
111.89
464.46
112.09
+1.15 kB
+205 B
WebGPU 538.61
145.41
538.61
145.41
+0 B
+0 B
WebGPU Nodes 494.73
135.28
494.73
135.28
+0 B
+0 B

@Mugen87 Mugen87 added this to the r170 milestone Oct 15, 2024
@gkjohnson
Copy link
Collaborator

Thanks! We should update the docs to indicate that srcTexture can be a DepthTexture of RenderTarget. And we should update the 3D texture version of the function, as well. Then I can test it with my use case (copying a block of array texture data between render targets when expanding the size).

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Oct 16, 2024

It seems the code for 3D render targets is the same since there is no difference on framebuffer level.

@gkjohnson
Copy link
Collaborator

gkjohnson commented Oct 16, 2024

I've copied the WebGLRenderer to my imported copy of three and I'm seeing errors when running the following cases - perhaps I'm not using it correctly?

// For 3d textures
const rt3d0 = new WebGLArrayRenderTarget( 10, 10, 10 );
const rt3d1 = new WebGLArrayRenderTarget( 10, 10, 10 );
renderer.initRenderTarget( rt3d0 );
renderer.initRenderTarget( rt3d1 );
renderer.copyTextureToTexture3D( rt3d0, rt3d1.texture );

// For 2d textures
const rt0 = new WebGLRenderTarget( 10, 10 );
const rt1 = new WebGLRenderTarget( 10, 10 );
renderer.initRenderTarget( rt0 );
renderer.initRenderTarget( rt1 );
renderer.copyTextureToTexture( rt0, rt1.texture );

It seems there's still some handling that needs to be worked out when dealing with render targets vs textures for the source. The original srcTexture.isTexture !== true check will fail when passing a render target in, for example, causing the argument order to be rearranged.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Oct 16, 2024

You have to pass in the texture reference, not the render target itself.

renderer.copyTextureToTexture3D( rt3d0.texture, rt3d1.texture );

Just do it like in your initial post: #29612 (comment) 😉 .

@gkjohnson
Copy link
Collaborator

Got it - I was tripped up by the isRenderTargetTexture and though that meant passing the render target in. I also wasn't able to get it working with 3d render targets even with either syntax.

Here's a repro script for array render targets failing to copy - though perhaps I've missed something here, too. You can paste this in to any example page script to see:

Repro snippet
			import * as THREE from 'three';
			import { FullScreenQuad } from 'three/addons/postprocessing/Pass.js';
			
			const width = window.innerWidth;
			const height = window.innerHeight;

			// init
			const camera = new THREE.PerspectiveCamera( 70, width / height, 0.01, 10 );
			camera.position.z = 1;

			// renderer
			const renderer = new THREE.WebGLRenderer();
			renderer.setSize( width, height );
			renderer.setClearColor( 0xffffff );
			renderer.setPixelRatio( window.devicePixelRatio );
			document.body.appendChild( renderer.domElement );

			// targets
			const quad = new FullScreenQuad( new THREE.MeshBasicMaterial( { color: 'red' } ) );
			const target1 = new THREE.WebGLArrayRenderTarget( 100, 100, 10 );
			const target2 = new THREE.WebGLArrayRenderTarget( 200, 200, 20 );

			// scene
			const scene = new THREE.Scene();
			const geometry = new THREE.PlaneGeometry();
			const material = new THREE.ShaderMaterial( {
				uniforms: {
					map: { value: null },
				},
				vertexShader: /* glsl */`
					varying vec2 vUv;
					void main() {
						vUv = uv;
						gl_Position = projectionMatrix * modelViewMatrix * vec4( position, 1.0 );
					}
				`,
				fragmentShader: /* glsl */`
					precision highp sampler2DArray;
					uniform sampler2DArray map;
					varying vec2 vUv;
					void main() {
						gl_FragColor = texture( map, vec3( vUv, 0 ) );
					}
				`,
			} );
			const mesh = new THREE.Mesh( geometry, material );
			scene.add( mesh );

			// populate layer 0 of the initial target with red 
			renderer.setRenderTarget( target1, 0 );
			quad.render( renderer );

			// copy the data of texture 1 into texture 2
			renderer.initRenderTarget( target2 );
			renderer.copyTextureToTexture3D( target1.texture, target2.texture );

			// final render render to the screen
			// NOTE: Setting "map" to texture1.texture displays red, as expected
			renderer.setRenderTarget( null );
			material.uniforms.map.value = target2.texture;
			renderer.render( scene, camera );

The above script:

  • Renders a red color into layer 0 of array target 1
  • Use copyTextureToTexture3D to copy all data from target 1 into target 2
  • Render a quad with the target 2 contents.

When rendering target 2 the contents are displayed as black. Rendering target 1 displays red as expected. The function works when using 2d render targets.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Oct 16, 2024

The 3D version requires calls of framebufferTextureLayer() for some reasons. The snippet seems to be fixed now.

@gkjohnson
Copy link
Collaborator

gkjohnson commented Oct 17, 2024

Hmm. It seems to only copy the first layer of the array target, now. Other layers are left uncopied. You can test this by rendering layer 0 and 1 as red, copying, and then rendering layers 0 and 1 of the new target separately. Layer 1 will render as black, 0 as red.

It looks like the signature of blitFramebuffer only supports arguments for 2d targets. I wonder if there's another way to copy all the 3d data at once? Is there someone who might know better we can ping? Maybe greggman?

An alternative would be to continue using this per-layer 2D blit copy but iterate over all layers inside the function so the full set of requested texture data is copied. It's still an improvement over the full screen quad render method.

@Makio64
Copy link
Contributor

Makio64 commented Oct 17, 2024

I was wondering if we could use ‘copyTexSubImage3D’ to copy on gpu level instead of copying on cpu to gpu (from what I understand by having a quicklook)

•	gl.copyTexSubImage3D copies data directly from the framebuffer to the texture.
•	gl.texSubImage3D uploads data from client memory (e.g., arrays) to the texture.

Use gl.copyTexSubImage3D when you’re copying from the framebuffer, and gl.texSubImage3D when you’re updating the texture from data in memory.

@gkjohnson
Copy link
Collaborator

I was wondering if we could use ‘copyTexSubImage3D’ to copy on gpu level instead of copying on cpu to gpu (from what I understand by having a quicklook)

From what I can tell this function also only copies a 2d sub window of data between the targets, not a 3d one. It sounds like what you'd want, though. You can see a more detailed description of the function in the opengl docs. Notice that it only allows for specifying a 2d space in the destination texture (ie x, y, width, height) rather than a 3d one. Confusingly the texSubImage3D allows for specifying a 3d volume in the destination texture (eg x, y, z, width, height, depth).

However it may still be worth using the copyTexSubImage* functions rather than blitFrameBuffer since it affords scaling and filtering which may or may not incur some overhead.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Oct 17, 2024

The problem is copyTexSubImage*() do not support copying depth which is a red flag for post processing use cases. So we potentially need to use both methods to cover all use cases.

@gkjohnson
Copy link
Collaborator

Oh gotcha - didn't realize that blit included the other buffers, too.

In terms of recommendations or comments on using one over the other - here are a few threads I found:

  • Ken Russell says that blit is not used for copying data between buffers internally (link)
  • And this stackoverflow post shows ~10% improvement using copyTexSubImage2d over blit (link)

Of course it will vary from hardware to hardware but it seems like the copy* functions are the better choice when we can use them.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Oct 19, 2024

TBH, I have a hard time to test the specific 3D/array render target setups which are uncommon for the post processing point of view. I was primarily interested in getting the basic operations right (base texture + depth).

@gkjohnson If you want to support more use cases, I would favor to merge the PR so you can start from this baseline with a different PR. I'd like to focus on a other topics.

@gkjohnson
Copy link
Collaborator

I understand! We can merge this and I'll make the remaining adjustments in another PR - #29612 shouldn't be closed yet, though, since it's not complete.

@Mugen87 Mugen87 merged commit 19db40b into mrdoob:dev Oct 19, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants