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 struct #30394

Merged
merged 13 commits into from
Jan 28, 2025
Merged

TSL: Introduce struct #30394

merged 13 commits into from
Jan 28, 2025

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Jan 25, 2025

Related issue: #29908, #29760

Description

PR introduces struct for TSL following the syntax below:

const BoundingBox = struct( {
	min: 'vec3',
	max: 'vec3'
} );

// style 1
const bb = BoundingBox( vec3( 0 ), vec3( 1 ) );

// style 1 - default value
const bb = BoundingBox();

// style 2
const bb = BoundingBox( {
	min: vec3( 0 ), 
	max: vec3( 1 ) 
} );

// style 2 - optional values
const bb = BoundingBox( {
	// min: vec3( 0 ),  // use optional value
	max: vec3( 1 ) 
} );

// assigns in TSL Fn
const bbFn = Fn( () => {

	const bb = BoundingBox();

	const bbMax = bb.get( 'max' );
	bbMax.assign( vec3( 1 ) );
	// or -> bb.get( 'max' ).assign( vec3( 1 ) );

	return bb;

} );

// debug
material.colorNode = bbFn().get( 'max' );

Copy link

github-actions bot commented Jan 25, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 336.27
78.29
336.27
78.29
+0 B
+0 B
WebGPU 513.63
142.61
516.74
143.47
+3.11 kB
+858 B
WebGPU Nodes 513.09
142.51
516.2
143.37
+3.11 kB
+857 B

🌳 Bundle size after tree-shaking

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

Before After Diff
WebGL 465.25
112.13
465.25
112.13
+0 B
+0 B
WebGPU 587.56
159.27
588.75
159.64
+1.19 kB
+364 B
WebGPU Nodes 542.94
148.83
544.13
149.2
+1.19 kB
+370 B

@sunag
Copy link
Collaborator Author

sunag commented Jan 28, 2025

I updated webgpu_compute_water, now we can use a single storage buffer to send multiples datas.

Previous

// Sphere Instance Storage
const sphereInstancePositionStorage = instancedArray( spherePositionArray, 'vec3' ).label( 'SpherePosition' );
const sphereVelocityStorage = instancedArray( sphereVelocityArray, 'vec2' ).label( 'SphereVelocity' );

Now

const SphereStruct = struct( {
	position: 'vec3',
	velocity: 'vec2'
} );

// Sphere Instance Storage
const sphereVelocityStorage = instancedArray( sphereArray, SphereStruct ).label( 'SphereData' );

@sunag sunag marked this pull request as ready for review January 28, 2025 17:38
@sunag sunag merged commit 2dc9a7e into mrdoob:dev Jan 28, 2025
12 checks passed
@sunag
Copy link
Collaborator Author

sunag commented Jan 28, 2025

It was somewhat well worked, but it worked, I had to make a lot of changes for this to create a new PR. Currently the struct works in both backends except if it is used in storage().

@Spiri0 I added your example and added you as a co-author. I had to make some small modifications to the example as well.

@cmhhelgeson
Copy link
Contributor

I updated webgpu_compute_water, now we can use a single storage buffer to send multiples datas.

Looks cool. If I have time this week, I think I'll update the helper functions in compute_water to return structs and have defined function layouts as well. The current helper functions for indices work fine, but are more fragile than explicitly defining named WGSL/GLSL functions using setLayout, especially if someone tries to call toVar() and assign the same name to variables.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 28, 2025

@sunag @Spiri0 Awesome work! 🎉

@Mugen87 Mugen87 added this to the r173 milestone Jan 28, 2025
@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 28, 2025

@cmhhelgeson While you are at it: Can you also check why webgpu_compute_water currently breaks with a WebGL 2 backend? The demo reports a runtime error in the vertex shader:

THREE.WebGLProgram: Shader Error 1282 - VALIDATE_STATUS false
Program Info Log: Must have a compiled vertex shader attached:
SHADER_INFO_LOG:
ERROR: 0:53: 'ndefined' : syntax error
VERTEX
ERROR: 0:53: 'ndefined' : syntax error

The ndefined looks suspicious...


/** @module StructNode **/

class StructNode extends Node {
Copy link
Collaborator

@Mugen87 Mugen87 Jan 28, 2025

Choose a reason for hiding this comment

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

Nit: It would be great to document this module and StructType 😇 .

@Spiri0
Copy link
Contributor

Spiri0 commented Jan 28, 2025

Ah here we go, very nice. Now threejs is really drawIndirect capable 💪 😊
Maybe I'll do more extensive examples in the future, but these simple ones convey quite a lot.
Docu is a good idea. I admit that I am not yet familiar with the standards. I have to read the threejs wiki.

@cmhhelgeson
Copy link
Contributor

@cmhhelgeson While you are at it: Can you also check why webgpu_compute_water currently breaks with a WebGL 2 backend? The demo reports a runtime error in the vertex shader:

THREE.WebGLProgram: Shader Error 1282 - VALIDATE_STATUS false
Program Info Log: Must have a compiled vertex shader attached:
SHADER_INFO_LOG:
ERROR: 0:53: 'ndefined' : syntax error
VERTEX
ERROR: 0:53: 'ndefined' : syntax error

The ndefined looks suspicious...

I'll see what I can do, but I've been time limited since the fall, so it might be a bit. Still got to get back to subgroupFunctionNode and additional compute samples as well.

@Methuselah96 Methuselah96 mentioned this pull request Jan 31, 2025
29 tasks
"imports": {
"three": "../src/three.webgpu.js",
"three/webgpu": "../src/three.webgpu.js",
"three/tsl": "../src/three.tsl.js",
Copy link
Owner

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

@cmhhelgeson
Copy link
Contributor

@cmhhelgeson While you are at it: Can you also check why webgpu_compute_water currently breaks with a WebGL 2 backend? The demo reports a runtime error in the vertex shader:

THREE.WebGLProgram: Shader Error 1282 - VALIDATE_STATUS false
Program Info Log: Must have a compiled vertex shader attached:
SHADER_INFO_LOG:
ERROR: 0:53: 'ndefined' : syntax error
VERTEX
ERROR: 0:53: 'ndefined' : syntax error

The ndefined looks suspicious...

@Mugen87 I just started looking into this issue twenty minutes ago, and it seems like it might be an issue with the getTypeFromAttribute function in GLSLNodeBuilder? In some of the vertex shaders, it appears like the nodeVarying5 variable is correctly assigned a value of float, but in each shader, nodeVarying5 is being assigned to the value of a node buffer attribute. I'm not sure why this would be necessary, as none of the position/vertex shaders are explicitly passing any new varyings to the fragment shader. The water position shader only modifies the existing varying of v_normalView.

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.

5 participants