From 9e46fca9a911db010a6be87780073336785efccb Mon Sep 17 00:00:00 2001 From: Max Vollmer Date: Wed, 29 Jan 2020 15:06:48 +0000 Subject: [PATCH 1/5] 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 bbe6f7f213b5e2ee689146df5c068dc1a55ea919 Mon Sep 17 00:00:00 2001 From: Hanif Bin Ariffin Date: Thu, 30 Jan 2020 18:42:30 -0500 Subject: [PATCH 2/5] 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 3/5] 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 4/5] 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 5/5] 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;