From 9343c6315dabab2cf17a4ceafd8af02a6e52573d Mon Sep 17 00:00:00 2001 From: Andreas Engberg <48772850+engbergandreas@users.noreply.github.com> Date: Tue, 14 Nov 2023 15:40:30 +0100 Subject: [PATCH] Adds warning and error messages on loading screen (#2941) Fixes task 1 of (#494) * Removed loading screen progress bar It was removed because it does not show an accurate estimation of load times and is rarely used (off by default) * Add status info to the loading screen on warnings and errors Add: Warning logs and above to loading screen, fixes task 1 of (#494) Removed code duplication in openspanceengine.cpp Fixed some bugs where completed assets would not disappear from the screen * Update the design & address PR comments * Address PR comments & add bool to show/hide log msg * Delete test_configuration.cpp * Update Ghoul submodule * Renders number of warnings and errors to screen * Update renderengine.cpp * Adapt new function to the coding style --------- Co-authored-by: Alexander Bock --- ext/ghoul | 2 +- include/openspace/engine/configuration.h | 2 +- include/openspace/rendering/loadingscreen.h | 24 +- openspace.cfg | 3 +- src/engine/configuration.cpp | 10 +- src/engine/openspaceengine.cpp | 94 +++----- src/rendering/loadingscreen.cpp | 232 +++++++++++--------- src/rendering/renderengine.cpp | 16 +- src/scene/sceneinitializer.cpp | 1 - 9 files changed, 192 insertions(+), 192 deletions(-) diff --git a/ext/ghoul b/ext/ghoul index f658ead151..50cf606576 160000 --- a/ext/ghoul +++ b/ext/ghoul @@ -1 +1 @@ -Subproject commit f658ead151995fe77830b43b6559f95b6eaa2981 +Subproject commit 50cf6065769420d36c07dad0c9c5a7f60cfa7bd6 diff --git a/include/openspace/engine/configuration.h b/include/openspace/engine/configuration.h index 20232bf756..9d7a64c433 100644 --- a/include/openspace/engine/configuration.h +++ b/include/openspace/engine/configuration.h @@ -88,7 +88,7 @@ struct Configuration { struct LoadingScreen { bool isShowingMessages = true; bool isShowingNodeNames = true; - bool isShowingProgressbar = true; + bool isShowingLogMessages = true; }; LoadingScreen loadingScreen; diff --git a/include/openspace/rendering/loadingscreen.h b/include/openspace/rendering/loadingscreen.h index 0dec3d3986..dc3049ce8e 100644 --- a/include/openspace/rendering/loadingscreen.h +++ b/include/openspace/rendering/loadingscreen.h @@ -25,6 +25,7 @@ #ifndef __OPENSPACE_CORE___LOADINGSCREEN___H__ #define __OPENSPACE_CORE___LOADINGSCREEN___H__ +#include #include #include #include @@ -49,11 +50,11 @@ class LoadingScreen { public: BooleanType(ShowMessage); BooleanType(ShowNodeNames); - BooleanType(ShowProgressbar); + BooleanType(ShowLogMessages); BooleanType(CatastrophicError); LoadingScreen(ShowMessage showMessage, ShowNodeNames showNodeNames, - ShowProgressbar showProgressbar); + ShowLogMessages showLogMessages); ~LoadingScreen(); void render(); @@ -63,10 +64,6 @@ public: void finalize(); - void setItemNumber(int nItems); - int itemNumber(); - void tickItem(); - enum class Phase { PreStart, Construction, @@ -94,19 +91,21 @@ public: ItemStatus newStatus, ProgressInfo progressInfo); private: - bool _showMessage; - bool _showNodeNames; - bool _showProgressbar; + + void renderLogMessages() const; + + bool _showMessage = true; + bool _showNodeNames = true; + bool _showLog = true; Phase _phase = Phase::PreStart; - std::atomic_int _iProgress = 0; - std::atomic_int _nItems = 0; std::unique_ptr _logoTexture; std::shared_ptr _loadingFont; std::shared_ptr _messageFont; std::shared_ptr _itemFont; + std::shared_ptr _logFont; bool _hasCatastrophicErrorOccurred = false; std::string _message; @@ -130,6 +129,9 @@ private: std::random_device _randomDevice; std::default_random_engine _randomEngine; + + // Non owning but we remove the log from LogManager on destruction + ScreenLog* _log = nullptr; }; } // namespace openspace diff --git a/openspace.cfg b/openspace.cfg index 9d39a87a04..987762dc5f 100644 --- a/openspace.cfg +++ b/openspace.cfg @@ -217,8 +217,7 @@ VersionCheckUrl = "http://data.openspaceproject.com/latest-version" UseMultithreadedInitialization = true LoadingScreen = { ShowMessage = true, - ShowNodeNames = true, - ShowProgressbar = false + ShowNodeNames = true } CheckOpenGLState = false LogEachOpenGLCall = false diff --git a/src/engine/configuration.cpp b/src/engine/configuration.cpp index 780a4298c6..d33f041821 100644 --- a/src/engine/configuration.cpp +++ b/src/engine/configuration.cpp @@ -297,9 +297,9 @@ namespace { // initialized) std::optional showNodeNames; - // If this value is set to 'true', the loading screen will contain a progress - // bar that gives an estimate of the loading progression - std::optional showProgressbar; + // If this value is set to 'true', the loading screen will display a list of + // warning and error messages + std::optional showLogMessages; }; // Values in this table describe the behavior of the loading screen that is // displayed while the scene graph is created and initialized @@ -413,10 +413,10 @@ void parseLuaState(Configuration& configuration) { const Parameters::LoadingScreen& l = *p.loadingScreen; c.loadingScreen.isShowingMessages = l.showMessage.value_or(c.loadingScreen.isShowingMessages); - c.loadingScreen.isShowingProgressbar = - l.showProgressbar.value_or(c.loadingScreen.isShowingProgressbar); c.loadingScreen.isShowingNodeNames = l.showNodeNames.value_or(c.loadingScreen.isShowingNodeNames); + c.loadingScreen.isShowingLogMessages = + l.showLogMessages.value_or(c.loadingScreen.isShowingLogMessages); } c.moduleConfigurations = p.moduleConfigurations.value_or(c.moduleConfigurations); diff --git a/src/engine/openspaceengine.cpp b/src/engine/openspaceengine.cpp index c273cf3bcc..0d79e58b38 100644 --- a/src/engine/openspaceengine.cpp +++ b/src/engine/openspaceengine.cpp @@ -62,6 +62,7 @@ #include #include #include +#include #include #include #include @@ -477,17 +478,13 @@ void OpenSpaceEngine::initializeGL() { LoadingScreen::ShowNodeNames( global::configuration->loadingScreen.isShowingNodeNames ), - LoadingScreen::ShowProgressbar( - global::configuration->loadingScreen.isShowingProgressbar + LoadingScreen::ShowLogMessages( + global::configuration->loadingScreen.isShowingLogMessages ) - ); + ); _loadingScreen->render(); - - - - LTRACE("OpenSpaceEngine::initializeGL::Console::initialize(begin)"); try { global::luaConsole->initialize(); @@ -743,6 +740,8 @@ void OpenSpaceEngine::loadAssets() { _loadingScreen->setPhase(LoadingScreen::Phase::Construction); _loadingScreen->postMessage("Loading assets"); + std::unordered_set finishedSynchronizations; + while (true) { _loadingScreen->render(); _assetManager->update(); @@ -752,59 +751,20 @@ void OpenSpaceEngine::loadAssets() { std::vector allSyncs = _assetManager->allSynchronizations(); - for (const ResourceSynchronization* sync : allSyncs) { - ZoneScopedN("Update resource synchronization"); - - if (sync->isSyncing()) { - LoadingScreen::ProgressInfo progressInfo; - - progressInfo.progress = [](const ResourceSynchronization* s) { - if (!s->nTotalBytesIsKnown()) { - return 0.f; - } - if (s->nTotalBytes() == 0) { - return 1.f; - } - return - static_cast(s->nSynchronizedBytes()) / - static_cast(s->nTotalBytes()); - }(sync); - - _loadingScreen->updateItem( - sync->identifier(), - sync->name(), - LoadingScreen::ItemStatus::Started, - progressInfo - ); + // Filter already synchronized assets so we don't check them anymore + auto syncIt = std::remove_if( + allSyncs.begin(), + allSyncs.end(), + [&finishedSynchronizations](const ResourceSynchronization* sync) { + return finishedSynchronizations.contains(sync); } - - if (sync->isRejected()) { - _loadingScreen->updateItem( - sync->identifier(), sync->name(), LoadingScreen::ItemStatus::Failed, - LoadingScreen::ProgressInfo() - ); - } - } - - _loadingScreen->setItemNumber(static_cast(allSyncs.size())); - - if (_shouldAbortLoading) { - global::windowDelegate->terminate(); - break; - } - - bool finishedLoading = std::all_of( - allAssets.begin(), - allAssets.end(), - [](const Asset* asset) { return asset->isInitialized() || asset->isFailed(); } ); - - if (finishedLoading) { - break; - } + allSyncs.erase(syncIt, allSyncs.end()); auto it = allSyncs.begin(); while (it != allSyncs.end()) { + ZoneScopedN("Update resource synchronization"); + if ((*it)->isSyncing()) { LoadingScreen::ProgressInfo progressInfo; @@ -835,7 +795,9 @@ void OpenSpaceEngine::loadAssets() { } else if ((*it)->isRejected()) { _loadingScreen->updateItem( - (*it)->identifier(), (*it)->name(), LoadingScreen::ItemStatus::Failed, + (*it)->identifier(), + (*it)->name(), + LoadingScreen::ItemStatus::Failed, LoadingScreen::ProgressInfo() ); ++it; @@ -844,17 +806,33 @@ void OpenSpaceEngine::loadAssets() { LoadingScreen::ProgressInfo progressInfo; progressInfo.progress = 1.f; - _loadingScreen->tickItem(); _loadingScreen->updateItem( (*it)->identifier(), (*it)->name(), LoadingScreen::ItemStatus::Finished, progressInfo ); + finishedSynchronizations.insert(*it); it = allSyncs.erase(it); } } - } + + if (_shouldAbortLoading) { + global::windowDelegate->terminate(); + break; + } + + bool finishedLoading = std::all_of( + allAssets.begin(), + allAssets.end(), + [](const Asset* asset) { return asset->isInitialized() || asset->isFailed(); } + ); + + if (finishedLoading) { + break; + } + } // while(true) + if (_shouldAbortLoading) { _loadingScreen = nullptr; return; diff --git a/src/rendering/loadingscreen.cpp b/src/rendering/loadingscreen.cpp index ba8672113f..a50aed0e92 100644 --- a/src/rendering/loadingscreen.cpp +++ b/src/rendering/loadingscreen.cpp @@ -47,16 +47,11 @@ namespace { constexpr float LoadingFontSize = 25.f; constexpr float MessageFontSize = 22.f; constexpr float ItemFontSize = 10.f; + constexpr float LogFontSize = 10.f; constexpr glm::vec2 LogoCenter = glm::vec2(0.f, 0.525f); // in NDC constexpr glm::vec2 LogoSize = glm::vec2(0.275f, 0.275); // in NDC - constexpr glm::vec2 ProgressbarCenter = glm::vec2(0.f, -0.75f); // in NDC - constexpr glm::vec2 ProgressbarSize = glm::vec2(0.7f, 0.0075f); // in NDC - constexpr float ProgressbarLineWidth = 0.0025f; // in NDC - - constexpr glm::vec4 ProgressbarOutlineColor = glm::vec4(0.9f, 0.9f, 0.9f, 1.f); - constexpr glm::vec4 PhaseColorConstruction = glm::vec4(0.7f, 0.7f, 0.f, 1.f); constexpr glm::vec4 PhaseColorSynchronization = glm::vec4(0.9f, 0.9f, 0.9f, 1.f); constexpr glm::vec4 PhaseColorInitialization = glm::vec4(0.1f, 0.75f, 0.1f, 1.f); @@ -70,6 +65,7 @@ namespace { constexpr float LoadingTextPosition = 0.275f; // in NDC constexpr float StatusMessageOffset = 0.225f; // in NDC + constexpr float LogBackgroundPosition = 0.125f; // in NDC constexpr int MaxNumberLocationSamples = 1000; @@ -103,15 +99,25 @@ namespace { namespace openspace { LoadingScreen::LoadingScreen(ShowMessage showMessage, ShowNodeNames showNodeNames, - ShowProgressbar showProgressbar) + ShowLogMessages showLogMessages) : _showMessage(showMessage) , _showNodeNames(showNodeNames) - , _showProgressbar(showProgressbar) + , _showLog(showLogMessages) , _randomEngine(_randomDevice()) { + constexpr std::chrono::seconds ScreenLogTimeToLive(20); + std::unique_ptr log = std::make_unique( + ScreenLogTimeToLive, + ScreenLog::LogLevel::Warning + ); + _log = log.get(); + ghoul::logging::LogManager::ref().addLog(std::move(log)); + + const float fontScaling = global::windowDelegate->osDpiScaling(); + _loadingFont = global::fontManager->font( "Loading", - LoadingFontSize, + LoadingFontSize * fontScaling, ghoul::fontrendering::FontManager::Outline::No, ghoul::fontrendering::FontManager::LoadGlyphs::No ); @@ -119,7 +125,7 @@ LoadingScreen::LoadingScreen(ShowMessage showMessage, ShowNodeNames showNodeName if (_showMessage) { _messageFont = global::fontManager->font( "Loading", - MessageFontSize, + MessageFontSize * fontScaling, ghoul::fontrendering::FontManager::Outline::No, ghoul::fontrendering::FontManager::LoadGlyphs::No ); @@ -128,7 +134,16 @@ LoadingScreen::LoadingScreen(ShowMessage showMessage, ShowNodeNames showNodeName if (_showNodeNames) { _itemFont = global::fontManager->font( "Loading", - ItemFontSize, + ItemFontSize * fontScaling, + ghoul::fontrendering::FontManager::Outline::No, + ghoul::fontrendering::FontManager::LoadGlyphs::No + ); + } + + if (_showLog) { + _logFont = global::fontManager->font( + "Loading", + LogFontSize * fontScaling, ghoul::fontrendering::FontManager::Outline::No, ghoul::fontrendering::FontManager::LoadGlyphs::No ); @@ -150,6 +165,8 @@ LoadingScreen::~LoadingScreen() { _loadingFont = nullptr; _messageFont = nullptr; _itemFont = nullptr; + ghoul::logging::LogManager::ref().removeLog(_log); + _log = nullptr; } void LoadingScreen::render() { @@ -201,59 +218,6 @@ void LoadingScreen::render() { rendering::helper::Anchor::Center ); - // - // Render progress bar - // - const glm::vec2 progressbarSize = glm::vec2( - ProgressbarSize.x, - ProgressbarSize.y * screenAspectRatio - ); - - if (_showProgressbar) { - const float progress = _nItems != 0 ? - static_cast(_iProgress) / static_cast(_nItems) : - 0.f; - - const float w = ProgressbarLineWidth / screenAspectRatio; - const float h = ProgressbarLineWidth; - rendering::helper::renderBox( - glm::vec2(1.f) - ((ProgressbarCenter + glm::vec2(1.f)) / 2.f), - progressbarSize + glm::vec2(2 * w, 2 * h), - ProgressbarOutlineColor, - rendering::helper::Anchor::Center - ); - - rendering::helper::renderBox( - glm::vec2(1.f) - ((ProgressbarCenter + glm::vec2(1.f)) / 2.f), - progressbarSize, - glm::vec4(0.f, 0.f, 0.f, 1.f), - rendering::helper::Anchor::Center - ); - - glm::vec4 color = glm::vec4(0.f); - switch (_phase) { - case Phase::PreStart: - break; - case Phase::Construction: - color = PhaseColorConstruction; - break; - case Phase::Synchronization: - color = PhaseColorSynchronization; - break; - case Phase::Initialization: - color = PhaseColorInitialization; - break; - } - - glm::vec2 p = glm::vec2(1.f) - ((ProgressbarCenter + glm::vec2(1.f)) / 2.f); - rendering::helper::renderBox( - p - progressbarSize / 2.f, - progressbarSize * glm::vec2(progress, 1.f), - color, - rendering::helper::Anchor::NW - ); - } - // // "Loading" text // @@ -295,6 +259,21 @@ void LoadingScreen::render() { renderer.render(*_messageFont, messageLl, _message); } + glm::vec2 logLl = glm::vec2(0.f, 0.f); + glm::vec2 logUr = glm::vec2(res.x, res.y * (LogBackgroundPosition + 0.015)); + + // Font rendering enables depth testing so we disable again to render the log box + glDisable(GL_DEPTH_TEST); + if (_showLog) { + constexpr glm::vec4 DarkGray = glm::vec4(glm::vec3(0.04f), 1.f); + rendering::helper::renderBox( + glm::vec2(0.f, 1.f), + glm::vec2(1.f, LogBackgroundPosition), + DarkGray, + rendering::helper::Anchor::SW + ); + } + if (_showNodeNames) { std::lock_guard guard(_itemsMutex); @@ -303,15 +282,6 @@ void LoadingScreen::render() { const glm::vec2 logoLl = glm::vec2(LogoCenter.x - size.x, LogoCenter.y - size.y); const glm::vec2 logoUr = glm::vec2(LogoCenter.x + size.x, LogoCenter.y + size.y); - const glm::vec2 progressbarLl = glm::vec2( - ProgressbarCenter.x - progressbarSize.x, - ProgressbarCenter.y - progressbarSize.y - ); - const glm::vec2 progressbarUr = glm::vec2( - ProgressbarCenter.x + progressbarSize.x , - ProgressbarCenter.y + progressbarSize.y - ); - for (Item& item : _items) { if (!item.hasLocation) { // Compute a new location @@ -351,16 +321,12 @@ void LoadingScreen::render() { rectOverlaps(messageLl, messageUr, ll, ur) : false; - const bool barOverlap = _showProgressbar ? - rectOverlaps( - ndcToScreen(progressbarLl, res), - ndcToScreen(progressbarUr, res), - ll, - ur - ) : - false; + const bool logOverlap = rectOverlaps( + logLl, logUr, + ll, ur + ); - if (logoOverlap || loadingOverlap || messageOverlap || barOverlap) { + if (logoOverlap || loadingOverlap || messageOverlap || logOverlap) { // We never want to have an overlap with these, so this try didn't // count against the maximum, thus ensuring that (if there has to // be an overlap, it's over other text that might disappear before @@ -462,7 +428,7 @@ void LoadingScreen::render() { _items.end(), [now](const Item& i) { if (i.status == ItemStatus::Finished) { - return i.finishedTime > now + TTL; + return (i.finishedTime + TTL) < now; } else { return false; @@ -474,6 +440,11 @@ void LoadingScreen::render() { } + // Render log messages last to make them slightly more visible if a download item + // is slightly overlapping + if (_showLog) { + renderLogMessages(); + } glEnable(GL_CULL_FACE); glEnable(GL_DEPTH_TEST); @@ -482,6 +453,82 @@ void LoadingScreen::render() { FrameMarkEnd("Loading"); } +void LoadingScreen::renderLogMessages() const { + ZoneScoped; + + constexpr size_t MaxNumberMessages = 6; + constexpr int MessageLength = 209; + + using FR = ghoul::fontrendering::FontRenderer; + const FR& renderer = FR::defaultRenderer(); + + const std::vector& entries = _log->entries(); + + size_t nRows = 0; + size_t j = std::min(MaxNumberMessages, entries.size()); + for (size_t i = 1; i <= j; i++) { + ZoneScopedN("Entry"); + + // Show only the j:th first log entries + const ScreenLog::LogEntry& it = entries[j - i]; + + std::ostringstream result; + // Split really long messages into multiple lines for better readability + if (it.message.size() > MessageLength) { + std::istringstream is(it.message); + + int charactersSinceNewLine = 0; + std::string word; + while (is >> word) { + charactersSinceNewLine += static_cast(word.size()); + // Insert a new line when we exceede messageLength + if (charactersSinceNewLine > MessageLength) { + result << '\n'; + charactersSinceNewLine = static_cast(word.size()); + ++nRows; + } + result << word << ' '; + ++charactersSinceNewLine; + } + } + + renderer.render( + *_logFont, + glm::vec2( + 10, + 10 + _logFont->pointSize() * nRows * 2 + ), + it.message.size() < MessageLength ? it.message : result.str(), + ghoul::toColor(it.level) + ); + ++nRows; + } + + const glm::vec2 dpiScaling = global::windowDelegate->dpiScaling(); + const glm::ivec2 res = + glm::vec2(global::windowDelegate->firstWindowResolution()) * dpiScaling; + + // Render # of warnings and error messages + std::map numberOfErrorsPerLevel; + for (auto& entry : _log->entries()) { + numberOfErrorsPerLevel[entry.level]++; + } + size_t row = 0; + for (auto& [level, amount] : numberOfErrorsPerLevel) { + const std::string text = fmt::format("{}: {}", ghoul::to_string(level), amount); + renderer.render( + *_logFont, + glm::vec2( + res.x - 0.07 * res.x, + 10 + _logFont->pointSize() * row * 2 + ), + text, + ghoul::toColor(level) + ); + ++row; + } +} + void LoadingScreen::postMessage(std::string message) { std::lock_guard guard(_messageMutex); _message = std::move(message); @@ -502,26 +549,13 @@ void LoadingScreen::finalize() { ), _items.end() ); - + _log->removeExpiredEntries(); + _showLog = _showLog && !_log->entries().empty(); render(); } - -void LoadingScreen::setItemNumber(int nItems) { - _nItems = nItems; -} - -int LoadingScreen::itemNumber() { - return _nItems; -} - -void LoadingScreen::tickItem() { - ++_iProgress; -} - void LoadingScreen::setPhase(Phase phase) { _phase = phase; - _iProgress = 0; } void LoadingScreen::updateItem(const std::string& itemIdentifier, @@ -555,7 +589,7 @@ void LoadingScreen::updateItem(const std::string& itemIdentifier, Item item = { .identifier = itemIdentifier, .name = itemName, - .status = ItemStatus::Started, + .status = newStatus, .progress = std::move(progressInfo), .hasLocation = false, .finishedTime = std::chrono::system_clock::from_time_t(0) diff --git a/src/rendering/renderengine.cpp b/src/rendering/renderengine.cpp index ec526678e3..a7d50bb032 100644 --- a/src/rendering/renderengine.cpp +++ b/src/rendering/renderengine.cpp @@ -1395,20 +1395,8 @@ void RenderEngine::renderScreenLog() { } { - const glm::vec4 color = [alpha, white](ScreenLog::LogLevel level) { - switch (level) { - case ghoul::logging::LogLevel::Debug: - return glm::vec4(0.f, 1.f, 0.f, alpha); - case ghoul::logging::LogLevel::Warning: - return glm::vec4(1.f, 1.f, 0.f, alpha); - case ghoul::logging::LogLevel::Error: - return glm::vec4(1.f, 0.f, 0.f, alpha); - case ghoul::logging::LogLevel::Fatal: - return glm::vec4(0.3f, 0.3f, 0.85f, alpha); - default: - return white; - } - }(it.level); + glm::vec4 color = ghoul::toColor(it.level); + color.a = alpha; const std::string_view lvl = ghoul::to_string(it.level); std::fill(buf.begin(), buf.end(), char(0)); diff --git a/src/scene/sceneinitializer.cpp b/src/scene/sceneinitializer.cpp index 6728338be7..7353a020fb 100644 --- a/src/scene/sceneinitializer.cpp +++ b/src/scene/sceneinitializer.cpp @@ -90,7 +90,6 @@ void MultiThreadedSceneInitializer::initializeNode(SceneGraphNode* node) { LoadingScreen* loadingScreen = global::openSpaceEngine->loadingScreen(); if (loadingScreen) { - loadingScreen->setItemNumber(loadingScreen->itemNumber() + 1); loadingScreen->updateItem( node->identifier(), node->guiName(),