glDrawElement, why does it cause the program to stop working?

Question

I am getting frustrated about this. I am trying to render a cube with Vertex Buffer Objects and I was learning about Projection so I am trying to make a frame of a cube. However, this code does not work. When I run the program on Code::Blocks, the program stops working.

I tried to find out the reason by commenting out the content of the render() method, and then the program doesn't stop working. So this line of code

glDrawElements(GL_TRIANGLES, m_index->size(), GL_UNSIGNED_INT, 0); // render the cube

may be the source of the problem. But I don't know how to fix this, because this is just what I usually do (I have made similar programs and they worked)

I really really appreciate your help!

ProjectionCube::ProjectionCube()
{
    rotationAngle = 0.0;
}

bool ProjectionCube::initialize()
{
#ifdef _WIN32
    glGenBuffers = (PFNGLGENBUFFERSARBPROC)wglGetProcAddress("glGenBuffers");
    glBindBuffer = (PFNGLBINDBUFFERPROC)wglGetProcAddress("glBindBuffer");
    glBufferData = (PFNGLBUFFERDATAPROC)wglGetProcAddress("glBufferData");
#else
    glGenBuffers = (PFNGLGENBUFFERSARBPROC)glXGetProcAddress((const GLubyte*)"glGenBuffers");
    glBindBuffer = (PFNGLBINDBUFFERPROC)glXGetProcAddress((const GLubyte*)"glBindBuffer");
    glBufferData = (PFNGLBUFFERDATAPROC)glXGetProcAddress((const GLubyte*)"glBufferData");
#endif

    if (!glGenBuffers || !glBindBuffer || !glBufferData)
    {
        std::cerr << "VBOs are not supported by your graphics card" << std::endl;
        return false;
    }

    glEnableClientState(GL_VERTEX_ARRAY);
    glEnableClientState(GL_COLOR_ARRAY);

    initializeVertexBuffers();

    // bind vertex buffer and index buffer
    // set vertex pointer to the buffer
    glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, m_vbos[INDEX_BUFFER]);
    glBindBuffer(GL_ARRAY_BUFFER, m_vbos[VERTEX_BUFFER]);
    glVertexPointer(3, GL_FLOAT, 0, 0); 

    // no color buffer to bind
    return true;
}

void ProjectionCube::initializeVertexBuffers()
{
    const float size = 0.5f;
    m_vertex = getCubeVertices(size);

    for (int i = 0; i < m_vertex->size(); i++) {
        std::cout << m_vertex->at(i) << ", ";
    }
    std::cout << std::endl;

    static const unsigned int index[] = {//using triangles to render
                                  0, 1, 2, 0, 2, 3, //bottom
                                  0, 4, 5, 0, 1, 5, //back
                                  0, 4, 7, 0, 3, 7, //left
                                  1, 5, 6, 1, 2, 6, //right
                                  4, 5, 6, 4, 7, 6, //top
                                  2, 6, 7, 2, 3, 7}; // front

    m_index = new vector<unsigned int>(index, index + sizeof(index) / sizeof(index[0]));

    for (int i = 0; i < m_index->size(); i++) {
        std::cout << m_index->at(i) << ", ";
    }
    std::cout << std::endl;

    glGenBuffers(1, &m_vbos[VERTEX_BUFFER]);
    glBindBuffer(GL_ARRAY_BUFFER, m_vbos[VERTEX_BUFFER]);
    glBufferData(GL_ARRAY_BUFFER, sizeof(GLfloat) * m_vertex->size(),
                 &m_vertex->at(0), GL_STATIC_DRAW);

    glGenBuffers(1, &m_vbos[INDEX_BUFFER]);
    glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, m_vbos[INDEX_BUFFER]);
    glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(unsigned int) * m_index->size(),
                 &m_index->at(0), GL_STATIC_DRAW);
}

void ProjectionCube::render(float m_x, float m_y, float m_z)
{
   glTranslatef(m_x, m_y, m_z); // move to where the cube is located
    //glRotatef(rotationAngle, 0.5f, 0.0f, 0.0f);
   glDrawElements(GL_TRIANGLES, m_index->size(), GL_UNSIGNED_INT, 0); // render the cube
}

void ProjectionCube::animate(float dt)
{
    const float SPEED = 15.0f;
    rotationAngle += SPEED * dt;

    if (rotationAngle >= 360 || rotationAngle <= 0) {
        rotationAngle = -rotationAngle;
    }
}

vector<GLfloat>* ProjectionCube::getCubeVertices(float r)
{
    static const GLfloat vertices[] = {//bottom square
                                       -r, -r, -r,
                                       r, -r, -r,
                                       r, -r, r,
                                       -r, -r, r,
                                       //top square
                                       -r, r, -r,
                                       r, r, -r,
                                       r, r, r,
                                       -r, r, r,};
    vector<GLfloat>* result = new vector<GLfloat>(vertices, vertices + sizeof(vertices) / sizeof(vertices[0]));
    return result;
}

Answer 1

Since you didn't post when render is called, all I can do is advise you into not using new vector. There is no need for it as far as I can see.

Since your error occurs when you use m_index in the render function, and assuming m_index is a pointer to a vector, then there is no need for it to be a pointer (assuming it's a member variable of ProjectionCube).

There are two issues with new vector. Why does your program dynamically allocate a in the getCubeVertices function? The following removes the dynamic allocation:

vector<GLfloat> ProjectionCube::getCubeVertices(float r)
{
    static const GLfloat vertices[] = {//bottom square
                                       -r, -r, -r,
                                       r, -r, -r,
                                       r, -r, r,
                                       -r, -r, r,
                                       //top square
                                       -r, r, -r,
                                       r, r, -r,
                                       r, r, r,
                                       -r, r, r,};
   return vector<GLfloat>(vertices, vertices + sizeof(vertices) / sizeof(vertices[0]));
}

Then in InitializeVertexBuffers(), the m_vertex member variable is no longer a pointer, but an object:

std::vector<GLfloat> m_vertex;  // assuming these are member variables in your class
std::vector<unsigned int> m_index;
//...
void ProjectionCube::initializeVertexBuffers()
{
    const float size = 0.5f;
    m_vertex = getCubeVertices(size);
    //...
    m_index = vector<unsigned int>(index, index + sizeof(index) / sizeof(index[0]));

Again, no need for new vector. You now use m_index. and m_vertex..

Now what does this buy you, as opposed to what you were doing before? Well, now you're guaranteed that m_index is valid when render is called. There is no way for m_index to have been deallocated, or the pointer corrupted, etc. since m_index is no longer a pointer.

You also rid yourself of a potential memory leak using the above approach.