1
\$\begingroup\$

I've been trying to make refactor really simple 2d sprite engine in OpenGL. As a start, I'm trying to use instanced rendering to render several copies of the same sprite in a square formation across my screen. The code works fine but it is quite slow; I am getting about 20 frames a second when rendering 25600 copies. Really would apppreciate any insight into how to improve it.

main.cpp:

int instances = 25600;
 float* data = new float[instances*sprite.getFloats()]; //sprite.getFloats just returns how much data each sprite pushes to the pipeline
 while (!quit)
 {
     glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);

     for (int i = 0; i < instances; ++i)
     {
         sprite.loadData(data,SpriteParameter(glm::vec4(i%(screenWidth/dimen)*dimen,i/(screenWidth/dimen)*dimen,dimen,dimen)),i*sprite.getFloats()); //this part fills an array of floats with transfomration data
     }

     sprite.draw(RenderProgram::basicProgram,data,instances); //now we actually draw the sprites in one go

     SDL_GL_SwapWindow(window);
 }

The main drawing loop is pretty simple. I'm not sure there's much room for optimization here.

render.h:

struct SpriteParameter //stores a bunch of information regarding how to render the sprite
{
     glm::vec4 rect = {0,0,0,0};
    float radians = 0;
    RenderEffect effect = NONE; //effects to do. (mirror, flip, etc)
    glm::vec4 tint = {1,1,1,1};
    float z = 0;
    glm::vec4 portion = {0,0,1,1};
};

SpriteParameter is a class that represents a bunch of effects I can use to change how my sprite is rendered, like what angle to rotate it at, what tint I should give it, as well as the position and dimensions it should be rendered at (rect). The "rect" field is the only one that's really of relevance in my code right now. I'm not really sure how else to pass in a bunch of rendering instructions to a function, so I've been using SpriteParameter for years now. Open to new suggestions

render.cpp:

void Sprite::loadData(GLfloat* data, const SpriteParameter& parameter, int index) //given a sprite parameter, write it's data into an array
{
    if (data!= nullptr)
    {
            glm::mat4 matt = glm::mat4(1.0f); //matt represents the transformation matrix
            matt = glm::translate(matt,{parameter.rect.x + (parameter.rect.z)/2,parameter.rect.y + (parameter.rect.a)/2,0}); 
            matt = glm::rotate(matt, parameter.radians, glm::vec3(0,0,1));
            matt = glm::scale(matt, {parameter.rect.z/2, parameter.rect.a/2,1});
            for (int j = 0; j < 16; j++)
            {
                data[j+index] = matt[j/4][j%4]; //copy matrix
            }
            data[index + 16]= parameter.effect; //load the rest of the data into our array
            data[index + 16 + 1] = parameter.tint.x;
            data[index + 16 + 2] =  parameter.tint.y;
            data[index + 16 + 3] = parameter.tint.z;
            data[index + 16 + 4] = parameter.tint.a;
            data[index + 20 + 1] = parameter.z;
            data[index + 20 + 2] = parameter.portion.x;
            data[index + 20 + 3] = parameter.portion.y;
            data[index + 20 + 4] = parameter.portion.z;
            data[index + 20 + 5] = parameter.portion.a;

        }
        else
        {
            throw new std::invalid_argument("null buffer");
        }
    }
void Sprite::draw(RenderProgram& program, GLfloat* data, int instances) //once we've written the data with Sprite::loadData, we draw it with Sprite::draw
{
    glBindVertexArray(VAO);
    glBindTexture(GL_TEXTURE_2D,texture);
    glBindBuffer(GL_ARRAY_BUFFER,modVBO);
    GLsizei vec4Size = 4*floatSize;
    int stride = floatSize*floats;
    glBufferData(GL_ARRAY_BUFFER,stride*instances,data,GL_DYNAMIC_DRAW);
    glVertexAttribPointer(3, 4, GL_FLOAT, GL_FALSE, stride, (void*)0); //3-6 inclusive are the transformation matrix
    glVertexAttribPointer(4, 4, GL_FLOAT, GL_FALSE, stride, (void*)(vec4Size));
    glVertexAttribPointer(5, 4, GL_FLOAT, GL_FALSE, stride, (void*)(2 * vec4Size));
    glVertexAttribPointer(6, 4, GL_FLOAT, GL_FALSE, stride, (void*)(3 * vec4Size));
    glVertexAttribPointer(7, 1, GL_FLOAT, GL_FALSE, stride, (void*)(4*vec4Size)); //effect
    glVertexAttribPointer(8, 4, GL_FLOAT, GL_FALSE, stride, (void*)(4*vec4Size + floatSize)); //color
    glVertexAttribPointer(9, 1,GL_FLOAT, GL_FALSE, stride, (void*)((floats-5)*floatSize)); //z
    glVertexAttribPointer(10, 4, GL_FLOAT, GL_FALSE, stride, (void*)((floats-4)*floatSize)); //portion
    glEnableVertexAttribArray(3);
    glEnableVertexAttribArray(4);
    glEnableVertexAttribArray(5);
    glEnableVertexAttribArray(6);
    glEnableVertexAttribArray(7);
    glEnableVertexAttribArray(8);
    glEnableVertexAttribArray(9);
    glEnableVertexAttribArray(10);

    glVertexAttribDivisor(3, 1);
    glVertexAttribDivisor(4, 1);
    glVertexAttribDivisor(5, 1);
    glVertexAttribDivisor(6, 1);
    glVertexAttribDivisor(7, 1);
    glVertexAttribDivisor(8, 1);
    glVertexAttribDivisor(9, 1);
    glVertexAttribDivisor(10, 1);

    glUseProgram(basicProgram);
    glDrawElementsInstanced(GL_TRIANGLES,6,GL_UNSIGNED_INT,indices,instances);
    glBindVertexArray(0);
    glBindBuffer(GL_ARRAY_BUFFER,0);
}

