mrdoob / three.js

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

PointLight + Phong Material #345

Closed fazeaction closed 13 years ago

fazeaction commented 13 years ago

I think pointlights and phong material it's not working properly because specular spot is not following the light source. Compare this two examples based on materials three.js sample:

Wrong: http://www.neveroccurs.com/lab/three.js/phong_error/webgl_materials.html

Right: http://www.neveroccurs.com/lab/three.js/phong_correct/webgl_materials.html

I have modified webglshader.js line 339:

vec3 pointHalfVector = normalize( vPointLight[ i ].xyz + vViewPosition );

for:

vec3 pointHalfVector = normalize( vPointLight[ i ].xyz + pointVector );

and seems that it's working now.

Please, can you check this? Thanks!!.

alteredq commented 13 years ago

Hmmm, yes, something is wrong with specular component of Phong material (and normal map material, they are very similar). Thanks for noticing.

Though your solution shouldn't work either - if you drop vViewPosition then there is no info about camera in the shader and specular lighting needs to take into account viewer position. I'm puzzled ...

I think proper way may be using normalized vViewPosition:

vec3 pointHalfVector = normalize( vPointLight[ i ].xyz + viewPosition );

This also produces specular spot which follows the light.

It looks like intended thing that got lost in some refactoring - viewPosition is created but never used.

I wrote the original shader so long ago I don't remember anything, please somebody correct me if I did some nonsense ;).

fazeaction commented 13 years ago

Makes sense to me. You are right, view position, clearly, is needed.

With my code, I have solved if a pointlight is moving with a fixed camera but not vice versa. In any case, i'm totally wrong, adding two pointlight position vectors to calculate pointHalfVector makes no sense at all.

As you say the right way is:

vec3 pointHalfVector = normalize( vPointLight[ i ].xyz + viewPosition );

Thanks alteredq!.

mrdoob commented 13 years ago

Indeed. I noticed webgl_materials_shaders.html was weird. Good to see it working nicely :')