Skip to content

Conversation

RenaudRohlinger
Copy link
Collaborator

@RenaudRohlinger RenaudRohlinger commented May 31, 2024

Related issue: #28455

Description

This PR adds support for Read Only and Read/Write Storage Textures access in TSL.

Example:

const width = 512, height = 512;

const storageTexture = new StorageTexture( width, height );
storageTexture.format = THREE.RedFormat;
storageTexture.type = THREE.FloatType;

const computeTexture = tslFn( ( { storageTexture } ) => {

	const posX = instanceIndex.remainder( width );
	const posY = instanceIndex.div( width );
	const indexUV = uvec2( posX, posY );

	const oldTextureValue = textureLoad( storageTexture, indexUV ).setAccess('read-write')

	const newTextureValue = oldTextureValue.add( 0.1 );

	textureStore( storageTexture, indexUV, newTextureValue, 'read-write' );

} );

This contribution is funded by Utsubo

@RenaudRohlinger RenaudRohlinger marked this pull request as draft May 31, 2024 06:43
@RenaudRohlinger RenaudRohlinger requested a review from sunag May 31, 2024 08:37
@RenaudRohlinger RenaudRohlinger marked this pull request as ready for review May 31, 2024 08:40
@RenaudRohlinger RenaudRohlinger added this to the r166 milestone Jun 1, 2024

if ( depthSnippet ) {

return `textureLoad( ${ textureProperty }, ${ uvIndexSnippet }, ${ depthSnippet }, ${ levelSnippet } )`;

} else if ( accessSnippet ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we detect if it's a StorageTexture here instead of moving the setAccess() to TextureNode?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically since textureLoad inherits from TextureNode and not StorageTextureNode it's not possible.
We need to be able to specify the access in the reading (textureLoad) via textureLoad( readTex, ivec2( x, y ) ).setAccess( 'read-only' ) that can differ from the writing textureStore( writeTex, uv, value, 'read-write' ).

Alternative would be to add a 4th argument for textureLoad:
textureLoad( readTex, ivec2( x, y ), null, 'read-only' )

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we would extend generateSnippet to support generateTextureLoad with access on StorageTextureNode and keep TextureNode as it was. This was part of the implementation although not finalized use textureStore just to read texture if you don't add value as parameter to assign, for example: textureStore( storageTexture, indexUV )

@mrdoob mrdoob modified the milestones: r166, r167 Jun 28, 2024
@mrdoob mrdoob modified the milestones: r167, r168 Jul 25, 2024
@mrdoob mrdoob modified the milestones: r168, r169 Aug 30, 2024
@mrdoob mrdoob modified the milestones: r169, r170 Sep 26, 2024
@mrdoob mrdoob modified the milestones: r170, r171 Oct 31, 2024
@mrdoob mrdoob modified the milestones: r171, r172 Nov 29, 2024
@mrdoob mrdoob modified the milestones: r172, r173 Dec 31, 2024
@mrdoob mrdoob modified the milestones: r173, r174 Jan 31, 2025
@mrdoob mrdoob modified the milestones: r174, r175 Feb 27, 2025
@mrdoob mrdoob modified the milestones: r175, r176 Mar 28, 2025
@RenaudRohlinger
Copy link
Collaborator Author

Fixed with #29881

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