improving lights API

improving tests

src/SharpGLTF/Schema2/gltf.Asset.cs

@@ -28,7 +28,7 @@ namespace SharpGLTF.Schema2
 
         private static readonly Version ZEROVERSION = new Version(0, 0);
         private static readonly Version MINVERSION = new Version(2, 0);
-        private static readonly Version MAXVERSION = new Version(2, 0);
+        private static readonly Version MAXVERSION = new Version(2, 1);
 
         public string Copyright { get => _copyright; set => _copyright = value.AsEmptyNullable(); }
         public string Generator { get => _generator; set => _generator = value.AsEmptyNullable(); }

src/SharpGLTF/Schema2/khr.lights.cs

@@ -2,11 +2,11 @@
 using System.Collections.Generic;
 using System.Text;
 using System.Numerics;
+using System.Linq;
 
 namespace SharpGLTF.Schema2
 {
     using Collections;
-    using System.Linq;
 
     partial class KHR_lights_punctualglTFextension
     {
@@ -23,9 +23,9 @@ namespace SharpGLTF.Schema2
 
         public IReadOnlyList<PunctualLight> Lights => _lights;
 
-        public PunctualLight CreateLight(string name = null)
+        public PunctualLight CreateLight(string name, PunctualLightType ltype)
         {
-            var light = new PunctualLight();
+            var light = new PunctualLight(ltype);
             light.Name = name;
 
             _lights.Add(light);
@@ -34,6 +34,9 @@ namespace SharpGLTF.Schema2
         }
     }
 
+    /// <summary>
+    /// Defines all the types of <see cref="PunctualLight"/> types.
+    /// </summary>
     public enum PunctualLightType { Directional, Point, Spot }
 
     /// <remarks>
@@ -42,51 +45,128 @@ namespace SharpGLTF.Schema2
     [System.Diagnostics.DebuggerDisplay("{LightType} {Color} {Intensity} {Range}")]
     public partial class PunctualLight
     {
-        /// <summary>
-        /// Gets the zero-based index of this <see cref="PunctualLight"/> at <see cref="ModelRoot.LogicalPunctualLights"/>
-        /// </summary>
-        public int LogicalIndex => this.LogicalParent.LogicalPunctualLights.IndexOfReference(this);
+        #region lifecycle
 
-        public Vector3 Color
+        internal PunctualLight() { }
+
+        internal PunctualLight(PunctualLightType ltype)
         {
-            get => _color.AsValue(_colorDefault);
-            set => _color = value.AsNullable(_colorDefault, Vector3.Zero, Vector3.One);
+            _type = ltype.ToString().ToLower();
+
+            if (ltype == PunctualLightType.Spot) _spot = new PunctualLightSpot();
         }
 
-        public Double Intensity
+        /// <summary>
+        /// Sets the cone angles for the <see cref="PunctualLightType.Spot" light/>.
+        /// </summary>
+        /// <param name="innerConeAngle">
+        /// Gets the Angle, in radians, from centre of spotlight where falloff begins.
+        /// Must be greater than or equal to 0 and less than outerConeAngle.
+        /// </param>
+        /// <param name="outerConeAngle">
+        /// Gets Angle, in radians, from centre of spotlight where falloff ends.
+        /// Must be greater than innerConeAngle and less than or equal to PI / 2.0.
+        /// </param>
+        /// <returns>This <see cref="PunctualLight"/> instance.</returns>
+        public PunctualLight WithSpotCone(float innerConeAngle, float outerConeAngle)
         {
-            get => _intensity.AsValue(_intensityDefault);
-            set => _intensity = value.AsNullable(_intensityDefault, _intensityMinimum, float.MaxValue);
+            if (_spot == null) throw new InvalidOperationException($"Expected {PunctualLightType.Spot} but found {LightType}");
+
+            if (innerConeAngle > outerConeAngle) throw new ArgumentException($"{nameof(innerConeAngle)} must be equal or smaller than {nameof(outerConeAngle)}");
+
+            _spot.InnerConeAngle = innerConeAngle;
+            _spot.OuterConeAngle = outerConeAngle;
+
+            return this;
         }
 
-        public Double Range
+        /// <summary>
+        /// Defines the light color, intensity and range for the current <see cref="PunctualLight"/>.
+        /// </summary>
+        /// <param name="color">RGB value for light's color in linear space.</param>
+        /// <param name="intensity">
+        /// Brightness of light in. The units that this is defined in depend on the type of light.
+        /// point and spot lights use luminous intensity in candela (lm/sr) while directional
+        /// lights use illuminance in lux (lm/m2)
+        /// </param>
+        /// <param name="range">
+        /// Hint defining a distance cutoff at which the light's intensity may be considered
+        /// to have reached zero. Supported only for point and spot lights. Must be > 0.
+        /// When undefined, range is assumed to be infinite.
+        /// </param>
+        /// <returns>This <see cref="PunctualLight"/> instance.</returns>
+        public PunctualLight WithColor(Vector3 color, float intensity = 1, float range = 0)
         {
-            get => _range.AsValue(0);
-            set => _range = value.AsNullable(0, _rangeMinimum, float.MaxValue);
+            this.Color = color;
+            this.Intensity = intensity;
+            this.Range = range;
+            return this;
         }
 
-        public PunctualLightType LightType
+        #endregion
+
+        #region properties
+
+        /// <summary>
+        /// For light types that have a direction (directional and spot lights),
+        /// the light's direction is defined as the 3-vector (0.0, 0.0, -1.0)
+        /// </summary>
+        public static Vector3 LocalDirection => -Vector3.UnitZ;
+
+        /// <summary>
+        /// Gets the zero-based index of this <see cref="PunctualLight"/> at <see cref="ModelRoot.LogicalPunctualLights"/>
+        /// </summary>
+        public int LogicalIndex => this.LogicalParent.LogicalPunctualLights.IndexOfReference(this);
+
+        /// <summary>
+        /// Gets the type of light.
+        /// </summary>
+        public PunctualLightType LightType => (PunctualLightType)Enum.Parse(typeof(PunctualLightType), _type, true);
+
+        /// <summary>
+        /// Gets the Angle, in radians, from centre of spotlight where falloff begins.
+        /// Must be greater than or equal to 0 and less than outerConeAngle.
+        /// </summary>
+        public float InnerConeAngle => this._spot == null ? 0 : (float)this._spot.InnerConeAngle;
+
+        /// <summary>
+        /// Gets Angle, in radians, from centre of spotlight where falloff ends.
+        /// Must be greater than innerConeAngle and less than or equal to PI / 2.0.
+        /// </summary>
+        public float OuterConeAngle => this._spot == null ? 0 : (float)this._spot.OuterConeAngle;
+
+        /// <summary>
+        /// Gets or sets the RGB value for light's color in linear space.
+        /// </summary>
+        public Vector3 Color
         {
-            get => (PunctualLightType)Enum.Parse(typeof(PunctualLightType), _type, true);
-            set
-            {
-                this._type = value.ToString().ToLower();
-                if (value != PunctualLightType.Spot) this._spot = null;
-                else if (this._spot == null) this._spot = new PunctualLightSpot();
-            }
+            get => _color.AsValue(_colorDefault);
+            set => _color = value.AsNullable(_colorDefault, Vector3.Zero, Vector3.One);
         }
 
-        public float InnerConeAngle
+        /// <summary>
+        /// Gets or sets the Brightness of light in. The units that this is defined in depend on the type of light.
+        /// point and spot lights use luminous intensity in candela (lm/sr) while directional
+        /// lights use illuminance in lux (lm/m2)
+        /// </summary>
+        public float Intensity
         {
-            get => this._spot == null ? 0 : (float)this._spot.InnerConeAngle;
-            set { if (this._spot != null) this._spot.InnerConeAngle = value; }
+            get => (float)_intensity.AsValue(_intensityDefault);
+            set => _intensity = ((double)value).AsNullable(_intensityDefault, _intensityMinimum, float.MaxValue);
         }
 
-        public float OuterConeAngle
+        /// <summary>
+        /// Gets or sets a Hint defining a distance cutoff at which the light's intensity may be considered
+        /// to have reached zero. Supported only for point and spot lights. Must be > 0.
+        /// When undefined, range is assumed to be infinite.
+        /// </summary>
+        public float Range
         {
-            get => this._spot == null ? 0 : (float)this._spot.OuterConeAngle;
-            set { if (this._spot != null) this._spot.OuterConeAngle = value; }
+            get => (float)_range.AsValue(0);
+            set => _range = LightType == PunctualLightType.Directional ? 0 : ((double)value).AsNullable(0, _rangeMinimum, float.MaxValue);
         }
+
+        #endregion
     }
 
     partial class PunctualLightSpot
@@ -123,13 +203,25 @@ namespace SharpGLTF.Schema2
             }
         }
 
