From 96719213317bf560e4baefcde6e10b3ab9cd3919 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Sat, 30 Aug 2014 10:01:11 +0200 Subject: [PATCH] More work towards a unified interface for accessing properties --- include/openspace/properties/propertyowner.h | 17 ++- include/openspace/query/query.h | 1 - include/openspace/scenegraph/scenegraphnode.h | 6 +- src/properties/propertyowner.cpp | 117 +++++++++++++++++- src/query/query.cpp | 31 +++-- src/rendering/renderable.cpp | 6 +- src/scenegraph/scenegraph.cpp | 8 +- src/scenegraph/scenegraphnode.cpp | 35 ++---- src/scripting/scriptfunctions.cpp | 8 +- 9 files changed, 168 insertions(+), 61 deletions(-) diff --git a/include/openspace/properties/propertyowner.h b/include/openspace/properties/propertyowner.h index 6211c877b4..6dfaf7c1af 100644 --- a/include/openspace/properties/propertyowner.h +++ b/include/openspace/properties/propertyowner.h @@ -35,27 +35,42 @@ namespace properties { class PropertyOwner { public: + static const char URISeparator = '.'; + PropertyOwner(); virtual ~PropertyOwner(); void setName(std::string name); virtual const std::string& name() const; + const std::vector& properties() const; Property* property(const std::string& id) const; + bool hasProperty(const std::string& id) const; + + const std::vector& subOwners() const; + PropertyOwner* subOwner(const std::string& name) const; + bool hasSubOwner(const std::string& name) const; void setPropertyGroupName(std::string groupID, std::string name); const std::string& propertyGroupName(const std::string& groupID) const; - + protected: void addProperty(Property* prop); void addProperty(Property& prop); + + void addPropertySubOwner(PropertyOwner* owner); + void addPropertySubOwner(PropertyOwner& owner); void removeProperty(Property* prop); void removeProperty(Property& prop); + + void removePropertySubOwner(PropertyOwner* owner); + void removePropertySubOwner(PropertyOwner& owner); private: std::string _name; std::vector _properties; + std::vector _subOwners; std::map _groupNames; }; diff --git a/include/openspace/query/query.h b/include/openspace/query/query.h index f985a31b7d..1bd1edf34b 100644 --- a/include/openspace/query/query.h +++ b/include/openspace/query/query.h @@ -38,7 +38,6 @@ class SceneGraphNode; SceneGraph* sceneGraph(); SceneGraphNode* sceneGraphNode(const std::string& name); properties::Property* property(const std::string& uri); -properties::Property* property(const std::string& nodeName, const std::string& propertyName); } // namespace diff --git a/include/openspace/scenegraph/scenegraphnode.h b/include/openspace/scenegraph/scenegraphnode.h index 775e844a05..b809f84ce4 100644 --- a/include/openspace/scenegraph/scenegraphnode.h +++ b/include/openspace/scenegraph/scenegraphnode.h @@ -28,6 +28,7 @@ // open space includes #include #include +#include #include #include @@ -40,7 +41,7 @@ namespace openspace { -class SceneGraphNode { +class SceneGraphNode : public properties::PropertyOwner { public: static std::string RootNodeName; @@ -61,11 +62,9 @@ public: // set & get void addNode(SceneGraphNode* child); - void setName(const std::string& name); void setParent(SceneGraphNode* parent); const psc& position() const; psc worldPosition() const; - std::string nodeName() const; SceneGraphNode* parent() const; const std::vector& children() const; @@ -86,7 +85,6 @@ private: // essential std::vector _children; SceneGraphNode* _parent; - std::string _nodeName; Ephemeris* _ephemeris; // renderable diff --git a/src/properties/propertyowner.cpp b/src/properties/propertyowner.cpp index 19dc8bd1cc..e5d332ffbc 100644 --- a/src/properties/propertyowner.cpp +++ b/src/properties/propertyowner.cpp @@ -37,6 +37,11 @@ bool propertyLess(Property* lhs, Property* rhs) { return lhs->identifier() < rhs->identifier(); } + +bool subOwnerLess(PropertyOwner* lhs, PropertyOwner* rhs) { + return lhs->name() < rhs->name(); +} + } PropertyOwner::PropertyOwner() @@ -64,17 +69,68 @@ Property* PropertyOwner::property(const std::string& id) const return prop->identifier() < str; }); - if (it == _properties.end() || (*it)->identifier() != id) + if (it == _properties.end() || (*it)->identifier() != id) { + // if we do not own the searched property, it must consist of a concatenated + // name and we can delegate it to a subowner + const size_t ownerSeparator = id.find(URISeparator); + if (ownerSeparator == std::string::npos) { + // if we do not own the property and there is no separator, it does not exist + LERROR("The identifier '" << id << "' did not exist in PropertyOwner '" << + name() << "'"); + return nullptr; + } + else { + const std::string ownerName = id.substr(0, ownerSeparator); + const std::string propertyName = id.substr(ownerSeparator); + + PropertyOwner* owner = subOwner(ownerName); + if (owner == nullptr) { + LERROR("Sub PropertyOwner '" << owner + << "' did not exist for PropertyOwner '" << name() << "'"); + return nullptr; + } + else { + // Recurse into the subOwner + return owner->property(propertyName); + } + } + } + else + return *it; +} + +bool PropertyOwner::hasProperty(const std::string& id) const { + return property(id) != nullptr; +} + +const std::vector& PropertyOwner::subOwners() const { + return _subOwners; +} + +PropertyOwner* PropertyOwner::subOwner(const std::string& name) const { + assert(std::is_sorted(_subOwners.begin(), _subOwners.end(), subOwnerLess)); + + std::vector::const_iterator it + = std::lower_bound(_subOwners.begin(), _subOwners.end(), name, + [](PropertyOwner* owner, const std::string& str) { + return owner->name() < str; + }); + + if (it == _subOwners.end() || (*it)->name() != name) return nullptr; else return *it; } + +bool PropertyOwner::hasSubOwner(const std::string& name) const { + return subOwner(name) != nullptr; +} void PropertyOwner::setPropertyGroupName(std::string groupID, std::string name) { _groupNames[std::move(groupID)] = std::move(name); } - + const std::string& PropertyOwner::propertyGroupName(const std::string& groupID) const { auto it = _groupNames.find(groupID); @@ -104,7 +160,8 @@ void PropertyOwner::addProperty(Property* prop) // If we found the property identifier, we need to bail out if (it != _properties.end() && (*it)->identifier() == prop->identifier()) { LERROR("Property identifier '" << prop->identifier() - << "' already present in PropertyOwner"); + << "' already present in PropertyOwner '" + << name() << "'"); return; } else { // Otherwise we have found the correct position to add it in @@ -117,6 +174,38 @@ void PropertyOwner::addProperty(Property& prop) { addProperty(&prop); } + +void PropertyOwner::addPropertySubOwner(openspace::properties::PropertyOwner* owner) { + assert(owner != nullptr); + assert(std::is_sorted(_subOwners.begin(), _subOwners.end(), subOwnerLess)); + + if (owner->name().empty()) { + LERROR("PropertyOwner did not have a name"); + return; + } + + // See if we can find the name of the propertyowner to add + std::vector::iterator it + = std::lower_bound(_subOwners.begin(), _subOwners.end(), owner->name(), + [](PropertyOwner* owner, const std::string& str) { + return owner->name() < str; + }); + + // If we found the propertyowner's name, we need to bail out + if (it != _subOwners.end() && (*it)->name() == owner->name()) { + LERROR("PropertyOwner '" << owner->name() + << "' already present in PropertyOwner '" << name() << "'"); + return; + } else { + // Otherwise we have found the correct position to add it in + _subOwners.insert(it, owner); + } + +} + +void PropertyOwner::addPropertySubOwner(openspace::properties::PropertyOwner& owner) { + addPropertySubOwner(&owner); +} void PropertyOwner::removeProperty(Property* prop) { @@ -142,6 +231,28 @@ void PropertyOwner::removeProperty(Property& prop) { removeProperty(&prop); } + +void PropertyOwner::removePropertySubOwner(openspace::properties::PropertyOwner* owner) { + assert(owner != nullptr); + + // See if we can find the name of the propertyowner to add + std::vector::iterator it + = std::lower_bound(_subOwners.begin(), _subOwners.end(), owner->name(), + [](PropertyOwner* owner, const std::string& str) { + return owner->name() < str; + }); + + // If we found the propertyowner, we can delete it + if (it != _subOwners.end() && (*it)->name() == owner->name()) { + _subOwners.erase(it); + } else + LERROR("PropertyOwner with name '" << owner->name() + << "' not found for removal."); +} + +void PropertyOwner::removePropertySubOwner(openspace::properties::PropertyOwner& owner) { + removePropertySubOwner(&owner); +} void PropertyOwner::setName(std::string name) { diff --git a/src/query/query.cpp b/src/query/query.cpp index ffe1143f93..404758a59f 100644 --- a/src/query/query.cpp +++ b/src/query/query.cpp @@ -30,7 +30,7 @@ namespace openspace { namespace { -const std::string _loggerCat = "Query"; + const std::string _loggerCat = "Query"; } SceneGraph* sceneGraph() @@ -46,28 +46,25 @@ SceneGraphNode* sceneGraphNode(const std::string& name) properties::Property* property(const std::string& uri) { - const size_t separator = uri.find('.'); - return property(uri.substr(0, separator), uri.substr(separator)); -} + // The URI consists of the following form at this stage: + // .{.}^(0..n) + + const size_t nodeNameSeparator = uri.find(properties::PropertyOwner::URISeparator); + if (nodeNameSeparator == std::string::npos) { + LERROR("Malformed URI '" << uri << "': At least one '" << nodeNameSeparator + << "' separator must be present."); + return nullptr; + } + const std::string nodeName = uri.substr(0, nodeNameSeparator); + const std::string remainingUri = uri.substr(nodeNameSeparator + 1); -properties::Property* property(const std::string& nodeName, const std::string& propertyName) -{ SceneGraphNode* node = sceneGraphNode(nodeName); if (!node) { LERROR("Node '" << nodeName << "' did not exist"); return nullptr; } - Renderable* propertyOwner = node->renderable(); - if (!propertyOwner) { - LERROR("Node '" << nodeName << "' is not a PropertyOwner"); - return nullptr; - } - properties::Property* property = propertyOwner->property(propertyName);; - if (!property) { - LERROR("Node '" << nodeName << "' did not have property '" << - propertyName << "'"); - return nullptr; - } + + properties::Property* property = node->property(remainingUri); return property; } diff --git a/src/rendering/renderable.cpp b/src/rendering/renderable.cpp index 9a432b02d7..aaa7455428 100644 --- a/src/rendering/renderable.cpp +++ b/src/rendering/renderable.cpp @@ -60,9 +60,9 @@ Renderable* Renderable::createFromDictionary(const ghoul::Dictionary& dictionary Renderable::Renderable(const ghoul::Dictionary& dictionary) { - std::string name; - dictionary.getValue(constants::scenegraphnode::keyName, name); - setName(name); +// std::string name; +// dictionary.getValue(constants::scenegraphnode::keyName, name); + setName("Renderable"); } Renderable::~Renderable() diff --git a/src/scenegraph/scenegraph.cpp b/src/scenegraph/scenegraph.cpp index f9001edd32..49e396dc81 100644 --- a/src/scenegraph/scenegraph.cpp +++ b/src/scenegraph/scenegraph.cpp @@ -57,7 +57,7 @@ namespace openspace { void printTree(SceneGraphNode* node, std::string pre = "") { - LDEBUGC("Tree", pre << node->nodeName()); + LDEBUGC("Tree", pre << node->name()); const std::vector& children = node->children(); for (SceneGraphNode* child : children) printTree(child, pre + " "); @@ -155,9 +155,9 @@ bool SceneGraph::initialize() for (auto node : _nodes) { bool success = node->initialize(); if (success) - LDEBUG(node->nodeName() << " initialized successfully!"); + LDEBUG(node->name() << " initialized successfully!"); else - LWARNING(node->nodeName() << " not initialized."); + LWARNING(node->name() << " not initialized."); } // update the position of all nodes @@ -325,7 +325,7 @@ void SceneGraph::loadModule(const std::string& modulePath) SceneGraphNode* node = SceneGraphNode::createFromDictionary(element); - _allNodes.emplace(node->nodeName(), node); + _allNodes.emplace(node->name(), node); _nodes.push_back(node); } diff --git a/src/scenegraph/scenegraphnode.cpp b/src/scenegraph/scenegraphnode.cpp index f7e19c560f..5c1105c2b6 100644 --- a/src/scenegraph/scenegraphnode.cpp +++ b/src/scenegraph/scenegraphnode.cpp @@ -63,21 +63,22 @@ SceneGraphNode* SceneGraphNode::createFromDictionary(const ghoul::Dictionary& di << keyName << "' key"); return nullptr; } - dictionary.getValue(keyName, result->_nodeName); + std::string name; + dictionary.getValue(keyName, name); + result->setName(name); if (dictionary.hasValue(keyRenderable)) { ghoul::Dictionary renderableDictionary; dictionary.getValue(keyRenderable, renderableDictionary); renderableDictionary.setValue(keyPathModule, path); - renderableDictionary.setValue(keyName, result->_nodeName); result->_renderable = Renderable::createFromDictionary(renderableDictionary); if (result->_renderable == nullptr) { LERROR("Failed to create renderable for SceneGraphNode '" - << result->_nodeName << "'"); + << result->name() << "'"); return nullptr; } - LDEBUG("Successfully create renderable for '" << result->_nodeName << "'"); + LDEBUG("Successfully create renderable for '" << result->name() << "'"); } if (dictionary.hasKey(keyEphemeris)) { ghoul::Dictionary ephemerisDictionary; @@ -86,10 +87,10 @@ SceneGraphNode* SceneGraphNode::createFromDictionary(const ghoul::Dictionary& di result->_ephemeris = Ephemeris::createFromDictionary(ephemerisDictionary); if (result->_ephemeris == nullptr) { LERROR("Failed to create ephemeris for SceneGraphNode '" - << result->_nodeName << "'"); + << result->name() << "'"); return nullptr; } - LDEBUG("Successfully create ephemeris for '" << result->_nodeName << "'"); + LDEBUG("Successfully create ephemeris for '" << result->name() << "'"); } std::string parentName; @@ -101,20 +102,19 @@ SceneGraphNode* SceneGraphNode::createFromDictionary(const ghoul::Dictionary& di SceneGraphNode* parentNode = sceneGraphNode(parentName); if (parentNode == nullptr) { LFATAL("Could not find parent named '" - << parentName << "' for '" << result->_nodeName << "'." + << parentName << "' for '" << result->name() << "'." << " Check module definition order. Skipping module."); } parentNode->addNode(result); LDEBUG("Successfully created SceneGraphNode '" - << result->_nodeName << "'"); + << result->name() << "'"); return result; } SceneGraphNode::SceneGraphNode() : _parent(nullptr) - , _nodeName("") , _ephemeris(new StaticEphemeris) , _renderable(nullptr) , _renderableVisible(false) @@ -139,7 +139,7 @@ bool SceneGraphNode::initialize() bool SceneGraphNode::deinitialize() { - LDEBUG("Deinitialize: " << _nodeName); + LDEBUG("Deinitialize: " << name()); delete _renderable; _renderable = nullptr; @@ -153,7 +153,6 @@ bool SceneGraphNode::deinitialize() // reset variables _parent = nullptr; - _nodeName = ""; _renderableVisible = false; _boundingSphereVisible = false; _boundingSphere = PowerScaledScalar(0.0, 0.0); @@ -232,11 +231,6 @@ void SceneGraphNode::addNode(SceneGraphNode* child) _children.push_back(child); } -void SceneGraphNode::setName(const std::string& name) -{ - _nodeName = name; -} - void SceneGraphNode::setParent(SceneGraphNode* parent) { _parent = parent; @@ -257,11 +251,6 @@ psc SceneGraphNode::worldPosition() const } } -std::string SceneGraphNode::nodeName() const -{ - return _nodeName; -} - SceneGraphNode* SceneGraphNode::parent() const { return _parent; @@ -353,7 +342,7 @@ bool SceneGraphNode::sphereInsideFrustum(const psc s_pos, const PowerScaledScala SceneGraphNode* SceneGraphNode::childNode(const std::string& name) { - if (_nodeName == name) + if (this->name() == name) return this; else for (auto it : _children) { @@ -367,7 +356,7 @@ SceneGraphNode* SceneGraphNode::childNode(const std::string& name) void SceneGraphNode::print() const { - std::cout << _nodeName << std::endl; + std::cout << name() << std::endl; for (auto it : _children) { it->print(); } diff --git a/src/scripting/scriptfunctions.cpp b/src/scripting/scriptfunctions.cpp index c5ffae260f..7d1e90f32e 100644 --- a/src/scripting/scriptfunctions.cpp +++ b/src/scripting/scriptfunctions.cpp @@ -95,8 +95,7 @@ int property_setValue(lua_State* L) const std::string _loggerCat = "property_setValue"; // TODO Check for argument number (ab) - std::string nodeName = luaL_checkstring(L, -3); - std::string propertyName = luaL_checkstring(L, -2); + std::string uri = luaL_checkstring(L, -2); const int type = lua_type(L, -1); boost::any propertyValue; switch (type) { @@ -122,10 +121,9 @@ int property_setValue(lua_State* L) break; } - Property* prop = property(nodeName, propertyName); + Property* prop = property(uri); if (!prop) { - LERROR("Property at " << nodeName << "." << propertyName << - " could not be found"); + LERROR("Property with uri '" << uri << "' could not be found"); return 0; }