Newbee needs help with Phong Shading

Hello all!

I’m taking a basic OpenGL course and the professor can be a little vague sometimes with the actual coding of things. I have a little app with a small “car” that drives around a little stage. He has asked us to take that app and change it to use Phong shading. He “suggests” I first “Re-model all objects with normals and surface reflectance properties”… But is pretty vague on exactly how to do that. I understand the theory of reflective and diffuse surfaces as well as faking an “ambient” light for the scene. What I don’t have a grasp on is how to do this in C++ and GLSL.

Does anybody know of any tutorials on going from per vertex lighting to per-pixel Phong shading/lighting? I hesitate to post the whole thing here as it’s pretty big between the cpp and vertex and fragment shaders, but I suppose could…

Thanks,
–Pete

http://www.ozone3d.net/tutorials/glsl_lighting_phong.php

this will show how to calculate a surface normal

http://www.opengl.org/wiki/Calculating_a_Surface_Normal

for the normal at a vertex, sum the normals for all the triangles it is shared by; divided by the number of triangles; then normalise this vector

Thanks! I’ll give those a look right away!

–Pete

Oh I guess I can add files without making a 12 page post via the “file manager for this post” ! Great forum!

While I’m looking at those threads, here’s my little “app” both before I started trying to add shading, and the current mess I’m in trying to do this…

–Pete

Hi StickyPete, I should not talk without looking at my code. I just posted this for another guy. It is similar to the code I use for calculating normals

I have had a quick look at your code “current mess”. I can see your shader is expecting a normal and some colour info for each vertex but it is nor obvious to me that your are sending that in your vertex buffer object

``````
CVector  normal[8], temp;

for (int i = 0; i < 8; i++)
{
normal[i].x = normal[i].y = normal[i].z = 0;
}

