diff --git a/UnitySDK/Assets/TiltBrush/Scripts/Gltf/GltfMaterialConverter.cs b/UnitySDK/Assets/TiltBrush/Scripts/Gltf/GltfMaterialConverter.cs index 10edb301..a146595d 100644 --- a/UnitySDK/Assets/TiltBrush/Scripts/Gltf/GltfMaterialConverter.cs +++ b/UnitySDK/Assets/TiltBrush/Scripts/Gltf/GltfMaterialConverter.cs @@ -32,7 +32,7 @@ public class GltfMaterialConverter { @".*([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})[/-]"); /// - /// Information about a Unity material generated from a Gltf node. + /// Information about a Unity material corresponding to a Gltf node. /// public struct UnityMaterial { /// @@ -49,12 +49,13 @@ public struct UnityMaterial { /// /// List of NEW Unity materials we have created. /// - private List newMaterials = new List(); + private List m_newMaterials = new List(); /// - /// Cache of Unity materials we have already created for each GltfMaterialBase. + /// For memoizing GetMaterial() /// - private Dictionary materials = new Dictionary(); + private Dictionary m_getMaterialMemo = + new Dictionary(); private static bool IsTiltBrushHostedUri(string uri) { // Will always look like "https://www.tiltbrush.com/shaders/..." @@ -111,42 +112,30 @@ public static IEnumerable LoadTexturesCoroutine( } /// - /// Gets (or creates) the Unity material corresponding to the given GLTF 2 material. + /// Gets (or creates) the Unity material corresponding to the given glTF material. /// - /// The GLTF material. + /// The glTF material. /// The Unity material that correpsonds to the given GLTF2 material. public UnityMaterial? GetMaterial(GltfMaterialBase gltfMaterial) { - // Have we already converted this material? - { - UnityMaterial result; - if (materials.TryGetValue(gltfMaterial, out result)) { - return result; - } + if (m_getMaterialMemo.TryGetValue(gltfMaterial, out UnityMaterial memo)) { + return memo; } - // Try to look up a global material first. - Material global; - if (null != (global = LookUpGlobalMaterial(gltfMaterial))) { - // Found it. - var result = new UnityMaterial { material = global, template = global }; - materials[gltfMaterial] = result; - return result; + if (LookUpGlobalMaterial(gltfMaterial) is UnityMaterial global) { + Debug.Assert(global.material == global.template); + m_getMaterialMemo[gltfMaterial] = global; + return global; } - // Ok, we will have to create a new material. - UnityMaterial? created = ConvertGltfMaterial(gltfMaterial); - if (created == null) { - Debug.LogErrorFormat("Failed to look up material {0}", gltfMaterial.name); - } else { - var result = created.Value; - materials[gltfMaterial] = result; - Debug.Assert(result.material != result.template); - if (result.material != result.template) { - newMaterials.Add(result.material); - } + if (ConvertGltfMaterial(gltfMaterial) is UnityMaterial created) { + Debug.Assert(created.material != created.template); + m_newMaterials.Add(created.material); + m_getMaterialMemo[gltfMaterial] = created; + return created; } - return created; + Debug.LogErrorFormat("Failed to convert material {0}", gltfMaterial.name); + return null; } /// @@ -154,7 +143,7 @@ public static IEnumerable LoadTexturesCoroutine( /// conversion process. /// public List GetGeneratedMaterials() { - return new List(newMaterials); + return new List(m_newMaterials); } /// true if there is a global material corresponding to the given glTF material, @@ -166,13 +155,13 @@ private static bool IsGlobalMaterial(GltfMaterialBase gltfMaterial) { /// /// Looks up a built-in global material that corresponds to the given GLTF material. - /// This will NOT create new materials, it will only look up global ones. + /// This will NOT create new materials; it will only look up global ones. /// /// The material to look up. /// The guid parsed from the material name, or Guid.None /// The global material that corresponds to the given GLTF material, /// if found. If not found, null. - private static Material LookUpGlobalMaterial(GltfMaterialBase gltfMaterial) { + private static UnityMaterial? LookUpGlobalMaterial(GltfMaterialBase gltfMaterial) { // Check if it's a Tilt Brush material. Guid guid = ParseGuidFromMaterial(gltfMaterial); if (guid != Guid.Empty) { @@ -180,7 +169,10 @@ private static Material LookUpGlobalMaterial(GltfMaterialBase gltfMaterial) { // these will be handled by the caller. BrushDescriptor desc; if (TbtSettings.Instance.TryGetBrush(guid, out desc)) { - return desc.Material; + return new UnityMaterial { + material = desc.Material, + template = desc.Material + }; } } return null; @@ -205,6 +197,8 @@ private static Material LookUpGlobalMaterial(GltfMaterialBase gltfMaterial) { /// The glTF1 material to convert. /// The result of the conversion, or null on failure. private UnityMaterial? ConvertGltf1Material(Gltf1Material gltfMat) { + // We know this guid doesn't map to a brush; if it did, LookupGlobalMaterial would + // have succeeded and we wouldn't be trying to create an new material. Guid instanceGuid = ParseGuidFromMaterial(gltfMat); Guid templateGuid = ParseGuidFromShader(gltfMat); @@ -240,7 +234,10 @@ private static Material LookUpGlobalMaterial(GltfMaterialBase gltfMaterial) { } // Tilt Brush doesn't support metallicRoughnessTexture (yet?) } - return CreateNewPbrMaterial(desc.Material, gltfMat.name, pbr); + var pbrInfo = new TbtSettings.PbrMaterialInfo { + material = desc.Material + }; + return CreateNewPbrMaterial(pbrInfo, gltfMat.name, pbr); } /// @@ -252,26 +249,25 @@ private static Material LookUpGlobalMaterial(GltfMaterialBase gltfMaterial) { /// The GLTF 2 material to convert. /// The result of the conversion private UnityMaterial? ConvertGltf2Material(Gltf2Material gltfMat) { - Material baseMaterial; { - string alphaMode = gltfMat.alphaMode == null ? null : gltfMat.alphaMode.ToUpperInvariant(); + TbtSettings.PbrMaterialInfo pbrInfo; - switch (alphaMode) { + string alphaMode = gltfMat.alphaMode == null ? null : gltfMat.alphaMode.ToUpperInvariant(); + switch (alphaMode) { case null: case "": case Gltf2Material.kAlphaModeOpaque: - baseMaterial = gltfMat.doubleSided - ? TbtSettings.Instance.m_BasePbrOpaqueDoubleSidedMaterial - : TbtSettings.Instance.m_BasePbrOpaqueSingleSidedMaterial; + pbrInfo = gltfMat.doubleSided + ? TbtSettings.Instance.m_PbrOpaqueDoubleSided + : TbtSettings.Instance.m_PbrOpaqueSingleSided; break; case Gltf2Material.kAlphaModeBlend: - baseMaterial = gltfMat.doubleSided - ? TbtSettings.Instance.m_BasePbrBlendDoubleSidedMaterial - : TbtSettings.Instance.m_BasePbrBlendSingleSidedMaterial; + pbrInfo = gltfMat.doubleSided + ? TbtSettings.Instance.m_PbrBlendDoubleSided + : TbtSettings.Instance.m_PbrBlendSingleSided; break; default: Debug.LogWarning($"Not yet supported: alphaMode={alphaMode}"); goto case Gltf2Material.kAlphaModeOpaque; - } } if (gltfMat.pbrMetallicRoughness == null) { @@ -291,13 +287,14 @@ private static Material LookUpGlobalMaterial(GltfMaterialBase gltfMaterial) { } } - return CreateNewPbrMaterial(baseMaterial, gltfMat.name, gltfMat.pbrMetallicRoughness); + return CreateNewPbrMaterial(pbrInfo, gltfMat.name, gltfMat.pbrMetallicRoughness); } // Helper for ConvertGltf{1,2}Material private UnityMaterial CreateNewPbrMaterial( - Material baseMaterial, string gltfMatName, Gltf2Material.PbrMetallicRoughness pbr) { - Material mat = UnityEngine.Object.Instantiate(baseMaterial); + TbtSettings.PbrMaterialInfo pbrInfo, string gltfMatName, + Gltf2Material.PbrMetallicRoughness pbr) { + Material mat = UnityEngine.Object.Instantiate(pbrInfo.material); Texture tex = null; if (pbr.baseColorTexture != null) { @@ -310,9 +307,8 @@ private UnityMaterial CreateNewPbrMaterial( mat.name = gltfMatName; } else { // No name in the gltf; make up something reasonable - string matName = baseMaterial.name.StartsWith("Base") - ? baseMaterial.name.Substring(4) - : baseMaterial.name; + string matName = pbrInfo.material.name; + if (matName.StartsWith("Base")) { matName = matName.Substring(4); } if (tex != null) { matName = string.Format("{0}_{1}", matName, tex.name); } @@ -323,7 +319,10 @@ private UnityMaterial CreateNewPbrMaterial( mat.SetFloat("_MetallicFactor", pbr.metallicFactor); mat.SetFloat("_RoughnessFactor", pbr.roughnessFactor); - return new UnityMaterial { material = mat, template = baseMaterial }; + return new UnityMaterial { + material = mat, + template = pbrInfo.material + }; } private static string SanitizeName(string uri) { diff --git a/UnitySDK/Assets/TiltBrush/Scripts/Gltf/ImportGltf.cs b/UnitySDK/Assets/TiltBrush/Scripts/Gltf/ImportGltf.cs index d5d9a7e2..1c0fa0d1 100644 --- a/UnitySDK/Assets/TiltBrush/Scripts/Gltf/ImportGltf.cs +++ b/UnitySDK/Assets/TiltBrush/Scripts/Gltf/ImportGltf.cs @@ -31,12 +31,15 @@ public class Null { protected Null() {} } -/// A callback that allows users to know when the import process has created a new material +/// A callback that allows users to know when the import process has processed +/// a gltf material into a Unity material. public interface IImportMaterialCollector { // Pass: // unityMaterial - the template material, and the actual material used // (which may be identical to the template material) - // gltfMaterial - the gltf node from which unityMaterial was generated + // gltfMaterial - the gltf node from which unityMaterial was looked up or created + // + // It's acceptable to call this redundantly. void Add(GltfMaterialConverter.UnityMaterial unityMaterial, GltfMaterialBase gltfMaterial); } diff --git a/UnitySDK/Assets/TiltBrush/Scripts/TbtSettings.cs b/UnitySDK/Assets/TiltBrush/Scripts/TbtSettings.cs index 5e296440..af438580 100644 --- a/UnitySDK/Assets/TiltBrush/Scripts/TbtSettings.cs +++ b/UnitySDK/Assets/TiltBrush/Scripts/TbtSettings.cs @@ -19,6 +19,10 @@ namespace TiltBrushToolkit { public class TbtSettings : ScriptableObject { + [Serializable] + public struct PbrMaterialInfo { + public Material material; + } const string kAssetName = "TiltBrushToolkitSettings"; private static TbtSettings sm_Instance; @@ -45,12 +49,10 @@ public static BrushManifest BrushManifest { [SerializeField] private BrushManifest m_BrushManifest = null; - // This is the same material used by the BrushDescriptor "PbrTemplate". - public Material m_BasePbrOpaqueDoubleSidedMaterial; - public Material m_BasePbrOpaqueSingleSidedMaterial; - // This is the same material used by the BrushDescriptor "PbrTransparentTemplate" - public Material m_BasePbrBlendDoubleSidedMaterial; - public Material m_BasePbrBlendSingleSidedMaterial; + public PbrMaterialInfo m_PbrOpaqueSingleSided; + public PbrMaterialInfo m_PbrOpaqueDoubleSided; + public PbrMaterialInfo m_PbrBlendSingleSided; + public PbrMaterialInfo m_PbrBlendDoubleSided; /// null if not found public bool TryGetBrush(Guid guid, out BrushDescriptor desc) {