+        /// <summary>
+        /// Creates a new <see cref="PunctualLight"/> instance.
+        /// and adds it to <see cref="ModelRoot.LogicalPunctualLights"/>.
+        /// </summary>
+        /// <param name="lightType">A value of <see cref="PunctualLightType"/> describing the type of light to create.</param>
+        /// <returns>A <see cref="PunctualLight"/> instance.</returns>
+        public PunctualLight CreatePunctualLight(PunctualLightType lightType)
+        {
+            return CreatePunctualLight(null, lightType);
+        }
+
         /// <summary>
         /// Creates a new <see cref="PunctualLight"/> instance.
         /// and adds it to <see cref="ModelRoot.LogicalPunctualLights"/>.
         /// </summary>
         /// <param name="name">The name of the instance.</param>
+        /// <param name="lightType">A value of <see cref="PunctualLightType"/> describing the type of light to create.</param>
         /// <returns>A <see cref="PunctualLight"/> instance.</returns>
-        public PunctualLight CreatePunctualLight(string name = null)
+        public PunctualLight CreatePunctualLight(string name, PunctualLightType lightType)
         {
             var ext = this.GetExtension<KHR_lights_punctualglTFextension>();
             if (ext == null)
@@ -140,7 +232,7 @@ namespace SharpGLTF.Schema2
                 this.SetExtension(ext);
             }
 
