Use unique_ptr to indicate ownership in cel3ds

src/cel3ds/3dsmodel.cpp

@@ -7,6 +7,8 @@
 // as published by the Free Software Foundation; either version 2
 // of the License, or (at your option) any later version.
 
+#include <utility>
+
 #include "3dsmodel.h"
 
 M3DColor::M3DColor() :
@@ -90,11 +92,6 @@ void M3DMaterial::setTextureMap(const std::string& _texmap)
 }
 
 
-M3DTriangleMesh::M3DTriangleMesh()
-{
-    matrix = Eigen::Matrix4f::Identity();
-}
-
 Eigen::Matrix4f M3DTriangleMesh::getMatrix() const
 {
     return matrix;
@@ -112,7 +109,7 @@ Eigen::Vector3f M3DTriangleMesh::getVertex(std::uint16_t n) const
 
 std::uint16_t M3DTriangleMesh::getVertexCount() const
 {
-    return (std::uint16_t) (points.size());
+    return static_cast<std::uint16_t>(points.size());
 }
 
 void M3DTriangleMesh::addVertex(const Eigen::Vector3f& p)
@@ -127,7 +124,7 @@ Eigen::Vector2f M3DTriangleMesh::getTexCoord(std::uint16_t n) const
 
 std::uint16_t M3DTriangleMesh::getTexCoordCount() const
 {
-    return (uint16_t) (texCoords.size());
+    return static_cast<std::uint16_t>(texCoords.size());
 }
 
 void M3DTriangleMesh::addTexCoord(const Eigen::Vector2f& p)
@@ -137,7 +134,7 @@ void M3DTriangleMesh::addTexCoord(const Eigen::Vector2f& p)
 
 void M3DTriangleMesh::getFace(std::uint16_t n, std::uint16_t& v0, std::uint16_t& v1, std::uint16_t& v2) const
 {
-    int m = (int) n * 3;
+    int m = static_cast<int>(n) * 3;
     v0 = faces[m];
     v1 = faces[m + 1];
     v2 = faces[m + 2];
@@ -145,7 +142,7 @@ void M3DTriangleMesh::getFace(std::uint16_t n, std::uint16_t& v0, std::uint16_t&
 
 std::uint16_t M3DTriangleMesh::getFaceCount() const
 {
-    return (std::uint16_t) (faces.size() / 3);
+    return static_cast<std::uint16_t>(faces.size() / 3);
 }
 
 void M3DTriangleMesh::addFace(std::uint16_t v0, std::uint16_t v1, std::uint16_t v2)
@@ -155,7 +152,7 @@ void M3DTriangleMesh::addFace(std::uint16_t v0, std::uint16_t v1, std::uint16_t
     faces.push_back(v2);
 }
 
-std::uint32_t M3DTriangleMesh::getSmoothingGroups(uint16_t face) const
+std::uint32_t M3DTriangleMesh::getSmoothingGroups(std::uint16_t face) const
 {
     return face < smoothingGroups.size() ? smoothingGroups[face] : 0;
 }
@@ -167,17 +164,17 @@ void M3DTriangleMesh::addSmoothingGroups(std::uint32_t smGroups)
 
 std::uint16_t M3DTriangleMesh::getSmoothingGroupCount() const
 {
-    return (std::uint16_t) (smoothingGroups.size());
+    return static_cast<std::uint16_t>(smoothingGroups.size());
 }
 
-void M3DTriangleMesh::addMeshMaterialGroup(M3DMeshMaterialGroup* matGroup)
+void M3DTriangleMesh::addMeshMaterialGroup(std::unique_ptr<M3DMeshMaterialGroup> matGroup)
 {
-    meshMaterialGroups.push_back(matGroup);
+    meshMaterialGroups.push_back(std::move(matGroup));
 }
 
 M3DMeshMaterialGroup* M3DTriangleMesh::getMeshMaterialGroup(std::uint32_t index) const
 {
-    return meshMaterialGroups[index];
+    return index < meshMaterialGroups.size() ? meshMaterialGroups[index].get() : nullptr;
 }
 
 std::uint32_t M3DTriangleMesh::getMeshMaterialGroupCount() const
@@ -186,15 +183,9 @@ std::uint32_t M3DTriangleMesh::getMeshMaterialGroupCount() const
 }
 
 
-M3DModel::~M3DModel()
-{
-    for (auto triMesh : triMeshes)
-        delete triMesh;
-}
-
 M3DTriangleMesh* M3DModel::getTriMesh(std::uint32_t n)
 {
-    return n < triMeshes.size() ? triMeshes[n] : nullptr;
+    return n < triMeshes.size() ? triMeshes[n].get() : nullptr;
 }
 
 std::uint32_t M3DModel::getTriMeshCount()
@@ -202,9 +193,9 @@ std::uint32_t M3DModel::getTriMeshCount()
     return triMeshes.size();
 }
 
-void M3DModel::addTriMesh(M3DTriangleMesh* triMesh)
+void M3DModel::addTriMesh(std::unique_ptr<M3DTriangleMesh> triMesh)
 {
-    triMeshes.push_back(triMesh);
+    triMeshes.push_back(std::move(triMesh));
 }
 
 void M3DModel::setName(const std::string& _name)
@@ -218,17 +209,9 @@ const std::string M3DModel::getName() const
 }
 
 
