Skip to content

WebGPURenderer: Add Storage3DTexture and StorageArrayTexture #31175

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.

Sign up for GitHub

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

Merged
merged 25 commits into from
Jun 26, 2025
Merged

WebGPURenderer: Add Storage3DTexture and StorageArrayTexture #31175

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
a167227
Add support for texture_storage_2d_array
May 26, 2025
a62fcf8
additional extensions
May 29, 2025
da2caad
additional extensions
May 29, 2025
540c1d1
update
May 29, 2025
6f7c308
update
May 30, 2025
21f8d30
update
May 30, 2025
1d18ef1
update
May 30, 2025
f471b4d
update
May 30, 2025
44ab9fa
changed isTextureArray to isArrayTexture and the conditions
May 31, 2025
a295334
changed isTextureArray to isArrayTexture and the conditions
May 31, 2025
efe63ff
changed isTextureArray to isArrayTexture and the conditions
May 31, 2025
f32ec26
changed isTextureArray to isArrayTexture and the conditions
May 31, 2025
6478e35
with new classes for storageArrayTexture and storage3DTexture
Jun 23, 2025
bb35de5
with new classes for storageArrayTexture and storage3DTexture
Jun 23, 2025
8395fb2
with new classes for storageArrayTexture and storage3DTexture
Jun 23, 2025
5ad8c2e
with new classes for storageArrayTexture and storage3DTexture
Jun 23, 2025
912b3c3
with new classes for storageArrayTexture and storage3DTexture
Jun 23, 2025
c26fab3
with new classes for storageArrayTexture and storage3DTexture
Jun 23, 2025
dafe742
with new classes for storageArrayTexture and storage3DTexture
Jun 23, 2025
e5643c4
[200~
Jun 23, 2025
a1e9f0b
isSampledTexture3D replaced by is3DTexture, since it does not matter …
Jun 23, 2025
6db99f1
isSampledTexture3D replaced by is3DTexture, since it does not matter …
Jun 23, 2025
2d97903
isSampledTexture3D replaced by is3DTexture, since it does not matter …
Jun 23, 2025
177c903
isSampledTexture3D replaced by is3DTexture, since it does not matter …
Jun 23, 2025
8f89039
removed new class exports from Three.TSL.js
Jun 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/Three.WebGPU.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ export { default as PostProcessing } from './renderers/common/PostProcessing.js'
import * as RendererUtils from './renderers/common/RendererUtils.js';
export { RendererUtils };
export { default as StorageTexture } from './renderers/common/StorageTexture.js';
export { default as StorageTextureArray } from './renderers/common/StorageTextureArray.js';
export { default as StorageBufferAttribute } from './renderers/common/StorageBufferAttribute.js';
export { default as StorageInstancedBufferAttribute } from './renderers/common/StorageInstancedBufferAttribute.js';
export { default as IndirectStorageBufferAttribute } from './renderers/common/IndirectStorageBufferAttribute.js';
Expand Down
16 changes: 13 additions & 3 deletions src/renderers/common/StorageTexture.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,18 @@
*
* @param {number} [width=1] - The storage texture's width.
* @param {number} [height=1] - The storage texture's height.
* @param {number} [depth=0] - The storage texture's height.
*/
constructor( width = 1, height = 1 ) {
constructor( width = 1, height = 1, depth = 0 ) {
Copy link
Collaborator

@sunag sunag Jun 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think the default should be 1? and array more than 1...

Copy link
Contributor Author

@Spiri0 Spiri0 Jun 21, 2025

Choose a reason for hiding this comment

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

The reason I chose 0 was because someone might come up with the idea of ​​wanting to define an array with a length of 1, for example, when providing an app with a dynamic image array length, because that's possible, and then complain that it doesn't work.

If you're working with a TextureArray on the Threejs side and the WGSLNodeBuilder suddenly wants to use a normal texture because of the length 1, it crashes because of the wrong shader texture format.

A value of 0 clearly defines a normal texture, and a depth > 0 clearly defines a TextureArray.

But if you'd like, I can set that to 1.
In this case, it can be argued that the user has to take this into account on the threejs side, but this significantly increases the effort.

Copy link
Collaborator

@sunag sunag Jun 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Following this spec, 3DTexture does not support array, so the fact that it has count does not seem to make sense. *ArrayTexture and *3DTexture are distinct and should exist individually.

I think that #30959 needs to be reviewed or the other textures need to be, as there seems to be an incompatibility.

Textures

Property Description Type
*Texture width, height 2D
*ArrayTexture height, height, depth Array
*3DTexture height, height, depth 3D

RenderTarget

Property Description Type
*RenderTarget width, height, { count } 2D
*RenderTargetArray height, height, depth, { count } Array
*RenderTarget3D height, height, depth 3D

RenderTarget (Currently)

Property Description Type
*RenderTarget width, height, { count, depth } 2D/Array
*RenderTarget3D height, height, depth 3D
Property Type
width 2D
height 2D
depth Array/3D
count RenderTargets count

I have a feeling we should go back to RenderTargetArray and its logic but now preserve the functionality of the count property so we can have Multi Render Target Arrays.

So I would go back in the same way so that we have a StorageArrayTexture and soon Storage3DTexture too.

Changing texture properties I imagine could be a lot of work for us and the users, and this seems like it can be avoided.

@Mugen87 @RenaudRohlinger any thoughts on this?

Copy link
Contributor Author

@Spiri0 Spiri0 Jun 22, 2025

Choose a reason for hiding this comment

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

So, should we go back to a separate class like I originally had?

I would prefer that because it allows the user to clearly define the texture format by choosing the respective class:

StorageTexture,
StorageArrayTexture
Storage3DTexture

Copy link
Contributor Author

@Spiri0 Spiri0 Jun 22, 2025

Choose a reason for hiding this comment

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

As far as I can see, the places for the case distinction in the WGSLNodeBuilder and WebGPUBindingUtils are the same, which makes things easier.

Copy link
Collaborator

@sunag sunag Jun 22, 2025

Choose a reason for hiding this comment

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

So, should we go back to a separate class like I originally had?

Yes, it seems like the best way to me.

Copy link
Contributor Author

@Spiri0 Spiri0 Jun 22, 2025

Choose a reason for hiding this comment

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

I'll take care of it in the new week. I'll also validate the Storage3DTexture in my app.
Then we have elegantly covered all three variants, because I think the separate classes make things very clear for the user.


super();

/**
* The image object which just represents the texture's dimension.
*
* @type {{width: number, height: number}}
* @type {{width: number, height: number, depth: number}}
*/
this.image = { width, height };
this.image = { width, height, depth };

/**
* The default `magFilter` for storage textures is `THREE.LinearFilter`.
Expand All @@ -52,6 +53,15 @@
*/
this.isStorageTexture = true;

/**
* Indicates whether this texture is a texture array.
* This property is inherited from `Texture` and set here depending on the `depth` value.
*
* @type {boolean}
*

Check failure on line 61 in src/renderers/common/StorageTexture.js

View workflow job for this annotation

GitHub Actions / Lint testing 

Trailing spaces not allowed
*/
this.isTextureArray = depth > 0;

}

}
Expand Down
61 changes: 0 additions & 61 deletions src/renderers/common/StorageTextureArray.js
View file
Open in desktop

This file was deleted.

4 changes: 4 additions & 0 deletions src/renderers/webgpu/nodes/WGSLNodeBuilder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1722,6 +1722,10 @@ ${ flowData.code }

textureType = `texture_storage_2d${ isArray }<${ format }, ${ access }>`;

} else if ( uniform.node.value.isTextureArray === true ) {

textureType = 'texture_2d_array<f32>';

} else {

const componentPrefix = this.getComponentTypeFromTexture( texture ).charAt( 0 );
Expand Down
4 changes: 2 additions & 2 deletions src/renderers/webgpu/utils/WebGPUBindingUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ class WebGPUBindingUtils {

texture.viewDimension = GPUTextureViewDimension.Cube;

} else if ( binding.texture.isArrayTexture || binding.texture.isDataArrayTexture || binding.texture.isCompressedArrayTexture ) {
} else if ( binding.texture.isArrayTexture || binding.texture.isDataArrayTexture || binding.texture.isCompressedArrayTexture || binding.texture.isTextureArray ) {
Copy link
Collaborator

@Mugen87 Mugen87 May 30, 2025

Choose a reason for hiding this comment

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

Why do we have isArrayTexture and isTextureArray now? Can't you use the existing isArrayTexture?

Copy link
Contributor Author

@Spiri0 Spiri0 May 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I generally only use what is already available as a parameter, so I used isTextureArray.
screenshot from a StorageTexture node with r176 without my extension
image

If isArrayTexture were available, I would have used it. I can, of course, implement it, and that's why I'm asking. But then there would be two indicators for the array state in the StorageTextures, which I don't think is right, because then both would have to be set.
I assume there were reasons why isTextureArray was introduced, but I don't know them, and that's why I'm asking. One indicator is certainly better than two.

I could trace isTextureArray back in the code to where it plays a role. Perhaps it was simply created but not used anywhere yet, since storage textures aren't array-capable yet. Then we could remove isTextureArray from the code and replace it with isArrayTexture.


texture.viewDimension = GPUTextureViewDimension.TwoDArray;

Expand Down Expand Up @@ -438,7 +438,7 @@ class WebGPUBindingUtils {

dimensionViewGPU = GPUTextureViewDimension.ThreeD;

} else if ( binding.texture.isArrayTexture || binding.texture.isDataArrayTexture || binding.texture.isCompressedArrayTexture || ( binding.texture.isStorageTexture && binding.texture.isTextureArray ) ) {
} else if ( binding.texture.isArrayTexture || binding.texture.isDataArrayTexture || binding.texture.isCompressedArrayTexture || binding.texture.isTextureArray ) {

dimensionViewGPU = GPUTextureViewDimension.TwoDArray;

Expand Down
Loading