From 206004c7d6e26bf116ebe6132380f942f2d2e832 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 25 Feb 2019 22:06:24 +0100 Subject: [PATCH 1/5] introduce simple skin-test +some findings. --- code/FBXConverter.cpp | 2 +- code/FBXDocument.cpp | 9 +- code/FBXDocument.h | 121 ++++++-------- code/FBXDocumentUtil.h | 14 +- code/FindInvalidDataProcess.cpp | 10 +- include/assimp/ai_assert.h | 7 +- include/assimp/mesh.h | 14 +- test/models/PLY/cube_test.ply | 24 +++ .../models/glTF2/simple_skin/simple_skin.gltf | 148 ++++++++++++++++++ test/unit/utglTF2ImportExport.cpp | 5 + 10 files changed, 251 insertions(+), 103 deletions(-) create mode 100644 test/models/PLY/cube_test.ply create mode 100644 test/models/glTF2/simple_skin/simple_skin.gltf diff --git a/code/FBXConverter.cpp b/code/FBXConverter.cpp index d88a3cacd..c9f8a08d9 100644 --- a/code/FBXConverter.cpp +++ b/code/FBXConverter.cpp @@ -1151,7 +1151,7 @@ namespace Assimp { } } } - size_t numAnimMeshes = animMeshes.size(); + const size_t numAnimMeshes = animMeshes.size(); if (numAnimMeshes > 0) { out_mesh->mNumAnimMeshes = static_cast(numAnimMeshes); out_mesh->mAnimMeshes = new aiAnimMesh*[numAnimMeshes]; diff --git a/code/FBXDocument.cpp b/code/FBXDocument.cpp index 2e0d00a79..1af08fe6d 100644 --- a/code/FBXDocument.cpp +++ b/code/FBXDocument.cpp @@ -69,8 +69,7 @@ LazyObject::LazyObject(uint64_t id, const Element& element, const Document& doc) : doc(doc) , element(element) , id(id) -, flags() -{ +, flags() { // empty } @@ -84,7 +83,7 @@ LazyObject::~LazyObject() const Object* LazyObject::Get(bool dieOnError) { if(IsBeingConstructed() || FailedToConstruct()) { - return NULL; + return nullptr; } if (object.get()) { @@ -553,7 +552,7 @@ const std::vector& Document::AnimationStacks() const LazyObject* Document::GetObject(uint64_t id) const { ObjectMap::const_iterator it = objects.find(id); - return it == objects.end() ? NULL : (*it).second; + return it == objects.end() ? nullptr : (*it).second; } #define MAX_CLASSNAMES 6 @@ -610,7 +609,7 @@ std::vector Document::GetConnectionsSequenced(uint64_t id, bo for (size_t i = 0; i < c; ++i) { ai_assert(classnames[i]); if(static_cast(std::distance(key.begin(),key.end())) == lengths[i] && !strncmp(classnames[i],obtype,lengths[i])) { - obtype = NULL; + obtype = nullptr; break; } } diff --git a/code/FBXDocument.h b/code/FBXDocument.h index cbc6d60cd..c849defdc 100644 --- a/code/FBXDocument.h +++ b/code/FBXDocument.h @@ -85,13 +85,11 @@ class Cluster; /** Represents a delay-parsed FBX objects. Many objects in the scene * are not needed by assimp, so it makes no sense to parse them * upfront. */ -class LazyObject -{ +class LazyObject { public: LazyObject(uint64_t id, const Element& element, const Document& doc); - ~LazyObject(); -public: + ~LazyObject(); const Object* Get(bool dieOnError = false); @@ -136,11 +134,8 @@ private: unsigned int flags; }; - - /** Base class for in-memory (DOM) representations of FBX objects */ -class Object -{ +class Object { public: Object(uint64_t id, const Element& element, const std::string& name); @@ -164,14 +159,12 @@ protected: const uint64_t id; }; - - /** DOM class for generic FBX NoteAttribute blocks. NoteAttribute's just hold a property table, * fixed members are added by deriving classes. */ -class NodeAttribute : public Object -{ +class NodeAttribute : public Object { public: NodeAttribute(uint64_t id, const Element& element, const Document& doc, const std::string& name); + virtual ~NodeAttribute(); const PropertyTable& Props() const { @@ -183,12 +176,11 @@ private: std::shared_ptr props; }; - /** DOM base class for FBX camera settings attached to a node */ -class CameraSwitcher : public NodeAttribute -{ +class CameraSwitcher : public NodeAttribute { public: CameraSwitcher(uint64_t id, const Element& element, const Document& doc, const std::string& name); + virtual ~CameraSwitcher(); int CameraID() const { @@ -209,7 +201,6 @@ private: std::string cameraIndexName; }; - #define fbx_stringize(a) #a #define fbx_simple_property(name, type, default_value) \ @@ -230,13 +221,12 @@ private: /** DOM base class for FBX cameras attached to a node */ -class Camera : public NodeAttribute -{ +class Camera : public NodeAttribute { public: Camera(uint64_t id, const Element& element, const Document& doc, const std::string& name); + virtual ~Camera(); -public: fbx_simple_property(Position, aiVector3D, aiVector3D(0,0,0)) fbx_simple_property(UpVector, aiVector3D, aiVector3D(0,1,0)) fbx_simple_property(InterestPosition, aiVector3D, aiVector3D(0,0,0)) @@ -256,33 +246,26 @@ public: fbx_simple_property(FocalLength, float, 1.0f) }; - /** DOM base class for FBX null markers attached to a node */ -class Null : public NodeAttribute -{ +class Null : public NodeAttribute { public: Null(uint64_t id, const Element& element, const Document& doc, const std::string& name); virtual ~Null(); }; - /** DOM base class for FBX limb node markers attached to a node */ -class LimbNode : public NodeAttribute -{ +class LimbNode : public NodeAttribute { public: LimbNode(uint64_t id, const Element& element, const Document& doc, const std::string& name); virtual ~LimbNode(); }; - /** DOM base class for FBX lights attached to a node */ -class Light : public NodeAttribute -{ +class Light : public NodeAttribute { public: Light(uint64_t id, const Element& element, const Document& doc, const std::string& name); virtual ~Light(); -public: enum Type { Type_Point, @@ -304,7 +287,6 @@ public: Decay_MAX // end-of-enum sentinel }; -public: fbx_simple_property(Color, aiVector3D, aiVector3D(1,1,1)) fbx_simple_enum_property(LightType, Type, 0) fbx_simple_property(CastLightOnObject, bool, false) @@ -338,10 +320,8 @@ public: fbx_simple_property(EnableBarnDoor, bool, true) }; - /** DOM base class for FBX models (even though its semantics are more "node" than "model" */ -class Model : public Object -{ +class Model : public Object { public: enum RotOrder { RotOrder_EulerXYZ = 0, @@ -356,7 +336,6 @@ public: RotOrder_MAX // end-of-enum sentinel }; - enum TransformInheritance { TransformInheritance_RrSs = 0, TransformInheritance_RSrs, @@ -490,13 +469,12 @@ private: }; /** DOM class for generic FBX textures */ -class Texture : public Object -{ +class Texture : public Object { public: Texture(uint64_t id, const Element& element, const Document& doc, const std::string& name); + virtual ~Texture(); -public: const std::string& Type() const { return type; } @@ -551,17 +529,15 @@ private: }; /** DOM class for layered FBX textures */ -class LayeredTexture : public Object -{ +class LayeredTexture : public Object { public: LayeredTexture(uint64_t id, const Element& element, const Document& doc, const std::string& name); virtual ~LayeredTexture(); - //Can only be called after construction of the layered texture object due to construction flag. + // Can only be called after construction of the layered texture object due to construction flag. void fillTexture(const Document& doc); - enum BlendMode - { + enum BlendMode { BlendMode_Translucent, BlendMode_Additive, BlendMode_Modulate, @@ -623,13 +599,12 @@ typedef std::fbx_unordered_map LayeredTextur /** DOM class for generic FBX videos */ -class Video : public Object -{ +class Video : public Object { public: Video(uint64_t id, const Element& element, const Document& doc, const std::string& name); + virtual ~Video(); -public: const std::string& Type() const { return type; } @@ -673,10 +648,10 @@ private: }; /** DOM class for generic FBX materials */ -class Material : public Object -{ +class Material : public Object { public: Material(uint64_t id, const Element& element, const Document& doc, const std::string& name); + virtual ~Material(); const std::string& GetShadingModel() const { @@ -713,8 +688,7 @@ typedef std::vector KeyTimeList; typedef std::vector KeyValueList; /** Represents a FBX animation curve (i.e. a 1-dimensional set of keyframes and values therefor) */ -class AnimationCurve : public Object -{ +class AnimationCurve : public Object { public: AnimationCurve(uint64_t id, const Element& element, const std::string& name, const Document& doc); virtual ~AnimationCurve(); @@ -725,14 +699,12 @@ public: return keys; } - /** get list of keyframe values. * Invariant: |GetKeys()| == |GetValues()| && |GetKeys()| > 0*/ const KeyValueList& GetValues() const { return values; } - const std::vector& GetAttributes() const { return attributes; } @@ -751,10 +723,8 @@ private: // property-name -> animation curve typedef std::map AnimationCurveMap; - /** Represents a FBX animation curve (i.e. a mapping from single animation curves to nodes) */ -class AnimationCurveNode : public Object -{ +class AnimationCurveNode : public Object { public: /* the optional white list specifies a list of property names for which the caller wants animations for. If the curve node does not match one of these, std::range_error @@ -804,8 +774,7 @@ private: typedef std::vector AnimationCurveNodeList; /** Represents a FBX animation layer (i.e. a list of node animations) */ -class AnimationLayer : public Object -{ +class AnimationLayer : public Object { public: AnimationLayer(uint64_t id, const Element& element, const std::string& name, const Document& doc); virtual ~AnimationLayer(); @@ -818,7 +787,7 @@ public: /* the optional white list specifies a list of property names for which the caller wants animations for. Curves not matching this list will not be added to the animation layer. */ - AnimationCurveNodeList Nodes(const char* const * target_prop_whitelist = NULL, size_t whitelist_size = 0) const; + AnimationCurveNodeList Nodes(const char* const * target_prop_whitelist = nullptr, size_t whitelist_size = 0) const; private: std::shared_ptr props; @@ -828,8 +797,7 @@ private: typedef std::vector AnimationLayerList; /** Represents a FBX animation stack (i.e. a list of animation layers) */ -class AnimationStack : public Object -{ +class AnimationStack : public Object { public: AnimationStack(uint64_t id, const Element& element, const std::string& name, const Document& doc); virtual ~AnimationStack(); @@ -855,8 +823,7 @@ private: /** DOM class for deformers */ -class Deformer : public Object -{ +class Deformer : public Object { public: Deformer(uint64_t id, const Element& element, const Document& doc, const std::string& name); virtual ~Deformer(); @@ -875,10 +842,10 @@ typedef std::vector WeightIndexArray; /** DOM class for BlendShapeChannel deformers */ -class BlendShapeChannel : public Deformer -{ +class BlendShapeChannel : public Deformer { public: BlendShapeChannel(uint64_t id, const Element& element, const Document& doc, const std::string& name); + virtual ~BlendShapeChannel(); float DeformPercent() const { @@ -892,6 +859,7 @@ public: const std::vector& GetShapeGeometries() const { return shapeGeometries; } + private: float percent; WeightArray fullWeights; @@ -899,10 +867,10 @@ private: }; /** DOM class for BlendShape deformers */ -class BlendShape : public Deformer -{ +class BlendShape : public Deformer { public: BlendShape(uint64_t id, const Element& element, const Document& doc, const std::string& name); + virtual ~BlendShape(); const std::vector& BlendShapeChannels() const { @@ -913,11 +881,11 @@ private: std::vector blendShapeChannels; }; -/** DOM class for skin deformer clusters (aka subdeformers) */ -class Cluster : public Deformer -{ +/** DOM class for skin deformer clusters (aka sub-deformers) */ +class Cluster : public Deformer { public: Cluster(uint64_t id, const Element& element, const Document& doc, const std::string& name); + virtual ~Cluster(); /** get the list of deformer weights associated with this cluster. @@ -958,10 +926,10 @@ private: }; /** DOM class for skin deformers */ -class Skin : public Deformer -{ +class Skin : public Deformer { public: Skin(uint64_t id, const Element& element, const Document& doc, const std::string& name); + virtual ~Skin(); float DeformAccuracy() const { @@ -978,10 +946,10 @@ private: }; /** Represents a link between two FBX objects. */ -class Connection -{ +class Connection { public: Connection(uint64_t insertionOrder, uint64_t src, uint64_t dest, const std::string& prop, const Document& doc); + ~Connection(); // note: a connection ensures that the source and dest objects exist, but @@ -1006,7 +974,7 @@ public: } int CompareTo(const Connection* c) const { - ai_assert( NULL != c ); + ai_assert( nullptr != c ); // note: can't subtract because this would overflow uint64_t if(InsertionOrder() > c->InsertionOrder()) { @@ -1019,7 +987,7 @@ public: } bool Compare(const Connection* c) const { - ai_assert( NULL != c ); + ai_assert( nullptr != c ); return InsertionOrder() < c->InsertionOrder(); } @@ -1047,6 +1015,7 @@ typedef std::multimap ConnectionMap; class FileGlobalSettings { public: FileGlobalSettings(const Document& doc, std::shared_ptr props); + ~FileGlobalSettings(); const PropertyTable& Props() const { @@ -1103,10 +1072,10 @@ private: }; /** DOM root for a FBX file */ -class Document -{ +class Document { public: Document(const Parser& parser, const ImportSettings& settings); + ~Document(); LazyObject* GetObject(uint64_t id) const; diff --git a/code/FBXDocumentUtil.h b/code/FBXDocumentUtil.h index c0435a663..2450109e5 100644 --- a/code/FBXDocumentUtil.h +++ b/code/FBXDocumentUtil.h @@ -56,7 +56,6 @@ namespace Assimp { namespace FBX { namespace Util { - /* DOM/Parse error reporting - does not return */ AI_WONT_RETURN void DOMError(const std::string& message, const Token& token) AI_WONT_RETURN_SUFFIX; AI_WONT_RETURN void DOMError(const std::string& message, const Element* element = NULL) AI_WONT_RETURN_SUFFIX; @@ -73,28 +72,28 @@ std::shared_ptr GetPropertyTable(const Document& doc, const Scope& sc, bool no_warn = false); - // ------------------------------------------------------------------------------------------------ template -inline const T* ProcessSimpleConnection(const Connection& con, +inline +const T* ProcessSimpleConnection(const Connection& con, bool is_object_property_conn, const char* name, const Element& element, - const char** propNameOut = NULL) + const char** propNameOut = nullptr) { if (is_object_property_conn && !con.PropertyName().length()) { DOMWarning("expected incoming " + std::string(name) + " link to be an object-object connection, ignoring", &element ); - return NULL; + return nullptr; } else if (!is_object_property_conn && con.PropertyName().length()) { DOMWarning("expected incoming " + std::string(name) + " link to be an object-property connection, ignoring", &element ); - return NULL; + return nullptr; } if(is_object_property_conn && propNameOut) { @@ -108,13 +107,12 @@ inline const T* ProcessSimpleConnection(const Connection& con, DOMWarning("failed to read source object for incoming " + std::string(name) + " link, ignoring", &element); - return NULL; + return nullptr; } return dynamic_cast(ob); } - } //!Util } //!FBX } //!Assimp diff --git a/code/FindInvalidDataProcess.cpp b/code/FindInvalidDataProcess.cpp index c99a98865..433f04244 100644 --- a/code/FindInvalidDataProcess.cpp +++ b/code/FindInvalidDataProcess.cpp @@ -282,9 +282,11 @@ void FindInvalidDataProcess::ProcessAnimation (aiAnimation* anim) { // ------------------------------------------------------------------------------------------------ void FindInvalidDataProcess::ProcessAnimationChannel (aiNodeAnim* anim) { - ai_assert( 0 != anim->mPositionKeys ); - ai_assert( 0 != anim->mRotationKeys ); - ai_assert( 0 != anim->mScalingKeys ); + ai_assert( nullptr != anim ); + if (anim->mNumPositionKeys == 0 && anim->mNumRotationKeys == 0 && anim->mNumScalingKeys == 0) { + ai_assert_entry(); + return; + } // Check whether all values in a tracks are identical - in this case // we can remove al keys except one. @@ -328,7 +330,7 @@ void FindInvalidDataProcess::ProcessAnimationChannel (aiNodeAnim* anim) { // ------------------------------------------------------------------------------------------------ // Search a mesh for invalid contents -int FindInvalidDataProcess::ProcessMesh (aiMesh* pMesh) +int FindInvalidDataProcess::ProcessMesh(aiMesh* pMesh) { bool ret = false; std::vector dirtyMask(pMesh->mNumVertices, pMesh->mNumFaces != 0); diff --git a/include/assimp/ai_assert.h b/include/assimp/ai_assert.h index daae23454..e5de5d3f3 100644 --- a/include/assimp/ai_assert.h +++ b/include/assimp/ai_assert.h @@ -46,9 +46,12 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #ifdef ASSIMP_BUILD_DEBUG # include -# define ai_assert(expression) assert(expression) +# define ai_assert(expression) assert( expression ) +# define ai_assert_entry() assert( false ) #else # define ai_assert(expression) -#endif // +# define ai_assert_entry() +#endif // ASSIMP_BUILD_DEBUG #endif // AI_ASSERT_H_INC + diff --git a/include/assimp/mesh.h b/include/assimp/mesh.h index 8d539c59e..36f3ed2af 100644 --- a/include/assimp/mesh.h +++ b/include/assimp/mesh.h @@ -469,10 +469,10 @@ struct aiAnimMesh { // fixme consider moving this to the ctor initializer list as well for( unsigned int a = 0; a < AI_MAX_NUMBER_OF_TEXTURECOORDS; a++){ - mTextureCoords[a] = NULL; + mTextureCoords[a] = nullptr; } for( unsigned int a = 0; a < AI_MAX_NUMBER_OF_COLOR_SETS; a++) { - mColors[a] = NULL; + mColors[a] = nullptr; } } @@ -493,34 +493,34 @@ struct aiAnimMesh /** Check whether the anim mesh overrides the vertex positions * of its host mesh*/ bool HasPositions() const { - return mVertices != NULL; + return mVertices != nullptr; } /** Check whether the anim mesh overrides the vertex normals * of its host mesh*/ bool HasNormals() const { - return mNormals != NULL; + return mNormals != nullptr; } /** Check whether the anim mesh overrides the vertex tangents * and bitangents of its host mesh. As for aiMesh, * tangents and bitangents always go together. */ bool HasTangentsAndBitangents() const { - return mTangents != NULL; + return mTangents != nullptr; } /** Check whether the anim mesh overrides a particular * set of vertex colors on his host mesh. * @param pIndex 0= AI_MAX_NUMBER_OF_COLOR_SETS ? false : mColors[pIndex] != NULL; + return pIndex >= AI_MAX_NUMBER_OF_COLOR_SETS ? false : mColors[pIndex] != nullptr; } /** Check whether the anim mesh overrides a particular * set of texture coordinates on his host mesh. * @param pIndex 0= AI_MAX_NUMBER_OF_TEXTURECOORDS ? false : mTextureCoords[pIndex] != NULL; + return pIndex >= AI_MAX_NUMBER_OF_TEXTURECOORDS ? false : mTextureCoords[pIndex] != nullptr; } #endif diff --git a/test/models/PLY/cube_test.ply b/test/models/PLY/cube_test.ply new file mode 100644 index 000000000..0266df0ee --- /dev/null +++ b/test/models/PLY/cube_test.ply @@ -0,0 +1,24 @@ +ply +format ascii 1.0 +comment Created by Open Asset Import Library - http://assimp.sf.net (v4.1.4208963464) +element vertex 8 +property float x +property float y +property float z +element face 6 +property list uchar int vertex_index +end_header +0 0 0 +0 0 1 +0 1 1 +0 1 0 +1 0 0 +1 0 1 +1 1 1 +1 1 0 +4 0 1 2 3 +4 7 6 5 4 +4 0 4 5 1 +4 1 5 6 2 +4 2 6 7 3 +4 3 7 4 0 diff --git a/test/models/glTF2/simple_skin/simple_skin.gltf b/test/models/glTF2/simple_skin/simple_skin.gltf new file mode 100644 index 000000000..e075bb34c --- /dev/null +++ b/test/models/glTF2/simple_skin/simple_skin.gltf @@ -0,0 +1,148 @@ +{ + "scenes" : [ { + "nodes" : [ 0 ] + } ], + + "nodes" : [ { + "skin" : 0, + "mesh" : 0, + "children" : [ 1 ] + }, { + "children" : [ 2 ], + "translation" : [ 0.0, 1.0, 0.0 ] + }, { + "rotation" : [ 0.0, 0.0, 0.0, 1.0 ] + } ], + + "meshes" : [ { + "primitives" : [ { + "attributes" : { + "POSITION" : 1, + "JOINTS_0" : 2, + "WEIGHTS_0" : 3 + }, + "indices" : 0 + } ] + } ], + + "skins" : [ { + "inverseBindMatrices" : 4, + "joints" : [ 1, 2 ] + } ], + + "animations" : [ { + "channels" : [ { + "sampler" : 0, + "target" : { + "node" : 2, + "path" : "rotation" + } + } ], + "samplers" : [ { + "input" : 5, + "interpolation" : "LINEAR", + "output" : 6 + } ] + } ], + + "buffers" : [ { + "uri" : "data:application/gltf-buffer;base64,AAABAAMAAAADAAIAAgADAAUAAgAFAAQABAAFAAcABAAHAAYABgAHAAkABgAJAAgAAAAAAAAAAAAAAAAAAACAPwAAAAAAAAAAAAAAAAAAAD8AAAAAAACAPwAAAD8AAAAAAAAAAAAAgD8AAAAAAACAPwAAgD8AAAAAAAAAAAAAwD8AAAAAAACAPwAAwD8AAAAAAAAAAAAAAEAAAAAAAACAPwAAAEAAAAAA", + "byteLength" : 168 + }, { + "uri" : "data:application/gltf-buffer;base64,AAABAAAAAAAAAAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAEAAAAAAAAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAEAAAAAAAAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAEAAAAAAAAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAAAAgD8AAAAAAAAAAAAAAAAAAIA/AAAAAAAAAAAAAAAAAABAPwAAgD4AAAAAAAAAAAAAQD8AAIA+AAAAAAAAAAAAAAA/AAAAPwAAAAAAAAAAAAAAPwAAAD8AAAAAAAAAAAAAgD4AAEA/AAAAAAAAAAAAAIA+AABAPwAAAAAAAAAAAAAAAAAAgD8AAAAAAAAAAAAAAAAAAIA/AAAAAAAAAAA=", + "byteLength" : 320 + }, { + "uri" : "data:application/gltf-buffer;base64,AACAPwAAAAAAAAAAAAAAAAAAAAAAAIA/AAAAAAAAAAAAAAAAAAAAAAAAgD8AAAAAAAAAvwAAgL8AAAAAAACAPwAAgD8AAAAAAAAAAAAAAAAAAAAAAACAPwAAAAAAAAAAAAAAAAAAAAAAAIA/AAAAAAAAAL8AAIC/AAAAAAAAgD8=", + "byteLength" : 128 + }, { + "uri" : "data:application/gltf-buffer;base64,AAAAAAAAAD8AAIA/AADAPwAAAEAAACBAAABAQAAAYEAAAIBAAACQQAAAoEAAALBAAAAAAAAAAAAAAAAAAACAPwAAAAAAAAAAkxjEPkSLbD8AAAAAAAAAAPT9ND/0/TQ/AAAAAAAAAAD0/TQ/9P00PwAAAAAAAAAAkxjEPkSLbD8AAAAAAAAAAAAAAAAAAIA/AAAAAAAAAAAAAAAAAACAPwAAAAAAAAAAkxjEvkSLbD8AAAAAAAAAAPT9NL/0/TQ/AAAAAAAAAAD0/TS/9P00PwAAAAAAAAAAkxjEvkSLbD8AAAAAAAAAAAAAAAAAAIA/", + "byteLength" : 240 + } ], + + "bufferViews" : [ { + "buffer" : 0, + "byteOffset" : 0, + "byteLength" : 48, + "target" : 34963 + }, { + "buffer" : 0, + "byteOffset" : 48, + "byteLength" : 120, + "target" : 34962 + }, { + "buffer" : 1, + "byteOffset" : 0, + "byteLength" : 320, + "byteStride" : 16 + }, { + "buffer" : 2, + "byteOffset" : 0, + "byteLength" : 128 + }, { + "buffer" : 3, + "byteOffset" : 0, + "byteLength" : 240 + } ], + + "accessors" : [ { + "bufferView" : 0, + "byteOffset" : 0, + "componentType" : 5123, + "count" : 24, + "type" : "SCALAR", + "max" : [ 9 ], + "min" : [ 0 ] + }, { + "bufferView" : 1, + "byteOffset" : 0, + "componentType" : 5126, + "count" : 10, + "type" : "VEC3", + "max" : [ 1.0, 2.0, 0.0 ], + "min" : [ 0.0, 0.0, 0.0 ] + }, { + "bufferView" : 2, + "byteOffset" : 0, + "componentType" : 5123, + "count" : 10, + "type" : "VEC4", + "max" : [ 0, 1, 0, 0 ], + "min" : [ 0, 1, 0, 0 ] + }, { + "bufferView" : 2, + "byteOffset" : 160, + "componentType" : 5126, + "count" : 10, + "type" : "VEC4", + "max" : [ 1.0, 1.0, 0.0, 0.0 ], + "min" : [ 0.0, 0.0, 0.0, 0.0 ] + }, { + "bufferView" : 3, + "byteOffset" : 0, + "componentType" : 5126, + "count" : 2, + "type" : "MAT4", + "max" : [ 1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, -0.5, -1.0, 0.0, 1.0 ], + "min" : [ 1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, -0.5, -1.0, 0.0, 1.0 ] + }, { + "bufferView" : 4, + "byteOffset" : 0, + "componentType" : 5126, + "count" : 12, + "type" : "SCALAR", + "max" : [ 5.5 ], + "min" : [ 0.0 ] + }, { + "bufferView" : 4, + "byteOffset" : 48, + "componentType" : 5126, + "count" : 12, + "type" : "VEC4", + "max" : [ 0.0, 0.0, 0.707, 1.0 ], + "min" : [ 0.0, 0.0, -0.707, 0.707 ] + } ], + + "asset" : { + "version" : "2.0" + } +} \ No newline at end of file diff --git a/test/unit/utglTF2ImportExport.cpp b/test/unit/utglTF2ImportExport.cpp index 3ad02645d..05dba1f1d 100644 --- a/test/unit/utglTF2ImportExport.cpp +++ b/test/unit/utglTF2ImportExport.cpp @@ -376,6 +376,11 @@ TEST_F(utglTF2ImportExport, importglTF2FromMemory) { EXPECT_EQ( nullptr, Scene );*/ } +TEST_F( utglTF2ImportExport, bug_import_simple_skin ) { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile( ASSIMP_TEST_MODELS_DIR "/glTF2/simple_skin/simple_skin.gltf", aiProcess_ValidateDataStructure ); + +} #ifndef ASSIMP_BUILD_NO_EXPORT TEST_F( utglTF2ImportExport, exportglTF2FromFileTest ) { EXPECT_TRUE( exporterTest() ); From 262bbc0d5eda11a1efecf4797f82937b5e2a3880 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 25 Feb 2019 23:19:21 +0100 Subject: [PATCH 2/5] fix compiler warnings. --- test/unit/utglTF2ImportExport.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unit/utglTF2ImportExport.cpp b/test/unit/utglTF2ImportExport.cpp index 05dba1f1d..bac76473d 100644 --- a/test/unit/utglTF2ImportExport.cpp +++ b/test/unit/utglTF2ImportExport.cpp @@ -379,8 +379,9 @@ TEST_F(utglTF2ImportExport, importglTF2FromMemory) { TEST_F( utglTF2ImportExport, bug_import_simple_skin ) { Assimp::Importer importer; const aiScene *scene = importer.ReadFile( ASSIMP_TEST_MODELS_DIR "/glTF2/simple_skin/simple_skin.gltf", aiProcess_ValidateDataStructure ); - + EXPECT_NE( nullptr, scene ); } + #ifndef ASSIMP_BUILD_NO_EXPORT TEST_F( utglTF2ImportExport, exportglTF2FromFileTest ) { EXPECT_TRUE( exporterTest() ); From 7b01ff6b8d4a84883d97b438fb6ad3f6d267922b Mon Sep 17 00:00:00 2001 From: Vitaly Ovchinnikov Date: Wed, 27 Feb 2019 12:15:03 +1300 Subject: [PATCH 3/5] ignoring invalid normals and uvs indices instead of canceling the import completely --- code/ObjFileImporter.cpp | 39 +++++++++++++++++++++++++-------- test/unit/utObjImportExport.cpp | 14 ++++++++++++ 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/code/ObjFileImporter.cpp b/code/ObjFileImporter.cpp index 782b62a73..814180c81 100644 --- a/code/ObjFileImporter.cpp +++ b/code/ObjFileImporter.cpp @@ -447,6 +447,7 @@ void ObjFileImporter::createVertexArray(const ObjFile::Model* pModel, } // Copy vertices, normals and textures into aiMesh instance + bool normalsok = true, uvok = true; unsigned int newIndex = 0, outIndex = 0; for ( size_t index=0; index < pObjMesh->m_Faces.size(); index++ ) { // Get source face @@ -466,12 +467,16 @@ void ObjFileImporter::createVertexArray(const ObjFile::Model* pModel, pMesh->mVertices[ newIndex ] = pModel->m_Vertices[ vertex ]; // Copy all normals - if ( !pModel->m_Normals.empty() && vertexIndex < pSourceFace->m_normals.size()) { + if ( normalsok && !pModel->m_Normals.empty() && vertexIndex < pSourceFace->m_normals.size()) { const unsigned int normal = pSourceFace->m_normals.at( vertexIndex ); - if ( normal >= pModel->m_Normals.size() ) { - throw DeadlyImportError( "OBJ: vertex normal index out of range" ); + if ( normal >= pModel->m_Normals.size() ) + { + normalsok = false; + } + else + { + pMesh->mNormals[ newIndex ] = pModel->m_Normals[ normal ]; } - pMesh->mNormals[ newIndex ] = pModel->m_Normals[ normal ]; } // Copy all vertex colors @@ -482,15 +487,19 @@ void ObjFileImporter::createVertexArray(const ObjFile::Model* pModel, } // Copy all texture coordinates - if ( !pModel->m_TextureCoord.empty() && vertexIndex < pSourceFace->m_texturCoords.size()) + if ( uvok && !pModel->m_TextureCoord.empty() && vertexIndex < pSourceFace->m_texturCoords.size()) { const unsigned int tex = pSourceFace->m_texturCoords.at( vertexIndex ); if ( tex >= pModel->m_TextureCoord.size() ) - throw DeadlyImportError("OBJ: texture coordinate index out of range"); - - const aiVector3D &coord3d = pModel->m_TextureCoord[ tex ]; - pMesh->mTextureCoords[ 0 ][ newIndex ] = aiVector3D( coord3d.x, coord3d.y, coord3d.z ); + { + uvok = false; + } + else + { + const aiVector3D &coord3d = pModel->m_TextureCoord[ tex ]; + pMesh->mTextureCoords[ 0 ][ newIndex ] = aiVector3D( coord3d.x, coord3d.y, coord3d.z ); + } } // Get destination face @@ -534,6 +543,18 @@ void ObjFileImporter::createVertexArray(const ObjFile::Model* pModel, ++newIndex; } } + + if (!normalsok) + { + delete [] pMesh->mNormals; + pMesh->mNormals = nullptr; + } + + if (!uvok) + { + delete [] pMesh->mTextureCoords[0]; + pMesh->mTextureCoords[0] = nullptr; + } } // ------------------------------------------------------------------------------------------------ diff --git a/test/unit/utObjImportExport.cpp b/test/unit/utObjImportExport.cpp index 0f715af14..2d1933094 100644 --- a/test/unit/utObjImportExport.cpp +++ b/test/unit/utObjImportExport.cpp @@ -377,6 +377,20 @@ TEST_F(utObjImportExport, 0based_array_Test) { EXPECT_EQ(nullptr, scene); } +TEST_F(utObjImportExport, invalid_normals_uvs) { + static const char *ObjModel = + "v -0.500000 0.000000 0.400000\n" + "v -0.500000 0.000000 -0.800000\n" + "v -0.500000 1.000000 -0.800000\n" + "vt 0 0\n" + "vn 0 1 0\n" + "f 1/1/1 1/1/1 2/2/2\nB"; + + Assimp::Importer myImporter; + const aiScene *scene = myImporter.ReadFileFromMemory(ObjModel, strlen(ObjModel), 0); + EXPECT_NE(nullptr, scene); +} + TEST_F( utObjImportExport, mtllib_after_g ) { ::Assimp::Importer importer; const aiScene *scene = importer.ReadFile( ASSIMP_TEST_MODELS_DIR "/OBJ/cube_mtllib_after_g.obj", aiProcess_ValidateDataStructure ); From e9eda062a17b1804a05134fe69a135a996413415 Mon Sep 17 00:00:00 2001 From: Lawrence Kok Date: Fri, 1 Mar 2019 10:43:18 +0700 Subject: [PATCH 4/5] In ColladaLoader in version 3.2.0 (release est. 2015) the name assignment was based on name attribute (xml) with optional fallbacks to id and sid. Over time this was changed to fix a bug and support for names was removed, only to be later (re)added using an optional `useColladaName` setting. We discovered a problem that with when using the name based assignment it doesn't assign automatically generated names like what it did in the logic in v3.2.0 and on the id based approach on current master. We discovered this causes problems with matching the aiCamera and aiLight. So we rectified this problem by auto generating names also on the name based method. --- code/ColladaLoader.cpp | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/code/ColladaLoader.cpp b/code/ColladaLoader.cpp index 2a3412ac2..6837dca4a 100644 --- a/code/ColladaLoader.cpp +++ b/code/ColladaLoader.cpp @@ -1926,21 +1926,28 @@ const Collada::Node* ColladaLoader::FindNodeBySID( const Collada::Node* pNode, c std::string ColladaLoader::FindNameForNode( const Collada::Node* pNode) { // If explicitly requested, just use the collada name. - if (useColladaName) { - return pNode->mName; - } - - // Now setup the name of the assimp node. The collada name might not be - // unique, so we use the collada ID. - if (!pNode->mID.empty()) - return pNode->mID; - else if (!pNode->mSID.empty()) - return pNode->mSID; - else + if (useColladaName) { - // No need to worry. Unnamed nodes are no problem at all, except - // if cameras or lights need to be assigned to them. - return format() << "$ColladaAutoName$_" << mNodeNameCounter++; + if (!pNode->mName.empty()) { + return pNode->mName; + } else { + return format() << "$ColladaAutoName$_" << mNodeNameCounter++; + } + } + else + { + // Now setup the name of the assimp node. The collada name might not be + // unique, so we use the collada ID. + if (!pNode->mID.empty()) + return pNode->mID; + else if (!pNode->mSID.empty()) + return pNode->mSID; + else + { + // No need to worry. Unnamed nodes are no problem at all, except + // if cameras or lights need to be assigned to them. + return format() << "$ColladaAutoName$_" << mNodeNameCounter++; + } } } From 99c3697d1230dbff354f2507174f76444f4738f0 Mon Sep 17 00:00:00 2001 From: Andy Maloney Date: Fri, 1 Mar 2019 12:38:59 -0500 Subject: [PATCH 5/5] Ensure our include directories get added in the correct order If you have assimp installed already and in the include path (e.g. I have it via homebrew), it can pick up the wrong headers. This forces the include order so our local ones are found first when building assimp. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 14c65c188..6417053ac 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -198,7 +198,7 @@ CONFIGURE_FILE( ${CMAKE_CURRENT_BINARY_DIR}/include/assimp/config.h ) -INCLUDE_DIRECTORIES( +INCLUDE_DIRECTORIES( BEFORE ./ include ${CMAKE_CURRENT_BINARY_DIR}