Просмотр исходного кода

Fix incorrect xml-parsing in collada importer.

Kim Kulling 4 лет назад
Родитель
Сommit
fa2354ebc3

+ 2 - 1
code/AssetLib/Collada/ColladaHelper.h

@@ -206,7 +206,8 @@ struct SemanticMappingTable {
     std::string mMatName;
 
     /// List of semantic map commands, grouped by effect semantic name
-    std::map<std::string, InputSemanticMapEntry> mMap;
+    using InputSemanticMap = std::map<std::string, InputSemanticMapEntry>;
+    InputSemanticMap mMap;
 
     /// For std::find
     bool operator==(const std::string &s) const {

+ 4 - 1
code/AssetLib/Collada/ColladaLoader.cpp

@@ -327,7 +327,7 @@ void ColladaLoader::ResolveNodeInstances(const ColladaParser &pParser, const Col
 // ------------------------------------------------------------------------------------------------
 // Resolve UV channels
 void ColladaLoader::ApplyVertexToEffectSemanticMapping(Collada::Sampler &sampler, const Collada::SemanticMappingTable &table) {
-    std::map<std::string, Collada::InputSemanticMapEntry>::const_iterator it = table.mMap.find(sampler.mUVChannel);
+    Collada::SemanticMappingTable::InputSemanticMap::const_iterator it = table.mMap.find(sampler.mUVChannel);
     if (it == table.mMap.end()) {
         return;
     }
@@ -692,6 +692,9 @@ aiMesh *ColladaLoader::CreateMesh(const ColladaParser &pParser, const Collada::M
     for (std::map<std::string, Collada::Controller>::const_iterator it = pParser.mControllerLibrary.begin();
             it != pParser.mControllerLibrary.end(); ++it) {
         const Collada::Controller &c = it->second;
+        /*if (c.mMeshId.empty()) {
+            continue;
+        }*/
         const Collada::Mesh *baseMesh = pParser.ResolveLibraryReference(pParser.mMeshLibrary, c.mMeshId);
 
         if (c.mType == Collada::Morph && baseMesh->mName == pSrcMesh->mName) {

+ 63 - 69
code/AssetLib/Collada/ColladaLoader.h

@@ -45,8 +45,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #ifndef AI_COLLADALOADER_H_INC
 #define AI_COLLADALOADER_H_INC
 
-#include <assimp/BaseImporter.h>
 #include "ColladaParser.h"
+#include <assimp/BaseImporter.h>
 
 struct aiNode;
 struct aiCamera;
@@ -54,28 +54,24 @@ struct aiLight;
 struct aiTexture;
 struct aiAnimation;
 
-namespace Assimp
-{
+namespace Assimp {
 
-struct ColladaMeshIndex
-{
+struct ColladaMeshIndex {
     std::string mMeshID;
     size_t mSubMesh;
     std::string mMaterial;
-    ColladaMeshIndex( const std::string& pMeshID, size_t pSubMesh, const std::string& pMaterial)
-        : mMeshID( pMeshID), mSubMesh( pSubMesh), mMaterial( pMaterial)
-    {   }
-
-    bool operator < (const ColladaMeshIndex& p) const
-    {
-        if( mMeshID == p.mMeshID)
-        {
-            if( mSubMesh == p.mSubMesh)
+    ColladaMeshIndex(const std::string &pMeshID, size_t pSubMesh, const std::string &pMaterial) :
+            mMeshID(pMeshID), mSubMesh(pSubMesh), mMaterial(pMaterial) {
+        ai_assert(!pMeshID.empty());
+    }
+
+    bool operator<(const ColladaMeshIndex &p) const {
+        if (mMeshID == p.mMeshID) {
+            if (mSubMesh == p.mSubMesh)
                 return mMaterial < p.mMaterial;
             else
                 return mSubMesh < p.mSubMesh;
-        } else
-        {
+        } else {
             return mMeshID < p.mMeshID;
         }
     }
@@ -84,105 +80,103 @@ struct ColladaMeshIndex
 /** Loader class to read Collada scenes. Collada is over-engineered to death, with every new iteration bringing
  * more useless stuff, so I limited the data to what I think is useful for games.
 */
-class ColladaLoader : public BaseImporter
-{
+class ColladaLoader : public BaseImporter {
 public:
     ColladaLoader();
-    ~ColladaLoader();
-
+    ~ColladaLoader() override;
 
 public:
     /** Returns whether the class can handle the format of the given file.
      * See BaseImporter::CanRead() for details. */
-    bool CanRead(const std::string& pFile, IOSystem* pIOHandler, bool checkSig) const override;
+    bool CanRead(const std::string &pFile, IOSystem *pIOHandler, bool checkSig) const override;
 
 protected:
     /** Return importer meta information.
      * See #BaseImporter::GetInfo for the details
      */
-    const aiImporterDesc* GetInfo () const override;
+    const aiImporterDesc *GetInfo() const override;
 
-    void SetupProperties(const Importer* pImp) override;
+    void SetupProperties(const Importer *pImp) override;
 
     /** Imports the given file into the given scene structure.
      * See BaseImporter::InternReadFile() for details
      */
-    void InternReadFile( const std::string& pFile, aiScene* pScene, IOSystem* pIOHandler) override;
+    void InternReadFile(const std::string &pFile, aiScene *pScene, IOSystem *pIOHandler) override;
 
     /** Recursively constructs a scene node for the given parser node and returns it. */
-    aiNode* BuildHierarchy( const ColladaParser& pParser, const Collada::Node* pNode);
+    aiNode *BuildHierarchy(const ColladaParser &pParser, const Collada::Node *pNode);
 
     /** Resolve node instances */
-    void ResolveNodeInstances( const ColladaParser& pParser, const Collada::Node* pNode,
-        std::vector<const Collada::Node*>& resolved);
+    void ResolveNodeInstances(const ColladaParser &pParser, const Collada::Node *pNode,
+            std::vector<const Collada::Node *> &resolved);
 
     /** Builds meshes for the given node and references them */
-    void BuildMeshesForNode( const ColladaParser& pParser, const Collada::Node* pNode,
-        aiNode* pTarget);
-		
-    aiMesh *findMesh(const std::string& meshid);
+    void BuildMeshesForNode(const ColladaParser &pParser, const Collada::Node *pNode,
+            aiNode *pTarget);
+
+    aiMesh *findMesh(const std::string &meshid);
 
     /** Creates a mesh for the given ColladaMesh face subset and returns the newly created mesh */
-    aiMesh* CreateMesh( const ColladaParser& pParser, const Collada::Mesh* pSrcMesh, const Collada::SubMesh& pSubMesh,
-        const Collada::Controller* pSrcController, size_t pStartVertex, size_t pStartFace);
+    aiMesh *CreateMesh(const ColladaParser &pParser, const Collada::Mesh *pSrcMesh, const Collada::SubMesh &pSubMesh,
+            const Collada::Controller *pSrcController, size_t pStartVertex, size_t pStartFace);
 
     /** Builds cameras for the given node and references them */
-    void BuildCamerasForNode( const ColladaParser& pParser, const Collada::Node* pNode,
-        aiNode* pTarget);
+    void BuildCamerasForNode(const ColladaParser &pParser, const Collada::Node *pNode,
+            aiNode *pTarget);
 
     /** Builds lights for the given node and references them */
-    void BuildLightsForNode( const ColladaParser& pParser, const Collada::Node* pNode,
-        aiNode* pTarget);
+    void BuildLightsForNode(const ColladaParser &pParser, const Collada::Node *pNode,
+            aiNode *pTarget);
 
     /** Stores all meshes in the given scene */
-    void StoreSceneMeshes( aiScene* pScene);
+    void StoreSceneMeshes(aiScene *pScene);
 
     /** Stores all materials in the given scene */
-    void StoreSceneMaterials( aiScene* pScene);
+    void StoreSceneMaterials(aiScene *pScene);
 
     /** Stores all lights in the given scene */
-    void StoreSceneLights( aiScene* pScene);
+    void StoreSceneLights(aiScene *pScene);
 
     /** Stores all cameras in the given scene */
-    void StoreSceneCameras( aiScene* pScene);
+    void StoreSceneCameras(aiScene *pScene);
 
     /** Stores all textures in the given scene */
-    void StoreSceneTextures( aiScene* pScene);
+    void StoreSceneTextures(aiScene *pScene);
 
     /** Stores all animations
      * @param pScene target scene to store the anims
      */
-    void StoreAnimations( aiScene* pScene, const ColladaParser& pParser);
+    void StoreAnimations(aiScene *pScene, const ColladaParser &pParser);
 
     /** Stores all animations for the given source anim and its nested child animations
      * @param pScene target scene to store the anims
      * @param pSrcAnim the source animation to process
      * @param pPrefix Prefix to the name in case of nested animations
      */
-    void StoreAnimations( aiScene* pScene, const ColladaParser& pParser, const Collada::Animation* pSrcAnim, const std::string& pPrefix);
+    void StoreAnimations(aiScene *pScene, const ColladaParser &pParser, const Collada::Animation *pSrcAnim, const std::string &pPrefix);
 
     /** Constructs the animation for the given source anim */
-    void CreateAnimation( aiScene* pScene, const ColladaParser& pParser, const Collada::Animation* pSrcAnim, const std::string& pName);
+    void CreateAnimation(aiScene *pScene, const ColladaParser &pParser, const Collada::Animation *pSrcAnim, const std::string &pName);
 
     /** Constructs materials from the collada material definitions */
-    void BuildMaterials( ColladaParser& pParser, aiScene* pScene);
+    void BuildMaterials(ColladaParser &pParser, aiScene *pScene);
 
     /** Fill materials from the collada material definitions */
-    void FillMaterials( const ColladaParser& pParser, aiScene* pScene);
+    void FillMaterials(const ColladaParser &pParser, aiScene *pScene);
 
     /** Resolve UV channel mappings*/
-    void ApplyVertexToEffectSemanticMapping(Collada::Sampler& sampler,
-        const Collada::SemanticMappingTable& table);
+    void ApplyVertexToEffectSemanticMapping(Collada::Sampler &sampler,
+            const Collada::SemanticMappingTable &table);
 
     /** Add a texture and all of its sampling properties to a material*/
-    void AddTexture ( aiMaterial& mat, const ColladaParser& pParser,
-        const Collada::Effect& effect,
-        const Collada::Sampler& sampler,
-        aiTextureType type, unsigned int idx = 0);
+    void AddTexture(aiMaterial &mat, const ColladaParser &pParser,
+            const Collada::Effect &effect,
+            const Collada::Sampler &sampler,
+            aiTextureType type, unsigned int idx = 0);
 
     /** Resolves the texture name for the given effect texture entry */
-    aiString FindFilenameForEffectTexture( const ColladaParser& pParser,
-        const Collada::Effect& pEffect, const std::string& pName);
+    aiString FindFilenameForEffectTexture(const ColladaParser &pParser,
+            const Collada::Effect &pEffect, const std::string &pName);
 
     /** Reads a float value from an accessor and its data array.
      * @param pAccessor The accessor to use for reading
@@ -191,7 +185,7 @@ protected:
      * @param pOffset Offset into the element, for multipart elements such as vectors or matrices
      * @return the specified value
      */
-    ai_real ReadFloat( const Collada::Accessor& pAccessor, const Collada::Data& pData, size_t pIndex, size_t pOffset) const;
+    ai_real ReadFloat(const Collada::Accessor &pAccessor, const Collada::Data &pData, size_t pIndex, size_t pOffset) const;
 
     /** Reads a string value from an accessor and its data array.
      * @param pAccessor The accessor to use for reading
@@ -199,18 +193,18 @@ protected:
      * @param pIndex The index of the element to retrieve
      * @return the specified value
      */
-    const std::string& ReadString( const Collada::Accessor& pAccessor, const Collada::Data& pData, size_t pIndex) const;
+    const std::string &ReadString(const Collada::Accessor &pAccessor, const Collada::Data &pData, size_t pIndex) const;
 
     /** Recursively collects all nodes into the given array */
-    void CollectNodes( const aiNode* pNode, std::vector<const aiNode*>& poNodes) const;
+    void CollectNodes(const aiNode *pNode, std::vector<const aiNode *> &poNodes) const;
 
     /** Finds a node in the collada scene by the given name */
-    const Collada::Node* FindNode( const Collada::Node* pNode, const std::string& pName) const;
+    const Collada::Node *FindNode(const Collada::Node *pNode, const std::string &pName) const;
     /** Finds a node in the collada scene by the given SID */
-    const Collada::Node* FindNodeBySID( const Collada::Node* pNode, const std::string& pSID) const;
+    const Collada::Node *FindNodeBySID(const Collada::Node *pNode, const std::string &pSID) const;
 
     /** Finds a proper name for a node derived from the collada-node's properties */
-    std::string FindNameForNode( const Collada::Node* pNode);
+    std::string FindNameForNode(const Collada::Node *pNode);
 
 protected:
     /** Filename, for a verbose error message */
@@ -223,25 +217,25 @@ protected:
     std::map<std::string, size_t> mMaterialIndexByName;
 
     /** Accumulated meshes for the target scene */
-    std::vector<aiMesh*> mMeshes;
-	
+    std::vector<aiMesh *> mMeshes;
+
     /** Accumulated morph target meshes */
-    std::vector<aiMesh*> mTargetMeshes;
+    std::vector<aiMesh *> mTargetMeshes;
 
     /** Temporary material list */
-    std::vector<std::pair<Collada::Effect*, aiMaterial*> > newMats;
+    std::vector<std::pair<Collada::Effect *, aiMaterial *>> newMats;
 
     /** Temporary camera list */
-    std::vector<aiCamera*> mCameras;
+    std::vector<aiCamera *> mCameras;
 
     /** Temporary light list */
-    std::vector<aiLight*> mLights;
+    std::vector<aiLight *> mLights;
 
     /** Temporary texture list */
-    std::vector<aiTexture*> mTextures;
+    std::vector<aiTexture *> mTextures;
 
     /** Accumulated animations for the target scene */
-    std::vector<aiAnimation*> mAnims;
+    std::vector<aiAnimation *> mAnims;
 
     bool noSkeletonMesh;
     bool ignoreUpDirection;

+ 55 - 59
code/AssetLib/Collada/ColladaParser.cpp

@@ -54,6 +54,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <assimp/light.h>
 #include <assimp/DefaultLogger.hpp>
 #include <assimp/IOSystem.hpp>
+#include <memory>
 
 using namespace Assimp;
 using namespace Assimp::Collada;
@@ -126,7 +127,7 @@ ColladaParser::ColladaParser(IOSystem *pIOHandler, const std::string &pFile) :
     // Determine type
     std::string extension = BaseImporter::GetExtension(pFile);
     if (extension != "dae") {
-        zip_archive.reset(new ZipArchiveIOSystem(pIOHandler, pFile));
+        zip_archive = std::make_unique<ZipArchiveIOSystem>(pIOHandler, pFile);
     }
 
     if (zip_archive && zip_archive->isOpen()) {
@@ -158,9 +159,9 @@ ColladaParser::ColladaParser(IOSystem *pIOHandler, const std::string &pFile) :
     if (colladaNode.empty()) {
         return;
     }
-    ReadContents(colladaNode);
 
-    // read embedded textures
+    // Read content and embedded textures
+    ReadContents(colladaNode);
     if (zip_archive && zip_archive->isOpen()) {
         ReadEmbeddedTextures(*zip_archive);
     }
@@ -169,11 +170,11 @@ ColladaParser::ColladaParser(IOSystem *pIOHandler, const std::string &pFile) :
 // ------------------------------------------------------------------------------------------------
 // Destructor, private as well
 ColladaParser::~ColladaParser() {
-    for (NodeLibrary::iterator it = mNodeLibrary.begin(); it != mNodeLibrary.end(); ++it) {
-        delete it->second;
+    for (auto & it : mNodeLibrary) {
+        delete it.second;
     }
-    for (MeshLibrary::iterator it = mMeshLibrary.begin(); it != mMeshLibrary.end(); ++it) {
-        delete it->second;
+    for (auto & it : mMeshLibrary) {
+        delete it.second;
     }
 }
 
@@ -289,7 +290,7 @@ void ColladaParser::ReadContents(XmlNode &node) {
 // Reads the structure of the file
 void ColladaParser::ReadStructure(XmlNode &node) {
     for (XmlNode &currentNode : node.children()) {
-        const std::string &currentName = std::string(currentNode.name());
+        const std::string &currentName = currentNode.name();
         if (currentName == "asset") {
             ReadAssetInfo(currentNode);
         } else if (currentName == "library_animations") {
@@ -407,7 +408,7 @@ void ColladaParser::ReadAnimationClipLibrary(XmlNode &node) {
         const std::string &currentName = currentNode.name();
         if (currentName == "instance_animation") {
             std::string url;
-            readUrlAttribute(node, url);
+            readUrlAttribute(currentNode, url);
             clip.second.push_back(url);
         }
 
@@ -419,8 +420,8 @@ void ColladaParser::ReadAnimationClipLibrary(XmlNode &node) {
 
 void ColladaParser::PostProcessControllers() {
     std::string meshId;
-    for (ControllerLibrary::iterator it = mControllerLibrary.begin(); it != mControllerLibrary.end(); ++it) {
-        meshId = it->second.mMeshId;
+    for (auto & it : mControllerLibrary) {
+        meshId = it.second.mMeshId;
         if (meshId.empty()) {
             continue;
         }
@@ -431,7 +432,7 @@ void ColladaParser::PostProcessControllers() {
             findItr = mControllerLibrary.find(meshId);
         }
 
-        it->second.mMeshId = meshId;
+        it.second.mMeshId = meshId;
     }
 }
 
@@ -444,17 +445,15 @@ void ColladaParser::PostProcessRootAnimations() {
     }
 
     Animation temp;
-    for (AnimationClipLibrary::iterator it = mAnimationClipLibrary.begin(); it != mAnimationClipLibrary.end(); ++it) {
-        std::string clipName = it->first;
+    for (auto & it : mAnimationClipLibrary) {
+        std::string clipName = it.first;
 
         Animation *clip = new Animation();
         clip->mName = clipName;
 
         temp.mSubAnims.push_back(clip);
 
-        for (std::vector<std::string>::iterator a = it->second.begin(); a != it->second.end(); ++a) {
-            std::string animationID = *a;
-
+        for (std::string animationID : it.second) {
             AnimationLibrary::iterator animation = mAnimationLibrary.find(animationID);
 
             if (animation != mAnimationLibrary.end()) {
@@ -494,7 +493,7 @@ void ColladaParser::ReadAnimation(XmlNode &node, Collada::Animation *pParent) {
 
     // an <animation> element may be a container for grouping sub-elements or an animation channel
     // this is the channel collection by ID, in case it has channels
-    using ChannelMap = std::map<std::string, AnimationChannel> ;
+    using ChannelMap = std::map<std::string, AnimationChannel>;
     ChannelMap channels;
     // this is the anim container in case we're a container
     Animation *anim = nullptr;
@@ -553,8 +552,8 @@ void ColladaParser::ReadAnimation(XmlNode &node, Collada::Animation *pParent) {
             pParent->mSubAnims.push_back(anim);
         }
 
-        for (ChannelMap::const_iterator it = channels.begin(); it != channels.end(); ++it) {
-            anim->mChannels.push_back(it->second);
+        for (const auto & channel : channels) {
+            anim->mChannels.push_back(channel.second);
         }
 
         if (idAttr) {
@@ -609,50 +608,61 @@ void ColladaParser::ReadControllerLibrary(XmlNode &node) {
         if (currentName != "controller") {
             continue;
         }
-        std::string id = node.attribute("id").as_string();
-        mControllerLibrary[id] = Controller();
-        ReadController(node, mControllerLibrary[id]);
+        std::string id;
+        if (XmlParser::getStdStrAttribute(currentNode, "id", id)) {
+            mControllerLibrary[id] = Controller();
+            ReadController(currentNode, mControllerLibrary[id]);
+        }
     }
 }
 
 // ------------------------------------------------------------------------------------------------
 // Reads a controller into the given mesh structure
-void ColladaParser::ReadController(XmlNode &node, Collada::Controller &pController) {
+void ColladaParser::ReadController(XmlNode &node, Collada::Controller &controller) {
     // initial values
-    pController.mType = Skin;
-    pController.mMethod = Normalized;
-    for (XmlNode &currentNode : node.children()) {
+    controller.mType = Skin;
+    controller.mMethod = Normalized;
+
+    XmlNodeIterator xmlIt(node);
+    xmlIt.collectChildrenPreOrder(node);
+    XmlNode currentNode;
+    while (xmlIt.getNext(currentNode)) {
+
+    //for (XmlNode &currentNode : node.children()) {
         const std::string &currentName = currentNode.name();
         if (currentName == "morph") {
-            pController.mType = Morph;
-            pController.mMeshId = currentNode.attribute("source").as_string();
+            controller.mType = Morph;
+            controller.mMeshId = currentNode.attribute("source").as_string();
             int methodIndex = currentNode.attribute("method").as_int();
             if (methodIndex > 0) {
                 std::string method;
                 XmlParser::getValueAsString(currentNode, method);
 
                 if (method == "RELATIVE") {
-                    pController.mMethod = Relative;
+                    controller.mMethod = Relative;
                 }
             }
         } else if (currentName == "skin") {
-            pController.mMeshId = currentNode.attribute("source").as_string();
+            std::string id;
+            if (XmlParser::getStdStrAttribute(currentNode, "source", id)) {
+                controller.mMeshId = id.substr(1, id.size()-1);
+            }
         } else if (currentName == "bind_shape_matrix") {
             std::string v;
             XmlParser::getValueAsString(currentNode, v);
             const char *content = v.c_str();
             for (unsigned int a = 0; a < 16; a++) {
                 // read a number
-                content = fast_atoreal_move<ai_real>(content, pController.mBindShapeMatrix[a]);
+                content = fast_atoreal_move<ai_real>(content, controller.mBindShapeMatrix[a]);
                 // skip whitespace after it
                 SkipSpacesAndLineEnd(&content);
             }
         } else if (currentName == "source") {
             ReadSource(currentNode);
         } else if (currentName == "joints") {
-            ReadControllerJoints(currentNode, pController);
+            ReadControllerJoints(currentNode, controller);
         } else if (currentName == "vertex_weights") {
-            ReadControllerWeights(currentNode, pController);
+            ReadControllerWeights(currentNode, controller);
         } else if (currentName == "targets") {
             for (XmlNode currentChildNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
                 const std::string &currentChildName = currentChildNode.name();
@@ -660,9 +670,9 @@ void ColladaParser::ReadController(XmlNode &node, Collada::Controller &pControll
                     const char *semantics = currentChildNode.attribute("semantic").as_string();
                     const char *source = currentChildNode.attribute("source").as_string();
                     if (strcmp(semantics, "MORPH_TARGET") == 0) {
-                        pController.mMorphTarget = source + 1;
+                        controller.mMorphTarget = source + 1;
                     } else if (strcmp(semantics, "MORPH_WEIGHT") == 0) {
-                        pController.mMorphWeight = source + 1;
+                        controller.mMorphWeight = source + 1;
                     }
                 }
             }
@@ -700,6 +710,7 @@ void ColladaParser::ReadControllerWeights(XmlNode &node, Collada::Controller &pC
     // Read vertex count from attributes and resize the array accordingly
     int vertexCount=0;
     XmlParser::getIntAttribute(node, "count", vertexCount);
+    pController.mWeightCounts.resize(vertexCount);
 
     for (XmlNode &currentNode : node.children()) {
         const std::string &currentName = currentNode.name();
@@ -725,7 +736,7 @@ void ColladaParser::ReadControllerWeights(XmlNode &node, Collada::Controller &pC
                 throw DeadlyImportError("Unknown semantic \"", attrSemantic, "\" in <vertex_weights> data <input> element");
             }
         } else if (currentName == "vcount" && vertexCount > 0) {
-            const char *text = currentNode.value();
+            const char *text = currentNode.text().as_string();
             size_t numWeights = 0;
             for (std::vector<size_t>::iterator it = pController.mWeightCounts.begin(); it != pController.mWeightCounts.end(); ++it) {
                 if (*text == 0) {
@@ -762,18 +773,15 @@ void ColladaParser::ReadControllerWeights(XmlNode &node, Collada::Controller &pC
 // ------------------------------------------------------------------------------------------------
 // Reads the image library contents
 void ColladaParser::ReadImageLibrary(XmlNode &node) {
-    if (node.empty()) {
-        return;
-    }
-
     for (XmlNode &currentNode : node.children()) {
         const std::string &currentName = currentNode.name();
         if (currentName == "image") {
-            std::string id = currentNode.attribute("id").as_string();
-            mImageLibrary[id] = Image();
-
-            // read on from there
-            ReadImage(currentNode, mImageLibrary[id]);
+            std::string id;
+            if (XmlParser::getStdStrAttribute( currentNode, "id", id )) {
+                mImageLibrary[id] = Image();
+                // read on from there
+                ReadImage(currentNode, mImageLibrary[id]);
+            }
         }
     }
 }
@@ -792,7 +800,7 @@ void ColladaParser::ReadImage(XmlNode &node, Collada::Image &pImage) {
                 if (!currentNode.empty()) {
                     // element content is filename - hopefully
                     const char *sz = currentNode.text().as_string();
-                    if (sz) {
+                    if (nullptr != sz) {
                         aiString filepath(sz);
                         UriDecodePath(filepath);
                         pImage.mFileName = filepath.C_Str();
@@ -842,10 +850,6 @@ void ColladaParser::ReadImage(XmlNode &node, Collada::Image &pImage) {
 // ------------------------------------------------------------------------------------------------
 // Reads the material library
 void ColladaParser::ReadMaterialLibrary(XmlNode &node) {
-    if (node.empty()) {
-        return;
-    }
-
     std::map<std::string, int> names;
     for (XmlNode &currentNode : node.children()) {
         std::string id = currentNode.attribute("id").as_string();
@@ -872,10 +876,6 @@ void ColladaParser::ReadMaterialLibrary(XmlNode &node) {
 // ------------------------------------------------------------------------------------------------
 // Reads the light library
 void ColladaParser::ReadLightLibrary(XmlNode &node) {
-    if (node.empty()) {
-        return;
-    }
-
     for (XmlNode &currentNode : node.children()) {
         const std::string &currentName = currentNode.name();
         if (currentName == "light") {
@@ -890,10 +890,6 @@ void ColladaParser::ReadLightLibrary(XmlNode &node) {
 // ------------------------------------------------------------------------------------------------
 // Reads the camera library
 void ColladaParser::ReadCameraLibrary(XmlNode &node) {
-    if (node.empty()) {
-        return;
-    }
-
     for (XmlNode &currentNode : node.children()) {
         const std::string &currentName = currentNode.name();
         if (currentName == "camera") {