diff --git a/graphics/src/AssimpLoader_TEST.cc b/graphics/src/AssimpLoader_TEST.cc index 5407a595..40a99f7f 100644 --- a/graphics/src/AssimpLoader_TEST.cc +++ b/graphics/src/AssimpLoader_TEST.cc @@ -49,6 +49,7 @@ TEST_F(AssimpLoader, LoadBox) // Make sure we can read a submesh name EXPECT_STREQ("Cube", mesh->SubMeshByIndex(0).lock()->Name().c_str()); + delete mesh; } ///////////////////////////////////////////////// @@ -104,6 +105,8 @@ TEST_F(AssimpLoader, Material) matOpaque->BlendFactors(srcFactor, dstFactor); EXPECT_DOUBLE_EQ(1.0, srcFactor); EXPECT_DOUBLE_EQ(0.0, dstFactor); + delete mesh; + delete meshOpaque; } ///////////////////////////////////////////////// @@ -155,6 +158,7 @@ TEST_F(AssimpLoader, ShareVertices) } } } + delete mesh; } ///////////////////////////////////////////////// @@ -164,6 +168,7 @@ TEST_F(AssimpLoader, LoadZeroCount) common::Mesh *mesh = loader.Load( common::testing::TestFile("data", "zero_count.dae")); ASSERT_TRUE(mesh); + delete mesh; } ///////////////////////////////////////////////// @@ -251,6 +256,7 @@ TEST_F(AssimpLoader, TexCoordSets) subMeshB->SetTexCoordBySet(2u, math::Vector2d(0.1, 0.2), 1u); EXPECT_EQ(math::Vector2d(0.1, 0.2), subMeshB->TexCoordBySet(2u, 1u)); + delete mesh; } // Test fails for assimp below 5.2.0 @@ -289,6 +295,7 @@ TEST_F(AssimpLoader, LoadBoxWithAnimationOutsideSkeleton) 0, 0, 1, 0, 0, 0, 0, 1); EXPECT_EQ(expectedTrans, poseEnd.at("Armature")); + delete mesh; } #endif @@ -308,6 +315,7 @@ TEST_F(AssimpLoader, LoadBoxInstControllerWithoutSkeleton) common::SkeletonPtr skeleton = mesh->MeshSkeleton(); EXPECT_LT(0u, skeleton->NodeCount()); EXPECT_NE(nullptr, skeleton->NodeById("Armature_Bone")); + delete mesh; } ///////////////////////////////////////////////// @@ -335,6 +343,7 @@ TEST_F(AssimpLoader, LoadBoxMultipleInstControllers) common::SkeletonPtr skeleton = mesh->MeshSkeleton(); EXPECT_LT(0u, skeleton->NodeCount()); EXPECT_NE(nullptr, skeleton->NodeById("Armature_Bone")); + delete mesh; } ///////////////////////////////////////////////// @@ -374,6 +383,7 @@ TEST_F(AssimpLoader, LoadBoxNestedAnimation) 0, 0, 1, 0, 0, 0, 0, 1); EXPECT_EQ(expectedTrans, poseEnd.at("Armature_Bone")); + delete mesh; } ///////////////////////////////////////////////// @@ -392,6 +402,7 @@ TEST_F(AssimpLoader, LoadBoxWithDefaultStride) ASSERT_NE(mesh->MeshSkeleton(), nullptr); // TODO(luca) not working, investigate // ASSERT_EQ(1u, mesh->MeshSkeleton()->AnimationCount()); + delete mesh; } ///////////////////////////////////////////////// @@ -409,6 +420,7 @@ TEST_F(AssimpLoader, LoadBoxWithMultipleGeoms) ASSERT_EQ(2u, mesh->SubMeshCount()); EXPECT_EQ(24u, mesh->SubMeshByIndex(0).lock()->NodeAssignmentsCount()); EXPECT_EQ(0u, mesh->SubMeshByIndex(1).lock()->NodeAssignmentsCount()); + delete mesh; } ///////////////////////////////////////////////// @@ -435,6 +447,7 @@ TEST_F(AssimpLoader, LoadBoxWithHierarchicalNodes) // nested node with name EXPECT_EQ("StaticCubeNested", mesh->SubMeshByIndex(4).lock()->Name()); + delete mesh; } ///////////////////////////////////////////////// @@ -448,6 +461,7 @@ TEST_F(AssimpLoader, MergeBoxWithDoubleSkeleton) // The two skeletons have been joined and their root is the // animation root, called Scene EXPECT_EQ(skeleton_ptr->RootNode()->Name(), std::string("Scene")); + delete mesh; } // For assimp below 5.2.0 mesh loading fails because of @@ -487,6 +501,7 @@ TEST_F(AssimpLoader, LoadCylinderAnimatedFrom3dsMax) // EXPECT_EQ("Bone02", anim->Name()); EXPECT_EQ(1u, anim->NodeCount()); EXPECT_TRUE(anim->HasNode("Bone02")); + delete mesh; } #endif @@ -521,6 +536,7 @@ TEST_F(AssimpLoader, LoadObjBox) EXPECT_EQ(mat->Diffuse(), math::Color(0.512f, 0.512f, 0.512f, 1.0f)); EXPECT_EQ(mat->Specular(), math::Color(0.25, 0.25, 0.25, 1.0)); EXPECT_DOUBLE_EQ(mat->Transparency(), 0.0); + delete mesh; } @@ -536,6 +552,7 @@ TEST_F(AssimpLoader, ObjInvalidMaterial) common::Mesh *mesh = loader.Load(meshFilename); EXPECT_TRUE(mesh != nullptr); + delete mesh; } ///////////////////////////////////////////////// @@ -550,6 +567,7 @@ TEST_F(AssimpLoader, NonExistingMesh) common::Mesh *mesh = loader.Load(meshFilename); EXPECT_EQ(mesh->SubMeshCount(), 0); + delete mesh; } ///////////////////////////////////////////////// @@ -584,6 +602,7 @@ TEST_F(AssimpLoader, LoadFbxBox) EXPECT_EQ(mat->Diffuse(), math::Color(0.8f, 0.8f, 0.8f, 1.0f)); EXPECT_EQ(mat->Specular(), math::Color(0.8f, 0.8f, 0.8f, 1.0f)); EXPECT_DOUBLE_EQ(mat->Transparency(), 0.0); + delete mesh; } ///////////////////////////////////////////////// @@ -618,6 +637,7 @@ TEST_F(AssimpLoader, LoadGlTF2Box) EXPECT_EQ(mat->Diffuse(), math::Color(0.8f, 0.8f, 0.8f, 1.0f)); EXPECT_EQ(mat->Specular(), math::Color(0.0f, 0.0f, 0.0f, 1.0f)); EXPECT_DOUBLE_EQ(mat->Transparency(), 0.0); + delete mesh; } ///////////////////////////////////////////////// @@ -642,6 +662,7 @@ TEST_F(AssimpLoader, LoadGlTF2BoxExternalTexture) auto testTextureFile = common::testing::TestFile("data/gltf", "PurpleCube_Diffuse.png"); EXPECT_EQ(testTextureFile, mat->TextureImage()); + delete mesh; } ///////////////////////////////////////////////// @@ -681,6 +702,7 @@ TEST_F(AssimpLoader, LoadGlTF2BoxWithJPEGTexture) EXPECT_EQ("Scene_Material_Diffuse", mat->TextureImage()); #endif EXPECT_NE(nullptr, mat->TextureData()); + delete mesh; } ///////////////////////////////////////////////// @@ -733,7 +755,6 @@ TEST_F(AssimpLoader, LoadGlbPbrAsset) EXPECT_EQ(img->Pixel(0, 0), math::Color(0.0f, 0.0f, 0.0f, 1.0f)); EXPECT_EQ(img->Pixel(100, 100), math::Color(1.0f, 1.0f, 1.0f, 1.0f)); - EXPECT_NE(pbr->NormalMapData(), nullptr); // Metallic roughness and alpha from textures only works in assimp > 5.2.0 #ifndef GZ_ASSIMP_PRE_5_2_0 @@ -765,6 +786,7 @@ TEST_F(AssimpLoader, LoadGlbPbrAsset) EXPECT_STREQ("Action1", skel->Animation(0)->Name().c_str()); EXPECT_STREQ("Action2", skel->Animation(1)->Name().c_str()); EXPECT_STREQ("Action3", skel->Animation(2)->Name().c_str()); + delete mesh; } ///////////////////////////////////////////////// @@ -782,6 +804,7 @@ TEST_F(AssimpLoader, CheckNonRootDisplacement) EXPECT_EQ(nullptr, rootNode); auto xDisplacement = skelAnim->XDisplacement(); ASSERT_TRUE(xDisplacement); + delete mesh; } ///////////////////////////////////////////////// @@ -825,4 +848,5 @@ TEST_F(AssimpLoader, LoadGLTF2Triangle) EXPECT_EQ(math::Vector2d(0, 1), subMeshB->TexCoord(0u)); EXPECT_EQ(math::Vector2d(0, 1), subMeshB->TexCoord(1u)); EXPECT_EQ(math::Vector2d(0, 1), subMeshB->TexCoord(2u)); + delete mesh; } diff --git a/graphics/src/ColladaExporter_TEST.cc b/graphics/src/ColladaExporter_TEST.cc index a6c7eb13..5b4851d2 100644 --- a/graphics/src/ColladaExporter_TEST.cc +++ b/graphics/src/ColladaExporter_TEST.cc @@ -112,6 +112,8 @@ TEST_F(ColladaExporter, ExportBox) meshReloaded->SubMeshByIndex(i).lock()->TexCoord(j)); } } + delete meshOriginal; + delete meshReloaded; } ///////////////////////////////////////////////// @@ -206,6 +208,8 @@ TEST_F(ColladaExporter, ExportCordlessDrill) meshReloaded->SubMeshByIndex(i).lock()->TexCoord(j)); } } + delete meshOriginal; + delete meshReloaded; } ///////////////////////////////////////////////// @@ -319,6 +323,10 @@ TEST_F(ColladaExporter, ExportMeshWithSubmeshes) EXPECT_EQ(outMesh.NormalCount(), meshReloaded->NormalCount()); EXPECT_EQ(outMesh.TexCoordCount(), meshReloaded->TexCoordCount()); + + delete boxMesh; + delete drillMesh; + delete meshReloaded; } TEST_F(ColladaExporter, ExportLights) @@ -448,4 +456,5 @@ TEST_F(ColladaExporter, ExportLights) } } EXPECT_EQ(node_with_light_count, 3); + delete meshOriginal; } diff --git a/graphics/src/ColladaLoader.cc b/graphics/src/ColladaLoader.cc index cbcb0bf7..2040222e 100644 --- a/graphics/src/ColladaLoader.cc +++ b/graphics/src/ColladaLoader.cc @@ -366,9 +366,7 @@ ColladaLoader::ColladaLoader() } ////////////////////////////////////////////////// -ColladaLoader::~ColladaLoader() -{ -} +ColladaLoader::~ColladaLoader() = default; ////////////////////////////////////////////////// Mesh *ColladaLoader::Load(const std::string &_filename) @@ -772,13 +770,16 @@ void ColladaLoader::Implementation::LoadController( { SkeletonNode *rootSkelNode = this->LoadSkeletonNodes(rootNodeXml, nullptr); + if (nullptr == skeleton) { - skeleton = SkeletonPtr(new Skeleton(rootSkelNode)); + skeleton = std::make_shared(rootSkelNode); _mesh->SetSkeleton(skeleton); } else if (nullptr != rootSkelNode) + { this->MergeSkeleton(skeleton, rootSkelNode); + } } if (nullptr == skeleton) { @@ -1191,7 +1192,7 @@ SkeletonNode *ColladaLoader::Implementation::LoadSkeletonNodes( return nullptr; } - auto node = this->LoadSingleSkeletonNode(_xml, _parent); + auto *node = this->LoadSingleSkeletonNode(_xml, _parent); this->SetSkeletonNodeTransform(_xml, node); auto childXml = _xml->FirstChildElement("node"); @@ -2800,8 +2801,14 @@ void ColladaLoader::Implementation::MergeSkeleton(SkeletonPtr _skeleton, if (currentRoot->Id() == _mergeNode->Id()) return; - if (_mergeNode->ChildById(currentRoot->Id())) + auto *rootInMergeNode = _mergeNode->ChildById(currentRoot->Id()); + + if (rootInMergeNode != nullptr) { + if (rootInMergeNode != currentRoot) + { + delete currentRoot; + } _skeleton->RootNode(_mergeNode); return; } @@ -2822,8 +2829,21 @@ void ColladaLoader::Implementation::MergeSkeleton(SkeletonPtr _skeleton, } if (mergeNodeContainsRoot) { + std::function delete_children; + delete_children = + [&delete_children](SkeletonNode *node)-> void { + for (size_t ii = 0; ii < node->ChildCount(); ++ii) + { + auto *child = node->Child(ii); + delete_children(child); + delete child; + } + }; + + // Since we are replacing the whole tree, recursively clean up + // the existing skeleton nodes _skeleton->RootNode(_mergeNode); - // TODO(anyone) since we are replacing the whole tree delete the old one + delete_children(currentRoot); delete currentRoot; return; } @@ -2834,6 +2854,7 @@ void ColladaLoader::Implementation::MergeSkeleton(SkeletonPtr _skeleton, dummyRoot = new SkeletonNode(nullptr, "dummy-root", "dummy-root"); } + if (dummyRoot != currentRoot) { dummyRoot->AddChild(currentRoot); diff --git a/graphics/src/ColladaLoader_TEST.cc b/graphics/src/ColladaLoader_TEST.cc index 63e0fe75..e8ea61f8 100644 --- a/graphics/src/ColladaLoader_TEST.cc +++ b/graphics/src/ColladaLoader_TEST.cc @@ -49,6 +49,7 @@ TEST_F(ColladaLoader, LoadBox) // Make sure we can read a submesh name EXPECT_STREQ("Cube", mesh->SubMeshByIndex(0).lock()->Name().c_str()); + delete mesh; } ///////////////////////////////////////////////// @@ -100,6 +101,7 @@ TEST_F(ColladaLoader, ShareVertices) } } } + delete mesh; } ///////////////////////////////////////////////// @@ -124,6 +126,7 @@ TEST_F(ColladaLoader, LoadZeroCount) EXPECT_NE(log.find("Normal source has a float_array with a count of zero"), std::string::npos); #endif + delete mesh; } ///////////////////////////////////////////////// @@ -179,6 +182,9 @@ TEST_F(ColladaLoader, Material) matOpaque->BlendFactors(srcFactor, dstFactor); EXPECT_DOUBLE_EQ(1.0, srcFactor); EXPECT_DOUBLE_EQ(0.0, dstFactor); + + delete mesh; + delete meshOpaque; } ///////////////////////////////////////////////// @@ -266,6 +272,7 @@ TEST_F(ColladaLoader, TexCoordSets) subMeshB->SetTexCoordBySet(2u, math::Vector2d(0.1, 0.2), 1u); EXPECT_EQ(math::Vector2d(0.1, 0.2), subMeshB->TexCoordBySet(2u, 1u)); + delete mesh; } ///////////////////////////////////////////////// @@ -302,6 +309,7 @@ TEST_F(ColladaLoader, LoadBoxWithAnimationOutsideSkeleton) 0, 0, 1, 0, 0, 0, 0, 1); EXPECT_EQ(expectedTrans, poseEnd.at("Armature")); + delete mesh; } ///////////////////////////////////////////////// @@ -320,6 +328,7 @@ TEST_F(ColladaLoader, LoadBoxInstControllerWithoutSkeleton) common::SkeletonPtr skeleton = mesh->MeshSkeleton(); EXPECT_LT(0u, skeleton->NodeCount()); EXPECT_NE(nullptr, skeleton->NodeById("Armature_Bone")); + delete mesh; } ///////////////////////////////////////////////// @@ -347,6 +356,7 @@ TEST_F(ColladaLoader, LoadBoxMultipleInstControllers) common::SkeletonPtr skeleton = mesh->MeshSkeleton(); EXPECT_NE(nullptr, skeleton->NodeById("Armature_Bone")); EXPECT_NE(nullptr, skeleton->NodeById("Armature_Bone2")); + delete mesh; } ///////////////////////////////////////////////// @@ -383,6 +393,7 @@ TEST_F(ColladaLoader, LoadBoxNestedAnimation) 0, 0, 1, 0, 0, 0, 0, 1); EXPECT_EQ(expectedTrans, poseEnd.at("Bone")); + delete mesh; } ///////////////////////////////////////////////// @@ -398,6 +409,7 @@ TEST_F(ColladaLoader, LoadBoxWithDefaultStride) EXPECT_EQ(1u, mesh->MaterialCount()); EXPECT_EQ(35u, mesh->TexCoordCount()); ASSERT_EQ(1u, mesh->MeshSkeleton()->AnimationCount()); + delete mesh; } ///////////////////////////////////////////////// @@ -415,6 +427,7 @@ TEST_F(ColladaLoader, LoadBoxWithMultipleGeoms) ASSERT_EQ(2u, mesh->SubMeshCount()); EXPECT_EQ(24u, mesh->SubMeshByIndex(0).lock()->NodeAssignmentsCount()); EXPECT_EQ(0u, mesh->SubMeshByIndex(1).lock()->NodeAssignmentsCount()); + delete mesh; } ///////////////////////////////////////////////// @@ -440,6 +453,7 @@ TEST_F(ColladaLoader, LoadBoxWithHierarchicalNodes) // Parent of nested node with name EXPECT_EQ("StaticCubeParent2", mesh->SubMeshByIndex(4).lock()->Name()); + delete mesh; } ///////////////////////////////////////////////// @@ -453,6 +467,7 @@ TEST_F(ColladaLoader, MergeBoxWithDoubleSkeleton) // The two skeletons have been joined and their root is the // animation root, called Armature EXPECT_EQ(skeleton_ptr->RootNode()->Name(), std::string("Armature")); + delete mesh; } ///////////////////////////////////////////////// @@ -488,4 +503,5 @@ TEST_F(ColladaLoader, LoadCylinderAnimatedFrom3dsMax) EXPECT_EQ("Bone02", anim->Name()); EXPECT_EQ(1u, anim->NodeCount()); EXPECT_TRUE(anim->HasNode("Bone02")); + delete mesh; } diff --git a/graphics/src/Image.cc b/graphics/src/Image.cc index 1bbdf74f..cfd01ffa 100644 --- a/graphics/src/Image.cc +++ b/graphics/src/Image.cc @@ -688,8 +688,17 @@ BOOL Image::Implementation::PixelIndex( ////////////////////////////////////////////////// void Image::Rescale(int _width, int _height) { - this->dataPtr->bitmap = FreeImage_Rescale( + auto *scaled = FreeImage_Rescale( this->dataPtr->bitmap, _width, _height, FILTER_LANCZOS3); + + if (!scaled) + { + gzerr << "Failed to rescale image\n"; + return; + } + + FreeImage_Unload(this->dataPtr->bitmap); + this->dataPtr->bitmap = scaled; } ////////////////////////////////////////////////// diff --git a/graphics/src/OBJLoader_TEST.cc b/graphics/src/OBJLoader_TEST.cc index aa2b7941..8ed7571f 100644 --- a/graphics/src/OBJLoader_TEST.cc +++ b/graphics/src/OBJLoader_TEST.cc @@ -59,6 +59,8 @@ TEST_F(OBJLoaderTest, LoadObjBox) EXPECT_EQ(mat->Diffuse(), math::Color(0.512f, 0.512f, 0.512f, 1.0f)); EXPECT_EQ(mat->Specular(), math::Color(0.25, 0.25, 0.25, 1.0)); EXPECT_DOUBLE_EQ(mat->Transparency(), 0.0); + + delete mesh; } ///////////////////////////////////////////////// @@ -73,6 +75,7 @@ TEST_F(OBJLoaderTest, InvalidMaterial) gz::common::Mesh *mesh = objLoader.Load(meshFilename); EXPECT_TRUE(mesh != nullptr); + delete mesh; } ///////////////////////////////////////////////// @@ -104,6 +107,7 @@ TEST_F(OBJLoaderTest, PBR) EXPECT_EQ("LightDome_Metalness.png", pbr->MetalnessMap()); EXPECT_EQ("LightDome_Roughness.png", pbr->RoughnessMap()); EXPECT_EQ("LightDome_Normal.png", pbr->NormalMap()); + delete mesh; } // load obj file exported by blender - it shoves pbr maps into @@ -131,5 +135,6 @@ TEST_F(OBJLoaderTest, PBR) EXPECT_EQ("mesh_Metal.png", pbr->MetalnessMap()); EXPECT_EQ("mesh_Rough.png", pbr->RoughnessMap()); EXPECT_EQ("mesh_Normal.png", pbr->NormalMap()); + delete mesh; } } diff --git a/graphics/src/STLLoader_TEST.cc b/graphics/src/STLLoader_TEST.cc index 7fd1e1d5..15189896 100644 --- a/graphics/src/STLLoader_TEST.cc +++ b/graphics/src/STLLoader_TEST.cc @@ -61,6 +61,7 @@ TEST_F(STLLoaderTest, LoadSTL) EXPECT_EQ(math::Vector3d(0, 0, -1), subMesh->Normal(2u)); EXPECT_STREQ("", mesh->SubMeshByIndex(0).lock()->Name().c_str()); + delete mesh; mesh = loader.Load( common::testing::TestFile("data", "cube_binary.stl")); @@ -88,4 +89,5 @@ TEST_F(STLLoaderTest, LoadSTL) EXPECT_EQ(math::Vector3d(0, 0, -1), subMesh->Normal(2u)); EXPECT_STREQ("", mesh->SubMeshByIndex(0).lock()->Name().c_str()); + delete mesh; } diff --git a/graphics/src/Skeleton.cc b/graphics/src/Skeleton.cc index cf0bfb95..f63f4f0b 100644 --- a/graphics/src/Skeleton.cc +++ b/graphics/src/Skeleton.cc @@ -72,10 +72,9 @@ Skeleton::Skeleton() ////////////////////////////////////////////////// Skeleton::Skeleton(SkeletonNode *_root) -: dataPtr(gz::utils::MakeUniqueImpl()) +: Skeleton() { - this->dataPtr->root = _root; - this->BuildNodeMap(); + this->RootNode(_root); } ////////////////////////////////////////////////// diff --git a/graphics/src/SkeletonAnimation_TEST.cc b/graphics/src/SkeletonAnimation_TEST.cc index 61e74869..eecb8a36 100644 --- a/graphics/src/SkeletonAnimation_TEST.cc +++ b/graphics/src/SkeletonAnimation_TEST.cc @@ -32,4 +32,6 @@ TEST_F(SkeletonAnimation, CheckNoXDisplacement) new common::SkeletonAnimation("emptyAnimation"); auto xDisplacement = skelAnim->XDisplacement(); ASSERT_FALSE(xDisplacement); + + delete skelAnim; } diff --git a/graphics/src/SkeletonNode.cc b/graphics/src/SkeletonNode.cc index dc562acf..b3e3e0c1 100644 --- a/graphics/src/SkeletonNode.cc +++ b/graphics/src/SkeletonNode.cc @@ -87,9 +87,7 @@ SkeletonNode::SkeletonNode(SkeletonNode *_parent, } ////////////////////////////////////////////////// -SkeletonNode::~SkeletonNode() -{ -} +SkeletonNode::~SkeletonNode() = default; ////////////////////////////////////////////////// void SkeletonNode::Name(const std::string &_name)