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: Editor #26270

Merged
merged 20 commits into from
Jun 17, 2023
Merged

TSL: Editor #26270

merged 20 commits into from
Jun 17, 2023

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Jun 16, 2023

Live

https://raw.githack.com/mrdoob/three.js/dev/examples/webgpu_tsl_editor.html

Description

In this editor we can better observe the code generated in WGSL and soon GLSL.

image

/cc @mrdoob

@sunag sunag added the TSL Three.js Shading Language label Jun 16, 2023
@sunag sunag marked this pull request as ready for review June 16, 2023 02:04
@sunag sunag added this to the r154 milestone Jun 16, 2023
@mrdoob
Copy link
Owner

mrdoob commented Jun 16, 2023

Super handy!

@@ -97,7 +97,7 @@ export const tangentGeometry = nodeImmutable( TangentNode, TangentNode.GEOMETRY
export const tangentLocal = nodeImmutable( TangentNode, TangentNode.LOCAL );
export const tangentView = nodeImmutable( TangentNode, TangentNode.VIEW );
export const tangentWorld = nodeImmutable( TangentNode, TangentNode.WORLD );
export const transformedTangentView = label( tangentView, 'TransformedTangentView' );
export const transformedTangentView = temp( tangentView, 'TransformedTangentView' );
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we can no longer use label here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think temp() would be responsible for creating a temporary variable, with a name or anonymous (automatic). label() must force the assignment of a name, usually related to bindings: uniform/textures/...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but here we exactly create a variable with the given name, aren't we?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I think that's what I mentioned above about with a name or anonymous... Seems easier to use node.temp( 'myVar' ) instead of node.temp().label( 'myVar' ). label() now doesn't create variables anymore, it just helps to define a name when it is declared.

@@ -80,10 +80,8 @@ class VarNode extends Node {

export default VarNode;

export const label = nodeProxy( VarNode );
export const temp = label;
export const temp = nodeProxy( VarNode );
Copy link
Contributor

@LeviPesin LeviPesin Jun 16, 2023

Choose a reason for hiding this comment

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

If we remove label from VarNode then I suggest to also remove name argument from it?

<script type="module">

import * as THREE from 'three';
import * as NODES from 'three/nodes';
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we used to import nodes as Nodes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I noticed certain users using NODES, I have doubts what to do here, all namespaces are uppercase by default, should we keep it or change it? @mrdoob

@sunag sunag merged commit b691d70 into mrdoob:dev Jun 17, 2023
@sunag sunag mentioned this pull request Jun 18, 2023
@sunag sunag deleted the dev-tsl-editor branch June 18, 2023 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TSL Three.js Shading Language
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants