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, should all the shaders from /examples be converted to the new format? #14252

Closed
pailhead opened this issue Jun 8, 2018 · 12 comments
Labels

Comments

@pailhead
Copy link
Contributor

pailhead commented Jun 8, 2018

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
Copy link
Collaborator

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 pailhead changed the title NodeMaterial, should all the shaders from /examples be converted to thew new format? NodeMaterial, should all the shaders from /examples be converted to the new format? Jun 8, 2018
@pailhead
Copy link
Contributor Author

pailhead commented Jun 8, 2018

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
Copy link
Collaborator

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
Copy link
Contributor Author

pailhead commented Jun 8, 2018

@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 #14250. In order to give a better comparison with what nodes to with textures and mapping, the templates could be refactored.

@pailhead
Copy link
Contributor Author

pailhead commented Jun 8, 2018

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
Copy link
Collaborator

donmccurdy commented Jun 8, 2018

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
Copy link
Contributor Author

pailhead commented Jun 8, 2018

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
Copy link
Collaborator

donmccurdy commented Jun 12, 2018

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Owner

mrdoob commented Jun 18, 2018

Agreed with everything @donmccurdy said.

@mrdoob mrdoob closed this as completed Jun 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants