diff --git a/doc/changelog.dox b/doc/changelog.dox index ede7dbf6f..3cef590d5 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -157,6 +157,10 @@ See also: - Reworked @ref GL::Mesh internals to make use of @gl_extension{ARB,direct_state_access} instead of being the last code path stuck on @gl_extension{EXT,direct_state_access} +- @ref GL::DebugOutput::setCallback() no longer assumes the debug message + callback is called from the same thread as the one with the corresponding + GL context (which is the case on Mesa AMD drivers). The documentation was + updated to reflect that as well. @subsubsection changelog-latest-changes-math Math library diff --git a/src/Magnum/GL/DebugOutput.cpp b/src/Magnum/GL/DebugOutput.cpp index 1497a3bc9..2b865f8e7 100644 --- a/src/Magnum/GL/DebugOutput.cpp +++ b/src/Magnum/GL/DebugOutput.cpp @@ -36,18 +36,10 @@ namespace Magnum { namespace GL { -namespace { - -void -#ifdef CORRADE_TARGET_WINDOWS -APIENTRY -#endif -callbackWrapper(GLenum source, GLenum type, GLuint id, GLenum severity, GLsizei length, const GLchar* message, const void* userParam) { - Context::current().state().debug->messageCallback(DebugOutput::Source(source), DebugOutput::Type(type), id, DebugOutput::Severity(severity), std::string{message, std::size_t(length)}, userParam); -} +namespace Implementation { -void defaultCallback(const DebugOutput::Source source, const DebugOutput::Type type, const UnsignedInt id, const DebugOutput::Severity severity, const std::string& string, const void*) { - Debug output; +void defaultDebugCallback(const DebugOutput::Source source, const DebugOutput::Type type, const UnsignedInt id, const DebugOutput::Severity severity, const std::string& string, std::ostream* out) { + Debug output{out}; output << "Debug output:"; switch(severity) { @@ -104,6 +96,19 @@ void defaultCallback(const DebugOutput::Source source, const DebugOutput::Type t } +namespace { + +void +#ifdef CORRADE_TARGET_WINDOWS +APIENTRY +#endif +callbackWrapper(GLenum source, GLenum type, GLuint id, GLenum severity, GLsizei length, const GLchar* message, const void* userParam) { + const auto& callback = *static_cast(userParam); + callback.callback(DebugOutput::Source(source), DebugOutput::Type(type), id, DebugOutput::Severity(severity), std::string{message, std::size_t(length)}, callback.userParam); +} + +} + Int DebugOutput::maxLoggedMessages() { if(!Context::current().isExtensionSupported()) return 0; @@ -143,7 +148,9 @@ void DebugOutput::setCallback(const Callback callback, const void* userParam) { } void DebugOutput::setDefaultCallback() { - setCallback(defaultCallback, nullptr); + setCallback([](DebugOutput::Source source, DebugOutput::Type type, UnsignedInt id, DebugOutput::Severity severity, const std::string& string, const void*) { + Implementation::defaultDebugCallback(source, type, id, severity, string, Debug::output()); + }); } void DebugOutput::setEnabledInternal(const GLenum source, const GLenum type, const GLenum severity, const std::initializer_list ids, const bool enabled) { @@ -169,12 +176,13 @@ void DebugOutput::callbackImplementationNoOp(Callback, const void*) {} #ifndef MAGNUM_TARGET_GLES2 void DebugOutput::callbackImplementationKhrDesktopES32(const Callback callback, const void* userParam) { /* Replace the callback */ - const Callback original = Context::current().state().debug->messageCallback; - Context::current().state().debug->messageCallback = callback; + const Callback original = Context::current().state().debug->messageCallback.callback; + Context::current().state().debug->messageCallback.callback = callback; + Context::current().state().debug->messageCallback.userParam = userParam; /* Adding callback */ if(!original && callback) - glDebugMessageCallback(callbackWrapper, userParam); + glDebugMessageCallback(callbackWrapper, &Context::current().state().debug->messageCallback); /* Deleting callback */ else if(original && !callback) @@ -185,12 +193,13 @@ void DebugOutput::callbackImplementationKhrDesktopES32(const Callback callback, #ifdef MAGNUM_TARGET_GLES void DebugOutput::callbackImplementationKhrES(const Callback callback, const void* userParam) { /* Replace the callback */ - const Callback original = Context::current().state().debug->messageCallback; - Context::current().state().debug->messageCallback = callback; + const Callback original = Context::current().state().debug->messageCallback.callback; + Context::current().state().debug->messageCallback.callback = callback; + Context::current().state().debug->messageCallback.userParam = userParam; /* Adding callback */ if(!original && callback) - glDebugMessageCallbackKHR(callbackWrapper, userParam); + glDebugMessageCallbackKHR(callbackWrapper, &Context::current().state().debug->messageCallback); /* Deleting callback */ else if(original && !callback) diff --git a/src/Magnum/GL/DebugOutput.h b/src/Magnum/GL/DebugOutput.h index 7f010ac3d..f1bf2c065 100644 --- a/src/Magnum/GL/DebugOutput.h +++ b/src/Magnum/GL/DebugOutput.h @@ -369,6 +369,13 @@ class MAGNUM_GL_EXPORT DebugOutput { * OpenGL ES 3.2 is not supported and @gl_extension{KHR,debug} desktop or * ES extension (covered also by @gl_extension{ANDROID,extension_pack_es31a}) * is not available, this function does nothing. + * + * @attention The function is not necessarily called from the same + * thread as the one that caused the message to appear --- in + * particular, you can't assume the @ref GL::Context will be + * present in the callback context. It might work on some drivers, + * but not on others. + * * @see @ref setDefaultCallback(), * @ref Renderer::Feature::DebugOutputSynchronous, * @fn_gl_keyword{DebugMessageCallback} @@ -805,6 +812,11 @@ class MAGNUM_GL_EXPORT DebugGroup { /** @debugoperatorclassenum{DebugGroup,DebugGroup::Source} */ MAGNUM_GL_EXPORT Debug& operator<<(Debug& debug, DebugGroup::Source value); +/* Exposed for testing */ +namespace Implementation { + MAGNUM_GL_EXPORT void defaultDebugCallback(DebugOutput::Source source, DebugOutput::Type type, UnsignedInt id, DebugOutput::Severity severity, const std::string& string, std::ostream* output); +} + }} #else #error this header is not available in WebGL build diff --git a/src/Magnum/GL/Implementation/DebugState.cpp b/src/Magnum/GL/Implementation/DebugState.cpp index 11c938a9e..6e61b4658 100644 --- a/src/Magnum/GL/Implementation/DebugState.cpp +++ b/src/Magnum/GL/Implementation/DebugState.cpp @@ -35,8 +35,7 @@ DebugState::DebugState(Context& context, std::vector& extensions): maxLabelLength{0}, maxLoggedMessages{0}, maxMessageLength{0}, - maxStackDepth{0}, - messageCallback(nullptr) + maxStackDepth{0} { #ifndef MAGNUM_TARGET_GLES2 #ifndef MAGNUM_TARGET_GLES diff --git a/src/Magnum/GL/Implementation/DebugState.h b/src/Magnum/GL/Implementation/DebugState.h index 432eda02b..d19b3f0ed 100644 --- a/src/Magnum/GL/Implementation/DebugState.h +++ b/src/Magnum/GL/Implementation/DebugState.h @@ -50,7 +50,10 @@ struct DebugState { void(*popGroupImplementation)(); GLint maxLabelLength, maxLoggedMessages, maxMessageLength, maxStackDepth; - DebugOutput::Callback messageCallback; + struct MessageCallback { + DebugOutput::Callback callback{}; + const void* userParam{}; + } messageCallback; }; }}} diff --git a/src/Magnum/GL/OpenGLTester.cpp b/src/Magnum/GL/OpenGLTester.cpp index dbb9b9160..a46076e99 100644 --- a/src/Magnum/GL/OpenGLTester.cpp +++ b/src/Magnum/GL/OpenGLTester.cpp @@ -50,7 +50,8 @@ OpenGLTester::OpenGLTester(): TestSuite::Tester{TestSuite::Tester::TesterConfigu DebugOutput::setDefaultCallback(); /* Disable "Buffer detailed info" message on NV (too spammy) */ - DebugOutput::setEnabled(DebugOutput::Source::Api, DebugOutput::Type::Other, {131185}, false); + if(Context::current().detectedDriver() & Context::DetectedDriver::NVidia) + DebugOutput::setEnabled(DebugOutput::Source::Api, DebugOutput::Type::Other, {131185}, false); } #endif } diff --git a/src/Magnum/GL/Test/DebugOutputGLTest.cpp b/src/Magnum/GL/Test/DebugOutputGLTest.cpp index 6a5db1ad3..f992ee436 100644 --- a/src/Magnum/GL/Test/DebugOutputGLTest.cpp +++ b/src/Magnum/GL/Test/DebugOutputGLTest.cpp @@ -35,7 +35,11 @@ namespace Magnum { namespace GL { namespace Test { namespace { struct DebugOutputGLTest: OpenGLTester { explicit DebugOutputGLTest(); - void setCallback(); + void setCallbackDefault(); + + void setup(); + void teardown(); + void setEnabled(); void messageNoOp(); @@ -45,11 +49,14 @@ struct DebugOutputGLTest: OpenGLTester { void groupNoOp(); void group(); void groupFallback(); + + std::ostringstream _out; }; DebugOutputGLTest::DebugOutputGLTest() { - addTests({&DebugOutputGLTest::setCallback, - &DebugOutputGLTest::setEnabled, + addTests({&DebugOutputGLTest::setCallbackDefault}); + + addTests({&DebugOutputGLTest::setEnabled, &DebugOutputGLTest::messageNoOp, &DebugOutputGLTest::message, @@ -57,10 +64,11 @@ DebugOutputGLTest::DebugOutputGLTest() { &DebugOutputGLTest::groupNoOp, &DebugOutputGLTest::group, - &DebugOutputGLTest::groupFallback}); + &DebugOutputGLTest::groupFallback}, + &DebugOutputGLTest::setup, &DebugOutputGLTest::teardown); } -void DebugOutputGLTest::setCallback() { +void DebugOutputGLTest::setCallbackDefault() { if(!Context::current().isExtensionSupported()) CORRADE_SKIP(Extensions::KHR::debug::string() + std::string(" is not supported")); @@ -70,16 +78,41 @@ void DebugOutputGLTest::setCallback() { MAGNUM_VERIFY_NO_GL_ERROR(); } +void DebugOutputGLTest::setup() { + _out.str({}); + + if(Context::current().isExtensionSupported()) + DebugOutput::setCallback([](DebugOutput::Source source, DebugOutput::Type type, UnsignedInt id, DebugOutput::Severity severity, const std::string& string, const void* userPtr) { + Implementation::defaultDebugCallback(source, type, id, severity, string, static_cast(const_cast(userPtr))); + }, &_out); +} + +void DebugOutputGLTest::teardown() { + if(Context::current().isExtensionSupported()) + DebugOutput::setDefaultCallback(); +} + void DebugOutputGLTest::setEnabled() { if(!Context::current().isExtensionSupported()) CORRADE_SKIP(Extensions::KHR::debug::string() + std::string(" is not supported")); - /* Try at least some combinations */ + /* Try at least some combinations. Calling a less-specific version will + reset the more-specific setting from earlier. */ + DebugOutput::setEnabled(true); DebugOutput::setEnabled(DebugOutput::Source::Application, true); DebugOutput::setEnabled(DebugOutput::Source::Application, DebugOutput::Type::UndefinedBehavior, {3168, 35487, 234487}, false); + + MAGNUM_VERIFY_NO_GL_ERROR(); + + DebugMessage::insert(DebugMessage::Source::Application, DebugMessage::Type::UndefinedBehavior, + 35487, DebugOutput::Severity::Notification, "This message should get ignored"); + + /* Reset everything back */ DebugOutput::setEnabled(true); MAGNUM_VERIFY_NO_GL_ERROR(); + + CORRADE_COMPARE(_out.str(), ""); } void DebugOutputGLTest::messageNoOp() { @@ -95,6 +128,8 @@ void DebugOutputGLTest::messageNoOp() { 1337, DebugOutput::Severity::Notification, "Hello from OpenGL command stream!"); MAGNUM_VERIFY_NO_GL_ERROR(); + + CORRADE_COMPARE(_out.str(), ""); } void DebugOutputGLTest::message() { @@ -102,13 +137,11 @@ void DebugOutputGLTest::message() { CORRADE_SKIP(Extensions::KHR::debug::string() + std::string(" is not supported")); /* Need to be careful, because the test runner is using debug output too */ - std::ostringstream out; - Debug redirectDebug{&out}; DebugMessage::insert(DebugMessage::Source::Application, DebugMessage::Type::Marker, 1337, DebugOutput::Severity::High, "Hello from OpenGL command stream!"); MAGNUM_VERIFY_NO_GL_ERROR(); - CORRADE_COMPARE(out.str(), + CORRADE_COMPARE(_out.str(), "Debug output: high severity application marker (1337): Hello from OpenGL command stream!\n"); } @@ -125,6 +158,8 @@ void DebugOutputGLTest::messageFallback() { 1337, DebugOutput::Severity::Notification, "Hello from OpenGL command stream!"); MAGNUM_VERIFY_NO_GL_ERROR(); + + CORRADE_COMPARE(_out.str(), ""); } void DebugOutputGLTest::groupNoOp() { @@ -137,6 +172,7 @@ void DebugOutputGLTest::groupNoOp() { } MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(_out.str(), ""); } void DebugOutputGLTest::group() { @@ -144,8 +180,6 @@ void DebugOutputGLTest::group() { CORRADE_SKIP(Extensions::KHR::debug::string() + std::string(" is not supported")); /* Need to be careful, because the test runner is using debug output too */ - std::ostringstream out; - Debug redirectDebug{&out}; { DebugGroup g1{DebugGroup::Source::Application, 42, "Automatic debug group"}; DebugGroup g2; @@ -154,7 +188,7 @@ void DebugOutputGLTest::group() { } MAGNUM_VERIFY_NO_GL_ERROR(); - CORRADE_COMPARE(out.str(), + CORRADE_COMPARE(_out.str(), "Debug output: application debug group enter (42): Automatic debug group\n" "Debug output: third party debug group enter (1337): Manual debug group\n" "Debug output: third party debug group leave (1337): Manual debug group\n" @@ -171,6 +205,7 @@ void DebugOutputGLTest::groupFallback() { } MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(_out.str(), ""); } }}}}