From d6e9a15b1540c2550bafe3a90de292ca7161f7e7 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Wed, 15 Jan 2020 13:23:13 +0200 Subject: [PATCH 1/8] Add MD5 importer unit tests --- test/CMakeLists.txt | 1 + test/unit/ImportExport/utMD5Importer.cpp | 80 ++++++++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 test/unit/ImportExport/utMD5Importer.cpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 321aeea70..3d382490c 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -129,6 +129,7 @@ SET( IMPORTERS unit/ImportExport/utNFFImportExport.cpp unit/ImportExport/utXGLImportExport.cpp unit/ImportExport/utMD2Importer.cpp + unit/ImportExport/utMD5Importer.cpp unit/ImportExport/utMDLImporter.cpp unit/ImportExport/MDL/MDLHL1TestFiles.h unit/ImportExport/MDL/utMDLImporter_HL1_ImportSettings.cpp diff --git a/test/unit/ImportExport/utMD5Importer.cpp b/test/unit/ImportExport/utMD5Importer.cpp new file mode 100644 index 000000000..941d20e9d --- /dev/null +++ b/test/unit/ImportExport/utMD5Importer.cpp @@ -0,0 +1,80 @@ +/* +--------------------------------------------------------------------------- +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, +with or without modification, are permitted provided that the following +conditions are met: + +* Redistributions of source code must retain the above +copyright notice, this list of conditions and the +following disclaimer. + +* Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the +following disclaimer in the documentation and/or other +materials provided with the distribution. + +* Neither the name of the assimp team, nor the names of its +contributors may be used to endorse or promote products +derived from this software without specific prior +written permission of the assimp team. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +--------------------------------------------------------------------------- +*/ + +#include "UnitTestPCH.h" + +#include +#include +#include + + +using namespace Assimp; + + + +TEST(utMD5Importer, importEmpty) { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/MD5/invalid/empty.md5mesh", aiProcess_ValidateDataStructure); + ASSERT_EQ(nullptr, scene); +} + + +TEST(utMD5Importer, importSimpleCube) { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/MD5/SimpleCube.md5mesh", aiProcess_ValidateDataStructure); + ASSERT_NE(nullptr, scene); +} + + +TEST(utMD5Importer, importBoarMan) { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_NONBSD_DIR "/MD5/BoarMan.md5mesh", aiProcess_ValidateDataStructure); + ASSERT_NE(nullptr, scene); +} + + +TEST(utMD5Importer, importBob) { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_NONBSD_DIR "/MD5/Bob.md5mesh", aiProcess_ValidateDataStructure); + ASSERT_NE(nullptr, scene); +} From a50b94dd63dd63d5759dfc79751f9f1ea3226b18 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Wed, 15 Jan 2020 14:10:09 +0200 Subject: [PATCH 2/8] Revert test model dwarf.cob to working version --- test/models/COB/dwarf.cob | Bin 962911 -> 962915 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/test/models/COB/dwarf.cob b/test/models/COB/dwarf.cob index 718d0c786e9ff18237c991370c30027362079425..1e6e1ddd700a52770fc311cbd0a71cbf920cfba1 100644 GIT binary patch delta 109 zcmccr$m;PUtA-ZF7N!>F7M3ln-a?GL?cPGHK+Fcj>_E)1-CKxrl@A*)mlFd6%k+t7 xc!j4=IKw-seSSPA5OZyxAJ3g#!N@xus7F7M3ln-a^xPZ*VKM=LxX_F&hxG12M<;JR!~wpXoJccvrPA ejOPSmuI&rsxw9+UfvUIj-r&jmvK=V-{0RW!Y9LYo From 26a80bb0197b48ad4dd5b4478bdd1e6bf25f5474 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Wed, 15 Jan 2020 14:15:29 +0200 Subject: [PATCH 3/8] Refactor COB import test to not use a class --- test/unit/ImportExport/utCOBImportExport.cpp | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/test/unit/ImportExport/utCOBImportExport.cpp b/test/unit/ImportExport/utCOBImportExport.cpp index d80757e53..3147dc143 100644 --- a/test/unit/ImportExport/utCOBImportExport.cpp +++ b/test/unit/ImportExport/utCOBImportExport.cpp @@ -43,22 +43,17 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "UnitTestPCH.h" #include "SceneDiffer.h" -#include "AbstractImportExportBase.h" #include #include using namespace Assimp; -class utCOBImportExport : public AbstractImportExportBase { -public: - virtual bool importerTest() { - Assimp::Importer importer; - const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/COB/molecule.cob", aiProcess_ValidateDataStructure); - return nullptr != scene; - } -}; -TEST_F(utCOBImportExport, importAMFFromFileTest) { - EXPECT_TRUE(importerTest()); +TEST(utCOBImporter, importMolecule) { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/COB/molecule.cob", aiProcess_ValidateDataStructure); + ASSERT_NE(nullptr, scene); } + + From 542b1f768858db91875680218e7ebdca3b9c969f Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Wed, 15 Jan 2020 14:34:25 +0200 Subject: [PATCH 4/8] Add more COB importer unit tests --- test/unit/ImportExport/utCOBImportExport.cpp | 51 ++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/test/unit/ImportExport/utCOBImportExport.cpp b/test/unit/ImportExport/utCOBImportExport.cpp index 3147dc143..c01487c84 100644 --- a/test/unit/ImportExport/utCOBImportExport.cpp +++ b/test/unit/ImportExport/utCOBImportExport.cpp @@ -50,6 +50,31 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. using namespace Assimp; +TEST(utCOBImporter, importDwarfASCII) { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/COB/dwarf_ascii.cob", aiProcess_ValidateDataStructure); + // FIXME: this is wrong, it should succeed + // change to ASSERT_NE after it's been fixed + ASSERT_EQ(nullptr, scene); +} + + +TEST(utCOBImporter, importDwarf) { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/COB/dwarf.cob", aiProcess_ValidateDataStructure); + ASSERT_NE(nullptr, scene); +} + + +TEST(utCOBImporter, importMoleculeASCII) { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/COB/molecule_ascii.cob", aiProcess_ValidateDataStructure); + // FIXME: this is wrong, it should succeed + // change to ASSERT_NE after it's been fixed + ASSERT_EQ(nullptr, scene); +} + + TEST(utCOBImporter, importMolecule) { Assimp::Importer importer; const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/COB/molecule.cob", aiProcess_ValidateDataStructure); @@ -57,3 +82,29 @@ TEST(utCOBImporter, importMolecule) { } +TEST(utCOBImporter, importSpider43ASCII) { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/COB/spider_4_3_ascii.cob", aiProcess_ValidateDataStructure); + ASSERT_NE(nullptr, scene); +} + + +TEST(utCOBImporter, importSpider43) { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/COB/spider_4_3.cob", aiProcess_ValidateDataStructure); + ASSERT_NE(nullptr, scene); +} + + +TEST(utCOBImporter, importSpider66ASCII) { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/COB/spider_6_6_ascii.cob", aiProcess_ValidateDataStructure); + ASSERT_NE(nullptr, scene); +} + + +TEST(utCOBImporter, importSpider66) { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/COB/spider_6_6.cob", aiProcess_ValidateDataStructure); + ASSERT_NE(nullptr, scene); +} From 1cae51615f81f11cd4283c3e5792a7c226815111 Mon Sep 17 00:00:00 2001 From: "Hui.Du" Date: Thu, 16 Jan 2020 11:49:00 +1300 Subject: [PATCH 5/8] Fix: gltf exporting memory leak --- code/glTF/glTFAsset.inl | 4 +++- code/glTF/glTFExporter.cpp | 9 ++++----- code/glTF/glTFExporter.h | 2 +- code/glTF2/glTF2Asset.inl | 7 +++++-- code/glTF2/glTF2Exporter.cpp | 6 ++---- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/code/glTF/glTFAsset.inl b/code/glTF/glTFAsset.inl index 500d49fcb..f93f195bb 100644 --- a/code/glTF/glTFAsset.inl +++ b/code/glTF/glTFAsset.inl @@ -688,7 +688,9 @@ inline void Image::SetData(uint8_t* data, size_t length, Asset& r) bufferView->byteOffset = b->AppendData(data, length); } else { // text file: will be stored as a data uri - this->mData.reset(data); + uint8_t *temp = new uint8_t[length]; + memcpy(temp, data, length); + this->mData.reset(temp); this->mDataLength = length; } } diff --git a/code/glTF/glTFExporter.cpp b/code/glTF/glTFExporter.cpp index cf8ef974a..4772c7dc6 100644 --- a/code/glTF/glTFExporter.cpp +++ b/code/glTF/glTFExporter.cpp @@ -100,17 +100,16 @@ glTFExporter::glTFExporter(const char* filename, IOSystem* pIOSystem, const aiSc { aiScene* sceneCopy_tmp; SceneCombiner::CopyScene(&sceneCopy_tmp, pScene); - aiScene *sceneCopy(sceneCopy_tmp); SplitLargeMeshesProcess_Triangle tri_splitter; tri_splitter.SetLimit(0xffff); - tri_splitter.Execute(sceneCopy); + tri_splitter.Execute(sceneCopy_tmp); SplitLargeMeshesProcess_Vertex vert_splitter; vert_splitter.SetLimit(0xffff); - vert_splitter.Execute(sceneCopy); + vert_splitter.Execute(sceneCopy_tmp); - mScene = sceneCopy; + mScene.reset(sceneCopy_tmp); mAsset.reset( new glTF::Asset( pIOSystem ) ); @@ -877,7 +876,7 @@ void glTFExporter::ExportMetadata() // Copyright aiString copyright_str; - if (mScene->mMetaData->Get(AI_METADATA_SOURCE_COPYRIGHT, copyright_str)) { + if (mScene->mMetaData != nullptr && mScene->mMetaData->Get(AI_METADATA_SOURCE_COPYRIGHT, copyright_str)) { asset.copyright = copyright_str.C_Str(); } } diff --git a/code/glTF/glTFExporter.h b/code/glTF/glTFExporter.h index d6c2e7f04..ffa2ce42e 100644 --- a/code/glTF/glTFExporter.h +++ b/code/glTF/glTFExporter.h @@ -90,7 +90,7 @@ namespace Assimp const char* mFilename; IOSystem* mIOSystem; - const aiScene* mScene; + std::shared_ptr mScene; const ExportProperties* mProperties; std::map mTexturesByPath; diff --git a/code/glTF2/glTF2Asset.inl b/code/glTF2/glTF2Asset.inl index e55857be1..3db37c72b 100644 --- a/code/glTF2/glTF2Asset.inl +++ b/code/glTF2/glTF2Asset.inl @@ -752,6 +752,7 @@ inline uint8_t* Image::StealData() return mData.release(); } +// Never take over the ownership of data whenever binary or not inline void Image::SetData(uint8_t* data, size_t length, Asset& r) { Ref b = r.GetBodyBuffer(); @@ -764,8 +765,10 @@ inline void Image::SetData(uint8_t* data, size_t length, Asset& r) bufferView->byteOffset = b->AppendData(data, length); } else { // text file: will be stored as a data uri - this->mData.reset(data); - this->mDataLength = length; + uint8_t *temp = new uint8_t[length]; + memcpy(temp, data, length); + this->mData.reset(temp); + this->mDataLength = length; } } diff --git a/code/glTF2/glTF2Exporter.cpp b/code/glTF2/glTF2Exporter.cpp index d7b16ab04..ee40694f9 100644 --- a/code/glTF2/glTF2Exporter.cpp +++ b/code/glTF2/glTF2Exporter.cpp @@ -352,10 +352,8 @@ void glTF2Exporter::GetMatTex(const aiMaterial* mat, Ref& texture, aiTe if (path[0] == '*') { // embedded aiTexture* tex = mScene->mTextures[atoi(&path[1])]; - // copy data since lifetime control is handed over to the asset - uint8_t* data = new uint8_t[tex->mWidth]; - memcpy(data, tex->pcData, tex->mWidth); - texture->source->SetData(data, tex->mWidth, *mAsset); + // The asset has its own buffer, see Image::SetData + texture->source->SetData(reinterpret_cast (tex->pcData), tex->mWidth, *mAsset); if (tex->achFormatHint[0]) { std::string mimeType = "image/"; From 08110be9f742b3167ff9bd6906b35e59d3595b4a Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Thu, 16 Jan 2020 13:41:01 +0200 Subject: [PATCH 6/8] Revert BoxTextured.glb to working version --- .../BoxTextured-glTF-Binary/BoxTextured.glb | Bin 4695 -> 4696 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/test/models/glTF2/BoxTextured-glTF-Binary/BoxTextured.glb b/test/models/glTF2/BoxTextured-glTF-Binary/BoxTextured.glb index 19b5546b04f56818f70f8bf809863ec8853b72f1..03cd2acf941a0838722aa3e7243d33a8f459c22e 100644 GIT binary patch delta 14 Vcmcbvazkaq4Gu=$%{MseIRP+R1+f4C delta 12 Tcmcbia$RM^4UWw>IqEq9DEtMa From 2a366388c2f9326c5f8f515dd21251ef19ef9212 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Thu, 16 Jan 2020 13:55:34 +0200 Subject: [PATCH 7/8] Revert Wuson.ms3d to working version --- test/models/MS3D/Wuson.ms3d | Bin 330211 -> 330213 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/test/models/MS3D/Wuson.ms3d b/test/models/MS3D/Wuson.ms3d index e78cbaa58e0ded211393ecea424f8b8e6ade237f..2fc79ac82666277998e21202ce5914e971c93561 100644 GIT binary patch delta 33 ocmaDnS>)+tk%kt=7N!>FEi6}h70$Z73Z&0WVi7L~0PANB8~^|S delta 29 lcmaDlS>*9#k%kt=7N!>FEi6}hwqNaG`N6vV+$0w9asbB|4FdoG From 89e622060b02a5911bdabcb64afdd8a20c16c0e8 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Thu, 16 Jan 2020 14:02:04 +0200 Subject: [PATCH 8/8] Preserve more exceptions in Half-Life MDL loader --- code/MDL/HalfLife/HL1MDLLoader.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/code/MDL/HalfLife/HL1MDLLoader.cpp b/code/MDL/HalfLife/HL1MDLLoader.cpp index 785183950..90a1479a3 100644 --- a/code/MDL/HalfLife/HL1MDLLoader.cpp +++ b/code/MDL/HalfLife/HL1MDLLoader.cpp @@ -175,9 +175,9 @@ void HL1MDLLoader::load_file() { release_resources(); - } catch (const std::exception &e) { + } catch (...) { release_resources(); - throw e; + throw; } }