Refactored the way used and required extensions are calculated. Now extension textures do not generate fallback references

Author: vpenades

src/SharpGLTF.Core/Schema2/Serialization.WriteSettings.cs

@@ -223,15 +223,13 @@ public void SaveGLTF(string filePath, WriteSettings settings = null)
             context.WriteTextSchema2(name, this);
         }
 
-        [Obsolete("Use GetJsonPreview", true)]
-        public string GetJSON(bool indented) { return GetJsonPreview(); }
-
         /// <summary>
         /// Gets the JSON document of this <see cref="MODEL"/>.
         /// </summary>
         /// <returns>A JSON content.</returns>
         /// <remarks>
-        /// ⚠ Beware: this method serializes the current model into a json, without taking care of the binary buffers,
+        /// ⚠ Beware: the generated json is intended to be used for debugging purposes only;
+        /// this method serializes the current model into a json, without taking care of the binary buffers and images,<br/>
         /// so the produced json might not be usable!
         /// </remarks>
         public string GetJsonPreview()

src/SharpGLTF.Core/Schema2/gltf.ExtensionsFactory.cs

@@ -259,75 +259,96 @@ partial class ModelRoot
 
         internal void UpdateExtensionsSupport()
         {
-            var used = GatherUsedExtensions();
+            var used = GatherUsedAndRequiredExtensions();
 
             // update the used list
             this._extensionsUsed.Clear();
-            this._extensionsUsed.AddRange(used);
+            this._extensionsUsed.AddRange(used.Select(item => item.ext));
 
-            _SetExtensionUsage("KHR_mesh_quantization", this._extensionsUsed.Contains("KHR_mesh_quantization"), true);
+            this._extensionsRequired.Clear();
+            this._extensionsRequired.AddRange(used.Where(item => item.isRequired == true).Select(item => item.ext));
         }
 
-        internal IEnumerable<string> GatherUsedExtensions()
+        
+
+        internal IEnumerable<(string ext, bool isRequired)> GatherUsedAndRequiredExtensions()
         {
             // retrieve ALL the property based objects of the whole model.
             var allObjects = new[] { this }
                 .Concat(GetLogicalChildrenFlattened())
                 .ToList();
 
             // check all the extensions used by each object
-            var used = new HashSet<string>();
-
-            // search for known extensions
-            foreach (var c in allObjects)
-            {
-                var ids = c.Extensions
-                    .Select(item => ExtensionsFactory.Identify(c.GetType(), item.GetType()))
-                    .Where(item => !string.IsNullOrWhiteSpace(item));
-
-                used.UnionWith(ids);
-            }
+            var exts = new Dictionary<string, bool>();
 
             // search for unknown extensions
             foreach (var unk in allObjects.SelectMany(item => item.Extensions).OfType<UnknownNode>())
             {
-                used.Add(unk.Name);
+                exts[unk.Name] = false;
             }
 
-            // search for special cases
+            // search for known extensions
+            foreach (var c in allObjects)
+            {
+                foreach(var ext in c.Extensions)
+                {
+                    var id = ExtensionsFactory.Identify(c.GetType(), ext.GetType());
+                    if (string.IsNullOrWhiteSpace(id)) continue;
 
-            var isQuantized = MeshPrimitive.CheckAttributesQuantizationRequired(this);
-            if (isQuantized) used.Add("KHR_mesh_quantization");
+                    bool isRequired = false;
 
-            return used;
-        }
+                    if (ext is IExtensionTypeInfo extInfo)
+                    {
+                        isRequired = extInfo.CheckIsRequiredExtension(c);
+                    }
 
-        private void _SetExtensionUsage(string extension, bool used, bool required)
-        {
-            if (!used)
-            {
-                this._extensionsUsed.Remove(extension);
-                this._extensionsRequired.Remove(extension);
-                return;
-            }
+                    if (exts.TryGetValue(id, out var stored)) { isRequired |= stored; }
 
-            if (!this._extensionsUsed.Contains(extension)) this._extensionsUsed.Add(extension);
-            if (required && !this._extensionsRequired.Contains(extension)) this._extensionsRequired.Add(extension);
-        }
+                    exts[id] = isRequired;
+                }
+            }            
 
