From 3f75ef68aec103d29751f5754edf4439d271c3c1 Mon Sep 17 00:00:00 2001 From: Max Vollmer Date: Wed, 11 Mar 2020 09:30:04 +0000 Subject: [PATCH 1/2] 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/2] 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); +}