From 9a6b07e5225ad566dab8e7399b121d5c348de9e7 Mon Sep 17 00:00:00 2001 From: kimkulling Date: Wed, 25 Jul 2018 15:11:24 +0200 Subject: [PATCH 1/2] closes https://github.com/assimp/assimp/issues/1724: add default material access to the material API. --- code/Assimp.cpp | 2 + code/Importer.cpp | 3 ++ code/MaterialSystem.cpp | 80 +++++++++++++++++++++++---------- code/ObjFileMtlImporter.cpp | 2 - code/STLLoader.cpp | 14 +++--- code/ScenePrivate.h | 16 ++++--- code/Version.cpp | 8 ++-- include/assimp/Macros.h | 2 +- include/assimp/material.h | 43 +++++++++++++++--- include/assimp/material.inl | 4 +- test/unit/utMaterialSystem.cpp | 20 ++++++++- test/unit/utSTLImportExport.cpp | 2 +- 12 files changed, 144 insertions(+), 52 deletions(-) diff --git a/code/Assimp.cpp b/code/Assimp.cpp index b682d257b..2a9b896e7 100644 --- a/code/Assimp.cpp +++ b/code/Assimp.cpp @@ -272,6 +272,8 @@ void aiReleaseImport( const aiScene* pScene) ASSIMP_BEGIN_EXCEPTION_REGION(); + aiReleaseDefaultMaterial(); + // find the importer associated with this data const ScenePrivateData* priv = ScenePriv(pScene); if( !priv || !priv->mOrigImporter) { diff --git a/code/Importer.cpp b/code/Importer.cpp index 186818c95..7aabe0a5f 100644 --- a/code/Importer.cpp +++ b/code/Importer.cpp @@ -178,6 +178,7 @@ Importer::~Importer() { // Delete all import plugins DeleteImporterInstanceList(pimpl->mImporter); + aiReleaseDefaultMaterial(); // Delete all post-processing plug-ins for( unsigned int a = 0; a < pimpl->mPostProcessingSteps.size(); a++) @@ -383,6 +384,8 @@ bool _ValidateFlags(unsigned int pFlags) void Importer::FreeScene( ) { ASSIMP_BEGIN_EXCEPTION_REGION(); + + aiReleaseDefaultMaterial(); delete pimpl->mScene; pimpl->mScene = NULL; diff --git a/code/MaterialSystem.cpp b/code/MaterialSystem.cpp index c8262dff0..3679c93c7 100644 --- a/code/MaterialSystem.cpp +++ b/code/MaterialSystem.cpp @@ -63,9 +63,9 @@ aiReturn aiGetMaterialProperty(const aiMaterial* pMat, unsigned int index, const aiMaterialProperty** pPropOut) { - ai_assert (pMat != NULL); - ai_assert (pKey != NULL); - ai_assert (pPropOut != NULL); + ai_assert( pMat != NULL ); + ai_assert( pKey != NULL ); + ai_assert( pPropOut != NULL ); /* Just search for a property with exactly this name .. * could be improved by hashing, but it's possibly @@ -76,7 +76,7 @@ aiReturn aiGetMaterialProperty(const aiMaterial* pMat, if (prop /* just for safety ... */ && 0 == strcmp( prop->mKey.data, pKey ) - && (UINT_MAX == type || prop->mSemantic == type) /* UINT_MAX is a wildcard, but this is undocumented :-) */ + && (UINT_MAX == type || prop->mSemantic == type) /* UINT_MAX is a wild-card, but this is undocumented :-) */ && (UINT_MAX == index || prop->mIndex == index)) { *pPropOut = pMat->mProperties[i]; @@ -96,8 +96,8 @@ aiReturn aiGetMaterialFloatArray(const aiMaterial* pMat, ai_real* pOut, unsigned int* pMax) { - ai_assert (pOut != NULL); - ai_assert (pMat != NULL); + ai_assert( pOut != NULL ); + ai_assert( pMat != NULL ); const aiMaterialProperty* prop; aiGetMaterialProperty(pMat,pKey,type,index, (const aiMaterialProperty**) &prop); @@ -182,8 +182,8 @@ aiReturn aiGetMaterialIntegerArray(const aiMaterial* pMat, int* pOut, unsigned int* pMax) { - ai_assert (pOut != NULL); - ai_assert (pMat != NULL); + ai_assert( pOut != NULL ); + ai_assert( pMat != NULL ); const aiMaterialProperty* prop; aiGetMaterialProperty(pMat,pKey,type,index,(const aiMaterialProperty**) &prop); @@ -274,7 +274,7 @@ aiReturn aiGetMaterialUVTransform(const aiMaterial* pMat, aiUVTransform* pOut) { unsigned int iMax = 4; - return aiGetMaterialFloatArray(pMat,pKey,type,index,(ai_real*)pOut,&iMax); + return aiGetMaterialFloatArray(pMat,pKey,type,index,(ai_real*)pOut,&iMax); } // ------------------------------------------------------------------------------------------------ @@ -314,7 +314,7 @@ aiReturn aiGetMaterialString(const aiMaterial* pMat, // ------------------------------------------------------------------------------------------------ // Get the number of textures on a particular texture stack -ASSIMP_API unsigned int aiGetMaterialTextureCount(const C_STRUCT aiMaterial* pMat, +unsigned int aiGetMaterialTextureCount(const C_STRUCT aiMaterial* pMat, C_ENUM aiTextureType type) { ai_assert (pMat != NULL); @@ -347,12 +347,14 @@ aiReturn aiGetMaterialTexture(const C_STRUCT aiMaterial* mat, unsigned int* flags /*= NULL*/ ) { - ai_assert(NULL != mat && NULL != path); + ai_assert( NULL != mat ); + ai_assert( NULL != path ); // Get the path to the texture if (AI_SUCCESS != aiGetMaterialString(mat,AI_MATKEY_TEXTURE(type,index),path)) { return AI_FAILURE; } + // Determine mapping type int mapping_ = static_cast(aiTextureMapping_UV); aiGetMaterialInteger(mat,AI_MATKEY_MAPPING(type,index), &mapping_); @@ -381,15 +383,37 @@ aiReturn aiGetMaterialTexture(const C_STRUCT aiMaterial* mat, if (flags){ aiGetMaterialInteger(mat,AI_MATKEY_TEXFLAGS(type,index),(int*)flags); } + return AI_SUCCESS; } +static aiMaterial *DefaultMaterial = nullptr; + +// ------------------------------------------------------------------------------------------------ +// Will return the default material. +aiMaterial *aiCreateAndRegisterDefaultMaterial() { + if (nullptr == DefaultMaterial) { + DefaultMaterial = new aiMaterial; + aiString s; + s.Set(AI_DEFAULT_MATERIAL_NAME); + DefaultMaterial->AddProperty(&s, AI_MATKEY_NAME); + } + + return DefaultMaterial; +} + +// ------------------------------------------------------------------------------------------------ +// Will return the default material. +void aiReleaseDefaultMaterial() { + DefaultMaterial = nullptr; +} + static const unsigned int DefaultNumAllocated = 5; // ------------------------------------------------------------------------------------------------ // Construction. Actually the one and only way to get an aiMaterial instance aiMaterial::aiMaterial() -: mProperties( NULL ) +: mProperties( nullptr ) , mNumProperties( 0 ) , mNumAllocated( DefaultNumAllocated ) { // Allocate 5 entries by default @@ -404,12 +428,20 @@ aiMaterial::~aiMaterial() delete[] mProperties; } +// ------------------------------------------------------------------------------------------------ +aiString aiMaterial::GetName() { + aiString name; + Get(AI_MATKEY_NAME, name); + + return name; +} + // ------------------------------------------------------------------------------------------------ void aiMaterial::Clear() { - for (unsigned int i = 0; i < mNumProperties;++i) { + for ( unsigned int i = 0; i < mNumProperties; ++i ) { // delete this entry - delete mProperties[i]; + delete mProperties[ i ]; AI_DEBUG_INVALIDATE_PTR(mProperties[i]); } mNumProperties = 0; @@ -418,11 +450,9 @@ void aiMaterial::Clear() } // ------------------------------------------------------------------------------------------------ -aiReturn aiMaterial::RemoveProperty (const char* pKey,unsigned int type, - unsigned int index - ) +aiReturn aiMaterial::RemoveProperty ( const char* pKey,unsigned int type, unsigned int index ) { - ai_assert(NULL != pKey); + ai_assert( nullptr != pKey ); for (unsigned int i = 0; i < mNumProperties;++i) { aiMaterialProperty* prop = mProperties[i]; @@ -454,17 +484,18 @@ aiReturn aiMaterial::AddBinaryProperty (const void* pInput, aiPropertyTypeInfo pType ) { - ai_assert (pInput != NULL); - ai_assert (pKey != NULL); - ai_assert (0 != pSizeInBytes); + ai_assert( pInput != NULL ); + ai_assert( pKey != NULL ); + ai_assert( 0 != pSizeInBytes ); if ( 0 == pSizeInBytes ) { } + // first search the list whether there is already an entry with this key - unsigned int iOutIndex = UINT_MAX; - for (unsigned int i = 0; i < mNumProperties;++i) { - aiMaterialProperty* prop = mProperties[i]; + unsigned int iOutIndex( UINT_MAX ); + for ( unsigned int i = 0; i < mNumProperties; ++i ) { + aiMaterialProperty *prop( mProperties[ i ] ); if (prop /* just for safety */ && !strcmp( prop->mKey.data, pKey ) && prop->mSemantic == type && prop->mIndex == index){ @@ -516,6 +547,7 @@ aiReturn aiMaterial::AddBinaryProperty (const void* pInput, } // push back ... mProperties[mNumProperties++] = pcNew; + return AI_SUCCESS; } diff --git a/code/ObjFileMtlImporter.cpp b/code/ObjFileMtlImporter.cpp index 8f7588819..711740353 100644 --- a/code/ObjFileMtlImporter.cpp +++ b/code/ObjFileMtlImporter.cpp @@ -84,8 +84,6 @@ static const std::string BumpOption = "-bm"; static const std::string ChannelOption = "-imfchan"; static const std::string TypeOption = "-type"; - - // ------------------------------------------------------------------- // Constructor ObjFileMtlImporter::ObjFileMtlImporter( std::vector &buffer, diff --git a/code/STLLoader.cpp b/code/STLLoader.cpp index 5abccc774..b556c3418 100644 --- a/code/STLLoader.cpp +++ b/code/STLLoader.cpp @@ -90,6 +90,9 @@ static bool IsBinarySTL(const char* buffer, unsigned int fileSize) { return expectedBinaryFileSize == fileSize; } +static const size_t BufferSize = 500; +static const char UnicodeBoundary = 127; + // An ascii STL buffer will begin with "solid NAME", where NAME is optional. // Note: The "solid NAME" check is necessary, but not sufficient, to determine // if the buffer is ASCII; a binary header could also begin with "solid NAME". @@ -108,10 +111,10 @@ static bool IsAsciiSTL(const char* buffer, unsigned int fileSize) { bool isASCII( strncmp( buffer, "solid", 5 ) == 0 ); if( isASCII ) { // A lot of importers are write solid even if the file is binary. So we have to check for ASCII-characters. - if( fileSize >= 500 ) { + if( fileSize >= BufferSize) { isASCII = true; - for( unsigned int i = 0; i < 500; i++ ) { - if( buffer[ i ] > 127 ) { + for( unsigned int i = 0; i < BufferSize; i++ ) { + if( buffer[ i ] > UnicodeBoundary) { isASCII = false; break; } @@ -211,10 +214,11 @@ void STLImporter::InternReadFile( const std::string& pFile, aiScene* pScene, IOS // create a single default material, using a white diffuse color for consistency with // other geometric types (e.g., PLY). - aiMaterial* pcMat = new aiMaterial(); + aiMaterial* pcMat = aiCreateAndRegisterDefaultMaterial(); + /*aiMaterial* pcMat = new aiMaterial(); aiString s; s.Set(AI_DEFAULT_MATERIAL_NAME); - pcMat->AddProperty(&s, AI_MATKEY_NAME); + pcMat->AddProperty(&s, AI_MATKEY_NAME);*/ aiColor4D clrDiffuse(ai_real(1.0),ai_real(1.0),ai_real(1.0),ai_real(1.0)); if (bMatClr) { diff --git a/code/ScenePrivate.h b/code/ScenePrivate.h index 50959f455..751b007d8 100644 --- a/code/ScenePrivate.h +++ b/code/ScenePrivate.h @@ -55,12 +55,8 @@ namespace Assimp { class Importer; struct ScenePrivateData { - ScenePrivateData() - : mOrigImporter( nullptr ) - , mPPStepsApplied( 0 ) - , mIsCopy( false ) { - // empty - } + // The struct constructor. + ScenePrivateData(); // Importer that originally loaded the scene though the C-API // If set, this object is owned by this private data instance. @@ -77,6 +73,14 @@ struct ScenePrivateData { bool mIsCopy; }; +inline +ScenePrivateData::ScenePrivateData() +: mOrigImporter( nullptr ) +, mPPStepsApplied( 0 ) +, mIsCopy( false ) { + // empty +} + // Access private data stored in the scene inline ScenePrivateData* ScenePriv(aiScene* in) { diff --git a/code/Version.cpp b/code/Version.cpp index e35344674..b823abd68 100644 --- a/code/Version.cpp +++ b/code/Version.cpp @@ -150,9 +150,11 @@ ASSIMP_API aiScene::~aiScene() { delete mMeshes[a]; delete [] mMeshes; - if (mNumMaterials && mMaterials) - for( unsigned int a = 0; a < mNumMaterials; a++) - delete mMaterials[a]; + if (mNumMaterials && mMaterials) { + for (unsigned int a = 0; a < mNumMaterials; ++a ) { + delete mMaterials[ a ]; + } + } delete [] mMaterials; if (mNumAnimations && mAnimations) diff --git a/include/assimp/Macros.h b/include/assimp/Macros.h index 4a2d07569..3aaed4e97 100644 --- a/include/assimp/Macros.h +++ b/include/assimp/Macros.h @@ -3,7 +3,7 @@ Open Asset Import Library (assimp) --------------------------------------------------------------------------- -Copyright (c) 2006-2012, assimp team +Copyright (c) 2006-2018, assimp team All rights reserved. diff --git a/include/assimp/material.h b/include/assimp/material.h index 83244af37..6564e047b 100644 --- a/include/assimp/material.h +++ b/include/assimp/material.h @@ -608,16 +608,17 @@ struct aiMaterialProperty #ifdef __cplusplus aiMaterialProperty() - : mSemantic( 0 ) - , mIndex( 0 ) - , mDataLength( 0 ) - , mType( aiPTI_Float ) - , mData( NULL ) - { + : mSemantic( 0 ) + , mIndex( 0 ) + , mDataLength( 0 ) + , mType( aiPTI_Float ) + , mData(nullptr) { + // empty } ~aiMaterialProperty() { delete[] mData; + mData = nullptr; } #endif @@ -651,6 +652,14 @@ public: aiMaterial(); ~aiMaterial(); + // ------------------------------------------------------------------- + /** + * @brief Returns the name of the material. + * @return The name of the material. + */ + // ------------------------------------------------------------------- + aiString GetName(); + // ------------------------------------------------------------------- /** @brief Retrieve an array of Type values with a specific key * from the material @@ -1556,10 +1565,32 @@ C_ENUM aiReturn aiGetMaterialTexture(const C_STRUCT aiMaterial* mat, unsigned int* flags /*= NULL*/); #endif // !#ifdef __cplusplus +// --------------------------------------------------------------------------- +/** @brief Helper function to get all values pertaining to a particular +* texture slot from a material structure. +* +* @return Pointer showing to the default material. +*/ +// --------------------------------------------------------------------------- +#ifdef __cplusplus +ASSIMP_API aiMaterial *aiCreateAndRegisterDefaultMaterial(void); +#else +C_STRUCT aiMaterial *aiCreateAndRegisterDefaultMaterial(void); +#endif // !#ifdef __cplusplus + +// --------------------------------------------------------------------------- +/** + * @brief Helper function to release the default material instance, the + * instance will not be destroyed. + */ +// --------------------------------------------------------------------------- +ASSIMP_API void aiReleaseDefaultMaterial(); + #ifdef __cplusplus } #include "material.inl" #endif //!__cplusplus + #endif //!!AI_MATERIAL_H_INC diff --git a/include/assimp/material.inl b/include/assimp/material.inl index b52b9befc..3d4b07c06 100644 --- a/include/assimp/material.inl +++ b/include/assimp/material.inl @@ -120,7 +120,7 @@ inline aiReturn aiMaterial::Get(const char* pKey,unsigned int type, const aiMaterialProperty* prop; const aiReturn ret = ::aiGetMaterialProperty(this,pKey,type,idx, (const aiMaterialProperty**)&prop); - if ( AI_SUCCESS == ret ) { + if ( AI_SUCCESS == ret ) { if (prop->mDataLength < sizeof(Type)) { return AI_FAILURE; @@ -130,7 +130,7 @@ inline aiReturn aiMaterial::Get(const char* pKey,unsigned int type, return AI_FAILURE; } - ::memcpy(&pOut,prop->mData,sizeof(Type)); + ::memcpy( &pOut, prop->mData, sizeof( Type ) ); } return ret; } diff --git a/test/unit/utMaterialSystem.cpp b/test/unit/utMaterialSystem.cpp index 529282393..70ee104c9 100644 --- a/test/unit/utMaterialSystem.cpp +++ b/test/unit/utMaterialSystem.cpp @@ -120,8 +120,7 @@ TEST_F(MaterialSystemTest, testColorProperty) } // ------------------------------------------------------------------------------------------------ -TEST_F(MaterialSystemTest, testStringProperty) -{ +TEST_F(MaterialSystemTest, testStringProperty) { aiString s; s.Set("Hello, this is a small test"); this->pcMat->AddProperty(&s,"testKey6"); @@ -129,3 +128,20 @@ TEST_F(MaterialSystemTest, testStringProperty) EXPECT_EQ(AI_SUCCESS, pcMat->Get("testKey6",0,0,s)); EXPECT_STREQ("Hello, this is a small test", s.data); } + +// ------------------------------------------------------------------------------------------------ +TEST_F(MaterialSystemTest, testDefaultMaterialAccess) { + aiMaterial *mat = aiCreateAndRegisterDefaultMaterial(); + EXPECT_NE(nullptr, mat); + aiReleaseDefaultMaterial(); +} + +// ------------------------------------------------------------------------------------------------ +TEST_F(MaterialSystemTest, testMaterialNameAccess) { + aiMaterial *mat = aiCreateAndRegisterDefaultMaterial(); + EXPECT_NE(nullptr, mat); + + aiString name = mat->GetName(); + const int retValue(strncmp(name.C_Str(), AI_DEFAULT_MATERIAL_NAME, name.length)); + EXPECT_EQ(0, retValue ); +} diff --git a/test/unit/utSTLImportExport.cpp b/test/unit/utSTLImportExport.cpp index de1e78a26..9ebfb0f05 100644 --- a/test/unit/utSTLImportExport.cpp +++ b/test/unit/utSTLImportExport.cpp @@ -63,7 +63,7 @@ public: } }; -TEST_F( utSTLImporterExporter, importXFromFileTest ) { +TEST_F( utSTLImporterExporter, importSTLFromFileTest ) { EXPECT_TRUE( importerTest() ); } From 7e6755331132289d3b8ecca1449f96c3c844599a Mon Sep 17 00:00:00 2001 From: kkulling Date: Wed, 25 Jul 2018 15:40:19 +0200 Subject: [PATCH 2/2] Fix memleak. --- test/unit/utMaterialSystem.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/unit/utMaterialSystem.cpp b/test/unit/utMaterialSystem.cpp index 70ee104c9..550dc63da 100644 --- a/test/unit/utMaterialSystem.cpp +++ b/test/unit/utMaterialSystem.cpp @@ -134,6 +134,8 @@ TEST_F(MaterialSystemTest, testDefaultMaterialAccess) { aiMaterial *mat = aiCreateAndRegisterDefaultMaterial(); EXPECT_NE(nullptr, mat); aiReleaseDefaultMaterial(); + + delete mat; } // ------------------------------------------------------------------------------------------------ @@ -144,4 +146,6 @@ TEST_F(MaterialSystemTest, testMaterialNameAccess) { aiString name = mat->GetName(); const int retValue(strncmp(name.C_Str(), AI_DEFAULT_MATERIAL_NAME, name.length)); EXPECT_EQ(0, retValue ); + + delete mat; }