From acf06eb6d8cc907efa327034f38ee3424324f008 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 21 Aug 2019 11:19:56 +0200 Subject: [PATCH] GL: don't implicitly enable debug output in OpenGLTester. It's just too spammy and we have a nice opt-in way now, so direct users to use that instead. --- doc/changelog.dox | 5 +++++ src/Magnum/GL/OpenGLTester.cpp | 23 +---------------------- src/Magnum/GL/OpenGLTester.h | 24 +++++++++--------------- src/Magnum/GL/Test/DebugOutputGLTest.cpp | 21 ++++++++++++++------- 4 files changed, 29 insertions(+), 44 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index a53a38635..77913b821 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -308,6 +308,11 @@ See also: from flags passed to `eglCreateContext()` as that otherwise causes NVidia to fail with `EGL_BAD_MATCH`. See @ref opengl-workarounds for more information. +- @ref GL::OpenGLTester no longer implicitly enables @ref GL::DebugOutput as + it can be quite spammy when complex shaders or benchmarks are allowed. For + easier debugging, users are encouraged to use the new + `--magnum-gpu-validation` @ref GL-Context-command-line "command line option" + instead. @subsubsection changelog-latest-changes-math Math library diff --git a/src/Magnum/GL/OpenGLTester.cpp b/src/Magnum/GL/OpenGLTester.cpp index f67c6892c..5f703cd27 100644 --- a/src/Magnum/GL/OpenGLTester.cpp +++ b/src/Magnum/GL/OpenGLTester.cpp @@ -35,28 +35,7 @@ namespace Magnum { namespace GL { -OpenGLTester::OpenGLTester(): TestSuite::Tester{TestSuite::Tester::TesterConfiguration{}.setSkippedArgumentPrefixes({"magnum"})}, _windowlessApplication{{arguments().first, arguments().second}} { - /* Try to create debug context, fallback to normal one if not possible. No - such thing on macOS, iOS or WebGL. */ - #if !defined(CORRADE_TARGET_APPLE) && !defined(MAGNUM_TARGET_WEBGL) - if(!_windowlessApplication.tryCreateContext(Platform::WindowlessApplication::Configuration{}.addFlags(Platform::WindowlessApplication::Configuration::Flag::Debug))) - _windowlessApplication.createContext(); - #else - _windowlessApplication.createContext(); - #endif - - #ifndef MAGNUM_TARGET_WEBGL - if(Context::current().isExtensionSupported()) { - Renderer::enable(Renderer::Feature::DebugOutput); - Renderer::enable(Renderer::Feature::DebugOutputSynchronous); - DebugOutput::setDefaultCallback(); - - /* Disable "Buffer detailed info" message on NV (too spammy) */ - if(Context::current().detectedDriver() & Context::DetectedDriver::NVidia) - DebugOutput::setEnabled(DebugOutput::Source::Api, DebugOutput::Type::Other, {131185}, false); - } - #endif -} +OpenGLTester::OpenGLTester(): TestSuite::Tester{TestSuite::Tester::TesterConfiguration{}.setSkippedArgumentPrefixes({"magnum"})}, _windowlessApplication{{arguments().first, arguments().second}} {} OpenGLTester::~OpenGLTester() = default; diff --git a/src/Magnum/GL/OpenGLTester.h b/src/Magnum/GL/OpenGLTester.h index 32c76a8cd..68649d1ef 100644 --- a/src/Magnum/GL/OpenGLTester.h +++ b/src/Magnum/GL/OpenGLTester.h @@ -121,16 +121,14 @@ to run single isolated test cases. @section GL-OpenGLTester-debug Debug context and error checking -On platforms that support it, the OpenGL context is created with synchronous -debug output, meaning that every OpenGL error is directly reported to standard -output. While it is possible, the tester class doesn't abort the test cases -upon encountering a GL error --- this should be done explicitly with -@ref MAGNUM_VERIFY_NO_GL_ERROR() instead, as the debug output is not available -on all platforms and not all GL errors are fatal. - -@note This overrides the `--magnum-gpu-validation` - @ref GL-Context-command-line "command line option", making it always - enabled. +Because @ref DebugOutput can be quite spammy in some cases, especially when +compiling more complex shaders or doing benchmarks, it's not implicitly enabled +by default to make the test output more readable. Instead of relying on debug +output to report errors, the @ref MAGNUM_VERIFY_NO_GL_ERROR() macro should be +used to reliably check for errors regardless of platform support. For easier +debugging of OpenGL errors users are encuraged to use the +`--magnum-gpu-validation` @ref GL-Context-command-line "command line option", +which is supported here as well as in all other application implementations. @section GL-OpenGLTester-benchmarks GPU time benchmarks @@ -263,12 +261,8 @@ class OpenGLTester: public TestSuite::Tester { #endif struct WindowlessApplication: Platform::WindowlessApplication { - explicit WindowlessApplication(const Arguments& arguments): Platform::WindowlessApplication{arguments, NoCreate} {} + explicit WindowlessApplication(const Arguments& arguments): Platform::WindowlessApplication{arguments} {} int exec() override final { return 0; } - - using Platform::WindowlessApplication::tryCreateContext; - using Platform::WindowlessApplication::createContext; - } _windowlessApplication; #ifndef MAGNUM_TARGET_WEBGL diff --git a/src/Magnum/GL/Test/DebugOutputGLTest.cpp b/src/Magnum/GL/Test/DebugOutputGLTest.cpp index 4112a1760..6b29054e1 100644 --- a/src/Magnum/GL/Test/DebugOutputGLTest.cpp +++ b/src/Magnum/GL/Test/DebugOutputGLTest.cpp @@ -73,7 +73,6 @@ void DebugOutputGLTest::setCallbackDefault() { if(!Context::current().isExtensionSupported()) CORRADE_SKIP(Extensions::KHR::debug::string() + std::string(" is not supported")); - /* Need to be careful, because the test runner is using debug output too */ DebugOutput::setDefaultCallback(); MAGNUM_VERIFY_NO_GL_ERROR(); @@ -82,15 +81,23 @@ void DebugOutputGLTest::setCallbackDefault() { 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); + if(!Context::current().isExtensionSupported()) + return; + + Renderer::enable(Renderer::Feature::DebugOutput); + Renderer::enable(Renderer::Feature::DebugOutputSynchronous); + 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(); + if(!Context::current().isExtensionSupported()) + return; + + Renderer::disable(Renderer::Feature::DebugOutput); + Renderer::disable(Renderer::Feature::DebugOutputSynchronous); + DebugOutput::setDefaultCallback(); } void DebugOutputGLTest::setEnabled() {