Sprite drawing class

Question

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<float>();
}

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 sprite max mid-frame as that would clear all the currently
    // buffered sprites, so we just avoid drawing them until next frame.
    if( CheckSpriteBufferSize() )
    {
        if( !matrix )
        {
            float                           cosz            = cos( ToRadians( rotation ) );
            float                           sinz            = sin( ToRadians( rotation ) );
            modelView.SetDataAt( 0, 0, cosz * scale.x );
            modelView.SetDataAt( 0, 1, -sinz * scale.x );
            modelView.SetDataAt( 0, 3, spriteBounds.x );

            modelView.SetDataAt( 1, 0, sinz * scale.y );
            modelView.SetDataAt( 1, 1, cosz * scale.y );
            modelView.SetDataAt( 1, 3, spriteBounds.y );

            modelView.SetDataAt( 2, 0, 0 );
            modelView.SetDataAt( 2, 1, 0 );
            modelView.SetDataAt( 2, 3, 1.0f );

            v1 = Point::Multiply( Point( -origin.x, -origin.y, 1.0f ), modelView );
            v2 = Point::Multiply( Point( spriteBounds.width - origin.x, -origin.y, 1.0f ), modelView );
            v3 = Point::Multiply( Point( -origin.x, spriteBounds.height - origin.y, 1.0f ), modelView );
            v4 = Point::Multiply( Point( spriteBounds.width - origin.x, spriteBounds.height - origin.y, 1.0f ), modelView );
        }
        else
        {
            v1 = Point::Multiply( Point( -origin.x, -origin.y, origin.z ), inModelView );
            v2 = Point::Multiply( Point( spriteBounds.width - origin.x, -origin.y, origin.z ), inModelView );
            v3 = Point::Multiply( Point( -origin.x, spriteBounds.height -origin.y, origin.z ), inModelView );
            v4 = Point::Multiply( Point( spriteBounds.width - origin.x, spriteBounds.height - origin.y, origin.z ), inModelView );
        }

        // Copy the vertex data into it's location in the mapped data pointer.
        int             offset = sprites.size() * 12;
        mappedData[offset] = v1.x;
        mappedData[offset + 1] = v1.y;
        mappedData[offset + 2] = v1.z;
        mappedData[offset + 3] = v2.x;
        mappedData[offset + 4] = v2.y;
        mappedData[offset + 5] = v2.z;
        mappedData[offset + 6] = v3.x;                                                                              
        mappedData[offset + 7] = v3.y;
        mappedData[offset + 8] = v3.z;
        mappedData[offset + 9] = v4.x;
        mappedData[offset + 10] = v4.y;
        mappedData[offset + 11] = v4.z;

        // Do the same for the texture coordinates. Here we add the max sprite count * 8, because the texture coordinate
        // data is stored after all the vertex data.
        offset = sprites.size() * 8 + ( maxSpriteCount ) * 12;
        mappedData[offset] =      sourceBounds.x / texture.GetWidth();                  
        mappedData[offset + 1] = ( sourceBounds.height + sourceBounds.y ) / texture.GetHeight();
        mappedData[offset + 2] = ( sourceBounds.width + sourceBounds.x ) / texture.GetWidth(); 
        mappedData[offset + 3] = ( sourceBounds.height + sourceBounds.y ) / texture.GetHeight();
        mappedData[offset + 4] = sourceBounds.x / texture.GetWidth();                       // Texture coordinates are defined by the triangle( srcPosition, srcSize )
        mappedData[offset + 5] = sourceBounds.y / texture.GetHeight();
        mappedData[offset + 6] = ( sourceBounds.width + sourceBounds.x ) / texture.GetWidth();
        mappedData[offset + 7] = sourceBounds.y / texture.GetHeight();

        // Save the sprite's colour.
        offset = sprites.size() * 16 + ( maxSpriteCount ) * 20;
        mappedData[offset]      = tint.r;
        mappedData[offset + 1]  = tint.g;
        mappedData[offset + 2]  = tint.b;
        mappedData[offset + 3]  = tint.a;
        mappedData[offset + 4]  = tint.r;
        mappedData[offset + 5]  = tint.g;
        mappedData[offset + 6]  = tint.b;
        mappedData[offset + 7]  = tint.a;
        mappedData[offset + 8]  = tint.r;
        mappedData[offset + 9]  = tint.g;
        mappedData[offset + 10] = tint.b;
        mappedData[offset + 11] = tint.a;
        mappedData[offset + 12] = tint.r;
        mappedData[offset + 13] = tint.g;
        mappedData[offset + 14] = tint.b;
        mappedData[offset + 15] = tint.a;

        // Add the sprite object.
        sprites.emplace_back( texture.GetID(), sprites.size() );
    }
}

