OpenGL 3.x Assimp trouble implementing phong shading (normals?)

Question

I'm having trouble getting phong shading to look right. I'm pretty sure there's something wrong with either my OpenGL calls, or the way I'm loading my normals, but I guess it could be something else since 3D graphics and Assimp are both still very new to me. When trying to load .obj/.mtl files, the problems I'm seeing are:

1. The models seem to be lit too intensely (less phong-style and more completely washed out, too bright).
2. Faces that are lit seem to be lit equally all over (with the exception of a specular highlight showing only when the light source position is moved to be practically right on top of the model)
3. Because of problems 1 and 2, spheres look very wrong:

picture of sphere

And things with larger faces look (less-noticeably) wrong too:

picture of cube

I could be wrong, but to me this doesn't look like proper phong shading.

Here's the code that I think might be relevant (I can post more if necessary):

file: assimpRenderer.cpp

#include "assimpRenderer.hpp"

namespace def
{

assimpRenderer::assimpRenderer(std::string modelFilename, float modelScale)
{
    initSFML();
    initOpenGL();

    if (assImport(modelFilename)) // if modelFile loaded successfully
    {
        initScene();
        mainLoop(modelScale);
        shutdownScene();
    }

    shutdownOpenGL();
    shutdownSFML();
}

assimpRenderer::~assimpRenderer()
{

}

void assimpRenderer::initSFML()
{
    windowWidth = 800;
    windowHeight = 600;
    settings.majorVersion = 3;
    settings.minorVersion = 3;
    app = NULL;
    shader = NULL;

    app = new sf::Window(sf::VideoMode(windowWidth,windowHeight,32), "OpenGL 3.x Window", sf::Style::Default, settings);
    app->setFramerateLimit(240);
    app->setActive();
    return;
}

void assimpRenderer::shutdownSFML()
{
    delete app;

    return;
}

void assimpRenderer::initOpenGL()
{
    GLenum err = glewInit();
    if (GLEW_OK != err)
    {
      /* Problem: glewInit failed, something is seriously wrong. */
      std::cerr << "Error: " << glewGetErrorString(err) << std::endl;
    }

    // check the OpenGL context version that's currently in use
    int glVersion[2] = {-1, -1};
    glGetIntegerv(GL_MAJOR_VERSION, &glVersion[0]); // get the OpenGL Major version
    glGetIntegerv(GL_MINOR_VERSION, &glVersion[1]); // get the OpenGL Minor version
    std::cout << "Using OpenGL Version: " << glVersion[0] << "." << glVersion[1] << std::endl;

    return;
}

void assimpRenderer::shutdownOpenGL()
{

    return;
}

void assimpRenderer::initScene()
{
    // allocate heap space for VAOs, VBOs, and IBOs
    vaoID = new GLuint[scene->mNumMeshes];
    vboID = new GLuint[scene->mNumMeshes*2];
    iboID = new GLuint[scene->mNumMeshes];

    glClearColor(0.4f, 0.6f, 0.9f, 0.0f);
    glEnable(GL_DEPTH_TEST);
    glDepthFunc(GL_LEQUAL);
    glEnable(GL_CULL_FACE);


    shader = new Shader("shader.vert", "shader.frag");
    projectionMatrix = glm::perspective(60.0f, (float)windowWidth / (float)windowHeight, 0.1f, 100.0f);

    rot = 0.0f;
    rotSpeed = 50.0f;

    faceIndex = 0;


    colorArrayA = NULL;
    colorArrayD = NULL;
    colorArrayS = NULL;

    normalArray = NULL;


    genVAOs();

    return;
}

void assimpRenderer::shutdownScene()
{
    delete [] iboID;
    delete [] vboID;
    delete [] vaoID;

    delete shader;
}

void assimpRenderer::renderScene(float modelScale)
{
    sf::Time elapsedTime = clock.getElapsedTime();
    clock.restart();

    if (rot > 360.0f)
        rot = 0.0f;
    rot += rotSpeed * elapsedTime.asSeconds();

    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT);
    viewMatrix = glm::translate(glm::mat4(1.0f), glm::vec3(0.0f, -3.0f, -10.0f)); // move back a bit
    modelMatrix = glm::scale(glm::mat4(1.0f), glm::vec3(modelScale));   // scale model
    modelMatrix = glm::rotate(modelMatrix, rot, glm::vec3(0, 1, 0));
    //modelMatrix = glm::rotate(modelMatrix, 25.0f, glm::vec3(0, 1, 0));


