From cedf1819c3331e22a42be31c46fdcd2e992c74de Mon Sep 17 00:00:00 2001 From: Jeremie Dumas Date: Thu, 4 Nov 2021 01:14:24 -0700 Subject: [PATCH 1/4] Do not build ziplib when 3MF exporter is disabled. --- code/CMakeLists.txt | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/code/CMakeLists.txt b/code/CMakeLists.txt index ebc3e0116..c24ef0612 100644 --- a/code/CMakeLists.txt +++ b/code/CMakeLists.txt @@ -934,24 +934,26 @@ ELSE() ENDIF() # zip (https://github.com/kuba--/zip) -IF(ASSIMP_HUNTER_ENABLED) - hunter_add_package(zip) - find_package(zip CONFIG REQUIRED) -ELSE() - SET( ziplib_SRCS - ../contrib/zip/src/miniz.h - ../contrib/zip/src/zip.c - ../contrib/zip/src/zip.h - ) +IF(3MF IN_LIST ASSIMP_EXPORTERS_ENABLED) + IF(ASSIMP_HUNTER_ENABLED) + hunter_add_package(zip) + find_package(zip CONFIG REQUIRED) + ELSE() + SET( ziplib_SRCS + ../contrib/zip/src/miniz.h + ../contrib/zip/src/zip.c + ../contrib/zip/src/zip.h + ) - # TODO if cmake required version has been updated to >3.12.0, collapse this to the second case only - if(${CMAKE_VERSION} VERSION_LESS "3.12.0") - add_definitions(-DMINIZ_USE_UNALIGNED_LOADS_AND_STORES=0) - else() - add_compile_definitions(MINIZ_USE_UNALIGNED_LOADS_AND_STORES=0) - endif() + # TODO if cmake required version has been updated to >3.12.0, collapse this to the second case only + if(${CMAKE_VERSION} VERSION_LESS "3.12.0") + add_definitions(-DMINIZ_USE_UNALIGNED_LOADS_AND_STORES=0) + else() + add_compile_definitions(MINIZ_USE_UNALIGNED_LOADS_AND_STORES=0) + endif() - SOURCE_GROUP( ziplib FILES ${ziplib_SRCS} ) + SOURCE_GROUP( ziplib FILES ${ziplib_SRCS} ) + ENDIF() ENDIF() # openddlparser @@ -1153,10 +1155,12 @@ IF(ASSIMP_HUNTER_ENABLED) ZLIB::zlib RapidJSON::rapidjson utf8cpp - zip::zip pugixml ) - + if(TARGET zip::zip) + target_link_libraries(assimp PUBLIC zip::zip) + endif() + if (ASSIMP_BUILD_DRACO) target_link_libraries(assimp PUBLIC ${draco_LIBRARIES}) endif() From 78d72bff59aa2fbba7c42cef6d94c8b99b90532a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Je=CC=81re=CC=81mie=20Dumas?= Date: Sat, 13 Nov 2021 08:56:52 -0800 Subject: [PATCH 2/4] Fix 3MF presence test. --- code/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/code/CMakeLists.txt b/code/CMakeLists.txt index 423057068..f5f3d754e 100644 --- a/code/CMakeLists.txt +++ b/code/CMakeLists.txt @@ -953,7 +953,8 @@ ELSE() ENDIF() # zip (https://github.com/kuba--/zip) -IF(3MF IN_LIST ASSIMP_EXPORTERS_ENABLED) +separate_arguments(ASSIMP_EXPORTERS_LIST UNIX_COMMAND ${ASSIMP_EXPORTERS_ENABLED}) +IF(3MF IN_LIST ASSIMP_EXPORTERS_LIST) IF(ASSIMP_HUNTER_ENABLED) hunter_add_package(zip) find_package(zip CONFIG REQUIRED) From bab8b8dbabfe87b912252167bc503916127747a3 Mon Sep 17 00:00:00 2001 From: RichardTea <31507749+RichardTea@users.noreply.github.com> Date: Wed, 17 Nov 2021 17:08:19 +0000 Subject: [PATCH 3/4] apply clangformat --- code/AssetLib/Collada/ColladaParser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/AssetLib/Collada/ColladaParser.cpp b/code/AssetLib/Collada/ColladaParser.cpp index 94b9b18ed..66d30f6b5 100644 --- a/code/AssetLib/Collada/ColladaParser.cpp +++ b/code/AssetLib/Collada/ColladaParser.cpp @@ -2204,7 +2204,7 @@ void ColladaParser::ReadMaterialVertexInputBinding(XmlNode &node, Collada::Seman void ColladaParser::ReadEmbeddedTextures(ZipArchiveIOSystem &zip_archive) { // Attempt to load any undefined Collada::Image in ImageLibrary - for (auto & it : mImageLibrary) { + for (auto &it : mImageLibrary) { Collada::Image &image = it.second; if (image.mImageData.empty()) { From 74b3be194d6db278f856483a1d64e4a7ab0502b6 Mon Sep 17 00:00:00 2001 From: RichardTea <31507749+RichardTea@users.noreply.github.com> Date: Wed, 17 Nov 2021 17:10:10 +0000 Subject: [PATCH 4/4] Read the Value, not the Attribute Correct some mistakes made when moving to pugixml from IrrXML Fixes #4179 --- code/AssetLib/Collada/ColladaParser.cpp | 59 +++++++++++++------------ include/assimp/XmlParser.h | 54 ++++++++++++++++------ test/unit/utColladaExport.cpp | 16 +++++-- 3 files changed, 83 insertions(+), 46 deletions(-) diff --git a/code/AssetLib/Collada/ColladaParser.cpp b/code/AssetLib/Collada/ColladaParser.cpp index 66d30f6b5..8bbdf02cc 100644 --- a/code/AssetLib/Collada/ColladaParser.cpp +++ b/code/AssetLib/Collada/ColladaParser.cpp @@ -924,6 +924,8 @@ void ColladaParser::ReadMaterial(XmlNode &node, Collada::Material &pMaterial) { void ColladaParser::ReadLight(XmlNode &node, Collada::Light &pLight) { XmlNodeIterator xmlIt(node, XmlNodeIterator::PreOrderMode); XmlNode currentNode; + // TODO: Check the current technique and skip over unsupported extra techniques + while (xmlIt.getNext(currentNode)) { const std::string ¤tName = currentNode.name(); if (currentName == "spot") { @@ -949,33 +951,34 @@ void ColladaParser::ReadLight(XmlNode &node, Collada::Light &pLight) { content = fast_atoreal_move(content, (ai_real &)pLight.mColor.b); SkipSpacesAndLineEnd(&content); } else if (currentName == "constant_attenuation") { - XmlParser::getRealAttribute(currentNode, "constant_attenuation", pLight.mAttConstant); + XmlParser::getValueAsFloat(currentNode, pLight.mAttConstant); } else if (currentName == "linear_attenuation") { - XmlParser::getRealAttribute(currentNode, "linear_attenuation", pLight.mAttLinear); + XmlParser::getValueAsFloat(currentNode, pLight.mAttLinear); } else if (currentName == "quadratic_attenuation") { - XmlParser::getRealAttribute(currentNode, "quadratic_attenuation", pLight.mAttQuadratic); + XmlParser::getValueAsFloat(currentNode, pLight.mAttQuadratic); } else if (currentName == "falloff_angle") { - XmlParser::getRealAttribute(currentNode, "falloff_angle", pLight.mFalloffAngle); + XmlParser::getValueAsFloat(currentNode, pLight.mFalloffAngle); } else if (currentName == "falloff_exponent") { - XmlParser::getRealAttribute(currentNode, "falloff_exponent", pLight.mFalloffExponent); + XmlParser::getValueAsFloat(currentNode, pLight.mFalloffExponent); } // FCOLLADA extensions // ------------------------------------------------------- else if (currentName == "outer_cone") { - XmlParser::getRealAttribute(currentNode, "outer_cone", pLight.mOuterAngle); - } else if (currentName == "penumbra_angle") { // ... and this one is even deprecated - XmlParser::getRealAttribute(currentNode, "penumbra_angle", pLight.mPenumbraAngle); + XmlParser::getValueAsFloat(currentNode, pLight.mOuterAngle); + } else if (currentName == "penumbra_angle") { // this one is deprecated, now calculated using outer_cone + XmlParser::getValueAsFloat(currentNode, pLight.mPenumbraAngle); } else if (currentName == "intensity") { - XmlParser::getRealAttribute(currentNode, "intensity", pLight.mIntensity); - } else if (currentName == "falloff") { - XmlParser::getRealAttribute(currentNode, "falloff", pLight.mOuterAngle); + XmlParser::getValueAsFloat(currentNode, pLight.mIntensity); + } + else if (currentName == "falloff") { + XmlParser::getValueAsFloat(currentNode, pLight.mOuterAngle); } else if (currentName == "hotspot_beam") { - XmlParser::getRealAttribute(currentNode, "hotspot_beam", pLight.mFalloffAngle); + XmlParser::getValueAsFloat(currentNode, pLight.mFalloffAngle); } // OpenCOLLADA extensions // ------------------------------------------------------- else if (currentName == "decay_falloff") { - XmlParser::getRealAttribute(currentNode, "decay_falloff", pLight.mOuterAngle); + XmlParser::getValueAsFloat(currentNode, pLight.mOuterAngle); } } } @@ -1109,7 +1112,7 @@ void ColladaParser::ReadEffectProfileCommon(XmlNode &node, Collada::Effect &pEff // GOOGLEEARTH/OKINO extensions // ------------------------------------------------------- else if (currentName == "double_sided") - XmlParser::getBoolAttribute(currentNode, currentName.c_str(), pEffect.mDoubleSided); + XmlParser::getValueAsBool(currentNode, pEffect.mDoubleSided); // FCOLLADA extensions // ------------------------------------------------------- @@ -1121,9 +1124,9 @@ void ColladaParser::ReadEffectProfileCommon(XmlNode &node, Collada::Effect &pEff // MAX3D extensions // ------------------------------------------------------- else if (currentName == "wireframe") { - XmlParser::getBoolAttribute(currentNode, currentName.c_str(), pEffect.mWireframe); + XmlParser::getValueAsBool(currentNode, pEffect.mWireframe); } else if (currentName == "faceted") { - XmlParser::getBoolAttribute(currentNode, currentName.c_str(), pEffect.mFaceted); + XmlParser::getValueAsBool(currentNode, pEffect.mFaceted); } } } @@ -1142,23 +1145,23 @@ void ColladaParser::ReadSamplerProperties(XmlNode &node, Sampler &out) { // MAYA extensions // ------------------------------------------------------- if (currentName == "wrapU") { - XmlParser::getBoolAttribute(currentNode, currentName.c_str(), out.mWrapU); + XmlParser::getValueAsBool(currentNode, out.mWrapU); } else if (currentName == "wrapV") { - XmlParser::getBoolAttribute(currentNode, currentName.c_str(), out.mWrapV); + XmlParser::getValueAsBool(currentNode, out.mWrapV); } else if (currentName == "mirrorU") { - XmlParser::getBoolAttribute(currentNode, currentName.c_str(), out.mMirrorU); + XmlParser::getValueAsBool(currentNode, out.mMirrorU); } else if (currentName == "mirrorV") { - XmlParser::getBoolAttribute(currentNode, currentName.c_str(), out.mMirrorV); + XmlParser::getValueAsBool(currentNode, out.mMirrorV); } else if (currentName == "repeatU") { - XmlParser::getRealAttribute(currentNode, currentName.c_str(), out.mTransform.mScaling.x); + XmlParser::getValueAsFloat(currentNode, out.mTransform.mScaling.x); } else if (currentName == "repeatV") { - XmlParser::getRealAttribute(currentNode, currentName.c_str(), out.mTransform.mScaling.y); + XmlParser::getValueAsFloat(currentNode, out.mTransform.mScaling.y); } else if (currentName == "offsetU") { - XmlParser::getRealAttribute(currentNode, currentName.c_str(), out.mTransform.mTranslation.x); + XmlParser::getValueAsFloat(currentNode, out.mTransform.mTranslation.x); } else if (currentName == "offsetV") { - XmlParser::getRealAttribute(currentNode, currentName.c_str(), out.mTransform.mTranslation.y); + XmlParser::getValueAsFloat(currentNode, out.mTransform.mTranslation.y); } else if (currentName == "rotateUV") { - XmlParser::getRealAttribute(currentNode, currentName.c_str(), out.mTransform.mRotation); + XmlParser::getValueAsFloat(currentNode, out.mTransform.mRotation); } else if (currentName == "blend_mode") { std::string v; XmlParser::getValueAsString(currentNode, v); @@ -1178,14 +1181,14 @@ void ColladaParser::ReadSamplerProperties(XmlNode &node, Sampler &out) { // OKINO extensions // ------------------------------------------------------- else if (currentName == "weighting") { - XmlParser::getRealAttribute(currentNode, currentName.c_str(), out.mWeighting); + XmlParser::getValueAsFloat(currentNode, out.mWeighting); } else if (currentName == "mix_with_previous_layer") { - XmlParser::getRealAttribute(currentNode, currentName.c_str(), out.mMixWithPrevious); + XmlParser::getValueAsFloat(currentNode, out.mMixWithPrevious); } // MAX3D extensions // ------------------------------------------------------- else if (currentName == "amount") { - XmlParser::getRealAttribute(currentNode, currentName.c_str(), out.mWeighting); + XmlParser::getValueAsFloat(currentNode, out.mWeighting); } } } diff --git a/include/assimp/XmlParser.h b/include/assimp/XmlParser.h index 9c2dc419e..dfaab5169 100644 --- a/include/assimp/XmlParser.h +++ b/include/assimp/XmlParser.h @@ -42,8 +42,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #ifndef INCLUDED_AI_IRRXML_WRAPPER #define INCLUDED_AI_IRRXML_WRAPPER -#include #include +#include #include "BaseImporter.h" #include "IOStream.hpp" @@ -112,7 +112,7 @@ public: /// @brief Will clear the parsed xml-file. void clear() { - if(mData.empty()) { + if (mData.empty()) { mDoc = nullptr; return; } @@ -243,7 +243,7 @@ public: /// @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 ) { + static inline bool getIntAttribute(XmlNode &xmlNode, const char *name, int &val) { pugi::xml_attribute attr = xmlNode.attribute(name); if (attr.empty()) { return false; @@ -258,7 +258,7 @@ public: /// @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 ) { + static inline bool getRealAttribute(XmlNode &xmlNode, const char *name, ai_real &val) { pugi::xml_attribute attr = xmlNode.attribute(name); if (attr.empty()) { return false; @@ -284,7 +284,6 @@ public: val = attr.as_float(); return true; - } /// @brief Will try to get a double attribute value. @@ -322,7 +321,7 @@ public: /// @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 ) { + static inline bool getBoolAttribute(XmlNode &xmlNode, const char *name, bool &val) { pugi::xml_attribute attr = xmlNode.attribute(name); if (attr.empty()) { return false; @@ -330,14 +329,13 @@ public: val = attr.as_bool(); return true; - } /// @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 ) { + static inline bool getValueAsString(XmlNode &node, std::string &text) { text = std::string(); if (node.empty()) { return false; @@ -352,7 +350,7 @@ public: /// @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 ) { + static inline bool getValueAsFloat(XmlNode &node, ai_real &v) { if (node.empty()) { return false; } @@ -360,10 +358,38 @@ public: v = node.text().as_float(); return true; - } - private: + /// @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; + } + + /// @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; + } + +private: pugi::xml_document *mDoc; TNodeType mCurrent; std::vector mData; @@ -376,8 +402,8 @@ class XmlNodeIterator { public: /// @brief The iteration mode. enum IterationMode { - PreOrderMode, ///< Pre-ordering, get the values, continue the iteration. - PostOrderMode ///< Post-ordering, continue the iteration, get the values. + 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. @@ -400,7 +426,7 @@ public: /// @brief Will iterate through all children in pre-order iteration. /// @param node [in] The nod to iterate through. - void collectChildrenPreOrder( XmlNode &node ) { + void collectChildrenPreOrder(XmlNode &node) { if (node != mParent && node.type() == pugi::node_element) { mNodes.push_back(node); } diff --git a/test/unit/utColladaExport.cpp b/test/unit/utColladaExport.cpp index 8880f81e0..d0c22ffb8 100644 --- a/test/unit/utColladaExport.cpp +++ b/test/unit/utColladaExport.cpp @@ -125,13 +125,21 @@ TEST_F(utColladaExport, testExportLight) { ASSERT_TRUE(pTest->HasLights()); const unsigned int origNumLights(pTest->mNumLights); - std::unique_ptr origLights(new aiLight[origNumLights]); - std::vector origNames; + // There are FIVE!!! LIGHTS!!! + EXPECT_EQ(5, origNumLights) << "lights.dae should contain five lights"; + + std::vector origLights(5); for (size_t i = 0; i < origNumLights; i++) { - origNames.push_back(pTest->mLights[i]->mName.C_Str()); origLights[i] = *(pTest->mLights[i]); } + // Check loaded first light properly + EXPECT_STREQ("Lamp", origLights[0].mName.C_Str()); + EXPECT_EQ(aiLightSource_POINT, origLights[0].mType); + EXPECT_FLOAT_EQ(1.0f, origLights[0].mAttenuationConstant); + EXPECT_FLOAT_EQ(0.0f, origLights[0].mAttenuationLinear); + EXPECT_FLOAT_EQ(0.00111109f, origLights[0].mAttenuationQuadratic); + // Common metadata // Confirm was loaded by the Collada importer aiString origImporter; @@ -191,7 +199,7 @@ TEST_F(utColladaExport, testExportLight) { for (size_t i = 0; i < origNumLights; i++) { const aiLight *orig = &origLights[i]; const aiLight *read = imported->mLights[i]; - EXPECT_EQ(0, strncmp(origNames[i].c_str(), read->mName.C_Str(), origNames[i].size())); + EXPECT_EQ(0, strcmp(orig->mName.C_Str(), read->mName.C_Str())); EXPECT_EQ(orig->mType, read->mType); EXPECT_FLOAT_EQ(orig->mAttenuationConstant, read->mAttenuationConstant); EXPECT_FLOAT_EQ(orig->mAttenuationLinear, read->mAttenuationLinear);