Skip to content

Commit

Permalink
Fixed MaterialBuilder's conversion to glTF not copying the IOR value.
Browse files Browse the repository at this point in the history
Fixes #246
  • Loading branch information
vpenades committed Sep 5, 2024
1 parent 7123ad3 commit 2073cf3
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 4 deletions.
10 changes: 9 additions & 1 deletion src/SharpGLTF.Toolkit/Schema2/MaterialExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ public static void CopyTo(this MaterialBuilder srcMaterial, Material dstMaterial
dstMaterial.Alpha = srcMaterial.AlphaMode.ToSchema2();
dstMaterial.AlphaCutoff = srcMaterial.AlphaCutoff;
dstMaterial.DoubleSided = srcMaterial.DoubleSided;
dstMaterial.IndexOfRefraction = srcMaterial.IndexOfRefraction;


var hasClearCoat
= srcMaterial.GetChannel("ClearCoat") != null
Expand Down Expand Up @@ -442,6 +442,10 @@ var hasVolume
defMaterial = srcMaterial.CompatibilityFallback;
}

// IOR must be set after dst material initialization,
// otherwise it's erased since it's stored in an extension
dstMaterial.IndexOfRefraction = srcMaterial.IndexOfRefraction;

if (defMaterial != null)
{
if (defMaterial.ShaderStyle != "PBRMetallicRoughness") throw new ArgumentException(nameof(srcMaterial.CompatibilityFallback.ShaderStyle));
Expand All @@ -452,6 +456,10 @@ var hasVolume
defMaterial.CopyChannelsTo(dstMaterial, "SpecularColor", "SpecularFactor");
defMaterial.CopyChannelsTo(dstMaterial, "VolumeThickness", "VolumeAttenuation");
}

// final validation

System.Diagnostics.Debug.Assert(dstMaterial.IndexOfRefraction == srcMaterial.IndexOfRefraction, "set IOR after dst material initialization");
}

public static void CopyChannelsTo(this MaterialBuilder srcMaterial, Material dstMaterial, params string[] channelKeys)
Expand Down
19 changes: 16 additions & 3 deletions tests/SharpGLTF.Toolkit.Tests/Materials/MaterialBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

using NUnit.Framework;

using SharpGLTF.Scenes;
using SharpGLTF.Schema2;
using SharpGLTF.Validation;

Expand All @@ -23,8 +24,8 @@ public void TestMaterialEquality()
// ... And we could use it for general equality checks, but then, since
// MaterialBuilder is NOT inmutable, it can mean that two materials can be equal
// at a given time, and non equal at another. Furthermore, it would imply having
// a hash code that changes over time. As a consequence, it could be impossible
// to use MaterialBuilder as a dictionary Key.
// a hash code that changes over time. As a consequence, using MaterialBuilder as
// a dictionary key is possible, but dangerous if not carefully handled.

var srcMaterial = _CreateUnlitMaterial();

Expand Down Expand Up @@ -100,6 +101,18 @@ public void CreateSpecularGlossinessWithFallback()
Assert.That(MaterialBuilder.AreEqualByContent(material, material.Clone()), Is.True);
}

[Test]
public void CreateIORWithFallback()
{
// https://github.com/vpenades/SharpGLTF/issues/246

var material = new MaterialBuilder("MaterialWithIOR");
material.IndexOfRefraction = 7;

Assert.That(MaterialBuilder.AreEqualByContent(material, material.Clone()), Is.True);
Assert.That(MaterialBuilder.AreEqualByContent(material, _Schema2Roundtrip(material)), Is.True);
}

private static MaterialBuilder _CreateUnlitMaterial()
{
var tex1 = ResourceInfo.From("shannon.png").FilePath;
Expand Down Expand Up @@ -192,7 +205,7 @@ private static MaterialBuilder _CreateClearCoatMaterial()
.WithClearCoatRoughness(tex1, 1);

return material;
}
}

[Obsolete("SpecularGlossiness has been deprecated by Khronos")]
private static MaterialBuilder _CreateSpecularGlossinessMaterialWithFallback()
Expand Down

0 comments on commit 2073cf3

Please sign in to comment.