# Calculating vertex normals - I'm doing something wrong

This topic is 3770 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

I've been working at this damn problem on and off for months now. It's time I sat down and finally fixed it. The first source is the optimised version of the slow code below. The lighting on my terrain seems non-existant, it's pure white.
		//For calculating face normals
SYM_VECTOR3 Point[3];

//Loop through faces
for (int f = 0; f < Face.size(); f++)
{
//Convert vertices of triangle to vectors
Point[0] = Vertex[ Face[f].a ];
Point[1] = Vertex[ Face[f].b ];
Point[2] = Vertex[ Face[f].c ];

//Calculate face normal
Face[f].Normal = GetFaceNormal(Point[0], Point[1], Point[2]);

//Add the face normal to the vertex normal of each vertex in this face
Vertex[ Face[f].a ].Normal += Face[f].Normal;
Vertex[ Face[f].b ].Normal += Face[f].Normal;
Vertex[ Face[f].c ].Normal += Face[f].Normal;
}

//Loop through vertices
for (int v = 0; v < Vertex.size(); v++)
{
//Normalise the summed normals
Vertex[v].Normal.Normalise();
}


My old code (which took several MINUTES to calculate for each model) worked ok. Terrain was smooth.

//CALCULATE FACE NORMALS

//For calculating normals
SYM_VECTOR Point[3];

//Calculate normals
for (int i = 0; i < Face.size(); i++)
{
Point[0].i = Vertex[Face.a].x;
Point[0].j = Vertex[Face.a].y;
Point[0].k = Vertex[Face.a].z;

Point[1].i = Vertex[Face.b].x;
Point[1].j = Vertex[Face.b].y;
Point[1].k = Vertex[Face.b].z;

Point[2].i = Vertex[Face.c].x;
Point[2].j = Vertex[Face.c].y;
Point[2].k = Vertex[Face.c].z;

//Calculate normal
Face.Normal = GetFaceNormal(Point[2], Point[1], Point[0]);

//Convert to array for OpenGL use
Face.NormalArray[0] = Face.Normal.i;
Face.NormalArray[1] = Face.Normal.j;
Face.NormalArray[2] = Face.Normal.k;
}

//CALCULATE VERTEX NORMALS

//Allocate a temp normal to store for calculation
SYM_VECTOR TempNormal;

//Loop through each vertex
for (int v = 0; v < Vertex.size(); v++)
{
//Reset the temp normal
///TODO: Implement '=' by scalar
TempNormal.i = 0;
TempNormal.j = 0;
TempNormal.k = 0;

//Keep count of normals found
int Incident = 0;

//Loop through each face
for (int f = 0; f < Face.size(); f++)
{
//See if face uses the vertex
if ((Vertex[Face[f].a] == Vertex[v]) || (Vertex[Face[f].b] == Vertex[v]) || (Vertex[Face[f].c] == Vertex[v]))
{
//This face uses the vertex, so store it's normal
TempNormal += Face[f].Normal;

//Count it
Incident++;
}
}

//Check if any instances have been found
if (Incident != 0)
{
//Get average normal
TempNormal = TempNormal / Incident;

//Normalise the averaged normal
Normalise(TempNormal);

//Store it
Vertex[v].Normal = TempNormal;

//Convert to array for OpenGL use
Vertex[v].NormalArray[0] = Vertex[v].Normal.i;
Vertex[v].NormalArray[1] = Vertex[v].Normal.j;
Vertex[v].NormalArray[2] = Vertex[v].Normal.k;
}
}


I'd appreciate it if anyone could hint me in the right direction. As far as I can see, the top code does exactly the same as the bottom, just faster.

##### Share on other sites
Try taking a look at these two posts:
http://www.gamedev.net/community/forums/topic.asp?topic_id=389021

&

http://www.gamedev.net/community/forums/topic.asp?topic_id=393131

I was having what seems to be the same problem as you and while I can't remember exactly which solution worked for me, I do know that one of those two posts has the correct method that I started using. So read through, understand and try to implement. It really helped me out of my problem.

Cheers.

##### Share on other sites
Two things you should check:

1. Are the vertex normals initialised to 0,0,0. In the "old code it didn't mattered if/how the normal was initialised.
2. in "old code" you set the vertex.normalArray which you don't do in the optimized version. (with the code you've posted it's impossible to tell if you made other changes so you don't need the normalArray anymore)

