2D Points painting program in C++

Question

I've written a simple program in C++ to practice OpenGL. It takes a set of numbers through stdin and then plots it in Decartes coordinate system with x for position in input, and y for the value itself.

Example

Input:

1 4 9 16 25 36 49 64 81 100 121 144 169 196 225 256 289 324 361 400

Points to paint:

(0, 1) (1, 4) (2, 9) (3, 16) ... (19, 400)

Code

Mind the headers when compiling.

main.cpp

#include <cstdlib>
#include <cmath>
#include <iostream>

#include <string>
#include <vector>

#include <GLUT/glut.h>

typedef float real_t;

std::vector <real_t> DATA;

real_t
    size_x = 800,
    size_y = 800,

    width(size_x * 4),
    height(size_y * 4),

    change = 0.05,

    BOLD = 3,
    bold_x(BOLD * width / size_x),
    bold_y(BOLD * height / size_y),

    shift_x = 0,
    shift_y = 0;

void DrawText(real_t x, real_t y, const char *string) {
    glRasterPos2f(x, y);
    for (const char *c = string; *c != '\0'; ++c)
        glutBitmapCharacter(GLUT_BITMAP_TIMES_ROMAN_24, *c);
}

inline void DrawPoint(real_t &x, real_t &y) {
    real_t
        point_x = 2 * x - shift_x,
        point_y = 2 * y - shift_y;
    if(
            point_x >= width
        ||
            point_x < 0
        ||
            point_y >= height
        ||
            point_y < 0
    )
        return;
    glPushMatrix();                                         
    glTranslatef(point_x, point_y, 0);                      
    glBegin(GL_QUADS);                                      
        glVertex3f(  bold_x,  bold_y, 0.0);             
        glVertex3f(  bold_x, -bold_y, 0.0);             
        glVertex3f( -bold_x, -bold_y, 0.0);             
        glVertex3f( -bold_x,  bold_y, 0.0);             
    glEnd();
    glPopMatrix();
}

void Display() {
    glLoadIdentity();                                       
    glClear(GL_COLOR_BUFFER_BIT);                           
    glMatrixMode(GL_PROJECTION);                            
    glOrtho(0, width, 0, height, 1, -1);                    

    glColor3f(0.8f,0.6f,0.0f);
    DrawText(width - 230 * width/size_x, height - 30 * height/size_y, std::string(std::string() + "Width:  " + std::to_string(width)).c_str());
    DrawText(width - 230 * width/size_x, height - 60 * height/size_y, std::string(std::string() + "Height: " + std::to_string(height)).c_str());

    glColor3f(0.0f,1.0f,0.0f);
    for(real_t i = 0; i < DATA.size(); ++i) {
        real_t
            *x = &i,
            *y = &DATA[i];
        DrawPoint(*x, *y);
    }

    glColor3f(1.0f,1.0f,0.0f);

    glPushMatrix();
    glTranslatef(0, -shift_y, 0);
    glBegin(GL_QUADS);
        glVertex3f( 0,      bold_y / 2  , 0.0);
        glVertex3f( 0,     -bold_y / 2  , 0.0);
        glVertex3f( width, -bold_y / 2  , 0.0);
        glVertex3f( width,  bold_y / 2  , 0.0);
    glEnd();
    glPopMatrix();


    glPushMatrix();
    glTranslatef(-shift_x, 0, 0);
    glBegin(GL_QUADS);
        glVertex3f( -bold_x / 2, 0      , 0.0);
        glVertex3f(  bold_x / 2, 0      , 0.0);
        glVertex3f(  bold_x / 2, height , 0.0);
        glVertex3f( -bold_x / 2, height , 0.0);
    glEnd();
    glPopMatrix();

    glutSwapBuffers();
}

void Reshape(int new_size_x, int new_size_y) {
    width *= real_t(new_size_x) / size_x;
    height *= real_t(new_size_y) / size_y;

    size_x = new_size_x;
    size_y = new_size_y;

    bold_x = BOLD * width/size_x;
    bold_y = BOLD * height/size_y;

    glViewport(0, 0, size_x, size_y);
    glutPostRedisplay();
}

void Keyboard(unsigned char key, int x, int y) {
    real_t
        increase = 1 + change,
        decrease = 1 / increase;
    switch(key) {
        case  27:
        case 'q':
        case 'Q':
            exit(0);
            break;
        case 'm':
            width   *= decrease;
            height  *= decrease;
            shift_x *= decrease;
            shift_y *= decrease;
            break;
        case 'M':
            width   *= increase;
            height  *= increase;
            shift_x *= increase;
            shift_y *= increase;
            break;
        case 'b':
            BOLD    *= decrease;
            break;
        case 'B':
            BOLD    *= increase;
            break;
        case 't':
            width   *= decrease;
            shift_x *= decrease;
            break;
        case 'T':
            width   *= increase;
            shift_x *= increase;
            break;
        case 'y':
            height  *= decrease;
            shift_y *= decrease;
            break;
        case 'Y':
            height  *= increase;
            shift_y *= increase;
            break;
        case 'w':
            shift_y += height * change;
            break;
        case 's':
            shift_y -= height * change;
            break;
        case 'a':
            shift_x -= width * change;
            break;
        case 'd':
            shift_x += width * change;
            break;
    }
    bold_x = BOLD * width/size_x;
    bold_y = BOLD * height/size_y;
    Display();
}

void Special(int key, int x, int y) {
    switch(key) {
        case 100 :
            Keyboard('a', x, y);
            break;
        case 101 :
            Keyboard('w', x, y);
            break;
        case 102 :
            Keyboard('d', x, y);
            break;
        case 103 :
            Keyboard('s', x, y);
            break;
    }
}