    glm::vec3 lightPosition( 0.0f, -100.0f, 0.0f );

    float lightPositionArray[3];
    lightPositionArray[0] = lightPosition[0];
    lightPositionArray[1] = lightPosition[1];
    lightPositionArray[2] = lightPosition[2];


    shader->bind();

    int projectionMatrixLocation = glGetUniformLocation(shader->id(), "projectionMatrix");
    int viewMatrixLocation = glGetUniformLocation(shader->id(), "viewMatrix");
    int modelMatrixLocation = glGetUniformLocation(shader->id(), "modelMatrix");
    int ambientLocation             = glGetUniformLocation(shader->id(), "ambientColor");
    int diffuseLocation             = glGetUniformLocation(shader->id(), "diffuseColor");
    int specularLocation            = glGetUniformLocation(shader->id(), "specularColor");
    int lightPositionLocation       = glGetUniformLocation(shader->id(), "lightPosition");
    int normalMatrixLocation        = glGetUniformLocation(shader->id(), "normalMatrix");

    glUniformMatrix4fv(projectionMatrixLocation, 1, GL_FALSE, &projectionMatrix[0][0]);
    glUniformMatrix4fv(viewMatrixLocation, 1, GL_FALSE, &viewMatrix[0][0]);
    glUniformMatrix4fv(modelMatrixLocation, 1, GL_FALSE, &modelMatrix[0][0]);
    glUniform3fv(lightPositionLocation, 1, lightPositionArray);

    for (unsigned int i = 0; i < scene->mNumMeshes; i++)
    {
        colorArrayA = new float[3];
        colorArrayD = new float[3];
        colorArrayS = new float[3];

        material = scene->mMaterials[scene->mNumMaterials-1];

        normalArray = new float[scene->mMeshes[i]->mNumVertices * 3];

        unsigned int normalIndex = 0;
        for (unsigned int j = 0; j < scene->mMeshes[i]->mNumVertices * 3; j+=3, normalIndex++)
        {
            normalArray[j] = scene->mMeshes[i]->mNormals[normalIndex].x; // x
            normalArray[j+1] = scene->mMeshes[i]->mNormals[normalIndex].y; // y
            normalArray[j+2] = scene->mMeshes[i]->mNormals[normalIndex].z; // z
        }
        normalIndex = 0;


        glUniformMatrix3fv(normalMatrixLocation, 1, GL_FALSE, normalArray);

        aiColor3D ambient(0.0f, 0.0f, 0.0f);
        material->Get(AI_MATKEY_COLOR_AMBIENT, ambient);

        aiColor3D diffuse(0.0f, 0.0f, 0.0f);
        material->Get(AI_MATKEY_COLOR_DIFFUSE, diffuse);

        aiColor3D specular(0.0f, 0.0f, 0.0f);
        material->Get(AI_MATKEY_COLOR_SPECULAR, specular);


        colorArrayA[0] = ambient.r; colorArrayA[1] = ambient.g; colorArrayA[2] = ambient.b;
        colorArrayD[0] = diffuse.r; colorArrayD[1] = diffuse.g; colorArrayD[2] = diffuse.b;
        colorArrayS[0] = specular.r; colorArrayS[1] = specular.g; colorArrayS[2] = specular.b;

        // bind color for each mesh
        glUniform3fv(ambientLocation, 1, colorArrayA);
        glUniform3fv(diffuseLocation, 1, colorArrayD);
        glUniform3fv(specularLocation, 1, colorArrayS);

        // render all meshes
        glBindVertexArray(vaoID[i]); // bind our VAO
        glDrawElements(GL_TRIANGLES, scene->mMeshes[i]->mNumFaces*3, GL_UNSIGNED_INT, 0);
        glBindVertexArray(0); // unbind our VAO


        delete [] normalArray;

        delete [] colorArrayA;
        delete [] colorArrayD;
        delete [] colorArrayS;
    }

