From 46df57f6d551d0961477b2e0192b5cb391401150 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 27 Feb 2021 11:46:43 +0100 Subject: [PATCH] GL,Platform: introduce GL::Context::Configuration::Flag::Windowless. With this flag set (which is done implicitly for all windowless apps and, conversely, not done for all windowed apps), the default framebuffer state isn't touched in any way, which should avoid potential race conditions with default framebuffer on another thread. --- doc/changelog.dox | 7 +++++++ src/Magnum/GL/Context.cpp | 12 ++++++++++-- src/Magnum/GL/Context.h | 14 ++++++++++++++ src/Magnum/GL/Framebuffer.cpp | 19 ++++++++++++------- src/Magnum/Platform/AbstractXApplication.h | 4 +++- src/Magnum/Platform/AndroidApplication.h | 4 +++- src/Magnum/Platform/EmscriptenApplication.h | 4 +++- src/Magnum/Platform/GlfwApplication.h | 4 +++- src/Magnum/Platform/Sdl2Application.h | 4 +++- .../Platform/WindowlessCglApplication.cpp | 4 ++++ .../Platform/WindowlessCglApplication.h | 6 +++++- .../Platform/WindowlessEglApplication.cpp | 1 + .../Platform/WindowlessEglApplication.h | 4 +++- .../Platform/WindowlessGlxApplication.cpp | 1 + .../Platform/WindowlessGlxApplication.h | 4 +++- .../Platform/WindowlessIosApplication.h | 6 +++++- .../Platform/WindowlessIosApplication.mm | 4 ++++ .../Platform/WindowlessWglApplication.cpp | 1 + .../Platform/WindowlessWglApplication.h | 4 +++- .../WindowlessWindowsEglApplication.cpp | 4 ++++ .../WindowlessWindowsEglApplication.h | 6 +++++- 21 files changed, 97 insertions(+), 20 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index b7b98df1f..eaed80ba0 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -196,6 +196,13 @@ See also: now advertised as a @cpp "intel-windows-chatty-shader-compiler" @ce workaround, instead of being done silently. See @ref opengl-workarounds for more information. +- @ref Platform::WindowlessEglApplication "Platform::Windowless*Application" + instances no longer touch default framebuffer state in order to avoid + potential race conditions with a windowed context on another thread. This + is also exposed via a new @ref GL::Context::Configuration::Flag::Windowless + 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). @subsubsection changelog-latest-changes-math Math library diff --git a/src/Magnum/GL/Context.cpp b/src/Magnum/GL/Context.cpp index 288a837b9..d1069ac8c 100644 --- a/src/Magnum/GL/Context.cpp +++ b/src/Magnum/GL/Context.cpp @@ -785,6 +785,10 @@ bool Context::tryCreate(const Configuration& configuration) { if(configuration.flags() & Configuration::Flag::GpuValidation) _configurationFlags |= Configuration::Flag::GpuValidation; + /* Same for windowless */ + if(configuration.flags() & Configuration::Flag::Windowless) + _configurationFlags |= Configuration::Flag::Windowless; + /* Driver workarounds get merged. Not using disableDriverWorkaround() here since the Configuration already contains the internal string views. */ for(const Containers::StringView workaround: configuration.disabledWorkarounds()) @@ -974,9 +978,13 @@ bool Context::tryCreate(const Configuration& configuration) { if(!workaround.second) Debug(output) << " " << workaround.first; } - /* Initialize functionality based on current OpenGL version and extensions */ /** @todo Get rid of these */ - DefaultFramebuffer::initializeContextBasedFunctionality(*this); + /* Initialize functionality based on current OpenGL version and extensions. + If we are on a windowless context don't touch the default framebuffer + to avoid potential race conditions with default framebuffer on another + thread. */ + if(!(_configurationFlags & Configuration::Flag::Windowless)) + DefaultFramebuffer::initializeContextBasedFunctionality(*this); Renderer::initializeContextBasedFunctionality(); /* Enable GPU validation, if requested */ diff --git a/src/Magnum/GL/Context.h b/src/Magnum/GL/Context.h index 9d38e146a..86a6763ed 100644 --- a/src/Magnum/GL/Context.h +++ b/src/Magnum/GL/Context.h @@ -70,6 +70,7 @@ namespace Implementation { Context before the Configuration class is defined, it has to be here */ enum class ContextConfigurationFlag: UnsignedLong { /* Keeping the 32-bit range reserved for actual GL context flags */ + Windowless = 1ull << 60, QuietLog = 1ull << 61, VerboseLog = 1ull << 62, GpuValidation = 1ull << 63 @@ -914,6 +915,19 @@ class MAGNUM_GL_EXPORT Context::Configuration { /* Docs only, keep in sync with Implementation::ContextConfigurationFlag please */ + /** + * Treat the context as windowless, assume there's no default + * framebuffer and thus don't touch @ref defaultFramebuffer in any + * way. Useful for preventing race conditions when creating OpenGL + * contexts in background threads. + * + * This flag is implicitly enabled in all + * @ref Platform::WindowlessEglContext::Configuration "Platform::Windowless*Application::Configuration", + * and, conversely, not possible to enable in any + * @ref Platform::Sdl2Application::GLConfiguration::Flag "Platform::*Application::GLConfiguration". + */ + Windowless = 1ull << 60, + /** * Print only warnings and errors instead of the usual startup log * listing used extensions and workarounds. Ignored if diff --git a/src/Magnum/GL/Framebuffer.cpp b/src/Magnum/GL/Framebuffer.cpp index 135534bdf..e1edba78a 100644 --- a/src/Magnum/GL/Framebuffer.cpp +++ b/src/Magnum/GL/Framebuffer.cpp @@ -120,18 +120,23 @@ Framebuffer::~Framebuffer() { if(!_id || !(_flags & ObjectFlag::DeleteOnDestruction)) return; /* If bound, remove itself from state */ - Implementation::FramebufferState& state = Context::current().state().framebuffer; + Context& context = Context::current(); + Implementation::FramebufferState& state = context.state().framebuffer; if(state.readBinding == _id) state.readBinding = 0; - /* For draw binding reset also viewport */ + /* For draw binding reset also viewport. Don't do that for windowless + contexts to avoid potential race conditions with default framebuffer on + another thread. */ if(state.drawBinding == _id) { state.drawBinding = 0; - /** - * @todo Less ugly solution (need to call setViewportInternal() to - * reset the viewport to size of default framebuffer) - */ - defaultFramebuffer.bind(); + if(!(context.configurationFlags() & Context::Configuration::Flag::Windowless)) { + /** + * @todo Less ugly solution (need to call setViewportInternal() to + * reset the viewport to size of default framebuffer) + */ + defaultFramebuffer.bind(); + } } glDeleteFramebuffers(1, &_id); diff --git a/src/Magnum/Platform/AbstractXApplication.h b/src/Magnum/Platform/AbstractXApplication.h index fc91893a4..3b02f92f7 100644 --- a/src/Magnum/Platform/AbstractXApplication.h +++ b/src/Magnum/Platform/AbstractXApplication.h @@ -343,7 +343,9 @@ class AbstractXApplication::GLConfiguration: public GL::Context::Configuration { /** * @brief Context flag * - * Includes also everything from @ref GL::Context::Configuration::Flag. + * Includes also everything from @ref GL::Context::Configuration::Flag + * except for @relativeref{GL::Context::Configuration,Flag::Windowless}, + * which is not meant to be enabled for windowed apps. * @see @ref Flags, @ref setFlags(), @ref GL::Context::Flag */ enum class Flag: UnsignedLong { diff --git a/src/Magnum/Platform/AndroidApplication.h b/src/Magnum/Platform/AndroidApplication.h index d6055afad..11ea3d620 100644 --- a/src/Magnum/Platform/AndroidApplication.h +++ b/src/Magnum/Platform/AndroidApplication.h @@ -456,7 +456,9 @@ class AndroidApplication::GLConfiguration: public GL::Context::Configuration { /** * @brief Context flag * - * Includes also everything from @ref GL::Context::Configuration::Flag. + * Includes also everything from @ref GL::Context::Configuration::Flag + * except for @relativeref{GL::Context::Configuration,Flag::Windowless}, + * which is not meant to be enabled for windowed apps. * @see @ref Flags, @ref setFlags(), @ref GL::Context::Flag */ enum class Flag: UnsignedLong { diff --git a/src/Magnum/Platform/EmscriptenApplication.h b/src/Magnum/Platform/EmscriptenApplication.h index 585c24af1..fd76e5867 100644 --- a/src/Magnum/Platform/EmscriptenApplication.h +++ b/src/Magnum/Platform/EmscriptenApplication.h @@ -942,7 +942,9 @@ class EmscriptenApplication::GLConfiguration: public GL::Context::Configuration /** * @brief Context flag * - * Includes also everything from @ref GL::Context::Configuration::Flag. + * Includes also everything from @ref GL::Context::Configuration::Flag + * except for @relativeref{GL::Context::Configuration,Flag::Windowless}, + * which is not meant to be enabled for windowed apps. * @see @ref Flags, @ref setFlags(), @ref GL::Context::Flag */ enum class Flag: UnsignedLong { diff --git a/src/Magnum/Platform/GlfwApplication.h b/src/Magnum/Platform/GlfwApplication.h index 2d27a7180..c4f491665 100644 --- a/src/Magnum/Platform/GlfwApplication.h +++ b/src/Magnum/Platform/GlfwApplication.h @@ -784,7 +784,9 @@ class GlfwApplication::GLConfiguration: public GL::Context::Configuration { /** * @brief Context flag * - * Includes also everything from @ref GL::Context::Configuration::Flag. + * Includes also everything from @ref GL::Context::Configuration::Flag + * except for @relativeref{GL::Context::Configuration,Flag::Windowless}, + * which is not meant to be enabled for windowed apps. * @see @ref Flags, @ref setFlags(), @ref GL::Context::Flag */ enum class Flag: UnsignedLong { diff --git a/src/Magnum/Platform/Sdl2Application.h b/src/Magnum/Platform/Sdl2Application.h index acb6279be..4963f0a7f 100644 --- a/src/Magnum/Platform/Sdl2Application.h +++ b/src/Magnum/Platform/Sdl2Application.h @@ -1236,7 +1236,9 @@ class Sdl2Application::GLConfiguration: public GL::Context::Configuration { /** * @brief Context flag * - * Includes also everything from @ref GL::Context::Configuration::Flag. + * Includes also everything from @ref GL::Context::Configuration::Flag + * except for @relativeref{GL::Context::Configuration,Flag::Windowless}, + * which is not meant to be enabled for windowed apps. * @see @ref Flags, @ref setFlags(), @ref GL::Context::Flag */ enum class Flag: UnsignedLong { diff --git a/src/Magnum/Platform/WindowlessCglApplication.cpp b/src/Magnum/Platform/WindowlessCglApplication.cpp index 146780ab7..ed8efa5d4 100644 --- a/src/Magnum/Platform/WindowlessCglApplication.cpp +++ b/src/Magnum/Platform/WindowlessCglApplication.cpp @@ -105,6 +105,10 @@ bool WindowlessCglContext::release() { return false; } +WindowlessCglContext::Configuration::Configuration() { + GL::Context::Configuration::addFlags(GL::Context::Configuration::Flag::Windowless); +} + #ifndef DOXYGEN_GENERATING_OUTPUT WindowlessCglApplication::WindowlessCglApplication(const Arguments& arguments): WindowlessCglApplication{arguments, Configuration{}} {} #endif diff --git a/src/Magnum/Platform/WindowlessCglApplication.h b/src/Magnum/Platform/WindowlessCglApplication.h index 20f8d8fda..ecb3dc915 100644 --- a/src/Magnum/Platform/WindowlessCglApplication.h +++ b/src/Magnum/Platform/WindowlessCglApplication.h @@ -166,7 +166,9 @@ class WindowlessCglContext::Configuration: public GL::Context::Configuration { /** * @brief Context flag * - * Includes also everything from @ref GL::Context::Configuration::Flag. + * Includes also everything from @ref GL::Context::Configuration::Flag + * except for @relativeref{GL::Context::Configuration,Flag::Windowless}, + * which is enabled implicitly by default. * @see @ref Flags, @ref setFlags(), @ref GL::Context::Flag */ enum class Flag: UnsignedLong { @@ -196,6 +198,8 @@ class WindowlessCglContext::Configuration: public GL::Context::Configuration { */ typedef Containers::EnumSet Flags; + /*implicit*/ Configuration(); + /** @brief Context flags */ Flags flags() const { return Flag(UnsignedLong(GL::Context::Configuration::flags())); diff --git a/src/Magnum/Platform/WindowlessEglApplication.cpp b/src/Magnum/Platform/WindowlessEglApplication.cpp index 9b8e3547b..347ef34d7 100644 --- a/src/Magnum/Platform/WindowlessEglApplication.cpp +++ b/src/Magnum/Platform/WindowlessEglApplication.cpp @@ -533,6 +533,7 @@ WindowlessEglContext::Configuration::Configuration() : _device{} #endif { + GL::Context::Configuration::addFlags(GL::Context::Configuration::Flag::Windowless); #ifndef MAGNUM_TARGET_GLES addFlags(Flag::ForwardCompatible); #endif diff --git a/src/Magnum/Platform/WindowlessEglApplication.h b/src/Magnum/Platform/WindowlessEglApplication.h index 6dbd46279..ecbe7c116 100644 --- a/src/Magnum/Platform/WindowlessEglApplication.h +++ b/src/Magnum/Platform/WindowlessEglApplication.h @@ -183,7 +183,9 @@ class WindowlessEglContext::Configuration: public GL::Context::Configuration { /** * @brief Context flag * - * Includes also everything from @ref GL::Context::Configuration::Flag. + * Includes also everything from @ref GL::Context::Configuration::Flag + * except for @relativeref{GL::Context::Configuration,Flag::Windowless}, + * which is enabled implicitly by default. * @see @ref Flags, @ref setFlags(), @ref GL::Context::Flag */ enum class Flag: UnsignedLong { diff --git a/src/Magnum/Platform/WindowlessGlxApplication.cpp b/src/Magnum/Platform/WindowlessGlxApplication.cpp index 0d0cb3fb4..01bea105a 100644 --- a/src/Magnum/Platform/WindowlessGlxApplication.cpp +++ b/src/Magnum/Platform/WindowlessGlxApplication.cpp @@ -274,6 +274,7 @@ bool WindowlessGlxContext::release() { } WindowlessGlxContext::Configuration::Configuration() { + GL::Context::Configuration::addFlags(GL::Context::Configuration::Flag::Windowless); #ifndef MAGNUM_TARGET_GLES addFlags(Flag::ForwardCompatible); #endif diff --git a/src/Magnum/Platform/WindowlessGlxApplication.h b/src/Magnum/Platform/WindowlessGlxApplication.h index 639625be5..158f19edf 100644 --- a/src/Magnum/Platform/WindowlessGlxApplication.h +++ b/src/Magnum/Platform/WindowlessGlxApplication.h @@ -188,7 +188,9 @@ class WindowlessGlxContext::Configuration: public GL::Context::Configuration { /** * @brief Context flag * - * Includes also everything from @ref GL::Context::Configuration::Flag. + * Includes also everything from @ref GL::Context::Configuration::Flag + * except for @relativeref{GL::Context::Configuration,Flag::Windowless}, + * which is enabled implicitly by default. * @see @ref Flags, @ref setFlags(), @ref GL::Context::Flag */ enum class Flag: UnsignedLong { diff --git a/src/Magnum/Platform/WindowlessIosApplication.h b/src/Magnum/Platform/WindowlessIosApplication.h index 472e6d92f..5b55b2643 100644 --- a/src/Magnum/Platform/WindowlessIosApplication.h +++ b/src/Magnum/Platform/WindowlessIosApplication.h @@ -157,7 +157,9 @@ class WindowlessIosContext::Configuration: public GL::Context::Configuration { /** * @brief Context flag * - * Includes also everything from @ref GL::Context::Configuration::Flag. + * Includes also everything from @ref GL::Context::Configuration::Flag + * except for @relativeref{GL::Context::Configuration,Flag::Windowless}, + * which is enabled implicitly by default. * @see @ref Flags, @ref setFlags(), @ref GL::Context::Flag */ enum class Flag: UnsignedLong { @@ -187,6 +189,8 @@ class WindowlessIosContext::Configuration: public GL::Context::Configuration { */ typedef Containers::EnumSet Flags; + /*implicit*/ Configuration(); + /** @brief Context flags */ Flags flags() const { return Flag(UnsignedLong(GL::Context::Configuration::flags())); diff --git a/src/Magnum/Platform/WindowlessIosApplication.mm b/src/Magnum/Platform/WindowlessIosApplication.mm index a174de8e2..c3778778f 100644 --- a/src/Magnum/Platform/WindowlessIosApplication.mm +++ b/src/Magnum/Platform/WindowlessIosApplication.mm @@ -82,6 +82,10 @@ bool WindowlessIosContext::release() { return false; } +WindowlessIosContext::Configuration::Configuration() { + GL::Context::Configuration::addFlags(GL::Context::Configuration::Flag::Windowless); +} + #ifndef DOXYGEN_GENERATING_OUTPUT WindowlessIosApplication::WindowlessIosApplication(const Arguments& arguments): WindowlessIosApplication{arguments, Configuration{}} {} #endif diff --git a/src/Magnum/Platform/WindowlessWglApplication.cpp b/src/Magnum/Platform/WindowlessWglApplication.cpp index 5f9482f3c..493e94b26 100644 --- a/src/Magnum/Platform/WindowlessWglApplication.cpp +++ b/src/Magnum/Platform/WindowlessWglApplication.cpp @@ -259,6 +259,7 @@ bool WindowlessWglContext::release() { } WindowlessWglContext::Configuration::Configuration() { + GL::Context::Configuration::addFlags(GL::Context::Configuration::Flag::Windowless); #ifndef MAGNUM_TARGET_GLES addFlags(Flag::ForwardCompatible); #endif diff --git a/src/Magnum/Platform/WindowlessWglApplication.h b/src/Magnum/Platform/WindowlessWglApplication.h index 71db719f2..067369fcc 100644 --- a/src/Magnum/Platform/WindowlessWglApplication.h +++ b/src/Magnum/Platform/WindowlessWglApplication.h @@ -175,7 +175,9 @@ class WindowlessWglContext::Configuration: public GL::Context::Configuration { /** * @brief Context flag * - * Includes also everything from @ref GL::Context::Configuration::Flag. + * Includes also everything from @ref GL::Context::Configuration::Flag + * except for @relativeref{GL::Context::Configuration,Flag::Windowless}, + * which is enabled implicitly by default. * @see @ref Flags, @ref setFlags(), @ref GL::Context::Flag */ enum class Flag: UnsignedLong { diff --git a/src/Magnum/Platform/WindowlessWindowsEglApplication.cpp b/src/Magnum/Platform/WindowlessWindowsEglApplication.cpp index b153ac85d..5eac49ebe 100644 --- a/src/Magnum/Platform/WindowlessWindowsEglApplication.cpp +++ b/src/Magnum/Platform/WindowlessWindowsEglApplication.cpp @@ -182,6 +182,10 @@ bool WindowlessWindowsEglContext::release() { return false; } +WindowlessWindowsEglContext::Configuration::Configuration() { + GL::Context::Configuration::addFlags(GL::Context::Configuration::Flag::Windowless); +} + #ifndef DOXYGEN_GENERATING_OUTPUT WindowlessWindowsEglApplication::WindowlessWindowsEglApplication(const Arguments& arguments): WindowlessWindowsEglApplication{arguments, Configuration{}} {} #endif diff --git a/src/Magnum/Platform/WindowlessWindowsEglApplication.h b/src/Magnum/Platform/WindowlessWindowsEglApplication.h index 64c4701bc..7268ad2b8 100644 --- a/src/Magnum/Platform/WindowlessWindowsEglApplication.h +++ b/src/Magnum/Platform/WindowlessWindowsEglApplication.h @@ -161,7 +161,9 @@ class WindowlessWindowsEglContext::Configuration: public GL::Context::Configurat /** * @brief Context flag * - * Includes also everything from @ref GL::Context::Configuration::Flag. + * Includes also everything from @ref GL::Context::Configuration::Flag + * except for @relativeref{GL::Context::Configuration,Flag::Windowless}, + * which is enabled implicitly by default. * @see @ref Flags, @ref setFlags(), @ref GL::Context::Flag */ enum class Flag: UnsignedLong { @@ -199,6 +201,8 @@ class WindowlessWindowsEglContext::Configuration: public GL::Context::Configurat */ typedef Containers::EnumSet Flags; + /*implicit*/ Configuration(); + /** @brief Context flags */ Flags flags() const { return Flag(UnsignedLong(GL::Context::Configuration::flags()));