From 5c99d6a864f9e2fc43960eb3113f212f32c7cb21 Mon Sep 17 00:00:00 2001 From: iamAdrianIusca Date: Sat, 15 Feb 2020 23:29:04 +0200 Subject: [PATCH 01/12] very small changes - FIND_PACKAGE(DirectX) is already used in the samples and assimp tool cmake files - so is not needed in the main cmake fil - other small changes --- CMakeLists.txt | 4 ---- code/Obj/ObjFileParser.cpp | 17 ++++++++--------- code/Obj/ObjFileParser.h | 4 ++-- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 38ebf3480..7be4f6d5f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -437,10 +437,6 @@ ELSE(HUNTER_ENABLED) DESTINATION "${ASSIMP_LIB_INSTALL_DIR}/cmake/assimp-${ASSIMP_VERSION_MAJOR}.${ASSIMP_VERSION_MINOR}" COMPONENT ${LIBASSIMP-DEV_COMPONENT}) ENDIF(HUNTER_ENABLED) -if (ASSIMP_BUILD_SAMPLES OR ASSIMP_BUILD_SAMPLES) - FIND_PACKAGE(DirectX) -endif(ASSIMP_BUILD_SAMPLES OR ASSIMP_BUILD_SAMPLES) - IF( BUILD_DOCS ) ADD_SUBDIRECTORY(doc) ENDIF( BUILD_DOCS ) diff --git a/code/Obj/ObjFileParser.cpp b/code/Obj/ObjFileParser.cpp index 48129c02c..8b1572067 100644 --- a/code/Obj/ObjFileParser.cpp +++ b/code/Obj/ObjFileParser.cpp @@ -53,6 +53,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include #include +#include +#include namespace Assimp { @@ -71,19 +73,19 @@ ObjFileParser::ObjFileParser() ObjFileParser::ObjFileParser( IOStreamBuffer &streamBuffer, const std::string &modelName, IOSystem *io, ProgressHandler* progress, - const std::string &originalObjFileName) : + std::string originalObjFileName) : m_DataIt(), m_DataItEnd(), m_pModel(nullptr), m_uiLine(0), - m_pIO( io ), + m_pIO(io), m_progress(progress), - m_originalObjFileName(originalObjFileName) + m_originalObjFileName(std::move(originalObjFileName)) { - std::fill_n(m_buffer,Buffersize,0); + std::fill_n(m_buffer, Buffersize,0); // Create the model instance to store all the data - m_pModel.reset(new ObjFile::Model()); + m_pModel = std::make_unique(); m_pModel->m_ModelName = modelName; // create default material and store it @@ -96,9 +98,6 @@ ObjFileParser::ObjFileParser( IOStreamBuffer &streamBuffer, const std::str parseFile( streamBuffer ); } -ObjFileParser::~ObjFileParser() { -} - void ObjFileParser::setBuffer( std::vector &buffer ) { m_DataIt = buffer.begin(); m_DataItEnd = buffer.end(); @@ -128,7 +127,7 @@ void ObjFileParser::parseFile( IOStreamBuffer &streamBuffer ) { processed = static_cast(filePos); lastFilePos = filePos; progressCounter++; - m_progress->UpdateFileRead( processed, progressTotal ); + m_progress->UpdateFileRead(processed, progressTotal); } // parse line diff --git a/code/Obj/ObjFileParser.h b/code/Obj/ObjFileParser.h index 124527413..9bbee66dd 100644 --- a/code/Obj/ObjFileParser.h +++ b/code/Obj/ObjFileParser.h @@ -78,9 +78,9 @@ public: /// @brief The default constructor. ObjFileParser(); /// @brief Constructor with data array. - ObjFileParser( IOStreamBuffer &streamBuffer, const std::string &modelName, IOSystem* io, ProgressHandler* progress, const std::string &originalObjFileName); + ObjFileParser(IOStreamBuffer &streamBuffer, const std::string &modelName, IOSystem* io, ProgressHandler* progress, std::string originalObjFileName); /// @brief Destructor - ~ObjFileParser(); + ~ObjFileParser() = default; /// @brief If you want to load in-core data. void setBuffer( std::vector &buffer ); /// @brief Model getter. From d9042e46098a903de391e74ac1f5f12340c3cfee Mon Sep 17 00:00:00 2001 From: Marc-Antoine Lortie Date: Sat, 15 Feb 2020 18:02:12 -0500 Subject: [PATCH 02/12] Fixed SimpleTexturedOpenGL sample. Several places in the sample's code were calling Unicode versions of Win32 functions with "multibyte" strings. A few changes were required to fix it. I added a class "UTFConverter", which handles calls to unicode/multibyte string conversions. This should help minimize the impacts on code change in case C++'s codecvt_utf8 ever changes. In addition, seveal memory leaks have been found, but these fixes will come in another PR because it goes beyond the scope of this PR. DevIL.lib was removed in CMakeFiles.txt, as it is unused in the sample. Here is a list of the changes: - Fixed MB string calls to Unicode functions. - Added class UTFConverter to handle string conversions. - Removed reference to DevIL.lib. - Fixed compile warnings. --- samples/SimpleTexturedOpenGL/CMakeLists.txt | 2 +- .../src/model_loading.cpp | 74 +++++++++++++------ 2 files changed, 54 insertions(+), 22 deletions(-) diff --git a/samples/SimpleTexturedOpenGL/CMakeLists.txt b/samples/SimpleTexturedOpenGL/CMakeLists.txt index 40138c49b..941e18cea 100644 --- a/samples/SimpleTexturedOpenGL/CMakeLists.txt +++ b/samples/SimpleTexturedOpenGL/CMakeLists.txt @@ -35,7 +35,7 @@ ADD_EXECUTABLE( assimp_simpletexturedogl WIN32 SET_PROPERTY(TARGET assimp_simpletexturedogl PROPERTY DEBUG_POSTFIX ${CMAKE_DEBUG_POSTFIX}) -TARGET_LINK_LIBRARIES( assimp_simpletexturedogl assimp ${OPENGL_LIBRARIES} ${GLUT_LIBRARIES} DevIL.lib ) +TARGET_LINK_LIBRARIES( assimp_simpletexturedogl assimp ${OPENGL_LIBRARIES} ${GLUT_LIBRARIES} ) SET_TARGET_PROPERTIES( assimp_simpletexturedogl PROPERTIES OUTPUT_NAME assimp_simpletexturedogl diff --git a/samples/SimpleTexturedOpenGL/SimpleTexturedOpenGL/src/model_loading.cpp b/samples/SimpleTexturedOpenGL/SimpleTexturedOpenGL/src/model_loading.cpp index 8d25aaaed..8428248d1 100644 --- a/samples/SimpleTexturedOpenGL/SimpleTexturedOpenGL/src/model_loading.cpp +++ b/samples/SimpleTexturedOpenGL/SimpleTexturedOpenGL/src/model_loading.cpp @@ -21,6 +21,8 @@ #define STB_IMAGE_IMPLEMENTATION #include "contrib/stb_image/stb_image.h" +#include +#include #include //to map image filenames to textureIds @@ -75,6 +77,36 @@ GLuint* textureIds; // pointer to texture Array // Create an instance of the Importer class Assimp::Importer importer; +// Used to convert between multibyte and unicode strings. +class UTFConverter { + using UTFConverterImpl = std::wstring_convert, wchar_t>; +public: + UTFConverter(const char* s) : + s_(s), + ws_(impl_.from_bytes(s)) { + } + UTFConverter(const std::string& s) : + s_(s), + ws_(impl_.from_bytes(s)) { + } + UTFConverter(const std::wstring& s) : + s_(impl_.to_bytes(s)), + ws_(s) { + } + inline const std::string& str() const { + return s_; + } + inline const wchar_t* c_wstr() const { + return ws_.c_str(); + } +private: + static UTFConverterImpl impl_; + std::string s_; + std::wstring ws_; +}; + +typename UTFConverter::UTFConverterImpl UTFConverter::impl_; + void createAILogger() { // Change this line to normal if you not want to analyse the import process @@ -120,7 +152,7 @@ bool Import3DFromFile( const std::string& pFile) } else { - MessageBox(NULL, ("Couldn't open file: " + pFile).c_str() , "ERROR", MB_OK | MB_ICONEXCLAMATION); + MessageBox(NULL, UTFConverter("Couldn't open file: " + pFile).c_wstr() , TEXT("ERROR"), MB_OK | MB_ICONEXCLAMATION); logInfo( importer.GetErrorString()); return false; } @@ -205,7 +237,7 @@ int LoadGLTextures(const aiScene* scene) } } - int numTextures = textureIdMap.size(); + const size_t numTextures = textureIdMap.size(); /* array with DevIL image IDs */ @@ -217,13 +249,13 @@ int LoadGLTextures(const aiScene* scene) /* create and fill array with GL texture ids */ textureIds = new GLuint[numTextures]; - glGenTextures(numTextures, textureIds); /* Texture name generation */ + glGenTextures(static_cast(numTextures), textureIds); /* Texture name generation */ /* get iterator */ std::map::iterator itr = textureIdMap.begin(); std::string basepath = getBasePath(modelpath); - for (int i=0; i 1) { std::wstring modelpathW(argv[1]); - modelpath = std::string(modelpathW.begin(), modelpathW.end()); + modelpath = UTFConverter(modelpathW).str(); } if (!Import3DFromFile(modelpath)) return 0; logInfo("=============== Post Import ===================="); - if (MessageBox(NULL, "Would You Like To Run In Fullscreen Mode?", "Start Fullscreen?", MB_YESNO|MB_ICONEXCLAMATION)==IDNO) + if (MessageBox(NULL, TEXT("Would You Like To Run In Fullscreen Mode?"), TEXT("Start Fullscreen?"), MB_YESNO|MB_ICONEXCLAMATION)==IDNO) { fullscreen=FALSE; } @@ -881,5 +913,5 @@ int WINAPI WinMain( HINSTANCE hInstance, // The instance destroyAILogger(); KillGLWindow(); - return (msg.wParam); + return static_cast(msg.wParam); } From eb8abfa02cc76f41ce93f3832972fb490f3b9321 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Sun, 16 Feb 2020 12:37:46 +0100 Subject: [PATCH 03/12] Update .clang-format Disable tabs. --- .clang-format | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.clang-format b/.clang-format index eba6d586f..70e0945fe 100644 --- a/.clang-format +++ b/.clang-format @@ -108,7 +108,7 @@ IndentWidth: 4 # SpacesInParentheses: false # SpacesInSquareBrackets: false TabWidth: 4 -UseTab: Always +UseTab: Never --- ### C++ specific config ### Language: Cpp From a65bac27e9ee0dca6c4178f5134b1bb10ce7ef91 Mon Sep 17 00:00:00 2001 From: iamAdrianIusca Date: Tue, 18 Feb 2020 18:01:53 +0200 Subject: [PATCH 04/12] fixed the make_unique --- code/Obj/ObjFileParser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/Obj/ObjFileParser.cpp b/code/Obj/ObjFileParser.cpp index 8b1572067..bfddb8461 100644 --- a/code/Obj/ObjFileParser.cpp +++ b/code/Obj/ObjFileParser.cpp @@ -85,7 +85,7 @@ ObjFileParser::ObjFileParser( IOStreamBuffer &streamBuffer, const std::str std::fill_n(m_buffer, Buffersize,0); // Create the model instance to store all the data - m_pModel = std::make_unique(); + m_pModel.reset(new ObjFile::Model()); m_pModel->m_ModelName = modelName; // create default material and store it From d0922230a93c2a9bac8676cf7f2f7d96f8aa6deb Mon Sep 17 00:00:00 2001 From: iamAdrianIusca Date: Tue, 18 Feb 2020 18:24:52 +0200 Subject: [PATCH 05/12] fixed the = default on the destructor --- code/Obj/ObjFileParser.cpp | 4 ++++ code/Obj/ObjFileParser.h | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/code/Obj/ObjFileParser.cpp b/code/Obj/ObjFileParser.cpp index bfddb8461..7ba2216ae 100644 --- a/code/Obj/ObjFileParser.cpp +++ b/code/Obj/ObjFileParser.cpp @@ -98,6 +98,10 @@ ObjFileParser::ObjFileParser( IOStreamBuffer &streamBuffer, const std::str parseFile( streamBuffer ); } +ObjFileParser::~ObjFileParser() +{ +} + void ObjFileParser::setBuffer( std::vector &buffer ) { m_DataIt = buffer.begin(); m_DataItEnd = buffer.end(); diff --git a/code/Obj/ObjFileParser.h b/code/Obj/ObjFileParser.h index 9bbee66dd..f0f5a2dc1 100644 --- a/code/Obj/ObjFileParser.h +++ b/code/Obj/ObjFileParser.h @@ -80,7 +80,7 @@ public: /// @brief Constructor with data array. ObjFileParser(IOStreamBuffer &streamBuffer, const std::string &modelName, IOSystem* io, ProgressHandler* progress, std::string originalObjFileName); /// @brief Destructor - ~ObjFileParser() = default; + ~ObjFileParser(); /// @brief If you want to load in-core data. void setBuffer( std::vector &buffer ); /// @brief Model getter. From 016c0a866557b7a22e6d47c9c89cc13939f121ce Mon Sep 17 00:00:00 2001 From: iamAdrianIusca Date: Tue, 18 Feb 2020 18:42:59 +0200 Subject: [PATCH 06/12] small changes --- code/Common/BaseImporter.cpp | 7 +++---- code/Common/Importer.cpp | 8 ++++---- include/assimp/Importer.hpp | 4 ++-- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/code/Common/BaseImporter.cpp b/code/Common/BaseImporter.cpp index 660a6a965..8d7b029ba 100644 --- a/code/Common/BaseImporter.cpp +++ b/code/Common/BaseImporter.cpp @@ -191,7 +191,7 @@ void BaseImporter::GetExtensionList(std::set& extensions) { } std::unique_ptr pStream (pIOHandler->Open(pFile)); - if (pStream.get() ) { + if (pStream) { // read 200 characters from the file std::unique_ptr _buffer (new char[searchBytes+1 /* for the '\0' */]); char *buffer( _buffer.get() ); @@ -283,7 +283,6 @@ std::string BaseImporter::GetExtension( const std::string& file ) { return ""; } - // thanks to Andy Maloney for the hint std::string ret = file.substr( pos + 1 ); std::transform( ret.begin(), ret.end(), ret.begin(), ToLower); @@ -309,7 +308,7 @@ std::string BaseImporter::GetExtension( const std::string& file ) { }; magic = reinterpret_cast(_magic); std::unique_ptr pStream (pIOHandler->Open(pFile)); - if (pStream.get() ) { + if (pStream) { // skip to offset pStream->Seek(offset,aiOrigin_SET); @@ -603,7 +602,7 @@ unsigned int BatchLoader::AddLoadRequest(const std::string& file, } // no, we don't have it. So add it to the queue ... - m_data->requests.push_back(LoadRequest(file,steps,map, m_data->next_id)); + m_data->requests.emplace_back(file, steps, map, m_data->next_id); return m_data->next_id++; } diff --git a/code/Common/Importer.cpp b/code/Common/Importer.cpp index 5601298f1..a59ec9812 100644 --- a/code/Common/Importer.cpp +++ b/code/Common/Importer.cpp @@ -1071,7 +1071,7 @@ ai_real Importer::GetPropertyFloat(const char* szName, ai_real iErrorReturn /*= // ------------------------------------------------------------------------------------------------ // Get a configuration property -const std::string Importer::GetPropertyString(const char* szName, const std::string& iErrorReturn /*= ""*/) const { +std::string Importer::GetPropertyString(const char* szName, const std::string& iErrorReturn /*= ""*/) const { ai_assert(nullptr != pimpl); return GetGenericProperty(pimpl->mStringProperties,szName,iErrorReturn); @@ -1079,7 +1079,7 @@ const std::string Importer::GetPropertyString(const char* szName, const std::str // ------------------------------------------------------------------------------------------------ // Get a configuration property -const aiMatrix4x4 Importer::GetPropertyMatrix(const char* szName, const aiMatrix4x4& iErrorReturn /*= aiMatrix4x4()*/) const { +aiMatrix4x4 Importer::GetPropertyMatrix(const char* szName, const aiMatrix4x4& iErrorReturn /*= aiMatrix4x4()*/) const { ai_assert(nullptr != pimpl); return GetGenericProperty(pimpl->mMatrixProperties,szName,iErrorReturn); @@ -1110,10 +1110,9 @@ void Importer::GetMemoryRequirements(aiMemoryInfo& in) const { aiScene* mScene = pimpl->mScene; // return if we have no scene loaded - if (!pimpl->mScene) + if (!mScene) return; - in.total = sizeof(aiScene); // add all meshes @@ -1202,5 +1201,6 @@ void Importer::GetMemoryRequirements(aiMemoryInfo& in) const { in.materials += pc->mProperties[a]->mDataLength; } } + in.total += in.materials; } diff --git a/include/assimp/Importer.hpp b/include/assimp/Importer.hpp index 7ec4f519c..df52471a9 100644 --- a/include/assimp/Importer.hpp +++ b/include/assimp/Importer.hpp @@ -285,7 +285,7 @@ public: * The return value remains valid until the property is modified. * @see GetPropertyInteger() */ - const std::string GetPropertyString(const char* szName, + std::string GetPropertyString(const char* szName, const std::string& sErrorReturn = "") const; // ------------------------------------------------------------------- @@ -294,7 +294,7 @@ public: * The return value remains valid until the property is modified. * @see GetPropertyInteger() */ - const aiMatrix4x4 GetPropertyMatrix(const char* szName, + aiMatrix4x4 GetPropertyMatrix(const char* szName, const aiMatrix4x4& sErrorReturn = aiMatrix4x4()) const; // ------------------------------------------------------------------- From bf85fc1386fa06c184be5874d0a54cb11828347e Mon Sep 17 00:00:00 2001 From: iamAdrianIusca Date: Tue, 18 Feb 2020 18:50:48 +0200 Subject: [PATCH 07/12] small changes --- code/Obj/ObjFileData.h | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/code/Obj/ObjFileData.h b/code/Obj/ObjFileData.h index 985a49a42..d0ea8d507 100644 --- a/code/Obj/ObjFileData.h +++ b/code/Obj/ObjFileData.h @@ -110,10 +110,7 @@ struct Object { std::vector m_Meshes; //! \brief Default constructor - Object() - : m_strObjName("") { - // empty - } + Object() = default; //! \brief Destructor ~Object() { @@ -191,16 +188,12 @@ struct Material { , illumination_model (1) , ior ( ai_real( 1.0 ) ) , transparent( ai_real( 1.0), ai_real (1.0), ai_real(1.0)) { - // empty - for (size_t i = 0; i < TextureTypeCount; ++i) { - clamp[ i ] = false; - } + + std::fill_n(clamp, TextureTypeCount,false); } // Destructor - ~Material() { - // empty - } + ~Material() = default; }; // ------------------------------------------------------------------------------------------------ From ad52c5c5f6ac771124c3bd3585f24daa61fbbdbd Mon Sep 17 00:00:00 2001 From: iamAdrianIusca Date: Tue, 18 Feb 2020 18:56:09 +0200 Subject: [PATCH 08/12] .clear in destructor is redundant --- code/Common/ZipArchiveIOSystem.cpp | 2 -- code/Obj/ObjFileData.h | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/code/Common/ZipArchiveIOSystem.cpp b/code/Common/ZipArchiveIOSystem.cpp index e83d3e50d..8d00da912 100644 --- a/code/Common/ZipArchiveIOSystem.cpp +++ b/code/Common/ZipArchiveIOSystem.cpp @@ -343,8 +343,6 @@ namespace Assimp { } ZipArchiveIOSystem::Implement::~Implement() { - m_ArchiveMap.clear(); - if (m_ZipFileHandle != nullptr) { unzClose(m_ZipFileHandle); m_ZipFileHandle = nullptr; diff --git a/code/Obj/ObjFileData.h b/code/Obj/ObjFileData.h index d0ea8d507..33184ce82 100644 --- a/code/Obj/ObjFileData.h +++ b/code/Obj/ObjFileData.h @@ -189,7 +189,7 @@ struct Material { , ior ( ai_real( 1.0 ) ) , transparent( ai_real( 1.0), ai_real (1.0), ai_real(1.0)) { - std::fill_n(clamp, TextureTypeCount,false); + std::fill_n(clamp, TextureTypeCount, false); } // Destructor From a5524ffcd9befe3f008e27d65b6de620b897c49c Mon Sep 17 00:00:00 2001 From: iamAdrianIusca Date: Tue, 18 Feb 2020 19:02:14 +0200 Subject: [PATCH 09/12] more changes --- code/PostProcessing/TextureTransform.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/code/PostProcessing/TextureTransform.cpp b/code/PostProcessing/TextureTransform.cpp index cebbd8042..bf60e2970 100644 --- a/code/PostProcessing/TextureTransform.cpp +++ b/code/PostProcessing/TextureTransform.cpp @@ -92,9 +92,8 @@ void TextureTransformStep::PreProcessUVTransform(STransformVecInfo& info) * are applied is - as always - scaling, rotation, translation. */ - char szTemp[512]; - int rounded = 0; - + int rounded; + char szTemp[512]; /* Optimize the rotation angle. That's slightly difficult as * we have an inprecise floating-point number (when comparing @@ -185,7 +184,6 @@ void TextureTransformStep::PreProcessUVTransform(STransformVecInfo& info) info.mTranslation.y = out; } } - return; } // ------------------------------------------------------------------------------------------------ @@ -428,7 +426,7 @@ void TextureTransformStep::Execute( aiScene* pScene) // at the end of the list bool ref[AI_MAX_NUMBER_OF_TEXTURECOORDS]; for (unsigned int n = 0; n < AI_MAX_NUMBER_OF_TEXTURECOORDS;++n) - ref[n] = (!mesh->mTextureCoords[n] ? true : false); + ref[n] = !mesh->mTextureCoords[n]; for (it = trafo.begin();it != trafo.end(); ++it) ref[(*it).uvIndex] = true; From 4de0237167dd8019838cb28a5622bba522f74842 Mon Sep 17 00:00:00 2001 From: iamAdrianIusca Date: Tue, 18 Feb 2020 21:55:35 +0200 Subject: [PATCH 10/12] small fix --- code/Obj/ObjFileData.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/Obj/ObjFileData.h b/code/Obj/ObjFileData.h index 33184ce82..298d1e1b7 100644 --- a/code/Obj/ObjFileData.h +++ b/code/Obj/ObjFileData.h @@ -189,7 +189,7 @@ struct Material { , ior ( ai_real( 1.0 ) ) , transparent( ai_real( 1.0), ai_real (1.0), ai_real(1.0)) { - std::fill_n(clamp, TextureTypeCount, false); + std::fill_n(clamp, static_cast(TextureTypeCount), false); } // Destructor From 8d6d8e2e3820c89fc6a2ff9758795c00e9aa0f77 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Wed, 19 Feb 2020 10:42:30 +0100 Subject: [PATCH 11/12] Update .clang-format Update clang.format to keep formatting of hashes in include file to optimize readabilaty. --- .clang-format | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.clang-format b/.clang-format index 70e0945fe..c8b8ab856 100644 --- a/.clang-format +++ b/.clang-format @@ -71,7 +71,7 @@ IncludeCategories: Priority: 3 # IncludeIsMainRegex: '(Test)?$' IndentCaseLabels: true -# IndentPPDirectives: None +IndentPPDirectives: AfterHash IndentWidth: 4 # IndentWrappedFunctionNames: false # JavaScriptQuotes: Leave From 4d27fccd0bec68a5d2525a740b1ab1eb34d0be1a Mon Sep 17 00:00:00 2001 From: Marc-Antoine Lortie Date: Wed, 19 Feb 2020 13:17:45 -0500 Subject: [PATCH 12/12] Fixed memory leaks in SimpleTexturedOpenGL sample. - Added function "cleanup" to centralize release of resources. - Added function "freeTextureIds" to free textureIds memory. - Added call to freeTextureIds in LoadGLTextures to free memory before it is allocated again. - Made several changes in KillGLWindow to prevent certain Win32 handle/resources from being released more than once. --- .../src/model_loading.cpp | 64 ++++++++++++------- 1 file changed, 42 insertions(+), 22 deletions(-) diff --git a/samples/SimpleTexturedOpenGL/SimpleTexturedOpenGL/src/model_loading.cpp b/samples/SimpleTexturedOpenGL/SimpleTexturedOpenGL/src/model_loading.cpp index 8428248d1..8c2a08b2e 100644 --- a/samples/SimpleTexturedOpenGL/SimpleTexturedOpenGL/src/model_loading.cpp +++ b/samples/SimpleTexturedOpenGL/SimpleTexturedOpenGL/src/model_loading.cpp @@ -202,8 +202,21 @@ std::string getBasePath(const std::string& path) return (std::string::npos == pos) ? "" : path.substr(0, pos + 1); } +void freeTextureIds() +{ + textureIdMap.clear(); //no need to delete pointers in it manually here. (Pointers point to textureIds deleted in next step) + + if (textureIds) + { + delete[] textureIds; + textureIds = NULL; + } +} + int LoadGLTextures(const aiScene* scene) { + freeTextureIds(); + //ILboolean success; /* Before calling ilInit() version should be checked. */ @@ -576,21 +589,24 @@ void KillGLWindow() // Properly Kill The Window hRC = NULL; } - if (hDC && !ReleaseDC(hWnd, hDC)) // Are We able to Release The DC? + if (hDC) { - MessageBox(NULL, TEXT("Release Device Context Failed."), TEXT("SHUTDOWN ERROR"), MB_OK | MB_ICONINFORMATION); - hDC=NULL; + if (!ReleaseDC(hWnd, hDC)) // Are We able to Release The DC? + MessageBox(NULL, TEXT("Release Device Context Failed."), TEXT("SHUTDOWN ERROR"), MB_OK | MB_ICONINFORMATION); + hDC = NULL; } - if (hWnd && !DestroyWindow(hWnd)) // Are We Able To Destroy The Window + if (hWnd) { - MessageBox(NULL, TEXT("Could Not Release hWnd."), TEXT("SHUTDOWN ERROR"), MB_OK | MB_ICONINFORMATION); + if (!DestroyWindow(hWnd)) // Are We Able To Destroy The Window + MessageBox(NULL, TEXT("Could Not Release hWnd."), TEXT("SHUTDOWN ERROR"), MB_OK | MB_ICONINFORMATION); hWnd = NULL; - } + } - if (!UnregisterClass(TEXT("OpenGL"), hInstance)) // Are We Able To Unregister Class + if (hInstance) { - MessageBox(NULL, TEXT("Could Not Unregister Class."), TEXT("SHUTDOWN ERROR"), MB_OK | MB_ICONINFORMATION); + if (!UnregisterClass(TEXT("OpenGL"), hInstance)) // Are We Able To Unregister Class + MessageBox(NULL, TEXT("Could Not Unregister Class."), TEXT("SHUTDOWN ERROR"), MB_OK | MB_ICONINFORMATION); hInstance = NULL; } } @@ -761,6 +777,16 @@ BOOL CreateGLWindow(const char* title, int width, int height, int bits, bool ful return TRUE; } +void cleanup() +{ + freeTextureIds(); + + destroyAILogger(); + + if (hWnd) + KillGLWindow(); +}; + LRESULT CALLBACK WndProc(HWND hWnd, // Handles for this Window UINT uMsg, // Message for this Window WPARAM wParam, // additional message Info @@ -842,7 +868,11 @@ int WINAPI WinMain( HINSTANCE hInstance, // The instance modelpath = UTFConverter(modelpathW).str(); } - if (!Import3DFromFile(modelpath)) return 0; + if (!Import3DFromFile(modelpath)) + { + cleanup(); + return 0; + } logInfo("=============== Post Import ===================="); @@ -853,6 +883,7 @@ int WINAPI WinMain( HINSTANCE hInstance, // The instance if (!CreateGLWindow(windowTitle, 640, 480, 16, fullscreen)) { + cleanup(); return 0; } @@ -893,6 +924,7 @@ int WINAPI WinMain( HINSTANCE hInstance, // The instance fullscreen=!fullscreen; if (!CreateGLWindow(windowTitle, 640, 480, 16, fullscreen)) { + cleanup(); return 0; } } @@ -900,18 +932,6 @@ int WINAPI WinMain( HINSTANCE hInstance, // The instance } // *** cleanup *** - - textureIdMap.clear(); //no need to delete pointers in it manually here. (Pointers point to textureIds deleted in next step) - - if (textureIds) - { - delete[] textureIds; - textureIds = NULL; - } - - // *** cleanup end *** - - destroyAILogger(); - KillGLWindow(); + cleanup(); return static_cast(msg.wParam); }