+            // search for special cases
+            var isQuantized = MeshPrimitive.CheckAttributesQuantizationRequired(this);
+            if (isQuantized) exts["KHR_mesh_quantization"] = true;
+
+            return exts.Select(kvp => (kvp.Key, kvp.Value));
+        }
+        
         internal void _ValidateExtensions(Validation.ValidationContext validate)
         {
             foreach (var iex in this.IncompatibleExtensions)
             {
                 validate._LinkThrow("Extensions", iex);
             }
 
-            foreach (var ext in GatherUsedExtensions())
+            foreach (var ext in GatherUsedAndRequiredExtensions().Select(item => item.ext))
             {
                 if (!this._extensionsUsed.Contains(ext)) validate._LinkThrow("Extensions", ext);
             }
+
+            foreach (var ext in GatherUsedAndRequiredExtensions().Where(item => item.isRequired).Select(item => item.ext))
+            {
+                if (!this._extensionsRequired.Contains(ext)) validate._LinkThrow("Extensions", ext);
+            }
         }
 
         #endregion
     }
+
+    /// <summary>
+    /// Implemented by extensions
+    /// </summary>
+    public interface IExtensionTypeInfo
+    {
+        /// <summary>
+        /// Checks whether the extension instance implementing this interface should be tagged as required.
+        /// </summary>
+        /// <param name="extensionOwner">the owner of the extension</param>
+        /// <returns>true if the extension should be tagged as required</returns>
+        /// <remarks>
+        /// This is called just before saving the model.
+        /// </remarks>
+        public bool CheckIsRequiredExtension(ExtraProperties extensionOwner);
+    }
 }

src/SharpGLTF.Core/Schema2/gltf.Material.cs

@@ -137,14 +137,41 @@ public float Dispersion
         /// <returns>A <see cref="MaterialChannel"/> structure. or null if it does not exist</returns>
         [System.Diagnostics.DebuggerStepThrough]
         public MaterialChannel? FindChannel(string channelKey)
+        {
+            return TryGetChannel(channelKey, out var channel) ? channel : null;
+        }
+
+        /// <summary>
+        /// Finds an instance of <see cref="MaterialChannel"/>
+        /// </summary>
+        /// <param name="channelKey">
+        /// The channel key. Currently, these values are used:
+        /// - "Normal"
+        /// - "Occlusion"
+        /// - "Emissive"
+        /// - When material is <see cref="MaterialPBRMetallicRoughness"/>:
+        ///   - "BaseColor"
+        ///   - "MetallicRoughness"
+        /// - When material is <see cref="MaterialPBRSpecularGlossiness"/>:
+        ///   - "Diffuse"
+        ///   - "SpecularGlossiness"
+        /// </param>
+        /// <returns>true if found</returns>
+        [System.Diagnostics.DebuggerStepThrough]
+        public bool TryGetChannel(string channelKey, out MaterialChannel channel)
         {
             foreach (var ch in Channels)
             {
-                if (ch.Key.Equals(channelKey, StringComparison.OrdinalIgnoreCase)) return ch;
+                if (ch.Key.Equals(channelKey, StringComparison.OrdinalIgnoreCase))
+                {
+                    channel = ch;
+                    return true;
+                }
             }
 
-            return null;
-        }        
+            channel = default;
+            return false;
+        }
 
         #endregion
 

src/SharpGLTF.Core/Schema2/gltf.Textures.cs

@@ -67,34 +67,41 @@ public void SetImage(Image primaryImage)
             Guard.NotNull(primaryImage, nameof(primaryImage));
             Guard.MustShareLogicalParent(this, primaryImage, nameof(primaryImage));
 
