mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
102.18k stars 35.34k forks source link

MeshPhongMaterial with shininess = 0 causes artifacts on Windows #6057

Closed doggan closed 9 years ago

doggan commented 9 years ago

Steps to reproduce the problem:

  1. Open up the MeshPhongMaterial reference document: http://threejs.org/docs/#Reference/Materials/MeshPhongMaterial
  2. Change the THREE.MeshPhongMaterial shininess value to 0.

What is the expected behavior? On Mac (OS X 10.10.2, Chrome 40.0.2214.111 (64-bit), Intel HD Graphics 5000), the object is rendered correctly: macss

What went wrong? On Windows ((Boot Camp) Windows 7, Chrome 40.0.2214.111 m, Intel HD Graphics 5000), the object is rendered with black artifacts: winss

I originally discovered the issue in my local project (verified with THREE.js v68 and v70) using MeshPhongMaterial. But the problem is easiest to see in the three.js MeshPhongMaterial documentation example.

Thanks.

crobi commented 9 years ago

Zero is not a valid value for shininess. The phong equation contains the shininess in an exponent, and if you set it to zero the shader will attempt to compute pow(0,0), which is undefined.

doggan commented 9 years ago

In OpenGL, the GL_SHININESS material parameter has a valid range of [0, 128]. Is it different for WebGL?

IMO, regardless of the internal implementation, a shininess of 0 seems like a reasonable value to specify for a Phong material that shouldn't be shiny.

If the internal implementation requires a specific range, it could be handled simply by clamping the value during material creation.

crobi commented 9 years ago

a shininess of 0 seems like a reasonable value to specify for a Phong material that shouldn't be shiny

That's not how the Phong reflectance model works. Decreasing the shininess value only increases the size of the specular highlight and does not change the brightness of the highlight. Asking for "zero" shininess would result in an "infinitely large" highlight. If you want a material that is not shiny at all, set the specular color to black.

Note: three.js uses Phong reflectance with an additional Fresnel term, so the example doesn't necessarily behave exactly like glMaterial in OpenGL 1.0.

Nevertheless, the example shouldn't result in artifacts. Either the user interface or THREE.MeshPhongMaterial should check that the shininess value is larger than zero.

mrdoob commented 9 years ago

/ping @TatumCreative

sayangel commented 9 years ago

Just noticed this issue too. Was having issues with OBJ+MTL I'm loading, but only on Windows. Turns out its the shininess property of the phong material. Any updates on this?

gregtatum commented 9 years ago

The docs issue with shininess was fixed in #6101. We could add a small check to set shininess to something like 0.001 if it is set to 0 in the constructor, or present some kind of warning to the user. I wouldn't think we'd want to check it in the loop, and doing a setter function seems like overkill to me.

Thoughts?

jee7 commented 9 years ago

I have the same problem with OBJ+MTL loader and shininess = 0. The MTL file format seems to specify, that shininess (Ns) can be in the range [0, 1000] http://www.andrewnoske.com/wiki/OBJ_file_format http://en.wikipedia.org/wiki/Wavefront_.obj_file#Basic_materials

Some sources say, that shininess = 0 is no shininess (no matter the mathematical pow(0, 0)). If I export from Blender with default material, then it will have shininess = 0, and the imported result in Three.js is wrong. It seems, that the MTLLoader only knows, how to create MeshPhongMaterial. It even sets the shininess "0" (string), not 0 (number). So, maybe this is more of a problem with the MTLLoader, and not with MeshPhongMaterial. Although, as I mentioned, the documentation (that I could find with a quick search) of MTL says that shininess = 0 is acceptable.

Currently, if I clone the geometry and the material, then set the shininess to something bigger than 0, and the specular color to black, I get the desired result. Maybe MeshPhongMaterial should do this sanity checking itself, and send the color black and not a 0 to the shader, if the shininess = 0. Or maybe the MTLLoader needs to create a MeshLambertMaterial. Not sure, what would be the best solution, but currently, with the default settings, exporting from Blender to OBJ+MTL and importing in Three.js produces a wrong result.

screen

Demo: http://tume-maailm.pri.ee/ylikool/ComputerGraphics-2015/other/modeling2/

Using Chrome 41.0.2272.101 m, Intel HD Graphics 4000, Windows 8.

gregtatum commented 9 years ago

In WebGLRenderer.js you could do something like:

function refreshUniformsPhong ( uniforms, material ) {

    var hasSpecular = material.shininess > 0

    uniforms.shininess.value = hasSpecular ? material.shininess : 1;

    uniforms.emissive.value = material.emissive;
    uniforms.specular.value = hasSpecular ? material.specular : 0;

    uniforms.lightMap.value = material.lightMap;
    uniforms.lightMapIntensity.value = material.lightMapIntensity;

    uniforms.aoMap.value = material.aoMap;
    uniforms.aoMapIntensity.value = material.aoMapIntensity;

}
sayangel commented 9 years ago

I put a check directly into the MTLLoader, but you maybe adding a check in the constructor would make more sense.

Any thoughts on why this only happens in Windows?

jee7 commented 9 years ago

It is probably just the difference in drivers. GLSL documentation says that pow(x, y) is undefined, if x=0 and y=0. Depending on the driver, this "undefined" might give you different results, for example 0.

mrdoob commented 9 years ago

Maybe we can do a Math.max( shininess, EPS )?

WestLangley commented 9 years ago

Fixed in #7252