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

Material: Deprecate material.morphTargets=true and material.morphNormals=true? #22166

Closed
donmccurdy opened this issue Jul 21, 2021 · 2 comments · Fixed by #22169
Closed

Material: Deprecate material.morphTargets=true and material.morphNormals=true? #22166

donmccurdy opened this issue Jul 21, 2021 · 2 comments · Fixed by #22169

Comments

@donmccurdy
Copy link
Collaborator

donmccurdy commented Jul 21, 2021

Similar to #22146 and #21788, both properties could be inferred from the parent Mesh, without requiring separate boolean flags on the material. Along with the recent removal of .skinning, this should help avoid situations where the user replaces a material provided by a loader and doesn't understand why the model stops animating.

Earlier threads proposed removing other properties that could be inferred (e.g. flatShading, vertexColors), but there has been some push-back against removing those and I don't feel strongly either way.

@WestLangley
Copy link
Collaborator

Hmm... so a material can no longer be compiled without a geometry. We started down that path with vertexAlphas... 🤔

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 22, 2021

Actually vertexAlphas was not the first step in that direction. You need information from many entities (renderer, scene, 3D object, material and geometry) in order to compile a shader program. In WebGPURenderer we maintain a node builder per render item right from the beginning in order to determine the correct shader program.

Removing morphTargets and morphNormals is only a logical consequence after #21788.

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

Successfully merging a pull request may close this issue.

3 participants