From 212903e935d64e8d0d3f20c68603744ddbc213f9 Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Wed, 15 Jul 2020 12:19:00 +0100 Subject: [PATCH] Unit test for all indices out of range, and fix. --- code/AssetLib/glTF2/glTF2Importer.cpp | 8 ++++++-- test/unit/utglTF2ImportExport.cpp | 25 ++++++++++++++++--------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/code/AssetLib/glTF2/glTF2Importer.cpp b/code/AssetLib/glTF2/glTF2Importer.cpp index 9ce488cbd..0b784e0d4 100644 --- a/code/AssetLib/glTF2/glTF2Importer.cpp +++ b/code/AssetLib/glTF2/glTF2Importer.cpp @@ -574,7 +574,7 @@ void glTF2Importer::ImportMeshes(glTF2::Asset &r) { case PrimitiveMode_TRIANGLE_FAN: nFaces = count - 2; facePtr = faces = new aiFace[nFaces]; - SetFaceAndAdvance(facePtr, data.GetUInt(0), data.GetUInt(1), data.GetUInt(2)); + SetFaceAndAdvance(facePtr, aim->mNumVertices, data.GetUInt(0), data.GetUInt(1), data.GetUInt(2)); for (unsigned int i = 1; i < nFaces; ++i) { SetFaceAndAdvance(facePtr, aim->mNumVertices, faces[0].mIndices[0], faces[i - 1].mIndices[2], data.GetUInt(i + 2)); } @@ -664,7 +664,11 @@ void glTF2Importer::ImportMeshes(glTF2::Asset &r) { aim->mFaces = faces; const unsigned int actualNumFaces = static_cast(facePtr - faces); if (actualNumFaces < nFaces) { - ASSIMP_LOG_WARN("Some faces had out-of-range indices. Those faces were dropped."); + ASSIMP_LOG_WARN("Some faces in mesh had out-of-range indices. Those faces were dropped."); + } + if (actualNumFaces == 0) + { + throw DeadlyImportError(std::string("Mesh \"") + aim->mName.C_Str() + "\" has no faces"); } aim->mNumFaces = actualNumFaces; ai_assert(CheckValidFacesIndices(faces, actualNumFaces, aim->mNumVertices)); diff --git a/test/unit/utglTF2ImportExport.cpp b/test/unit/utglTF2ImportExport.cpp index 8bc20e950..c3ad2d994 100644 --- a/test/unit/utglTF2ImportExport.cpp +++ b/test/unit/utglTF2ImportExport.cpp @@ -46,11 +46,13 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include #include +#include +#include + #include #include - using namespace Assimp; class utglTF2ImportExport : public AbstractImportExportBase { @@ -542,14 +544,11 @@ TEST_F(utglTF2ImportExport, norootnode_issue_3269) { ASSERT_EQ(scene, nullptr); } -#include -#include - TEST_F(utglTF2ImportExport, indexOutOfRange) { // The contents of an asset should not lead to an assert. Assimp::Importer importer; - struct WarningObserver : Assimp::LogStream + struct LogObserver : Assimp::LogStream { bool m_observedWarning = false; void write(const char *message) override @@ -557,15 +556,23 @@ TEST_F(utglTF2ImportExport, indexOutOfRange) { m_observedWarning = m_observedWarning || std::strstr(message, "faces were dropped"); } }; - WarningObserver warningObserver; + LogObserver logObserver; - DefaultLogger::get()->attachStream(&warningObserver); + DefaultLogger::get()->attachStream(&logObserver); const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/glTF2/IndexOutOfRange/IndexOutOfRange.gltf", aiProcess_ValidateDataStructure); ASSERT_NE(scene, nullptr); ASSERT_NE(scene->mRootNode, nullptr); ASSERT_EQ(scene->mNumMeshes, 1); EXPECT_EQ(scene->mMeshes[0]->mNumFaces, 11); - DefaultLogger::get()->detachStream(&warningObserver); - EXPECT_TRUE(warningObserver.m_observedWarning); + DefaultLogger::get()->detachStream(&logObserver); + EXPECT_TRUE(logObserver.m_observedWarning); } +TEST_F(utglTF2ImportExport, allIndicesOutOfRange) { + // The contents of an asset should not lead to an assert. + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/glTF2/IndexOutOfRange/AllIndicesOutOfRange.gltf", aiProcess_ValidateDataStructure); + ASSERT_EQ(scene, nullptr); + std::string error = importer.GetErrorString(); + ASSERT_NE(error.find("Mesh \"Mesh\" has no faces"), std::string::npos); +}