From e01a6b427648f30144957c1add05baac21273d17 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 18 May 2021 21:15:48 +0200 Subject: [PATCH 1/2] Add xml doc. --- code/AssetLib/Collada/ColladaParser.cpp | 40 +++----- include/assimp/Exceptional.h | 3 + include/assimp/XmlParser.h | 131 ++++++++++++++++++++++-- test/unit/Common/utXmlParser.cpp | 4 +- test/unit/utImporter.cpp | 116 +++++++++------------ 5 files changed, 189 insertions(+), 105 deletions(-) diff --git a/code/AssetLib/Collada/ColladaParser.cpp b/code/AssetLib/Collada/ColladaParser.cpp index 42166fdd4..8e2cb3f9f 100644 --- a/code/AssetLib/Collada/ColladaParser.cpp +++ b/code/AssetLib/Collada/ColladaParser.cpp @@ -623,8 +623,7 @@ void ColladaParser::ReadController(XmlNode &node, Collada::Controller &controlle controller.mType = Skin; controller.mMethod = Normalized; - XmlNodeIterator xmlIt(node); - xmlIt.collectChildrenPreOrder(node); + XmlNodeIterator xmlIt(node, XmlNodeIterator::PreOrderMode); XmlNode currentNode; while (xmlIt.getNext(currentNode)) { @@ -929,8 +928,7 @@ void ColladaParser::ReadMaterial(XmlNode &node, Collada::Material &pMaterial) { // ------------------------------------------------------------------------------------------------ // Reads a light entry into the given light void ColladaParser::ReadLight(XmlNode &node, Collada::Light &pLight) { - XmlNodeIterator xmlIt(node); - xmlIt.collectChildrenPreOrder(node); + XmlNodeIterator xmlIt(node, XmlNodeIterator::PreOrderMode); XmlNode currentNode; while (xmlIt.getNext(currentNode)) { const std::string ¤tName = currentNode.name(); @@ -991,10 +989,8 @@ void ColladaParser::ReadLight(XmlNode &node, Collada::Light &pLight) { // ------------------------------------------------------------------------------------------------ // Reads a camera entry into the given light void ColladaParser::ReadCamera(XmlNode &node, Collada::Camera &camera) { - XmlNodeIterator xmlIt(node); - xmlIt.collectChildrenPreOrder(node); + XmlNodeIterator xmlIt(node, XmlNodeIterator::PreOrderMode); XmlNode currentNode; - while (xmlIt.getNext(currentNode)) { const std::string ¤tName = currentNode.name(); if (currentName == "orthographic") { @@ -1050,11 +1046,10 @@ void ColladaParser::ReadEffect(XmlNode &node, Collada::Effect &pEffect) { // ------------------------------------------------------------------------------------------------ // Reads an COMMON effect profile void ColladaParser::ReadEffectProfileCommon(XmlNode &node, Collada::Effect &pEffect) { - XmlNodeIterator xmlIt(node); - xmlIt.collectChildrenPreOrder(node); + XmlNodeIterator xmlIt(node, XmlNodeIterator::PreOrderMode); XmlNode currentNode; while (xmlIt.getNext(currentNode)) { - const std::string ¤tName = currentNode.name(); + const std::string currentName = currentNode.name(); if (currentName == "newparam") { // save ID std::string sid = currentNode.attribute("sid").as_string(); @@ -1145,10 +1140,9 @@ void ColladaParser::ReadSamplerProperties(XmlNode &node, Sampler &out) { if (node.empty()) { return; } - XmlNodeIterator xmlIt(node); - xmlIt.collectChildrenPreOrder(node); - XmlNode currentNode; + XmlNodeIterator xmlIt(node, XmlNodeIterator::PreOrderMode); + XmlNode currentNode; while (xmlIt.getNext(currentNode)) { const std::string ¤tName = currentNode.name(); // MAYA extensions @@ -1208,10 +1202,9 @@ void ColladaParser::ReadEffectColor(XmlNode &node, aiColor4D &pColor, Sampler &p if (node.empty()) { return; } - XmlNodeIterator xmlIt(node); - xmlIt.collectChildrenPreOrder(node); - XmlNode currentNode; + XmlNodeIterator xmlIt(node, XmlNodeIterator::PreOrderMode); + XmlNode currentNode; while (xmlIt.getNext(currentNode)) { const std::string ¤tName = currentNode.name(); if (currentName == "color") { @@ -1273,8 +1266,7 @@ void ColladaParser::ReadEffectParam(XmlNode &node, Collada::EffectParam &pParam) return; } - XmlNodeIterator xmlIt(node); - xmlIt.collectChildrenPreOrder(node); + XmlNodeIterator xmlIt(node, XmlNodeIterator::PreOrderMode); XmlNode currentNode; while (xmlIt.getNext(currentNode)) { const std::string ¤tName = currentNode.name(); @@ -1360,8 +1352,7 @@ void ColladaParser::ReadMesh(XmlNode &node, Mesh &pMesh) { return; } - XmlNodeIterator xmlIt(node); - xmlIt.collectChildrenPreOrder(node); + XmlNodeIterator xmlIt(node, XmlNodeIterator::PreOrderMode); XmlNode currentNode; while (xmlIt.getNext(currentNode)) { const std::string ¤tName = currentNode.name(); @@ -1386,8 +1377,7 @@ void ColladaParser::ReadSource(XmlNode &node) { std::string sourceID; XmlParser::getStdStrAttribute(node, "id", sourceID); - XmlNodeIterator xmlIt(node); - xmlIt.collectChildrenPreOrder(node); + XmlNodeIterator xmlIt(node, XmlNodeIterator::PreOrderMode); XmlNode currentNode; while (xmlIt.getNext(currentNode)) { const std::string ¤tName = currentNode.name(); @@ -1490,8 +1480,7 @@ void ColladaParser::ReadAccessor(XmlNode &node, const std::string &pID) { acc.mSource = source.c_str() + 1; // ignore the leading '#' acc.mSize = 0; // gets incremented with every param - XmlNodeIterator xmlIt(node); - xmlIt.collectChildrenPreOrder(node); + XmlNodeIterator xmlIt(node, XmlNodeIterator::PreOrderMode); XmlNode currentNode; while (xmlIt.getNext(currentNode)) { const std::string ¤tName = currentNode.name(); @@ -1608,8 +1597,7 @@ void ColladaParser::ReadIndexData(XmlNode &node, Mesh &pMesh) { ai_assert(primType != Prim_Invalid); // also a number of elements, but in addition a

primitive collection and probably index counts for all primitives - XmlNodeIterator xmlIt(node); - xmlIt.collectChildrenPreOrder(node); + XmlNodeIterator xmlIt(node, XmlNodeIterator::PreOrderMode); XmlNode currentNode; while (xmlIt.getNext(currentNode)) { const std::string ¤tName = currentNode.name(); diff --git a/include/assimp/Exceptional.h b/include/assimp/Exceptional.h index 36d60d63a..98e2a3321 100644 --- a/include/assimp/Exceptional.h +++ b/include/assimp/Exceptional.h @@ -71,6 +71,9 @@ protected: * nullptr instead of a valid aiScene then. */ class ASSIMP_API DeadlyImportError : public DeadlyErrorBase { public: + DeadlyImportError(const char *message) : + DeadlyErrorBase(Assimp::Formatter::format(), std::forward(message)) {} + /** Constructor with arguments */ template explicit DeadlyImportError(T&&... args) : diff --git a/include/assimp/XmlParser.h b/include/assimp/XmlParser.h index 91fb9907f..5e63a4e79 100644 --- a/include/assimp/XmlParser.h +++ b/include/assimp/XmlParser.h @@ -66,6 +66,8 @@ struct find_node_by_name_predicate { } }; +/// @brief Will convert an attribute to its int value. +/// @tparam TNodeType The node type. template struct NodeConverter { public: @@ -78,19 +80,37 @@ public: using XmlNode = pugi::xml_node; using XmlAttribute = pugi::xml_attribute; +/// @brief The Xml-Parser class. +/// +/// Use this parser if you have to import any kind of xml-format. +/// +/// An example: +/// @code +/// TXmlParser theParser; +/// if (theParser.parse(fileStream)) { +/// auto node = theParser.getRootNode(); +/// for ( auto currentNode : node.children()) { +/// // Will loop over all children +/// } +/// } +/// @endcode +/// @tparam TNodeType template class TXmlParser { public: + /// @brief The default class constructor. TXmlParser() : mDoc(nullptr), mData() { // empty } + /// @brief The class destructor. ~TXmlParser() { clear(); } + /// @brief Will clear the parsed xml-file. void clear() { if(mData.empty()) { mDoc = nullptr; @@ -101,6 +121,9 @@ public: mDoc = nullptr; } + /// @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; @@ -119,10 +142,16 @@ public: return &mCurrent; } + /// @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); } + /// @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."); @@ -138,34 +167,53 @@ public: pugi::xml_parse_result parse_result = mDoc->load_string(&mData[0], pugi::parse_full); if (parse_result.status == pugi::status_ok) { return true; - } else { - ASSIMP_LOG_DEBUG("Error while parse xml."); - return false; - } + } + + ASSIMP_LOG_DEBUG("Error while parse xml."); + return false; } + /// @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; } + /// @brief Will return the root node, const version. + /// @return The root node. const TNodeType getRootNode() const { return mDoc->root(); } + /// @brief Will return the root node, non-const version. + /// @return The root node. TNodeType getRootNode() { return mDoc->root(); } + /// @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(); } + /// @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(); } + /// @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()) { @@ -176,6 +224,11 @@ public: return true; } + /// @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()) { @@ -186,6 +239,11 @@ public: return true; } + /// @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()) { @@ -199,7 +257,12 @@ public: return true; } - static inline bool getFloatAttribute(XmlNode &xmlNode, const char *name, float &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; @@ -210,7 +273,12 @@ public: } - static inline bool getDoubleAttribute( XmlNode &xmlNode, const char *name, double &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; @@ -220,6 +288,11 @@ public: return true; } + /// @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()) { @@ -230,6 +303,11 @@ public: return true; } + /// @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()) { @@ -241,6 +319,10 @@ public: } + /// @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()) { @@ -252,6 +334,10 @@ public: return true; } + /// @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; @@ -271,17 +357,36 @@ public: using XmlParser = TXmlParser; +/// @brief This class declares an iterator to loop through all children of the root node. class XmlNodeIterator { public: - XmlNodeIterator(XmlNode &parent) : + /// @brief The iteration mode. + enum IterationMode { + PreOrderMode, ///< Pre-ordering, get the values, continue the iteration. + PostOrderMode ///< Post-ordering, continue the iteration, get the values. + }; + /// @brief The class constructor + /// @param parent [in] The xml parent to to iterate through. + /// @param mode [in] The iteration mode. + explicit XmlNodeIterator(XmlNode &parent, IterationMode mode) : mParent(parent), mNodes(), mIndex(0) { + if (mode == PreOrderMode) { + collectChildrenPreOrder(parent); + } else { + collectChildrenPostOrder(parent); + } + } + + /// @brief The class destructor. + ~XmlNodeIterator() { // empty } + /// @brief Will iterate through all children in pre-order iteration. + /// @param node [in] The nod to iterate through. void collectChildrenPreOrder( XmlNode &node ) { - if (node != mParent && node.type() == pugi::node_element) { mNodes.push_back(node); } @@ -290,6 +395,8 @@ public: } } + /// @brief Will iterate through all children in post-order iteration. + /// @param node [in] The nod to iterate through. void collectChildrenPostOrder(XmlNode &node) { for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) { collectChildrenPostOrder(currentNode); @@ -299,6 +406,9 @@ public: } } + /// @brief Will iterate through all collected nodes. + /// @param next The next node, if there is any. + /// @return true, if there is a node left. bool getNext(XmlNode &next) { if (mIndex == mNodes.size()) { return false; @@ -310,14 +420,19 @@ public: return true; } + /// @brief Will return the number of collected nodes. + /// @return The number of collected nodes. size_t size() const { return mNodes.size(); } + /// @brief Returns true, if the node is empty. + /// @return true, if the node is empty, false if not. bool isEmpty() const { return mNodes.empty(); } + /// @brief Will clear all collected nodes. void clear() { if (mNodes.empty()) { return; diff --git a/test/unit/Common/utXmlParser.cpp b/test/unit/Common/utXmlParser.cpp index c27669dd1..7e992f9df 100644 --- a/test/unit/Common/utXmlParser.cpp +++ b/test/unit/Common/utXmlParser.cpp @@ -73,9 +73,7 @@ TEST_F(utXmlParser, parse_xml_and_traverse_test) { EXPECT_TRUE(result); XmlNode root = parser.getRootNode(); - XmlNodeIterator nodeIt(root); - EXPECT_TRUE(nodeIt.isEmpty()); - nodeIt.collectChildrenPreOrder(root); + XmlNodeIterator nodeIt(root, XmlNodeIterator::PreOrderMode); const size_t numNodes = nodeIt.size(); bool empty = nodeIt.isEmpty(); EXPECT_FALSE(empty); diff --git a/test/unit/utImporter.cpp b/test/unit/utImporter.cpp index 65b7f99ee..9d3b971a3 100644 --- a/test/unit/utImporter.cpp +++ b/test/unit/utImporter.cpp @@ -280,104 +280,84 @@ TEST_F(ImporterTest, SearchFileHeaderForTokenTest) { // BaseImporter::SearchFileHeaderForToken( &ioSystem, assetPath, Token, 2 ) } +namespace { +// Description for an importer which fails in specific ways. +aiImporterDesc s_failingImporterDescription = { + "Failing importer", + "assimp team", + "", + "", + 0, + 1, + 0, + 1, + 0, + "fail" +}; -namespace -{ - // Description for an importer which fails in specific ways. - aiImporterDesc s_failingImporterDescription = { - "Failing importer", - "assimp team", - "", - "", - 0, - 1, - 0, - 1, - 0, - "fail" - }; +// This importer fails in specific ways. +class FailingImporter : public Assimp::BaseImporter { +public: + virtual ~FailingImporter() = default; + virtual bool CanRead(const std::string &, Assimp::IOSystem *, bool) const override { + return true; + } - // This importer fails in specific ways. - class FailingImporter : public Assimp::BaseImporter { - public: - virtual ~FailingImporter() = default; - virtual bool CanRead( const std::string&, Assimp::IOSystem*, bool ) const override - { - return true; +protected: + const aiImporterDesc *GetInfo() const override { + return &s_failingImporterDescription; + } + + void InternReadFile(const std::string &pFile, aiScene *, Assimp::IOSystem *) override { + if (pFile == "deadlyImportError.fail") { + throw DeadlyImportError("Deadly import error test. Details: ", 42, " More Details: ", "Failure"); + } else if (pFile == "stdException.fail") { + throw std::runtime_error("std::exception test"); + } else if (pFile == "unexpectedException.fail") { + throw 5; } + } +}; +} // namespace - protected: - virtual const aiImporterDesc* GetInfo() const override { return &s_failingImporterDescription; } - - virtual void InternReadFile( const std::string& pFile, aiScene*, Assimp::IOSystem* ) override - { - if (pFile == "deadlyImportError.fail") - { - throw DeadlyImportError("Deadly import error test. Details: ", 42, " More Details: ", "Failure"); - } - else if (pFile == "stdException.fail") - { - throw std::runtime_error("std::exception test"); - } - else if (pFile == "unexpectedException.fail") - { - throw 5; - } - } - }; -} - -TEST_F(ImporterTest, deadlyImportError) -{ +TEST_F(ImporterTest, deadlyImportError) { pImp->RegisterLoader(new FailingImporter); pImp->SetIOHandler(new TestIOSystem); - const aiScene* scene = pImp->ReadFile("deadlyImportError.fail", 0); + const aiScene *scene = pImp->ReadFile("deadlyImportError.fail", 0); EXPECT_EQ(scene, nullptr); EXPECT_STREQ(pImp->GetErrorString(), "Deadly import error test. Details: 42 More Details: Failure"); EXPECT_NE(pImp->GetException(), std::exception_ptr()); } -TEST_F(ImporterTest, stdException) -{ +TEST_F(ImporterTest, stdException) { pImp->RegisterLoader(new FailingImporter); pImp->SetIOHandler(new TestIOSystem); - const aiScene* scene = pImp->ReadFile("stdException.fail", 0); + const aiScene *scene = pImp->ReadFile("stdException.fail", 0); EXPECT_EQ(scene, nullptr); EXPECT_STREQ(pImp->GetErrorString(), "std::exception test"); EXPECT_NE(pImp->GetException(), std::exception_ptr()); - try - { + try { std::rethrow_exception(pImp->GetException()); - } - catch(const std::exception& e) - { + } catch (const std::exception &e) { EXPECT_STREQ(e.what(), "std::exception test"); - } - catch(...) - { + } catch (...) { EXPECT_TRUE(false); } } -TEST_F(ImporterTest, unexpectedException) -{ +TEST_F(ImporterTest, unexpectedException) { pImp->RegisterLoader(new FailingImporter); pImp->SetIOHandler(new TestIOSystem); - const aiScene* scene = pImp->ReadFile("unexpectedException.fail", 0); + const aiScene *scene = pImp->ReadFile("unexpectedException.fail", 0); EXPECT_EQ(scene, nullptr); EXPECT_STREQ(pImp->GetErrorString(), "Unknown exception"); ASSERT_NE(pImp->GetException(), std::exception_ptr()); - try - { + try { std::rethrow_exception(pImp->GetException()); - } - catch(int x) - { + } catch (int x) { EXPECT_EQ(x, 5); - } - catch(...) - { + } catch (...) { EXPECT_TRUE(false); } } From 3726b2eef462f2941d1479e16fc7eea424e3f11e Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 18 May 2021 21:21:43 +0200 Subject: [PATCH 2/2] fix the build --- include/assimp/XmlParser.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/assimp/XmlParser.h b/include/assimp/XmlParser.h index 11f937b56..35ab0471e 100644 --- a/include/assimp/XmlParser.h +++ b/include/assimp/XmlParser.h @@ -169,7 +169,7 @@ public: return true; } - ASSIMP_LOG_DEBUG("Error while parse xml.", parse_result.description() << " @ ", parse_result.offset); + ASSIMP_LOG_DEBUG("Error while parse xml.", std::string(parse_result.description()), " @ ", parse_result.offset); return false; }