Sprite drawing class

I have written a very XNA spritebatch like interface for drawing sprites in OpenGL. When begin is called the vertex data buffer is mapped to a float*. The index buffer and vertex buffer are bound in begin, and it's assumed no other drawing is done in this OpenGL context between begin and end. In between begin and end, DrawSprite is called. DrawSprite has a bunch of overloads allowing one to draw with a scale, and matrix, a source rectangle etc. However, they all take their parameters and call BufferSprite which actually writes the sprite data to memory (the x,y,z position, the x,y texture coordinates, and the rgba colour values.) When end is called, the vertices are drawn in as few glDrawElements calls as possible.

Here is Begin:

//
// Begin
// Sets up the buffer for drawing.
//
void Nixin::Canvas::SpriteBuffer::Begin( ShaderProgram& spriteShader )
{
    // Check that two begins have not been called without an end.
    if( hasBegun )
    {
        Debug::FatalError( "Begin was called twice before an end." );
    }
    hasBegun = true;
    shader = &spriteShader;

    spriteDataBuffer.Bind();
    spriteIndexBuffer.Bind();

    // Bind the sprite buffer.
    mappedData = spriteDataBuffer.Map();
}

Here is buffer sprite:

```
//
// BufferSprite
// Transforms a sprite, and defines the texture source. Once this is done, the sprite is buffered and is ready for drawing.
//
void Nixin::Canvas::SpriteBuffer::BufferSprite( const Texture& texture, const Rectangle& spriteBounds, const Point& scale, const float rotation, const Colour& tint, const Rectangle& sourceBounds, const Point& origin, const bool matrix, const Matrix& inModelView )
{
Point v1;
Point v2;
Point v3;
Point v4;

if( !hasBegun )
{
Debug::FatalError( "Begin must be called before drawing a sprite." );
}

// Check if we have reached the current sprite max. If so, wait for the end of the frame to draw them.
// This is a compromise. We can't increase the

I would avoid the testing for calling Begin and End appropriately by creating a separate class that does the begin actions during construction and the end actions during destruction.

As for speeding up the code, I think the first real step is to clarify the code a bit so it's easier to see what's really going on and what's needed. Just for example, you have:

for( int i = count + 1; i < sprites.size(); i++ )
    {
        if( sprites[count].texture == sprites[i].texture )
        {
            drawCount++;
        }
        else
        {
            break;
        }
    }

It looks like this could be rewritten to use std::find_if instead, and end up quite a bit simpler and more readable.

if (sprites.end() != std::find_if(sprites.begin()+count=1, sprites.end(),
[](sprite const &a, sprite const &b) { return a.texture == b.texture; }))
++drawCount;

[Actually, re-reading, that's wrong--the idea's correct, but I misunderstood your code a little bit, and translated it incorrectly.]

The obvious next step to take would be to avoid the linear search for the texture for every sprite. You haven't shown enough of the rest of the system to see an obvious way to do that, but chances seem pretty decent that you can probably avoid it.

Looking at the OpenGL part, I don't see an obvious way to improve performance a lot. You're already using glDrawRangeElements to draw all the sprites in one call. I suppose you might be able to switch from GL_TRIANGLES to something like GL_TRIANGLE_FAN or GL_TRIANGLE_STRIP to reduce the number of vertices you use, but 1) I didn't try to look at the code carefully enough to be sure you can use those, and 2) even if you can, it probably won't make a huge difference anyway.

StackExchange Code Review Q#47325, answer score: 3