-            return ext.CreateLight(name);
+            return ext.CreateLight(name, lightType);
         }
     }
 

src/SharpGLTF/_Extensions.cs

@@ -221,14 +221,28 @@ namespace SharpGLTF
         internal static T? AsNullable<T>(this T value, T defval, T minval, T maxval)
             where T : struct, IEquatable<T>, IComparable<T>
         {
+            if (value.Equals(defval)) return null;
+
             if (value.CompareTo(minval) < 0) value = minval;
             if (value.CompareTo(maxval) > 0) value = maxval;
 
             return value.Equals(defval) ? (T?)null : value;
         }
 
+        internal static Vector2? AsNullable(this Vector2 value, Vector2 defval, Vector2 minval, Vector2 maxval)
+        {
+            if (value.Equals(defval)) return null;
+
+            value = Vector2.Min(value, minval);
+            value = Vector2.Max(value, maxval);
+
+            return value.Equals(defval) ? (Vector2?)null : value;
+        }
+
         internal static Vector3? AsNullable(this Vector3 value, Vector3 defval, Vector3 minval, Vector3 maxval)
         {
+            if (value.Equals(defval)) return null;
+
             value = Vector3.Min(value, minval);
             value = Vector3.Max(value, maxval);
 
@@ -237,6 +251,8 @@ namespace SharpGLTF
 
         internal static Vector4? AsNullable(this Vector4 value, Vector4 defval, Vector4 minval, Vector4 maxval)
         {
+            if (value.Equals(defval)) return (Vector4?)null;
+
             value = Vector4.Min(value, minval);
             value = Vector4.Max(value, maxval);
 

tests/SharpGLTF.Tests/TestAssemblyAPI.cs → tests/SharpGLTF.Tests/AssemblyAPITests.cs

@@ -8,7 +8,7 @@ using NUnit.Framework;
 namespace SharpGLTF
 {
     [TestFixture]
-    public class TestAssemblyAPI
+    public class AssemblyAPITests
     {
         [Test]
         public void DumpCurrentAPI()

tests/SharpGLTF.Tests/ExtensionsTests.cs

@@ -0,0 +1,34 @@
+using System;
+using System.Collections.Generic;
+using System.Text;
+
+using NUnit.Framework;
+
+namespace SharpGLTF
+{
+    [TestFixture]
+    public class ExtensionsTests
+    {
+        [Test]
+        public void TestAsNullableExtensions()
+        {
+            // the AsNullable extensions are a bit tricky;
+            // they should default to null regardless of the value being inside or outside the bounds of min-max
+            // but if after the min.max has affected the value, the default-to-null check still applies.
+
+            Assert.AreEqual(null, 5.AsNullable(5));
+
+            Assert.AreEqual(1,    0.AsNullable(3, 1, 5));
+            Assert.AreEqual(1,    1.AsNullable(3, 1, 5));
+            Assert.AreEqual(2,    2.AsNullable(3, 1, 5));
+            Assert.AreEqual(null, 3.AsNullable(3, 1, 5));
+            Assert.AreEqual(4,    4.AsNullable(3, 1, 5));
+            Assert.AreEqual(5,    5.AsNullable(3, 1, 5));
+            Assert.AreEqual(5,    6.AsNullable(3, 1, 5));
+
+            // special case: default values outside the min-max range should also return null
+            Assert.AreEqual(null,  0.AsNullable( 0, 1, 5));
+            Assert.AreEqual(null, 10.AsNullable(10, 1, 5));
+        }
+    }
+}

tests/SharpGLTF.Tests/GltfUtils.cs

@@ -16,8 +16,6 @@ namespace SharpGLTF
         {
             try
             {
-                TestContext.Progress.WriteLine($"Loading {filePath.ToShortDisplayPath()}");
-
                 return ModelRoot.Load(filePath);
             }
             catch (IO.UnsupportedExtensionException eex)

tests/SharpGLTF.Tests/Schema2/Authoring/CreateModelTests.cs

@@ -76,12 +76,19 @@ namespace SharpGLTF.Schema2.Authoring
             TestContext.CurrentContext.AttachGltfValidatorLink();
 
             var root = ModelRoot.CreateModel();
-            var scene = root.UseScene("Empty Scene");            
-            var node = scene.CreateNode();
-            node.PunctualLight = root.CreatePunctualLight("Directional Light");
-            node.PunctualLight.LightType = PunctualLightType.Directional;
+            var scene = root.UseScene("Empty Scene");
+
+            scene.CreateNode()
+                .PunctualLight = root.CreatePunctualLight(PunctualLightType.Directional)
+                .WithColor(Vector3.UnitX, 2);
+
+            var node2 = scene.CreateNode()
+                .PunctualLight = root.CreatePunctualLight(PunctualLightType.Spot)
+                .WithColor(Vector3.UnitY, 3, 10)
+                .WithSpotCone(0.2f, 0.3f);
 
             root.AttachToCurrentTest("sceneWithLight.gltf");
+            root.AttachToCurrentTest("sceneWithLight.glb");
         }
 
         [Test(Description ="Creates a model with a triangle mesh")]

tests/SharpGLTF.Tests/Schema2/LoadAndSave/LoadModelTests.cs

@@ -27,48 +27,73 @@ namespace SharpGLTF.Schema2.LoadAndSave
         {
             TestContext.CurrentContext.AttachShowDirLink();
 
-            foreach (var f in TestFiles.GetGeneratedFilePaths())
+            var files = TestFiles.GetGeneratedFilePaths();
+
+            foreach (var f in files)
             {
-                var model = GltfUtils.LoadModel(f);
+                var errors = _LoadNumErrorsForModel(f);
 
-                Assert.NotNull(model);
+                if (errors > 0) continue;
 
-                model.AttachToCurrentTest(System.IO.Path.ChangeExtension(System.IO.Path.GetFileName(f), ".obj"));
+                try
+                {
+                    var model = ModelRoot.Load(f);
+                    model.AttachToCurrentTest(System.IO.Path.ChangeExtension(System.IO.Path.GetFileName(f), ".obj"));
+                }
+                catch (IO.UnsupportedExtensionException eex)
+                {
+                    TestContext.WriteLine($"{f.ToShortDisplayPath()} ERROR: {eex.Message}");
+                }
             }
         }
-        
-        [TestCase(0)]        
-        [TestCase(6)]
-        public void TestLoadCompatibleModels(int idx)
+
+        // right now this test does not make much sense...
+        // [Test]
+        public void TestLoadInvalidModels()
         {
-            var filePath = TestFiles.GetCompatibilityFilePath(idx);
+            TestContext.CurrentContext.AttachShowDirLink();
 
-            var model = GltfUtils.LoadModel(filePath);
+            var files = TestFiles.GetGeneratedFilePaths();
 
-            Assert.NotNull(model);
-        }
-
-        [TestCase(1)]
-        [TestCase(2)]
-        [TestCase(3)]
-        [TestCase(4)]
-        [TestCase(5)]
-        public void TestLoadInvalidModels(int idx)
-        {
-            var filePath = TestFiles.GetCompatibilityFilePath(idx);
+            bool passed = true;
 
-            try
-            {                
-                ModelRoot.Load(filePath);
-                Assert.Fail("Did not throw!");
-            }
-            catch(IO.ModelException ex)
+            foreach (var f in files)
             {
-                TestContext.WriteLine($"{filePath} threw {ex.Message}");
+                var errors = _LoadNumErrorsForModel(f);
+
+                if (errors == 0) continue;
+                
+                try
+                {
+                    var model = GltfUtils.LoadModel(f);
+                    passed = false;
+
+                    TestContext.WriteLine($"FAILED {f.ToShortDisplayPath()}");
+                }
+                catch (Exception ex)
+                {
+                    TestContext.WriteLine($"PASSED {f.ToShortDisplayPath()}");
+                }                
             }
-                       
+
+            Assert.IsTrue(passed);
         }
 
+        private static int _LoadNumErrorsForModel(string gltfPath)
+        {
+            var dir = System.IO.Path.GetDirectoryName(gltfPath);
+            var fn = System.IO.Path.GetFileNameWithoutExtension(gltfPath);
+
+            var jsonPath = System.IO.Path.Combine(dir, "ValidatorResults", System.IO.Path.ChangeExtension(fn, "json"));
+
+            var content = System.IO.File.ReadAllText(jsonPath);
+            var doc = Newtonsoft.Json.Linq.JObject.Parse(content);
+
+            var token = doc.SelectToken("issues").SelectToken("numErrors");
+
+            return (int)token;
+        }
+        
         #endregion
 
         #region testing models of https://github.com/KhronosGroup/glTF-Sample-Models.git

tests/SharpGLTF.Tests/TestFiles.cs

@@ -57,17 +57,17 @@ namespace SharpGLTF
 
         #region API
 
-        public static string[] GetSchemaFilePaths()
+        public static IReadOnlyList<string> GetSchemaFilePaths()
         {
             var dir = System.IO.Path.Combine(_SchemaDir, "extensions", "2.0");
 
             var gltf = System.IO.Directory.GetFiles(dir, "*.gltf", System.IO.SearchOption.AllDirectories);
             var glbb = System.IO.Directory.GetFiles(dir, "*.glb", System.IO.SearchOption.AllDirectories);
 
-            return gltf.Concat(glbb).ToArray();            
+            return gltf.Concat(glbb).ToList();            
         }
 
-        public static string[] GetSampleFilePaths()
+        public static IReadOnlyList<string> GetSampleFilePaths()
         {
             var dir = System.IO.Path.Combine(_SampleModelsDir, "2.0");
 
@@ -88,27 +88,20 @@ namespace SharpGLTF
             return files
                 .OrderBy(item => item)
                 .Where(item => !item.Contains("\\glTF-Draco\\"))
-                .ToArray();
+                .ToList();
         }
 
-        public static string[] GetGeneratedFilePaths()
+        public static IReadOnlyList<string> GetGeneratedFilePaths()
         {
             var dir = System.IO.Path.Combine(_GeneratedAssetsDir, "Output");
 
             var gltf = System.IO.Directory
-                .GetFiles(dir, "*.gltf", System.IO.SearchOption.AllDirectories)
-                .Where(item => !item.Contains("Output\\Compatibility\\Compatibility"));
+                .GetFiles(dir, "*.gltf", System.IO.SearchOption.AllDirectories);
 
             var glbb = System.IO.Directory
-                .GetFiles(dir, "*.glb", System.IO.SearchOption.AllDirectories)
-                .Where(item => !item.Contains("Output\\Compatibility\\Compatibility"));
+                .GetFiles(dir, "*.glb", System.IO.SearchOption.AllDirectories);                
 
-            return gltf.Concat(glbb).OrderBy(item => item).ToArray();
-        }
-
-        public static string GetCompatibilityFilePath(int idx)
-        {
-            return System.IO.Path.Combine(_GeneratedAssetsDir, $"Output\\Compatibility\\Compatibility_0{idx}.gltf");
+            return gltf.Concat(glbb).OrderBy(item => item).ToList();
         }
 
         public static string GetPollyFilePath()