Skip to content

Commit

Permalink
fix review findings.
Browse files Browse the repository at this point in the history
  • Loading branch information
kimkulling committed Mar 20, 2018
1 parent 5814e6f commit af3bba1
Show file tree
Hide file tree
Showing 9 changed files with 122 additions and 127 deletions.
5 changes: 2 additions & 3 deletions code/Importer/IFC/IFCLoader.cpp
Expand Up @@ -583,9 +583,8 @@ typedef std::map<std::string, std::string> Metadata;

// ------------------------------------------------------------------------------------------------
void ProcessMetadata(const Schema_2x3::ListOf< Schema_2x3::Lazy< Schema_2x3::IfcProperty >, 1, 0 >& set, ConversionData& conv, Metadata& properties,
const std::string& prefix = "",
unsigned int nest = 0)
{
const std::string& prefix = "",
unsigned int nest = 0) {
for(const Schema_2x3::IfcProperty& property : set) {
const std::string& key = prefix.length() > 0 ? (prefix + "." + property.Name) : property.Name;
if (const Schema_2x3::IfcPropertySingleValue* const singleValue = property.ToPtr<Schema_2x3::IfcPropertySingleValue>()) {
Expand Down
27 changes: 13 additions & 14 deletions code/ObjExporter.cpp
Expand Up @@ -132,18 +132,18 @@ ObjExporter::ObjExporter(const char* _filename, const aiScene* pScene, bool noMt
mOutputMat.precision(16);

WriteGeometryFile(noMtl);
if (!noMtl)
if ( !noMtl ) {
WriteMaterialFile();
}
}

// ------------------------------------------------------------------------------------------------
ObjExporter::~ObjExporter() {

// empty
}

// ------------------------------------------------------------------------------------------------
std::string ObjExporter :: GetMaterialLibName()
{
std::string ObjExporter::GetMaterialLibName() {
// within the Obj file, we use just the relative file name with the path stripped
const std::string& s = GetMaterialLibFileName();
std::string::size_type il = s.find_last_of("/\\");
Expand All @@ -158,8 +158,9 @@ std::string ObjExporter :: GetMaterialLibName()
std::string ObjExporter::GetMaterialLibFileName() {
// Remove existing .obj file extension so that the final material file name will be fileName.mtl and not fileName.obj.mtl
size_t lastdot = filename.find_last_of('.');
if (lastdot != std::string::npos)
return filename.substr(0, lastdot) + MaterialExt;
if ( lastdot != std::string::npos ) {
return filename.substr( 0, lastdot ) + MaterialExt;
}

return filename + MaterialExt;
}
Expand All @@ -172,8 +173,7 @@ void ObjExporter::WriteHeader(std::ostringstream& out) {
}

// ------------------------------------------------------------------------------------------------
std::string ObjExporter::GetMaterialName(unsigned int index)
{
std::string ObjExporter::GetMaterialName(unsigned int index) {
const aiMaterial* const mat = pScene->mMaterials[index];
if ( nullptr == mat ) {
static const std::string EmptyStr;
Expand All @@ -191,8 +191,7 @@ std::string ObjExporter::GetMaterialName(unsigned int index)
}

// ------------------------------------------------------------------------------------------------
void ObjExporter::WriteMaterialFile()
{
void ObjExporter::WriteMaterialFile() {
WriteHeader(mOutputMat);

for(unsigned int i = 0; i < pScene->mNumMaterials; ++i) {
Expand Down Expand Up @@ -310,8 +309,9 @@ void ObjExporter::WriteGeometryFile(bool noMtl) {
if (!m.name.empty()) {
mOutput << "g " << m.name << endl;
}
if (!noMtl)
if ( !noMtl ) {
mOutput << "usemtl " << m.matname << endl;
}

for(const Face& f : m.faces) {
mOutput << f.kind << ' ';
Expand Down Expand Up @@ -382,7 +382,7 @@ void ObjExporter::colIndexMap::getColors( std::vector<aiColor4D> &colors ) {

// ------------------------------------------------------------------------------------------------
void ObjExporter::AddMesh(const aiString& name, const aiMesh* m, const aiMatrix4x4& mat) {
mMeshes.push_back(MeshInstance());
mMeshes.push_back(MeshInstance() );
MeshInstance& mesh = mMeshes.back();

mesh.name = std::string( name.data, name.length );
Expand Down Expand Up @@ -436,8 +436,7 @@ void ObjExporter::AddMesh(const aiString& name, const aiMesh* m, const aiMatrix4
}

// ------------------------------------------------------------------------------------------------
void ObjExporter::AddNode(const aiNode* nd, const aiMatrix4x4& mParent)
{
void ObjExporter::AddNode(const aiNode* nd, const aiMatrix4x4& mParent) {
const aiMatrix4x4& mAbs = mParent * nd->mTransformation;

for(unsigned int i = 0; i < nd->mNumMeshes; ++i) {
Expand Down
30 changes: 10 additions & 20 deletions code/ObjFileImporter.cpp
Expand Up @@ -207,30 +207,24 @@ void ObjFileImporter::CreateDataFromImport(const ObjFile::Model* pModel, aiScene

// Create the root node of the scene
pScene->mRootNode = new aiNode;
if ( !pModel->m_ModelName.empty() )
{
if ( !pModel->m_ModelName.empty() ) {
// Set the name of the scene
pScene->mRootNode->mName.Set(pModel->m_ModelName);
}
else
{
} else {
// This is a fatal error, so break down the application
ai_assert(false);
}

// Create nodes for the whole scene
std::vector<aiMesh*> MeshArray;
for (size_t index = 0; index < pModel->m_Objects.size(); index++)
{
for (size_t index = 0; index < pModel->m_Objects.size(); ++index ) {
createNodes(pModel, pModel->m_Objects[ index ], pScene->mRootNode, pScene, MeshArray);
}

// Create mesh pointer buffer for this scene
if (pScene->mNumMeshes > 0)
{
if (pScene->mNumMeshes > 0) {
pScene->mMeshes = new aiMesh*[ MeshArray.size() ];
for (size_t index =0; index < MeshArray.size(); index++)
{
for (size_t index =0; index < MeshArray.size(); ++index ) {
pScene->mMeshes[ index ] = MeshArray[ index ];
}
}
Expand Down Expand Up @@ -261,8 +255,7 @@ aiNode *ObjFileImporter::createNodes(const ObjFile::Model* pModel, const ObjFile
appendChildToParentNode( pParent, pNode );
}

for ( size_t i=0; i< pObject->m_Meshes.size(); i++ )
{
for ( size_t i=0; i< pObject->m_Meshes.size(); ++i ) {
unsigned int meshId = pObject->m_Meshes[ i ];
aiMesh *pMesh = createTopology( pModel, pObject, meshId );
if( pMesh ) {
Expand All @@ -275,8 +268,7 @@ aiNode *ObjFileImporter::createNodes(const ObjFile::Model* pModel, const ObjFile
}

// Create all nodes from the sub-objects stored in the current object
if ( !pObject->m_SubObjects.empty() )
{
if ( !pObject->m_SubObjects.empty() ) {
size_t numChilds = pObject->m_SubObjects.size();
pNode->mNumChildren = static_cast<unsigned int>( numChilds );
pNode->mChildren = new aiNode*[ numChilds ];
Expand All @@ -286,16 +278,14 @@ aiNode *ObjFileImporter::createNodes(const ObjFile::Model* pModel, const ObjFile

// Set mesh instances into scene- and node-instances
const size_t meshSizeDiff = MeshArray.size()- oldMeshSize;
if ( meshSizeDiff > 0 )
{
if ( meshSizeDiff > 0 ) {
pNode->mMeshes = new unsigned int[ meshSizeDiff ];
pNode->mNumMeshes = static_cast<unsigned int>( meshSizeDiff );
size_t index = 0;
for (size_t i = oldMeshSize; i < MeshArray.size(); i++)
{
for (size_t i = oldMeshSize; i < MeshArray.size(); ++i ) {
pNode->mMeshes[ index ] = pScene->mNumMeshes;
pScene->mNumMeshes++;
index++;
++index;
}
}

Expand Down
7 changes: 4 additions & 3 deletions code/ObjFileParser.cpp
Expand Up @@ -612,13 +612,14 @@ void ObjFileParser::getMaterialLib() {
if ( '/' != *path.rbegin() ) {
path += '/';
}
absName = path + strMatName;
absName += path;
absName += strMatName;
} else {
absName = strMatName;
}
IOStream *pFile = m_pIO->Open( absName );

if (!pFile ) {
IOStream *pFile = m_pIO->Open( absName );
if ( nullptr == pFile ) {
DefaultLogger::get()->error("OBJ: Unable to locate material file " + strMatName);
std::string strMatFallbackName = m_originalObjFileName.substr(0, m_originalObjFileName.length() - 3) + "mtl";
DefaultLogger::get()->info("OBJ: Opening fallback material file " + strMatFallbackName);
Expand Down
55 changes: 23 additions & 32 deletions code/OgreParsingUtils.h
Expand Up @@ -52,27 +52,23 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include <sstream>
#include <cctype>

namespace Assimp
{
namespace Ogre
{
namespace Assimp {
namespace Ogre {

/// Returns a lower cased copy of @s.
static inline std::string ToLower(std::string s)
static AI_FORCE_INLINE
std::string ToLower(std::string s)
{
std::transform(s.begin(), s.end(), s.begin(), ::tolower);
return s;
}

/// Returns if @c s ends with @c suffix. If @c caseSensitive is false, both strings will be lower cased before matching.
static inline bool EndsWith(const std::string &s, const std::string &suffix, bool caseSensitive = true)
{
if (s.empty() || suffix.empty())
{
static AI_FORCE_INLINE
bool EndsWith(const std::string &s, const std::string &suffix, bool caseSensitive = true) {
if (s.empty() || suffix.empty()) {
return false;
}
else if (s.length() < suffix.length())
{
} else if (s.length() < suffix.length()) {
return false;
}

Expand All @@ -82,57 +78,52 @@ static inline bool EndsWith(const std::string &s, const std::string &suffix, boo

size_t len = suffix.length();
std::string sSuffix = s.substr(s.length()-len, len);

return (ASSIMP_stricmp(sSuffix, suffix) == 0);
}

// Below trim functions adapted from http://stackoverflow.com/questions/216823/whats-the-best-way-to-trim-stdstring

/// Trim from start
static inline std::string &TrimLeft(std::string &s, bool newlines = true)
{
if (!newlines)
{
static AI_FORCE_INLINE
std::string &TrimLeft(std::string &s, bool newlines = true) {
if (!newlines) {
s.erase(s.begin(), std::find_if(s.begin(), s.end(), [](char c) { return !Assimp::IsSpace<char>(c); }));
}
else
{
} else {
s.erase(s.begin(), std::find_if(s.begin(), s.end(), [](char c) { return !Assimp::IsSpaceOrNewLine<char>(c); }));
}
return s;
}

/// Trim from end
static inline std::string &TrimRight(std::string &s, bool newlines = true)
{
if (!newlines)
{
static AI_FORCE_INLINE
std::string &TrimRight(std::string &s, bool newlines = true) {
if (!newlines) {
s.erase(std::find_if(s.rbegin(), s.rend(), [](char c) { return !Assimp::IsSpace<char>(c); }).base(),s.end());
}
else
{
} else {
s.erase(s.begin(), std::find_if(s.begin(), s.end(), [](char c) { return !Assimp::IsSpaceOrNewLine<char>(c); }));
}
return s;
}

/// Trim from both ends
static inline std::string &Trim(std::string &s, bool newlines = true)
{
static AI_FORCE_INLINE
std::string &Trim(std::string &s, bool newlines = true) {
return TrimLeft(TrimRight(s, newlines), newlines);
}

/// Skips a line from current @ss position until a newline. Returns the skipped part.
static inline std::string SkipLine(std::stringstream &ss)
{
static AI_FORCE_INLINE
std::string SkipLine(std::stringstream &ss) {
std::string skipped;
getline(ss, skipped);
return skipped;
}

/// Skips a line and reads next element from @c ss to @c nextElement.
/** @return Skipped line content until newline. */
static inline std::string NextAfterNewLine(std::stringstream &ss, std::string &nextElement)
{
static AI_FORCE_INLINE
std::string NextAfterNewLine(std::stringstream &ss, std::string &nextElement) {
std::string skipped = SkipLine(ss);
ss >> nextElement;
return skipped;
Expand Down
19 changes: 9 additions & 10 deletions code/OgreXmlSerializer.cpp
Expand Up @@ -213,18 +213,18 @@ std::string &OgreXmlSerializer::SkipCurrentNode()
DefaultLogger::get()->debug("Skipping node <" + m_currentNodeName + ">");
#endif

for(;;)
{
if (!m_reader->read())
{
for(;;) {
if (!m_reader->read()) {
m_currentNodeName = "";
return m_currentNodeName;
}
if (m_reader->getNodeType() != irr::io::EXN_ELEMENT_END)
if ( m_reader->getNodeType() != irr::io::EXN_ELEMENT_END ) {
continue;
else if (std::string(m_reader->getNodeName()) == m_currentNodeName)
} else if ( std::string( m_reader->getNodeName() ) == m_currentNodeName ) {
break;
}
}

return NextNode();
}

Expand Down Expand Up @@ -303,17 +303,16 @@ static const char *anZ = "z";

// Mesh

MeshXml *OgreXmlSerializer::ImportMesh(XmlReader *reader)
{
MeshXml *OgreXmlSerializer::ImportMesh(XmlReader *reader) {
OgreXmlSerializer serializer(reader);

MeshXml *mesh = new MeshXml();
serializer.ReadMesh(mesh);

return mesh;
}

void OgreXmlSerializer::ReadMesh(MeshXml *mesh)
{
void OgreXmlSerializer::ReadMesh(MeshXml *mesh) {
if (NextNode() != nnMesh) {
throw DeadlyImportError("Root node is <" + m_currentNodeName + "> expecting <mesh>");
}
Expand Down

0 comments on commit af3bba1

Please sign in to comment.