Improve state tracker resetting functionality.

doc/changelog.dox

@@ -346,6 +346,7 @@ See also:
     @ref Image::release() instead.
 -   @ref Buffer::map() now returns @ref Corrade::Containers::ArrayView instead
     of a plain pointer for better security
+-   Improved @ref Context::resetState() to better handle corner cases with VAOs
 -   Graceful handling of broken GL contexts
 -   Behavior of @ref Version::GLES200 and upwards on desktop OpenGL is changed
     to request an ES dialect of GLSL when used in @ref Shader (instead of a

src/Magnum/Context.cpp

@@ -836,6 +836,8 @@ void Context::resetState(const States states) {
         _state->framebuffer->reset();
     if(states & State::Meshes)
         _state->mesh->reset();
+    if(states & State::MeshVao)
+        _state->mesh->bindVAOImplementation(0);
 
     if(states & State::PixelStorage) {
         _state->renderer->unpackPixelStorage.reset();

src/Magnum/Context.h

@@ -178,7 +178,7 @@ class MAGNUM_EXPORT Context {
         /**
          * @brief State to reset
          *
-         * @see @ref States, @ref resetState()
+         * @see @ref States, @ref resetState(), @ref opengl-state-tracking
          */
         enum class State: UnsignedInt {
             /** Reset tracked buffer-related bindings and state */
@@ -190,22 +190,63 @@ class MAGNUM_EXPORT Context {
             /** Reset tracked mesh-related bindings */
             Meshes = 1 << 2,
 
+            /**
+             * Unbind currently bound VAO.
+             *
+             * Magnum by default uses VAOs --- each time a @ref Mesh is drawn
+             * or configured, its VAO is bound, but it is *not* unbound
+             * afterwards to avoid needless state changes. This may introduce
+             * problems when using third-party OpenGL code --- it may break
+             * internal state of a mesh that was used the most recently.
+             * Similar issue can happen the other way. Calling @ref resetState()
+             * with @ref State::MeshVao included unbounds any currently bound
+             * VAO to fix such case.
+             */
+            MeshVao = 1 << 3,
+
             /** Reset tracked pixel storage-related state */
-            PixelStorage = 1 << 3,
+            PixelStorage = 1 << 4,
 
             /** Reset tracked renderer-related state */
-            Renderer = 1 << 4,
+            Renderer = 1 << 5,
 
             /** Reset tracked shader-related bindings */
-            Shaders = 1 << 5,
+            Shaders = 1 << 6,
 
             /** Reset tracked texture-related bindings and state */
-            Textures = 1 << 6,
+            Textures = 1 << 7,
 
             #ifndef MAGNUM_TARGET_GLES2
             /** Reset tracked transform feedback-related bindings */
-            TransformFeedback = 1 << 7
+            TransformFeedback = 1 << 8,
             #endif
+
+            /**
+             * Reset state on entering section with external OpenGL code.
+             *
+             * Resets all state that could cause external code to accidentally
+             * modify Magnum objects. This includes only @ref State::MeshVao.
+             */
+            EnterExternal = UnsignedInt(State::MeshVao),
+
+            /**
+             * Reset state on exiting section with external OpenGL code.
+             *
+             * Resets Magnum state tracker to avoid being confused by external
+             * state changes. This resets all states.
+             */
+            ExitExternal =
+                UnsignedInt(State::Buffers)|
+                UnsignedInt(State::Framebuffers)|
+                UnsignedInt(State::Meshes)|
+                UnsignedInt(State::MeshVao)|
+                UnsignedInt(State::PixelStorage)|
+                UnsignedInt(State::Renderer)|
+                UnsignedInt(State::Shaders)|
+                UnsignedInt(State::Textures)
+                #ifndef MAGNUM_TARGET_GLES2
+                |UnsignedInt(State::TransformFeedback)
+                #endif
         };
 
         /**
@@ -554,12 +595,21 @@ class MAGNUM_EXPORT Context {
 
         /**
          * @brief Reset internal state tracker
-         * @param states    Tracked states to reset. Default is all state.
          *
          * The engine internally tracks object bindings and other state to
          * avoid redundant OpenGL calls. In some cases (e.g. when non-Magnum
          * code makes GL calls) the internal tracker no longer reflects actual
-         * state and needs to be reset to avoid strange issues.
+         * state. Equivalently the third party code can cause accidental
+         * modifications of Magnum objects. It's thus advised to call this
+         * function as a barrier between Magnum code and third-party GL code.
+         *
+         * The default, when calling this function with no parameters, will
+         * reset all state. That's the safest option, but may have considerable
+         * performance impact when third-party and Magnum code is combined very
+         * often. For greater control it's possible to reset only particular
+         * states from the @ref State enum.
+         *
+         * See also @ref opengl-state-tracking for more information.
          */
         void resetState(States states = ~States{});
 

src/Magnum/Mesh.h

@@ -1044,7 +1044,7 @@ class MAGNUM_EXPORT Mesh: public AbstractObject {
         #endif
 
         /* Unconditionally binds a specified VAO and updates the state tracker.
-           Used also in Buffer::bindSomewhereInternal(). */
+           Used also in Buffer::bindSomewhereInternal() and Context::resetState(). */
         static void MAGNUM_LOCAL bindVAOImplementationDefault(GLuint id);
         static void MAGNUM_LOCAL bindVAOImplementationVAO(GLuint id);
 

src/Magnum/Test/MeshGLTest.cpp

@@ -113,6 +113,7 @@ struct MeshGLTest: OpenGLTester {
     void setIndexBufferUnsignedInt();
 
     void unbindVAOWhenSettingIndexBufferData();
+    void unbindVAOBeforeEnteringExternalSection();
 
     #ifndef MAGNUM_TARGET_GLES
     void setBaseVertex();
@@ -209,6 +210,7 @@ MeshGLTest::MeshGLTest() {
               &MeshGLTest::setIndexBufferUnsignedInt,
 
               &MeshGLTest::unbindVAOWhenSettingIndexBufferData,
+              &MeshGLTest::unbindVAOBeforeEnteringExternalSection,
 
               #ifndef MAGNUM_TARGET_GLES
               &MeshGLTest::setBaseVertex,
@@ -1741,6 +1743,52 @@ void MeshGLTest::unbindVAOWhenSettingIndexBufferData() {
     CORRADE_COMPARE(value, 92);
 }
 
+void MeshGLTest::unbindVAOBeforeEnteringExternalSection() {
+    #ifndef MAGNUM_TARGET_GLES
+    if(!Context::current().isExtensionSupported<Extensions::GL::ARB::vertex_array_object>())
+        CORRADE_SKIP(Extensions::GL::ARB::vertex_array_object::string() + std::string(" is not available."));
+    #elif defined(MAGNUM_TARGET_GLES2)
+    if(!Context::current().isExtensionSupported<Extensions::GL::OES::vertex_array_object>())
+        CORRADE_SKIP(Extensions::GL::OES::vertex_array_object::string() + std::string(" is not available."));
+    #endif
+
+    typedef Attribute<0, Float> Attribute;
+
+    const Float data[] = { -0.7f, Math::unpack<Float, UnsignedByte>(92), Math::unpack<Float, UnsignedByte>(32) };
+    Buffer buffer{Buffer::TargetHint::Array};
+    buffer.setData(data, BufferUsage::StaticDraw);
+
+    Buffer indices{Buffer::TargetHint::ElementArray};
+    indices.setData(std::vector<UnsignedByte>{5, 0}, BufferUsage::StaticDraw);
+
+    Mesh mesh;
+    mesh.addVertexBuffer(buffer, 4, Attribute{})
+        .setIndexBuffer(indices, 0, Mesh::IndexType::UnsignedByte);
+
+    {
+        /* Comment this out to watch the world burn */
+        Context::current().resetState(Context::State::MeshVao);
+
+        glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
+
+        /* Be nice to the other tests */
+        Context::current().resetState(Context::State::ExitExternal);
+    }
+
+    MAGNUM_VERIFY_NO_ERROR();
+
+    const auto value = Checker(FloatShader("float", "vec4(valueInterpolated, 0.0, 0.0, 0.0)"),
+        #ifndef MAGNUM_TARGET_GLES2
+        RenderbufferFormat::RGBA8,
+        #else
+        RenderbufferFormat::RGBA4,
+        #endif
+        mesh).get<UnsignedByte>(PixelFormat::RGBA, PixelType::UnsignedByte);
+
+    MAGNUM_VERIFY_NO_ERROR();
+    CORRADE_COMPARE(value, 92);
+}
+
 #ifndef MAGNUM_TARGET_GLES
 void MeshGLTest::setBaseVertex() {
     if(!Context::current().isExtensionSupported<Extensions::GL::ARB::draw_elements_base_vertex>())