From 39e52341c23283e14591c59e1fb4042a7d18d9b7 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Sun, 14 Aug 2016 21:24:00 +0200 Subject: [PATCH 01/28] Fix copy-paste-error. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index af472a0b4..8b7ac1af4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -156,7 +156,7 @@ ENDIF() # Ensure that we do not run into issues like http://www.tcm.phy.cam.ac.uk/sw/inodes64.html on 32 bit linux IF( UNIX ) - IF (BUILD_SHARED_LIBS AND CMAKE_SIZEOF_VOID_P EQUAL 4) # only necessary for 32-bit linux + IF ( CMAKE_SIZEOF_VOID_P EQUAL 4) # only necessary for 32-bit linux ADD_DEFINITIONS(-D_FILE_OFFSET_BITS=64 ) ENDIF() ENDIF() From 27f81884f9dc93aa9f124f046374c6f2516403d6 Mon Sep 17 00:00:00 2001 From: "roshan.c" Date: Mon, 15 Aug 2016 09:58:53 -0700 Subject: [PATCH 02/28] adding support to store all the layered textures --- code/FBXConverter.cpp | 153 ++++++++++++++++++++++-------------------- code/FBXDocument.h | 15 +++-- code/FBXMaterial.cpp | 5 +- 3 files changed, 92 insertions(+), 81 deletions(-) diff --git a/code/FBXConverter.cpp b/code/FBXConverter.cpp index d0f1c0de4..1f6274d8f 100644 --- a/code/FBXConverter.cpp +++ b/code/FBXConverter.cpp @@ -1916,57 +1916,92 @@ void Converter::TrySetTextureProperties( aiMaterial* out_mat, const LayeredTextu return; } - const Texture* const tex = ( *it ).second->getTexture(); + int texCount = (*it).second->textureCount(); + + // Set the blend mode for layered textures + int blendmode= (*it).second->GetBlendMode(); + out_mat->AddProperty(&blendmode,1,_AI_MATKEY_TEXOP_BASE,target,0); - aiString path; - path.Set( tex->RelativeFilename() ); + for(int texIndex = 0; texIndex < texCount; texIndex++){ + + const Texture* const tex = ( *it ).second->getTexture(texIndex); - out_mat->AddProperty( &path, _AI_MATKEY_TEXTURE_BASE, target, 0 ); + aiString path; + path.Set( tex->RelativeFilename() ); - aiUVTransform uvTrafo; - // XXX handle all kinds of UV transformations - uvTrafo.mScaling = tex->UVScaling(); - uvTrafo.mTranslation = tex->UVTranslation(); - out_mat->AddProperty( &uvTrafo, 1, _AI_MATKEY_UVTRANSFORM_BASE, target, 0 ); + out_mat->AddProperty( &path, _AI_MATKEY_TEXTURE_BASE, target, texIndex ); - const PropertyTable& props = tex->Props(); + aiUVTransform uvTrafo; + // XXX handle all kinds of UV transformations + uvTrafo.mScaling = tex->UVScaling(); + uvTrafo.mTranslation = tex->UVTranslation(); + out_mat->AddProperty( &uvTrafo, 1, _AI_MATKEY_UVTRANSFORM_BASE, target, texIndex ); - int uvIndex = 0; + const PropertyTable& props = tex->Props(); - bool ok; - const std::string& uvSet = PropertyGet( props, "UVSet", ok ); - if ( ok ) { - // "default" is the name which usually appears in the FbxFileTexture template - if ( uvSet != "default" && uvSet.length() ) { - // this is a bit awkward - we need to find a mesh that uses this - // material and scan its UV channels for the given UV name because - // assimp references UV channels by index, not by name. + int uvIndex = 0; - // XXX: the case that UV channels may appear in different orders - // in meshes is unhandled. A possible solution would be to sort - // the UV channels alphabetically, but this would have the side - // effect that the primary (first) UV channel would sometimes - // be moved, causing trouble when users read only the first - // UV channel and ignore UV channel assignments altogether. + bool ok; + const std::string& uvSet = PropertyGet( props, "UVSet", ok ); + if ( ok ) { + // "default" is the name which usually appears in the FbxFileTexture template + if ( uvSet != "default" && uvSet.length() ) { + // this is a bit awkward - we need to find a mesh that uses this + // material and scan its UV channels for the given UV name because + // assimp references UV channels by index, not by name. - const unsigned int matIndex = static_cast( std::distance( materials.begin(), - std::find( materials.begin(), materials.end(), out_mat ) - ) ); + // XXX: the case that UV channels may appear in different orders + // in meshes is unhandled. A possible solution would be to sort + // the UV channels alphabetically, but this would have the side + // effect that the primary (first) UV channel would sometimes + // be moved, causing trouble when users read only the first + // UV channel and ignore UV channel assignments altogether. - uvIndex = -1; - if ( !mesh ) - { - for( const MeshMap::value_type& v : meshes_converted ) { - const MeshGeometry* const mesh = dynamic_cast ( v.first ); - if ( !mesh ) { - continue; + const unsigned int matIndex = static_cast( std::distance( materials.begin(), + std::find( materials.begin(), materials.end(), out_mat ) + ) ); + + uvIndex = -1; + if ( !mesh ) + { + for( const MeshMap::value_type& v : meshes_converted ) { + const MeshGeometry* const mesh = dynamic_cast ( v.first ); + if ( !mesh ) { + continue; + } + + const MatIndexArray& mats = mesh->GetMaterialIndices(); + if ( std::find( mats.begin(), mats.end(), matIndex ) == mats.end() ) { + continue; + } + + int index = -1; + for ( unsigned int i = 0; i < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++i ) { + if ( mesh->GetTextureCoords( i ).empty() ) { + break; + } + const std::string& name = mesh->GetTextureCoordChannelName( i ); + if ( name == uvSet ) { + index = static_cast( i ); + break; + } + } + if ( index == -1 ) { + FBXImporter::LogWarn( "did not find UV channel named " + uvSet + " in a mesh using this material" ); + continue; + } + + if ( uvIndex == -1 ) { + uvIndex = index; + } + else { + FBXImporter::LogWarn( "the UV channel named " + uvSet + + " appears at different positions in meshes, results will be wrong" ); + } } - - const MatIndexArray& mats = mesh->GetMaterialIndices(); - if ( std::find( mats.begin(), mats.end(), matIndex ) == mats.end() ) { - continue; - } - + } + else + { int index = -1; for ( unsigned int i = 0; i < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++i ) { if ( mesh->GetTextureCoords( i ).empty() ) { @@ -1980,48 +2015,22 @@ void Converter::TrySetTextureProperties( aiMaterial* out_mat, const LayeredTextu } if ( index == -1 ) { FBXImporter::LogWarn( "did not find UV channel named " + uvSet + " in a mesh using this material" ); - continue; } if ( uvIndex == -1 ) { uvIndex = index; } - else { - FBXImporter::LogWarn( "the UV channel named " + uvSet + - " appears at different positions in meshes, results will be wrong" ); - } - } - } - else - { - int index = -1; - for ( unsigned int i = 0; i < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++i ) { - if ( mesh->GetTextureCoords( i ).empty() ) { - break; - } - const std::string& name = mesh->GetTextureCoordChannelName( i ); - if ( name == uvSet ) { - index = static_cast( i ); - break; - } - } - if ( index == -1 ) { - FBXImporter::LogWarn( "did not find UV channel named " + uvSet + " in a mesh using this material" ); } if ( uvIndex == -1 ) { - uvIndex = index; + FBXImporter::LogWarn( "failed to resolve UV channel " + uvSet + ", using first UV channel" ); + uvIndex = 0; } } - - if ( uvIndex == -1 ) { - FBXImporter::LogWarn( "failed to resolve UV channel " + uvSet + ", using first UV channel" ); - uvIndex = 0; - } } - } - out_mat->AddProperty( &uvIndex, 1, _AI_MATKEY_UVWSRC_BASE, target, 0 ); + out_mat->AddProperty( &uvIndex, 1, _AI_MATKEY_UVWSRC_BASE, target, texIndex ); + } } void Converter::SetTextureProperties( aiMaterial* out_mat, const TextureMap& textures, const MeshGeometry* const mesh ) @@ -2030,6 +2039,7 @@ void Converter::SetTextureProperties( aiMaterial* out_mat, const TextureMap& tex TrySetTextureProperties( out_mat, textures, "AmbientColor", aiTextureType_AMBIENT, mesh ); TrySetTextureProperties( out_mat, textures, "EmissiveColor", aiTextureType_EMISSIVE, mesh ); TrySetTextureProperties( out_mat, textures, "SpecularColor", aiTextureType_SPECULAR, mesh ); + TrySetTextureProperties( out_mat, textures, "SpecularFactor", aiTextureType_SPECULAR, mesh); TrySetTextureProperties( out_mat, textures, "TransparentColor", aiTextureType_OPACITY, mesh ); TrySetTextureProperties( out_mat, textures, "ReflectionColor", aiTextureType_REFLECTION, mesh ); TrySetTextureProperties( out_mat, textures, "DisplacementColor", aiTextureType_DISPLACEMENT, mesh ); @@ -2044,6 +2054,7 @@ void Converter::SetTextureProperties( aiMaterial* out_mat, const LayeredTextureM TrySetTextureProperties( out_mat, layeredTextures, "AmbientColor", aiTextureType_AMBIENT, mesh ); TrySetTextureProperties( out_mat, layeredTextures, "EmissiveColor", aiTextureType_EMISSIVE, mesh ); TrySetTextureProperties( out_mat, layeredTextures, "SpecularColor", aiTextureType_SPECULAR, mesh ); + TrySetTextureProperties( out_mat, layeredTextures, "SpecularFactor", aiTextureType_SPECULAR, mesh); TrySetTextureProperties( out_mat, layeredTextures, "TransparentColor", aiTextureType_OPACITY, mesh ); TrySetTextureProperties( out_mat, layeredTextures, "ReflectionColor", aiTextureType_REFLECTION, mesh ); TrySetTextureProperties( out_mat, layeredTextures, "DisplacementColor", aiTextureType_DISPLACEMENT, mesh ); diff --git a/code/FBXDocument.h b/code/FBXDocument.h index 62d463bab..6016805ed 100644 --- a/code/FBXDocument.h +++ b/code/FBXDocument.h @@ -594,23 +594,24 @@ public: BlendMode_BlendModeCount }; - const Texture* getTexture() const + const Texture* getTexture(int index=0) const { - return texture; - } + return textures[index]; - BlendMode GetBlendMode() + } + const int textureCount() const { + return textures.size(); + } + const BlendMode GetBlendMode() const { return blendMode; } - float Alpha() { return alpha; } - private: - const Texture* texture; + std::vector textures; BlendMode blendMode; float alpha; }; diff --git a/code/FBXMaterial.cpp b/code/FBXMaterial.cpp index 43e501d67..e5e9cd259 100644 --- a/code/FBXMaterial.cpp +++ b/code/FBXMaterial.cpp @@ -227,7 +227,6 @@ Texture::~Texture() LayeredTexture::LayeredTexture(uint64_t id, const Element& element, const Document& /*doc*/, const std::string& name) : Object(id,element,name) -,texture(0) ,blendMode(BlendMode_Modulate) ,alpha(1) { @@ -249,7 +248,7 @@ LayeredTexture::LayeredTexture(uint64_t id, const Element& element, const Docume LayeredTexture::~LayeredTexture() { - + } void LayeredTexture::fillTexture(const Document& doc) @@ -267,7 +266,7 @@ void LayeredTexture::fillTexture(const Document& doc) const Texture* const tex = dynamic_cast(ob); - texture = tex; + textures.push_back(tex); } } From e9ecd6f8a7bce581655d88fe331ed7fc65af310e Mon Sep 17 00:00:00 2001 From: James Walker Date: Wed, 17 Aug 2016 17:35:16 -0700 Subject: [PATCH 03/28] Add typecasts in glTFAssetWriter.inl to fix compile errors about ambiguous constructors, see: --- code/glTFAssetWriter.inl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/code/glTFAssetWriter.inl b/code/glTFAssetWriter.inl index 0fa8556c0..f14afc4d8 100644 --- a/code/glTFAssetWriter.inl +++ b/code/glTFAssetWriter.inl @@ -105,7 +105,7 @@ namespace glTF { type = "arraybuffer"; } - obj.AddMember("byteLength", b.byteLength, w.mAl); + obj.AddMember("byteLength", (uint64_t)b.byteLength, w.mAl); obj.AddMember("type", StringRef(type), w.mAl); obj.AddMember("uri", Value(dataURI, w.mAl).Move(), w.mAl); } @@ -113,8 +113,8 @@ namespace glTF { inline void Write(Value& obj, BufferView& bv, AssetWriter& w) { obj.AddMember("buffer", Value(bv.buffer->id, w.mAl).Move(), w.mAl); - obj.AddMember("byteOffset", bv.byteOffset, w.mAl); - obj.AddMember("byteLength", bv.byteLength, w.mAl); + obj.AddMember("byteOffset", (uint64_t)bv.byteOffset, w.mAl); + obj.AddMember("byteLength", (uint64_t)bv.byteLength, w.mAl); obj.AddMember("target", int(bv.target), w.mAl); } From c2c12c1db5c8d7610ad46b1970aabf4afde0eef4 Mon Sep 17 00:00:00 2001 From: Daniel Knezevic Date: Thu, 18 Aug 2016 10:51:20 +0200 Subject: [PATCH 04/28] Use Assimp namespace to fix build for big-endian architectures --- code/glTFAsset.inl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/code/glTFAsset.inl b/code/glTFAsset.inl index d52c825c4..470246c94 100644 --- a/code/glTFAsset.inl +++ b/code/glTFAsset.inl @@ -40,6 +40,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "StringUtils.h" +using namespace Assimp; + namespace glTF { namespace { From 666d1ce164323367a0bff8b34cfa71a723989a22 Mon Sep 17 00:00:00 2001 From: James Walker Date: Thu, 18 Aug 2016 10:55:24 -0700 Subject: [PATCH 05/28] Changed C-style casts to static_cast. --- code/glTFAssetWriter.inl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/code/glTFAssetWriter.inl b/code/glTFAssetWriter.inl index f14afc4d8..415db6714 100644 --- a/code/glTFAssetWriter.inl +++ b/code/glTFAssetWriter.inl @@ -105,7 +105,7 @@ namespace glTF { type = "arraybuffer"; } - obj.AddMember("byteLength", (uint64_t)b.byteLength, w.mAl); + obj.AddMember("byteLength", static_cast(b.byteLength), w.mAl); obj.AddMember("type", StringRef(type), w.mAl); obj.AddMember("uri", Value(dataURI, w.mAl).Move(), w.mAl); } @@ -113,8 +113,8 @@ namespace glTF { inline void Write(Value& obj, BufferView& bv, AssetWriter& w) { obj.AddMember("buffer", Value(bv.buffer->id, w.mAl).Move(), w.mAl); - obj.AddMember("byteOffset", (uint64_t)bv.byteOffset, w.mAl); - obj.AddMember("byteLength", (uint64_t)bv.byteLength, w.mAl); + obj.AddMember("byteOffset", static_cast(bv.byteOffset), w.mAl); + obj.AddMember("byteLength", static_cast(bv.byteLength), w.mAl); obj.AddMember("target", int(bv.target), w.mAl); } From 50a2e80e96caf458b7a70d4dcc1003ab0441b6c2 Mon Sep 17 00:00:00 2001 From: Daniel Knezevic Date: Thu, 18 Aug 2016 10:51:20 +0200 Subject: [PATCH 06/28] Use Assimp namespace to fix build for big-endian architectures --- code/glTFAsset.inl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/code/glTFAsset.inl b/code/glTFAsset.inl index d52c825c4..470246c94 100644 --- a/code/glTFAsset.inl +++ b/code/glTFAsset.inl @@ -40,6 +40,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "StringUtils.h" +using namespace Assimp; + namespace glTF { namespace { From aef1b1c51b4fc8207745c1e9de2744d6d18eb312 Mon Sep 17 00:00:00 2001 From: Lucas Stanek Date: Thu, 18 Aug 2016 00:03:29 +0000 Subject: [PATCH 07/28] Building static ZLIB on 64b LInux requires -fPIC for C compiler. --- CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8b7ac1af4..c47fa5bd3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -164,6 +164,9 @@ ENDIF() IF((CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_GNUCXX) AND NOT CMAKE_COMPILER_IS_MINGW) IF (BUILD_SHARED_LIBS AND CMAKE_SIZEOF_VOID_P EQUAL 8) # -fPIC is only required for shared libs on 64 bit SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fPIC") + IF(ASSIMP_BUILD_ZLIB) + SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fPIC") + ENDIF() ENDIF() # hide all not-exported symbols SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fvisibility=hidden -Wall -std=c++0x" ) From 4dfe2a1bad1ae67e85aae6a483e4a80df8dc29b1 Mon Sep 17 00:00:00 2001 From: Lucas Stanek Date: Thu, 18 Aug 2016 22:23:12 +0000 Subject: [PATCH 08/28] Add -fPIC to C Flags for 64bit linux Shared Object builds without checking if zlib is being built. --- CMakeLists.txt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c47fa5bd3..5105e3a1f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -164,9 +164,7 @@ ENDIF() IF((CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_GNUCXX) AND NOT CMAKE_COMPILER_IS_MINGW) IF (BUILD_SHARED_LIBS AND CMAKE_SIZEOF_VOID_P EQUAL 8) # -fPIC is only required for shared libs on 64 bit SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fPIC") - IF(ASSIMP_BUILD_ZLIB) - SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fPIC") - ENDIF() + SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fPIC") ENDIF() # hide all not-exported symbols SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fvisibility=hidden -Wall -std=c++0x" ) From 243df452a4397b44aa2f20186ecd70534f234ae8 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Thu, 25 Aug 2016 18:20:52 +0200 Subject: [PATCH 09/28] GeometryBuilder: fix update of vertices. --- code/ObjFileImporter.cpp | 2 -- tools/assimp_view/assimp_view.cpp | 1 - 2 files changed, 3 deletions(-) diff --git a/code/ObjFileImporter.cpp b/code/ObjFileImporter.cpp index 082709dc7..2d6c859bf 100644 --- a/code/ObjFileImporter.cpp +++ b/code/ObjFileImporter.cpp @@ -39,7 +39,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. --------------------------------------------------------------------------- */ - #ifndef ASSIMP_BUILD_NO_OBJ_IMPORTER #include "DefaultIOSystem.h" @@ -52,7 +51,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include - static const aiImporterDesc desc = { "Wavefront Object Importer", "", diff --git a/tools/assimp_view/assimp_view.cpp b/tools/assimp_view/assimp_view.cpp index ddf4b0b89..811fe47e7 100644 --- a/tools/assimp_view/assimp_view.cpp +++ b/tools/assimp_view/assimp_view.cpp @@ -160,7 +160,6 @@ DWORD WINAPI LoadThreadProc(LPVOID lpParameter) // Call ASSIMPs C-API to load the file g_pcAsset->pcScene = (aiScene*)aiImportFileExWithProperties(g_szFileName, - ppsteps | /* configurable pp steps */ aiProcess_GenSmoothNormals | // generate smooth normal vectors if not existing aiProcess_SplitLargeMeshes | // split large, unrenderable meshes into submeshes From a66e644bf3f975fe8b962b8852700740fa7724ab Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 29 Aug 2016 15:24:24 +0200 Subject: [PATCH 10/28] Fix coverity finding: fix possible use after free.. --- code/XGLLoader.cpp | 83 ++++++++++++++++++++++++---------------------- code/XGLLoader.h | 9 ++--- 2 files changed, 46 insertions(+), 46 deletions(-) diff --git a/code/XGLLoader.cpp b/code/XGLLoader.cpp index fd079ef14..2df72849b 100644 --- a/code/XGLLoader.cpp +++ b/code/XGLLoader.cpp @@ -102,12 +102,16 @@ static const aiImporterDesc desc = { // ------------------------------------------------------------------------------------------------ // Constructor to be privately used by Importer XGLImporter::XGLImporter() -{} +: m_reader( nullptr ) +, m_scene( nullptr ) { + // empty +} // ------------------------------------------------------------------------------------------------ // Destructor, private as well -XGLImporter::~XGLImporter() -{} +XGLImporter::~XGLImporter() { + // empty +} // ------------------------------------------------------------------------------------------------ // Returns whether the class can handle the format of the given file. @@ -149,7 +153,7 @@ void XGLImporter::InternReadFile( const std::string& pFile, free_it free_it_really(dest); #endif - scene = pScene; + m_scene = pScene; std::shared_ptr stream( pIOHandler->Open( pFile, "rb")); // check whether we can read from the file @@ -211,14 +215,13 @@ void XGLImporter::InternReadFile( const std::string& pFile, // construct the irrXML parser CIrrXML_IOStreamReader st(stream.get()); - std::unique_ptr read( createIrrXMLReader((IFileReadCallBack*) &st) ); - reader = read.get(); + m_reader.reset( createIrrXMLReader( ( IFileReadCallBack* ) &st ) ); // parse the XML file TempScope scope; while (ReadElement()) { - if (!ASSIMP_stricmp(reader->getNodeName(),"world")) { + if (!ASSIMP_stricmp(m_reader->getNodeName(),"world")) { ReadWorld(scope); } } @@ -231,21 +234,21 @@ void XGLImporter::InternReadFile( const std::string& pFile, } // copy meshes - scene->mNumMeshes = static_cast(meshes.size()); - scene->mMeshes = new aiMesh*[scene->mNumMeshes](); - std::copy(meshes.begin(),meshes.end(),scene->mMeshes); + m_scene->mNumMeshes = static_cast(meshes.size()); + m_scene->mMeshes = new aiMesh*[m_scene->mNumMeshes](); + std::copy(meshes.begin(),meshes.end(),m_scene->mMeshes); // copy materials - scene->mNumMaterials = static_cast(materials.size()); - scene->mMaterials = new aiMaterial*[scene->mNumMaterials](); - std::copy(materials.begin(),materials.end(),scene->mMaterials); + m_scene->mNumMaterials = static_cast(materials.size()); + m_scene->mMaterials = new aiMaterial*[m_scene->mNumMaterials](); + std::copy(materials.begin(),materials.end(),m_scene->mMaterials); if (scope.light) { - scene->mNumLights = 1; - scene->mLights = new aiLight*[1]; - scene->mLights[0] = scope.light; + m_scene->mNumLights = 1; + m_scene->mLights = new aiLight*[1]; + m_scene->mLights[0] = scope.light; - scope.light->mName = scene->mRootNode->mName; + scope.light->mName = m_scene->mRootNode->mName; } scope.dismiss(); @@ -254,8 +257,8 @@ void XGLImporter::InternReadFile( const std::string& pFile, // ------------------------------------------------------------------------------------------------ bool XGLImporter::ReadElement() { - while(reader->read()) { - if (reader->getNodeType() == EXN_ELEMENT) { + while(m_reader->read()) { + if (m_reader->getNodeType() == EXN_ELEMENT) { return true; } } @@ -265,11 +268,11 @@ bool XGLImporter::ReadElement() // ------------------------------------------------------------------------------------------------ bool XGLImporter::ReadElementUpToClosing(const char* closetag) { - while(reader->read()) { - if (reader->getNodeType() == EXN_ELEMENT) { + while(m_reader->read()) { + if (m_reader->getNodeType() == EXN_ELEMENT) { return true; } - else if (reader->getNodeType() == EXN_ELEMENT_END && !ASSIMP_stricmp(reader->getNodeName(),closetag)) { + else if (m_reader->getNodeType() == EXN_ELEMENT_END && !ASSIMP_stricmp(m_reader->getNodeName(),closetag)) { return false; } } @@ -280,11 +283,11 @@ bool XGLImporter::ReadElementUpToClosing(const char* closetag) // ------------------------------------------------------------------------------------------------ bool XGLImporter::SkipToText() { - while(reader->read()) { - if (reader->getNodeType() == EXN_TEXT) { + while(m_reader->read()) { + if (m_reader->getNodeType() == EXN_TEXT) { return true; } - else if (reader->getNodeType() == EXN_ELEMENT || reader->getNodeType() == EXN_ELEMENT_END) { + else if (m_reader->getNodeType() == EXN_ELEMENT || m_reader->getNodeType() == EXN_ELEMENT_END) { ThrowException("expected text contents but found another element (or element end)"); } } @@ -294,7 +297,7 @@ bool XGLImporter::SkipToText() // ------------------------------------------------------------------------------------------------ std::string XGLImporter::GetElementName() { - const char* s = reader->getNodeName(); + const char* s = m_reader->getNodeName(); size_t len = strlen(s); std::string ret; @@ -328,7 +331,7 @@ void XGLImporter::ReadWorld(TempScope& scope) nd->mName.Set("WORLD"); } - scene->mRootNode = nd; + m_scene->mRootNode = nd; } // ------------------------------------------------------------------------------------------------ @@ -586,29 +589,29 @@ bool XGLImporter::ReadMesh(TempScope& scope) ReadMaterial(scope); } else if (s == "p") { - if (!reader->getAttributeValue("ID")) { + if (!m_reader->getAttributeValue("ID")) { LogWarn("no ID attribute on

