Simple OpenGL-2D Program

Question

I have created mini-opengl program that draw simple chess board on screen by using SFML windows model only. The program runs fine. The main purpose of this program is to keep it as simple as possible and it is not rely on any of OpenGL helper functions like glOrtho or gluOrtho2D for converting coodinates. Also, it is not using any external library for loading textures.

I would like to know how can I improve it?

#include <SFML/OpenGL.hpp>
#include <SFML/Window.hpp>
#include <vector>

int main()
{
    sf::Window window(sf::VideoMode(800, 600), "OpenGL");

    // Set Orthographic
    auto width = static_cast<float>(window.getSize().x);
    auto height = static_cast<float>(window.getSize().y);
    auto a = 2.f / width;
    auto b = 2.f / height;
    std::vector<float> matrix =
    {
         a , 0, 0, 0,
         0, -b, 0, 0,
         0,  0, 1, 0,
        -1,  1, 0, 1
    };

    // Set the projection matrix
    glMatrixMode(GL_PROJECTION);
    glLoadMatrixf(matrix.data());

    // Initialize vertices: 
    std::vector<float> vertices =
    {
        0,     0,
        0,     height,
        width, height,
        width, 0
    };

    // Initialize colors
    std::vector<float> colors =
    {
        1, 0, 0,
        0, 1, 0,
        0, 0, 1,
        0, 1, 1
    };

    // Initialize texture virtice
    std::vector<float> texCoord =
    {
        0, 0,
        0, 1,
        1, 1,
        1, 0
    };

    // Create texture: simple chess board 8x8
    auto numRows = 8u;
    auto numCols = 8u;
    auto character = 172u;
    auto remain = 255u - character;
    std::vector<unsigned char> texture(numCols * numRows);
    for (auto i = 0u; i < texture.size(); ++i)
        texture[i] = ((i + (i / numCols)) % 2) * remain + character;

    // Upload to GPU texture
    unsigned textureID;
    glGenTextures(1, &textureID);
    glBindTexture(GL_TEXTURE_2D, textureID);
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
    glTexImage2D(GL_TEXTURE_2D, 0, GL_LUMINANCE, numCols, numRows, 0, GL_LUMINANCE, GL_UNSIGNED_BYTE, texture.data());

    // Initialize clear colors
    glClearColor(0.f, 0.f, 0.f, 1.f);

    // Activate necessary states
    glEnable(GL_TEXTURE_2D);

    glEnableClientState(GL_VERTEX_ARRAY);
    glEnableClientState(GL_COLOR_ARRAY);
    glEnableClientState(GL_TEXTURE_COORD_ARRAY);

    glVertexPointer(2, GL_FLOAT, 0, vertices.data());
    glColorPointer(3, GL_FLOAT, 0, colors.data());
    glTexCoordPointer(2, GL_FLOAT, 0, texCoord.data());

    while (window.isOpen())
    {
        sf::Event windowEvent;
        while (window.pollEvent(windowEvent))
        {
            if (windowEvent.type == sf::Event::Closed)
                window.close();
        }

        // render
        glClear(GL_COLOR_BUFFER_BIT);

        glDrawArrays(GL_QUADS, 0, 4);

        window.display();
    }
}

Answer 1

This is a pretty straightforward program. Overall, very easy to read, and makes sense. Here are a few things I'd do differently:

Use Types

You have several places where you use an unstructured vector of floats. While that's the formate you need to send to the GPU, you don't have to make it so hard on yourself (and anyone who's going to read your code in the future). You should make a 2D point structure, an RGB color structure, and a matrix class to handle those very different types of data. You might look into glm which has several data types that mimic those used in vertex and fragment shaders.

Or create your own:

struct Point2D {
    float x;
    float y;
};

struct RGBColor {
    float red;
    float green;
    float blue;
};

etc.

Use Functions

You code for creating and uploading the texture is really tangential to the rest of the code. You should put those into either a single other function, or 2 other functions (one to create the data and one to upload it). If it were me, I'd do 2, as I know that I upload a lot of textures when working with OpenGL and the code is going to mostly be the same.

Speaking of creating the texture, your code is difficult to understand. It would be simpler to do something like this:

// Create texture: simple chess board 8x8
const unsigned char kBlack = 0;
const unsigned char kWhite = 0;
auto numRows = 8u;
auto numCols = 8u;
std::vector<unsigned char> texture(numCols * numRows);
for (auto row = 0u; row < numRows; ++row)
{
    for (auto col = 0u; col < numCols; ++col)
    {
        bool evenRow = (row % 2) == 0;
        bool evenCol = (col % 2) == 0;
        if (evenRow == evenCol)
        {
            texture [ row * numCols + col ] = kWhite;
        }
        else
        {
            texture [ row * numCols + col ] = kBlack;
        }
    }
}

If You Set State, Use It

I notice that you're using a color array, but then you upload a texture, and you don't specify GL_MODULATE for the texture parameters. What is the point of setting the vertex colors if you aren't going to use them? (I don't see any shaders, so presumably you aren't using them there, either.) If you aren't going to use the vertex colors, don't waste time uploading them to the card.

Also, when you've completed drawing the arrays, you should disable the client state you set above, as when you inevitably expand this code in the future, those values will need to change for each object you draw. Your draw loop should look something like this:

while (window.isOpen())
{
    sf::Event windowEvent;
    while (window.pollEvent(windowEvent))
    {
        if (windowEvent.type == sf::Event::Closed)
            window.close();
    }

    // render
    glClear(GL_COLOR_BUFFER_BIT);

    glEnableClientState(GL_VERTEX_ARRAY);
    glEnableClientState(GL_COLOR_ARRAY);
    glEnableClientState(GL_TEXTURE_COORD_ARRAY);

    glVertexPointer(2, GL_FLOAT, 0, vertices.data());
    glColorPointer(3, GL_FLOAT, 0, colors.data());
    glTexCoordPointer(2, GL_FLOAT, 0, texCoord.data());

    glDrawArrays(GL_QUADS, 0, 4);

    glDisableClientState(GL_VERTEX_ARRAY);
    glDisableClientState(GL_COLOR_ARRAY);
    glDisableClientState(GL_TEXTURE_COORD_ARRAY);

    window.display();
}

And really all the drawing stuff should be in some sort of drawObjects() function.