diff --git a/include/openspace/engine/openspaceengine.h b/include/openspace/engine/openspaceengine.h index b86d953cd4..51f8b4f742 100644 --- a/include/openspace/engine/openspaceengine.h +++ b/include/openspace/engine/openspaceengine.h @@ -132,6 +132,9 @@ public: AssetManager& assetManager(); LoadingScreen* loadingScreen(); + void invalidatePropertyCache(); + const std::vector& allProperties() const; + void createUserDirectoriesIfNecessary(); uint64_t ramInUse() const; @@ -174,6 +177,9 @@ private: int _nextCallbackHandle = 0; std::vector> _modeChangeCallbacks; + + mutable bool _isAllPropertiesCacheDirty = true; + mutable std::vector _allPropertiesCache; }; /** diff --git a/include/openspace/properties/property.h b/include/openspace/properties/property.h index 0e13005bad..02a786b560 100644 --- a/include/openspace/properties/property.h +++ b/include/openspace/properties/property.h @@ -277,7 +277,7 @@ public: * * \return The fully qualified identifier for this Property */ - std::string uri() const; + std::string_view uri() const; /** * Returns the PropertyOwner of this Property or `nullptr`, if it does not have an @@ -475,6 +475,13 @@ public: */ void resetToUnchanged(); + /** + * This function must be called whenever this property's URI changes. Examples of this + * are if the PropertyOwner to which this Property belongs changes identifier or is + * reparented in any way. + */ + void updateUriCache(); + protected: /** * This method must be called by all subclasses whenever the encapsulated value has @@ -509,6 +516,11 @@ protected: /// The callback functions that will be invoked whenever the value changes std::vector>> _onDeleteCallbacks; + /// A cached version of the full URI of this property, which includes the identifiers + /// of all owners + std::string _uriCache; + bool _isUriCacheDirty = true; + /// Flag indicating that this property value has been changed after initialization bool _isValueDirty = false; diff --git a/include/openspace/properties/propertyowner.h b/include/openspace/properties/propertyowner.h index 5bd852d783..8c7e8761f8 100644 --- a/include/openspace/properties/propertyowner.h +++ b/include/openspace/properties/propertyowner.h @@ -73,7 +73,7 @@ public: * The destructor will remove all Propertys and PropertyOwners it owns along with * itself. */ - virtual ~PropertyOwner(); + virtual ~PropertyOwner() = default; /** * Sets the identifier for this PropertyOwner. If the PropertyOwner does not have an @@ -193,8 +193,8 @@ public: */ bool hasProperty(const Property* prop) const; - void setPropertyOwner(PropertyOwner* owner) { _owner = owner; } - PropertyOwner* owner() const { return _owner; } + void setPropertyOwner(PropertyOwner* owner); + PropertyOwner* owner() const; /** * Returns a list of all sub-owners this PropertyOwner has. Each name of a sub-owner @@ -337,6 +337,11 @@ protected: std::map _groupNames; /// Collection of string tag(s) assigned to this property std::vector _tags; + + +private: + /// Will regenerate the uri caches for all directly or indirectly owned properties + void updateUriCaches(); }; } // namespace openspace::properties diff --git a/modules/imgui/src/renderproperties.cpp b/modules/imgui/src/renderproperties.cpp index 31dcd3497f..edc0317262 100644 --- a/modules/imgui/src/renderproperties.cpp +++ b/modules/imgui/src/renderproperties.cpp @@ -63,15 +63,13 @@ void renderTooltip(Property* prop, double delay) { ImGui::TextWrapped("%s", prop->description().c_str()); ImGui::Spacing(); } - ImGui::Text( - "%s", - (std::string("Identifier: ") + prop->uri()).c_str() - ); + std::string t = std::format("Identifier: {}", prop->uri()); + ImGui::Text("%s", t.c_str()); ImGui::EndTooltip(); } } -void executeSetPropertyScript(const std::string& id, const std::string& value) { +void executeSetPropertyScript(std::string_view id, const std::string& value) { global::scriptEngine->queueScript( std::format("openspace.setPropertyValueSingle('{}', {});", id, value) ); @@ -219,7 +217,7 @@ void renderStringProperty(Property* prop, const std::string& ownerName, ImGui::PopID(); } -void renderListProperty(const std::string& name, const std::string& fullIdentifier, +void renderListProperty(const std::string& name, std::string_view fullIdentifier, const std::string& stringValue) { // Remove brackets from the string value diff --git a/src/engine/globals.cpp b/src/engine/globals.cpp index 8fe81ad6d6..4ad57fce5a 100644 --- a/src/engine/globals.cpp +++ b/src/engine/globals.cpp @@ -133,6 +133,14 @@ void create() { memoryManager = new MemoryManager; #endif // WIN32 +#ifdef WIN32 + openSpaceEngine = new (currentPos) OpenSpaceEngine; + ghoul_assert(openSpaceEngine, "No openSpaceEngine"); + currentPos += sizeof(OpenSpaceEngine); +#else // ^^^ WIN32 / !WIN32 vvv + openSpaceEngine = new OpenSpaceEngine; +#endif // WIN32 + #ifdef WIN32 eventEngine = new (currentPos) EventEngine; ghoul_assert(eventEngine, "No eventEngine"); @@ -197,14 +205,6 @@ void create() { moduleEngine = new ModuleEngine; #endif // WIN32 -#ifdef WIN32 - openSpaceEngine = new (currentPos) OpenSpaceEngine; - ghoul_assert(openSpaceEngine, "No openSpaceEngine"); - currentPos += sizeof(OpenSpaceEngine); -#else // ^^^ WIN32 / !WIN32 vvv - openSpaceEngine = new OpenSpaceEngine; -#endif // WIN32 - #ifdef WIN32 parallelPeer = new (currentPos) ParallelPeer; ghoul_assert(parallelPeer, "No parallelPeer"); @@ -577,13 +577,6 @@ void destroy() { delete parallelPeer; #endif // WIN32 - LDEBUGC("Globals", "Destroying 'OpenSpaceEngine'"); -#ifdef WIN32 - openSpaceEngine->~OpenSpaceEngine(); -#else // ^^^ WIN32 / !WIN32 vvv - delete openSpaceEngine; -#endif // WIN32 - LDEBUGC("Globals", "Destroying 'ModuleEngine'"); #ifdef WIN32 moduleEngine->~ModuleEngine(); @@ -640,6 +633,13 @@ void destroy() { delete eventEngine; #endif // WIN32 + LDEBUGC("Globals", "Destroying 'OpenSpaceEngine'"); +#ifdef WIN32 + openSpaceEngine->~OpenSpaceEngine(); +#else // ^^^ WIN32 / !WIN32 vvv + delete openSpaceEngine; +#endif // WIN32 + LDEBUGC("Globals", "Destroying 'MemoryManager'"); #ifdef WIN32 memoryManager->~MemoryManager(); diff --git a/src/engine/openspaceengine.cpp b/src/engine/openspaceengine.cpp index 298d225aa3..846ce21ed5 100644 --- a/src/engine/openspaceengine.cpp +++ b/src/engine/openspaceengine.cpp @@ -1726,6 +1726,19 @@ LoadingScreen* OpenSpaceEngine::loadingScreen() { return _loadingScreen.get(); } +void OpenSpaceEngine::invalidatePropertyCache() { + _isAllPropertiesCacheDirty = true; +} + +const std::vector& OpenSpaceEngine::allProperties() const { + if (_isAllPropertiesCacheDirty) { + _allPropertiesCache = global::rootPropertyOwner->propertiesRecursive(); + _isAllPropertiesCacheDirty = false; + } + + return _allPropertiesCache; +} + AssetManager& OpenSpaceEngine::assetManager() { ghoul_assert(_assetManager, "Asset Manager must not be nullptr"); return *_assetManager; diff --git a/src/interaction/sessionrecordinghandler.cpp b/src/interaction/sessionrecordinghandler.cpp index 834dcb3108..a5540d18b0 100644 --- a/src/interaction/sessionrecordinghandler.cpp +++ b/src/interaction/sessionrecordinghandler.cpp @@ -572,7 +572,7 @@ void SessionRecordingHandler::savePropertyBaseline(properties::Property& prop) { "NavigationHandler.OrbitalNavigator.RetargetAim" }; - const std::string propIdentifier = prop.uri(); + const std::string propIdentifier = std::string(prop.uri()); for (std::string_view reject : PropertyBaselineRejects) { if (propIdentifier.starts_with(reject)) { return; diff --git a/src/properties/misc/optionproperty.cpp b/src/properties/misc/optionproperty.cpp index 81d1f78a3a..96bbb7c661 100644 --- a/src/properties/misc/optionproperty.cpp +++ b/src/properties/misc/optionproperty.cpp @@ -190,7 +190,7 @@ void OptionProperty::setLuaValue(lua_State* state) { if (it == _options.cend()) { throw ghoul::RuntimeError( std::format("Could not find option '{}'", value), - uri() + std::string(uri()) ); } thisValue = it->value; diff --git a/src/properties/property.cpp b/src/properties/property.cpp index f844d4a2bf..e7f7370898 100644 --- a/src/properties/property.cpp +++ b/src/properties/property.cpp @@ -65,9 +65,9 @@ const std::string& Property::identifier() const { return _identifier; } -std::string Property::uri() const { - const std::string& ownerUri = owner()->uri(); - return !ownerUri.empty() ? std::format("{}.{}", ownerUri, _identifier) : ""; +std::string_view Property::uri() const { + ZoneScoped; + return _uriCache; } const std::type_info& Property::type() const { @@ -199,6 +199,7 @@ const PropertyOwner* Property::owner() const { void Property::setPropertyOwner(PropertyOwner* owner) { _owner = owner; + updateUriCache(); } void Property::notifyChangeListeners() { @@ -215,10 +216,15 @@ void Property::resetToUnchanged() { _isValueDirty = false; } +void Property::updateUriCache() { + const std::string& ownerUri = _owner ? _owner->uri() : ""; + _uriCache = !ownerUri.empty() ? std::format("{}.{}", ownerUri, _identifier) : ""; +} + std::string Property::generateJsonDescription() const { const std::string cName = escapedJson(std::string(className())); - const std::string identifier = uri(); - const std::string identifierSan = escapedJson(identifier); + const std::string_view identifier = uri(); + const std::string identifierSan = escapedJson(std::string(identifier)); const std::string& gName = guiName(); const std::string gNameSan = escapedJson(gName); const std::string metaData = generateMetaDataJsonDescription(); diff --git a/src/properties/propertyowner.cpp b/src/properties/propertyowner.cpp index 3d238f8715..dbd3061f59 100644 --- a/src/properties/propertyowner.cpp +++ b/src/properties/propertyowner.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -38,7 +39,6 @@ namespace { constexpr std::string_view _loggerCat = "PropertyOwner"; - using namespace openspace; // The URIs have to be validated because it is not known in what order things are // constructed. For example, a SceneGraphNode can be created before its Renderable, @@ -46,13 +46,17 @@ namespace { // know what is created first is because if the child is created first, it has not // been added to the property tree so URI will be invalid and not sent. But the parent // will be added later, which will include the child in it's subowners - void publishPropertyTreeUpdatedEvent(const std::string& uri) { + void publishPropertyTreeUpdatedEvent(std::string_view uri) { + using namespace openspace; + if (!uri.empty()) { global::eventEngine->publishEvent(uri); } } - void publishPropertyTreePrunedEvent(const std::string& uri) { + void publishPropertyTreePrunedEvent(std::string_view uri) { + using namespace openspace; + if (!uri.empty()) { global::eventEngine->publishEvent(uri); } @@ -76,11 +80,6 @@ PropertyOwner::PropertyOwner(PropertyOwnerInfo info) ); } -PropertyOwner::~PropertyOwner() { - _properties.clear(); - _subOwners.clear(); -} - const std::vector& PropertyOwner::properties() const { return _properties; } @@ -174,6 +173,14 @@ bool PropertyOwner::hasProperty(const std::string& uri) const { return property(uri) != nullptr; } +void PropertyOwner::setPropertyOwner(PropertyOwner* owner) { + _owner = owner; +} + +PropertyOwner* PropertyOwner::owner() const { + return _owner; +} + bool PropertyOwner::hasProperty(const Property* prop) const { ghoul_precondition(prop != nullptr, "prop must not be nullptr"); @@ -259,6 +266,9 @@ void PropertyOwner::addProperty(Property* prop) { else { _properties.push_back(prop); prop->setPropertyOwner(this); + if (global::openSpaceEngine) { + global::openSpaceEngine->invalidatePropertyCache(); + } // Notify change so we can update the UI publishPropertyTreeUpdatedEvent(prop->uri()); @@ -310,6 +320,10 @@ void PropertyOwner::addPropertySubOwner(openspace::properties::PropertyOwner* ow else { _subOwners.push_back(owner); owner->setPropertyOwner(this); + updateUriCaches(); + if (global::openSpaceEngine) { + global::openSpaceEngine->invalidatePropertyCache(); + } // Notify change so UI gets updated publishPropertyTreeUpdatedEvent(owner->uri()); @@ -337,6 +351,9 @@ void PropertyOwner::removeProperty(Property* prop) { publishPropertyTreePrunedEvent(prop->uri()); (*it)->setPropertyOwner(nullptr); + if (global::openSpaceEngine) { + global::openSpaceEngine->invalidatePropertyCache(); + } _properties.erase(it); } else { @@ -366,6 +383,14 @@ void PropertyOwner::removePropertySubOwner(openspace::properties::PropertyOwner* if (it != _subOwners.end() && (*it)->identifier() == owner->identifier()) { // Notify the change so the UI can update publishPropertyTreePrunedEvent(owner->uri()); + + // When a PropertyOwner is removed as a subowner, it means that it will probably + // be deleted soon and we wouldn't need to invalidate the cache, but we can't be + // 100% sure of that, so we are invalidating the cache anyway + owner->updateUriCaches(); + if (global::openSpaceEngine) { + global::openSpaceEngine->invalidatePropertyCache(); + } _subOwners.erase(it); } else { @@ -422,4 +447,13 @@ void PropertyOwner::removeTag(const std::string& tag) { _tags.erase(std::remove(_tags.begin(), _tags.end(), tag), _tags.end()); } +void PropertyOwner::updateUriCaches() { + for (Property* property : _properties) { + property->updateUriCache(); + } + for (PropertyOwner* owner : _subOwners) { + owner->updateUriCaches(); + } +} + } // namespace openspace::properties diff --git a/src/query/query.cpp b/src/query/query.cpp index 5e2cd8d029..001650a4e6 100644 --- a/src/query/query.cpp +++ b/src/query/query.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include @@ -61,7 +62,7 @@ properties::PropertyOwner* propertyOwner(const std::string& uri) { } std::vector allProperties() { - return global::rootPropertyOwner->propertiesRecursive(); + return global::openSpaceEngine->allProperties(); } } // namespace openspace diff --git a/src/scene/scene_lua.inl b/src/scene/scene_lua.inl index 4d6a64a963..eba58e031e 100644 --- a/src/scene/scene_lua.inl +++ b/src/scene/scene_lua.inl @@ -58,6 +58,7 @@ #include #include #include +#include #include namespace { @@ -140,26 +141,29 @@ std::vector findMatchesInAllProperties( } } - // 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. - for (properties::Property* prop : properties) { - // Check the regular expression for all properties - const std::string id = prop->uri(); + std::mutex mutex; + std::for_each( + std::execution::par_unseq, + properties.cbegin(), + properties.cend(), + [&](properties::Property* prop) { + // Check the regular expression for all properties + const std::string_view uri = prop->uri(); - 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; + if (isLiteral && uri != propertyName) { + return; + } + else if (!propertyName.empty()) { + size_t propertyPos = uri.find(propertyName); + if ( + // Check if the propertyName appears in the URI at all + (propertyPos == std::string::npos) || + // Check that the propertyName fully matches the property in uri + ((propertyPos + propertyName.length() + 1) < uri.length()) || + // Match node name + (!nodeName.empty() && uri.find(nodeName) == std::string::npos)) + { + return; } // Check tag @@ -167,36 +171,35 @@ std::vector findMatchesInAllProperties( const properties::PropertyOwner* matchingTaggedOwner = findPropertyOwnerWithMatchingGroupTag(prop, groupName); if (!matchingTaggedOwner) { - continue; + return; } } } - else { - continue; - } - } - else if (!nodeName.empty()) { - size_t nodePos = id.find(nodeName); - if (nodePos != std::string::npos) { - // Check tag - if (isGroupMode) { - const properties::PropertyOwner* matchingTaggedOwner = - findPropertyOwnerWithMatchingGroupTag(prop, groupName); - if (!matchingTaggedOwner) { - continue; + else if (!nodeName.empty()) { + size_t nodePos = uri.find(nodeName); + if (nodePos != std::string::npos) { + // Check tag + if (isGroupMode) { + const properties::PropertyOwner* matchingTaggedOwner = + findPropertyOwnerWithMatchingGroupTag(prop, groupName); + if (!matchingTaggedOwner) { + return; + } + } + // Check that the nodeName fully matches the node in id + else if (nodePos != 0) { + return; } } - // Check that the nodeName fully matches the node in id - else if (nodePos != 0) { - continue; + else { + return; } } - else { - continue; - } + std::lock_guard g(mutex); + matches.push_back(prop); } - matches.push_back(prop); - } + ); + return matches; } @@ -337,6 +340,8 @@ int setPropertyCallSingle(properties::Property& prop, const std::string& uri, template int propertySetValue(lua_State* L) { + ZoneScoped; + int nParameters = ghoul::lua::checkArgumentsAndThrow( L, { 2, 6 }, @@ -547,21 +552,21 @@ namespace { std::vector res; for (properties::Property* prop : props) { // Check the regular expression for all properties - const std::string& id = prop->uri(); + const std::string_view uri = prop->uri(); - if (isLiteral && id != propertyName) { + if (isLiteral && uri != propertyName) { continue; } else if (!propertyName.empty()) { - size_t propertyPos = id.find(propertyName); + size_t propertyPos = uri.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 ((propertyPos + propertyName.length() + 1) < uri.length()) { continue; } // Match node name - if (!nodeName.empty() && id.find(nodeName) == std::string::npos) { + if (!nodeName.empty() && uri.find(nodeName) == std::string::npos) { continue; } @@ -579,7 +584,7 @@ namespace { } } else if (!nodeName.empty()) { - size_t nodePos = id.find(nodeName); + size_t nodePos = uri.find(nodeName); if (nodePos != std::string::npos) { // Check tag if (!groupName.empty()) { @@ -599,7 +604,7 @@ namespace { } } - res.push_back(id); + res.push_back(std::string(uri)); } return res;