From 7c5c903f473f8fc391f64039b0a593e98f4b49dc Mon Sep 17 00:00:00 2001 From: Andrew Tribick Date: Sun, 28 Nov 2021 11:30:53 +0100 Subject: [PATCH] Remove pointers from mesh and model vectors - Hold models in unique_ptr --- src/celengine/meshmanager.cpp | 99 +++---- src/celengine/modelgeometry.cpp | 4 +- src/celengine/spheremesh.cpp | 10 +- src/celengine/spheremesh.h | 2 +- src/celmodel/material.h | 8 + src/celmodel/mesh.cpp | 110 +++++--- src/celmodel/mesh.h | 20 +- src/celmodel/model.cpp | 78 +++--- src/celmodel/model.h | 18 +- src/celmodel/modelfile.cpp | 252 ++++++++---------- src/celmodel/modelfile.h | 6 +- src/tools/cmod/3dstocmod/3dstocmod.cpp | 15 +- src/tools/cmod/cmodfix/cmodfix.cpp | 42 +-- src/tools/cmod/cmodview/mainwindow.cpp | 93 +++---- src/tools/cmod/cmodview/mainwindow.h | 4 +- src/tools/cmod/cmodview/modelviewwidget.cpp | 16 +- src/tools/cmod/cmodview/modelviewwidget.h | 16 +- src/tools/cmod/common/cmodops.cpp | 127 +++------ src/tools/cmod/common/cmodops.h | 12 +- src/tools/cmod/common/convert3ds.cpp | 41 ++- src/tools/cmod/common/convert3ds.h | 6 +- src/tools/cmod/common/convertobj.cpp | 30 +-- src/tools/cmod/common/convertobj.h | 10 +- .../cmod_bin_ascii_roundtrip_test.cpp | 10 +- 24 files changed, 473 insertions(+), 556 deletions(-) diff --git a/src/celengine/meshmanager.cpp b/src/celengine/meshmanager.cpp index 13d1cb7e6..09f30f708 100644 --- a/src/celengine/meshmanager.cpp +++ b/src/celengine/meshmanager.cpp @@ -77,7 +77,7 @@ NoiseDisplacementFunc(float u, float v, void* info) // TODO: The Celestia mesh format is deprecated -cmod::Model* +std::unique_ptr LoadCelestiaMesh(const fs::path& filename) { std::ifstream meshFile(filename.string(), std::ios::in); @@ -137,21 +137,19 @@ LoadCelestiaMesh(const fs::path& filename) delete meshDefValue; - cmod::Model* model = new cmod::Model(); - SphereMesh* sphereMesh = new SphereMesh(params.size, - static_cast(params.rings), - static_cast(params.slices), - NoiseDisplacementFunc, - (void*) ¶ms); - cmod::Mesh* mesh = sphereMesh->convertToMesh(); - model->addMesh(mesh); - delete sphereMesh; + auto model = std::make_unique(); + SphereMesh sphereMesh(params.size, + static_cast(params.rings), + static_cast(params.slices), + NoiseDisplacementFunc, + (void*) ¶ms); + model->addMesh(sphereMesh.convertToMesh()); return model; } -cmod::Mesh* +cmod::Mesh ConvertTriangleMesh(const M3DTriangleMesh& mesh, const M3DScene& scene) { @@ -191,16 +189,12 @@ ConvertTriangleMesh(const M3DTriangleMesh& mesh, // bool smooth = (mesh.getSmoothingGroupCount() == nFaces); - Eigen::Vector3f* faceNormals = new Eigen::Vector3f[nFaces]; - Eigen::Vector3f* vertexNormals = new Eigen::Vector3f[nFaces * 3]; - auto* faceCounts = new int[nVertices]; - auto** vertexFaces = new int*[nVertices]; - - for (int i = 0; i < nVertices; i++) - { - faceCounts[i] = 0; - vertexFaces[i] = nullptr; - } + std::vector faceNormals; + faceNormals.reserve(nFaces); + std::vector vertexNormals; + vertexNormals.reserve(nFaces * 3); + std::vector faceCounts(nVertices, 0); + std::vector> vertexFaces(nVertices); // generate face normals for (int i = 0; i < nFaces; i++) @@ -215,7 +209,7 @@ ConvertTriangleMesh(const M3DTriangleMesh& mesh, Eigen::Vector3f p0 = mesh.getVertex(v0); Eigen::Vector3f p1 = mesh.getVertex(v1); Eigen::Vector3f p2 = mesh.getVertex(v2); - faceNormals[i] = (p1 - p0).cross(p2 - p1).normalized(); + faceNormals.push_back((p1 - p0).cross(p2 - p1).normalized()); } #if 0 @@ -223,9 +217,9 @@ ConvertTriangleMesh(const M3DTriangleMesh& mesh, { for (int i = 0; i < nFaces; i++) { - vertexNormals[i * 3] = faceNormals[i]; - vertexNormals[i * 3 + 1] = faceNormals[i]; - vertexNormals[i * 3 + 2] = faceNormals[i]; + vertexNormals.push_back(faceNormals[i]); + vertexNormals.push_back(faceNormals[i]); + vertexNormals.push_back(faceNormals[i]); } } else @@ -234,7 +228,7 @@ ConvertTriangleMesh(const M3DTriangleMesh& mesh, // allocate space for vertex face indices for (int i = 0; i < nVertices; i++) { - vertexFaces[i] = new int[faceCounts[i] + 1]; + vertexFaces[i].resize(faceCounts[i] + 1); vertexFaces[i][0] = faceCounts[i]; } @@ -262,7 +256,7 @@ ConvertTriangleMesh(const M3DTriangleMesh& mesh, if (faceNormals[i].dot(faceNormals[k]) > 0.5f) v += faceNormals[k]; } - vertexNormals[i * 3] = v.normalized(); + vertexNormals.push_back(v.normalized()); v = Eigen::Vector3f::Zero(); for (int j = 1; j <= vertexFaces[v1][0]; j++) @@ -272,7 +266,7 @@ ConvertTriangleMesh(const M3DTriangleMesh& mesh, if (faceNormals[i].dot(faceNormals[k]) > 0.5f) v += faceNormals[k]; } - vertexNormals[i * 3 + 1] = v.normalized(); + vertexNormals.push_back(v.normalized()); v = Eigen::Vector3f::Zero(); for (int j = 1; j <= vertexFaces[v2][0]; j++) @@ -282,7 +276,7 @@ ConvertTriangleMesh(const M3DTriangleMesh& mesh, if (faceNormals[i].dot(faceNormals[k]) > 0.5f) v += faceNormals[k]; } - vertexNormals[i * 3 + 2] = v.normalized(); + vertexNormals.push_back(v.normalized()); } } @@ -312,9 +306,9 @@ ConvertTriangleMesh(const M3DTriangleMesh& mesh, } // Create the mesh - cmod::Mesh* newMesh = new cmod::Mesh(); - newMesh->setVertexDescription(cmod::VertexDescription(std::move(attributes))); - newMesh->setVertices(nFaces * 3, std::move(vertexData)); + cmod::Mesh newMesh; + newMesh.setVertexDescription(cmod::VertexDescription(std::move(attributes))); + newMesh.setVertices(nFaces * 3, std::move(vertexData)); for (uint32_t i = 0; i < mesh.getMeshMaterialGroupCount(); ++i) { @@ -345,40 +339,30 @@ ConvertTriangleMesh(const M3DTriangleMesh& mesh, } } - newMesh->addGroup(cmod::PrimitiveGroupType::TriList, materialIndex, std::move(indices)); + newMesh.addGroup(cmod::PrimitiveGroupType::TriList, materialIndex, std::move(indices)); } - // clean up - delete[] faceNormals; - delete[] vertexNormals; - delete[] faceCounts; - for (int i = 0; i < nVertices; i++) - { - delete[] vertexFaces[i]; - } - delete[] vertexFaces; - return newMesh; } -cmod::Model* +std::unique_ptr Convert3DSModel(const M3DScene& scene, const fs::path& texPath) { - cmod::Model* model = new cmod::Model(); + auto model = std::make_unique(); // Convert the materials for (std::uint32_t i = 0; i < scene.getMaterialCount(); i++) { const M3DMaterial* material = scene.getMaterial(i); - cmod::Material* newMaterial = new cmod::Material(); + cmod::Material newMaterial; M3DColor diffuse = material->getDiffuseColor(); - newMaterial->diffuse = cmod::Color(diffuse.red, diffuse.green, diffuse.blue); - newMaterial->opacity = material->getOpacity(); + newMaterial.diffuse = cmod::Color(diffuse.red, diffuse.green, diffuse.blue); + newMaterial.opacity = material->getOpacity(); M3DColor specular = material->getSpecularColor(); - newMaterial->specular = cmod::Color(specular.red, specular.green, specular.blue); + newMaterial.specular = cmod::Color(specular.red, specular.green, specular.blue); float shininess = material->getShininess(); @@ -386,17 +370,17 @@ Convert3DSModel(const M3DScene& scene, const fs::path& texPath) // range that OpenGL uses for the specular exponent. The // current equation is just a guess at the mapping that // 3DS actually uses. - newMaterial->specularPower = std::pow(2.0f, 1.0f + 0.1f * shininess); - if (newMaterial->specularPower > 128.0f) - newMaterial->specularPower = 128.0f; + newMaterial.specularPower = std::pow(2.0f, 1.0f + 0.1f * shininess); + if (newMaterial.specularPower > 128.0f) + newMaterial.specularPower = 128.0f; if (!material->getTextureMap().empty()) { ResourceHandle tex = GetTextureManager()->getHandle(TextureInfo(material->getTextureMap(), texPath, TextureInfo::WrapTexture)); - newMaterial->setMap(cmod::TextureSemantic::DiffuseMap, tex); + newMaterial.setMap(cmod::TextureSemantic::DiffuseMap, tex); } - model->addMaterial(newMaterial); + model->addMaterial(std::move(newMaterial)); } // Convert all models in the scene. Some confusing terminology: a 3ds 'scene' is the same @@ -411,8 +395,7 @@ Convert3DSModel(const M3DScene& scene, const fs::path& texPath) const M3DTriangleMesh* mesh = model3ds->getTriMesh(j); if (mesh) { - cmod::Mesh* newMesh = ConvertTriangleMesh(*mesh, scene); - model->addMesh(newMesh); + model->addMesh(ConvertTriangleMesh(*mesh, scene)); } } } @@ -474,7 +457,7 @@ GeometryInfo::load(const fs::path& resolvedFilename) fs::path filename = resolvedFilename.native().substr(0, uniquifyingSuffixStart); std::clog << fmt::sprintf(_("Loading model: %s\n"), filename); - cmod::Model* model = nullptr; + std::unique_ptr model = nullptr; ContentType fileType = DetermineFileType(filename); if (fileType == Content_3DStudio) @@ -561,7 +544,7 @@ GeometryInfo::load(const fs::path& resolvedFilename) originalMaterialCount, model->getMaterialCount()); - return new ModelGeometry(std::unique_ptr(model)); + return new ModelGeometry(std::move(model)); } else { diff --git a/src/celengine/modelgeometry.cpp b/src/celengine/modelgeometry.cpp index 435a52aa8..fd9024fda 100644 --- a/src/celengine/modelgeometry.cpp +++ b/src/celengine/modelgeometry.cpp @@ -78,7 +78,7 @@ ModelGeometry::render(RenderContext& rc, double /* t */) for (unsigned int i = 0; i < m_model->getMeshCount(); ++i) { - cmod::Mesh* mesh = m_model->getMesh(i); + const cmod::Mesh* mesh = m_model->getMesh(i); const cmod::VertexDescription& vertexDesc = mesh->getVertexDescription(); GLuint vboId = 0; @@ -124,7 +124,7 @@ ModelGeometry::render(RenderContext& rc, double /* t */) // Iterate over all meshes in the model for (unsigned int meshIndex = 0; meshIndex < m_model->getMeshCount(); ++meshIndex) { - cmod::Mesh* mesh = m_model->getMesh(meshIndex); + const cmod::Mesh* mesh = m_model->getMesh(meshIndex); const void* currentData = nullptr; GLuint currentVboId = 0; diff --git a/src/celengine/spheremesh.cpp b/src/celengine/spheremesh.cpp index ebb971b8a..29a7a7d3c 100644 --- a/src/celengine/spheremesh.cpp +++ b/src/celengine/spheremesh.cpp @@ -336,7 +336,7 @@ void SphereMesh::displace(DisplacementMapFunc func, void* info) } -cmod::Mesh* SphereMesh::convertToMesh() const +cmod::Mesh SphereMesh::convertToMesh() const { static_assert(sizeof(float) == sizeof(cmod::VWord), "Float size mismatch with vertex data word size"); constexpr std::uint32_t stride = 8; @@ -353,9 +353,9 @@ cmod::Mesh* SphereMesh::convertToMesh() const 6), }; - cmod::Mesh* mesh = new cmod::Mesh(); + cmod::Mesh mesh; - mesh->setVertexDescription(cmod::VertexDescription(std::move(attributes))); + mesh.setVertexDescription(cmod::VertexDescription(std::move(attributes))); // Copy the vertex data from the separate position, normal, and texture coordinate // arrays into a single array. @@ -369,7 +369,7 @@ cmod::Mesh* SphereMesh::convertToMesh() const std::memcpy(vertex + 6, texCoords + (i * 2), sizeof(float) * 2); } - mesh->setVertices(nVertices, std::move(vertexData)); + mesh.setVertices(nVertices, std::move(vertexData)); for (int i = 0; i < nRings - 1; i++) { @@ -381,7 +381,7 @@ cmod::Mesh* SphereMesh::convertToMesh() const indexData.push_back((i + 1) * (nSlices + 1) + j); } - mesh->addGroup(cmod::PrimitiveGroupType::TriStrip, ~0u, std::move(indexData)); + mesh.addGroup(cmod::PrimitiveGroupType::TriStrip, ~0u, std::move(indexData)); } return mesh; diff --git a/src/celengine/spheremesh.h b/src/celengine/spheremesh.h index 53867def0..124eaa375 100644 --- a/src/celengine/spheremesh.h +++ b/src/celengine/spheremesh.h @@ -43,7 +43,7 @@ public: ~SphereMesh(); //! Convert this object into a standard Celestia mesh. - cmod::Mesh* convertToMesh() const; + cmod::Mesh convertToMesh() const; private: void createSphere(float radius, int _nRings, int _nSlices); diff --git a/src/celmodel/material.h b/src/celmodel/material.h index 677978432..1145d3085 100644 --- a/src/celmodel/material.h +++ b/src/celmodel/material.h @@ -116,6 +116,10 @@ class Material public: Material(); ~Material() = default; + Material(Material&&) = default; + Material& operator=(Material&&) = default; + + Material clone() const { return *this; } inline ResourceHandle getMap(TextureSemantic semantic) const { @@ -141,6 +145,10 @@ public: // Define an ordering for materials; required for elimination of duplicate // materials. bool operator<(const Material& other) const; + +private: + Material(const Material&) = default; + Material& operator=(const Material&) = default; }; } // namespace cmod diff --git a/src/celmodel/mesh.cpp b/src/celmodel/mesh.cpp index 1d48aec2b..69cb248e4 100644 --- a/src/celmodel/mesh.cpp +++ b/src/celmodel/mesh.cpp @@ -119,6 +119,22 @@ bool operator<(const VertexDescription& a, const VertexDescription& b) } +PrimitiveGroup +PrimitiveGroup::clone() const +{ + PrimitiveGroup newGroup; + newGroup.prim = prim; + newGroup.materialIndex = materialIndex; + newGroup.indices = indices; + newGroup.primOverride = primOverride; + newGroup.vertexOverride = vertexOverride; + newGroup.vertexCountOverride = vertexCountOverride; + newGroup.indicesOverride = indicesOverride; + newGroup.vertexDescriptionOverride = vertexDescriptionOverride.clone(); + return newGroup; +} + + unsigned int PrimitiveGroup::getPrimitiveCount() const { @@ -143,10 +159,18 @@ PrimitiveGroup::getPrimitiveCount() const } -Mesh::~Mesh() +Mesh +Mesh::clone() const { - for (const auto group : groups) - delete group; + Mesh newMesh; + newMesh.vertexDesc = vertexDesc.clone(); + newMesh.nVertices = nVertices; + newMesh.vertices = vertices; + newMesh.groups.reserve(groups.size()); + std::transform(groups.cbegin(), groups.cend(), std::back_inserter(newMesh.groups), + [](const PrimitiveGroup& group) { return group.clone(); }); + newMesh.name = name; + return newMesh; } @@ -181,7 +205,7 @@ Mesh::getGroup(unsigned int index) const if (index >= groups.size()) return nullptr; - return groups[index]; + return &groups[index]; } @@ -191,19 +215,19 @@ Mesh::getGroup(unsigned int index) if (index >= groups.size()) return nullptr; - return groups[index]; + return &groups[index]; } unsigned int -Mesh::addGroup(PrimitiveGroup* group) +Mesh::addGroup(PrimitiveGroup&& group) { - groups.push_back(group); + groups.push_back(std::move(group)); return groups.size(); } -PrimitiveGroup* +PrimitiveGroup Mesh::createLinePrimitiveGroup(bool lineStrip, const std::vector& indices) { // Transform LINE_STRIP/LINES to triangle vertices @@ -281,12 +305,13 @@ Mesh::createLinePrimitiveGroup(bool lineStrip, const std::vector& indic VertexAttribute(VertexAttributeSemantic::NextPosition, positionAttributes.format, originalStride), VertexAttribute(VertexAttributeSemantic::ScaleFactor, VertexAttributeFormat::Float1, originalStride + positionSize), }; - auto* g = new PrimitiveGroup(); - g->vertexOverride = data; - g->vertexCountOverride = lineVertexCount; - g->vertexDescriptionOverride = appendingAttributes(vertexDesc, newAttributes.cbegin(), newAttributes.cend()); - g->indicesOverride = std::move(newIndices); - g->primOverride = PrimitiveGroupType::TriList; + + PrimitiveGroup g; + g.vertexOverride = std::move(data); + g.vertexCountOverride = lineVertexCount; + g.vertexDescriptionOverride = appendingAttributes(vertexDesc, newAttributes.cbegin(), newAttributes.cend()); + g.indicesOverride = std::move(newIndices); + g.primOverride = PrimitiveGroupType::TriList; return g; } @@ -296,21 +321,21 @@ Mesh::addGroup(PrimitiveGroupType prim, unsigned int materialIndex, std::vector&& indices) { - PrimitiveGroup* g; + PrimitiveGroup g; if (prim == PrimitiveGroupType::LineStrip || prim == PrimitiveGroupType::LineList) { g = createLinePrimitiveGroup(prim == PrimitiveGroupType::LineStrip, indices); } else { - g = new PrimitiveGroup(); - g->primOverride = prim; + g.primOverride = prim; } - g->indices = std::move(indices); - g->prim = prim; - g->materialIndex = materialIndex; - return addGroup(g); + g.indices = std::move(indices); + g.prim = prim; + g.materialIndex = materialIndex; + + return addGroup(std::move(g)); } @@ -324,9 +349,6 @@ Mesh::getGroupCount() const void Mesh::clearGroups() { - for (const auto group : groups) - delete group; - groups.clear(); } @@ -348,9 +370,9 @@ Mesh::setName(std::string&& _name) void Mesh::remapIndices(const std::vector& indexMap) { - for (auto group : groups) + for (auto& group : groups) { - for (auto& index : group->indices) + for (auto& index : group.indices) { index = indexMap[index]; } @@ -361,8 +383,8 @@ Mesh::remapIndices(const std::vector& indexMap) void Mesh::remapMaterials(const std::vector& materialMap) { - for (auto group : groups) - group->materialIndex = materialMap[group->materialIndex]; + for (auto& group : groups) + group.materialIndex = materialMap[group.materialIndex]; } @@ -370,9 +392,9 @@ void Mesh::aggregateByMaterial() { std::sort(groups.begin(), groups.end(), - [](const PrimitiveGroup* g0, const PrimitiveGroup* g1) + [](const PrimitiveGroup& g0, const PrimitiveGroup& g1) { - return g0->materialIndex < g1->materialIndex; + return g0.materialIndex < g1.materialIndex; }); } @@ -396,10 +418,10 @@ Mesh::pick(const Eigen::Vector3d& rayOrigin, const Eigen::Vector3d& rayDirection const VWord* vdata = vertices.data(); // Iterate over all primitive groups in the mesh - for (const auto group : groups) + for (const auto& group : groups) { - PrimitiveGroupType primType = group->prim; - Index32 nIndices = group->indices.size(); + PrimitiveGroupType primType = group.prim; + Index32 nIndices = group.indices.size(); // Only attempt to compute the intersection of the ray with triangle // groups. @@ -411,9 +433,9 @@ Mesh::pick(const Eigen::Vector3d& rayOrigin, const Eigen::Vector3d& rayDirection { unsigned int primitiveIndex = 0; Index32 index = 0; - Index32 i0 = group->indices[0]; - Index32 i1 = group->indices[1]; - Index32 i2 = group->indices[2]; + Index32 i0 = group.indices[0]; + Index32 i1 = group.indices[1]; + Index32 i2 = group.indices[2]; // Iterate over the triangles in the primitive group do @@ -462,7 +484,7 @@ Mesh::pick(const Eigen::Vector3d& rayOrigin, const Eigen::Vector3d& rayDirection closest = t; if (result) { - result->group = group; + result->group = &group; result->primitiveIndex = primitiveIndex; result->distance = closest; } @@ -477,9 +499,9 @@ Mesh::pick(const Eigen::Vector3d& rayOrigin, const Eigen::Vector3d& rayDirection index += 3; if (index < nIndices) { - i0 = group->indices[index + 0]; - i1 = group->indices[index + 1]; - i2 = group->indices[index + 2]; + i0 = group.indices[index + 0]; + i1 = group.indices[index + 1]; + i2 = group.indices[index + 2]; } } else if (primType == PrimitiveGroupType::TriStrip) @@ -489,7 +511,7 @@ Mesh::pick(const Eigen::Vector3d& rayOrigin, const Eigen::Vector3d& rayDirection { i0 = i1; i1 = i2; - i2 = group->indices[index]; + i2 = group.indices[index]; // TODO: alternate orientation of triangles in a strip } } @@ -500,7 +522,7 @@ Mesh::pick(const Eigen::Vector3d& rayOrigin, const Eigen::Vector3d& rayDirection { index += 1; i1 = i2; - i2 = group->indices[index]; + i2 = group.indices[index]; } } @@ -638,8 +660,8 @@ Mesh::getPrimitiveCount() const { unsigned int count = 0; - for (const auto group : groups) - count += group->getPrimitiveCount(); + for (const auto& group : groups) + count += group.getPrimitiveCount(); return count; } diff --git a/src/celmodel/mesh.h b/src/celmodel/mesh.h index b55e52f85..86d35877d 100644 --- a/src/celmodel/mesh.h +++ b/src/celmodel/mesh.h @@ -161,7 +161,10 @@ struct PrimitiveGroup ~PrimitiveGroup() = default; PrimitiveGroup(const PrimitiveGroup&) = delete; PrimitiveGroup& operator=(const PrimitiveGroup&) = delete; + PrimitiveGroup(PrimitiveGroup&&) = default; + PrimitiveGroup& operator=(PrimitiveGroup&&) = default; + PrimitiveGroup clone() const; unsigned int getPrimitiveCount() const; PrimitiveGroupType prim{ PrimitiveGroupType::InvalidPrimitiveGroupType }; @@ -183,25 +186,28 @@ class Mesh public: PickResult() = default; - Mesh* mesh{ nullptr }; - PrimitiveGroup* group{ nullptr }; + const Mesh* mesh{ nullptr }; + const PrimitiveGroup* group{ nullptr }; unsigned int primitiveIndex{ 0 }; double distance{ -1.0 }; }; Mesh() = default; - ~Mesh(); + ~Mesh() = default; Mesh(const Mesh&) = delete; Mesh& operator=(const Mesh&) = delete; + Mesh(Mesh&&) = default; + Mesh& operator=(Mesh&&) = default; + + Mesh clone() const; void setVertices(unsigned int _nVertices, std::vector&& vertexData); bool setVertexDescription(VertexDescription&& desc); const VertexDescription& getVertexDescription() const; - PrimitiveGroup* createLinePrimitiveGroup(bool lineStrip, const std::vector& indices); const PrimitiveGroup* getGroup(unsigned int index) const; PrimitiveGroup* getGroup(unsigned int index); - unsigned int addGroup(PrimitiveGroup* group); + unsigned int addGroup(PrimitiveGroup&& group); unsigned int addGroup(PrimitiveGroupType prim, unsigned int materialIndex, std::vector&& indices); @@ -232,14 +238,14 @@ class Mesh unsigned int getPrimitiveCount() const; private: - void recomputeBoundingBox(); + PrimitiveGroup createLinePrimitiveGroup(bool lineStrip, const std::vector& indices); VertexDescription vertexDesc{ }; unsigned int nVertices{ 0 }; std::vector vertices{ }; - std::vector groups; + std::vector groups; std::string name; }; diff --git a/src/celmodel/model.cpp b/src/celmodel/model.cpp index c23be8ad6..35b4f2d50 100644 --- a/src/celmodel/model.cpp +++ b/src/celmodel/model.cpp @@ -9,7 +9,9 @@ // of the License, or (at your option) any later version. #include +#include #include +#include #include @@ -42,13 +44,6 @@ Model::Model() } -Model::~Model() -{ - for (const auto mesh : meshes) - delete mesh; -} - - /*! Return the material with the specified index, or nullptr if * the index is out of range. The returned material pointer * is const. @@ -57,7 +52,7 @@ const Material* Model::getMaterial(unsigned int index) const { if (index < materials.size()) - return materials[index]; + return &materials[index]; else return nullptr; } @@ -67,7 +62,7 @@ Model::getMaterial(unsigned int index) const * return value is the number of materials in the model. */ unsigned int -Model::addMaterial(const Material* m) +Model::addMaterial(Material&& m) { // Update the texture map usage information for the model. Since // the material being added isn't necessarily used by a mesh within @@ -76,13 +71,13 @@ Model::addMaterial(const Material* m) // if it forces multipass rendering when it's not required. for (int i = 0; i < static_cast(TextureSemantic::TextureSemanticMax); i++) { - if (m->maps[i] != InvalidResource) + if (m.maps[i] != InvalidResource) { textureUsage[i] = true; } } - materials.push_back(m); + materials.push_back(std::move(m)); return materials.size(); } @@ -99,8 +94,8 @@ Model::getVertexCount() const { unsigned int count = 0; - for (const auto mesh : meshes) - count += mesh->getVertexCount(); + for (const auto& mesh : meshes) + count += mesh.getVertexCount(); return count; } @@ -111,8 +106,8 @@ Model::getPrimitiveCount() const { unsigned int count = 0; - for (const auto mesh : meshes) - count += mesh->getPrimitiveCount(); + for (const auto& mesh : meshes) + count += mesh.getPrimitiveCount(); return count; } @@ -126,19 +121,29 @@ Model::getMeshCount() const Mesh* +Model::getMesh(unsigned int index) +{ + if (index < meshes.size()) + return &meshes[index]; + else + return nullptr; +} + + +const Mesh* Model::getMesh(unsigned int index) const { if (index < meshes.size()) - return meshes[index]; + return &meshes[index]; else return nullptr; } unsigned int -Model::addMesh(Mesh* m) +Model::addMesh(Mesh&& m) { - meshes.push_back(m); + meshes.push_back(std::move(m)); return meshes.size(); } @@ -152,15 +157,15 @@ Model::pick(const Eigen::Vector3d& rayOrigin, double closest = maxDistance; Mesh::PickResult closestResult; - for (const auto mesh : meshes) + for (const auto& mesh : meshes) { Mesh::PickResult result; - if (mesh->pick(rayOrigin, rayDirection, &result)) + if (mesh.pick(rayOrigin, rayDirection, &result)) { if (result.distance < closest) { closestResult = result; - closestResult.mesh = mesh; + closestResult.mesh = &mesh; closest = result.distance; } } @@ -200,8 +205,8 @@ Model::pick(const Eigen::Vector3d& rayOrigin, const Eigen::Vector3d& rayDirectio void Model::transform(const Eigen::Vector3f& translation, float scale) { - for (const auto mesh : meshes) - mesh->transform(translation, scale); + for (auto& mesh : meshes) + mesh.transform(translation, scale); } @@ -210,8 +215,8 @@ Model::normalize(const Eigen::Vector3f& centerOffset) { Eigen::AlignedBox bbox; - for (const auto mesh : meshes) - bbox.extend(mesh->getBoundingBox()); + for (const auto& mesh : meshes) + bbox.extend(mesh.getBoundingBox()); Eigen::Vector3f center = (bbox.min() + bbox.max()) * 0.5f + centerOffset; Eigen::Vector3f extents = bbox.max() - bbox.min(); @@ -236,22 +241,22 @@ Model::uniquifyMaterials() // Sort the material indices so that we can uniquify the materials std::sort(indices.begin(), indices.end(), - [&](unsigned int a, unsigned int b) { return *materials[a] < *materials[b]; }); + [&](unsigned int a, unsigned int b) { return materials[a] < materials[b]; }); // From the sorted index list construct the list of unique materials // and a map to convert old material indices into indices that can be // used with the uniquified material list. std::vector materialMap(materials.size()); - std::vector uniqueMaterials; + std::vector uniqueMaterials; uniqueMaterials.reserve(materials.size()); for (std::size_t i = 0; i < indices.size(); ++i) { unsigned int index = indices[i]; - if (i == 0 || *materials[index] != *uniqueMaterials.back()) + if (i == 0 || materials[index] != uniqueMaterials.back()) { - uniqueMaterials.push_back(materials[index]); + uniqueMaterials.push_back(std::move(materials[index])); } materialMap[index] = uniqueMaterials.size() - 1; @@ -260,9 +265,9 @@ Model::uniquifyMaterials() // Remap all the material indices in the model. Even if no materials have // been eliminated we've still sorted them by opacity, which is useful // when reordering meshes so that translucent ones are rendered last. - for (Mesh* mesh : meshes) + for (auto& mesh : meshes) { - mesh->remapMaterials(materialMap); + mesh.remapMaterials(materialMap); } materials = std::move(uniqueMaterials); @@ -274,8 +279,8 @@ Model::determineOpacity() { for (unsigned int i = 0; i < materials.size(); i++) { - if ((materials[i]->opacity > 0.01f && materials[i]->opacity < 1.0f) || - materials[i]->blend == BlendMode::AdditiveBlend) + if ((materials[i].opacity > 0.01f && materials[i].opacity < 1.0f) || + materials[i].blend == BlendMode::AdditiveBlend) { opaque = false; return; @@ -308,12 +313,11 @@ Model::sortMeshes(const MeshComparator& comparator) { // Sort submeshes by material; if materials have been uniquified, // then the submeshes will be ordered so that opaque ones are first. - for (const auto mesh : meshes) - mesh->aggregateByMaterial(); + for (auto& mesh : meshes) + mesh.aggregateByMaterial(); // Sort the meshes so that completely opaque ones are first - std::sort(meshes.begin(), meshes.end(), - [&](const Mesh* a, const Mesh* b) { return comparator(*a, *b); }); + std::sort(meshes.begin(), meshes.end(), std::ref(comparator)); } } // end namespace cmod diff --git a/src/celmodel/model.h b/src/celmodel/model.h index 0cd1d464a..40a986756 100644 --- a/src/celmodel/model.h +++ b/src/celmodel/model.h @@ -36,11 +36,11 @@ class Model { public: Model(); - ~Model(); + ~Model() = default; const Material* getMaterial(unsigned int index) const; void setMaterial(unsigned int index, const Material* material); - unsigned int addMaterial(const Material* material); + unsigned int addMaterial(Material&& material); /*! Return the number of materials in the model */ @@ -54,10 +54,12 @@ class Model */ unsigned int getPrimitiveCount() const; + Mesh* getMesh(unsigned int index); + /*! Return the mesh with the specified index, or nullptr if the * index is out of range. */ - Mesh* getMesh(unsigned int index) const; + const Mesh* getMesh(unsigned int index) const; /*! Return the total number of meshes withing the model. */ @@ -66,7 +68,7 @@ class Model /*! Add a new mesh to the model; the return value is the * total number of meshes in the model. */ - unsigned int addMesh(Mesh* mesh); + unsigned int addMesh(Mesh&& mesh); /** Find the closest intersection between the ray (given * by origin and direction) and the model. If the ray @@ -148,14 +150,12 @@ class Model { public: OpacityComparator() = default; - virtual ~OpacityComparator() = default; - - virtual bool operator()(const Mesh&, const Mesh&) const; + bool operator()(const Mesh&, const Mesh&) const override; }; private: - std::vector materials; - std::vector meshes; + std::vector materials{ }; + std::vector meshes{ }; std::array(TextureSemantic::TextureSemanticMax)> textureUsage; bool opaque{ true }; diff --git a/src/celmodel/modelfile.cpp b/src/celmodel/modelfile.cpp index c12f28401..14f5747e6 100644 --- a/src/celmodel/modelfile.cpp +++ b/src/celmodel/modelfile.cpp @@ -90,7 +90,7 @@ public: explicit ModelLoader(HandleGetter& _handleGetter) : handleGetter(_handleGetter) {} virtual ~ModelLoader() = default; - virtual Model* load() = 0; + virtual std::unique_ptr load() = 0; const std::string& getErrorMessage() const { return errorMessage; } ResourceHandle getHandle(const fs::path& path) { return handleGetter(path); } @@ -281,12 +281,12 @@ public: {} ~AsciiModelLoader() override = default; - Model* load() override; - void reportError(const std::string& /*msg*/) override; + std::unique_ptr load() override; + void reportError(const std::string& msg) override; - Material* loadMaterial(); + bool loadMaterial(Material& material); VertexDescription loadVertexDescription(); - Mesh* loadMesh(); + bool loadMesh(Mesh& mesh); std::vector loadVertices(const VertexDescription& vertexDesc, unsigned int& vertexCount); @@ -303,22 +303,20 @@ AsciiModelLoader::reportError(const std::string& msg) } -Material* -AsciiModelLoader::loadMaterial() +bool +AsciiModelLoader::loadMaterial(Material& material) { if (tok.nextToken() != Tokenizer::TokenName || tok.getNameValue() != MaterialToken) { reportError("Material definition expected"); - return nullptr; + return false; } - auto* material = new Material(); - - material->diffuse = DefaultDiffuse; - material->specular = DefaultSpecular; - material->emissive = DefaultEmissive; - material->specularPower = DefaultSpecularPower; - material->opacity = DefaultOpacity; + material.diffuse = DefaultDiffuse; + material.specular = DefaultSpecular; + material.emissive = DefaultEmissive; + material.specularPower = DefaultSpecularPower; + material.opacity = DefaultOpacity; while (tok.nextToken() == Tokenizer::TokenName && tok.getNameValue() != EndMaterialToken) { @@ -330,14 +328,13 @@ AsciiModelLoader::loadMaterial() if (tok.nextToken() != Tokenizer::TokenString) { reportError("Texture name expected"); - delete material; - return nullptr; + return false; } std::string textureName = tok.getStringValue(); ResourceHandle tex = getHandle(textureName); - material->setMap(texType, tex); + material.setMap(texType, tex); } else if (property == "blend") { @@ -357,11 +354,10 @@ AsciiModelLoader::loadMaterial() if (blendMode == BlendMode::InvalidBlend) { reportError("Bad blend mode in material"); - delete material; - return nullptr; + return false; } - material->blend = blendMode; + material.blend = blendMode; } else { @@ -379,8 +375,7 @@ AsciiModelLoader::loadMaterial() if (tok.nextToken() != Tokenizer::TokenNumber) { reportError("Bad property value in material"); - delete material; - return nullptr; + return false; } data[i] = tok.getNumberValue(); } @@ -395,34 +390,33 @@ AsciiModelLoader::loadMaterial() if (property == "diffuse") { - material->diffuse = colorVal; + material.diffuse = colorVal; } else if (property == "specular") { - material->specular = colorVal; + material.specular = colorVal; } else if (property == "emissive") { - material->emissive = colorVal; + material.emissive = colorVal; } else if (property == "opacity") { - material->opacity = (float) data[0]; + material.opacity = static_cast(data[0]); } else if (property == "specpower") { - material->specularPower = (float) data[0]; + material.specularPower = static_cast(data[0]); } } } if (tok.getTokenType() != Tokenizer::TokenName) { - delete material; - return nullptr; + return false; } - return material; + return true; } @@ -599,29 +593,28 @@ AsciiModelLoader::loadVertices(const VertexDescription& vertexDesc, } -Mesh* -AsciiModelLoader::loadMesh() +bool +AsciiModelLoader::loadMesh(Mesh& mesh) { if (tok.nextToken() != Tokenizer::TokenName && tok.getNameValue() != MeshToken) { reportError("Mesh definition expected"); - return nullptr; + return false; } VertexDescription vertexDesc = loadVertexDescription(); if (vertexDesc.attributes.empty()) - return nullptr; + return false; unsigned int vertexCount = 0; std::vector vertexData = loadVertices(vertexDesc, vertexCount); if (vertexData.empty()) { - return nullptr; + return false; } - auto* mesh = new Mesh(); - mesh->setVertexDescription(std::move(vertexDesc)); - mesh->setVertices(vertexCount, std::move(vertexData)); + mesh.setVertexDescription(std::move(vertexDesc)); + mesh.setVertices(vertexCount, std::move(vertexData)); while (tok.nextToken() == Tokenizer::TokenName && tok.getNameValue() != EndMeshToken) { @@ -629,15 +622,13 @@ AsciiModelLoader::loadMesh() if (type == PrimitiveGroupType::InvalidPrimitiveGroupType) { reportError("Bad primitive group type: " + tok.getStringValue()); - delete mesh; - return nullptr; + return false; } if (tok.nextToken() != Tokenizer::TokenNumber || !tok.isInteger()) { reportError("Material index expected in primitive group"); - delete mesh; - return nullptr; + return false; } unsigned int materialIndex; @@ -653,8 +644,7 @@ AsciiModelLoader::loadMesh() if (tok.nextToken() != Tokenizer::TokenNumber || !tok.isInteger()) { reportError("Index count expected in primitive group"); - delete mesh; - return nullptr; + return false; } unsigned int indexCount = (unsigned int) tok.getIntegerValue(); @@ -667,32 +657,30 @@ AsciiModelLoader::loadMesh() if (tok.nextToken() != Tokenizer::TokenNumber || !tok.isInteger()) { reportError("Incomplete index list in primitive group"); - delete mesh; - return nullptr; + return false; } unsigned int index = (unsigned int) tok.getIntegerValue(); if (index >= vertexCount) { reportError("Index out of range"); - delete mesh; - return nullptr; + return false; } indices.push_back(index); } - mesh->addGroup(type, materialIndex, std::move(indices)); + mesh.addGroup(type, materialIndex, std::move(indices)); } - return mesh; + return true; } -Model* +std::unique_ptr AsciiModelLoader::load() { - auto* model = new Model(); + auto model = std::make_unique(); bool seenMeshes = false; // Parse material and mesh definitions @@ -708,43 +696,38 @@ AsciiModelLoader::load() if (seenMeshes) { reportError("Materials must be defined before meshes"); - delete model; return nullptr; } - Material* material = loadMaterial(); - if (material == nullptr) + Material material; + if (!loadMaterial(material)) { - delete model; return nullptr; } - model->addMaterial(material); + model->addMaterial(std::move(material)); } else if (name == "mesh") { seenMeshes = true; - Mesh* mesh = loadMesh(); - if (mesh == nullptr) + Mesh mesh; + if (!loadMesh(mesh)) { - delete model; return nullptr; } - model->addMesh(mesh); + model->addMesh(std::move(mesh)); } else { reportError(fmt::format("Error: Unknown block type {}", name)); - delete model; return nullptr; } } else { reportError("Block name expected"); - delete model; return nullptr; } } @@ -1240,12 +1223,12 @@ public: {} ~BinaryModelLoader() override = default; - Model* load() override; + std::unique_ptr load() override; void reportError(const std::string& /*msg*/) override; - Material* loadMaterial(); + bool loadMaterial(Material& material); VertexDescription loadVertexDescription(); - Mesh* loadMesh(); + bool loadMesh(Mesh& mesh); std::vector loadVertices(const VertexDescription& vertexDesc, unsigned int& vertexCount); @@ -1262,10 +1245,10 @@ BinaryModelLoader::reportError(const std::string& msg) } -Model* +std::unique_ptr BinaryModelLoader::load() { - auto* model = new Model(); + auto model = std::make_unique(); bool seenMeshes = false; // Parse material and mesh definitions @@ -1276,7 +1259,6 @@ BinaryModelLoader::load() { if (in.eof()) { break; } reportError("Failed to read token"); - delete model; return nullptr; } @@ -1285,36 +1267,32 @@ BinaryModelLoader::load() if (seenMeshes) { reportError("Materials must be defined before meshes"); - delete model; return nullptr; } - Material* material = loadMaterial(); - if (material == nullptr) + Material material; + if (!loadMaterial(material)) { - delete model; return nullptr; } - model->addMaterial(material); + model->addMaterial(std::move(material)); } else if (tok == CMOD_Mesh) { seenMeshes = true; - Mesh* mesh = loadMesh(); - if (mesh == nullptr) + Mesh mesh; + if (!loadMesh(mesh)) { - delete model; return nullptr; } - model->addMesh(mesh); + model->addMesh(std::move(mesh)); } else { reportError("Error: Unknown block type in model"); - delete model; return nullptr; } } @@ -1323,16 +1301,14 @@ BinaryModelLoader::load() } -Material* -BinaryModelLoader::loadMaterial() +bool +BinaryModelLoader::loadMaterial(Material& material) { - auto* material = new Material(); - - material->diffuse = DefaultDiffuse; - material->specular = DefaultSpecular; - material->emissive = DefaultEmissive; - material->specularPower = DefaultSpecularPower; - material->opacity = DefaultOpacity; + material.diffuse = DefaultDiffuse; + material.specular = DefaultSpecular; + material.emissive = DefaultEmissive; + material.specularPower = DefaultSpecularPower; + material.opacity = DefaultOpacity; for (;;) { @@ -1340,54 +1316,48 @@ BinaryModelLoader::loadMaterial() if (!readToken(in, tok)) { reportError("Error reading token type"); - delete material; - return nullptr; + return false; } switch (tok) { case CMOD_Diffuse: - if (!readTypeColor(in, material->diffuse)) + if (!readTypeColor(in, material.diffuse)) { reportError("Incorrect type for diffuse color"); - delete material; - return nullptr; + return false; } break; case CMOD_Specular: - if (!readTypeColor(in, material->specular)) + if (!readTypeColor(in, material.specular)) { reportError("Incorrect type for specular color"); - delete material; - return nullptr; + return false; } break; case CMOD_Emissive: - if (!readTypeColor(in, material->emissive)) + if (!readTypeColor(in, material.emissive)) { reportError("Incorrect type for emissive color"); - delete material; - return nullptr; + return false; } break; case CMOD_SpecularPower: - if (!readTypeFloat1(in, material->specularPower)) + if (!readTypeFloat1(in, material.specularPower)) { reportError("Float expected for specularPower"); - delete material; - return nullptr; + return false; } break; case CMOD_Opacity: - if (!readTypeFloat1(in, material->opacity)) + if (!readTypeFloat1(in, material.opacity)) { reportError("Float expected for opacity"); - delete material; - return nullptr; + return false; } break; @@ -1398,10 +1368,9 @@ BinaryModelLoader::loadMaterial() || blendMode < 0 || blendMode >= static_cast(BlendMode::BlendMax)) { reportError("Bad blend mode"); - delete material; - return nullptr; + return false; } - material->blend = static_cast(blendMode); + material.blend = static_cast(blendMode); } break; @@ -1412,39 +1381,35 @@ BinaryModelLoader::loadMaterial() || texType < 0 || texType >= static_cast(TextureSemantic::TextureSemanticMax)) { reportError("Bad texture type"); - delete material; - return nullptr; + return false; } std::string texfile; if (!readTypeString(in, texfile)) { reportError("String expected for texture filename"); - delete material; - return nullptr; + return false; } if (texfile.empty()) { reportError("Zero length texture name in material definition"); - delete material; - return nullptr; + return false; } ResourceHandle tex = getHandle(texfile); - material->maps[texType] = tex; + material.maps[texType] = tex; } break; case CMOD_EndMaterial: - return material; + return true; default: // Skip unrecognized tokens if (!ignoreValue(in)) { - delete material; - return nullptr; + return false; } } // switch } // for @@ -1520,23 +1485,18 @@ BinaryModelLoader::loadVertexDescription() } -Mesh* -BinaryModelLoader::loadMesh() +bool +BinaryModelLoader::loadMesh(Mesh& mesh) { VertexDescription vertexDesc = loadVertexDescription(); - if (vertexDesc.attributes.empty()) - return nullptr; + if (vertexDesc.attributes.empty()) { return false; } unsigned int vertexCount = 0; std::vector vertexData = loadVertices(vertexDesc, vertexCount); - if (vertexData.empty()) - { - return nullptr; - } + if (vertexData.empty()) { return false; } - auto* mesh = new Mesh(); - mesh->setVertexDescription(std::move(vertexDesc)); - mesh->setVertices(vertexCount, std::move(vertexData)); + mesh.setVertexDescription(std::move(vertexDesc)); + mesh.setVertices(vertexCount, std::move(vertexData)); for (;;) { @@ -1544,8 +1504,7 @@ BinaryModelLoader::loadMesh() if (!celutil::readLE(in, tok)) { reportError("Failed to read token type"); - delete mesh; - return nullptr; + return false; } if (tok == CMOD_EndMesh) @@ -1555,8 +1514,7 @@ BinaryModelLoader::loadMesh() if (tok < 0 || tok >= static_cast(PrimitiveGroupType::PrimitiveTypeMax)) { reportError("Bad primitive group type"); - delete mesh; - return nullptr; + return false; } PrimitiveGroupType type = static_cast(tok); @@ -1565,8 +1523,7 @@ BinaryModelLoader::loadMesh() || !celutil::readLE(in, indexCount)) { reportError("Could not read primitive indices"); - delete mesh; - return nullptr; + return false; } std::vector indices; @@ -1578,17 +1535,16 @@ BinaryModelLoader::loadMesh() if (!celutil::readLE(in, index) || index >= vertexCount) { reportError("Index out of range"); - delete mesh; - return nullptr; + return false; } indices.push_back(index); } - mesh->addGroup(type, materialIndex, std::move(indices)); + mesh.addGroup(type, materialIndex, std::move(indices)); } - return mesh; + return true; } @@ -1942,7 +1898,8 @@ BinaryModelWriter::writeMaterial(const Material& material) } -ModelLoader* OpenModel(std::istream& in, HandleGetter& getHandle) +std::unique_ptr +OpenModel(std::istream& in, HandleGetter& getHandle) { char header[CEL_MODEL_HEADER_LENGTH]; if (!in.read(header, CEL_MODEL_HEADER_LENGTH).good()) @@ -1953,11 +1910,11 @@ ModelLoader* OpenModel(std::istream& in, HandleGetter& getHandle) if (std::strncmp(header, CEL_MODEL_HEADER_ASCII, CEL_MODEL_HEADER_LENGTH) == 0) { - return new AsciiModelLoader(in, getHandle); + return std::make_unique(in, getHandle); } if (std::strncmp(header, CEL_MODEL_HEADER_BINARY, CEL_MODEL_HEADER_LENGTH) == 0) { - return new BinaryModelLoader(in, getHandle); + return std::make_unique(in, getHandle); } else { @@ -1970,20 +1927,19 @@ ModelLoader* OpenModel(std::istream& in, HandleGetter& getHandle) } // end unnamed namespace -Model* LoadModel(std::istream& in, HandleGetter handleGetter) +std::unique_ptr +LoadModel(std::istream& in, HandleGetter handleGetter) { - ModelLoader* loader = OpenModel(in, handleGetter); + std::unique_ptr loader = OpenModel(in, handleGetter); if (loader == nullptr) return nullptr; - Model* model = loader->load(); + std::unique_ptr model = loader->load(); if (model == nullptr) { std::cerr << "Error in model file: " << loader->getErrorMessage() << '\n'; } - delete loader; - return model; } diff --git a/src/celmodel/modelfile.h b/src/celmodel/modelfile.h index b2e205d59..b1b5d2c11 100644 --- a/src/celmodel/modelfile.h +++ b/src/celmodel/modelfile.h @@ -12,19 +12,19 @@ #include #include +#include #include #include -#include "material.h" +#include "model.h" namespace cmod { -class Model; using HandleGetter = std::function; using SourceGetter = std::function; -Model* LoadModel(std::istream& in, HandleGetter getHandle); +std::unique_ptr LoadModel(std::istream& in, HandleGetter getHandle); bool SaveModelAscii(const Model* model, std::ostream& out, SourceGetter getSource); bool SaveModelBinary(const Model* model, std::ostream& out, SourceGetter getSource); diff --git a/src/tools/cmod/3dstocmod/3dstocmod.cpp b/src/tools/cmod/3dstocmod/3dstocmod.cpp index fed763f9d..1d7ca99a7 100644 --- a/src/tools/cmod/3dstocmod/3dstocmod.cpp +++ b/src/tools/cmod/3dstocmod/3dstocmod.cpp @@ -46,7 +46,7 @@ int main(int argc, char* argv[]) } std::cerr << "Converting...\n"; - cmod::Model* model = Convert3DSModel(*scene, GetPathManager()->getHandle); + std::unique_ptr model = Convert3DSModel(*scene, GetPathManager()->getHandle); if (!model) { std::cerr << "Error converting 3DS file to Celestia model\n"; @@ -58,25 +58,22 @@ int main(int argc, char* argv[]) double weldTolerance = 1.0e-6; bool weldVertices = true; - cmod::Model* newModel = GenerateModelNormals(*model, celmath::degToRad(smoothAngle), weldVertices, weldTolerance); - delete model; + model = GenerateModelNormals(*model, celmath::degToRad(smoothAngle), weldVertices, weldTolerance); - if (!newModel) + if (!model) { std::cerr << "Ran out of memory while generating surface normals.\n"; return 1; } // Automatically uniquify vertices - for (unsigned int i = 0; newModel->getMesh(i) != nullptr; i++) + for (unsigned int i = 0; model->getMesh(i) != nullptr; i++) { - cmod::Mesh* mesh = newModel->getMesh(i); + cmod::Mesh* mesh = model->getMesh(i); UniquifyVertices(*mesh); } - model = newModel; - - SaveModelAscii(model, std::cout, GetPathManager()->getSource); + SaveModelAscii(model.get(), std::cout, GetPathManager()->getSource); return 0; } diff --git a/src/tools/cmod/cmodfix/cmodfix.cpp b/src/tools/cmod/cmodfix/cmodfix.cpp index 58c361565..929712115 100644 --- a/src/tools/cmod/cmodfix/cmodfix.cpp +++ b/src/tools/cmod/cmodfix/cmodfix.cpp @@ -14,7 +14,9 @@ #include #include #include +#include #include +#include #include #include @@ -151,7 +153,7 @@ int main(int argc, char* argv[]) PathManager pathManager; - cmod::Model* model = nullptr; + std::unique_ptr model = nullptr; if (!inputFilename.empty()) { std::ifstream in(inputFilename, std::ios::in | std::ios::binary); @@ -172,52 +174,50 @@ int main(int argc, char* argv[]) if (genNormals || genTangents) { - cmod::Model* newModel = new cmod::Model(); + auto newModel = std::make_unique(); std::uint32_t i; // Copy materials for (i = 0; model->getMaterial(i) != nullptr; i++) { - newModel->addMaterial(model->getMaterial(i)); + newModel->addMaterial(model->getMaterial(i)->clone()); } // Generate normals and/or tangents for each model in the mesh for (i = 0; model->getMesh(i) != nullptr; i++) { - cmod::Mesh* mesh = model->getMesh(i); - cmod::Mesh* newMesh = nullptr; + cmod::Mesh mesh = model->getMesh(i)->clone(); if (genNormals) { - newMesh = GenerateNormals(*mesh, - celmath::degToRad(smoothAngle), - weldVertices); - if (newMesh == nullptr) + cmod::Mesh newMesh = GenerateNormals(mesh, + celmath::degToRad(smoothAngle), + weldVertices); + if (newMesh.getVertexCount() == 0) { std::cerr << "Error generating normals!\n"; return 1; } - // TODO: clean up old mesh - mesh = newMesh; + + mesh = std::move(newMesh); } if (genTangents) { - newMesh = GenerateTangents(*mesh, weldVertices); - if (newMesh == nullptr) + cmod::Mesh newMesh = GenerateTangents(mesh, weldVertices); + if (newMesh.getVertexCount() == 0) { std::cerr << "Error generating tangents!\n"; return 1; } // TODO: clean up old mesh - mesh = newMesh; + mesh = std::move(newMesh); } - newModel->addMesh(mesh); + newModel->addMesh(std::move(mesh)); } - // delete model; - model = newModel; + model = std::move(newModel); } if (mergeMeshes) @@ -249,9 +249,9 @@ int main(int argc, char* argv[]) if (outputFilename.empty()) { if (outputBinary) - SaveModelBinary(model, std::cout, GetPathManager()->getSource); + SaveModelBinary(model.get(), std::cout, GetPathManager()->getSource); else - SaveModelAscii(model, std::cout, GetPathManager()->getSource); + SaveModelAscii(model.get(), std::cout, GetPathManager()->getSource); } else { @@ -270,9 +270,9 @@ int main(int argc, char* argv[]) } if (outputBinary) - SaveModelBinary(model, out, GetPathManager()->getSource); + SaveModelBinary(model.get(), out, GetPathManager()->getSource); else - SaveModelAscii(model, out, GetPathManager()->getSource); + SaveModelAscii(model.get(), out, GetPathManager()->getSource); } return 0; diff --git a/src/tools/cmod/cmodview/mainwindow.cpp b/src/tools/cmod/cmodview/mainwindow.cpp index 8e2bb86c5..f43fa2612 100644 --- a/src/tools/cmod/cmodview/mainwindow.cpp +++ b/src/tools/cmod/cmodview/mainwindow.cpp @@ -10,8 +10,8 @@ #include #include -#include #include +#include #include #include @@ -60,22 +60,6 @@ namespace // value whenever new tool widgets are added or removed. constexpr int CMODVIEW_STATE_VERSION = 1; - -cmod::Material* -cloneMaterial(const cmod::Material* other) -{ - cmod::Material* material = new cmod::Material(); - material->diffuse = other->diffuse; - material->specular = other->specular; - material->emissive = other->emissive; - material->specularPower = other->specularPower; - material->opacity = other->opacity; - material->blend = other->blend; - material->maps = other->maps; - - return material; -} - } // end unnamed namespace @@ -277,11 +261,11 @@ MainWindow::eventFilter(QObject* obj, QEvent* e) void -MainWindow::setModel(const QString& fileName, cmod::Model* model) +MainWindow::setModel(const QString& fileName, std::unique_ptr&& model) { QFileInfo info(fileName); QString modelDir = info.absoluteDir().path(); - m_modelView->setModel(model, modelDir); + m_modelView->setModel(std::move(model), modelDir); // Only reset the camera when we've loaded a new model. Leaving // the camera fixed allows to see model changes more easily. @@ -300,7 +284,7 @@ MainWindow::setModel(const QString& fileName, cmod::Model* model) void MainWindow::showModelStatistics() { - cmod::Model* model = m_modelView->model(); + const cmod::Model* model = m_modelView->model(); if (model) { // Count triangles and vertices in the mesh @@ -410,7 +394,7 @@ MainWindow::openModel(const QString& fileName) return; } - cmod::Model* model = Convert3DSModel(*scene, GetPathManager()->getHandle); + std::unique_ptr model = Convert3DSModel(*scene, GetPathManager()->getHandle); if (model == nullptr) { QMessageBox::warning(this, "Load error", tr("Internal error converting 3DS file %1").arg(fileName)); @@ -422,28 +406,29 @@ MainWindow::openModel(const QString& fileName) double weldTolerance = 1.0e-6; bool weldVertices = true; - cmod::Model* newModel = GenerateModelNormals(*model, celmath::degToRad(smoothAngle), weldVertices, weldTolerance); - delete model; + model = GenerateModelNormals(*model, + celmath::degToRad(smoothAngle), + weldVertices, + weldTolerance); - if (!newModel) + if (!model) { QMessageBox::warning(this, tr("Mesh Load Error"), tr("Internal error when loading mesh")); } else { // Automatically uniquify vertices - for (unsigned int i = 0; newModel->getMesh(i) != nullptr; i++) + for (unsigned int i = 0; model->getMesh(i) != nullptr; i++) { - cmod::Mesh* mesh = newModel->getMesh(i); + cmod::Mesh* mesh = model->getMesh(i); UniquifyVertices(*mesh); } - setModel(fileName, newModel); + setModel(fileName, std::move(model)); } } else if (info.suffix().toLower() == "obj") { - cmod::Model* model = nullptr; std::ifstream in(fileNameStd, std::ios::in | std::ios::binary); if (!in.good()) { @@ -452,7 +437,7 @@ MainWindow::openModel(const QString& fileName) } WavefrontLoader loader(in); - model = loader.load(); + std::unique_ptr model = loader.load(); if (model == nullptr) { @@ -467,11 +452,10 @@ MainWindow::openModel(const QString& fileName) UniquifyVertices(*mesh); } - setModel(fileName, model); + setModel(fileName, std::move(model)); } else if (info.suffix().toLower() == "cmod") { - cmod::Model* model = nullptr; std::ifstream in(fileNameStd, std::ios::in | std::ios::binary); if (!in.good()) { @@ -479,14 +463,14 @@ MainWindow::openModel(const QString& fileName) return; } - model = cmod::LoadModel(in, GetPathManager()->getHandle); + std::unique_ptr model = cmod::LoadModel(in, GetPathManager()->getHandle); if (model == nullptr) { QMessageBox::warning(this, "Load error", tr("Error reading CMOD file %1").arg(fileName)); return; } - setModel(fileName, model); + setModel(fileName, std::move(model)); } else { @@ -580,7 +564,7 @@ MainWindow::setRenderPath(QAction* action) void MainWindow::generateNormals() { - cmod::Model* model = m_modelView->model(); + const cmod::Model* model = m_modelView->model(); if (!model) { return; @@ -624,17 +608,17 @@ MainWindow::generateNormals() double weldTolerance = toleranceEdit->text().toDouble(); bool weldVertices = true; - cmod::Model* newModel = GenerateModelNormals(*model, - celmath::degToRad(smoothAngle), - weldVertices, - weldTolerance); + std::unique_ptr newModel = GenerateModelNormals(*model, + celmath::degToRad(smoothAngle), + weldVertices, + weldTolerance); if (!newModel) { QMessageBox::warning(this, tr("Internal Error"), tr("Out of memory error during normal generation")); } else { - setModel(modelFileName(), newModel); + setModel(modelFileName(), std::move(newModel)); } settings.setValue("SmoothAngle", smoothAngle); @@ -646,7 +630,7 @@ MainWindow::generateNormals() void MainWindow::generateTangents() { - cmod::Model* model = m_modelView->model(); + const cmod::Model* model = m_modelView->model(); if (!model) { return; @@ -678,32 +662,30 @@ MainWindow::generateTangents() double weldTolerance = toleranceEdit->text().toDouble(); bool weldVertices = true; - cmod::Model* newModel = new cmod::Model(); + auto newModel = std::make_unique(); // Copy materials for (unsigned int i = 0; model->getMaterial(i) != nullptr; i++) { - newModel->addMaterial(cloneMaterial(model->getMaterial(i))); + newModel->addMaterial(model->getMaterial(i)->clone()); } for (unsigned int i = 0; model->getMesh(i) != nullptr; i++) { - cmod::Mesh* mesh = model->getMesh(i); - cmod::Mesh* newMesh = nullptr; - - newMesh = GenerateTangents(*mesh, weldVertices); - if (newMesh == nullptr) + const cmod::Mesh* mesh = model->getMesh(i); + cmod::Mesh newMesh = GenerateTangents(*mesh, weldVertices); + if (newMesh.getVertexCount() == 0) { std::cerr << "Error generating normals!\n"; // TODO: Clone the mesh and add it to the model } else { - newModel->addMesh(newMesh); + newModel->addMesh(std::move(newMesh)); } } - setModel(modelFileName(), newModel); + setModel(modelFileName(), std::move(newModel)); settings.setValue("WeldTolerance", weldTolerance); } @@ -733,14 +715,13 @@ MainWindow::uniquifyVertices() void MainWindow::mergeMeshes() { - cmod::Model* model = m_modelView->model(); + const cmod::Model* model = m_modelView->model(); if (!model) { return; } - cmod::Model* newModel = MergeModelMeshes(*model); - setModel(modelFileName(), newModel); + setModel(modelFileName(), MergeModelMeshes(*model)); } @@ -754,8 +735,8 @@ MainWindow::updateSelectionInfo() else { m_materialWidget->setEnabled(true); - QSetIterator iter(m_modelView->selection()); - cmod::PrimitiveGroup* selectedGroup = iter.next(); + QSetIterator iter(m_modelView->selection()); + const cmod::PrimitiveGroup* selectedGroup = iter.next(); const cmod::Material* material = m_modelView->model()->getMaterial(selectedGroup->materialIndex); if (material) { @@ -770,8 +751,8 @@ MainWindow::changeCurrentMaterial(const cmod::Material& material) { if (!m_modelView->selection().isEmpty()) { - QSetIterator iter(m_modelView->selection()); - cmod::PrimitiveGroup* selectedGroup = iter.next(); + QSetIterator iter(m_modelView->selection()); + const cmod::PrimitiveGroup* selectedGroup = iter.next(); m_modelView->setMaterial(selectedGroup->materialIndex, material); } } diff --git a/src/tools/cmod/cmodview/mainwindow.h b/src/tools/cmod/cmodview/mainwindow.h index 1a05e87aa..63ee30586 100644 --- a/src/tools/cmod/cmodview/mainwindow.h +++ b/src/tools/cmod/cmodview/mainwindow.h @@ -10,6 +10,8 @@ #pragma once +#include + #include #include #include @@ -34,7 +36,7 @@ class MainWindow : public QMainWindow public: MainWindow(); - void setModel(const QString& filename, cmod::Model* model); + void setModel(const QString& filename, std::unique_ptr&& model); void setModelFileName(const QString& fileName); QString modelFileName() const { diff --git a/src/tools/cmod/cmodview/modelviewwidget.cpp b/src/tools/cmod/cmodview/modelviewwidget.cpp index fe36ad165..fa0deeeca 100644 --- a/src/tools/cmod/cmodview/modelviewwidget.cpp +++ b/src/tools/cmod/cmodview/modelviewwidget.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -282,18 +283,13 @@ ModelViewWidget::ModelViewWidget(QWidget *parent) : ModelViewWidget::~ModelViewWidget() { delete m_materialLibrary; - delete m_model; } void -ModelViewWidget::setModel(cmod::Model* model, const QString& modelDirPath) +ModelViewWidget::setModel(std::unique_ptr&& model, const QString& modelDirPath) { - if (m_model && m_model != model) - { - delete m_model; - } - m_model = model; + m_model = std::move(model); delete m_materialLibrary; m_materialLibrary = new MaterialLibrary(this, modelDirPath); @@ -634,12 +630,12 @@ ModelViewWidget::paintGL() if (m_model) { - renderModel(m_model); + renderModel(m_model.get()); if (!m_selection.isEmpty()) { glEnable(GL_POLYGON_OFFSET_FILL); glPolygonOffset(-0.0f, -1.0f); - renderSelection(m_model); + renderSelection(m_model.get()); } } @@ -1589,7 +1585,7 @@ ModelViewWidget::renderShadow(unsigned int lightIndex) glMatrixMode(GL_MODELVIEW); glLoadMatrixf(directionalLightMatrix(lightDirection).data()); - renderDepthOnly(m_model); + renderDepthOnly(m_model.get()); shadowBuffer->unbind(); diff --git a/src/tools/cmod/cmodview/modelviewwidget.h b/src/tools/cmod/cmodview/modelviewwidget.h index 86a6e0bdc..606aa8084 100644 --- a/src/tools/cmod/cmodview/modelviewwidget.h +++ b/src/tools/cmod/cmodview/modelviewwidget.h @@ -10,6 +10,8 @@ #pragma once +#include + #include #include #include @@ -146,11 +148,9 @@ public: ModelViewWidget(QWidget *parent); ~ModelViewWidget(); - void setModel(cmod::Model* model, const QString& modelDir); - cmod::Model* model() const - { - return m_model; - } + void setModel(std::unique_ptr&& model, const QString& modelDir); + cmod::Model* model() { return m_model.get(); } + const cmod::Model* model() const { return m_model.get(); } void resetCamera(); @@ -172,7 +172,7 @@ public: void wheelEvent(QWheelEvent* event); void select(const Eigen::Vector2f& point); - QSet selection() + QSet selection() { return m_selection; } @@ -238,7 +238,7 @@ private: GLShaderProgram* createShader(const ShaderKey& shaderKey); private: - cmod::Model* m_model; + std::unique_ptr m_model; double m_modelBoundingRadius; Eigen::Vector3d m_cameraPosition; Eigen::Quaterniond m_cameraOrientation; @@ -249,7 +249,7 @@ private: MaterialLibrary* m_materialLibrary; - QSet m_selection; + QSet m_selection; QHash m_shaderCache; QColor m_backgroundColor; diff --git a/src/tools/cmod/common/cmodops.cpp b/src/tools/cmod/common/cmodops.cpp index 763bbb4c7..51cfa86b8 100644 --- a/src/tools/cmod/common/cmodops.cpp +++ b/src/tools/cmod/common/cmodops.cpp @@ -351,28 +351,6 @@ addGroupWithOffset(cmod::Mesh& mesh, } -cmod::Material* -cloneMaterial(const cmod::Material* other) -{ - cmod::Material* material = new cmod::Material(); - material->diffuse = other->diffuse; - material->specular = other->specular; - material->emissive = other->emissive; - material->specularPower = other->specularPower; - material->opacity = other->opacity; - material->blend = other->blend; - for (int i = 0; i < static_cast(cmod::TextureSemantic::TextureSemanticMax); ++i) - { - if (other->maps[i] != InvalidResource) - { - material->maps[i] = other->maps[i]; - } - } - - return material; -} - - template void joinVertices(std::vector& faces, const cmod::VWord* vertexData, @@ -448,7 +426,7 @@ joinVertices(std::vector& faces, * @param weldTolerance maximum difference between positions that should be * considered identical during the weld step. */ -cmod::Mesh* +cmod::Mesh GenerateNormals(const cmod::Mesh& mesh, float smoothAngle, bool weld, float weldTolerance) { std::uint32_t nVertices = mesh.getVertexCount(); @@ -459,7 +437,7 @@ GenerateNormals(const cmod::Mesh& mesh, float smoothAngle, bool weld, float weld if (desc.getAttribute(cmod::VertexAttributeSemantic::Position).format != cmod::VertexAttributeFormat::Float3) { std::cerr << "Vertex position must be a float3\n"; - return nullptr; + return cmod::Mesh(); } std::uint32_t posOffset = desc.getAttribute(cmod::VertexAttributeSemantic::Position).offsetWords; @@ -477,7 +455,7 @@ GenerateNormals(const cmod::Mesh& mesh, float smoothAngle, bool weld, float weld if (group->indices.size() < 3 || group->indices.size() % 3 != 0) { std::cerr << "Triangle list has invalid number of indices\n"; - return nullptr; + return cmod::Mesh(); } nFaces += group->indices.size() / 3; break; @@ -487,14 +465,14 @@ GenerateNormals(const cmod::Mesh& mesh, float smoothAngle, bool weld, float weld if (group->indices.size() < 3) { std::cerr << "Error: tri strip or fan has less than three indices\n"; - return nullptr; + return cmod::Mesh(); } nFaces += group->indices.size() - 2; break; default: std::cerr << "Cannot generate normals for non-triangle primitives\n"; - return nullptr; + return cmod::Mesh(); } } @@ -581,15 +559,8 @@ GenerateNormals(const cmod::Mesh& mesh, float smoothAngle, bool weld, float weld } // For each vertex, create a list of faces that contain it - std::uint32_t* faceCounts = new std::uint32_t[nVertices]; - std::uint32_t** vertexFaces = new std::uint32_t*[nVertices]; - - // Initialize the lists - for (i = 0; i < nVertices; i++) - { - faceCounts[i] = 0; - vertexFaces[i] = nullptr; - } + std::vector faceCounts(nVertices, 0); + std::vector> vertexFaces(nVertices); // If we're welding vertices before generating normals, find identical // points and merge them. Otherwise, the point indices will be the same @@ -624,7 +595,7 @@ GenerateNormals(const cmod::Mesh& mesh, float smoothAngle, bool weld, float weld { if (faceCounts[i] > 0) { - vertexFaces[i] = new uint32_t[faceCounts[i] + 1]; + vertexFaces[i].resize(faceCounts[i] + 1); vertexFaces[i][0] = faceCounts[i]; } } @@ -708,9 +679,9 @@ GenerateNormals(const cmod::Mesh& mesh, float smoothAngle, bool weld, float weld } // Create the Celestia mesh - cmod::Mesh* newMesh = new cmod::Mesh(); - newMesh->setVertexDescription(std::move(newDesc)); - newMesh->setVertices(nFaces * 3, std::move(newVertexData)); + cmod::Mesh newMesh; + newMesh.setVertexDescription(std::move(newDesc)); + newMesh.setVertices(nFaces * 3, std::move(newVertexData)); std::uint32_t firstIndex = 0; for (std::uint32_t groupIndex = 0; mesh.getGroup(groupIndex) != 0; ++groupIndex) @@ -736,26 +707,17 @@ GenerateNormals(const cmod::Mesh& mesh, float smoothAngle, bool weld, float weld std::vector indices(faceCount * 3); std::iota(indices.begin(), indices.end(), firstIndex); - newMesh->addGroup(cmod::PrimitiveGroupType::TriList, - mesh.getGroup(groupIndex)->materialIndex, - std::move(indices)); + newMesh.addGroup(cmod::PrimitiveGroupType::TriList, + mesh.getGroup(groupIndex)->materialIndex, + std::move(indices)); firstIndex += faceCount * 3; } - // Clean up - delete[] faceCounts; - for (i = 0; i < nVertices; i++) - { - if (vertexFaces[i] != nullptr) - delete[] vertexFaces[i]; - } - delete[] vertexFaces; - return newMesh; } -cmod::Mesh* +cmod::Mesh GenerateTangents(const cmod::Mesh& mesh, bool weld) { uint32_t nVertices = mesh.getVertexCount(); @@ -766,25 +728,25 @@ GenerateTangents(const cmod::Mesh& mesh, bool weld) if (desc.getAttribute(cmod::VertexAttributeSemantic::Position).format != cmod::VertexAttributeFormat::Float3) { std::cerr << "Vertex position must be a float3\n"; - return nullptr; + return cmod::Mesh(); } if (desc.getAttribute(cmod::VertexAttributeSemantic::Normal).format != cmod::VertexAttributeFormat::Float3) { std::cerr << "float3 format vertex normal required\n"; - return nullptr; + return cmod::Mesh(); } if (desc.getAttribute(cmod::VertexAttributeSemantic::Texture0).format == cmod::VertexAttributeFormat::InvalidFormat) { std::cerr << "Texture coordinates must be present in mesh to generate tangents\n"; - return nullptr; + return cmod::Mesh(); } if (desc.getAttribute(cmod::VertexAttributeSemantic::Texture0).format != cmod::VertexAttributeFormat::Float2) { std::cerr << "Texture coordinate must be a float2\n"; - return nullptr; + return cmod::Mesh(); } // Count the number of faces in the mesh. @@ -802,7 +764,7 @@ GenerateTangents(const cmod::Mesh& mesh, bool weld) else { std::cerr << "Mesh should contain just triangle lists\n"; - return nullptr; + return cmod::Mesh(); } } @@ -987,9 +949,9 @@ GenerateTangents(const cmod::Mesh& mesh, bool weld) } // Create the Celestia mesh - cmod::Mesh* newMesh = new cmod::Mesh(); - newMesh->setVertexDescription(std::move(newDesc)); - newMesh->setVertices(nFaces * 3, std::move(newVertexData)); + cmod::Mesh newMesh; + newMesh.setVertexDescription(std::move(newDesc)); + newMesh.setVertices(nFaces * 3, std::move(newVertexData)); std::uint32_t firstIndex = 0; for (std::uint32_t groupIndex = 0; mesh.getGroup(groupIndex) != 0; ++groupIndex) @@ -1015,9 +977,9 @@ GenerateTangents(const cmod::Mesh& mesh, bool weld) std::vector indices(faceCount * 3); std::iota(indices.begin(), indices.end(), firstIndex); - newMesh->addGroup(cmod::PrimitiveGroupType::TriList, - mesh.getGroup(groupIndex)->materialIndex, - std::move(indices)); + newMesh.addGroup(cmod::PrimitiveGroupType::TriList, + mesh.getGroup(groupIndex)->materialIndex, + std::move(indices)); firstIndex += faceCount * 3; } @@ -1100,10 +1062,10 @@ UniquifyVertices(cmod::Mesh& mesh) // Merge all meshes that share the same vertex description -cmod::Model* +std::unique_ptr MergeModelMeshes(const cmod::Model& model) { - std::vector meshes; + std::vector meshes; for (std::uint32_t i = 0; model.getMesh(i) != nullptr; i++) { @@ -1114,12 +1076,12 @@ MergeModelMeshes(const cmod::Model& model) std::sort(meshes.begin(), meshes.end(), [](const cmod::Mesh* a, const cmod::Mesh* b) { return a->getVertexDescription() < b->getVertexDescription(); }); - cmod::Model* newModel = new cmod::Model(); + auto newModel = std::make_unique(); // Copy materials into the new model for (std::uint32_t i = 0; model.getMaterial(i) != nullptr; i++) { - newModel->addMaterial(model.getMaterial(i)); + newModel->addMaterial(model.getMaterial(i)->clone()); } std::uint32_t meshIndex = 0; @@ -1162,9 +1124,9 @@ MergeModelMeshes(const cmod::Model& model) } // Create the new empty mesh - cmod::Mesh* mergedMesh = new cmod::Mesh(); - mergedMesh->setVertexDescription(desc.clone()); - mergedMesh->setVertices(totalVertices, std::move(vertexData)); + cmod::Mesh mergedMesh; + mergedMesh.setVertexDescription(desc.clone()); + mergedMesh.setVertices(totalVertices, std::move(vertexData)); // Reindex and add primitive groups vertexCount = 0; @@ -1173,7 +1135,7 @@ MergeModelMeshes(const cmod::Model& model) const cmod::Mesh* mesh = meshes[j]; for (std::uint32_t k = 0; mesh->getGroup(k) != nullptr; k++) { - addGroupWithOffset(*mergedMesh, *mesh->getGroup(k), + addGroupWithOffset(mergedMesh, *mesh->getGroup(k), vertexCount); } @@ -1181,7 +1143,7 @@ MergeModelMeshes(const cmod::Model& model) } assert(vertexCount == totalVertices); - newModel->addMesh(mergedMesh); + newModel->addMesh(std::move(mergedMesh)); meshIndex += nMatchingMeshes; } @@ -1193,31 +1155,29 @@ MergeModelMeshes(const cmod::Model& model) /*! Generate normals for an entire model. Return the new model, or null if * normal generation failed due to an out of memory error. */ -cmod::Model* +std::unique_ptr GenerateModelNormals(const cmod::Model& model, float smoothAngle, bool weldVertices, float weldTolerance) { - cmod::Model* newModel = new cmod::Model(); + auto newModel = std::make_unique(); // Copy materials for (unsigned int i = 0; model.getMaterial(i) != nullptr; i++) { - newModel->addMaterial(cloneMaterial(model.getMaterial(i))); + newModel->addMaterial(model.getMaterial(i)->clone()); } bool ok = true; for (unsigned int i = 0; model.getMesh(i) != nullptr; i++) { - cmod::Mesh* mesh = model.getMesh(i); - cmod::Mesh* newMesh = nullptr; - - newMesh = GenerateNormals(*mesh, smoothAngle, weldVertices, weldTolerance); - if (newMesh == nullptr) + const cmod::Mesh* mesh = model.getMesh(i); + cmod::Mesh newMesh = GenerateNormals(*mesh, smoothAngle, weldVertices, weldTolerance); + if (newMesh.getVertexCount() == 0) { ok = false; } else { - newModel->addMesh(newMesh); + newModel->addMesh(std::move(newMesh)); } } @@ -1226,8 +1186,7 @@ GenerateModelNormals(const cmod::Model& model, float smoothAngle, bool weldVerti // If all of the meshes weren't processed due to an out of memory error, // delete the new model and return nullptr rather than a partially processed // model. - delete newModel; - newModel = nullptr; + return nullptr; } return newModel; diff --git a/src/tools/cmod/common/cmodops.h b/src/tools/cmod/common/cmodops.h index 092da8cfc..2a7b05336 100644 --- a/src/tools/cmod/common/cmodops.h +++ b/src/tools/cmod/common/cmodops.h @@ -12,6 +12,7 @@ #pragma once #include +#include #include @@ -25,13 +26,16 @@ class Model; // Mesh operations -extern cmod::Mesh* GenerateNormals(const cmod::Mesh& mesh, float smoothAngle, bool weld, float weldTolerance = 0.0f); -extern cmod::Mesh* GenerateTangents(const cmod::Mesh& mesh, bool weld); +extern cmod::Mesh GenerateNormals(const cmod::Mesh& mesh, float smoothAngle, bool weld, float weldTolerance = 0.0f); +extern cmod::Mesh GenerateTangents(const cmod::Mesh& mesh, bool weld); extern bool UniquifyVertices(cmod::Mesh& mesh); // Model operations -extern cmod::Model* MergeModelMeshes(const cmod::Model& model); -extern cmod::Model* GenerateModelNormals(const cmod::Model& model, float smoothAngle, bool weldVertices, float weldTolerance); +extern std::unique_ptr MergeModelMeshes(const cmod::Model& model); +extern std::unique_ptr GenerateModelNormals(const cmod::Model& model, + float smoothAngle, + bool weldVertices, + float weldTolerance); #ifdef TRISTRIP extern bool ConvertToStrips(cmod::Mesh& mesh); #endif diff --git a/src/tools/cmod/common/convert3ds.cpp b/src/tools/cmod/common/convert3ds.cpp index 7efc34389..16202dec8 100644 --- a/src/tools/cmod/common/convert3ds.cpp +++ b/src/tools/cmod/common/convert3ds.cpp @@ -23,17 +23,18 @@ namespace { -cmod::Material* + +cmod::Material convert3dsMaterial(const M3DMaterial* material3ds, cmod::HandleGetter& handleGetter) { - cmod::Material* newMaterial = new cmod::Material(); + cmod::Material newMaterial; M3DColor diffuse = material3ds->getDiffuseColor(); - newMaterial->diffuse = cmod::Color(diffuse.red, diffuse.green, diffuse.blue); - newMaterial->opacity = material3ds->getOpacity(); + newMaterial.diffuse = cmod::Color(diffuse.red, diffuse.green, diffuse.blue); + newMaterial.opacity = material3ds->getOpacity(); M3DColor specular = material3ds->getSpecularColor(); - newMaterial->specular = cmod::Color(specular.red, specular.green, specular.blue); + newMaterial.specular = cmod::Color(specular.red, specular.green, specular.blue); float shininess = material3ds->getShininess(); @@ -41,24 +42,22 @@ convert3dsMaterial(const M3DMaterial* material3ds, cmod::HandleGetter& handleGet // range that OpenGL uses for the specular exponent. The // current equation is just a guess at the mapping that // 3DS actually uses. - newMaterial->specularPower = std::pow(2.0f, 1.0f + 0.1f * shininess); - if (newMaterial->specularPower > 128.0f) - newMaterial->specularPower = 128.0f; + newMaterial.specularPower = std::pow(2.0f, 1.0f + 0.1f * shininess); + if (newMaterial.specularPower > 128.0f) + newMaterial.specularPower = 128.0f; if (!material3ds->getTextureMap().empty()) { - newMaterial->setMap(cmod::TextureSemantic::DiffuseMap, handleGetter(material3ds->getTextureMap())); + newMaterial.setMap(cmod::TextureSemantic::DiffuseMap, handleGetter(material3ds->getTextureMap())); } return newMaterial; } + } // end unnamed namespace - - - void Convert3DSMesh(cmod::Model& model, const M3DTriangleMesh& mesh3ds, @@ -108,11 +107,11 @@ Convert3DSMesh(cmod::Model& model, } // Create the Celestia mesh - cmod::Mesh* mesh = new cmod::Mesh(); - mesh->setVertexDescription(cmod::VertexDescription(std::move(attributes))); - mesh->setVertices(nVertices, std::move(vertices)); + cmod::Mesh mesh; + mesh.setVertexDescription(cmod::VertexDescription(std::move(attributes))); + mesh.setVertices(nVertices, std::move(vertices)); - mesh->setName(std::move(meshName)); + mesh.setName(std::move(meshName)); if (mesh3ds.getMeshMaterialGroupCount() == 0) { @@ -131,7 +130,7 @@ Convert3DSMesh(cmod::Model& model, indices.push_back(v2); } - mesh->addGroup(cmod::PrimitiveGroupType::TriList, ~0, std::move(indices)); + mesh.addGroup(cmod::PrimitiveGroupType::TriList, ~0, std::move(indices)); } else { @@ -170,18 +169,18 @@ Convert3DSMesh(cmod::Model& model, } } - mesh->addGroup(cmod::PrimitiveGroupType::TriList, materialIndex, std::move(indices)); + mesh.addGroup(cmod::PrimitiveGroupType::TriList, materialIndex, std::move(indices)); } } - model.addMesh(mesh); + model.addMesh(std::move(mesh)); } -cmod::Model* +std::unique_ptr Convert3DSModel(const M3DScene& scene, cmod::HandleGetter handleGetter) { - cmod::Model* model = new cmod::Model(); + auto model = std::make_unique(); // Convert materials for (unsigned int i = 0; i < scene.getMaterialCount(); i++) diff --git a/src/tools/cmod/common/convert3ds.h b/src/tools/cmod/common/convert3ds.h index 45ed60077..e09474665 100644 --- a/src/tools/cmod/common/convert3ds.h +++ b/src/tools/cmod/common/convert3ds.h @@ -11,6 +11,7 @@ #pragma once +#include #include #include @@ -22,5 +23,6 @@ extern void Convert3DSMesh(cmod::Model& model, M3DTriangleMesh& mesh3ds, const M3DScene& scene, std::string&& meshName); -extern cmod::Model* Convert3DSModel(const M3DScene& scene, - cmod::HandleGetter handleGetter); + +extern std::unique_ptr Convert3DSModel(const M3DScene& scene, + cmod::HandleGetter handleGetter); diff --git a/src/tools/cmod/common/convertobj.cpp b/src/tools/cmod/common/convertobj.cpp index 0da67406e..78ffa8dba 100644 --- a/src/tools/cmod/common/convertobj.cpp +++ b/src/tools/cmod/common/convertobj.cpp @@ -17,7 +17,6 @@ #include #include -#include #include "convertobj.h" @@ -78,7 +77,7 @@ WavefrontLoader::WavefrontLoader(std::istream& in) : } -cmod::Model* +std::unique_ptr WavefrontLoader::load() { std::string line; @@ -87,7 +86,7 @@ WavefrontLoader::load() ObjVertex::VertexType lastVertexType = ObjVertex::Point; int currentMaterialIndex = -1; - m_model = new cmod::Model(); + m_model = std::make_unique(); while (std::getline(m_in, line)) { @@ -135,12 +134,12 @@ WavefrontLoader::load() } else if (keyword == "usemtl") { - cmod::Material* material = new cmod::Material(); - material->diffuse = cmod::Color(1.0f, 1.0f, 1.0f); - currentMaterialIndex = m_model->addMaterial(material) - 1; + cmod::Material material; + material.diffuse = cmod::Color(1.0f, 1.0f, 1.0f); + currentMaterialIndex = m_model->addMaterial(std::move(material)) - 1; if (!m_materialGroups.empty()) { - if (m_materialGroups.back().firstIndex == (int) m_indexData.size()) + if (m_materialGroups.back().firstIndex == static_cast(m_indexData.size())) { m_materialGroups.back().materialIndex = currentMaterialIndex; } @@ -293,7 +292,7 @@ WavefrontLoader::load() createMesh(lastVertexType, vertexCount); } - return m_model; + return std::move(m_model); } @@ -305,7 +304,6 @@ WavefrontLoader::reportError(const std::string& message) os << "Line " << m_lineNumber << ": " << message; m_errorMessage = os.str(); - delete m_model; m_model = nullptr; } @@ -360,9 +358,9 @@ WavefrontLoader::createMesh(ObjVertex::VertexType vertexType, unsigned int verte std::memcpy(vertexDataCopy.data(), m_vertexData.data(), m_vertexData.size() * sizeof(cmod::VWord)); // Create the Celestia mesh - cmod::Mesh* mesh = new cmod::Mesh(); - mesh->setVertexDescription(cmod::VertexDescription(std::move(attributes))); - mesh->setVertices(vertexCount, std::move(vertexDataCopy)); + cmod::Mesh mesh; + mesh.setVertexDescription(cmod::VertexDescription(std::move(attributes))); + mesh.setVertices(vertexCount, std::move(vertexDataCopy)); // Add primitive groups for (unsigned int i = 0; i < m_materialGroups.size(); ++i) @@ -381,9 +379,9 @@ WavefrontLoader::createMesh(ObjVertex::VertexType vertexType, unsigned int verte if (indexCount > 0) { auto copyStart = m_indexData.begin() + firstIndex; - mesh->addGroup(cmod::PrimitiveGroupType::TriList, - m_materialGroups[i].materialIndex, - std::vector(copyStart, copyStart + indexCount)); + mesh.addGroup(cmod::PrimitiveGroupType::TriList, + m_materialGroups[i].materialIndex, + std::vector(copyStart, copyStart + indexCount)); } } @@ -391,5 +389,5 @@ WavefrontLoader::createMesh(ObjVertex::VertexType vertexType, unsigned int verte m_indexData.clear(); m_materialGroups.clear(); - m_model->addMesh(mesh); + m_model->addMesh(std::move(mesh)); } diff --git a/src/tools/cmod/common/convertobj.h b/src/tools/cmod/common/convertobj.h index a80815537..18e9442de 100644 --- a/src/tools/cmod/common/convertobj.h +++ b/src/tools/cmod/common/convertobj.h @@ -13,15 +13,13 @@ #pragma once #include +#include #include #include #include -namespace cmod -{ -class Model; -} +#include class WavefrontLoader @@ -30,7 +28,7 @@ public: WavefrontLoader(std::istream& in); ~WavefrontLoader() = default; - cmod::Model* load(); + std::unique_ptr load(); std::string errorMessage() const { return m_errorMessage; @@ -100,7 +98,7 @@ private: std::vector m_indexData; std::vector m_materialGroups; - cmod::Model* m_model; + std::unique_ptr m_model; std::string m_errorMessage; }; diff --git a/test/integration/cmod_bin_ascii_roundtrip_test.cpp b/test/integration/cmod_bin_ascii_roundtrip_test.cpp index 16d69fdb6..bf87c67bf 100644 --- a/test/integration/cmod_bin_ascii_roundtrip_test.cpp +++ b/test/integration/cmod_bin_ascii_roundtrip_test.cpp @@ -2,12 +2,14 @@ #include #include #include +#include #include #include #include #include +#include #include #include @@ -35,17 +37,17 @@ TEST_CASE("CMOD binary to ASCII roundtrip", "[cmod] [integration]") std::stringstream sourceData; sourceData << f.rdbuf(); - cmod::Model* modelFromBinary = cmod::LoadModel(sourceData, handleGetter); + std::unique_ptr modelFromBinary = cmod::LoadModel(sourceData, handleGetter); REQUIRE(modelFromBinary != nullptr); std::stringstream asciiData; - REQUIRE(cmod::SaveModelAscii(modelFromBinary, asciiData, sourceGetter)); + REQUIRE(cmod::SaveModelAscii(modelFromBinary.get(), asciiData, sourceGetter)); - cmod::Model* modelFromAscii = cmod::LoadModel(asciiData, handleGetter); + std::unique_ptr modelFromAscii = cmod::LoadModel(asciiData, handleGetter); REQUIRE(modelFromAscii != nullptr); std::stringstream roundtrippedData; - REQUIRE(cmod::SaveModelBinary(modelFromAscii, roundtrippedData, sourceGetter)); + REQUIRE(cmod::SaveModelBinary(modelFromAscii.get(), roundtrippedData, sourceGetter)); sourceData.clear(); REQUIRE(sourceData.seekg(0, std::ios_base::beg).good());