And here is End:

//
// End
//
void Nixin::Canvas::SpriteBuffer::End( Canvas& canvas )
{
    // Check for validity of end call.
    if( !hasBegun )
    {
        Debug::FatalError( "End was called before begin." );
    }


    // Unmap the buffer data pointer, because we're about to draw with it.
    spriteDataBuffer.Unmap();

    // Use custom shader.
    glUseProgram( shader->GetID() );

    // Set the projection matrix, and vertex attributes in the shader.
    shader->SetUniformMatrix( "projection", canvas.camera.GetProjectionMatrix() );
    shader->SetVertexAttributePointer( "vertexPosition", 3, Texture::DataType::FLOAT, 0, 0 );
    shader->SetVertexAttributePointer( "texCoords", 2, Texture::DataType::FLOAT, 0, 12 * maxSpriteCount * sizeof( float ) );
    shader->SetVertexAttributePointer( "tint", 4, Texture::DataType::FLOAT, 0, 20 * maxSpriteCount * sizeof( float ) );


    // Sort sprites.
    if( spriteSortMode != SpriteSortMode::NO_SORTING )
    {
        // Prepared for drawing.
        SortSprites();
        OrderIndices();
    }

    if( drawingMode == SpriteDrawingMode::DEPTH_TESTED )
    {
        canvas.EnableDepthTesting();
    }

    // This will be the total number of sprites draw.
    int count = 0;
    // While we haven't drawn all the sprites.
    while( count < sprites.size() )         
    {
        // We assume that at least one sprite is going to be drawn this loop.
        int             drawCount = 1;                  

        // Set the texture sampler.
        shader->SetSampler2D( "spriteTexture", 0, sprites[count].texture );

        // We step through the buffer, looking for a change in texture. If we find one, then take all the sprites that are next to each, that also have the same texture.
        for( int i = count + 1; i < sprites.size(); i++ )
        {
            if( sprites[count].texture == sprites[i].texture )
            {
                drawCount++;
            }
            else
            {
                break;
            }
        }

        // Finally, draw this group of sprites.
        glDrawRangeElements( GL_TRIANGLES, NULL + count  * indicesPerSprite * sizeof( unsigned int ), NULL + count  * indicesPerSprite * sizeof( unsigned int ) + indicesPerSprite * drawCount, indicesPerSprite * drawCount, GL_UNSIGNED_INT, ( GLvoid* )( NULL + count  * indicesPerSprite * sizeof( unsigned int ) ) );

        // Increase the number of sprites drawn.
        count += drawCount;
    } 
    if( drawingMode == SpriteDrawingMode::DEPTH_TESTED )
    {
        canvas.DisableDepthTesting();
    }

    // Check if the sprite buffer needs to grow.
    if( growNextEnd )
    {
        Grow( 2.0f );

        // Set this to false, as the growing is complete.
        growNextEnd = false;
    }
    else
    {
        //glInvalidateBufferData( spriteBuffer->spriteDataBufferLocation );
        spriteDataBuffer.BufferData( nullptr, sizeof( float ) * 36 * maxSpriteCount, VBufferAccess::STREAM_DRAW );
    }

    // Clear the sprite list.
    sprites.clear();

    // No longer drawing.
    hasBegun = false;
}

I know it's a lot, but I'd appreciate it if someone could help me make this faster. If there is only one texture begin drawn over and over again in release mode, I get about 60FPS for about 33k sprites( The SAME texture ). This only gets worse as I interleave textures. I've done a bit of profiling and omitting code, and it looks to me like BufferSprite is taking the most time by far. I'd just like to know if there's anything obvious I'm not doing right, such as writing to the buffer in a poor way, or maybe I should be uploading different data to the shader.

Also, I realise a lot of OpenGL stuff in this is wrapped in my own classes, so if you need any specific source code I'll edit this post.

Also, the computer I'm testing this on has a Intel Core Quad Q8200 @ 2.33GHz, and a AMD Radeon HD 6850.

EDIT: Sleepy profile times:

Answer 1

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.