From 677b8ec95c4e9ad690397f1b816048931f4455f0 Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Wed, 28 Oct 2020 16:17:52 -0500 Subject: [PATCH 1/6] Move graph creation to sdf::Root from sdf::World and sdf::Model. Signed-off-by: Addisu Z. Taddese --- include/sdf/Model.hh | 12 +++-- include/sdf/World.hh | 21 +++++++++ src/Model.cc | 102 +----------------------------------------- src/Root.cc | 82 +++++++++++++++++++++++++++++++++- src/World.cc | 103 ++++++++++++++++--------------------------- 5 files changed, 148 insertions(+), 172 deletions(-) diff --git a/include/sdf/Model.hh b/include/sdf/Model.hh index 027a016c1..ca9313873 100644 --- a/include/sdf/Model.hh +++ b/include/sdf/Model.hh @@ -301,15 +301,17 @@ namespace sdf public: void SetPlacementFrameName(const std::string &_name); /// \brief Give the Scoped PoseRelativeToGraph to be used for resolving - /// poses. This is private and is intended to be called by World::Load and - /// Model::Load if this is a nested model. + /// poses. This is private and is intended to be called by Root::Load or + /// World::SetPoseRelativeToGraph if this is a standalone model and + /// Model::SetPoseRelativeToGraph if this is a nested model. /// \param[in] _graph Scoped PoseRelativeToGraph object. private: void SetPoseRelativeToGraph( sdf::ScopedGraph _graph); /// \brief Give the Scoped FrameAttachedToGraph to be used for resolving /// attached bodies. This is private and is intended to be called by - /// World::Load and Model::Load if this is a nested model. + /// Root::Load or World::SetFrameAttachedToGraph if this is a standalone + /// model and Model::SetFrameAttachedToGraph if this is a nested model. /// \param[in] _graph Scoped FrameAttachedToGraph object. private: void SetFrameAttachedToGraph( sdf::ScopedGraph _graph); @@ -321,8 +323,10 @@ namespace sdf private: std::pair CanonicalLinkAndRelativeName() const; - /// \brief Allow World::Load to call SetPoseRelativeToGraph and + /// \brief Allow Root::Load, World::SetPoseRelativeToGraph, or + /// World::SetFrameAttachedToGraph to call SetPoseRelativeToGraph and /// SetFrameAttachedToGraph + friend class Root; friend class World; /// \brief Allow helper function in FrameSemantics.cc to call diff --git a/include/sdf/World.hh b/include/sdf/World.hh index 2f513eaaa..c257666b3 100644 --- a/include/sdf/World.hh +++ b/include/sdf/World.hh @@ -41,6 +41,9 @@ namespace sdf class Model; class Physics; class WorldPrivate; + struct PoseRelativeToGraph; + struct FrameAttachedToGraph; + template class ScopedGraph; class SDFORMAT_VISIBLE World { @@ -269,6 +272,24 @@ namespace sdf /// \return True if there exists a physics profile with the given name. public: bool PhysicsNameExists(const std::string &_name) const; + /// \brief Give the Scoped PoseRelativeToGraph to be passed on to child + /// entities for resolving poses. This is private and is intended to be + /// called by Root::Load. + /// \param[in] _graph Scoped PoseRelativeToGraph object. + private: void SetPoseRelativeToGraph( + sdf::ScopedGraph _graph); + + /// \brief Give the Scoped FrameAttachedToGraph to be passed on to child + /// entities for attached bodes. This is private and is intended to be + /// called by Root::Load. + /// \param[in] _graph Scoped FrameAttachedToGraph object. + private: void SetFrameAttachedToGraph( + sdf::ScopedGraph _graph); + + /// \brief Allow Root::Load to call SetPoseRelativeToGraph and + /// SetFrameAttachedToGraph + friend class Root; + /// \brief Private data pointer. private: WorldPrivate *dataPtr = nullptr; }; diff --git a/src/Model.cc b/src/Model.cc index 3a7158357..c5bd5841a 100644 --- a/src/Model.cc +++ b/src/Model.cc @@ -78,22 +78,10 @@ class sdf::ModelPrivate /// \brief The SDF element pointer used during load. public: sdf::ElementPtr sdf; - /// \brief Frame Attached-To Graph constructed during Load. - /// This would only be allocated if this model is a top level model, i.e, a - /// model file. - public: std::shared_ptr ownedFrameAttachedToGraph; - - /// \brief Scoped Frame Attached-To graph that can either point to a graph - /// owned by this model or a subgraph of a parent entity. + /// \brief Scoped Frame Attached-To graph at the parent model or world scope. public: sdf::ScopedGraph frameAttachedToGraph; - /// \brief Pose Relative-To Graph constructed during Load. - /// This would only be allocated if this model is a top level model, i.e, a - /// model file. - public: std::shared_ptr ownedPoseGraph; - - /// \brief Scoped Pose Relative-To graph that can either point to a graph - /// owned by this model or a subgraph of a parent entity. + /// \brief Scoped Pose Relative-To graph at the parent model or world scope. public: sdf::ScopedGraph poseGraph; /// \brief Scope name of parent Pose Relative-To Graph (world or __model__). @@ -117,25 +105,6 @@ Model::~Model() Model::Model(const Model &_model) : dataPtr(new ModelPrivate(*_model.dataPtr)) { - if (_model.dataPtr->ownedFrameAttachedToGraph) - { - // If the model owns the frameAttachedToGraph, we need to allocate a new - // sdf::FrameAttachedToGraph object and copy it. We also need to assign the - // ScopedGraph to this object - this->dataPtr->ownedFrameAttachedToGraph = - std::make_shared( - *_model.dataPtr->ownedFrameAttachedToGraph); - this->dataPtr->frameAttachedToGraph = - ScopedGraph( - this->dataPtr->ownedFrameAttachedToGraph); - } - if (_model.dataPtr->ownedPoseGraph) - { - this->dataPtr->ownedPoseGraph = std::make_shared( - *_model.dataPtr->ownedPoseGraph); - this->dataPtr->poseGraph = - ScopedGraph(this->dataPtr->ownedPoseGraph); - } for (auto &link : this->dataPtr->links) { link.SetPoseRelativeToGraph(this->dataPtr->poseGraph); @@ -239,22 +208,6 @@ Errors Model::Load(ElementPtr _sdf) << this->Name() << "].\n"; } - // Whether this model is defined at the root level of an SDFormat file (i.e - // in XPath "/sdf/model") - bool isModelAtRootOfXmlFile = false; - - auto parentElem = this->dataPtr->sdf->GetParent(); - if (parentElem && parentElem->GetName() == "sdf") - { - this->dataPtr->ownedPoseGraph = std::make_shared(); - this->dataPtr->poseGraph = ScopedGraph(this->dataPtr->ownedPoseGraph); - this->dataPtr->ownedFrameAttachedToGraph = - std::make_shared(); - this->dataPtr->frameAttachedToGraph = - ScopedGraph(this->dataPtr->ownedFrameAttachedToGraph); - isModelAtRootOfXmlFile = true; - } - // Set of implicit and explicit frame names in this model for tracking // name collisions std::unordered_set frameNames; @@ -395,42 +348,6 @@ Errors Model::Load(ElementPtr _sdf) frameNames.insert(frameName); } - // Build the graphs. - - // Build the FrameAttachedToGraph if the model is not static. - // Re-enable this when the buildFrameAttachedToGraph implementation handles - // static models. - if (!this->Static()) - { - if (isModelAtRootOfXmlFile) - { - Errors frameAttachedToGraphErrors = - buildFrameAttachedToGraph(this->dataPtr->frameAttachedToGraph, this); - errors.insert(errors.end(), frameAttachedToGraphErrors.begin(), - frameAttachedToGraphErrors.end()); - Errors validateFrameAttachedGraphErrors = - validateFrameAttachedToGraph(this->dataPtr->frameAttachedToGraph); - errors.insert(errors.end(), validateFrameAttachedGraphErrors.begin(), - validateFrameAttachedGraphErrors.end()); - - this->SetFrameAttachedToGraph(this->dataPtr->frameAttachedToGraph); - } - } - - // Build the PoseRelativeToGraph - if (isModelAtRootOfXmlFile) - { - Errors poseGraphErrors = - buildPoseRelativeToGraph(this->dataPtr->poseGraph, this); - errors.insert(errors.end(), poseGraphErrors.begin(), poseGraphErrors.end()); - Errors validatePoseGraphErrors = - validatePoseRelativeToGraph(this->dataPtr->poseGraph); - errors.insert(errors.end(), validatePoseGraphErrors.begin(), - validatePoseGraphErrors.end()); - - this->SetPoseRelativeToGraph(this->dataPtr->poseGraph); - } - return errors; } @@ -749,13 +666,6 @@ void Model::SetPoseRelativeTo(const std::string &_frame) ///////////////////////////////////////////////// void Model::SetPoseRelativeToGraph(sdf::ScopedGraph _graph) { - // If the scoped graph doesn't point to the graph owned by this model, we - // clear the owned graph to maintain the invariant that if - // ownedPoseGraph is valid poseGraph points to it. - if (!_graph.PointsTo(this->dataPtr->ownedPoseGraph)) - { - this->dataPtr->ownedPoseGraph.reset(); - } this->dataPtr->poseGraph = _graph; this->dataPtr->poseGraphScopeVertexName = _graph.VertexLocalName(_graph.ScopeVertexId()); @@ -784,14 +694,6 @@ void Model::SetPoseRelativeToGraph(sdf::ScopedGraph _graph) void Model::SetFrameAttachedToGraph( sdf::ScopedGraph _graph) { - // If the scoped graph doesn't point to the graph owned by this model, we - // clear the owned graph to maintain the invariant that if - // ownedFrameAttachedToGraph is valid frameAttachedToGraph points to it. - if (!_graph.PointsTo(this->dataPtr->ownedFrameAttachedToGraph)) - { - this->dataPtr->ownedFrameAttachedToGraph.reset(); - } - this->dataPtr->frameAttachedToGraph = _graph; auto childFrameAttachedToGraph = diff --git a/src/Root.cc b/src/Root.cc index 7898182c2..82bdf364c 100644 --- a/src/Root.cc +++ b/src/Root.cc @@ -26,6 +26,8 @@ #include "sdf/World.hh" #include "sdf/parser.hh" #include "sdf/sdf_config.h" +#include "FrameSemantics.hh" +#include "ScopedGraph.hh" #include "Utils.hh" using namespace sdf; @@ -33,6 +35,13 @@ using namespace sdf; /// \brief Private data for sdf::Root class sdf::RootPrivate { + public: template + ScopedGraph + AddFrameAttachedToGraph(const T &_domObj, Errors &_errors); + public: template + ScopedGraph + AddPoseRealtiveToGraph(const T &_domObj, Errors &_errors); + /// \brief Version string public: std::string version = ""; @@ -48,10 +57,54 @@ class sdf::RootPrivate /// \brief The actors specified under the root SDF element public: std::vector actors; + /// \brief Frame Attached-To Graphs constructed during Load. + public: std::vector> + frameAttachedToGraphs; + + /// \brief Pose Relative-To Graphs constructed during Load. + public: std::vector> + poseRelativeToGraphs; + /// \brief The SDF element pointer generated during load. public: sdf::ElementPtr sdf; }; +///////////////////////////////////////////////// +template +ScopedGraph RootPrivate::AddFrameAttachedToGraph( + const T &_domObj, Errors &_errors) +{ + auto &ownedGraph = this->frameAttachedToGraphs.emplace_back( + std::make_shared()); + ScopedGraph frameGraph(ownedGraph); + + Errors buildErrors = buildFrameAttachedToGraph(frameGraph, &_domObj); + _errors.insert(_errors.end(), buildErrors.begin(), buildErrors.end()); + + Errors validateErrors = validateFrameAttachedToGraph(frameGraph); + _errors.insert(_errors.end(), validateErrors.begin(), validateErrors.end()); + + return frameGraph; +} + +///////////////////////////////////////////////// +template +ScopedGraph RootPrivate::AddPoseRealtiveToGraph( + const T &_domObj, Errors &_errors) +{ + auto &ownedGraph = this->poseRelativeToGraphs.emplace_back( + std::make_shared()); + ScopedGraph poseGraph(ownedGraph); + + Errors buildErrors = buildPoseRelativeToGraph(poseGraph, &_domObj); + _errors.insert(_errors.end(), buildErrors.begin(), buildErrors.end()); + + Errors validateErrors = validatePoseRelativeToGraph(poseGraph); + _errors.insert(_errors.end(), validateErrors.begin(), validateErrors.end()); + + return poseGraph; +} + ///////////////////////////////////////////////// Root::Root() : dataPtr(new RootPrivate) @@ -151,6 +204,17 @@ Errors Root::Load(SDFPtr _sdf) World world; Errors worldErrors = world.Load(elem); + + // Build the graphs. + auto frameAttachedToGraph = + this->dataPtr->AddFrameAttachedToGraph(world, worldErrors); + + world.SetFrameAttachedToGraph(frameAttachedToGraph); + + auto poseRelativeToGraph = + this->dataPtr->AddPoseRealtiveToGraph(world, worldErrors); + world.SetPoseRelativeToGraph(poseRelativeToGraph); + // Attempt to load the world if (worldErrors.empty()) { @@ -169,16 +233,30 @@ Errors Root::Load(SDFPtr _sdf) errors.push_back({ErrorCode::ELEMENT_INVALID, "Failed to load a world."}); } + this->dataPtr->worlds.push_back(std::move(world)); elem = elem->GetNextElement("world"); } } // Load all the models. - Errors modelLoadErrors = loadUniqueRepeated(this->dataPtr->sdf, - "model", this->dataPtr->models); + Errors modelLoadErrors = loadUniqueRepeated( + this->dataPtr->sdf, "model", this->dataPtr->models); errors.insert(errors.end(), modelLoadErrors.begin(), modelLoadErrors.end()); + // Build the graphs. + for (sdf::Model &model : this->dataPtr->models) + { + auto frameAttachedToGraph = + this->dataPtr->AddFrameAttachedToGraph(model, errors); + + model.SetFrameAttachedToGraph(frameAttachedToGraph); + + auto poseRelativeToGraph = + this->dataPtr->AddPoseRealtiveToGraph(model, errors); + model.SetPoseRelativeToGraph(poseRelativeToGraph); + } + // Load all the lights. Errors lightLoadErrors = loadUniqueRepeated(this->dataPtr->sdf, "light", this->dataPtr->lights); diff --git a/src/World.cc b/src/World.cc index ff4ae55b5..b0b2632e5 100644 --- a/src/World.cc +++ b/src/World.cc @@ -35,7 +35,7 @@ using namespace sdf; class sdf::WorldPrivate { /// \brief Default constructor - public: WorldPrivate(); + public: WorldPrivate() = default; /// \brief Copy constructor /// \param[in] _worldPrivate Joint axis to move. @@ -89,30 +89,15 @@ class sdf::WorldPrivate public: ignition::math::Vector3d windLinearVelocity = ignition::math::Vector3d::Zero; - /// \brief Frame Attached-To Graph constructed during Load. - public: std::shared_ptr ownedFrameAttachedToGraph; - /// \brief Scoped Frame Attached-To graph that points to a graph owned /// by this world. public: sdf::ScopedGraph frameAttachedToGraph; - /// \brief Pose Relative-To Graph constructed during Load. - public: std::shared_ptr ownedPoseRelativeToGraph; - /// \brief Scoped Pose Relative-To graph that points to a graph owned by this /// world. public: sdf::ScopedGraph poseRelativeToGraph; }; -///////////////////////////////////////////////// -WorldPrivate::WorldPrivate() - : ownedFrameAttachedToGraph(std::make_shared()) - , frameAttachedToGraph(ownedFrameAttachedToGraph) - , ownedPoseRelativeToGraph(std::make_shared()) - , poseRelativeToGraph(ownedPoseRelativeToGraph) -{ -} - ///////////////////////////////////////////////// WorldPrivate::WorldPrivate(const WorldPrivate &_worldPrivate) : audioDevice(_worldPrivate.audioDevice), @@ -136,19 +121,6 @@ WorldPrivate::WorldPrivate(const WorldPrivate &_worldPrivate) { this->gui = std::make_unique(*(_worldPrivate.gui)); } - if (_worldPrivate.ownedFrameAttachedToGraph) - { - this->ownedFrameAttachedToGraph = - std::make_shared( - *(_worldPrivate.ownedFrameAttachedToGraph)); - this->frameAttachedToGraph = ScopedGraph(this->ownedFrameAttachedToGraph); - } - if (_worldPrivate.ownedPoseRelativeToGraph) - { - this->ownedPoseRelativeToGraph = std::make_shared( - *(_worldPrivate.ownedPoseRelativeToGraph)); - this->poseRelativeToGraph = ScopedGraph(this->ownedPoseRelativeToGraph); - } if (_worldPrivate.scene) { this->scene = std::make_unique(*(_worldPrivate.scene)); @@ -362,43 +334,6 @@ Errors World::Load(sdf::ElementPtr _sdf) errors.insert(errors.end(), sceneLoadErrors.begin(), sceneLoadErrors.end()); } - // Build the graphs. - Errors frameAttachedToGraphErrors = - buildFrameAttachedToGraph(this->dataPtr->frameAttachedToGraph, this); - errors.insert(errors.end(), frameAttachedToGraphErrors.begin(), - frameAttachedToGraphErrors.end()); - Errors validateFrameAttachedGraphErrors = - validateFrameAttachedToGraph(this->dataPtr->frameAttachedToGraph); - errors.insert(errors.end(), validateFrameAttachedGraphErrors.begin(), - validateFrameAttachedGraphErrors.end()); - for (auto &frame : this->dataPtr->frames) - { - frame.SetFrameAttachedToGraph(this->dataPtr->frameAttachedToGraph); - } - - Errors poseRelativeToGraphErrors = - buildPoseRelativeToGraph(this->dataPtr->poseRelativeToGraph, this); - errors.insert(errors.end(), poseRelativeToGraphErrors.begin(), - poseRelativeToGraphErrors.end()); - Errors validatePoseGraphErrors = - validatePoseRelativeToGraph(this->dataPtr->poseRelativeToGraph); - errors.insert(errors.end(), validatePoseGraphErrors.begin(), - validatePoseGraphErrors.end()); - for (auto &frame : this->dataPtr->frames) - { - frame.SetPoseRelativeToGraph(this->dataPtr->poseRelativeToGraph); - } - for (auto &model : this->dataPtr->models) - { - model.SetPoseRelativeToGraph(this->dataPtr->poseRelativeToGraph); - model.SetFrameAttachedToGraph(this->dataPtr->frameAttachedToGraph); - } - for (auto &light : this->dataPtr->lights) - { - light.SetXmlParentName("world"); - light.SetPoseRelativeToGraph(this->dataPtr->poseRelativeToGraph); - } - return errors; } @@ -682,3 +617,39 @@ bool World::PhysicsNameExists(const std::string &_name) const return false; } + +///////////////////////////////////////////////// +void World::SetPoseRelativeToGraph(sdf::ScopedGraph _graph) +{ + this->dataPtr->poseRelativeToGraph = _graph; + + for (auto &model : this->dataPtr->models) + { + model.SetPoseRelativeToGraph(this->dataPtr->poseRelativeToGraph); + } + for (auto &frame : this->dataPtr->frames) + { + frame.SetPoseRelativeToGraph(this->dataPtr->poseRelativeToGraph); + } + for (auto &light : this->dataPtr->lights) + { + light.SetXmlParentName("world"); + light.SetPoseRelativeToGraph(this->dataPtr->poseRelativeToGraph); + } +} + +///////////////////////////////////////////////// +void World::SetFrameAttachedToGraph( + sdf::ScopedGraph _graph) +{ + this->dataPtr->frameAttachedToGraph = _graph; + + for (auto &frame : this->dataPtr->frames) + { + frame.SetFrameAttachedToGraph(this->dataPtr->frameAttachedToGraph); + } + for (auto &model : this->dataPtr->models) + { + model.SetFrameAttachedToGraph(this->dataPtr->frameAttachedToGraph); + } +} From 65ec64343eefe662f59f8468f09b7e3ce195c7fa Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Thu, 29 Oct 2020 10:42:42 -0500 Subject: [PATCH 2/6] Convert ScopedGraph to hold a strong reference to the underlying graph Signed-off-by: Addisu Z. Taddese --- src/ScopedGraph.hh | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/ScopedGraph.hh b/src/ScopedGraph.hh index 521cf6baa..c173c7ac1 100644 --- a/src/ScopedGraph.hh +++ b/src/ScopedGraph.hh @@ -240,9 +240,9 @@ class ScopedGraph public: std::pair FindAndRemovePrefix( const std::string &_name) const; - /// \brief Weak pointer to either a FrameAttachedToGraph or + /// \brief Shared pointer to either a FrameAttachedToGraph or /// PoseRelativeToGraph. - private: std::weak_ptr graphWeak; + private: std::shared_ptr graphPtr; /// \brief Shared pointer to the scope's data. A shared_ptr is used because /// this data has to be shared among many DOM objects. @@ -252,7 +252,7 @@ class ScopedGraph ///////////////////////////////////////////////// template ScopedGraph::ScopedGraph(const std::shared_ptr &_graph) - : graphWeak(_graph) + : graphPtr(_graph) , dataPtr(std::make_shared()) { } @@ -274,21 +274,21 @@ ScopedGraph ScopedGraph::ChildModelScope(const std::string &_name) const template ScopedGraph::operator bool() const { - return !this->graphWeak.expired() && (this->dataPtr != nullptr); + return (this->graphPtr != nullptr) && (this->dataPtr != nullptr); } ///////////////////////////////////////////////// template auto ScopedGraph::Graph() const -> const MathGraphType & { - return this->graphWeak.lock()->graph; + return this->graphPtr->graph; } ///////////////////////////////////////////////// template auto ScopedGraph::Map() const -> const MapType & { - return this->graphWeak.lock()->map; + return this->graphPtr->map; } ///////////////////////////////////////////////// @@ -311,10 +311,9 @@ template auto ScopedGraph::AddVertex( const std::string &_name, const VertexType &_data) -> Vertex & { - auto graph = this->graphWeak.lock(); const std::string newName = this->AddPrefix(_name); - Vertex &vert = graph->graph.AddVertex(newName, _data); - graph->map[newName] = vert.Id(); + Vertex &vert = this->graphPtr->graph.AddVertex(newName, _data); + this->graphPtr->map[newName] = vert.Id(); return vert; } @@ -324,8 +323,7 @@ auto ScopedGraph::AddEdge( const ignition::math::graph::VertexId_P &_vertexPair, const EdgeType &_data) -> Edge & { - auto graph = graphWeak.lock(); - Edge &edge = graph->graph.AddEdge(_vertexPair, _data); + Edge &edge = this->graphPtr->graph.AddEdge(_vertexPair, _data); return edge; } @@ -367,7 +365,7 @@ void ScopedGraph::UpdateEdge(Edge &_edge, const EdgeType &_data) // insert a new one with the new pose. auto tailVertexId = _edge.Tail(); auto headVertexId = _edge.Head(); - auto &graph = this->graphWeak.lock()->graph; + auto &graph = this->graphPtr->graph; graph.RemoveEdge(_edge.Id()); _edge = graph.AddEdge({tailVertexId, headVertexId}, _data); } @@ -390,7 +388,7 @@ void ScopedGraph::SetScopeName(const std::string &_name) template std::size_t ScopedGraph::Count(const std::string &_name) const { - return this->graphWeak.lock()->map.count(this->AddPrefix(_name)); + return this->graphPtr->map.count(this->AddPrefix(_name)); } ///////////////////////////////////////////////// @@ -409,7 +407,7 @@ auto ScopedGraph::VertexIdByName(const std::string &_name) const -> VertexId template auto ScopedGraph::ScopeVertex() const -> const Vertex & { - return this->graphWeak.lock()->graph.VertexFromId( + return this->graphPtr->graph.VertexFromId( this->dataPtr->scopeVertexId); } @@ -424,7 +422,7 @@ auto ScopedGraph::ScopeVertexId() const -> VertexId template bool ScopedGraph::PointsTo(const std::shared_ptr &_graph) const { - return this->graphWeak.lock() == _graph; + return this->graphPtr == _graph; } ///////////////////////////////////////////////// From 75f4b1238f04f165de89f095ac7f74ff79d6290a Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Thu, 29 Oct 2020 10:43:24 -0500 Subject: [PATCH 3/6] Add copy/move constructors to sdf::Root Signed-off-by: Addisu Z. Taddese --- include/sdf/Root.hh | 18 +++++ src/Model.cc | 18 ----- src/Root.cc | 115 ++++++++++++++++++++--------- src/Root_TEST.cc | 172 ++++++++++++++++++++++++++++++++++++++++++++ src/ScopedGraph.hh | 5 ++ src/World.cc | 18 +---- 6 files changed, 280 insertions(+), 66 deletions(-) diff --git a/include/sdf/Root.hh b/include/sdf/Root.hh index ae88cd037..c960150a2 100644 --- a/include/sdf/Root.hh +++ b/include/sdf/Root.hh @@ -56,6 +56,24 @@ namespace sdf /// \brief Default constructor public: Root(); + /// \brief Copy constructor + /// \param[in] _root Root to copy. + public: Root(const Root &_root); + + /// \brief Move constructor + /// \param[in] _root Root to move. + public: Root(Root &&_root) noexcept; + + /// \brief Move assignment operator. + /// \param[in] _root Root to move. + /// \return Reference to this. + public: Root &operator=(Root &&_root) noexcept; + + /// \brief Copy assignment operator. + /// \param[in] _root Root to copy. + /// \return Reference to this. + public: Root &operator=(const Root &_root); + /// \brief Destructor public: ~Root(); diff --git a/src/Model.cc b/src/Model.cc index c5bd5841a..0c381decc 100644 --- a/src/Model.cc +++ b/src/Model.cc @@ -105,24 +105,6 @@ Model::~Model() Model::Model(const Model &_model) : dataPtr(new ModelPrivate(*_model.dataPtr)) { - for (auto &link : this->dataPtr->links) - { - link.SetPoseRelativeToGraph(this->dataPtr->poseGraph); - } - for (auto &model : this->dataPtr->models) - { - model.SetPoseRelativeToGraph(this->dataPtr->poseGraph); - } - for (auto &joint : this->dataPtr->joints) - { - joint.SetFrameAttachedToGraph(this->dataPtr->frameAttachedToGraph); - joint.SetPoseRelativeToGraph(this->dataPtr->poseGraph); - } - for (auto &frame : this->dataPtr->frames) - { - frame.SetFrameAttachedToGraph(this->dataPtr->frameAttachedToGraph); - frame.SetPoseRelativeToGraph(this->dataPtr->poseGraph); - } } ///////////////////////////////////////////////// diff --git a/src/Root.cc b/src/Root.cc index 82bdf364c..4fe99991d 100644 --- a/src/Root.cc +++ b/src/Root.cc @@ -35,13 +35,6 @@ using namespace sdf; /// \brief Private data for sdf::Root class sdf::RootPrivate { - public: template - ScopedGraph - AddFrameAttachedToGraph(const T &_domObj, Errors &_errors); - public: template - ScopedGraph - AddPoseRealtiveToGraph(const T &_domObj, Errors &_errors); - /// \brief Version string public: std::string version = ""; @@ -57,13 +50,21 @@ class sdf::RootPrivate /// \brief The actors specified under the root SDF element public: std::vector actors; - /// \brief Frame Attached-To Graphs constructed during Load. - public: std::vector> - frameAttachedToGraphs; + /// \brief Frame Attached-To Graphs constructed when loading Worlds. + public: std::vector> + worldFrameAttachedToGraphs; + + /// \brief Frame Attached-To Graphs constructed when loading Models. + public: std::vector> + modelFrameAttachedToGraphs; - /// \brief Pose Relative-To Graphs constructed during Load. - public: std::vector> - poseRelativeToGraphs; + /// \brief Pose Relative-To Graphs constructed when loading Worlds. + public: std::vector> + worldPoseRelativeToGraphs; + + /// \brief Pose Relative-To Graphs constructed when loading Models. + public: std::vector> + modelPoseRelativeToGraphs; /// \brief The SDF element pointer generated during load. public: sdf::ElementPtr sdf; @@ -71,17 +72,18 @@ class sdf::RootPrivate ///////////////////////////////////////////////// template -ScopedGraph RootPrivate::AddFrameAttachedToGraph( - const T &_domObj, Errors &_errors) +sdf::ScopedGraph addFrameAttachedToGraph( + std::vector> &_graphList, + const T &_domObj, sdf::Errors &_errors) { - auto &ownedGraph = this->frameAttachedToGraphs.emplace_back( - std::make_shared()); - ScopedGraph frameGraph(ownedGraph); + auto &frameGraph = + _graphList.emplace_back(std::make_shared()); - Errors buildErrors = buildFrameAttachedToGraph(frameGraph, &_domObj); + sdf::Errors buildErrors = + sdf::buildFrameAttachedToGraph(frameGraph, &_domObj); _errors.insert(_errors.end(), buildErrors.begin(), buildErrors.end()); - Errors validateErrors = validateFrameAttachedToGraph(frameGraph); + sdf::Errors validateErrors = sdf::validateFrameAttachedToGraph(frameGraph); _errors.insert(_errors.end(), validateErrors.begin(), validateErrors.end()); return frameGraph; @@ -89,12 +91,12 @@ ScopedGraph RootPrivate::AddFrameAttachedToGraph( ///////////////////////////////////////////////// template -ScopedGraph RootPrivate::AddPoseRealtiveToGraph( +ScopedGraph addPoseRealtiveToGraph( + std::vector> &_graphList, const T &_domObj, Errors &_errors) { - auto &ownedGraph = this->poseRelativeToGraphs.emplace_back( - std::make_shared()); - ScopedGraph poseGraph(ownedGraph); + auto &poseGraph = + _graphList.emplace_back(std::make_shared()); Errors buildErrors = buildPoseRelativeToGraph(poseGraph, &_domObj); _errors.insert(_errors.end(), buildErrors.begin(), buildErrors.end()); @@ -111,6 +113,54 @@ Root::Root() { } +///////////////////////////////////////////////// +Root::Root(const Root &_root) + : dataPtr(new RootPrivate(*_root.dataPtr)) +{ + // TODO(addisu) Do we need to make deep copies of the graphs? + // + // By construction the sizes of the worlds vector, the + // worldFrameAttachedToGraphs vector and the worldPoseRelativeToGraphs vector + // should be the same. + for (std::size_t i = 0; i < this->dataPtr->worlds.size(); ++i) + { + this->dataPtr->worlds[i].SetFrameAttachedToGraph( + this->dataPtr->worldFrameAttachedToGraphs[i]); + this->dataPtr->worlds[i].SetPoseRelativeToGraph( + this->dataPtr->worldPoseRelativeToGraphs[i]); + } + + // By construction the sizes of the models vector, the + // modelFrameAttachedToGraphs vector and the modelPoseRelativeToGraphs vector + // should be the same. + for (std::size_t i = 0; i < this->dataPtr->models.size(); ++i) + { + this->dataPtr->models[i].SetFrameAttachedToGraph( + this->dataPtr->modelFrameAttachedToGraphs[i]); + this->dataPtr->models[i].SetPoseRelativeToGraph( + this->dataPtr->modelPoseRelativeToGraphs[i]); + } +} + +///////////////////////////////////////////////// +Root::Root(Root &&_root) noexcept + : dataPtr(std::exchange(_root.dataPtr, nullptr)) +{ +} + +///////////////////////////////////////////////// +Root &Root::operator=(const Root &_root) +{ + return *this = Root(_root); +} + +///////////////////////////////////////////////// +Root &Root::operator=(Root &&_root) noexcept +{ + std::swap(this->dataPtr, _root.dataPtr); + return *this; +} + ///////////////////////////////////////////////// Root::~Root() { @@ -206,13 +256,12 @@ Errors Root::Load(SDFPtr _sdf) Errors worldErrors = world.Load(elem); // Build the graphs. - auto frameAttachedToGraph = - this->dataPtr->AddFrameAttachedToGraph(world, worldErrors); - + auto frameAttachedToGraph = addFrameAttachedToGraph( + this->dataPtr->worldFrameAttachedToGraphs, world, worldErrors); world.SetFrameAttachedToGraph(frameAttachedToGraph); - auto poseRelativeToGraph = - this->dataPtr->AddPoseRealtiveToGraph(world, worldErrors); + auto poseRelativeToGraph = addPoseRealtiveToGraph( + this->dataPtr->worldPoseRelativeToGraphs, world, worldErrors); world.SetPoseRelativeToGraph(poseRelativeToGraph); // Attempt to load the world @@ -247,13 +296,13 @@ Errors Root::Load(SDFPtr _sdf) // Build the graphs. for (sdf::Model &model : this->dataPtr->models) { - auto frameAttachedToGraph = - this->dataPtr->AddFrameAttachedToGraph(model, errors); + auto frameAttachedToGraph = addFrameAttachedToGraph( + this->dataPtr->modelFrameAttachedToGraphs, model, errors); model.SetFrameAttachedToGraph(frameAttachedToGraph); - auto poseRelativeToGraph = - this->dataPtr->AddPoseRealtiveToGraph(model, errors); + auto poseRelativeToGraph = addPoseRealtiveToGraph( + this->dataPtr->modelPoseRelativeToGraphs, model, errors); model.SetPoseRelativeToGraph(poseRelativeToGraph); } diff --git a/src/Root_TEST.cc b/src/Root_TEST.cc index 55fb747db..6ba8aea5f 100644 --- a/src/Root_TEST.cc +++ b/src/Root_TEST.cc @@ -23,6 +23,8 @@ #include "sdf/Link.hh" #include "sdf/Light.hh" #include "sdf/Model.hh" +#include "sdf/World.hh" +#include "sdf/Frame.hh" #include "sdf/Root.hh" ///////////////////////////////////////////////// @@ -56,6 +58,67 @@ TEST(DOMRoot, Construction) EXPECT_TRUE(root.ActorByIndex(1) == nullptr); } +///////////////////////////////////////////////// +TEST(DOMRoot, CopyConstructor) +{ + sdf::Root root; + root.SetVersion("test_root"); + + sdf::Root root2(root); + EXPECT_EQ("test_root", root2.Version()); +} + +///////////////////////////////////////////////// +TEST(DOMRoot, CopyAssignmentOperator) +{ + sdf::Root root; + root.SetVersion("test_root"); + + sdf::Root root2; + root2 = root; + EXPECT_EQ("test_root", root2.Version()); +} + +///////////////////////////////////////////////// +TEST(DOMRoot, MoveConstructor) +{ + sdf::Root root; + root.SetVersion("test_root"); + + sdf::Root root2(root); + EXPECT_EQ("test_root", root2.Version()); +} + +///////////////////////////////////////////////// +TEST(DOMRoot, MoveAssignmentOperator) +{ + sdf::Root root; + root.SetVersion("test_root"); + + sdf::Root root2; + root2 = std::move(root); + EXPECT_EQ("test_root", root2.Version()); +} + +///////////////////////////////////////////////// +TEST(DOMRoot, CopyAssignmentAfterMove) +{ + sdf::Root root1; + root1.SetVersion("root1"); + + sdf::Root root2; + root2.SetVersion("root2"); + + // This is similar to what std::swap does except it uses std::move for each + // assignment + sdf::Root tmp = std::move(root1); + root1 = root2; + root2 = tmp; + + EXPECT_EQ("root2", root1.Version()); + EXPECT_EQ("root1", root2.Version()); +} + ///////////////////////////////////////////////// TEST(DOMRoot, StringParse) { @@ -140,3 +203,112 @@ TEST(DOMRoot, Set) root.SetVersion(SDF_PROTOCOL_VERSION); EXPECT_STREQ(SDF_PROTOCOL_VERSION, root.Version().c_str()); } + +///////////////////////////////////////////////// +TEST(DOMRoot, FrameSemanticsOnCopyAndMove) +{ + const std::string sdfString1 = R"( + + + + 0 1 0 0 0 0 + + + )"; + + const std::string sdfString2 = R"( + + + + 1 1 0 0 0 0 + + + )"; + + auto testFrame1 = [](const sdf::Root &_root) + { + auto *world = _root.WorldByIndex(0); + ASSERT_NE(nullptr, world); + auto *frame = world->FrameByIndex(0); + ASSERT_NE(nullptr, frame); + EXPECT_EQ("frame1", frame->Name()); + ignition::math::Pose3d pose; + sdf::Errors errors = frame->SemanticPose().Resolve(pose); + EXPECT_TRUE(errors.empty()) << errors; + EXPECT_EQ(ignition::math::Pose3d(0, 1, 0, 0, 0, 0), pose); + }; + + auto testFrame2 = [](const sdf::Root &_root) + { + auto *world = _root.WorldByIndex(0); + ASSERT_NE(nullptr, world); + auto *frame = world->FrameByIndex(0); + ASSERT_NE(nullptr, frame); + EXPECT_EQ("frame2", frame->Name()); + ignition::math::Pose3d pose; + sdf::Errors errors = frame->SemanticPose().Resolve(pose); + EXPECT_TRUE(errors.empty()) << errors; + EXPECT_EQ(ignition::math::Pose3d(1, 1, 0, 0, 0, 0), pose); + }; + + { + sdf::Root root1; + sdf::Errors errors = root1.LoadSdfString(sdfString1); + EXPECT_TRUE(errors.empty()) << errors; + testFrame1(root1); + } + + { + sdf::Root root2; + sdf::Errors errors = root2.LoadSdfString(sdfString2); + EXPECT_TRUE(errors.empty()) << errors; + testFrame2(root2); + } + + { + sdf::Root root1; + sdf::Errors errors = root1.LoadSdfString(sdfString1); + EXPECT_TRUE(errors.empty()) << errors; + // root2 is copy constructed from root1 + sdf::Root root2(root1); + testFrame1(root1); + testFrame1(root2); + } + + { + sdf::Root root1; + sdf::Errors errors = root1.LoadSdfString(sdfString1); + EXPECT_TRUE(errors.empty()) << errors; + sdf::Root root2; + errors = root2.LoadSdfString(sdfString2); + EXPECT_TRUE(errors.empty()) << errors; + + // root1 is copied into root2 via the assignment operator + root2 = root1; + testFrame1(root1); + testFrame1(root2); + } + + { + sdf::Root root1; + sdf::Errors errors = root1.LoadSdfString(sdfString1); + EXPECT_TRUE(errors.empty()) << errors; + + // then root1 is moved into root2 via the move constructor + sdf::Root root2(std::move(root1)); + testFrame1(root2); + } + + { + sdf::Root root1; + sdf::Errors errors = root1.LoadSdfString(sdfString1); + EXPECT_TRUE(errors.empty()) << errors; + sdf::Root root2; + errors = root2.LoadSdfString(sdfString2); + EXPECT_TRUE(errors.empty()) << errors; + + // root1 is moved into root2 via the move assignment operator. + root2 = std::move(root1); + testFrame1(root2); + } +} diff --git a/src/ScopedGraph.hh b/src/ScopedGraph.hh index c173c7ac1..a2dfca251 100644 --- a/src/ScopedGraph.hh +++ b/src/ScopedGraph.hh @@ -123,6 +123,11 @@ class ScopedGraph /// passed in graph. /// \param[in] _graph A shared pointer to PoseRelativeTo or FrameAttachedTo /// graph. + /// \remark The state of the ScopedGraph after construction is invalid because + /// member variables such as "ScopedGraphData::prefix" and + /// "ScopedGraphData::scopeName" will be empty. Therefore, it is imperative + /// that this constructor should only be used right before calling either + /// buildPoseRelativeToGraph or buildFrameAttachedToGraph. public: explicit ScopedGraph(const std::shared_ptr &_graph); /// \brief Creates a scope anchored at an existing child model vertex. diff --git a/src/World.cc b/src/World.cc index b0b2632e5..704408e13 100644 --- a/src/World.cc +++ b/src/World.cc @@ -110,7 +110,9 @@ WorldPrivate::WorldPrivate(const WorldPrivate &_worldPrivate) name(_worldPrivate.name), physics(_worldPrivate.physics), sdf(_worldPrivate.sdf), - windLinearVelocity(_worldPrivate.windLinearVelocity) + windLinearVelocity(_worldPrivate.windLinearVelocity), + frameAttachedToGraph(_worldPrivate.frameAttachedToGraph), + poseRelativeToGraph(_worldPrivate.poseRelativeToGraph) { if (_worldPrivate.atmosphere) { @@ -145,20 +147,6 @@ World::~World() World::World(const World &_world) : dataPtr(new WorldPrivate(*_world.dataPtr)) { - for (auto &frame : this->dataPtr->frames) - { - frame.SetFrameAttachedToGraph(this->dataPtr->frameAttachedToGraph); - frame.SetPoseRelativeToGraph(this->dataPtr->poseRelativeToGraph); - } - for (auto &model : this->dataPtr->models) - { - model.SetPoseRelativeToGraph(this->dataPtr->poseRelativeToGraph); - } - for (auto &light : this->dataPtr->lights) - { - light.SetXmlParentName("world"); - light.SetPoseRelativeToGraph(this->dataPtr->poseRelativeToGraph); - } } ///////////////////////////////////////////////// From 033cada26f5543d79a7673044ce8a82d170a72f7 Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Tue, 3 Nov 2020 17:40:34 -0600 Subject: [PATCH 4/6] Additional test of sdf::Root objects before copy and move Signed-off-by: Addisu Z. Taddese --- src/Root_TEST.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Root_TEST.cc b/src/Root_TEST.cc index 6ba8aea5f..db6e4bb0c 100644 --- a/src/Root_TEST.cc +++ b/src/Root_TEST.cc @@ -283,6 +283,9 @@ TEST(DOMRoot, FrameSemanticsOnCopyAndMove) errors = root2.LoadSdfString(sdfString2); EXPECT_TRUE(errors.empty()) << errors; + testFrame1(root1); + testFrame2(root2); + // root1 is copied into root2 via the assignment operator root2 = root1; testFrame1(root1); @@ -307,6 +310,9 @@ TEST(DOMRoot, FrameSemanticsOnCopyAndMove) errors = root2.LoadSdfString(sdfString2); EXPECT_TRUE(errors.empty()) << errors; + testFrame1(root1); + testFrame2(root2); + // root1 is moved into root2 via the move assignment operator. root2 = std::move(root1); testFrame1(root2); From 4997432aef311072c497e62f99022e204c5d5523 Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Tue, 3 Nov 2020 17:42:31 -0600 Subject: [PATCH 5/6] Fix typo Signed-off-by: Addisu Z. Taddese --- src/Root.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Root.cc b/src/Root.cc index 4fe99991d..1e9065e1b 100644 --- a/src/Root.cc +++ b/src/Root.cc @@ -91,7 +91,7 @@ sdf::ScopedGraph addFrameAttachedToGraph( ///////////////////////////////////////////////// template -ScopedGraph addPoseRealtiveToGraph( +ScopedGraph addPoseRelativeToGraph( std::vector> &_graphList, const T &_domObj, Errors &_errors) { @@ -260,7 +260,7 @@ Errors Root::Load(SDFPtr _sdf) this->dataPtr->worldFrameAttachedToGraphs, world, worldErrors); world.SetFrameAttachedToGraph(frameAttachedToGraph); - auto poseRelativeToGraph = addPoseRealtiveToGraph( + auto poseRelativeToGraph = addPoseRelativeToGraph( this->dataPtr->worldPoseRelativeToGraphs, world, worldErrors); world.SetPoseRelativeToGraph(poseRelativeToGraph); @@ -301,7 +301,7 @@ Errors Root::Load(SDFPtr _sdf) model.SetFrameAttachedToGraph(frameAttachedToGraph); - auto poseRelativeToGraph = addPoseRealtiveToGraph( + auto poseRelativeToGraph = addPoseRelativeToGraph( this->dataPtr->modelPoseRelativeToGraphs, model, errors); model.SetPoseRelativeToGraph(poseRelativeToGraph); } From d5e405ad53e30c168f42d6202913185894f7b19b Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Thu, 5 Nov 2020 07:42:53 -0600 Subject: [PATCH 6/6] Explicitly delete copy constructor and assignment. Signed-off-by: Addisu Z. Taddese --- include/sdf/Root.hh | 13 ++++----- src/Root.cc | 35 ---------------------- src/Root_TEST.cc | 71 ++------------------------------------------- 3 files changed, 8 insertions(+), 111 deletions(-) diff --git a/include/sdf/Root.hh b/include/sdf/Root.hh index c960150a2..d3b3ce931 100644 --- a/include/sdf/Root.hh +++ b/include/sdf/Root.hh @@ -56,9 +56,9 @@ namespace sdf /// \brief Default constructor public: Root(); - /// \brief Copy constructor - /// \param[in] _root Root to copy. - public: Root(const Root &_root); + /// \brief Copy constructor is explicitly deleted to avoid copying the + /// FrameAttachedToGraph and PoseRelativeToGraphs contained in Root. + public: Root(const Root &_root) = delete; /// \brief Move constructor /// \param[in] _root Root to move. @@ -69,10 +69,9 @@ namespace sdf /// \return Reference to this. public: Root &operator=(Root &&_root) noexcept; - /// \brief Copy assignment operator. - /// \param[in] _root Root to copy. - /// \return Reference to this. - public: Root &operator=(const Root &_root); + /// \brief Copy assignment operator is explicitly deleted to avoid copying + /// the FrameAttachedToGraph and PoseRelativeToGraphs contained in Root. + public: Root &operator=(const Root &_root) = delete; /// \brief Destructor public: ~Root(); diff --git a/src/Root.cc b/src/Root.cc index 1e9065e1b..30a08203c 100644 --- a/src/Root.cc +++ b/src/Root.cc @@ -113,47 +113,12 @@ Root::Root() { } -///////////////////////////////////////////////// -Root::Root(const Root &_root) - : dataPtr(new RootPrivate(*_root.dataPtr)) -{ - // TODO(addisu) Do we need to make deep copies of the graphs? - // - // By construction the sizes of the worlds vector, the - // worldFrameAttachedToGraphs vector and the worldPoseRelativeToGraphs vector - // should be the same. - for (std::size_t i = 0; i < this->dataPtr->worlds.size(); ++i) - { - this->dataPtr->worlds[i].SetFrameAttachedToGraph( - this->dataPtr->worldFrameAttachedToGraphs[i]); - this->dataPtr->worlds[i].SetPoseRelativeToGraph( - this->dataPtr->worldPoseRelativeToGraphs[i]); - } - - // By construction the sizes of the models vector, the - // modelFrameAttachedToGraphs vector and the modelPoseRelativeToGraphs vector - // should be the same. - for (std::size_t i = 0; i < this->dataPtr->models.size(); ++i) - { - this->dataPtr->models[i].SetFrameAttachedToGraph( - this->dataPtr->modelFrameAttachedToGraphs[i]); - this->dataPtr->models[i].SetPoseRelativeToGraph( - this->dataPtr->modelPoseRelativeToGraphs[i]); - } -} - ///////////////////////////////////////////////// Root::Root(Root &&_root) noexcept : dataPtr(std::exchange(_root.dataPtr, nullptr)) { } -///////////////////////////////////////////////// -Root &Root::operator=(const Root &_root) -{ - return *this = Root(_root); -} - ///////////////////////////////////////////////// Root &Root::operator=(Root &&_root) noexcept { diff --git a/src/Root_TEST.cc b/src/Root_TEST.cc index db6e4bb0c..60e2e3746 100644 --- a/src/Root_TEST.cc +++ b/src/Root_TEST.cc @@ -58,34 +58,13 @@ TEST(DOMRoot, Construction) EXPECT_TRUE(root.ActorByIndex(1) == nullptr); } -///////////////////////////////////////////////// -TEST(DOMRoot, CopyConstructor) -{ - sdf::Root root; - root.SetVersion("test_root"); - - sdf::Root root2(root); - EXPECT_EQ("test_root", root2.Version()); -} - -///////////////////////////////////////////////// -TEST(DOMRoot, CopyAssignmentOperator) -{ - sdf::Root root; - root.SetVersion("test_root"); - - sdf::Root root2; - root2 = root; - EXPECT_EQ("test_root", root2.Version()); -} - ///////////////////////////////////////////////// TEST(DOMRoot, MoveConstructor) { sdf::Root root; root.SetVersion("test_root"); - sdf::Root root2(root); + sdf::Root root2(std::move(root)); EXPECT_EQ("test_root", root2.Version()); } @@ -100,25 +79,6 @@ TEST(DOMRoot, MoveAssignmentOperator) EXPECT_EQ("test_root", root2.Version()); } -///////////////////////////////////////////////// -TEST(DOMRoot, CopyAssignmentAfterMove) -{ - sdf::Root root1; - root1.SetVersion("root1"); - - sdf::Root root2; - root2.SetVersion("root2"); - - // This is similar to what std::swap does except it uses std::move for each - // assignment - sdf::Root tmp = std::move(root1); - root1 = root2; - root2 = tmp; - - EXPECT_EQ("root2", root1.Version()); - EXPECT_EQ("root1", root2.Version()); -} - ///////////////////////////////////////////////// TEST(DOMRoot, StringParse) { @@ -205,7 +165,7 @@ TEST(DOMRoot, Set) } ///////////////////////////////////////////////// -TEST(DOMRoot, FrameSemanticsOnCopyAndMove) +TEST(DOMRoot, FrameSemanticsOnMove) { const std::string sdfString1 = R"( @@ -265,33 +225,6 @@ TEST(DOMRoot, FrameSemanticsOnCopyAndMove) testFrame2(root2); } - { - sdf::Root root1; - sdf::Errors errors = root1.LoadSdfString(sdfString1); - EXPECT_TRUE(errors.empty()) << errors; - // root2 is copy constructed from root1 - sdf::Root root2(root1); - testFrame1(root1); - testFrame1(root2); - } - - { - sdf::Root root1; - sdf::Errors errors = root1.LoadSdfString(sdfString1); - EXPECT_TRUE(errors.empty()) << errors; - sdf::Root root2; - errors = root2.LoadSdfString(sdfString2); - EXPECT_TRUE(errors.empty()) << errors; - - testFrame1(root1); - testFrame2(root2); - - // root1 is copied into root2 via the assignment operator - root2 = root1; - testFrame1(root1); - testFrame1(root2); - } - { sdf::Root root1; sdf::Errors errors = root1.LoadSdfString(sdfString1);