# Basic OpenGL drawing shapes

I'm drawing shapes to OpenGL using the standard commands and function calls to OpenGL. Everything works fine, however since this is a learning process I know sometimes what works in theory and using simple basic examples doesnt always work in real time situations so I wanted to ask if this is correct what I am doing here with my drawing commands.

Everything works fine, and draws correctly as expected but I am just curious is it correct way to do things and could i result in problems when drawing far more objects in a real world rendering scene.

Here is the drawing part of the code:

//raw data
GLfloat vec[] = {0.0f, 0.0f,
1.0f, -1.0f,
-1.0f, -1.0f};

GLfloat vec2[] = {0.0f, 1.0f,
1.0f, 0.0f,
-1.0f, 0.0f};

//next step is to upload data to graphics memory

//generating a buffer from openGL
GLuint vbo;
GLuint vao;

glGenBuffers(1, &vbo);

glGenVertexArrays(1, &vao);

//to upload the actual data must make the object active by binding it to a target
glBindBuffer(GL_ARRAY_BUFFER, vbo);
//upload the data of active object to memory
glBufferData(GL_ARRAY_BUFFER, sizeof(vec), vec, GL_STATIC_DRAW);

//first triangle bind and enable
glBindVertexArray(vao);

glEnableVertexAttribArray(0);
glVertexAttribPointer(0,2,GL_FLOAT, GL_FALSE, 0, NULL);

glDrawArrays (GL_TRIANGLES, 0, 3);
glXSwapBuffers ( dpy, glxWin );

sleep(1);

glClear(GL_COLOR_BUFFER_BIT);

glBufferData(GL_ARRAY_BUFFER, sizeof(vec2), vec2, GL_STATIC_DRAW);

glDrawArrays (GL_TRIANGLES, 0, 3);
glXSwapBuffers ( dpy, glxWin );

sleep(1);


So as you can see what i am doing , since the triangles are the same dimensions just different vector positions, i am simply just uploading/transferring the data to the server memory of one object instead of creating whole other object with exactly the same set-up as the first object but different position vectors and i would like to know is this safe to do and practical?

For the most part this looks like a typical way to draw geometry in OpenGL. I see a few things that could be improved.

# Use structures for structured data

Currently you're storing your vertex coordinates in a 1 dimensional array of floats. But that data isn't just 6 floats. It's 3 points. So you should create a point structure:

struct Point2D {
float x;
float y;
};


And your vertex coordinates should be defined using that struct:

struct Point2D triangle1[] = {
{ 0.0f, 0.0f },
{ 1.0f, -1.0f },
{ -1.0f, -1.0f }
};

struct Point2D triangle2[] = {
{ 0.0f, 1.0f },
{ 1.0f, 0.0f },
{ -1.0f, 0.0f }
};


This will make it easier to read and understand. It also makes it much easier to find bugs. If your geometry looks weird, and it turns out you missed an x coordinate in one vertex, good luck figuring out that's the problem with a 1D array of floats. If you miss a coordinate with this struct, you'll get a compiler error.

And since you're working with triangles in this case, you could even make a Triangle2D structure containing 3 Point2Ds. It might be overkill for this simple example, though.

# Use meaningful variable names

Names like vao and vbo tell you that those variables store the names for vertex array objects and vertex buffer objects. But they don't tell you which object in your scene they're used for, or how they differ from any other vertex array or buffer you're going to allocate later in the code. Obviously in this example, they only represent the arrays/buffers for the 2 triangles you're drawing, so suggesting a better name is difficult. But at least something like triArrayObj and triBufferObj would be more clear. In a game, for example, you might keep one set around for the scenery and another for the player's character, and another for an enemy, or something like that. They should be named as such.

The names of vec and vec2 are even worse, though. They're vectors, but vectors of what? As with the VAOs and VBOs, there's no real meaning in this particular code since it's just an example, so I'd at least name them tri1 and tri2 for "Triangle 1" and "Triangle 2".

# Don't use sleep

You should not be calling sleep() in this code. It's not clear what you're trying to accomplish by doing that. It looks like you're drawing a frame with 1 triangle, then clearing it and drawing a frame with another triangle. Normally, you'd have an event driven application that takes input from the user and tells you to draw with some period, so you wouldn't want to use sleep() because you'd slow it down.

# Efficiency

You ask a fair question about whether the way you're doing things is efficient or not. It's a little hard to answer given the simplicity of your example, but here are my thoughts.

In general you want to generate OpenGL objects like vertex array objects and vertex buffer objects infrequently. Allocating memory takes time. You also want to minimize sending data to the GPU. What that means in general is allocate whatever you can upfront and load it onto the GPU as infrequently as possible. In many cases you have a large amount of geometry that won't change, and a little bit of geometry that will change. You might also have a set of textures that are used throughout your rendering. If you can load all your unchanging geometry and unchanging textures onto the card at the start of your app (or assuming it's a game, the start of a level), that will help minimize the overhead of sending data to the GPU.

But you have to be careful. If you load more data onto the GPU than you have memory for, you could get VRAM swapping, or even just rendering errors. It's often the case that all the data you need to draw a scene won't fit on the GPU at once and you have no choice but to upload some of it every frame, or in severe cases, multiple times per frame.

You may find this becomes more difficult than it sounds at first as you attempt to support a wider range of possible GPUs. Your users may have hardware that ranges from extremely low-end to extremely high-end.

• thanks for that very well elaborated response , so my main question was directed about using glBufferData() in the way that i did use it and whether it was safe and correct i.e copying data over data in graphics memory – hopjopper Jan 8 '16 at 2:04
• It's safe to do. Whether it's "correct" or not depends on what you're doing. Generally, if you're going to change data in a buffer like that, you don't set it to GL_STATIC_DRAW, though. You'll normally set it to GL_DYNAMIC_DRAW. That tells the driver that you'll be changing it and it can decide on a better caching policy, for example. – user1118321 Jan 8 '16 at 3:39
• oh okay thanks for explaining that , cheers – hopjopper Jan 8 '16 at 10:15