##### Share on other sites
Quote:
 Original post by Woodoo2. in "old code" you set the vertex.normalArray which you don't do in the optimized version. (with the code you've posted it's impossible to tell if you made other changes so you don't need the normalArray anymore)

Well, that fixed the first problem! I must have done that yesterday accidentally.

Now the normals are actually showing up, this is what I was getting before they suddenly "disappeared":

And here is my code so far:

		//For calculating face normals		SYM_VECTOR3 Point[3];		//Loop through faces		for (int f = 0; f < Face.size(); f++)		{			//Convert vertices of triangle to vectors			Point[0] = Vertex[ Face[f].a ];			Point[1] = Vertex[ Face[f].b ];			Point[2] = Vertex[ Face[f].c ];			//Calculate face normal			Face[f].Normal = GetFaceNormal(Point[0], Point[1], Point[2]);			//Convert to array for OpenGL use			Face[f].NormalArray[0] = Face[f].Normal.i;			Face[f].NormalArray[1] = Face[f].Normal.j;			Face[f].NormalArray[2] = Face[f].Normal.k;			//Add the face normal to the vertex normal of each vertex in this face			Vertex[ Face[f].a ].Normal += Face[f].Normal;			Vertex[ Face[f].b ].Normal += Face[f].Normal;			Vertex[ Face[f].c ].Normal += Face[f].Normal;			//Keep count of the number of normals added			Vertex[ Face[f].a ].NormalCounter++;			Vertex[ Face[f].b ].NormalCounter++;			Vertex[ Face[f].c ].NormalCounter++;		}		//Loop through vertices		for (int v = 0; v < Vertex.size(); v++)		{			//Average the normal			if(Vertex[v].NormalCounter != 0) Vertex[v].Normal = Vertex[v].Normal / Vertex[v].NormalCounter;			//Normalise the summed normals			Vertex[v].Normal.Normalise();			//Create opengl array			Vertex[v].NormalArray[0] = Vertex[v].Normal.i;			Vertex[v].NormalArray[1] = Vertex[v].Normal.j;			Vertex[v].NormalArray[2] = Vertex[v].Normal.k;		}

I get the same effect whether I average the vertex normal or not, or if I nomalise the face normal before calculating the vertex normal or not.

I've traced over the code in my head several times. I can't spot the flaw in the method. Here's what I (in theory) am doing:

1. Loop through faces
- Calculate the face normal
- (Optionally) normalise the face normal
- Add the face normal to the normal of each vertex attached to that face
- (Optionally) Increment a counter on the vertex for averaging later

2. Loop through vertices
- (Optionally) Average the vertex normal
- Normalise the vertex normal

It's an optimised version of the "old" method (which is explained in Enosch's post here: http://www.gamedev.net/community/forums/topic.asp?topic_id=393131)

And yes, the constructor of VECTOR3 initialises i, j and k to 0.

##### Share on other sites
I'm using a simple function that works great (for terrain at least).
Start at [x+1,y+1] and loop through this and the eight surrounding vertices.
The current normal becomes the sum of all normals divided by nine.

All vertices needs to have their normals supplied first, but this is created when loading the heightmap. Also edges aren't affected by normal-smoothing, but that shouldn't be a problem for most occasions.

##### Share on other sites
It makes sense except for this line:

Quote:
 Original post by RasmadrakAll vertices needs to have their normals supplied first

Was it a typo, and you meant faces? Otherwise I don't see what you mean.

I'll give that a try, but I'm still unsure why my technique doesn't work :(

##### Share on other sites

//Calculate face normalFace[f].Normal = GetFaceNormal(Point[0], Point[1], Point[2]);

//Calculate normalFace.Normal = GetFaceNormal(Point[2], Point[1], Point[0]);

Check each normal before you sum them together. Make sure they are all pointing in the same direction (up in this case). A dot product test with (0,1,0) should work.

##### Share on other sites
Thanks Paranoia, that was just trying reversing the winding at the time I posted the code, it's set back to normal now.

The face normals are correct. I'm using the same method to calculate face normals in both versions of the code (they are visually correct too).

I'm trying a testing measure: In the loop, if the vertex is equal to 47 I cout the face numbers attached to that vertex.

