OpenGL program / animation

I have been playing around with OpenGL and created a fairly basic 2D animation was just hoping someone could take a look and give me some pointers.

Main.cpp:

```
#include "glut.h"
#include
#include

int bc = 0.4; //building color
int frame=0;

class points{
public:
float x, y;
};
points startLine[2] = {{300,800},{300,800}};

points endLine[2] = {{250,170},{250,170}};

points tweenLine[4];

int numpoints=2;

float proportion = 0.0; //helicopter scene

float lScale = 0.0; //line scale
float fScale = 0.5;
float bScale = 0.5;
float bTrans = 0.0;
int flag = 0;

void tween(points source[10], points destination[10], double proportion, points tweenLine[10])
{

for( int i = 0; i 0.5) proportion +=0.01;
if(proportion >0.8) proportion =0.8;
}

void window()
{
glColor3f(0.9, 0.9, 0.0);
glBegin(GL_POLYGON);
glVertex2i(330, 170);
glVertex2i(340, 170);
glVertex2i(340, 150);
glVertex2i(330, 150);
glEnd();
}
void backdrop()
{
// Back strip
glColor3f(bc, bc, bc);
glBegin(GL_POLYGON);
glVertex2i(0, 35);
glVertex2i(900, 35);
glVertex2i(900, 0);
glVertex2i(0, 0);
glEnd();
//
glColor3f(bc, bc, bc);
glBegin(GL_POLYGON);
glVertex2i(0, 60);
glVertex2i(40, 70);
glVertex2i(40, 0);
glVertex2i(0, 0);
glEnd();

//building
glColor3f(bc, bc, bc);
glBegin(GL_POLYGON);
glVertex2i(70, 90);
glVertex2i(100, 90);
glVertex2i(100, 0);
glVertex2i(70, 0);
glEnd();

//building 1
glColor3f(bc, bc, bc);
glBegin(GL_POLYGON);
glVertex2i(50, 110);
glVertex2i(70, 110);
glVertex2i(70, 0);
glVertex2i(50, 0);
glEnd();

//building 3
glColor3f(bc, bc, bc);
glBegin(GL_POLYGON);
glVertex2i(110, 160);
glVertex2i(180, 160);
glVertex2i(180, 0);
glVertex2i(110, 0);
glEnd();

//building 3 roof
glColor3f(bc, bc, bc);
glBegin(GL_POLYGON);
glVertex2i(110, 160);
glVertex2i(180, 160);
glVertex2i(145, 180

Was just hoping someone could take a look and give me some pointers.

Sure, there you go:

0x7fff52ce196c
0x7fff52ce1960
0x7fff52ce195c
0x7fff52ce1950

:P

If you got past the lame joke, here are some suggestions that can improve your code:

Avoid global mutable data

It is tempting to just drop a few globals on such a small single-source-file program, but that's a weak escuse for doing so. Passing your data as function arguments is no trouble at all and has so many benefits over global variables.

Function parameters give you explicit dependencies, and that makes it so much easier reasoning about the code. Consider making those globals at the top level local to each function that uses them, passing as parameters where appropriate.

You can also group related data to make things simpler, for instance, you could have a Line structure:

struct Line {
    Point start;
    Point end;
};

Speaking of structures...

Use struct for "dumb" data aggregates

The usual convention in C++ (note that this is not a language requirement, just a convention for programmers) is to use class for "smart" types, things with methods and private data, while struct is used for simple data aggregates with public fields.

My suggestion is to make your point a simple struct for that reason. I'd also suggest using a capitalized first letter for Point, that's a very common naming convention for user-defined types, so you can for example declare Point point{};.

Don't Repeat Yourself!

This block of code is repeated dozens of times, only changing the parameters:

glColor3f(bc, bc, bc);
glBegin(GL_POLYGON);
glVertex2i(0, 35);
glVertex2i(900, 35);
glVertex2i(900, 0);
glVertex2i(0, 0);
glEnd();

Code duplication is one of your worst enemies, fight it by defining reusable functions:

struct Color {
    float r, g, b;
};

struct Rectangle {
    int x, y;
    int width, height;
};

void drawGLRectangle(const Color& color, const Rectangle& rect)
{
    glColor3f(color.r, color.g, color.b);
    glBegin(GL_QUADS);
    glVertex2i(rect.x, rect.y);
    glVertex2i(rect.x, rect.y + rect.height);
    glVertex2i(rect.x + rect.width, rect.y + rect.height);
    glVertex2i(rect.x + rect.width, rect.y);
    glEnd();
}

Also note how I use structures to group related variables. Passing a Rectangle and a Color conveys a lot more meaning than passing a bunch of floats and ints, plus it makes a lot harder to mismatch the parameters, since a Color doesn't convert to a Rectangle.

Side Note: I think you can use GL_QUADS for glBegin(), since you're drawing a simple rectangle.

Use more named constants

You have a bunch of magic numbers in your code, which I have no idea of the meaning. You will probably also forget yourself why a particular value was used here-and-there six months from now, so replace as many of them as you can with named constants that describe what that value is meant to represent and why it was chosen. Where you think a constant is not necessary, at least provide a comment.

Use const pointer where appropriate

When taking a read-only parameter by pointer or reference, always use const to clearly indicate that and disallow modification:

//                  V---- here
void drawBitmapText(const char* string, float x, float y, float z) 
{
    glRasterPos3f(x, y, z);

    //   V---- and here
    for (const char* c = string; *c != '\0'; c++) 
    {
        glutBitmapCharacter(GLUT_BITMAP_TIMES_ROMAN_10, *c);
    }
}

Fix your indentation

Your code is not very consistently indented and this hurts readability. Manually indenting code is a thing of the past, install a formatting tool like Clang-Format in your editor, it works wonders!

Classes can help you better organize your program

Lastly a suggestion: Take a look at classes and some concepts of Object Oriented Programming (OOP). It might help you better organize your code in the future. You can start from here.

StackExchange Code Review Q#117600, answer score: 4