From f15f37634d5942d46aeff11730f9372357515b4c Mon Sep 17 00:00:00 2001 From: Charlie Gettys Date: Sat, 30 Mar 2019 14:12:13 -0400 Subject: [PATCH] BlenderDNA.h: * Fix rethrow that would crash the program. * QUESTION: am I throwing the right exception here. COBLoader: * catch exception by const ref * fix equality checks using strncmp taht weren't actually checking equality FBXMaterial: Catch exception by const ref (+ Debug log that it's done so) FBXConverter: Rename local variables to avoid shadowing parameter ImageExtractor: Remove duplicated/unreachable code FBXConverter: Another shadowed variable fix MD5Loader: 2 shadowed variables IRRLoader: shadowed parameter StepFileReader.cpp: Shadowed parameter IRRLoader: remove empty else statement STLExporter: Throw error instead of silently ignoring unimplemented option Misc empty blocks removed or debug logging added --- code/AssxmlExporter.cpp | 2 -- code/BlenderDNA.h | 2 +- code/COBLoader.cpp | 10 +++---- code/D3MFOpcPackage.cpp | 5 +++- code/FBXConverter.cpp | 30 ++++++++++----------- code/FBXMaterial.cpp | 5 +++- code/IRRLoader.cpp | 25 +++++++++-------- code/Importer/STEPParser/STEPFileReader.cpp | 6 ++--- code/MD5Loader.cpp | 18 ++++++------- code/STLExporter.cpp | 2 +- code/glTFExporter.cpp | 2 +- tools/assimp_cmd/ImageExtractor.cpp | 9 +++---- 12 files changed, 58 insertions(+), 58 deletions(-) diff --git a/code/AssxmlExporter.cpp b/code/AssxmlExporter.cpp index fc9a6bae5..937eebe92 100644 --- a/code/AssxmlExporter.cpp +++ b/code/AssxmlExporter.cpp @@ -555,8 +555,6 @@ void WriteDump(const aiScene* scene, IOStream* io, bool shortened) { mesh->mNormals[n].z); } } - else { - } ioprintf(io,"\t\t\n"); } diff --git a/code/BlenderDNA.h b/code/BlenderDNA.h index 5d3a4f6ea..375d0c4bf 100644 --- a/code/BlenderDNA.h +++ b/code/BlenderDNA.h @@ -416,7 +416,7 @@ template <> struct Structure :: _defaultInitializer { void operator ()(T& /*out*/,const char* = "") { // obviously, it is crucial that _DefaultInitializer is used // only from within a catch clause. - throw; + throw DeadlyImportError("Constructing BlenderDNA Structure encountered an error"); } }; diff --git a/code/COBLoader.cpp b/code/COBLoader.cpp index efb22e08b..b7e0f9728 100644 --- a/code/COBLoader.cpp +++ b/code/COBLoader.cpp @@ -144,7 +144,7 @@ void COBImporter::InternReadFile( const std::string& pFile, aiScene* pScene, IOS // check header char head[32]; stream->CopyAndAdvance(head,32); - if (strncmp(head,"Caligari ",9)) { + if (strncmp(head,"Caligari ",9) != 0) { ThrowException("Could not found magic id: `Caligari`"); } @@ -656,14 +656,14 @@ void COBImporter::ReadLght_Ascii(Scene& out, LineSplitter& splitter, const Chunk ReadFloat3Tuple_Ascii(msh.color ,&rgb); SkipSpaces(&rgb); - if (strncmp(rgb,"cone angle",10)) { + if (strncmp(rgb,"cone angle",10) != 0) { ASSIMP_LOG_WARN_F( "Expected `cone angle` entity in `color` line in `Lght` chunk ", nfo.id ); } SkipSpaces(rgb+10,&rgb); msh.angle = fast_atof(&rgb); SkipSpaces(&rgb); - if (strncmp(rgb,"inner angle",11)) { + if (strncmp(rgb,"inner angle",11) != 0) { ASSIMP_LOG_WARN_F( "Expected `inner angle` entity in `color` line in `Lght` chunk ", nfo.id); } SkipSpaces(rgb+11,&rgb); @@ -903,7 +903,7 @@ public: if(nfo.size != static_cast(-1)) { try { reader.IncPtr( static_cast< int >( nfo.size ) - reader.GetCurrentPos() + cur ); - } catch ( DeadlyImportError e ) { + } catch (const DeadlyImportError& e ) { // out of limit so correct the value reader.IncPtr( reader.GetReadLimit() ); } @@ -1214,7 +1214,7 @@ void COBImporter::ReadGrou_Binary(COB::Scene& out, StreamReaderLE& reader, const const chunk_guard cn(nfo,reader); - out.nodes.push_back(std::shared_ptr(new Group())); + out.nodes.push_back(std::make_shared()); Group& msh = (Group&)(*out.nodes.back().get()); msh = nfo; diff --git a/code/D3MFOpcPackage.cpp b/code/D3MFOpcPackage.cpp index 2545a2750..aeadba5cc 100644 --- a/code/D3MFOpcPackage.cpp +++ b/code/D3MFOpcPackage.cpp @@ -476,8 +476,11 @@ D3MFOpcPackage::D3MFOpcPackage(IOSystem* pIOHandler, const std::string& rFile) mZipArchive->Close( fileStream ); } else if( file == D3MF::XmlTag::CONTENT_TYPES_ARCHIVE) { - + ASSIMP_LOG_WARN_F("Ignored file of unsupported type CONTENT_TYPES_ARCHIVES",file); + } else { + ASSIMP_LOG_WARN_F("Ignored file of unknown type: ",file); } + } } diff --git a/code/FBXConverter.cpp b/code/FBXConverter.cpp index a61ae4006..000c4ed53 100644 --- a/code/FBXConverter.cpp +++ b/code/FBXConverter.cpp @@ -1712,22 +1712,22 @@ namespace Assimp { if (!mesh) { for (const MeshMap::value_type& v : meshes_converted) { - const MeshGeometry* const mesh = dynamic_cast (v.first); - if (!mesh) { + const MeshGeometry* const meshGeom = dynamic_cast (v.first); + if (!meshGeom) { continue; } - const MatIndexArray& mats = mesh->GetMaterialIndices(); + const MatIndexArray& mats = meshGeom->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()) { + if (meshGeom->GetTextureCoords(i).empty()) { break; } - const std::string& name = mesh->GetTextureCoordChannelName(i); + const std::string& name = meshGeom->GetTextureCoordChannelName(i); if (name == uvSet) { index = static_cast(i); break; @@ -1835,22 +1835,22 @@ namespace Assimp { if (!mesh) { for (const MeshMap::value_type& v : meshes_converted) { - const MeshGeometry* const mesh = dynamic_cast (v.first); - if (!mesh) { + const MeshGeometry* const meshGeom = dynamic_cast (v.first); + if (!meshGeom) { continue; } - const MatIndexArray& mats = mesh->GetMaterialIndices(); + const MatIndexArray& mats = meshGeom->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()) { + if (meshGeom->GetTextureCoords(i).empty()) { break; } - const std::string& name = mesh->GetTextureCoordChannelName(i); + const std::string& name = meshGeom->GetTextureCoordChannelName(i); if (name == uvSet) { index = static_cast(i); break; @@ -2191,22 +2191,22 @@ void FBXConverter::SetShadingPropertiesRaw(aiMaterial* out_mat, const PropertyTa if (!mesh) { for (const MeshMap::value_type& v : meshes_converted) { - const MeshGeometry* const mesh = dynamic_cast(v.first); - if (!mesh) { + const MeshGeometry* const meshGeom = dynamic_cast(v.first); + if (!meshGeom) { continue; } - const MatIndexArray& mats = mesh->GetMaterialIndices(); + const MatIndexArray& mats = meshGeom->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()) { + if (meshGeom->GetTextureCoords(i).empty()) { break; } - const std::string& name = mesh->GetTextureCoordChannelName(i); + const std::string& name = meshGeom->GetTextureCoordChannelName(i); if (name == uvSet) { index = static_cast(i); break; diff --git a/code/FBXMaterial.cpp b/code/FBXMaterial.cpp index f5f6fda03..08347740f 100644 --- a/code/FBXMaterial.cpp +++ b/code/FBXMaterial.cpp @@ -326,8 +326,11 @@ Video::Video(uint64_t id, const Element& element, const Document& doc, const std content = new uint8_t[len]; ::memcpy(content, data + 5, len); } - } catch (runtime_error runtimeError) { + } catch (const runtime_error& runtimeError) + { //we don't need the content data for contents that has already been loaded + ASSIMP_LOG_DEBUG_F("Caught exception in FBXMaterial (likely because content was already loaded): ", + runtimeError.what()); } } diff --git a/code/IRRLoader.cpp b/code/IRRLoader.cpp index 6bc4b8ce4..796b73164 100644 --- a/code/IRRLoader.cpp +++ b/code/IRRLoader.cpp @@ -300,13 +300,10 @@ int ClampSpline(int idx, int size) { // ------------------------------------------------------------------------------------------------ inline void FindSuitableMultiple(int& angle) { - if (angle < 3)angle = 3; + if (angle < 3) angle = 3; else if (angle < 10) angle = 10; else if (angle < 20) angle = 20; else if (angle < 30) angle = 30; - else - { - } } // ------------------------------------------------------------------------------------------------ @@ -317,6 +314,8 @@ void IRRImporter::ComputeAnimations(Node* root, aiNode* real, std::vectoranimators.empty()) { return; @@ -674,38 +673,38 @@ void IRRImporter::GenerateGraph(Node* root,aiNode* rootOut ,aiScene* scene, // Get the loaded mesh from the scene and add it to // the list of all scenes to be attached to the // graph we're currently building - aiScene* scene = batch.GetImport(root->id); - if (!scene) { + aiScene* localScene = batch.GetImport(root->id); + if (!localScene) { ASSIMP_LOG_ERROR("IRR: Unable to load external file: " + root->meshPath); break; } - attach.push_back(AttachmentInfo(scene,rootOut)); + attach.push_back(AttachmentInfo(localScene,rootOut)); // Now combine the material we've loaded for this mesh // with the real materials we got from the file. As we // don't execute any pp-steps on the file, the numbers // should be equal. If they are not, we can impossibly // do this ... - if (root->materials.size() != (unsigned int)scene->mNumMaterials) { + if (root->materials.size() != (unsigned int)localScene->mNumMaterials) { ASSIMP_LOG_WARN("IRR: Failed to match imported materials " "with the materials found in the IRR scene file"); break; } - for (unsigned int i = 0; i < scene->mNumMaterials;++i) { + for (unsigned int i = 0; i < localScene->mNumMaterials;++i) { // Delete the old material, we don't need it anymore - delete scene->mMaterials[i]; + delete localScene->mMaterials[i]; std::pair& src = root->materials[i]; - scene->mMaterials[i] = src.first; + localScene->mMaterials[i] = src.first; } // NOTE: Each mesh should have exactly one material assigned, // but we do it in a separate loop if this behaviour changes // in future. - for (unsigned int i = 0; i < scene->mNumMeshes;++i) { + for (unsigned int i = 0; i < localScene->mNumMeshes;++i) { // Process material flags - aiMesh* mesh = scene->mMeshes[i]; + aiMesh* mesh = localScene->mMeshes[i]; // If "trans_vertex_alpha" mode is enabled, search all vertex colors diff --git a/code/Importer/STEPParser/STEPFileReader.cpp b/code/Importer/STEPParser/STEPFileReader.cpp index 1707899a1..f099d2be7 100644 --- a/code/Importer/STEPParser/STEPFileReader.cpp +++ b/code/Importer/STEPParser/STEPFileReader.cpp @@ -278,10 +278,10 @@ void STEP::ReadFile(DB& db,const EXPRESS::ConversionSchema& scheme, std::transform( type.begin(), type.end(), type.begin(), &Assimp::ToLower ); const char* sz = scheme.GetStaticStringForToken(type); if(sz) { - const std::string::size_type len = n2-n1+1; - char* const copysz = new char[len+1]; + const std::string::size_type szLen = n2-n1+1; + char* const copysz = new char[szLen+1]; std::copy(s.c_str()+n1,s.c_str()+n2+1,copysz); - copysz[len] = '\0'; + copysz[szLen] = '\0'; db.InternInsert(new LazyObject(db,id,line,sz,copysz)); } if(!has_next) { diff --git a/code/MD5Loader.cpp b/code/MD5Loader.cpp index 346b59504..38c44b515 100644 --- a/code/MD5Loader.cpp +++ b/code/MD5Loader.cpp @@ -443,10 +443,10 @@ void MD5Importer::LoadMD5MeshFile () for (MD5::VertexList::const_iterator iter = meshSrc.mVertices.begin();iter != meshSrc.mVertices.end();++iter,++pv) { for (unsigned int jub = (*iter).mFirstWeight, w = jub; w < jub + (*iter).mNumWeights;++w) { - MD5::WeightDesc& desc = meshSrc.mWeights[w]; + MD5::WeightDesc& weightDesc = meshSrc.mWeights[w]; /* FIX for some invalid exporters */ - if (!(desc.mWeight < AI_MD5_WEIGHT_EPSILON && desc.mWeight >= -AI_MD5_WEIGHT_EPSILON )) - ++piCount[desc.mBone]; + if (!(weightDesc.mWeight < AI_MD5_WEIGHT_EPSILON && weightDesc.mWeight >= -AI_MD5_WEIGHT_EPSILON )) + ++piCount[weightDesc.mBone]; } } @@ -493,20 +493,20 @@ void MD5Importer::LoadMD5MeshFile () if (w >= meshSrc.mWeights.size()) throw DeadlyImportError("MD5MESH: Invalid weight index"); - MD5::WeightDesc& desc = meshSrc.mWeights[w]; - if ( desc.mWeight < AI_MD5_WEIGHT_EPSILON && desc.mWeight >= -AI_MD5_WEIGHT_EPSILON) { + MD5::WeightDesc& weightDesc = meshSrc.mWeights[w]; + if ( weightDesc.mWeight < AI_MD5_WEIGHT_EPSILON && weightDesc.mWeight >= -AI_MD5_WEIGHT_EPSILON) { continue; } - const ai_real fNewWeight = desc.mWeight / fSum; + const ai_real fNewWeight = weightDesc.mWeight / fSum; // transform the local position into worldspace - MD5::BoneDesc& boneSrc = meshParser.mJoints[desc.mBone]; - const aiVector3D v = boneSrc.mRotationQuatConverted.Rotate (desc.vOffsetPosition); + MD5::BoneDesc& boneSrc = meshParser.mJoints[weightDesc.mBone]; + const aiVector3D v = boneSrc.mRotationQuatConverted.Rotate (weightDesc.vOffsetPosition); // use the original weight to compute the vertex position // (some MD5s seem to depend on the invalid weight values ...) - *pv += ((boneSrc.mPositionXYZ+v)* (ai_real)desc.mWeight); + *pv += ((boneSrc.mPositionXYZ+v)* (ai_real)weightDesc.mWeight); aiBone* bone = mesh->mBones[boneSrc.mMap]; *bone->mWeights++ = aiVertexWeight((unsigned int)(pv-mesh->mVertices),fNewWeight); diff --git a/code/STLExporter.cpp b/code/STLExporter.cpp index 00fced6ff..d56c42835 100644 --- a/code/STLExporter.cpp +++ b/code/STLExporter.cpp @@ -127,7 +127,7 @@ STLExporter::STLExporter(const char* _filename, const aiScene* pScene, bool expo mOutput.write((char *)&meshnum, 4); if (exportPointClouds) { - + throw DeadlyExportError("This functionality is not yet implemented for binary output."); } for(unsigned int i = 0; i < pScene->mNumMeshes; ++i) { diff --git a/code/glTFExporter.cpp b/code/glTFExporter.cpp index 9daf202cb..6bf7415d3 100644 --- a/code/glTFExporter.cpp +++ b/code/glTFExporter.cpp @@ -245,7 +245,7 @@ inline Ref ExportData(Asset& a, std::string& meshName, Ref& bu namespace { void GetMatScalar(const aiMaterial* mat, float& val, const char* propName, int type, int idx) { - if (mat->Get(propName, type, idx, val) == AI_SUCCESS) {} + ai_assert(mat->Get(propName, type, idx, val) == AI_SUCCESS); } } diff --git a/tools/assimp_cmd/ImageExtractor.cpp b/tools/assimp_cmd/ImageExtractor.cpp index f9d55b8e8..a587ab434 100644 --- a/tools/assimp_cmd/ImageExtractor.cpp +++ b/tools/assimp_cmd/ImageExtractor.cpp @@ -228,7 +228,8 @@ int DoExport(const aiTexture* tx, FILE* p, const std::string& extension, // Implementation of the assimp extract utility int Assimp_Extract (const char* const* params, unsigned int num) { - const char* const invalid = "assimp extract: Invalid number of arguments. See \'assimp extract --help\'\n"; + const char* const invalid = "assimp extract: Invalid number of arguments. See \'assimp extract --help\'\n"; + // assimp extract in out [options] if (num < 1) { printf(invalid); return 1; @@ -240,11 +241,7 @@ int Assimp_Extract (const char* const* params, unsigned int num) return 0; } - // asssimp extract in out [options] - if (num < 1) { - printf(invalid); - return 1; - } + std::string in = std::string(params[0]); std::string out = (num > 1 ? std::string(params[1]) : "-");