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

WebGPURenderer: Introduced .toConst(), Const(), Var() #30251

Merged
merged 12 commits into from
Jan 8, 2025

Conversation

RenaudRohlinger
Copy link
Collaborator

@RenaudRohlinger RenaudRohlinger commented Jan 2, 2025

Description

Currently it's only possible to create var in WGSL. This PR introduces the ability to generate let and const variables via const a = node.toConst('a')
Using let instead of var in TSL ensures immutability, improves code safety, allows better optimization, and clarifies developer intent by preventing unintended modifications.

I also took the opportunity to replace id by globalId in WGSL as it was confusing and globalId can be very useful, so I exposed it to userland.

This contribution is funded by Utsubo

Copy link

github-actions bot commented Jan 2, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 339.49
79.09
339.49
79.09
+0 B
+0 B
WebGPU 489.97
135.94
490.85
136.31
+874 B
+377 B
WebGPU Nodes 489.44
135.82
490.31
136.2
+874 B
+383 B

🌳 Bundle size after tree-shaking

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

Before After Diff
WebGL 465.35
112.14
465.35
112.14
+0 B
+0 B
WebGPU 559.35
151.41
560.16
151.73
+812 B
+322 B
WebGPU Nodes 515.45
141.2
516.26
141.52
+812 B
+320 B

@sunag
Copy link
Collaborator

sunag commented Jan 3, 2025

I suggest renaming it to .toConst(). Instead of the declaractionType parameter use an isConst or maybe readonly?

let for WGSL is immutable and needs an initial value in the declaration, just like const is for JS. TSL can follow the JS style here, and then we can detect if the input value is primitive number and use const in GLSL and WGSL for better optimization as alternative.

@RenaudRohlinger
Copy link
Collaborator Author

We need .toLet() alongside .toConst() because WGSL let allows runtime-evaluated values while remaining immutable, unlike const, which requires compile-time constants. This distinction aligns with JavaScript’s let vs. const and ensures flexibility for runtime-initialized immutables (let) and optimization for compile-time constants (const).

So I would suggest that we keep declarationType and introduce in another PR 'const' along 'let'.

@sunag
Copy link
Collaborator

sunag commented Jan 3, 2025

let is ambiguous with var. Many of our users have never used WGSL before and maybe never will use, so they don't need to know about it these differences that let has compared to WGSL, which is exactly like const in JS. If we implement toConst() I can later implement this logic in another PR, it will automatically declare const if it uses primitive values ​, and let as fallback if you are applying it to a variable and keep it as it is for GLSL. I can imagine the number of users who declared toLet() and then tried to do myLet.assign() because of this ambiguity. We can simplify this step with .toConst(), and this can be fully aligned with WGSL let.

@sunag
Copy link
Collaborator

sunag commented Jan 3, 2025

The example below shows how it would look in the API. We don't need to worry about declaring const in the backend language at this PR. I think the most important thing is to be aligned with the next updates.

const uniformValue = uniform( 1 );
const primitiveValue = float( 1 );

const val1 = primitiveValue.add( primitiveValue ).toConst(); // it will use `const` in WGSL/GLSL
const val2 = uniformValue.add( uniformValue ).toConst(); // it will use `let` in WGSL and keep as default in GLSL
const val3 = uniformValue.add( primitiveValue ).toConst(); // it will use `let` in WGSL and keep as default in GLSL

src/nodes/core/VarNode.js Fixed Show fixed Hide fixed
@RenaudRohlinger
Copy link
Collaborator Author

RenaudRohlinger commented Jan 4, 2025

@sunag I made some updates implementing toConst(), but I believe a part of the logic shouldn't be handled in the VarNode. I'm not sure how you see it.
I'm uncertain about how to detect if a node is a valid const, currently this PR breaks the let/const distinction in the WebGPUBackend as I'm struggling to sort them.

Feel free to push into this PR if you have something in mind, or it would be great if you could guide me more on these topics.

@RenaudRohlinger RenaudRohlinger changed the title WebGPURenderer: Introduced variable.toLet() WebGPURenderer: Introduced var.toConst() Jan 4, 2025
@sunag sunag added this to the r173 milestone Jan 4, 2025
@sunag
Copy link
Collaborator

sunag commented Jan 4, 2025

I made some small changes and added a function to detect if the code flow is deterministic.

deterministic flow

// tsl
const myConst = vec3( 1, 0, 1 ).add( 0 ).normalize().toConst();

// wgsl
const nodeVar0 = normalize( ( vec3<f32>( 1.0, 0.0, 1.0 ) + vec3<f32>( 0.0 ) ) );

// glsl
const vec3 nodeVar0 = normalize( ( vec3( 1.0, 0.0, 1.0 ) + vec3( 0.0 ) ) );

non-deterministic flow

// tsl
const myUniform = uniform( vec3( 0, 0, 1 ) );
const myConst = vec3( 1, 0, 1 ).add( 0 ).normalize().add( myUniform ).toConst();