    shader->unbind();

    app->display();

    return;
}

void assimpRenderer::handleEvents()
{
    sf::Event event;

    while (app->pollEvent(event))
    {
        if (event.type == sf::Event::Closed)
        {
            app->close();
        }

        if ((event.type == sf::Event::KeyPressed) && (event.key.code == sf::Keyboard::Escape))
        {
            app->close();
        }

        if (event.type == sf::Event::Resized)
        {
            glViewport(0, 0, event.size.width, event.size.height);
        }
    }

    return;
}

void assimpRenderer::mainLoop(float modelScale)
{
    while (app->isOpen())
    {
        renderScene(modelScale);
        handleEvents();
    }
}

bool assimpRenderer::assImport(const std::string& pFile)
{
    // read the file with some example postprocessing
    scene = importer.ReadFile(pFile,
            aiProcess_CalcTangentSpace      |
            aiProcess_Triangulate           |
            aiProcess_JoinIdenticalVertices |
            aiProcess_SortByPType);

    // if the import failed, report it
    if (!scene)
    {
        std::cerr << "Error: " << importer.GetErrorString() << std::endl;
        return false;
    }

    return true;
}

void assimpRenderer::genVAOs()
{

    int vboIndex = 0;
    for (unsigned int i = 0; i < scene->mNumMeshes; i++, vboIndex+=2)
    {
        mesh = scene->mMeshes[i];
        indexArray = new unsigned int[mesh->mNumFaces * sizeof(unsigned int) * 3];

        // convert assimp faces format to array
        faceIndex = 0;

        for (unsigned int t = 0; t < mesh->mNumFaces; ++t)
        {
            const struct aiFace* face = &mesh->mFaces[t];
            std::memcpy(&indexArray[faceIndex], face->mIndices, sizeof(float) * 3);
            faceIndex += 3;
        }

        // generate VAO
        glGenVertexArrays(1, &vaoID[i]);
        glBindVertexArray(vaoID[i]);

        // generate IBO for faces
        glGenBuffers(1, &iboID[i]);
        glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, iboID[i]);
        glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(GLuint) * mesh->mNumFaces * 3, indexArray, GL_STATIC_DRAW);

        // generate VBO for vertices
        if (mesh->HasPositions())
        {
            glGenBuffers(1, &vboID[vboIndex]);
            glBindBuffer(GL_ARRAY_BUFFER, vboID[vboIndex]);
            glBufferData(GL_ARRAY_BUFFER, mesh->mNumVertices * sizeof(GLfloat) * 3, mesh->mVertices, GL_STATIC_DRAW);
            glEnableVertexAttribArray((GLuint)0);
            glVertexAttribPointer((GLuint)0, 3, GL_FLOAT, GL_FALSE, 0, 0);
        }

        // generate VBO for normals
        if (mesh->HasNormals())
        {
            normalArray = new float[scene->mMeshes[i]->mNumVertices * 3];

            unsigned int normalIndex = 0;
            for (unsigned int j = 0; j < scene->mMeshes[i]->mNumVertices * 3; j+=3, normalIndex++)
            {
                normalArray[j] = scene->mMeshes[i]->mNormals[normalIndex].x; // x
                normalArray[j+1] = scene->mMeshes[i]->mNormals[normalIndex].y; // y
                normalArray[j+2] = scene->mMeshes[i]->mNormals[normalIndex].z; // z
            }
            normalIndex = 0;

            glGenBuffers(1, &vboID[vboIndex+1]);
            glBindBuffer(GL_ARRAY_BUFFER, vboID[vboIndex+1]);
            glBufferData(GL_ARRAY_BUFFER, mesh->mNumVertices * sizeof(GLfloat) * 3, normalArray, GL_STATIC_DRAW);
            glEnableVertexAttribArray((GLuint)1);
            glVertexAttribPointer((GLuint)1, 3, GL_FLOAT, GL_FALSE, 0, 0);

            delete [] normalArray;
        }

        // tex coord stuff goes here

        // unbind buffers
        glBindVertexArray(0);
        glBindBuffer(GL_ARRAY_BUFFER, 0);
        glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);

        delete [] indexArray;
    }
    vboIndex = 0;

    return;
}

}