Sprite class loads data into an array and then ships it off to the rendering pipeline.

That's pretty much it. From my tests, the main bottleneck is with Sprite::loadData, but I'm not really sure how to make that any faster since all it's doing is writing data to an array.

Any help would be very appreciated!

\$\endgroup\$
1
  • \$\begingroup\$ Welcome to Code Review! You'll receive better reviews if you show a complete example. For example, I recommend that you edit to show the necessary #include lines, and a main() that shows how to call your function. It can really help reviewers if they are able to compile and run your program. \$\endgroup\$ Jan 7, 2023 at 13:11

2 Answers 2

1
\$\begingroup\$

Expected performance

The code works fine but it is quite slow; I am getting about 20 frames a second when rendering 25600 copies. Really would apppreciate any insight into how to improve it.

20 fps is of course low, you want 60 fps or more to get fluid motion on screen. But it's instructive to ask yourself: what performance did you expect? That's not the same as the performance you want.

If you have 25600 objects and 20 fps, then that's 512000 objects per second to handle. Assuming roughly an 1 GHz CPU (it's probably the right order of magnitude), you have \$10^9 / 25600 / 20 \approx 2000\$ cycles per object to do whatever needs to be done. 2000 cycles seems like a lot, but let's look at what you are doing per object:

  • You create a SpriteParameter, and initialize it with a glm::vec4 that has 4 division operations in it. Divisions are slow; even if the throughput of 32-bit integer divisions on the latest CPUs is now just a handful of cycles, it still has tens of cycles of latency.
  • In loadData() you do 3 operations on a 4 by 4 matrix. That's actually a lot of floating point operations, especially because of glm::rotate(), which uses trigonometric functions behind the scenes. These operations together easily use hundreds of cycles.
  • A lot of data is being generated: at least 26 floats, or 104 bytes. Remember, this needs to multiplied by 512000 to get the bandwidth per second: about 507 MB/s. And then it has to be read by the GPU as well, so you have to double that number to 1014 MB/s.

Did you compile your code with optimizations enabled? If not, then that would explain a lot. If you are running on an older CPU or on a laptop or mobile phone, then even with compiler optimizations this might explain the framerate you see.

Move more work into the vertex shader

You are doing a lot of work on the CPU that could be done by the GPU. If you know you are CPU-bound, then that is what I would try to do first. Consider that the GPU can do all these matrix transformations for you. Also, when doing instanced rendering, you can use gl_InstanceID in the vertex shader to know which instance you are rending. Thus, it can even calculate the rect.

If you have certain effects, like tint varying gradually based on position and time, you can also let the vertex shader calculate that based on gl_InstanceID, and pass the time as a uniform to the shader.

Ideally, you don't have data[]; you just pass the minimum amount of uniforms to the shader to have it calculate everything you want, so the amount of calculations necessary and the amount of bandwidth needed is minimal.

If you still need per-instance data that cannot be calculated per-frame on the GPU, then at least try to minimize the amount of data that the CPU has to generate and pass to the shader.

What if you are GPU-bound?

The above assumes the bottleneck is the CPU. It could also be that it is the GPU that is the bottleneck. How big are the sprites? How many fragments does it need to render per second? How complex is the fragment shader? You could do some rough estimations again to see if your GPU can actually handle the load.

Use a profiling tool

I recommend that you use a profiling tool to figure out exactly where the bottlenecks are in your code. Linux perf is a good way to do this, assuming you are running on Linux of course. If the bottleneck is the GPU, then you might have to look for profiling tools from your GPU's vendor.

