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

Suggestion AnyMaterial.toShaderMaterial() #15537

Open
pailhead opened this Issue Jan 6, 2019 · 4 comments

Comments

Projects
None yet
4 participants
@pailhead
Copy link
Contributor

pailhead commented Jan 6, 2019

I've been experimenting with removing all the "built in" materials from THREE.WebGLRenderer, i think that there are many benefits to this and ill try to list some here:

  1. Having all the chunks needed to build all the materials that WebGLRenderer is aware of increases the size of the library.
  2. It may be less likely for different surface materials to be mixed in the same app. For example, does it make sense to have Phong and PBR materials side by side. Lambert could be a fallback solution perhaps since it's the only one that does vertex lighting, but still, it could be lazy loaded on different platforms etc.
  3. There are some cumbersome patterns ( like onBeforeCompile ) that are constrained by the current system.
  4. There is a lot of code with a lot of branching dealing with specific materials in WebGLRenderer. Since it's already a large class, maybe refactoring some of this would help.
  5. Material super class suffers from similar problems as the renderer. If you want to modify any property, rename it etc, it's hardcoded in many places. (toJSON on Material knows about all the possible properties that people added to various materials over time)

I made a rough experiment that can be seen here, it seems to be working fine with all of the properties i tested, but i haven't looked into skinning and morphing.

The jist of it is:

Imagine if we could call new THREE.MeshBasicMaterial().toShaderMaterial()

It seems to me that this sort of happens inside the WebGLRenderer but it's not consolidated and abstracted. If you look at this code:

const config = {...}
const shaderMat = new THREE.ShaderMaterial(config)

const otherMat = new MeshStandardMaterial()
otherMat.onBeforeCompile = shader => {...}

shader == config

onBeforeCompile seems to indicate that at this point, where you access through the callback, you basically have a ShaderMaterial.

I'm curious about any thoughts on this. I want to explore this more and am hoping to get someone else interested as well. I'd also like to understand more about the historic design of three, what possible pitfalls could be encountered when removing lamberts, phongs and such from the renderer?

@pailhead

This comment has been minimized.

Copy link
Contributor

pailhead commented Jan 6, 2019

I think it would also help keep the renderer agnostic of how you decide to manage GLSL.

chunkBasedMaterial.toShaderMaterial() //samples chunks
nodeBasedMaterial.toShaderMaterial() //compiles nodes
@mrdoob

This comment has been minimized.

Copy link
Owner

mrdoob commented Jan 7, 2019

This is something I had on my list of things to investigate. It'd definitely be good to move that code out of the renderer. I would first try to identify what's the code that can be moved out of the renderer.

@donmccurdy

This comment has been minimized.

Copy link
Collaborator

donmccurdy commented Jan 7, 2019

Rather than having an actual toShaderMaterial() method, perhaps consider ShaderMaterial an interface that other materials can implement by exposing the right properties and methods.

But yes, +1 on investigating how much can be moved out of the renderer.

@pailhead

This comment has been minimized.

Copy link
Contributor

pailhead commented Jan 7, 2019

Well, you might be interested in taking a look at this then https://github.com/pailhead/three-refactor-chunk-material.

Some of the materials listed there are hooked up already (i think) as you describe @donmccurdy . I think i verified that only the ShaderMaterial branches are being hit, but i havent removed any code.

@Mugen87 Mugen87 added the Suggestion label Jan 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment