GL: deduplicate global defaultFramebuffer state across shared libraries.

Can't really automatically test this, unfortunately, as the windowless
tests don't operate on the default framebuffer.

doc/changelog.dox

@@ -283,6 +283,9 @@ See also:
     flag for integration with custom-created OpenGL contexts. See also
     [mosra/magnum#493](https://github.com/mosra/magnum/pull/493) and
     [mosra/magnum#494](https://github.com/mosra/magnum/pull/494).
+-   The @ref GL::defaultFramebuffer global now deduplicates its state when a
+    statically-built Magnum is linked to multiple shared libraries, following
+    what the global @ref GL::Context::current() already does
 
 @subsubsection changelog-latest-changes-math Math library
 

src/Magnum/GL/AbstractFramebuffer.cpp

@@ -285,12 +285,23 @@ void AbstractFramebuffer::blitImplementationNV(AbstractFramebuffer& source, Abst
 }
 #endif
 
+Range2Di AbstractFramebuffer::viewport() const {
+    /* For default framebuffer the viewport is stored inside the state tracker
+       instead. See the _viewport variable docs for details. */
+    return _id ? _viewport : Context::current().state().framebuffer.defaultViewport;
+}
+
 AbstractFramebuffer& AbstractFramebuffer::setViewport(const Range2Di& rectangle) {
+    Implementation::FramebufferState& state = Context::current().state().framebuffer;
+
     CORRADE_INTERNAL_ASSERT(rectangle != Implementation::FramebufferState::DisengagedViewport);
-    _viewport = rectangle;
+
+    /* For default framebuffer the viewport is stored inside the state tracker
+       instead. See the _viewport variable docs for details. */
+    (_id ? _viewport : state.defaultViewport) = rectangle;
 
     /* Update the viewport if the framebuffer is currently bound */
-    if(Context::current().state().framebuffer.drawBinding == _id)
+    if(state.drawBinding == _id)
         setViewportInternal();
 
     return *this;
@@ -299,16 +310,20 @@ AbstractFramebuffer& AbstractFramebuffer::setViewport(const Range2Di& rectangle)
 void AbstractFramebuffer::setViewportInternal() {
     Implementation::FramebufferState& state = Context::current().state().framebuffer;
 
-    CORRADE_INTERNAL_ASSERT(_viewport != Implementation::FramebufferState::DisengagedViewport);
+    /* For default framebuffer the viewport is stored inside the state tracker
+       instead. See the _viewport variable docs for details. */
+    const Range2Di& viewport = _id ? _viewport : Context::current().state().framebuffer.defaultViewport;
+
+    CORRADE_INTERNAL_ASSERT(viewport != Implementation::FramebufferState::DisengagedViewport);
     CORRADE_INTERNAL_ASSERT(state.drawBinding == _id);
 
     /* Already up-to-date, nothing to do */
-    if(state.viewport == _viewport)
+    if(state.viewport == viewport)
         return;
 
     /* Update the state and viewport */
-    state.viewport = _viewport;
+    state.viewport = viewport;
-    glViewport(_viewport.left(), _viewport.bottom(), _viewport.sizeX(), _viewport.sizeY());
+    glViewport(viewport.left(), viewport.bottom(), viewport.sizeX(), viewport.sizeY());
 }
 
 AbstractFramebuffer& AbstractFramebuffer::clear(const FramebufferClearMask mask) {

src/Magnum/GL/AbstractFramebuffer.h

@@ -262,7 +262,7 @@ class MAGNUM_GL_EXPORT AbstractFramebuffer {
         void bind();
 
         /** @brief Viewport rectangle */
-        Range2Di viewport() const { return _viewport; }
+        Range2Di viewport() const;
 
         /**
          * @brief Set viewport
@@ -740,8 +740,10 @@ class MAGNUM_GL_EXPORT AbstractFramebuffer {
     #else
     protected:
     #endif
-        /* Used by the (constexpr) DefaultFramebuffer constructor and both
+        /* Used by the (constexpr) DefaultFramebuffer constructor (which passes
-           the NoCreate and normal constructor of Framebuffer */
+           in an empty viewport anyway so we don't need to pass it to the state
+           tracker instead) and both the NoCreate and normal constructor of
+           Framebuffer */
         constexpr explicit AbstractFramebuffer(GLuint id, const Range2Di& viewport, ObjectFlags flags) noexcept: _id{id}, _viewport{viewport}, _flags{flags} {}
 
         ~AbstractFramebuffer() = default;
@@ -767,6 +769,16 @@ class MAGNUM_GL_EXPORT AbstractFramebuffer {
         void MAGNUM_GL_LOCAL setViewportInternal();
 
         GLuint _id;
+        /* Used only if _id != 0, for default framebuffer the viewport is
+           stored inside the state tracker. This is done in order to avoid race
+           conditions when using Magnum from multiple threads (due to the
+           global GL::defaultFramebuffer) and to work around issues caused by
+           this very global symbol being duplicated when statically-compiled
+           Magnum is linked to multiple shared libraries (such as every shared
+           library thinking the default viewport is different). It's stored in
+           the state tracker because it wouldn't be possible to employ our
+           symbol deduplication tricks on GL::defaultFramebuffer on all
+           platforms without turning it into a pointer or a function. */
         Range2Di _viewport;
         ObjectFlags _flags;
 

src/Magnum/GL/Context.cpp

@@ -41,7 +41,6 @@
 #ifndef MAGNUM_TARGET_WEBGL
 #include "Magnum/GL/DebugOutput.h"
 #endif
-#include "Magnum/GL/DefaultFramebuffer.h"
 #include "Magnum/GL/Extensions.h"
 #include "Magnum/GL/Framebuffer.h"
 #include "Magnum/GL/Mesh.h"
@@ -982,13 +981,26 @@ bool Context::tryCreate(const Configuration& configuration) {
             if(!workaround.second) Debug(output) << "   " << workaround.first;
     }
 
-    /** @todo Get rid of these */
+    /* Fetch default framebuffer size and set up default clear color. If we are
-    /* Initialize functionality based on current OpenGL version and extensions.
+       on a windowless context don't bother retrieving the default framebuffer
-       If we are on a windowless context don't touch the default framebuffer
+       viewport. This used to touch the GL::defaultFramebuffer global and the
-       to avoid potential race conditions with default framebuffer on another
+       Windowless flag got originally added to prevent a race when initializing
-       thread. */
+       Magnum from multiple threads. But in order to work around issues caused
-    if(!(_configurationFlags & Configuration::Flag::Windowless))
+       by this very global symbol being duplicated when statically-compiled
-        DefaultFramebuffer::initializeContextBasedFunctionality(*this);
+       Magnum is linked to multiple shared libraries (such as every shared
+       library thinking the default viewport is different), the viewport had to
+       be moved to the state tracker, which is immune against symbol
+       duplication. It wouldn't be possible to deduplicate the
+       GL::defaultFramebuffer global itself without turning it into a pointer
+       or a function. */
+    if(!(_configurationFlags & Configuration::Flag::Windowless)) {
+        Implementation::FramebufferState& state = _state->framebuffer;
+        GLint viewport[4];
+        glGetIntegerv(GL_VIEWPORT, viewport);
+        state.defaultViewport = state.viewport = Range2Di::fromSize({viewport[0], viewport[1]}, {viewport[2], viewport[3]});
+        CORRADE_INTERNAL_ASSERT(state.defaultViewport != Implementation::FramebufferState::DisengagedViewport);
+    }
+    /** @todo Get rid of this as well somehow */
     Renderer::initializeContextBasedFunctionality();
 
     /* Enable GPU validation, if requested */

src/Magnum/GL/Context.h

@@ -925,9 +925,8 @@ class MAGNUM_GL_EXPORT Context::Configuration {
 
             /**
              * Treat the context as windowless, assume there's no default
-             * framebuffer and thus don't touch @ref defaultFramebuffer in any
+             * framebuffer and thus don't initialize or touch
-             * way. Useful for preventing race conditions when creating OpenGL
+             * @ref defaultFramebuffer in any way.
-             * contexts in background threads.
              *
              * This flag is implicitly enabled in all
              * @ref Platform::WindowlessEglContext::Configuration "Platform::Windowless*Application::Configuration",

src/Magnum/GL/DefaultFramebuffer.cpp

@@ -113,16 +113,6 @@ void DefaultFramebuffer::invalidate(std::initializer_list<InvalidationAttachment
 }
 #endif
 
-void DefaultFramebuffer::initializeContextBasedFunctionality(Context& context) {
-    Implementation::FramebufferState& state = context.state().framebuffer;
-
-    /* Initial framebuffer size */
-    GLint viewport[4];
-    glGetIntegerv(GL_VIEWPORT, viewport);
-    defaultFramebuffer._viewport = state.viewport = Range2Di::fromSize({viewport[0], viewport[1]}, {viewport[2], viewport[3]});
-    CORRADE_INTERNAL_ASSERT(defaultFramebuffer._viewport != Implementation::FramebufferState::DisengagedViewport);
-}
-
 #ifndef DOXYGEN_GENERATING_OUTPUT
 Debug& operator<<(Debug& debug, const DefaultFramebuffer::Status value) {
     debug << "GL::DefaultFramebuffer::Status" << Debug::nospace;

src/Magnum/GL/DefaultFramebuffer.h

@@ -72,8 +72,6 @@ functions @ref checkStatus(), @ref mapForDraw(), @ref mapForRead() and
 See their respective documentation for more information.
 */
 class MAGNUM_GL_EXPORT DefaultFramebuffer: public AbstractFramebuffer {
-    friend Context;
-
     public:
         /**
          * @brief Status
@@ -495,9 +493,6 @@ class MAGNUM_GL_EXPORT DefaultFramebuffer: public AbstractFramebuffer {
         }
         #endif
         #endif
-
-    private:
-        static void MAGNUM_GL_LOCAL initializeContextBasedFunctionality(Context& context);
 };
 
 /** @brief Default framebuffer instance */

src/Magnum/GL/Implementation/FramebufferState.h

@@ -114,7 +114,7 @@ struct FramebufferState {
     #ifndef MAGNUM_TARGET_GLES
     GLint maxDualSourceDrawBuffers;
     #endif
-    Range2Di viewport;
+    Range2Di defaultViewport, viewport;
     Vector2i maxViewportSize;
 };