Fragment Diffuse value changing with camera location/rotation

Question

I am attempting to get some simple diffuse lighting to work in GLSL. I have a cube that is being passed in as an array of points and I'm calculating the face normals inside my geometry shader (because I intend to deform the mesh at run-time so I'll need the new face normals.)

My problem is that the diffuse value is changing as I move the camera around the world. so the shading on a face of my cube changes as the camera moves. I have not been able to figure out what I am missing that is causing this. My shaders are as follows:

Vertex:

#version 330 core

layout(location = 0) in vec3 vertexPosition_modelspace;

uniform mat4 MVP;

void main(){     
    gl_Position =  MVP * vec4(vertexPosition_modelspace,1);
}

Geometry:

#version 330

precision highp float;

layout (triangles) in;
layout (triangle_strip) out;
layout (max_vertices = 3) out;

out vec3 normal;
uniform mat4 MVP;
uniform mat4 MV;

void main(void)
{
    for (int i = 0; i < gl_in.length(); i++) {
        gl_Position = gl_in[i].gl_Position;


        vec3 U = gl_in[1].gl_Position.xyz - gl_in[0].gl_Position.xyz;
        vec3 V = gl_in[2].gl_Position.xyz - gl_in[0].gl_Position.xyz;

        normal.x = (U.y * V.z) - (U.z * V.y);
        normal.y = (U.z * V.x) - (U.x * V.z);
        normal.z = (U.x * V.y) - (U.y * V.x);

        normal = normalize(transpose(inverse(MV)) * vec4(normal,1)).xyz;

        EmitVertex();
    }
    EndPrimitive();
}

Fragment:

#version 330 core

in vec3 normal;
out vec4 out_color;
const vec3 lightDir = vec3(-1,-1,1);

uniform mat4 MV;

void main()
{
    vec3 nlightDir = normalize(vec4(lightDir,1)).xyz;
    float diffuse = clamp(dot(nlightDir,normal),0,1);

    out_color = vec4(diffuse*vec3(0,1,0),1.0);
}

Thanks

Answer 1

There are a lot of wrong things in your code. Most of your problems come from completely forgetting what space various vectors are in. You cannot meaningfully do computations between vectors that are in different spaces.

normal = normalize(transpose(inverse(MV)) * vec4(normal,1)).xyz;

By using 1 as the fourth component of the normal, you completely break this computation. It causes the normal to be translated, which is not appropriate.

Furthermore, your normal value is computed based on gl_Position. And gl_Position is in clip space, not model space. So all you get is the clip-space normal, which is not what you need, want, or can even use.

If you want to compute the camera-space normal, then compute it from camera-space positions. Or compute the model-space normal from model-space positions and use the model/view matrix to transform it to camera-space.

Also, do the inverse/transpose on the CPU and pass it to the shader. Oh, and take all of the normal computations out of the loop; you only need to do it once per triangle (store it in a local variable and copy it to the output for each vertex). And stop doing the cross-product manually; use the built-in GLSL cross function.

vec3 nlightDir = normalize(vec4(lightDir,1)).xyz;

This makes no more sense than using 1 as the forth component in your transform before. Just normalize lightDir directly.

Equally importantly, if you're doing lighting in camera space, then the light direction needs to change with the camera in order for it to remain in the same apparent direction in the world. So you're going to have to take your world-space light position and transform it to camera space (typically on the CPU, passed in as a uniform).