From 906321689315f0245cff1c73921e9d7c87bdee6a Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Sat, 2 Jul 2022 21:21:31 +0200 Subject: [PATCH] Fix a memory leak --- code/AssetLib/3MF/3MFXmlTags.h | 2 + code/AssetLib/3MF/XmlSerializer.cpp | 25 +- code/AssetLib/Irr/IRRLoader.cpp | 2 +- code/Common/ScenePreprocessor.cpp | 57 ++-- include/assimp/XmlParser.h | 464 ++++++++++++++++------------ 5 files changed, 326 insertions(+), 224 deletions(-) diff --git a/code/AssetLib/3MF/3MFXmlTags.h b/code/AssetLib/3MF/3MFXmlTags.h index 63f18b455..333d169aa 100644 --- a/code/AssetLib/3MF/3MFXmlTags.h +++ b/code/AssetLib/3MF/3MFXmlTags.h @@ -74,6 +74,8 @@ namespace XmlTag { const char* const pid = "pid"; const char* const pindex = "pindex"; const char* const p1 = "p1"; + const char *const p2 = "p2"; + const char *const p3 = "p3"; const char* const name = "name"; const char* const type = "type"; const char* const build = "build"; diff --git a/code/AssetLib/3MF/XmlSerializer.cpp b/code/AssetLib/3MF/XmlSerializer.cpp index 740bc5658..9d7a1800f 100644 --- a/code/AssetLib/3MF/XmlSerializer.cpp +++ b/code/AssetLib/3MF/XmlSerializer.cpp @@ -64,7 +64,7 @@ bool validateColorString(const char *color) { return true; } -aiFace ReadTriangle(XmlNode &node) { +aiFace ReadTriangle(XmlNode &node, unsigned int &texId0, unsigned int &texId1, unsigned int &texId2) { aiFace face; face.mNumIndices = 3; @@ -73,6 +73,10 @@ aiFace ReadTriangle(XmlNode &node) { face.mIndices[1] = static_cast(std::atoi(node.attribute(XmlTag::v2).as_string())); face.mIndices[2] = static_cast(std::atoi(node.attribute(XmlTag::v3).as_string())); + texId0 = static_cast(std::atoi(node.attribute(XmlTag::p1).as_string())); + texId1 = static_cast(std::atoi(node.attribute(XmlTag::p2).as_string())); + texId2 = static_cast(std::atoi(node.attribute(XmlTag::p3).as_string())); + return face; } @@ -412,6 +416,8 @@ void XmlSerializer::ImportTriangles(XmlNode &node, aiMesh *mesh) { bool hasPid = getNodeAttribute(currentNode, D3MF::XmlTag::pid, pid); bool hasP1 = getNodeAttribute(currentNode, D3MF::XmlTag::p1, p1); + unsigned int texId[3]; + aiFace face = ReadTriangle(currentNode, texId[0], texId[1], texId[2]); if (hasPid && hasP1) { auto it = mResourcesDictionnary.find(pid); if (it != mResourcesDictionnary.end()) { @@ -420,6 +426,11 @@ void XmlSerializer::ImportTriangles(XmlNode &node, aiMesh *mesh) { mesh->mMaterialIndex = baseMaterials->mMaterialIndex[p1]; } else if (it->second->getType() == ResourceType::RT_Texture2DGroup) { if (mesh->mTextureCoords[0] == nullptr) { + mesh->mNumUVComponents[0] = 2; + for (unsigned int i = 1; i < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++i) { + mesh->mNumUVComponents[i] = 0; + } + Texture2DGroup *group = static_cast(it->second); const std::string name = ai_to_string(group->mTexId); for (size_t i = 0; i < mMaterials.size(); ++i) { @@ -427,8 +438,9 @@ void XmlSerializer::ImportTriangles(XmlNode &node, aiMesh *mesh) { mesh->mMaterialIndex = static_cast(i); } } - mesh->mTextureCoords[0] = new aiVector3D[group->mTex2dCoords.size()]; - for (unsigned int i = 0; i < group->mTex2dCoords.size(); ++i) { + mesh->mTextureCoords[0] = new aiVector3D[mesh->mNumVertices]; + for (unsigned int i = 0; i < mesh->mNumVertices; ++i) { + mesh->mTextureCoords[0][i] = aiVector3D(group->mTex2dCoords[i].x, group->mTex2dCoords[i].y, 0); } } @@ -436,7 +448,6 @@ void XmlSerializer::ImportTriangles(XmlNode &node, aiMesh *mesh) { } } - aiFace face = ReadTriangle(currentNode); faces.push_back(face); } } @@ -578,11 +589,15 @@ aiMaterial *XmlSerializer::readMaterialDef(XmlNode &node, unsigned int basemater } void XmlSerializer::StoreMaterialsInScene(aiScene *scene) { - if (nullptr == scene || mMaterials.empty()) { + if (nullptr == scene) { return; } scene->mNumMaterials = static_cast(mMaterials.size()); + if (scene->mNumMaterials == 0) { + return; + } + scene->mMaterials = new aiMaterial *[scene->mNumMaterials]; for (size_t i = 0; i < mMaterials.size(); ++i) { scene->mMaterials[i] = mMaterials[i]; diff --git a/code/AssetLib/Irr/IRRLoader.cpp b/code/AssetLib/Irr/IRRLoader.cpp index 0061634a6..b16565559 100644 --- a/code/AssetLib/Irr/IRRLoader.cpp +++ b/code/AssetLib/Irr/IRRLoader.cpp @@ -874,7 +874,7 @@ void IRRImporter::InternReadFile(const std::string &pFile, aiScene *pScene, IOSy // Batch loader used to load external models BatchLoader batch(pIOHandler); - // batch.SetBasePath(pFile); + //batch.SetBasePath(pFile); cameras.reserve(5); lights.reserve(5); diff --git a/code/Common/ScenePreprocessor.cpp b/code/Common/ScenePreprocessor.cpp index 2ef291eeb..60133f651 100644 --- a/code/Common/ScenePreprocessor.cpp +++ b/code/Common/ScenePreprocessor.cpp @@ -105,36 +105,39 @@ void ScenePreprocessor::ProcessMesh(aiMesh *mesh) { for (unsigned int i = 0; i < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++i) { if (!mesh->mTextureCoords[i]) { mesh->mNumUVComponents[i] = 0; - } else { - if (!mesh->mNumUVComponents[i]) { - mesh->mNumUVComponents[i] = 2; + continue; + } + + if (!mesh->mNumUVComponents[i]) { + mesh->mNumUVComponents[i] = 2; + } + + aiVector3D *p = mesh->mTextureCoords[i], *end = p + mesh->mNumVertices; + + // Ensure unused components are zeroed. This will make 1D texture channels work + // as if they were 2D channels .. just in case an application doesn't handle + // this case + if (2 == mesh->mNumUVComponents[i]) { + size_t num = 0; + for (; p != end; ++p) { + p->z = 0.f; + num++; } - - aiVector3D *p = mesh->mTextureCoords[i], *end = p + mesh->mNumVertices; - - // Ensure unused components are zeroed. This will make 1D texture channels work - // as if they were 2D channels .. just in case an application doesn't handle - // this case - if (2 == mesh->mNumUVComponents[i]) { - for (; p != end; ++p) { - p->z = 0.f; - } - } else if (1 == mesh->mNumUVComponents[i]) { - for (; p != end; ++p) { - p->z = p->y = 0.f; - } - } else if (3 == mesh->mNumUVComponents[i]) { - // Really 3D coordinates? Check whether the third coordinate is != 0 for at least one element - for (; p != end; ++p) { - if (p->z != 0) { - break; - } - } - if (p == end) { - ASSIMP_LOG_WARN("ScenePreprocessor: UVs are declared to be 3D but they're obviously not. Reverting to 2D."); - mesh->mNumUVComponents[i] = 2; + } else if (1 == mesh->mNumUVComponents[i]) { + for (; p != end; ++p) { + p->z = p->y = 0.f; + } + } else if (3 == mesh->mNumUVComponents[i]) { + // Really 3D coordinates? Check whether the third coordinate is != 0 for at least one element + for (; p != end; ++p) { + if (p->z != 0) { + break; } } + if (p == end) { + ASSIMP_LOG_WARN("ScenePreprocessor: UVs are declared to be 3D but they're obviously not. Reverting to 2D."); + mesh->mNumUVComponents[i] = 2; + } } } diff --git a/include/assimp/XmlParser.h b/include/assimp/XmlParser.h index 28f2be8e8..67dfdeebe 100644 --- a/include/assimp/XmlParser.h +++ b/include/assimp/XmlParser.h @@ -99,295 +99,129 @@ template class TXmlParser { public: /// @brief The default class constructor. - TXmlParser() : - mDoc(nullptr), - mData() { - // empty - } + TXmlParser(); /// @brief The class destructor. - ~TXmlParser() { - clear(); - } + ~TXmlParser(); /// @brief Will clear the parsed xml-file. - void clear() { - if (mData.empty()) { - mDoc = nullptr; - return; - } - mData.clear(); - delete mDoc; - mDoc = nullptr; - } + void clear(); /// @brief Will search for a child-node by its name /// @param name [in] The name of the child-node. /// @return The node instance or nullptr, if nothing was found. - TNodeType *findNode(const std::string &name) { - if (name.empty()) { - return nullptr; - } - - if (nullptr == mDoc) { - return nullptr; - } - - find_node_by_name_predicate predicate(name); - mCurrent = mDoc->find_node(predicate); - if (mCurrent.empty()) { - return nullptr; - } - - return &mCurrent; - } + TNodeType *findNode(const std::string &name); /// @brief Will return true, if the node is a child-node. /// @param name [in] The name of the child node to look for. /// @return true, if the node is a child-node or false if not. - bool hasNode(const std::string &name) { - return nullptr != findNode(name); - } + bool hasNode(const std::string &name); /// @brief Will parse an xml-file from a given stream. /// @param stream The input stream. /// @return true, if the parsing was successful, false if not. - bool parse(IOStream *stream) { - if (nullptr == stream) { - ASSIMP_LOG_DEBUG("Stream is nullptr."); - return false; - } + bool parse(IOStream *stream); - const size_t len = stream->FileSize(); - mData.resize(len + 1); - memset(&mData[0], '\0', len + 1); - stream->Read(&mData[0], 1, len); - - mDoc = new pugi::xml_document(); - pugi::xml_parse_result parse_result = mDoc->load_string(&mData[0], pugi::parse_full); - if (parse_result.status == pugi::status_ok) { - return true; - } - - ASSIMP_LOG_DEBUG("Error while parse xml.", std::string(parse_result.description()), " @ ", parse_result.offset); - - return false; - } - - /// @brief Will return truem if a root node is there. + /// @brief Will return true if a root node is there. /// @return true in case of an existing root. - bool hasRoot() const { - return nullptr != mDoc; - } + bool hasRoot() const; + /// @brief Will return the document pointer, is nullptr if no xml-file was parsed. /// @return The pointer showing to the document. - pugi::xml_document *getDocument() const { - return mDoc; - } + pugi::xml_document *getDocument() const; /// @brief Will return the root node, const version. /// @return The root node. - const TNodeType getRootNode() const { - static pugi::xml_node none; - if (nullptr == mDoc) { - return none; - } - return mDoc->root(); - } + const TNodeType getRootNode() const; /// @brief Will return the root node, non-const version. /// @return The root node. - TNodeType getRootNode() { - static pugi::xml_node none; - if (nullptr == mDoc) { - return none; - } - return mDoc->root(); - } + TNodeType getRootNode(); /// @brief Will check if a node with the given name is in. /// @param node [in] The node to look in. /// @param name [in] The name of the child-node. /// @return true, if node was found, false if not. - static inline bool hasNode(XmlNode &node, const char *name) { - pugi::xml_node child = node.find_child(find_node_by_name_predicate(name)); - return !child.empty(); - } + static inline bool hasNode(XmlNode &node, const char *name); /// @brief Will check if an attribute is part of the XmlNode. /// @param xmlNode [in] The node to search in. /// @param name [in} The attribute name to look for. /// @return true, if the was found, false if not. - static inline bool hasAttribute(XmlNode &xmlNode, const char *name) { - pugi::xml_attribute attr = xmlNode.attribute(name); - return !attr.empty(); - } + static inline bool hasAttribute(XmlNode &xmlNode, const char *name); /// @brief Will try to get an unsigned int attribute value. /// @param xmlNode [in] The node to search in. /// @param name [in] The attribute name to look for. /// @param val [out] The unsigned int value from the attribute. /// @return true, if the node contains an attribute with the given name and if the value is an unsigned int. - static inline bool getUIntAttribute(XmlNode &xmlNode, const char *name, unsigned int &val) { - pugi::xml_attribute attr = xmlNode.attribute(name); - if (attr.empty()) { - return false; - } - - val = attr.as_uint(); - return true; - } + static inline bool getUIntAttribute(XmlNode &xmlNode, const char *name, unsigned int &val); /// @brief Will try to get an int attribute value. /// @param xmlNode [in] The node to search in. /// @param name [in] The attribute name to look for. /// @param val [out] The int value from the attribute. /// @return true, if the node contains an attribute with the given name and if the value is an int. - static inline bool getIntAttribute(XmlNode &xmlNode, const char *name, int &val) { - pugi::xml_attribute attr = xmlNode.attribute(name); - if (attr.empty()) { - return false; - } - - val = attr.as_int(); - return true; - } + static inline bool getIntAttribute(XmlNode &xmlNode, const char *name, int &val); /// @brief Will try to get a real attribute value. /// @param xmlNode [in] The node to search in. /// @param name [in] The attribute name to look for. /// @param val [out] The real value from the attribute. /// @return true, if the node contains an attribute with the given name and if the value is a real. - static inline bool getRealAttribute(XmlNode &xmlNode, const char *name, ai_real &val) { - pugi::xml_attribute attr = xmlNode.attribute(name); - if (attr.empty()) { - return false; - } -#ifdef ASSIMP_DOUBLE_PRECISION - val = attr.as_double(); -#else - val = attr.as_float(); -#endif - return true; - } + static inline bool getRealAttribute(XmlNode &xmlNode, const char *name, ai_real &val); /// @brief Will try to get a float attribute value. /// @param xmlNode [in] The node to search in. /// @param name [in] The attribute name to look for. /// @param val [out] The float value from the attribute. /// @return true, if the node contains an attribute with the given name and if the value is a float. - static inline bool getFloatAttribute(XmlNode &xmlNode, const char *name, float &val) { - pugi::xml_attribute attr = xmlNode.attribute(name); - if (attr.empty()) { - return false; - } - - val = attr.as_float(); - return true; - } + static inline bool getFloatAttribute(XmlNode &xmlNode, const char *name, float &val); /// @brief Will try to get a double attribute value. /// @param xmlNode [in] The node to search in. /// @param name [in] The attribute name to look for. /// @param val [out] The double value from the attribute. /// @return true, if the node contains an attribute with the given name and if the value is a double. - 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 getDoubleAttribute(XmlNode &xmlNode, const char *name, double &val); /// @brief Will try to get a std::string attribute value. /// @param xmlNode [in] The node to search in. /// @param name [in] The attribute name to look for. /// @param val [out] The std::string value from the attribute. /// @return true, if the node contains an attribute with the given name and if the value is a std::string. - static inline bool getStdStrAttribute(XmlNode &xmlNode, const char *name, std::string &val) { - pugi::xml_attribute attr = xmlNode.attribute(name); - if (attr.empty()) { - return false; - } - - val = attr.as_string(); - return true; - } + static inline bool getStdStrAttribute(XmlNode &xmlNode, const char *name, std::string &val); /// @brief Will try to get a bool attribute value. /// @param xmlNode [in] The node to search in. /// @param name [in] The attribute name to look for. /// @param val [out] The bool value from the attribute. /// @return true, if the node contains an attribute with the given name and if the value is a bool. - static inline bool getBoolAttribute(XmlNode &xmlNode, const char *name, bool &val) { - pugi::xml_attribute attr = xmlNode.attribute(name); - if (attr.empty()) { - return false; - } - - val = attr.as_bool(); - return true; - } + static inline bool getBoolAttribute(XmlNode &xmlNode, const char *name, bool &val); /// @brief Will try to get the value of the node as a string. /// @param node [in] The node to search in. /// @param text [out] The value as a text. /// @return true, if the value can be read out. - static inline bool getValueAsString(XmlNode &node, std::string &text) { - text = std::string(); - if (node.empty()) { - return false; - } - - text = node.text().as_string(); - - return true; - } + static inline bool getValueAsString(XmlNode &node, std::string &text); /// @brief Will try to get the value of the node as a float. /// @param node [in] The node to search in. /// @param text [out] The value as a float. /// @return true, if the value can be read out. - static inline bool getValueAsFloat(XmlNode &node, ai_real &v) { - if (node.empty()) { - return false; - } - - v = node.text().as_float(); - - return true; - } + static inline bool getValueAsFloat(XmlNode &node, ai_real &v); /// @brief Will try to get the value of the node as an integer. /// @param node [in] The node to search in. /// @param text [out] The value as a int. /// @return true, if the value can be read out. - static inline bool getValueAsInt(XmlNode &node, int &v) { - if (node.empty()) { - return false; - } - - v = node.text().as_int(); - - return true; - } + static inline bool getValueAsInt(XmlNode &node, int &v); /// @brief Will try to get the value of the node as an bool. /// @param node [in] The node to search in. /// @param text [out] The value as a bool. /// @return true, if the value can be read out. - static inline bool getValueAsBool(XmlNode& node, bool& v) - { - if (node.empty()) { - return false; - } - - v = node.text().as_bool(); - - return true; - } + static inline bool getValueAsBool(XmlNode &node, bool &v); private: pugi::xml_document *mDoc; @@ -395,6 +229,254 @@ private: std::vector mData; }; +template +inline TXmlParser::TXmlParser() : + mDoc(nullptr), + mData() { + // empty +} + +template +inline TXmlParser::~TXmlParser() { + clear(); +} + +template +inline void TXmlParser::clear() { + if (mData.empty()) { + if (mDoc) { + delete mDoc; + } + mDoc = nullptr; + return; + } + + mData.clear(); + delete mDoc; + mDoc = nullptr; +} + +template +inline TNodeType *TXmlParser::findNode(const std::string &name) { + if (name.empty()) { + return nullptr; + } + + if (nullptr == mDoc) { + return nullptr; + } + + find_node_by_name_predicate predicate(name); + mCurrent = mDoc->find_node(predicate); + if (mCurrent.empty()) { + return nullptr; + } + + return &mCurrent; +} + +template +bool TXmlParser::hasNode(const std::string &name) { + return nullptr != findNode(name); +} + +template +bool TXmlParser::parse(IOStream *stream) { + if (hasRoot()) { + clear(); + } + + if (nullptr == stream) { + ASSIMP_LOG_DEBUG("Stream is nullptr."); + return false; + } + + const size_t len = stream->FileSize(); + mData.resize(len + 1); + memset(&mData[0], '\0', len + 1); + stream->Read(&mData[0], 1, len); + + mDoc = new pugi::xml_document(); + pugi::xml_parse_result parse_result = mDoc->load_string(&mData[0], pugi::parse_full); + if (parse_result.status == pugi::status_ok) { + return true; + } + + ASSIMP_LOG_DEBUG("Error while parse xml.", std::string(parse_result.description()), " @ ", parse_result.offset); + + return false; +} + +template +bool TXmlParser::hasRoot() const { + return nullptr != mDoc; +} + +template +pugi::xml_document *TXmlParser::getDocument() const { + return mDoc; +} + +template +const TNodeType TXmlParser::getRootNode() const { + static pugi::xml_node none; + if (nullptr == mDoc) { + return none; + } + return mDoc->root(); +} + +template +TNodeType TXmlParser::getRootNode() { + static pugi::xml_node none; + if (nullptr == mDoc) { + return none; + } + + return mDoc->root(); +} + +template +inline bool TXmlParser::hasNode(XmlNode &node, const char *name) { + pugi::xml_node child = node.find_child(find_node_by_name_predicate(name)); + return !child.empty(); +} + +template +inline bool TXmlParser::hasAttribute(XmlNode &xmlNode, const char *name) { + pugi::xml_attribute attr = xmlNode.attribute(name); + return !attr.empty(); +} + +template +inline bool TXmlParser::getUIntAttribute(XmlNode &xmlNode, const char *name, unsigned int &val) { + pugi::xml_attribute attr = xmlNode.attribute(name); + if (attr.empty()) { + return false; + } + + val = attr.as_uint(); + return true; +} + +template +inline bool TXmlParser::getIntAttribute(XmlNode &xmlNode, const char *name, int &val) { + pugi::xml_attribute attr = xmlNode.attribute(name); + if (attr.empty()) { + return false; + } + + val = attr.as_int(); + return true; +} + +template +inline bool TXmlParser::getRealAttribute(XmlNode &xmlNode, const char *name, ai_real &val) { + pugi::xml_attribute attr = xmlNode.attribute(name); + if (attr.empty()) { + return false; + } +#ifdef ASSIMP_DOUBLE_PRECISION + val = attr.as_double(); +#else + val = attr.as_float(); +#endif + return true; +} + +template +inline bool TXmlParser::getFloatAttribute(XmlNode &xmlNode, const char *name, float &val) { + pugi::xml_attribute attr = xmlNode.attribute(name); + if (attr.empty()) { + return false; + } + + val = attr.as_float(); + + return true; +} + +template +inline bool TXmlParser::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; +} + +template +inline bool TXmlParser::getStdStrAttribute(XmlNode &xmlNode, const char *name, std::string &val) { + pugi::xml_attribute attr = xmlNode.attribute(name); + if (attr.empty()) { + return false; + } + + val = attr.as_string(); + + return true; +} + +template +inline bool TXmlParser::getBoolAttribute(XmlNode &xmlNode, const char *name, bool &val) { + pugi::xml_attribute attr = xmlNode.attribute(name); + if (attr.empty()) { + return false; + } + + val = attr.as_bool(); + + return true; +} + +template +inline bool TXmlParser::getValueAsString(XmlNode &node, std::string &text) { + text = std::string(); + if (node.empty()) { + return false; + } + + text = node.text().as_string(); + + return true; +} + +template +inline bool TXmlParser::getValueAsFloat(XmlNode &node, ai_real &v) { + if (node.empty()) { + return false; + } + + v = node.text().as_float(); + + return true; +} + +template +inline bool TXmlParser::getValueAsInt(XmlNode &node, int &v) { + if (node.empty()) { + return false; + } + + v = node.text().as_int(); + + return true; +} + +template +inline bool TXmlParser::getValueAsBool(XmlNode &node, bool &v) { + if (node.empty()) { + return false; + } + + v = node.text().as_bool(); + + return true; +} + using XmlParser = TXmlParser; /// @brief This class declares an iterator to loop through all children of the root node.