From a6cc2af7237edee354b5cf6b806c21271d939ded Mon Sep 17 00:00:00 2001 From: Jeremy Nimmer Date: Fri, 3 Jan 2025 07:29:06 -0800 Subject: [PATCH 1/2] Remove unncessary includes (#1523) Including iostream means introducing the static (global) constructors and destructors for std::cin, std::cerr, and std::cout. That extra init and fini code is undesirable when those streams are not actually used. Signed-off-by: Jeremy Nimmer Co-authored-by: Steve Peters --- include/sdf/Error.hh | 4 ++-- include/sdf/Exception.hh | 2 +- src/Console.cc | 1 + src/Element.cc | 1 + src/FrameSemantics.cc | 1 + src/SDF.cc | 3 ++- 6 files changed, 8 insertions(+), 4 deletions(-) diff --git a/include/sdf/Error.hh b/include/sdf/Error.hh index 54cebf951..761e37b88 100644 --- a/include/sdf/Error.hh +++ b/include/sdf/Error.hh @@ -17,9 +17,9 @@ #ifndef SDF_ERROR_HH_ #define SDF_ERROR_HH_ -#include -#include #include +#include +#include #include #include #include "sdf/Console.hh" diff --git a/include/sdf/Exception.hh b/include/sdf/Exception.hh index 271a885b1..aa70907d9 100644 --- a/include/sdf/Exception.hh +++ b/include/sdf/Exception.hh @@ -19,8 +19,8 @@ #define SDF_EXCEPTION_HH_ #include -#include #include +#include #include #include diff --git a/src/Console.cc b/src/Console.cc index c87ff536e..d875cd082 100644 --- a/src/Console.cc +++ b/src/Console.cc @@ -16,6 +16,7 @@ */ #include +#include #include #include #include diff --git a/src/Element.cc b/src/Element.cc index 7c95b6a9c..92f22ac41 100644 --- a/src/Element.cc +++ b/src/Element.cc @@ -16,6 +16,7 @@ */ #include +#include #include #include diff --git a/src/FrameSemantics.cc b/src/FrameSemantics.cc index 40df3c4c4..ee6e56739 100644 --- a/src/FrameSemantics.cc +++ b/src/FrameSemantics.cc @@ -15,6 +15,7 @@ * */ #include +#include #include #include #include diff --git a/src/SDF.cc b/src/SDF.cc index 04fc53748..af1965d79 100644 --- a/src/SDF.cc +++ b/src/SDF.cc @@ -17,10 +17,11 @@ #include #include -#include #include +#include #include #include +#include #include #include From eef861d2381ecfb2ba5b50af8955184f8fc51cb4 Mon Sep 17 00:00:00 2001 From: Jeremy Nimmer Date: Mon, 6 Jan 2025 01:42:23 -0800 Subject: [PATCH 2/2] Add non-const overload for Root::Model() getter (#1524) * Add non-const overload for Root::Model() getter This is consistent with the getters for World which also overload on const-ness. Co-authored-by: Sean Curtis Signed-off-by: Jeremy Nimmer --- include/sdf/Root.hh | 5 +++++ python/src/sdf/pyRoot.cc | 2 +- src/Root.cc | 6 ++++++ src/Root_TEST.cc | 23 ++++++++++++++++------- 4 files changed, 28 insertions(+), 8 deletions(-) diff --git a/include/sdf/Root.hh b/include/sdf/Root.hh index 6ad91ba63..87a4b86b7 100644 --- a/include/sdf/Root.hh +++ b/include/sdf/Root.hh @@ -170,6 +170,11 @@ namespace sdf /// \return A pointer to the model, nullptr if it doesn't exist public: const sdf::Model *Model() const; + /// \brief Get a mutable pointer to the model object if it exists. + /// + /// \return A pointer to the model; nullptr if it doesn't exist. + public: sdf::Model *Model(); + /// \brief Set the model object. This will override any existing model, /// actor, and light object. /// \param[in] _model The model to use. diff --git a/python/src/sdf/pyRoot.cc b/python/src/sdf/pyRoot.cc index af94e78b5..d7ca58044 100644 --- a/python/src/sdf/pyRoot.cc +++ b/python/src/sdf/pyRoot.cc @@ -83,7 +83,7 @@ void defineRoot(pybind11::object module) "Get a world based on an index.") .def("world_name_exists", &sdf::Root::WorldNameExists, "Get whether a world name exists.") - .def("model", &sdf::Root::Model, + .def("model", pybind11::overload_cast<>(&sdf::Root::Model), pybind11::return_value_policy::reference_internal, "Get a model object if it exists.") .def("set_model", &sdf::Root::SetModel, diff --git a/src/Root.cc b/src/Root.cc index b6e412d2f..2ff9a88dd 100644 --- a/src/Root.cc +++ b/src/Root.cc @@ -475,6 +475,12 @@ const Model *Root::Model() const return std::get_if(&this->dataPtr->modelLightOrActor); } +///////////////////////////////////////////////// +Model *Root::Model() +{ + return std::get_if(&this->dataPtr->modelLightOrActor); +} + ///////////////////////////////////////////////// void Root::SetModel(const sdf::Model &_model) { diff --git a/src/Root_TEST.cc b/src/Root_TEST.cc index 629d6b7e7..6af3cfde5 100644 --- a/src/Root_TEST.cc +++ b/src/Root_TEST.cc @@ -46,6 +46,9 @@ TEST(DOMRoot, Construction) EXPECT_EQ(nullptr, root.Model()); EXPECT_EQ(nullptr, root.Light()); EXPECT_EQ(nullptr, root.Actor()); + + const sdf::Root &const_root = root; + EXPECT_EQ(nullptr, const_root.Model()); } ///////////////////////////////////////////////// @@ -424,6 +427,7 @@ TEST(DOMRoot, ToElementEmpty) TEST(DOMRoot, ToElementModel) { sdf::Root root; + const sdf::Root &const_root = root; sdf::Actor actor1; actor1.SetName("actor1"); @@ -438,6 +442,7 @@ TEST(DOMRoot, ToElementModel) root.SetModel(model1); ASSERT_NE(nullptr, root.Model()); + ASSERT_NE(nullptr, const_root.Model()); ASSERT_EQ(nullptr, root.Light()); ASSERT_EQ(nullptr, root.Actor()); EXPECT_EQ(0u, root.WorldCount()); @@ -447,12 +452,15 @@ TEST(DOMRoot, ToElementModel) ASSERT_NE(nullptr, elem); sdf::Root root2; + const sdf::Root &const_root2 = root2; root2.LoadSdfString(elem->ToString("")); EXPECT_EQ(SDF_VERSION, root2.Version()); ASSERT_NE(nullptr, root2.Model()); + ASSERT_NE(nullptr, const_root2.Model()); EXPECT_EQ("model1", root2.Model()->Name()); + EXPECT_EQ("model1", const_root2.Model()->Name()); ASSERT_EQ(nullptr, root2.Actor()); ASSERT_EQ(nullptr, root2.Light()); @@ -651,24 +659,25 @@ TEST(DOMRoot, CopyConstructor) TEST(DOMRoot, WorldByName) { sdf::Root root; - EXPECT_EQ(0u, root.WorldCount()); + const sdf::Root &const_root = root; + EXPECT_EQ(0u, const_root.WorldCount()); sdf::World world; world.SetName("world1"); EXPECT_TRUE(root.AddWorld(world).empty()); - EXPECT_EQ(1u, root.WorldCount()); + EXPECT_EQ(1u, const_root.WorldCount()); - EXPECT_TRUE(root.WorldNameExists("world1")); - const sdf::World *wPtr = root.WorldByName("world1"); - EXPECT_NE(nullptr, wPtr); + EXPECT_TRUE(const_root.WorldNameExists("world1")); + EXPECT_NE(nullptr, root.WorldByName("world1")); + EXPECT_NE(nullptr, const_root.WorldByName("world1")); // Modify the world sdf::World *w = root.WorldByName("world1"); ASSERT_NE(nullptr, w); EXPECT_EQ("world1", w->Name()); w->SetName("world2"); - ASSERT_TRUE(root.WorldNameExists("world2")); - EXPECT_EQ("world2", root.WorldByName("world2")->Name()); + ASSERT_TRUE(const_root.WorldNameExists("world2")); + EXPECT_EQ("world2", const_root.WorldByName("world2")->Name()); } /////////////////////////////////////////////////