file: shader.vert

#version 150 core

in vec3 in_Position;
in vec3 in_Normal;

uniform mat4 projectionMatrix;
uniform mat4 viewMatrix;
uniform mat4 modelMatrix;
uniform vec3 lightPosition;
uniform mat3 normalMatrix;

smooth out vec3 vVaryingNormal;
smooth out vec3 vVaryingLightDir;

void main()
{       
    // derive MVP and MV matrices
    mat4 modelViewProjectionMatrix = projectionMatrix * viewMatrix * modelMatrix;
    mat4 modelViewMatrix = viewMatrix * modelMatrix;

    // get surface normal in eye coordinates
    vVaryingNormal = normalMatrix * in_Normal;

    // get vertex position in eye coordinates
    vec4 vPosition4 = modelViewMatrix * vec4(in_Position, 1.0);
    vec3 vPosition3 = vPosition4.xyz / vPosition4.w;

    // get vector to light source
    vVaryingLightDir = normalize(lightPosition - vPosition3);

    // Set the position of the current vertex 
    gl_Position = modelViewProjectionMatrix * vec4(in_Position, 1.0);

}

file: shader.frag

#version 150 core

out vec4 out_Color;

uniform vec3 ambientColor;
uniform vec3 diffuseColor;
uniform vec3 specularColor;

smooth in vec3 vVaryingNormal;
smooth in vec3 vVaryingLightDir;

void main()
{
    // dot product gives us diffuse intensity
    float diff = max(0.0, dot(normalize(vVaryingNormal), normalize(vVaryingLightDir)));

    // multiply intensity by diffuse color, force alpha to 1.0
    out_Color = vec4(diff * diffuseColor, 1.0);

    // add in ambient light
    out_Color += vec4(ambientColor, 1.0);

    // specular light
    vec3 vReflection = normalize(reflect(-normalize(vVaryingLightDir), normalize(vVaryingNormal)));
    float spec = max(0.0, dot(normalize(vVaryingNormal), vReflection));

    if (diff != 0)
    {
        float fSpec = pow(spec, 128.0);
        // Set the output color of our current pixel
        out_Color.rgb += vec3(fSpec, fSpec, fSpec);
    }
}

I know it's a lot to look through, but I'm putting most of the code up so as not to assume where the problem is.

Answer 1

Are you doing the right thing for the normal matrix? This looks quite bizarre to me.

    for (unsigned int j = 0; j < scene->mMeshes[i]->mNumVertices * 3; j+=3, normalIndex++)
    {
        normalArray[j] = scene->mMeshes[i]->mNormals[normalIndex].x; // x
        normalArray[j+1] = scene->mMeshes[i]->mNormals[normalIndex].y; // y
        normalArray[j+2] = scene->mMeshes[i]->mNormals[normalIndex].z; // z
    }
    glUniformMatrix3fv(normalMatrixLocation, 1, GL_FALSE, normalArray);

Why does the normalMatrix have anything to do with the vertices of the mesh? It should be identical to your modelMatrix (provided that you're not doing any non-uniform scaling).