-
-
Notifications
You must be signed in to change notification settings - Fork 35.3k
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
Support for MTL map_Ns (shininess map) #9339
Comments
We do not support shininess material on |
Perhaps that is a work-around for you. |
I thought shininess is here: https://github.com/mrdoob/three.js/blob/master/src/materials/MeshPhongMaterial.js#L59 So I think it is supported. |
Ah, I see the issue, "material.specularStrength" is applied to the whole of the Phong Specular BRDF instead of just applied to the shininess parameter. I'd suggest that we replace specularStrengthn with shinininessMap that modulates the shininess instead. I think that is actually much more useful and more physically correct. |
@bhouston Our implementation of But if you want to change the model, I would not oppose. The thing to do would be to add |
👍 |
@WestLangley wrote:
The ThreeJS implementation of BlinnPhong is not physically correct, that is true. Other renders do have much more physically correct implementations -- V-Ray and PRMan have really good implementations of BlinnPhong that are essentially as physically correct as MeshSTandardMAterial/MeshPhysicalMaterial -- they differ only in terms of parameterization. The reason why ThreeJS's MeshPhongMaterial are so bad is because of three issues with our implementation: (1) We are not energy conserving. Energy reflected by specular layer should not be made available to the diffuse layer. If specular is 1, 1, 1 (which means the surface is metallic and fully reflective) then the light getting to the diffuse layer should be 0,0,0. (This energy conservation is part of MeshStandardMaterial in the way that increasing metalness reduces the intensity of the diffuse layer.) (2) ShininessMap should modulate shininess. (3) SpecularMap should be a color map that modulates specularColor. This would be useful to have right because then ThreeJS can load MTL, FBX and other BlinnPhong model using files with as full fidelity as other professional rendering packages. |
Just as a point of reference in defense of properly implemented Blinn-Phong, V-Ray uses Blinn-Phong almost exclusively and its stuff looks amazing: https://www.google.ca/search?q=v-ray+architecture&source=lnms&tbm=isch |
@bhouston I agree, your suggestions make perfect sense. I wonder why it wasn't implemented that way in the first place... |
There was a previous attempt to implement energy conservation in BlinnPhong -- I think it was relatively correct as it followed roughly how professional renderers do it. It is important to note that one doesn't have to use a Fresnel term in the energy conservation as it isn't done this way in Standard/Physical either when blending towards metallic just linearly decreases available light at the diffuse layer. |
@mrdoob I didn't think so. I would like it very much if you did consider it. |
This is turning out to be an enormous change. Here is some code for reference. float specularStrength;
#ifdef USE_SPECULARMAP
vec4 texelSpecular = texture2D( specularMap, vUv );
specularStrength = texelSpecular.r;
#else
specularStrength = 1.0;
#endif
#ifdef ENVMAP_BLENDING_MULTIPLY
outgoingLight = mix( outgoingLight, outgoingLight * envColor.xyz, specularStrength * reflectivity );
#elif defined( ENVMAP_BLENDING_MIX )
outgoingLight = mix( outgoingLight, envColor.xyz, specularStrength * reflectivity );
#elif defined( ENVMAP_BLENDING_ADD )
outgoingLight += envColor.xyz * specularStrength * reflectivity;
#endif Objective
Consequences
Previously, we have wondered why we are supporting |
should not be constant, as with other parameters there should be a float available to override the texture. If one has the ability to use a texture containing values in the 0-1 range, why is the alternative locked to 1? Why not 0, or .5346? |
This feature is under consideration for the glTF 2.0 Blinn-Phong extension (see discussion), so I will be interested in what is decided here as well. /cc @takahirox |
IMO, there is still need for more performance-tuned materials on mobile devices e.g. Cardboard/Daydream/GearVR.
I don't think I follow... Is there something about this proposal that makes |
@donmccurdy The implementation of "BlinnPhong" in Three.JS is fairly wrong compared to the correct implementation in V-Ray and other top quality renderers. I describe the real issues here and how to address them: My recommendations are not that hard to implement and but it is a breaking change for a bunch of people who are already suing the incorrect BlinnPhong. |
I don't expect
What we are currently calling I mentioned it because these are breaking changes. |
Properly implemented BlinnPhong should be computationally equivalent to properly implemented Physical materials within some small margin of error. |
Thanks @WestLangley @bhouston! In that case I don't have an opinion about maintaining MeshPhongMaterial alongside MeshStandardMaterial. Unlit shading is probably what I need to be looking at; I'd like to get that definition updated for glTF2.0. I will retire from this thread. 😬 |
For the record, I implemented The primary reason is Regarding an RGB My recommendation at this point is to leave |
I still would be in favor of renaming the current grayscale Note, we use the property |
I would suggest studying V-Ray, Blender's Cycles and Unity to figure out what they do and then take the average solution in terms of how things are modulated and what things are called. This would achieve the most compatibility possible as these are the tools that I think have the most widely accepted Blinn-Phong shading models. |
Closing in favor of #7290 which tracks the mentioned changes to |
Currently/To the best of my knowledge the only shininess setting for phong material is shininess (mtl Ns) but mtl supports a shininess map (map_Ns). I'm sure it's a pretty big deal but I'd be eternally grateful if this feature was added.
Three.js version
Browser
OS
The text was updated successfully, but these errors were encountered: