Handling empty meshes and primitives...

src/SharpGLTF.Toolkit/Geometry/PackedMeshBuilder.cs

@@ -38,6 +38,12 @@ namespace SharpGLTF.Geometry
                 .SelectMany(item => item.Primitives)
                 .Where(item => !item.IsEmpty());
 
+            if (!meshPrimitives.Any())
+            {
+                foreach (var mb in meshBuilders) yield return new PackedMeshBuilder<TMaterial>(mb.Name);
+                yield break;
+            }
+
             var vertexBlocks = VertexTypes.VertexUtils
                 .CreateVertexMemoryAccessors( meshPrimitives.Select(item => item.Vertices) )
                 .ToList();
@@ -46,6 +52,8 @@ namespace SharpGLTF.Geometry
                 .CreateIndexMemoryAccessors( meshPrimitives.Select(item => item.Indices) )
                 .ToList();
 
+            if (vertexBlocks.Count != indexBlocks.Count) throw new InvalidOperationException("Vertex and index blocks count mismatch");
+
             int idx = 0;
 
             foreach (var meshBuilder in meshBuilders)
@@ -108,10 +116,17 @@ namespace SharpGLTF.Geometry
 
         internal PackedPrimitiveBuilder(TMaterial material, int primitiveVertexCount, Memory.MemoryAccessor[] vrtAccessors, Memory.MemoryAccessor idxAccessor)
         {
+            Guard.MustBeBetweenOrEqualTo(primitiveVertexCount, 1, 3, nameof(primitiveVertexCount));
+
+            Guard.NotNull(vrtAccessors, nameof(vrtAccessors));
+
+            if (primitiveVertexCount == 1) Guard.MustBeNull(idxAccessor, nameof(idxAccessor));
+            else                           Guard.NotNull(idxAccessor, nameof(idxAccessor));
+
             _Material = material;
             _VerticesPerPrimitive = primitiveVertexCount;
             _VertexAccessors = vrtAccessors;
-            _IndexAccessors = idxAccessor;
+            _IndexAccessors = idxAccessor; // indices can be null for points
         }
 
         #endregion

src/SharpGLTF.Toolkit/Geometry/VertexTypes/VertexUtils.cs

