XNA Vertex Normal Calculation not working for Cube

Question

I have a function which calculates Vertex Normals for most primitives just right. But it can't seem to deal with Cubes.

When I try the diffuse light shader it gives me this result (same on the right hand side)

This is my code to Calculate Normals:

public virtual void GenerateNormals()
    {
        for (int i = 0; i < vertices.Length; i++)
            vertices[i].Normal = new Vector3(0, 0, 0);

        for (int i = 0; i < indices.Length / 3; i++)
        {
            Vector3 firstvec = vertices[indices[i * 3 + 1]].Position - vertices[indices[i * 3]].Position;
            Vector3 secondvec = vertices[indices[i * 3]].Position - vertices[indices[i * 3 + 2]].Position;
            Vector3 normal = Vector3.Cross(firstvec, secondvec);
            normal.Normalize();
            vertices[indices[i * 3]].Normal += normal;
            vertices[indices[i * 3 + 1]].Normal += normal;
            vertices[indices[i * 3 + 2]].Normal += normal;
        }

        for (int i = 0; i < vertices.Length; i++)
            if (vertices[i].Normal != Vector3.Zero) vertices[i].Normal.Normalize();
    }

The Cube's Vertices are generated like that:

Vector3[] vectors = new Vector3[8];

        vectors[0] = new Vector3(-1, 1, -1);
        vectors[1] = new Vector3(1, 1, -1);
        vectors[2] = new Vector3(-1, 1, 1);
        vectors[3] = new Vector3(1, 1, 1);
        vectors[4] = new Vector3(-1, -1, -1);
        vectors[5] = new Vector3(1, -1, -1);
        vectors[6] = new Vector3(-1, -1, 1);
        vectors[7] = new Vector3(1, -1, 1);


        vertices = new VertexPositionNormalColored[24];

        //Top
        vertices[0].Position = vectors[0];
        vertices[0].Color = TopColor;
        vertices[1].Position = vectors[1];
        vertices[1].Color = TopColor;
        vertices[2].Position = vectors[2];
        vertices[2].Color = TopColor;
        vertices[3].Position = vectors[3];
        vertices[3].Color = TopColor;

        //Bottom
        vertices[4].Position = vectors[4];
        vertices[4].Color = BottomColor;
        vertices[5].Position = vectors[5];
        vertices[5].Color = BottomColor;
        vertices[6].Position = vectors[6];
        vertices[6].Color = BottomColor;
        vertices[7].Position = vectors[7];
        vertices[7].Color = BottomColor;

        //Left
        vertices[8].Position = vectors[2];
        vertices[8].Color = LeftColor;
        vertices[9].Position = vectors[0];
        vertices[9].Color = LeftColor;
        vertices[10].Position = vectors[6];
        vertices[10].Color = LeftColor;
        vertices[11].Position = vectors[4];
        vertices[11].Color = LeftColor;

        //Right
        vertices[12].Position = vectors[3];
        vertices[12].Color = RightColor;
        vertices[13].Position = vectors[1];
        vertices[13].Color = RightColor;
        vertices[14].Position = vectors[7];
        vertices[14].Color = RightColor;
        vertices[15].Position = vectors[5];
        vertices[15].Color = RightColor;

        //Back
        vertices[16].Position = vectors[0];
        vertices[16].Color = BackColor;
        vertices[17].Position = vectors[1];
        vertices[17].Color = BackColor;
        vertices[18].Position = vectors[4];
        vertices[18].Color = BackColor;
        vertices[19].Position = vectors[5];
        vertices[19].Color = BackColor;

        //Front
        vertices[20].Position = vectors[2];
        vertices[20].Color = FrontColor;
        vertices[21].Position = vectors[3];
        vertices[21].Color = FrontColor;
        vertices[22].Position = vectors[6];
        vertices[22].Color = FrontColor;
        vertices[23].Position = vectors[7];
        vertices[23].Color = FrontColor;

And the indices like that:

indices = new int[36];

        //Top
        indices[0] = 0;
        indices[1] = 1;
        indices[2] = 2;
        indices[3] = 2;
        indices[4] = 3;
        indices[5] = 1;

        //Back
        indices[6] = 16;
        indices[7] = 17;
        indices[8] = 18;
        indices[9] = 18;
        indices[10] = 19;
        indices[11] = 17;
        //Left
        indices[12] = 8;
        indices[13] = 9;
        indices[14] = 10;
        indices[15] = 10;
        indices[16] = 11;
        indices[17] = 9;
        //Front
        indices[18] = 20;
        indices[19] = 21;
        indices[20] = 22;
        indices[21] = 22;
        indices[22] = 23;
        indices[23] = 21;
        //Right
        indices[24] = 12;
        indices[25] = 13;
        indices[26] = 14;
        indices[27] = 14;
        indices[28] = 15;
        indices[29] = 13;
        //Bottom
        indices[30] = 4;
        indices[31] = 5;
        indices[32] = 6;
        indices[33] = 6;
        indices[34] = 7;
        indices[35] = 5;

I hope experts can help me find the solution. Thanks in advance!

Answer 1

You get this result because vertices in two different triangles do not have the same order.

In a right handed system, your first triangle {(-1,1,-1), (1,1,-1) , (-1,1,1)} is in clockwise order while the second {(-1,1,1), (1,1,1) , (1,1,-1)} is counterclockwise.

The normals for these triangles will have different directions.

Answer 2

Hmm, looks like you're calculating (and summing) the normal for some of the vertices twice (since you're going through the indices array, and each face reuse some of the vertices). The normals should be the same though, and the normalizing should take that out. I suspect that the calculated normal goes in one direction on the first go, and then in the opposite on the next, resulting in a normal of (0,0,0).

Try skipping the indices array and go through the vertices array directly, calculating the normal for each vertex only once.