// wgsl
let nodeVar0 = ( normalize( ( vec3<f32>( 1.0, 0.0, 1.0 ) + vec3<f32>( 0.0 ) ) ) + object.nodeUniform0 );

// glsl
vec3 nodeVar0;
nodeVar0 = ( normalize( ( vec3( 1.0, 0.0, 1.0 ) + vec3( 0.0 ) ) ) + f_nodeUniform0 );

@RenaudRohlinger
Copy link
Collaborator Author

RenaudRohlinger commented Jan 4, 2025

Thanks @sunag! I still have two cases not handled:

WorkgroupArray and similar not properly converted (ConvertNode?):

Error while parsing WGSL: :55:20 error: cannot assign 'u32' to 'vec2<u32>':
sharedData[ lid ] = NodeBuffer_538.nodeUniform0[ gid ].x;

and when having let and split (SplitOperator?):

 Error while parsing WGSL: 151:8 error: redeclaration of 'nodeVar4'
 let nodeVar4 = NodeBuffer_538.nodeUniform9[ ( k ) ].x;
          ^^^^^^^^

Error while parsing WGSL: 150:8 note: 'nodeVar4' previously declared here
let nodeVar4 = NodeBuffer_538.nodeUniform9[ ( k ) ].y;

@RenaudRohlinger
Copy link
Collaborator Author

This issue is actually smaller than I thought, the auto attribution of name is failing:

const a = buffers.indices.element(k).x.toConst('');
const b = buffers.indices.element(k).y.toConst('');

breaks since 2 let nodeVar4_readOnly -->

let nodeVar4_readOnly = NodeBuffer_538.nodeUniform9[ ( k ) ].x;
let nodeVar4_readOnly = NodeBuffer_538.nodeUniform9[ ( k ) ].y;

const a = buffers.indices.element(k).x.toConst('a');
const b = buffers.indices.element(k).y.toConst('b');

works -->

let a_readOnly = NodeBuffer_538.nodeUniform9[ ( k ) ].x;
let b_readOnly = NodeBuffer_538.nodeUniform9[ ( k ) ].y;

/cc @sunag

@mrdoob
Copy link
Owner

mrdoob commented Jan 4, 2025

Related reading 👀
https://www.dneto.dev/posts/2025/wgsl-evaluation-phase/

@sunag
Copy link
Collaborator

sunag commented Jan 4, 2025

@RenaudRohlinger Would it be possible to reproduce the type conversion problem in some existing example? I tried in webgpu_compute_sort_bitonic but I couldn't reproduce the problem.

@sunag sunag changed the title WebGPURenderer: Introduced var.toConst() WebGPURenderer: Introduced node.toConst() Jan 4, 2025
@RenaudRohlinger
Copy link
Collaborator Author

I just forgot to merge upstream, the fix was in #30222! All good, thanks for the help @sunag 🙌

@Makio64
Copy link
Contributor

Makio64 commented Jan 5, 2025

Happy new year everyone !

I hope you had a wonderful time @RenaudRohlinger @sunag !

I'm catching up with all the commit & updates!

Is toConst() integrated in the glsl / wgsl -> tsl convertor ?

Also as the user point of view, I agree with @sunag, I don't plane to use wgsl in my threejs projects and want to rely on under-optimization made by threejs tsl engine.

Thanks for your involvement to make our creative coding life easier and able to focus on rendering only !

Wish you a productive year with lot of loves & success!

Best,
D

@boytchev
Copy link
Contributor

boytchev commented Jan 6, 2025

I'm not quite sure whether this could be considered off-topic, or whether it has already been discussed. The chaining tends to push the top-level functions towards the back of an expression. I often find myself forgetting adding .toVar when needed, or using .toVar where not needed and forgetting about it. Would it make sense to also have non-chained version:

Example:

const myConst = vec3( 1, 0, 1 ).add( 0 ).normalize().toConst();  // method
const myConst = toConst( vec3( 1, 0, 1 ).add( 0 ).normalize() ); // function

The same concept could be extended to other existing or future functions:

const myConst = vec3( 1, 0, 1 ).add( 0 ).normalize().toVar();  // method
const myConst = toVar( vec3( 1, 0, 1 ).add( 0 ).normalize() ); // function

In a more iconoclastic context, what about using Const (and Var):

const myConst = Const( vec3( 1, 0, 1 ).add( 0 ).normalize() ); // function
const myConst = Var( vec3( 1, 0, 1 ).add( 0 ).normalize() ); // function

This might be consistent with things like float and .toFloat. The capital first letter looks strange, as it is not a class, but there are already exceptions like If and Loop.

@sunag
Copy link
Collaborator

sunag commented Jan 8, 2025

Thanks @RenaudRohlinger @mrdoob @Makio64 @boytchev.

The Var/Const idea sounds great, updated.

@sunag sunag changed the title WebGPURenderer: Introduced node.toConst() WebGPURenderer: Introduced .toConst(), Const(), Var() Jan 8, 2025
@sunag sunag merged commit 73eddcb into mrdoob:dev Jan 8, 2025
12 checks passed
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