+            ClearImages();
+
             if (primaryImage.Content.IsExtendedFormat)
-            {
-                var fallback = LogicalParent.UseImage(Memory.MemoryImage.DefaultPngImage);
-                SetImages(primaryImage, fallback);
+            {                
+                _SetExtendedImage(primaryImage);
             }
             else
-            {
-                ClearImages();
+            {                
                 _source = primaryImage.LogicalIndex;
-            }
+            }            
         }
 
         public void SetImages(Image primaryImage, Image fallbackImage)
         {
-            Guard.NotNull(primaryImage, nameof(primaryImage));
-            Guard.NotNull(fallbackImage, nameof(fallbackImage));
-            Guard.MustShareLogicalParent(this, primaryImage, nameof(primaryImage));
-            Guard.MustShareLogicalParent(this, fallbackImage, nameof(fallbackImage));
-            Guard.IsTrue(primaryImage.Content.IsExtendedFormat, "Primary image must be DDS, WEBP or KTX2");
+            Guard.NotNull(fallbackImage, nameof(fallbackImage));            
+            Guard.MustShareLogicalParent(this, fallbackImage, nameof(fallbackImage));            
             Guard.IsTrue(fallbackImage.Content.IsJpg || fallbackImage.Content.IsPng, nameof(fallbackImage), "Fallback image must be PNG or JPEG");
 
-            if (primaryImage.Content.IsDds) { _UseDDSTexture().Image = primaryImage; }
-            if (primaryImage.Content.IsWebp) { _UseWEBPTexture().Image = primaryImage; }
-            if (primaryImage.Content.IsKtx2) { _UseKTX2Texture().Image = primaryImage; }
-
+            _SetExtendedImage(primaryImage);
             _source = fallbackImage.LogicalIndex;
         }
 
+        private void _SetExtendedImage(Image extendedImage)
+        {
+            Guard.NotNull(extendedImage, nameof(extendedImage));
+            Guard.MustShareLogicalParent(this, extendedImage, nameof(extendedImage));
+            Guard.IsTrue(extendedImage.Content.IsExtendedFormat, "Primary image must be DDS, WEBP or KTX2");
+
+            if (extendedImage.Content.IsDds) { _UseDDSTexture().Image = extendedImage; return; }
+            if (extendedImage.Content.IsWebp) { _UseWEBPTexture().Image = extendedImage; return; }
+            if (extendedImage.Content.IsKtx2) { _UseKTX2Texture().Image = extendedImage; return; }
+
+            throw new NotImplementedException("Unknown image format");
+        }
+
         public void ClearImages()
         {
             _source = null;
@@ -149,7 +156,7 @@ protected override void OnValidateReferences(ValidationContext validate)
         #endregion
     }
 
-    partial class TextureDDS
+    partial class TextureDDS : IExtensionTypeInfo
     {
         internal TextureDDS(Texture parent)
         {
@@ -172,9 +179,16 @@ public Image Image
                 _source = value?.LogicalIndex;
             }
         }
+
+        public bool CheckIsRequiredExtension(ExtraProperties extensionOwner)
+        {
+            if (extensionOwner is not Texture tex) return false;
+
+            return tex.FallbackImage == null;
+        }
     }
 
-    partial class TextureWEBP
+    partial class TextureWEBP : IExtensionTypeInfo
     {
         internal TextureWEBP(Texture parent)
         {
@@ -197,9 +211,16 @@ public Image Image
                 _source = value?.LogicalIndex;
             }
         }
+
+        public bool CheckIsRequiredExtension(ExtraProperties extensionOwner)
+        {
+            if (extensionOwner is not Texture tex) return false;
+
+            return tex.FallbackImage == null;
+        }
     }
 
-    partial class TextureKTX2
+    partial class TextureKTX2 : IExtensionTypeInfo
     {
         internal TextureKTX2(Texture parent)
         {
@@ -222,6 +243,13 @@ public Image Image
                 _source = value?.LogicalIndex;
             }
         }