, ignoring"); } else { - int id = reader->getAttributeValueAsInt("ID"); + int id = m_reader->getAttributeValueAsInt("ID"); t.points[id] = ReadVec3(); } } else if (s == "n") { - if (!reader->getAttributeValue("ID")) { + if (!m_reader->getAttributeValue("ID")) { LogWarn("no ID attribute on , ignoring"); } else { - int id = reader->getAttributeValueAsInt("ID"); + int id = m_reader->getAttributeValueAsInt("ID"); t.normals[id] = ReadVec3(); } } else if (s == "tc") { - if (!reader->getAttributeValue("ID")) { + if (!m_reader->getAttributeValue("ID")) { LogWarn("no ID attribute on , ignoring"); } else { - int id = reader->getAttributeValueAsInt("ID"); + int id = m_reader->getAttributeValueAsInt("ID"); t.uvs[id] = ReadVec2(); } } @@ -828,10 +831,10 @@ void XGLImporter::ReadFaceVertex(const TempMesh& t, TempFace& out) // ------------------------------------------------------------------------------------------------ unsigned int XGLImporter::ReadIDAttr() { - for(int i = 0, e = reader->getAttributeCount(); i < e; ++i) { + for(int i = 0, e = m_reader->getAttributeCount(); i < e; ++i) { - if(!ASSIMP_stricmp(reader->getAttributeName(i),"id")) { - return reader->getAttributeValueAsInt(i); + if(!ASSIMP_stricmp(m_reader->getAttributeName(i),"id")) { + return m_reader->getAttributeValueAsInt(i); } } return ~0u; @@ -844,7 +847,7 @@ float XGLImporter::ReadFloat() LogError("unexpected EOF reading float element contents"); return 0.f; } - const char* s = reader->getNodeData(), *se; + const char* s = m_reader->getNodeData(), *se; if(!SkipSpaces(&s)) { LogError("unexpected EOL, failed to parse float"); @@ -869,7 +872,7 @@ unsigned int XGLImporter::ReadIndexFromText() LogError("unexpected EOF reading index element contents"); return ~0u; } - const char* s = reader->getNodeData(), *se; + const char* s = m_reader->getNodeData(), *se; if(!SkipSpaces(&s)) { LogError("unexpected EOL, failed to parse index element"); return ~0u; @@ -894,7 +897,7 @@ aiVector2D XGLImporter::ReadVec2() LogError("unexpected EOF reading vec2 contents"); return vec; } - const char* s = reader->getNodeData(); + const char* s = m_reader->getNodeData(); for(int i = 0; i < 2; ++i) { if(!SkipSpaces(&s)) { @@ -923,7 +926,7 @@ aiVector3D XGLImporter::ReadVec3() LogError("unexpected EOF reading vec3 contents"); return vec; } - const char* s = reader->getNodeData(); + const char* s = m_reader->getNodeData(); for(int i = 0; i < 3; ++i) { if(!SkipSpaces(&s)) { diff --git a/code/XGLLoader.h b/code/XGLLoader.h index 757f9d564..c8a68ba31 100644 --- a/code/XGLLoader.h +++ b/code/XGLLoader.h @@ -51,6 +51,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include #include +#include struct aiNode; @@ -203,12 +204,8 @@ private: unsigned int ResolveMaterialRef(TempScope& scope); private: - - -private: - - irr::io::IrrXMLReader* reader; - aiScene* scene; + std::shared_ptr m_reader; + aiScene* m_scene; }; } // end of namespace Assimp From 238f14f30fb2bb7ad2d413dac86be878d5f51103 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 29 Aug 2016 15:28:37 +0200 Subject: [PATCH 11/28] Fix coverity findings: fix possible usage after calling free. --- code/glTFExporter.cpp | 6 +++--- code/glTFExporter.h | 5 ++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/code/glTFExporter.cpp b/code/glTFExporter.cpp index 7f0c086cf..3b9c1b67b 100644 --- a/code/glTFExporter.cpp +++ b/code/glTFExporter.cpp @@ -93,11 +93,11 @@ glTFExporter::glTFExporter(const char* filename, IOSystem* pIOSystem, const aiSc , mScene(pScene) , mProperties(pProperties) { - std::unique_ptr asset(new glTF::Asset(pIOSystem)); - mAsset = asset.get(); + std::unique_ptr asset(); + mAsset.reset( new glTF::Asset( pIOSystem ) ); if (isBinary) { - asset->SetAsBinary(); + mAsset->SetAsBinary(); } ExportMetadata(); diff --git a/code/glTFExporter.h b/code/glTFExporter.h index 884d1fe38..dfd0108b5 100644 --- a/code/glTFExporter.h +++ b/code/glTFExporter.h @@ -46,13 +46,12 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include + #include #include #include - #include - struct aiScene; struct aiNode; struct aiMaterial; @@ -89,7 +88,7 @@ namespace Assimp std::map mTexturesByPath; - glTF::Asset* mAsset; + std::shared_ptr mAsset; std::vector mBodyData; From 2545dee58d90c0e33c58ed7309277ffc7a678592 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 29 Aug 2016 15:32:27 +0200 Subject: [PATCH 12/28] Fix coverity findings: fix possible out-of-bound access. --- code/ASEParser.cpp | 40 +++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/code/ASEParser.cpp b/code/ASEParser.cpp index 01c58d259..b72c34269 100644 --- a/code/ASEParser.cpp +++ b/code/ASEParser.cpp @@ -25,7 +25,6 @@ conditions are met: 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 @@ -1463,30 +1462,29 @@ void Parser::ParseLV2MeshBlock(ASE::Mesh& mesh) continue; } // another mesh UV channel ... - if (TokenMatch(filePtr,"MESH_MAPPINGCHANNEL" ,19)) - { - - unsigned int iIndex = 0; + if (TokenMatch(filePtr,"MESH_MAPPINGCHANNEL" ,19)) { + unsigned int iIndex( 0 ); ParseLV4MeshLong(iIndex); - - if (iIndex < 2) - { - LogWarning("Mapping channel has an invalid index. Skipping UV channel"); + if ( 0 == iIndex ) { + LogWarning( "Mapping channel has an invalid index. Skipping UV channel" ); // skip it ... SkipSection(); + } else { + if ( iIndex < 2 ) { + LogWarning( "Mapping channel has an invalid index. Skipping UV channel" ); + // skip it ... + SkipSection(); + } + if ( iIndex > AI_MAX_NUMBER_OF_TEXTURECOORDS ) { + LogWarning( "Too many UV channels specified. Skipping channel .." ); + // skip it ... + SkipSection(); + } else { + // parse the mapping channel + ParseLV3MappingChannel( iIndex - 1, mesh ); + } + continue; } - if (iIndex > AI_MAX_NUMBER_OF_TEXTURECOORDS) - { - LogWarning("Too many UV channels specified. Skipping channel .."); - // skip it ... - SkipSection(); - } - else - { - // parse the mapping channel - ParseLV3MappingChannel(iIndex-1,mesh); - } - continue; } // mesh animation keyframe. Not supported if (TokenMatch(filePtr,"MESH_ANIMATION" ,14)) From 26aa18c75a9c1606a800b235aa75ab76ae1891c1 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 29 Aug 2016 18:23:41 +0200 Subject: [PATCH 13/28] Fix findings from code review. --- code/ASEParser.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/code/ASEParser.cpp b/code/ASEParser.cpp index b72c34269..d34da2578 100644 --- a/code/ASEParser.cpp +++ b/code/ASEParser.cpp @@ -25,7 +25,8 @@ conditions are met: derived from this software without specific prior written permission of the assimp team. -"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + 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, From 147921ac3983c3b201af2f1549c574a18a7e0535 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 29 Aug 2016 19:56:38 +0200 Subject: [PATCH 14/28] MakeVerboseFormat: fix invalid delete statement. --- code/MakeVerboseFormat.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/MakeVerboseFormat.cpp b/code/MakeVerboseFormat.cpp index d51642cf6..df3b97a4d 100644 --- a/code/MakeVerboseFormat.cpp +++ b/code/MakeVerboseFormat.cpp @@ -171,7 +171,7 @@ bool MakeVerboseFormatProcess::MakeVerboseFormat(aiMesh* pcMesh) // build output vertex weights for (unsigned int i = 0;i < pcMesh->mNumBones;++i) { - delete pcMesh->mBones[i]->mWeights; + delete [] pcMesh->mBones[i]->mWeights; if (!newWeights[i].empty()) { pcMesh->mBones[i]->mWeights = new aiVertexWeight[newWeights[i].size()]; aiVertexWeight *weightToCopy = &( newWeights[i][0] ); From c6f670ff50a5099e7f12daf91c96b46a04498f81 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 29 Aug 2016 20:05:29 +0200 Subject: [PATCH 15/28] glTFImporter: avoid out-of-bounds-access. --- code/glTFImporter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/glTFImporter.cpp b/code/glTFImporter.cpp index 6a6619b81..a1781b04c 100644 --- a/code/glTFImporter.cpp +++ b/code/glTFImporter.cpp @@ -303,7 +303,7 @@ void glTFImporter::ImportMeshes(glTF::Asset& r) attr.normal[0]->ExtractData(aim->mNormals); } - for (size_t tc = 0; tc < attr.texcoord.size() && tc <= AI_MAX_NUMBER_OF_TEXTURECOORDS; ++tc) { + for (size_t tc = 0; tc < attr.texcoord.size() && tc < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++tc) { attr.texcoord[tc]->ExtractData(aim->mTextureCoords[tc]); aim->mNumUVComponents[tc] = attr.texcoord[tc]->GetNumComponents(); From 659a55be4afe50ec2a87ca185fe8a6ce970a90ad Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 30 Aug 2016 19:46:34 +0200 Subject: [PATCH 16/28] Fix coverity finding: use deep copy instead of reference showing to mem-adress when reading and calling push_back in a std::vector. --- code/IFCGeometry.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/IFCGeometry.cpp b/code/IFCGeometry.cpp index 5a2e4a09d..3d0cda31d 100644 --- a/code/IFCGeometry.cpp +++ b/code/IFCGeometry.cpp @@ -282,7 +282,7 @@ void ProcessRevolvedAreaSolid(const IfcRevolvedAreaSolid& solid, TempMesh& resul const size_t next = (i+1)%size; result.vertcnt.push_back(4); - const IfcVector3& base_0 = out[base+i*4+3],base_1 = out[base+next*4+3]; + const IfcVector3 base_0 = out[base+i*4+3],base_1 = out[base+next*4+3]; out.push_back(base_0); out.push_back(base_1); From 2568797015d707801632faebb3d553e09e4e6cbe Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 30 Aug 2016 19:50:53 +0200 Subject: [PATCH 17/28] MDLMoader: fix resource leak. --- code/MDLMaterialLoader.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/code/MDLMaterialLoader.cpp b/code/MDLMaterialLoader.cpp index 4c36c3329..a84afad22 100644 --- a/code/MDLMaterialLoader.cpp +++ b/code/MDLMaterialLoader.cpp @@ -731,6 +731,9 @@ void MDLImporter::ParseSkinLump_3DGS_MDL7( } VALIDATE_FILE_SIZE(szCurrent); *szCurrentOut = szCurrent; + if ( nullptr != pcNew ) { + delete pcNew; + } } // ------------------------------------------------------------------------------------------------ From 23417ead46968449ab55d4feb616628e65bf3d21 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Wed, 31 Aug 2016 19:50:14 +0200 Subject: [PATCH 18/28] CREDITS: first update. --- CREDITS | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CREDITS b/CREDITS index 9186c092e..1cfe4f640 100644 --- a/CREDITS +++ b/CREDITS @@ -7,13 +7,13 @@ The following is a non-exhaustive list of all constributors over the years. If you think your name should be listed here, drop us a line and we'll add you. - Alexander Gessler, -3DS-, BLEND-, ASE-, DXF-, HMP-, MDL-, MD2-, MD3-, MD5-, MDC-, NFF-, PLY-, STL-, RAW-, OFF-, MS3D-, Q3D- and LWO-Loader, Assimp-Viewer, assimp-cmd, -noboost, Website (Admin and Design). +3DS-, BLEND-, ASE-, DXF-, HMP-, MDL-, MD2-, MD3-, MD5-, MDC-, NFF-, PLY-, STL-, RAW-, OFF-, MS3D-, Q3D- and LWO-Loader, Assimp-Viewer, assimp-cmd, -noboost, Website (Design). - Thomas Schulze, X-, Collada-, BVH-Loader, Postprocessing framework. Data structure & Interface design, documentation. - Kim Kulling, -Obj-Loader, Logging system, Scons-build environment, CMake build environment, Linux build. +Obj-, Q3BSD-, OpenGEX-Loader, Logging system, CMake-build-environment, Linux-build, Website ( Admin ), Coverity ( Admin ), Glitter ( Admin ). - R.Schmidt, Linux build, eclipse support. From b240b9d30dae43fff8009aceefbd0b0c4b8ec613 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Fri, 2 Sep 2016 20:06:31 +0200 Subject: [PATCH 19/28] Blender: fix invalid OnjectCompare op. --- code/BlenderIntermediate.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/code/BlenderIntermediate.h b/code/BlenderIntermediate.h index fc5d7bf57..83b10e870 100644 --- a/code/BlenderIntermediate.h +++ b/code/BlenderIntermediate.h @@ -123,8 +123,7 @@ namespace Blender { struct ObjectCompare { bool operator() (const Object* left, const Object* right) const { - printf( "left: %s, right: %s\n", left->id.name, right->id.name ); - return ::strncmp(left->id.name, right->id.name, strlen( left->id.name ) ) == 0; + return ::strncmp(left->id.name, right->id.name, strlen( left->id.name ) ) == -1; } }; @@ -145,8 +144,7 @@ namespace Blender { struct ObjectCompare { bool operator() (const Object* left, const Object* right) const { - printf( "left: %s, right: %s\n", left->id.name, right->id.name ); - return ::strncmp( left->id.name, right->id.name, strlen( left->id.name ) ) == 0; + return ::strncmp( left->id.name, right->id.name, strlen( left->id.name ) ) == -1; } }; From e51b7d2a6168f02573287cff497ea9630b291d9f Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Fri, 2 Sep 2016 20:06:56 +0200 Subject: [PATCH 20/28] IFC: fix possible use after free access bug. --- code/IFCOpenings.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/code/IFCOpenings.cpp b/code/IFCOpenings.cpp index 51ea14314..3197426cf 100644 --- a/code/IFCOpenings.cpp +++ b/code/IFCOpenings.cpp @@ -715,14 +715,14 @@ void FindAdjacentContours(ContourVector::iterator current, const ContourVector& const Contour& mcontour = (*it).contour; for (size_t n = 0; n < ncontour.size(); ++n) { - const IfcVector2& n0 = ncontour[n]; - const IfcVector2& n1 = ncontour[(n+1) % ncontour.size()]; + const IfcVector2 n0 = ncontour[n]; + const IfcVector2 n1 = ncontour[(n+1) % ncontour.size()]; for (size_t m = 0, mend = (is_me ? n : mcontour.size()); m < mend; ++m) { ai_assert(&mcontour != &ncontour || m < n); - const IfcVector2& m0 = mcontour[m]; - const IfcVector2& m1 = mcontour[(m+1) % mcontour.size()]; + const IfcVector2 m0 = mcontour[m]; + const IfcVector2 m1 = mcontour[(m+1) % mcontour.size()]; IfcVector2 isect0, isect1; if (IntersectingLineSegments(n0,n1, m0, m1, isect0, isect1)) { From bcdc79ba73e325510c451d343bec10230cf552f8 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Sun, 4 Sep 2016 20:22:04 +0200 Subject: [PATCH 21/28] Fix invalid release of mat + mesh. --- code/IRRMeshLoader.cpp | 49 ++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/code/IRRMeshLoader.cpp b/code/IRRMeshLoader.cpp index 0a2a54e84..567c98188 100644 --- a/code/IRRMeshLoader.cpp +++ b/code/IRRMeshLoader.cpp @@ -116,6 +116,16 @@ const aiImporterDesc* IRRMeshImporter::GetInfo () const return &desc; } +static void releaseMaterial( aiMaterial **mat ) { + delete mat; + *mat = nullptr; +} + +static void releaseMesh( aiMesh **mesh ) { + delete mesh; + *mesh = nullptr; +} + // ------------------------------------------------------------------------------------------------ // Imports the given file into the given scene structure. void IRRMeshImporter::InternReadFile( const std::string& pFile, @@ -135,7 +145,7 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, std::vector materials; std::vector meshes; materials.reserve (5); - meshes.reserve (5); + meshes.reserve(5); // temporary data - current mesh buffer aiMaterial* curMat = NULL; @@ -159,11 +169,10 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, if (!ASSIMP_stricmp(reader->getNodeName(),"buffer") && (curMat || curMesh)) { // end of previous buffer. A material and a mesh should be there if ( !curMat || !curMesh) { - DefaultLogger::get()->error("IRRMESH: A buffer must contain a mesh and a material"); - delete curMat; - delete curMesh; - } - else { + DefaultLogger::get()->error("IRRMESH: A buffer must contain a mesh and a material"); + releaseMaterial( &curMat ); + releaseMesh( &curMesh ); + } else { materials.push_back(curMat); meshes.push_back(curMesh); } @@ -183,7 +192,7 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, if (!ASSIMP_stricmp(reader->getNodeName(),"material")) { if (curMat) { DefaultLogger::get()->warn("IRRMESH: Only one material description per buffer, please"); - delete curMat;curMat = NULL; + releaseMaterial( &curMat ); } curMat = ParseMaterial(curMatFlags); } @@ -195,17 +204,16 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, // This is possible ... remove the mesh from the list and skip further reading DefaultLogger::get()->warn("IRRMESH: Found mesh with zero vertices"); - delete curMat;curMat = NULL; - - curMesh = NULL; + releaseMaterial( &curMat ); + releaseMesh( &curMesh ); textMeaning = 0; continue; } - curVertices.reserve (num); - curNormals.reserve (num); - curColors.reserve (num); - curUVs.reserve (num); + curVertices.reserve(num); + curNormals.reserve(num); + curColors.reserve(num); + curUVs.reserve(num); // Determine the file format const char* t = reader->getAttributeValueSafe("type"); @@ -240,7 +248,7 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, vertexFormat = 2; } else if (ASSIMP_stricmp("standard", t)) { - delete curMat; + releaseMaterial( &curMat ); DefaultLogger::get()->warn("IRRMESH: Unknown vertex format"); } else vertexFormat = 0; @@ -248,7 +256,7 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, } else if (!ASSIMP_stricmp(reader->getNodeName(),"indices")) { if (curVertices.empty() && curMat) { - delete curMat; + releaseMaterial( &curMat ); throw DeadlyImportError("IRRMESH: indices must come after vertices"); } @@ -264,10 +272,10 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, DefaultLogger::get()->warn("IRRMESH: Found mesh with zero indices"); // mesh - away - delete curMesh; curMesh = NULL; + releaseMesh( &curMesh ); // material - away - delete curMat;curMat = NULL; + releaseMaterial( &curMat ); textMeaning = 0; continue; @@ -469,7 +477,6 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, break; default: - // GCC complains here ... break; @@ -480,8 +487,8 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, if (curMat || curMesh) { if ( !curMat || !curMesh) { DefaultLogger::get()->error("IRRMESH: A buffer must contain a mesh and a material"); - delete curMat; - delete curMesh; + releaseMaterial( &curMat ); + releaseMesh( &curMesh ); } else { materials.push_back(curMat); From 9d4d2b2a1c549d4e958c8e0637d483a7f4e948d4 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Sun, 4 Sep 2016 20:35:07 +0200 Subject: [PATCH 22/28] ComputeUVMappingprocess: add missing initialization for scalar value. --- code/ComputeUVMappingProcess.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/ComputeUVMappingProcess.cpp b/code/ComputeUVMappingProcess.cpp index 33a6f4edb..ff5142fc0 100644 --- a/code/ComputeUVMappingProcess.cpp +++ b/code/ComputeUVMappingProcess.cpp @@ -437,7 +437,7 @@ void ComputeUVMappingProcess::Execute( aiScene* pScene) } } - unsigned int idx; + unsigned int idx( 99999999 ); // Check whether we have this mapping mode already std::list::iterator it = std::find (mappingStack.begin(),mappingStack.end(), info); From cc860ede66422c84d4a2211d52ad971011c1b0c5 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Sun, 4 Sep 2016 20:40:34 +0200 Subject: [PATCH 23/28] Fix coverity findings: fix usage after free. --- code/SMDLoader.cpp | 31 ++++++++++++++++--------------- code/SMDLoader.h | 2 +- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/code/SMDLoader.cpp b/code/SMDLoader.cpp index 7aef7bc56..2db9f15e7 100644 --- a/code/SMDLoader.cpp +++ b/code/SMDLoader.cpp @@ -74,20 +74,22 @@ static const aiImporterDesc desc = { // ------------------------------------------------------------------------------------------------ // Constructor to be privately used by Importer SMDImporter::SMDImporter() - : configFrameID(), - mBuffer(), - pScene(), - iFileSize(), - iSmallestFrame(), - dLengthOfAnim(), - bHasUVs(), - iLineNumber() -{} +: configFrameID(), +mBuffer(), +pScene( nullptr ), +iFileSize( 0 ), +iSmallestFrame( -1 ), +dLengthOfAnim( 0.0 ), +bHasUVs(false ), +iLineNumber(-1) { + // empty +} // ------------------------------------------------------------------------------------------------ // Destructor, private as well -SMDImporter::~SMDImporter() -{} +SMDImporter::~SMDImporter() { + // empty +} // ------------------------------------------------------------------------------------------------ // Returns whether the class can handle the format of the given file. @@ -133,9 +135,8 @@ void SMDImporter::InternReadFile( const std::string& pFile, aiScene* pScene, IOS // Allocate storage and copy the contents of the file to a memory buffer this->pScene = pScene; - std::vector buff(iFileSize+1); - TextFileToBuffer(file.get(),buff); - mBuffer = &buff[0]; + mBuffer.resize( iFileSize + 1 ); + TextFileToBuffer(file.get(), mBuffer ); iSmallestFrame = (1 << 31); bHasUVs = true; @@ -694,7 +695,7 @@ void SMDImporter::CreateOutputMaterials() // Parse the file void SMDImporter::ParseFile() { - const char* szCurrent = mBuffer; + const char* szCurrent = &mBuffer[0]; // read line per line ... for ( ;; ) diff --git a/code/SMDLoader.h b/code/SMDLoader.h index 0b069cc25..96ea1d195 100644 --- a/code/SMDLoader.h +++ b/code/SMDLoader.h @@ -372,7 +372,7 @@ private: unsigned int configFrameID; /** Buffer to hold the loaded file */ - const char* mBuffer; + std::vector mBuffer; /** Output scene to be filled */ From 8f808f0e2e2a49a50ef0e31645853364133784ce Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 5 Sep 2016 00:27:59 +0200 Subject: [PATCH 24/28] Doc: fix redundant doc. --- doc/dox_cmd.h | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/doc/dox_cmd.h b/doc/dox_cmd.h index e740886a1..78e755d56 100644 --- a/doc/dox_cmd.h +++ b/doc/dox_cmd.h @@ -494,21 +494,11 @@ more information can be found in the aiPostProcess.h header. --transform-uv-coords Will transform uv-coordinates if possible. - - -guv - --gen-uvcoords - Will generate uv-coordinates for textures if possible. - -fid --find-invalid-data Will look for invalid data in the imported model structure. - - -fixn - --fix normals - Imported normal vector will be fixed. - -db --debone From 9e19b5103cf1666c48b11c1d5080bc220f856d35 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 5 Sep 2016 10:48:30 +0200 Subject: [PATCH 25/28] IrrImporter: Fix release functions. --- code/IRRMeshLoader.cpp | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/code/IRRMeshLoader.cpp b/code/IRRMeshLoader.cpp index 567c98188..a06449dca 100644 --- a/code/IRRMeshLoader.cpp +++ b/code/IRRMeshLoader.cpp @@ -116,14 +116,14 @@ const aiImporterDesc* IRRMeshImporter::GetInfo () const return &desc; } -static void releaseMaterial( aiMaterial **mat ) { +static void releaseMaterial( aiMaterial *mat ) { delete mat; - *mat = nullptr; + mat = nullptr; } -static void releaseMesh( aiMesh **mesh ) { +static void releaseMesh( aiMesh *mesh ) { delete mesh; - *mesh = nullptr; + mesh = nullptr; } // ------------------------------------------------------------------------------------------------ @@ -170,8 +170,8 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, // end of previous buffer. A material and a mesh should be there if ( !curMat || !curMesh) { DefaultLogger::get()->error("IRRMESH: A buffer must contain a mesh and a material"); - releaseMaterial( &curMat ); - releaseMesh( &curMesh ); + releaseMaterial( curMat ); + releaseMesh( curMesh ); } else { materials.push_back(curMat); meshes.push_back(curMesh); @@ -192,7 +192,7 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, if (!ASSIMP_stricmp(reader->getNodeName(),"material")) { if (curMat) { DefaultLogger::get()->warn("IRRMESH: Only one material description per buffer, please"); - releaseMaterial( &curMat ); + releaseMaterial( curMat ); } curMat = ParseMaterial(curMatFlags); } @@ -204,8 +204,8 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, // This is possible ... remove the mesh from the list and skip further reading DefaultLogger::get()->warn("IRRMESH: Found mesh with zero vertices"); - releaseMaterial( &curMat ); - releaseMesh( &curMesh ); + releaseMaterial( curMat ); + releaseMesh( curMesh ); textMeaning = 0; continue; } @@ -248,7 +248,7 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, vertexFormat = 2; } else if (ASSIMP_stricmp("standard", t)) { - releaseMaterial( &curMat ); + releaseMaterial( curMat ); DefaultLogger::get()->warn("IRRMESH: Unknown vertex format"); } else vertexFormat = 0; @@ -256,7 +256,7 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, } else if (!ASSIMP_stricmp(reader->getNodeName(),"indices")) { if (curVertices.empty() && curMat) { - releaseMaterial( &curMat ); + releaseMaterial( curMat ); throw DeadlyImportError("IRRMESH: indices must come after vertices"); } @@ -272,10 +272,10 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, DefaultLogger::get()->warn("IRRMESH: Found mesh with zero indices"); // mesh - away - releaseMesh( &curMesh ); + releaseMesh( curMesh ); // material - away - releaseMaterial( &curMat ); + releaseMaterial( curMat ); textMeaning = 0; continue; @@ -487,8 +487,8 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, if (curMat || curMesh) { if ( !curMat || !curMesh) { DefaultLogger::get()->error("IRRMESH: A buffer must contain a mesh and a material"); - releaseMaterial( &curMat ); - releaseMesh( &curMesh ); + releaseMaterial( curMat ); + releaseMesh( curMesh ); } else { materials.push_back(curMat); From 0f2cea7ba6d046a02bf3bf8813bb1dfbe73fc253 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 6 Sep 2016 10:42:02 +0200 Subject: [PATCH 26/28] Blender: revert fix for suspicious crash in blender on windows. --- code/BlenderIntermediate.h | 4 ++-- code/ObjFileImporter.cpp | 43 +++++++++++++++++++------------------- code/ObjFileImporter.h | 12 +++-------- 3 files changed, 26 insertions(+), 33 deletions(-) diff --git a/code/BlenderIntermediate.h b/code/BlenderIntermediate.h index 83b10e870..0290185de 100644 --- a/code/BlenderIntermediate.h +++ b/code/BlenderIntermediate.h @@ -123,7 +123,7 @@ namespace Blender { struct ObjectCompare { bool operator() (const Object* left, const Object* right) const { - return ::strncmp(left->id.name, right->id.name, strlen( left->id.name ) ) == -1; + return ::strncmp(left->id.name, right->id.name, strlen( left->id.name ) ) == 0; } }; @@ -144,7 +144,7 @@ namespace Blender { struct ObjectCompare { bool operator() (const Object* left, const Object* right) const { - return ::strncmp( left->id.name, right->id.name, strlen( left->id.name ) ) == -1; + return ::strncmp( left->id.name, right->id.name, strlen( left->id.name ) ) == 0; } }; diff --git a/code/ObjFileImporter.cpp b/code/ObjFileImporter.cpp index 2d6c859bf..a70d69b4a 100644 --- a/code/ObjFileImporter.cpp +++ b/code/ObjFileImporter.cpp @@ -387,8 +387,7 @@ void ObjFileImporter::createVertexArray(const ObjFile::Model* pModel, const ObjFile::Object* pCurrentObject, unsigned int uiMeshIndex, aiMesh* pMesh, - unsigned int numIndices) -{ + unsigned int numIndices) { // Checking preconditions ai_assert( NULL != pCurrentObject ); @@ -398,8 +397,9 @@ void ObjFileImporter::createVertexArray(const ObjFile::Model* pModel, // Get current mesh ObjFile::Mesh *pObjMesh = pModel->m_Meshes[ uiMeshIndex ]; - if ( NULL == pObjMesh || pObjMesh->m_uiNumIndices < 1) + if ( NULL == pObjMesh || pObjMesh->m_uiNumIndices < 1 ) { return; + } // Copy vertices of this mesh instance pMesh->mNumVertices = numIndices; @@ -427,27 +427,25 @@ void ObjFileImporter::createVertexArray(const ObjFile::Model* pModel, // Copy vertices, normals and textures into aiMesh instance unsigned int newIndex = 0, outIndex = 0; - for ( size_t index=0; index < pObjMesh->m_Faces.size(); index++ ) - { + for ( size_t index=0; index < pObjMesh->m_Faces.size(); index++ ) { // Get source face ObjFile::Face *pSourceFace = pObjMesh->m_Faces[ index ]; // Copy all index arrays - for ( size_t vertexIndex = 0, outVertexIndex = 0; vertexIndex < pSourceFace->m_pVertices->size(); vertexIndex++ ) - { + for ( size_t vertexIndex = 0, outVertexIndex = 0; vertexIndex < pSourceFace->m_pVertices->size(); vertexIndex++ ) { const unsigned int vertex = pSourceFace->m_pVertices->at( vertexIndex ); - if ( vertex >= pModel->m_Vertices.size() ) + if ( vertex >= pModel->m_Vertices.size() ) { throw DeadlyImportError( "OBJ: vertex index out of range" ); + } pMesh->mVertices[ newIndex ] = pModel->m_Vertices[ vertex ]; // Copy all normals - if ( !pModel->m_Normals.empty() && vertexIndex < pSourceFace->m_pNormals->size()) - { + if ( !pModel->m_Normals.empty() && vertexIndex < pSourceFace->m_pNormals->size()) { const unsigned int normal = pSourceFace->m_pNormals->at( vertexIndex ); - if ( normal >= pModel->m_Normals.size() ) - throw DeadlyImportError("OBJ: vertex normal index out of range"); - + if ( normal >= pModel->m_Normals.size() ) { + throw DeadlyImportError( "OBJ: vertex normal index out of range" ); + } pMesh->mNormals[ newIndex ] = pModel->m_Normals[ normal ]; } @@ -544,20 +542,21 @@ void ObjFileImporter::countObjects(const std::vector &rObjects // ------------------------------------------------------------------------------------------------ // Add clamp mode property to material if necessary -void ObjFileImporter::addTextureMappingModeProperty(aiMaterial* mat, aiTextureType type, int clampMode) -{ - ai_assert( NULL != mat); - mat->AddProperty(&clampMode, 1, AI_MATKEY_MAPPINGMODE_U(type, 0)); - mat->AddProperty(&clampMode, 1, AI_MATKEY_MAPPINGMODE_V(type, 0)); +void ObjFileImporter::addTextureMappingModeProperty( aiMaterial* mat, aiTextureType type, int clampMode) { + if ( nullptr == mat ) { + return; + } + + mat->AddProperty( &clampMode, 1, AI_MATKEY_MAPPINGMODE_U( type, 0 ) ); + mat->AddProperty( &clampMode, 1, AI_MATKEY_MAPPINGMODE_V( type, 0 ) ); } // ------------------------------------------------------------------------------------------------ // Creates the material -void ObjFileImporter::createMaterials(const ObjFile::Model* pModel, aiScene* pScene ) -{ - ai_assert( NULL != pScene ); - if ( NULL == pScene ) +void ObjFileImporter::createMaterials(const ObjFile::Model* pModel, aiScene* pScene ) { + if ( NULL == pScene ) { return; + } const unsigned int numMaterials = (unsigned int) pModel->m_MaterialLib.size(); pScene->mNumMaterials = 0; diff --git a/code/ObjFileImporter.h b/code/ObjFileImporter.h index b03d00b9b..8c482017b 100644 --- a/code/ObjFileImporter.h +++ b/code/ObjFileImporter.h @@ -37,8 +37,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. ---------------------------------------------------------------------- */ - - #ifndef OBJ_FILE_IMPORTER_H_INC #define OBJ_FILE_IMPORTER_H_INC @@ -49,11 +47,9 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. struct aiMesh; struct aiNode; -namespace Assimp -{ +namespace Assimp { -namespace ObjFile -{ +namespace ObjFile { struct Object; struct Model; } @@ -62,8 +58,7 @@ struct Model; /// \class ObjFileImporter /// \brief Imports a waveform obj file // ------------------------------------------------------------------------------------------------ -class ObjFileImporter : public BaseImporter -{ +class ObjFileImporter : public BaseImporter { public: /// \brief Default constructor ObjFileImporter(); @@ -77,7 +72,6 @@ public: bool CanRead( const std::string& pFile, IOSystem* pIOHandler, bool checkSig) const; private: - //! \brief Appends the supported extension. const aiImporterDesc* GetInfo () const; From ffdca3593bbdd291379321269cd5aa1586add42b Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 6 Sep 2016 15:41:37 +0200 Subject: [PATCH 27/28] ObjImporter: remove unused code. --- code/ObjFileData.h | 2 -- code/ObjFileParser.cpp | 18 ++++++++---------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/code/ObjFileData.h b/code/ObjFileData.h index 65d982997..2d282e6a9 100644 --- a/code/ObjFileData.h +++ b/code/ObjFileData.h @@ -285,8 +285,6 @@ struct Model ObjFile::Material *m_pDefaultMaterial; //! Vector with all generated materials std::vector m_MaterialLib; - //! Vector with all generated group - std::vector m_GroupLib; //! Vector with all generated vertices std::vector m_Vertices; //! vector with all generated normals diff --git a/code/ObjFileParser.cpp b/code/ObjFileParser.cpp index 5314826c3..4c3232fa7 100644 --- a/code/ObjFileParser.cpp +++ b/code/ObjFileParser.cpp @@ -692,38 +692,36 @@ int ObjFileParser::getMaterialIndex( const std::string &strMaterialName ) // ------------------------------------------------------------------- // Getter for a group name. -void ObjFileParser::getGroupName() -{ - std::string strGroupName; +void ObjFileParser::getGroupName() { + std::string groupName; // here we skip 'g ' from line m_DataIt = getNextToken(m_DataIt, m_DataItEnd); - m_DataIt = getName(m_DataIt, m_DataItEnd, strGroupName); + m_DataIt = getName(m_DataIt, m_DataItEnd, groupName); if( isEndOfBuffer( m_DataIt, m_DataItEnd ) ) { return; } // Change active group, if necessary - if ( m_pModel->m_strActiveGroup != strGroupName ) - { + if ( m_pModel->m_strActiveGroup != groupName ) { // Search for already existing entry - ObjFile::Model::ConstGroupMapIt it = m_pModel->m_Groups.find(strGroupName); + ObjFile::Model::ConstGroupMapIt it = m_pModel->m_Groups.find(groupName); // We are mapping groups into the object structure - createObject( strGroupName ); + createObject( groupName ); // New group name, creating a new entry if (it == m_pModel->m_Groups.end()) { std::vector *pFaceIDArray = new std::vector; - m_pModel->m_Groups[ strGroupName ] = pFaceIDArray; + m_pModel->m_Groups[ groupName ] = pFaceIDArray; m_pModel->m_pGroupFaceIDs = (pFaceIDArray); } else { m_pModel->m_pGroupFaceIDs = (*it).second; } - m_pModel->m_strActiveGroup = strGroupName; + m_pModel->m_strActiveGroup = groupName; } m_DataIt = skipLine( m_DataIt, m_DataItEnd, m_uiLine ); } From 702d57fbae1a87919cf106b99e718022a7f9870f Mon Sep 17 00:00:00 2001 From: johnmaf Date: Wed, 7 Sep 2016 17:03:19 -0400 Subject: [PATCH 28/28] Split mesh before exporting gltf. Fixes #995 --- code/glTFExporter.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/code/glTFExporter.cpp b/code/glTFExporter.cpp index 3b9c1b67b..e333272be 100644 --- a/code/glTFExporter.cpp +++ b/code/glTFExporter.cpp @@ -49,6 +49,9 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "StringComparison.h" #include "ByteSwapper.h" +#include "SplitLargeMeshes.h" +#include "SceneCombiner.h" + #include #include #include @@ -70,8 +73,20 @@ namespace Assimp { // Worker function for exporting a scene to GLTF. Prototyped and registered in Exporter.cpp void ExportSceneGLTF(const char* pFile, IOSystem* pIOSystem, const aiScene* pScene, const ExportProperties* pProperties) { + aiScene* sceneCopy_tmp; + SceneCombiner::CopyScene(&sceneCopy_tmp, pScene); + std::unique_ptr sceneCopy(sceneCopy_tmp); + + SplitLargeMeshesProcess_Triangle tri_splitter; + tri_splitter.SetLimit(0xffff); + tri_splitter.Execute(sceneCopy.get()); + + SplitLargeMeshesProcess_Vertex vert_splitter; + vert_splitter.SetLimit(0xffff); + vert_splitter.Execute(sceneCopy.get()); + // invoke the exporter - glTFExporter exporter(pFile, pIOSystem, pScene, pProperties, false); + glTFExporter exporter(pFile, pIOSystem, sceneCopy.get(), pProperties, false); } // ------------------------------------------------------------------------------------------------