From 73fa2cbe88ae2707e8b3ddcd7e16c2019c1f25b7 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 1 Sep 2020 21:48:50 +0200 Subject: [PATCH] Fix memory handling of xml-nodes in the parser. --- code/AssetLib/3MF/D3MFImporter.cpp | 2 +- code/AssetLib/3MF/D3MFOpcPackage.cpp | 7 +- code/AssetLib/Collada/ColladaParser.cpp | 19 +- code/AssetLib/Irr/IRRLoader.cpp | 8 +- code/AssetLib/Irr/IRRMeshLoader.cpp | 9 +- code/AssetLib/Ogre/OgreXmlSerializer.cpp | 353 +---------------------- code/AssetLib/XGL/XGLLoader.cpp | 25 +- include/assimp/XmlParser.h | 34 ++- test/unit/Common/utXmlParser.cpp | 27 +- test/unit/utD3MFImportExport.cpp | 8 +- 10 files changed, 86 insertions(+), 406 deletions(-) diff --git a/code/AssetLib/3MF/D3MFImporter.cpp b/code/AssetLib/3MF/D3MFImporter.cpp index dd685a27d..6b4eeb445 100644 --- a/code/AssetLib/3MF/D3MFImporter.cpp +++ b/code/AssetLib/3MF/D3MFImporter.cpp @@ -93,7 +93,7 @@ public: std::vector children; std::string nodeName; - XmlNode node = *mXmlParser->getRootNode(); + XmlNode node = mXmlParser->getRootNode(); for (XmlNode currentNode = node.first_child(); currentNode; currentNode = currentNode.next_sibling()) { const std::string ¤tNodeName = currentNode.name(); diff --git a/code/AssetLib/3MF/D3MFOpcPackage.cpp b/code/AssetLib/3MF/D3MFOpcPackage.cpp index 4170d25d2..96692fccf 100644 --- a/code/AssetLib/3MF/D3MFOpcPackage.cpp +++ b/code/AssetLib/3MF/D3MFOpcPackage.cpp @@ -69,11 +69,8 @@ typedef std::shared_ptr OpcPackageRelationshipPtr; class OpcPackageRelationshipReader { public: OpcPackageRelationshipReader(XmlParser &parser) { - XmlNode *root = parser.getRootNode(); - if (nullptr != root) { - XmlNode node = *root; - ParseRootNode(node); - } + XmlNode root = parser.getRootNode(); + ParseRootNode(root); } void ParseRootNode(XmlNode &node) { diff --git a/code/AssetLib/Collada/ColladaParser.cpp b/code/AssetLib/Collada/ColladaParser.cpp index a832b3ee0..238966fcb 100644 --- a/code/AssetLib/Collada/ColladaParser.cpp +++ b/code/AssetLib/Collada/ColladaParser.cpp @@ -118,13 +118,13 @@ ColladaParser::ColladaParser(IOSystem *pIOHandler, const std::string &pFile) : } // generate a XML reader for it - pugi::xml_node *rootPtr = mXmlParser.parse(daefile.get()); - if (nullptr == rootPtr) { + ; + if (!mXmlParser.parse(daefile.get())) { ThrowException("Unable to read file, malformed XML"); } // start reading - XmlNode node = *rootPtr; - std::string name = rootPtr->name(); + XmlNode node = mXmlParser.getRootNode(); + std::string name = node.name(); ReadContents(node); // read embedded textures @@ -159,19 +159,18 @@ std::string ColladaParser::ReadZaeManifest(ZipArchiveIOSystem &zip_archive) { return file_list.front(); } XmlParser manifestParser; - XmlNode *root = manifestParser.parse(manifestfile.get()); - if (nullptr == root) { + if (!manifestParser.parse(manifestfile.get())) { return std::string(); } - - const std::string &name = root->name(); + XmlNode root = manifestParser.getRootNode(); + const std::string &name = root.name(); if (name != "dae_root") { - root = manifestParser.findNode("dae_root"); + root = *manifestParser.findNode("dae_root"); if (nullptr == root) { return std::string(); } std::string v; - XmlParser::getValueAsString(*root, v); + XmlParser::getValueAsString(root, v); aiString ai_str(v); UriDecodePath(ai_str); return std::string(ai_str.C_Str()); diff --git a/code/AssetLib/Irr/IRRLoader.cpp b/code/AssetLib/Irr/IRRLoader.cpp index 2221371bb..e176e83a6 100644 --- a/code/AssetLib/Irr/IRRLoader.cpp +++ b/code/AssetLib/Irr/IRRLoader.cpp @@ -864,8 +864,10 @@ void IRRImporter::InternReadFile(const std::string &pFile, aiScene *pScene, IOSy // Construct the irrXML parser XmlParser st; - pugi::xml_node *rootElement = st.parse(file.get()); - // reader = createIrrXMLReader((IFileReadCallBack*) &st); + if (!st.parse( file.get() )) { + return; + } + pugi::xml_node rootElement = st.getRootNode(); // The root node of the scene Node *root = new Node(Node::DUMMY); @@ -897,7 +899,7 @@ void IRRImporter::InternReadFile(const std::string &pFile, aiScene *pScene, IOSy // Parse the XML file //while (reader->read()) { - for (pugi::xml_node child : rootElement->children()) + for (pugi::xml_node child : rootElement.children()) switch (child.type()) { case pugi::node_element: if (!ASSIMP_stricmp(child.name(), "node")) { diff --git a/code/AssetLib/Irr/IRRMeshLoader.cpp b/code/AssetLib/Irr/IRRMeshLoader.cpp index cb39802f3..b6a9b4777 100644 --- a/code/AssetLib/Irr/IRRMeshLoader.cpp +++ b/code/AssetLib/Irr/IRRMeshLoader.cpp @@ -139,9 +139,10 @@ void IRRMeshImporter::InternReadFile(const std::string &pFile, // Construct the irrXML parser XmlParser parser; - pugi::xml_node *root = parser.parse(file.get()); - /*CIrrXML_IOStreamReader st(file.get()); - reader = createIrrXMLReader((IFileReadCallBack*) &st);*/ + if (!parser.parse( file.get() )) { + return; + } + XmlNode root = parser.getRootNode(); // final data std::vector materials; @@ -164,7 +165,7 @@ void IRRMeshImporter::InternReadFile(const std::string &pFile, bool useColors = false; // Parse the XML file - for (pugi::xml_node child : root->children()) { + for (pugi::xml_node child : root.children()) { if (child.type() == pugi::node_element) { if (!ASSIMP_stricmp(child.name(), "buffer") && (curMat || curMesh)) { // end of previous buffer. A material and a mesh should be there diff --git a/code/AssetLib/Ogre/OgreXmlSerializer.cpp b/code/AssetLib/Ogre/OgreXmlSerializer.cpp index 9555b052b..b52437b5a 100644 --- a/code/AssetLib/Ogre/OgreXmlSerializer.cpp +++ b/code/AssetLib/Ogre/OgreXmlSerializer.cpp @@ -225,12 +225,12 @@ MeshXml *OgreXmlSerializer::ImportMesh(XmlParser *parser) { } void OgreXmlSerializer::ReadMesh(MeshXml *mesh) { - XmlNode *root = mParser->getRootNode(); - if (nullptr == root || std::string(nnMesh)!=root->name()) { - throw DeadlyImportError("Root node is <" + std::string(root->name()) + "> expecting "); + XmlNode root = mParser->getRootNode(); + if (nullptr == root || std::string(nnMesh) != root.name()) { + throw DeadlyImportError("Root node is <" + std::string(root.name()) + "> expecting "); } - for (XmlNode currentNode : root->children()) { + for (XmlNode currentNode : root.children()) { const std::string currentName = currentNode.name(); if (currentName == nnSharedGeometry) { mesh->sharedVertexData = new VertexDataXml(); @@ -242,43 +242,8 @@ void OgreXmlSerializer::ReadMesh(MeshXml *mesh) { } else if (currentName == nnSkeletonLink) { } } - /*if (NextNode() != nnMesh) { - }*/ ASSIMP_LOG_VERBOSE_DEBUG("Reading Mesh"); - - //NextNode(); - - // Root level nodes - /*while (m_currentNodeName == nnSharedGeometry || - m_currentNodeName == nnSubMeshes || - m_currentNodeName == nnSkeletonLink || - m_currentNodeName == nnBoneAssignments || - m_currentNodeName == nnLOD || - m_currentNodeName == nnSubMeshNames || - m_currentNodeName == nnExtremes || - m_currentNodeName == nnPoses || - m_currentNodeName == nnAnimations) { - if (m_currentNodeName == nnSharedGeometry) { - mesh->sharedVertexData = new VertexDataXml(); - ReadGeometry(mesh->sharedVertexData); - } else if (m_currentNodeName == nnSubMeshes) { - NextNode(); - while (m_currentNodeName == nnSubMesh) { - ReadSubMesh(mesh); - } - } else if (m_currentNodeName == nnBoneAssignments) { - ReadBoneAssignments(mesh->sharedVertexData); - } else if (m_currentNodeName == nnSkeletonLink) { - mesh->skeletonRef = ReadAttribute("name"); - ASSIMP_LOG_VERBOSE_DEBUG_F("Read skeleton link ", mesh->skeletonRef); - NextNode(); - } - // Assimp incompatible/ignored nodes - else { - SkipCurrentNode(); - } - }*/ } void OgreXmlSerializer::ReadGeometry(XmlNode &node, VertexDataXml *dest) { @@ -291,10 +256,6 @@ void OgreXmlSerializer::ReadGeometry(XmlNode &node, VertexDataXml *dest) { ReadGeometryVertexBuffer(currentNode, dest); } } - //NextNode(); - /*while (m_currentNodeName == nnVertexBuffer) { - ReadGeometryVertexBuffer(dest); - }*/ } void OgreXmlSerializer::ReadGeometryVertexBuffer(XmlNode &node, VertexDataXml *dest) { @@ -328,11 +289,6 @@ void OgreXmlSerializer::ReadGeometryVertexBuffer(XmlNode &node, VertexDataXml *d } } - /*bool warnBinormal = true; - bool warnColorDiffuse = true; - bool warnColorSpecular = true;*/ - - //NextNode(); for (XmlNode currentNode : node.children()) { const std::string ¤tName = currentNode.name(); if (positions && currentName == nnPosition) { @@ -363,84 +319,6 @@ void OgreXmlSerializer::ReadGeometryVertexBuffer(XmlNode &node, VertexDataXml *d } } - /*while (m_currentNodeName == nnVertex || - m_currentNodeName == nnPosition || - m_currentNodeName == nnNormal || - m_currentNodeName == nnTangent || - m_currentNodeName == nnBinormal || - m_currentNodeName == nnTexCoord || - m_currentNodeName == nnColorDiffuse || - m_currentNodeName == nnColorSpecular) { - if (m_currentNodeName == nnVertex) { - NextNode(); - } - - /// @todo Implement nnBinormal, nnColorDiffuse and nnColorSpecular - - if (positions && m_currentNodeName == nnPosition) { - aiVector3D pos; - pos.x = ReadAttribute(anX); - pos.y = ReadAttribute(anY); - pos.z = ReadAttribute(anZ); - dest->positions.push_back(pos); - } else if (normals && m_currentNodeName == nnNormal) { - aiVector3D normal; - normal.x = ReadAttribute(anX); - normal.y = ReadAttribute(anY); - normal.z = ReadAttribute(anZ); - dest->normals.push_back(normal); - } else if (tangents && m_currentNodeName == nnTangent) { - aiVector3D tangent; - tangent.x = ReadAttribute(anX); - tangent.y = ReadAttribute(anY); - tangent.z = ReadAttribute(anZ); - dest->tangents.push_back(tangent); - } else if (uvs > 0 && m_currentNodeName == nnTexCoord) { - for (auto &curUvs : dest->uvs) { - if (m_currentNodeName != nnTexCoord) { - throw DeadlyImportError("Vertex buffer declared more UVs than can be found in a vertex"); - } - - aiVector3D uv; - uv.x = ReadAttribute("u"); - uv.y = (ReadAttribute("v") * -1) + 1; // Flip UV from Ogre to Assimp form - curUvs.push_back(uv); - - NextNode(); - } - // Continue main loop as above already read next node - continue; - } else { - /// @todo Remove this stuff once implemented. We only want to log warnings once per element. - bool warn = true; - if (m_currentNodeName == nnBinormal) { - if (warnBinormal) { - warnBinormal = false; - } else { - warn = false; - } - } else if (m_currentNodeName == nnColorDiffuse) { - if (warnColorDiffuse) { - warnColorDiffuse = false; - } else { - warn = false; - } - } else if (m_currentNodeName == nnColorSpecular) { - if (warnColorSpecular) { - warnColorSpecular = false; - } else { - warn = false; - } - } - if (warn) { - ASSIMP_LOG_WARN_F("Vertex buffer attribute read not implemented for element: ", m_currentNodeName); - } - }*/ - - // Advance - //NextNode(); - //} - // Sanity checks if (dest->positions.size() != dest->count) { throw DeadlyImportError(Formatter::format() << "Read only " << dest->positions.size() << " positions when should have read " << dest->count); @@ -525,57 +403,6 @@ void OgreXmlSerializer::ReadSubMesh(XmlNode &node, MeshXml *mesh) { } } - /*NextNode(); - while (m_currentNodeName == nnFaces || - m_currentNodeName == nnGeometry || - m_currentNodeName == nnTextures || - m_currentNodeName == nnBoneAssignments) { - if (m_currentNodeName == nnFaces) { - submesh->indexData->faceCount = ReadAttribute(anCount); - submesh->indexData->faces.reserve(submesh->indexData->faceCount); - - NextNode(); - while (m_currentNodeName == nnFace) { - aiFace face; - face.mNumIndices = 3; - face.mIndices = new unsigned int[3]; - face.mIndices[0] = ReadAttribute(anV1); - face.mIndices[1] = ReadAttribute(anV2); - face.mIndices[2] = ReadAttribute(anV3); - - /// @todo Support quads if Ogre even supports them in XML (I'm not sure but I doubt it) - if (!quadWarned && HasAttribute(anV4)) { - ASSIMP_LOG_WARN("Submesh has quads with , only triangles are supported at the moment!"); - quadWarned = true; - } - - submesh->indexData->faces.push_back(face); - - // Advance - NextNode(); - } - - if (submesh->indexData->faces.size() == submesh->indexData->faceCount) { - ASSIMP_LOG_VERBOSE_DEBUG_F(" - Faces ", submesh->indexData->faceCount); - } else { - throw DeadlyImportError(Formatter::format() << "Read only " << submesh->indexData->faces.size() << " faces when should have read " << submesh->indexData->faceCount); - } - } else if (m_currentNodeName == nnGeometry) { - if (submesh->usesSharedVertexData) { - throw DeadlyImportError("Found in when use shared geometry is true. Invalid mesh file."); - } - - submesh->vertexData = new VertexDataXml(); - ReadGeometry(submesh->vertexData); - } else if (m_currentNodeName == nnBoneAssignments) { - ReadBoneAssignments(submesh->vertexData); - } - // Assimp incompatible/ignored nodes - else { - SkipCurrentNode(); - } - }*/ - submesh->index = static_cast(mesh->subMeshes.size()); mesh->subMeshes.push_back(submesh); } @@ -603,19 +430,6 @@ void OgreXmlSerializer::ReadBoneAssignments(XmlNode &node, VertexDataXml *dest) } } - /*NextNode(); - while (m_currentNodeName == nnVertexBoneAssignment) { - VertexBoneAssignment ba; - ba.vertexIndex = ReadAttribute(anVertexIndex); - ba.boneIndex = ReadAttribute(anBoneIndex); - ba.weight = ReadAttribute(anWeight); - - dest->boneAssignments.push_back(ba); - influencedVertices.insert(ba.vertexIndex); - - NextNode(); - }*/ - /** Normalize bone weights. Some exporters won't care if the sum of all bone weights for a single vertex equals 1 or not, so validate here. */ @@ -662,8 +476,8 @@ bool OgreXmlSerializer::ImportSkeleton(Assimp::IOSystem *pIOHandler, MeshXml *me Skeleton *skeleton = new Skeleton(); OgreXmlSerializer serializer(xmlParser.get()); - XmlNode *root = xmlParser->getRootNode(); - serializer.ReadSkeleton(*root, skeleton); + XmlNode root = xmlParser->getRootNode(); + serializer.ReadSkeleton(root, skeleton); mesh->skeleton = skeleton; return true; } @@ -680,12 +494,9 @@ bool OgreXmlSerializer::ImportSkeleton(Assimp::IOSystem *pIOHandler, Mesh *mesh) Skeleton *skeleton = new Skeleton(); OgreXmlSerializer serializer(xmlParser.get()); - XmlNode *root = xmlParser->getRootNode(); - if (nullptr == root) { - return false; - } + XmlNode root = xmlParser->getRootNode(); - serializer.ReadSkeleton(*root, skeleton); + serializer.ReadSkeleton(root, skeleton); mesh->skeleton = skeleton; return true; @@ -736,22 +547,6 @@ void OgreXmlSerializer::ReadSkeleton(XmlNode &node, Skeleton *skeleton) { ReadAnimations(currentNode, skeleton); } } - /*NextNode(); - - // Root level nodes - while (m_currentNodeName == nnBones || - m_currentNodeName == nnBoneHierarchy || - m_currentNodeName == nnAnimations || - m_currentNodeName == nnAnimationLinks) { - if (m_currentNodeName == nnBones) - ReadBones(skeleton); - else if (m_currentNodeName == nnBoneHierarchy) - ReadBoneHierarchy(skeleton); - else if (m_currentNodeName == nnAnimations) - ReadAnimations(skeleton); - else - SkipCurrentNode(); - }*/ } void OgreXmlSerializer::ReadAnimations(XmlNode &node, Skeleton *skeleton) { @@ -778,22 +573,6 @@ void OgreXmlSerializer::ReadAnimations(XmlNode &node, Skeleton *skeleton) { } } } - -/* NextNode(); - while (m_currentNodeName == nnAnimation) { - Animation *anim = new Animation(skeleton); - anim->name = ReadAttribute("name"); - anim->length = ReadAttribute("length"); - - if (NextNode() != nnTracks) { - throw DeadlyImportError(Formatter::format() << "No found in " << anim->name); - } - - ReadAnimationTracks(anim); - skeleton->animations.push_back(anim); - - ASSIMP_LOG_VERBOSE_DEBUG_F(" ", anim->name, " (", anim->length, " sec, ", anim->tracks.size(), " tracks)"); - }*/ } void OgreXmlSerializer::ReadAnimationTracks(XmlNode &node, Animation *dest) { @@ -815,20 +594,6 @@ void OgreXmlSerializer::ReadAnimationTracks(XmlNode &node, Animation *dest) { } } - /*NextNode(); - while (m_currentNodeName == nnTrack) { - VertexAnimationTrack track; - track.type = VertexAnimationTrack::VAT_TRANSFORM; - track.boneName = ReadAttribute("bone"); - - if (NextNode() != nnKeyFrames) { - throw DeadlyImportError(Formatter::format() << "No found in " << dest->name); - } - - ReadAnimationKeyFrames(dest, &track); - - dest->tracks.push_back(track); - }*/ } void OgreXmlSerializer::ReadAnimationKeyFrames(XmlNode &node, Animation *anim, VertexAnimationTrack *dest) { @@ -873,46 +638,6 @@ void OgreXmlSerializer::ReadAnimationKeyFrames(XmlNode &node, Animation *anim, V } dest->transformKeyFrames.push_back(keyframe); } - /*NextNode(); - while (m_currentNodeName == nnKeyFrame) { - TransformKeyFrame keyframe; - keyframe.timePos = ReadAttribute("time"); - - NextNode(); - while (m_currentNodeName == nnTranslate || m_currentNodeName == nnRotate || m_currentNodeName == nnScale) { - if (m_currentNodeName == nnTranslate) { - keyframe.position.x = ReadAttribute(anX); - keyframe.position.y = ReadAttribute(anY); - keyframe.position.z = ReadAttribute(anZ); - } else if (m_currentNodeName == nnRotate) { - float angle = ReadAttribute("angle"); - - if (NextNode() != nnAxis) { - throw DeadlyImportError("No axis specified for keyframe rotation in animation " + anim->name); - } - - aiVector3D axis; - axis.x = ReadAttribute(anX); - axis.y = ReadAttribute(anY); - axis.z = ReadAttribute(anZ); - if (axis.Equal(zeroVec)) { - axis.x = 1.0f; - if (angle != 0) { - ASSIMP_LOG_WARN_F("Found invalid a key frame with a zero rotation axis in animation: ", anim->name); - } - } - keyframe.rotation = aiQuaternion(axis, angle); - } else if (m_currentNodeName == nnScale) { - keyframe.scale.x = ReadAttribute(anX); - keyframe.scale.y = ReadAttribute(anY); - keyframe.scale.z = ReadAttribute(anZ); - } - - NextNode(); - } - - dest->transformKeyFrames.push_back(keyframe); - }*/ } void OgreXmlSerializer::ReadBoneHierarchy(XmlNode &node, Skeleton *skeleton) { @@ -937,20 +662,6 @@ void OgreXmlSerializer::ReadBoneHierarchy(XmlNode &node, Skeleton *skeleton) { } } - - /*while (NextNode() == nnBoneParent) { - const std::string name = ReadAttribute("bone"); - const std::string parentName = ReadAttribute("parent"); - - Bone *bone = skeleton->BoneByName(name); - Bone *parent = skeleton->BoneByName(parentName); - - if (bone && parent) - parent->AddChild(bone); - else - throw DeadlyImportError("Failed to find bones for parenting: Child " + name + " for parent " + parentName); - }*/ - // Calculate bone matrices for root bones. Recursively calculates their children. for (size_t i = 0, len = skeleton->bones.size(); i < len; ++i) { Bone *bone = skeleton->bones[i]; @@ -1014,54 +725,6 @@ void OgreXmlSerializer::ReadBones(XmlNode &node, Skeleton *skeleton) { } } - /*NextNode(); - while (m_currentNodeName == nnBone) { - Bone *bone = new Bone(); - bone->id = ReadAttribute("id"); - bone->name = ReadAttribute("name"); - - NextNode(); - while (m_currentNodeName == nnPosition || - m_currentNodeName == nnRotation || - m_currentNodeName == nnScale) { - if (m_currentNodeName == nnPosition) { - bone->position.x = ReadAttribute(anX); - bone->position.y = ReadAttribute(anY); - bone->position.z = ReadAttribute(anZ); - } else if (m_currentNodeName == nnRotation) { - float angle = ReadAttribute("angle"); - - if (NextNode() != nnAxis) { - throw DeadlyImportError(Formatter::format() << "No axis specified for bone rotation in bone " << bone->id); - } - - aiVector3D axis; - axis.x = ReadAttribute(anX); - axis.y = ReadAttribute(anY); - axis.z = ReadAttribute(anZ); - - bone->rotation = aiQuaternion(axis, angle); - } else if (m_currentNodeName == nnScale) { - /// @todo Implement taking scale into account in matrix/pose calculations! - if (HasAttribute("factor")) { - float factor = ReadAttribute("factor"); - bone->scale.Set(factor, factor, factor); - } else { - if (HasAttribute(anX)) - bone->scale.x = ReadAttribute(anX); - if (HasAttribute(anY)) - bone->scale.y = ReadAttribute(anY); - if (HasAttribute(anZ)) - bone->scale.z = ReadAttribute(anZ); - } - } - - NextNode(); - } - - skeleton->bones.push_back(bone); - }*/ - // Order bones by Id std::sort(skeleton->bones.begin(), skeleton->bones.end(), BoneCompare); diff --git a/code/AssetLib/XGL/XGLLoader.cpp b/code/AssetLib/XGL/XGLLoader.cpp index 306841b4f..39e239b8c 100644 --- a/code/AssetLib/XGL/XGLLoader.cpp +++ b/code/AssetLib/XGL/XGLLoader.cpp @@ -199,27 +199,18 @@ void XGLImporter::InternReadFile(const std::string &pFile, #endif } - // construct the irrXML parser - /*CIrrXML_IOStreamReader st(stream.get()); - m_reader.reset( createIrrXMLReader( ( IFileReadCallBack* ) &st ) );*/ - mXmlParser = new XmlParser; - XmlNode *root = mXmlParser->parse(stream.get()); - if (nullptr == root) { + // parse the XML file + mXmlParser = new XmlParser; + if (!mXmlParser->parse(stream.get())) { return; } - // parse the XML file + XmlNode root = mXmlParser->getRootNode(); TempScope scope; - if (!ASSIMP_stricmp(root->name(), "world")) { + if (!ASSIMP_stricmp(root.name(), "world")) { ReadWorld(scope); } - /* while (ReadElement()) { - if (!ASSIMP_stricmp(m_reader->getNodeName(),"world")) { - ReadWorld(scope); - } - }*/ - std::vector &meshes = scope.meshes_linear; std::vector &materials = scope.materials_linear; if (!meshes.size() || !materials.size()) { @@ -249,8 +240,8 @@ void XGLImporter::InternReadFile(const std::string &pFile, // ------------------------------------------------------------------------------------------------ void XGLImporter::ReadWorld(TempScope &scope) { - XmlNode *root = mXmlParser->getRootNode(); - for (XmlNode &node : root->children()) { + XmlNode root = mXmlParser->getRootNode(); + for (XmlNode &node : root.children()) { const std::string &s = node.name(); // XXX right now we'd skip if it comes after // or @@ -261,7 +252,7 @@ void XGLImporter::ReadWorld(TempScope &scope) { } } - aiNode *const nd = ReadObject( *root, scope, true); + aiNode *const nd = ReadObject(root, scope, true); if (!nd) { ThrowException("failure reading "); } diff --git a/include/assimp/XmlParser.h b/include/assimp/XmlParser.h index c5d45579e..bc827b0f2 100644 --- a/include/assimp/XmlParser.h +++ b/include/assimp/XmlParser.h @@ -79,7 +79,6 @@ class TXmlParser { public: TXmlParser() : mDoc(nullptr), - mRoot(nullptr), mData() { // empty } @@ -90,7 +89,6 @@ public: void clear() { mData.resize(0); - mRoot = nullptr; delete mDoc; mDoc = nullptr; } @@ -117,34 +115,33 @@ public: return nullptr != findNode(name); } - TNodeType *parse(IOStream *stream) { - mRoot = nullptr; + bool parse(IOStream *stream) { if (nullptr == stream) { - return nullptr; + return false; } + bool result = false; mData.resize(stream->FileSize()); stream->Read(&mData[0], mData.size(), 1); mDoc = new pugi::xml_document(); - pugi::xml_parse_result result = mDoc->load_string(&mData[0], pugi::parse_full); - if (result.status == pugi::status_ok) { - pugi::xml_node root = mDoc->document_element(); - mRoot = &root; + pugi::xml_parse_result parse_result = mDoc->load_string(&mData[0], pugi::parse_full); + if (parse_result.status == pugi::status_ok) { + result = true; } - return mRoot; + return result; } pugi::xml_document *getDocument() const { return mDoc; } - const TNodeType *getRootNode() const { - return mRoot; + const TNodeType getRootNode() const { + return mDoc->root(); } - TNodeType *getRootNode() { - return mRoot; + TNodeType getRootNode() { + return mDoc->root(); } static inline bool hasNode(XmlNode &node, const char *name) { @@ -222,7 +219,6 @@ public: private: pugi::xml_document *mDoc; - TNodeType *mRoot; TNodeType mCurrent; std::vector mData; }; @@ -268,6 +264,14 @@ public: return true; } + size_t size() const { + return mNodes.size(); + } + + bool isEmpty() const { + return mNodes.empty(); + } + void clear() { if (mNodes.empty()) { return; diff --git a/test/unit/Common/utXmlParser.cpp b/test/unit/Common/utXmlParser.cpp index 8825f5837..8973dee42 100644 --- a/test/unit/Common/utXmlParser.cpp +++ b/test/unit/Common/utXmlParser.cpp @@ -56,14 +56,33 @@ protected: }; TEST_F(utXmlParser, parse_xml_test) { - DefaultIOSystem ioSystem; - XmlParser parser; - std::string filename = ASSIMP_TEST_MODELS_DIR "/3D/box_a.3d"; - IOStream *stream = ioSystem.Open(filename.c_str(), "rb"); + std::string filename = ASSIMP_TEST_MODELS_DIR "/x3d/ComputerKeyboard.x3d"; + IOStream *stream = mIoSystem.Open(filename.c_str(), "rb"); bool result = parser.parse(stream); EXPECT_TRUE(result); + mIoSystem.Close(stream); } TEST_F(utXmlParser, parse_xml_and_traverse_test) { + XmlParser parser; + std::string filename = ASSIMP_TEST_MODELS_DIR "/x3d/ComputerKeyboard.x3d"; + + IOStream *stream = mIoSystem.Open(filename.c_str(), "rb"); + bool result = parser.parse(stream); + EXPECT_TRUE(result); + XmlNode root = parser.getRootNode(); + std::string name = root.name(); + std::string name1 = root.name(); + EXPECT_NE(nullptr, root); + mIoSystem.Close(stream); + std::string name2 = root.name(); + + XmlNodeIterator nodeIt(root); + EXPECT_TRUE(nodeIt.isEmpty()); + nodeIt.collectChildrenPreOrder(root); + const size_t numNodes = nodeIt.size(); + bool empty = nodeIt.isEmpty(); + EXPECT_FALSE(empty); + EXPECT_NE(numNodes, 0U); } diff --git a/test/unit/utD3MFImportExport.cpp b/test/unit/utD3MFImportExport.cpp index f252ecfb4..31c5d2c84 100644 --- a/test/unit/utD3MFImportExport.cpp +++ b/test/unit/utD3MFImportExport.cpp @@ -50,9 +50,13 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. class utD3MFImporterExporter : public AbstractImportExportBase { public: - virtual bool importerTest() { + bool importerTest() override { Assimp::Importer importer; const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/3MF/box.3mf", aiProcess_ValidateDataStructure); + if (nullptr == scene) { + return false; + } + EXPECT_EQ(1u, scene->mNumMeshes); aiMesh *mesh = scene->mMeshes[0]; EXPECT_NE(nullptr, mesh); @@ -64,7 +68,7 @@ public: #ifndef ASSIMP_BUILD_NO_EXPORT - virtual bool exporterTest() { + bool exporterTest() override { Assimp::Importer importer; const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/3MF/box.3mf", 0);