refactoring of ArrayBuffer and VertexArrayObject

* move mMode fom ArrayBuffer to VAO
* add attachAllAttributes method to VAO
* allow adding VAO attributes without binding them (location == -1)
* store state of previous VAO binding in static field

include/ACGL/OpenGL/Controller/ArrayBufferControl.hh

@@ -41,7 +41,6 @@ public:
 public:
     ArrayBufferControl(void)
     :   mUsage(GL_STATIC_DRAW),
-        mMode(GL_TRIANGLES),
         mElements(0),
         mpData(NULL),
         mAttributeDefines()
@@ -53,7 +52,6 @@ public:
     // ==================================================================================================== \/
 public:
     inline ArrayBufferControl& usage (GLenum _usage) { mUsage = _usage; return *this; }
-    inline ArrayBufferControl& mode  (GLenum _mode)  { mMode = _mode;   return *this; }
 
     inline ArrayBufferControl& data (const GLvoid* _pData, GLsizei _elements)
     {
@@ -80,7 +78,6 @@ public:
     // =================================================================================================== \/
 protected:
     GLenum  mUsage;
-    GLenum  mMode;
     GLsizei mElements;
     const GLvoid* mpData;
     AttributeDefineVec mAttributeDefines;

include/ACGL/OpenGL/Objects/ArrayBuffer.hh

@@ -39,6 +39,8 @@
 namespace ACGL{
 namespace OpenGL{
 
+/*
+
 class ArrayBuffer
 {
     ACGL_NOT_COPYABLE(ArrayBuffer)
@@ -223,6 +225,7 @@ protected:
 
 ACGL_SHARED_TYPEDEF(ArrayBuffer)
 
+*/
 
 
 
@@ -230,8 +233,7 @@ ACGL_SHARED_TYPEDEF(ArrayBuffer)
 
 
 
-
-class ArrayBufferX : Buffer
+class ArrayBuffer : public Buffer
 {
     // ==================================================================================================== \/
     // ============================================================================================ STRUCTS \/
@@ -257,13 +259,13 @@ public:
     // ============================================================================================ CONSTRUCTORS \/
     // ========================================================================================================= \/
 public:
-    ArrayBufferX()
+    ArrayBuffer()
         : Buffer(GL_ARRAY_BUFFER),
         mStride(0),
         mAttributes()
     {}
 
-    ArrayBufferX( SharedBufferObject _pBuffer )
+    ArrayBuffer( SharedBufferObject _pBuffer )
         : Buffer(_pBuffer, GL_ARRAY_BUFFER),
         mStride(0),
         mAttributes()
@@ -357,6 +359,9 @@ public:
         mAttributes.push_back( _attribute );
     }
 
+    //! Returns the index of a named attribute
+    int_t getAttributeIndexByName(const std::string& _nameInArray) const;
+
     //! Setting of the stride size explicitly is not needed if the attributes are defined correctly (with padding)
     inline void setStride( GLsizei _stride ) {
         mStride = _stride;
@@ -369,6 +374,12 @@ public:
         mAttributes.clear();
     }
 
+    //! Set data for this buffer for a given number of elements
+    //! Use only after all attributes have been defined
+    inline void setDataElements( uint_t _elements, const GLvoid *_pData = NULL, GLenum _usage = GL_STATIC_DRAW ) {
+        setData( mTarget, _elements * mStride, _pData, _usage );
+    }
+
     //! Overloaded from the base class to _prevent_ redefining of the binding target! (see Buffer)
     inline void setTarget( GLenum ) {
         ACGL::Utils::error() << "DON'T redefine the target binding point of an ArrayBuffer" << std::endl;
@@ -383,7 +394,7 @@ protected:
     AttributeVec mAttributes;
 };
 
-ACGL_SHARED_TYPEDEF(ArrayBufferX)
+ACGL_SHARED_TYPEDEF(ArrayBuffer)
 
 
 

include/ACGL/OpenGL/Objects/VertexArrayObject.hh

@@ -63,7 +63,8 @@ public:
     VertexArrayObject()
     :   mObjectName(0),
         mpElementArrayBuffer(),
-        mAttributes()
+        mAttributes(),
+        mMode(GL_TRIANGLES)
     {
         glGenVertexArrays(1, &mObjectName);
         if (openGLCriticalErrorOccured() ) {
@@ -77,12 +78,19 @@ public:
         glDeleteVertexArrays(1, &mObjectName);
     }
 
+    // ==================================================================================================== \/
+    // ============================================================================================ SETTERS \/
+    // ==================================================================================================== \/
+public:
+    inline void setMode(GLenum _mode) { mMode = _mode; }
+
     // ==================================================================================================== \/
     // ============================================================================================ GETTERS \/
     // ==================================================================================================== \/
 public:
     inline GLuint operator()   (void) const { return mObjectName; }
     inline GLuint getObjectName(void) const { return mObjectName; }
+    inline GLenum getMode(void)       const { return mMode;       }
 
     // ==================================================================================================== \/
     // ============================================================================================ METHODS \/
@@ -101,20 +109,20 @@ public:
      */
     void attachElementArrayBuffer( const SharedElementArrayBuffer& _elementArrayBuffer )
     {
-        // query old VAO
-        GLint oldVAO; glGetIntegerv( GL_VERTEX_ARRAY_BINDING, &oldVAO );
+        enable();
 
         mpElementArrayBuffer = _elementArrayBuffer;
 
-        bind();
-        if (mpElementArrayBuffer) { // could be set to NULL!
+        if (mpElementArrayBuffer) // could be set to NULL!
+        {
             mpElementArrayBuffer->bind();
-        } else {
+        }
+        else
+        {
             glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
         }
 
-        // restore old VAO
-        glBindVertexArray( oldVAO ); openGLRareError();
+        disable();
     }
 
     inline void detachElementArrayBuffer() { attachElementArrayBuffer( SharedElementArrayBuffer() ); }
@@ -123,20 +131,38 @@ public:
      * Will set the attribute _arrayBufferAttribute of ArrayBuffer _arrayBuffer to the given attribute location.
      * If that location was already used it will get overwritten.
      * The _attributeLocation has to be lower than GL_MAX_VERTEX_ATTRIBS
+     * An attribute location of -1 indicates that the attribute should remain unbound for now
      */
-    void attachAttribute( const SharedArrayBuffer &_arrayBuffer,
+    inline void attachAttribute( const SharedArrayBuffer& _arrayBuffer,
                           uint32_t _arrayBufferAttribute,
-                          GLuint   _attributeLocation)
+                          GLint   _attributeLocation = -1)
     {
         Attribute newAttribute = { _arrayBuffer, _arrayBufferAttribute, _attributeLocation };
         attachAttribute( newAttribute );
     }
 
+    /**
+     * Will set the attribute named _arrayBufferAttributeName of ArrayBuffer _arrayBuffer to the given attribute location.
+     * If that location was already used it will get overwritten.
+     * The _attributeLocation has to be lower than GL_MAX_VERTEX_ATTRIBS
+     * An attribute location of -1 indicates that the attribute should remain unbound for now
+     */
+    inline void attachAttribute( const SharedArrayBuffer& _arrayBuffer,
+                          const std::string& _arrayBufferAttributeName,
+                          GLint   _attributeLocation = -1)
+    {
+        attachAttribute(_arrayBuffer,
+                        _arrayBuffer->getAttributeIndexByName(_arrayBufferAttributeName),
+                        _attributeLocation);
+    }
+
     void attachAttribute( const Attribute &_attribute )
     {
         GLint maxAttributes;
         glGetIntegerv(GL_MAX_VERTEX_ATTRIBS, &maxAttributes); // TODO: clever caching
-        if (mAttributes.size() >= (uint32_t) maxAttributes) {
+
+        if(mAttributes.size() >= (uint32_t) maxAttributes)
+        {
             ACGL::Utils::error() << "can't attach attribute " << _attribute.arrayBuffer->getAttributes()[_attribute.attributeID].name
                                  << " - maximum number of attributes reached: " << maxAttributes << std::endl;
             return;
@@ -144,15 +170,23 @@ public:
 
         mAttributes.push_back( _attribute );
 
-        // query old VAO
-        GLint oldVAO; glGetIntegerv( GL_VERTEX_ARRAY_BINDING, &oldVAO );
-
-        bind();
-        glEnableVertexAttribArray( _attribute.location );
-        _attribute.arrayBuffer->setAttributePointer( _attribute.attributeID, _attribute.location );
+        enable();
+        setAttributePointer( _attribute );
+        disable();
+    }
 
-        // restore old VAO
-        glBindVertexArray( oldVAO ); openGLRareError();
+    /**
+     * Attaches all attributes defined by an ArrayBuffer
+     * The attributes are attached to the default location of -1, indicating they should remain unbound for now
+     * Afterwards, you might want to automatically wire up the attributes with a ShaderProgram, using setAttributeLocationsByShaderProgram
+     */
+    void attachAllAttributes( const SharedArrayBuffer& _arrayBuffer )
+    {
+        ArrayBuffer::AttributeVec attributes = _arrayBuffer->getAttributes();
+        for(ArrayBuffer::AttributeVec::size_type i = 0; i < attributes.size(); ++i)
+        {
+            attachAttribute(_arrayBuffer, i);
+        }
     }
 
     /**
@@ -162,7 +196,8 @@ public:
     {
         for (AttributeVec::size_type i = 0; i < mAttributes.size(); ++i)
         {
-            if (mAttributes[i].location == _location) {
+            if (mAttributes[i].location == _location)
+            {
                 // the other pointer data is still set, but that isn't relevant if the attribute itself is deactivated
                 glDisableVertexArrayAttribEXT( mObjectName, mAttributes[i].location );
                 mAttributes.erase( mAttributes.begin()+i );
@@ -180,7 +215,8 @@ public:
     {
         for (AttributeVec::size_type i = 0; i < mAttributes.size(); ++i)
         {
-            if (mAttributes[i].arrayBuffer->getAttributes()[ mAttributes[i].attributeID ].name == _name) {
+            if (mAttributes[i].arrayBuffer->getAttributes()[ mAttributes[i].attributeID ].name == _name)
+            {
                 // the other pointer data is still set, but that isn't relevant if the attribute itself is deactivated
                 glDisableVertexArrayAttribEXT( mObjectName, mAttributes[i].location );
                 mAttributes.erase( mAttributes.begin()+i );
@@ -195,7 +231,7 @@ public:
      * Return true if setAttributeMappingsByShaderProgram(_shaderProgram) would do nothing.
      * That means the VAO and ShaderProgram have already matching layouts.
      */
-    bool mappingsMatchShaderProgram( const ConstSharedShaderProgram &_shaderProgram )
+    bool mappingsMatchShaderProgram( const ConstSharedShaderProgram &_shaderProgram ) const
     {
         for (AttributeVec::size_type i = 0; i < mAttributes.size(); ++i)
         {
@@ -211,7 +247,7 @@ public:
      * Query the attribute locations based on the attribute names in the ArrayBuffers and the ShaderProgram
      * If they match, use the location reported from the ShaderProgram.
      */
-    void setAttributeMappingsByShaderProgram( const ConstSharedShaderProgram &_shaderProgram )
+    void setAttributeLocationsByShaderProgram( const ConstSharedShaderProgram &_shaderProgram )
     {
         bool fullUpdateNeeded = false;
 
@@ -220,12 +256,14 @@ public:
             std::string attributeName  = mAttributes[i].arrayBuffer->getAttributes()[ mAttributes[i].attributeID ].name;
             GLint       shaderLocation = _shaderProgram->getAttributeLocation( attributeName );
 
-            if (shaderLocation == -1) {
+            if (shaderLocation == -1)
+            {
                 ACGL::Utils::error() << "can't update VAO mappings, attribute " << attributeName << " does not exist in shader" << std::endl;
                 continue; // try to match as much as possible
             }
 
-            if (mAttributes[i].location != shaderLocation) {
+            if (mAttributes[i].location != shaderLocation)
+            {
                 mAttributes[i].location  = shaderLocation;
                 fullUpdateNeeded = true;
             }
@@ -233,28 +271,52 @@ public:
 
         // why the full update? setting the new location right when a change is detected will get problamatic
         // if two attributes exchange there position...
-        if (fullUpdateNeeded) {
-            // query old VAO
-            GLint oldVAO; glGetIntegerv( GL_VERTEX_ARRAY_BINDING, &oldVAO );
+        if (fullUpdateNeeded)
+        {
+            enable();
 
-            bind();
+            // disable all attributes
             GLint maxAttributes;
             glGetIntegerv( GL_MAX_VERTEX_ATTRIBS, &maxAttributes );
+            for (GLint i = 0; i < maxAttributes; ++i)
+                glDisableVertexAttribArray( i );
 
-            // disable all attributes
-            for (GLint i = 0; i < maxAttributes; ++i) glDisableVertexAttribArray( i );
             // set all attributes:
-
-            for (uint32_t i = 0; i < mAttributes.size(); ++i) {
-                glEnableVertexAttribArray( mAttributes[i].location );
-                mAttributes[i].arrayBuffer->setAttributePointer( mAttributes[i].attributeID, mAttributes[i].location );
+            for (uint32_t i = 0; i < mAttributes.size(); ++i)
+            {
+                setAttributePointer(mAttributes[i]);
             }
 
-            // restore old VAO
-            glBindVertexArray( oldVAO ); openGLRareError();
+            disable();
         }
     }
 
+private:
+    //! Sets the vertex attribute pointer for the current VAO according to the specified attribute data
+    //! Note: expects that this VAO is currently bound
+    //! Note: will bind the ArrayBuffer referenced by _attribute.arrayBuffer
+    void setAttributePointer(const Attribute& _attribute)
+    {
+        if(_attribute.location == -1)
+        {
+            // An attribute location of -1 indicates that this attribute should remain unbound for now
+            return;
+        }
+
+        SharedArrayBuffer arrayBuffer = _attribute.arrayBuffer;
+        arrayBuffer->bind();
+
+        ArrayBuffer::Attribute arrayBufferAttribute = arrayBuffer->getAttributes()[_attribute.attributeID];
+        glVertexAttribPointer(_attribute.location,
+                              arrayBufferAttribute.size,
+                              arrayBufferAttribute.type,
+                              arrayBufferAttribute.normalized,
+                              arrayBuffer->getStride(),
+                              reinterpret_cast<GLvoid*>(arrayBufferAttribute.offset)
+                              );
+        glEnableVertexAttribArray(_attribute.location);
+    }
+
     // ===================================================================================================== \/
     // ============================================================================================ WRAPPERS \/
     // ===================================================================================================== \/
@@ -265,11 +327,22 @@ public:
         glBindVertexArray( mObjectName );
     }
 
-    inline void enable  (void) const { bind(); }
-    inline void disable (void) const { glBindVertexArray(0); }
+    //! Bind this VAO and remember the previously bound VAO
+    //! Note: every call to this method must be paired with a corresponding call to disable()
+    inline void enable(void) const
+    {
+        // remember old VAO
+        glGetIntegerv( GL_VERTEX_ARRAY_BINDING, &sPreviousVAOName );
+        bind();
+    }
+
+    //! Bind the VAO that was bound before the most recent enable() call
+    inline void disable(void) const
+    {
+        glBindVertexArray(sPreviousVAOName);
+    }
 
     //! Nothing has to be prepared for a render call
-    //! Note: the previously bound VAO will not get restored, instead VAO 0 will get bound
     inline void render (void)  const
     {
         enable();
@@ -280,6 +353,13 @@ public:
     //! Will select the matching draw call. Remember to enable first!
     void draw(void) const
     {
+        // TODO: fail silently?
+        if(mAttributes.empty())
+        {
+            ACGL::Utils::error() << "cannot draw VAO with no attributes attached" << std::endl;
+            return;
+        }
+
         if(mpElementArrayBuffer)
             drawElements();
         else
@@ -289,15 +369,29 @@ public:
     }
 
     //! Can be called directly instead of draw() iff the caller knows this is the correct call!
+    inline void drawElements(GLsizei _elements, const GLvoid* _offset = 0) const
+    {
+        glDrawElements(mMode, _elements, mpElementArrayBuffer->getType(), _offset);
+    }
+
+    //! Can be called directly instead of draw() iff the caller knows this is the correct call!
+    //! Draws all elements
     inline void drawElements(void) const
     {
-        mpElementArrayBuffer->draw();
+        drawElements(mpElementArrayBuffer->getElements());
     }
 
     //! Can be called directly instead of draw() iff the caller knows this is the correct call!
+    inline void drawArrays(GLsizei _elements, GLint _offset = 0) const
+    {
+        glDrawArrays(mMode, _offset, _elements);
+    }
+
+    //! Can be called directly instead of draw() iff the caller knows this is the correct call!
+    //! Draws all elements
     inline void drawArrays(void) const
     {
-        mAttributes[0].arrayBuffer->draw();
+        drawArrays(mAttributes[0].arrayBuffer->getElements());
     }
 
     // ===================================================================================================== \/
@@ -313,6 +407,10 @@ protected:
     GLuint                   mObjectName;          // OpenGL object name
     SharedElementArrayBuffer mpElementArrayBuffer; // optional EAB
     AttributeVec             mAttributes;          // vertex attributes
+    GLenum                   mMode;                // primitive type to render (e.g. GL_TRIANGLES)
+
+private:
+    static GLint             sPreviousVAOName;     // the VAO that was bound before the last enable() call
 };
 
 ACGL_SHARED_TYPEDEF(VertexArrayObject)

include/ACGL/OpenGL/Objects/VertexBufferObject.hh

@@ -78,9 +78,9 @@ public:
 
     inline void setAttributePointer(AttributeVec::size_type _indexInArray, GLuint _indexInShader) const
     {
-        mArrayBuffers[mAttributes[_indexInArray].bufferID]->setAttributePointer(
-                mAttributes[_indexInArray].attributeID,
-                _indexInShader);
+//        mArrayBuffers[mAttributes[_indexInArray].bufferID]->setAttributePointer(
+//                mAttributes[_indexInArray].attributeID,
+//                _indexInShader);
     }
 
     void validate (void) const;
@@ -106,12 +106,12 @@ public:
         const std::string& _attributeName,
         GLint _indexInShader)
     {
-        Attribute attribute = {
-            _name,
-            _bufferID,
-            mArrayBuffers[_bufferID]->getAttributeIndexByName(_attributeName),
-            _indexInShader};
-        mAttributes.push_back(attribute);
+//        Attribute attribute = {
+//            _name,
+//            _bufferID,
+//            mArrayBuffers[_bufferID]->getAttributeIndexByName(_attributeName),
+//            _indexInShader};
+//        mAttributes.push_back(attribute);
     }
 
     inline void removeAttributes(void)
@@ -143,7 +143,7 @@ public:
     {
         //If no ElementArrayBuffer is specified we use the convention that
         //the first ArrayBuffers determines the mode and the number of elements.
-        mArrayBuffers[0]->draw();
+//        mArrayBuffers[0]->draw();
     }
 
     void draw(void) const

src/ACGL/OpenGL/Controller/ArrayBufferControl.cc

@@ -9,12 +9,10 @@ using namespace ACGL::OpenGL;
 
 SharedArrayBuffer ArrayBufferControl::create(void)
 {
-    SharedArrayBuffer arrayBuffer(new ArrayBuffer(
-        mUsage,
-        mMode));
+    SharedArrayBuffer arrayBuffer(new ArrayBuffer());
     for(AttributeDefineVec::size_type i = 0; i < mAttributeDefines.size(); i++)
     {
-        arrayBuffer->attachAttribute(
+        arrayBuffer->defineAttribute(
             mAttributeDefines[i].name,
             mAttributeDefines[i].type,
             mAttributeDefines[i].dimension,
@@ -23,8 +21,7 @@ SharedArrayBuffer ArrayBufferControl::create(void)
     if(mpData != NULL)
     {
         arrayBuffer->bind();
-        arrayBuffer->setData(mpData, mElements);
-        //glBindBuffer(GL_ARRAY_BUFFER, 0);
+        arrayBuffer->setDataElements(mElements, mpData, mUsage);
     }
     return arrayBuffer;
 }

src/ACGL/OpenGL/Controller/ArrayBufferControlFileATB.cc

@@ -23,7 +23,6 @@ SharedArrayBuffer ArrayBufferControlFileATB::create(void)
     updateFileModificationTime();
 
     SharedArrayBuffer arrayBuffer = ArrayBufferControl().
-            mode(GL_TRIANGLES).
             usage(GL_STATIC_DRAW).
             create();
 
@@ -162,9 +161,10 @@ bool ArrayBufferControlFileATB::loadATB(SharedArrayBuffer& _arrayBuffer)
         fileStream.close();
 
         _arrayBuffer->removeAttributes();
-        _arrayBuffer->attachAttribute("aAttribute", GL_FLOAT, attributeDimension, GL_FALSE);
+        //_arrayBuffer->attachAttribute("aAttribute", GL_FLOAT, attributeDimension, GL_FALSE);
+        _arrayBuffer->defineAttribute("aAttribute", GL_FLOAT, attributeDimension, GL_FALSE);
         _arrayBuffer->bind();
-        _arrayBuffer->setData(&dataVector[0], index);
+        _arrayBuffer->setDataElements(index, &dataVector[0]);
 
         return true;
     }

src/ACGL/OpenGL/Objects/ArrayBuffer.cc

@@ -15,21 +15,3 @@ int_t ArrayBuffer::getAttributeIndexByName(const std::string& _nameInArray) cons
             return i;
     return -1;
 }
-
-void ArrayBuffer::render(void) const
-{
-    glBindBuffer(GL_ARRAY_BUFFER, mObjectName);
-    for(AttributeVec::size_type i = 0; i < mAttributes.size(); ++i)
-    {
-        setAttributePointer(i, i);
-        glEnableVertexAttribArray(i);
-        openGLRareError();
-    }
-    draw();
-    for(AttributeVec::size_type i = 0; i < mAttributes.size(); ++i)
-    {
-        glDisableVertexAttribArray(i);
-        openGLRareError();
-    }
-    glBindBuffer(GL_ARRAY_BUFFER, 0);
-}

src/ACGL/OpenGL/Objects/RenderObject.cc

@@ -66,6 +66,8 @@ void RenderObject::useShaderProgramObject (void) const
 
 void RenderObject::enableVertexBufferObject (void) const
 {
+    // TODO: deprecate
+    /*
     if(mpVertexBufferObject->getElementArrayBuffer())
         mpVertexBufferObject->getElementArrayBuffer()->bind();
 
@@ -85,10 +87,13 @@ void RenderObject::enableVertexBufferObject (void) const
             openGLRareError();
         }
     }
+    */
 }
 
 void RenderObject::disableVertexBufferObject (void) const
 {
+    // TODO: deprecate
+    /*
     int currentArrayBufferID = -1;
     for(AttributeMappingVec::size_type i = 0; i < mAttributeMappings.size(); i++)
     {
@@ -103,6 +108,7 @@ void RenderObject::disableVertexBufferObject (void) const
             openGLRareError();
         }
     }
+    */
 }
 
 #endif

src/ACGL/OpenGL/Objects/VertexArrayObject.cc

@@ -9,14 +9,14 @@
 using namespace ACGL;
 using namespace ACGL::OpenGL;
 
+GLint VertexArrayObject::sPreviousVAOName = 0;
+
 bool VertexArrayObject::isValid(void) const
 {
-    GLenum  mode;
     GLsizei elements = 0;
 
     if (mAttributes.size() > 0)
     {
-        mode     = mAttributes[0].arrayBuffer->getMode();
         elements = mAttributes[0].arrayBuffer->getElements();
     } else {
         Utils::error() << "VertexArrayObject has no attributes attached" << std::endl;
@@ -24,18 +24,13 @@ bool VertexArrayObject::isValid(void) const
     }
     if (mpElementArrayBuffer)
     {
-        mode     = mpElementArrayBuffer->getMode();
         elements = mpElementArrayBuffer->getElements();
     }
 
     for (AttributeVec::size_type i = 0; i < mAttributes.size(); ++i)
     {
-        if (mAttributes[0].arrayBuffer->getMode() != mode) {
-            Utils::error() << "VertexArrayObject validation failed: Attribute "<< i << " has different mode."<< std::endl;
-            return false;
-        }
-
-        if (mAttributes[0].arrayBuffer->getElements() != elements) {
+        if (mAttributes[0].arrayBuffer->getElements() != elements)
+        {
             Utils::error() << "VertexArrayObject validation failed: Attribute "<< i << " has different number of elements."<< std::endl;
             return false;
         }