NodeEditor: programatic node body function

[fyoudine]

Generate Functions programmatically so complexe nodes can create function and call it

[fyoudine]

@sunag Can I let you check for potential conflicts with WGSL

[sunag]

The idea is really interesting, maybe NodalFunctionNode can be a ShaderFunctionNode, or ShaderNode with some extra arguments...

I think that users doesn't need to declare the temp context for all.

It should be done from an optimization process mainly, for it I think that need of a own context for something like ShaderFunctionNode can be make a "SubNodeBuilder" for example for BlockCodes / Functions.

The priority of arguments in Nodes is done thinking in first the need to execution, non-optional arguments first. In this context, a signature like: new NodalFunctionNode( name, returnsType, inputs, blockCodeFN ]

can become for example: new NodalFunctionNode( blockCodeFN, returnsType = 'auto', inputs = [], name = 'auto' ]

[sunag]

I will think about how to merge it in the next days... Thanks for now, it is a great PR.

[sunag]

I also have doubts with this about the performance... As far as I know the GPU bakes/inlining all functions, current all ShaderNode is inline, maybe the inline approach would be closer to the final product? Which result in less GLSL or WGSL compilation time?

/ping @mrdoob

[fyoudine]

I also have doubts with this about the performance... As far as I know the GPU bakes/inlining all functions, current all ShaderNode is inline, maybe the inline approach would be closer to the final product? Which result in less GLSL or WGSL compilation time?

I was not aware of the compiler's optimisation process, in the other hand inlining function would make it harder to debug (as I like to see the computed shader, btw it would be nice to have access to the final glsl code ) but performances prior the readability.

It should be done from an optimization process mainly, for it I think that need of a own context for something like ShaderFunctionNode can be make a "SubNodeBuilder" for example for BlockCodes / Functions.

Actualy I was thinking about an automatic optimisation process. The main challenge to achieve this would be to count usage of each node within the computation tree. If we unlock that it would be easy to automatically determinate the usage of a temp var. Nowadays the only way I found to prevent usage of temporary variables is to use a Context but doing so it will never store intermediate result. This is the only reason of the temp helper.

[sunag]

@fyoudine I think that the ideal would be to move all NodalFunctionNode features to ShaderNode, thus we can to choose via some property of ShaderNode or NodeBuilder the way we want to build the code.

[fyoudine]

@sunag you mean that it would take the form of an inline option may be ? How would you treat in and out parameters ?

[fyoudine]

Also what about the variable optimisation process ?

[sunag]

I think that we can include the function call() in ShaderNode and add some extra method to create the function scheme like:

const checkerShaderNode = new ShaderNode( ( inputs ) => {

	const { uv } = inputs;
	const tmp = temp( floor, mul( uv, 2.0 ) );
	return temp_off( sign( mod( add( tmp.x, tmp.y ), 2.0 ) ) );

} );

checkerShaderNode.setName(  'checker' ); 
checkerShaderNode.setOutputType( NodeType.Float ); 
//checkerShaderNode.setInputsType( { uv : 'vec2' } ); 
checkerShaderNode.setInputsType( [ new NodeFunctionInput( 'vec2', 'uv' ) ] );

checkerShaderNode.inline = false;

// usage
checkerShaderNode.call( { uv: uvNode } ).build( builder );

[fyoudine]

I think that we can include the function call() in ShaderNode and add some extra method to create the function scheme like:

const checkerShaderNode = new ShaderNode( ( inputs ) => {

	const { uv } = inputs;
	const tmp = temp( floor, mul( uv, 2.0 ) );
	return temp_off( sign( mod( add( tmp.x, tmp.y ), 2.0 ) ) );

} );

checkerShaderNode.setName(  'checker' ); 
checkerShaderNode.setOutputType( NodeType.Float ); 
//checkerShaderNode.setInputsType( { uv : 'vec2' } ); 
checkerShaderNode.setInputsType( [ new NodeFunctionInput( 'vec2', 'uv' ) ] );

checkerShaderNode.inline = false;

// usage
checkerShaderNode.call( { uv: uvNode } ).build( builder );

Isn't it a bit confusing to have NodeFunctionInputs inside a ShaderNode ? Btw In think it would be easier to call parameters directly in the right order instead of passing on key:value pair object ?

