Merge pull request #3466 from assimp/collada_cleanup

Collada cleanup

code/AssetLib/Collada/ColladaHelper.cpp

@@ -1,5 +1,3 @@
-/** Helper structures for the Collada loader */
-
 /*
 Open Asset Import Library (assimp)
 ----------------------------------------------------------------------
@@ -40,6 +38,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 ----------------------------------------------------------------------
 */
+/** Helper structures for the Collada loader */
 
 #include "ColladaHelper.h"
 

code/AssetLib/Collada/ColladaHelper.h

@@ -1,12 +1,9 @@
-/** Helper structures for the Collada loader */
-
 /*
 Open Asset Import Library (assimp)
 ----------------------------------------------------------------------
 
 Copyright (c) 2006-2020, assimp team
 
-
 All rights reserved.
 
 Redistribution and use of this software in source and binary forms,
@@ -42,12 +39,15 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 ----------------------------------------------------------------------
 */
 
+/** Helper structures for the Collada loader */
+
 #ifndef AI_COLLADAHELPER_H_INC
 #define AI_COLLADAHELPER_H_INC
 
 #include <assimp/light.h>
 #include <assimp/material.h>
 #include <assimp/mesh.h>
+
 #include <stdint.h>
 #include <map>
 #include <set>
@@ -58,14 +58,14 @@ struct aiMaterial;
 namespace Assimp {
 namespace Collada {
 
-/** Collada file versions which evolved during the years ... */
+/// Collada file versions which evolved during the years ...
 enum FormatVersion {
     FV_1_5_n,
     FV_1_4_n,
     FV_1_3_n
 };
 
-/** Transformation types that can be applied to a node */
+/// Transformation types that can be applied to a node
 enum TransformType {
     TF_LOOKAT,
     TF_ROTATE,
@@ -75,7 +75,7 @@ enum TransformType {
     TF_MATRIX
 };
 
-/** Different types of input data to a vertex or face */
+/// Different types of input data to a vertex or face
 enum InputType {
     IT_Invalid,
     IT_Vertex, // special type for per-index data referring to the <vertices> element carrying the per-vertex data.
@@ -87,38 +87,39 @@ enum InputType {
     IT_Bitangent
 };
 
-/** Supported controller types */
+/// Supported controller types
 enum ControllerType {
     Skin,
     Morph
 };
 
-/** Supported morph methods */
+/// Supported morph methods
 enum MorphMethod {
     Normalized,
     Relative
 };
 
-/** Common metadata keys as <Collada, Assimp> */
-typedef std::pair<std::string, std::string> MetaKeyPair;
-typedef std::vector<MetaKeyPair> MetaKeyPairVector;
+/// Common metadata keys as <Collada, Assimp>
+using MetaKeyPair = std::pair<std::string, std::string>;
+using MetaKeyPairVector = std::vector<MetaKeyPair>;
 
-// Collada as lower_case (native)
+/// Collada as lower_case (native)
 const MetaKeyPairVector &GetColladaAssimpMetaKeys();
+
 // Collada as CamelCase (used by Assimp for consistency)
 const MetaKeyPairVector &GetColladaAssimpMetaKeysCamelCase();
 
-/** Convert underscore_separated to CamelCase "authoring_tool" becomes "AuthoringTool" */
+/// Convert underscore_separated to CamelCase "authoring_tool" becomes "AuthoringTool"
 void ToCamelCase(std::string &text);
 
-/** Contains all data for one of the different transformation types */
+/// Contains all data for one of the different transformation types
 struct Transform {
     std::string mID; ///< SID of the transform step, by which anim channels address their target node
     TransformType mType;
     ai_real f[16]; ///< Interpretation of data depends on the type of the transformation
 };
 
-/** A collada camera. */
+/// A collada camera.
 struct Camera {
     Camera() :
             mOrtho(false),
@@ -128,22 +129,22 @@ struct Camera {
             mZNear(0.1f),
             mZFar(1000.f) {}
 
-    // Name of camera
+    /// Name of camera
     std::string mName;
 
-    // True if it is an orthografic camera
+    /// True if it is an orthographic camera
     bool mOrtho;
 
-    //! Horizontal field of view in degrees
+    /// Horizontal field of view in degrees
     ai_real mHorFov;
 
-    //! Vertical field of view in degrees
+    /// Vertical field of view in degrees
     ai_real mVerFov;
 
-    //! Screen aspect
+    /// Screen aspect
     ai_real mAspect;
 
-    //! Near& far z
+    /// Near& far z
     ai_real mZNear, mZFar;
 };
 
@@ -162,27 +163,27 @@ struct Light {
             mOuterAngle(ASSIMP_COLLADA_LIGHT_ANGLE_NOT_SET),
             mIntensity(1.f) {}
 
-    //! Type of the light source aiLightSourceType + ambient
+    /// Type of the light source aiLightSourceType + ambient
     unsigned int mType;
 
-    //! Color of the light
+    /// Color of the light
     aiColor3D mColor;
 
-    //! Light attenuation
+    /// Light attenuation
     ai_real mAttConstant, mAttLinear, mAttQuadratic;
 
-    //! Spot light falloff
+    /// Spot light falloff
     ai_real mFalloffAngle;
     ai_real mFalloffExponent;
 
     // -----------------------------------------------------
     // FCOLLADA extension from here
 
-    //! ... related stuff from maja and max extensions
+    /// ... related stuff from maja and max extensions
     ai_real mPenumbraAngle;
     ai_real mOuterAngle;
 
-    //! Common light intensity
+    /// Common light intensity
     ai_real mIntensity;
 };
 
@@ -192,30 +193,29 @@ struct InputSemanticMapEntry {
             mSet(0),
             mType(IT_Invalid) {}
 
-    //! Index of set, optional
+    /// Index of set, optional
     unsigned int mSet;
 
-    //! Type of referenced vertex input
+    /// Type of referenced vertex input
     InputType mType;
 };
 
-/** Table to map from effect to vertex input semantics */
+/// Table to map from effect to vertex input semantics
 struct SemanticMappingTable {
-    //! Name of material
+    /// Name of material
     std::string mMatName;
 
-    //! List of semantic map commands, grouped by effect semantic name
+    /// List of semantic map commands, grouped by effect semantic name
     std::map<std::string, InputSemanticMapEntry> mMap;
 
-    //! For std::find
+    /// For std::find
     bool operator==(const std::string &s) const {
         return s == mMatName;
     }
 };
 
-/** A reference to a mesh inside a node, including materials assigned to the various subgroups.
- * The ID refers to either a mesh or a controller which specifies the mesh
- */
+/// A reference to a mesh inside a node, including materials assigned to the various subgroups.
+/// The ID refers to either a mesh or a controller which specifies the mesh
 struct MeshInstance {
     ///< ID of the mesh or controller to be instanced
     std::string mMeshOrController;
@@ -224,25 +224,25 @@ struct MeshInstance {
     std::map<std::string, SemanticMappingTable> mMaterials;
 };
 
-/** A reference to a camera inside a node*/
+/// A reference to a camera inside a node
 struct CameraInstance {
     ///< ID of the camera
     std::string mCamera;
 };
 
-/** A reference to a light inside a node*/
+/// A reference to a light inside a node
 struct LightInstance {
     ///< ID of the camera
     std::string mLight;
 };
 
-/** A reference to a node inside a node*/
+/// A reference to a node inside a node
 struct NodeInstance {
     ///< ID of the node
     std::string mNode;
 };
 
-/** A node in a scene hierarchy */
+/// A node in a scene hierarchy
 struct Node {
     std::string mName;
     std::string mID;
@@ -250,52 +250,53 @@ struct Node {
     Node *mParent;
     std::vector<Node *> mChildren;
 
-    /** Operations in order to calculate the resulting transformation to parent. */
+    /// Operations in order to calculate the resulting transformation to parent.
     std::vector<Transform> mTransforms;
 
-    /** Meshes at this node */
+    /// Meshes at this node
     std::vector<MeshInstance> mMeshes;
 
-    /** Lights at this node */
+    /// Lights at this node
     std::vector<LightInstance> mLights;
 
-    /** Cameras at this node */
+    /// Cameras at this node
     std::vector<CameraInstance> mCameras;
 
-    /** Node instances at this node */
+    /// Node instances at this node
     std::vector<NodeInstance> mNodeInstances;
 
-    /** Root-nodes: Name of primary camera, if any */
+    /// Root-nodes: Name of primary camera, if any
     std::string mPrimaryCamera;
 
-    //! Constructor. Begin with a zero parent
+    /// Constructor. Begin with a zero parent
     Node() :
             mParent(nullptr) {
         // empty
     }
 
-    //! Destructor: delete all children subsequently
+    /// Destructor: delete all children subsequently
     ~Node() {
-        for (std::vector<Node *>::iterator it = mChildren.begin(); it != mChildren.end(); ++it)
+        for (std::vector<Node *>::iterator it = mChildren.begin(); it != mChildren.end(); ++it) {
             delete *it;
+        }
     }
 };
 
-/** Data source array: either floats or strings */
+/// Data source array: either floats or strings
 struct Data {
     bool mIsStringArray;
     std::vector<ai_real> mValues;
     std::vector<std::string> mStrings;
 };
 
-/** Accessor to a data array */
+/// Accessor to a data array
 struct Accessor {
     size_t mCount; // in number of objects
     size_t mSize; // size of an object, in elements (floats or strings, mostly 1)
     size_t mOffset; // in number of values
     size_t mStride; // Stride in number of values
     std::vector<std::string> mParams; // names of the data streams in the accessors. Empty string tells to ignore.
-    size_t mSubOffset[4]; // Suboffset inside the object for the common 4 elements. For a vector, that's XYZ, for a color RGBA and so on.
+    size_t mSubOffset[4]; // Sub-offset inside the object for the common 4 elements. For a vector, that's XYZ, for a color RGBA and so on.
             // For example, SubOffset[0] denotes which of the values inside the object is the vector X component.
     std::string mSource; // URL of the source array
     mutable const Data *mData; // Pointer to the source array, if resolved. nullptr else
@@ -310,12 +311,12 @@ struct Accessor {
     }
 };
 
-/** A single face in a mesh */
+/// A single face in a mesh
 struct Face {
     std::vector<size_t> mIndices;
 };
 
-/** An input channel for mesh data, referring to a single accessor */
+/// An input channel for mesh data, referring to a single accessor
 struct InputChannel {
     InputType mType; // Type of the data
     size_t mIndex; // Optional index, if multiple sets of the same data type are given
@@ -331,18 +332,19 @@ struct InputChannel {
     }
 };
 
-/** Subset of a mesh with a certain material */
+/// Subset of a mesh with a certain material
 struct SubMesh {
     std::string mMaterial; ///< subgroup identifier
-    size_t mNumFaces; ///< number of faces in this submesh
+    size_t mNumFaces; ///< number of faces in this sub-mesh
 };
 
-/** Contains data for a single mesh */
+/// Contains data for a single mesh
 struct Mesh {
     Mesh(const std::string &id) :
             mId(id) {
-        for (unsigned int i = 0; i < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++i)
+        for (unsigned int i = 0; i < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++i) {
             mNumUVComponents[i] = 2;
+        }
     }
 
     const std::string mId;
@@ -373,11 +375,11 @@ struct Mesh {
     // necessary for bone weight assignment
     std::vector<size_t> mFacePosIndices;
 
-    // Submeshes in this mesh, each with a given material
+    // Sub-meshes in this mesh, each with a given material
     std::vector<SubMesh> mSubMeshes;
 };
 
-/** Which type of primitives the ReadPrimitives() function is going to read */
+/// Which type of primitives the ReadPrimitives() function is going to read
 enum PrimitiveType {
     Prim_Invalid,
     Prim_Lines,
@@ -389,7 +391,7 @@ enum PrimitiveType {
     Prim_Polygon
 };
 
-/** A skeleton controller to deform a mesh with the use of joints */
+/// A skeleton controller to deform a mesh with the use of joints
 struct Controller {
     // controller type
     ControllerType mType;
@@ -424,25 +426,25 @@ struct Controller {
     std::string mMorphWeight;
 };
 
-/** A collada material. Pretty much the only member is a reference to an effect. */
+/// A collada material. Pretty much the only member is a reference to an effect.
 struct Material {
     std::string mName;
     std::string mEffect;
 };
 
-/** Type of the effect param */
+/// Type of the effect param
 enum ParamType {
     Param_Sampler,
     Param_Surface
 };
 
-/** A param for an effect. Might be of several types, but they all just refer to each other, so I summarize them */
+/// A param for an effect. Might be of several types, but they all just refer to each other, so I summarize them
 struct EffectParam {
     ParamType mType;
     std::string mReference; // to which other thing the param is referring to.
 };
 
-/** Shading type supported by the standard effect spec of Collada */
+/// Shading type supported by the standard effect spec of Collada
 enum ShadeType {
     Shade_Invalid,
     Shade_Constant,
@@ -451,7 +453,7 @@ enum ShadeType {
     Shade_Blinn
 };
 
-/** Represents a texture sampler in collada */
+/// Represents a texture sampler in collada
 struct Sampler {
     Sampler() :
             mWrapU(true),
@@ -463,77 +465,66 @@ struct Sampler {
             mWeighting(1.f),
             mMixWithPrevious(1.f) {}
 
-    /** Name of image reference
-     */
+    /// Name of image reference
     std::string mName;
 
-    /** Wrap U?
-     */
+    /// Wrap U?
     bool mWrapU;
 
-    /** Wrap V?
-     */
+    /// Wrap V?
     bool mWrapV;
 
-    /** Mirror U?
-     */
+    /// Mirror U?
     bool mMirrorU;
 
-    /** Mirror V?
-     */
+    /// Mirror V?
     bool mMirrorV;
 
-    /** Blend mode
-     */
+    /// Blend mode
     aiTextureOp mOp;
 
-    /** UV transformation
-     */
+    /// UV transformation
     aiUVTransform mTransform;
 
-    /** Name of source UV channel
-     */
+    /// Name of source UV channel
     std::string mUVChannel;
 
-    /** Resolved UV channel index or UINT_MAX if not known
-     */
+    /// Resolved UV channel index or UINT_MAX if not known
     unsigned int mUVId;
 
     // OKINO/MAX3D extensions from here
     // -------------------------------------------------------
 
-    /** Weighting factor
-     */
+    /// Weighting factor
     ai_real mWeighting;
 
-    /** Mixing factor from OKINO
-     */
+    /// Mixing factor from OKINO
     ai_real mMixWithPrevious;
 };
 
-/** A collada effect. Can contain about anything according to the Collada spec,
-    but we limit our version to a reasonable subset. */
+/// A collada effect. Can contain about anything according to the Collada spec,
+/// but we limit our version to a reasonable subset.
 struct Effect {
-    // Shading mode
+    /// Shading mode
     ShadeType mShadeType;
 
-    // Colors
+    /// Colors
     aiColor4D mEmissive, mAmbient, mDiffuse, mSpecular,
             mTransparent, mReflective;
 
-    // Textures
+    /// Textures
     Sampler mTexEmissive, mTexAmbient, mTexDiffuse, mTexSpecular,
             mTexTransparent, mTexBump, mTexReflective;
 
-    // Scalar factory
+    /// Scalar factory
     ai_real mShininess, mRefractIndex, mReflectivity;
     ai_real mTransparency;
     bool mHasTransparency;
     bool mRGBTransparency;
     bool mInvertTransparency;
 
-    // local params referring to each other by their SID
-    typedef std::map<std::string, Collada::EffectParam> ParamLibrary;
+    /// local params referring to each other by their SID
+    using ParamLibrary = std::map<std::string, Collada::EffectParam>;
     ParamLibrary mParams;
 
     // MAX3D extensions
@@ -561,65 +552,64 @@ struct Effect {
     }
 };
 
-/** An image, meaning texture */
+/// An image, meaning texture
 struct Image {
     std::string mFileName;
 
-    /** Embedded image data */
+    /// Embedded image data
     std::vector<uint8_t> mImageData;
 
-    /** File format hint of embedded image data */
+    /// File format hint of embedded image data
     std::string mEmbeddedFormat;
 };
 
-/** An animation channel. */
+/// An animation channel.
 struct AnimationChannel {
-    /** URL of the data to animate. Could be about anything, but we support only the
-     * "NodeID/TransformID.SubElement" notation
-     */
+    /// URL of the data to animate. Could be about anything, but we support only the
+    /// "NodeID/TransformID.SubElement" notation
     std::string mTarget;
 
-    /** Source URL of the time values. Collada calls them "input". Meh. */
+    /// Source URL of the time values. Collada calls them "input". Meh.
     std::string mSourceTimes;
-    /** Source URL of the value values. Collada calls them "output". */
+    /// Source URL of the value values. Collada calls them "output".
     std::string mSourceValues;
-    /** Source URL of the IN_TANGENT semantic values. */
+    /// Source URL of the IN_TANGENT semantic values.
     std::string mInTanValues;
-    /** Source URL of the OUT_TANGENT semantic values. */
+    /// Source URL of the OUT_TANGENT semantic values.
     std::string mOutTanValues;
-    /** Source URL of the INTERPOLATION semantic values. */
+    /// Source URL of the INTERPOLATION semantic values.
     std::string mInterpolationValues;
 };
 
-/** An animation. Container for 0-x animation channels or 0-x animations */
+/// An animation. Container for 0-x animation channels or 0-x animations
 struct Animation {
-    /** Anim name */
+    /// Anim name
     std::string mName;
 
-    /** the animation channels, if any */
+    /// the animation channels, if any
     std::vector<AnimationChannel> mChannels;
 
-    /** the sub-animations, if any */
+    /// the sub-animations, if any
     std::vector<Animation *> mSubAnims;
 
-    /** Destructor */
+    /// Destructor
     ~Animation() {
-        for (std::vector<Animation *>::iterator it = mSubAnims.begin(); it != mSubAnims.end(); ++it)
+        for (std::vector<Animation *>::iterator it = mSubAnims.begin(); it != mSubAnims.end(); ++it) {
             delete *it;
+        }
     }
 
-    /** Collect all channels in the animation hierarchy into a single channel list. */
+    /// Collect all channels in the animation hierarchy into a single channel list.
     void CollectChannelsRecursively(std::vector<AnimationChannel> &channels) {
         channels.insert(channels.end(), mChannels.begin(), mChannels.end());
 
         for (std::vector<Animation *>::iterator it = mSubAnims.begin(); it != mSubAnims.end(); ++it) {
             Animation *pAnim = (*it);
-
             pAnim->CollectChannelsRecursively(channels);
         }
     }
 
-    /** Combine all single-channel animations' channel into the same (parent) animation channel list. */
+    /// Combine all single-channel animations' channel into the same (parent) animation channel list.
     void CombineSingleChannelAnimations() {
         CombineSingleChannelAnimationsRecursively(this);
     }
@@ -658,9 +648,9 @@ struct Animation {
     }
 };
 
-/** Description of a collada animation channel which has been determined to affect the current node */
+/// Description of a collada animation channel which has been determined to affect the current node
 struct ChannelEntry {
-    const Collada::AnimationChannel *mChannel; ///> the source channel
+    const Collada::AnimationChannel *mChannel; ///< the source channel
     std::string mTargetId;
     std::string mTransformId; // the ID of the transformation step of the node which is influenced
     size_t mTransformIndex; // Index into the node's transform chain to apply the channel to

code/AssetLib/Collada/ColladaLoader.cpp

@@ -55,12 +55,9 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <assimp/fast_atof.h>
 #include <assimp/importerdesc.h>
 #include <assimp/scene.h>
-#include <math.h>
-#include <time.h>
-#include <algorithm>
 #include <assimp/DefaultLogger.hpp>
 #include <assimp/Importer.hpp>
-#include <memory>
+
 #include <numeric>
 
 namespace Assimp {
@@ -331,13 +328,15 @@ 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);
-    if (it != table.mMap.end()) {
-        if (it->second.mType != Collada::IT_Texcoord) {
-            ASSIMP_LOG_ERROR("Collada: Unexpected effect input mapping");
-        }
+    if (it == table.mMap.end()) {
+        return;
+    }
 
-        sampler.mUVId = it->second.mSet;
+    if (it->second.mType != Collada::IT_Texcoord) {
+        ASSIMP_LOG_ERROR("Collada: Unexpected effect input mapping");
     }
+
+    sampler.mUVId = it->second.mSet;
 }
 
 // ------------------------------------------------------------------------------------------------

code/AssetLib/Collada/ColladaParser.cpp

@@ -48,7 +48,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include "ColladaParser.h"
 #include <assimp/ParsingUtils.h>
 #include <assimp/StringUtils.h>
-#include <assimp/TinyFormatter.h>
 #include <assimp/ZipArchiveIOSystem.h>
 #include <assimp/commonMetaData.h>
 #include <assimp/fast_atof.h>
@@ -56,14 +55,47 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <assimp/DefaultLogger.hpp>
 #include <assimp/IOSystem.hpp>
 
-#include <stdarg.h>
-#include <memory>
-#include <sstream>
-
 using namespace Assimp;
 using namespace Assimp::Collada;
 using namespace Assimp::Formatter;
 
+static void ReportWarning(const char *msg, ...) {
+    ai_assert(nullptr != msg);
+
+    va_list args;
+    va_start(args, msg);
+
+    char szBuffer[3000];
+    const int iLen = vsprintf(szBuffer, msg, args);
+    ai_assert(iLen > 0);
+
+    va_end(args);
+    ASSIMP_LOG_WARN_F("Validation warning: ", std::string(szBuffer, iLen));
+}
+
+static bool FindCommonKey(const std::string &collada_key, const MetaKeyPairVector &key_renaming, size_t &found_index) {
+    for (size_t i = 0; i < key_renaming.size(); ++i) {
+        if (key_renaming[i].first == collada_key) {
+            found_index = i;
+            return true;
+        }
+    }
+    found_index = std::numeric_limits<size_t>::max();
+
+    return false;
+}
+
+static void readUrlAttribute(XmlNode &node, std::string &url) {
+    url.clear();
+    if (!XmlParser::getStdStrAttribute(node, "url", url)) {
+        return;
+    }
+    if (url[0] != '#') {
+        throw DeadlyImportError("Unknown reference format");
+    }
+    url = url.c_str() + 1;
+}
+
 // ------------------------------------------------------------------------------------------------
 // Constructor to be privately used by Importer
 ColladaParser::ColladaParser(IOSystem *pIOHandler, const std::string &pFile) :
@@ -256,35 +288,35 @@ void ColladaParser::ReadContents(XmlNode &node) {
 // ------------------------------------------------------------------------------------------------
 // Reads the structure of the file
 void ColladaParser::ReadStructure(XmlNode &node) {
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
-        const std::string name = std::string(currentNode.name());
-        ASSIMP_LOG_DEBUG("last name" + name);
-        if (name == "asset")
+    for (XmlNode &currentNode : node.children()) {
+        const std::string &currentName = std::string(currentNode.name());
+        if (currentName == "asset") {
             ReadAssetInfo(currentNode);
-        else if (name == "library_animations")
+        } else if (currentName == "library_animations") {
             ReadAnimationLibrary(currentNode);
-        else if (name == "library_animation_clips")
+        } else if (currentName == "library_animation_clips") {
             ReadAnimationClipLibrary(currentNode);
-        else if (name == "library_controllers")
+        } else if (currentName == "library_controllers") {
             ReadControllerLibrary(currentNode);
-        else if (name == "library_images")
+        } else if (currentName == "library_images") {
             ReadImageLibrary(currentNode);
-        else if (name == "library_materials")
+        } else if (currentName == "library_materials") {
             ReadMaterialLibrary(currentNode);
-        else if (name == "library_effects")
+        } else if (currentName == "library_effects") {
             ReadEffectLibrary(currentNode);
-        else if (name == "library_geometries")
+        } else if (currentName == "library_geometries") {
             ReadGeometryLibrary(currentNode);
-        else if (name == "library_visual_scenes")
+        } else if (currentName == "library_visual_scenes") {
             ReadSceneLibrary(currentNode);
-        else if (name == "library_lights")
+        } else if (currentName == "library_lights") {
             ReadLightLibrary(currentNode);
-        else if (name == "library_cameras")
+        } else if (currentName == "library_cameras") {
             ReadCameraLibrary(currentNode);
-        else if (name == "library_nodes")
+        } else if (currentName == "library_nodes") {
             ReadSceneNode(currentNode, nullptr); /* some hacking to reuse this piece of code */
-        else if (name == "scene")
+        } else if (currentName == "scene") {
             ReadScene(currentNode);
+        }
     }
 
     PostProcessRootAnimations();
@@ -298,17 +330,16 @@ void ColladaParser::ReadAssetInfo(XmlNode &node) {
         return;
     }
 
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
-        const std::string name = currentNode.name();
-        if (name == "unit") {
-            pugi::xml_attribute attr = currentNode.attribute("meter");
+    for (XmlNode &currentNode : node.children()) {
+        const std::string &currentName = currentNode.name();
+        if (currentName == "unit") {
             mUnitSize = 1.f;
-            if (attr) {
-                mUnitSize = static_cast<ai_real>(attr.as_double());
-            }
-        } else if (name == "up_axis") {
+            XmlParser::getFloatAttribute(node, "meter", mUnitSize);
+        } else if (currentName == "up_axis") {
             std::string v;
-            XmlParser::getValueAsString(currentNode, v);
+            if (!XmlParser::getValueAsString(currentNode, v)) {
+                continue;
+            }
             if (v == "X_UP") {
                 mUpDirection = UP_X;
             } else if (v == "Z_UP") {
@@ -316,9 +347,9 @@ void ColladaParser::ReadAssetInfo(XmlNode &node) {
             } else {
                 mUpDirection = UP_Y;
             }
-        } else if (name == "contributor") {
-            for (XmlNode currentChldNode : currentNode.children()) {
-                ReadMetaDataItem(currentChldNode, mAssetMetaData);
+        } else if (currentName == "contributor") {
+            for (XmlNode currentChildNode : currentNode.children()) {
+                ReadMetaDataItem(currentChildNode, mAssetMetaData);
             }
         } else {
             ReadMetaDataItem(currentNode, mAssetMetaData);
@@ -326,43 +357,32 @@ void ColladaParser::ReadAssetInfo(XmlNode &node) {
     }
 }
 
-static bool FindCommonKey(const std::string &collada_key, const MetaKeyPairVector &key_renaming, size_t &found_index) {
-    for (size_t i = 0; i < key_renaming.size(); ++i) {
-        if (key_renaming[i].first == collada_key) {
-            found_index = i;
-            return true;
-        }
-    }
-    found_index = std::numeric_limits<size_t>::max();
-
-    return false;
-}
-
 // ------------------------------------------------------------------------------------------------
 // Reads a single string metadata item
 void ColladaParser::ReadMetaDataItem(XmlNode &node, StringMetaData &metadata) {
     const Collada::MetaKeyPairVector &key_renaming = GetColladaAssimpMetaKeysCamelCase();
-
     const std::string name = node.name();
     if (name.empty()) {
         return;
     }
 
     std::string v;
-    if (XmlParser::getValueAsString(node, v)) {
-        trim(v);
-        aiString aistr;
-        aistr.Set(v);
+    if (!XmlParser::getValueAsString(node, v)) {
+        return;
+    }
 
-        std::string camel_key_str(name);
-        ToCamelCase(camel_key_str);
+    trim(v);
+    aiString aistr;
+    aistr.Set(v);
 
-        size_t found_index;
-        if (FindCommonKey(camel_key_str, key_renaming, found_index)) {
-            metadata.emplace(key_renaming[found_index].second, aistr);
-        } else {
-            metadata.emplace(camel_key_str, aistr);
-        }
+    std::string camel_key_str(name);
+    ToCamelCase(camel_key_str);
+
+    size_t found_index;
+    if (FindCommonKey(camel_key_str, key_renaming, found_index)) {
+        metadata.emplace(key_renaming[found_index].second, aistr);
+    } else {
+        metadata.emplace(camel_key_str, aistr);
     }
 }
 
@@ -374,14 +394,8 @@ void ColladaParser::ReadAnimationClipLibrary(XmlNode &node) {
     }
 
     std::string animName;
-    pugi::xml_attribute nameAttr = node.attribute("name");
-    if (nameAttr) {
-        animName = nameAttr.as_string();
-    } else {
-        pugi::xml_attribute idAttr = node.attribute("id");
-        if (idAttr) {
-            animName = idAttr.as_string();
-        } else {
+    if (!XmlParser::getStdStrAttribute(node, "name", animName)) {
+        if (!XmlParser::getStdStrAttribute( node, "id", animName )) {
             animName = std::string("animation_") + to_string(mAnimationClipLibrary.size());
         }
     }
@@ -389,17 +403,12 @@ void ColladaParser::ReadAnimationClipLibrary(XmlNode &node) {
     std::pair<std::string, std::vector<std::string>> clip;
     clip.first = animName;
 
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
-        const std::string currentName = currentNode.name();
+    for (XmlNode &currentNode : node.children()) {
+        const std::string &currentName = currentNode.name();
         if (currentName == "instance_animation") {
-            pugi::xml_attribute url = currentNode.attribute("url");
-            if (url) {
-                const std::string urlName = url.as_string();
-                if (urlName[0] != '#') {
-                    throw DeadlyImportError("Unknown reference format");
-                }
-                clip.second.push_back(url.as_string());
-            }
+            std::string url;
+            readUrlAttribute(node, url);
+            clip.second.push_back(url);
         }
 
         if (clip.second.size() > 0) {
@@ -469,8 +478,8 @@ void ColladaParser::ReadAnimationLibrary(XmlNode &node) {
         return;
     }
 
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
-        const std::string currentName = currentNode.name();
+    for (XmlNode &currentNode : node.children()) {
+        const std::string &currentName = currentNode.name();
         if (currentName == "animation") {
             ReadAnimation(currentNode, &mAnims);
         }
@@ -486,17 +495,14 @@ 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
-    typedef std::map<std::string, AnimationChannel> ChannelMap;
+    using ChannelMap = std::map<std::string, AnimationChannel> ;
     ChannelMap channels;
     // this is the anim container in case we're a container
     Animation *anim = nullptr;
 
     // optional name given as an attribute
     std::string animName;
-    pugi::xml_attribute nameAttr = node.attribute("name");
-    if (nameAttr) {
-        animName = nameAttr.as_string();
-    } else {
+    if (!XmlParser::getStdStrAttribute(node, "name", animName)) {
         animName = "animation";
     }
 
@@ -506,8 +512,8 @@ void ColladaParser::ReadAnimation(XmlNode &node, Collada::Animation *pParent) {
         animID = idAttr.as_string();
     }
 
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
-        const std::string currentName = currentNode.name();
+    for (XmlNode &currentNode : node.children()) {
+        const std::string &currentName = currentNode.name();
         if (currentName == "animation") {
             if (!anim) {
                 anim = new Animation;
@@ -520,23 +526,21 @@ void ColladaParser::ReadAnimation(XmlNode &node, Collada::Animation *pParent) {
         } else if (currentName == "source") {
             ReadSource(currentNode);
         } else if (currentName == "sampler") {
-            pugi::xml_attribute sampler_id = currentNode.attribute("id");
-            if (sampler_id) {
-                std::string id = sampler_id.as_string();
-                ChannelMap::iterator newChannel = channels.insert(std::make_pair(id, AnimationChannel())).first;
-
+            std::string id;
+            if (XmlParser::getStdStrAttribute(currentNode, "id", id)) {
                 // have it read into a channel
+                ChannelMap::iterator newChannel = channels.insert(std::make_pair(id, AnimationChannel())).first;
                 ReadAnimationSampler(currentNode, newChannel->second);
             } else if (currentName == "channel") {
-                pugi::xml_attribute target = currentNode.attribute("target");
-                pugi::xml_attribute source = currentNode.attribute("source");
-                std::string source_name = source.as_string();
+                std::string source_name, target;
+                XmlParser::getStdStrAttribute(currentNode, "source", source_name);
+                XmlParser::getStdStrAttribute(currentNode, "target", target);
                 if (source_name[0] == '#') {
                     source_name = source_name.substr(1, source_name.size() - 1);
                 }
                 ChannelMap::iterator cit = channels.find(source_name);
                 if (cit != channels.end()) {
-                    cit->second.mTarget = target.as_string();
+                    cit->second.mTarget = target;
                 }
             }
         }
@@ -563,8 +567,8 @@ void ColladaParser::ReadAnimation(XmlNode &node, Collada::Animation *pParent) {
 // ------------------------------------------------------------------------------------------------
 // Reads an animation sampler into the given anim channel
 void ColladaParser::ReadAnimationSampler(XmlNode &node, Collada::AnimationChannel &pChannel) {
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
-        const std::string currentName = currentNode.name();
+    for (XmlNode &currentNode : node.children()) {
+        const std::string &currentName = currentNode.name();
         if (currentName == "input") {
             if (XmlParser::hasAttribute(currentNode, "semantic")) {
                 std::string semantic, sourceAttr;
@@ -577,16 +581,17 @@ void ColladaParser::ReadAnimationSampler(XmlNode &node, Collada::AnimationChanne
                     }
                     source++;
 
-                    if (semantic == "INPUT")
+                    if (semantic == "INPUT") {
                         pChannel.mSourceTimes = source;
-                    else if (semantic == "OUTPUT")
+                    } else if (semantic == "OUTPUT") {
                         pChannel.mSourceValues = source;
-                    else if (semantic == "IN_TANGENT")
+                    } else if (semantic == "IN_TANGENT") {
                         pChannel.mInTanValues = source;
-                    else if (semantic == "OUT_TANGENT")
+                    } else if (semantic == "OUT_TANGENT") {
                         pChannel.mOutTanValues = source;
-                    else if (semantic == "INTERPOLATION")
+                    } else if (semantic == "INTERPOLATION") {
                         pChannel.mInterpolationValues = source;
+                    }
                 }
             }
         }
@@ -604,7 +609,6 @@ void ColladaParser::ReadControllerLibrary(XmlNode &node) {
         const std::string &currentName = currentNode.name();
         if (currentName != "controller") {
             continue;
-            ;
         }
         std::string id = node.attribute("id").as_string();
         mControllerLibrary[id] = Controller();
@@ -618,7 +622,7 @@ void ColladaParser::ReadController(XmlNode &node, Collada::Controller &pControll
     // initial values
     pController.mType = Skin;
     pController.mMethod = Normalized;
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
+    for (XmlNode &currentNode : node.children()) {
         const std::string &currentName = currentNode.name();
         if (currentName == "morph") {
             pController.mType = Morph;
@@ -670,8 +674,8 @@ void ColladaParser::ReadController(XmlNode &node, Collada::Controller &pControll
 // ------------------------------------------------------------------------------------------------
 // Reads the joint definitions for the given controller
 void ColladaParser::ReadControllerJoints(XmlNode &node, Collada::Controller &pController) {
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
-        const std::string currentName = currentNode.name();
+    for (XmlNode &currentNode : node.children()) {
+        const std::string &currentName = currentNode.name();
         if (currentName == "input") {
             const char *attrSemantic = currentNode.attribute("semantic").as_string();
             const char *attrSource = currentNode.attribute("source").as_string();
@@ -698,8 +702,8 @@ void ColladaParser::ReadControllerWeights(XmlNode &node, Collada::Controller &pC
     int vertexCount=0;
     XmlParser::getIntAttribute(node, "count", vertexCount);
 
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
-        std::string currentName = currentNode.name();
+    for (XmlNode &currentNode : node.children()) {
+        const std::string &currentName = currentNode.name();
         if (currentName == "input") {
             InputChannel channel;
 
@@ -763,9 +767,9 @@ void ColladaParser::ReadImageLibrary(XmlNode &node) {
         return;
     }
 
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
-        const std::string name = currentNode.name();
-        if (name == "image") {
+    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();
 
@@ -778,7 +782,7 @@ void ColladaParser::ReadImageLibrary(XmlNode &node) {
 // ------------------------------------------------------------------------------------------------
 // Reads an image entry into the given image
 void ColladaParser::ReadImage(XmlNode &node, Collada::Image &pImage) {
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
+    for (XmlNode &currentNode : node.children()) {
         const std::string currentName = currentNode.name();
         if (currentName == "image") {
             // Ignore
@@ -798,23 +802,6 @@ void ColladaParser::ReadImage(XmlNode &node, Collada::Image &pImage) {
                 if (!pImage.mFileName.length()) {
                     pImage.mFileName = "unknown_texture";
                 }
-            } else if (mFormat == FV_1_5_n) {
-                // make sure we skip over mip and array initializations, which
-                // we don't support, but which could confuse the loader if
-                // they're not skipped.
-                //int v = currentNode.attribute("ref").as_int();
-                /*                if (v y) {
-                    ASSIMP_LOG_WARN("Collada: Ignoring texture array index");
-                    continue;
-                }*/
-
-                //v = currentNode.attribute("mip_index").as_int();
-                /*if (attrib != -1 && v > 0) {
-                    ASSIMP_LOG_WARN("Collada: Ignoring MIP map layer");
-                    continue;
-                }*/
-
-                // TODO: correctly jump over cube and volume maps?
             }
         } else if (mFormat == FV_1_5_n) {
             std::string value;
@@ -861,8 +848,7 @@ void ColladaParser::ReadMaterialLibrary(XmlNode &node) {
     }
 
     std::map<std::string, int> names;
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
-        const std::string currentName = currentNode.name();
+    for (XmlNode &currentNode : node.children()) {
         std::string id = currentNode.attribute("id").as_string();
         std::string name = currentNode.attribute("name").as_string();
         mMaterialLibrary[id] = Material();
@@ -891,11 +877,13 @@ void ColladaParser::ReadLightLibrary(XmlNode &node) {
         return;
     }
 
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
+    for (XmlNode &currentNode : node.children()) {
         const std::string &currentName = currentNode.name();
         if (currentName == "light") {
-            std::string id = currentNode.attribute("id").as_string();
-            ReadLight(currentNode, mLightLibrary[id] = Light());
+            std::string id;
+            if (XmlParser::getStdStrAttribute(currentNode, "id", id)) {
+                ReadLight(currentNode, mLightLibrary[id] = Light());
+            }
         }
     }
 }
@@ -907,18 +895,24 @@ void ColladaParser::ReadCameraLibrary(XmlNode &node) {
         return;
     }
 
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
+    for (XmlNode &currentNode : node.children()) {
         const std::string &currentName = currentNode.name();
         if (currentName == "camera") {
-            std::string id = currentNode.attribute("id").as_string();
+            std::string id;
+            if (!XmlParser::getStdStrAttribute(currentNode, "id", id)) {
+                continue;
+            }
 
             // create an entry and store it in the library under its ID
             Camera &cam = mCameraLibrary[id];
-            std::string name = currentNode.attribute("name").as_string();
+            std::string name;
+            if (!XmlParser::getStdStrAttribute(currentNode, "name", name)) {
+                continue;
+            }
             if (!name.empty()) {
                 cam.mName = name;
             }
-            ReadCamera(currentNode, cam);
+            ReadCamera(currentNode, cam);            
         }
     }
 }
@@ -926,14 +920,12 @@ void ColladaParser::ReadCameraLibrary(XmlNode &node) {
 // ------------------------------------------------------------------------------------------------
 // Reads a material entry into the given material
 void ColladaParser::ReadMaterial(XmlNode &node, Collada::Material &pMaterial) {
-    for (XmlNode currentNode : node.children()) {
+    for (XmlNode &currentNode : node.children()) {
         const std::string &currentName = currentNode.name();
         if (currentName == "instance_effect") {
-            const char *url = currentNode.attribute("url").as_string();
-            if (url[0] != '#') {
-                throw DeadlyImportError("Unknown reference format");
-            }
-            pMaterial.mEffect = url + 1;
+            std::string url;
+            readUrlAttribute(currentNode, url);
+            pMaterial.mEffect = url.c_str();
         }
     }
 }
@@ -1032,7 +1024,7 @@ void ColladaParser::ReadEffectLibrary(XmlNode &node) {
         return;
     }
 
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
+    for (XmlNode &currentNode : node.children()) {
         const std::string &currentName = currentNode.name();
         if (currentName == "effect") {
             // read ID. Do I have to repeat my ranting about "optional" attributes?
@@ -1051,7 +1043,7 @@ void ColladaParser::ReadEffectLibrary(XmlNode &node) {
 // ------------------------------------------------------------------------------------------------
 // Reads an effect entry into the given effect
 void ColladaParser::ReadEffect(XmlNode &node, Collada::Effect &pEffect) {
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
+    for (XmlNode &currentNode : node.children()) {
         const std::string &currentName = currentNode.name();
         if (currentName == "profile_COMMON") {
             ReadEffectProfileCommon(currentNode, pEffect);
@@ -1256,8 +1248,6 @@ void ColladaParser::ReadEffectColor(XmlNode &node, aiColor4D &pColor, Sampler &p
         } else if (currentName == "technique") {
             std::string profile;
             XmlParser::getStdStrAttribute(currentNode, "profile", profile);
-            //const int _profile = GetAttribute("profile");
-            //const char *profile = mReader->getAttributeValue(_profile);
 
             // Some extensions are quite useful ... ReadSamplerProperties processes
             // several extensions in MAYA, OKINO and MAX3D profiles.
@@ -1330,7 +1320,7 @@ void ColladaParser::ReadGeometryLibrary(XmlNode &node) {
     if (node.empty()) {
         return;
     }
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
+    for (XmlNode &currentNode : node.children()) {
         const std::string &currentName = currentNode.name();
         if (currentName == "geometry") {
             // read ID. Another entry which is "optional" by design but obligatory in reality
@@ -1359,7 +1349,7 @@ void ColladaParser::ReadGeometry(XmlNode &node, Collada::Mesh &pMesh) {
     if (node.empty()) {
         return;
     }
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
+    for (XmlNode &currentNode : node.children()) {
         const std::string &currentName = currentNode.name();
         if (currentName == "mesh") {
             ReadMesh(currentNode, pMesh);
@@ -1383,7 +1373,9 @@ void ColladaParser::ReadMesh(XmlNode &node, Mesh &pMesh) {
             ReadSource(currentNode);
         } else if (currentName == "vertices") {
             ReadVertexData(currentNode, pMesh);
-        } else if (currentName == "triangles" || currentName == "lines" || currentName == "linestrips" || currentName == "polygons" || currentName == "polylist" || currentName == "trifans" || currentName == "tristrips") {
+        } else if (currentName == "triangles" || currentName == "lines" || currentName == "linestrips" ||
+                currentName == "polygons" || currentName == "polylist" || currentName == "trifans" ||
+                currentName == "tristrips") {
             ReadIndexData(currentNode, pMesh);
         }
     }
@@ -1541,16 +1533,11 @@ void ColladaParser::ReadAccessor(XmlNode &node, const std::string &pID) {
                     acc.mSubOffset[1] = acc.mParams.size();
                 else if (name == "P")
                     acc.mSubOffset[2] = acc.mParams.size();
-                //  else if( name == "Q") acc.mSubOffset[3] = acc.mParams.size();
-                /* 4D uv coordinates are not supported in Assimp */
-
                 /* Generic extra data, interpreted as UV data, too*/
                 else if (name == "U")
                     acc.mSubOffset[0] = acc.mParams.size();
                 else if (name == "V")
                     acc.mSubOffset[1] = acc.mParams.size();
-                //else
-                //  DefaultLogger::get()->warn( format() << "Unknown accessor parameter \"" << name << "\". Ignoring data channel." );
             }
             if (XmlParser::hasAttribute(currentNode, "type")) {
                 // read data type
@@ -1575,7 +1562,7 @@ void ColladaParser::ReadAccessor(XmlNode &node, const std::string &pID) {
 void ColladaParser::ReadVertexData(XmlNode &node, Mesh &pMesh) {
     // extract the ID of the <vertices> element. Not that we care, but to catch strange referencing schemes we should warn about
     XmlParser::getStdStrAttribute(node, "id", pMesh.mVertexID);
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
+    for (XmlNode &currentNode : node.children()) {
         const std::string &currentName = currentNode.name();
         if (currentName == "input") {
             ReadInputChannel(currentNode, pMesh.mPerVertexData);
@@ -1733,20 +1720,20 @@ size_t ColladaParser::ReadPrimitives(XmlNode &node, Mesh &pMesh, std::vector<Inp
     // determine the expected number of indices
     size_t expectedPointCount = 0;
     switch (pPrimType) {
-    case Prim_Polylist: {
-        for (size_t i : pVCount)
-            expectedPointCount += i;
-        break;
-    }
-    case Prim_Lines:
-        expectedPointCount = 2 * pNumPrimitives;
-        break;
-    case Prim_Triangles:
-        expectedPointCount = 3 * pNumPrimitives;
-        break;
-    default:
-        // other primitive types don't state the index count upfront... we need to guess
-        break;
+        case Prim_Polylist: {
+            for (size_t i : pVCount)
+                expectedPointCount += i;
+            break;
+        }
+        case Prim_Lines:
+            expectedPointCount = 2 * pNumPrimitives;
+            break;
+        case Prim_Triangles:
+            expectedPointCount = 3 * pNumPrimitives;
+            break;
+        default:
+            // other primitive types don't state the index count upfront... we need to guess
+            break;
     }
 
     // and read all indices into a temporary array
@@ -1778,7 +1765,6 @@ size_t ColladaParser::ReadPrimitives(XmlNode &node, Mesh &pMesh, std::vector<Inp
         } else {
             throw DeadlyImportError("Expected different index count in <p> element.");
         }
-
     } else if (expectedPointCount == 0 && (indices.size() % numOffsets) != 0) {
         throw DeadlyImportError("Expected different index count in <p> element.");
     }
@@ -2030,7 +2016,7 @@ void ColladaParser::ReadSceneLibrary(XmlNode &node) {
         return;
     }
 
-    for (XmlNode currentNode : node.children()) {
+    for (XmlNode &currentNode : node.children()) {
         const std::string &currentName = currentNode.name();
         if (currentName == "visual_scene") {
             // read ID. Is optional according to the spec, but how on earth should a scene_instance refer to it then?
@@ -2197,11 +2183,8 @@ void ColladaParser::ReadNodeTransformation(XmlNode &node, Node *pNode, Transform
 // ------------------------------------------------------------------------------------------------
 // Processes bind_vertex_input and bind elements
 void ColladaParser::ReadMaterialVertexInputBinding(XmlNode &node, Collada::SemanticMappingTable &tbl) {
-    //XmlNodeIterator xmlIt(node);
-    //xmlIt.collectChildrenPreOrder(node);
-    //XmlNode currentNode;
     std::string name = node.name();
-    for (XmlNode currentNode : node.children()) {
+    for (XmlNode &currentNode : node.children()) {
         const std::string &currentName = currentNode.name();
         if (currentName == "bind_vertex_input") {
             Collada::InputSemanticMapEntry vn;
@@ -2296,8 +2279,8 @@ void ColladaParser::ReadScene(XmlNode &node) {
         return;
     }
 
-    for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) {
-        const std::string currentName = currentNode.name();
+    for (XmlNode &currentNode : node.children()) {
+        const std::string &currentName = currentNode.name();
         if (currentName == "instance_visual_scene") {
             // should be the first and only occurrence
             if (mRootNode) {
@@ -2321,20 +2304,6 @@ void ColladaParser::ReadScene(XmlNode &node) {
     }
 }
 
-void ColladaParser::ReportWarning(const char *msg, ...) {
-    ai_assert(nullptr != msg);
-
-    va_list args;
-    va_start(args, msg);
-
-    char szBuffer[3000];
-    const int iLen = vsprintf(szBuffer, msg, args);
-    ai_assert(iLen > 0);
-
-    va_end(args);
-    ASSIMP_LOG_WARN_F("Validation warning: ", std::string(szBuffer, iLen));
-}
-
 // ------------------------------------------------------------------------------------------------
 // Calculates the resulting transformation from all the given transform steps
 aiMatrix4x4 ColladaParser::CalculateResultTransform(const std::vector<Transform> &pTransforms) const {

code/AssetLib/Collada/ColladaParser.h

@@ -243,8 +243,6 @@ protected:
     void ReadEmbeddedTextures(ZipArchiveIOSystem &zip_archive);
 
 protected:
-    void ReportWarning(const char *msg, ...);
-
     /** Calculates the resulting transformation from all the given transform steps */
     aiMatrix4x4 CalculateResultTransform(const std::vector<Collada::Transform> &pTransforms) const;
 
@@ -260,56 +258,55 @@ protected:
     std::string mFileName;
 
     // XML reader, member for everyday use
-    //irr::io::IrrXMLReader *mReader;
     XmlParser mXmlParser;
 
     /** All data arrays found in the file by ID. Might be referred to by actually
          everyone. Collada, you are a steaming pile of indirection. */
-    typedef std::map<std::string, Collada::Data> DataLibrary;
+    using DataLibrary = std::map<std::string, Collada::Data> ;
     DataLibrary mDataLibrary;
 
     /** Same for accessors which define how the data in a data array is accessed. */
-    typedef std::map<std::string, Collada::Accessor> AccessorLibrary;
+    using AccessorLibrary = std::map<std::string, Collada::Accessor> ;
     AccessorLibrary mAccessorLibrary;
 
     /** Mesh library: mesh by ID */
-    typedef std::map<std::string, Collada::Mesh *> MeshLibrary;
+    using MeshLibrary = std::map<std::string, Collada::Mesh *>;
     MeshLibrary mMeshLibrary;
 
     /** node library: root node of the hierarchy part by ID */
-    typedef std::map<std::string, Collada::Node *> NodeLibrary;
+    using NodeLibrary = std::map<std::string, Collada::Node *>;
     NodeLibrary mNodeLibrary;
 
     /** Image library: stores texture properties by ID */
-    typedef std::map<std::string, Collada::Image> ImageLibrary;
+    using ImageLibrary = std::map<std::string, Collada::Image> ;
     ImageLibrary mImageLibrary;
 
     /** Effect library: surface attributes by ID */
-    typedef std::map<std::string, Collada::Effect> EffectLibrary;
+    using EffectLibrary = std::map<std::string, Collada::Effect> ;
     EffectLibrary mEffectLibrary;
 
     /** Material library: surface material by ID */
-    typedef std::map<std::string, Collada::Material> MaterialLibrary;
+    using MaterialLibrary = std::map<std::string, Collada::Material> ;
     MaterialLibrary mMaterialLibrary;
 
     /** Light library: surface light by ID */
-    typedef std::map<std::string, Collada::Light> LightLibrary;
+    using LightLibrary = std::map<std::string, Collada::Light> ;
     LightLibrary mLightLibrary;
 
     /** Camera library: surface material by ID */
-    typedef std::map<std::string, Collada::Camera> CameraLibrary;
+    using CameraLibrary = std::map<std::string, Collada::Camera> ;
     CameraLibrary mCameraLibrary;
 
     /** Controller library: joint controllers by ID */
-    typedef std::map<std::string, Collada::Controller> ControllerLibrary;
+    using ControllerLibrary = std::map<std::string, Collada::Controller> ;
     ControllerLibrary mControllerLibrary;
 
     /** Animation library: animation references by ID */
-    typedef std::map<std::string, Collada::Animation *> AnimationLibrary;
+    using AnimationLibrary = std::map<std::string, Collada::Animation *> ;
     AnimationLibrary mAnimationLibrary;
 
     /** Animation clip library: clip animation references by ID */
-    typedef std::vector<std::pair<std::string, std::vector<std::string>>> AnimationClipLibrary;
+    using AnimationClipLibrary = std::vector<std::pair<std::string, std::vector<std::string>>> ;
     AnimationClipLibrary mAnimationClipLibrary;
 
     /** Pointer to the root node. Don't delete, it just points to one of
@@ -334,13 +331,6 @@ protected:
     Collada::FormatVersion mFormat;
 };
 
-// ------------------------------------------------------------------------------------------------
-// Check for element match
-/*inline bool ColladaParser::IsElement(const char *pName) const {
-    ai_assert(mReader->getNodeType() == irr::io::EXN_ELEMENT);
-    return ::strcmp(mReader->getNodeName(), pName) == 0;
-}*/
-
 // ------------------------------------------------------------------------------------------------
 // Finds the item in the given library by its reference, throws if not found
 template <typename Type>

include/assimp/StringUtils.h

@@ -44,22 +44,22 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #define INCLUDED_AI_STRINGUTILS_H
 
 #ifdef __GNUC__
-#   pragma GCC system_header
+#pragma GCC system_header
 #endif
 
 #include <assimp/defs.h>
 
-#include <sstream>
 #include <stdarg.h>
-#include <cstdlib>
-#include <algorithm> 
+#include <algorithm>
 #include <cctype>
+#include <cstdlib>
 #include <locale>
+#include <sstream>
 
 #ifdef _MSC_VER
-#   define AI_SIZEFMT "%Iu"
+#define AI_SIZEFMT "%Iu"
 #else
-#   define AI_SIZEFMT "%zu"
+#define AI_SIZEFMT "%zu"
 #endif
 
 ///	@fn		ai_snprintf
@@ -71,35 +71,35 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 ///	@return	The number of written characters if the buffer size was big enough. If an encoding error occurs, a negative number is returned.
 #if defined(_MSC_VER) && _MSC_VER < 1900
 
-    AI_FORCE_INLINE
-    int c99_ai_vsnprintf(char *outBuf, size_t size, const char *format, va_list ap) {
-		int count(-1);
-		if (0 != size) {
-			count = _vsnprintf_s(outBuf, size, _TRUNCATE, format, ap);
-		}
-		if (count == -1) {
-			count = _vscprintf(format, ap);
-		}
+AI_FORCE_INLINE
+int c99_ai_vsnprintf(char *outBuf, size_t size, const char *format, va_list ap) {
+    int count(-1);
+    if (0 != size) {
+        count = _vsnprintf_s(outBuf, size, _TRUNCATE, format, ap);
+    }
+    if (count == -1) {
+        count = _vscprintf(format, ap);
+    }
 
-		return count;
-	}
+    return count;
+}
 
-    AI_FORCE_INLINE
-    int ai_snprintf(char *outBuf, size_t size, const char *format, ...) {
-		int count;
-		va_list ap;
+AI_FORCE_INLINE
+int ai_snprintf(char *outBuf, size_t size, const char *format, ...) {
+    int count;
+    va_list ap;
 
-		va_start(ap, format);
-		count = c99_ai_vsnprintf(outBuf, size, format, ap);
-		va_end(ap);
+    va_start(ap, format);
+    count = c99_ai_vsnprintf(outBuf, size, format, ap);
+    va_end(ap);
 
-		return count;
-	}
+    return count;
+}
 
 #elif defined(__MINGW32__)
-#   define ai_snprintf __mingw_snprintf
+#define ai_snprintf __mingw_snprintf
 #else
-#   define ai_snprintf snprintf
+#define ai_snprintf snprintf
 #endif
 
 ///	@fn		to_string
@@ -107,8 +107,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 ///	@param	value   The value to write into the std::string.
 ///	@return	The value as a std::string
 template <typename T>
-AI_FORCE_INLINE
-std::string to_string( T value ) {
+AI_FORCE_INLINE std::string to_string(T value) {
     std::ostringstream os;
     os << value;
 
@@ -121,17 +120,17 @@ std::string to_string( T value ) {
 /// @param  end     The last character
 ///	@return	The float value, 0.0f in cas of an error.
 AI_FORCE_INLINE
-float ai_strtof( const char *begin, const char *end ) {
-    if ( nullptr == begin ) {
+float ai_strtof(const char *begin, const char *end) {
+    if (nullptr == begin) {
         return 0.0f;
     }
-    float val( 0.0f );
-    if ( nullptr == end ) {
-        val = static_cast< float >( ::atof( begin ) );
+    float val(0.0f);
+    if (nullptr == end) {
+        val = static_cast<float>(::atof(begin));
     } else {
-        std::string::size_type len( end - begin );
-        std::string token( begin, len );
-        val = static_cast< float >( ::atof( token.c_str() ) );
+        std::string::size_type len(end - begin);
+        std::string token(begin, len);
+        val = static_cast<float>(::atof(token.c_str()));
     }
 
     return val;
@@ -141,16 +140,15 @@ float ai_strtof( const char *begin, const char *end ) {
 ///	@brief	The portable to convert a decimal value into a hexadecimal string.
 ///	@param	toConvert   Value to convert
 ///	@return	The hexadecimal string, is empty in case of an error.
-template<class T>
-AI_FORCE_INLINE
-std::string DecimalToHexa( T toConvert ) {
+template <class T>
+AI_FORCE_INLINE std::string DecimalToHexa(T toConvert) {
     std::string result;
     std::stringstream ss;
     ss << std::hex << toConvert;
     ss >> result;
 
-    for ( size_t i = 0; i < result.size(); ++i ) {
-        result[ i ] = (char) toupper( result[ i ] );
+    for (size_t i = 0; i < result.size(); ++i) {
+        result[i] = (char)toupper(result[i]);
     }
 
     return result;
@@ -164,31 +162,32 @@ std::string DecimalToHexa( T toConvert ) {
 ///	@param	with_head   #
 ///	@return	The hexadecimal string, is empty in case of an error.
 AI_FORCE_INLINE std::string Rgba2Hex(int r, int g, int b, int a, bool with_head) {
-	std::stringstream ss;
-	if (with_head) {
-		ss << "#";
+    std::stringstream ss;
+    if (with_head) {
+        ss << "#";
     }
-	ss << std::hex << (r << 24 | g << 16 | b << 8 | a);
+    ss << std::hex << (r << 24 | g << 16 | b << 8 | a);
 
     return ss.str();
 }
 
 // trim from start (in place)
-inline void ltrim(std::string &s) {
+AI_FORCE_INLINE void ltrim(std::string &s) {
     s.erase(s.begin(), std::find_if(s.begin(), s.end(), [](unsigned char ch) {
         return !std::isspace(ch);
     }));
 }
 
 // trim from end (in place)
-inline void rtrim(std::string &s) {
+AI_FORCE_INLINE void rtrim(std::string &s) {
     s.erase(std::find_if(s.rbegin(), s.rend(), [](unsigned char ch) {
         return !std::isspace(ch);
-    }).base(), s.end());
+    }).base(),
+            s.end());
 }
 
 // trim from both ends (in place)
-inline void trim(std::string &s) {
+AI_FORCE_INLINE void trim(std::string &s) {
     ltrim(s);
     rtrim(s);
 }

include/assimp/XmlParser.h

@@ -179,7 +179,7 @@ public:
         return true;
     }
 
-    static inline bool getFloatAttribute( XmlNode &xmlNode, const char *name, float &val ) {
+    static inline bool getFloatAttribute(XmlNode &xmlNode, const char *name, float &val ) {
         pugi::xml_attribute attr = xmlNode.attribute(name);
         if (attr.empty()) {
             return false;
@@ -190,6 +190,16 @@ public:
 
     }
 
+    static inline bool getDoubleAttribute( XmlNode &xmlNode, const char *name, double &val ) {
+        pugi::xml_attribute attr = xmlNode.attribute(name);
+        if (attr.empty()) {
+            return false;
+        }
+
+        val = attr.as_double();
+        return true;
+    }
+
     static inline bool getStdStrAttribute(XmlNode &xmlNode, const char *name, std::string &val) {
         pugi::xml_attribute attr = xmlNode.attribute(name);
         if (attr.empty()) {