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

NodeMaterial revision #16796

Merged
merged 19 commits into from
Jun 20, 2019
Merged

NodeMaterial revision #16796

merged 19 commits into from
Jun 20, 2019

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Jun 16, 2019

This includes fixes and example of @DanielSturk, irradiance feature and better interface for Color Space using nodes.

Related:
#16788
#16668
#16375

Changes

  • unify Math1Node, Math2Node, Math3Node to MathNode
  • new ColorSpaceNode
  • new example webgl_materials_envmaps_hdr_nodes.html
  • rename Node.parse to Node.analyze
  • rename Node.buildCode to Node.flow
  • rename Node.eval to Node.parse
  • move NodeBuilder.optimize = false to NodeBuilder.context.ignoreCache = true
  • fix irradiance LDR, HDR and RGBM16
  • fix RectAreaLight

https://rawgit.com/sunag/three.js/dev-fix-TextureCubeNode/examples/index.html?q=nodes#webgl_materials_envmaps_hdr_nodes

image

@mrdoob mrdoob added this to the r106 milestone Jun 17, 2019
- rename Node.parse() to Node.analyze()
- rename Node.buildCode() to Node.flow()
- rename Node.eval to Node.parse
- move NodeBuilder.optimize to NodeBuilder.context.ignoreCache
@sunag
Copy link
Collaborator Author

sunag commented Jun 17, 2019

How should we call the vector2?
This is the indirect(irradiance) cube coords vector.
/ping @WestLangley @bhouston @mrdoob

Select vector2 in scope option:
https://rawgit.com/sunag/three.js/dev-fix-TextureCubeNode/examples/webgl_materials_nodes.html?e=node-reflect

@sunag
Copy link
Collaborator Author

sunag commented Jun 17, 2019

Here the original code:

vec3 queryVec = vec3( flipEnvMap * worldNormal.x, worldNormal.yz );

@sunag
Copy link
Collaborator Author

sunag commented Jun 17, 2019

Opts it is a world normal with flipEnvMap!?

new THREE.NormalNode( THREE.NormalNode.WORLD )

@DanielSturk
Copy link
Contributor

Great progress! Looking forward to seeing these changes made

@sunag
Copy link
Collaborator Author

sunag commented Jun 17, 2019

Thanks @DanielSturk!

Done the fixes and update the example. The visual result should be the same for both nowMeshStandardNodeMaterial and MeshStandardMaterial.

https://rawgit.com/sunag/three.js/dev-fix-TextureCubeNode/examples/webgl_materials_envmaps_hdr_nodes.html

@sunag
Copy link
Collaborator Author

sunag commented Jun 18, 2019

@donmccurdy following your tip.
4665931

@@ -18,6 +18,22 @@ function NodeMaterial( vertex, fragment ) {

this.updaters = [];

// onBeforeCompile can't be in the prototype because onBeforeCompile.toString varies per material
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where does onBeforeCompile.toString get called?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is used to create a hash modification in onBeforeCompile. More details:
#14442
#16668

Copy link
Collaborator Author

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.

I don't see anything calling toString() in these PRs?

This comment was marked as off-topic.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In 15674 have a commit calling toString
07236bb
The description of the problem:
#15674 (comment)

I think that this approach will be improved in the future...

array.push( material.onBeforeCompile.toString() );

@donmccurdy
Copy link
Collaborator

@sunag thank you! :)


In words, how would you define 'analyze', 'parse', and 'flow' as they relate to NodeMaterial? I'm having some trouble understanding what "analyze" means — the word usually doesn't imply side effects, but in this case analyze() calls methods like build() and addFlow().

@sunag
Copy link
Collaborator Author

sunag commented Jun 18, 2019

Primary process

  1. analyze() It is necessary to analyze and validate all nodes and slots(channels), it is will generate info necessary for build the code, share nodes(create temp vars), optimization as well the variable type and the return value.

  2. flow() It generates codes, includes functions, defines, uniforms, expressions, and other codes need to work the shader. It depends on analyze be execute before flow to generate info necessary to build the code/shader. flow returns all data (code and extras values) of the slot/channel.

  3. analyzeAndFlow() Call the two methods analyze and after the flow. It does not optimize the shader if used many slots/channels, for the reason, it is mostly used in position slot of the vertex shader.

Secundary process

  1. build returns the final value(expression), generate by generate function or the variable generated by NodeTemp just like any other optimization node.

  2. generate create the snippets fundamental to generate the shader. Here too, is responsible to include functions, defines, etc returned in flow. generate return only the final expression converted to output or parent node using format function.

The process superficially is similar to this diagram:

Compiler

@sunag
Copy link
Collaborator Author

sunag commented Jun 18, 2019

parse is used only to parse GLSL code, example is FunctionNode and StructNode

@donmccurdy
Copy link
Collaborator

Thanks! I'm confused, but I appreciate the detailed writeup. Probably I need to read the code more and see what calls what. 😅

@mrdoob
Copy link
Owner

mrdoob commented Jun 20, 2019

@donmccurdy should I merge this one for now or do you want to take a closer look?

Copy link
Collaborator

@donmccurdy donmccurdy left a comment

Choose a reason for hiding this comment

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

Good to merge. 👍

Repository owner deleted a comment from mvc1006 Jun 20, 2019
@mrdoob mrdoob merged commit 95c6dcc into mrdoob:dev Jun 20, 2019
@mrdoob
Copy link
Owner

mrdoob commented Jun 20, 2019

Thanks!

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