PLY: prepare bugfix by adding missing unittest.

code/PlyLoader.h

@@ -53,7 +53,7 @@ struct aiNode;
 struct aiMaterial;
 struct aiMesh;
 
-namespace Assimp    {
+namespace Assimp {
 
 
 using namespace PLY;

code/PlyParser.cpp

@@ -53,9 +53,10 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 using namespace Assimp;
 
 // ------------------------------------------------------------------------------------------------
-PLY::EDataType PLY::Property::ParseDataType(const char* pCur,const char** pCurOut)
+PLY::EDataType PLY::Property::ParseDataType(const char* pCur,const char** pCurOut) {
-{
+    ai_assert( NULL != pCur );
-    ai_assert(NULL != pCur && NULL != pCurOut);
+    ai_assert( NULL != pCurOut );
+
     PLY::EDataType eOut = PLY::EDT_INVALID;
 
     if (TokenMatch(pCur,"char",4) ||
@@ -100,33 +101,25 @@ PLY::EDataType PLY::Property::ParseDataType(const char* pCur,const char** pCurOu
         DefaultLogger::get()->info("Found unknown data type in PLY file. This is OK");
     }
     *pCurOut = pCur;
+
     return eOut;
 }
 
 // ------------------------------------------------------------------------------------------------
-PLY::ESemantic PLY::Property::ParseSemantic(const char* pCur,const char** pCurOut)
+PLY::ESemantic PLY::Property::ParseSemantic(const char* pCur,const char** pCurOut) {
-{
+    ai_assert (NULL != pCur );
-    ai_assert(NULL != pCur && NULL != pCurOut);
+    ai_assert( NULL != pCurOut );
 
     PLY::ESemantic eOut = PLY::EST_INVALID;
-    if (TokenMatch(pCur,"red",3))
+    if (TokenMatch(pCur,"red",3)) {
-    {
         eOut = PLY::EST_Red;
-    }
+    } else if (TokenMatch(pCur,"green",5)) {
-    else if (TokenMatch(pCur,"green",5))
-    {
         eOut = PLY::EST_Green;
-    }
+    } else if (TokenMatch(pCur,"blue",4)) {
-    else if (TokenMatch(pCur,"blue",4))
-    {
         eOut = PLY::EST_Blue;
-    }
+    } else if (TokenMatch(pCur,"alpha",5)) {
-    else if (TokenMatch(pCur,"alpha",5))
-    {
         eOut = PLY::EST_Alpha;
-    }
+    } else if (TokenMatch(pCur,"vertex_index",12) || TokenMatch(pCur,"vertex_indices",14)) {
-    else if (TokenMatch(pCur,"vertex_index",12) || TokenMatch(pCur,"vertex_indices",14))
-    {
         eOut = PLY::EST_VertexIndex;
     }
     else if (TokenMatch(pCur,"material_index",14))
@@ -248,7 +241,8 @@ bool PLY::Property::ParseProperty (const char* pCur,
     const char** pCurOut,
     PLY::Property* pOut)
 {
-    ai_assert(NULL != pCur && NULL != pCurOut);
+    ai_assert( NULL != pCur );
+    ai_assert( NULL != pCurOut );
 
     // Forms supported:
     // "property float x"
@@ -256,7 +250,9 @@ bool PLY::Property::ParseProperty (const char* pCur,
     *pCurOut = pCur;
 
     // skip leading spaces
-    if (!SkipSpaces(pCur,&pCur))return false;
+    if (!SkipSpaces(pCur,&pCur)) {
+        return false;
+    }
 
     // skip the "property" string at the beginning
     if (!TokenMatch(pCur,"property",8))
@@ -265,7 +261,9 @@ bool PLY::Property::ParseProperty (const char* pCur,
         return false;
     }
     // get next word
-    if (!SkipSpaces(pCur,&pCur))return false;
+    if (!SkipSpaces(pCur,&pCur)) {
+        return false;
+    }
     if (TokenMatch(pCur,"list",4))
     {
         pOut->bIsList = true;
@@ -313,6 +311,7 @@ bool PLY::Property::ParseProperty (const char* pCur,
 
     SkipSpacesAndLineEnd(pCur,&pCur);
     *pCurOut = pCur;
+
     return true;
 }
 
@@ -350,6 +349,7 @@ PLY::EElementSemantic PLY::Element::ParseSemantic(const char* pCur,
         eOut = PLY::EEST_Material;
     }
     *pCurOut = pCur;
+
     return eOut;
 }
 
@@ -358,13 +358,17 @@ bool PLY::Element::ParseElement (const char* pCur,
     const char** pCurOut,
     PLY::Element* pOut)
 {
-    ai_assert(NULL != pCur && NULL != pCurOut && NULL != pOut);
+    ai_assert( NULL != pCur );
+    ai_assert( NULL != pCurOut );
+    ai_assert( NULL != pOut );
 
     // Example format: "element vertex 8"
     *pCurOut = pCur;
 
     // skip leading spaces
-    if (!SkipSpaces(&pCur))return false;
+    if (!SkipSpaces(&pCur)) {
+        return false;
+    }
 
     // skip the "element" string at the beginning
     if (!TokenMatch(pCur,"element",7))
@@ -405,6 +409,7 @@ bool PLY::Element::ParseElement (const char* pCur,
         pOut->alProperties.push_back(prop);
     }
     *pCurOut = pCur;
+
     return true;
 }
 
@@ -412,30 +417,35 @@ bool PLY::Element::ParseElement (const char* pCur,
 bool PLY::DOM::SkipComments (const char* pCur,
     const char** pCurOut)
 {
-    ai_assert(NULL != pCur && NULL != pCurOut);
+    ai_assert( NULL != pCur );
+    ai_assert( NULL != pCurOut );
     *pCurOut = pCur;
 
     // skip spaces
-    if (!SkipSpaces(pCur,&pCur))return false;
+    if (!SkipSpaces(pCur,&pCur)) {
+        return false;
+    }
 
     if (TokenMatch(pCur,"comment",7))
     {
-	     if ( !IsLineEnd(pCur[-1]) )
+        if ( !IsLineEnd(pCur[-1]) )
-	     {
+        {
-	         SkipLine(pCur,&pCur);
+            SkipLine(pCur,&pCur);
-	     }
+        }
         SkipComments(pCur,&pCur);
         *pCurOut = pCur;
         return true;
     }
     *pCurOut = pCur;
+
     return false;
 }
 
 // ------------------------------------------------------------------------------------------------
-bool PLY::DOM::ParseHeader (const char* pCur,const char** pCurOut,bool isBinary)
+bool PLY::DOM::ParseHeader (const char* pCur,const char** pCurOut,bool isBinary) {
-{
+    ai_assert( NULL != pCur );
-    ai_assert(NULL != pCur && NULL != pCurOut);
+    ai_assert( NULL != pCurOut );
+
     DefaultLogger::get()->debug("PLY::DOM::ParseHeader() begin");
 
     // after ply and format line
@@ -479,7 +489,8 @@ bool PLY::DOM::ParseElementInstanceLists (
     const char* pCur,
     const char** pCurOut)
 {
-    ai_assert(NULL != pCur && NULL != pCurOut);
+    ai_assert( NULL != pCur );
+    ai_assert( NULL != pCurOut );
 
     DefaultLogger::get()->debug("PLY::DOM::ParseElementInstanceLists() begin");
     *pCurOut = pCur;
@@ -507,7 +518,8 @@ bool PLY::DOM::ParseElementInstanceListsBinary (
     const char** pCurOut,
     bool p_bBE)
 {
-    ai_assert(NULL != pCur && NULL != pCurOut);
+    ai_assert( NULL != pCur );
+    ai_assert( NULL != pCurOut);
 
     DefaultLogger::get()->debug("PLY::DOM::ParseElementInstanceListsBinary() begin");
     *pCurOut = pCur;
@@ -532,7 +544,8 @@ bool PLY::DOM::ParseElementInstanceListsBinary (
 // ------------------------------------------------------------------------------------------------
 bool PLY::DOM::ParseInstanceBinary (const char* pCur,DOM* p_pcOut,bool p_bBE)
 {
-    ai_assert(NULL != pCur && NULL != p_pcOut);
+    ai_assert( NULL != pCur );
+    ai_assert( NULL != p_pcOut );
 
     DefaultLogger::get()->debug("PLY::DOM::ParseInstanceBinary() begin");
 
@@ -580,7 +593,10 @@ bool PLY::ElementInstanceList::ParseInstanceList (
     const PLY::Element* pcElement,
     PLY::ElementInstanceList* p_pcOut)
 {
-    ai_assert(NULL != pCur && NULL != pCurOut && NULL != pcElement && NULL != p_pcOut);
+    ai_assert( NULL != pCur );
+    ai_assert( NULL != pCurOut );
+    ai_assert( NULL != pcElement );
+    ai_assert( NULL != p_pcOut );
 
     if (EEST_INVALID == pcElement->eSemantic || pcElement->alProperties.empty())
     {
@@ -614,7 +630,10 @@ bool PLY::ElementInstanceList::ParseInstanceListBinary (
     PLY::ElementInstanceList* p_pcOut,
     bool p_bBE /* = false */)
 {
-    ai_assert(NULL != pCur && NULL != pCurOut && NULL != pcElement && NULL != p_pcOut);
+    ai_assert( NULL != pCur );
+    ai_assert( NULL != pCurOut );
+    ai_assert( NULL != pcElement );
+    ai_assert( NULL != p_pcOut );
 
     // we can add special handling code for unknown element semantics since
     // we can't skip it as a whole block (we don't know its exact size
@@ -636,9 +655,14 @@ bool PLY::ElementInstance::ParseInstance (
     const PLY::Element* pcElement,
     PLY::ElementInstance* p_pcOut)
 {
-    ai_assert(NULL != pCur && NULL != pCurOut && NULL != pcElement && NULL != p_pcOut);
+    ai_assert( NULL != pCur );
+    ai_assert( NULL != pCurOut );
+    ai_assert( NULL != pcElement );
+    ai_assert( NULL != p_pcOut );
 
-    if (!SkipSpaces(pCur, &pCur))return false;
+    if (!SkipSpaces(pCur, &pCur)) {
+        return false;
+    }
 
     // allocate enough storage
     p_pcOut->alProperties.resize(pcElement->alProperties.size());
@@ -671,7 +695,10 @@ bool PLY::ElementInstance::ParseInstanceBinary (
     PLY::ElementInstance* p_pcOut,
     bool p_bBE /* = false */)
 {
-    ai_assert(NULL != pCur && NULL != pCurOut && NULL != pcElement && NULL != p_pcOut);
+    ai_assert( NULL != pCur );
+    ai_assert( NULL != pCurOut );
+    ai_assert( NULL != pcElement );
+    ai_assert( NULL != p_pcOut );
 
     // allocate enough storage
     p_pcOut->alProperties.resize(pcElement->alProperties.size());
@@ -696,12 +723,17 @@ bool PLY::ElementInstance::ParseInstanceBinary (
 bool PLY::PropertyInstance::ParseInstance (const char* pCur,const char** pCurOut,
     const PLY::Property* prop, PLY::PropertyInstance* p_pcOut)
 {
-    ai_assert(NULL != pCur && NULL != pCurOut && NULL !=  prop && NULL != p_pcOut);
+    ai_assert( NULL != pCur );
+    ai_assert( NULL != pCurOut );
+    ai_assert( NULL != prop );
+    ai_assert( NULL != p_pcOut );
 
     *pCurOut = pCur;
 
     // skip spaces at the beginning
-    if (!SkipSpaces(pCur, &pCur))return false;
+    if (!SkipSpaces(pCur, &pCur)) {
+        return false;
+    }
 
     if (prop->bIsList)
     {
@@ -741,7 +773,10 @@ bool PLY::PropertyInstance::ParseInstanceBinary (
     PLY::PropertyInstance* p_pcOut,
     bool p_bBE)
 {
-    ai_assert(NULL != pCur && NULL != pCurOut && NULL != prop && NULL != p_pcOut);
+    ai_assert( NULL != pCur );
+    ai_assert( NULL != pCurOut );
+    ai_assert( NULL != prop );
+    ai_assert( NULL != p_pcOut );
 
     if (prop->bIsList)
     {
@@ -770,8 +805,7 @@ bool PLY::PropertyInstance::ParseInstanceBinary (
 }
 
 // ------------------------------------------------------------------------------------------------
-PLY::PropertyInstance::ValueUnion PLY::PropertyInstance::DefaultValue(
+PLY::PropertyInstance::ValueUnion PLY::PropertyInstance::DefaultValue( PLY::EDataType eType )
-    PLY::EDataType eType)
 {
     PLY::PropertyInstance::ValueUnion out;
 
@@ -798,7 +832,9 @@ bool PLY::PropertyInstance::ParseValue(
     PLY::EDataType eType,
     PLY::PropertyInstance::ValueUnion* out)
 {
-    ai_assert(NULL != pCur && NULL != pCurOut && NULL != out);
+    ai_assert( NULL != pCur );
+    ai_assert( NULL != pCurOut );
+    ai_assert( NULL != out );
 
     bool ret = true;
     *pCurOut = pCur;
@@ -827,7 +863,6 @@ bool PLY::PropertyInstance::ParseValue(
         break;
 
     case EDT_Double:
-
         double d;
         pCur = fast_atoreal_move<double>(pCur,d);
         out->fDouble = (double)d;
@@ -835,8 +870,10 @@ bool PLY::PropertyInstance::ParseValue(
 
     default:
         ret = false;
+        break;
     }
     *pCurOut = pCur;
+
     return ret;
 }
 
@@ -848,7 +885,9 @@ bool PLY::PropertyInstance::ParseValueBinary(
     PLY::PropertyInstance::ValueUnion* out,
     bool p_bBE)
 {
-    ai_assert(NULL != pCur && NULL != pCurOut && NULL != out);
+    ai_assert( NULL != pCur );
+    ai_assert( NULL != pCurOut );
+    ai_assert( NULL != out );
 
     bool ret = true;
     switch (eType)
@@ -925,6 +964,7 @@ bool PLY::PropertyInstance::ParseValueBinary(
         ret = false;
     }
     *pCurOut = pCur;
+
     return ret;
 }
 

test/CMakeLists.txt

@@ -104,6 +104,7 @@ SET( TEST_SRCS
   unit/utSIBImporter.cpp
   unit/utObjImportExport.cpp
   unit/utPretransformVertices.cpp
+  unit/utPLYImportExport.cpp
   unit/utRemoveComments.cpp
   unit/utRemoveComponent.cpp
   unit/utRemoveRedundantMaterials.cpp

test/models/PLY/float-color.ply

@@ -0,0 +1,18 @@
+ply
+format ascii 1.0
+comment VCGLIB generated
+element vertex 3
+property float x
+property float y
+property float z
+property float red
+property float green
+property float blue
+property float alpha
+element face 1
+property list uchar int vertex_indices
+end_header
+0.0 0.0 0.0 0 0 1 1
+100.0 0.0 0.0 0 0 1 1
+200.0 200.0 0.0 0 0 1 1
+3 0 1 2

test/unit/utPLYImportExport.cpp

@@ -0,0 +1,59 @@
+/*
+---------------------------------------------------------------------------
+Open Asset Import Library (assimp)
+---------------------------------------------------------------------------
+
+Copyright (c) 2006-2017, assimp team
+
+All rights reserved.
+
+Redistribution and use of this software in source and binary forms,
+with or without modification, are permitted provided that the following
+conditions are met:
+
+* Redistributions of source code must retain the above
+copyright notice, this list of conditions and the
+following disclaimer.
+
+* Redistributions in binary form must reproduce the above
+copyright notice, this list of conditions and the
+following disclaimer in the documentation and/or other
+materials provided with the distribution.
+
+* Neither the name of the assimp team, nor the names of its
+contributors may be used to endorse or promote products
+derived from this software without specific prior
+written permission of the assimp team.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+---------------------------------------------------------------------------
+*/
+#include "UnitTestPCH.h"
+
+#include <assimp/Importer.hpp>
+#include "AbstractImportExportBase.h"
+
+using namespace ::Assimp;
+
+class utPLYImportExport : public AbstractImportExportBase {
+public:
+    virtual bool importerTest() {
+        Assimp::Importer importer;
+        const aiScene *scene = importer.ReadFile( ASSIMP_TEST_MODELS_DIR "/PLY/cube.ply", 0 );
+        return nullptr != scene;
+    }
+};
+
+TEST_F( utPLYImportExport, importTest ) {
+    EXPECT_TRUE( importerTest() );
+}