Search code examples
c++openglglfwglew

How to use GL_TRIANGLE_FAN to draw a circle in OpenGL?


Seasons Greetings everyone! I adapted the code from this tutorial to support using a VAO/VBO, but now I get this:

weirdest circle ever

Instead of a nice round circle. Here's the code:

#define GLEW_STATIC

#include <GLEW/glew.h>
#include <GLFW/glfw3.h>

#include <corecrt_math_defines.h>
#include <cmath>
#include <vector>

#define SCREEN_WIDTH 3000
#define SCREEN_HEIGHT 1900

void drawPolygon(GLuint& vao, GLuint& vbo, GLfloat x,
    GLfloat y, GLdouble radius, GLint numberOfSides);

int main(void) {
    if (!glfwInit()) {
        return -1;
    }

    GLFWwindow* window = glfwCreateWindow(SCREEN_WIDTH,
        SCREEN_HEIGHT, "Hello World", NULL, NULL);
    if (!window) {
        glfwTerminate();
        return -1;
    }

    glfwMakeContextCurrent(window);

    glewExperimental = GL_TRUE;
    glewInit();
    glGetError();

    glViewport(0.0f, 0.0f, SCREEN_WIDTH, SCREEN_HEIGHT);

    GLuint vao, vbo;
    glGenVertexArrays(1, &vao);
    glGenBuffers(1, &vbo);

    glMatrixMode(GL_PROJECTION);
    glLoadIdentity();
    glOrtho(0, SCREEN_WIDTH, 0, SCREEN_HEIGHT, -1, 1);

    while (!glfwWindowShouldClose(window)) {
        glClear(GL_COLOR_BUFFER_BIT);
        drawPolygon(vao, vbo, SCREEN_WIDTH / 2,
            SCREEN_HEIGHT / 2, 250.0f, 50);
        glfwSwapBuffers(window);
        glfwPollEvents();
    }

    glfwTerminate();
    return 0;
}

void drawPolygon(GLuint& vao, GLuint& vbo, GLfloat x,
    GLfloat y, GLdouble radius, GLint numberOfSides) {
    int numVertices = numberOfSides + 2;

    GLdouble twicePi = 2.0f * M_PI;

    vector<GLdouble> circleVerticesX;
    vector<GLdouble> circleVerticesY;

    circleVerticesX.push_back(x);
    circleVerticesY.push_back(y);

    for (int i = 1; i < numVertices; i++) {
        circleVerticesX.push_back(x + (radius *
            cos(i * twicePi / numberOfSides)));
        circleVerticesY.push_back(y + (radius *
            sin(i * twicePi / numberOfSides)));
    }

    vector<GLdouble> vertices;
    for (int i = 0; i < numVertices; i++) {
        vertices.push_back(circleVerticesX[i]);
        vertices.push_back(circleVerticesY[i]);
    }

    glBindBuffer(GL_ARRAY_BUFFER, vbo);
    glBufferData(GL_ARRAY_BUFFER, numVertices * sizeof(
        GLdouble), vertices.data(), GL_STATIC_DRAW);
    glBindVertexArray(vao);

    glEnableVertexAttribArray(0);
    glVertexAttribPointer(0, 2, GL_DOUBLE, GL_FALSE,
        2 * sizeof(GLdouble), (void*)0);

    glDrawArrays(GL_TRIANGLE_FAN, 0, 27);

    glBindBuffer(GL_ARRAY_BUFFER, 0);
    glBindVertexArray(0);
}

What the heck have I done wrong?! Using the original code works, so I am absolutely baffled by this ridiculous result! MTIA to anyone who can help :-)


Solution

  • The size of your VBO is numVertices * sizeof(GLdouble), which is half the actual size of your vertex data (there is an x and a y component for each vertex). Thus, you end up drawing twice as many vertices as your VBO actually has vertex data for. Reading out of bounds of your VBO seems to result in just zeroes in your OpenGL implementation, which is why all vertices of the lower half of your circle are just the bottom left corner (this is not guaranteed unless you explicitly enable robust buffer access, just what your driver and GPU seem to be doing anyways)…

    couple of notes:

    • You generally don't want to use double unless you need it. double takes twice the memory bandwidth, and arithmetic on double is generally at least a bit slower than float (yes, even on an x86 CPU since floating point arithmetic is not really done using the x87 FPU anymore nowadays). GPUs in particular are built for float arithmetic. Especially on consumer GPUs, double arithmetic is significantly (an order of magnitude) slower than float.
    • Why not simply push the vertex data directly into vertices rather than first into circleVerticesX and circleVerticesY and then copying it over into vertices from there?
    • You know exactly how many vertices are going to be generated. Thus, there's no need to dynamically grow your vertex container in the loop that generates the coordinates (among other things, the .push_back() will almost certainly prevent vectorization of the loop). I would suggest to at least .reserve() the corresponding number of elements (assuming this is an std::vector) before entering the loop. Personally, I would just allocate an array of the appropriate, fixed size via

      auto vertex_data = std::unique_ptr<GLfloat[]> { new GLfloat[numVertices * 2] };
      

      in this case.

    • You don't actually need a center point. Since a circle is a convex shape, you can simply use one of the points on the circle as the central vertex of your fan.
    • This is not necessarily the most efficient way to draw a circle (a lot of long, thin triangles; more on that here)
    • You probably don't want to generate and upload your vertex data again and again every frame unless something about it changes.
    • Apart from that, you will probably want to make your glDrawArrays call draw the actual number of vertices rather than just always 27