@@ -210,11 +210,13 @@ namespace SharpGLTF.Geometry.VertexTypes
         public static IEnumerable<MemoryAccessor[]> CreateVertexMemoryAccessors<TVertex>(this IEnumerable<IReadOnlyList<TVertex>> vertexBlocks)
             where TVertex : IVertexBuilder
         {
+            Guard.IsTrue(vertexBlocks.Any(), nameof(vertexBlocks));
+            Guard.IsTrue(vertexBlocks.All(item => item.Count > 0), nameof(vertexBlocks));
+            Guard.IsTrue(vertexBlocks.Select(item => item[0].GetType()).Distinct().Count() == 1, "multiple vertex types found");
+
             // total number of vertices
             var totalCount = vertexBlocks.Sum(item => item.Count);
 
-            if (totalCount == 0) yield break;
-
             // determine the vertex attributes from the first vertex.
             var firstVertex = vertexBlocks
                 .First(item => item.Count > 0)[0];
@@ -257,11 +259,11 @@ namespace SharpGLTF.Geometry.VertexTypes
 
         public static IEnumerable<MemoryAccessor> CreateIndexMemoryAccessors(this IEnumerable<IReadOnlyList<Int32>> indexBlocks)
         {
+            Guard.IsTrue(indexBlocks.Any(), nameof(indexBlocks));
+
             // get attributes
             var totalCount = indexBlocks.Sum(item => item.Count);
 
-            if (totalCount == 0) yield break;
-
             var attribute = new MemoryAccessInfo("INDEX", 0, totalCount, 0, Schema2.DimensionType.SCALAR, Schema2.EncodingType.UNSIGNED_INT);
 
             // create master index buffer
@@ -272,6 +274,12 @@ namespace SharpGLTF.Geometry.VertexTypes
 
             foreach (var block in indexBlocks)
             {
+                if (block.Count == 0)
+                {
+                    yield return null;
+                    continue;
+                }
+
                 var accessor = new MemoryAccessor(ibuffer, attribute.Slice(baseIndicesIndex, block.Count));
 
                 accessor.AsIntegerArray().Fill(block);

src/SharpGLTF.Toolkit/Materials/MaterialBuilder.cs

@@ -49,7 +49,7 @@ namespace SharpGLTF.Materials
 
         public String ShaderStyle { get; set; } = SHADERPBRMETALLICROUGHNESS;
 
-        public static bool AreEqual(MaterialBuilder x, MaterialBuilder y)
+        public static bool AreEqualByContent(MaterialBuilder x, MaterialBuilder y)
         {
             #pragma warning disable IDE0041 // Use 'is null' check
             if (Object.ReferenceEquals(x, y)) return true;
@@ -69,7 +69,7 @@ namespace SharpGLTF.Materials
             if (x.DoubleSided != y.DoubleSided) return false;
             if (x.ShaderStyle != y.ShaderStyle) return false;
 
-            if (!AreEqual(x._CompatibilityFallbackMaterial, y._CompatibilityFallbackMaterial)) return false;
+            if (!AreEqualByContent(x._CompatibilityFallbackMaterial, y._CompatibilityFallbackMaterial)) return false;
 
             // gather all unique channel keys used by both materials.
 
@@ -314,7 +314,7 @@ namespace SharpGLTF.Materials
 
             public bool Equals(MaterialBuilder x, MaterialBuilder y)
             {
-                return MaterialBuilder.AreEqual(x, y);
+                return MaterialBuilder.AreEqualByContent(x, y);
             }
 
             public int GetHashCode(MaterialBuilder obj)

src/SharpGLTF.Toolkit/Schema2/MeshExtensions.cs

@@ -28,6 +28,15 @@ namespace SharpGLTF.Schema2
 
         public static IReadOnlyList<Mesh> CreateMeshes(this ModelRoot root, params IMeshBuilder<Materials.MaterialBuilder>[] meshBuilders)
         {
+            // until this point, even if the multiple material instances used by the meshes have the same content definition,
+            // we must handling material equality by its object reference and nothing else, because Materials.MaterialBuilder
+            // is a mutable object, and we cannot guarantee two material instances will keep having the same content.
+
+            // it is at this point where we can coalesce materials with the same content.
+
+            // TODO: in order to coalesce MaterialBuilder instances with same content
+            // an IMeshBuilder could wrap the incoming mesh, and merge primitives with shared meshes.
+
             // MaterialBuilder instances can be grouped by their content, so we use this dictionary
             // to reduce the number of equal materials. This is specially useful for the default material.
 
@@ -50,7 +59,7 @@ namespace SharpGLTF.Schema2
 
             foreach (var m in meshBuilders) m.Validate();
 
-            // create a new material for every unique material in the mesh builders.
+            // create a new material for every unique (byRef) material in the mesh builders.
             var mapMaterials = meshBuilders
                 .SelectMany(item => item.Primitives)
                 .Where(item => !item.IsEmpty())
@@ -630,7 +639,7 @@ namespace SharpGLTF.Schema2
                 srcMaterial.CopyTo(dstMaterial);
 
                 // if we find an exiting match, we will use it instead.
-                var oldMaterial = materials.Values.FirstOrDefault(item => Materials.MaterialBuilder.AreEqual(dstMaterial, item));
+                var oldMaterial = materials.Values.FirstOrDefault(item => Materials.MaterialBuilder.AreEqualByContent(dstMaterial, item));
                 if (oldMaterial != null) dstMaterial = oldMaterial;
 
                 return materials[srcMaterial] = dstMaterial;

tests/SharpGLTF.Tests/Geometry/MeshBuilderTests.cs

@@ -126,6 +126,11 @@ namespace SharpGLTF.Geometry
         [Test]
         public void CreateEmptyMesh()
         {
+            // empty meshes are valid in the toolkit,
+            // but are invalid in glTF schema, so
+            // special measures need to be taken
+            // for empty meshes.
+
             var mesh1 = VERTEX1.CreateCompatibleMesh();
 
             var model = Schema2.ModelRoot.CreateModel();
@@ -199,5 +204,45 @@ namespace SharpGLTF.Geometry
 
             Assert.AreEqual(1, dstMeshes[0].Primitives.Count);
         }
+
+        [Test]
+        public static void CreateWithMutableSharedMaterial()
+        {
+            var material1 = Materials.MaterialBuilder.CreateDefault();
+            var material2 = Materials.MaterialBuilder.CreateDefault();
+            var material3 = Materials.MaterialBuilder.CreateDefault();            
+
+            Assert.IsTrue(Materials.MaterialBuilder.AreEqualByContent(material1, material2));
+            Assert.IsTrue(Materials.MaterialBuilder.AreEqualByContent(material1, material3));
+
+            Assert.AreNotEqual(material1, material2);
+            Assert.AreNotEqual(material1, material3);
+
+            // MeshBuilder should split primitives by material reference,
+            // because in general, materials will not be immutable.
+            var mesh = new MeshBuilder<VertexPosition>();            
+            mesh.UsePrimitive(material1, 1).AddPoint(default);
+            mesh.UsePrimitive(material2, 1).AddPoint(default);
+            mesh.UsePrimitive(material3, 1).AddPoint(default);
+
+            Assert.AreEqual(3, mesh.Primitives.Count);
+
+            var model = Schema2.ModelRoot.CreateModel();
+
+            // CreateMeshes should identify that, at this point, material1, material2 and material3
+            // represent the same material, and coalesce to a single material.
+            var mesh1 = model.CreateMeshes(mesh)[0];
+            Assert.AreEqual(1, model.LogicalMaterials.Count);
+
+            // since Materials.MaterialBuilder is not immutable we can change the contents,
+            // so now, material1, material2 and material3 no longer represent the same material
+            material1.WithMetallicRoughnessShader().WithChannelParam(Materials.KnownChannels.BaseColor, Vector4.One * 0.2f);
+            material2.WithMetallicRoughnessShader().WithChannelParam(Materials.KnownChannels.BaseColor, Vector4.One * 0.4f);
+            material3.WithMetallicRoughnessShader().WithChannelParam(Materials.KnownChannels.BaseColor, Vector4.One * 0.6f);
+
+            var mesh2 = model.CreateMeshes(mesh)[0];
+            Assert.AreEqual(4, model.LogicalMaterials.Count);
+
+        }
     }
 }