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

TSL: Introduce attributeArray and instancedArray #29881

Merged
merged 7 commits into from
Nov 23, 2024
Merged

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Nov 14, 2024

Description

Node allows the addition of anonymous attributes, so we don't need to link it to geometry for different effects. This also allows us to simplify the code for whoever is creating their shaders, making the process much simpler, closer to what would we do in conventional (non-gpu) JS code.

I made a revision so that toReadOnly() can be automatically applied to WebGPU if it is not in the compute shader, simplifying the process.

Common usage would be attributeArray( count|array, type ) or instancedArray( count|array, type ) can be seen in the example below.

webgpu_compute_birds example

New approach

// Labels applied to storage nodes and uniform nodes are reflected within the shader output,
// and are useful for debugging purposes.
				
const positionStorage = attributeArray( positionArray, 'vec3' ).label( 'positionStorage' );
const velocityStorage = attributeArray( velocityArray, 'vec3' ).label( 'velocityStorage' );
const phaseStorage = attributeArray( phaseArray, 'float' ).label( 'phaseStorage' );

Previous approach

// Create storage buffer attributes.

const positionBufferAttribute = new THREE.StorageBufferAttribute( positionArray, 3 );
const velocityBufferAttribute = new THREE.StorageBufferAttribute( velocityArray, 3 );
const phaseBufferAttribute = new THREE.StorageBufferAttribute( phaseArray, 1 );

// Labels applied to storage nodes and uniform nodes are reflected within the shader output,
// and are useful for debugging purposes.

// Access storage buffer attribute data from within shaders with a StorageNode.

const positionStorage = storage( positionBufferAttribute, 'vec3', positionBufferAttribute.count ).label( 'positionStorage' );
const velocityStorage = storage( velocityBufferAttribute, 'vec3', velocityBufferAttribute.count ).label( 'velocityStorage' );
const phaseStorage = storage( phaseBufferAttribute, 'float', phaseBufferAttribute.count ).label( 'phaseStorage' );

// Create read-only storage nodes. Storage nodes can only be accessed outside of compute shaders in a read-only state.

const positionRead = storageObject( positionBufferAttribute, 'vec3', positionBufferAttribute.count ).toReadOnly();
const velocityRead = storageObject( velocityBufferAttribute, 'vec3', velocityBufferAttribute.count ).toReadOnly();
const phaseRead = storageObject( phaseBufferAttribute, 'float', phaseBufferAttribute.count ).toReadOnly();

Copy link

github-actions bot commented Nov 14, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 339.13
79
339.13
79
+0 B
+0 B
WebGPU 482.81
133.76
483.66
134
+856 B
+239 B
WebGPU Nodes 482.27
133.66
483.13
133.9
+856 B
+237 B

🌳 Bundle size after tree-shaking

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

Before After Diff
WebGL 464.59
111.96
464.59
111.96
+0 B
+0 B
WebGPU 550.87
149.23
550.96
149.22
+93 B
-8 B
WebGPU Nodes 506.75
138.95
506.84
138.94
+93 B
-10 B

@cmhhelgeson
Copy link
Contributor

This is a great encapsulation of the existing storage buffer functionality. Maybe you're still planning on adding this, but would users still have the ability to specify their storage node as an atomic?

@sunag
Copy link
Collaborator Author

sunag commented Nov 14, 2024

The PR does not remove the functionality of storage(), as array is storage, it could be used like: array().toAtomic().

@sunag
Copy link
Collaborator Author

sunag commented Nov 23, 2024

I'm thinking about renaming array to vertexArray or attributeArray. I have a feeling this might confuse users since uniformArray has a similar purpose.

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 23, 2024

In that case I vote for attributeArray().

@sunag sunag changed the title TSL: Introduce array and instancedArray TSL: Introduce attributeArray and instancedArray Nov 23, 2024
@sunag sunag marked this pull request as ready for review November 23, 2024 18:17
@sunag sunag merged commit 89d1a06 into mrdoob:dev Nov 23, 2024
12 checks passed
@sunag sunag deleted the dev-arrays branch November 23, 2024 18:27
@Mugen87 Mugen87 added this to the r171 milestone Nov 23, 2024
// The Pixel Buffer Object (PBO) is required to get the GPU computed data to the CPU in the WebGL2 fallback.
// As used in `renderer.getArrayBufferAsync( waveArray.value )`.

waveNode.setPBO( true );
Copy link
Collaborator

Choose a reason for hiding this comment

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

@sunag The example throws a runtime error right now since setPBO() does not exist:

Uncaught (in promise) TypeError: waveNode.setPBO is not a function
at init (webgpu_compute_audio.html:107:14)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Commented out the line for now: #29972

The effect in WebGL 2 does not sound correct though.

@Mugen87 Mugen87 mentioned this pull request Nov 26, 2024
@@ -113,6 +113,7 @@ const exceptionList = [
// Awaiting for WebGL backend support
'webgpu_clearcoat',
'webgpu_compute_audio',
"webgpu_compute_birds",
Copy link
Collaborator

@Mugen87 Mugen87 Nov 26, 2024

Choose a reason for hiding this comment

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

It seems it was necessary to comment out the example because the WebGL 2 version is broken with this PR. The following WebGL warning occurs:

WebGL warning: drawArraysInstanced: Vertex fetch requires 147456, but attribs only supply 16384.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My idea was to release another PR after this for deprecating storageObject(), I hope to do it this week. The fallback was working but with differences even before the PR, as you can see in the image, after a few seconds in the WebGL2 version the birds are concentrated in the center, in the WebGPU version they spread.

image

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