From 37eff1149bacc61085c7229b6b7426bba8a0bc49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 10 Apr 2012 02:47:54 +0200 Subject: [PATCH] Using Vertex Array Objects for mesh drawing. * All attribute pointers and buffer binding is now stored in each mesh own vertex array object, thus each mesh drawing now needs only three OpenGL calls. * Removed default VAO from Scene, which fixes unit test crashes. --- PKGBUILD | 2 +- src/IndexedMesh.cpp | 30 +++++++----------------- src/Mesh.cpp | 38 +++++++++++++++--------------- src/Mesh.h | 57 +++++++++++++++++++++++---------------------- src/Scene.cpp | 8 ------- src/Scene.h | 15 ++---------- 6 files changed, 60 insertions(+), 90 deletions(-) diff --git a/PKGBUILD b/PKGBUILD index b01721986..ea12b1c35 100644 --- a/PKGBUILD +++ b/PKGBUILD @@ -24,7 +24,7 @@ build() { check() { cd "$startdir/build" - ctest -E "ObjectTest|SceneTest" # fix me! + ctest } package() { diff --git a/src/IndexedMesh.cpp b/src/IndexedMesh.cpp index bcc3befcd..6bc28e515 100644 --- a/src/IndexedMesh.cpp +++ b/src/IndexedMesh.cpp @@ -23,42 +23,30 @@ using namespace Corrade::Utility; namespace Magnum { void IndexedMesh::draw() { + bind(); + /* Finalize the mesh, if it is not already */ finalize(); - /* Enable vertex arrays for all attributes */ - for(set::const_iterator it = attributes().begin(); it != attributes().end(); ++it) - glEnableVertexAttribArray(*it); - - /* Bind attributes to vertex buffers */ - for(map > >::const_iterator it = buffers().begin(); it != buffers().end(); ++it) { - /* Bind buffer */ - it->first->bind(); - - /* Bind all attributes to this buffer */ - for(vector::const_iterator ait = it->second.second.begin(); ait != it->second.second.end(); ++ait) - if(TypeInfo::isIntegral(ait->type)) - glVertexAttribIPointer(ait->attribute, ait->size, static_cast(ait->type), ait->stride, ait->pointer); - else glVertexAttribPointer(ait->attribute, ait->size, static_cast(ait->type), GL_FALSE, ait->stride, ait->pointer); - } - - /* Bind index array, draw the elements and unbind */ - _indexBuffer.bind(); /** @todo Start at given index */ glDrawElements(static_cast(primitive()), _indexCount, static_cast(_indexType), nullptr); - /* Disable vertex arrays for all attributes */ - for(set::const_iterator it = attributes().begin(); it != attributes().end(); ++it) - glDisableVertexAttribArray(*it); + unbind(); } void IndexedMesh::finalize() { + if(isFinalized()) return; + if(!_indexCount) { Error() << "IndexedMesh: the mesh has zero index count!"; assert(0); } + /* Finalize attribute positions */ Mesh::finalize(); + + /* Bind index buffer */ + _indexBuffer.bind(); } } diff --git a/src/Mesh.cpp b/src/Mesh.cpp index 1030ad6a7..dc8a6c98b 100644 --- a/src/Mesh.cpp +++ b/src/Mesh.cpp @@ -27,6 +27,8 @@ namespace Magnum { Mesh::~Mesh() { for(map > >::iterator it = _buffers.begin(); it != _buffers.end(); ++it) delete it->first; + + glDeleteVertexArrays(1, &vao); } Buffer* Mesh::addBuffer(bool interleaved) { @@ -40,30 +42,15 @@ Buffer* Mesh::addBuffer(bool interleaved) { } void Mesh::draw() { + bind(); + /* Finalize the mesh, if it is not already */ finalize(); - /* Enable vertex arrays for all attributes */ - for(set::const_iterator it = _attributes.begin(); it != _attributes.end(); ++it) - glEnableVertexAttribArray(*it); - - /* Bind attributes to vertex buffers */ - for(map > >::const_iterator it = _buffers.begin(); it != _buffers.end(); ++it) { - /* Bind buffer */ - it->first->bind(); - - /* Bind all attributes to this buffer */ - for(vector::const_iterator ait = it->second.second.begin(); ait != it->second.second.end(); ++ait) - if(TypeInfo::isIntegral(ait->type)) - glVertexAttribIPointer(ait->attribute, ait->size, static_cast(ait->type), ait->stride, ait->pointer); - else glVertexAttribPointer(ait->attribute, ait->size, static_cast(ait->type), GL_FALSE, ait->stride, ait->pointer); - } - + /** @todo Start at given index */ glDrawArrays(static_cast(_primitive), 0, _vertexCount); - /* Disable vertex arrays for all attributes */ - for(set::const_iterator it = _attributes.begin(); it != _attributes.end(); ++it) - glDisableVertexAttribArray(*it); + unbind(); } void Mesh::finalize() { @@ -75,6 +62,10 @@ void Mesh::finalize() { assert(0); } + /* Enable vertex arrays for all attributes */ + for(set::const_iterator it = _attributes.begin(); it != _attributes.end(); ++it) + glEnableVertexAttribArray(*it); + /* Finalize attribute positions for every buffer */ for(map > >::iterator it = _buffers.begin(); it != _buffers.end(); ++it) { /* Avoid confustion */ @@ -109,6 +100,15 @@ void Mesh::finalize() { position += ait->size*TypeInfo::sizeOf(ait->type)*_vertexCount; } } + + /* Bind buffer */ + it->first->bind(); + + /* Bind all attributes to this buffer */ + for(vector::const_iterator ait = attributes.begin(); ait != attributes.end(); ++ait) + if(TypeInfo::isIntegral(ait->type)) + glVertexAttribIPointer(ait->attribute, ait->size, static_cast(ait->type), ait->stride, ait->pointer); + else glVertexAttribPointer(ait->attribute, ait->size, static_cast(ait->type), GL_FALSE, ait->stride, ait->pointer); } /* Mesh is now finalized, attribute binding is not allowed */ diff --git a/src/Mesh.h b/src/Mesh.h index 7b4a9400b..614955131 100644 --- a/src/Mesh.h +++ b/src/Mesh.h @@ -134,14 +134,18 @@ class MAGNUM_EXPORT Mesh { * data. Note that you have to call setVertexCount() manually for mesh * to draw properly. */ - inline Mesh(Primitive primitive = Primitive::Triangles): _primitive(primitive), _vertexCount(0), finalized(false) {} + inline Mesh(Primitive primitive = Primitive::Triangles): _primitive(primitive), _vertexCount(0), finalized(false) { + glGenVertexArrays(1, &vao); + } /** * @brief Constructor * @param primitive Primitive type * @param vertexCount Vertex count */ - inline Mesh(Primitive primitive, GLsizei vertexCount): _primitive(primitive), _vertexCount(vertexCount), finalized(false) {} + inline Mesh(Primitive primitive, GLsizei vertexCount): _primitive(primitive), _vertexCount(vertexCount), finalized(false) { + glGenVertexArrays(1, &vao); + } /** * @brief Destructor @@ -226,6 +230,21 @@ class MAGNUM_EXPORT Mesh { virtual void draw(); protected: + /** @brief Unbind any vertex array object */ + inline static void unbind() { glBindVertexArray(0); } + + /** @brief Bind vertex array object of current mesh */ + inline void bind() { glBindVertexArray(vao); } + + /** + * @brief Finalize the mesh + * + * Computes location and stride of each attribute in its buffer. After + * this function is called, no new attribute can be bound. + */ + void finalize(); + + private: /** @brief Vertex attribute */ struct Attribute { GLuint attribute; /**< @brief %Attribute ID */ @@ -235,43 +254,25 @@ class MAGNUM_EXPORT Mesh { const GLvoid* pointer; /**< @brief Pointer to first attribute of this type in the buffer */ }; + GLuint vao; + Primitive _primitive; + GLsizei _vertexCount; + bool finalized; + /** * @brief Buffers with their attributes - * @return Map of associated buffers, evey buffer has: + * + * Map of associated buffers, evey buffer has: * - boolean value which signalizes whether the buffer is interleaved * - list of bound attributes */ - inline const std::map > >& buffers() { return _buffers; } + std::map > > _buffers; /** * @brief List of all bound attributes * * List of all bound attributes bound with bindAttribute(). */ - inline const std::set& attributes() { return _attributes; } - - /** - * @brief Finalize the mesh - * - * Computes location and stride of each attribute in its buffer. After - * this function is called, no new attribute can be bound. - */ - virtual void finalize(); - - /** - * @brief Set the mesh dirty - * - * Sets the attribute locations in buffers as dirty, so they are - * recalculated on next drawing. - */ - inline void setDirty() { finalized = false; } - - private: - Primitive _primitive; - GLsizei _vertexCount; - bool finalized; - - std::map > > _buffers; std::set _attributes; void bindAttribute(Buffer* buffer, GLuint attribute, GLint size, Type type); diff --git a/src/Scene.cpp b/src/Scene.cpp index 67b4eea78..2c2901d14 100644 --- a/src/Scene.cpp +++ b/src/Scene.cpp @@ -17,14 +17,6 @@ namespace Magnum { -Scene::Scene(): Object(nullptr), _features(0) { - _parent = this; - - /* Bind default VAO */ - glGenVertexArrays(1, &vao); - glBindVertexArray(vao); -} - void Scene::setFeature(Scene::Feature feature, bool enabled) { GLenum _feature; switch(feature) { diff --git a/src/Scene.h b/src/Scene.h index 1ea23a899..207bac665 100644 --- a/src/Scene.h +++ b/src/Scene.h @@ -33,19 +33,8 @@ class MAGNUM_EXPORT Scene: public Object { FaceCulling = 0x04 /**< @brief Face culling */ }; - /** - * @brief Constructor - * - * Creates one default vertex array. - */ - Scene(); - - /** - * @brief Destructor - * - * Deletes the default vertex array. - */ - inline ~Scene() { glDeleteVertexArrays(1, &vao); } + /** @brief Constructor */ + inline Scene(): _features(0) { _parent = this; } void setParent(Object* parent) = delete; void setTransformation(const Matrix4& transformation) = delete;