In the old code, vertex 47 is attached to faces 20, 21, 23, 50, 52 and 53.
in the new code, it just outputs face 23.

Somehow, the new code is only finding one face per vertex. I don't see what the problem is, for ever face I add the normal to Vertex[Face.a], Vertex[Face.b] AND Vertex[Face.c].

I'm really baffled :/

##### Share on other sites
Could it be because (in the new code) you are adding the normals to the vertex object but not counting the number of normals added and dividing by that number before normalizing?

Could you post the code you just said you were using to test?

##### Share on other sites
There are a few articles discussing whether or not the normals should be averaged. It's just a slightly different visual effect, usually not worth the hassle.

The 'old' code works fine if I don't average them, so I've taken it out for simplicity (and to try and narrow the problem down).

Here is the code for debugging the old method:

		//For calculating normals		SYM_VECTOR3 Point[3];		//Calculate face normals		for (int i = 0; i < Face.size(); i++)		{			Point[0] = Vertex[Face.a];			Point[1] = Vertex[Face.b];			Point[2] = Vertex[Face.c];			//Calculate normal			Face.Normal = GetFaceNormal(Point[0], Point[1], Point[2]);		}		//CALCULATE VERTEX NORMALS		//Allocate a temp normal to store for calculation		SYM_VECTOR3 TempNormal;		//Loop through each vertex		for (int v = 0; v < Vertex.size(); v++)		{			//Reset the temp normal			///TODO: Implement '=' by scalar			TempNormal.i = 0;			TempNormal.j = 0;			TempNormal.k = 0;			//Loop through each face			for (int f = 0; f < Face.size(); f++)			{				//See if face uses the vertex				if ((Vertex[Face[f].a] == Vertex[v]) || (Vertex[Face[f].b] == Vertex[v]) || (Vertex[Face[f].c] == Vertex[v]))				{					//This face uses the vertex, so store it's normal					TempNormal += Face[f].Normal;					if(v == 47) cout << "Face " << f << " uses vertex 47\n";				}			}			//Normalise the averaged normal			TempNormal.Normalise();			//Store it			Vertex[v].Normal = TempNormal;			//Convert to array for OpenGL use			Vertex[v].NormalArray[0] = Vertex[v].Normal.i;			Vertex[v].NormalArray[1] = Vertex[v].Normal.j;			Vertex[v].NormalArray[2] = Vertex[v].Normal.k;		}

This outputs the following:

Quote:
 Face 20 uses vertex 47Face 21 uses vertex 47Face 23 uses vertex 47Face 50 uses vertex 47Face 52 uses vertex 47Face 53 uses vertex 47

And the code for the new method:

		//For calculating face normals		SYM_VECTOR3 Point[3];		//Loop through faces		for (int f = 0; f < Face.size(); f++)		{			//Convert vertices of triangle to vectors			Point[0] = Vertex[ Face[f].a ];			Point[1] = Vertex[ Face[f].b ];			Point[2] = Vertex[ Face[f].c ];			//Calculate face normal			Face[f].Normal = GetFaceNormal(Point[0], Point[1], Point[2]);			//Add the face normal to the vertex normal of each vertex in this face			Vertex[ Face[f].a ].Normal += Face[f].Normal;			Vertex[ Face[f].b ].Normal += Face[f].Normal;			Vertex[ Face[f].c ].Normal += Face[f].Normal;			if(Face[f].a == 47 || Face[f].b == 47 || Face[f].c == 47) cout << "Face " << f << " uses vertex 47\n";		}		//Loop through vertices		for (int v = 0; v < Vertex.size(); v++)		{			//Normalise the summed normals			Vertex[v].Normal.Normalise();			//Create opengl array			Vertex[v].NormalArray[0] = Vertex[v].Normal.i;			Vertex[v].NormalArray[1] = Vertex[v].Normal.j;			Vertex[v].NormalArray[2] = Vertex[v].Normal.k;		}

Which outputs:

Quote:
 Face 23 uses vertex 47

1. 1
2. 2
Rutin
19
3. 3
JoeJ
16
4. 4
5. 5

• 30
• 22
• 13
• 13
• 17
• ### Forum Statistics

• Total Topics
631700
• Total Posts
3001800
×