From 548d673e8bf968b6d8d0567e4fe36dab236faab3 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Thu, 11 Feb 2021 13:43:36 +0100 Subject: [PATCH 01/22] Add drag and drop support for images and assets (#1497) --- apps/OpenSpace/main.cpp | 12 ++++++ include/openspace/engine/openspaceengine.h | 1 + scripts/drag_drop_handler.lua | 43 ++++++++++++++++++++ src/engine/openspaceengine.cpp | 47 ++++++++++++++++++++++ 4 files changed, 103 insertions(+) create mode 100644 scripts/drag_drop_handler.lua diff --git a/apps/OpenSpace/main.cpp b/apps/OpenSpace/main.cpp index b370da39e6..2efc3e7061 100644 --- a/apps/OpenSpace/main.cpp +++ b/apps/OpenSpace/main.cpp @@ -660,6 +660,17 @@ void mainCharCallback(unsigned int codepoint, int modifiers) { +void mainDropCallback(int amount, const char** paths) { + ghoul_assert(amount > 0, "Expected at least one file path"); + ghoul_assert(paths, "expected non-nullptr"); + + for (int i = 0; i < amount; ++i) { + global::openSpaceEngine->handleDragDrop(paths[i]); + } +} + + + std::vector mainEncodeFun() { ZoneScoped LTRACE("main::mainEncodeFun(begin)"); @@ -1252,6 +1263,7 @@ int main(int argc, char* argv[]) { callbacks.mousePos = mainMousePosCallback; callbacks.mouseScroll = mainMouseScrollCallback; callbacks.character = mainCharCallback; + callbacks.drop = mainDropCallback; callbacks.encode = mainEncodeFun; callbacks.decode = mainDecodeFun; Log::instance().setNotifyLevel(Log::Level::Debug); diff --git a/include/openspace/engine/openspaceengine.h b/include/openspace/engine/openspaceengine.h index 42612a9368..2710a709f3 100644 --- a/include/openspace/engine/openspaceengine.h +++ b/include/openspace/engine/openspaceengine.h @@ -86,6 +86,7 @@ public: void touchDetectionCallback(TouchInput input); void touchUpdateCallback(TouchInput input); void touchExitCallback(TouchInput input); + void handleDragDrop(const std::string& file); std::vector encode(); void decode(std::vector data); diff --git a/scripts/drag_drop_handler.lua b/scripts/drag_drop_handler.lua new file mode 100644 index 0000000000..7042333633 --- /dev/null +++ b/scripts/drag_drop_handler.lua @@ -0,0 +1,43 @@ +-- This script gets two parameters in its global scope: +-- filename: The full path for the file that was dropped on the application. +-- Example: C:/OpenSpace/openspace.cfg +-- basename: Only the name of the actual file with extension, but without the full rest +-- of the path. +-- Example: openspace.cfg +-- extension: The extention of the file +-- Example: .cfg +-- +-- From this script, we need to return the script that we want to be executed in response +-- to the drag event. If we don't want anything to happen, don't return anything or +-- return an empty string + +if filename == nil or filename == "" or + basename == nil or basename == "" or + extension == nil or extension == "" then + do return "" end +end + +-- Lua doesn't enjoy \ that are used by Windows extensively. So we convert all \ into / +filename = filename:gsub("\\", "/") +basename = basename:gsub("\\", "/") +basename_without_extension = basename:sub(0, #basename - extension:len()) + +is_image_file = function(extension) + return extension == ".png" or extension == ".jpg" or extension == ".jpeg" or + extension == ".tif" or extension == ".tga" or extension == ".bmp" or + extension == ".psd" or extension == ".gif" or extension == ".hdr" or + extension == ".pic" or extension == ".pnm" +end + +if is_image_file(extension) then + identifier = basename_without_extension:gsub(" ", "_") + return [[openspace.addScreenSpaceRenderable({ + Identifier = "]] .. identifier .. [[", + Type = "ScreenSpaceImageLocal", + TexturePath = "]] .. filename .. [[" + });]] +elseif extension == ".asset" then + return [[openspace.asset.add("]] .. filename .. [[")]] +elseif extension == ".osrec" or extension == ".osrectxt" then + return [[openspace.sessionRecording.startPlayback("]] .. basename .. [[")]] +end diff --git a/src/engine/openspaceengine.cpp b/src/engine/openspaceengine.cpp index 00c135152d..3d1d6dd868 100644 --- a/src/engine/openspaceengine.cpp +++ b/src/engine/openspaceengine.cpp @@ -83,6 +83,7 @@ #include #include #include +#include #include #include #include @@ -1444,6 +1445,52 @@ void OpenSpaceEngine::touchExitCallback(TouchInput input) { } } +void OpenSpaceEngine::handleDragDrop(const std::string& file) { + std::filesystem::path f(file); + + ghoul::lua::LuaState s(ghoul::lua::LuaState::IncludeStandardLibrary::Yes); + std::string absolutePath = absPath("${SCRIPTS}/drag_drop_handler.lua"); + int status = luaL_loadfile(s, absolutePath.c_str()); + if (status != LUA_OK) { + std::string error = lua_tostring(s, -1); + LERROR(error); + return; + } + + ghoul::lua::push(s, file); + lua_setglobal(s, "filename"); + + std::string basename = f.filename().string(); + ghoul::lua::push(s, basename); + lua_setglobal(s, "basename"); + + std::string extension = f.extension().string(); + std::transform( + extension.begin(), extension.end(), + extension.begin(), + [](char c) { return static_cast(::tolower(c)); } + ); + ghoul::lua::push(s, extension); + lua_setglobal(s, "extension"); + + status = lua_pcall(s, 0, 1, 0); + if (status != LUA_OK) { + std::string error = lua_tostring(s, -1); + LERROR(error); + return; + } + + if (lua_isnil(s, -1)) { + LWARNING(fmt::format("Unhandled file dropped: {}", file)); + return; + } + + std::string script = ghoul::lua::value(s); + global::scriptEngine->queueScript( + script, + scripting::ScriptEngine::RemoteScripting::Yes + ); +} std::vector OpenSpaceEngine::encode() { ZoneScoped From 88122d1dbbc9cef9c48bf78bc6222063aa8b50e6 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Thu, 11 Feb 2021 14:44:13 +0100 Subject: [PATCH 02/22] Adapt to new Ghoul; fix previously undetected errors in uniform setting --- ext/ghoul | 2 +- modules/base/rendering/renderablemodel.cpp | 6 ++---- .../rendering/renderableplanetprojection.cpp | 2 +- .../volume/rendering/basicvolumeraycaster.cpp | 4 ++-- modules/volume/tasks/generaterawvolumetask.cpp | 8 ++++++++ src/documentation/documentation.cpp | 17 ++++++++++------- 6 files changed, 24 insertions(+), 15 deletions(-) diff --git a/ext/ghoul b/ext/ghoul index 2f1bb7c483..6cc191d429 160000 --- a/ext/ghoul +++ b/ext/ghoul @@ -1 +1 @@ -Subproject commit 2f1bb7c483f4792317b8f1a3da17e2270c0d0ae7 +Subproject commit 6cc191d4293a68faf559d090dd33e340b9137d63 diff --git a/modules/base/rendering/renderablemodel.cpp b/modules/base/rendering/renderablemodel.cpp index ec98dc83e8..274bdd1d6a 100644 --- a/modules/base/rendering/renderablemodel.cpp +++ b/modules/base/rendering/renderablemodel.cpp @@ -438,13 +438,11 @@ void RenderableModel::render(const RenderData& data, RendererTasks&) { ); _program->setUniform( _uniformCache.lightIntensities, - _lightIntensitiesBuffer.data(), - nLightSources + _lightIntensitiesBuffer ); _program->setUniform( _uniformCache.lightDirectionsViewSpace, - _lightDirectionsViewSpaceBuffer.data(), - nLightSources + _lightDirectionsViewSpaceBuffer ); _program->setUniform( _uniformCache.modelViewTransform, diff --git a/modules/spacecraftinstruments/rendering/renderableplanetprojection.cpp b/modules/spacecraftinstruments/rendering/renderableplanetprojection.cpp index 23ac686770..4c2f2479ee 100644 --- a/modules/spacecraftinstruments/rendering/renderableplanetprojection.cpp +++ b/modules/spacecraftinstruments/rendering/renderableplanetprojection.cpp @@ -481,7 +481,7 @@ void RenderablePlanetProjection::imageProjectGPU( if (_geometry->hasProperty("Radius")) { std::any r = _geometry->property("Radius")->get(); if (glm::vec3* radius = std::any_cast(&r)){ - _fboProgramObject->setUniform(_fboUniformCache.radius, radius); + _fboProgramObject->setUniform(_fboUniformCache.radius, *radius); } } else { diff --git a/modules/volume/rendering/basicvolumeraycaster.cpp b/modules/volume/rendering/basicvolumeraycaster.cpp index d4f8516134..ed547063ca 100644 --- a/modules/volume/rendering/basicvolumeraycaster.cpp +++ b/modules/volume/rendering/basicvolumeraycaster.cpp @@ -135,8 +135,8 @@ void BasicVolumeRaycaster::preRaycast(const RaycastData& data, int nClips = static_cast(clipNormals.size()); program.setUniform("nClips_" + id, nClips); - program.setUniform("clipNormals_" + id, clipNormals.data(), nClips); - program.setUniform("clipOffsets_" + id, clipOffsets.data(), nClips); + program.setUniform("clipNormals_" + id, clipNormals); + program.setUniform("clipOffsets_" + id, clipOffsets); program.setUniform("opacity_" + id, _opacity); program.setUniform("rNormalization_" + id, _rNormalization); program.setUniform("rUpperBound_" + id, _rUpperBound); diff --git a/modules/volume/tasks/generaterawvolumetask.cpp b/modules/volume/tasks/generaterawvolumetask.cpp index 68e889fc72..6de0d6af6a 100644 --- a/modules/volume/tasks/generaterawvolumetask.cpp +++ b/modules/volume/tasks/generaterawvolumetask.cpp @@ -101,11 +101,15 @@ void GenerateRawVolumeTask::perform(const Task::ProgressCallback& progressCallba ghoul::lua::LuaState state; ghoul::lua::runScript(state, _valueFunctionLua); +#if (defined(NDEBUG) || defined(DEBUG)) ghoul::lua::verifyStackSize(state, 1); +#endif int functionReference = luaL_ref(state, LUA_REGISTRYINDEX); +#if (defined(NDEBUG) || defined(DEBUG)) ghoul::lua::verifyStackSize(state, 0); +#endif glm::vec3 domainSize = _upperDomainBound - _lowerDomainBound; @@ -117,14 +121,18 @@ void GenerateRawVolumeTask::perform(const Task::ProgressCallback& progressCallba glm::vec3 coord = _lowerDomainBound + glm::vec3(cell) / glm::vec3(_dimensions) * domainSize; +#if (defined(NDEBUG) || defined(DEBUG)) ghoul::lua::verifyStackSize(state, 0); +#endif lua_rawgeti(state, LUA_REGISTRYINDEX, functionReference); lua_pushnumber(state, coord.x); lua_pushnumber(state, coord.y); lua_pushnumber(state, coord.z); +#if (defined(NDEBUG) || defined(DEBUG)) ghoul::lua::verifyStackSize(state, 4); +#endif if (lua_pcall(state, 3, 1, 0) != LUA_OK) { return; diff --git a/src/documentation/documentation.cpp b/src/documentation/documentation.cpp index ef1bfc8843..4cf633f28e 100644 --- a/src/documentation/documentation.cpp +++ b/src/documentation/documentation.cpp @@ -129,17 +129,20 @@ namespace openspace::documentation { const std::string DocumentationEntry::Wildcard = "*"; +std::string concatenate(const std::vector& offenses) { + std::string result = "Error in specification ("; + for (const TestResult::Offense& o : offenses) { + result += o.offender + ','; + } + result.back() = ')'; + return result; +} + SpecificationError::SpecificationError(TestResult res, std::string comp) - : ghoul::RuntimeError("Error in specification", std::move(comp)) + : ghoul::RuntimeError(concatenate(res.offenses), std::move(comp)) , result(std::move(res)) { ghoul_assert(!result.success, "Result's success must be false"); - - message += " ("; - for (const TestResult::Offense& o : result.offenses) { - message += o.offender + ','; - } - message.back() = ')'; } DocumentationEntry::DocumentationEntry(std::string k, std::shared_ptr v, From aeb62978ab1641479ac7cd469b2f8488487f9248 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Thu, 11 Feb 2021 14:51:53 +0100 Subject: [PATCH 03/22] Set the default startup project in Visual Studio to 'OpenSpace' (closes #1503) --- CMakeLists.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index a3c2a8c767..96ab1ca6c9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -203,6 +203,12 @@ begin_header("Configuring Applications") add_subdirectory("${OPENSPACE_APPS_DIR}") end_header("End: Configuring Applications") + +if (MSVC AND TARGET OpenSpace) + set_property(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} PROPERTY VS_STARTUP_PROJECT OpenSpace) +endif() + + option(OPENSPACE_HAVE_TESTS "Activate the OpenSpace unit tests" ON) if (OPENSPACE_HAVE_TESTS) begin_header("Generating OpenSpace unit test") From 11e5bc36abc2cc122f2c5d8ad4ab6c01634a26ac Mon Sep 17 00:00:00 2001 From: Emma Broman Date: Thu, 11 Feb 2021 16:24:36 +0100 Subject: [PATCH 04/22] Prevent exception when querying renderable of non-existing SGN --- src/query/query.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/query/query.cpp b/src/query/query.cpp index 5f84919aab..6ab37a893d 100644 --- a/src/query/query.cpp +++ b/src/query/query.cpp @@ -45,6 +45,9 @@ SceneGraphNode* sceneGraphNode(const std::string& name) { const Renderable* renderable(const std::string& name) { SceneGraphNode* node = sceneGraphNode(name); + if (!node) { + return nullptr; + } return node->renderable(); } From e70ba0075728faa56de035637211775948e4d95f Mon Sep 17 00:00:00 2001 From: Malin Ejdbo Date: Fri, 12 Feb 2021 09:19:14 +0100 Subject: [PATCH 05/22] Remove regex from scene_lua.inl * Replace with find functions --- src/scene/scene_lua.inl | 228 +++++++++++++++++++++++++++------------- 1 file changed, 153 insertions(+), 75 deletions(-) diff --git a/src/scene/scene_lua.inl b/src/scene/scene_lua.inl index be155ed426..2134a6716b 100644 --- a/src/scene/scene_lua.inl +++ b/src/scene/scene_lua.inl @@ -27,7 +27,6 @@ #include #include #include -#include namespace openspace { @@ -71,17 +70,57 @@ void applyRegularExpression(lua_State* L, const std::string& regex, const int type = lua_type(L, -1); + // Extract the property and node name to be searched for from regex + std::string propertyName = ""; + std::string nodeName = ""; + size_t wildPos = regex.find_first_of("*"); + if (wildPos != std::string::npos) { + propertyName = regex.substr(wildPos + 1, regex.length()); + nodeName = regex.substr(0, wildPos); + + if (propertyName.empty()) { + LERRORC( + "applyRegularExpression", + fmt::format( + "Malformed regular expression: '{}': " + "Could not find any propertry name after '*'", regex + ) + ); + return; + } + if (!isGroupMode && nodeName.empty()) { + LERRORC( + "applyRegularExpression", + fmt::format( + "Malformed regular expression: '{}': " + "Could not find any node name before '*'", regex + ) + ); + return; + } + } + else { + LERRORC( + "applyRegularExpression", + fmt::format( + "Malformed regular expression: '{}': Could not find '*'", regex + ) + ); + return; + } + // Stores whether we found at least one matching property. If this is false at the end // of the loop, the property name regex was probably misspelled. bool foundMatching = false; - std::regex r(regex); for (properties::Property* prop : properties) { // Check the regular expression for all properties const std::string& id = prop->fullyQualifiedIdentifier(); - if (std::regex_match(id, r)) { + if (id.find(propertyName) != std::string::npos) { // If the fully qualified id matches the regular expression, we queue the // value change if the types agree + + // Check tag if (isGroupMode) { properties::PropertyOwner* matchingTaggedOwner = findPropertyOwnerWithMatchingGroupTag( @@ -92,6 +131,12 @@ void applyRegularExpression(lua_State* L, const std::string& regex, continue; } } + else { + // Match node name + if (id.find(nodeName) == std::string::npos) { + continue; + } + } if (type != prop->typeLua()) { LERRORC( @@ -153,12 +198,7 @@ bool doesUriContainGroupTag(const std::string& command, std::string& groupName) std::string replaceUriWithGroupName(const std::string& uri, const std::string& ownerName) { size_t pos = uri.find_first_of("."); - return ownerName + "." + uri.substr(pos); -} - -std::string extractUriWithoutGroupName(const std::string& uri) { - size_t pos = uri.find_first_of("."); - return uri.substr(pos); + return ownerName + uri.substr(pos); } } // namespace @@ -275,60 +315,31 @@ int property_setValue(lua_State* L) { } if (optimization.empty()) { - // Replace all wildcards * with the correct regex (.*) - size_t startPos = uriOrRegex.find("*"); - while (startPos != std::string::npos) { - uriOrRegex.replace(startPos, 1, "(.*)"); - startPos += 4; // (.*) - startPos = uriOrRegex.find("*", startPos); - } - std::string groupName; if (doesUriContainGroupTag(uriOrRegex, groupName)) { - std::string pathRemainderToMatch = extractUriWithoutGroupName(uriOrRegex); - // Remove group name from start of regex and replace with '.*' - uriOrRegex = replaceUriWithGroupName(uriOrRegex, ".*"); + // Remove group name from start of regex and replace with '*' + uriOrRegex = replaceUriWithGroupName(uriOrRegex, "*"); } - try { - applyRegularExpression( - L, - uriOrRegex, - allProperties(), - interpolationDuration, - groupName, - easingMethod - ); - } - catch (const std::regex_error& e) { - LERRORC( - "property_setValue", - fmt::format( - "Malformed regular expression: '{}': {}", uriOrRegex, e.what() - ) - ); - } + applyRegularExpression( + L, + uriOrRegex, + allProperties(), + interpolationDuration, + groupName, + easingMethod + ); return 0; } else if (optimization == "regex") { - try { - applyRegularExpression( - L, - uriOrRegex, - allProperties(), - interpolationDuration, - "", - easingMethod - ); - } - catch (const std::regex_error& e) { - LERRORC( - "property_setValueRegex", - fmt::format( - "Malformed regular expression: '{}': {}", uriOrRegex, e.what() - ) - ); - } + applyRegularExpression( + L, + uriOrRegex, + allProperties(), + interpolationDuration, + "", + easingMethod + ); } else if (optimization == "single") { properties::Property* prop = property(uriOrRegex); @@ -445,28 +456,57 @@ int property_getProperty(lua_State* L) { std::string groupName; if (doesUriContainGroupTag(regex, groupName)) { - std::string pathRemainderToMatch = extractUriWithoutGroupName(regex); - // Remove group name from start of regex and replace with '.*' - regex = replaceUriWithGroupName(regex, ".*"); + // Remove group name from start of regex and replace with '*' + regex = replaceUriWithGroupName(regex, "*"); } - // Replace all wildcards * with the correct regex (.*) - size_t startPos = regex.find("*"); - while (startPos != std::string::npos) { - regex.replace(startPos, 1, "(.*)"); - startPos += 4; // (.*) - startPos = regex.find("*", startPos); + // Extract the property and node name to be searched for from regex + std::string propertyName = ""; + std::string nodeName = ""; + size_t wildPos = regex.find_first_of("*"); + if (wildPos != std::string::npos) { + propertyName = regex.substr(wildPos + 1, regex.length()); + nodeName = regex.substr(0, wildPos); + + if (propertyName.empty()) { + LERRORC( + "property_getProperty", + fmt::format( + "Malformed regular expression: '{}': " + "Could not find any propertry name after '*'", regex + ) + ); + return; + } + if (groupName.empty() && nodeName.empty()) { + LERRORC( + "property_getProperty", + fmt::format( + "Malformed regular expression: '{}': " + "Could not find any node name before '*'", regex + ) + ); + return; + } + } + else { + LERRORC( + "property_getProperty", + fmt::format( + "Malformed regular expression: '{}': Could not find '*'", regex + ) + ); + return; } // Get all matching property uris and save to res - std::regex r(regex); std::vector props = allProperties(); std::vector res; for (properties::Property* prop : props) { // Check the regular expression for all properties const std::string& id = prop->fullyQualifiedIdentifier(); - if (std::regex_match(id, r)) { + if (id.find(propertyName) != std::string::npos) { // Filter on the groupname if there was one if (!groupName.empty()) { properties::PropertyOwner* matchingTaggedOwner = @@ -480,6 +520,10 @@ int property_getProperty(lua_State* L) { res.push_back(id); } else { + // Match node name + if (id.find(nodeName) == std::string::npos) { + continue; + } res.push_back(id); } } @@ -602,21 +646,55 @@ int removeSceneGraphNodesFromRegex(lua_State* L) { const std::vector& nodes = global::renderEngine->scene()->allSceneGraphNodes(); - // Replace all wildcards * with the correct regex (.*) - size_t startPos = name.find("*"); - while (startPos != std::string::npos) { - name.replace(startPos, 1, "(.*)"); - startPos += 4; // (.*) - startPos = name.find("*", startPos); + // Extract the property and node name to be searched for from name + std::string propertyName = ""; + std::string nodeName = ""; + size_t wildPos = name.find_first_of("*"); + if (wildPos != std::string::npos) { + propertyName = name.substr(wildPos + 1, name.length()); + nodeName = name.substr(0, wildPos); + + if (propertyName.empty()) { + LERRORC( + "removeSceneGraphNodesFromRegex", + fmt::format( + "Malformed regular expression: '{}': " + "Could not find any propertry name after '*'", name + ) + ); + return; + } + if (nodeName.empty()) { + LERRORC( + "removeSceneGraphNodesFromRegex", + fmt::format( + "Malformed regular expression: '{}': " + "Could not find any node name before '*'", name + ) + ); + return; + } + } + else { + LERRORC( + "removeSceneGraphNodesFromRegex", + fmt::format( + "Malformed regular expression: '{}': Could not find '*'", name + ) + ); + return; } bool foundMatch = false; std::vector markedList; - std::regex r(name); for (SceneGraphNode* node : nodes) { const std::string& identifier = node->identifier(); - if (std::regex_match(identifier, r)) { + if (identifier.find(propertyName) != std::string::npos) { + // Match node name + if (identifier.find(nodeName) == std::string::npos) { + continue; + } foundMatch = true; SceneGraphNode* parent = node->parent(); if (!parent) { From 5772fc621cff7ffbf7d439f52aa156de87abb41e Mon Sep 17 00:00:00 2001 From: Malin Ejdbo Date: Fri, 12 Feb 2021 11:23:45 +0100 Subject: [PATCH 06/22] Improve find that replace regex * Enable * in the begining of string * Enable search without * --- src/scene/scene_lua.inl | 84 +++++++++-------------------------------- 1 file changed, 18 insertions(+), 66 deletions(-) diff --git a/src/scene/scene_lua.inl b/src/scene/scene_lua.inl index 2134a6716b..740a46de70 100644 --- a/src/scene/scene_lua.inl +++ b/src/scene/scene_lua.inl @@ -88,25 +88,10 @@ void applyRegularExpression(lua_State* L, const std::string& regex, ); return; } - if (!isGroupMode && nodeName.empty()) { - LERRORC( - "applyRegularExpression", - fmt::format( - "Malformed regular expression: '{}': " - "Could not find any node name before '*'", regex - ) - ); - return; - } } + // Else search for the literal string without regex else { - LERRORC( - "applyRegularExpression", - fmt::format( - "Malformed regular expression: '{}': Could not find '*'", regex - ) - ); - return; + propertyName = regex; } // Stores whether we found at least one matching property. If this is false at the end @@ -131,11 +116,9 @@ void applyRegularExpression(lua_State* L, const std::string& regex, continue; } } - else { - // Match node name - if (id.find(nodeName) == std::string::npos) { - continue; - } + // Match node name + else if (!nodeName.empty() && id.find(nodeName) == std::string::npos) { + continue; } if (type != prop->typeLua()) { @@ -476,27 +459,12 @@ int property_getProperty(lua_State* L) { "Could not find any propertry name after '*'", regex ) ); - return; - } - if (groupName.empty() && nodeName.empty()) { - LERRORC( - "property_getProperty", - fmt::format( - "Malformed regular expression: '{}': " - "Could not find any node name before '*'", regex - ) - ); - return; + return 0; } } + // Else search for the literal string without regex else { - LERRORC( - "property_getProperty", - fmt::format( - "Malformed regular expression: '{}': Could not find '*'", regex - ) - ); - return; + propertyName = regex; } // Get all matching property uris and save to res @@ -517,15 +485,13 @@ int property_getProperty(lua_State* L) { if (!matchingTaggedOwner) { continue; } - res.push_back(id); } - else { - // Match node name - if (id.find(nodeName) == std::string::npos) { - continue; - } - res.push_back(id); + // Match node name + else if (!nodeName.empty() && id.find(nodeName) == std::string::npos){ + continue; } + + res.push_back(id); } } @@ -662,27 +628,12 @@ int removeSceneGraphNodesFromRegex(lua_State* L) { "Could not find any propertry name after '*'", name ) ); - return; - } - if (nodeName.empty()) { - LERRORC( - "removeSceneGraphNodesFromRegex", - fmt::format( - "Malformed regular expression: '{}': " - "Could not find any node name before '*'", name - ) - ); - return; + return 0; } } + // Else search for the literal string without regex else { - LERRORC( - "removeSceneGraphNodesFromRegex", - fmt::format( - "Malformed regular expression: '{}': Could not find '*'", name - ) - ); - return; + propertyName = name; } bool foundMatch = false; @@ -692,9 +643,10 @@ int removeSceneGraphNodesFromRegex(lua_State* L) { if (identifier.find(propertyName) != std::string::npos) { // Match node name - if (identifier.find(nodeName) == std::string::npos) { + if (!nodeName.empty() && identifier.find(nodeName) == std::string::npos) { continue; } + foundMatch = true; SceneGraphNode* parent = node->parent(); if (!parent) { From afd484044d1810841f5f2d4e461b8d548fbac108 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Fri, 12 Feb 2021 12:59:00 +0100 Subject: [PATCH 07/22] Better support for colors in dictionaries (closes #1489) - Add a new Color3Verifier and Color4Verifier that checks components to be in [0,1] - Update codegen to add a new attribute [[codegen::color()]] to mark a vec3/vec4 to be a color value - Update RenderableTrail to use the new verion --- include/openspace/documentation/verifier.h | 128 +++++++++---------- modules/base/rendering/renderabletrail.cpp | 136 ++++++++------------- src/documentation/verifier.cpp | 67 ++++++++++ support/coding/codegen | 2 +- 4 files changed, 179 insertions(+), 154 deletions(-) diff --git a/include/openspace/documentation/verifier.h b/include/openspace/documentation/verifier.h index 4817be5492..f4af4228a9 100644 --- a/include/openspace/documentation/verifier.h +++ b/include/openspace/documentation/verifier.h @@ -243,39 +243,48 @@ struct IntListVerifier : public TableVerifier { */ struct VectorVerifier {}; -/** - * This Verifier checks whether the value is of type glm::tvec2 - */ +/// This Verifier checks whether the value is of type glm::tvec2 template struct Vector2Verifier : public TemplateVerifier>, public VectorVerifier { std::string type() const override; }; -/** - * This Verifier checks whether the value is of type glm::tvec3 - */ +/// This Verifier checks whether the value is of type glm::tvec3 template struct Vector3Verifier : public TemplateVerifier>, public VectorVerifier { std::string type() const override; }; -/** - * This Verifier checks whether the value is of type glm::tvec4 - */ +/// This Verifier checks whether the value is of type glm::tvec4 template struct Vector4Verifier : public TemplateVerifier>, public VectorVerifier { std::string type() const override; }; +struct Color3Verifier : public Vector3Verifier { + TestResult operator()(const ghoul::Dictionary& dictionary, + const std::string& key) const override; + + std::string type() const override; +}; + +struct Color4Verifier : public Vector4Verifier { + TestResult operator()(const ghoul::Dictionary& dictionary, + const std::string& key) const override; + + std::string type() const override; +}; + /** * A Verifier that checks whether all values contained in a Table are of * type glm::tvec2 */ template struct Vector2ListVerifier : public TableVerifier { - Vector2ListVerifier(std::string elementDocumentation = "") : TableVerifier({ + Vector2ListVerifier(std::string elementDocumentation = "") + : TableVerifier({ { "*", new Vector2Verifier, Optional::No, std::move(elementDocumentation) } - }) + }) {} std::string type() const override { @@ -289,9 +298,10 @@ struct Vector2ListVerifier : public TableVerifier { */ template struct Vector3ListVerifier : public TableVerifier { - Vector3ListVerifier(std::string elementDocumentation = "") : TableVerifier({ - { "*", new Vector3Verifier, Optional::No, std::move(elementDocumentation) } - }) + Vector3ListVerifier(std::string elementDocumentation = "") + : TableVerifier({ + { "*", new Vector3Verifier, Optional::No, std::move(elementDocumentation) } + }) {} std::string type() const override { @@ -305,15 +315,17 @@ struct Vector3ListVerifier : public TableVerifier { */ template struct Vector4ListVerifier : public TableVerifier { - Vector4ListVerifier(std::string elementDocumentation = "") : TableVerifier({ - { "*", new Vector4Verifier, Optional::No, std::move(elementDocumentation) } - }) + Vector4ListVerifier(std::string elementDocumentation = "") + : TableVerifier({ + { "*", new Vector4Verifier, Optional::No, std::move(elementDocumentation) } + }) {} std::string type() const override { return "List of ints"; } }; + //---------------------------------------------------------------------------------------- // Matrix verifiers //---------------------------------------------------------------------------------------- @@ -330,8 +342,7 @@ struct MatrixVerifier {}; * This Verifier checks whether the value is of type glm::mat2x2 */ template -struct Matrix2x2Verifier : - public TemplateVerifier>, public MatrixVerifier +struct Matrix2x2Verifier : public TemplateVerifier>, public MatrixVerifier { std::string type() const override; }; @@ -340,8 +351,8 @@ struct Matrix2x2Verifier : * This Verifier checks whether the value is of type glm::mat2x3 */ template -struct Matrix2x3Verifier : - public TemplateVerifier>, public MatrixVerifier { +struct Matrix2x3Verifier : public TemplateVerifier>, public MatrixVerifier +{ std::string type() const override; }; @@ -349,8 +360,8 @@ struct Matrix2x3Verifier : * This Verifier checks whether the value is of type glm::mat2x4 */ template -struct Matrix2x4Verifier : - public TemplateVerifier>, public MatrixVerifier { +struct Matrix2x4Verifier : public TemplateVerifier>, public MatrixVerifier +{ std::string type() const override; }; @@ -358,8 +369,8 @@ struct Matrix2x4Verifier : * This Verifier checks whether the value is of type glm::mat3x2 */ template -struct Matrix3x2Verifier : - public TemplateVerifier>, public MatrixVerifier { +struct Matrix3x2Verifier : public TemplateVerifier>, public MatrixVerifier +{ std::string type() const override; }; @@ -367,8 +378,8 @@ struct Matrix3x2Verifier : * This Verifier checks whether the value is of type glm::mat3x3 */ template -struct Matrix3x3Verifier : - public TemplateVerifier>, public MatrixVerifier { +struct Matrix3x3Verifier : public TemplateVerifier>, public MatrixVerifier +{ std::string type() const override; }; @@ -376,8 +387,8 @@ struct Matrix3x3Verifier : * This Verifier checks whether the value is of type glm::mat3x4 */ template -struct Matrix3x4Verifier : - public TemplateVerifier>, public MatrixVerifier { +struct Matrix3x4Verifier : public TemplateVerifier>, public MatrixVerifier +{ std::string type() const override; }; @@ -385,8 +396,8 @@ struct Matrix3x4Verifier : * This Verifier checks whether the value is of type glm::mat4x2 */ template -struct Matrix4x2Verifier : - public TemplateVerifier>, public MatrixVerifier { +struct Matrix4x2Verifier : public TemplateVerifier>, public MatrixVerifier +{ std::string type() const override; }; @@ -394,8 +405,8 @@ struct Matrix4x2Verifier : * This Verifier checks whether the value is of type glm::mat4x3 */ template -struct Matrix4x3Verifier : - public TemplateVerifier>, public MatrixVerifier { +struct Matrix4x3Verifier : public TemplateVerifier>, public MatrixVerifier +{ std::string type() const override; }; @@ -403,8 +414,8 @@ struct Matrix4x3Verifier : * This Verifier checks whether the value is of type glm::mat4x4 */ template -struct Matrix4x4Verifier : - public TemplateVerifier>, public MatrixVerifier { +struct Matrix4x4Verifier : public TemplateVerifier>, public MatrixVerifier +{ std::string type() const override; }; @@ -465,18 +476,11 @@ struct OperatorVerifier : public T { */ template struct LessVerifier : public OperatorVerifier> { + static_assert(!std::is_base_of::value, "T cannot be BoolVerifier"); static_assert( - !std::is_base_of::value, - "T cannot be BoolVerifier" - ); - static_assert( - !std::is_base_of::value, - "T cannot be StringVerifier" - ); - static_assert( - !std::is_base_of::value, - "T cannot be TableVerifier" + !std::is_base_of::value, "T cannot be StringVerifier" ); + static_assert(!std::is_base_of::value, "T cannot be TableVerifier"); static_assert( !std::is_base_of::value, "T cannot be VectorVerifier" @@ -496,18 +500,12 @@ struct LessVerifier : public OperatorVerifier> { */ template struct LessEqualVerifier : public OperatorVerifier> { - static_assert( - !std::is_base_of::value, - "T cannot be BoolVerifier" - ); + static_assert(!std::is_base_of::value, "T cannot be BoolVerifier"); static_assert( !std::is_base_of::value, "T cannot be StringVerifier" ); - static_assert( - !std::is_base_of::value, - "T cannot be TableVerifier" - ); + static_assert(!std::is_base_of::value, "T cannot be TableVerifier"); static_assert( !std::is_base_of::value, "T cannot be VectorVerifier" @@ -527,22 +525,16 @@ struct LessEqualVerifier : public OperatorVerifier struct GreaterVerifier : public OperatorVerifier> { - static_assert( - !std::is_base_of::value, - "T cannot be BoolVerifier" - ); + static_assert(!std::is_base_of::value, "T cannot be BoolVerifier"); static_assert( !std::is_base_of::value, "T cannot be StringVerifier" - ); - static_assert( - !std::is_base_of::value, - "T cannot be TableVerifier" - ); + ); + static_assert(!std::is_base_of::value, "T cannot be TableVerifier"); static_assert( !std::is_base_of::value, "T cannot be VectorVerifier" - ); + ); using OperatorVerifier>::OperatorVerifier; @@ -560,18 +552,12 @@ template struct GreaterEqualVerifier : public OperatorVerifier> { - static_assert( - !std::is_base_of::value, - "T cannot be BoolVerifier" - ); + static_assert(!std::is_base_of::value, "T cannot be BoolVerifier"); static_assert( !std::is_base_of::value, "T cannot be StringVerifier" ); - static_assert( - !std::is_base_of::value, - "T cannot be TableVerifier" - ); + static_assert(!std::is_base_of::value, "T cannot be TableVerifier"); static_assert( !std::is_base_of::value, "T cannot be VectorVerifier" diff --git a/modules/base/rendering/renderabletrail.cpp b/modules/base/rendering/renderabletrail.cpp index 12fd349498..42147ba54e 100644 --- a/modules/base/rendering/renderabletrail.cpp +++ b/modules/base/rendering/renderabletrail.cpp @@ -35,6 +35,7 @@ #include #include #include +#include namespace { constexpr const char* ProgramName = "EphemerisProgram"; @@ -144,64 +145,44 @@ namespace { "atmospheres if needed." }; + struct [[codegen::Dictionary(RenderableTrail)]] Parameters { + // This object is used to compute locations along the path. Any Translation object + // can be used here + std::monostate translation [[codegen::reference("core_transform_translation")]]; + + // [[codegen::verbatim(LineColorInfo.description)]] + glm::vec3 color [[codegen::color()]]; + + // [[codegen::verbatim(EnableFadeInfo.description)]] + std::optional enableFade; + + // [[codegen::verbatim(FadeInfo.description)]] + std::optional fade; + + // [[codegen::verbatim(LineWidthInfo.description)]] + std::optional lineWidth; + + // [[codegen::verbatim(PointSizeInfo.description)]] + std::optional pointSize; + + enum class RenderingMode { + Lines, + Points, + LinesPoints [[codegen::key("Lines+Points")]], + PointsLines [[codegen::key("Lines+Points")]] + }; + // [[codegen::verbatim(RenderingModeInfo.description)]] + std::optional renderingMode [[codegen::key("Rendering")]]; + }; +#include "renderabletrail_codegen.cpp" } // namespace namespace openspace { documentation::Documentation RenderableTrail::Documentation() { - using namespace documentation; - return { - "RenderableTrail", - "base_renderable_renderabletrail", - { - { - KeyTranslation, - new ReferencingVerifier("core_transform_translation"), - Optional::No, - "This object is used to compute locations along the path. Any " - "Translation object can be used here." - }, - { - LineColorInfo.identifier, - new DoubleVector3Verifier, - Optional::No, - LineColorInfo.description - }, - { - EnableFadeInfo.identifier, - new BoolVerifier, - Optional::Yes, - EnableFadeInfo.description - }, - { - FadeInfo.identifier, - new DoubleVerifier, - Optional::Yes, - FadeInfo.description - }, - { - LineWidthInfo.identifier, - new DoubleVerifier, - Optional::Yes, - LineWidthInfo.description - }, - { - PointSizeInfo.identifier, - new DoubleVerifier, - Optional::Yes, - PointSizeInfo.description - }, - { - RenderingModeInfo.identifier, - new StringInListVerifier( - // Taken from the RenderingModeConversion map above - { "Lines", "Points", "Lines+Points", "Points+Lines" } - ), - Optional::Yes, - RenderingModeInfo.description - } - } - }; + documentation::Documentation doc = codegen::doc(); + doc.id = "base_renderable_renderabletrail"; + return doc; } RenderableTrail::Appearance::Appearance() @@ -233,6 +214,8 @@ RenderableTrail::Appearance::Appearance() RenderableTrail::RenderableTrail(const ghoul::Dictionary& dictionary) : Renderable(dictionary) { + const Parameters p = codegen::bake(dictionary); + setRenderBin(RenderBin::Overlay); addProperty(_opacity); @@ -241,36 +224,25 @@ RenderableTrail::RenderableTrail(const ghoul::Dictionary& dictionary) ); addPropertySubOwner(_translation.get()); - _appearance.lineColor = dictionary.value(LineColorInfo.identifier); + _appearance.lineColor = p.color; + _appearance.useLineFade = p.enableFade.value_or(_appearance.useLineFade); + _appearance.lineFade = p.fade.value_or(_appearance.lineFade); + _appearance.lineWidth = p.lineWidth.value_or(_appearance.lineWidth); + _appearance.pointSize = p.pointSize.value_or(_appearance.pointSize); - if (dictionary.hasValue(EnableFadeInfo.identifier)) { - _appearance.useLineFade = dictionary.value(EnableFadeInfo.identifier); - } - - if (dictionary.hasValue(FadeInfo.identifier)) { - _appearance.lineFade = static_cast( - dictionary.value(FadeInfo.identifier) - ); - } - - if (dictionary.hasValue(LineWidthInfo.identifier)) { - _appearance.lineWidth = static_cast(dictionary.value( - LineWidthInfo.identifier - )); - } - - if (dictionary.hasValue(PointSizeInfo.identifier)) { - _appearance.pointSize = static_cast( - dictionary.value(PointSizeInfo.identifier) - ); - } - - // This map is not accessed out of order as long as the Documentation is adapted - // whenever the map changes. The documentation will check for valid values - if (dictionary.hasValue(RenderingModeInfo.identifier)) { - _appearance.renderingModes = RenderingModeConversion.at( - dictionary.value(RenderingModeInfo.identifier) - ); + if (p.renderingMode.has_value()) { + switch (*p.renderingMode) { + case Parameters::RenderingMode::Lines: + _appearance.renderingModes = RenderingModeLines; + break; + case Parameters::RenderingMode::Points: + _appearance.renderingModes = RenderingModePoints; + break; + case Parameters::RenderingMode::LinesPoints: + case Parameters::RenderingMode::PointsLines: + _appearance.renderingModes = RenderingModeLinesPoints; + break; + } } else { _appearance.renderingModes = RenderingModeLines; diff --git a/src/documentation/verifier.cpp b/src/documentation/verifier.cpp index 3cea6786bc..780b5ea14f 100644 --- a/src/documentation/verifier.cpp +++ b/src/documentation/verifier.cpp @@ -177,6 +177,73 @@ std::string StringVerifier::type() const { return "String"; } +TestResult Color3Verifier::operator()(const ghoul::Dictionary& dictionary, + const std::string& key) const +{ + TestResult res = Vector3Verifier::operator()(dictionary, key); + if (!res.success) { + return res; + } + + glm::dvec3 values = dictionary.value(key); + if (values.x < 0.0 || values.x > 1.0) { + res.success = false; + res.offenses.push_back({ key + ".x", TestResult::Offense::Reason::Verification }); + } + + if (values.y < 0.0 || values.y > 1.0) { + res.success = false; + res.offenses.push_back({ key + ".y", TestResult::Offense::Reason::Verification }); + } + + if (values.z < 0.0 || values.z > 1.0) { + res.success = false; + res.offenses.push_back({ key + ".z", TestResult::Offense::Reason::Verification }); + } + + return res; +} + +std::string Color3Verifier::type() const { + return std::string("Color3"); +} + +TestResult Color4Verifier::operator()(const ghoul::Dictionary& dictionary, + const std::string& key) const +{ + TestResult res = Vector4Verifier::operator()(dictionary, key); + if (!res.success) { + return res; + } + + std::vector values = dictionary.value>(key); + if (values[0] < 0.0 || values[0] > 1.0) { + res.success = false; + res.offenses.push_back({ key + ".x", TestResult::Offense::Reason::Verification }); + } + + if (values[1] < 0.0 || values[1] > 1.0) { + res.success = false; + res.offenses.push_back({ key + ".y", TestResult::Offense::Reason::Verification }); + } + + if (values[2] < 0.0 || values[2] > 1.0) { + res.success = false; + res.offenses.push_back({ key + ".z", TestResult::Offense::Reason::Verification }); + } + + if (values[3] < 0.0 || values[3] > 1.0) { + res.success = false; + res.offenses.push_back({ key + ".a", TestResult::Offense::Reason::Verification }); + } + + return res; +} + +std::string Color4Verifier::type() const { + return std::string("Color4"); +} + template <> TestResult TemplateVerifier::operator()(const ghoul::Dictionary& dict, const std::string& key) const diff --git a/support/coding/codegen b/support/coding/codegen index 1e942f53f1..14ed29d516 160000 --- a/support/coding/codegen +++ b/support/coding/codegen @@ -1 +1 @@ -Subproject commit 1e942f53f1e9543bc5040c2a8d52eeea1559b788 +Subproject commit 14ed29d516c47151e5601a119ed68e7151c12ef5 From e21edaa13e5e0ef0e1d8ac66df3a9cec1f733afa Mon Sep 17 00:00:00 2001 From: Emma Broman Date: Fri, 12 Feb 2021 14:32:37 +0100 Subject: [PATCH 08/22] Expose exoplanet creation settings to user as module properties (#1499) * Expose exoplanet creation settings to user as module properties * Set default textures from asset to avoid explicit paths to sync folder --- .../milkyway/exoplanets/exoplanets_data.asset | 8 + .../exoplanets/exoplanets_textures.asset | 34 +++- modules/exoplanets/exoplanetsmodule.cpp | 170 +++++++++++++++++- modules/exoplanets/exoplanetsmodule.h | 23 ++- modules/exoplanets/exoplanetsmodule_lua.inl | 120 +++++++------ openspace.cfg | 5 + 6 files changed, 302 insertions(+), 58 deletions(-) diff --git a/data/assets/scene/milkyway/exoplanets/exoplanets_data.asset b/data/assets/scene/milkyway/exoplanets/exoplanets_data.asset index 0d2ab171e8..934d276c5c 100644 --- a/data/assets/scene/milkyway/exoplanets/exoplanets_data.asset +++ b/data/assets/scene/milkyway/exoplanets/exoplanets_data.asset @@ -4,6 +4,14 @@ local DataPath = asset.syncedResource({ Identifier = "exoplanets_data", Version = 2 }) + +asset.onInitialize(function () + local p = "Modules.Exoplanets.DataFolder"; + if(openspace.getPropertyValue(p) == "") then + openspace.setPropertyValueSingle(p, DataPath) + end +end) + asset.export("DataPath", DataPath) asset.meta = { diff --git a/data/assets/scene/milkyway/exoplanets/exoplanets_textures.asset b/data/assets/scene/milkyway/exoplanets/exoplanets_textures.asset index 9efd63b65c..c2898923b3 100644 --- a/data/assets/scene/milkyway/exoplanets/exoplanets_textures.asset +++ b/data/assets/scene/milkyway/exoplanets/exoplanets_textures.asset @@ -1,4 +1,5 @@ -asset.require('./../habitable_zones/habitable_zone_textures') +local habitableZoneTextures = + asset.require('./../habitable_zones/habitable_zone_textures').TexturesPath local TexturesPath = asset.syncedResource({ Name = "Exoplanet Textures", @@ -6,6 +7,37 @@ local TexturesPath = asset.syncedResource({ Identifier = "exoplanets_textures", Version = 2 }) + +asset.onInitialize(function () + local starTexture = TexturesPath .. "/sun.jpg" + local noDataTexture = TexturesPath .. "/grid-32.png" + local discTexture = TexturesPath .. "/disc_bw_texture.png" + + local hzTexture = habitableZoneTextures .. "/hot_to_cold_faded.png" + + -- Set the default textures used for the exoplanet system creation + -- (Check if already set, to not override value in config file) + local p = "Modules.Exoplanets.StarTexture"; + if(openspace.getPropertyValue(p) == "") then + openspace.setPropertyValueSingle(p, starTexture) + end + + p = "Modules.Exoplanets.NoDataTexture"; + if(openspace.getPropertyValue(p) == "") then + openspace.setPropertyValueSingle(p, noDataTexture) + end + + p = "Modules.Exoplanets.OrbitDiscTexture"; + if(openspace.getPropertyValue(p) == "") then + openspace.setPropertyValueSingle(p, discTexture) + end + + p = "Modules.Exoplanets.HabitableZoneTexture"; + if(openspace.getPropertyValue(p) == "") then + openspace.setPropertyValueSingle(p, hzTexture) + end +end) + asset.export("TexturesPath", TexturesPath) asset.meta = { diff --git a/modules/exoplanets/exoplanetsmodule.cpp b/modules/exoplanets/exoplanetsmodule.cpp index b962f758e1..18904a32cf 100644 --- a/modules/exoplanets/exoplanetsmodule.cpp +++ b/modules/exoplanets/exoplanetsmodule.cpp @@ -38,11 +38,166 @@ #include "exoplanetsmodule_lua.inl" +namespace { + constexpr const openspace::properties::Property::PropertyInfo DataFolderInfo = { + "DataFolder", + "Data Folder", + "The path to the folder containing the exoplanets data and lookup table" + }; + + constexpr const openspace::properties::Property::PropertyInfo StarTextureInfo = { + "StarTexture", + "Star Texture", + "The path to a grayscale image that is used for the host star surfaces" + }; + + constexpr const openspace::properties::Property::PropertyInfo NoDataTextureInfo = { + "NoDataTexture", + "No Data Star Texture", + "A path to a texture that is used to represent that there is missing data about " + "the star. For example no color information" + }; + + constexpr const openspace::properties::Property::PropertyInfo OrbitDiscTextureInfo = + { + "OrbitDiscTexture", + "Orbit Disc Texture", + "A path to a 1-dimensional image used as a transfer function for the " + "exoplanets' orbit uncertainty disc" + }; + + constexpr const openspace::properties::Property::PropertyInfo + HabitableZoneTextureInfo = + { + "HabitableZoneTexture", + "Habitable Zone Texture", + "A path to a 1-dimensional image used as a transfer function for the " + "habitable zone disc" + }; + + constexpr const openspace::properties::Property::PropertyInfo + ShowComparisonCircleInfo = + { + "ShowComparisonCircle", + "Show Comparison Circle", + "If true, the 1 AU size comparison circle is enabled per default when an " + "exoplanet system is created" + }; + + constexpr const openspace::properties::Property::PropertyInfo + ShowHabitableZoneInfo = + { + "ShowHabitableZone", + "Show Habitable Zone", + "If true, the habitable zone disc is enabled per default when an " + "exoplanet system is created" + }; + + constexpr const openspace::properties::Property::PropertyInfo UseOptimisticZoneInfo = + { + "UseOptimisticZone", + "Use Optimistic Zone Boundaries", + "If true, the habitable zone is computed with optimistic boundaries per default " + "when an exoplanet system is created" + }; + + constexpr const char ExoplanetsDataFileName[] = "exoplanets_data.bin"; + constexpr const char LookupTableFileName[] = "lookup.txt"; + + struct [[codegen::Dictionary(ExoplanetsModule)]] Parameters { + // [[codegen::verbatim(DataFolderInfo.description)]] + std::optional dataFolder; + + // [[codegen::verbatim(StarTextureInfo.description)]] + std::optional starTexture; + + // [[codegen::verbatim(NoDataTextureInfo.description)]] + std::optional noDataTexture; + + // [[codegen::verbatim(OrbitDiscTextureInfo.description)]] + std::optional orbitDiscTexture; + + // [[codegen::verbatim(HabitableZoneTextureInfo.description)]] + std::optional habitableZoneTexture; + + // [[codegen::verbatim(ShowComparisonCircleInfo.description)]] + std::optional showComparisonCircle; + + // [[codegen::verbatim(ShowHabitableZoneInfo.description)]] + std::optional showHabitableZone; + + // [[codegen::verbatim(UseOptimisticZoneInfo.description)]] + std::optional useOptimisticZone; + }; +#include "exoplanetsmodule_codegen.cpp" +} // namespace + namespace openspace { using namespace exoplanets; -ExoplanetsModule::ExoplanetsModule() : OpenSpaceModule(Name) {} +ExoplanetsModule::ExoplanetsModule() + : OpenSpaceModule(Name) + , _exoplanetsDataFolder(DataFolderInfo) + , _starTexturePath(StarTextureInfo) + , _noDataTexturePath(NoDataTextureInfo) + , _orbitDiscTexturePath(OrbitDiscTextureInfo) + , _habitableZoneTexturePath(HabitableZoneTextureInfo) + , _showComparisonCircle(ShowComparisonCircleInfo, false) + , _showHabitableZone(ShowHabitableZoneInfo, true) + , _useOptimisticZone(UseOptimisticZoneInfo, true) +{ + _exoplanetsDataFolder.setReadOnly(true); + + addProperty(_exoplanetsDataFolder); + addProperty(_starTexturePath); + addProperty(_noDataTexturePath); + addProperty(_orbitDiscTexturePath); + addProperty(_habitableZoneTexturePath); + addProperty(_showComparisonCircle); + addProperty(_showHabitableZone); + addProperty(_useOptimisticZone); +} + +std::string ExoplanetsModule::exoplanetsDataPath() const { + return absPath( + fmt::format("{}/{}", _exoplanetsDataFolder, ExoplanetsDataFileName) + ); +}; + +std::string ExoplanetsModule::lookUpTablePath() const { + return absPath( + fmt::format("{}/{}", _exoplanetsDataFolder, LookupTableFileName) + ); +}; + +std::string ExoplanetsModule::starTexturePath() const { + return _starTexturePath; +} + +std::string ExoplanetsModule::noDataTexturePath() const { + return _noDataTexturePath; +} + +std::string ExoplanetsModule::orbitDiscTexturePath() const { + return _orbitDiscTexturePath; +} + +std::string ExoplanetsModule::habitableZoneTexturePath() const { + return _habitableZoneTexturePath; +} + +bool ExoplanetsModule::showComparisonCircle() const { + return _showComparisonCircle; +} + +bool ExoplanetsModule::showHabitableZone() const { + return _showHabitableZone; +} + +bool ExoplanetsModule::useOptimisticZone() const { + return _useOptimisticZone; +} scripting::LuaLibrary ExoplanetsModule::luaLibrary() const { scripting::LuaLibrary res; @@ -83,7 +238,18 @@ scripting::LuaLibrary ExoplanetsModule::luaLibrary() const { return res; } -void ExoplanetsModule::internalInitialize(const ghoul::Dictionary&) { +void ExoplanetsModule::internalInitialize(const ghoul::Dictionary& dict) { + const Parameters p = codegen::bake(dict); + _exoplanetsDataFolder = p.dataFolder.value_or(_exoplanetsDataFolder); + _starTexturePath = p.starTexture.value_or(_starTexturePath); + _noDataTexturePath = p.noDataTexture.value_or(_noDataTexturePath); + _orbitDiscTexturePath = p.orbitDiscTexture.value_or(_orbitDiscTexturePath); + _habitableZoneTexturePath = p.habitableZoneTexture.value_or(_habitableZoneTexturePath); + + _showComparisonCircle = p.showComparisonCircle.value_or(_showComparisonCircle); + _showHabitableZone = p.showHabitableZone.value_or(_showHabitableZone); + _useOptimisticZone = p.useOptimisticZone.value_or(_useOptimisticZone); + auto fTask = FactoryManager::ref().factory(); auto fRenderable = FactoryManager::ref().factory(); ghoul_assert(fTask, "No task factory existed"); diff --git a/modules/exoplanets/exoplanetsmodule.h b/modules/exoplanets/exoplanetsmodule.h index 76058f99b5..13eb62d492 100644 --- a/modules/exoplanets/exoplanetsmodule.h +++ b/modules/exoplanets/exoplanetsmodule.h @@ -28,6 +28,8 @@ #include #include +#include +#include namespace openspace { @@ -38,11 +40,30 @@ public: ExoplanetsModule(); virtual ~ExoplanetsModule() = default; + std::string exoplanetsDataPath() const; + std::string lookUpTablePath() const; + std::string starTexturePath() const; + std::string noDataTexturePath() const; + std::string orbitDiscTexturePath() const; + std::string habitableZoneTexturePath() const; + bool showComparisonCircle() const; + bool showHabitableZone() const; + bool useOptimisticZone() const; + scripting::LuaLibrary luaLibrary() const override; std::vector documentations() const override; protected: - void internalInitialize(const ghoul::Dictionary&) override; + void internalInitialize(const ghoul::Dictionary& dict) override; + + properties::StringProperty _exoplanetsDataFolder; + properties::StringProperty _starTexturePath; + properties::StringProperty _noDataTexturePath; + properties::StringProperty _orbitDiscTexturePath; + properties::StringProperty _habitableZoneTexturePath; + properties::BoolProperty _showComparisonCircle; + properties::BoolProperty _showHabitableZone; + properties::BoolProperty _useOptimisticZone; }; } // namespace openspace diff --git a/modules/exoplanets/exoplanetsmodule_lua.inl b/modules/exoplanets/exoplanetsmodule_lua.inl index a92408f154..bedbb0ca23 100644 --- a/modules/exoplanets/exoplanetsmodule_lua.inl +++ b/modules/exoplanets/exoplanetsmodule_lua.inl @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -40,47 +41,42 @@ namespace { constexpr const char _loggerCat[] = "ExoplanetsModule"; + + constexpr const char ExoplanetsGuiPath[] = "/Milky Way/Exoplanets/Exoplanet Systems/"; + + // Lua cannot handle backslashes, so replace these with forward slashes + std::string formatPathToLua(const std::string& path) { + std::string resPath = path; + std::replace(resPath.begin(), resPath.end(), '\\', '/'); + return resPath; + } } // namespace namespace openspace::exoplanets::luascriptfunctions { -constexpr const char ExoplanetsGuiPath[] = "/Milky Way/Exoplanets/Exoplanet Systems/"; - -constexpr const char LookUpTablePath[] = "${SYNC}/http/exoplanets_data/2/lookup.txt"; -constexpr const char ExoplanetsDataPath[] = - "${SYNC}/http/exoplanets_data/2/exoplanets_data.bin"; - -constexpr const char StarTextureFile[] = "${SYNC}/http/exoplanets_textures/2/sun.jpg"; -constexpr const char NoDataTextureFile[] = - "${SYNC}/http/exoplanets_textures/2/grid-32.png"; -constexpr const char DiscTextureFile[] = - "${SYNC}/http/exoplanets_textures/2/disc_bw_texture.png"; -constexpr const char HabitableZoneTextureFile[] = - "${SYNC}/http/habitable_zone_textures/1/hot_to_cold_faded.png"; - constexpr const float AU = static_cast(distanceconstants::AstronomicalUnit); constexpr const float SolarRadius = static_cast(distanceconstants::SolarRadius); constexpr const float JupiterRadius = static_cast(distanceconstants::JupiterRadius); ExoplanetSystem findExoplanetSystemInData(std::string_view starName) { - ExoplanetSystem system; + const ExoplanetsModule* module = global::moduleEngine->module(); - std::ifstream data(absPath(ExoplanetsDataPath), std::ios::in | std::ios::binary); + const std::string binPath = module->exoplanetsDataPath(); + std::ifstream data(absPath(binPath), std::ios::in | std::ios::binary); if (!data.good()) { - LERROR(fmt::format( - "Failed to open exoplanets data file: '{}'", absPath(ExoplanetsDataPath) - )); + LERROR(fmt::format("Failed to open exoplanets data file: '{}'", binPath)); return ExoplanetSystem(); } - std::ifstream lut(absPath(LookUpTablePath)); + const std::string lutPath = module->lookUpTablePath(); + std::ifstream lut(absPath(lutPath)); if (!lut.good()) { - LERROR(fmt::format( - "Failed to open exoplanets look-up table: '{}'", absPath(LookUpTablePath) - )); + LERROR(fmt::format("Failed to open exoplanets look-up table: '{}'", lutPath)); return ExoplanetSystem(); } + ExoplanetSystem system; + // 1. search lut for the starname and return the corresponding location // 2. go to that location in the data file // 3. read sizeof(exoplanet) bytes into an exoplanet object. @@ -167,6 +163,8 @@ void createExoplanetSystem(const std::string& starName) { return; } + const ExoplanetsModule* module = global::moduleEngine->module(); + const glm::dvec3 starPos = static_cast(starPosInParsec) * distanceconstants::Parsec; const glm::dmat3 exoplanetSystemRotation = computeSystemRotation(starPos); @@ -182,6 +180,7 @@ void createExoplanetSystem(const std::string& starName) { if (!std::isnan(bv)) { const glm::vec3 color = starColor(bv); + const std::string starTexture = module->starTexturePath(); colorLayers = "{" "Identifier = 'StarColor'," @@ -192,18 +191,17 @@ void createExoplanetSystem(const std::string& starName) { "}," "{" "Identifier = 'StarTexture'," - "FilePath = " + - fmt::format("openspace.absPath('{}')", StarTextureFile) + "," + "FilePath = openspace.absPath('" + formatPathToLua(starTexture) + "')," "BlendMode = 'Color'," "Enabled = true" "}"; } else { + const std::string noDataTexture = module->noDataTexturePath(); colorLayers = "{" "Identifier = 'NoDataStarTexture'," - "FilePath = " + - fmt::format("openspace.absPath('{}')", NoDataTextureFile) + "," + "FilePath = openspace.absPath('" + formatPathToLua(noDataTexture) + "')," "BlendMode = 'Color'," "Enabled = true" "}"; @@ -313,7 +311,6 @@ void createExoplanetSystem(const std::string& starName) { const std::string planetNode = "{" "Identifier = '" + planetIdentifier + "'," "Parent = '" + starIdentifier + "'," - "Enabled = true," "Renderable = {" "Type = 'RenderableGlobe'," "Enabled = " + enabled + "," @@ -342,7 +339,6 @@ void createExoplanetSystem(const std::string& starName) { const std::string planetTrailNode = "{" "Identifier = '" + planetIdentifier + "_Trail'," "Parent = '" + starIdentifier + "'," - "Enabled = true," "Renderable = {" "Type = 'RenderableTrailOrbit'," "Period = " + std::to_string(planet.per) + "," @@ -376,13 +372,16 @@ void createExoplanetSystem(const std::string& starName) { const float lowerOffset = static_cast(planet.aLower / planet.a); const float upperOffset = static_cast(planet.aUpper / planet.a); + const std::string discTexture = module->orbitDiscTexturePath(); + const std::string discNode = "{" "Identifier = '" + planetIdentifier + "_Disc'," "Parent = '" + starIdentifier + "'," - "Enabled = true," "Renderable = {" "Type = 'RenderableOrbitDisc'," - "Texture = openspace.absPath('" + DiscTextureFile + "')," + "Texture = openspace.absPath('" + + formatPathToLua(discTexture) + + "')," "Size = " + std::to_string(semiMajorAxisInMeter) + "," "Eccentricity = " + std::to_string(planet.ecc) + "," "Offset = { " + @@ -410,7 +409,6 @@ void createExoplanetSystem(const std::string& starName) { } } - float meanInclination = 0.f; for (const ExoplanetDataEntry& p : system.planetsData) { meanInclination += p.i; @@ -419,14 +417,16 @@ void createExoplanetSystem(const std::string& starName) { const glm::dmat4 rotation = computeOrbitPlaneRotationMatrix(meanInclination); const glm::dmat3 meanOrbitPlaneRotationMatrix = static_cast(rotation); - // 1 AU Size Comparison Ring - const std::string ringIdentifier = starIdentifier + "_1AU_Ring"; - const std::string ring = "{" - "Identifier = '" + starIdentifier + "_1AU_Ring'," + bool isCircleEnabled = module->showComparisonCircle(); + const std::string isCircleEnabledString = isCircleEnabled ? "true" : "false"; + + // 1 AU Size Comparison Circle + const std::string circle = "{" + "Identifier = '" + starIdentifier + "_1AU_Circle'," "Parent = '" + starIdentifier + "'," - "Enabled = false," "Renderable = {" "Type = 'RenderableRadialGrid'," + "Enabled = " + isCircleEnabledString + "," "OuterRadius = " + std::to_string(AU) + "," "CircleSegments = 64," "LineWidth = 2.0," @@ -438,13 +438,13 @@ void createExoplanetSystem(const std::string& starName) { "}" "}," "GUI = {" - "Name = '1 AU Size Comparison Ring'," + "Name = '1 AU Size Comparison Circle'," "Path = '" + guiPath + "'" "}" "}"; openspace::global::scriptEngine->queueScript( - "openspace.addSceneGraphNode(" + ring + ");", + "openspace.addSceneGraphNode(" + circle + ");", scripting::ScriptEngine::RemoteScripting::Yes ); @@ -463,16 +463,24 @@ void createExoplanetSystem(const std::string& starName) { "the greenhouse effect would not be able to maintain surface temperature " "above freezing anywhere on the planet."; + const std::string hzTexture = module->habitableZoneTexturePath(); + + bool isHzEnabled = module->showHabitableZone(); + const std::string isHzEnabledString = isHzEnabled ? "true" : "false"; + + bool useOptimistic = module->useOptimisticZone(); + const std::string useOptimisticString = useOptimistic ? "true" : "false"; + const std::string zoneDiscNode = "{" "Identifier = '" + starIdentifier + "_HZ_Disc'," "Parent = '" + starIdentifier + "'," - "Enabled = true," "Renderable = {" "Type = 'RenderableHabitableZone'," - "Texture = openspace.absPath('" + HabitableZoneTextureFile + "')," + "Enabled = " + isHzEnabledString + "," + "Texture = openspace.absPath('" + formatPathToLua(hzTexture) + "')," "Luminosity = " + std::to_string(system.starData.luminosity) + "," "EffectiveTemperature = " + std::to_string(system.starData.teff) + "," - "Optimistic = true," + "Optimistic = " + useOptimisticString + "," "Opacity = 0.07" "}," "Transform = {" @@ -545,21 +553,25 @@ int removeExoplanetSystem(lua_State* L) { } std::vector hostStarsWithSufficientData() { + const ExoplanetsModule* module = global::moduleEngine->module(); + + const std::string lutPath = module->lookUpTablePath(); + std::ifstream lookupTableFile(absPath(lutPath)); + if (!lookupTableFile.good()) { + LERROR(fmt::format("Failed to open lookup table file '{}'", lutPath)); + return {}; + } + + const std::string binPath = module->exoplanetsDataPath(); + std::ifstream data(absPath(binPath), std::ios::in | std::ios::binary); + if (!data.good()) { + LERROR(fmt::format("Failed to open data file '{}'", binPath)); + return {}; + } + std::vector names; std::string line; - std::ifstream lookupTableFile(absPath(LookUpTablePath)); - if (!lookupTableFile.good()) { - LERROR(fmt::format("Failed to open lookup table file '{}'", LookUpTablePath)); - return {}; - } - - std::ifstream data(absPath(ExoplanetsDataPath), std::ios::in | std::ios::binary); - if (!data.good()) { - LERROR(fmt::format("Failed to open data file '{}'", ExoplanetsDataPath)); - return {}; - } - // Read number of lines int nExoplanets = 0; while (std::getline(lookupTableFile, line)) { diff --git a/openspace.cfg b/openspace.cfg index 2eba4ff450..a37a89ff7d 100644 --- a/openspace.cfg +++ b/openspace.cfg @@ -167,6 +167,11 @@ ModuleConfigurations = { }, Space = { ShowExceptions = false + }, + Exoplanets = { + ShowComparisonCircle = false, + ShowHabitableZone = true, + UseOptimisticZone = true } } From f34805beeb9893dfdb4d38e99d5fd1722ed813cc Mon Sep 17 00:00:00 2001 From: Malin Ejdbo Date: Fri, 12 Feb 2021 16:53:04 +0100 Subject: [PATCH 09/22] Remove false matches while matching whole string --- src/scene/scene_lua.inl | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/scene/scene_lua.inl b/src/scene/scene_lua.inl index 740a46de70..f468c45dd5 100644 --- a/src/scene/scene_lua.inl +++ b/src/scene/scene_lua.inl @@ -120,6 +120,10 @@ void applyRegularExpression(lua_State* L, const std::string& regex, else if (!nodeName.empty() && id.find(nodeName) == std::string::npos) { continue; } + // Match entire string + else if (id != propertyName) { + continue; + } if (type != prop->typeLua()) { LERRORC( @@ -490,6 +494,10 @@ int property_getProperty(lua_State* L) { else if (!nodeName.empty() && id.find(nodeName) == std::string::npos){ continue; } + // Match entire string + else if (id != propertyName) { + continue; + } res.push_back(id); } @@ -646,6 +654,10 @@ int removeSceneGraphNodesFromRegex(lua_State* L) { if (!nodeName.empty() && identifier.find(nodeName) == std::string::npos) { continue; } + // Match entire string + else if (identifier != propertyName) { + continue; + } foundMatch = true; SceneGraphNode* parent = node->parent(); From e7bcb774ee370f90ccf3b95518aaba1154156bf9 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Sat, 13 Feb 2021 10:44:21 +0100 Subject: [PATCH 10/22] Add new verifiers for files and directories. Update codegen to be able to create these --- include/openspace/documentation/verifier.h | 22 ++++++++++++ src/documentation/verifier.cpp | 41 ++++++++++++++++++++++ support/coding/codegen | 2 +- 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/include/openspace/documentation/verifier.h b/include/openspace/documentation/verifier.h index f4af4228a9..0126a804f7 100644 --- a/include/openspace/documentation/verifier.h +++ b/include/openspace/documentation/verifier.h @@ -158,6 +158,28 @@ struct StringVerifier : public TemplateVerifier { std::string type() const override; }; +/** + * A Verifier that checks whether a given key inside a ghoul::Dictionary is a string and + * refers to an existing file on disk. + */ +struct FileVerifier : public StringVerifier { + TestResult operator()(const ghoul::Dictionary& dict, + const std::string& key) const override; + + std::string type() const override; +}; + +/** +* A Verifier that checks whether a given key inside a ghoul::Dictionary is a string and +* refers to an existing directory on disk. +*/ +struct DirectoryVerifier : public StringVerifier { + TestResult operator()(const ghoul::Dictionary& dict, + const std::string& key) const override; + + std::string type() const override; +}; + /** * A Verifier that checks whether a given key inside a ghoul::Dictionary is another * ghoul::Dictionary. The constructor takes a list of DocumentationEntry%s, which are used diff --git a/src/documentation/verifier.cpp b/src/documentation/verifier.cpp index 780b5ea14f..5a766cee8f 100644 --- a/src/documentation/verifier.cpp +++ b/src/documentation/verifier.cpp @@ -27,6 +27,7 @@ #include #include #include +#include namespace openspace::documentation { @@ -177,6 +178,46 @@ std::string StringVerifier::type() const { return "String"; } +TestResult FileVerifier::operator()(const ghoul::Dictionary& dict, + const std::string& key) const +{ + TestResult res = StringVerifier::operator()(dict, key); + if (!res.success) { + return res; + } + + std::string file = dict.value(key); + if (!std::filesystem::exists(file) || !std::filesystem::is_regular_file(file)) { + res.success = false; + res.offenses.push_back({ key, TestResult::Offense::Reason::Verification }); + } + return res; +} + +std::string FileVerifier::type() const { + return "File"; +} + +TestResult DirectoryVerifier::operator()(const ghoul::Dictionary& dict, + const std::string& key) const +{ + TestResult res = StringVerifier::operator()(dict, key); + if (!res.success) { + return res; + } + + std::string dir = dict.value(key); + if (!std::filesystem::exists(dir) || !std::filesystem::is_directory(dir)) { + res.success = false; + res.offenses.push_back({ key, TestResult::Offense::Reason::Verification }); + } + return res; +} + +std::string DirectoryVerifier::type() const { + return "Directory"; +} + TestResult Color3Verifier::operator()(const ghoul::Dictionary& dictionary, const std::string& key) const { diff --git a/support/coding/codegen b/support/coding/codegen index 14ed29d516..1ca72c0202 160000 --- a/support/coding/codegen +++ b/support/coding/codegen @@ -1 +1 @@ -Subproject commit 14ed29d516c47151e5601a119ed68e7151c12ef5 +Subproject commit 1ca72c0202e3bd4b61510f84797db131591c8ca3 From dfa223abef9da90570e2d27a31d27cf8d3a439e3 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Sat, 13 Feb 2021 12:00:26 +0100 Subject: [PATCH 11/22] Make use of std::filesystem::path and cleanup specification error messages --- .../openspace/documentation/documentation.h | 4 ++- modules/space/rendering/renderablestars.cpp | 26 +++++++------- src/documentation/documentation.cpp | 34 ++++++++++++------- src/documentation/verifier.cpp | 12 +++++-- src/scene/asset.cpp | 5 ++- src/scene/scene_lua.inl | 4 +-- 6 files changed, 52 insertions(+), 33 deletions(-) diff --git a/include/openspace/documentation/documentation.h b/include/openspace/documentation/documentation.h index f40ebb1d06..39ed62320e 100644 --- a/include/openspace/documentation/documentation.h +++ b/include/openspace/documentation/documentation.h @@ -66,6 +66,8 @@ struct TestResult { std::string offender; /// The Reason that caused this offense Reason reason; + /// An optional explanation for when a verification fails + std::string explanation; }; /** @@ -79,7 +81,7 @@ struct TestResult { * The reason for the warning */ enum class Reason { - Deprecated ///< The value is marked as deprecated and should not used + Deprecated ///< The value is marked as deprecated and should not used }; /// The offending key that caused the Warning. In the case of a nested table, diff --git a/modules/space/rendering/renderablestars.cpp b/modules/space/rendering/renderablestars.cpp index c1d159cb8e..a1b1879f14 100644 --- a/modules/space/rendering/renderablestars.cpp +++ b/modules/space/rendering/renderablestars.cpp @@ -308,10 +308,10 @@ namespace { struct [[codegen::Dictionary(RenderableStars)]] Parameters { // The path to the SPECK file containing information about the stars being rendered - std::string speckFile [[codegen::key("File")]]; + std::filesystem::path speckFile [[codegen::key("File")]]; // [[codegen::verbatim(ColorTextureInfo.description)]] - std::string colorMap; + std::filesystem::path colorMap; enum class ColorOption { Color, @@ -350,7 +350,7 @@ namespace { std::string renderMethod; // [[codegen::verbatim(PsfTextureInfo.description)]] - std::string texture; + std::filesystem::path texture; // [[codegen::verbatim(SizeCompositionOptionInfo.description)]] std::optional sizeComposition; @@ -434,11 +434,11 @@ RenderableStars::RenderableStars(const ghoul::Dictionary& dictionary) addProperty(_opacity); registerUpdateRenderBinFromOpacity(); - _speckFile = absPath(p.speckFile); + _speckFile = p.speckFile.string(); _speckFile.onChange([&]() { _speckFileIsDirty = true; }); addProperty(_speckFile); - _colorTexturePath = absPath(p.colorMap); + _colorTexturePath = p.colorMap.string(); _colorTextureFile = std::make_unique(_colorTexturePath); /*_shapeTexturePath = absPath(dictionary.value( @@ -525,7 +525,7 @@ RenderableStars::RenderableStars(const ghoul::Dictionary& dictionary) _renderingMethodOption = RenderOptionTexture; } - _pointSpreadFunctionTexturePath = absPath(p.texture); + _pointSpreadFunctionTexturePath = absPath(p.texture.string()); _pointSpreadFunctionFile = std::make_unique(_pointSpreadFunctionTexturePath); _pointSpreadFunctionTexturePath.onChange([&]() { _pointSpreadFunctionTextureIsDirty = true; @@ -1237,13 +1237,13 @@ void RenderableStars::loadShapeTexture() { */ void RenderableStars::loadData() { - std::string _file = _speckFile; - if (!FileSys.fileExists(absPath(_file))) { + std::string file = absPath(_speckFile); + if (!FileSys.fileExists(file)) { return; } std::string cachedFile = FileSys.cacheManager()->cachedFilename( - _file, + file, ghoul::filesystem::CacheManager::Persistent::Yes ); @@ -1255,7 +1255,7 @@ void RenderableStars::loadData() { bool hasCachedFile = FileSys.fileExists(cachedFile); if (hasCachedFile) { LINFO(fmt::format("Cached file '{}' used for Speck file '{}'", - cachedFile, _file + cachedFile, file )); bool success = loadCachedFile(cachedFile); @@ -1263,15 +1263,15 @@ void RenderableStars::loadData() { return; } else { - FileSys.cacheManager()->removeCacheFile(_file); + FileSys.cacheManager()->removeCacheFile(file); // Intentional fall-through to the 'else' computation to generate the cache // file for the next run } } else { - LINFO(fmt::format("Cache for Speck file '{}' not found", _file)); + LINFO(fmt::format("Cache for Speck file '{}' not found", file)); } - LINFO(fmt::format("Loading Speck file '{}'", _file)); + LINFO(fmt::format("Loading Speck file '{}'", file)); readSpeckFile(); diff --git a/src/documentation/documentation.cpp b/src/documentation/documentation.cpp index 4cf633f28e..0d68ba63a2 100644 --- a/src/documentation/documentation.cpp +++ b/src/documentation/documentation.cpp @@ -71,14 +71,18 @@ std::string to_string(const openspace::documentation::TestResult& value) { } else { std::stringstream stream; - stream << "Failure." << '\n'; + stream << "Specification Failure. "; for (const TestResult::Offense& offense : value.offenses) { - stream << " " << ghoul::to_string(offense) << '\n'; + stream << fmt::format(" {}", ghoul::to_string(offense)); + if (!offense.explanation.empty()) { + stream << fmt::format(" ({})", offense.explanation); + } + stream << '\n'; } for (const TestResult::Warning& warning : value.warnings) { - stream << " " << ghoul::to_string(warning) << '\n'; + stream << fmt::format(" {}\n", ghoul::to_string(warning)); } return stream.str(); @@ -129,17 +133,23 @@ namespace openspace::documentation { const std::string DocumentationEntry::Wildcard = "*"; -std::string concatenate(const std::vector& offenses) { - std::string result = "Error in specification ("; - for (const TestResult::Offense& o : offenses) { - result += o.offender + ','; - } - result.back() = ')'; - return result; -} +//std::string concatenate(const std::vector& offenses) { +// std::string result = "Error in specification ("; +// for (const TestResult::Offense& o : offenses) { +// if (o.explanation.empty()) { +// result += fmt::format("{} ({}), ", o.offender, ghoul::to_string(o.reason)); +// } +// else { +// result += fmt::format("{} ({}: {}), ", o.offender, ghoul::to_string(o.reason), o.explanation); +// } +// } +// result.pop_back(); +// result.back() = ')'; +// return result; +//} SpecificationError::SpecificationError(TestResult res, std::string comp) - : ghoul::RuntimeError(concatenate(res.offenses), std::move(comp)) + : ghoul::RuntimeError("Error in specification", std::move(comp)) , result(std::move(res)) { ghoul_assert(!result.success, "Result's success must be false"); diff --git a/src/documentation/verifier.cpp b/src/documentation/verifier.cpp index 5a766cee8f..ed326b1910 100644 --- a/src/documentation/verifier.cpp +++ b/src/documentation/verifier.cpp @@ -189,7 +189,11 @@ TestResult FileVerifier::operator()(const ghoul::Dictionary& dict, std::string file = dict.value(key); if (!std::filesystem::exists(file) || !std::filesystem::is_regular_file(file)) { res.success = false; - res.offenses.push_back({ key, TestResult::Offense::Reason::Verification }); + TestResult::Offense off; + off.offender = key; + off.reason = TestResult::Offense::Reason::Verification; + off.explanation = "File did not exist"; + res.offenses.push_back(off); } return res; } @@ -209,7 +213,11 @@ TestResult DirectoryVerifier::operator()(const ghoul::Dictionary& dict, std::string dir = dict.value(key); if (!std::filesystem::exists(dir) || !std::filesystem::is_directory(dir)) { res.success = false; - res.offenses.push_back({ key, TestResult::Offense::Reason::Verification }); + TestResult::Offense off; + off.offender = key; + off.reason = TestResult::Offense::Reason::Verification; + off.explanation = "Directory did not exist"; + res.offenses.push_back(off); } return res; } diff --git a/src/scene/asset.cpp b/src/scene/asset.cpp index 39cae2b715..b264fc1e2a 100644 --- a/src/scene/asset.cpp +++ b/src/scene/asset.cpp @@ -520,9 +520,8 @@ bool Asset::initialize() { loader()->callOnInitialize(this); } catch (const ghoul::lua::LuaRuntimeException& e) { - LERROR(fmt::format( - "Failed to initialize asset {}; {}: {}", id(), e.component, e.message - )); + LERROR(fmt::format("Failed to initialize asset {}", id())); + LERROR(fmt::format("{}: {}", e.component, e.message)); // TODO: rollback; setState(State::InitializationFailed); return false; diff --git a/src/scene/scene_lua.inl b/src/scene/scene_lua.inl index be155ed426..dbed4c3757 100644 --- a/src/scene/scene_lua.inl +++ b/src/scene/scene_lua.inl @@ -528,10 +528,10 @@ int addSceneGraphNode(lua_State* L) { global::renderEngine->scene()->initializeNode(node); } catch (const documentation::SpecificationError& e) { + LERRORC("Scene", ghoul::to_string(e.result)); return ghoul::lua::luaError( L, - fmt::format("Error loading scene graph node: {}: {}", - e.what(), ghoul::to_string(e.result)) + fmt::format("Error loading scene graph node: {}", e.what()) ); } catch (const ghoul::RuntimeError& e) { From 2de8ade2b6791b451296748d38fbaf218c94fa64 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Sat, 13 Feb 2021 17:47:18 +0100 Subject: [PATCH 12/22] Update Ghoul repository --- ext/ghoul | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/ghoul b/ext/ghoul index 6cc191d429..47f7ea9533 160000 --- a/ext/ghoul +++ b/ext/ghoul @@ -1 +1 @@ -Subproject commit 6cc191d4293a68faf559d090dd33e340b9137d63 +Subproject commit 47f7ea9533e23d89203fc6351bbbe310264ff0b7 From 23f24c50fea5a1084f7507e807b7ed227a8b6ca4 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Sat, 13 Feb 2021 18:33:28 +0100 Subject: [PATCH 13/22] Update Ghoul and remove explicit generation of ConsoleLog --- apps/OpenSpace/main.cpp | 1 - ext/ghoul | 2 +- src/engine/openspaceengine.cpp | 2 -- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/apps/OpenSpace/main.cpp b/apps/OpenSpace/main.cpp index 2efc3e7061..bc6c0af5d1 100644 --- a/apps/OpenSpace/main.cpp +++ b/apps/OpenSpace/main.cpp @@ -1036,7 +1036,6 @@ int main(int argc, char* argv[]) { { using namespace ghoul::logging; LogManager::initialize(LogLevel::Debug, LogManager::ImmediateFlush::Yes); - LogMgr.addLog(std::make_unique()); #ifdef WIN32 if (IsDebuggerPresent()) { LogMgr.addLog(std::make_unique()); diff --git a/ext/ghoul b/ext/ghoul index 47f7ea9533..9b7242f37b 160000 --- a/ext/ghoul +++ b/ext/ghoul @@ -1 +1 @@ -Subproject commit 47f7ea9533e23d89203fc6351bbbe310264ff0b7 +Subproject commit 9b7242f37bde8ce039ccb9f00195febb564fb79d diff --git a/src/engine/openspaceengine.cpp b/src/engine/openspaceengine.cpp index 3d1d6dd868..fc7dca060a 100644 --- a/src/engine/openspaceengine.cpp +++ b/src/engine/openspaceengine.cpp @@ -71,7 +71,6 @@ #include #include #include -#include #include #include #include @@ -235,7 +234,6 @@ void OpenSpaceEngine::initialize() { using ImmediateFlush = ghoul::logging::LogManager::ImmediateFlush; ghoul::logging::LogManager::initialize(level, ImmediateFlush(immediateFlush)); - LogMgr.addLog(std::make_unique()); for (const ghoul::Dictionary& log : global::configuration->logging.logs) { try { From da31d1e375a41d07c39206eb100d6bcff8467348 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Mon, 15 Feb 2021 11:20:27 +0100 Subject: [PATCH 14/22] Properly handle SpecificationErrors when initializing Modules (closes #1502) --- ext/ghoul | 2 +- src/engine/moduleengine.cpp | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/ext/ghoul b/ext/ghoul index 9b7242f37b..157212fb3d 160000 --- a/ext/ghoul +++ b/ext/ghoul @@ -1 +1 @@ -Subproject commit 9b7242f37bde8ce039ccb9f00195febb564fb79d +Subproject commit 157212fb3d16dad0770fb220ec2454b56ef1d7a3 diff --git a/src/engine/moduleengine.cpp b/src/engine/moduleengine.cpp index 1880c0393c..33acaab9b5 100644 --- a/src/engine/moduleengine.cpp +++ b/src/engine/moduleengine.cpp @@ -58,7 +58,20 @@ void ModuleEngine::initialize( if (it != moduleConfigurations.end()) { configuration = it->second; } - m->initialize(configuration); + try { + m->initialize(configuration); + } + catch (const documentation::SpecificationError& e) { + //LFATALC(e.component, e.message); + for (const documentation::TestResult::Offense& o : e.result.offenses) { + LERRORC(e.component, o.offender + ": " + ghoul::to_string(o.reason)); + } + for (const documentation::TestResult::Warning& w : e.result.warnings) { + LWARNINGC(e.component, w.offender + ": " + ghoul::to_string(w.reason)); + } + throw; + } + addPropertySubOwner(m); } From ace8df0692f45eb645991c415135a0094bbc78f4 Mon Sep 17 00:00:00 2001 From: Malin Ejdbo Date: Mon, 15 Feb 2021 14:28:23 +0100 Subject: [PATCH 15/22] Do not miss matches when searching with regex --- src/scene/scene_lua.inl | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/scene/scene_lua.inl b/src/scene/scene_lua.inl index f468c45dd5..c00971da07 100644 --- a/src/scene/scene_lua.inl +++ b/src/scene/scene_lua.inl @@ -67,6 +67,7 @@ void applyRegularExpression(lua_State* L, const std::string& regex, using ghoul::lua::luaTypeToString; const bool isGroupMode = !groupName.empty(); + bool isLiteral = false; const int type = lua_type(L, -1); @@ -92,6 +93,7 @@ void applyRegularExpression(lua_State* L, const std::string& regex, // Else search for the literal string without regex else { propertyName = regex; + isLiteral = true; } // Stores whether we found at least one matching property. If this is false at the end @@ -121,7 +123,7 @@ void applyRegularExpression(lua_State* L, const std::string& regex, continue; } // Match entire string - else if (id != propertyName) { + else if (isLiteral && id != propertyName) { continue; } @@ -448,6 +450,7 @@ int property_getProperty(lua_State* L) { } // Extract the property and node name to be searched for from regex + bool isLiteral = false; std::string propertyName = ""; std::string nodeName = ""; size_t wildPos = regex.find_first_of("*"); @@ -469,6 +472,7 @@ int property_getProperty(lua_State* L) { // Else search for the literal string without regex else { propertyName = regex; + isLiteral = true; } // Get all matching property uris and save to res @@ -495,7 +499,7 @@ int property_getProperty(lua_State* L) { continue; } // Match entire string - else if (id != propertyName) { + else if (isLiteral && id != propertyName) { continue; } @@ -621,6 +625,7 @@ int removeSceneGraphNodesFromRegex(lua_State* L) { global::renderEngine->scene()->allSceneGraphNodes(); // Extract the property and node name to be searched for from name + bool isLiteral = false; std::string propertyName = ""; std::string nodeName = ""; size_t wildPos = name.find_first_of("*"); @@ -642,6 +647,7 @@ int removeSceneGraphNodesFromRegex(lua_State* L) { // Else search for the literal string without regex else { propertyName = name; + isLiteral = true; } bool foundMatch = false; @@ -655,7 +661,7 @@ int removeSceneGraphNodesFromRegex(lua_State* L) { continue; } // Match entire string - else if (identifier != propertyName) { + else if (isLiteral && identifier != propertyName) { continue; } From edff4b537c4afd3bf0cc894039420a25a3fe0e78 Mon Sep 17 00:00:00 2001 From: Malin Ejdbo Date: Mon, 15 Feb 2021 14:30:25 +0100 Subject: [PATCH 16/22] Add warning if regex contains two or more wildcards * Find that replaces std::regex currently does not support two or more wildcards in an expression --- src/scene/scene_lua.inl | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/scene/scene_lua.inl b/src/scene/scene_lua.inl index c00971da07..a12b55b4ac 100644 --- a/src/scene/scene_lua.inl +++ b/src/scene/scene_lua.inl @@ -89,6 +89,17 @@ void applyRegularExpression(lua_State* L, const std::string& regex, ); return; } + + if (regex.find_first_of("*", wildPos + 1) != std::string::npos) { + LERRORC( + "applyRegularExpression", + fmt::format( + "Malformed regular expression: '{}': " + "Currently only one '*' is supported", regex + ) + ); + return; + } } // Else search for the literal string without regex else { @@ -468,6 +479,17 @@ int property_getProperty(lua_State* L) { ); return 0; } + + if (regex.find_first_of("*", wildPos + 1) != std::string::npos) { + LERRORC( + "property_getProperty", + fmt::format( + "Malformed regular expression: '{}': " + "Currently only one '*' is supported", regex + ) + ); + return 0; + } } // Else search for the literal string without regex else { @@ -643,6 +665,17 @@ int removeSceneGraphNodesFromRegex(lua_State* L) { ); return 0; } + + if (name.find_first_of("*", wildPos + 1) != std::string::npos) { + LERRORC( + "removeSceneGraphNodesFromRegex", + fmt::format( + "Malformed regular expression: '{}': " + "Currently only one '*' is supported", name + ) + ); + return 0; + } } // Else search for the literal string without regex else { From aeb726e5f26d4a7e304290bbf1a83099037c64ae Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Mon, 15 Feb 2021 17:46:48 +0100 Subject: [PATCH 17/22] Update Ghoul submodule to make it compile in Linux again --- ext/ghoul | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/ghoul b/ext/ghoul index 157212fb3d..cbc1e5464d 160000 --- a/ext/ghoul +++ b/ext/ghoul @@ -1 +1 @@ -Subproject commit 157212fb3d16dad0770fb220ec2454b56ef1d7a3 +Subproject commit cbc1e5464d8e624f667c4f91d3d790dcc83b6efc From 9143316111a6f200e8166aee3357693dbd4cc68f Mon Sep 17 00:00:00 2001 From: Malin Ejdbo Date: Tue, 16 Feb 2021 09:28:02 +0100 Subject: [PATCH 18/22] Match property name better to avoid false matches --- src/scene/scene_lua.inl | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/scene/scene_lua.inl b/src/scene/scene_lua.inl index a12b55b4ac..08dc465c9e 100644 --- a/src/scene/scene_lua.inl +++ b/src/scene/scene_lua.inl @@ -114,10 +114,16 @@ void applyRegularExpression(lua_State* L, const std::string& regex, // Check the regular expression for all properties const std::string& id = prop->fullyQualifiedIdentifier(); - if (id.find(propertyName) != std::string::npos) { + size_t propertyPos = id.find(propertyName); + if (propertyPos != std::string::npos) { // If the fully qualified id matches the regular expression, we queue the // value change if the types agree + // Check that the propertyName fully matches the property in id + if ((propertyPos + propertyName.length() + 1) < id.length()) { + continue; + } + // Check tag if (isGroupMode) { properties::PropertyOwner* matchingTaggedOwner = @@ -133,7 +139,7 @@ void applyRegularExpression(lua_State* L, const std::string& regex, else if (!nodeName.empty() && id.find(nodeName) == std::string::npos) { continue; } - // Match entire string + // Match entire string if literal else if (isLiteral && id != propertyName) { continue; } @@ -504,7 +510,13 @@ int property_getProperty(lua_State* L) { // Check the regular expression for all properties const std::string& id = prop->fullyQualifiedIdentifier(); - if (id.find(propertyName) != std::string::npos) { + size_t propertyPos = id.find(propertyName); + if (propertyPos != std::string::npos) { + // Check that the propertyName fully matches the property in id + if ((propertyPos + propertyName.length() + 1) < id.length()) { + continue; + } + // Filter on the groupname if there was one if (!groupName.empty()) { properties::PropertyOwner* matchingTaggedOwner = @@ -520,7 +532,7 @@ int property_getProperty(lua_State* L) { else if (!nodeName.empty() && id.find(nodeName) == std::string::npos){ continue; } - // Match entire string + // Match entire string if literal else if (isLiteral && id != propertyName) { continue; } @@ -688,12 +700,18 @@ int removeSceneGraphNodesFromRegex(lua_State* L) { for (SceneGraphNode* node : nodes) { const std::string& identifier = node->identifier(); - if (identifier.find(propertyName) != std::string::npos) { + size_t propertyPos = identifier.find(propertyName); + if (propertyPos != std::string::npos) { + // Check that the propertyName fully matches the property in id + if ((propertyPos + propertyName.length() + 1) < identifier.length()) { + continue; + } + // Match node name if (!nodeName.empty() && identifier.find(nodeName) == std::string::npos) { continue; } - // Match entire string + // Match entire string if literal else if (isLiteral && identifier != propertyName) { continue; } From 9372382e78914e0cde39e830a31d13118d3add8e Mon Sep 17 00:00:00 2001 From: Malin Ejdbo Date: Wed, 17 Feb 2021 14:16:10 +0100 Subject: [PATCH 19/22] Add possibility to have * at the end and tags + * --- src/scene/scene_lua.inl | 300 +++++++++++++++++++++++++--------------- 1 file changed, 191 insertions(+), 109 deletions(-) diff --git a/src/scene/scene_lua.inl b/src/scene/scene_lua.inl index 08dc465c9e..1ba4b799b8 100644 --- a/src/scene/scene_lua.inl +++ b/src/scene/scene_lua.inl @@ -76,20 +76,22 @@ void applyRegularExpression(lua_State* L, const std::string& regex, std::string nodeName = ""; size_t wildPos = regex.find_first_of("*"); if (wildPos != std::string::npos) { - propertyName = regex.substr(wildPos + 1, regex.length()); nodeName = regex.substr(0, wildPos); + propertyName = regex.substr(wildPos + 1, regex.length()); - if (propertyName.empty()) { + // If none then malformed regular expression + if (propertyName.empty() && nodeName.empty()) { LERRORC( "applyRegularExpression", fmt::format( "Malformed regular expression: '{}': " - "Could not find any propertry name after '*'", regex + "Empty both before and after '*'", regex ) ); return; } + // Currently do not support several wildcards if (regex.find_first_of("*", wildPos + 1) != std::string::npos) { LERRORC( "applyRegularExpression", @@ -101,10 +103,12 @@ void applyRegularExpression(lua_State* L, const std::string& regex, return; } } - // Else search for the literal string without regex + // Literal or tag else { propertyName = regex; - isLiteral = true; + if (!isGroupMode) { + isLiteral = true; + } } // Stores whether we found at least one matching property. If this is false at the end @@ -114,64 +118,93 @@ void applyRegularExpression(lua_State* L, const std::string& regex, // Check the regular expression for all properties const std::string& id = prop->fullyQualifiedIdentifier(); - size_t propertyPos = id.find(propertyName); - if (propertyPos != std::string::npos) { - // If the fully qualified id matches the regular expression, we queue the - // value change if the types agree + if (isLiteral && id != propertyName) { + continue; + } + else if (!propertyName.empty()){ + size_t propertyPos = id.find(propertyName); + if (propertyPos != std::string::npos) { + // Check that the propertyName fully matches the property in id + if ((propertyPos + propertyName.length() + 1) < id.length()) { + continue; + } - // Check that the propertyName fully matches the property in id - if ((propertyPos + propertyName.length() + 1) < id.length()) { + // Match node name + if (!nodeName.empty() && id.find(nodeName) == std::string::npos) { + continue; + } + + // Check tag + if (isGroupMode) { + properties::PropertyOwner* matchingTaggedOwner = + findPropertyOwnerWithMatchingGroupTag( + prop, + groupName + ); + if (!matchingTaggedOwner) { + continue; + } + } + } + else { continue; } + } + else if (!nodeName.empty()) { + size_t nodePos = id.find(nodeName); + if (nodePos != std::string::npos) { - // Check tag - if (isGroupMode) { - properties::PropertyOwner* matchingTaggedOwner = - findPropertyOwnerWithMatchingGroupTag( - prop, - groupName - ); - if (!matchingTaggedOwner) { + // Check tag + if (isGroupMode) { + properties::PropertyOwner* matchingTaggedOwner = + findPropertyOwnerWithMatchingGroupTag( + prop, + groupName + ); + if (!matchingTaggedOwner) { + continue; + } + } + // Check that the nodeName fully matches the node in id + else if (nodePos != 0) { continue; } } - // Match node name - else if (!nodeName.empty() && id.find(nodeName) == std::string::npos) { - continue; - } - // Match entire string if literal - else if (isLiteral && id != propertyName) { + else { continue; } + } - if (type != prop->typeLua()) { - LERRORC( - "property_setValue", - fmt::format( - "{}: Property '{}' does not accept input of type '{}'. " - "Requested type: '{}'", - errorLocation(L), - prop->fullyQualifiedIdentifier(), - luaTypeToString(type), - luaTypeToString(prop->typeLua()) - ) - ); + // Check that the types match + if (type != prop->typeLua()) { + LERRORC( + "property_setValue", + fmt::format( + "{}: Property '{}' does not accept input of type '{}'. " + "Requested type: '{}'", + errorLocation(L), + prop->fullyQualifiedIdentifier(), + luaTypeToString(type), + luaTypeToString(prop->typeLua()) + ) + ); + } + else { + // If the fully qualified id matches the regular expression, we queue the + // value change if the types agree + foundMatching = true; + + if (interpolationDuration == 0.0) { + global::renderEngine->scene()->removePropertyInterpolation(prop); + prop->setLuaValue(L); } else { - foundMatching = true; - - if (interpolationDuration == 0.0) { - global::renderEngine->scene()->removePropertyInterpolation(prop); - prop->setLuaValue(L); - } - else { - prop->setLuaInterpolationTarget(L); - global::renderEngine->scene()->addPropertyInterpolation( - prop, - static_cast(interpolationDuration), - easingFunction - ); - } + prop->setLuaInterpolationTarget(L); + global::renderEngine->scene()->addPropertyInterpolation( + prop, + static_cast(interpolationDuration), + easingFunction + ); } } } @@ -201,10 +234,8 @@ bool doesUriContainGroupTag(const std::string& command, std::string& groupName) } } -std::string replaceUriWithGroupName(const std::string& uri, const std::string& ownerName) -{ - size_t pos = uri.find_first_of("."); - return ownerName + uri.substr(pos); +std::string removeGroupNameFromUri(const std::string& uri) { + return uri.substr(uri.find_first_of(".")); } } // namespace @@ -324,7 +355,7 @@ int property_setValue(lua_State* L) { std::string groupName; if (doesUriContainGroupTag(uriOrRegex, groupName)) { // Remove group name from start of regex and replace with '*' - uriOrRegex = replaceUriWithGroupName(uriOrRegex, "*"); + uriOrRegex = removeGroupNameFromUri(uriOrRegex); } applyRegularExpression( @@ -463,7 +494,7 @@ int property_getProperty(lua_State* L) { std::string groupName; if (doesUriContainGroupTag(regex, groupName)) { // Remove group name from start of regex and replace with '*' - regex = replaceUriWithGroupName(regex, "*"); + regex = removeGroupNameFromUri(regex); } // Extract the property and node name to be searched for from regex @@ -472,20 +503,22 @@ int property_getProperty(lua_State* L) { std::string nodeName = ""; size_t wildPos = regex.find_first_of("*"); if (wildPos != std::string::npos) { - propertyName = regex.substr(wildPos + 1, regex.length()); nodeName = regex.substr(0, wildPos); + propertyName = regex.substr(wildPos + 1, regex.length()); - if (propertyName.empty()) { + // If none then malformed regular expression + if (propertyName.empty() && nodeName.empty()) { LERRORC( "property_getProperty", fmt::format( "Malformed regular expression: '{}': " - "Could not find any propertry name after '*'", regex + "Empty both before and after '*'", regex ) ); return 0; } + // Currently do not support several wildcards if (regex.find_first_of("*", wildPos + 1) != std::string::npos) { LERRORC( "property_getProperty", @@ -497,10 +530,12 @@ int property_getProperty(lua_State* L) { return 0; } } - // Else search for the literal string without regex + // Literal or tag else { propertyName = regex; - isLiteral = true; + if (groupName.empty()) { + isLiteral = true; + } } // Get all matching property uris and save to res @@ -510,35 +545,64 @@ int property_getProperty(lua_State* L) { // Check the regular expression for all properties const std::string& id = prop->fullyQualifiedIdentifier(); - size_t propertyPos = id.find(propertyName); - if (propertyPos != std::string::npos) { - // Check that the propertyName fully matches the property in id - if ((propertyPos + propertyName.length() + 1) < id.length()) { + if (isLiteral && id != propertyName) { + continue; + } + else if (!propertyName.empty()) { + size_t propertyPos = id.find(propertyName); + if (propertyPos != std::string::npos) { + // Check that the propertyName fully matches the property in id + if ((propertyPos + propertyName.length() + 1) < id.length()) { + continue; + } + + // Match node name + if (!nodeName.empty() && id.find(nodeName) == std::string::npos) { + continue; + } + + // Check tag + if (!groupName.empty()) { + properties::PropertyOwner* matchingTaggedOwner = + findPropertyOwnerWithMatchingGroupTag( + prop, + groupName + ); + if (!matchingTaggedOwner) { + continue; + } + } + } + else { continue; } + } + else if (!nodeName.empty()) { + size_t nodePos = id.find(nodeName); + if (nodePos != std::string::npos) { - // Filter on the groupname if there was one - if (!groupName.empty()) { - properties::PropertyOwner* matchingTaggedOwner = - findPropertyOwnerWithMatchingGroupTag( - prop, - groupName - ); - if (!matchingTaggedOwner) { + // Check tag + if (!groupName.empty()) { + properties::PropertyOwner* matchingTaggedOwner = + findPropertyOwnerWithMatchingGroupTag( + prop, + groupName + ); + if (!matchingTaggedOwner) { + continue; + } + } + // Check that the nodeName fully matches the node in id + else if (nodePos != 0) { continue; } } - // Match node name - else if (!nodeName.empty() && id.find(nodeName) == std::string::npos){ + else { continue; } - // Match entire string if literal - else if (isLiteral && id != propertyName) { - continue; - } - - res.push_back(id); } + + res.push_back(id); } lua_newtable(L); @@ -664,20 +728,22 @@ int removeSceneGraphNodesFromRegex(lua_State* L) { std::string nodeName = ""; size_t wildPos = name.find_first_of("*"); if (wildPos != std::string::npos) { - propertyName = name.substr(wildPos + 1, name.length()); nodeName = name.substr(0, wildPos); + propertyName = name.substr(wildPos + 1, name.length()); - if (propertyName.empty()) { + // If none then malformed regular expression + if (propertyName.empty() && nodeName.empty()) { LERRORC( "removeSceneGraphNodesFromRegex", fmt::format( "Malformed regular expression: '{}': " - "Could not find any propertry name after '*'", name + "Empty both before and after '*'", name ) ); return 0; } + // Currently do not support several wildcards if (name.find_first_of("*", wildPos + 1) != std::string::npos) { LERRORC( "removeSceneGraphNodesFromRegex", @@ -689,7 +755,7 @@ int removeSceneGraphNodesFromRegex(lua_State* L) { return 0; } } - // Else search for the literal string without regex + // Literal or tag else { propertyName = name; isLiteral = true; @@ -700,34 +766,50 @@ int removeSceneGraphNodesFromRegex(lua_State* L) { for (SceneGraphNode* node : nodes) { const std::string& identifier = node->identifier(); - size_t propertyPos = identifier.find(propertyName); - if (propertyPos != std::string::npos) { - // Check that the propertyName fully matches the property in id - if ((propertyPos + propertyName.length() + 1) < identifier.length()) { - continue; - } + if (isLiteral && identifier != propertyName) { + continue; + } + else if (!propertyName.empty()) { + size_t propertyPos = identifier.find(propertyName); + if (propertyPos != std::string::npos) { + // Check that the propertyName fully matches the property in id + if ((propertyPos + propertyName.length() + 1) < identifier.length()) { + continue; + } - // Match node name - if (!nodeName.empty() && identifier.find(nodeName) == std::string::npos) { - continue; - } - // Match entire string if literal - else if (isLiteral && identifier != propertyName) { - continue; - } - - foundMatch = true; - SceneGraphNode* parent = node->parent(); - if (!parent) { - LERRORC( - "removeSceneGraphNodesFromRegex", - fmt::format("Cannot remove root node") - ); + // Match node name + if (!nodeName.empty() && identifier.find(nodeName) == std::string::npos) { + continue; + } } else { - markedList.push_back(node); + continue; } } + else if (!nodeName.empty()) { + size_t nodePos = identifier.find(nodeName); + if (nodePos != std::string::npos) { + // Check that the nodeName fully matches the node in id + if (nodePos != 0) { + continue; + } + } + else { + continue; + } + } + + foundMatch = true; + SceneGraphNode* parent = node->parent(); + if (!parent) { + LERRORC( + "removeSceneGraphNodesFromRegex", + fmt::format("Cannot remove root node") + ); + } + else { + markedList.push_back(node); + } } if (!foundMatch) { From 6837bad5b0769650657f8c30ed670b8b61b29f00 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Wed, 17 Feb 2021 16:18:08 +0100 Subject: [PATCH 20/22] Tiny cleanups --- modules/space/translation/keplertranslation.cpp | 2 +- src/engine/moduleengine.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/space/translation/keplertranslation.cpp b/modules/space/translation/keplertranslation.cpp index 2dcedbe797..865a2fe299 100644 --- a/modules/space/translation/keplertranslation.cpp +++ b/modules/space/translation/keplertranslation.cpp @@ -254,7 +254,7 @@ glm::dvec3 KeplerTranslation::position(const UpdateData& data) const { _orbitPlaneDirty = false; } - const double t = data.time.j2000Seconds() -_epoch; + const double t = data.time.j2000Seconds() - _epoch; const double meanMotion = glm::two_pi() / _period; const double meanAnomaly = glm::radians(_meanAnomalyAtEpoch.value()) + t * meanMotion; const double e = eccentricAnomaly(meanAnomaly); diff --git a/src/engine/moduleengine.cpp b/src/engine/moduleengine.cpp index 33acaab9b5..05d98a3a04 100644 --- a/src/engine/moduleengine.cpp +++ b/src/engine/moduleengine.cpp @@ -24,6 +24,7 @@ #include +#include #include #include #include From f4e7d01aa7167f551fb2d8d9226b55df6225605c Mon Sep 17 00:00:00 2001 From: Gene Payne Date: Wed, 17 Feb 2021 12:41:05 -0700 Subject: [PATCH 21/22] Fixes for Issue #1409 (#1501) * Fixed renderablesmallbody selective rendering props to accept asset file settings * Set to only show selective rendering log info msg if render size is 1 * Mirrored selective rendering behavior from smallbody to satellites * Added property coupling of values to satellite/asteroid render settings for size, start index, upperlimit * Fix for selective rendering with synced render settings * Removed redundant property definitions for codegen compliance * Improvements to documentation and handling values spec'd in asset file * Fix for satellites: selective rendering settings specified from assets --- data/assets/util/tle_helper.asset | 4 +- .../rendering/renderableorbitalkepler.cpp | 178 ++++++++++-------- .../space/rendering/renderableorbitalkepler.h | 8 +- .../space/rendering/renderablesatellites.cpp | 76 ++++---- .../space/rendering/renderablesatellites.h | 2 + .../space/rendering/renderablesmallbody.cpp | 162 +++++++++------- modules/space/rendering/renderablesmallbody.h | 9 + 7 files changed, 246 insertions(+), 193 deletions(-) diff --git a/data/assets/util/tle_helper.asset b/data/assets/util/tle_helper.asset index c87a21b231..fdcccdbb43 100644 --- a/data/assets/util/tle_helper.asset +++ b/data/assets/util/tle_helper.asset @@ -85,7 +85,9 @@ function satellites(title, file, color, group) SegmentQuality = 3, Color = color, Fade = 1.5, - RenderBinMode = "PostDeferredTransparent" + RenderBinMode = "PostDeferredTransparent", + StartRenderIdx = group.StartRenderIdx, + RenderSize = group.RenderSize }, Tag = { "earth_satellites" }, GUI = { diff --git a/modules/space/rendering/renderableorbitalkepler.cpp b/modules/space/rendering/renderableorbitalkepler.cpp index 5e898fb39d..6b231c6f5b 100644 --- a/modules/space/rendering/renderableorbitalkepler.cpp +++ b/modules/space/rendering/renderableorbitalkepler.cpp @@ -45,6 +45,7 @@ #include namespace { + constexpr const char* _loggerCat = "OrbitalKepler"; constexpr const char* ProgramName = "OrbitalKepler"; // Fragile! Keep in sync with documentation @@ -114,62 +115,6 @@ namespace { LeapSecond{ 2017, 1 } }; - static const openspace::properties::Property::PropertyInfo PathInfo = { - "Path", - "Path", - "The file path to the data file to read" - }; - static const openspace::properties::Property::PropertyInfo SegmentQualityInfo = { - "SegmentQuality", - "Segment Quality", - "A segment quality value for the orbital trail. A value from 1 (lowest) to " - "10 (highest) that controls the number of line segments in the rendering of the " - "orbital trail. This does not control the direct number of segments because " - "these automatically increase according to the eccentricity of the orbit." - }; - constexpr openspace::properties::Property::PropertyInfo LineWidthInfo = { - "LineWidth", - "Line Width", - "This value specifies the line width of the trail if the selected rendering " - "method includes lines. If the rendering mode is set to Points, this value is " - "ignored." - }; - constexpr openspace::properties::Property::PropertyInfo LineColorInfo = { - "Color", - "Color", - "This value determines the RGB main color for the lines and points of the trail." - }; - constexpr openspace::properties::Property::PropertyInfo TrailFadeInfo = { - "TrailFade", - "Trail Fade", - "This value determines how fast the trail fades and is an appearance property. " - }; - static const openspace::properties::Property::PropertyInfo UpperLimitInfo = { - "UpperLimit", - "Upper Limit", - "Upper limit on the number of objects for this renderable, regardless of " - "how many objects are contained in the data file. Produces an evenly-distributed" - "sample from the data file." - }; - static const openspace::properties::Property::PropertyInfo StartRenderIdxInfo = { - "StartRenderIdx", - "Starting Index of Render", - "Index of object in renderable group to start rendering (all prior objects will " - "be ignored)." - }; - static const openspace::properties::Property::PropertyInfo RenderSizeInfo = { - "RenderSizeInfo", - "Size of Render Block", - "Number of objects to render sequentially from StartRenderIdx" - }; - - constexpr openspace::properties::Property::PropertyInfo RenderBinModeInfo = { - "RenderBinMode", - "RenderBin Mode", - "Determines if the trails will be rendered after all other elements, including" - "atmospheres if needed." - }; - // Count the number of full days since the beginning of 2000 to the beginning of // the parameter 'year' int countDays(int year) { @@ -238,10 +183,90 @@ namespace { return dayCount; } + static const openspace::properties::Property::PropertyInfo PathInfo = { + "Path", + "Path", + "The file path to the data file to read" + }; + static const openspace::properties::Property::PropertyInfo SegmentQualityInfo = { + "SegmentQuality", + "Segment Quality", + "A segment quality value for the orbital trail. A value from 1 (lowest) to " + "10 (highest) that controls the number of line segments in the rendering of the " + "orbital trail. This does not control the direct number of segments because " + "these automatically increase according to the eccentricity of the orbit." + }; + constexpr openspace::properties::Property::PropertyInfo LineWidthInfo = { + "LineWidth", + "Line Width", + "This value specifies the line width of the trail if the selected rendering " + "method includes lines. If the rendering mode is set to Points, this value is " + "ignored." + }; + constexpr openspace::properties::Property::PropertyInfo LineColorInfo = { + "Color", + "Color", + "This value determines the RGB main color for the lines and points of the trail." + }; + constexpr openspace::properties::Property::PropertyInfo TrailFadeInfo = { + "TrailFade", + "Trail Fade", + "This value determines how fast the trail fades and is an appearance property. " + }; + static const openspace::properties::Property::PropertyInfo StartRenderIdxInfo = { + "StartRenderIdx", + "Contiguous Starting Index of Render", + "Index of object in renderable group to start rendering (all prior objects will " + "be ignored)." + }; + static const openspace::properties::Property::PropertyInfo RenderSizeInfo = { + "RenderSize", + "Contiguous Size of Render Block", + "Number of objects to render sequentially from StartRenderIdx" + }; + constexpr openspace::properties::Property::PropertyInfo RenderBinModeInfo = { + "RenderBinMode", + "RenderBin Mode", + "Determines if the trails will be rendered after all other elements, including" + "atmospheres if needed." + }; + + struct [[codegen::Dictionary(RenderableOrbitalKepler)]] Parameters { + // [[codegen::verbatim(PathInfo.description)]] + std::string path; + + // [[codegen::verbatim(SegmentQualityInfo.description)]] + double segmentQuality; + + // [[codegen::verbatim(LineWidthInfo.description)]] + std::optional lineWidth; + + // [[codegen::verbatim(LineColorInfo.description)]] + glm::dvec3 color; + + // [[codegen::verbatim(TrailFadeInfo.description)]] + std::optional trailFade; + + // [[codegen::verbatim(StartRenderIdxInfo.description)]] + std::optional startRenderIdx; + + // [[codegen::verbatim(RenderSizeInfo.description)]] + std::optional renderSize; + + // [[codegen::verbatim(RenderBinModeInfo.description)]] + std::optional renderBinMode; + }; +#include "renderableorbitalkepler_codegen.cpp" } // namespace namespace openspace { +documentation::Documentation RenderableOrbitalKepler::Documentation() { + documentation::Documentation doc = codegen::doc(); + doc.id = "space_renderableorbitalkepler"; + return doc; +} + double RenderableOrbitalKepler::calculateSemiMajorAxis(double meanMotion) const { constexpr const double GravitationalConstant = 6.6740831e-11; constexpr const double MassEarth = 5.9721986e24; @@ -363,7 +388,6 @@ double RenderableOrbitalKepler::epochFromYMDdSubstring(const std::string& epochS RenderableOrbitalKepler::RenderableOrbitalKepler(const ghoul::Dictionary& dict) : Renderable(dict) - , _upperLimit(UpperLimitInfo, 1000, 1, 1000000) , _segmentQuality(SegmentQualityInfo, 2, 1, 10) , _startRenderIdx(StartRenderIdxInfo, 0, 0, 1) , _sizeRender(RenderSizeInfo, 1, 1, 2) @@ -374,7 +398,6 @@ RenderableOrbitalKepler::RenderableOrbitalKepler(const ghoul::Dictionary& dict) dict, "RenderableOrbitalKepler" ); - _path = dict.value(PathInfo.identifier); _segmentQuality = static_cast(dict.value(SegmentQualityInfo.identifier)); @@ -387,20 +410,21 @@ RenderableOrbitalKepler::RenderableOrbitalKepler(const ghoul::Dictionary& dict) static_cast(dict.value(TrailFadeInfo.identifier)) : 20.f; - _upperLimit = - dict.hasValue(UpperLimitInfo.identifier) ? - static_cast(dict.value(UpperLimitInfo.identifier)) : - 0u; + if (dict.hasValue(StartRenderIdxInfo.identifier)) { + _startRenderIdx = static_cast( + dict.value(StartRenderIdxInfo.identifier)); + } + else { + _startRenderIdx = 0u; + } - _startRenderIdx = - dict.hasValue(StartRenderIdxInfo.identifier) ? - static_cast(dict.value(StartRenderIdxInfo.identifier)) : - 0u; - - _sizeRender = - dict.hasValue(RenderSizeInfo.identifier) ? - static_cast(dict.value(RenderSizeInfo.identifier)) : - 0u; + if (dict.hasValue(RenderSizeInfo.identifier)) { + _sizeRender = static_cast( + dict.value(RenderSizeInfo.identifier)); + } + else { + _sizeRender = 0u; + } _appearance.lineWidth = dict.hasValue(LineWidthInfo.identifier) ? @@ -415,13 +439,6 @@ RenderableOrbitalKepler::RenderableOrbitalKepler(const ghoul::Dictionary& dict) addProperty(_path); addProperty(_segmentQuality); addProperty(_opacity); - addProperty(_startRenderIdx); - addProperty(_sizeRender); - - _updateStartRenderIdxSelect = std::function([this] { initializeGL(); }); - _updateRenderSizeSelect = std::function([this] { initializeGL(); }); - _startRenderIdxCallbackHandle = _startRenderIdx.onChange(_updateStartRenderIdxSelect); - _sizeRenderCallbackHandle = _sizeRender.onChange(_updateRenderSizeSelect); if (dict.hasValue(RenderBinModeInfo.identifier)) { Renderable::RenderBin cfgRenderBin = RenderBinConversion.at( @@ -481,6 +498,11 @@ void RenderableOrbitalKepler::render(const RenderData& data, RendererTasks&) { return; } + if (_updateDataBuffersAtNextRender) { + _updateDataBuffersAtNextRender = false; + initializeGL(); + } + _programObject->activate(); _programObject->setUniform(_uniformCache.opacity, _opacity); _programObject->setUniform(_uniformCache.inGameTime, data.time.j2000Seconds()); diff --git a/modules/space/rendering/renderableorbitalkepler.h b/modules/space/rendering/renderableorbitalkepler.h index 9e269d7d7b..bfc4212fc9 100644 --- a/modules/space/rendering/renderableorbitalkepler.h +++ b/modules/space/rendering/renderableorbitalkepler.h @@ -37,6 +37,8 @@ namespace openspace { +namespace documentation { struct Documentation; } + class RenderableOrbitalKepler : public Renderable { public: RenderableOrbitalKepler(const ghoul::Dictionary& dictionary); @@ -62,6 +64,8 @@ public: virtual void readDataFile(const std::string& filename) = 0; protected: + static documentation::Documentation Documentation(); + double calculateSemiMajorAxis(double meanMotion) const; double epochFromSubstring(const std::string& epochString) const; double epochFromYMDdSubstring(const std::string& epochString); @@ -81,15 +85,15 @@ protected: double epoch = 0.0; double period = 0.0; }; + + bool _updateDataBuffersAtNextRender = false; std::streamoff _numObjects; bool _isFileReadinitialized = false; inline static constexpr double convertAuToKm = 1.496e8; inline static constexpr double convertDaysToSecs = 86400.0; std::vector _data; std::vector _segmentSize; - properties::UIntProperty _upperLimit; properties::UIntProperty _segmentQuality; - properties::Property::OnChangeHandle _upperLimitCallbackHandle; properties::UIntProperty _startRenderIdx; properties::UIntProperty _sizeRender; properties::Property::OnChangeHandle _startRenderIdxCallbackHandle; diff --git a/modules/space/rendering/renderablesatellites.cpp b/modules/space/rendering/renderablesatellites.cpp index c50736ad34..2acc9a244b 100644 --- a/modules/space/rendering/renderablesatellites.cpp +++ b/modules/space/rendering/renderablesatellites.cpp @@ -47,49 +47,15 @@ namespace { constexpr const char* _loggerCat = "Satellites"; - static const openspace::properties::Property::PropertyInfo PathInfo = { - "Path", - "Path", - "The file path to the TLE file to read" - }; static const openspace::properties::Property::PropertyInfo SegmentsInfo = { "Segments", "Segments", "The number of segments to use for each orbit ellipse" }; - constexpr openspace::properties::Property::PropertyInfo LineWidthInfo = { - "LineWidth", - "Line Width", - "This value specifies the line width of the trail if the selected rendering " - "method includes lines. If the rendering mode is set to Points, this value is " - "ignored." - }; - constexpr openspace::properties::Property::PropertyInfo LineColorInfo = { - "Color", - "Color", - "This value determines the RGB main color for the lines and points of the trail." - }; - constexpr openspace::properties::Property::PropertyInfo TrailFadeInfo = { - "TrailFade", - "Trail Fade", - "This value determines how fast the trail fades and is an appearance property. " - }; struct [[codegen::Dictionary(RenderableSatellites)]] Parameters { // [[codegen::verbatim(SegmentsInfo.description)]] double segments; - - // [[codegen::verbatim(PathInfo.description)]] - std::string path; - - // [[codegen::verbatim(LineWidthInfo.description)]] - std::optional lineWidth; - - // [[codegen::verbatim(LineColorInfo.description)]] - glm::dvec3 color; - - // [[codegen::verbatim(TrailFadeInfo.description)]] - std::optional trailFade; }; #include "renderablesatellites_codegen.cpp" } @@ -98,7 +64,17 @@ namespace openspace { documentation::Documentation RenderableSatellites::Documentation() { documentation::Documentation doc = codegen::doc(); - doc.id = "space_renderable_satellites"; + doc.id = "space_renderablesatellites"; + + // Insert the parents documentation entries until we have a verifier that can deal + // with class hierarchy + documentation::Documentation parentDoc = RenderableOrbitalKepler::Documentation(); + doc.entries.insert( + doc.entries.end(), + parentDoc.entries.begin(), + parentDoc.entries.end() + ); + return doc; } @@ -109,6 +85,23 @@ RenderableSatellites::RenderableSatellites(const ghoul::Dictionary& dictionary) // probably want a codegen::check function that only does the checking without // actually creating a Parameter objects // codegen::bake(dictionary); + addProperty(_startRenderIdx); + addProperty(_sizeRender); + + _updateStartRenderIdxSelect = std::function([this] { + if ((_numObjects - _startRenderIdx) < _sizeRender) { + _sizeRender = _numObjects - _startRenderIdx; + } + initializeGL(); + }); + _updateRenderSizeSelect = std::function([this] { + if (_sizeRender > (_numObjects - _startRenderIdx)) { + _startRenderIdx = _numObjects - _sizeRender; + } + initializeGL(); + }); + _startRenderIdxCallbackHandle = _startRenderIdx.onChange(_updateStartRenderIdxSelect); + _sizeRenderCallbackHandle = _sizeRender.onChange(_updateRenderSizeSelect); } void RenderableSatellites::readDataFile(const std::string& filename) { @@ -162,7 +155,7 @@ void RenderableSatellites::readDataFile(const std::string& filename) { // 5 12-14 International Designator (Launch number of the year) // 6 15-17 International Designator(piece of the launch) A name += " " + line.substr(2, 15); - if (_startRenderIdx > 0 && _startRenderIdx == i) { + if (_startRenderIdx == i && _sizeRender == 1) { LINFO(fmt::format( "Set render block to start at object {}", name @@ -251,16 +244,11 @@ void RenderableSatellites::readDataFile(const std::string& filename) { } void RenderableSatellites::initializeFileReading() { - _startRenderIdx.removeOnChange(_startRenderIdxCallbackHandle); - _sizeRender.removeOnChange(_sizeRenderCallbackHandle); _startRenderIdx.setMaxValue(static_cast(_numObjects - 1)); _sizeRender.setMaxValue(static_cast(_numObjects)); - _startRenderIdx = static_cast(0); - _sizeRender = static_cast(_numObjects); - _startRenderIdxCallbackHandle = _startRenderIdx.onChange( - _updateStartRenderIdxSelect); - _sizeRenderCallbackHandle = _sizeRender.onChange( - _updateRenderSizeSelect); + if (_sizeRender == 0u) { + _sizeRender = static_cast(_numObjects); + } } void RenderableSatellites::skipSingleEntryInFile(std::ifstream& file) { diff --git a/modules/space/rendering/renderablesatellites.h b/modules/space/rendering/renderablesatellites.h index e90c18cc65..1638d40929 100644 --- a/modules/space/rendering/renderablesatellites.h +++ b/modules/space/rendering/renderablesatellites.h @@ -38,6 +38,8 @@ namespace openspace { +namespace documentation { struct Documentation; } + class RenderableSatellites : public RenderableOrbitalKepler { public: RenderableSatellites(const ghoul::Dictionary& dictionary); diff --git a/modules/space/rendering/renderablesmallbody.cpp b/modules/space/rendering/renderablesmallbody.cpp index 6963d03a9c..0a0259eb98 100644 --- a/modules/space/rendering/renderablesmallbody.cpp +++ b/modules/space/rendering/renderablesmallbody.cpp @@ -48,41 +48,20 @@ namespace { constexpr const char* _loggerCat = "SmallSolarSystemBody"; - static const openspace::properties::Property::PropertyInfo PathInfo = { - "Path", - "Path", - "The file path to the SBDB .csv file to read" - }; - static const openspace::properties::Property::PropertyInfo SegmentQualityInfo = { - "SegmentQuality", - "Segment Quality", - "A segment quality value for the orbital trail. A value from 1 (lowest) to " - "100 (highest) that controls the number of line segments in the rendering of the " - "orbital trail. This does not control the direct number of segments because " - "these automatically increase according to the eccentricity of the orbit." - }; - constexpr openspace::properties::Property::PropertyInfo LineWidthInfo = { - "LineWidth", - "Line Width", - "This value specifies the line width of the trail if the selected rendering " - "method includes lines. If the rendering mode is set to Points, this value is " - "ignored." - }; - constexpr openspace::properties::Property::PropertyInfo LineColorInfo = { - "Color", - "Color", - "This value determines the RGB main color for the lines and points of the trail." - }; - constexpr openspace::properties::Property::PropertyInfo TrailFadeInfo = { - "TrailFade", - "Trail Fade", - "This value determines how fast the trail fades and is an appearance property. " + static const openspace::properties::Property::PropertyInfo ContiguousModeInfo = { + "ContiguousMode", + "Contiguous Mode", + "If enabled, then the contiguous set of objects starting from StartRenderIdx " + "of size RenderSize will be rendered. If disabled, then the number of objects " + "defined by UpperLimit will rendered from an evenly dispersed sample of the " + "full length of the data file." }; static const openspace::properties::Property::PropertyInfo UpperLimitInfo = { "UpperLimit", "Upper Limit", "Upper limit on the number of objects for this renderable, regardless of " - "how many objects are contained in the data file" + "how many objects are contained in the data file. Produces an evenly-distributed" + "sample from the data file." }; double importAngleValue(const std::string& angle) { @@ -106,23 +85,11 @@ namespace { } struct [[codegen::Dictionary(RenderableSmallBody)]] Parameters { - // [[codegen::verbatim(SegmentQualityInfo.description)]] - double segmentQuality; + // [[codegen::verbatim(ContiguousModeInfo.description)]] + std::optional contiguousMode; // [[codegen::verbatim(UpperLimitInfo.description)]] std::optional upperLimit; - - // [[codegen::verbatim(PathInfo.description)]] - std::string path; - - // [[codegen::verbatim(LineWidthInfo.description)]] - std::optional lineWidth; - - // [[codegen::verbatim(LineColorInfo.description)]] - glm::dvec3 color; - - // [[codegen::verbatim(TrailFadeInfo.description)]] - std::optional trailFade; }; #include "renderablesmallbody_codegen.cpp" } // namespace @@ -131,17 +98,77 @@ namespace openspace { documentation::Documentation RenderableSmallBody::Documentation() { documentation::Documentation doc = codegen::doc(); - doc.id = "space_renderable_small_body"; + doc.id = "space_renderablesmallbody"; + + // Insert the parents documentation entries until we have a verifier that can deal + // with class hierarchy + documentation::Documentation parentDoc = RenderableOrbitalKepler::Documentation(); + doc.entries.insert( + doc.entries.end(), + parentDoc.entries.begin(), + parentDoc.entries.end() + ); return doc; } RenderableSmallBody::RenderableSmallBody(const ghoul::Dictionary& dictionary) : RenderableOrbitalKepler(dictionary) + , _upperLimit(UpperLimitInfo, 1000, 1, 1000000) + , _contiguousMode(ContiguousModeInfo, false) { codegen::bake(dictionary); - _upperLimitCallbackHandle = _upperLimit.onChange(_reinitializeTrailBuffers); + addProperty(_startRenderIdx); + addProperty(_sizeRender); + addProperty(_contiguousMode); addProperty(_upperLimit); + + if (dictionary.hasValue(UpperLimitInfo.identifier)) { + _upperLimit = static_cast( + dictionary.value(UpperLimitInfo.identifier)); + } + else { + _upperLimit = 0u; + } + + if (dictionary.hasValue(ContiguousModeInfo.identifier)) { + _contiguousMode = static_cast( + dictionary.value(ContiguousModeInfo.identifier)); + } + else { + _contiguousMode = false; + } + + _updateStartRenderIdxSelect = std::function([this] { + if (_contiguousMode) { + if ((_numObjects - _startRenderIdx) < _sizeRender) { + _sizeRender = _numObjects - _startRenderIdx; + } + _updateDataBuffersAtNextRender = true; + } + }); + _updateRenderSizeSelect = std::function([this] { + if (_contiguousMode) { + if (_sizeRender > (_numObjects - _startRenderIdx)) { + _startRenderIdx = _numObjects - _sizeRender; + } + _updateDataBuffersAtNextRender = true; + } + }); + _updateRenderUpperLimitSelect = std::function([this] { + if (!_contiguousMode) { + _updateDataBuffersAtNextRender = true; + } + }); + _updateContiguousModeSelect = std::function([this] { + _updateDataBuffersAtNextRender = true; + }); + + _startRenderIdxCallbackHandle = _startRenderIdx.onChange(_updateStartRenderIdxSelect); + _sizeRenderCallbackHandle = _sizeRender.onChange(_updateRenderSizeSelect); + _upperLimitCallbackHandle = _upperLimit.onChange(_updateRenderUpperLimitSelect); + _contiguousModeCallbackhandle = + _contiguousMode.onChange(_updateContiguousModeSelect); } void RenderableSmallBody::readDataFile(const std::string& filename) { @@ -183,14 +210,18 @@ void RenderableSmallBody::readDataFile(const std::string& filename) { _isFileReadinitialized = true; initializeFileReading(); } + + unsigned int startElement = 0; + unsigned int endElement; + if (_contiguousMode) { + lineSkipFraction = 1.0; + startElement = _startRenderIdx; + endElement = _startRenderIdx + _sizeRender - 1; + } else { - if (_sizeRender < _numObjects || _startRenderIdx > 0) { - lineSkipFraction = 1.0; - } - else { - lineSkipFraction = static_cast(_upperLimit) - / static_cast(_numObjects); - } + lineSkipFraction = static_cast(_upperLimit) + / static_cast(_numObjects); + endElement = _numObjects - 1; } if (line.compare(expectedHeaderLine) != 0) { @@ -202,18 +233,17 @@ void RenderableSmallBody::readDataFile(const std::string& filename) { } unsigned int sequentialLineErrors = 0; - unsigned int endElement = _startRenderIdx + _sizeRender - 1; endElement = (endElement >= _numObjects) ? static_cast(_numObjects - 1) : endElement; // Burn lines if not starting at first element - for (unsigned int k = 0; k < _startRenderIdx; ++k) { + for (unsigned int k = 0; k < startElement; ++k) { skipSingleLineInFile(file); } bool firstDataLine = true; int lastLineCount = -1; - for (csvLine = _startRenderIdx + 1; + for (csvLine = startElement + 1; csvLine <= endElement + 1; csvLine++, sequentialLineErrors++) { @@ -272,20 +302,16 @@ void RenderableSmallBody::readDataFile(const std::string& filename) { } void RenderableSmallBody::initializeFileReading() { - _startRenderIdx.removeOnChange(_startRenderIdxCallbackHandle); - _sizeRender.removeOnChange(_sizeRenderCallbackHandle); _startRenderIdx.setMaxValue(static_cast(_numObjects - 1)); _sizeRender.setMaxValue(static_cast(_numObjects)); - _startRenderIdx = static_cast(0); - _sizeRender = static_cast(_numObjects); - _startRenderIdxCallbackHandle = _startRenderIdx.onChange(_updateStartRenderIdxSelect); - _sizeRenderCallbackHandle = _sizeRender.onChange(_updateRenderSizeSelect); - // If a limit wasn't specified in dictionary, set it to # lines in file - // minus the header line (but temporarily disable callback to avoid 2nd call) - _upperLimit.removeOnChange(_upperLimitCallbackHandle); + if (_sizeRender == 0u) { + _sizeRender = static_cast(_numObjects); + } + _upperLimit.setMaxValue(static_cast(_numObjects)); - _upperLimit = static_cast(_numObjects); - _upperLimitCallbackHandle = _upperLimit.onChange(_reinitializeTrailBuffers); + if (_upperLimit == 0u) { + _upperLimit = static_cast(_numObjects); + } } void RenderableSmallBody::skipSingleLineInFile(std::ifstream& file) { @@ -312,7 +338,7 @@ void RenderableSmallBody::readOrbitalParamsFromThisLine(bool firstDataLine, // Object designator string std::getline(file, name, ','); - if (_startRenderIdx > 0 && _startRenderIdx == (csvLine - 1)) { + if (_startRenderIdx > 0 && _startRenderIdx == (csvLine - 1) && _sizeRender == 1) { formatObjectName(name); LINFO(fmt::format("Set render block to start at object {}", name)); } diff --git a/modules/space/rendering/renderablesmallbody.h b/modules/space/rendering/renderablesmallbody.h index b1fc1d7dc3..1be028b539 100644 --- a/modules/space/rendering/renderablesmallbody.h +++ b/modules/space/rendering/renderablesmallbody.h @@ -38,6 +38,8 @@ namespace openspace { +namespace documentation { struct Documentation; } + class RenderableSmallBody : public RenderableOrbitalKepler { public: RenderableSmallBody(const ghoul::Dictionary& dictionary); @@ -51,10 +53,17 @@ private: void skipSingleLineInFile(std::ifstream& file); std::vector _sbNames; + std::function _updateContiguousModeSelect; + std::function _updateRenderUpperLimitSelect; /// The index array that is potentially used in the draw call. If this is empty, no /// element draw call is used. std::vector _indexBufferData; + bool contiguousMode = true; + properties::BoolProperty _contiguousMode; + properties::UIntProperty _upperLimit; + properties::Property::OnChangeHandle _contiguousModeCallbackhandle; + properties::Property::OnChangeHandle _upperLimitCallbackHandle; }; } // namespace openspace From cb5929972afd218fd634079e98a8f6ec71721ab4 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Thu, 18 Feb 2021 22:42:17 +0100 Subject: [PATCH 22/22] Don't call RenderableOrbitalKeplers's initializeGL only once --- ext/ghoul | 2 +- .../space/rendering/renderableorbitalkepler.cpp | 2 ++ modules/space/rendering/renderableorbitalkepler.h | 2 +- modules/space/rendering/renderablesatellites.cpp | 14 +++++++------- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/ext/ghoul b/ext/ghoul index cbc1e5464d..d7dd0a7144 160000 --- a/ext/ghoul +++ b/ext/ghoul @@ -1 +1 @@ -Subproject commit cbc1e5464d8e624f667c4f91d3d790dcc83b6efc +Subproject commit d7dd0a714467ee974426c939c4169bec8a065feb diff --git a/modules/space/rendering/renderableorbitalkepler.cpp b/modules/space/rendering/renderableorbitalkepler.cpp index 6b231c6f5b..b60c20abe8 100644 --- a/modules/space/rendering/renderableorbitalkepler.cpp +++ b/modules/space/rendering/renderableorbitalkepler.cpp @@ -452,6 +452,8 @@ RenderableOrbitalKepler::RenderableOrbitalKepler(const ghoul::Dictionary& dict) } void RenderableOrbitalKepler::initializeGL() { + ghoul_assert(_vertexArray == 0, "Vertex array object already existed"); + ghoul_assert(_vertexBuffer == 0, "Vertex buffer object already existed"); glGenVertexArrays(1, &_vertexArray); glGenBuffers(1, &_vertexBuffer); diff --git a/modules/space/rendering/renderableorbitalkepler.h b/modules/space/rendering/renderableorbitalkepler.h index bfc4212fc9..6500455d07 100644 --- a/modules/space/rendering/renderableorbitalkepler.h +++ b/modules/space/rendering/renderableorbitalkepler.h @@ -69,6 +69,7 @@ protected: double calculateSemiMajorAxis(double meanMotion) const; double epochFromSubstring(const std::string& epochString) const; double epochFromYMDdSubstring(const std::string& epochString); + void updateBuffers(); std::function _reinitializeTrailBuffers; std::function _updateStartRenderIdxSelect; @@ -125,7 +126,6 @@ private: GLuint _vertexArray; GLuint _vertexBuffer; - void updateBuffers(); ghoul::opengl::ProgramObject* _programObject; properties::StringProperty _path; diff --git a/modules/space/rendering/renderablesatellites.cpp b/modules/space/rendering/renderablesatellites.cpp index 2acc9a244b..64d0f887d7 100644 --- a/modules/space/rendering/renderablesatellites.cpp +++ b/modules/space/rendering/renderablesatellites.cpp @@ -88,18 +88,18 @@ RenderableSatellites::RenderableSatellites(const ghoul::Dictionary& dictionary) addProperty(_startRenderIdx); addProperty(_sizeRender); - _updateStartRenderIdxSelect = std::function([this] { + _updateStartRenderIdxSelect = [this]() { if ((_numObjects - _startRenderIdx) < _sizeRender) { _sizeRender = _numObjects - _startRenderIdx; } - initializeGL(); - }); - _updateRenderSizeSelect = std::function([this] { + updateBuffers(); + }; + _updateRenderSizeSelect = [this]() { if (_sizeRender > (_numObjects - _startRenderIdx)) { _startRenderIdx = _numObjects - _sizeRender; } - initializeGL(); - }); + updateBuffers(); + }; _startRenderIdxCallbackHandle = _startRenderIdx.onChange(_updateStartRenderIdxSelect); _sizeRenderCallbackHandle = _sizeRender.onChange(_updateRenderSizeSelect); } @@ -107,7 +107,7 @@ RenderableSatellites::RenderableSatellites(const ghoul::Dictionary& dictionary) void RenderableSatellites::readDataFile(const std::string& filename) { if (!FileSys.fileExists(filename)) { throw ghoul::RuntimeError(fmt::format( - "Satellite TLE file {} does not exist.", filename + "Satellite TLE file {} does not exist", filename )); } _data.clear();