From f012f8c28013cf2cfe5a62cbdac888b07c472ce6 Mon Sep 17 00:00:00 2001 From: reuk Date: Tue, 6 Jun 2023 19:53:55 +0100 Subject: [PATCH] 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 { width, height }); } --- .../juce_opengl/opengl/juce_OpenGLContext.cpp | 64 +-------- .../opengl/juce_OpenGLGraphicsContext.cpp | 121 +++++++++++++++++- 2 files changed, 119 insertions(+), 66 deletions(-) diff --git a/modules/juce_opengl/opengl/juce_OpenGLContext.cpp b/modules/juce_opengl/opengl/juce_OpenGLContext.cpp index 2c999c1298..b1c54c2565 100644 --- a/modules/juce_opengl/opengl/juce_OpenGLContext.cpp +++ b/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 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& targetClipArea, if (areShadersAvailable()) { + OpenGLRendering::SavedBinding vaoBinding; + struct OverlayShaderProgram : public ReferenceCountedObject { OverlayShaderProgram (OpenGLContext& context) @@ -1660,9 +1612,7 @@ void OpenGLContext::copyTexture (const Rectangle& 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 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& 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 { diff --git a/modules/juce_opengl/opengl/juce_OpenGLGraphicsContext.cpp b/modules/juce_opengl/opengl/juce_OpenGLGraphicsContext.cpp index 172b1e9840..09bc9dbb7a 100644 --- a/modules/juce_opengl/opengl/juce_OpenGLGraphicsContext.cpp +++ b/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 + static void bind (T x) { gl::glBindVertexArray (static_cast (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 + static void bind (T x) { gl::glBindBuffer (GL_ARRAY_BUFFER, static_cast (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 + static void bind (T x) { gl::glBindBuffer (GL_ELEMENT_ARRAY_BUFFER, static_cast (x)); } + static bool predicate() { return true; } +}; + +template +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, ¤t); + 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 savedArrayBuffer; + SavedBinding savedElementArrayBuffer; VertexInfo vertexData[maxNumQuads * 4]; GLushort indexData[maxNumQuads * 6]; const OpenGLContext& context; @@ -1579,6 +1685,7 @@ struct GLState private: GLuint previousFrameBufferTarget; + SavedBinding savedVAOBinding; }; //==============================================================================