From ee16c251250db817ac4c63f694324a7395821405 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= <mosra@centrum.cz>
Date: Thu, 28 Feb 2019 23:55:57 +0100
Subject: [PATCH] GL: further harden VAO and index buffer binding state
 interaction tests.

This one crashes. Turns out the 169031fb7bf30e2c98c81cbf865aa5a4662aa716
contained a random temporary test state instead of the real solution
(and so the comment didn't even match the code, it should have been
resetting that to 0). That also made some tests fail with DSA disabled,
but none of the tests were actual Mesh tests, just accidentally hitting
the problematic code path.

I took the opportunity to look at this more closely and investigate
*why* this failed -- turns out, in setIndexBuffer(), I was resetting
the state tracker to a VAO state that was about to be set in the very
next step, and then, when doing that next step the state tracker
"optimized away" the state change because it thought it was already done
(even though it wasn't). The new test in MeshGLTest covers this
particular case.
---
 src/Magnum/GL/Test/MeshGLTest.cpp | 51 +++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/src/Magnum/GL/Test/MeshGLTest.cpp b/src/Magnum/GL/Test/MeshGLTest.cpp
index 31da858a2..421ce9810 100644
--- a/src/Magnum/GL/Test/MeshGLTest.cpp
+++ b/src/Magnum/GL/Test/MeshGLTest.cpp
@@ -132,6 +132,7 @@ struct MeshGLTest: OpenGLTester {
 
     void unbindVAOWhenSettingIndexBufferData();
     void unbindIndexBufferWhenBindingVao();
+    void resetIndexBufferBindingWhenBindingVao();
     void unbindVAOBeforeEnteringExternalSection();
     void bindScratchVaoWhenEnteringExternalSection();
 
@@ -253,6 +254,7 @@ MeshGLTest::MeshGLTest() {
 
               &MeshGLTest::unbindVAOWhenSettingIndexBufferData,
               &MeshGLTest::unbindIndexBufferWhenBindingVao,
+              &MeshGLTest::resetIndexBufferBindingWhenBindingVao,
               &MeshGLTest::unbindVAOBeforeEnteringExternalSection,
               &MeshGLTest::bindScratchVaoWhenEnteringExternalSection,
 
@@ -2128,6 +2130,55 @@ void MeshGLTest::unbindIndexBufferWhenBindingVao() {
     CORRADE_COMPARE(value, 92);
 }
 
+void MeshGLTest::resetIndexBufferBindingWhenBindingVao() {
+    #ifndef MAGNUM_TARGET_GLES
+    if(!Context::current().isExtensionSupported<Extensions::ARB::vertex_array_object>())
+        CORRADE_SKIP(Extensions::ARB::vertex_array_object::string() + std::string(" is not available."));
+    if(Context::current().isExtensionSupported<Extensions::ARB::direct_state_access>())
+        CORRADE_SKIP(Extensions::ARB::direct_state_access::string() + std::string(" is active with circumvents the issue tested here."));
+    if(Context::current().isExtensionSupported<Extensions::EXT::direct_state_access>())
+        CORRADE_SKIP(Extensions::EXT::direct_state_access::string() + std::string(" is active with circumvents the issue tested here."));
+    #elif defined(MAGNUM_TARGET_GLES2)
+    if(!Context::current().isExtensionSupported<Extensions::OES::vertex_array_object>())
+        CORRADE_SKIP(Extensions::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 vertices{Buffer::TargetHint::Array};
+    vertices.setData(data);
+
+    /* Create an indexed mesh */
+    Mesh indexed;
+    indexed.addVertexBuffer(vertices, 0, Attribute{});
+
+    /* Create an index buffer and fill it (the VAO is bound now, so it'll get
+       unbound to avoid messing with its state). */
+    Buffer indices{Buffer::TargetHint::ElementArray};
+    indices.setData(std::vector<UnsignedByte>{5, 1});
+
+    /* Add the index buffer. The VAO is unbound, so it gets bound. That resets
+       the element array buffer binding and then the buffer gets bound to the
+       VAO. */
+    indexed.setIndexBuffer(indices, 0, MeshIndexType::UnsignedByte);
+
+    MAGNUM_VERIFY_NO_GL_ERROR();
+
+    /* Draw the indexed mesh. The index buffer should be correctly bound,
+       picking the second vertex with value of 92. */
+    const auto value = Checker(FloatShader("float", "vec4(valueInterpolated, 0.0, 0.0, 0.0)"),
+        #ifndef MAGNUM_TARGET_GLES2
+        RenderbufferFormat::RGBA8,
+        #else
+        RenderbufferFormat::RGBA4,
+        #endif
+        indexed).get<UnsignedByte>(PixelFormat::RGBA, PixelType::UnsignedByte);
+
+    MAGNUM_VERIFY_NO_GL_ERROR();
+    CORRADE_COMPARE(value, 92);
+}
+
 void MeshGLTest::unbindVAOBeforeEnteringExternalSection() {
     #ifndef MAGNUM_TARGET_GLES
     if(!Context::current().isExtensionSupported<Extensions::ARB::vertex_array_object>())