three.js
three.js copied to clipboard
Introduce MeshGouraudMaterial
Related issue: #24452.
This PR adds MeshGouraudMaterial.js to the the newly-created /examples/jsm/materials/ directory.
MeshGouraudMaterial implements a Lambert illumination model with Gouraud (per-vertex) shading.
MeshGouraudMaterial has the same feature set, and uses the same shaders, as the legacy version of MeshLambertMaterial.
Current limitations:
-
Equirectangular environment maps do not work for this custom material. There is apparently a conflict when the renderer auto-converts the equirectangular map to a cube map.
WebGL: INVALID_OPERATION: bindTexture: textures can not be used with multiple targets
AFAIK, there are no other limitations.
@Mugen87 I expect the getter/setter for envMap is related to the problem, because built-in materials do not use such a pattern. (Actually, maybe built-in materials should use it, as it makes the renderer more material-agnostic. I think the use of the pattern in built-in materials would eliminate the refreshUniformsFoo pattern in WebGLRenderer. But that's a separate issue.)
I expect the getter/setter for envMap is related to the problem, because built-in materials do not use such a pattern.
Um, adding the getter should actually resolve the issue since the renderer can now access the common envMap material property. The shader materials in WebGLBackground do it like that.
The shader materials in WebGLBackground do it like that.
Oh, see #15187. FWIW, I think that pattern is no longer needed, because the code injection (getTexelDecodingFunction()) was removed.
Good news! However, if you want automatic cube map conversion for MeshGouraudMaterial, you still need the envMap property on material level.
Good news! However, if you want automatic cube map conversion for MeshGouraudMaterial, you still need the envMap property on material level.
Right. It's main purpose was to enable automatic uniform updates, because the renderer does not do that for custom materials.
//
BTW, this was really hard to get working... I'd hate to ask users to have to do this. That is why I created an actual material -- not just a shader.
In any event, for whatever reason, automatic equirectangular conversion for (at least) this non-built-in material is not working, and I have no idea why...
I'll download your branch and debug offline. Maybe I find something.
The problem is that the shader of MeshGouraudMaterial expects a cube map, however, the renderer still uploads the equirectangular texture extracted from material.envMap. This does not happen for built-in materials because of the following section in refreshUniformsCommon().
https://github.com/mrdoob/three.js/blob/9e5512f067e3a0de8d4069217d20903c1725edf7/src/renderers/webgl/WebGLMaterials.js#L172-L184
As you can see the code correctly configures the uniforms with the environment map from WebGLCubeMaps before the uniform upload happens. But that works only for built-in materials.
So adding the envMap property is unfortunately not enough for custom shaders. Something like #18399 would potentially solve this issue. Or we just force ShaderMaterial to go through refreshUniformsCommon() when e.g. a new flag is enabled or something.
Thanks for your sleuthing, @Mugen87. :-)
As you can see the code correctly configures the uniforms with the environment map from WebGLCubeMaps before the uniform upload happens. But that works only for built-in materials.
So it's a sequencing thing...
In order to get things going without breaking the equirect support, I suggest to evaluated isMeshGouraudMaterial in the renderer for now which triggers the environment map update. The code could be placed below this line and look like so:
if ( material.isMeshGouraudMaterial ) {
m_uniforms.envMap.value = envMap;
m_uniforms.flipEnvMap.value = ( envMap.isCubeTexture && envMap.isRenderTargetTexture === false ) ? - 1 : 1;
}
We can refactor this later.
@Mugen87 Ugh... You have to also test for null in your code snippet, but even doing that, it is rendering reflected on the first render for me, and then corrects subsequently. (tip: don't test with an animation loop). So there is still a sequencing problem -- apparently...
I agree, it would be nice if the app does not have to set flipEnvMap. Currently I have to expose that property in this PR.
Sorry, the code block is executed too late. Can you please move it below this if block:
https://github.com/mrdoob/three.js/blob/d80e640a2990277a942b9ea43f1ea4acdb04daec/src/renderers/WebGLRenderer.js#L1730-L1735
PR looks good to me now!
What do you think about using the new material in at least one example? Maybe webgl_materials_envmaps? It would be also okay to create a new example like webgl_materials_gouraud. Separate PR if you prefer.
OK, in a separate PR... Let me recover from this one first. :-)
Thanks!
This PR requires a new build.
Done!