From bf16c76addd9710852e205d51922d546ad67f6cf Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Tue, 18 Apr 2017 14:29:23 -0400 Subject: [PATCH] Feature/opengldebug (#290) * Add InLIstVerifier * Initial implementation of OpenGL debug messages * Moving functionality into Ghoul * Compile fix * Update Ghoul repository Make use of the moved OpenGL debug functions * Update Ghoul to fix GCC and Clang compiler errors * Add unit test for IntListVerifier --- ext/ghoul | 2 +- include/openspace/documentation/verifier.h | 13 +++ .../openspace/engine/configurationmanager.h | 17 ++- openspace.cfg | 10 +- src/documentation/verifier.cpp | 8 ++ src/engine/configurationmanager.cpp | 9 ++ src/engine/configurationmanager_doc.inl | 75 ++++++++++++- src/engine/openspaceengine.cpp | 106 ++++++++++++++++++ tests/test_documentation.inl | 60 ++++++++++ 9 files changed, 296 insertions(+), 4 deletions(-) diff --git a/ext/ghoul b/ext/ghoul index 9788308e69..598d54bc70 160000 --- a/ext/ghoul +++ b/ext/ghoul @@ -1 +1 @@ -Subproject commit 9788308e69e69d9d4bf46cdb13e00e98c1686623 +Subproject commit 598d54bc70f8248fa88eec5793c03be1bf15fdd0 diff --git a/include/openspace/documentation/verifier.h b/include/openspace/documentation/verifier.h index f431add35f..ddf39e6c9a 100644 --- a/include/openspace/documentation/verifier.h +++ b/include/openspace/documentation/verifier.h @@ -216,6 +216,19 @@ struct StringListVerifier : public TableVerifier { std::string type() const override; }; +/** +* A Verifier that checks whether all values contained in a Table are of type \c int. +*/ +struct IntListVerifier : public TableVerifier { + /** + * Constructor for a IntListVerifier. + * \param elementDocumentation The documentation for each string in the list + */ + IntListVerifier(std::string elementDocumentation = ""); + + std::string type() const override; +}; + //---------------------------------------------------------------------------------------- // Vector verifiers //---------------------------------------------------------------------------------------- diff --git a/include/openspace/engine/configurationmanager.h b/include/openspace/engine/configurationmanager.h index 906704219b..92909e6789 100644 --- a/include/openspace/engine/configurationmanager.h +++ b/include/openspace/engine/configurationmanager.h @@ -126,7 +126,22 @@ public: static const std::string PartHttpProxyUser; /// The key that stores the password to use for authentication to access the http proxy static const std::string PartHttpProxyPassword; - + /// The key that stores the dictionary containing information about debug contexts + static const std::string KeyOpenGLDebugContext; + /// The part of the key storing whether an OpenGL Debug context should be created + static const std::string PartActivate; + /// The part of the key storing whether the debug callbacks are performed synchronous + static const std::string PartSynchronous; + /// The part of the key storing a list of identifiers that should be filtered out + static const std::string PartFilterIdentifier; + /// The part of the key that stores the source of the ignored identifier + static const std::string PartFilterIdentifierSource; + /// The part of the key that stores the type of the ignored identifier + static const std::string PartFilterIdentifierType; + /// The part of the key that stores the identifier of the ignored identifier + static const std::string PartFilterIdentifierIdentifier; + /// The part of the key storing a list of severities that should be filtered out + static const std::string PartFilterSeverity; /** * Iteratively walks the directory structure starting with \p filename to find the diff --git a/openspace.cfg b/openspace.cfg index 971434804f..92a71d88c9 100644 --- a/openspace.cfg +++ b/openspace.cfg @@ -90,6 +90,14 @@ return { -- DisableRenderingOnMaster = true, KeyDisableSceneOnMaster = true, DownloadRequestURL = "http://data.openspaceproject.com/request.cgi", - RenderingMethod = "Framebuffer" + RenderingMethod = "Framebuffer", + OpenGLDebugContext = { + Activate = true, + FilterIdentifier = { + { Type = "Other", Source = "API", Identifier = 131185 } + }, +-- FilterSeverity = { } + + } --RenderingMethod = "ABuffer" -- alternative: "Framebuffer" } diff --git a/src/documentation/verifier.cpp b/src/documentation/verifier.cpp index eb74aacb6f..872541cc3d 100644 --- a/src/documentation/verifier.cpp +++ b/src/documentation/verifier.cpp @@ -206,6 +206,14 @@ std::string StringListVerifier::type() const { return "List of strings"; } +IntListVerifier::IntListVerifier(std::string elementDocumentation) + : TableVerifier({ { "*", new IntVerifier, std::move(elementDocumentation) } }) +{} + +std::string IntListVerifier::type() const { + return "List of ints"; +} + ReferencingVerifier::ReferencingVerifier(std::string id) : identifier(std::move(id)) { diff --git a/src/engine/configurationmanager.cpp b/src/engine/configurationmanager.cpp index 2c4d466603..514719b2c3 100644 --- a/src/engine/configurationmanager.cpp +++ b/src/engine/configurationmanager.cpp @@ -87,6 +87,15 @@ const string ConfigurationManager::PartHttpProxyAuthentication = "Authentication const string ConfigurationManager::PartHttpProxyUser = "User"; const string ConfigurationManager::PartHttpProxyPassword = "Password"; +const string ConfigurationManager::KeyOpenGLDebugContext = "OpenGLDebugContext"; +const string ConfigurationManager::PartActivate = "Activate"; +const string ConfigurationManager::PartSynchronous = "Synchronous"; +const string ConfigurationManager::PartFilterIdentifier = "FilterIdentifier"; +const string ConfigurationManager::PartFilterIdentifierSource = "Source"; +const string ConfigurationManager::PartFilterIdentifierType = "Type"; +const string ConfigurationManager::PartFilterIdentifierIdentifier = "Identifier"; +const string ConfigurationManager::PartFilterSeverity = "PartFilterSeverity"; + string ConfigurationManager::findConfiguration(const string& filename) { using ghoul::filesystem::Directory; diff --git a/src/engine/configurationmanager_doc.inl b/src/engine/configurationmanager_doc.inl index 193299bd08..a00228b045 100644 --- a/src/engine/configurationmanager_doc.inl +++ b/src/engine/configurationmanager_doc.inl @@ -385,7 +385,80 @@ documentation::Documentation ConfigurationManager::Documentation() { "This defines the use for a proxy when fetching data over http." "No proxy will be used if this is left out.", Optional::Yes - } + }, + { + ConfigurationManager::KeyOpenGLDebugContext, + new TableVerifier({ + { + ConfigurationManager::PartActivate, + new BoolVerifier, + "Determines whether the OpenGL context should be a debug context", + Optional::No + }, + { + ConfigurationManager::PartSynchronous, + new BoolVerifier, + "Determines whether the OpenGL debug callbacks are performed " + "synchronously. If set to the callbacks are in the same thead " + "as the context and in the scope of the OpenGL function that " + "triggered the message. The default value is .", + Optional::Yes + }, + { + ConfigurationManager::PartFilterIdentifier, + new TableVerifier({{ + "*", + new TableVerifier({ + { + ConfigurationManager::PartFilterIdentifierIdentifier, + new IntVerifier, + "The identifier that is to be filtered", + Optional::No + }, + { + ConfigurationManager::PartFilterIdentifierSource, + new StringInListVerifier({ + // Taken from ghoul::debugcontext.cpp + "API", "Window System", "Shader Compiler", + "Third Party", "Application", "Other", "Don't care" + }), + "The source of the identifier to be filtered", + Optional::No + }, + { + ConfigurationManager::PartFilterIdentifierType, + new StringInListVerifier({ + // Taken from ghoul::debugcontext.cpp + "Error", "Deprecated", "Undefined", "Portability", + "Performance", "Marker", "Push group", "Pop group", + "Other", "Don't care" + }), + "The type of the identifier to be filtered" + } + }), + "Individual OpenGL debug message identifiers" + }}), + "A list of OpenGL debug messages identifiers that are filtered", + Optional::Yes + }, + { + ConfigurationManager::PartFilterSeverity, + new TableVerifier({ + { + "*", + new StringInListVerifier( + // ghoul::debugcontext.cpp + { "High", "Medium", "Low", "Notification" } + ) + } + }), + "A list of severities that can are filtered out", + Optional::Yes + } + }), + "Determines the settings for the creation of an OpenGL debug context.", + Optional::Yes + } } }; }; diff --git a/src/engine/openspaceengine.cpp b/src/engine/openspaceengine.cpp index c6d7147bba..f36328ed04 100644 --- a/src/engine/openspaceengine.cpp +++ b/src/engine/openspaceengine.cpp @@ -67,6 +67,7 @@ #include #include #include +#include #include @@ -907,6 +908,111 @@ void OpenSpaceEngine::configureLogging() { void OpenSpaceEngine::initializeGL() { LTRACE("OpenSpaceEngine::initializeGL(begin)"); + const std::string key = ConfigurationManager::KeyOpenGLDebugContext; + if (_configurationManager->hasKey(key)) { + ghoul::Dictionary dict = _configurationManager->value(key); + bool debug = dict.value(ConfigurationManager::PartActivate); + + // Debug output is not available before 4.3 + const ghoul::systemcapabilities::Version minVersion = { 4, 3, 0 }; + if (OpenGLCap.openGLVersion() < minVersion) { + LINFO("OpenGL Debug context requested, but insufficient version available"); + debug = false; + } + + if (debug) { + using namespace ghoul::opengl::debug; + + bool synchronous = true; + if (dict.hasKey(ConfigurationManager::PartSynchronous)) { + synchronous = dict.value(ConfigurationManager::PartSynchronous); + } + + setDebugOutput(DebugOutput(debug), SynchronousOutput(synchronous)); + + + if (dict.hasKey(ConfigurationManager::PartFilterIdentifier)) { + ghoul::Dictionary filterDict = dict.value( + ConfigurationManager::PartFilterIdentifier + ); + + for (int i = 1; i <= filterDict.size(); ++i) { + ghoul::Dictionary id = filterDict.value( + std::to_string(i) + ); + + const unsigned int identifier = static_cast( + id.value( + ConfigurationManager::PartFilterIdentifierIdentifier + ) + ); + + const std::string s = id.value( + ConfigurationManager::PartFilterIdentifierSource + ); + + const std::string t = id.value( + ConfigurationManager::PartFilterIdentifierType + ); + + setDebugMessageControl( + ghoul::from_string(s), + ghoul::from_string(t), + { identifier }, + Enabled::No + ); + } + } + + if (dict.hasKey(ConfigurationManager::PartFilterSeverity)) { + ghoul::Dictionary filterDict = dict.value( + ConfigurationManager::PartFilterIdentifier + ); + + for (int i = 1; i <= filterDict.size(); ++i) { + std::string severity = filterDict.value( + std::to_string(i) + ); + + setDebugMessageControl( + Source::DontCare, + Type::DontCare, + ghoul::from_string(severity), + Enabled::No + ); + } + } + + auto callback = [](Source source, Type type, Severity severity, + unsigned int id, std::string message) -> void + { + const std::string s = std::to_string(source); + const std::string t = std::to_string(type); + + const std::string category = + "OpenGL (" + s + ") [" + t + "] {" + std::to_string(id) + "}"; + switch (severity) { + case Severity::High: + LERRORC(category, std::string(message)); + break; + case Severity::Medium: + LWARNINGC(category, std::string(message)); + break; + case Severity::Low: + LINFOC(category, std::string(message)); + break; + case Severity::Notification: + LDEBUGC(category, std::string(message)); + break; + default: + ghoul_assert(false, "Missing case label"); + } + }; + ghoul::opengl::debug::setDebugCallback(callback); + } + } + + LINFO("Initializing Rendering Engine"); // @CLEANUP: Remove the return statement and replace with exceptions ---abock _renderEngine->initializeGL(); diff --git a/tests/test_documentation.inl b/tests/test_documentation.inl index 2f1f168c0d..a1ee9982ff 100644 --- a/tests/test_documentation.inl +++ b/tests/test_documentation.inl @@ -80,6 +80,9 @@ TEST_F(DocumentationTest, Constructor) { doc.entries.emplace_back("NotInListInt", new IntNotInListVerifier({ 0, 1 })); doc.entries.emplace_back("NotInListString", new StringNotInListVerifier({ "", "a" })); + doc.entries.emplace_back("StringListVerifier", new StringListVerifier); + doc.entries.emplace_back("IntListVerifier", new IntListVerifier); + // Range Verifiers doc.entries.emplace_back("InListDouble", new DoubleInRangeVerifier({ 0.0, 1.0 })); doc.entries.emplace_back("InListInt", new IntInRangeVerifier({ 0, 1 })); @@ -393,6 +396,63 @@ TEST_F(DocumentationTest, StringListVerifierType) { EXPECT_EQ(TestResult::Offense::Reason::MissingKey, negativeRes.offenses[0].reason); } +TEST_F(DocumentationTest, IntListVerifierType) { + using namespace openspace::documentation; + using namespace std::string_literals; + + Documentation doc { + { { "IntList", new IntListVerifier } } + }; + + ghoul::Dictionary positive { + { + "IntList", + ghoul::Dictionary{ + { "1", 1 }, + { "2", 2 }, + { "3", 3 } + } + } + }; + TestResult positiveRes = testSpecification(doc, positive); + EXPECT_TRUE(positiveRes.success); + EXPECT_EQ(0, positiveRes.offenses.size()); + + ghoul::Dictionary negative{ + { "IntList", 0 } + }; + TestResult negativeRes = testSpecification(doc, negative); + EXPECT_FALSE(negativeRes.success); + ASSERT_EQ(1, negativeRes.offenses.size()); + EXPECT_EQ("IntList", negativeRes.offenses[0].offender); + EXPECT_EQ(TestResult::Offense::Reason::WrongType, negativeRes.offenses[0].reason); + + ghoul::Dictionary negative2 { + { + "IntList", + ghoul::Dictionary{ + { "1", "a"s }, + { "2", 1 }, + { "3", 2 } + } + } + }; + negativeRes = testSpecification(doc, negative2); + EXPECT_FALSE(negativeRes.success); + ASSERT_EQ(1, negativeRes.offenses.size()); + EXPECT_EQ("IntList.1", negativeRes.offenses[0].offender); + EXPECT_EQ(TestResult::Offense::Reason::WrongType, negativeRes.offenses[0].reason); + + ghoul::Dictionary negativeExist { + { "IntList2", ghoul::Dictionary{} } + }; + negativeRes = testSpecification(doc, negativeExist); + EXPECT_FALSE(negativeRes.success); + ASSERT_EQ(1, negativeRes.offenses.size()); + EXPECT_EQ("IntList", negativeRes.offenses[0].offender); + EXPECT_EQ(TestResult::Offense::Reason::MissingKey, negativeRes.offenses[0].reason); +} + TEST_F(DocumentationTest, MixedVerifiers) { using namespace openspace::documentation; using namespace std::string_literals;