Patch to resolve an issue where a model with empty normals calls MeshHelper.CalculateTangentFrames (MonoGame#8139)

*Note
FIx currently skips geometry with empty normals as the CalculateNormals function is not working (failing tests)

Author: SimonDarksideJ

MonoGame.Framework.Content.Pipeline/Graphics/MeshHelper.cs

@@ -1,8 +1,6 @@
 using System;
 using System.Collections.Generic;
-using System.Collections.ObjectModel;
 using System.Diagnostics;
-using System.Linq;
 
 namespace Microsoft.Xna.Framework.Content.Pipeline.Graphics
 {
@@ -143,8 +141,22 @@ public static void CalculateTangentFrames(MeshContent mesh, string textureCoordi
                 CalculateTangentFrames(geom, textureCoordinateChannelName, tangentChannelName, binormalChannelName);                            
         }
 
+        /// <summary>
+        /// Generate the tangents and binormals (tangent frames) for each vertex in the mesh geometry.
+        /// </summary>
+        /// <param name="geom">The mesh geometry which will have add tangent and binormal channels added.</param>
+        /// <param name="textureCoordinateChannelName">The Vector2 texture coordinate channel used to generate tangent frames.</param>
+        /// <param name="tangentChannelName"></param>
+        /// <param name="binormalChannelName"></param>
         public static void CalculateTangentFrames(GeometryContent geom, string textureCoordinateChannelName, string tangentChannelName, string binormalChannelName)
         {
+            if (!geom.Vertices.Channels.Contains(VertexChannelNames.Normal(0)))
+            {
+                return;
+                // TODO: We could generate the normals here, but it's not working.
+                //MeshHelper.CalculateNormals(geom, true);
+            }
+
             var verts = geom.Vertices;
             var indices = geom.Indices;
             var channels = geom.Vertices.Channels;

Tests/Assets/Models/level1.fbx

@@ -2,13 +2,12 @@
 // This file is subject to the terms and conditions defined in
 // file 'LICENSE.txt', which is part of this source code package.
 
-using System;
 using Microsoft.Xna.Framework;
 using Microsoft.Xna.Framework.Content.Pipeline;
 using Microsoft.Xna.Framework.Content.Pipeline.Graphics;
-using Microsoft.Xna.Framework.Graphics;
-using NUnit.Framework;
 using Microsoft.Xna.Framework.Content.Pipeline.Processors;
+using NUnit.Framework;
+using System;
 
 namespace MonoGame.Tests.ContentPipeline
 {
@@ -281,5 +280,51 @@ public void DefaultEffectTest()
 
             //Assert.IsInstanceOf(typeof(SkinnedMaterialContent), output.Meshes[0].MeshParts[0].Material);
         }
+
+        [Test]
+        /// <summary>
+        /// Test to validate a model with missing normals does not throw an exception using the default ModelProcessor.
+        /// </summary>
+        public void MissingNormalsTestDefault()
+        {
+            string level1fbx = "Assets/Models/level1.fbx";
+            var importer = new FbxImporter();
+            var context = new TestImporterContext("TestObj", "TestBin");
+            var nodeContent = importer.Import(level1fbx, context);
+
+            ModelProcessor processor = new ModelProcessor();
+            var processorContext = new TestProcessorContext(TargetPlatform.Windows, "level1.xnb");
+
+            ModelContent output = null;
+            // Validate that the processor does not throw an exception when normals are missing from the mesh
+            Assert.DoesNotThrow(() => output = processor.Process(nodeContent, processorContext));
+
+            // Test some basics.
+            Assert.NotNull(output);
+            Assert.NotNull(output.Meshes);
+        }
+
+        [Test]
+        /// <summary>
+        /// Test to validate a model with missing normals does not throw an exception using a custom ModelProcessor using MeshHelper.CalculateTangentFrames directly.
+        /// </summary>
+        public void MissingNormalsTestCustom()
+        {
+            string level1fbx = "Assets/Models/level1.fbx";
+            var importer = new FbxImporter();
+            var context = new TestImporterContext("TestObj", "TestBin");
+            var nodeContent = importer.Import(level1fbx, context);
+
+            NormalMappingModelProcessor processor = new NormalMappingModelProcessor();
+            var processorContext = new TestProcessorContext(TargetPlatform.Windows, "level1_costum.xnb");
+
+            ModelContent output = null;
+            // Validate that the custom processor does not throw an exception when normals are missing from the mesh
+            Assert.DoesNotThrow(() => output = processor.Process(nodeContent, processorContext));
+
+            // Test some basics.
+            Assert.NotNull(output);
+            Assert.NotNull(output.Meshes);
+        }
     }
-}
+}

Tools/MonoGame.Tools.Tests/ModelProcessorTests.cs

@@ -0,0 +1,60 @@
+#region File Description
+//-----------------------------------------------------------------------------
+// NormalMappingModelProcessor.cs
+//
+// This file is subject to the terms and conditions defined in file 'LICENSE.txt', which is part of this source code package.
+// MonoGame - Copyright (C) The MonoGame Team
+//-----------------------------------------------------------------------------
+#endregion
+
+#region Using Statements
+using Microsoft.Xna.Framework.Content.Pipeline;
+using Microsoft.Xna.Framework.Content.Pipeline.Graphics;
+using Microsoft.Xna.Framework.Content.Pipeline.Processors;
+using System;
+
+#endregion
+
+namespace MonoGame.Tests.ContentPipeline
+{
+    /// <summary>
+    /// The NormalMappingModelProcessor is used to change the material/effect applied
+    /// to a model. After going through this processor, the output model will be set
+    /// up to be rendered with NormalMapping.fx.
+    /// </summary>
+    [ContentProcessor(DisplayName = "Normal Mapping Validation")]
+    public class NormalMappingModelProcessor : ModelProcessor
+    {
+        public override ModelContent Process(NodeContent input,
+            ContentProcessorContext context)
+        {
+            if (input == null)
+            {
+                throw new ArgumentNullException("input");
+            }
+            context.Logger.LogImportantMessage("processing: " + input.Name);
+            PreprocessSceneHierarchy(input, context, input.Name);
+            return base.Process(input, context);
+        }
+
+
+        /// <summary>
+        /// Recursively calls MeshHelper.CalculateTangentFrames for every MeshContent
+        /// object in the NodeContent scene.
+        /// </summary>
+        /// <param initialFileName="input">A node in the scene.  The function should be called
+        /// with the root of the scene.</param>
+        private void PreprocessSceneHierarchy(NodeContent input,
+            ContentProcessorContext context, string inputName)
+        {
+            MeshContent mesh = input as MeshContent;
+            if (mesh != null)
+            {
+                MeshHelper.CalculateTangentFrames(mesh,
+                    VertexChannelNames.TextureCoordinate(0),
+                    VertexChannelNames.Tangent(0),
+                    VertexChannelNames.Binormal(0));
+            }
+        }
+    }
+}