for (int i = 0; i < 24; i += 3)
{
CVector3D vAB = m_vVertices.pObj[m_VertexIndices.pObj[i+1]]-m_vVertices.pObj[m_VertexIndices.pObj[i];
CVector3D vAC = m_vVertices.pObj[m_VertexIndices.pObj[i+2]]-m_vVertices.pObj[m_VertexIndices.pObj[i];
temp = Vector3DCross(vAB, vAC);

for (int j = 0; j < 3; j++)
{
normal[m_VertexIndices.pObj[i+1]] += temp
}
}

for (int i = 0; i < 8; i++)
{
Vector3DNormalize(normal);
}

``````

Thanks! I haven’t had any time to look at those posts you first gave me (had to do the Easter Brunch thing at my mother-in-law’s) but I’m going to look at them now…

As for my “current mess” yes, that’s what I’m trying to do, with the emphasis on “trying”… OK I got the general idea that I have to calculate the normal for each vertex and then “pass” that info on to my vertex shader (vshader_car.glsl) and then just “hand it off” to the fragment shader (fshader_car.glsl) who will re-normalize it and then display everything real nice & 3D like… But going from theory to code is more difficult for me…

For example my sphere’s are all created with triangles using a function shamelessly stolen from Angel’s textbook “Interactive Computer Graphics”. Originally that function had lines like this:

``````sphereVerts[k] = vec4(sin(thetar)*cos(phir), cos(thetar)*cos(phir), sin(phir), 1.0);
k++;

sphereVerts[k] = vec4(sin(thetar)*cos(phir20), cos(thetar)*cos(phir20), sin(phir20), 1.0);
k++;
(etc...)
``````

And the color would get set with the vcolor attribute while I was transforming it, putting it in the modelview and then actually drawing an example of a sphere in display() like this: (I’m actually drawing a TON of sphere’s like this)

``````mv = mv*Translate(20.0, 1.7, 20.0)*Scale(3.0, 3.0, 3.0);
glUniformMatrix4fv(model_view, 1, GL_TRUE, mv);
glBindVertexArray(vao[2]);
glVertexAttrib4f(vColor, 0.0, 0.6, 1.0, 1.0);
glDrawArrays(GL_TRIANGLES, 0, 1140);
``````

All worked well, I could make a bunch of objects for my little car to “drive around” as well as the drivers “head” and eyes by just transforming that original sphere over and over again…

OK, now to go from a flat vcolor to Phong shading I changed my gensphere() code to calculate the normal (sphereNormals) like this:

``````
sphereNormals[k] = vec3(sin(thetar)*cos(phir), cos(thetar)*cos(phir), sin(phir));
sphereVerts[k] = vec4(sin(thetar)*cos(phir), cos(thetar)*cos(phir), sin(phir), 1.0);
k++;

sphereNormals[k] = vec3(sin(thetar)*cos(phir20), cos(thetar)*cos(phir20), sin(phir20));
sphereVerts[k] = vec4(sin(thetar)*cos(phir20), cos(thetar)*cos(phir20), sin(phir20), 1.0);
k++;
(etc...)
``````

And now the calls to create the sphere’s have all been changed to use vAmbientDiffuseColor rather than vcolor like this:

``````mv = mv*Translate(20.0, 1.7, 20.0)*Scale(3.0, 3.0, 3.0);
glUniformMatrix4fv(model_view, 1, GL_TRUE, mv);
glBindVertexArray(vao[2]);
glVertexAttrib4f(vAmbientDiffuseColor, 0.0, 0.6, 1.0, 1.0);
glDrawArrays(GL_TRIANGLES, 0, 1140);
``````

OK all fine and good (I hope), but now, unlike just having flat lights in my cpp file, I need to somehow get all this into my vshader and then my fshader, right?
So in my vertex shader I have:

``````in vec4 vAmbientDiffuseColor;
in vec3 sphereNormals;
.....
void
main()
{
vec4 sphereNormals = vec4(sphereNormals, 0.0);
AmbientDiffuseColor = vAmbientDiffuseColor;
....
vN = normalize(model_view * sphereNormals).xyz;
.....
out vec3 vN;
out vec4 AmbientDiffuseColor;
``````

So I’m hoping that takes in my diffuse colors and my normals and passes them out…'Then my fragment shader looks like this:

``````in vec3 vN;
in vec4 AmbientDiffuseColor;
....
uniform vec4 ambient_light;
...

``````

And then the main() function in the fragment shader has some stuff I admit I’m not totally following, I got it from Angel’s website (code examples for the textbook) and perhaps that’s my real problem… Here is the ENTIRE fragment shader code:

``````#version 150

in vec3 position;
in vec3 vN;

in vec4 AmbientDiffuseColor;
in vec4 SpecularColor;
in float SpecularExponent;

uniform vec4 light_position;
uniform vec4 light_color;
uniform vec4 ambient_light;

out vec4  fColor;

void main()
{

vec3 L = normalize( light_position.xyz - position.xyz);
vec3 E = normalize(-position.xyz);
vec3 N = normalize(vN);

vec3 H = normalize(L+E);
vec4 amb = AmbientDiffuseColor * ambient_light;
vec4 diff = max(dot(L,N), 0.0) * AmbientDiffuseColor * light_color;
vec4 spec = pow( max (dot(N,H), 0.0), SpecularExponent) *  SpecularColor * light_color  ;
if(dot(L,N) < 0.0){
spec = vec4(1,0,0,1);
}
fColor = amb + diff + spec;

}
``````

But when I run this it looks pretty close to the same output I had without ANY shading or lighting changes with the possible exception of the fact that it’s darker on half of the stage…

Any idea’s? I’m going to try to really hit those links you posted as well as the code you just posted above…

–Pete

PS. The reason I posted the zip file before I started all this (my first attachment) is because I’m totally open to suggestions on accomplishing this a different way. What I want is to add Phong shading with a low level ambient light. If you can think of a better way to do this I’ll take all the help I can get!

PSS. I added 3 screen shots if you’re interested. It shows how the program looks right now with my attempts at lighting/shading from the three “camera views”. Notice that if you’re in one of the two mobile cams (viewpoint is from the drivers “head” and chase cam is up and behind) that there is a big specular reflection on the stage! I’m assuming this is coming from this:

``````glVertexAttrib4fv(vAmbientDiffuseColor, vec4(.5, 0, 0, 1));
glVertexAttrib4fv(vSpecularColor, vec4(1.0f,1.0f,1.0f,1.0f));
glVertexAttrib1f(vSpecularExponent, 100.0);
glUniform4fv(light_color, 1, vec4(1,1,1,1));
glUniform4fv(ambient_light, 1, vec4(.5, .5, .5, 5));
``````

but why does that work so easy? Is that all I need to do for each object? That can’t be right…

I haven’t used glVertexAttrib4f but it probably works ok for setting the colour. I do mine with a uniform. Someone else might comment on that.

You don’t seem to be telling the shader where to get the normal data from so you will get junk.

There are different ways of doing this but this it what I do.

step 1.
Create a vertex structure with position and normal

``````
struct VertexN
{
float		x;
float		y;
float		z;
float		nx;
float		ny;
float		nz;
};

``````

step 2

``````
// this is pseudo code
sphere[k].nx = sin(thetar)*cos(phir);
sphere[k].ny = cos(thetar)*cos(phir);
sphere[k].nz =  sin(phir);
sphere[k].x = sin(thetar)*cos(phir);
sphere[k].y = cos(thetar)*cos(phir);
sphere[k].z = sin(phir);
k++;

``````

step 3
build the vao, but include something like this to tell the shader where to find the posiotn and normal in the vertex structure

``````
glVertexAttribPointer(VERTEX_BINDINGS_POSITION, 3, GL_FLOAT,  GL_FALSE, sizeof(VertexN), 0);
glEnableVertexAttribArray(VERTEX_BINDINGS_POSITION);
glVertexAttribPointer(VERTEX_BINDINGS_NORMAL, 3, GL_FLOAT, GL_FALSE,sizeof(VertexN),(const void*)(sizeof(float)*3));
glEnableVertexAttribArray(VERTEX_BINDINGS_NORMAL);

``````

step 4
render

Wow that looks like EXACTLY the kind of help I’m looking for! Thank YOU!

OK After reading your posts you gave me the first time I decided to rip out my messy attempts of “cutting and pasting what some examples in the book were doing…” and am now back to a clean non-shaded version of my program. Along with some of the other fixes I had made like only calling LookAt() once and using my stack to store the modelview, etc…

So I will now take your excellent step by step help above and only code something if I know what it is doing!

Thanks again & wish me luck! And maybe check back once and a while? Even if all goes well here with the shading, spotlights is next!

Thanks again - You ROCK!
–Pete

HELP!!

OK things didn’t go so well… But I think I’m pretty close…

Can you tell me what I did wrong here? If I comment out line 380 I get a car body and wheels with no sphere’s on the stage at all; and, of course, it’s the sphere’s I’m trying to shade!

I created an array and (I think) properly calculated the normal vectors, but when I try to use them things explode, Windows gives me a memory violation exception, which makes sense when commenting out the binding to the buffer array stops the exception…

Can you look at what I’m doing in my init() function in car.cpp (specifically lines 380 and 381) and tell me where I’m going wrong?

Thanks,
–Pete

If this is the code you used when you tried to reference vbo[5], that will be a problem; you either need to generate 2 buffer, or have another gnerate call

``````
glGenBuffers( 1, &vbo[4] ); //just position this time

``````

I recommend checking OpenGL error throughtout your code; it will help with debugging

``````
unsigned int lastError = glGetError();

``````

I have mine in a subroutine that does various checks and displays information to the debug file

Ah ha! I figured it was something like that!

2 things, first the buffer. Can I use the same buffer as the vertex’s (vbo[4]) like this:

``````//Now the sphere for the head, eyes and stage clutter
glBindVertexArray( vao[2] );
glGenBuffers( 1, &vbo[4] ); //just position this time

//Sphere vertex's
glBindBuffer( GL_ARRAY_BUFFER, vbo[4] );
glBufferData( GL_ARRAY_BUFFER, sizeof(sphereVerts), sphereVerts, GL_STATIC_DRAW );

//colors for each sphere vertex
glBindBuffer( GL_ARRAY_BUFFER, vbo[4] );
glBufferData( GL_ARRAY_BUFFER, sizeof(vec3), sphereNormals, GL_STATIC_DRAW );
``````

No that can’t work can it? You said I “either need to generate 2 buffers, or have another generate call”… I thought I was using a second buffer with the vbo[5]…
Oh wait is this what I need to do?

``````//Now the sphere for the head, eyes and stage clutter
glBindVertexArray( vao[2] );
glGenBuffers( 1, &vbo[4] ); //just position this time
//Sphere vertex's
glBindBuffer( GL_ARRAY_BUFFER, vbo[4] );
glBufferData( GL_ARRAY_BUFFER, sizeof(sphereVerts), sphereVerts, GL_STATIC_DRAW );

glBindVertexArray( vao[2] );
glGenBuffers( 1, &vbo[5] );
//colors for each sphere vertex
glBindBuffer( GL_ARRAY_BUFFER, vbo[5] );
glBufferData( GL_ARRAY_BUFFER, sizeof(vec3), sphereNormals, GL_STATIC_DRAW );
``````

Will that bind both vbo[4] (the vertex’s) and vbo[5] (the normals) to vao[2]? Can I do that? Is that what you meant by “have another generate call”?

And last a OpenGL debug file would be #\$%@ FANTASTIC!
If I put “unsigned int lastError = glGetError();” up at the top of my cpp file how do I get it to write out information to a file? just something like

``````lastError > debug.txt;
``````

No that would just check if lastError was greater than debug.txt and barf… Come to think of it I’ve never outputted debug into a file in C++…

Thanks again,
–Pete

Hmmm, well adding the second glBindVertexArray and glGenBuffers stopped it from blowing up, but I still don’t have any sphere’s on the screen… So you were right about the error, but I’m still obviously doing something wrong…

I’m going to try the OpenGL debugging

–Pete

OK I figured out a little more, I had a little typo in my adding another genbuffers code I had a “sizeof(vec4), sphereNormals, GL_STATIC_DRAW );” ending the last line of code below; when I changed that to sizeof(sphereNormals),… it worked but… it’s a mess really this time!
(Also I realized I didn’t need the second binding to the array vao[4] either.)
So here’s what I have now in init():

``````//Now the sphere for the head, eyes and stage clutter
glBindVertexArray( vao[2] );
glGenBuffers( 2, &vbo[4] );
//Sphere vertex's
glBindBuffer( GL_ARRAY_BUFFER, vbo[4] );
glBufferData( GL_ARRAY_BUFFER, sizeof(sphereVerts), sphereVerts, GL_STATIC_DRAW );

glGenBuffers( 2, &vbo[8] );
//colors for each sphere vertex
glBindBuffer( GL_ARRAY_BUFFER, vbo[8] );
glBufferData( GL_ARRAY_BUFFER, sizeof(sphereNormals), sphereNormals, GL_STATIC_DRAW );
``````

Like I said that second block of text is new and should bind all the calculated normals to my spheres (vao[4]), right?
But now when I run it I get a jumbled mess of colored triangles all over the screen (see attached screenshot). And if I “drive” the car some of them move around the screen all crazy (which I assume is the “head” and headlights that move with the car) while most stay still, but I can’t even see the car in all this mess…
At first I thought it meant it wasn’t calculating normals right, but that has to do with positions not normals… Any idea what’s up this time? did I mess up genSphere() when I added the calculations of the normal vectors? That’s where I’m going to look next…

Thanks,
–Pete

I GOT IT!!!

Again without even being there, you helped me!
You were right it was my array handling, A little more looking made me realize I needed to “re-bind” my buffers before I tried setting anything on them, an addition of two more lines before I set my vPosition and vNormal to the shaders was the key for going from a jumbled mess to the beautiful attached photo! Right after doing the buffer handling like this (in init()):

``````//Now the spheres, used for the head, eyes and stage clutter
glBindVertexArray( vao[2] ); //sphere
glGenBuffers( 2, &vbo[4] );
//Sphere vertex's
glBindBuffer( GL_ARRAY_BUFFER, vbo[4] );
glBufferData( GL_ARRAY_BUFFER, sizeof(sphereVerts), sphereVerts, GL_STATIC_DRAW );

//colors for each sphere vertex
glGenBuffers( 2, &vbo[5] );
glBindBuffer( GL_ARRAY_BUFFER, vbo[5] );
glBufferData( GL_ARRAY_BUFFER, sizeof(sphereNormals), sphereNormals, GL_STATIC_DRAW );
``````

I was doing the position and normal assignments that eventually is picked up by the two shader files like this:

``````glBindBuffer( GL_ARRAY_BUFFER, vbo[4] );
vPosition = glGetAttribLocation(program, "vPosition");
glEnableVertexAttribArray(vPosition);
glVertexAttribPointer(vPosition, 4, GL_FLOAT, GL_FALSE, 0, 0);

glBindBuffer( GL_ARRAY_BUFFER, vbo[5] );
vNormal = glGetAttribLocation(program, "vNormal");
glEnableVertexAttribArray(vNormal);
glVertexAttribPointer(vNormal, 3, GL_FLOAT, GL_FALSE, 0, 0);
``````

Now before I did NOT have those two “glBindBuffer( GL_ARRAY_BUFFER, vbo[X]);” lines at the start of each block of code, and the result was the mess attached to my previous post… And in hindsight since the shaders didn’t know which array’s to get the position and normals from it’s not too surprising it was a jumbled mess… Of course hindsight is usually much clearer than pre-sight! My first thought was that something had happened to my genSphere() function when I added the normal calculations (thanks for that help too!) but that was just fine…

So next I’m going to shade the car (hopefully that goes a little easier!) and then try my hand at point lights…

But it was me staring at the code and thinking about your comment that originally I hadn’t coded the buffer array for my new normals properly that led me to realize that now I wasn’t using those buffers right either, so thanks for that very insightful comment, you must be part “psychic programmer”!

Take care,
–Pete

Or I could just reorder them like this and then I don’t need to re-bind the buffer array’s :o

``````//Now the spheres, used for the head, eyes and stage clutter
glBindVertexArray( vao[2] ); //sphere
glGenBuffers( 2, &vbo[4] );

//Sphere vertex's
glBindBuffer( GL_ARRAY_BUFFER, vbo[4] );
glBufferData( GL_ARRAY_BUFFER, sizeof(sphereVerts), sphereVerts, GL_STATIC_DRAW );
vPosition = glGetAttribLocation(program, "vPosition");
glEnableVertexAttribArray(vPosition);
glVertexAttribPointer(vPosition, 4, GL_FLOAT, GL_FALSE, 0, 0);

//normals for each sphere vertex
glGenBuffers( 2, &vbo[5] );
glBindBuffer( GL_ARRAY_BUFFER, vbo[5] );
glBufferData( GL_ARRAY_BUFFER, sizeof(sphereNormals), sphereNormals, GL_STATIC_DRAW );
vNormal = glGetAttribLocation(program, "vNormal");
glEnableVertexAttribArray(vNormal);
glVertexAttribPointer(vNormal, 3, GL_FLOAT, GL_FALSE, 0, 0);

``````

Again, hindsight…

–Pete

Your last post has a small error

glGenBuffers( 2, &vbo[4] );

allocates 2 buffer objects and puts the values in vbo[4] and vbo[5]

glGenBuffers( 2, &vbo[5] );

allocates 2 more buffer objects and puts the values in vbo[5] and vbo[6]!

Opps! (funny it still works though!) Those should be 1’s not 2’s…

Let me see if changing those has any visible effect…

Nope, looks the same… Probably because I’m allocating way more array’s than I need anyway, I should cut that back when I’m “done”…

But of course you’re right, I “wanted” to just use one buffer not two… I’m just lucky it was a buffer array and not an object one or everything would have been off by one and accessing them later with bindings would have been off by one and when I think I’m building a “spoke” I would have gotten a wheel and asking for a wheel would have given me a “whitewall” etc… And THAT defiantly would have a “visible effect”…

Thanks again!
–Pete