From 3685791e0d05623c68d990ba3878f17d99888211 Mon Sep 17 00:00:00 2001
From: Kim Kulling <kim.kulling@googlemail.com>
Date: Sun, 4 Feb 2018 22:15:18 +0100
Subject: [PATCH 1/8] closes https://github.com/assimp/assimp/issues/1729:
check for bit flip when unsigned int overflow happens in x-file parsing.
---
code/XFileParser.cpp | 241 ++++++++++++++++++++++---------------------
code/XFileParser.h | 59 +++++------
2 files changed, 151 insertions(+), 149 deletions(-)
diff --git a/code/XFileParser.cpp b/code/XFileParser.cpp
index c7efb83c4..34d8c89f9 100644
--- a/code/XFileParser.cpp
+++ b/code/XFileParser.cpp
@@ -87,59 +87,60 @@ static void dummy_free (void* /*opaque*/, void* address) {
// ------------------------------------------------------------------------------------------------
// Constructor. Creates a data structure out of the XFile given in the memory block.
XFileParser::XFileParser( const std::vector<char>& pBuffer)
-{
- mMajorVersion = mMinorVersion = 0;
- mIsBinaryFormat = false;
- mBinaryNumCount = 0;
- P = End = NULL;
- mLineNumber = 0;
- mScene = NULL;
-
+: mMajorVersion( 0 )
+, mMinorVersion( 0 )
+, mIsBinaryFormat( false )
+, mBinaryNumCount( 0 )
+, mP( nullptr )
+, mEnd( nullptr )
+, mLineNumber( 0 )
+, mScene( nullptr ) {
// vector to store uncompressed file for INFLATE'd X files
std::vector<char> uncompressed;
// set up memory pointers
- P = &pBuffer.front();
- End = P + pBuffer.size() - 1;
+ mP = &pBuffer.front();
+ mEnd = mP + pBuffer.size() - 1;
// check header
- if( strncmp( P, "xof ", 4) != 0)
- throw DeadlyImportError( "Header mismatch, file is not an XFile.");
+ if ( 0 != strncmp( mP, "xof ", 4 ) ) {
+ throw DeadlyImportError( "Header mismatch, file is not an XFile." );
+ }
// read version. It comes in a four byte format such as "0302"
- mMajorVersion = (unsigned int)(P[4] - 48) * 10 + (unsigned int)(P[5] - 48);
- mMinorVersion = (unsigned int)(P[6] - 48) * 10 + (unsigned int)(P[7] - 48);
+ mMajorVersion = (unsigned int)(mP[4] - 48) * 10 + (unsigned int)(mP[5] - 48);
+ mMinorVersion = (unsigned int)(mP[6] - 48) * 10 + (unsigned int)(mP[7] - 48);
bool compressed = false;
// txt - pure ASCII text format
- if( strncmp( P + 8, "txt ", 4) == 0)
+ if( strncmp( mP + 8, "txt ", 4) == 0)
mIsBinaryFormat = false;
// bin - Binary format
- else if( strncmp( P + 8, "bin ", 4) == 0)
+ else if( strncmp( mP + 8, "bin ", 4) == 0)
mIsBinaryFormat = true;
// tzip - Inflate compressed text format
- else if( strncmp( P + 8, "tzip", 4) == 0)
+ else if( strncmp( mP + 8, "tzip", 4) == 0)
{
mIsBinaryFormat = false;
compressed = true;
}
// bzip - Inflate compressed binary format
- else if( strncmp( P + 8, "bzip", 4) == 0)
+ else if( strncmp( mP + 8, "bzip", 4) == 0)
{
mIsBinaryFormat = true;
compressed = true;
}
else ThrowException( format() << "Unsupported xfile format '" <<
- P[8] << P[9] << P[10] << P[11] << "'");
+ mP[8] << mP[9] << mP[10] << mP[11] << "'");
// float size
- mBinaryFloatSize = (unsigned int)(P[12] - 48) * 1000
- + (unsigned int)(P[13] - 48) * 100
- + (unsigned int)(P[14] - 48) * 10
- + (unsigned int)(P[15] - 48);
+ mBinaryFloatSize = (unsigned int)(mP[12] - 48) * 1000
+ + (unsigned int)(mP[13] - 48) * 100
+ + (unsigned int)(mP[14] - 48) * 10
+ + (unsigned int)(mP[15] - 48);
if( mBinaryFloatSize != 32 && mBinaryFloatSize != 64)
ThrowException( format() << "Unknown float size " << mBinaryFloatSize << " specified in xfile header." );
@@ -147,7 +148,7 @@ XFileParser::XFileParser( const std::vector<char>& pBuffer)
// The x format specifies size in bits, but we work in bytes
mBinaryFloatSize /= 8;
- P += 16;
+ mP += 16;
// If this is a compressed X file, apply the inflate algorithm to it
if (compressed)
@@ -186,13 +187,13 @@ XFileParser::XFileParser( const std::vector<char>& pBuffer)
::inflateInit2(&stream, -MAX_WBITS);
// skip unknown data (checksum, flags?)
- P += 6;
+ mP += 6;
// First find out how much storage we'll need. Count sections.
- const char* P1 = P;
+ const char* P1 = mP;
unsigned int est_out = 0;
- while (P1 + 3 < End)
+ while (P1 + 3 < mEnd)
{
// read next offset
uint16_t ofs = *((uint16_t*)P1);
@@ -216,18 +217,18 @@ XFileParser::XFileParser( const std::vector<char>& pBuffer)
// Allocate storage and terminating zero and do the actual uncompressing
uncompressed.resize(est_out + 1);
char* out = &uncompressed.front();
- while (P + 3 < End)
+ while (mP + 3 < mEnd)
{
- uint16_t ofs = *((uint16_t*)P);
+ uint16_t ofs = *((uint16_t*)mP);
AI_SWAP2(ofs);
- P += 4;
+ mP += 4;
- if (P + ofs > End + 2) {
+ if (mP + ofs > mEnd + 2) {
throw DeadlyImportError("X: Unexpected EOF in compressed chunk");
}
// push data to the stream
- stream.next_in = (Bytef*)P;
+ stream.next_in = (Bytef*)mP;
stream.avail_in = ofs;
stream.next_out = (Bytef*)out;
stream.avail_out = MSZIP_BLOCK;
@@ -242,15 +243,15 @@ XFileParser::XFileParser( const std::vector<char>& pBuffer)
// and advance to the next offset
out += MSZIP_BLOCK - stream.avail_out;
- P += ofs;
+ mP += ofs;
}
// terminate zlib
::inflateEnd(&stream);
// ok, update pointers to point to the uncompressed file data
- P = &uncompressed[0];
- End = out;
+ mP = &uncompressed[0];
+ mEnd = out;
// FIXME: we don't need the compressed data anymore, could release
// it already for better memory usage. Consider breaking const-co.
@@ -647,8 +648,8 @@ void XFileParser::ParseDataObjectMeshVertexColors( Mesh* pMesh)
if( !mIsBinaryFormat)
{
FindNextNoneWhiteSpace();
- if( *P == ';' || *P == ',')
- P++;
+ if( *mP == ';' || *mP == ',')
+ mP++;
}
}
@@ -678,8 +679,8 @@ void XFileParser::ParseDataObjectMeshMaterialList( Mesh* pMesh)
// commented out version check, as version 03.03 exported from blender also has 2 semicolons
if( !mIsBinaryFormat) // && MajorVersion == 3 && MinorVersion <= 2)
{
- if(P < End && *P == ';')
- ++P;
+ if(mP < mEnd && *mP == ';')
+ ++mP;
}
// if there was only a single material index, replicate it on all faces
@@ -1029,12 +1030,12 @@ void XFileParser::TestForSeparator()
return;
FindNextNoneWhiteSpace();
- if( P >= End)
+ if( mP >= mEnd)
return;
// test and skip
- if( *P == ';' || *P == ',')
- P++;
+ if( *mP == ';' || *mP == ',')
+ mP++;
}
// ------------------------------------------------------------------------------------------------
@@ -1062,46 +1063,54 @@ std::string XFileParser::GetNextToken()
// in binary mode it will only return NAME and STRING token
// and (correctly) skip over other tokens.
- if( End - P < 2) return s;
+ if( mEnd - mP < 2) return s;
unsigned int tok = ReadBinWord();
unsigned int len;
// standalone tokens
switch( tok)
{
- case 1:
- // name token
- if( End - P < 4) return s;
- len = ReadBinDWord();
- if( End - P < int(len)) return s;
- s = std::string(P, len);
- P += len;
+ case 1: {
+ // name token
+ if ( mEnd - mP < 4 ) return s;
+ len = ReadBinDWord();
+ const int bounds( mEnd - mP );
+ const int iLen( len );
+ if ( iLen < 0 ) {
+ return s;
+ }
+ if ( bounds < iLen ) {
+ return s;
+ }
+ s = std::string( mP, len );
+ mP += len;
+ }
return s;
case 2:
// string token
- if( End - P < 4) return s;
+ if( mEnd - mP < 4) return s;
len = ReadBinDWord();
- if( End - P < int(len)) return s;
- s = std::string(P, len);
- P += (len + 2);
+ if( mEnd - mP < int(len)) return s;
+ s = std::string(mP, len);
+ mP += (len + 2);
return s;
case 3:
// integer token
- P += 4;
+ mP += 4;
return "<integer>";
case 5:
// GUID token
- P += 16;
+ mP += 16;
return "<guid>";
case 6:
- if( End - P < 4) return s;
+ if( mEnd - mP < 4) return s;
len = ReadBinDWord();
- P += (len * 4);
+ mP += (len * 4);
return "<int_list>";
case 7:
- if( End - P < 4) return s;
+ if( mEnd - mP < 4) return s;
len = ReadBinDWord();
- P += (len * mBinaryFloatSize);
+ mP += (len * mBinaryFloatSize);
return "<flt_list>";
case 0x0a:
return "{";
@@ -1159,19 +1168,19 @@ std::string XFileParser::GetNextToken()
else
{
FindNextNoneWhiteSpace();
- if( P >= End)
+ if( mP >= mEnd)
return s;
- while( (P < End) && !isspace( (unsigned char) *P))
+ while( (mP < mEnd) && !isspace( (unsigned char) *mP))
{
// either keep token delimiters when already holding a token, or return if first valid char
- if( *P == ';' || *P == '}' || *P == '{' || *P == ',')
+ if( *mP == ';' || *mP == '}' || *mP == '{' || *mP == ',')
{
if( !s.size())
- s.append( P++, 1);
+ s.append( mP++, 1);
break; // stop for delimiter
}
- s.append( P++, 1);
+ s.append( mP++, 1);
}
}
return s;
@@ -1186,18 +1195,18 @@ void XFileParser::FindNextNoneWhiteSpace()
bool running = true;
while( running )
{
- while( P < End && isspace( (unsigned char) *P))
+ while( mP < mEnd && isspace( (unsigned char) *mP))
{
- if( *P == '\n')
+ if( *mP == '\n')
mLineNumber++;
- ++P;
+ ++mP;
}
- if( P >= End)
+ if( mP >= mEnd)
return;
// check if this is a comment
- if( (P[0] == '/' && P[1] == '/') || P[0] == '#')
+ if( (mP[0] == '/' && mP[1] == '/') || mP[0] == '#')
ReadUntilEndOfLine();
else
break;
@@ -1214,22 +1223,22 @@ void XFileParser::GetNextTokenAsString( std::string& poString)
}
FindNextNoneWhiteSpace();
- if( P >= End)
+ if( mP >= mEnd)
ThrowException( "Unexpected end of file while parsing string");
- if( *P != '"')
+ if( *mP != '"')
ThrowException( "Expected quotation mark.");
- ++P;
+ ++mP;
- while( P < End && *P != '"')
- poString.append( P++, 1);
+ while( mP < mEnd && *mP != '"')
+ poString.append( mP++, 1);
- if( P >= End-1)
+ if( mP >= mEnd-1)
ThrowException( "Unexpected end of file while parsing string");
- if( P[1] != ';' || P[0] != '"')
+ if( mP[1] != ';' || mP[0] != '"')
ThrowException( "Expected quotation mark and semicolon at the end of a string.");
- P+=2;
+ mP+=2;
}
// ------------------------------------------------------------------------------------------------
@@ -1238,35 +1247,35 @@ void XFileParser::ReadUntilEndOfLine()
if( mIsBinaryFormat)
return;
- while( P < End)
+ while( mP < mEnd)
{
- if( *P == '\n' || *P == '\r')
+ if( *mP == '\n' || *mP == '\r')
{
- ++P; mLineNumber++;
+ ++mP; mLineNumber++;
return;
}
- ++P;
+ ++mP;
}
}
// ------------------------------------------------------------------------------------------------
unsigned short XFileParser::ReadBinWord()
{
- ai_assert(End - P >= 2);
- const unsigned char* q = (const unsigned char*) P;
+ ai_assert(mEnd - mP >= 2);
+ const unsigned char* q = (const unsigned char*) mP;
unsigned short tmp = q[0] | (q[1] << 8);
- P += 2;
+ mP += 2;
return tmp;
}
// ------------------------------------------------------------------------------------------------
-unsigned int XFileParser::ReadBinDWord()
-{
- ai_assert(End - P >= 4);
- const unsigned char* q = (const unsigned char*) P;
+unsigned int XFileParser::ReadBinDWord() {
+ ai_assert(mEnd - mP >= 4);
+
+ const unsigned char* q = (const unsigned char*) mP;
unsigned int tmp = q[0] | (q[1] << 8) | (q[2] << 16) | (q[3] << 24);
- P += 4;
+ mP += 4;
return tmp;
}
@@ -1275,20 +1284,20 @@ unsigned int XFileParser::ReadInt()
{
if( mIsBinaryFormat)
{
- if( mBinaryNumCount == 0 && End - P >= 2)
+ if( mBinaryNumCount == 0 && mEnd - mP >= 2)
{
unsigned short tmp = ReadBinWord(); // 0x06 or 0x03
- if( tmp == 0x06 && End - P >= 4) // array of ints follows
+ if( tmp == 0x06 && mEnd - mP >= 4) // array of ints follows
mBinaryNumCount = ReadBinDWord();
else // single int follows
mBinaryNumCount = 1;
}
--mBinaryNumCount;
- if ( End - P >= 4) {
+ if ( mEnd - mP >= 4) {
return ReadBinDWord();
} else {
- P = End;
+ mP = mEnd;
return 0;
}
} else
@@ -1299,24 +1308,24 @@ unsigned int XFileParser::ReadInt()
// check preceding minus sign
bool isNegative = false;
- if( *P == '-')
+ if( *mP == '-')
{
isNegative = true;
- P++;
+ mP++;
}
// at least one digit expected
- if( !isdigit( *P))
+ if( !isdigit( *mP))
ThrowException( "Number expected.");
// read digits
unsigned int number = 0;
- while( P < End)
+ while( mP < mEnd)
{
- if( !isdigit( *P))
+ if( !isdigit( *mP))
break;
- number = number * 10 + (*P - 48);
- P++;
+ number = number * 10 + (*mP - 48);
+ mP++;
}
CheckForSeparator();
@@ -1329,10 +1338,10 @@ ai_real XFileParser::ReadFloat()
{
if( mIsBinaryFormat)
{
- if( mBinaryNumCount == 0 && End - P >= 2)
+ if( mBinaryNumCount == 0 && mEnd - mP >= 2)
{
unsigned short tmp = ReadBinWord(); // 0x07 or 0x42
- if( tmp == 0x07 && End - P >= 4) // array of floats following
+ if( tmp == 0x07 && mEnd - mP >= 4) // array of floats following
mBinaryNumCount = ReadBinDWord();
else // single float following
mBinaryNumCount = 1;
@@ -1341,22 +1350,22 @@ ai_real XFileParser::ReadFloat()
--mBinaryNumCount;
if( mBinaryFloatSize == 8)
{
- if( End - P >= 8) {
- ai_real result = (ai_real) (*(double*) P);
- P += 8;
+ if( mEnd - mP >= 8) {
+ ai_real result = (ai_real) (*(double*) mP);
+ mP += 8;
return result;
} else {
- P = End;
+ mP = mEnd;
return 0;
}
} else
{
- if( End - P >= 4) {
- ai_real result = *(ai_real*) P;
- P += 4;
+ if( mEnd - mP >= 4) {
+ ai_real result = *(ai_real*) mP;
+ mP += 4;
return result;
} else {
- P = End;
+ mP = mEnd;
return 0;
}
}
@@ -1367,21 +1376,21 @@ ai_real XFileParser::ReadFloat()
// check for various special strings to allow reading files from faulty exporters
// I mean you, Blender!
// Reading is safe because of the terminating zero
- if( strncmp( P, "-1.#IND00", 9) == 0 || strncmp( P, "1.#IND00", 8) == 0)
+ if( strncmp( mP, "-1.#IND00", 9) == 0 || strncmp( mP, "1.#IND00", 8) == 0)
{
- P += 9;
+ mP += 9;
CheckForSeparator();
return 0.0;
} else
- if( strncmp( P, "1.#QNAN0", 8) == 0)
+ if( strncmp( mP, "1.#QNAN0", 8) == 0)
{
- P += 8;
+ mP += 8;
CheckForSeparator();
return 0.0;
}
ai_real result = 0.0;
- P = fast_atoreal_move<ai_real>( P, result);
+ mP = fast_atoreal_move<ai_real>( mP, result);
CheckForSeparator();
diff --git a/code/XFileParser.h b/code/XFileParser.h
index 050ecb9ca..24eb6104d 100644
--- a/code/XFileParser.h
+++ b/code/XFileParser.h
@@ -49,10 +49,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include <assimp/types.h>
-namespace Assimp
-{
- namespace XFile
- {
+namespace Assimp {
+ namespace XFile {
struct Node;
struct Mesh;
struct Scene;
@@ -61,21 +59,20 @@ namespace Assimp
struct AnimBone;
}
-/** The XFileParser reads a XFile either in text or binary form and builds a temporary
- * data structure out of it.
- */
-class XFileParser
-{
+/**
+ * @brief The XFileParser reads a XFile either in text or binary form and builds a temporary
+ * data structure out of it.
+ */
+class XFileParser {
public:
- /** Constructor. Creates a data structure out of the XFile given in the memory block.
- * @param pBuffer Null-terminated memory buffer containing the XFile
- */
+ /// Constructor. Creates a data structure out of the XFile given in the memory block.
+ /// @param pBuffer Null-terminated memory buffer containing the XFile
explicit XFileParser( const std::vector<char>& pBuffer);
- /** Destructor. Destroys all imported data along with it */
+ /// Destructor. Destroys all imported data along with it
~XFileParser();
- /** Returns the temporary representation of the imported data */
+ /// Returns the temporary representation of the imported data.
XFile::Scene* GetImportedData() const { return mScene; }
protected:
@@ -101,10 +98,10 @@ protected:
//! places pointer to next begin of a token, and ignores comments
void FindNextNoneWhiteSpace();
- //! returns next parseable token. Returns empty string if no token there
+ //! returns next valid token. Returns empty string if no token there
std::string GetNextToken();
- //! reads header of dataobject including the opening brace.
+ //! reads header of data object including the opening brace.
//! returns false if error happened, and writes name of object
//! if there is one
void readHeadOfDataObject( std::string* poName = NULL);
@@ -118,8 +115,8 @@ protected:
//! checks for a separator char, either a ',' or a ';'
void CheckForSeparator();
- /// tests and possibly consumes a separator char, but does nothing if there was no separator
- void TestForSeparator();
+ /// tests and possibly consumes a separator char, but does nothing if there was no separator
+ void TestForSeparator();
//! reads a x file style string
void GetNextTokenAsString( std::string& poString);
@@ -138,27 +135,23 @@ protected:
/** Throws an exception with a line number and the given text. */
AI_WONT_RETURN void ThrowException( const std::string& pText) AI_WONT_RETURN_SUFFIX;
- /** Filters the imported hierarchy for some degenerated cases that some exporters produce.
- * @param pData The sub-hierarchy to filter
- */
+ /**
+ * @brief Filters the imported hierarchy for some degenerated cases that some exporters produce.
+ * @param pData The sub-hierarchy to filter
+ */
void FilterHierarchy( XFile::Node* pNode);
protected:
unsigned int mMajorVersion, mMinorVersion; ///< version numbers
bool mIsBinaryFormat; ///< true if the file is in binary, false if it's in text form
unsigned int mBinaryFloatSize; ///< float size in bytes, either 4 or 8
- // counter for number arrays in binary format
- unsigned int mBinaryNumCount;
-
- const char* P;
- const char* End;
-
- /// Line number when reading in text format
- unsigned int mLineNumber;
-
- /// Imported data
- XFile::Scene* mScene;
+ unsigned int mBinaryNumCount; /// < counter for number arrays in binary format
+ const char* mP;
+ const char* mEnd;
+ unsigned int mLineNumber; ///< Line number when reading in text format
+ XFile::Scene* mScene; ///< Imported data
};
-}
+} //! ns Assimp
+
#endif // AI_XFILEPARSER_H_INC
From 862a145ea50088585319512f49aaaca2397034a2 Mon Sep 17 00:00:00 2001
From: Kim Kulling <kim.kulling@googlemail.com>
Date: Sun, 4 Feb 2018 22:18:54 +0100
Subject: [PATCH 2/8] X: add unittest to ensure no exception will be thrown.
---
test/unit/utXImporterExporter.cpp | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/test/unit/utXImporterExporter.cpp b/test/unit/utXImporterExporter.cpp
index 770e2be47..d4c742e6f 100644
--- a/test/unit/utXImporterExporter.cpp
+++ b/test/unit/utXImporterExporter.cpp
@@ -62,3 +62,8 @@ public:
TEST_F( utXImporterExporter, importXFromFileTest ) {
EXPECT_TRUE( importerTest() );
}
+
+TEST_F( utXImporterExporter, heap_overflow_in_tokenizer ) {
+ Assimp::Importer importer;
+ EXPECT_NO_THROW( importer.ReadFile( ASSIMP_TEST_MODELS_DIR "/X/OV_GetNextToken", 0 ) );
+}
From a8f940dd8365ac8076889ee7751949dc879862bc Mon Sep 17 00:00:00 2001
From: Kim Kulling <kim.kulling@googlemail.com>
Date: Mon, 5 Feb 2018 00:05:08 +0100
Subject: [PATCH 3/8] add missing test file.
---
test/models/X/OV_GetNextToken | Bin 0 -> 775 bytes
1 file changed, 0 insertions(+), 0 deletions(-)
create mode 100644 test/models/X/OV_GetNextToken
diff --git a/test/models/X/OV_GetNextToken b/test/models/X/OV_GetNextToken
new file mode 100644
index 0000000000000000000000000000000000000000..8b2a0d0ca49350f0a8dcdda8967ccd910da116bd
GIT binary patch
literal 775
zcmV+i1Ni)SZ)PAcGcYq^dTDSlFf%d$3jhG{3hx3#OWjsYXcIvceu*Y+vQ7G9tkp|F
z5mHYEa`R_NQ)(lo5gUK-woRt#&}3J3w>6+(4uW{{AYMgKMDgNH(L;On;@?5<dhsTn
z1P`Y3W@qBKY}%%vpc@`}@6Gpq=DjyNI}RZj1b_>3OVlh9XK{uYRbs~>0<Y<}qgUHc
zrf#o&zcv5P{*-3WF$Oh@MibHJ39T)<&S_$wBeqMLDL}+%AY$665N|z->ywsAoD^s<
zE_`|(rj{?&X~l4<WfsgTtq=$ED4hN9{ME-hg`02Y9zS}T`8>gF`q7KPqyRO;s43aB
zMopuR?!2WeU+iOx)I%qI)`KAP(3n=q8bco$Nm%b+)N=RUw)U3}{&<-Yfn3`#odwHo
z%vx14Ycz<L1NSS;W#|Oct?=Cyd<h63CXj_gBFoF3G6)oc7}FFD*QU*EGaG^dCN{WO
zT3B$1D>pL2^f{9@%CxeC#h)V;%dFyl@)vV=A6{`z-pD^YcH+zRm5Hus^wr%{l&t}8
zoF)=wX%U+m_1~p9+Y^@c!TkfeB!HHlPOnWu`{(fvDGz##u>WbB)ine=8GFXE+&ELG
zq`EBY?^OfC8gb*64uOxV=X@&&6A(JTJWsYCdh0q2jq&gZs*!(Hx({YN?5ZGL&}sll
zz5ca*jpO=*?bQDc)*l1)K0khYPF-THcF8knP2T{kZ(_r9j6#Oh2v5ciQQI)1kRnSS
z3G0k6q=+I%c&$AdXF1L&rpPfqqn?a$l8h3HjQpUGaWe1>4J$J8yM&BWg)hwAij4do
zo~I_}VU|ssQFnrdK@LPuyuNk5SKqzn)JrZKp65XR{}~2NTs$}qWzuw)ZIZVvyXqT?
z!|?Av+NPfbQP?MTJf^fU>}y+8v`)4uE?Onqk`%3xZ5R=)kYyiLN<Sn@FYmQe)NwJ7
zq=-GRjeW*_H>buU1%8V~`Nl#DX+hsB;m*C0VkUe4QhoZ^XPrEHf13EU?ITs+9fQ@y
F-#UtLa=-up
literal 0
HcmV?d00001
From ff556027ef4424ba2f1ed64908be54bdcf98038e Mon Sep 17 00:00:00 2001
From: Kim Kulling <kim.kulling@googlemail.com>
Date: Mon, 5 Feb 2018 00:33:41 +0100
Subject: [PATCH 4/8] X: fix some review findings.
---
code/XFileParser.cpp | 61 ++++++++++++++++++++++----------------------
1 file changed, 31 insertions(+), 30 deletions(-)
diff --git a/code/XFileParser.cpp b/code/XFileParser.cpp
index 34d8c89f9..92d8a579b 100644
--- a/code/XFileParser.cpp
+++ b/code/XFileParser.cpp
@@ -466,12 +466,11 @@ void XFileParser::ParseDataObjectMesh( Mesh* pMesh)
// read position faces
unsigned int numPosFaces = ReadInt();
pMesh->mPosFaces.resize( numPosFaces);
- for( unsigned int a = 0; a < numPosFaces; a++)
- {
+ for( unsigned int a = 0; a < numPosFaces; ++a) {
// read indices
unsigned int numIndices = ReadInt();
Face& face = pMesh->mPosFaces[a];
- for (unsigned int b = 0; b < numIndices; b++) {
+ for (unsigned int b = 0; b < numIndices; ++b) {
face.mIndices.push_back( ReadInt() );
}
TestForSeparator();
@@ -479,11 +478,10 @@ void XFileParser::ParseDataObjectMesh( Mesh* pMesh)
// here, other data objects may follow
bool running = true;
- while ( running )
- {
+ while ( running ) {
std::string objectName = GetNextToken();
- if( objectName.size() == 0)
+ if( objectName.empty() )
ThrowException( "Unexpected end of file while parsing mesh structure");
else
if( objectName == "}")
@@ -518,8 +516,10 @@ void XFileParser::ParseDataObjectMesh( Mesh* pMesh)
}
// ------------------------------------------------------------------------------------------------
-void XFileParser::ParseDataObjectSkinWeights( Mesh *pMesh)
-{
+void XFileParser::ParseDataObjectSkinWeights( Mesh *pMesh) {
+ if ( nullptr == pMesh ) {
+ return;
+ }
readHeadOfDataObject();
std::string transformNodeName;
@@ -1053,39 +1053,40 @@ void XFileParser::readHeadOfDataObject( std::string* poName)
}
// ------------------------------------------------------------------------------------------------
-std::string XFileParser::GetNextToken()
-{
+std::string XFileParser::GetNextToken() {
std::string s;
// process binary-formatted file
- if( mIsBinaryFormat)
- {
+ if( mIsBinaryFormat) {
// in binary mode it will only return NAME and STRING token
// and (correctly) skip over other tokens.
-
- if( mEnd - mP < 2) return s;
+ if ( mEnd - mP < 2 ) {
+ return s;
+ }
unsigned int tok = ReadBinWord();
unsigned int len;
// standalone tokens
- switch( tok)
- {
+ switch( tok ) {
case 1: {
- // name token
- if ( mEnd - mP < 4 ) return s;
- len = ReadBinDWord();
- const int bounds( mEnd - mP );
- const int iLen( len );
- if ( iLen < 0 ) {
- return s;
- }
- if ( bounds < iLen ) {
- return s;
- }
- s = std::string( mP, len );
- mP += len;
+ // name token
+ if ( mEnd - mP < 4 ) {
+ return s;
}
- return s;
+ len = ReadBinDWord();
+ const int bounds( mEnd - mP );
+ const int iLen( len );
+ if ( iLen < 0 ) {
+ return s;
+ }
+ if ( bounds < iLen ) {
+ return s;
+ }
+ s = std::string( mP, len );
+ mP += len;
+ }
+ return s;
+
case 2:
// string token
if( mEnd - mP < 4) return s;
From eb23946fe74955eca8978e168e5a6fb00eba6e36 Mon Sep 17 00:00:00 2001
From: Kim Kulling <kimkulling@users.noreply.github.com>
Date: Mon, 5 Feb 2018 13:44:19 +0100
Subject: [PATCH 5/8] Update XFileParser.cpp
Fix alignment for float and double.
---
code/XFileParser.cpp | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/code/XFileParser.cpp b/code/XFileParser.cpp
index 92d8a579b..76ea71e21 100644
--- a/code/XFileParser.cpp
+++ b/code/XFileParser.cpp
@@ -1349,20 +1349,20 @@ ai_real XFileParser::ReadFloat()
}
--mBinaryNumCount;
- if( mBinaryFloatSize == 8)
- {
+ if( mBinaryFloatSize == 8) {
if( mEnd - mP >= 8) {
- ai_real result = (ai_real) (*(double*) mP);
+ double res;
+ ::memcpy( &res, mP, 8 );
mP += 8;
return result;
} else {
mP = mEnd;
return 0;
}
- } else
- {
+ } else {
if( mEnd - mP >= 4) {
- ai_real result = *(ai_real*) mP;
+ ai_real result;
+ ::memcpy( &result, mP, 4 );
mP += 4;
return result;
} else {
From dceb7257ddf888908581380fd8bf1a7ec1a98d66 Mon Sep 17 00:00:00 2001
From: Kim Kulling <kimkulling@users.noreply.github.com>
Date: Mon, 5 Feb 2018 13:53:06 +0100
Subject: [PATCH 6/8] Update XFileParser.cpp
Fix the build.
---
code/XFileParser.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/code/XFileParser.cpp b/code/XFileParser.cpp
index 76ea71e21..cf3529653 100644
--- a/code/XFileParser.cpp
+++ b/code/XFileParser.cpp
@@ -1354,6 +1354,7 @@ ai_real XFileParser::ReadFloat()
double res;
::memcpy( &res, mP, 8 );
mP += 8;
+ const ai_real result( static_cast<ai_real>( res ) );
return result;
} else {
mP = mEnd;
From 495ae70cc5423b1cda7c5999c4b44cf9fb9de0cd Mon Sep 17 00:00:00 2001
From: Kim Kulling <kim.kulling@googlemail.com>
Date: Tue, 6 Feb 2018 19:21:56 +0100
Subject: [PATCH 7/8] XFileParser: release x-file-based scene when throwing an
exception.
---
code/3DSLoader.cpp | 16 +-
code/PlyLoader.cpp | 352 +++++++++++++++-----------------
code/PlyLoader.h | 1 -
code/PlyParser.h | 43 ++--
code/XFileParser.cpp | 42 ++--
test/unit/utPLYImportExport.cpp | 25 +++
6 files changed, 243 insertions(+), 236 deletions(-)
diff --git a/code/3DSLoader.cpp b/code/3DSLoader.cpp
index 7a5e2b6e5..9d61e125c 100644
--- a/code/3DSLoader.cpp
+++ b/code/3DSLoader.cpp
@@ -113,22 +113,24 @@ Discreet3DSImporter::Discreet3DSImporter()
, mScene()
, mMasterScale()
, bHasBG()
-, bIsPrj()
-{}
+, bIsPrj() {
+ // empty
+}
// ------------------------------------------------------------------------------------------------
// Destructor, private as well
-Discreet3DSImporter::~Discreet3DSImporter()
-{}
+Discreet3DSImporter::~Discreet3DSImporter() {
+ // empty
+}
// ------------------------------------------------------------------------------------------------
// Returns whether the class can handle the format of the given file.
-bool Discreet3DSImporter::CanRead( const std::string& pFile, IOSystem* pIOHandler, bool checkSig) const
-{
+bool Discreet3DSImporter::CanRead( const std::string& pFile, IOSystem* pIOHandler, bool checkSig) const {
std::string extension = GetExtension(pFile);
if(extension == "3ds" || extension == "prj" ) {
return true;
}
+
if (!extension.length() || checkSig) {
uint16_t token[3];
token[0] = 0x4d4d;
@@ -210,7 +212,7 @@ void Discreet3DSImporter::InternReadFile( const std::string& pFile,
ConvertScene(pScene);
// Generate the node graph for the scene. This is a little bit
- // tricky since we'll need to split some meshes into submeshes
+ // tricky since we'll need to split some meshes into sub-meshes
GenerateNodeGraph(pScene);
// Now apply the master scaling factor to the scene
diff --git a/code/PlyLoader.cpp b/code/PlyLoader.cpp
index b6ae368df..5f4e556c1 100644
--- a/code/PlyLoader.cpp
+++ b/code/PlyLoader.cpp
@@ -58,16 +58,16 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
using namespace Assimp;
static const aiImporterDesc desc = {
- "Stanford Polygon Library (PLY) Importer",
- "",
- "",
- "",
- aiImporterFlags_SupportBinaryFlavour | aiImporterFlags_SupportTextFlavour,
- 0,
- 0,
- 0,
- 0,
- "ply"
+ "Stanford Polygon Library (PLY) Importer",
+ "",
+ "",
+ "",
+ aiImporterFlags_SupportBinaryFlavour | aiImporterFlags_SupportTextFlavour,
+ 0,
+ 0,
+ 0,
+ 0,
+ "ply"
};
@@ -92,229 +92,209 @@ namespace
// ------------------------------------------------------------------------------------------------
// Constructor to be privately used by Importer
PLYImporter::PLYImporter()
- : mBuffer(nullptr)
- , pcDOM(nullptr)
- , mGeneratedMesh(nullptr){
- // empty
+: mBuffer(nullptr)
+, pcDOM(nullptr)
+, mGeneratedMesh(nullptr) {
+ // empty
}
// ------------------------------------------------------------------------------------------------
// Destructor, private as well
PLYImporter::~PLYImporter() {
- // empty
+ // empty
}
// ------------------------------------------------------------------------------------------------
// Returns whether the class can handle the format of the given file.
-bool PLYImporter::CanRead(const std::string& pFile, IOSystem* pIOHandler, bool checkSig) const
-{
- const std::string extension = GetExtension(pFile);
+bool PLYImporter::CanRead(const std::string& pFile, IOSystem* pIOHandler, bool checkSig) const {
+ const std::string extension = GetExtension(pFile);
- if (extension == "ply")
- return true;
- else if (!extension.length() || checkSig)
- {
- if (!pIOHandler)return true;
- const char* tokens[] = { "ply" };
- return SearchFileHeaderForToken(pIOHandler, pFile, tokens, 1);
- }
- return false;
+ if ( extension == "ply" ) {
+ return true;
+ } else if (!extension.length() || checkSig) {
+ if ( !pIOHandler ) {
+ return true;
+ }
+ static const char* tokens[] = { "ply" };
+ return SearchFileHeaderForToken(pIOHandler, pFile, tokens, 1);
+ }
+
+ return false;
}
// ------------------------------------------------------------------------------------------------
-const aiImporterDesc* PLYImporter::GetInfo() const
-{
- return &desc;
+const aiImporterDesc* PLYImporter::GetInfo() const {
+ return &desc;
}
// ------------------------------------------------------------------------------------------------
static bool isBigEndian(const char* szMe) {
- ai_assert(NULL != szMe);
+ ai_assert(NULL != szMe);
- // binary_little_endian
- // binary_big_endian
- bool isBigEndian(false);
+ // binary_little_endian
+ // binary_big_endian
+ bool isBigEndian(false);
#if (defined AI_BUILD_BIG_ENDIAN)
- if ( 'l' == *szMe || 'L' == *szMe ) {
- isBigEndian = true;
- }
+ if ( 'l' == *szMe || 'L' == *szMe ) {
+ isBigEndian = true;
+ }
#else
- if ('b' == *szMe || 'B' == *szMe) {
- isBigEndian = true;
- }
+ if ('b' == *szMe || 'B' == *szMe) {
+ isBigEndian = true;
+ }
#endif // ! AI_BUILD_BIG_ENDIAN
- return isBigEndian;
+ return isBigEndian;
}
// ------------------------------------------------------------------------------------------------
// Imports the given file into the given scene structure.
-void PLYImporter::InternReadFile(const std::string& pFile,
- aiScene* pScene, IOSystem* pIOHandler)
-{
- static const std::string mode = "rb";
- std::unique_ptr<IOStream> fileStream(pIOHandler->Open(pFile, mode));
- if (!fileStream.get()) {
- throw DeadlyImportError("Failed to open file " + pFile + ".");
- }
+void PLYImporter::InternReadFile(const std::string& pFile, aiScene* pScene, IOSystem* pIOHandler) {
+ static const std::string mode = "rb";
+ std::unique_ptr<IOStream> fileStream(pIOHandler->Open(pFile, mode));
+ if (!fileStream.get()) {
+ throw DeadlyImportError("Failed to open file " + pFile + ".");
+ }
- // Get the file-size
- size_t fileSize = fileStream->FileSize();
- if ( 0 == fileSize ) {
- throw DeadlyImportError("File " + pFile + " is empty.");
- }
+ // Get the file-size
+ const size_t fileSize( fileStream->FileSize() );
+ if ( 0 == fileSize ) {
+ throw DeadlyImportError("File " + pFile + " is empty.");
+ }
- IOStreamBuffer<char> streamedBuffer(1024 * 1024);
- streamedBuffer.open(fileStream.get());
+ IOStreamBuffer<char> streamedBuffer(1024 * 1024);
+ streamedBuffer.open(fileStream.get());
- // the beginning of the file must be PLY - magic, magic
- std::vector<char> headerCheck;
- streamedBuffer.getNextLine(headerCheck);
+ // the beginning of the file must be PLY - magic, magic
+ std::vector<char> headerCheck;
+ streamedBuffer.getNextLine(headerCheck);
- if ((headerCheck.size() < 3) ||
- (headerCheck[0] != 'P' && headerCheck[0] != 'p') ||
- (headerCheck[1] != 'L' && headerCheck[1] != 'l') ||
- (headerCheck[2] != 'Y' && headerCheck[2] != 'y') )
- {
- streamedBuffer.close();
- throw DeadlyImportError("Invalid .ply file: Magic number \'ply\' is no there");
- }
+ if ((headerCheck.size() < 3) ||
+ (headerCheck[0] != 'P' && headerCheck[0] != 'p') ||
+ (headerCheck[1] != 'L' && headerCheck[1] != 'l') ||
+ (headerCheck[2] != 'Y' && headerCheck[2] != 'y') ) {
+ streamedBuffer.close();
+ throw DeadlyImportError("Invalid .ply file: Magic number \'ply\' is no there");
+ }
- std::vector<char> mBuffer2;
- streamedBuffer.getNextLine(mBuffer2);
- mBuffer = (unsigned char*)&mBuffer2[0];
+ std::vector<char> mBuffer2;
+ streamedBuffer.getNextLine(mBuffer2);
+ mBuffer = (unsigned char*)&mBuffer2[0];
- char* szMe = (char*)&this->mBuffer[0];
- SkipSpacesAndLineEnd(szMe, (const char**)&szMe);
+ char* szMe = (char*)&this->mBuffer[0];
+ SkipSpacesAndLineEnd(szMe, (const char**)&szMe);
- // determine the format of the file data and construct the aimesh
- PLY::DOM sPlyDom;
- this->pcDOM = &sPlyDom;
+ // determine the format of the file data and construct the aimesh
+ PLY::DOM sPlyDom;
+ this->pcDOM = &sPlyDom;
- if (TokenMatch(szMe, "format", 6)) {
- if (TokenMatch(szMe, "ascii", 5)) {
- SkipLine(szMe, (const char**)&szMe);
- if (!PLY::DOM::ParseInstance(streamedBuffer, &sPlyDom, this))
- {
- if (mGeneratedMesh != NULL)
- {
- delete(mGeneratedMesh);
- mGeneratedMesh = nullptr;
+ if (TokenMatch(szMe, "format", 6)) {
+ if (TokenMatch(szMe, "ascii", 5)) {
+ SkipLine(szMe, (const char**)&szMe);
+ if (!PLY::DOM::ParseInstance(streamedBuffer, &sPlyDom, this)) {
+ if (mGeneratedMesh != NULL) {
+ delete(mGeneratedMesh);
+ mGeneratedMesh = nullptr;
+ }
+
+ streamedBuffer.close();
+ throw DeadlyImportError("Invalid .ply file: Unable to build DOM (#1)");
+ }
+ } else if (!::strncmp(szMe, "binary_", 7)) {
+ szMe += 7;
+ const bool bIsBE(isBigEndian(szMe));
+
+ // skip the line, parse the rest of the header and build the DOM
+ if (!PLY::DOM::ParseInstanceBinary(streamedBuffer, &sPlyDom, this, bIsBE)) {
+ if (mGeneratedMesh != NULL) {
+ delete(mGeneratedMesh);
+ mGeneratedMesh = nullptr;
+ }
+
+ streamedBuffer.close();
+ throw DeadlyImportError("Invalid .ply file: Unable to build DOM (#2)");
+ }
+ } else {
+ if (mGeneratedMesh != NULL) {
+ delete(mGeneratedMesh);
+ mGeneratedMesh = nullptr;
+ }
+
+ streamedBuffer.close();
+ throw DeadlyImportError("Invalid .ply file: Unknown file format");
+ }
+ } else {
+ AI_DEBUG_INVALIDATE_PTR(this->mBuffer);
+ if (mGeneratedMesh != NULL) {
+ delete(mGeneratedMesh);
+ mGeneratedMesh = nullptr;
}
streamedBuffer.close();
- throw DeadlyImportError("Invalid .ply file: Unable to build DOM (#1)");
- }
+ throw DeadlyImportError("Invalid .ply file: Missing format specification");
}
- else if (!::strncmp(szMe, "binary_", 7))
- {
- szMe += 7;
- const bool bIsBE(isBigEndian(szMe));
- // skip the line, parse the rest of the header and build the DOM
- if (!PLY::DOM::ParseInstanceBinary(streamedBuffer, &sPlyDom, this, bIsBE))
- {
- if (mGeneratedMesh != NULL)
- {
- delete(mGeneratedMesh);
- mGeneratedMesh = nullptr;
+ //free the file buffer
+ streamedBuffer.close();
+
+ if (mGeneratedMesh == NULL) {
+ throw DeadlyImportError("Invalid .ply file: Unable to extract mesh data ");
+ }
+
+ // if no face list is existing we assume that the vertex
+ // list is containing a list of points
+ bool pointsOnly = mGeneratedMesh->mFaces == NULL ? true : false;
+ if (pointsOnly) {
+ if (mGeneratedMesh->mNumVertices < 3) {
+ if (mGeneratedMesh != NULL) {
+ delete(mGeneratedMesh);
+ mGeneratedMesh = nullptr;
+ }
+
+ streamedBuffer.close();
+ throw DeadlyImportError("Invalid .ply file: Not enough "
+ "vertices to build a proper face list. ");
}
- streamedBuffer.close();
- throw DeadlyImportError("Invalid .ply file: Unable to build DOM (#2)");
- }
- }
- else
- {
- if (mGeneratedMesh != NULL)
- {
- delete(mGeneratedMesh);
- mGeneratedMesh = nullptr;
- }
+ const unsigned int iNum = (unsigned int)mGeneratedMesh->mNumVertices / 3;
+ mGeneratedMesh->mNumFaces = iNum;
+ mGeneratedMesh->mFaces = new aiFace[mGeneratedMesh->mNumFaces];
- streamedBuffer.close();
- throw DeadlyImportError("Invalid .ply file: Unknown file format");
- }
- }
- else
- {
- AI_DEBUG_INVALIDATE_PTR(this->mBuffer);
- if (mGeneratedMesh != NULL)
- {
- delete(mGeneratedMesh);
- mGeneratedMesh = nullptr;
+ for (unsigned int i = 0; i < iNum; ++i) {
+ mGeneratedMesh->mFaces[i].mNumIndices = 3;
+ mGeneratedMesh->mFaces[i].mIndices = new unsigned int[3];
+ mGeneratedMesh->mFaces[i].mIndices[0] = (i * 3);
+ mGeneratedMesh->mFaces[i].mIndices[1] = (i * 3) + 1;
+ mGeneratedMesh->mFaces[i].mIndices[2] = (i * 3) + 2;
+ }
}
- streamedBuffer.close();
- throw DeadlyImportError("Invalid .ply file: Missing format specification");
- }
+ // now load a list of all materials
+ std::vector<aiMaterial*> avMaterials;
+ std::string defaultTexture;
+ LoadMaterial(&avMaterials, defaultTexture, pointsOnly);
- //free the file buffer
- streamedBuffer.close();
-
- if (mGeneratedMesh == NULL)
- {
- throw DeadlyImportError("Invalid .ply file: Unable to extract mesh data ");
- }
-
- // if no face list is existing we assume that the vertex
- // list is containing a list of points
- bool pointsOnly = mGeneratedMesh->mFaces == NULL ? true : false;
- if (pointsOnly)
- {
- if (mGeneratedMesh->mNumVertices < 3)
- {
- if (mGeneratedMesh != NULL)
- {
- delete(mGeneratedMesh);
- mGeneratedMesh = nullptr;
- }
-
- streamedBuffer.close();
- throw DeadlyImportError("Invalid .ply file: Not enough "
- "vertices to build a proper face list. ");
+ // now generate the output scene object. Fill the material list
+ pScene->mNumMaterials = (unsigned int)avMaterials.size();
+ pScene->mMaterials = new aiMaterial*[pScene->mNumMaterials];
+ for (unsigned int i = 0; i < pScene->mNumMaterials; ++i) {
+ pScene->mMaterials[i] = avMaterials[i];
}
- const unsigned int iNum = (unsigned int)mGeneratedMesh->mNumVertices / 3;
- mGeneratedMesh->mNumFaces = iNum;
- mGeneratedMesh->mFaces = new aiFace[mGeneratedMesh->mNumFaces];
+ // fill the mesh list
+ pScene->mNumMeshes = 1;
+ pScene->mMeshes = new aiMesh*[pScene->mNumMeshes];
+ pScene->mMeshes[0] = mGeneratedMesh;
+ mGeneratedMesh = nullptr;
- for (unsigned int i = 0; i < iNum; ++i)
- {
- mGeneratedMesh->mFaces[i].mNumIndices = 3;
- mGeneratedMesh->mFaces[i].mIndices = new unsigned int[3];
- mGeneratedMesh->mFaces[i].mIndices[0] = (i * 3);
- mGeneratedMesh->mFaces[i].mIndices[1] = (i * 3) + 1;
- mGeneratedMesh->mFaces[i].mIndices[2] = (i * 3) + 2;
+ // generate a simple node structure
+ pScene->mRootNode = new aiNode();
+ pScene->mRootNode->mNumMeshes = pScene->mNumMeshes;
+ pScene->mRootNode->mMeshes = new unsigned int[pScene->mNumMeshes];
+
+ for (unsigned int i = 0; i < pScene->mRootNode->mNumMeshes; ++i) {
+ pScene->mRootNode->mMeshes[i] = i;
}
- }
-
- // now load a list of all materials
- std::vector<aiMaterial*> avMaterials;
- std::string defaultTexture;
- LoadMaterial(&avMaterials, defaultTexture, pointsOnly);
-
- // now generate the output scene object. Fill the material list
- pScene->mNumMaterials = (unsigned int)avMaterials.size();
- pScene->mMaterials = new aiMaterial*[pScene->mNumMaterials];
- for (unsigned int i = 0; i < pScene->mNumMaterials; ++i) {
- pScene->mMaterials[i] = avMaterials[i];
- }
-
- // fill the mesh list
- pScene->mNumMeshes = 1;
- pScene->mMeshes = new aiMesh*[pScene->mNumMeshes];
- pScene->mMeshes[0] = mGeneratedMesh;
- mGeneratedMesh = nullptr;
-
- // generate a simple node structure
- pScene->mRootNode = new aiNode();
- pScene->mRootNode->mNumMeshes = pScene->mNumMeshes;
- pScene->mRootNode->mMeshes = new unsigned int[pScene->mNumMeshes];
-
- for (unsigned int i = 0; i < pScene->mRootNode->mNumMeshes; ++i) {
- pScene->mRootNode->mMeshes[i] = i;
- }
}
void PLYImporter::LoadVertex(const PLY::Element* pcElement, const PLY::ElementInstance* instElement, unsigned int pos) {
@@ -521,9 +501,7 @@ void PLYImporter::LoadVertex(const PLY::Element* pcElement, const PLY::ElementIn
// ------------------------------------------------------------------------------------------------
// Convert a color component to [0...1]
-ai_real PLYImporter::NormalizeColorValue(PLY::PropertyInstance::ValueUnion val,
- PLY::EDataType eType)
-{
+ai_real PLYImporter::NormalizeColorValue(PLY::PropertyInstance::ValueUnion val, PLY::EDataType eType) {
switch (eType)
{
case EDT_Float:
diff --git a/code/PlyLoader.h b/code/PlyLoader.h
index beada5f23..9199e17d7 100644
--- a/code/PlyLoader.h
+++ b/code/PlyLoader.h
@@ -57,7 +57,6 @@ struct aiMesh;
namespace Assimp {
-
using namespace PLY;
// ---------------------------------------------------------------------------
diff --git a/code/PlyParser.h b/code/PlyParser.h
index cba48a4b6..261ac7b82 100644
--- a/code/PlyParser.h
+++ b/code/PlyParser.h
@@ -39,12 +39,11 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
----------------------------------------------------------------------
*/
-
/** @file Defines the helper data structures for importing PLY files */
+#pragma once
#ifndef AI_PLYFILEHELPER_H_INC
#define AI_PLYFILEHELPER_H_INC
-
#include <assimp/ParsingUtils.h>
#include <assimp/IOStreamBuffer.h>
#include <vector>
@@ -58,8 +57,7 @@ class PLYImporter;
// http://local.wasp.uwa.edu.au/~pbourke/dataformats/ply/
// http://w3.impa.br/~lvelho/outgoing/sossai/old/ViHAP_D4.4.2_PLY_format_v1.1.pdf
// http://www.okino.com/conv/exp_ply.htm
-namespace PLY
-{
+namespace PLY {
// ---------------------------------------------------------------------------------
/*
@@ -78,8 +76,7 @@ int8
int16
uint8 ... forms are also used
*/
-enum EDataType
-{
+enum EDataType {
EDT_Char = 0x0u,
EDT_UChar,
EDT_Short,
@@ -98,8 +95,7 @@ enum EDataType
*
* Semantics define the usage of a property, e.g. x coordinate
*/
-enum ESemantic
-{
+enum ESemantic {
//! vertex position x coordinate
EST_XCoord = 0x0u,
//! vertex position x coordinate
@@ -182,15 +178,14 @@ enum ESemantic
*
* Semantics define the usage of an element, e.g. vertex or material
*/
-enum EElementSemantic
-{
+enum EElementSemantic {
//! The element is a vertex
EEST_Vertex = 0x0u,
//! The element is a face description (index table)
EEST_Face,
- //! The element is a tristrip description (index table)
+ //! The element is a triangle-strip description (index table)
EEST_TriStrip,
//! The element is an edge description (ignored)
@@ -211,17 +206,16 @@ enum EElementSemantic
*
* This can e.g. be a part of the vertex declaration
*/
-class Property
-{
+class Property {
public:
-
//! Default constructor
Property()
- : eType (EDT_Int),
- Semantic(),
- bIsList(false),
- eFirstType(EDT_UChar)
- {}
+ : eType (EDT_Int)
+ , Semantic()
+ , bIsList(false)
+ , eFirstType(EDT_UChar) {
+ // empty
+ }
//! Data type of the property
EDataType eType;
@@ -260,15 +254,14 @@ public:
* This can e.g. be the vertex declaration. Elements contain a
* well-defined number of properties.
*/
-class Element
-{
+class Element {
public:
-
//! Default constructor
Element()
- : eSemantic (EEST_INVALID)
- , NumOccur(0)
- {}
+ : eSemantic (EEST_INVALID)
+ , NumOccur(0) {
+ // empty
+ }
//! List of properties assigned to the element
//! std::vector to support operator[]
diff --git a/code/XFileParser.cpp b/code/XFileParser.cpp
index cf3529653..4017ed596 100644
--- a/code/XFileParser.cpp
+++ b/code/XFileParser.cpp
@@ -1047,8 +1047,10 @@ void XFileParser::readHeadOfDataObject( std::string* poName)
if( poName)
*poName = nameOrBrace;
- if( GetNextToken() != "{")
- ThrowException( "Opening brace expected.");
+ if ( GetNextToken() != "{" ) {
+ delete mScene;
+ ThrowException( "Opening brace expected." );
+ }
}
}
@@ -1224,21 +1226,29 @@ void XFileParser::GetNextTokenAsString( std::string& poString)
}
FindNextNoneWhiteSpace();
- if( mP >= mEnd)
- ThrowException( "Unexpected end of file while parsing string");
+ if ( mP >= mEnd ) {
+ delete mScene;
+ ThrowException( "Unexpected end of file while parsing string" );
+ }
- if( *mP != '"')
- ThrowException( "Expected quotation mark.");
+ if ( *mP != '"' ) {
+ delete mScene;
+ ThrowException( "Expected quotation mark." );
+ }
++mP;
while( mP < mEnd && *mP != '"')
poString.append( mP++, 1);
- if( mP >= mEnd-1)
- ThrowException( "Unexpected end of file while parsing string");
+ if ( mP >= mEnd - 1 ) {
+ delete mScene;
+ ThrowException( "Unexpected end of file while parsing string" );
+ }
- if( mP[1] != ';' || mP[0] != '"')
- ThrowException( "Expected quotation mark and semicolon at the end of a string.");
+ if ( mP[ 1 ] != ';' || mP[ 0 ] != '"' ) {
+ delete mScene;
+ ThrowException( "Expected quotation mark and semicolon at the end of a string." );
+ }
mP+=2;
}
@@ -1449,15 +1459,15 @@ aiColor3D XFileParser::ReadRGB()
// ------------------------------------------------------------------------------------------------
// Throws an exception with a line number and the given text.
-AI_WONT_RETURN void XFileParser::ThrowException( const std::string& pText)
-{
- if( mIsBinaryFormat)
- throw DeadlyImportError( pText);
- else
+AI_WONT_RETURN void XFileParser::ThrowException( const std::string& pText) {
+ delete mScene;
+ if ( mIsBinaryFormat ) {
+ throw DeadlyImportError( pText );
+ } else {
throw DeadlyImportError( format() << "Line " << mLineNumber << ": " << pText );
+ }
}
-
// ------------------------------------------------------------------------------------------------
// Filters the imported hierarchy for some degenerated cases that some exporters produce.
void XFileParser::FilterHierarchy( XFile::Node* pNode)
diff --git a/test/unit/utPLYImportExport.cpp b/test/unit/utPLYImportExport.cpp
index fdd20008d..633beda5f 100644
--- a/test/unit/utPLYImportExport.cpp
+++ b/test/unit/utPLYImportExport.cpp
@@ -103,3 +103,28 @@ TEST_F( utPLYImportExport, vertexColorTest ) {
const aiScene *scene = importer.ReadFile( ASSIMP_TEST_MODELS_DIR "/PLY/float-color.ply", 0 );
EXPECT_NE( nullptr, scene );
}
+
+static const char *test_file =
+ "ply\n"
+ "format ascii 1.0\n"
+ "element vertex 4\n"
+ "property float x\n"
+ "property float y\n"
+ "property float z\n"
+ "property uchar red\n"
+ "property uchar green\n"
+ "property uchar blue\n"
+ "property float nx\n"
+ "property float ny\n"
+ "property float nz\n"
+ "end_header\n"
+ "0.0 0.0 0.0 255 255 255 0.0 1.0 0.0\n"
+ "0.0 0.0 1.0 255 0 255 0.0 0.0 1.0\n"
+ "0.0 1.0 0.0 255 255 0 1.0 0.0 0.0\n"
+ "0.0 1.0 1.0 0 255 255 1.0 1.0 0.0\n";
+
+TEST_F( utPLYImportExport, parseErrorTest ) {
+ Assimp::Importer importer;
+ const aiScene *scene = importer.ReadFileFromMemory( test_file, strlen( test_file ), 0 );
+ EXPECT_NE( nullptr, scene );
+}
From 57c1fe5954aa44107e86a7c17b0df2ff9a2cd846 Mon Sep 17 00:00:00 2001
From: Kim Kulling <kim.kulling@googlemail.com>
Date: Tue, 6 Feb 2018 23:59:46 +0100
Subject: [PATCH 8/8] x-parser: fix the crash.
---
code/XFileImporter.cpp | 341 ++++++++++++++++++-----------------------
code/XFileParser.cpp | 1 -
2 files changed, 153 insertions(+), 189 deletions(-)
diff --git a/code/XFileImporter.cpp b/code/XFileImporter.cpp
index 93ae6173b..fd28fbb79 100644
--- a/code/XFileImporter.cpp
+++ b/code/XFileImporter.cpp
@@ -5,8 +5,6 @@ Open Asset Import Library (assimp)
Copyright (c) 2006-2018, assimp team
-
-
All rights reserved.
Redistribution and use of this software in source and binary forms,
@@ -44,7 +42,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
* @brief Implementation of the XFile importer class
*/
-
#ifndef ASSIMP_BUILD_NO_X_IMPORTER
#include "XFileImporter.h"
@@ -79,17 +76,19 @@ static const aiImporterDesc desc = {
// ------------------------------------------------------------------------------------------------
// Constructor to be privately used by Importer
XFileImporter::XFileImporter()
-{}
+: mBuffer() {
+ // empty
+}
// ------------------------------------------------------------------------------------------------
// Destructor, private as well
-XFileImporter::~XFileImporter()
-{}
+XFileImporter::~XFileImporter() {
+ // empty
+}
// ------------------------------------------------------------------------------------------------
// Returns whether the class can handle the format of the given file.
-bool XFileImporter::CanRead( const std::string& pFile, IOSystem* pIOHandler, bool checkSig) const
-{
+bool XFileImporter::CanRead( const std::string& pFile, IOSystem* pIOHandler, bool checkSig) const {
std::string extension = GetExtension(pFile);
if(extension == "x") {
return true;
@@ -104,23 +103,24 @@ bool XFileImporter::CanRead( const std::string& pFile, IOSystem* pIOHandler, boo
// ------------------------------------------------------------------------------------------------
// Get file extension list
-const aiImporterDesc* XFileImporter::GetInfo () const
-{
+const aiImporterDesc* XFileImporter::GetInfo () const {
return &desc;
}
// ------------------------------------------------------------------------------------------------
// Imports the given file into the given scene structure.
-void XFileImporter::InternReadFile( const std::string& pFile, aiScene* pScene, IOSystem* pIOHandler)
-{
+void XFileImporter::InternReadFile( const std::string& pFile, aiScene* pScene, IOSystem* pIOHandler) {
// read file into memory
std::unique_ptr<IOStream> file( pIOHandler->Open( pFile));
- if( file.get() == NULL)
- throw DeadlyImportError( "Failed to open file " + pFile + ".");
+ if ( file.get() == NULL ) {
+ throw DeadlyImportError( "Failed to open file " + pFile + "." );
+ }
+ static const size_t MinSize = 16;
size_t fileSize = file->FileSize();
- if( fileSize < 16)
- throw DeadlyImportError( "XFile is too small.");
+ if ( fileSize < MinSize ) {
+ throw DeadlyImportError( "XFile is too small." );
+ }
// in the hope that binary files will never start with a BOM ...
mBuffer.resize( fileSize + 1);
@@ -134,8 +134,9 @@ void XFileImporter::InternReadFile( const std::string& pFile, aiScene* pScene, I
CreateDataRepresentationFromImport( pScene, parser.GetImportedData());
// if nothing came from it, report it as error
- if( !pScene->mRootNode)
- throw DeadlyImportError( "XFile is ill-formatted - no content imported.");
+ if ( !pScene->mRootNode ) {
+ throw DeadlyImportError( "XFile is ill-formatted - no content imported." );
+ }
}
// ------------------------------------------------------------------------------------------------
@@ -146,17 +147,15 @@ void XFileImporter::CreateDataRepresentationFromImport( aiScene* pScene, XFile::
ConvertMaterials( pScene, pData->mGlobalMaterials);
// copy nodes, extracting meshes and materials on the way
- pScene->mRootNode = CreateNodes( pScene, NULL, pData->mRootNode);
+ pScene->mRootNode = CreateNodes( pScene, nullptr, pData->mRootNode);
// extract animations
CreateAnimations( pScene, pData);
// read the global meshes that were stored outside of any node
- if( pData->mGlobalMeshes.size() > 0)
- {
+ if( !pData->mGlobalMeshes.empty() ) {
// create a root node to hold them if there isn't any, yet
- if( pScene->mRootNode == NULL)
- {
+ if( pScene->mRootNode == nullptr ) {
pScene->mRootNode = new aiNode;
pScene->mRootNode->mName.Set( "$dummy_node");
}
@@ -180,8 +179,7 @@ void XFileImporter::CreateDataRepresentationFromImport( aiScene* pScene, XFile::
flipper.Execute(pScene);
// finally: create a dummy material if not material was imported
- if( pScene->mNumMaterials == 0)
- {
+ if( pScene->mNumMaterials == 0) {
pScene->mNumMaterials = 1;
// create the Material
aiMaterial* mat = new aiMaterial;
@@ -205,10 +203,10 @@ void XFileImporter::CreateDataRepresentationFromImport( aiScene* pScene, XFile::
// ------------------------------------------------------------------------------------------------
// Recursively creates scene nodes from the imported hierarchy.
-aiNode* XFileImporter::CreateNodes( aiScene* pScene, aiNode* pParent, const XFile::Node* pNode)
-{
- if( !pNode)
- return NULL;
+aiNode* XFileImporter::CreateNodes( aiScene* pScene, aiNode* pParent, const XFile::Node* pNode) {
+ if ( !pNode ) {
+ return nullptr;
+ }
// create node
aiNode* node = new aiNode;
@@ -222,13 +220,13 @@ aiNode* XFileImporter::CreateNodes( aiScene* pScene, aiNode* pParent, const XFil
CreateMeshes( pScene, node, pNode->mMeshes);
// handle childs
- if( pNode->mChildren.size() > 0)
- {
+ if( !pNode->mChildren.empty() ) {
node->mNumChildren = (unsigned int)pNode->mChildren.size();
node->mChildren = new aiNode* [node->mNumChildren];
- for( unsigned int a = 0; a < pNode->mChildren.size(); a++)
- node->mChildren[a] = CreateNodes( pScene, node, pNode->mChildren[a]);
+ for ( unsigned int a = 0; a < pNode->mChildren.size(); ++a ) {
+ node->mChildren[ a ] = CreateNodes( pScene, node, pNode->mChildren[ a ] );
+ }
}
return node;
@@ -236,16 +234,14 @@ aiNode* XFileImporter::CreateNodes( aiScene* pScene, aiNode* pParent, const XFil
// ------------------------------------------------------------------------------------------------
// Creates the meshes for the given node.
-void XFileImporter::CreateMeshes( aiScene* pScene, aiNode* pNode, const std::vector<XFile::Mesh*>& pMeshes)
-{
+void XFileImporter::CreateMeshes( aiScene* pScene, aiNode* pNode, const std::vector<XFile::Mesh*>& pMeshes) {
if (pMeshes.empty()) {
return;
}
// create a mesh for each mesh-material combination in the source node
std::vector<aiMesh*> meshes;
- for( unsigned int a = 0; a < pMeshes.size(); a++)
- {
+ for( unsigned int a = 0; a < pMeshes.size(); ++a ) {
XFile::Mesh* sourceMesh = pMeshes[a];
if ( nullptr == sourceMesh ) {
continue;
@@ -255,35 +251,30 @@ void XFileImporter::CreateMeshes( aiScene* pScene, aiNode* pNode, const std::vec
ConvertMaterials( pScene, sourceMesh->mMaterials);
unsigned int numMaterials = std::max( (unsigned int)sourceMesh->mMaterials.size(), 1u);
- for( unsigned int b = 0; b < numMaterials; b++)
- {
+ for( unsigned int b = 0; b < numMaterials; ++b ) {
// collect the faces belonging to this material
std::vector<unsigned int> faces;
unsigned int numVertices = 0;
- if( sourceMesh->mFaceMaterials.size() > 0)
- {
+ if( !sourceMesh->mFaceMaterials.empty() ) {
// if there is a per-face material defined, select the faces with the corresponding material
- for( unsigned int c = 0; c < sourceMesh->mFaceMaterials.size(); c++)
- {
- if( sourceMesh->mFaceMaterials[c] == b)
- {
+ for( unsigned int c = 0; c < sourceMesh->mFaceMaterials.size(); ++c ) {
+ if( sourceMesh->mFaceMaterials[c] == b) {
faces.push_back( c);
numVertices += (unsigned int)sourceMesh->mPosFaces[c].mIndices.size();
}
}
- } else
- {
+ } else {
// if there is no per-face material, place everything into one mesh
- for( unsigned int c = 0; c < sourceMesh->mPosFaces.size(); c++)
- {
+ for( unsigned int c = 0; c < sourceMesh->mPosFaces.size(); ++c ) {
faces.push_back( c);
numVertices += (unsigned int)sourceMesh->mPosFaces[c].mIndices.size();
}
}
// no faces/vertices using this material? strange...
- if( numVertices == 0)
+ if ( numVertices == 0 ) {
continue;
+ }
// create a submesh using this material
aiMesh* mesh = new aiMesh;
@@ -291,11 +282,9 @@ void XFileImporter::CreateMeshes( aiScene* pScene, aiNode* pNode, const std::vec
// find the material in the scene's material list. Either own material
// or referenced material, it should already have a valid index
- if( sourceMesh->mFaceMaterials.size() > 0)
- {
+ if( !sourceMesh->mFaceMaterials.empty() ) {
mesh->mMaterialIndex = static_cast<unsigned int>(sourceMesh->mMaterials[b].sceneIndex);
- } else
- {
+ } else {
mesh->mMaterialIndex = 0;
}
@@ -310,28 +299,28 @@ void XFileImporter::CreateMeshes( aiScene* pScene, aiNode* pNode, const std::vec
mesh->mName.Set(sourceMesh->mName);
// normals?
- if( sourceMesh->mNormals.size() > 0)
- mesh->mNormals = new aiVector3D[numVertices];
+ if ( sourceMesh->mNormals.size() > 0 ) {
+ mesh->mNormals = new aiVector3D[ numVertices ];
+ }
// texture coords
- for( unsigned int c = 0; c < AI_MAX_NUMBER_OF_TEXTURECOORDS; c++)
- {
- if( sourceMesh->mTexCoords[c].size() > 0)
- mesh->mTextureCoords[c] = new aiVector3D[numVertices];
+ for( unsigned int c = 0; c < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++c ) {
+ if ( !sourceMesh->mTexCoords[ c ].empty() ) {
+ mesh->mTextureCoords[ c ] = new aiVector3D[ numVertices ];
+ }
}
// vertex colors
- for( unsigned int c = 0; c < AI_MAX_NUMBER_OF_COLOR_SETS; c++)
- {
- if( sourceMesh->mColors[c].size() > 0)
- mesh->mColors[c] = new aiColor4D[numVertices];
+ for( unsigned int c = 0; c < AI_MAX_NUMBER_OF_COLOR_SETS; ++c ) {
+ if ( !sourceMesh->mColors[ c ].empty() ) {
+ mesh->mColors[ c ] = new aiColor4D[ numVertices ];
+ }
}
// now collect the vertex data of all data streams present in the imported mesh
- unsigned int newIndex = 0;
+ unsigned int newIndex( 0 );
std::vector<unsigned int> orgPoints; // from which original point each new vertex stems
orgPoints.resize( numVertices, 0);
- for( unsigned int c = 0; c < faces.size(); c++)
- {
+ for( unsigned int c = 0; c < faces.size(); ++c ) {
unsigned int f = faces[c]; // index of the source face
const XFile::Face& pf = sourceMesh->mPosFaces[f]; // position source face
@@ -341,30 +330,30 @@ void XFileImporter::CreateMeshes( aiScene* pScene, aiNode* pNode, const std::vec
df.mIndices = new unsigned int[ df.mNumIndices];
// collect vertex data for indices of this face
- for( unsigned int d = 0; d < df.mNumIndices; d++)
- {
+ for( unsigned int d = 0; d < df.mNumIndices; ++d ) {
df.mIndices[d] = newIndex;
orgPoints[newIndex] = pf.mIndices[d];
// Position
mesh->mVertices[newIndex] = sourceMesh->mPositions[pf.mIndices[d]];
// Normal, if present
- if( mesh->HasNormals())
- mesh->mNormals[newIndex] = sourceMesh->mNormals[sourceMesh->mNormFaces[f].mIndices[d]];
+ if ( mesh->HasNormals() ) {
+ mesh->mNormals[ newIndex ] = sourceMesh->mNormals[ sourceMesh->mNormFaces[ f ].mIndices[ d ] ];
+ }
// texture coord sets
- for( unsigned int e = 0; e < AI_MAX_NUMBER_OF_TEXTURECOORDS; e++)
- {
- if( mesh->HasTextureCoords( e))
- {
+ for( unsigned int e = 0; e < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++e ) {
+ if( mesh->HasTextureCoords( e)) {
aiVector2D tex = sourceMesh->mTexCoords[e][pf.mIndices[d]];
mesh->mTextureCoords[e][newIndex] = aiVector3D( tex.x, 1.0f - tex.y, 0.0f);
}
}
// vertex color sets
- for( unsigned int e = 0; e < AI_MAX_NUMBER_OF_COLOR_SETS; e++)
- if( mesh->HasVertexColors( e))
- mesh->mColors[e][newIndex] = sourceMesh->mColors[e][pf.mIndices[d]];
+ for ( unsigned int e = 0; e < AI_MAX_NUMBER_OF_COLOR_SETS; ++e ) {
+ if ( mesh->HasVertexColors( e ) ) {
+ mesh->mColors[ e ][ newIndex ] = sourceMesh->mColors[ e ][ pf.mIndices[ d ] ];
+ }
+ }
newIndex++;
}
@@ -376,28 +365,29 @@ void XFileImporter::CreateMeshes( aiScene* pScene, aiNode* pNode, const std::vec
// convert all bones of the source mesh which influence vertices in this newly created mesh
const std::vector<XFile::Bone>& bones = sourceMesh->mBones;
std::vector<aiBone*> newBones;
- for( unsigned int c = 0; c < bones.size(); c++)
- {
+ for( unsigned int c = 0; c < bones.size(); ++c ) {
const XFile::Bone& obone = bones[c];
// set up a vertex-linear array of the weights for quick searching if a bone influences a vertex
std::vector<ai_real> oldWeights( sourceMesh->mPositions.size(), 0.0);
- for( unsigned int d = 0; d < obone.mWeights.size(); d++)
- oldWeights[obone.mWeights[d].mVertex] = obone.mWeights[d].mWeight;
+ for ( unsigned int d = 0; d < obone.mWeights.size(); ++d ) {
+ oldWeights[ obone.mWeights[ d ].mVertex ] = obone.mWeights[ d ].mWeight;
+ }
// collect all vertex weights that influence a vertex in the new mesh
std::vector<aiVertexWeight> newWeights;
newWeights.reserve( numVertices);
- for( unsigned int d = 0; d < orgPoints.size(); d++)
- {
+ for( unsigned int d = 0; d < orgPoints.size(); ++d ) {
// does the new vertex stem from an old vertex which was influenced by this bone?
ai_real w = oldWeights[orgPoints[d]];
- if( w > 0.0)
- newWeights.push_back( aiVertexWeight( d, w));
+ if ( w > 0.0 ) {
+ newWeights.push_back( aiVertexWeight( d, w ) );
+ }
}
// if the bone has no weights in the newly created mesh, ignore it
- if( newWeights.size() == 0)
+ if ( newWeights.empty() ) {
continue;
+ }
// create
aiBone* nbone = new aiBone;
@@ -407,14 +397,14 @@ void XFileImporter::CreateMeshes( aiScene* pScene, aiNode* pNode, const std::vec
nbone->mOffsetMatrix = obone.mOffsetMatrix;
nbone->mNumWeights = (unsigned int)newWeights.size();
nbone->mWeights = new aiVertexWeight[nbone->mNumWeights];
- for( unsigned int d = 0; d < newWeights.size(); d++)
- nbone->mWeights[d] = newWeights[d];
+ for ( unsigned int d = 0; d < newWeights.size(); ++d ) {
+ nbone->mWeights[ d ] = newWeights[ d ];
+ }
}
// store the bones in the mesh
mesh->mNumBones = (unsigned int)newBones.size();
- if( newBones.size() > 0)
- {
+ if( !newBones.empty()) {
mesh->mBones = new aiBone*[mesh->mNumBones];
std::copy( newBones.begin(), newBones.end(), mesh->mBones);
}
@@ -424,8 +414,7 @@ void XFileImporter::CreateMeshes( aiScene* pScene, aiNode* pNode, const std::vec
// reallocate scene mesh array to be large enough
aiMesh** prevArray = pScene->mMeshes;
pScene->mMeshes = new aiMesh*[pScene->mNumMeshes + meshes.size()];
- if( prevArray)
- {
+ if( prevArray) {
memcpy( pScene->mMeshes, prevArray, pScene->mNumMeshes * sizeof( aiMesh*));
delete [] prevArray;
}
@@ -435,8 +424,7 @@ void XFileImporter::CreateMeshes( aiScene* pScene, aiNode* pNode, const std::vec
pNode->mMeshes = new unsigned int[pNode->mNumMeshes];
// store all meshes in the mesh library of the scene and store their indices in the node
- for( unsigned int a = 0; a < meshes.size(); a++)
- {
+ for( unsigned int a = 0; a < meshes.size(); a++) {
pScene->mMeshes[pScene->mNumMeshes] = meshes[a];
pNode->mMeshes[a] = pScene->mNumMeshes;
pScene->mNumMeshes++;
@@ -445,16 +433,15 @@ void XFileImporter::CreateMeshes( aiScene* pScene, aiNode* pNode, const std::vec
// ------------------------------------------------------------------------------------------------
// Converts the animations from the given imported data and creates them in the scene.
-void XFileImporter::CreateAnimations( aiScene* pScene, const XFile::Scene* pData)
-{
+void XFileImporter::CreateAnimations( aiScene* pScene, const XFile::Scene* pData) {
std::vector<aiAnimation*> newAnims;
- for( unsigned int a = 0; a < pData->mAnims.size(); a++)
- {
+ for( unsigned int a = 0; a < pData->mAnims.size(); ++a ) {
const XFile::Animation* anim = pData->mAnims[a];
// some exporters mock me with empty animation tags.
- if( anim->mAnims.size() == 0)
+ if ( anim->mAnims.empty() ) {
continue;
+ }
// create a new animation to hold the data
aiAnimation* nanim = new aiAnimation;
@@ -466,15 +453,14 @@ void XFileImporter::CreateAnimations( aiScene* pScene, const XFile::Scene* pData
nanim->mNumChannels = (unsigned int)anim->mAnims.size();
nanim->mChannels = new aiNodeAnim*[nanim->mNumChannels];
- for( unsigned int b = 0; b < anim->mAnims.size(); b++)
- {
+ for( unsigned int b = 0; b < anim->mAnims.size(); ++b ) {
const XFile::AnimBone* bone = anim->mAnims[b];
aiNodeAnim* nbone = new aiNodeAnim;
nbone->mNodeName.Set( bone->mBoneName);
nanim->mChannels[b] = nbone;
// keyframes are given as combined transformation matrix keys
- if( bone->mTrafoKeys.size() > 0)
+ if( !bone->mTrafoKeys.empty() )
{
nbone->mNumPositionKeys = (unsigned int)bone->mTrafoKeys.size();
nbone->mPositionKeys = new aiVectorKey[nbone->mNumPositionKeys];
@@ -483,8 +469,7 @@ void XFileImporter::CreateAnimations( aiScene* pScene, const XFile::Scene* pData
nbone->mNumScalingKeys = (unsigned int)bone->mTrafoKeys.size();
nbone->mScalingKeys = new aiVectorKey[nbone->mNumScalingKeys];
- for( unsigned int c = 0; c < bone->mTrafoKeys.size(); c++)
- {
+ for( unsigned int c = 0; c < bone->mTrafoKeys.size(); ++c) {
// deconstruct each matrix into separate position, rotation and scaling
double time = bone->mTrafoKeys[c].mTime;
aiMatrix4x4 trafo = bone->mTrafoKeys[c].mMatrix;
@@ -516,13 +501,11 @@ void XFileImporter::CreateAnimations( aiScene* pScene, const XFile::Scene* pData
// longest lasting key sequence determines duration
nanim->mDuration = std::max( nanim->mDuration, bone->mTrafoKeys.back().mTime);
- } else
- {
+ } else {
// separate key sequences for position, rotation, scaling
nbone->mNumPositionKeys = (unsigned int)bone->mPosKeys.size();
nbone->mPositionKeys = new aiVectorKey[nbone->mNumPositionKeys];
- for( unsigned int c = 0; c < nbone->mNumPositionKeys; c++)
- {
+ for( unsigned int c = 0; c < nbone->mNumPositionKeys; ++c ) {
aiVector3D pos = bone->mPosKeys[c].mValue;
nbone->mPositionKeys[c].mTime = bone->mPosKeys[c].mTime;
@@ -532,8 +515,7 @@ void XFileImporter::CreateAnimations( aiScene* pScene, const XFile::Scene* pData
// rotation
nbone->mNumRotationKeys = (unsigned int)bone->mRotKeys.size();
nbone->mRotationKeys = new aiQuatKey[nbone->mNumRotationKeys];
- for( unsigned int c = 0; c < nbone->mNumRotationKeys; c++)
- {
+ for( unsigned int c = 0; c < nbone->mNumRotationKeys; ++c ) {
aiMatrix3x3 rotmat = bone->mRotKeys[c].mValue.GetMatrix();
nbone->mRotationKeys[c].mTime = bone->mRotKeys[c].mTime;
@@ -573,56 +555,51 @@ void XFileImporter::CreateAnimations( aiScene* pScene, const XFile::Scene* pData
void XFileImporter::ConvertMaterials( aiScene* pScene, std::vector<XFile::Material>& pMaterials)
{
// count the non-referrer materials in the array
- unsigned int numNewMaterials = 0;
- for( unsigned int a = 0; a < pMaterials.size(); a++)
- if( !pMaterials[a].mIsReference)
- numNewMaterials++;
+ unsigned int numNewMaterials( 0 );
+ for ( unsigned int a = 0; a < pMaterials.size(); ++a ) {
+ if ( !pMaterials[ a ].mIsReference ) {
+ ++numNewMaterials;
+ }
+ }
// resize the scene's material list to offer enough space for the new materials
- if( numNewMaterials > 0 )
- {
- aiMaterial** prevMats = pScene->mMaterials;
- pScene->mMaterials = new aiMaterial*[pScene->mNumMaterials + numNewMaterials];
- if( prevMats)
- {
- memcpy( pScene->mMaterials, prevMats, pScene->mNumMaterials * sizeof( aiMaterial*));
- delete [] prevMats;
- }
- }
+ if( numNewMaterials > 0 ) {
+ aiMaterial** prevMats = pScene->mMaterials;
+ pScene->mMaterials = new aiMaterial*[pScene->mNumMaterials + numNewMaterials];
+ if( nullptr != prevMats) {
+ ::memcpy( pScene->mMaterials, prevMats, pScene->mNumMaterials * sizeof( aiMaterial*));
+ delete [] prevMats;
+ }
+ }
// convert all the materials given in the array
- for( unsigned int a = 0; a < pMaterials.size(); a++)
- {
+ for( unsigned int a = 0; a < pMaterials.size(); ++a ) {
XFile::Material& oldMat = pMaterials[a];
- if( oldMat.mIsReference)
- {
- // find the material it refers to by name, and store its index
- for( size_t a = 0; a < pScene->mNumMaterials; ++a )
- {
- aiString name;
- pScene->mMaterials[a]->Get( AI_MATKEY_NAME, name);
- if( strcmp( name.C_Str(), oldMat.mName.data()) == 0 )
- {
- oldMat.sceneIndex = a;
- break;
+ if( oldMat.mIsReference) {
+ // find the material it refers to by name, and store its index
+ for( size_t a = 0; a < pScene->mNumMaterials; ++a ) {
+ aiString name;
+ pScene->mMaterials[a]->Get( AI_MATKEY_NAME, name);
+ if( strcmp( name.C_Str(), oldMat.mName.data()) == 0 ) {
+ oldMat.sceneIndex = a;
+ break;
+ }
+ }
+
+ if( oldMat.sceneIndex == SIZE_MAX ) {
+ DefaultLogger::get()->warn( format() << "Could not resolve global material reference \"" << oldMat.mName << "\"" );
+ oldMat.sceneIndex = 0;
+ }
+
+ continue;
}
- }
-
- if( oldMat.sceneIndex == SIZE_MAX )
- {
- DefaultLogger::get()->warn( format() << "Could not resolve global material reference \"" << oldMat.mName << "\"" );
- oldMat.sceneIndex = 0;
- }
-
- continue;
- }
aiMaterial* mat = new aiMaterial;
aiString name;
name.Set( oldMat.mName);
mat->AddProperty( &name, AI_MATKEY_NAME);
- // Shading model: hardcoded to PHONG, there is no such information in an XFile
+ // Shading model: hard-coded to PHONG, there is no such information in an XFile
// FIX (aramis): If the specular exponent is 0, use gouraud shading. This is a bugfix
// for some models in the SDK (e.g. good old tiny.x)
int shadeMode = (int)oldMat.mSpecularExponent == 0.0f
@@ -630,8 +607,8 @@ void XFileImporter::ConvertMaterials( aiScene* pScene, std::vector<XFile::Materi
mat->AddProperty<int>( &shadeMode, 1, AI_MATKEY_SHADING_MODEL);
// material colours
- // Unclear: there's no ambient colour, but emissive. What to put for ambient?
- // Probably nothing at all, let the user select a suitable default.
+ // Unclear: there's no ambient colour, but emissive. What to put for ambient?
+ // Probably nothing at all, let the user select a suitable default.
mat->AddProperty( &oldMat.mEmissive, 1, AI_MATKEY_COLOR_EMISSIVE);
mat->AddProperty( &oldMat.mDiffuse, 1, AI_MATKEY_COLOR_DIFFUSE);
mat->AddProperty( &oldMat.mSpecular, 1, AI_MATKEY_COLOR_SPECULAR);
@@ -639,36 +616,33 @@ void XFileImporter::ConvertMaterials( aiScene* pScene, std::vector<XFile::Materi
// texture, if there is one
- if (1 == oldMat.mTextures.size())
- {
+ if (1 == oldMat.mTextures.size() ) {
const XFile::TexEntry& otex = oldMat.mTextures.back();
- if (otex.mName.length())
- {
+ if (otex.mName.length()) {
// if there is only one texture assume it contains the diffuse color
aiString tex( otex.mName);
- if( otex.mIsNormalMap)
- mat->AddProperty( &tex, AI_MATKEY_TEXTURE_NORMALS(0));
- else
- mat->AddProperty( &tex, AI_MATKEY_TEXTURE_DIFFUSE(0));
+ if ( otex.mIsNormalMap ) {
+ mat->AddProperty( &tex, AI_MATKEY_TEXTURE_NORMALS( 0 ) );
+ } else {
+ mat->AddProperty( &tex, AI_MATKEY_TEXTURE_DIFFUSE( 0 ) );
+ }
}
- }
- else
- {
+ } else {
// Otherwise ... try to search for typical strings in the
// texture's file name like 'bump' or 'diffuse'
unsigned int iHM = 0,iNM = 0,iDM = 0,iSM = 0,iAM = 0,iEM = 0;
- for( unsigned int b = 0; b < oldMat.mTextures.size(); b++)
- {
+ for( unsigned int b = 0; b < oldMat.mTextures.size(); ++b ) {
const XFile::TexEntry& otex = oldMat.mTextures[b];
std::string sz = otex.mName;
- if (!sz.length())continue;
-
+ if ( !sz.length() ) {
+ continue;
+ }
// find the file name
- //const size_t iLen = sz.length();
std::string::size_type s = sz.find_last_of("\\/");
- if (std::string::npos == s)
+ if ( std::string::npos == s ) {
s = 0;
+ }
// cut off the file extension
std::string::size_type sExt = sz.find_last_of('.');
@@ -677,36 +651,27 @@ void XFileImporter::ConvertMaterials( aiScene* pScene, std::vector<XFile::Materi
}
// convert to lower case for easier comparison
- for( unsigned int c = 0; c < sz.length(); c++)
- if( isalpha( sz[c]))
- sz[c] = tolower( sz[c]);
-
+ for ( unsigned int c = 0; c < sz.length(); ++c ) {
+ if ( isalpha( sz[ c ] ) ) {
+ sz[ c ] = tolower( sz[ c ] );
+ }
+ }
// Place texture filename property under the corresponding name
aiString tex( oldMat.mTextures[b].mName);
// bump map
- if (std::string::npos != sz.find("bump", s) || std::string::npos != sz.find("height", s))
- {
+ if (std::string::npos != sz.find("bump", s) || std::string::npos != sz.find("height", s)) {
mat->AddProperty( &tex, AI_MATKEY_TEXTURE_HEIGHT(iHM++));
- } else
- if (otex.mIsNormalMap || std::string::npos != sz.find( "normal", s) || std::string::npos != sz.find("nm", s))
- {
+ } else if (otex.mIsNormalMap || std::string::npos != sz.find( "normal", s) || std::string::npos != sz.find("nm", s)) {
mat->AddProperty( &tex, AI_MATKEY_TEXTURE_NORMALS(iNM++));
- } else
- if (std::string::npos != sz.find( "spec", s) || std::string::npos != sz.find( "glanz", s))
- {
+ } else if (std::string::npos != sz.find( "spec", s) || std::string::npos != sz.find( "glanz", s)) {
mat->AddProperty( &tex, AI_MATKEY_TEXTURE_SPECULAR(iSM++));
- } else
- if (std::string::npos != sz.find( "ambi", s) || std::string::npos != sz.find( "env", s))
- {
+ } else if (std::string::npos != sz.find( "ambi", s) || std::string::npos != sz.find( "env", s)) {
mat->AddProperty( &tex, AI_MATKEY_TEXTURE_AMBIENT(iAM++));
- } else
- if (std::string::npos != sz.find( "emissive", s) || std::string::npos != sz.find( "self", s))
- {
+ } else if (std::string::npos != sz.find( "emissive", s) || std::string::npos != sz.find( "self", s)) {
mat->AddProperty( &tex, AI_MATKEY_TEXTURE_EMISSIVE(iEM++));
- } else
- {
+ } else {
// Assume it is a diffuse texture
mat->AddProperty( &tex, AI_MATKEY_TEXTURE_DIFFUSE(iDM++));
}
diff --git a/code/XFileParser.cpp b/code/XFileParser.cpp
index 4017ed596..bda59ba59 100644
--- a/code/XFileParser.cpp
+++ b/code/XFileParser.cpp
@@ -1460,7 +1460,6 @@ aiColor3D XFileParser::ReadRGB()
// ------------------------------------------------------------------------------------------------
// Throws an exception with a line number and the given text.
AI_WONT_RETURN void XFileParser::ThrowException( const std::string& pText) {
- delete mScene;
if ( mIsBinaryFormat ) {
throw DeadlyImportError( pText );
} else {