Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple memory cleanup fixes #571

Merged
merged 10 commits into from
Jan 29, 2024
26 changes: 25 additions & 1 deletion graphics/src/AssimpLoader_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -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;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -155,6 +158,7 @@ TEST_F(AssimpLoader, ShareVertices)
}
}
}
delete mesh;
}

/////////////////////////////////////////////////
Expand All @@ -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;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -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;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -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;
}

/////////////////////////////////////////////////
Expand All @@ -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;
}

/////////////////////////////////////////////////
Expand All @@ -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;
}

/////////////////////////////////////////////////
Expand All @@ -435,6 +447,7 @@ TEST_F(AssimpLoader, LoadBoxWithHierarchicalNodes)

// nested node with name
EXPECT_EQ("StaticCubeNested", mesh->SubMeshByIndex(4).lock()->Name());
delete mesh;
}

/////////////////////////////////////////////////
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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;
}


Expand All @@ -536,6 +552,7 @@ TEST_F(AssimpLoader, ObjInvalidMaterial)
common::Mesh *mesh = loader.Load(meshFilename);

EXPECT_TRUE(mesh != nullptr);
delete mesh;
}

/////////////////////////////////////////////////
Expand All @@ -550,6 +567,7 @@ TEST_F(AssimpLoader, NonExistingMesh)
common::Mesh *mesh = loader.Load(meshFilename);

EXPECT_EQ(mesh->SubMeshCount(), 0);
delete mesh;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -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;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -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;
}

/////////////////////////////////////////////////
Expand All @@ -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;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -681,6 +702,7 @@ TEST_F(AssimpLoader, LoadGlTF2BoxWithJPEGTexture)
EXPECT_EQ("Scene_Material_Diffuse", mat->TextureImage());
#endif
EXPECT_NE(nullptr, mat->TextureData());
delete mesh;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

/////////////////////////////////////////////////
Expand All @@ -782,6 +804,7 @@ TEST_F(AssimpLoader, CheckNonRootDisplacement)
EXPECT_EQ(nullptr, rootNode);
auto xDisplacement = skelAnim->XDisplacement();
ASSERT_TRUE(xDisplacement);
delete mesh;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -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;
}
9 changes: 9 additions & 0 deletions graphics/src/ColladaExporter_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ TEST_F(ColladaExporter, ExportBox)
meshReloaded->SubMeshByIndex(i).lock()->TexCoord(j));
}
}
delete meshOriginal;
delete meshReloaded;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -206,6 +208,8 @@ TEST_F(ColladaExporter, ExportCordlessDrill)
meshReloaded->SubMeshByIndex(i).lock()->TexCoord(j));
}
}
delete meshOriginal;
delete meshReloaded;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -448,4 +456,5 @@ TEST_F(ColladaExporter, ExportLights)
}
}
EXPECT_EQ(node_with_light_count, 3);
delete meshOriginal;
}
16 changes: 16 additions & 0 deletions graphics/src/ColladaLoader_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -100,6 +101,7 @@ TEST_F(ColladaLoader, ShareVertices)
}
}
}
delete mesh;
}

/////////////////////////////////////////////////
Expand All @@ -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;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -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;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -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;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -302,6 +309,7 @@ TEST_F(ColladaLoader, LoadBoxWithAnimationOutsideSkeleton)
0, 0, 1, 0,
0, 0, 0, 1);
EXPECT_EQ(expectedTrans, poseEnd.at("Armature"));
delete mesh;
}

/////////////////////////////////////////////////
Expand All @@ -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;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -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;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -383,6 +393,7 @@ TEST_F(ColladaLoader, LoadBoxNestedAnimation)
0, 0, 1, 0,
0, 0, 0, 1);
EXPECT_EQ(expectedTrans, poseEnd.at("Bone"));
delete mesh;
}

/////////////////////////////////////////////////
Expand All @@ -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;
}

/////////////////////////////////////////////////
Expand All @@ -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;
}

/////////////////////////////////////////////////
Expand All @@ -440,6 +453,7 @@ TEST_F(ColladaLoader, LoadBoxWithHierarchicalNodes)

// Parent of nested node with name
EXPECT_EQ("StaticCubeParent2", mesh->SubMeshByIndex(4).lock()->Name());
delete mesh;
}

/////////////////////////////////////////////////
Expand All @@ -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;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -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;
}
11 changes: 10 additions & 1 deletion graphics/src/Image.cc
Original file line number Diff line number Diff line change
Expand Up @@ -688,8 +688,17 @@
//////////////////////////////////////////////////
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;

Check warning on line 697 in graphics/src/Image.cc

View check run for this annotation

Codecov / codecov/patch

graphics/src/Image.cc#L696-L697

Added lines #L696 - L697 were not covered by tests
}

FreeImage_Unload(this->dataPtr->bitmap);
this->dataPtr->bitmap = scaled;
}

//////////////////////////////////////////////////
Expand Down
5 changes: 5 additions & 0 deletions graphics/src/OBJLoader_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/////////////////////////////////////////////////
Expand All @@ -73,6 +75,7 @@ TEST_F(OBJLoaderTest, InvalidMaterial)
gz::common::Mesh *mesh = objLoader.Load(meshFilename);

EXPECT_TRUE(mesh != nullptr);
delete mesh;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
}
Loading
Loading