-M3DScene::~M3DScene()
-{
-    for (auto model : models)
-        delete model;
-    for (auto material : materials)
-        delete material;
-}
-
 M3DModel* M3DScene::getModel(std::uint32_t n) const
 {
-    return n < models.size() ? models[n] : nullptr;
+    return n < models.size() ? models[n].get() : nullptr;
 }
 
 std::uint32_t M3DScene::getModelCount() const
@@ -236,14 +219,14 @@ std::uint32_t M3DScene::getModelCount() const
     return models.size();
 }
 
-void M3DScene::addModel(M3DModel* model)
+void M3DScene::addModel(std::unique_ptr<M3DModel> model)
 {
-    models.push_back(model);
+    models.push_back(std::move(model));
 }
 
 M3DMaterial* M3DScene::getMaterial(std::uint32_t n) const
 {
-    return n < materials.size() ? materials[n] : nullptr;
+    return n < materials.size() ? materials[n].get() : nullptr;
 }
 
 std::uint32_t M3DScene::getMaterialCount() const
@@ -251,9 +234,9 @@ std::uint32_t M3DScene::getMaterialCount() const
     return materials.size();
 }
 
-void M3DScene::addMaterial(M3DMaterial* material)
+void M3DScene::addMaterial(std::unique_ptr<M3DMaterial> material)
 {
-    materials.push_back(material);
+    materials.push_back(std::move(material));
 }
 
 M3DColor M3DScene::getBackgroundColor() const

src/cel3ds/3dsmodel.h

@@ -10,6 +10,7 @@
 #pragma once
 
 #include <cstdint>
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -29,8 +30,6 @@ class M3DColor
 class M3DMaterial
 {
  public:
-    M3DMaterial() = default;
-
     std::string getName() const;
     void setName(std::string);
     M3DColor getAmbientColor() const;
@@ -47,8 +46,8 @@ class M3DMaterial
     void setTextureMap(const std::string&);
 
  private:
-    std::string name;
+    std::string name{};
-    std::string texmap;
+    std::string texmap{};
     M3DColor ambient{ 0, 0, 0 };
     M3DColor diffuse{ 0, 0, 0 };
     M3DColor specular{ 0, 0, 0 };
@@ -70,9 +69,6 @@ class M3DTriangleMesh
  public:
     EIGEN_MAKE_ALIGNED_OPERATOR_NEW
 
-    M3DTriangleMesh();
-    ~M3DTriangleMesh() = default;
-
     Eigen::Matrix4f getMatrix() const;
     void setMatrix(const Eigen::Matrix4f&);
 
@@ -92,57 +88,51 @@ class M3DTriangleMesh
     std::uint32_t getSmoothingGroups(std::uint16_t) const;
     std::uint16_t getSmoothingGroupCount() const;
 
-    void addMeshMaterialGroup(M3DMeshMaterialGroup* matGroup);
+    void addMeshMaterialGroup(std::unique_ptr<M3DMeshMaterialGroup> matGroup);
     M3DMeshMaterialGroup* getMeshMaterialGroup(std::uint32_t) const;
     std::uint32_t getMeshMaterialGroupCount() const;
 
  private:
-    std::vector<Eigen::Vector3f> points;
+    std::vector<Eigen::Vector3f> points{};
-    std::vector<Eigen::Vector2f, Eigen::aligned_allocator<Eigen::Vector2f>> texCoords;
+    std::vector<Eigen::Vector2f, Eigen::aligned_allocator<Eigen::Vector2f>> texCoords{};
-    std::vector<std::uint16_t> faces;
+    std::vector<std::uint16_t> faces{};
-    std::vector<std::uint32_t> smoothingGroups;
+    std::vector<std::uint32_t> smoothingGroups{};
-    std::vector<M3DMeshMaterialGroup*> meshMaterialGroups;
+    std::vector<std::unique_ptr<M3DMeshMaterialGroup>> meshMaterialGroups{};
-    Eigen::Matrix4f matrix;
+    Eigen::Matrix4f matrix{ Eigen::Matrix4f::Identity() };
 };
 
 
 class M3DModel
 {
  public:
-    M3DModel() = default;
+    M3DTriangleMesh* getTriMesh(std::uint32_t);
-    ~M3DModel();
+    std::uint32_t getTriMeshCount();
-
+    void addTriMesh(std::unique_ptr<M3DTriangleMesh>);
-    M3DTriangleMesh* getTriMesh(uint32_t);
-    uint32_t getTriMeshCount();
-    void addTriMesh(M3DTriangleMesh*);
     void setName(const std::string&);
     const std::string getName() const;
 
  private:
-    std::string name;
+    std::string name{};
-    std::vector<M3DTriangleMesh*> triMeshes;
+    std::vector<std::unique_ptr<M3DTriangleMesh>> triMeshes{};
 };
 
 
 class M3DScene
 {
  public:
-    M3DScene() = default;
-    ~M3DScene();
-
     M3DModel* getModel(std::uint32_t) const;
     std::uint32_t getModelCount() const;
-    void addModel(M3DModel*);
+    void addModel(std::unique_ptr<M3DModel>);
 
     M3DMaterial* getMaterial(std::uint32_t) const;
     std::uint32_t getMaterialCount() const;
-    void addMaterial(M3DMaterial*);
+    void addMaterial(std::unique_ptr<M3DMaterial>);
 
     M3DColor getBackgroundColor() const;
     void setBackgroundColor(M3DColor);
 
  private:
-    std::vector<M3DModel*> models;
+    std::vector<std::unique_ptr<M3DModel>> models{};
-    std::vector<M3DMaterial*> materials;
+    std::vector<std::unique_ptr<M3DMaterial>> materials{};
-    M3DColor backgroundColor;
+    M3DColor backgroundColor{};
 };

