From fa2354ebc3e5eaaaf3f5af75a23a66e44e1cd61f Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 2 Feb 2021 22:06:33 +0100 Subject: [PATCH] Fix incorrect xml-parsing in collada importer. --- code/AssetLib/Collada/ColladaHelper.h | 3 +- code/AssetLib/Collada/ColladaLoader.cpp | 5 +- code/AssetLib/Collada/ColladaLoader.h | 130 +++++++++++------------- code/AssetLib/Collada/ColladaParser.cpp | 114 ++++++++++----------- 4 files changed, 123 insertions(+), 129 deletions(-) diff --git a/code/AssetLib/Collada/ColladaHelper.h b/code/AssetLib/Collada/ColladaHelper.h index bfd57918e..2448dc2fe 100644 --- a/code/AssetLib/Collada/ColladaHelper.h +++ b/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 mMap; + using InputSemanticMap = std::map; + InputSemanticMap mMap; /// For std::find bool operator==(const std::string &s) const { diff --git a/code/AssetLib/Collada/ColladaLoader.cpp b/code/AssetLib/Collada/ColladaLoader.cpp index 96599f22f..466f0137f 100644 --- a/code/AssetLib/Collada/ColladaLoader.cpp +++ b/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::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::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) { diff --git a/code/AssetLib/Collada/ColladaLoader.h b/code/AssetLib/Collada/ColladaLoader.h index 198b7a215..e425430cf 100644 --- a/code/AssetLib/Collada/ColladaLoader.h +++ b/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 #include "ColladaParser.h" +#include 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) - { } + 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) + 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& resolved); + void ResolveNodeInstances(const ColladaParser &pParser, const Collada::Node *pNode, + std::vector &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& poNodes) const; + void CollectNodes(const aiNode *pNode, std::vector &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 mMaterialIndexByName; /** Accumulated meshes for the target scene */ - std::vector mMeshes; - + std::vector mMeshes; + /** Accumulated morph target meshes */ - std::vector mTargetMeshes; + std::vector mTargetMeshes; /** Temporary material list */ - std::vector > newMats; + std::vector> newMats; /** Temporary camera list */ - std::vector mCameras; + std::vector mCameras; /** Temporary light list */ - std::vector mLights; + std::vector mLights; /** Temporary texture list */ - std::vector mTextures; + std::vector mTextures; /** Accumulated animations for the target scene */ - std::vector mAnims; + std::vector mAnims; bool noSkeletonMesh; bool ignoreUpDirection; diff --git a/code/AssetLib/Collada/ColladaParser.cpp b/code/AssetLib/Collada/ColladaParser.cpp index f78b51047..b700f14c4 100644 --- a/code/AssetLib/Collada/ColladaParser.cpp +++ b/code/AssetLib/Collada/ColladaParser.cpp @@ -54,6 +54,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include #include +#include 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(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 ¤tNode : node.children()) { - const std::string ¤tName = std::string(currentNode.name()); + const std::string ¤tName = currentNode.name(); if (currentName == "asset") { ReadAssetInfo(currentNode); } else if (currentName == "library_animations") { @@ -407,7 +408,7 @@ void ColladaParser::ReadAnimationClipLibrary(XmlNode &node) { const std::string ¤tName = 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::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 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 ; + using ChannelMap = std::map; 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 ¤tNode : node.children()) { + controller.mType = Skin; + controller.mMethod = Normalized; + + XmlNodeIterator xmlIt(node); + xmlIt.collectChildrenPreOrder(node); + XmlNode currentNode; + while (xmlIt.getNext(currentNode)) { + + //for (XmlNode ¤tNode : node.children()) { const std::string ¤tName = 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(content, pController.mBindShapeMatrix[a]); + content = fast_atoreal_move(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 ¤tChildName = 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 ¤tNode : node.children()) { const std::string ¤tName = currentNode.name(); @@ -725,7 +736,7 @@ void ColladaParser::ReadControllerWeights(XmlNode &node, Collada::Controller &pC throw DeadlyImportError("Unknown semantic \"", attrSemantic, "\" in data 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::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 ¤tNode : node.children()) { const std::string ¤tName = 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 names; for (XmlNode ¤tNode : 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 ¤tNode : node.children()) { const std::string ¤tName = 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 ¤tNode : node.children()) { const std::string ¤tName = currentNode.name(); if (currentName == "camera") {