+
+        public bool CheckIsRequiredExtension(ExtraProperties extensionOwner)
+        {
+            if (extensionOwner is not Texture tex) return false;
+
+            return tex.FallbackImage == null;
+        }
     }
 
     [System.Diagnostics.DebuggerDisplay("TextureSampler[{LogicalIndex}] {Name}")]
@@ -410,6 +438,9 @@ public Texture UseTexture(Image primary, Image fallback, TextureSampler sampler
 
             tex.Sampler = sampler;
 
+            System.Diagnostics.Debug.Assert((primary != null) == (tex.PrimaryImage != null), "primary image incorrectly set");
+            System.Diagnostics.Debug.Assert((fallback != null) == (tex.FallbackImage != null), "fallback image incorrectly set");
+
             return tex;
         }
     }

src/SharpGLTF.Toolkit/Schema2/MaterialExtensions.cs

@@ -505,10 +505,9 @@ public static void CopyChannelsTo(this MaterialBuilder srcMaterial, Material dst
                 var src = srcMaterial.GetChannel(k);
                 if (src == null) continue;
 
-                var dst = dstMaterial.FindChannel(k.ToString());
-                if (dst == null) continue;
+                if (!dstMaterial.TryGetChannel(k.ToString(), out var dst)) continue;
 
-                src.CopyTo(dst.Value);
+                src.CopyTo(dst);
             }
         }
 
@@ -549,7 +548,7 @@ public static void CopyTo(this ChannelBuilder srcChannel, MaterialChannel dstCha
             if (srcXform != null)
             {
                 dstChannel.SetTransform(srcXform.Offset, srcXform.Scale, srcXform.Rotation, srcXform.CoordinateSetOverride);
-            }
+            }            
         }
 
         private static Image _ConvertToImage(MaterialChannel dstChannel, ImageBuilder srcImage)

tests/SharpGLTF.Core.Tests/Schema2/LoadAndSave/LoadSampleTests.cs

@@ -58,7 +58,7 @@ private static ModelRoot _LoadModel(string f, bool tryFix = false)
                 // check extensions used
                 if (unsupportedExtensions.All(uex => !model.ExtensionsUsed.Contains(uex)))
                 {
-                    var detectedExtensions = model.GatherUsedExtensions().ToArray();
+                    var detectedExtensions = model.GatherUsedAndRequiredExtensions().Select(item => item.ext).ToArray();
                     Assert.That(detectedExtensions, Is.EquivalentTo(model.ExtensionsUsed));
                 }
             }

tests/SharpGLTF.Core.Tests/Schema2/LoadAndSave/LoadSpecialModelsTest.cs

@@ -114,7 +114,7 @@ public void LoadUniVRM()
             var model = ModelRoot.Load(path);
             Assert.That(model, Is.Not.Null);
 
-            var flattenExtensions = model.GatherUsedExtensions().ToArray();
+            var flattenExtensions = model.GatherUsedAndRequiredExtensions().ToArray();
 
             model.AttachToCurrentTest("AliceModel.glb");
         }

tests/SharpGLTF.NUnit/TestFiles.cs

@@ -205,10 +205,11 @@ public static IReadOnlyList<string> GetKhronosValidationPaths()
         public static IReadOnlyList<string> GetBabylonJSModelsPaths()
         {
             var skipAlways = new string[]
-            {
+            {                
                 "ClearCoatTest.gltf", // validator reports errors.
                 "ClearCoatTest.glb", // validator reports errors.
                 "\\Elf\\Elf.gltf", // validator reports invalid inverse bind matrices.
+                "\\meshes\\Bee.glb", // validator reports errors.
                 "\\meshes\\Tests\\AssetGenerator", // already covered separately.
                 "\\meshes\\KHR_materials_volume_testing.glb", // draco compression-
                 "\\meshes\\Yeti\\MayaExport\\", // validator reports out of bounds accesor