OpenGL: Keep track of previously-attached VAOs and buffers when creating additional GL-backed Graphics contexts

Previously, code such as the following (where MyGLComponent is rendering
using an OpenGLContext) could result in GL errors, as the destruction of
the inner context unbound the array buffer and element array buffer
after use, instead of setting them to the previous values set up by the
outer context.

Additionally, a VAO was only set up in the OpenGLContext, so rendering
into a GL-backed LowLevel graphics context could fail if no VAO was
bound.

    void MyGLComponent::paint (juce::Graphics& g)
    {
        juce::Image image { juce::Image::PixelFormat::ARGB, width, height, false, juce::OpenGLImageType() };

        {
            juce::Graphics innerContext { image };
            // draw into innerContext
        }

        g.drawImage (image, juce::Rectangle<float> { width, height });
    }

modules/juce_opengl/opengl/juce_OpenGLContext.cpp

@@ -404,8 +404,6 @@ public:
                 context.currentRenderScale = currentAreaAndScale.scale;
                 context.renderer->renderOpenGL();
                 clearGLError();
-
-                bindVertexArray();
             }
 
             if (context.renderComponents)
@@ -486,13 +484,6 @@ public:
         }
     }
 
-    void bindVertexArray() noexcept
-    {
-        if (shouldUseCustomVAO())
-            if (vertexArrayObject != 0)
-                context.extensions.glBindVertexArray (vertexArrayObject);
-    }
-
     void checkViewportBounds()
     {
         auto screenBounds = component.getTopLevelComponent()->getScreenBounds();
@@ -537,7 +528,7 @@ public:
 
     void drawComponentBuffer()
     {
-        if (! isCoreProfile())
+        if (! OpenGLRendering::TraitsVAO::isCoreProfile())
             glEnable (GL_TEXTURE_2D);
 
        #if JUCE_WINDOWS
@@ -552,7 +543,6 @@ public:
         }
 
         glBindTexture (GL_TEXTURE_2D, cachedImageFrameBuffer.getTextureID());
-        bindVertexArray();
 
         const Rectangle<int> cacheBounds (cachedImageFrameBuffer.getWidth(), cachedImageFrameBuffer.getHeight());
         context.copyTexture (cacheBounds, cacheBounds, cacheBounds.getWidth(), cacheBounds.getHeight(), false);
@@ -634,12 +624,6 @@ public:
 
         gl::loadFunctions();
 
-        if (shouldUseCustomVAO())
-        {
-            context.extensions.glGenVertexArrays (1, &vertexArrayObject);
-            bindVertexArray();
-        }
-
        #if JUCE_DEBUG
         if (getOpenGLVersion() >= Version { 4, 3 } && glDebugMessageCallback != nullptr)
         {
@@ -675,40 +659,6 @@ public:
         return InitResult::success;
     }
 
-    bool isCoreProfile() const
-    {
-       #if JUCE_OPENGL_ES
-        return true;
-       #else
-        clearGLError();
-        GLint mask = 0;
-        glGetIntegerv (GL_CONTEXT_PROFILE_MASK, &mask);
-
-        // The context isn't aware of the profile mask, so it pre-dates the core profile
-        if (glGetError() == GL_INVALID_ENUM)
-            return false;
-
-        // Also assumes a compatibility profile if the mask is completely empty for some reason
-        return (mask & (GLint) GL_CONTEXT_CORE_PROFILE_BIT) != 0;
-       #endif
-    }
-
-    /*  Returns true if the context requires a non-zero vertex array object (VAO) to be bound.
-
-        If the context is a compatibility context, we can just pretend that VAOs don't exist,
-        and use the default VAO all the time instead. This provides a more consistent experience
-        in user code, which might make calls (like glVertexPointer()) that only work when VAO 0 is
-        bound in OpenGL 3.2+.
-    */
-    bool shouldUseCustomVAO() const
-    {
-       #if JUCE_OPENGL_ES
-        return false;
-       #else
-        return isCoreProfile();
-       #endif
-    }
-
     //==============================================================================
     struct BlockingWorker  : public OpenGLContext::AsyncWorker
     {
@@ -1453,7 +1403,7 @@ void* OpenGLContext::getRawContext() const noexcept
 bool OpenGLContext::isCoreProfile() const
 {
     auto* c = getCachedImage();
-    return c != nullptr && c->isCoreProfile();
+    return c != nullptr && OpenGLRendering::TraitsVAO::isCoreProfile();
 }
 
 OpenGLContext::CachedImage* OpenGLContext::getCachedImage() const noexcept
@@ -1570,6 +1520,8 @@ void OpenGLContext::copyTexture (const Rectangle<int>& targetClipArea,
 
     if (areShadersAvailable())
     {
+        OpenGLRendering::SavedBinding<OpenGLRendering::TraitsVAO> vaoBinding;
+
         struct OverlayShaderProgram  : public ReferenceCountedObject
         {
             OverlayShaderProgram (OpenGLContext& context)
@@ -1660,9 +1612,7 @@ void OpenGLContext::copyTexture (const Rectangle<int>& targetClipArea,
         auto& program = OverlayShaderProgram::select (*this);
         program.params.set ((float) contextWidth, (float) contextHeight, anchorPosAndTextureSize.toFloat(), flippedVertically);
 
-        GLuint vertexBuffer = 0;
-        extensions.glGenBuffers (1, &vertexBuffer);
-        extensions.glBindBuffer (GL_ARRAY_BUFFER, vertexBuffer);
+        OpenGLRendering::SavedBinding<OpenGLRendering::TraitsArrayBuffer> savedArrayBuffer;
         extensions.glBufferData (GL_ARRAY_BUFFER, sizeof (vertices), vertices, GL_STATIC_DRAW);
 
         auto index = (GLuint) program.params.positionAttribute.attributeID;
@@ -1673,11 +1623,7 @@ void OpenGLContext::copyTexture (const Rectangle<int>& targetClipArea,
         if (extensions.glCheckFramebufferStatus (GL_FRAMEBUFFER) == GL_FRAMEBUFFER_COMPLETE)
         {
             glDrawArrays (GL_TRIANGLE_STRIP, 0, 4);
-
-            extensions.glBindBuffer (GL_ARRAY_BUFFER, 0);
-            extensions.glUseProgram (0);
             extensions.glDisableVertexAttribArray (index);
-            extensions.glDeleteBuffers (1, &vertexBuffer);
         }
         else
         {

modules/juce_opengl/opengl/juce_OpenGLGraphicsContext.cpp

@@ -817,6 +817,114 @@ struct ShaderPrograms  : public ReferenceCountedObject
     MaskTextureProgram maskTexture;
 };
 
+//==============================================================================
+struct TraitsVAO
+{
+    static bool isCoreProfile()
+    {
+       #if JUCE_OPENGL_ES
+        return true;
+       #else
+        clearGLError();
+        GLint mask = 0;
+        glGetIntegerv (GL_CONTEXT_PROFILE_MASK, &mask);
+
+        // The context isn't aware of the profile mask, so it pre-dates the core profile
+        if (glGetError() == GL_INVALID_ENUM)
+            return false;
+
+        // Also assumes a compatibility profile if the mask is completely empty for some reason
+        return (mask & (GLint) GL_CONTEXT_CORE_PROFILE_BIT) != 0;
+       #endif
+    }
+
+    /*  Returns true if the context requires a non-zero vertex array object (VAO) to be bound.
+
+        If the context is a compatibility context, we can just pretend that VAOs don't exist,
+        and use the default VAO all the time instead. This provides a more consistent experience
+        in user code, which might make calls (like glVertexPointer()) that only work when VAO 0 is
+        bound in OpenGL 3.2+.
+    */
+    static bool shouldUseCustomVAO()
+    {
+       #if JUCE_OPENGL_ES
+        return false;
+       #else
+        return isCoreProfile();
+       #endif
+    }
+
+    static constexpr auto value = GL_VERTEX_ARRAY_BINDING;
+    static constexpr auto& gen = glGenVertexArrays;
+    static constexpr auto& del = glDeleteVertexArrays;
+    template <typename T>
+    static void bind (T x) { gl::glBindVertexArray (static_cast<GLuint> (x)); }
+    static constexpr auto predicate = shouldUseCustomVAO;
+};
+
+struct TraitsArrayBuffer
+{
+    static constexpr auto value = GL_ARRAY_BUFFER_BINDING;
+    static constexpr auto& gen = glGenBuffers;
+    static constexpr auto& del = glDeleteBuffers;
+    template <typename T>
+    static void bind (T x) { gl::glBindBuffer (GL_ARRAY_BUFFER, static_cast<GLuint> (x)); }
+    static bool predicate() { return true; }
+};
+
+struct TraitsElementArrayBuffer
+{
+    static constexpr auto value = GL_ELEMENT_ARRAY_BUFFER_BINDING;
+    static constexpr auto& gen = glGenBuffers;
+    static constexpr auto& del = glDeleteBuffers;
+    template <typename T>
+    static void bind (T x) { gl::glBindBuffer (GL_ELEMENT_ARRAY_BUFFER, static_cast<GLuint> (x)); }
+    static bool predicate() { return true; }
+};
+
+template <typename Traits>
+class SavedBinding
+{
+public:
+    SavedBinding() = default;
+
+    ~SavedBinding()
+    {
+        if (! Traits::predicate())
+            return;
+
+        Traits::bind (values.previous);
+        Traits::del (1, &values.current);
+    }
+
+    void bind() const { Traits::bind (values.current); }
+
+    JUCE_DECLARE_NON_COPYABLE (SavedBinding)
+    JUCE_DECLARE_NON_MOVEABLE (SavedBinding)
+
+private:
+    struct Values
+    {
+        GLint previous{};
+        GLuint current{};
+    };
+
+    Values values = []
+    {
+        if (! Traits::predicate())
+            return Values{};
+
+        GLint previous{};
+        glGetIntegerv (Traits::value, &previous);
+
+        GLuint current{};
+        Traits::gen (1, &current);
+        Traits::bind (current);
+
+        return Values { previous, current };
+    }();
+};
+
 //==============================================================================
 struct StateHelpers
 {
@@ -1164,9 +1272,6 @@ struct StateHelpers
         ~ShaderQuadQueue() noexcept
         {
             static_assert (sizeof (VertexInfo) == 8, "Sanity check VertexInfo size");
-            context.extensions.glBindBuffer (GL_ARRAY_BUFFER, 0);
-            context.extensions.glBindBuffer (GL_ELEMENT_ARRAY_BUFFER, 0);
-            context.extensions.glDeleteBuffers (2, buffers);
         }
 
         void initialise() noexcept
@@ -1190,10 +1295,10 @@ struct StateHelpers
                 indexData[i + 5] = (GLushort) (v + 3);
             }
 
-            context.extensions.glGenBuffers (2, buffers);
-            context.extensions.glBindBuffer (GL_ELEMENT_ARRAY_BUFFER, buffers[0]);
+            savedElementArrayBuffer.bind();
             context.extensions.glBufferData (GL_ELEMENT_ARRAY_BUFFER, sizeof (indexData), indexData, GL_STATIC_DRAW);
-            context.extensions.glBindBuffer (GL_ARRAY_BUFFER, buffers[1]);
+
+            savedArrayBuffer.bind();
             context.extensions.glBufferData (GL_ARRAY_BUFFER, sizeof (vertexData), vertexData, GL_STREAM_DRAW);
             JUCE_CHECK_OPENGL_ERROR
         }
@@ -1277,7 +1382,8 @@ struct StateHelpers
 
         enum { maxNumQuads = 256 };
 
-        GLuint buffers[2];
+        SavedBinding<TraitsArrayBuffer> savedArrayBuffer;
+        SavedBinding<TraitsElementArrayBuffer> savedElementArrayBuffer;
         VertexInfo vertexData[maxNumQuads * 4];
         GLushort indexData[maxNumQuads * 6];
         const OpenGLContext& context;
@@ -1579,6 +1685,7 @@ struct GLState
 
 private:
     GLuint previousFrameBufferTarget;
+    SavedBinding<TraitsVAO> savedVAOBinding;
 };
 
 //==============================================================================