src/cel3ds/3dsread.cpp

@@ -11,8 +11,8 @@
 #include <cstdint>
 #include <fstream>
 #include <iostream>
-#include <memory>
 #include <string>
+#include <utility>
 
 #include <Eigen/Core>
 #include <fmt/ostream.h>
@@ -200,13 +200,13 @@ std::int32_t readPointArray(std::istream& in, M3DTriangleMesh* triMesh)
 {
     std::uint16_t nPoints;
     if (!readUshort(in, nPoints)) { return READ_FAILURE; }
-    std::int32_t bytesRead = static_cast<int>(sizeof(nPoints));
+    std::int32_t bytesRead = static_cast<std::int32_t>(sizeof(nPoints));
 
     for (int i = 0; i < static_cast<int>(nPoints); i++)
     {
         float x, y, z;
         if (!readFloat(in, x) || !readFloat(in, y) || !readFloat(in, z)) { return READ_FAILURE; }
-        bytesRead += static_cast<int>(3 * sizeof(float));
+        bytesRead += static_cast<std::int32_t>(3 * sizeof(float));
         triMesh->addVertex(Eigen::Vector3f(x, y, z));
     }
 
@@ -220,13 +220,13 @@ std::int32_t readTextureCoordArray(std::istream& in, M3DTriangleMesh* triMesh)
 
     std::uint16_t nPoints;
     if (!readUshort(in, nPoints)) { return READ_FAILURE; }
-    bytesRead += static_cast<int>(sizeof(nPoints));
+    bytesRead += static_cast<std::int32_t>(sizeof(nPoints));
 
     for (int i = 0; i < static_cast<int>(nPoints); i++)
     {
         float u, v;
         if (!readFloat(in, u) || !readFloat(in, v)) { return READ_FAILURE; }
-        bytesRead += static_cast<int>(2 * sizeof(float));
+        bytesRead += static_cast<std::int32_t>(2 * sizeof(float));
         triMesh->addTexCoord(Eigen::Vector2f(u, -v));
     }
 
@@ -250,17 +250,17 @@ std::int32_t processFaceArrayChunk(std::istream& in,
 
         bytesRead = readString(in, matGroup->materialName);
         if (bytesRead == READ_FAILURE || !readUshort(in, nFaces)) { return READ_FAILURE; }
-        bytesRead += static_cast<int>(sizeof(nFaces));
+        bytesRead += static_cast<std::int32_t>(sizeof(nFaces));
 
         for (std::uint16_t i = 0; i < nFaces; i++)
         {
             std::uint16_t faceIndex;
             if (!readUshort(in, faceIndex)) { return READ_FAILURE; }
-            bytesRead += static_cast<int>(sizeof(faceIndex));
+            bytesRead += static_cast<std::int32_t>(sizeof(faceIndex));
             matGroup->faces.push_back(faceIndex);
         }
 
-        triMesh->addMeshMaterialGroup(matGroup.release());
+        triMesh->addMeshMaterialGroup(std::move(matGroup));
 
         return bytesRead;
 
@@ -271,7 +271,7 @@ std::int32_t processFaceArrayChunk(std::istream& in,
         {
             std::int32_t groups;
             if (!readInt(in, groups) || groups < 0) { return READ_FAILURE; }
-            bytesRead += static_cast<int>(sizeof(groups));
+            bytesRead += static_cast<std::int32_t>(sizeof(groups));
             triMesh->addSmoothingGroups(static_cast<std::uint32_t>(groups));
         }
         return bytesRead;
@@ -286,7 +286,7 @@ std::int32_t readFaceArray(std::istream& in, M3DTriangleMesh* triMesh, std::int3
 {
     std::uint16_t nFaces;
     if (!readUshort(in, nFaces)) { return READ_FAILURE; }
-    std::int32_t bytesRead = static_cast<int>(sizeof(nFaces));
+    std::int32_t bytesRead = static_cast<std::int32_t>(sizeof(nFaces));
 
     for (int i = 0; i < static_cast<int>(nFaces); i++)
     {
@@ -295,7 +295,7 @@ std::int32_t readFaceArray(std::istream& in, M3DTriangleMesh* triMesh, std::int3
         {
             return READ_FAILURE;
         }
-        bytesRead += static_cast<int>(4 * sizeof(std::uint16_t));
+        bytesRead += static_cast<std::int32_t>(4 * sizeof(std::uint16_t));
         triMesh->addFace(v0, v1, v2);
     }
 
@@ -351,7 +351,7 @@ std::int32_t processModelChunk(std::istream& in,
         auto triMesh = std::make_unique<M3DTriangleMesh>();
         std::int32_t bytesRead = read3DSChunks(in, contentSize, processTriMeshChunk, triMesh.get());
         if (bytesRead == READ_FAILURE) { return READ_FAILURE; }
-        model->addTriMesh(triMesh.release());
+        model->addTriMesh(std::move(triMesh));
         return bytesRead;
     }
 
@@ -489,7 +489,7 @@ std::int32_t processSceneChunk(std::istream& in,
                                    processModelChunk,
                                    model.get());
         if (chunksSize < 0) { return READ_FAILURE; }
-        scene->addModel(model.release());
+        scene->addModel(std::move(model));
 
         return bytesRead + chunksSize;
     case M3DCHUNK_MATERIAL_ENTRY:
@@ -499,7 +499,7 @@ std::int32_t processSceneChunk(std::istream& in,
                                   processMaterialChunk,
                                   material.get());
         if (bytesRead < 0) { return READ_FAILURE; }
-        scene->addMaterial(material.release());
+        scene->addMaterial(std::move(material));
 
         return bytesRead;
     case M3DCHUNK_BACKGROUND_COLOR:
@@ -529,7 +529,7 @@ std::int32_t processTopLevelChunk(std::istream& in,
 } // end namespace
 
 
-M3DScene* Read3DSFile(std::istream& in)
+std::unique_ptr<M3DScene> Read3DSFile(std::istream& in)
 {
     std::uint16_t chunkType;
     if (!readUshort(in, chunkType) || chunkType != M3DCHUNK_MAGIC)
@@ -557,11 +557,11 @@ M3DScene* Read3DSFile(std::istream& in)
         return nullptr;
     }
 
-    return scene.release();
+    return scene;
 }
 
 
-M3DScene* Read3DSFile(const fs::path& filename)
+std::unique_ptr<M3DScene> Read3DSFile(const fs::path& filename)
 {
     std::ifstream in(filename.string(), std::ios::in | std::ios::binary);
     if (!in.good())
@@ -570,7 +570,7 @@ M3DScene* Read3DSFile(const fs::path& filename)
         return nullptr;
     }
 
-    M3DScene* scene = Read3DSFile(in);
+    std::unique_ptr<M3DScene> scene = Read3DSFile(in);
     in.close();
     return scene;
 }

src/cel3ds/3dsread.h

@@ -10,9 +10,10 @@
 #pragma once
 
 #include <iosfwd>
+#include <memory>
 #include <celcompat/filesystem.h>
 
 class M3DScene;
 
-M3DScene* Read3DSFile(std::istream& in);
+std::unique_ptr<M3DScene> Read3DSFile(std::istream& in);
-M3DScene* Read3DSFile(const fs::path& filename);
+std::unique_ptr<M3DScene> Read3DSFile(const fs::path& filename);

src/celengine/meshmanager.cpp

@@ -126,7 +126,7 @@ Geometry* GeometryInfo::load(const fs::path& resolvedFilename)
 
     if (fileType == Content_3DStudio)
     {
-        M3DScene* scene = Read3DSFile(filename);
+        std::unique_ptr<M3DScene> scene = Read3DSFile(filename);
         if (scene != nullptr)
         {
             if (resolvedToPath)
@@ -138,8 +138,6 @@ Geometry* GeometryInfo::load(const fs::path& resolvedFilename)
                 model->normalize(center);
             else
                 model->transform(center, scale);
-
-            delete scene;
         }
     }
     else if (fileType == Content_CelestiaModel)