From 9e46fca9a911db010a6be87780073336785efccb Mon Sep 17 00:00:00 2001 From: Max Vollmer Date: Wed, 29 Jan 2020 15:06:48 +0000 Subject: [PATCH 1/6] Added missing checks for tempData and uvIndices sizes in all cases --- code/FBX/FBXMeshGeometry.cpp | 38 ++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/code/FBX/FBXMeshGeometry.cpp b/code/FBX/FBXMeshGeometry.cpp index 70e35ccb3..003f0870c 100644 --- a/code/FBX/FBXMeshGeometry.cpp +++ b/code/FBX/FBXMeshGeometry.cpp @@ -448,6 +448,12 @@ void ResolveVertexDataArray(std::vector& data_out, const Scope& source, std::vector tempData; ParseVectorDataArray(tempData, GetRequiredElement(source, dataElementName)); + if (tempData.size() != vertex_count) { + FBXImporter::LogError(Formatter::format("length of input data unexpected for ByVertice mapping: ") + << tempData.size() << ", expected " << vertex_count); + return; + } + data_out.resize(vertex_count); for (size_t i = 0, e = tempData.size(); i < e; ++i) { @@ -461,10 +467,23 @@ void ResolveVertexDataArray(std::vector& data_out, const Scope& source, std::vector tempData; ParseVectorDataArray(tempData, GetRequiredElement(source, dataElementName)); - data_out.resize(vertex_count); + if (tempData.size() != vertex_count) { + FBXImporter::LogError(Formatter::format("length of input data unexpected for ByVertice mapping: ") + << tempData.size() << ", expected " << vertex_count); + return; + } std::vector uvIndices; ParseVectorDataArray(uvIndices,GetRequiredElement(source,indexDataElementName)); + + if (uvIndices.size() != vertex_count) { + FBXImporter::LogError(Formatter::format("length of input data unexpected for ByVertice mapping: ") + << uvIndices.size() << ", expected " << vertex_count); + return; + } + + data_out.resize(vertex_count); + for (size_t i = 0, e = uvIndices.size(); i < e; ++i) { const unsigned int istart = mapping_offsets[i], iend = istart + mapping_counts[i]; @@ -493,15 +512,22 @@ void ResolveVertexDataArray(std::vector& data_out, const Scope& source, std::vector tempData; ParseVectorDataArray(tempData, GetRequiredElement(source, dataElementName)); - data_out.resize(vertex_count); + if (tempData.size() != vertex_count) { + FBXImporter::LogError(Formatter::format("length of input data unexpected for ByPolygonVertex mapping: ") + << tempData.size() << ", expected " << vertex_count); + return; + } std::vector uvIndices; ParseVectorDataArray(uvIndices,GetRequiredElement(source,indexDataElementName)); - if (uvIndices.size() != vertex_count) { - FBXImporter::LogError("length of input data unexpected for ByPolygonVertex mapping"); - return; - } + if (uvIndices.size() != vertex_count) { + FBXImporter::LogError(Formatter::format("length of input data unexpected for ByPolygonVertex mapping: ") + << uvIndices.size() << ", expected " << vertex_count); + return; + } + + data_out.resize(vertex_count); const T empty; unsigned int next = 0; From 2c1c1d846e4584990e2397f195fb3225ec6a8668 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Lortie Date: Thu, 30 Jan 2020 16:40:34 -0500 Subject: [PATCH 2/6] Renamed WriteDumb.cpp to WriteDump.cpp This includes as well changes to places referencing WriteDumb.cpp. --- code/Common/assbin_chunks.h | 2 +- tools/assimp_cmd/CMakeLists.txt | 2 +- tools/assimp_cmd/Main.h | 2 +- tools/assimp_cmd/{WriteDumb.cpp => WriteDump.cpp} | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename tools/assimp_cmd/{WriteDumb.cpp => WriteDump.cpp} (99%) diff --git a/code/Common/assbin_chunks.h b/code/Common/assbin_chunks.h index 15e4af5e7..822df5198 100644 --- a/code/Common/assbin_chunks.h +++ b/code/Common/assbin_chunks.h @@ -37,7 +37,7 @@ The ASSBIN file format is composed of chunks to represent the hierarchical aiSce This makes the format extensible and allows backward-compatibility with future data structure versions. The <root>/code/assbin_chunks.h header contains some magic constants for use by stand-alone ASSBIN loaders. Also, Assimp's own file writer can be found -in <root>/tools/assimp_cmd/WriteDumb.cpp (yes, the 'b' is no typo ...). +in <root>/tools/assimp_cmd/WriteDump.cpp (yes, the 'b' is no typo ...). @verbatim diff --git a/tools/assimp_cmd/CMakeLists.txt b/tools/assimp_cmd/CMakeLists.txt index ef7b7f054..fcf36c356 100644 --- a/tools/assimp_cmd/CMakeLists.txt +++ b/tools/assimp_cmd/CMakeLists.txt @@ -54,7 +54,7 @@ ADD_EXECUTABLE( assimp_cmd Main.cpp Main.h resource.h - WriteDumb.cpp + WriteDump.cpp Info.cpp Export.cpp ) diff --git a/tools/assimp_cmd/Main.h b/tools/assimp_cmd/Main.h index 7a6ac942d..ecc65345d 100644 --- a/tools/assimp_cmd/Main.h +++ b/tools/assimp_cmd/Main.h @@ -168,7 +168,7 @@ bool ExportModel(const aiScene* pOut, // ------------------------------------------------------------------------------ /** assimp_dump utility - * @param params Command line parameters to 'assimp dumb' + * @param params Command line parameters to 'assimp dump' * @param Number of params * @return An #AssimpCmdError value.*/ int Assimp_Dump ( diff --git a/tools/assimp_cmd/WriteDumb.cpp b/tools/assimp_cmd/WriteDump.cpp similarity index 99% rename from tools/assimp_cmd/WriteDumb.cpp rename to tools/assimp_cmd/WriteDump.cpp index 0403f3966..67d1a0e76 100644 --- a/tools/assimp_cmd/WriteDumb.cpp +++ b/tools/assimp_cmd/WriteDump.cpp @@ -41,7 +41,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. --------------------------------------------------------------------------- */ -/** @file WriteTextDumb.cpp +/** @file WriteDump.cpp * @brief Implementation of the 'assimp dump' utility */ From bbe6f7f213b5e2ee689146df5c068dc1a55ea919 Mon Sep 17 00:00:00 2001 From: Hanif Bin Ariffin Date: Thu, 30 Jan 2020 18:42:30 -0500 Subject: [PATCH 3/6] Fixed a bunch of clang warnings 1. Fix misleading indentation warnings. 2. Fix creating a temporary const copy when iterating over a map (thanks to range analysis warnings) 3. Fix creating a copy when iterating over a range without reference qualifier (also thanks to range analysis warnings) --- code/COB/COBLoader.cpp | 2 +- code/Importer/IFC/IFCCurve.cpp | 4 ++-- code/MDL/MDLLoader.cpp | 2 +- code/XGL/XGLLoader.cpp | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/code/COB/COBLoader.cpp b/code/COB/COBLoader.cpp index 7b82ab662..8e7c81192 100644 --- a/code/COB/COBLoader.cpp +++ b/code/COB/COBLoader.cpp @@ -250,7 +250,7 @@ aiNode* COBImporter::BuildNodes(const Node& root,const Scene& scin,aiScene* fill const Mesh& ndmesh = (const Mesh&)(root); if (ndmesh.vertex_positions.size() && ndmesh.texture_coords.size()) { - typedef std::pair Entry; + typedef std::pair Entry; for(const Entry& reflist : ndmesh.temp_map) { { // create mesh size_t n = 0; diff --git a/code/Importer/IFC/IFCCurve.cpp b/code/Importer/IFC/IFCCurve.cpp index 6ff330052..b16df9c5c 100644 --- a/code/Importer/IFC/IFCCurve.cpp +++ b/code/Importer/IFC/IFCCurve.cpp @@ -323,7 +323,7 @@ public: // oh well. bool have_param = false, have_point = false; IfcVector3 point; - for(const Entry sel :entity.Trim1) { + for(const Entry& sel :entity.Trim1) { if (const ::Assimp::STEP::EXPRESS::REAL* const r = sel->ToPtr<::Assimp::STEP::EXPRESS::REAL>()) { range.first = *r; have_param = true; @@ -340,7 +340,7 @@ public: } } have_param = false, have_point = false; - for(const Entry sel :entity.Trim2) { + for(const Entry& sel :entity.Trim2) { if (const ::Assimp::STEP::EXPRESS::REAL* const r = sel->ToPtr<::Assimp::STEP::EXPRESS::REAL>()) { range.second = *r; have_param = true; diff --git a/code/MDL/MDLLoader.cpp b/code/MDL/MDLLoader.cpp index 21a6bc29b..8894f46ec 100644 --- a/code/MDL/MDLLoader.cpp +++ b/code/MDL/MDLLoader.cpp @@ -1421,7 +1421,7 @@ void MDLImporter::InternReadFile_3DGS_MDL7( ) avOutList[i].reserve(3); // buffer to held the names of all groups in the file - const size_t buffersize( AI_MDL7_MAX_GROUPNAMESIZE*pcHeader->groups_num ); + const size_t buffersize(AI_MDL7_MAX_GROUPNAMESIZE*pcHeader->groups_num); char* aszGroupNameBuffer = new char[ buffersize ]; // read all groups diff --git a/code/XGL/XGLLoader.cpp b/code/XGL/XGLLoader.cpp index 24ed5d57c..403205e03 100644 --- a/code/XGL/XGLLoader.cpp +++ b/code/XGL/XGLLoader.cpp @@ -685,7 +685,7 @@ bool XGLImporter::ReadMesh(TempScope& scope) } // finally extract output meshes and add them to the scope - typedef std::pair pairt; + typedef std::pair pairt; for(const pairt& p : bymat) { aiMesh* const m = ToOutputMesh(p.second); scope.meshes_linear.push_back(m); From ede860173eeb232db58fb768d9c097ed14edf2b4 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Lortie Date: Fri, 31 Jan 2020 16:43:20 -0500 Subject: [PATCH 4/6] Fixed mValues allocated twice. mValues is already allocated in aiMetadata::Alloc(). --- code/Common/SceneCombiner.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/code/Common/SceneCombiner.cpp b/code/Common/SceneCombiner.cpp index 9471907de..29b6082a8 100644 --- a/code/Common/SceneCombiner.cpp +++ b/code/Common/SceneCombiner.cpp @@ -1312,7 +1312,6 @@ void SceneCombiner::Copy(aiMetadata** _dest, const aiMetadata* src) { aiMetadata* dest = *_dest = aiMetadata::Alloc( src->mNumProperties ); std::copy(src->mKeys, src->mKeys + src->mNumProperties, dest->mKeys); - dest->mValues = new aiMetadataEntry[src->mNumProperties]; for (unsigned int i = 0; i < src->mNumProperties; ++i) { aiMetadataEntry& in = src->mValues[i]; aiMetadataEntry& out = dest->mValues[i]; From 8e009c3d0cd11bcabf7a66952d33e3fa55b9ae12 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 3 Feb 2020 19:06:36 +0100 Subject: [PATCH 5/6] Update MDLLoader.cpp fix a tab. --- code/MDL/MDLLoader.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/code/MDL/MDLLoader.cpp b/code/MDL/MDLLoader.cpp index 8894f46ec..11c6e3ecb 100644 --- a/code/MDL/MDLLoader.cpp +++ b/code/MDL/MDLLoader.cpp @@ -1422,10 +1422,10 @@ void MDLImporter::InternReadFile_3DGS_MDL7( ) // buffer to held the names of all groups in the file const size_t buffersize(AI_MDL7_MAX_GROUPNAMESIZE*pcHeader->groups_num); - char* aszGroupNameBuffer = new char[ buffersize ]; + char* aszGroupNameBuffer = new char[ buffersize ]; // read all groups - for (unsigned int iGroup = 0; iGroup < (unsigned int)pcHeader->groups_num;++iGroup) { + for (unsigned int iGroup = 0; iGroup < (unsigned int)pcHeader->groups_num; ++iGroup) { MDL::IntGroupInfo_MDL7 groupInfo((BE_NCONST MDL::Group_MDL7*)szCurrent,iGroup); szCurrent = (const unsigned char*)(groupInfo.pcGroup+1); From cb55e2658db85558104b828d17b3455d0a24c6f0 Mon Sep 17 00:00:00 2001 From: Max Vollmer Date: Wed, 5 Feb 2020 11:07:39 +0000 Subject: [PATCH 6/6] Removed unnecessary checks that may result in false positives rejecting valid models --- code/FBX/FBXMeshGeometry.cpp | 46 +++++++++++++----------------------- 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/code/FBX/FBXMeshGeometry.cpp b/code/FBX/FBXMeshGeometry.cpp index 003f0870c..2f2782182 100644 --- a/code/FBX/FBXMeshGeometry.cpp +++ b/code/FBX/FBXMeshGeometry.cpp @@ -448,11 +448,11 @@ void ResolveVertexDataArray(std::vector& data_out, const Scope& source, std::vector tempData; ParseVectorDataArray(tempData, GetRequiredElement(source, dataElementName)); - if (tempData.size() != vertex_count) { - FBXImporter::LogError(Formatter::format("length of input data unexpected for ByVertice mapping: ") - << tempData.size() << ", expected " << vertex_count); - return; - } + if (tempData.size() != vertex_count) { + FBXImporter::LogError(Formatter::format("length of input data unexpected for ByVertice mapping: ") + << tempData.size() << ", expected " << vertex_count); + return; + } data_out.resize(vertex_count); for (size_t i = 0, e = tempData.size(); i < e; ++i) { @@ -467,22 +467,16 @@ void ResolveVertexDataArray(std::vector& data_out, const Scope& source, std::vector tempData; ParseVectorDataArray(tempData, GetRequiredElement(source, dataElementName)); - if (tempData.size() != vertex_count) { - FBXImporter::LogError(Formatter::format("length of input data unexpected for ByVertice mapping: ") - << tempData.size() << ", expected " << vertex_count); - return; - } - std::vector uvIndices; ParseVectorDataArray(uvIndices,GetRequiredElement(source,indexDataElementName)); - if (uvIndices.size() != vertex_count) { - FBXImporter::LogError(Formatter::format("length of input data unexpected for ByVertice mapping: ") - << uvIndices.size() << ", expected " << vertex_count); - return; - } + if (uvIndices.size() != vertex_count) { + FBXImporter::LogError(Formatter::format("length of input data unexpected for ByVertice mapping: ") + << uvIndices.size() << ", expected " << vertex_count); + return; + } - data_out.resize(vertex_count); + data_out.resize(vertex_count); for (size_t i = 0, e = uvIndices.size(); i < e; ++i) { @@ -512,22 +506,16 @@ void ResolveVertexDataArray(std::vector& data_out, const Scope& source, std::vector tempData; ParseVectorDataArray(tempData, GetRequiredElement(source, dataElementName)); - if (tempData.size() != vertex_count) { - FBXImporter::LogError(Formatter::format("length of input data unexpected for ByPolygonVertex mapping: ") - << tempData.size() << ", expected " << vertex_count); - return; - } - std::vector uvIndices; ParseVectorDataArray(uvIndices,GetRequiredElement(source,indexDataElementName)); - if (uvIndices.size() != vertex_count) { - FBXImporter::LogError(Formatter::format("length of input data unexpected for ByPolygonVertex mapping: ") - << uvIndices.size() << ", expected " << vertex_count); - return; - } + if (uvIndices.size() != vertex_count) { + FBXImporter::LogError(Formatter::format("length of input data unexpected for ByPolygonVertex mapping: ") + << uvIndices.size() << ", expected " << vertex_count); + return; + } - data_out.resize(vertex_count); + data_out.resize(vertex_count); const T empty; unsigned int next = 0;