From 8742dae4a43f095bc1d1b776b6e298a760598d5b Mon Sep 17 00:00:00 2001 From: Maciej Suminski Date: Thu, 6 Jun 2013 11:52:39 +0200 Subject: [PATCH] Removed indices storing from VBO_ITEM as they are always consecutive numbers. Removed storing pointers to VBO_ITEMs that have to be drawn - instead they are memcpied to mapped GPU memory. Some functions of VBO_ITEM became inline. --- common/gal/opengl/opengl_gal.cpp | 53 +++++++++--------- common/gal/opengl/vbo_item.cpp | 95 -------------------------------- include/gal/opengl/opengl_gal.h | 5 +- include/gal/opengl/vbo_item.h | 70 ++++++++++++++--------- 4 files changed, 73 insertions(+), 150 deletions(-) diff --git a/common/gal/opengl/opengl_gal.cpp b/common/gal/opengl/opengl_gal.cpp index c9d3987402..c0128a9515 100644 --- a/common/gal/opengl/opengl_gal.cpp +++ b/common/gal/opengl/opengl_gal.cpp @@ -414,9 +414,19 @@ void OPENGL_GAL::BeginDrawing() if( vboNeedsUpdate ) rebuildVbo(); - // Clear indices buffer - itemsToDraw.clear(); - itemsToDrawSize = 0; + // Number of vertices to be drawn + indicesSize = 0; + + glBindBuffer( GL_ELEMENT_ARRAY_BUFFER, curVboIndId ); + // Discard old buffer, so we can use it again + glBufferData( GL_ELEMENT_ARRAY_BUFFER, vboSize * VBO_ITEM::IndByteSize, NULL, GL_STREAM_DRAW ); + + // Map the GPU memory, so we can store indices that are going to be drawn + indicesPtr = static_cast( glMapBuffer( GL_ELEMENT_ARRAY_BUFFER, GL_WRITE_ONLY ) ); + if( indicesPtr == NULL ) + { + wxLogError( wxT( "Could not map GPU memory" ) ); + } } @@ -473,6 +483,11 @@ void OPENGL_GAL::EndDrawing() // TODO Checking if we are using right VBOs, in other case do the binding. // Right now there is only one VBO, so there is no problem. + if( !glUnmapBuffer( GL_ELEMENT_ARRAY_BUFFER ) ) + { + wxLogError( wxT( "Unmapping indices buffer failed" ) ); + } + // Prepare buffers glEnableClientState( GL_VERTEX_ARRAY ); glEnableClientState( GL_COLOR_ARRAY ); @@ -492,28 +507,7 @@ void OPENGL_GAL::EndDrawing() VBO_ITEM::VertByteSize, (GLvoid*) VBO_ITEM::ShaderByteOffset ); } - glBindBuffer( GL_ELEMENT_ARRAY_BUFFER, curVboIndId ); - GLuint* indicesPtr = static_cast( glMapBuffer( GL_ELEMENT_ARRAY_BUFFER, GL_WRITE_ONLY ) ); - - if( indicesPtr == NULL ) - { - wxLogError( wxT( "Could not map GPU memory" ) ); - } - - // Copy indices of items that should be drawn to GPU memory - std::list::const_iterator it, end; - for( it = itemsToDraw.begin(), end = itemsToDraw.end(); it != end; ++it ) - { - memcpy( indicesPtr, (*it)->GetIndices(), (*it)->GetSize() * VBO_ITEM::IndByteSize ); - indicesPtr += (*it)->GetSize() * VBO_ITEM::IndStride; - } - - if( !glUnmapBuffer( GL_ELEMENT_ARRAY_BUFFER ) ) - { - wxLogError( wxT( "Unmapping indices buffer failed" ) ); - } - - glDrawElements( GL_TRIANGLES, itemsToDrawSize, GL_UNSIGNED_INT, (GLvoid*) 0 ); + glDrawElements( GL_TRIANGLES, indicesSize, GL_UNSIGNED_INT, (GLvoid*) 0 ); glBindBuffer( GL_ELEMENT_ARRAY_BUFFER, 0 ); glBindBuffer( GL_ARRAY_BUFFER, 0 ); @@ -1704,8 +1698,13 @@ void OPENGL_GAL::DeleteGroup( int aGroupNumber ) void OPENGL_GAL::DrawGroup( int aGroupNumber ) { - itemsToDraw.push_back( vboItems[aGroupNumber] ); - itemsToDrawSize += vboItems[aGroupNumber]->GetSize(); + int size = vboItems[aGroupNumber]->GetSize(); + int offset = vboItems[aGroupNumber]->GetOffset(); + + // Copy indices of items that should be drawn to GPU memory + for( int i = offset; i < offset + size; *indicesPtr++ = i++ ); + + indicesSize += size; } diff --git a/common/gal/opengl/vbo_item.cpp b/common/gal/opengl/vbo_item.cpp index 6aeb98034d..1412a83b57 100644 --- a/common/gal/opengl/vbo_item.cpp +++ b/common/gal/opengl/vbo_item.cpp @@ -34,7 +34,6 @@ using namespace KiGfx; VBO_ITEM::VBO_ITEM() : m_vertices( NULL ), - m_indices( NULL ), m_offset( 0 ), m_size( 0 ), m_isDirty( true ), @@ -56,17 +55,10 @@ VBO_ITEM::~VBO_ITEM() std::list::const_iterator v_it, v_end; for( v_it = m_vertBlocks.begin(), v_end = m_vertBlocks.end(); v_it != v_end; ++v_it ) delete[] *v_it; - - std::list::const_iterator i_it, i_end; - for( i_it = m_indBlocks.begin(), i_end = m_indBlocks.end(); i_it != i_end; ++i_it ) - delete[] *i_it; } if( m_vertices ) delete m_vertices; - - if( m_indices ) - delete m_indices; } @@ -100,10 +92,6 @@ void VBO_ITEM::PushVertex( const GLfloat* aVertex ) // Move to the next free space m_vertPtr++; - // Add the new index - *m_indPtr = m_offset + m_size; - m_indPtr++; - m_size++; m_isDirty = true; m_spaceLeft--; @@ -128,50 +116,6 @@ GLfloat* VBO_ITEM::GetVertices() } -GLuint* VBO_ITEM::GetIndices() -{ - if( m_isDirty ) - prepareFinal(); - - return m_indices; -} - - -int VBO_ITEM::GetSize() const -{ - return m_size; -} - - -void VBO_ITEM::SetOffset( int aOffset ) -{ - if( m_offset == aOffset ) - return; - - int delta = aOffset - m_offset; - - // Change offset for all the stored indices - for( int i = 0; i < m_size; ++i ) - { - m_indices += delta; - } - - m_offset = aOffset; -} - - -int VBO_ITEM::GetOffset() const -{ - return m_offset; -} - - -void VBO_ITEM::SetTransformMatrix( const glm::mat4* aMatrix ) -{ - m_transform = aMatrix; -} - - void VBO_ITEM::ChangeColor( const COLOR4D& aColor ) { if( m_isDirty ) @@ -191,31 +135,12 @@ void VBO_ITEM::ChangeColor( const COLOR4D& aColor ) } -void VBO_ITEM::UseColor( const COLOR4D& aColor ) -{ - m_color[0] = aColor.r; - m_color[1] = aColor.g; - m_color[2] = aColor.b; - m_color[3] = aColor.a; -} - - -void VBO_ITEM::UseShader( const GLfloat* aShader ) -{ - memcpy( m_shader, aShader, ShaderByteSize ); -} - - void VBO_ITEM::useNewBlock() { VBO_VERTEX* newVertBlock = new VBO_VERTEX[BLOCK_SIZE]; - GLuint* newIndBlock = new GLuint[BLOCK_SIZE]; m_vertPtr = newVertBlock; - m_indPtr = newIndBlock; - m_vertBlocks.push_back( newVertBlock ); - m_indBlocks.push_back( newIndBlock ); m_spaceLeft = BLOCK_SIZE; } @@ -243,25 +168,5 @@ void VBO_ITEM::prepareFinal() // In the last block we need to copy only used vertices memcpy( vertPtr, *v_it, ( BLOCK_SIZE - m_spaceLeft ) * VertByteSize ); - if( m_indices ) - delete m_indices; - - // Allocate memory that would store all of indices - m_indices = new GLuint[m_size * IndStride]; - // Set the pointer that will move along the buffer - GLuint* indPtr = m_indices; - - // Copy blocks of indices one after another to m_indices - std::list::const_iterator i_it; - for( i_it = m_indBlocks.begin(); *i_it != m_indBlocks.back(); ++i_it ) - { - memcpy( indPtr, *i_it, BLOCK_SIZE * IndByteSize ); - delete[] *i_it; - indPtr += ( BLOCK_SIZE * IndStride ); - } - - // In the last block we need to copy only used indices - memcpy( indPtr, *i_it, ( BLOCK_SIZE - m_spaceLeft ) * IndByteSize ); - m_isDirty = false; } diff --git a/include/gal/opengl/opengl_gal.h b/include/gal/opengl/opengl_gal.h index 56a2c38911..0c04447040 100644 --- a/include/gal/opengl/opengl_gal.h +++ b/include/gal/opengl/opengl_gal.h @@ -353,9 +353,8 @@ private: bool vboNeedsUpdate; ///< Flag indicating if VBO should be rebuilt glm::mat4 transform; ///< Current transformation matrix std::stack transformStack; ///< Stack of transformation matrices - std::list itemsToDraw; ///< Stores items that are going to be - ///< drawn in the current frame - int itemsToDrawSize; ///< Number of indices to be drawn + int indicesSize; ///< Number of indices to be drawn + GLuint* indicesPtr; ///< Pointer to mapped GPU memory double curvePoints[12]; ///< Coefficients for curves // FIXME to be removed: diff --git a/include/gal/opengl/vbo_item.h b/include/gal/opengl/vbo_item.h index 028105506f..a3b476cbb5 100644 --- a/include/gal/opengl/vbo_item.h +++ b/include/gal/opengl/vbo_item.h @@ -45,8 +45,7 @@ typedef struct VBO_VERTEX GLfloat x, y, z; // Coordinates GLfloat r, g, b, a; // Color GLfloat shader[4]; // Shader type & params -} VBO_VERTEX_DATA; - +} VBO_VERTEX; class VBO_ITEM { @@ -81,33 +80,36 @@ public: */ GLfloat* GetVertices(); - /** - * Function GetIndices() - * Returns a pointer to the array containing all indices of vertices. - * @return Pointer to indices. - */ - GLuint* GetIndices(); /** * Function GetSize() * Returns information about number of vertices stored. * @param Amount of vertices. */ - int GetSize() const; + inline int GetSize() const + { + return m_size; + } /** * Function SetOffset() * Sets data offset in the VBO. * @param aOffset is the offset expressed as a number of vertices. */ - void SetOffset( int aOffset ); + void SetOffset( int aOffset ) + { + m_offset = aOffset; + } /** * Function GetOffset() * Returns data offset in the VBO. * @return Data offset expressed as a number of vertices. */ - int GetOffset() const; + inline int GetOffset() const + { + return m_offset; + } /** * Function SetTransformMatrix() @@ -116,7 +118,10 @@ public: * @param aMatrix is the new transform matrix or NULL if you do not want to use transformation * matrix. */ - void SetTransformMatrix( const glm::mat4* aMatrix ); + void SetTransformMatrix( const glm::mat4* aMatrix ) + { + m_transform = aMatrix; + } /** * Function ChangeColor() @@ -130,18 +135,37 @@ public: * Sets color used for all added vertices. * @param aColor is the color used for added vertices. */ - void UseColor( const COLOR4D& aColor ); + void UseColor( const COLOR4D& aColor ) + { + m_color[0] = aColor.r; + m_color[1] = aColor.g; + m_color[2] = aColor.b; + m_color[3] = aColor.a; + } /** * Function UseShader() * Sets shader and its parameters used for all added vertices. * @param aShader is the array that contains shader number followed by its parameters. */ - void UseShader( const GLfloat* aShader ); + inline void UseShader( const GLfloat* aShader ) + { + for( int i = 0; i < ShaderStride; ++i ) + { + m_shader[i] = aShader[i]; + } + } + + + inline void FreeVerticesData() + { + if( m_vertices && !m_isDirty ) + { + delete[] m_vertices; + m_vertices = NULL; + } + } - ///< Functions for getting VBO ids. - //void SetVbo( int aVboId ); - //int GetVbo() const; ///< Data organization information for vertices {X,Y,Z,R,G,B,A} (@see VBO_VERTEX). static const int VertByteSize = sizeof(VBO_VERTEX); @@ -164,26 +188,22 @@ public: static const int ShaderByteSize = sizeof(VBO_VERTEX().shader); static const int ShaderStride = ShaderByteSize / sizeof(GLfloat); - static const int IndStride = 1; - static const int IndByteSize = IndStride * sizeof(GLuint); + static const int IndByteSize = sizeof(GLuint); private: ///< Contains vertices coordinates and colors. ///< Packed by 7 floats for each vertex: {X, Y, Z, R, G, B, A} GLfloat* m_vertices; - ///< Indices of vertices - GLuint* m_indices; - ///< Lists of data blocks storing vertices std::list m_vertBlocks; - std::list m_indBlocks; + ///< Pointers to current blocks that should be used for storing data VBO_VERTEX* m_vertPtr; - GLuint* m_indPtr; + ///< How many vertices can be stored in the current buffer int m_spaceLeft; - ///< Number of vertices & indices stored in a single block + ///< Number of vertices stored in a single block static const int BLOCK_SIZE = 256; ///< Creates a new block for storing vertices data void useNewBlock();