From 2f2fa3b03124e2b26edad59ae03f85386629487d Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Wed, 1 Mar 2017 22:03:48 -0500 Subject: [PATCH] Update Ghoul repository to have the factory return unique_ptrs Adjust accordingly --- ext/ghoul | 2 +- include/openspace/rendering/renderable.h | 2 +- include/openspace/rendering/screenspacerenderable.h | 4 +++- include/openspace/scene/rotation.h | 4 +++- include/openspace/scene/scale.h | 2 +- include/openspace/scene/scenegraphnode.h | 4 ++-- include/openspace/scene/translation.h | 4 +++- modules/base/rendering/modelgeometry.cpp | 6 ++++-- modules/base/rendering/modelgeometry.h | 4 +++- modules/base/rendering/renderablemodel.cpp | 3 +-- modules/base/rendering/renderablemodel.h | 4 +++- .../globebrowsing/tile/tileprovider/tileprovider.cpp | 4 ++-- .../globebrowsing/tile/tileprovider/tileprovider.h | 2 +- .../tile/tileprovider/tileproviderbyindex.cpp | 10 +++++----- .../tile/tileprovider/tileproviderbylevel.cpp | 7 ++++--- modules/newhorizons/util/decoder.cpp | 4 ++-- modules/space/rendering/planetgeometry.cpp | 4 ++-- modules/space/rendering/planetgeometry.h | 4 +++- modules/space/rendering/renderableplanet.cpp | 4 ++-- modules/space/rendering/renderableplanet.h | 5 +++-- src/rendering/renderable.cpp | 6 ++++-- src/rendering/screenspacerenderable.cpp | 7 +++---- src/scene/rotation.cpp | 4 ++-- src/scene/scale.cpp | 4 ++-- src/scene/scenegraphnode.cpp | 11 +++++------ src/scene/translation.cpp | 6 ++++-- 26 files changed, 69 insertions(+), 52 deletions(-) diff --git a/ext/ghoul b/ext/ghoul index 7279b3eb92..abf36fd08d 160000 --- a/ext/ghoul +++ b/ext/ghoul @@ -1 +1 @@ -Subproject commit 7279b3eb924d1d90bf4dfa46e2f44a9a6f3a90a5 +Subproject commit abf36fd08d2405ccede17ff1c90d0a0c21b0a162 diff --git a/include/openspace/rendering/renderable.h b/include/openspace/rendering/renderable.h index ba11cec5f0..c4b1e731c2 100644 --- a/include/openspace/rendering/renderable.h +++ b/include/openspace/rendering/renderable.h @@ -59,7 +59,7 @@ public: Overlay = 8 }; - static Renderable* createFromDictionary(const ghoul::Dictionary& dictionary); + static std::unique_ptr createFromDictionary(const ghoul::Dictionary& dictionary); // constructors & destructor Renderable(); diff --git a/include/openspace/rendering/screenspacerenderable.h b/include/openspace/rendering/screenspacerenderable.h index 74eff1eaf5..bd9b4ebafc 100644 --- a/include/openspace/rendering/screenspacerenderable.h +++ b/include/openspace/rendering/screenspacerenderable.h @@ -36,6 +36,8 @@ #include #include +#include + namespace openspace { namespace documentation { struct Documentation; } @@ -49,7 +51,7 @@ namespace documentation { struct Documentation; } */ class ScreenSpaceRenderable : public properties::PropertyOwner { public: - static ScreenSpaceRenderable* createFromDictionary( + static std::unique_ptr createFromDictionary( const ghoul::Dictionary& dictionary); ScreenSpaceRenderable(const ghoul::Dictionary& dictionary); diff --git a/include/openspace/scene/rotation.h b/include/openspace/scene/rotation.h index be0dcdfca0..140f95d9df 100644 --- a/include/openspace/scene/rotation.h +++ b/include/openspace/scene/rotation.h @@ -29,6 +29,8 @@ #include +#include + namespace ghoul { class Dictionary; } namespace openspace { @@ -37,7 +39,7 @@ namespace documentation { struct Documentation; } class Rotation : public properties::PropertyOwner { public: - static Rotation* createFromDictionary(const ghoul::Dictionary& dictionary); + static std::unique_ptr createFromDictionary(const ghoul::Dictionary& dictionary); Rotation(const ghoul::Dictionary& dictionary); virtual ~Rotation(); diff --git a/include/openspace/scene/scale.h b/include/openspace/scene/scale.h index c272f3c3a9..8b80ed18f2 100644 --- a/include/openspace/scene/scale.h +++ b/include/openspace/scene/scale.h @@ -36,7 +36,7 @@ namespace documentation { struct Documentation; }; class Scale : public properties::PropertyOwner { public: - static Scale* createFromDictionary(const ghoul::Dictionary& dictionary); + static std::unique_ptr createFromDictionary(const ghoul::Dictionary& dictionary); virtual ~Scale(); virtual bool initialize(); diff --git a/include/openspace/scene/scenegraphnode.h b/include/openspace/scene/scenegraphnode.h index 092b04955c..1acce2442e 100644 --- a/include/openspace/scene/scenegraphnode.h +++ b/include/openspace/scene/scenegraphnode.h @@ -97,7 +97,7 @@ public: const PerformanceRecord& performanceRecord() const { return _performanceRecord; } - void setRenderable(Renderable* renderable); + void setRenderable(std::unique_ptr renderable); const Renderable* renderable() const; Renderable* renderable(); @@ -115,7 +115,7 @@ private: PerformanceRecord _performanceRecord; - Renderable* _renderable; + std::unique_ptr _renderable; bool _renderableVisible; bool _boundingSphereVisible; diff --git a/include/openspace/scene/translation.h b/include/openspace/scene/translation.h index 4337f12035..4e3a631f7c 100644 --- a/include/openspace/scene/translation.h +++ b/include/openspace/scene/translation.h @@ -29,6 +29,8 @@ #include +#include + namespace ghoul { class Dictionary; } namespace openspace { @@ -37,7 +39,7 @@ namespace documentation { struct Documentation; } class Translation : public properties::PropertyOwner { public: - static Translation* createFromDictionary(const ghoul::Dictionary& dictionary); + static std::unique_ptr createFromDictionary(const ghoul::Dictionary& dictionary); virtual ~Translation(); virtual bool initialize(); diff --git a/modules/base/rendering/modelgeometry.cpp b/modules/base/rendering/modelgeometry.cpp index 55463821d0..8ca084be67 100644 --- a/modules/base/rendering/modelgeometry.cpp +++ b/modules/base/rendering/modelgeometry.cpp @@ -68,7 +68,9 @@ documentation:: Documentation ModelGeometry::Documentation() { } -ModelGeometry* ModelGeometry::createFromDictionary(const ghoul::Dictionary& dictionary) { +std::unique_ptr ModelGeometry::createFromDictionary( + const ghoul::Dictionary& dictionary) +{ if (!dictionary.hasKeyAndValue(keyType)) { throw ghoul::RuntimeError("Dictionary did not contain a key 'Type'"); } @@ -76,7 +78,7 @@ ModelGeometry* ModelGeometry::createFromDictionary(const ghoul::Dictionary& dict std::string geometryType = dictionary.value(keyType); auto factory = FactoryManager::ref().factory(); - ModelGeometry* result = factory->create(geometryType, dictionary); + std::unique_ptr result = factory->create(geometryType, dictionary); if (result == nullptr) { throw ghoul::RuntimeError( "Failed to create a ModelGeometry object of type '" + geometryType + "'" diff --git a/modules/base/rendering/modelgeometry.h b/modules/base/rendering/modelgeometry.h index e878eecf77..508b523101 100644 --- a/modules/base/rendering/modelgeometry.h +++ b/modules/base/rendering/modelgeometry.h @@ -38,7 +38,9 @@ namespace modelgeometry { class ModelGeometry : public properties::PropertyOwner { public: - static ModelGeometry* createFromDictionary(const ghoul::Dictionary& dictionary); + static std::unique_ptr createFromDictionary( + const ghoul::Dictionary& dictionary + ); struct Vertex { GLfloat location[4]; diff --git a/modules/base/rendering/renderablemodel.cpp b/modules/base/rendering/renderablemodel.cpp index 7ce5288317..1adca3f960 100644 --- a/modules/base/rendering/renderablemodel.cpp +++ b/modules/base/rendering/renderablemodel.cpp @@ -85,7 +85,7 @@ RenderableModel::RenderableModel(const ghoul::Dictionary& dictionary) if (success) _colorTexturePath = absPath(texturePath); - addPropertySubOwner(_geometry); + addPropertySubOwner(_geometry.get()); addProperty(_colorTexturePath); _colorTexturePath.onChange(std::bind(&RenderableModel::loadTexture, this)); @@ -153,7 +153,6 @@ bool RenderableModel::initialize() { bool RenderableModel::deinitialize() { if (_geometry) { _geometry->deinitialize(); - delete _geometry; _geometry = nullptr; } _texture = nullptr; diff --git a/modules/base/rendering/renderablemodel.h b/modules/base/rendering/renderablemodel.h index c885b8dd84..153964d60a 100644 --- a/modules/base/rendering/renderablemodel.h +++ b/modules/base/rendering/renderablemodel.h @@ -36,6 +36,8 @@ #include #include +#include + namespace openspace { namespace modelgeometry { @@ -65,7 +67,7 @@ private: std::unique_ptr _programObject; std::unique_ptr _texture; - modelgeometry::ModelGeometry* _geometry; + std::unique_ptr _geometry; glm::dmat3 _modelTransform; diff --git a/modules/globebrowsing/tile/tileprovider/tileprovider.cpp b/modules/globebrowsing/tile/tileprovider/tileprovider.cpp index f1a3ee74d5..bf44816fe3 100644 --- a/modules/globebrowsing/tile/tileprovider/tileprovider.cpp +++ b/modules/globebrowsing/tile/tileprovider/tileprovider.cpp @@ -41,11 +41,11 @@ namespace openspace { namespace globebrowsing { namespace tileprovider { -TileProvider* TileProvider::createFromDictionary(const ghoul::Dictionary& dictionary) { +std::unique_ptr TileProvider::createFromDictionary(const ghoul::Dictionary& dictionary) { std::string type = "LRUCaching"; dictionary.getValue(KeyType, type); auto factory = FactoryManager::ref().factory(); - TileProvider* result = factory->create(type, dictionary); + std::unique_ptr result = factory->create(type, dictionary); if (result == nullptr) { LERROR("Failed creating TileProvider of type '" << type << "'"); diff --git a/modules/globebrowsing/tile/tileprovider/tileprovider.h b/modules/globebrowsing/tile/tileprovider/tileprovider.h index 9d26165b4d..2efc34b9bb 100644 --- a/modules/globebrowsing/tile/tileprovider/tileprovider.h +++ b/modules/globebrowsing/tile/tileprovider/tileprovider.h @@ -47,7 +47,7 @@ public: * define a key specifying what implementation of TileProvider * to be instantiated. */ - static TileProvider* createFromDictionary(const ghoul::Dictionary& dictionary); + static std::unique_ptr createFromDictionary(const ghoul::Dictionary& dictionary); /** * Empty default constructor diff --git a/modules/globebrowsing/tile/tileprovider/tileproviderbyindex.cpp b/modules/globebrowsing/tile/tileprovider/tileproviderbyindex.cpp index edebf63d74..b785d1c12f 100644 --- a/modules/globebrowsing/tile/tileprovider/tileproviderbyindex.cpp +++ b/modules/globebrowsing/tile/tileprovider/tileproviderbyindex.cpp @@ -43,11 +43,10 @@ TileProviderByIndex::TileProviderByIndex(const ghoul::Dictionary& dictionary) { ghoul::Dictionary defaultProviderDict = dictionary.value( KeyDefaultProvider ); - TileProvider* defaultProvider = TileProvider::createFromDictionary( + _defaultTileProvider = TileProvider::createFromDictionary( defaultProviderDict ); - _defaultTileProvider = std::shared_ptr(defaultProvider); - + ghoul::Dictionary indexProvidersDict = dictionary.value( KeyProviders ); @@ -64,8 +63,9 @@ TileProviderByIndex::TileProviderByIndex(const ghoul::Dictionary& dictionary) { ); TileIndex tileIndex(tileIndexDict); - TileProvider* tileProvider = TileProvider::createFromDictionary(providerDict); - std::shared_ptr stp = std::shared_ptr(tileProvider); + std::shared_ptr stp = TileProvider::createFromDictionary( + providerDict + ); TileIndex::TileHashKey key = tileIndex.hashKey(); _tileProviderMap.insert(std::make_pair(key, stp)); } diff --git a/modules/globebrowsing/tile/tileprovider/tileproviderbylevel.cpp b/modules/globebrowsing/tile/tileprovider/tileproviderbylevel.cpp index 37a56007f8..50553e8caa 100644 --- a/modules/globebrowsing/tile/tileprovider/tileproviderbylevel.cpp +++ b/modules/globebrowsing/tile/tileprovider/tileproviderbylevel.cpp @@ -61,9 +61,10 @@ TileProviderByLevel::TileProviderByLevel(const ghoul::Dictionary& dictionary) { ); } - TileProvider* tileProvider = TileProvider::createFromDictionary(providerDict); - _levelTileProviders.push_back(std::shared_ptr(tileProvider)); - + _levelTileProviders.push_back( + std::shared_ptr(TileProvider::createFromDictionary(providerDict)) + ); + // Ensure we can represent the max level if(_providerIndices.size() < maxLevel){ _providerIndices.resize(maxLevel+1, -1); diff --git a/modules/newhorizons/util/decoder.cpp b/modules/newhorizons/util/decoder.cpp index 58c23dc138..e820da47ba 100644 --- a/modules/newhorizons/util/decoder.cpp +++ b/modules/newhorizons/util/decoder.cpp @@ -35,7 +35,7 @@ std::unique_ptr Decoder::createFromDictionary( const ghoul::Dictionary& dictionary, const std::string& type) { ghoul::TemplateFactory* factory = FactoryManager::ref().factory(); - Decoder* result = factory->create(type, dictionary); + std::unique_ptr result = factory->create(type, dictionary); if (result == nullptr) { throw ghoul::RuntimeError( @@ -43,7 +43,7 @@ std::unique_ptr Decoder::createFromDictionary( "Decoder" ); } - return std::unique_ptr(result); + return result; } Decoder::~Decoder() {} diff --git a/modules/space/rendering/planetgeometry.cpp b/modules/space/rendering/planetgeometry.cpp index eb62be82bf..f847dc561c 100644 --- a/modules/space/rendering/planetgeometry.cpp +++ b/modules/space/rendering/planetgeometry.cpp @@ -53,7 +53,7 @@ documentation::Documentation PlanetGeometry::Documentation() { }; } -PlanetGeometry* PlanetGeometry::createFromDictionary(const ghoul::Dictionary& dictionary) +std::unique_ptr PlanetGeometry::createFromDictionary(const ghoul::Dictionary& dictionary) { documentation::testSpecificationAndThrow( Documentation(), @@ -64,7 +64,7 @@ PlanetGeometry* PlanetGeometry::createFromDictionary(const ghoul::Dictionary& di std::string geometryType = dictionary.value(KeyType); auto factory = FactoryManager::ref().factory(); - PlanetGeometry* result = factory->create(geometryType, dictionary); + std::unique_ptr result = factory->create(geometryType, dictionary); if (result == nullptr) { throw ghoul::RuntimeError( "Failed to create a PlanetGeometry object of type '" + geometryType + "'" diff --git a/modules/space/rendering/planetgeometry.h b/modules/space/rendering/planetgeometry.h index 6e495f882b..eae02b77d1 100644 --- a/modules/space/rendering/planetgeometry.h +++ b/modules/space/rendering/planetgeometry.h @@ -36,7 +36,9 @@ namespace planetgeometry { class PlanetGeometry : public properties::PropertyOwner { public: - static PlanetGeometry* createFromDictionary(const ghoul::Dictionary& dictionary); + static std::unique_ptr createFromDictionary( + const ghoul::Dictionary& dictionary + ); PlanetGeometry(); virtual ~PlanetGeometry(); diff --git a/modules/space/rendering/renderableplanet.cpp b/modules/space/rendering/renderableplanet.cpp index 2b860fb361..cceedf0122 100644 --- a/modules/space/rendering/renderableplanet.cpp +++ b/modules/space/rendering/renderableplanet.cpp @@ -125,7 +125,7 @@ RenderablePlanet::RenderablePlanet(const ghoul::Dictionary& dictionary) _heightMapTexturePath = absPath(heightMapTexturePath); } - addPropertySubOwner(_geometry); + addPropertySubOwner(_geometry.get()); addProperty(_colorTexturePath); _colorTexturePath.onChange(std::bind(&RenderablePlanet::loadTexture, this)); @@ -309,7 +309,7 @@ bool RenderablePlanet::initialize() { bool RenderablePlanet::deinitialize() { if(_geometry) { _geometry->deinitialize(); - delete _geometry; + _geometry = nullptr; } RenderEngine& renderEngine = OsEng.renderEngine(); diff --git a/modules/space/rendering/renderableplanet.h b/modules/space/rendering/renderableplanet.h index 9c2bd4d08a..88fd81b7ad 100644 --- a/modules/space/rendering/renderableplanet.h +++ b/modules/space/rendering/renderableplanet.h @@ -35,8 +35,9 @@ #include -#include +#include #include +#include namespace ghoul { namespace opengl { @@ -93,7 +94,7 @@ private: properties::FloatProperty _heightExaggeration; - planetgeometry::PlanetGeometry* _geometry; + std::unique_ptr _geometry; properties::BoolProperty _performShading; properties::IntProperty _rotation; float _alpha; diff --git a/src/rendering/renderable.cpp b/src/rendering/renderable.cpp index cf9fb6ac42..6795842a01 100644 --- a/src/rendering/renderable.cpp +++ b/src/rendering/renderable.cpp @@ -65,7 +65,9 @@ documentation::Documentation Renderable::Documentation() { }; } -Renderable* Renderable::createFromDictionary(const ghoul::Dictionary& dictionary) { +std::unique_ptr Renderable::createFromDictionary( + const ghoul::Dictionary& dictionary) +{ // The name is passed down from the SceneGraphNode std::string name; bool success = dictionary.getValue(SceneGraphNode::KeyName, name); @@ -76,7 +78,7 @@ Renderable* Renderable::createFromDictionary(const ghoul::Dictionary& dictionary std::string renderableType = dictionary.value(KeyType); auto factory = FactoryManager::ref().factory(); - Renderable* result = factory->create(renderableType, dictionary); + std::unique_ptr result = factory->create(renderableType, dictionary); if (result == nullptr) { LERROR("Failed to create a Renderable object of type '" << renderableType << "'"); return nullptr; diff --git a/src/rendering/screenspacerenderable.cpp b/src/rendering/screenspacerenderable.cpp index 1acaf704d6..23b0885dd9 100644 --- a/src/rendering/screenspacerenderable.cpp +++ b/src/rendering/screenspacerenderable.cpp @@ -73,8 +73,8 @@ documentation::Documentation ScreenSpaceRenderable::Documentation() { }; } -ScreenSpaceRenderable* ScreenSpaceRenderable::createFromDictionary( - const ghoul::Dictionary& dictionary) +std::unique_ptr ScreenSpaceRenderable::createFromDictionary( + const ghoul::Dictionary& dictionary) { documentation::testSpecificationAndThrow( Documentation(), @@ -85,7 +85,7 @@ ScreenSpaceRenderable* ScreenSpaceRenderable::createFromDictionary( std::string renderableType = dictionary.value(KeyType); auto factory = FactoryManager::ref().factory(); - ScreenSpaceRenderable* result = factory->create(renderableType, dictionary); + std::unique_ptr result = factory->create(renderableType, dictionary); if (result == nullptr) { LERROR("Failed to create a ScreenSpaceRenderable object of type '" << renderableType << "'" @@ -96,7 +96,6 @@ ScreenSpaceRenderable* ScreenSpaceRenderable::createFromDictionary( return result; } - ScreenSpaceRenderable::ScreenSpaceRenderable(const ghoul::Dictionary& dictionary) : _enabled("enabled", "Is Enabled", true) , _useFlatScreen("flatScreen", "Flat Screen", true) diff --git a/src/scene/rotation.cpp b/src/scene/rotation.cpp index c064cd96c4..2db6ee1068 100644 --- a/src/scene/rotation.cpp +++ b/src/scene/rotation.cpp @@ -58,12 +58,12 @@ documentation::Documentation Rotation::Documentation() { }; } -Rotation* Rotation::createFromDictionary(const ghoul::Dictionary& dictionary) { +std::unique_ptr Rotation::createFromDictionary(const ghoul::Dictionary& dictionary) { documentation::testSpecificationAndThrow(Documentation(), dictionary, "Rotation"); std::string rotationType = dictionary.value(KeyType); auto factory = FactoryManager::ref().factory(); - Rotation* result = factory->create(rotationType, dictionary); + std::unique_ptr result = factory->create(rotationType, dictionary); if (result == nullptr) { LERROR("Failed creating Rotation object of type '" << rotationType << "'"); return nullptr; diff --git a/src/scene/scale.cpp b/src/scene/scale.cpp index 9da2f23d4e..5f2e7dfccc 100644 --- a/src/scene/scale.cpp +++ b/src/scene/scale.cpp @@ -60,13 +60,13 @@ documentation::Documentation Scale::Documentation() { } -Scale* Scale::createFromDictionary(const ghoul::Dictionary& dictionary) { +std::unique_ptr Scale::createFromDictionary(const ghoul::Dictionary& dictionary) { documentation::testSpecificationAndThrow(Documentation(), dictionary, "Scale"); std::string scaleType = dictionary.value(KeyType); auto factory = FactoryManager::ref().factory(); - Scale* result = factory->create(scaleType, dictionary); + std::unique_ptr result = factory->create(scaleType, dictionary); result->setName("Scale"); if (result == nullptr) { LERROR("Failed creating Scale object of type '" << scaleType << "'"); diff --git a/src/scene/scenegraphnode.cpp b/src/scene/scenegraphnode.cpp index 8aaa29e3bd..d27c0164f9 100644 --- a/src/scene/scenegraphnode.cpp +++ b/src/scene/scenegraphnode.cpp @@ -90,7 +90,7 @@ SceneGraphNode* SceneGraphNode::createFromDictionary(const ghoul::Dictionary& di delete result; return nullptr; } - result->addPropertySubOwner(result->_renderable); + result->addPropertySubOwner(result->_renderable.get()); LDEBUG("Successfully created renderable for '" << result->name() << "'"); } @@ -186,7 +186,6 @@ bool SceneGraphNode::deinitialize() { if (_renderable) { _renderable->deinitialize(); - delete _renderable; _renderable = nullptr; } _children.clear(); @@ -499,17 +498,17 @@ PowerScaledScalar SceneGraphNode::boundingSphere() const{ } // renderable -void SceneGraphNode::setRenderable(Renderable* renderable) { - _renderable = renderable; +void SceneGraphNode::setRenderable(std::unique_ptr renderable) { + _renderable = std::move(renderable); } const Renderable* SceneGraphNode::renderable() const { - return _renderable; + return _renderable.get(); } Renderable* SceneGraphNode::renderable() { - return _renderable; + return _renderable.get(); } // private helper methods diff --git a/src/scene/translation.cpp b/src/scene/translation.cpp index 4452d566da..fd54a65e93 100644 --- a/src/scene/translation.cpp +++ b/src/scene/translation.cpp @@ -58,7 +58,9 @@ documentation::Documentation Translation::Documentation() { }; } -Translation* Translation::createFromDictionary(const ghoul::Dictionary& dictionary) { +std::unique_ptr Translation::createFromDictionary( + const ghoul::Dictionary& dictionary) +{ if (!dictionary.hasValue(KeyType)) { LERROR("Translation did not have key '" << KeyType << "'"); return nullptr; @@ -68,7 +70,7 @@ Translation* Translation::createFromDictionary(const ghoul::Dictionary& dictiona dictionary.getValue(KeyType, translationType); ghoul::TemplateFactory* factory = FactoryManager::ref().factory(); - Translation* result = factory->create(translationType, dictionary); + std::unique_ptr result = factory->create(translationType, dictionary); result->setName("Translation"); if (result == nullptr) { LERROR("Failed creating Translation object of type '" << translationType << "'");