const checkerShaderNode = new ShaderNode( (/* vec2 */  uv ) => {

	const tmp = temp( floor, mul( uv, 2.0 ) );
	return temp_off( sign( mod( add( tmp.x, tmp.y ), 2.0 ) ) );

} );

checkerShaderNode.setName(  'checker' ); 
checkerShaderNode.setOutputType( NodeType.Float ); 
//checkerShaderNode.setInputsType( { uv : 'vec2' } ); 
checkerShaderNode.setInputsType( [ new NodeFunctionInput( 'vec2', 'uv' ) ] );

checkerShaderNode.inline = false;

// usage
checkerShaderNode.call( uvNode ).build( builder );

[fyoudine]

@sunag More I look your new ShaderNode definition more I thinks it would be a base for every Nodes :

class NodeBase {
  void setName( name ){}
  void setType( type ){}
  set inlined( bool b ) {}
  get bool inlined(){ }
  
  void addInput( name, type, readonly, ...others  ){}
  void connectInput( name, newnode=null ){ oldNode.unregister(this); newnode.register(this) }
  
  void register( node ) { _children.add(node) }
  void unregister( node ) { _children.remove(node) }
  int usage() { return _children.length }
  
  NodeBase[] traverse(){ }
}

might be a headache for splitNode but I think it's feasible

[sunag]

Isn't it a bit confusing to have NodeFunctionInputs inside a ShaderNode ?

I think that no, maybe a better name for the function like:

checkerShaderNode.setInputs( [ new NodeFunctionInput( 'vec2', 'uv' ) ] );

Btw In think it would be easier to call parameters directly in the right order instead of passing on key:value pair object ?

Let's keep it like this and avoid changing too much.

@sunag More I look your new ShaderNode definition more I thinks it would be a base for every Nodes :

I think one of the pillars of the NodeMaterial is simplification. That's why I don't want to add anything more than necessary...

[fyoudine]

@sunag More I look your new ShaderNode definition more I thinks it would be a base for every Nodes :

I think one of the pillars of the NodeMaterial is simplification. That's why I don't want to add anything more than necessary...

Imho this could simplify the build process and lets have a node usage counting but anyway you're maybe right ; digging into NodeMaterial code is not realy trivial so it don't need to be more complex

[sunag]

Imho this could simplify the build process and lets have a node usage counting

I think we have a count here: https://github.com/mrdoob/three.js/blob/9db14dd2abd922874656c2f8d1e04c792fb0f594/examples/jsm/renderers/nodes/core/NodeBuilder.js#L23

[fyoudine]

https://github.com/mrdoob/three.js/blob/9db14dd2abd922874656c2f8d1e04c792fb0f594/examples/jsm/renderers/nodes/core/NodeBuilder.js#L23

to my understanding this is not a usage count and the place where it is executed (in the build loop) would tell 1 for the first usage then 2 for the second usage and so on ... so it couldn't be use to set temp variable usage (once again if my understanding is good) To my opinion there's two place to create this and each have a cost : The first one would be the one discussed just above it needs to replace each class members Math.aNode = node, Math.bNode = node, etc) by a call to a base class method ( this.setInput( 'a', node ), etc ) Or a new control loop that count usage in each Nodes but needs to be done manually in each NodeClass and is subject to the dev goodwill

// resampled before each build 
Math.countUsage(builder) { 
  builder.increments( this.aNode ); 
  builder.increments( this.bNode ); 
  builder.increments( this.cNode ); 
 }

[sunag]

I made it optimization process in old NodeMaterial system but using the NodeBuilder, it uses Node.analyze() approach.

https://github.com/mrdoob/three.js/blob/9db14dd2abd922874656c2f8d1e04c792fb0f594/examples/jsm/nodes/core/Node.js#L17

[fyoudine]

was there a reason for removing it ?

[sunag]

was there a reason for removing it ?

I want to do something a little better this time.

[fyoudine]

@sunag I kept thinking about the way code is built and how you could optimise it and I remember about visitor pattern that would be a good benefit for descending the node structure with different executor https://en.wikipedia.org/wiki/Visitor_pattern

What do you think ?

[sunag]

What do you think ?

@fyoudine I would refer to use methods closer to classic compilers like analyze() I think this is more sense to prepare nodes for generate() code.

The improvement I wanted to make is related to not generating the code(string) twice as it happened, now analyze() method can use getNodesKeys() ( https://github.com/mrdoob/three.js/pull/23314 ) to evaluate how many times each node was used for example.