From bb5a6abae0ef105e5dc9126181d239d8cc3294f8 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Fri, 24 Feb 2023 12:43:06 +0200 Subject: [PATCH 01/11] Add more LWO files to unit tests --- test/unit/utLWOImportExport.cpp | 323 ++++++++++++++++++++++++++++++++ 1 file changed, 323 insertions(+) diff --git a/test/unit/utLWOImportExport.cpp b/test/unit/utLWOImportExport.cpp index 266105030..e0053020f 100644 --- a/test/unit/utLWOImportExport.cpp +++ b/test/unit/utLWOImportExport.cpp @@ -75,3 +75,326 @@ TEST_F(utLWOImportExport, importLWOformatdetection) { EXPECT_NE(nullptr, scene); } + + +TEST_F(utLWOImportExport, importLWOempty) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/invalid/empty.lwo", aiProcess_ValidateDataStructure); + + EXPECT_EQ(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWObox_2uv_1unused) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/box_2uv_1unused.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWObox_2vc_1unused) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/box_2vc_1unused.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOconcave_polygon) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/concave_polygon.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOconcave_self_intersecting) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/concave_self_intersecting.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOhierarchy) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/hierarchy.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOhierarchy_smoothed) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/hierarchy_smoothed.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOearth_cylindrical_x) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/MappingModes/earth_cylindrical_x.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOearth_cylindrical_x_scale_222_wrap_21) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/MappingModes/earth_cylindrical_x_scale_222_wrap_21.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOearth_cylindrical_y) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/MappingModes/earth_cylindrical_y.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOearth_cylindrical_y_scale_111) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/MappingModes/earth_cylindrical_y_scale_111.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOearth_cylindrical_y_scale_111_wrap_21) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/MappingModes/earth_cylindrical_y_scale_111_wrap_21.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOearth_cylindrical_z) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/MappingModes/earth_cylindrical_z.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOearth_planar_x) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/MappingModes/earth_planar_x.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOearth_planar_y) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/MappingModes/earth_planar_y.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOearth_planar_z) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/MappingModes/earth_planar_z.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOearth_planar_z_scale_111) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/MappingModes/earth_planar_z_scale_111.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOearth_spherical_x) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/MappingModes/earth_spherical_x.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOearth_spherical_x_scale_222_wrap_22) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/MappingModes/earth_spherical_x_scale_222_wrap_22.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOearth_spherical_y) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/MappingModes/earth_spherical_y.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOearth_spherical_) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/MappingModes/earth_spherical_z.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOearth_spherical_z_wrap_22) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/MappingModes/earth_spherical_z_wrap_22.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOearth_uv_cylindrical_y) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/MappingModes/earth_uv_cylindrical_y.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOModoExport_vertNormals) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/ModoExport_vertNormals.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOnonplanar_polygon) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/nonplanar_polygon.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOCellShader) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/shader_test/CellShader.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOfastFresne) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/shader_test/fastFresnel.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOrealFresnel) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/shader_test/realFresnel.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOSuperCellShader) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/shader_test/SuperCellShader.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOsphere_with_gradient) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/sphere_with_gradient.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOsphere_with_mat_gloss_10pc) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/sphere_with_mat_gloss_10pc.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOSubdivision) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/Subdivision.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOtransparency) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/transparency.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOUglyVertexColors) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/UglyVertexColors.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOuvtest) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWO2/uvtest.lwo", aiProcess_ValidateDataStructure); + + EXPECT_NE(nullptr, scene); +} + + +// These tests leak memory and complain on ASAN CI build +#if 0 + + +TEST_F(utLWOImportExport, importLWOBConcavePolygon) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWOB/ConcavePolygon.lwo", aiProcess_ValidateDataStructure); + + // FIXME: this should succees but there's a bug in the importer + EXPECT_EQ(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOBbluewithcylindrictex) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWOB/MappingModes/bluewithcylindrictexz.lwo", aiProcess_ValidateDataStructure); + + // FIXME: this should succees but there's a bug in the importer + EXPECT_EQ(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOBsphere_with_mat_gloss_10pc) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWOB/sphere_with_mat_gloss_10pc.lwo", aiProcess_ValidateDataStructure); + + // FIXME: this should succees but there's a bug in the importer + EXPECT_EQ(nullptr, scene); +} + + +TEST_F(utLWOImportExport, importLWOBsphere_with_mat_gloss_50pc) { + ::Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWOB/sphere_with_mat_gloss_50pc.lwo", aiProcess_ValidateDataStructure); + + // FIXME: this should succees but there's a bug in the importer + EXPECT_EQ(nullptr, scene); +} + + +#endif // ASAN failing tests From 28d4e394c0c44dee234d468eb40e90e8359b7a8a Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Fri, 24 Feb 2023 13:17:32 +0200 Subject: [PATCH 02/11] Use std::unique_ptr a bit --- code/AssetLib/LWO/LWOLoader.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/code/AssetLib/LWO/LWOLoader.cpp b/code/AssetLib/LWO/LWOLoader.cpp index 96fed248b..75ae6a416 100644 --- a/code/AssetLib/LWO/LWOLoader.cpp +++ b/code/AssetLib/LWO/LWOLoader.cpp @@ -429,7 +429,7 @@ void LWOImporter::InternReadFile(const std::string &pFile, // Generate nodes to render the mesh. Store the source layer in the mParent member of the nodes unsigned int num = static_cast(apcMeshes.size() - meshStart); if (layer.mName != "" || num > 0) { - aiNode *pcNode = new aiNode(); + std::unique_ptr pcNode(new aiNode()); pcNode->mName.Set(layer.mName); pcNode->mParent = (aiNode *)&layer; pcNode->mNumMeshes = num; @@ -439,7 +439,7 @@ void LWOImporter::InternReadFile(const std::string &pFile, for (unsigned int p = 0; p < pcNode->mNumMeshes; ++p) pcNode->mMeshes[p] = p + meshStart; } - apcNodes[layer.mIndex] = pcNode; + apcNodes[layer.mIndex] = pcNode.release(); } } From 92f7c50c915414a41ae3905fcb5714435c2e65d9 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Fri, 24 Feb 2023 13:18:04 +0200 Subject: [PATCH 03/11] Remove dead code --- code/AssetLib/LWO/LWOLoader.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/code/AssetLib/LWO/LWOLoader.cpp b/code/AssetLib/LWO/LWOLoader.cpp index 75ae6a416..1a3cf4e1a 100644 --- a/code/AssetLib/LWO/LWOLoader.cpp +++ b/code/AssetLib/LWO/LWOLoader.cpp @@ -398,14 +398,6 @@ void LWOImporter::InternReadFile(const std::string &pFile, pvVC[w]++; } -#if 0 - // process vertex weights. We can't properly reconstruct the whole skeleton for now, - // but we can create dummy bones for all weight channels which we have. - for (unsigned int w = 0; w < layer.mWeightChannels.size();++w) - { - } -#endif - face.mIndices[q] = vert; } pf->mIndices = face.mIndices; From 53c6cc096634392b3cc23c12bbdf7e8f4ddce2ad Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Fri, 24 Feb 2023 13:19:38 +0200 Subject: [PATCH 04/11] Add a scope so mapPivot is destroyed when no longer used --- code/AssetLib/LWO/LWOLoader.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/code/AssetLib/LWO/LWOLoader.cpp b/code/AssetLib/LWO/LWOLoader.cpp index 1a3cf4e1a..9d76d6bc7 100644 --- a/code/AssetLib/LWO/LWOLoader.cpp +++ b/code/AssetLib/LWO/LWOLoader.cpp @@ -565,6 +565,7 @@ void LWOImporter::GenerateNodeGraph(std::map &apcNodes) { root->mName.Set(""); //Set parent of all children, inserting pivots + { std::map mapPivot; for (auto itapcNodes = apcNodes.begin(); itapcNodes != apcNodes.end(); ++itapcNodes) { @@ -599,6 +600,7 @@ void LWOImporter::GenerateNodeGraph(std::map &apcNodes) { for (auto itMapPivot = mapPivot.begin(); itMapPivot != mapPivot.end(); ++itMapPivot) { apcNodes[itMapPivot->first] = itMapPivot->second; } + } //Set children of all parents apcNodes[(uint16_t)-1] = root; From be28f0949b3ed004659f3a1d266be5557c84f1f0 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Fri, 24 Feb 2023 13:20:40 +0200 Subject: [PATCH 05/11] Whitespace --- code/AssetLib/LWO/LWOLoader.cpp | 58 ++++++++++++++++----------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/code/AssetLib/LWO/LWOLoader.cpp b/code/AssetLib/LWO/LWOLoader.cpp index 9d76d6bc7..7a09b60a2 100644 --- a/code/AssetLib/LWO/LWOLoader.cpp +++ b/code/AssetLib/LWO/LWOLoader.cpp @@ -566,40 +566,40 @@ void LWOImporter::GenerateNodeGraph(std::map &apcNodes) { //Set parent of all children, inserting pivots { - std::map mapPivot; - for (auto itapcNodes = apcNodes.begin(); itapcNodes != apcNodes.end(); ++itapcNodes) { + std::map mapPivot; + for (auto itapcNodes = apcNodes.begin(); itapcNodes != apcNodes.end(); ++itapcNodes) { - //Get the parent index - LWO::Layer *nodeLayer = (LWO::Layer *)(itapcNodes->second->mParent); - uint16_t parentIndex = nodeLayer->mParent; + //Get the parent index + LWO::Layer *nodeLayer = (LWO::Layer *)(itapcNodes->second->mParent); + uint16_t parentIndex = nodeLayer->mParent; - //Create pivot node, store it into the pivot map, and set the parent as the pivot - aiNode *pivotNode = new aiNode(); - pivotNode->mName.Set("Pivot-" + std::string(itapcNodes->second->mName.data)); - itapcNodes->second->mParent = pivotNode; + //Create pivot node, store it into the pivot map, and set the parent as the pivot + aiNode *pivotNode = new aiNode(); + pivotNode->mName.Set("Pivot-" + std::string(itapcNodes->second->mName.data)); + itapcNodes->second->mParent = pivotNode; - //Look for the parent node to attach the pivot to - if (apcNodes.find(parentIndex) != apcNodes.end()) { - pivotNode->mParent = apcNodes[parentIndex]; - } else { - //If not, attach to the root node - pivotNode->mParent = root; + //Look for the parent node to attach the pivot to + if (apcNodes.find(parentIndex) != apcNodes.end()) { + pivotNode->mParent = apcNodes[parentIndex]; + } else { + //If not, attach to the root node + pivotNode->mParent = root; + } + + //Set the node and the pivot node transformation + itapcNodes->second->mTransformation.a4 = -nodeLayer->mPivot.x; + itapcNodes->second->mTransformation.b4 = -nodeLayer->mPivot.y; + itapcNodes->second->mTransformation.c4 = -nodeLayer->mPivot.z; + pivotNode->mTransformation.a4 = nodeLayer->mPivot.x; + pivotNode->mTransformation.b4 = nodeLayer->mPivot.y; + pivotNode->mTransformation.c4 = nodeLayer->mPivot.z; + mapPivot[-(itapcNodes->first + 2)] = pivotNode; } - //Set the node and the pivot node transformation - itapcNodes->second->mTransformation.a4 = -nodeLayer->mPivot.x; - itapcNodes->second->mTransformation.b4 = -nodeLayer->mPivot.y; - itapcNodes->second->mTransformation.c4 = -nodeLayer->mPivot.z; - pivotNode->mTransformation.a4 = nodeLayer->mPivot.x; - pivotNode->mTransformation.b4 = nodeLayer->mPivot.y; - pivotNode->mTransformation.c4 = nodeLayer->mPivot.z; - mapPivot[-(itapcNodes->first + 2)] = pivotNode; - } - - //Merge pivot map into node map - for (auto itMapPivot = mapPivot.begin(); itMapPivot != mapPivot.end(); ++itMapPivot) { - apcNodes[itMapPivot->first] = itMapPivot->second; - } + //Merge pivot map into node map + for (auto itMapPivot = mapPivot.begin(); itMapPivot != mapPivot.end(); ++itMapPivot) { + apcNodes[itMapPivot->first] = itMapPivot->second; + } } //Set children of all parents From 3bdfecb0ed61118fa39955454c82e1bc889c333a Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Fri, 24 Feb 2023 13:26:34 +0200 Subject: [PATCH 06/11] Destroy mapPivot nodes as we go --- code/AssetLib/LWO/LWOLoader.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/code/AssetLib/LWO/LWOLoader.cpp b/code/AssetLib/LWO/LWOLoader.cpp index 7a09b60a2..a44c1dc96 100644 --- a/code/AssetLib/LWO/LWOLoader.cpp +++ b/code/AssetLib/LWO/LWOLoader.cpp @@ -597,8 +597,9 @@ void LWOImporter::GenerateNodeGraph(std::map &apcNodes) { } //Merge pivot map into node map - for (auto itMapPivot = mapPivot.begin(); itMapPivot != mapPivot.end(); ++itMapPivot) { + for (auto itMapPivot = mapPivot.begin(); itMapPivot != mapPivot.end();) { apcNodes[itMapPivot->first] = itMapPivot->second; + itMapPivot = mapPivot.erase(itMapPivot); } } From 657c1d8ad0b0afba9c96cd1526d8bb2ea305a2c0 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Fri, 24 Feb 2023 13:44:40 +0200 Subject: [PATCH 07/11] Use unique_ptr to store pivot nodes until moved to mapPivot --- code/AssetLib/LWO/LWOLoader.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/code/AssetLib/LWO/LWOLoader.cpp b/code/AssetLib/LWO/LWOLoader.cpp index a44c1dc96..35ebc512c 100644 --- a/code/AssetLib/LWO/LWOLoader.cpp +++ b/code/AssetLib/LWO/LWOLoader.cpp @@ -574,9 +574,9 @@ void LWOImporter::GenerateNodeGraph(std::map &apcNodes) { uint16_t parentIndex = nodeLayer->mParent; //Create pivot node, store it into the pivot map, and set the parent as the pivot - aiNode *pivotNode = new aiNode(); + std::unique_ptr pivotNode(new aiNode()); pivotNode->mName.Set("Pivot-" + std::string(itapcNodes->second->mName.data)); - itapcNodes->second->mParent = pivotNode; + itapcNodes->second->mParent = pivotNode.get(); //Look for the parent node to attach the pivot to if (apcNodes.find(parentIndex) != apcNodes.end()) { @@ -593,7 +593,7 @@ void LWOImporter::GenerateNodeGraph(std::map &apcNodes) { pivotNode->mTransformation.a4 = nodeLayer->mPivot.x; pivotNode->mTransformation.b4 = nodeLayer->mPivot.y; pivotNode->mTransformation.c4 = nodeLayer->mPivot.z; - mapPivot[-(itapcNodes->first + 2)] = pivotNode; + mapPivot[-(itapcNodes->first + 2)] = pivotNode.release(); } //Merge pivot map into node map From 3bdc43a216a39b5b08423422acf5e17fd4881030 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Fri, 24 Feb 2023 14:22:44 +0200 Subject: [PATCH 08/11] Add debug logging to LWO node graph generation --- code/AssetLib/LWO/LWOLoader.cpp | 35 ++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/code/AssetLib/LWO/LWOLoader.cpp b/code/AssetLib/LWO/LWOLoader.cpp index 35ebc512c..7cca332b1 100644 --- a/code/AssetLib/LWO/LWOLoader.cpp +++ b/code/AssetLib/LWO/LWOLoader.cpp @@ -431,6 +431,7 @@ void LWOImporter::InternReadFile(const std::string &pFile, for (unsigned int p = 0; p < pcNode->mNumMeshes; ++p) pcNode->mMeshes[p] = p + meshStart; } + ASSIMP_LOG_DEBUG("insert apcNode for layer ", layer.mIndex, " \"", layer.mName, "\""); apcNodes[layer.mIndex] = pcNode.release(); } } @@ -564,6 +565,11 @@ void LWOImporter::GenerateNodeGraph(std::map &apcNodes) { aiNode *root = mScene->mRootNode = new aiNode(); root->mName.Set(""); + ASSIMP_LOG_DEBUG("apcNodes initial size: ", apcNodes.size()); + if (!apcNodes.empty()) { + ASSIMP_LOG_DEBUG("first apcNode is: ", apcNodes.begin()->first, " \"", apcNodes.begin()->second->mName.C_Str(), "\""); + } + //Set parent of all children, inserting pivots { std::map mapPivot; @@ -593,14 +599,30 @@ void LWOImporter::GenerateNodeGraph(std::map &apcNodes) { pivotNode->mTransformation.a4 = nodeLayer->mPivot.x; pivotNode->mTransformation.b4 = nodeLayer->mPivot.y; pivotNode->mTransformation.c4 = nodeLayer->mPivot.z; - mapPivot[-(itapcNodes->first + 2)] = pivotNode.release(); + uint16_t pivotNodeId = static_cast(-(itapcNodes->first + 2)); + ASSIMP_LOG_DEBUG("insert pivot node: ", pivotNodeId); + auto oldNodeIt = mapPivot.find(pivotNodeId); + if (oldNodeIt != mapPivot.end()) { + ASSIMP_LOG_ERROR("attempted to insert pivot node which already exists in pivot map ", pivotNodeId, " \"", pivotNode->mName.C_Str(), "\""); + } else { + mapPivot.emplace(pivotNodeId, pivotNode.release()); + } } + ASSIMP_LOG_DEBUG("pivot nodes: ", mapPivot.size()); //Merge pivot map into node map for (auto itMapPivot = mapPivot.begin(); itMapPivot != mapPivot.end();) { - apcNodes[itMapPivot->first] = itMapPivot->second; + uint16_t pivotNodeId = itMapPivot->first; + auto oldApcNodeIt = apcNodes.find(pivotNodeId); + if (oldApcNodeIt != apcNodes.end()) { + ASSIMP_LOG_ERROR("attempted to insert pivot node which already exists in apc nodes ", pivotNodeId, " \"", itMapPivot->second->mName.C_Str(), "\""); + } else { + apcNodes.emplace(pivotNodeId, itMapPivot->second); + } + itMapPivot->second = nullptr; itMapPivot = mapPivot.erase(itMapPivot); } + ASSIMP_LOG_DEBUG("total nodes: ", apcNodes.size()); } //Set children of all parents @@ -622,8 +644,15 @@ void LWOImporter::GenerateNodeGraph(std::map &apcNodes) { } } - if (!mScene->mRootNode->mNumChildren) + if (!mScene->mRootNode->mNumChildren) { + ASSIMP_LOG_DEBUG("All apcNodes:"); + for (auto nodeIt = apcNodes.begin(); nodeIt != apcNodes.end(); ) { + ASSIMP_LOG_DEBUG("Node ", nodeIt->first, " \"", nodeIt->second->mName.C_Str(), "\""); + nodeIt->second = nullptr; + nodeIt = apcNodes.erase(nodeIt); + } throw DeadlyImportError("LWO: Unable to build a valid node graph"); + } // Remove a single root node with no meshes assigned to it ... if (1 == mScene->mRootNode->mNumChildren) { From e8f0eb930d8b33b44bbf50810040662f6da05991 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Fri, 24 Feb 2023 14:34:32 +0200 Subject: [PATCH 09/11] Fix loading binary LWO files --- code/AssetLib/LWO/LWOLoader.cpp | 2 +- test/unit/utLWOImportExport.cpp | 19 ++++--------------- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/code/AssetLib/LWO/LWOLoader.cpp b/code/AssetLib/LWO/LWOLoader.cpp index 7cca332b1..1bf39b2da 100644 --- a/code/AssetLib/LWO/LWOLoader.cpp +++ b/code/AssetLib/LWO/LWOLoader.cpp @@ -178,7 +178,7 @@ void LWOImporter::InternReadFile(const std::string &pFile, mLayers->push_back(Layer()); mCurLayer = &mLayers->back(); mCurLayer->mName = ""; - mCurLayer->mIndex = (uint16_t) -1; + mCurLayer->mIndex = 1; // old lightwave file format (prior to v6) mIsLWO2 = false; diff --git a/test/unit/utLWOImportExport.cpp b/test/unit/utLWOImportExport.cpp index e0053020f..2ab1c4722 100644 --- a/test/unit/utLWOImportExport.cpp +++ b/test/unit/utLWOImportExport.cpp @@ -357,16 +357,11 @@ TEST_F(utLWOImportExport, importLWOuvtest) { } -// These tests leak memory and complain on ASAN CI build -#if 0 - - TEST_F(utLWOImportExport, importLWOBConcavePolygon) { ::Assimp::Importer importer; const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWOB/ConcavePolygon.lwo", aiProcess_ValidateDataStructure); - // FIXME: this should succees but there's a bug in the importer - EXPECT_EQ(nullptr, scene); + EXPECT_NE(nullptr, scene); } @@ -374,8 +369,7 @@ TEST_F(utLWOImportExport, importLWOBbluewithcylindrictex) { ::Assimp::Importer importer; const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWOB/MappingModes/bluewithcylindrictexz.lwo", aiProcess_ValidateDataStructure); - // FIXME: this should succees but there's a bug in the importer - EXPECT_EQ(nullptr, scene); + EXPECT_NE(nullptr, scene); } @@ -383,8 +377,7 @@ TEST_F(utLWOImportExport, importLWOBsphere_with_mat_gloss_10pc) { ::Assimp::Importer importer; const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWOB/sphere_with_mat_gloss_10pc.lwo", aiProcess_ValidateDataStructure); - // FIXME: this should succees but there's a bug in the importer - EXPECT_EQ(nullptr, scene); + EXPECT_NE(nullptr, scene); } @@ -392,9 +385,5 @@ TEST_F(utLWOImportExport, importLWOBsphere_with_mat_gloss_50pc) { ::Assimp::Importer importer; const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/LWO/LWOB/sphere_with_mat_gloss_50pc.lwo", aiProcess_ValidateDataStructure); - // FIXME: this should succees but there's a bug in the importer - EXPECT_EQ(nullptr, scene); + EXPECT_NE(nullptr, scene); } - - -#endif // ASAN failing tests From 60da5e7e963d5753a5a532cea1ff78600cf8185f Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 28 Feb 2023 23:27:46 +0100 Subject: [PATCH 10/11] Update UTFConverter.cpp --- samples/SharedCode/UTFConverter.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/samples/SharedCode/UTFConverter.cpp b/samples/SharedCode/UTFConverter.cpp index e6c07e946..383ae43f1 100644 --- a/samples/SharedCode/UTFConverter.cpp +++ b/samples/SharedCode/UTFConverter.cpp @@ -5,8 +5,6 @@ Open Asset Import Library (assimp) Copyright (c) 2006-2020, assimp team - - All rights reserved. Redistribution and use of this software in source and binary forms, @@ -46,7 +44,5 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. namespace AssimpSamples { namespace SharedCode { -//typename UTFConverter::UTFConverterImpl UTFConverter::impl_; - } } From 534ee288c5d5864f871a924a44ef5462c2acf435 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 28 Feb 2023 23:28:24 +0100 Subject: [PATCH 11/11] Update UTFConverter.h --- samples/SharedCode/UTFConverter.h | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/samples/SharedCode/UTFConverter.h b/samples/SharedCode/UTFConverter.h index 34b2293de..8173d474c 100644 --- a/samples/SharedCode/UTFConverter.h +++ b/samples/SharedCode/UTFConverter.h @@ -3,9 +3,7 @@ Open Asset Import Library (assimp) --------------------------------------------------------------------------- -Copyright (c) 2006-2020, assimp team - - +Copyright (c) 2006-2023, assimp team All rights reserved. @@ -56,11 +54,9 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. namespace AssimpSamples { namespace SharedCode { -// Used to convert between multibyte and unicode strings. +/// @brief Used to convert between multibyte and unicode strings. class UTFConverter { public: - //utf8::utf16to8(start, end, back_inserter(str)); - UTFConverter(const char* s) : s_(s), ws_() { std::vector str; utf8::utf8to16(s, s + std::strlen(s) + 1, back_inserter(str));