mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
102.68k stars 35.37k forks source link

NodeMaterial, should all the shaders from /examples be converted to the new format? #14252

Closed pailhead closed 6 years ago

pailhead commented 6 years ago

Extracting various topics from #7522.

Today we have a lot of examples in /examples. A great example of something that could be part of three.js core from a user's perspective, but not are fat lines.

A user would like to be able to:

set thickness to lines

And often they expect a very simple interface:

lineMaterial.thickness = 5 //px

This is not possible today out of the box, but it is possible by using the atomic building blocks that three.js exposes - ShaderMaterial, InstancedBufferGeometry, InterleavedBuffer etc.

If we consider something like this: https://github.com/mrdoob/three.js/blob/dev/examples/js/lines/LineMaterial.js

There are several ways for me to take this shader and modify it to suit the needs of my app. For example, i can inject things into the template, i could look at the string in onBeforeCompile etc etc.

How are these random scattered shaders/materials going to be treated once NodeMaterial becomes the default system?

For example, if one looks at this comment, one could deduce that just having an API may not be enough for users who don't code GLSL, and the expectation would be that various example materials/effects would all be converted.

donmccurdy commented 6 years ago

If NodeMaterial moves into src/, we should implement backward-compatible wrappers for MeshFooMaterial classes on top of it. I don't see any particular need for existing examples to be converted to use node syntax, unless doing so simplifies the code for whatever reason.

pailhead commented 6 years ago

Do you have an idea what would happen to https://github.com/mrdoob/three.js/blob/dev/examples/js/lines/LineMaterial.js specifically? I didn't understand:

unless doing so simplifies the code for whatever reason.

If NodeMaterial is supposed to make materials extensible, something thats not written in it wont be?

donmccurdy commented 6 years ago

For example:

var material = new THREE.MeshStandardMaterial();
material.map = textureLoader.load('foo.png');

var node = material.node;
node.color.coord.index = 1;

console.log( material.node.color ); // THREE.TextureNode

But this really comes down to implementation details, a PR would be necessary to answer these questions in any detail.

pailhead commented 6 years ago

@donmccurdy

that's a great example but i think it's for a different topic. If you don't mind i'd open another issue and move this there?

In the block above, how can I reason about material.node where does it come from? I have to know that .color somehow took a TextureNode not a THREE.Color or THREE.Texture? Or do i have to know that each .prop has some corresponding .node.prop?

Why not:

material.map = textureLoader.load('foo.png'); // returns Texture not Node
material.map.channel  = 1

Also related is https://github.com/mrdoob/three.js/issues/14250. In order to give a better comparison with what nodes to with textures and mapping, the templates could be refactored.

pailhead commented 6 years ago

For this topic, it sounds like: https://github.com/mrdoob/three.js/blob/dev/examples/js/lines/LineMaterial.js

Wouldn't actually be rewriitten, since it's already a ShaderMaterial and one can modify the provided code already.

What's confusing is how does this apply to the non-artist people. I would assume that for some reason this shader is different, could be labeled more utility than effect/beauty. It's job is to make the lines work with complicated low level structs, like instancing. As such, there is no expectation to expose it to someone who doesn't know GLSL. /

I would like to establish some kind of a nomenclature or classification to better understand why some materials are eligible for these out of the box modifications and others are not.

One rule could be "if its in /examples, not our concern". Another could be "if it does FOO, not a concern".

donmccurdy commented 6 years ago

I don't think we're going to be able to answer these questions until we begin implementing something... to your original question of "should all shaders from /examples be converted..." I think the answer is no.

pailhead commented 6 years ago

I think the biggest pain point in this whole story is what move to /src entails:

until we begin implementing something...

i can't relate this to: https://github.com/mrdoob/three.js/tree/dev/examples/js/nodes

Isn't it already implemented? What does implementation refer to in this context?

donmccurdy commented 6 years ago

I believe I misread, thinking you were referring to the backward-compatible MeshStandardMaterial replacement that would transparently create nodes from the current syntax (but which doesn't exist yet).

For LineMaterial, I think it could very reasonably be left as-is. It's a reasonable example of using ShaderMaterial, and NodeMaterial is not meant to replace ShaderMaterial.

pailhead commented 6 years ago

I think i'm overall confused with what NodeMaterial is supposed to do, from all that i've read, i thought that ShaderMaterial will go away, but then i realized it's just another way of assembling a ShaderMaterial. If we ever decide to have fat lines in the core, that too seems like it should be assembled from nodes?

pailhead commented 6 years ago

Each one of these will extend NodeMaterial?

https://github.com/mrdoob/three.js/tree/dev/src/materials

stuff in /examples will be left as is?

donmccurdy commented 6 years ago

I think we're pretty far down a chain of assumptions on this discussion. I've written my own suggested steps on this (buried in #7522 somewhere now) which would involve (1) putting NodeMaterial in src, and (2) eventually rewriting core MeshFooMaterial materials to use nodes internally. But mrdoob has not weighed in on whether to merge NodeMaterial at all, and if so he might certainly prefer to keep the existing materials around as-is, or wait a while before changing them.

So I think there are too many unanswered questions to really get into this. But (IMO) nothing needs to be converted... if examples can be improved with nodes, we'll improve them. If not, no worries.

mrdoob commented 6 years ago

Agreed with everything @donmccurdy said.