From 25da9833c2929476c26d83ab6c55fc4c9950807d Mon Sep 17 00:00:00 2001 From: binary1248 Date: Sun, 26 Mar 2023 04:41:26 +0200 Subject: [PATCH] Fixed GlContext not being activated if it shares the same address as a previously destroyed GlContext. --- src/SFML/Window/CMakeLists.txt | 11 ++++++ src/SFML/Window/GlContext.cpp | 72 +++++++++++++++++++--------------- 2 files changed, 51 insertions(+), 32 deletions(-) diff --git a/src/SFML/Window/CMakeLists.txt b/src/SFML/Window/CMakeLists.txt index 7b4c6e78..d1759bda 100644 --- a/src/SFML/Window/CMakeLists.txt +++ b/src/SFML/Window/CMakeLists.txt @@ -346,3 +346,14 @@ elseif(SFML_OS_IOS) elseif(SFML_OS_ANDROID) target_link_libraries(sfml-window PRIVATE android) endif() + +# on some platforms (e.g. Raspberry Pi 3 armhf), GCC requires linking libatomic to use features +# that aren't supported by native CPU instructions (64-bit atomic operations on 32-bit architecture) +if(SFML_COMPILER_GCC) + include(CheckCXXSourceCompiles) + check_cxx_source_compiles("#include + int main(){std::atomic_ullong x(1); return x.fetch_add(1);}" ATOMIC_TEST) + if(NOT ATOMIC_TEST) + target_link_libraries(sfml-window PRIVATE atomic) + endif() +endif() diff --git a/src/SFML/Window/GlContext.cpp b/src/SFML/Window/GlContext.cpp index 6a968bbc..25d90aeb 100644 --- a/src/SFML/Window/GlContext.cpp +++ b/src/SFML/Window/GlContext.cpp @@ -32,6 +32,7 @@ #include #include +#include #include #include #include @@ -175,15 +176,23 @@ std::recursive_mutex mutex; // OpenGL resources counter unsigned int resourceCount = 0; -// This per-thread variable holds the current context for each thread -thread_local sf::priv::GlContext* currentContext(nullptr); -thread_local unsigned int currentContextTransientCount(0); +// This structure contains all the state necessary to +// track current context information for each thread +struct CurrentContext +{ + std::uint64_t id{}; + sf::priv::GlContext* ptr{}; + unsigned int transientCount{}; +}; + +// This per-thread variable holds the current context information for each thread +thread_local CurrentContext currentContext; // The hidden, inactive context that will be shared with all other contexts std::unique_ptr sharedContext; // Unique identifier, used for identifying contexts when managing unshareable OpenGL resources -std::uint64_t id = 1; // start at 1, zero is "no context" +std::atomic nextContextId(1); // start at 1, zero is "no context" // Set containing callback functions to be called whenever a // context is going to be destroyed @@ -204,7 +213,7 @@ struct TransientContext { // TransientContext should never be created if there is // already a context active on the current thread - assert(!currentContext); + assert(!currentContext.id); std::unique_lock lock(mutex); @@ -218,7 +227,7 @@ struct TransientContext } else { - // GlResources exist, currentContext not yet set + // GlResources exist, currentContextId not yet set assert(sharedContext); // Lock the shared context for temporary use @@ -411,26 +420,25 @@ void GlContext::registerContextDestroyCallback(ContextDestroyCallback callback, void GlContext::acquireTransientContext() { using GlContextImpl::currentContext; - using GlContextImpl::currentContextTransientCount; using GlContextImpl::TransientContext; using GlContextImpl::transientContext; // Fast path if we already have a context active on this thread - if (currentContext) + if (currentContext.id) { - ++currentContextTransientCount; + ++currentContext.transientCount; return; } // If we don't already have a context active on this thread the count should be 0 - assert(!currentContextTransientCount); + assert(!currentContext.transientCount); - // If currentContext is not set, this must be the first + // If currentContextId is not set, this must be the first // TransientContextLock on this thread, construct the state object transientContext.emplace(); // Make sure a context is active at this point - assert(currentContext); + assert(currentContext.id); } @@ -438,20 +446,19 @@ void GlContext::acquireTransientContext() void GlContext::releaseTransientContext() { using GlContextImpl::currentContext; - using GlContextImpl::currentContextTransientCount; using GlContextImpl::transientContext; // Make sure a context was left active after acquireTransientContext() was called - assert(currentContext); + assert(currentContext.id); // Fast path if we already had a context active on this thread before acquireTransientContext() was called - if (currentContextTransientCount) + if (currentContext.transientCount) { - --currentContextTransientCount; + --currentContext.transientCount; return; } - // If currentContext is set and currentContextTransientCount is 0, + // If currentContextId is set and currentContextTransientCount is 0, // this is the last TransientContextLock that is released, destroy the state object transientContext.reset(); } @@ -611,7 +618,8 @@ GlFunctionPointer GlContext::getFunction(const char* name) const GlContext* GlContext::getActiveContext() { using GlContextImpl::currentContext; - return currentContext; + + return currentContext.ptr; } @@ -619,7 +627,8 @@ const GlContext* GlContext::getActiveContext() std::uint64_t GlContext::getActiveContextId() { using GlContextImpl::currentContext; - return currentContext ? currentContext->m_id : 0; + + return currentContext.id; } @@ -627,13 +636,11 @@ std::uint64_t GlContext::getActiveContextId() GlContext::~GlContext() { using GlContextImpl::currentContext; - using GlContextImpl::sharedContext; - // Deactivate the context before killing it, unless we're inside Cleanup() - if (sharedContext) + if (m_id == currentContext.id) { - if (this == currentContext) - currentContext = nullptr; + currentContext.id = 0; + currentContext.ptr = nullptr; } } @@ -650,11 +657,10 @@ bool GlContext::setActive(bool active) { using GlContextImpl::currentContext; using GlContextImpl::mutex; - using GlContextImpl::sharedContext; if (active) { - if (this != currentContext) + if (m_id != currentContext.id) { std::lock_guard lock(mutex); @@ -662,7 +668,8 @@ bool GlContext::setActive(bool active) if (makeCurrent(true)) { // Set it as the new current context for this thread - currentContext = this; + currentContext.id = m_id; + currentContext.ptr = this; return true; } else @@ -678,14 +685,15 @@ bool GlContext::setActive(bool active) } else { - if (this == currentContext) + if (m_id == currentContext.id) { std::lock_guard lock(mutex); // Deactivate the context if (makeCurrent(false)) { - currentContext = nullptr; + currentContext.id = 0; + currentContext.ptr = nullptr; return true; } else @@ -703,7 +711,7 @@ bool GlContext::setActive(bool active) //////////////////////////////////////////////////////////// -GlContext::GlContext() : m_id(GlContextImpl::id++) +GlContext::GlContext() : m_id(GlContextImpl::nextContextId.fetch_add(1)) { // Nothing to do } @@ -754,10 +762,10 @@ void GlContext::cleanupUnsharedResources() using GlContextImpl::currentContext; // Save the current context so we can restore it later - GlContext* contextToRestore = currentContext; + GlContext* contextToRestore = currentContext.ptr; // If this context is already active there is no need to save it - if (contextToRestore == this) + if (m_id == currentContext.id) contextToRestore = nullptr; // Make this context active so resources can be freed