Why refreshUniforms isn't overrided per material type?

Questions
1

Hello,
please, I looked through code and in WebGLRenderer around line #1880 are sequence of conditions for material type, like:
…
} else if ( material.isMeshLambertMaterial ) {
refreshUniformsCommon( m_uniforms, material );
refreshUniformsLambert( m_uniforms, material );
} else if ( material.isMeshPhongMaterial ) {
…

And I’m curious what is the reason to not write it as
…
material.refreshUniforms( m_uniforms );
…
and have such method overrided in each material type, and call base type (refreshUniformsCommon) inside.

I think that one overrided method is faster than sequence of conditions, and be easier to add new material type (I especially think about GLTFMaterialsPbrSpecularGlossinessExtension in Gltf loader)

I understand that “real” classes are only since ES6, but what am I overlooked?

PS: I don’t want discuss here what prototype vs class based means.

1 Like
2

I think the design decision was to setup m_uniforms exclusively in WebGLRenderer and not delegate this task to other entities. If you look at the material source files, you will notice that their structure is very simple. They only set properties in their constructor and provide a copy() method, that’s all. I don’t think it’s good if we start to move WebGLRenderer related code to these files.

3

You could check this out. I’ve recreated all the materials using ShaderMaterial.

You should be able to remove all references to materials in the renderer.

4

Thanks for responses and correction of question’s text.

@Mugen87: I understand this point of view, but search for clean way how to add new materials (custom extensions in gltf) and current GLTFMaterialsPbrSpecularGlossinessExtension didn’t looks clean to me. And overridable per-material-type method looks as easier solution.
To explain - I thought this way: Is renderer the one who should know about all ever existing uniforms, or is material the one who should care about its own uniforms?

@pailhead: This looks good, maybe this is solution. I looked over before, but didn’t try it yet, because search for something official. Is there some known limitations? No problems with morph and skinning?

5

I’d advise extreme caution when advocating what you’re advocating. I have been banned from threejs and many of my messages here have been deleted because I was arguing the same argument as you.

Unfortunately I haven’t tested he chunk material with morphing and skinning but I aim to. If ShaderMaterials are able to work with both then I see no problems.

Maybe this would be helpful too. I think they’re really conflating the renderer with things it should’t relate to. This weird usage of ‘onbeforerender’ is one such example.