From a03cb84d0c845a7bd3ea859ec22d71a8a4cab48f Mon Sep 17 00:00:00 2001 From: Andy Maloney Date: Sat, 15 Jun 2013 18:00:40 -0400 Subject: [PATCH 1/3] {COLLADA} Add detail to some errors/warnings & prefer '<>' for elements for readability --- code/ColladaLoader.cpp | 6 ++-- code/ColladaParser.cpp | 82 +++++++++++++++++++++--------------------- 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/code/ColladaLoader.cpp b/code/ColladaLoader.cpp index 52e592b89..662f31532 100644 --- a/code/ColladaLoader.cpp +++ b/code/ColladaLoader.cpp @@ -472,7 +472,7 @@ void ColladaLoader::BuildMeshesForNode( const ColladaParser& pParser, const Coll } else { - DefaultLogger::get()->warn( boost::str( boost::format( "Collada: No material specified for subgroup \"%s\" in geometry \"%s\".") % submesh.mMaterial % mid.mMeshOrController)); + DefaultLogger::get()->warn( boost::str( boost::format( "Collada: No material specified for subgroup <%s> in geometry <%s>.") % submesh.mMaterial % mid.mMeshOrController)); if( !mid.mMaterials.empty() ) meshMaterial = mid.mMaterials.begin()->second.mMatName; } @@ -642,7 +642,7 @@ aiMesh* ColladaLoader::CreateMesh( const ColladaParser& pParser, const Collada:: throw DeadlyImportError( "Data type mismatch while resolving mesh joints"); // sanity check: we rely on the vertex weights always coming as pairs of BoneIndex-WeightIndex if( pSrcController->mWeightInputJoints.mOffset != 0 || pSrcController->mWeightInputWeights.mOffset != 1) - throw DeadlyImportError( "Unsupported vertex_weight adressing scheme. "); + throw DeadlyImportError( "Unsupported vertex_weight addressing scheme. "); // create containers to collect the weights for each bone size_t numBones = jointNames.mStrings.size(); @@ -973,7 +973,7 @@ void ColladaLoader::CreateAnimation( aiScene* pScene, const ColladaParser& pPars else if( subElement == "Z") entry.mSubElement = 2; else - DefaultLogger::get()->warn( boost::str( boost::format( "Unknown anim subelement \"%s\". Ignoring") % subElement)); + DefaultLogger::get()->warn( boost::str( boost::format( "Unknown anim subelement <%s>. Ignoring") % subElement)); } else { // no subelement following, transformId is remaining string diff --git a/code/ColladaParser.cpp b/code/ColladaParser.cpp index 843dd618e..442896a39 100644 --- a/code/ColladaParser.cpp +++ b/code/ColladaParser.cpp @@ -140,7 +140,7 @@ void ColladaParser::ReadContents() ReadStructure(); } else { - DefaultLogger::get()->debug( boost::str( boost::format( "Ignoring global element \"%s\".") % mReader->getNodeName())); + DefaultLogger::get()->debug( boost::str( boost::format( "Ignoring global element <%s>.") % mReader->getNodeName())); SkipElement(); } } else @@ -240,7 +240,7 @@ void ColladaParser::ReadAssetInfo() else if( mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { if( strcmp( mReader->getNodeName(), "asset") != 0) - ThrowException( "Expected end of \"asset\" element."); + ThrowException( "Expected end of element."); break; } @@ -271,7 +271,7 @@ void ColladaParser::ReadAnimationLibrary() else if( mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { if( strcmp( mReader->getNodeName(), "library_animations") != 0) - ThrowException( "Expected end of \"library_animations\" element."); + ThrowException( "Expected end of element."); break; } @@ -362,7 +362,7 @@ void ColladaParser::ReadAnimation( Collada::Animation* pParent) else if( mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { if( strcmp( mReader->getNodeName(), "animation") != 0) - ThrowException( "Expected end of \"animation\" element."); + ThrowException( "Expected end of element."); break; } @@ -425,7 +425,7 @@ void ColladaParser::ReadAnimationSampler( Collada::AnimationChannel& pChannel) else if( mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { if( strcmp( mReader->getNodeName(), "sampler") != 0) - ThrowException( "Expected end of \"sampler\" element."); + ThrowException( "Expected end of element."); break; } @@ -463,7 +463,7 @@ void ColladaParser::ReadControllerLibrary() else if( mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { if( strcmp( mReader->getNodeName(), "library_controllers") != 0) - ThrowException( "Expected end of \"library_controllers\" element."); + ThrowException( "Expected end of element."); break; } @@ -531,7 +531,7 @@ void ColladaParser::ReadController( Collada::Controller& pController) if( strcmp( mReader->getNodeName(), "controller") == 0) break; else if( strcmp( mReader->getNodeName(), "skin") != 0) - ThrowException( "Expected end of \"controller\" element."); + ThrowException( "Expected end of element."); } } } @@ -554,7 +554,7 @@ void ColladaParser::ReadControllerJoints( Collada::Controller& pController) // local URLS always start with a '#'. We don't support global URLs if( attrSource[0] != '#') - ThrowException( boost::str( boost::format( "Unsupported URL format in \"%s\"") % attrSource)); + ThrowException( boost::str( boost::format( "Unsupported URL format in \"%s\" in source attribute of data element") % attrSource)); attrSource++; // parse source URL to corresponding source @@ -563,7 +563,7 @@ void ColladaParser::ReadControllerJoints( Collada::Controller& pController) else if( strcmp( attrSemantic, "INV_BIND_MATRIX") == 0) pController.mJointOffsetMatrixSource = attrSource; else - ThrowException( boost::str( boost::format( "Unknown semantic \"%s\" in joint data") % attrSemantic)); + ThrowException( boost::str( boost::format( "Unknown semantic \"%s\" in data element") % attrSemantic)); // skip inner data, if present if( !mReader->isEmptyElement()) @@ -578,7 +578,7 @@ void ColladaParser::ReadControllerJoints( Collada::Controller& pController) else if( mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { if( strcmp( mReader->getNodeName(), "joints") != 0) - ThrowException( "Expected end of \"joints\" element."); + ThrowException( "Expected end of element."); break; } @@ -613,7 +613,7 @@ void ColladaParser::ReadControllerWeights( Collada::Controller& pController) // local URLS always start with a '#'. We don't support global URLs if( attrSource[0] != '#') - ThrowException( boost::str( boost::format( "Unsupported URL format in \"%s\"") % attrSource)); + ThrowException( boost::str( boost::format( "Unsupported URL format in \"%s\" in source attribute of data element") % attrSource)); channel.mAccessor = attrSource + 1; // parse source URL to corresponding source @@ -622,7 +622,7 @@ void ColladaParser::ReadControllerWeights( Collada::Controller& pController) else if( strcmp( attrSemantic, "WEIGHT") == 0) pController.mWeightInputWeights = channel; else - ThrowException( boost::str( boost::format( "Unknown semantic \"%s\" in vertex_weight data") % attrSemantic)); + ThrowException( boost::str( boost::format( "Unknown semantic \"%s\" in data element") % attrSemantic)); // skip inner data, if present if( !mReader->isEmptyElement()) @@ -636,7 +636,7 @@ void ColladaParser::ReadControllerWeights( Collada::Controller& pController) for( std::vector::iterator it = pController.mWeightCounts.begin(); it != pController.mWeightCounts.end(); ++it) { if( *text == 0) - ThrowException( "Out of data while reading vcount"); + ThrowException( "Out of data while reading "); *it = strtoul10( text, &text); numWeights += *it; @@ -656,11 +656,11 @@ void ColladaParser::ReadControllerWeights( Collada::Controller& pController) for( std::vector< std::pair >::iterator it = pController.mWeights.begin(); it != pController.mWeights.end(); ++it) { if( *text == 0) - ThrowException( "Out of data while reading vertex_weights"); + ThrowException( "Out of data while reading "); it->first = strtoul10( text, &text); SkipSpacesAndLineEnd( &text); if( *text == 0) - ThrowException( "Out of data while reading vertex_weights"); + ThrowException( "Out of data while reading "); it->second = strtoul10( text, &text); SkipSpacesAndLineEnd( &text); } @@ -676,7 +676,7 @@ void ColladaParser::ReadControllerWeights( Collada::Controller& pController) else if( mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { if( strcmp( mReader->getNodeName(), "vertex_weights") != 0) - ThrowException( "Expected end of \"vertex_weights\" element."); + ThrowException( "Expected end of element."); break; } @@ -712,7 +712,7 @@ void ColladaParser::ReadImageLibrary() } else if( mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { if( strcmp( mReader->getNodeName(), "library_images") != 0) - ThrowException( "Expected end of \"library_images\" element."); + ThrowException( "Expected end of element."); break; } @@ -838,7 +838,7 @@ void ColladaParser::ReadMaterialLibrary() else if( mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { if( strcmp( mReader->getNodeName(), "library_materials") != 0) - ThrowException( "Expected end of \"library_materials\" element."); + ThrowException( "Expected end of element."); break; } @@ -872,7 +872,7 @@ void ColladaParser::ReadLightLibrary() } else if( mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { if( strcmp( mReader->getNodeName(), "library_lights") != 0) - ThrowException( "Expected end of \"library_lights\" element."); + ThrowException( "Expected end of element."); break; } @@ -911,7 +911,7 @@ void ColladaParser::ReadCameraLibrary() } else if( mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { if( strcmp( mReader->getNodeName(), "library_cameras") != 0) - ThrowException( "Expected end of \"library_cameras\" element."); + ThrowException( "Expected end of element."); break; } @@ -947,7 +947,7 @@ void ColladaParser::ReadMaterial( Collada::Material& pMaterial) } else if( mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { if( strcmp( mReader->getNodeName(), "material") != 0) - ThrowException( "Expected end of \"material\" element."); + ThrowException( "Expected end of element."); break; } @@ -1112,7 +1112,7 @@ void ColladaParser::ReadEffectLibrary() } else if( mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { if( strcmp( mReader->getNodeName(), "library_effects") != 0) - ThrowException( "Expected end of \"library_effects\" element."); + ThrowException( "Expected end of element."); break; } @@ -1136,7 +1136,7 @@ void ColladaParser::ReadEffect( Collada::Effect& pEffect) else if( mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { if( strcmp( mReader->getNodeName(), "effect") != 0) - ThrowException( "Expected end of \"effect\" element."); + ThrowException( "Expected end of element."); break; } @@ -1504,7 +1504,7 @@ void ColladaParser::ReadGeometryLibrary() else if( mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { if( strcmp( mReader->getNodeName(), "library_geometries") != 0) - ThrowException( "Expected end of \"library_geometries\" element."); + ThrowException( "Expected end of element."); break; } @@ -1535,7 +1535,7 @@ void ColladaParser::ReadGeometry( Collada::Mesh* pMesh) else if( mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { if( strcmp( mReader->getNodeName(), "geometry") != 0) - ThrowException( "Expected end of \"geometry\" element."); + ThrowException( "Expected end of element."); break; } @@ -1587,7 +1587,7 @@ void ColladaParser::ReadMesh( Mesh* pMesh) } else { // everything else should be punished - ThrowException( "Expected end of \"mesh\" element."); + ThrowException( "Expected end of element."); } } } @@ -1634,7 +1634,7 @@ void ColladaParser::ReadSource() } else { // everything else should be punished - ThrowException( "Expected end of \"source\" element."); + ThrowException( "Expected end of element."); } } } @@ -1713,7 +1713,7 @@ void ColladaParser::ReadAccessor( const std::string& pID) int attrSource = GetAttribute( "source"); const char* source = mReader->getAttributeValue( attrSource); if( source[0] != '#') - ThrowException( boost::str( boost::format( "Unknown reference format in url \"%s\".") % source)); + ThrowException( boost::str( boost::format( "Unknown reference format in url \"%s\" in source attribute of element.") % source)); int attrCount = GetAttribute( "count"); unsigned int count = (unsigned int) mReader->getAttributeValueAsInt( attrCount); int attrOffset = TestAttribute( "offset"); @@ -1898,7 +1898,7 @@ void ColladaParser::ReadIndexData( Mesh* pMesh) for( unsigned int a = 0; a < numPrimitives; a++) { if( *content == 0) - ThrowException( "Expected more values while reading vcount contents."); + ThrowException( "Expected more values while reading contents."); // read a number vcount.push_back( (size_t) strtoul10( content, &content)); // skip whitespace after it @@ -1946,7 +1946,7 @@ void ColladaParser::ReadInputChannel( std::vector& poChannels) int attrSource = GetAttribute( "source"); const char* source = mReader->getAttributeValue( attrSource); if( source[0] != '#') - ThrowException( boost::str( boost::format( "Unknown reference format in url \"%s\".") % source)); + ThrowException( boost::str( boost::format( "Unknown reference format in url \"%s\" in source attribute of element.") % source)); channel.mAccessor = source+1; // skipping the leading #, hopefully the remaining text is the accessor ID only // read index offset, if per-index @@ -1960,7 +1960,7 @@ void ColladaParser::ReadInputChannel( std::vector& poChannels) if(attrSet > -1){ attrSet = mReader->getAttributeValueAsInt( attrSet); if(attrSet < 0) - ThrowException( boost::str( boost::format( "Invalid index \"%i\" for set attribute") % (attrSet))); + ThrowException( boost::str( boost::format( "Invalid index \"%i\" in set attribute of element") % (attrSet))); channel.mIndex = attrSet; } @@ -2578,18 +2578,18 @@ void ColladaParser::ReadScene() { // should be the first and only occurence if( mRootNode) - ThrowException( "Invalid scene containing multiple root nodes"); + ThrowException( "Invalid scene containing multiple root nodes in element"); // read the url of the scene to instance. Should be of format "#some_name" int urlIndex = GetAttribute( "url"); const char* url = mReader->getAttributeValue( urlIndex); if( url[0] != '#') - ThrowException( "Unknown reference format"); + ThrowException( "Unknown reference format in element"); // find the referred scene, skip the leading # NodeLibrary::const_iterator sit = mNodeLibrary.find( url+1); if( sit == mNodeLibrary.end()) - ThrowException( "Unable to resolve visual_scene reference \"" + std::string(url) + "\"."); + ThrowException( "Unable to resolve visual_scene reference \"" + std::string(url) + "\" in element."); mRootNode = sit->second; } else { SkipElement(); @@ -2641,14 +2641,14 @@ void ColladaParser::TestOpening( const char* pName) { // read element start if( !mReader->read()) - ThrowException( boost::str( boost::format( "Unexpected end of file while beginning of \"%s\" element.") % pName)); + ThrowException( boost::str( boost::format( "Unexpected end of file while beginning of <%s> element.") % pName)); // whitespace in front is ok, just read again if found if( mReader->getNodeType() == irr::io::EXN_TEXT) if( !mReader->read()) - ThrowException( boost::str( boost::format( "Unexpected end of file while reading beginning of \"%s\" element.") % pName)); + ThrowException( boost::str( boost::format( "Unexpected end of file while reading beginning of <%s> element.") % pName)); if( mReader->getNodeType() != irr::io::EXN_ELEMENT || strcmp( mReader->getNodeName(), pName) != 0) - ThrowException( boost::str( boost::format( "Expected start of \"%s\" element.") % pName)); + ThrowException( boost::str( boost::format( "Expected start of <%s> element.") % pName)); } // ------------------------------------------------------------------------------------------------ @@ -2661,15 +2661,15 @@ void ColladaParser::TestClosing( const char* pName) // if not, read some more if( !mReader->read()) - ThrowException( boost::str( boost::format( "Unexpected end of file while reading end of \"%s\" element.") % pName)); + ThrowException( boost::str( boost::format( "Unexpected end of file while reading end of <%s> element.") % pName)); // whitespace in front is ok, just read again if found if( mReader->getNodeType() == irr::io::EXN_TEXT) if( !mReader->read()) - ThrowException( boost::str( boost::format( "Unexpected end of file while reading end of \"%s\" element.") % pName)); + ThrowException( boost::str( boost::format( "Unexpected end of file while reading end of <%s> element.") % pName)); // but this has the be the closing tag, or we're lost if( mReader->getNodeType() != irr::io::EXN_ELEMENT_END || strcmp( mReader->getNodeName(), pName) != 0) - ThrowException( boost::str( boost::format( "Expected end of \"%s\" element.") % pName)); + ThrowException( boost::str( boost::format( "Expected end of <%s> element.") % pName)); } // ------------------------------------------------------------------------------------------------ @@ -2681,7 +2681,7 @@ int ColladaParser::GetAttribute( const char* pAttr) const return index; // attribute not found -> throw an exception - ThrowException( boost::str( boost::format( "Expected attribute \"%s\" at element \"%s\".") % pAttr % mReader->getNodeName())); + ThrowException( boost::str( boost::format( "Expected attribute \"%s\" for element <%s>.") % pAttr % mReader->getNodeName())); return -1; } From d7c6cd9ecea2b563d4bd5dfd9f80376c17d92aaa Mon Sep 17 00:00:00 2001 From: Andy Maloney Date: Thu, 20 Jun 2013 08:40:08 -0400 Subject: [PATCH 2/3] {OBJ} Allow spaces in group names --- code/ObjFileParser.cpp | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/code/ObjFileParser.cpp b/code/ObjFileParser.cpp index 5dcdf4b9a..83f25c40a 100644 --- a/code/ObjFileParser.cpp +++ b/code/ObjFileParser.cpp @@ -528,18 +528,12 @@ int ObjFileParser::getMaterialIndex( const std::string &strMaterialName ) // Getter for a group name. void ObjFileParser::getGroupName() { - // Get next word from data buffer - m_DataIt = getNextToken(m_DataIt, m_DataItEnd); - m_DataIt = getNextWord(m_DataIt, m_DataItEnd); + std::string strGroupName; + + m_DataIt = getName(m_DataIt, m_DataItEnd, strGroupName); if ( isEndOfBuffer( m_DataIt, m_DataItEnd ) ) return; - // Store the group name in the group library - char *pStart = &(*m_DataIt); - while ( m_DataIt != m_DataItEnd && !isSeparator(*m_DataIt) ) - m_DataIt++; - std::string strGroupName( pStart, &(*m_DataIt) ); - // Change active group, if necessary if ( m_pModel->m_strActiveGroup != strGroupName ) { From 1da281c1f8bb0f7c274bf1c599dc4cba74a7309d Mon Sep 17 00:00:00 2001 From: Andy Maloney Date: Fri, 21 Jun 2013 10:56:11 -0400 Subject: [PATCH 3/3] Make sure members are initialized properly Prefer initialization lists Assignment operator should not return a const ref --- code/ObjFileData.h | 1 + include/assimp/mesh.h | 54 +++++++++++++++++++++---------------------- 2 files changed, 28 insertions(+), 27 deletions(-) mode change 100644 => 100755 code/ObjFileData.h diff --git a/code/ObjFileData.h b/code/ObjFileData.h old mode 100644 new mode 100755 index 905f69346..4331a7d10 --- a/code/ObjFileData.h +++ b/code/ObjFileData.h @@ -283,6 +283,7 @@ struct Model m_pCurrent(NULL), m_pCurrentMaterial(NULL), m_pDefaultMaterial(NULL), + m_pGroupFaceIDs(NULL), m_strActiveGroup(""), m_pCurrentMesh(NULL) { diff --git a/include/assimp/mesh.h b/include/assimp/mesh.h index 449451632..01d5baf7c 100644 --- a/include/assimp/mesh.h +++ b/include/assimp/mesh.h @@ -135,8 +135,9 @@ struct aiFace //! Default constructor aiFace() + : mNumIndices( 0 ) + , mIndices( NULL ) { - mNumIndices = 0; mIndices = NULL; } //! Default destructor. Delete the index array @@ -147,13 +148,13 @@ struct aiFace //! Copy constructor. Copy the index array aiFace( const aiFace& o) + : mIndices( NULL ) { - mIndices = NULL; *this = o; } //! Assignment operator. Copy the index array - const aiFace& operator = ( const aiFace& o) + aiFace& operator = ( const aiFace& o) { if (&o == this) return *this; @@ -248,17 +249,17 @@ struct aiBone //! Default constructor aiBone() + : mNumWeights( 0 ) + , mWeights( NULL ) { - mNumWeights = 0; mWeights = NULL; } //! Copy constructor aiBone(const aiBone& other) + : mName( other.mName ) + , mNumWeights( other.mNumWeights ) + , mOffsetMatrix( other.mOffsetMatrix ) { - mNumWeights = other.mNumWeights; - mOffsetMatrix = other.mOffsetMatrix; - mName = other.mName; - if (other.mWeights && other.mNumWeights) { mWeights = new aiVertexWeight[mNumWeights]; @@ -378,10 +379,10 @@ struct aiAnimMesh #ifdef __cplusplus aiAnimMesh() - : mVertices() - , mNormals() - , mTangents() - , mBitangents() + : mVertices( NULL ) + , mNormals( NULL ) + , mTangents( NULL ) + , mBitangents( NULL ) , mNumVertices( 0 ) { // fixme consider moving this to the ctor initializer list as well @@ -610,29 +611,28 @@ struct aiMesh //! Default constructor. Initializes all members to 0 aiMesh() + : mPrimitiveTypes( 0 ) + , mNumVertices( 0 ) + , mNumFaces( 0 ) + , mVertices( NULL ) + , mNormals( NULL ) + , mTangents( NULL ) + , mBitangents( NULL ) + , mFaces( NULL ) + , mNumBones( 0 ) + , mBones( 0 ) + , mMaterialIndex( 0 ) + , mNumAnimMeshes( 0 ) + , mAnimMeshes( NULL ) { - mNumVertices = 0; - mNumFaces = 0; - - mNumAnimMeshes = 0; - - mPrimitiveTypes = 0; - mVertices = NULL; mFaces = NULL; - mNormals = NULL; mTangents = NULL; - mBitangents = NULL; - mAnimMeshes = NULL; - for( unsigned int a = 0; a < AI_MAX_NUMBER_OF_TEXTURECOORDS; a++) { mNumUVComponents[a] = 0; mTextureCoords[a] = NULL; } + for( unsigned int a = 0; a < AI_MAX_NUMBER_OF_COLOR_SETS; a++) mColors[a] = NULL; - mNumBones = 0; mBones = NULL; - mMaterialIndex = 0; - mNumAnimMeshes = 0; - mAnimMeshes = NULL; } //! Deletes all storage allocated for the mesh