\$\endgroup\$
3
  • \$\begingroup\$ Wow that is a very comprehensive answer, thank you! I think part of my problem is that I'm going for a very "one size fits all" solution. The reason I don't calculate the position in the GPU and the reason why I pass in so much data per instance is because in the future I expect this engine to handle several objects that might all require wildly different orientations, transformations, tints, etc. But from what I've seen with a lot of similar questions on Stack Exchange, it's that OpenGL doesn't really do that and its power is in being very specific. \$\endgroup\$
    – Ruglord
    Jan 7, 2023 at 18:19
  • \$\begingroup\$ Do you have any recommendations for how I would handle transforming each sprite by a different, random amount? I feel like for that I would have to pass in one matrix per instance. \$\endgroup\$
    – Ruglord
    Jan 7, 2023 at 18:20
  • \$\begingroup\$ You could implement a simple hash function in the shader and use hash(gl_InstanceID) to get a random value per instance. If you want to also randomize it every frame, add a frame counter or time stamp (passed as a uniform) to the hash. If you still want to pass data from the CPU, then consider how much you need. Do you just need scale and rotation? That's just two floats instead of a mat4 that's 16 floats. \$\endgroup\$
    – G. Sliepen
    Jan 7, 2023 at 20:38
1
\$\begingroup\$

One additional point about your draw call:

glBindVertexArray(VAO);
glBindTexture(GL_TEXTURE_2D,texture);
glBindBuffer(GL_ARRAY_BUFFER,modVBO);
GLsizei vec4Size = 4*floatSize;
int stride = floatSize*floats;
glBufferData(GL_ARRAY_BUFFER,stride*instances,data,GL_DYNAMIC_DRAW);
glVertexAttribPointer(3, 4, GL_FLOAT, GL_FALSE, stride, (void*)0); //3-6 inclusive are the transformation matrix
glVertexAttribPointer(4, 4, GL_FLOAT, GL_FALSE, stride, (void*)(vec4Size));
glVertexAttribPointer(5, 4, GL_FLOAT, GL_FALSE, stride, (void*)(2 * vec4Size));
glVertexAttribPointer(6, 4, GL_FLOAT, GL_FALSE, stride, (void*)(3 * vec4Size));
glVertexAttribPointer(7, 1, GL_FLOAT, GL_FALSE, stride, (void*)(4*vec4Size)); //effect
glVertexAttribPointer(8, 4, GL_FLOAT, GL_FALSE, stride, (void*)(4*vec4Size + floatSize)); //color
glVertexAttribPointer(9, 1,GL_FLOAT, GL_FALSE, stride, (void*)((floats-5)*floatSize)); //z
glVertexAttribPointer(10, 4, GL_FLOAT, GL_FALSE, stride, (void*)((floats-4)*floatSize)); //portion
glEnableVertexAttribArray(3);
glEnableVertexAttribArray(4);
glEnableVertexAttribArray(5);
glEnableVertexAttribArray(6);
glEnableVertexAttribArray(7);
glEnableVertexAttribArray(8);
glEnableVertexAttribArray(9);
glEnableVertexAttribArray(10);

glVertexAttribDivisor(3, 1);
glVertexAttribDivisor(4, 1);
glVertexAttribDivisor(5, 1);
glVertexAttribDivisor(6, 1);
glVertexAttribDivisor(7, 1);
glVertexAttribDivisor(8, 1);
glVertexAttribDivisor(9, 1);
glVertexAttribDivisor(10, 1);

You don't need to call glVertexAttribPointer, glEnableVertexAttribArray, and glVertexAttribDivisor every frame. These calls should be made once at the start to bind the VBO to the VAO, e.g.:

// vao setup:

glBindVertexArray(VAO);
glBindBuffer(GL_ARRAY_BUFFER, vbo); // note: the VAO will remember this and bind it automatically when you bind the VAO
glVertexAttribPointer(...); // note: you need to call glBufferData once before this point, even if you pass it a nullptr and create a zero sized buffer!
...
glEnableVertexAttribArray(...);
...
glVertexAttribDivisor(...);

glBindBuffer(GL_ARRAY_BUFFER, 0); // note: it seems weird to do this, but the VAO will remember the VBO anyway
glBindVertexArray(0);

After the initial setup, binding the VAO will restore those settings. So your draw call can be reduced to uploading the new data to the vbo, binding the textures and setting up the shader program, something like:

// draw call:

glBindBuffer(GL_ARRAY_BUFFER, vbo);
glBufferData(GL_ARRAY_BUFFER, ...); // upload new instance data
glBindBuffer(GL_ARRAY_BUFFER, 0);

glBindTexture(...);

glUseProgram(...);

glBindVertexArray(VAO);
glDrawElementsInstanced(...);

... unbind ...

Also... glm::value_ptr can be used to get a pointer to the underlying data in a glm vector or matrix type. So if you wanted to, the copying could be done more like:

data = std::copy_n(glm::value_ptr(matt), matt.length(), data);
data = std::copy_n(glm::value_ptr(parameter.tint), parameter.tint.length(), data);
data = std::copy_n(&parameter.z, 1, data);
data = std::copy_n(glm::value_ptr(parameter.portion), parameter.portion.length(), data);
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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