From c3a21666dadd01f7a2f532d3dc0ecaf2045aa81b Mon Sep 17 00:00:00 2001
From: Jeremy Cytryn <jcytryn@fb.com>
Date: Thu, 7 May 2020 01:21:54 -0700
Subject: [PATCH] Make gltf2 export normal normalization safe

This avoids introducing NaNs e.g. when the input mesh has 0-length normals
---
 code/AssetLib/glTF/glTFExporter.cpp              |   2 +-
 code/AssetLib/glTF2/glTF2Exporter.cpp            |   2 +-
 .../BoxBadNormals-glTF-Binary/BoxBadNormals.glb  | Bin 0 -> 1924 bytes
 test/unit/utglTF2ImportExport.cpp                |  15 +++++++++++++++
 4 files changed, 17 insertions(+), 2 deletions(-)
 create mode 100644 test/models/glTF2/BoxBadNormals-glTF-Binary/BoxBadNormals.glb

diff --git a/code/AssetLib/glTF/glTFExporter.cpp b/code/AssetLib/glTF/glTFExporter.cpp
index e6c14e7dd..b85affc08 100644
--- a/code/AssetLib/glTF/glTFExporter.cpp
+++ b/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);
diff --git a/code/AssetLib/glTF2/glTF2Exporter.cpp b/code/AssetLib/glTF2/glTF2Exporter.cpp
index 7ed4f2670..566f95e80 100644
--- a/code/AssetLib/glTF2/glTF2Exporter.cpp
+++ b/code/AssetLib/glTF2/glTF2Exporter.cpp
@@ -758,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();
             }
         }
 
diff --git a/test/models/glTF2/BoxBadNormals-glTF-Binary/BoxBadNormals.glb b/test/models/glTF2/BoxBadNormals-glTF-Binary/BoxBadNormals.glb
new file mode 100644
index 0000000000000000000000000000000000000000..c36727d12ce675125a3f48e1ebeb68fd1a3992b2
GIT binary patch
literal 1924
zcmb_d+iu!G5OweO`|TC&Q!6>hHX#XqsYsMaRDe@R`w~T!fdwr2g4QNYgpl&kkLb@-
z{ee{KC-uy-OH2}}M2gK}E;~E3GdsH;gnK(T4a4|x&oF-8HjGbguQ_3Un(~af6L!c&
z8YD4unNzfw$p$>;!p{-`jW^<Pp-Ps8S~N;TR;ULZ;fv$Kd(01_QQ@#$w2IaTHZ_^w
z?ea8DM9SR#3G0meeJ;KR{5x|klXZ?W?)CdJCd6(6cQKE%z2gyQZrOI$NI8zNBxluR
znN+i{+gl|pDe{lt(<+utg#+jUEO~#06!X7>sks=~zLvnYiT5m|ZLZMoLNI5EVC8w1
zB_yn}Y;RTT)z&;ur!<dc9ZbQ`N;xZQ>_?~<+f0reOrWaPc|6F5%w2b^Gh?cIejhH|
zgZyM?V*|_a#ejOEv|3qH$+R;O^bpW?&Psd1mz;cU-pcE!Ox~NlN)3@)TZ!_F{EUmh
z4;RYP;yqMl)DaDy`C%AzTgiAZjM0|V=^a1iTS=IR9ltC48!feA$;Y-Uj#Y6griEAT
zwQ`_M(X&GP(mH!;G9Cpu2!~wj_Q`>T_4(mw=r<B79`kmqpTBHlWxZtF33(eO?J>8H
z#dDboPltRVj|Mc9hzOz}gZJ5BB)y>+I(NAfLEH;)@NjMrG{b&%#u<qdw(GTPdo`~q
z&6-}TQLSSqr&HNfF`O;Ll!|Y31Hioq=)~Ew95N&7d0CsBit5xI24PQdI*YRtp&E4N
z%_eV@iz|CG?c#1xK~?x!hsyyn`iqNdv)06IW&Bq72udJ1PqjWz^ER5NapKQ|^m)+u
zWishC^5dGPG2%7VJjq(8>G?6OCwd;*R#O@$Udv#w_WU_L{iSF8pFg2E>RE;-(`D?m
z%?ahBo@Mx}*v|ajz!duK-BOffeEy6(z+C{JLrF=#4?F-K0*@3W`7!VWcnUmIl;r2Y
V3*aU2N>P$u18;!0z&k}r{s%56wT%D(

literal 0
HcmV?d00001

diff --git a/test/unit/utglTF2ImportExport.cpp b/test/unit/utglTF2ImportExport.cpp
index 4ba7c69d4..f0f18d503 100644
--- a/test/unit/utglTF2ImportExport.cpp
+++ b/test/unit/utglTF2ImportExport.cpp
@@ -446,6 +446,21 @@ TEST_F(utglTF2ImportExport, export_bad_accessor_bounds) {
     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) {