Obj: fix code review findings.

code/BlenderLoader.cpp

@@ -96,8 +96,9 @@ static const aiImporterDesc blenderDesc = {
 // ------------------------------------------------------------------------------------------------
 // Constructor to be privately used by Importer
 BlenderImporter::BlenderImporter()
-: modifier_cache(new BlenderModifierShowcase())
+: modifier_cache(new BlenderModifierShowcase()) {
-{}
+    // empty
+}
 
 // ------------------------------------------------------------------------------------------------
 // Destructor, private as well
@@ -145,8 +146,7 @@ void BlenderImporter::SetupProperties(const Importer* /*pImp*/)
     // nothing to be done for the moment
 }
 
-struct free_it
+struct free_it {
-{
     free_it(void* free) : free(free) {}
     ~free_it() {
         ::free(this->free);

code/BlenderScene.cpp

@@ -522,7 +522,7 @@ template <> void Structure :: Convert<MVert> (
     ReadFieldArray<ErrorPolicy_Fail>(dest.co,"co",db);
     ReadFieldArray<ErrorPolicy_Fail>(dest.no,"no",db);
     ReadField<ErrorPolicy_Igno>(dest.flag,"flag",db);
-    ReadField<ErrorPolicy_Warn>(dest.mat_nr,"mat_nr",db);
+    //ReadField<ErrorPolicy_Warn>(dest.mat_nr,"mat_nr",db);
     ReadField<ErrorPolicy_Igno>(dest.bweight,"bweight",db);
 
     db.reader->IncPtr(size);

code/Exporter.cpp

@@ -201,65 +201,48 @@ public:
     std::vector<Exporter::ExportFormatEntry> mExporters;
 };
 
-
 } // end of namespace Assimp
 
-
-
-
-
 using namespace Assimp;
 
-
 // ------------------------------------------------------------------------------------------------
 Exporter :: Exporter()
-: pimpl(new ExporterPimpl())
+: pimpl(new ExporterPimpl()) {
-{
+    // empty
 }
 
-
 // ------------------------------------------------------------------------------------------------
-Exporter :: ~Exporter()
+Exporter::~Exporter() {
-{
     FreeBlob();
 
     delete pimpl;
 }
 
-
 // ------------------------------------------------------------------------------------------------
-void Exporter :: SetIOHandler( IOSystem* pIOHandler)
+void Exporter::SetIOHandler( IOSystem* pIOHandler) {
-{
     pimpl->mIsDefaultIOHandler = !pIOHandler;
     pimpl->mIOSystem.reset(pIOHandler);
 }
 
-
 // ------------------------------------------------------------------------------------------------
-IOSystem* Exporter :: GetIOHandler() const
+IOSystem* Exporter::GetIOHandler() const {
-{
     return pimpl->mIOSystem.get();
 }
 
-
 // ------------------------------------------------------------------------------------------------
-bool Exporter :: IsDefaultIOHandler() const
+bool Exporter::IsDefaultIOHandler() const {
-{
     return pimpl->mIsDefaultIOHandler;
 }
 
-
 // ------------------------------------------------------------------------------------------------
-const aiExportDataBlob* Exporter :: ExportToBlob(  const aiScene* pScene, const char* pFormatId, unsigned int, const ExportProperties* pProperties)
+const aiExportDataBlob* Exporter::ExportToBlob( const aiScene* pScene, const char* pFormatId, 
-{
+                                                unsigned int, const ExportProperties* pProperties ) {
     if (pimpl->blob) {
         delete pimpl->blob;
         pimpl->blob = NULL;
     }
 
-
     std::shared_ptr<IOSystem> old = pimpl->mIOSystem;
-
     BlobIOSystem* blobio = new BlobIOSystem();
     pimpl->mIOSystem = std::shared_ptr<IOSystem>( blobio );
 
@@ -274,10 +257,8 @@ const aiExportDataBlob* Exporter :: ExportToBlob(  const aiScene* pScene, const
     return pimpl->blob;
 }
 
-
 // ------------------------------------------------------------------------------------------------
-bool IsVerboseFormat(const aiMesh* mesh)
+bool IsVerboseFormat(const aiMesh* mesh) {
-{
     // avoid slow vector<bool> specialization
     std::vector<unsigned int> seen(mesh->mNumVertices,0);
     for(unsigned int i = 0; i < mesh->mNumFaces; ++i) {
@@ -292,10 +273,8 @@ bool IsVerboseFormat(const aiMesh* mesh)
     return true;
 }
 
-
 // ------------------------------------------------------------------------------------------------
-bool IsVerboseFormat(const aiScene* pScene)
+bool IsVerboseFormat(const aiScene* pScene) {
-{
     for(unsigned int i = 0; i < pScene->mNumMeshes; ++i) {
         if(!IsVerboseFormat(pScene->mMeshes[i])) {
             return false;
@@ -304,10 +283,8 @@ bool IsVerboseFormat(const aiScene* pScene)
     return true;
 }
 
-
 // ------------------------------------------------------------------------------------------------
-aiReturn Exporter :: Export( const aiScene* pScene, const char* pFormatId, const char* pPath, unsigned int pPreprocessing, const ExportProperties* pProperties)
+aiReturn Exporter::Export( const aiScene* pScene, const char* pFormatId, const char* pPath, unsigned int pPreprocessing, const ExportProperties* pProperties) {
-{
     ASSIMP_BEGIN_EXCEPTION_REGION();
 
     // when they create scenes from scratch, users will likely create them not in verbose
@@ -320,9 +297,7 @@ aiReturn Exporter :: Export( const aiScene* pScene, const char* pFormatId, const
     for (size_t i = 0; i < pimpl->mExporters.size(); ++i) {
         const Exporter::ExportFormatEntry& exp = pimpl->mExporters[i];
         if (!strcmp(exp.mDescription.id,pFormatId)) {
-
             try {
-
                 // Always create a full copy of the scene. We might optimize this one day,
                 // but for now it is the most pragmatic way.
                 aiScene* scenecopy_tmp = NULL;
@@ -341,18 +316,17 @@ aiReturn Exporter :: Export( const aiScene* pScene, const char* pFormatId, const
                     ? (priv->mPPStepsApplied & ~nonIdempotentSteps)
                     : 0u);
 
-                // If no extra postprocessing was specified, and we obtained this scene from an
+                // If no extra post-processing was specified, and we obtained this scene from an
                 // Assimp importer, apply the reverse steps automatically.
                 // TODO: either drop this, or document it. Otherwise it is just a bad surprise.
                 //if (!pPreprocessing && priv) {
                 //  pp |= (nonIdempotentSteps & priv->mPPStepsApplied);
                 //}
 
-                // If the input scene is not in verbose format, but there is at least postprocessing step that relies on it,
+                // If the input scene is not in verbose format, but there is at least post-processing step that relies on it,
                 // we need to run the MakeVerboseFormat step first.
                 bool must_join_again = false;
                 if (!is_verbose_format) {
-
                     bool verbosify = false;
                     for( unsigned int a = 0; a < pimpl->mPostProcessingSteps.size(); a++) {
                         BaseProcess* const p = pimpl->mPostProcessingSteps[a];
@@ -423,8 +397,7 @@ aiReturn Exporter :: Export( const aiScene* pScene, const char* pFormatId, const
 
                 ExportProperties emptyProperties;  // Never pass NULL ExportProperties so Exporters don't have to worry.
                 exp.mExportFunction(pPath,pimpl->mIOSystem.get(),scenecopy.get(), pProperties ? pProperties : &emptyProperties);
-            }
+            } catch (DeadlyExportError& err) {
-            catch (DeadlyExportError& err) {
                 pimpl->mError = err.what();
                 return AI_FAILURE;
             }
@@ -437,64 +410,53 @@ aiReturn Exporter :: Export( const aiScene* pScene, const char* pFormatId, const
     return AI_FAILURE;
 }
 
-
 // ------------------------------------------------------------------------------------------------
-const char* Exporter :: GetErrorString() const
+const char* Exporter::GetErrorString() const {
-{
     return pimpl->mError.c_str();
 }
 
 
 // ------------------------------------------------------------------------------------------------
-void Exporter :: FreeBlob( )
+void Exporter::FreeBlob() {
-{
     delete pimpl->blob;
     pimpl->blob = NULL;
 
     pimpl->mError = "";
 }
 
-
 // ------------------------------------------------------------------------------------------------
-const aiExportDataBlob* Exporter :: GetBlob() const
+const aiExportDataBlob* Exporter::GetBlob() const {
-{
     return pimpl->blob;
 }
 
-
 // ------------------------------------------------------------------------------------------------
-const aiExportDataBlob* Exporter :: GetOrphanedBlob() const
+const aiExportDataBlob* Exporter::GetOrphanedBlob() const {
-{
     const aiExportDataBlob* tmp = pimpl->blob;
     pimpl->blob = NULL;
     return tmp;
 }
 
-
 // ------------------------------------------------------------------------------------------------
-size_t Exporter :: GetExportFormatCount() const
+size_t Exporter::GetExportFormatCount() const {
-{
     return pimpl->mExporters.size();
 }
 
 // ------------------------------------------------------------------------------------------------
-const aiExportFormatDesc* Exporter :: GetExportFormatDescription( size_t pIndex ) const
+const aiExportFormatDesc* Exporter::GetExportFormatDescription( size_t index ) const {
-{
+    if (index >= GetExportFormatCount()) {
-    if (pIndex >= GetExportFormatCount()) {
         return NULL;
     }
 
     // Return from static storage if the requested index is built-in.
-    if (pIndex < sizeof(gExporters) / sizeof(gExporters[0])) {
+    if (index < sizeof(gExporters) / sizeof(gExporters[0])) {
-        return &gExporters[pIndex].mDescription;
+        return &gExporters[index].mDescription;
     }
 
-    return &pimpl->mExporters[pIndex].mDescription;
+    return &pimpl->mExporters[index].mDescription;
 }
 
 // ------------------------------------------------------------------------------------------------
-aiReturn Exporter :: RegisterExporter(const ExportFormatEntry& desc)
+aiReturn Exporter::RegisterExporter(const ExportFormatEntry& desc) {
-{
     for(const ExportFormatEntry& e : pimpl->mExporters) {
         if (!strcmp(e.mDescription.id,desc.mDescription.id)) {
             return aiReturn_FAILURE;
@@ -505,10 +467,8 @@ aiReturn Exporter :: RegisterExporter(const ExportFormatEntry& desc)
     return aiReturn_SUCCESS;
 }
 
-
 // ------------------------------------------------------------------------------------------------
-void Exporter :: UnregisterExporter(const char* id)
+void Exporter::UnregisterExporter(const char* id) {
-{
     for(std::vector<ExportFormatEntry>::iterator it = pimpl->mExporters.begin(); it != pimpl->mExporters.end(); ++it) {
         if (!strcmp((*it).mDescription.id,id)) {
             pimpl->mExporters.erase(it);
@@ -517,29 +477,30 @@ void Exporter :: UnregisterExporter(const char* id)
     }
 }
 
-ExportProperties :: ExportProperties() {}
+// ------------------------------------------------------------------------------------------------
+ExportProperties::ExportProperties() {
+    // empty
+}
 
+// ------------------------------------------------------------------------------------------------
 ExportProperties::ExportProperties(const ExportProperties &other)
-     : mIntProperties(other.mIntProperties),
+: mIntProperties(other.mIntProperties)
-   mFloatProperties(other.mFloatProperties),
+, mFloatProperties(other.mFloatProperties)
-   mStringProperties(other.mStringProperties),
+, mStringProperties(other.mStringProperties)
-   mMatrixProperties(other.mMatrixProperties)
+, mMatrixProperties(other.mMatrixProperties) {
-{
+    // empty
-
 }
 
 
 // ------------------------------------------------------------------------------------------------
 // Set a configuration property
-bool ExportProperties :: SetPropertyInteger(const char* szName, int iValue)
+bool ExportProperties::SetPropertyInteger(const char* szName, int iValue) {
-{
     return SetGenericProperty<int>(mIntProperties, szName,iValue);
 }
 
 // ------------------------------------------------------------------------------------------------
 // Set a configuration property
-bool ExportProperties :: SetPropertyFloat(const char* szName, ai_real iValue)
+bool ExportProperties::SetPropertyFloat(const char* szName, ai_real iValue) {
-{
     return SetGenericProperty<ai_real>(mFloatProperties, szName,iValue);
 }
 

code/ObjExporter.cpp

@@ -57,8 +57,7 @@ namespace Assimp {
 
 // ------------------------------------------------------------------------------------------------
 // Worker function for exporting a scene to Wavefront OBJ. Prototyped and registered in Exporter.cpp
-void ExportSceneObj(const char* pFile,IOSystem* pIOSystem, const aiScene* pScene, const ExportProperties* pProperties)
+void ExportSceneObj(const char* pFile,IOSystem* pIOSystem, const aiScene* pScene, const ExportProperties* pProperties) {
-{
     // invoke the exporter
     ObjExporter exporter(pFile, pScene);
 
@@ -84,7 +83,7 @@ void ExportSceneObj(const char* pFile,IOSystem* pIOSystem, const aiScene* pScene
 static const std::string MaterialExt = ".mtl";
 
 // ------------------------------------------------------------------------------------------------
-ObjExporter :: ObjExporter(const char* _filename, const aiScene* pScene)
+ObjExporter::ObjExporter(const char* _filename, const aiScene* pScene)
 : filename(_filename)
 , pScene(pScene)
 , endl("\n")
@@ -103,6 +102,11 @@ ObjExporter :: ObjExporter(const char* _filename, const aiScene* pScene)
     WriteMaterialFile();
 }
 
+// ------------------------------------------------------------------------------------------------
+ObjExporter::~ObjExporter() {
+
+}
+
 // ------------------------------------------------------------------------------------------------
 std::string ObjExporter :: GetMaterialLibName()
 {

code/ObjExporter.h

@@ -63,12 +63,10 @@ class ObjExporter {
 public:
     /// Constructor for a specific scene to export
     ObjExporter(const char* filename, const aiScene* pScene);
-
+    ~ObjExporter();
-public:
     std::string GetMaterialLibName();
     std::string GetMaterialLibFileName();
     
-public:
     /// public string-streams to write all output into
     std::ostringstream mOutput, mOutputMat;
 
@@ -94,13 +92,11 @@ private:
     };
 
     struct MeshInstance {
-
         std::string name, matname;
         std::vector<Face> faces;
     };
 
     void WriteHeader(std::ostringstream& out);
-
     void WriteMaterialFile();
     void WriteGeometryFile();
 

include/assimp/Exporter.hpp

@@ -296,8 +296,7 @@ protected:
     ExporterPimpl* pimpl;
 };
 
-class ASSIMP_API ExportProperties
+class ASSIMP_API ExportProperties {
-{
 public:
     // Data type to store the key hash
     typedef unsigned int KeyType;

test/models/OBJ/cube_with_vertexcolors.obj

@@ -0,0 +1,30 @@
+g cube
+
+v  0.0  0.0  0.0  124  110  120
+v  0.0  0.0  1.0  24  0  121
+v  0.0  1.0  0.0  4  0  44
+v  0.0  1.0  1.0  224  0  10
+v  1.0  0.0  0.0  24  200  25
+v  1.0  0.0  1.0  124  10  56
+v  1.0  1.0  0.0  78  10  50
+v  1.0  1.0  1.0  23  0  200
+
+vn 0.0 0.0 1.0
+vn 0.0 0.0 -1.0
+vn 0.0 1.0 0.0
+vn 0.0 -1.0 0.0
+vn 1.0 0.0 0.0
+vn -1.0 0.0 0.0
+
+f 1//2 7//2 5//2
+f 1//2 3//2 7//2
+f 1//6 4//6 3//6
+f 1//6 2//6 4//6
+f 3//3 8//3 7//3
+f 3//3 4//3 8//3
+f 5//5 7//5 8//5
+f 5//5 8//5 6//5
+f 1//4 5//4 6//4
+f 1//4 6//4 2//4
+f 2//1 6//1 8//1
+f 2//1 8//1 4//1