OpenGL Calculating Normals (Quads)

Question

My issue is regarding OpenGL, and Normals, I understand the math behind them, and I am having some success.

The function I've attached below accepts an interleaved Vertex Array, and calculates the normals for every 4 vertices. These represent QUADS that having the same directions. By my understanding these 4 vertices should share the same Normal. So long as they face the same way.

The problem I am having is that my QUADS are rendering with a diagonal gradient, much like this: Light Effect - Except that the shadow is in the middle, with the light in the corners.

I draw my QUADS in a consistent fashion. TopLeft, TopRight, BottomRight, BottomLeft, and the vertices I use to calculate my normals are TopRight - TopLeft, and BottomRight - TopLeft.

Hopefully someone can see something I've made a blunder on, but I have been at this for hours to no prevail.

For the record I render a Cube, and a Teapot next to my objects to check my lighting is functioning, so I'm fairly sure there is no issue regarding Light position.

void CalculateNormals(point8 toCalc[], int toCalcLength)
{
    GLfloat N[3], U[3], V[3];//N will be our final calculated normal, U and V will be the subjects of cross-product
    float length;

for (int i = 0; i < toCalcLength; i+=4) //Starting with every first corner QUAD vertice
{
    U[0] = toCalc[i+1][5] - toCalc[i][5]; U[1] = toCalc[i+1][6] - toCalc[i][6]; U[2] = toCalc[i+1][7] - toCalc[i][7]; //Calculate Ux Uy Uz 
    V[0] = toCalc[i+3][5] - toCalc[i][5]; V[1] = toCalc[i+3][6] - toCalc[i][6]; V[2] = toCalc[i+3][7] - toCalc[i][7]; //Calculate Vx Vy Vz

    N[0] = (U[1]*V[2]) - (U[2] * V[1]);
    N[1] = (U[2]*V[0]) - (U[0] * V[2]);
    N[2] = (U[0]*V[1]) - (U[1] * V[0]);

    //Calculate length for normalising
    length = (float)sqrt((pow(N[0],2)) + (pow(N[1],2)) + (pow(N[2],2)));

    for (int a = 0; a < 3; a++)
    {
        N[a]/=length;
    }

    for (int j = 0; i < 4; i++)
    {
                    //Apply normals to QUAD vertices (3,4,5 index position of normals in interleaved array)
        toCalc[i+j][3] = N[0]; toCalc[i+j][4] = N[1]; toCalc[i+j][5] = N[2];
    }
}
}

Answer 1

It seems like you are taking the vertex position values for use in calculations from indices 5, 6, and 7, and then writing out the normals at indices 3, 4, and 5. Note how index 5 is used on both. I suppose one of them is not correct.

Answer 2

It looks like your for-loops are biting you.

for (int i = 0; i < toCalcLength; i+=4) //Starting with every first corner QUAD vertice
{
  ...
  for (int j = 0; i < 4; i++)
  { //            ^      ^
    // Should you be using 'j' instead of 'i' here?
    // j will never increment
    // This loop won't be called at all after the first time through the outer loop
    ...
  }
}

Answer 3

You use indexes 3, 4, and 5 for storing normal:

toCalc[i+j][3] = N[0]; toCalc[i+j][4] = N[1]; toCalc[i+j][5] = N[2];

AND you use indexes 5, 6 and 7 to get point coordinates:

U[0] = toCalc[i+1][5] - toCalc[i][5]; U[1] = toCalc[i+1][6] - toCalc[i][6]; U[2] = toCalc[i+1][7] - toCalc[i][7];

Those indexes overlap (normal.x shares same index as position.z), which shouldn't be happening.

---

Recommendations:

1. Put everything into structures.
2. Either:
   1. Use math library.
   2. OR put vector arithmetics into separate appropriately named subroutines.
3. Use named variables instead of indexes.

By doing so you'll reduce number of bugs in your code. a.position.x is easier to read than quad[0][5], and it is easier to fix a typo in vector operation when the code hasn't been copy-pasted.

---

You can use unions to access vector components by both index and name:

struct Vector3{
    union{
        struct{
            float x, y, z;
        };
        float v[3];
    };
};    

---

For calcualting normal in quad ABCD

A--B
|  |
C--D

Use formula:

normal = normalize((B.position - A.position) X (C.position - A.position)).

OR

normal = normalize((D.position - A.position) X (C.position - B.position)).

Where "X" means "cross-product".

Either way will work fine.