added optional consistancy check in VAO binding

include/ACGL/OpenGL/Objects/ElementArrayBuffer.hh

@@ -19,6 +19,22 @@
  *       The original extension that introduced these two new buffer types was called
  *       ARB_vertex_buffer_object but the buffers itself are called ArrayBuffer and
  *       ElementArrayBuffer.
+ *
+ * WARNING: On word of warning about the EAB: Setting data (e.g. via setData() ) will
+ *          of course have to bind this buffer first. The binding of an EAB is a state
+ *          of the currently bound VAO, not a global OpenGL state so make sure you bind
+ *          the corresponding VAO or VAO 0 first, otherwise there will be a side-effect
+ *          of having the wrong EAB bound to some random VAO that just happened to be
+ *          bound by chance.
+ *          While this behaviour is also true for other buffers and textures, those are
+ *          bound to a global OpenGL state that gets changed all the time anyway so you
+ *          are probably used to rebind those before using while a VAO does not rebind
+ *          its own EAB before drawing as the binding is a state of the VAO.
+ *          In debug mode the VAO will check for this kind of inconsistancy that arise
+ *          from wrong usage.
+ *          Other reasons why a wrong EAB is set to a VAO: Driver bug (some older Intel
+ *          drivers did not store the EAB as a VAO state), manual binding (calling of bare
+ *          OpenGL commands).
  */
 
 #include <ACGL/ACGL.hh>

include/ACGL/OpenGL/Objects/VertexArrayObject.hh

@@ -43,7 +43,7 @@
 namespace ACGL{
 namespace OpenGL{
 
-// if a VAO is present either because GL 3.0+ is present or because extensions are expected, singnal this using this define
+// if a VAO is present either because GL 3.0+ is present or because extensions are expected, signal this using this define
 // for classes that expect VAO to be present!
 #define ACGL_SUPPORT_VAO
 class VertexArrayObject
@@ -144,21 +144,9 @@ public:
      * The _attributeLocation has to be lower than GL_MAX_VERTEX_ATTRIBS
      * An attribute location of -1 indicates that the attribute should should get the next free location
      */
-    inline void attachAttribute( const ConstSharedArrayBuffer& _arrayBuffer,
-                                 const std::string& _arrayBufferAttributeName,
-                                 GLint   _attributeLocation = -1)
-    {
-        int32_t arrayBufferAttribute = _arrayBuffer->getAttributeIndexByName(_arrayBufferAttributeName);
-
-        if(arrayBufferAttribute == -1) {
-            ACGL::Utils::error() << "can't attach attribute: attribute " << _arrayBufferAttributeName << " not present in array buffer" << std::endl;
-            return;
-        }
-
-        attachAttribute(_arrayBuffer,
-                        arrayBufferAttribute,
-                        _attributeLocation);
-    }
+    void attachAttribute( const ConstSharedArrayBuffer& _arrayBuffer,
+                          const std::string& _arrayBufferAttributeName,
+                                GLint        _attributeLocation = -1);
 
     //! if the location was already in use, the new attribute will replace it, if a free loc should be used but no location is free,
     //! nothing will get attached.
@@ -187,12 +175,13 @@ public:
     void detachAllAttributes();
 
     /**
-     * Query the attribute locations based on the attribute names in the ArrayBuffers.
-     * If they match, use the location reported from _locationMappings.
+     * Query the attribute names from the ArrayBuffers.
+     * If they match a name from _locationMappings, use the location from _locationMappings.
+     * All remaining locations will get a unique location as well (how this is calculated is undefined).
      */
     void setAttributeLocations( ConstSharedLocationMappings _locationMappings );
 
-    //! get a list of attribute locations and names that can be used to set up a ShaderProgram
+    //! get a list of attribute locations and names of this VAO. That list can then be used to set up a ShaderProgram.
     SharedLocationMappings getAttributeLocations() const;
 
 private:
@@ -209,10 +198,15 @@ private:
     // ===================================================================================================== \/
 public:
     //! Bind this VAO
-    inline void bind(void) const
+    //! If error checking is set to common or higher, some consistancy checks are made
+#ifdef ACGL_CHECK_COMMON_GL_ERRORS
+    void bind();
+#else
+    inline void bind()
     {
         glBindVertexArray( mObjectName );
     }
+#endif
 
     //! Nothing has to be prepared for a render call
     inline void render (void)

src/ACGL/OpenGL/Objects/VertexArrayObject.cc

@@ -28,6 +28,29 @@ VertexArrayObject::VertexArrayObject( GLenum _mode ) :
     mAttributes.resize( maxAttributes ); // reserve probably 16 slots, the size() can now be used to query the MAX_VERTEX_ATTRIBS
 }
 
+#ifdef ACGL_CHECK_COMMON_GL_ERRORS
+void VertexArrayObject::bind()
+{
+    glBindVertexArray( mObjectName );
+
+    // check if the EAB actually bound is the same as the one this ACGL object has stored:
+    GLint myEAB = 0;
+    GLint boundEAB = 0;
+    if (mpElementArrayBuffer) {
+        myEAB = mpElementArrayBuffer->getObjectName();
+    }
+    glGetIntegerv( GL_ELEMENT_ARRAY_BUFFER_BINDING, &boundEAB );
+
+    if (myEAB != boundEAB) {
+        error() << "wrong EAB bound to VAO " << mObjectName << " check EAB header for possible reasons!" << std::endl
+                << "trying to correct this..." << std::endl;
+        // don't just call attachElementArrayBuffer( mpElementArrayBuffer ); as it will call bind()
+        // an we end up in an infinite recursion...
+        glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, myEAB);
+    }
+}
+#endif
+
 void VertexArrayObject::attachElementArrayBuffer( const ConstSharedElementArrayBuffer& _elementArrayBuffer )
 {
     storeOldVAOandBind();
@@ -89,6 +112,22 @@ GLint VertexArrayObject::getFreeAttributeLocation()
     return -1;
 }
 
+void VertexArrayObject::attachAttribute( const ConstSharedArrayBuffer& _arrayBuffer,
+                             const std::string& _arrayBufferAttributeName,
+                             GLint   _attributeLocation )
+{
+    int32_t arrayBufferAttribute = _arrayBuffer->getAttributeIndexByName(_arrayBufferAttributeName);
+
+    if(arrayBufferAttribute == -1) {
+        ACGL::Utils::error() << "can't attach attribute: attribute " << _arrayBufferAttributeName << " not present in array buffer" << std::endl;
+        return;
+    }
+
+    attachAttribute(_arrayBuffer,
+                    arrayBufferAttribute,
+                    _attributeLocation);
+}
+
 void VertexArrayObject::attachAllAttributes( const ConstSharedArrayBuffer& _arrayBuffer )
 {
     ArrayBuffer::AttributeVec attributes = _arrayBuffer->getAttributes();
@@ -176,6 +215,7 @@ void VertexArrayObject::setAttributeLocations( ConstSharedLocationMappings _loca
         //        << " to " << location << std::endl;
     }
 
+    // query the currently bound VAO and restore that later to be side-effect free:
     storeOldVAOandBind();
     // disable all attributes & set the new ones
     for (GLuint i = 0; i < mAttributes.size(); ++i) {