From 3f75ef68aec103d29751f5754edf4439d271c3c1 Mon Sep 17 00:00:00 2001 From: Max Vollmer Date: Wed, 11 Mar 2020 09:30:04 +0000 Subject: [PATCH 1/4] Assimp guarantees in its docs that mRootNode is never NULL. glTF2Importer::ImportNodes therefore must always create a root node, or throw an exception. A GLTF2 file is invalid without a scene, so the importer should throw in that case. For GLTF2 files with a scene without nodes, it should create an empty root node. --- code/glTF2/glTF2Importer.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/code/glTF2/glTF2Importer.cpp b/code/glTF2/glTF2Importer.cpp index 8c33674e1..56645b737 100644 --- a/code/glTF2/glTF2Importer.cpp +++ b/code/glTF2/glTF2Importer.cpp @@ -953,8 +953,8 @@ aiNode *ImportNode(aiScene *pScene, glTF2::Asset &r, std::vector & void glTF2Importer::ImportNodes(glTF2::Asset &r) { if (!r.scene) { - return; - } + throw DeadlyImportError("GLTF: No scene"); + } std::vector> rootNodes = r.scene->nodes; @@ -971,6 +971,8 @@ void glTF2Importer::ImportNodes(glTF2::Asset &r) { root->mChildren[root->mNumChildren++] = node; } mScene->mRootNode = root; + } else { + mScene->mRootNode = new aiNode("ROOT"); } } From a4bbd9b9365ae880015413270adbd526113e9bf8 Mon Sep 17 00:00:00 2001 From: Max Vollmer Date: Wed, 11 Mar 2020 15:56:54 +0000 Subject: [PATCH 2/4] Added two unit tests for cases where Assimp returned a scene that didn't have a root node: - NoScene tests that Assimp correctly fails importing an invalid GLTF2 file that doesn't have a scene. - SceneWithoutNodes tests that Assimp correctly creates an empty root node for GLTF2 files with a scene that has no nodes. --- test/models/glTF2/TestNoRootNode/NoScene.gltf | 6 ++++++ .../glTF2/TestNoRootNode/SceneWithoutNodes.gltf | 10 ++++++++++ test/unit/utglTF2ImportExport.cpp | 13 +++++++++++++ 3 files changed, 29 insertions(+) create mode 100644 test/models/glTF2/TestNoRootNode/NoScene.gltf create mode 100644 test/models/glTF2/TestNoRootNode/SceneWithoutNodes.gltf diff --git a/test/models/glTF2/TestNoRootNode/NoScene.gltf b/test/models/glTF2/TestNoRootNode/NoScene.gltf new file mode 100644 index 000000000..6cad71ed4 --- /dev/null +++ b/test/models/glTF2/TestNoRootNode/NoScene.gltf @@ -0,0 +1,6 @@ +{ + "asset": { + "version": "2.0" + }, + "scene": 0 +} diff --git a/test/models/glTF2/TestNoRootNode/SceneWithoutNodes.gltf b/test/models/glTF2/TestNoRootNode/SceneWithoutNodes.gltf new file mode 100644 index 000000000..d426ca569 --- /dev/null +++ b/test/models/glTF2/TestNoRootNode/SceneWithoutNodes.gltf @@ -0,0 +1,10 @@ +{ + "asset": { + "version": "2.0" + }, + "scene": 0, + "scenes": [ + { + } + ] +} diff --git a/test/unit/utglTF2ImportExport.cpp b/test/unit/utglTF2ImportExport.cpp index 8b91577f6..927c01f3e 100644 --- a/test/unit/utglTF2ImportExport.cpp +++ b/test/unit/utglTF2ImportExport.cpp @@ -493,3 +493,16 @@ TEST_F(utglTF2ImportExport, texcoords) { EXPECT_EQ(aiGetMaterialInteger(material, AI_MATKEY_GLTF_TEXTURE_TEXCOORD(aiTextureType_UNKNOWN, 0), &uvIndex), aiReturn_SUCCESS); EXPECT_EQ(uvIndex, 1); } + +TEST_F(utglTF2ImportExport, norootnode_noscene) { + Assimp::Importer importer; + const aiScene* scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/glTF2/TestNoRootNode/NoScene.gltf", aiProcess_ValidateDataStructure); + ASSERT_EQ(scene, nullptr); +} + +TEST_F(utglTF2ImportExport, norootnode_scenewithoutnodes) { + Assimp::Importer importer; + const aiScene* scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/glTF2/TestNoRootNode/SceneWithoutNodes.gltf", aiProcess_ValidateDataStructure); + ASSERT_NE(scene, nullptr); + ASSERT_NE(scene->mRootNode, nullptr); +} From 7c3fd351d3e84dd10cdefd2ac10ceee61bc7ead1 Mon Sep 17 00:00:00 2001 From: "chenmou.cm" Date: Fri, 20 Mar 2020 17:19:27 +0800 Subject: [PATCH 3/4] fix no preservePivots bug --- code/FBX/FBXConverter.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/code/FBX/FBXConverter.cpp b/code/FBX/FBXConverter.cpp index 22616a480..971f8f684 100644 --- a/code/FBX/FBXConverter.cpp +++ b/code/FBX/FBXConverter.cpp @@ -883,10 +883,12 @@ namespace Assimp { // name passed to the method is already unique nd->mName.Set(name); - for (const auto &transform : chain) { - nd->mTransformation = nd->mTransformation * transform; - } - return false; + // for (const auto &transform : chain) { + // skip inverse chain for no preservePivots + for (unsigned int i = TransformationComp_Translation; i < TransformationComp_MAXIMUM; i++) { + nd->mTransformation = nd->mTransformation * chain[i]; + } + return false; } void FBXConverter::SetupNodeMetadata(const Model& model, aiNode& nd) From 046f50880f79c1434556e2a356d8ded6d38fc2b5 Mon Sep 17 00:00:00 2001 From: Andy Maloney Date: Mon, 23 Mar 2020 12:35:32 -0400 Subject: [PATCH 4/4] {cmake} Prefix assimp options with "ASSIMP_" to avoid pollution when included as a submodule When libraries are included as submodules in large projects, having an option with a generic name like "BUILD_DOCS" is not very helpful. (e.g. one project I work on includes many libraries as submodules) It can also clash with options from other libraries which can break things. --- CMakeLists.txt | 44 +++++++++++++++++++++--------------------- code/CMakeLists.txt | 32 +++++++++++++++--------------- contrib/CMakeLists.txt | 2 +- 3 files changed, 39 insertions(+), 39 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f01ea52ea..ecd131b8c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -39,9 +39,9 @@ SET(CMAKE_POLICY_DEFAULT_CMP0074 NEW) CMAKE_MINIMUM_REQUIRED( VERSION 3.0 ) # Toggles the use of the hunter package manager -option(HUNTER_ENABLED "Enable Hunter package manager support" OFF) +option(ASSIMP_HUNTER_ENABLED "Enable Hunter package manager support" OFF) -IF(HUNTER_ENABLED) +IF(ASSIMP_HUNTER_ENABLED) include("cmake/HunterGate.cmake") HunterGate( URL "https://github.com/ruslo/hunter/archive/v0.23.176.tar.gz" @@ -60,7 +60,7 @@ OPTION( BUILD_SHARED_LIBS ON ) -OPTION( BUILD_FRAMEWORK +OPTION( ASSIMP_BUILD_FRAMEWORK "Build package as Mac OS X Framework bundle." OFF ) @@ -101,7 +101,7 @@ OPTION ( ASSIMP_COVERALLS OFF ) OPTION( ASSIMP_INSTALL - "DIsable this if you want to use assimp as a submodule." + "Disable this if you want to use assimp as a submodule." ON ) OPTION ( ASSIMP_ERROR_MAX @@ -120,25 +120,25 @@ OPTION ( ASSIMP_UBSAN "Enable Undefined Behavior sanitizer." OFF ) -OPTION ( SYSTEM_IRRXML +OPTION ( ASSIMP_SYSTEM_IRRXML "Use system installed Irrlicht/IrrXML library." OFF ) -OPTION ( BUILD_DOCS +OPTION ( ASSIMP_BUILD_DOCS "Build documentation using Doxygen." OFF ) -OPTION( INJECT_DEBUG_POSTFIX +OPTION( ASSIMP_INJECT_DEBUG_POSTFIX "Inject debug postfix in .a/.so/.dll lib names" ON ) -OPTION ( IGNORE_GIT_HASH +OPTION ( ASSIMP_IGNORE_GIT_HASH "Don't call git to get the hash." OFF ) -IF (IOS AND NOT HUNTER_ENABLED) +IF (IOS AND NOT ASSIMP_HUNTER_ENABLED) IF (NOT CMAKE_BUILD_TYPE) SET(CMAKE_BUILD_TYPE "Release") ENDIF () @@ -161,7 +161,7 @@ IF(MSVC) ENDIF() ENDIF() -IF (BUILD_FRAMEWORK) +IF (ASSIMP_BUILD_FRAMEWORK) SET (BUILD_SHARED_LIBS ON) MESSAGE(STATUS "Framework bundle building enabled") ENDIF() @@ -181,12 +181,12 @@ SET (ASSIMP_VERSION ${ASSIMP_VERSION_MAJOR}.${ASSIMP_VERSION_MINOR}.${ASSIMP_VER SET (ASSIMP_SOVERSION 5) SET( ASSIMP_PACKAGE_VERSION "0" CACHE STRING "the package-specific version used for uploading the sources" ) -if(NOT HUNTER_ENABLED) +if(NOT ASSIMP_HUNTER_ENABLED) # Enable C++11 support globally set_property( GLOBAL PROPERTY CXX_STANDARD 11 ) endif() -IF(NOT IGNORE_GIT_HASH) +IF(NOT ASSIMP_IGNORE_GIT_HASH) # Get the current working branch EXECUTE_PROCESS( COMMAND git rev-parse --abbrev-ref HEAD @@ -245,7 +245,7 @@ ENDIF() # Grouped compiler settings IF ((CMAKE_C_COMPILER_ID MATCHES "GNU") AND NOT CMAKE_COMPILER_IS_MINGW) - IF(NOT HUNTER_ENABLED) + IF(NOT ASSIMP_HUNTER_ENABLED) SET(CMAKE_CXX_FLAGS "-fPIC -std=c++0x ${CMAKE_CXX_FLAGS}") SET(CMAKE_C_FLAGS "-fPIC ${CMAKE_C_FLAGS}") ENDIF() @@ -262,7 +262,7 @@ ELSEIF(MSVC) ENDIF() SET(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /D_DEBUG /Zi /Od") ELSEIF ( "${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang" ) - IF(NOT HUNTER_ENABLED) + IF(NOT ASSIMP_HUNTER_ENABLED) SET(CMAKE_CXX_FLAGS "-fPIC -std=c++11 ${CMAKE_CXX_FLAGS}") SET(CMAKE_C_FLAGS "-fPIC ${CMAKE_C_FLAGS}") ENDIF() @@ -274,7 +274,7 @@ ELSEIF( CMAKE_COMPILER_IS_MINGW ) ELSEIF(CMAKE_CXX_COMPILER_VERSION VERSION_LESS 7.3) message(WARNING "MinGW is old, if you experience errors, update MinGW.") ENDIF() - IF(NOT HUNTER_ENABLED) + IF(NOT ASSIMP_HUNTER_ENABLED) SET(CMAKE_CXX_FLAGS "-std=c++11 ${CMAKE_CXX_FLAGS}") SET(CMAKE_C_FLAGS "-fPIC ${CMAKE_C_FLAGS}") ENDIF() @@ -283,7 +283,7 @@ ELSEIF( CMAKE_COMPILER_IS_MINGW ) ADD_DEFINITIONS( -U__STRICT_ANSI__ ) ENDIF() -IF ( IOS AND NOT HUNTER_ENABLED) +IF ( IOS AND NOT ASSIMP_HUNTER_ENABLED) IF (CMAKE_BUILD_TYPE STREQUAL "Debug") SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fembed-bitcode -Og") SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fembed-bitcode -Og") @@ -360,7 +360,7 @@ SET( ASSIMP_BIN_INSTALL_DIR "bin" CACHE STRING get_cmake_property(is_multi_config GENERATOR_IS_MULTI_CONFIG) -IF (INJECT_DEBUG_POSTFIX AND (is_multi_config OR CMAKE_BUILD_TYPE STREQUAL "Debug")) +IF (ASSIMP_INJECT_DEBUG_POSTFIX AND (is_multi_config OR CMAKE_BUILD_TYPE STREQUAL "Debug")) SET(CMAKE_DEBUG_POSTFIX "d" CACHE STRING "Debug Postfix for lib, samples and tools") ELSE() SET(CMAKE_DEBUG_POSTFIX "" CACHE STRING "Debug Postfix for lib, samples and tools") @@ -373,7 +373,7 @@ IF (NOT TARGET uninstall) ADD_CUSTOM_TARGET(uninstall "${CMAKE_COMMAND}" -P "${CMAKE_CURRENT_BINARY_DIR}/cmake_uninstall.cmake") ENDIF() -IF(HUNTER_ENABLED) +IF(ASSIMP_HUNTER_ENABLED) set(CONFIG_INSTALL_DIR "lib/cmake/${PROJECT_NAME}") set(INCLUDE_INSTALL_DIR "include") @@ -440,18 +440,18 @@ ELSE() DESTINATION "${ASSIMP_LIB_INSTALL_DIR}/cmake/assimp-${ASSIMP_VERSION_MAJOR}.${ASSIMP_VERSION_MINOR}" COMPONENT ${LIBASSIMP-DEV_COMPONENT}) ENDIF() -IF( BUILD_DOCS ) +IF( ASSIMP_BUILD_DOCS ) ADD_SUBDIRECTORY(doc) ENDIF() # Look for system installed irrXML -IF ( SYSTEM_IRRXML ) +IF ( ASSIMP_SYSTEM_IRRXML ) FIND_PACKAGE( IrrXML REQUIRED ) ENDIF() # Search for external dependencies, and build them from source if not found # Search for zlib -IF(HUNTER_ENABLED) +IF(ASSIMP_HUNTER_ENABLED) hunter_add_package(ZLIB) find_package(ZLIB CONFIG REQUIRED) @@ -575,7 +575,7 @@ ELSE () ADD_DEFINITIONS( -DASSIMP_BUILD_NO_C4D_IMPORTER ) ENDIF () -IF(NOT HUNTER_ENABLED) +IF(NOT ASSIMP_HUNTER_ENABLED) ADD_SUBDIRECTORY(contrib) ENDIF() diff --git a/code/CMakeLists.txt b/code/CMakeLists.txt index 0776b2b39..c872e7b20 100644 --- a/code/CMakeLists.txt +++ b/code/CMakeLists.txt @@ -886,7 +886,7 @@ SET( Extra_SRCS SOURCE_GROUP( Extra FILES ${Extra_SRCS}) # irrXML -IF(HUNTER_ENABLED) +IF(ASSIMP_HUNTER_ENABLED) hunter_add_package(irrXML) find_package(irrXML CONFIG REQUIRED) ELSE() @@ -894,7 +894,7 @@ ELSE() ENDIF() # utf8 -IF(HUNTER_ENABLED) +IF(ASSIMP_HUNTER_ENABLED) hunter_add_package(utf8) find_package(utf8 CONFIG REQUIRED) ELSE() @@ -902,7 +902,7 @@ ELSE() ENDIF() # polyclipping -IF(HUNTER_ENABLED) +IF(ASSIMP_HUNTER_ENABLED) hunter_add_package(polyclipping) find_package(polyclipping CONFIG REQUIRED) ELSE() @@ -914,7 +914,7 @@ ELSE() ENDIF() # poly2tri -IF(HUNTER_ENABLED) +IF(ASSIMP_HUNTER_ENABLED) hunter_add_package(poly2tri) find_package(poly2tri CONFIG REQUIRED) ELSE() @@ -935,7 +935,7 @@ ELSE() ENDIF() # minizip/unzip -IF(HUNTER_ENABLED) +IF(ASSIMP_HUNTER_ENABLED) hunter_add_package(minizip) find_package(minizip CONFIG REQUIRED) ELSE() @@ -950,7 +950,7 @@ ELSE() ENDIF() # zip (https://github.com/kuba--/zip) -IF(HUNTER_ENABLED) +IF(ASSIMP_HUNTER_ENABLED) hunter_add_package(zip) find_package(zip CONFIG REQUIRED) ELSE() @@ -971,7 +971,7 @@ ELSE() ENDIF() # openddlparser -IF(HUNTER_ENABLED) +IF(ASSIMP_HUNTER_ENABLED) hunter_add_package(openddlparser) find_package(openddlparser CONFIG REQUIRED) ELSE() @@ -994,7 +994,7 @@ ELSE() ENDIF() # Open3DGC -IF(HUNTER_ENABLED) +IF(ASSIMP_HUNTER_ENABLED) # Nothing to do, not available in Hunter yet. ELSE() SET ( open3dgc_SRCS @@ -1035,7 +1035,7 @@ ENDIF() # RT-extensions is used in "contrib/Open3DGC/o3dgcTimer.h" for collecting statistics. Pointed file # has implementation for different platforms: WIN32, __MACH__ and other ("else" block). FIND_PACKAGE(RT QUIET) -IF (NOT HUNTER_ENABLED AND (RT_FOUND OR MSVC)) +IF (NOT ASSIMP_HUNTER_ENABLED AND (RT_FOUND OR MSVC)) SET( ASSIMP_IMPORTER_GLTF_USE_OPEN3DGC 1 ) ADD_DEFINITIONS( -DASSIMP_IMPORTER_GLTF_USE_OPEN3DGC=1 ) ELSE () @@ -1045,7 +1045,7 @@ ELSE () ENDIF () # RapidJSON -IF(HUNTER_ENABLED) +IF(ASSIMP_HUNTER_ENABLED) hunter_add_package(RapidJSON) find_package(RapidJSON CONFIG REQUIRED) ELSE() @@ -1068,7 +1068,7 @@ if ( MSVC ) ADD_DEFINITIONS( -D_CRT_SECURE_NO_WARNINGS ) endif () -IF(NOT HUNTER_ENABLED) +IF(NOT ASSIMP_HUNTER_ENABLED) if (UNZIP_FOUND) SET (unzip_compile_SRCS "") else () @@ -1118,7 +1118,7 @@ SET( assimp_src ) ADD_DEFINITIONS( -DOPENDDLPARSER_BUILD ) -IF(NOT HUNTER_ENABLED) +IF(NOT ASSIMP_HUNTER_ENABLED) INCLUDE_DIRECTORIES( ${IRRXML_INCLUDE_DIR} ../contrib/openddlparser/include @@ -1139,7 +1139,7 @@ TARGET_INCLUDE_DIRECTORIES ( assimp PUBLIC $ ) -IF(HUNTER_ENABLED) +IF(ASSIMP_HUNTER_ENABLED) TARGET_LINK_LIBRARIES(assimp PUBLIC polyclipping::polyclipping @@ -1212,7 +1212,7 @@ SET_TARGET_PROPERTIES( assimp PROPERTIES ) if (APPLE) - if (BUILD_FRAMEWORK) + if (ASSIMP_BUILD_FRAMEWORK) SET_TARGET_PROPERTIES( assimp PROPERTIES FRAMEWORK TRUE FRAMEWORK_VERSION C @@ -1232,7 +1232,7 @@ ENDIF() # Build against external unzip, or add ../contrib/unzip so # assimp can #include "unzip.h" -IF(NOT HUNTER_ENABLED) +IF(NOT ASSIMP_HUNTER_ENABLED) if (UNZIP_FOUND) INCLUDE_DIRECTORIES(${UNZIP_INCLUDE_DIRS}) TARGET_LINK_LIBRARIES(assimp ${UNZIP_LIBRARIES}) @@ -1246,7 +1246,7 @@ IF (RT_FOUND AND ASSIMP_IMPORTER_GLTF_USE_OPEN3DGC) TARGET_LINK_LIBRARIES(assimp ${RT_LIBRARY}) ENDIF () -IF(HUNTER_ENABLED) +IF(ASSIMP_HUNTER_ENABLED) INSTALL( TARGETS assimp EXPORT "${TARGETS_EXPORT_NAME}" LIBRARY DESTINATION ${ASSIMP_LIB_INSTALL_DIR} diff --git a/contrib/CMakeLists.txt b/contrib/CMakeLists.txt index 8394ad703..f1d023bec 100644 --- a/contrib/CMakeLists.txt +++ b/contrib/CMakeLists.txt @@ -1,4 +1,4 @@ # Compile internal irrXML only if system is not requested -if( NOT SYSTEM_IRRXML ) +if( NOT ASSIMP_SYSTEM_IRRXML ) add_subdirectory(irrXML) endif()