WebGLRenderer: Enable subframe upload in copyTextureToTexture, align API to 3d version

[gkjohnson]

Related issue: -

Description

Adds a Box2 "bounds" argument to copyTextureToTexture so we can upload just a small portion of a texture.

renderer.copyTextureToTexture( box, position, src, dst, level );

Specifically my use case is to be able copy just a single pixel of an id texture to a render target so it can be read back to the GPU to identify the id at the given pixel (see the EXT_mesh_features Cesium glTF extension). With this change we can copy just a single pixel from a large source texture to a 1x1 render target and then read it back whereas previously we'd have to copy to a render target of equivalent size. As far as I know there's no other way to reliably read just a single pixel from an arbitrary image.

From my testing with reading single pixel from a 2048 x 2048 image this improves the upload and read time from ~10ms to ~2ms. If async read is used presumably the ~2ms would go down quite a bit further, as well.

Benchmark code

const quad = new FullScreenQuad( new THREE.MeshBasicMaterial( { color: 0x0000ff } ) );
const srcTex = new THREE.DataTexture( new Uint8Array( 2048 * 2048 * 4 ).fill( 255 ), 4096, 4096 );
srcTex.needsUpdate = true;

{

	const target = new THREE.WebGLRenderTarget( 1, 1 );

	// initialize target
	renderer.setRenderTarget( target );
	quad.render( renderer );
	renderer.setRenderTarget( null );

	console.time( 'READ' );

	// copy data
	const bounds = new THREE.Box2();
	bounds.min.setScalar( 0 );
	bounds.max.setScalar( 1 );
	renderer.copyTextureToTexture( bounds, bounds.min, srcTex, target.texture );

	// read data
	const result = new Uint8Array( 4 );
	renderer.readRenderTargetPixels( target, 0, 0, 1, 1, result );
	console.log( result );

	console.timeEnd( 'READ' ); // ~2ms

}

{

	const target = new THREE.WebGLRenderTarget( 2048, 2048 );

	// initialize target
	renderer.setRenderTarget( target );
	quad.render( renderer );
	renderer.setRenderTarget( null );

	console.time( 'READ' );

	// copy data
	const bounds = new THREE.Box2();
	bounds.min.setScalar( 0 );
	bounds.max.setScalar( 2048 );
	renderer.copyTextureToTexture( bounds, bounds.min, srcTex, target.texture );

	// read data
	const result = new Uint8Array( 4 );
	renderer.readRenderTargetPixels( target, 0, 0, 1, 1, result );
	console.log( result );

	console.timeEnd( 'READ' ); // ~10ms

}

This contribution is funded by the Cesium GIS Ecosystem Grant

[github-actions]

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev	Filesize PR	Diff
677.2 kB (167.8 kB)	678.4 kB (168.1 kB)	+1.21 kB

🌳 Bundle size after tree-shaking

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

Filesize dev	Filesize PR	Diff
456.3 kB (110.1 kB)	457.5 kB (110.3 kB)	+1.21 kB

[gkjohnson]

One other discrepancy I noticed between the copyTextureToTexture and copyTextureToTexture3D implementations is that the 3D variant used "level" to index into the source textures mipmap array whereas it was always 0 in the 2d version. I've aligned to what the 3d function was doing but am happy to change it either way.

[Mugen87]

In general, I'm okay with this change. When adding copyTextureToTexture() I've originally wanted the signature as simple as possible. But I agree not having the option to define the source box is unfortunate.

Actually, I would prefer to have sourceBox as an optional parameter at the end of the signature but that would mean changing copyTextureToTexture3D() as well. Not sure this makes sense at this point...

[gkjohnson]

Actually, I would prefer to have sourceBox as an optional parameter at the end of the signature but that would mean changing copyTextureToTexture3D() as well. Not sure this makes sense at this point...

Either way the end result is the same: one function signature incurs a breaking change. If you'd prefer an optional argument now would be the time to do it, though the argument grouping becomes a little strange.

[Mugen87]

I vote for:

copyTextureToTexture( position, srcTexture, dstTexture, level, srcBox );
copyTextureToTexture3D( position, srcTexture, dstTexture, level, srcBox );

I agree the grouping is not ideal but always asking the users to define the source box like in the example feels inconvenient. There are for sure use cases where users want to copy the entire source texture into the destination texture and that should work as easy as possible.

[gkjohnson]

Just updated the signatures and update both examples!

[mrdoob]

I think srcRegion is more clear and follows the rest of the parameter names.

[mrdoob]

Or something like this?

this.copyTextureToTexture = function ( srcCoords: Vector2 | Box2, srcTexture, dstTexture, level = 0 )

[gkjohnson]

Or something like this?

this.copyTextureToTexture = function ( srcCoords: Vector2 | Box2, srcTexture, dstTexture, level = 0 )

position represents the position to copy the data into in the destination while sourceBox represents the region to copy from the source texture.

We can change the naming to something like the following:

dstPosition, srcTexture, dstTexture, level = 0, srcRegion = null

Though in my opinion something like this might make the most sense. We can change to this with backwards compatibility for 10 releases:

this.copyTextureToTexture = function ( srcTexture, dstTexture, srcRegion = null, dstPosition = null, level = 0 )

[mrdoob]

Though in my opinion something like this might make the most sense. We can change to this with backwards compatibility for 10 releases:

this.copyTextureToTexture = function ( srcTexture, dstTexture, srcRegion = null, dstPosition = null, level = 0 )

That looks much nicer indeed 👌

[gkjohnson]

Okay! It should be ready now

[mrdoob]

Should be like this?

// support previous signature with dstPosition first

[gkjohnson]

Oh yeah - copy paste error

[Mugen87]

0e01940