template <class anything_t>
void fill_data(std::vector <anything_t> &data) {
    real_t value;
    while(std::cin >> value)
        data.push_back(value);
}

int main(int argc, char **argv) {
    fill_data(DATA);
    glutInit(&argc, argv);      
    glutInitDisplayMode(GLUT_DOUBLE | GLUT_RGBA | GLUT_DEPTH);  
    glutInitWindowSize(size_x, size_y);
    glutCreateWindow("gl_world");

    glEnable(GL_BLEND);
    glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);

    glutDisplayFunc(Display);
    glutReshapeFunc(Reshape);
    glutKeyboardFunc(Keyboard);
    glutSpecialFunc(Special);

    Display();

    glutMainLoop();

    return 0;
}

Could you, please, help me to improve it's readability, performance and style? What are the better practices?

glampert · Accepted Answer · 2015-09-23 01:58:51Z

First, a note on legacy OpenGL

This OpenGL+GLUT API that you're using is the legacy, AKA fixed-function, OpenGL. It is an alright option for learning the basics of 2D/3D hardware accelerated graphics, but... nobody uses it anymore, that's why it's called legacy :P. Graphics hardware has evolved A LOT since the first release of OpenGL, some 20-ish years ago, so it is only natural that the API has evolved and changed a lot in the process too. Todays graphics take advantage of the so called "programable pipeline", meaning you actually write programs in a shading language that run directly on the GPU.

Once you get more or less familiar with the basics, I suggest that you take the leap to modern GL at once (aim OpenGL version 3 and above), since there's not much point in learning the finer aspects of an outdated technology (apart from retro-programming exercises, of course). At first you might find it a bit daunting to see the amount of extra work needed to get a simple triangle to draw on screen with OpenGL 3+, and that's the way it is. GL has become a much lower-level API for the sake of perfomance. That's not a bad thing really, because there are dozens of high-level libraries built on top of it to choose from if you just want some quick 2D/3D drawing, leaving GL for the high-perf, low-level crew. Nevertheless, I suggest you do take a look at a couple tutorials of modern OpenGL to get a feel for it and better understand the 3D pipeline, even if in the end you just want something simple and easy to set up and switch to some high-level framework.

Code Review

It would be nice to use a class, even if this is a main.cpp-only kind of program. That avoids the bad habit of global variables and keeps the code (hopefully) better organized.

DrawPoint:

It takes x and y by reference, but that's not necessary, since the function doesn't change those parameters.

Also this formatting is not very practical:

if(
        point_x >= width
    ||
        point_x < 0
    ||
        point_y >= height
    ||
        point_y < 0
)

That statement isn't even that long, it could be all in the same line, or breaking once at most.

All that stringification here is only creating temporary instances of strings that get discarded right away.

DrawText(width - 230 * width/size_x, height - 30 * height/size_y, std::string(std::string() + "Width:  " + std::to_string(width)).c_str());

This should be enough:

DrawText(width - 230 * width/size_x, 
         height - 30 * height/size_y, 
        ("Width:  " + std::to_string(width)).c_str());

Also, that's one place where breaking the lines actually helps readability, unlike the if previously mentioned.

By-the-way, why not just take a const std::string & in DrawText? Both usage instances of that function are dealing with C++ strings, so the char * parameter seems inadequate here.

Not much reason to make fill_data a template function. In the body, you read a real_t value, so template type anything_t has to be implicitly convertible to real_t, plus, in the only usage case we have a std::vector<real_t>, so just keep it simple as a non-template function taking a ref to std::vector<real_t>.

Be consistent with your naming conventions. You have variables named using snake_case, but also ALL_UPPERCASE. The uppercase notation is usually associated with macro constants. Your functions are all MultiCase, with the exception of fill_data. A consistent naming convention will help you and readers of the code to remember names more easily.

Thanks for your review! This will definitely improve either my sense of code and my sense of technology. There are only few things which are debatable, could you, please, make them more clear for me? 1) Isn't it a good practice to make const real_t &x instead of const real_t x in DrawPoint? So that x always takes similar number of bytes whereas in the second case it takes sizeof(real_t), which may vary. 2) Doesn't making a class/struct in such a small program make it slightly incomprehensible and ugly? I mean something like glVertex3f(0, constants::bold['y'], 0.0). — theoden8, Sep 23, 2015 at 15:08
@theoden, For native types that fit in a register (int, float, etc) there's no gain in passing by const reference or pointer, it might actually be slower at the instruction level due to pointer dereferences. Using const ref is usually the default when dealing with objects (classes/structs), because then you do want to avoid unnecessary copies. But even this has changed a bit with C++11 and moveable types. A good thread to read about it: stackoverflow.com/questions/7592630/… — glampert, Sep 23, 2015 at 15:17
@theoden, In your program, it wouldn't make a huge difference using a class right now, but avoiding the global data is always good, in case you expand that code into a couple more files. And naturally, the variables would be members of the class, so there would be little change in the way you write the code, with the exception that everything is nicely contained into an object. My understanding is that well defined dependencies actually make for more comprehensible code instead! — glampert, Sep 23, 2015 at 15:18

Stack Exchange Network

current community

your communities

more stack exchange communities

2D Points painting program in C++

Example

Code

main.cpp

1 Answer 1

First, a note on legacy OpenGL

Code Review

Your Answer

Not the answer you're looking for? Browse other questions tagged c++ opengl or ask your own question.

Hot Network Questions

2D Points painting program in C++

Example

Code

main.cpp

1 Answer 1

First, a note on legacy OpenGL

Code Review

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged c++ opengl or ask your own question.

Related

Hot Network Questions