Merge pull request #3214 from jercytryn/master

[gltf2 Export] More robust handling for non-finites and 0-length normals

code/AssetLib/glTF/glTFAssetWriter.inl

@@ -59,7 +59,7 @@ namespace glTF {
     namespace {
 
         template<typename T, size_t N>
-        inline 
+        inline
         Value& MakeValue(Value& val, T(&r)[N], MemoryPoolAllocator<>& al) {
             val.SetArray();
             val.Reserve(N, al);
@@ -70,7 +70,7 @@ namespace glTF {
         }
 
         template<typename T>
-        inline 
+        inline
         Value& MakeValue(Value& val, const std::vector<T> & r, MemoryPoolAllocator<>& al) {
             val.SetArray();
             val.Reserve(static_cast<rapidjson::SizeType>(r.size()), al);
@@ -530,7 +530,9 @@ namespace glTF {
         StringBuffer docBuffer;
 
         PrettyWriter<StringBuffer> writer(docBuffer);
-        mDoc.Accept(writer);
+        if (!mDoc.Accept(writer)) {
+            throw DeadlyExportError("Failed to write scene data!");
+        }
 
         if (jsonOutFile->Write(docBuffer.GetString(), docBuffer.GetSize(), 1) != 1) {
             throw DeadlyExportError("Failed to write scene data!");
@@ -569,7 +571,9 @@ namespace glTF {
 
         StringBuffer docBuffer;
         Writer<StringBuffer> writer(docBuffer);
-        mDoc.Accept(writer);
+        if (!mDoc.Accept(writer)) {
+            throw DeadlyExportError("Failed to write scene data!");
+        }
 
         if (outfile->Write(docBuffer.GetString(), docBuffer.GetSize(), 1) != 1) {
             throw DeadlyExportError("Failed to write scene data!");

code/AssetLib/glTF/glTFExporter.cpp

@@ -348,7 +348,7 @@ void glTFExporter::GetMatColorOrTex(const aiMaterial* mat, glTF::TexProperty& pr
 
                     if (path[0] == '*') { // embedded
                         aiTexture* curTex = mScene->mTextures[atoi(&path[1])];
-						
+
                         prop.texture->source->name = curTex->mFilename.C_Str();
 
                         uint8_t *data = reinterpret_cast<uint8_t *>(curTex->pcData);

code/AssetLib/glTF2/glTF2AssetWriter.inl

@@ -613,7 +613,9 @@ namespace glTF2 {
         StringBuffer docBuffer;
 
         PrettyWriter<StringBuffer> writer(docBuffer);
-        mDoc.Accept(writer);
+        if (!mDoc.Accept(writer)) {
+            throw DeadlyExportError("Failed to write scene data!");
+        }
 
         if (jsonOutFile->Write(docBuffer.GetString(), docBuffer.GetSize(), 1) != 1) {
             throw DeadlyExportError("Failed to write scene data!");
@@ -664,7 +666,9 @@ namespace glTF2 {
 
         StringBuffer docBuffer;
         Writer<StringBuffer> writer(docBuffer);
-        mDoc.Accept(writer);
+        if (!mDoc.Accept(writer)) {
+            throw DeadlyExportError("Failed to write scene data!");
+        }
 
         uint32_t jsonChunkLength = (docBuffer.GetSize() + 3) & ~3; // Round up to next multiple of 4
         auto paddingLength = jsonChunkLength - docBuffer.GetSize();
@@ -816,5 +820,3 @@ namespace glTF2 {
     }
 
 }
-
-

code/AssetLib/glTF2/glTF2Exporter.cpp

@@ -1,4 +1,4 @@
-/*
+/*
 Open Asset Import Library (assimp)
 ----------------------------------------------------------------------
 
@@ -172,6 +172,13 @@ void SetAccessorRange(Ref<Accessor> acc, void* data, size_t count,
 		for (unsigned int j = 0 ; j < numCompsOut ; j++) {
 			double valueTmp = buffer_ptr[j];
 
+			// Gracefully tolerate rogue NaN's in buffer data
+			// Any NaNs/Infs introduced in accessor bounds will end up in
+			// document and prevent rapidjson from writing out valid JSON
+			if (!std::isfinite(valueTmp)) {
+				continue;
+			}
+
 			if (valueTmp < acc->min[j]) {
 				acc->min[j] = valueTmp;
 			}
@@ -348,7 +355,7 @@ void glTF2Exporter::GetMatTex(const aiMaterial* mat, Ref<Texture>& texture, aiTe
 
                     if (path[0] == '*') { // embedded
                         aiTexture* curTex = mScene->mTextures[atoi(&path[1])];
-						
+
                         texture->source->name = curTex->mFilename.C_Str();
 
                         // The asset has its own buffer, see Image::SetData
@@ -751,7 +758,7 @@ void glTF2Exporter::ExportMeshes()
         // Normalize all normals as the validator can emit a warning otherwise
         if ( nullptr != aim->mNormals) {
             for ( auto i = 0u; i < aim->mNumVertices; ++i ) {
-                aim->mNormals[ i ].Normalize();
+                aim->mNormals[ i ].NormalizeSafe();
             }
         }
 
@@ -762,7 +769,7 @@ void glTF2Exporter::ExportMeshes()
         for (int i = 0; i < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++i) {
 			if (!aim->HasTextureCoords(i))
 				continue;
-			
+
             // Flip UV y coords
             if (aim -> mNumUVComponents[i] > 1) {
                 for (unsigned int j = 0; j < aim->mNumVertices; ++j) {

test/unit/utglTF2ImportExport.cpp

@@ -436,6 +436,31 @@ TEST_F(utglTF2ImportExport, error_string_preserved) {
     ASSERT_NE(error.find("BoxTextured0.bin"), std::string::npos) << "Error string should contain an error about missing .bin file";
 }
 
+TEST_F(utglTF2ImportExport, export_bad_accessor_bounds) {
+    Assimp::Importer importer;
+    Assimp::Exporter exporter;
+    const aiScene* scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/glTF2/BoxWithInfinites-glTF-Binary/BoxWithInfinites.glb", aiProcess_ValidateDataStructure);
+    ASSERT_NE(scene, nullptr);
+
+    EXPECT_EQ(aiReturn_SUCCESS, exporter.Export(scene, "glb2", ASSIMP_TEST_MODELS_DIR "/glTF2/BoxWithInfinites-glTF-Binary/BoxWithInfinites_out.glb"));
+    EXPECT_EQ(aiReturn_SUCCESS, exporter.Export(scene, "gltf2", ASSIMP_TEST_MODELS_DIR "/glTF2/BoxWithInfinites-glTF-Binary/BoxWithInfinites_out.gltf"));
+}
+
+TEST_F(utglTF2ImportExport, export_normalized_normals) {
+    Assimp::Importer importer;
+    Assimp::Exporter exporter;
+    const aiScene* scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/glTF2/BoxBadNormals-glTF-Binary/BoxBadNormals.glb", aiProcess_ValidateDataStructure);
+    ASSERT_NE(scene, nullptr);
+    EXPECT_EQ(aiReturn_SUCCESS, exporter.Export(scene, "glb2", ASSIMP_TEST_MODELS_DIR "/glTF2/BoxBadNormals-glTF-Binary/BoxBadNormals_out.glb"));
+
+    // load in again and ensure normal-length normals but no Nan's or Inf's introduced
+    scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/glTF2/BoxBadNormals-glTF-Binary/BoxBadNormals_out.glb", aiProcess_ValidateDataStructure);
+    for ( auto i = 0u; i < scene->mMeshes[0]->mNumVertices; ++i ) {
+        const auto length = scene->mMeshes[0]->mNormals[i].Length();
+        EXPECT_TRUE(abs(length) < 1e-6 || abs(length - 1) < 1e-6);
+    }
+}
+
 #endif // ASSIMP_BUILD_NO_EXPORT
 
 TEST_F(utglTF2ImportExport, sceneMetadata) {