From 1ead4b1649e54e7929a6022b3cc0ac21d3e46b9e Mon Sep 17 00:00:00 2001
From: Wojtek Figat <wojtek@figat.pl>
Date: Sun, 21 Feb 2021 17:40:37 +0100
Subject: [PATCH] Fix MaterialParams sync bug

---
 .../Content/Assets/MaterialInstance.cpp       | 31 ++++-----
 .../Graphics/Materials/MaterialParams.cpp     | 61 +++++++++++++++++
 .../Graphics/Materials/MaterialParams.h       | 65 ++-----------------
 3 files changed, 82 insertions(+), 75 deletions(-)

diff --git a/Source/Engine/Content/Assets/MaterialInstance.cpp b/Source/Engine/Content/Assets/MaterialInstance.cpp
index ace2fcee9..426c32b0b 100644
--- a/Source/Engine/Content/Assets/MaterialInstance.cpp
+++ b/Source/Engine/Content/Assets/MaterialInstance.cpp
@@ -23,17 +23,25 @@ void MaterialInstance::OnBaseSet()
     _baseMaterial->OnUnloaded.Bind<MaterialInstance, &MaterialInstance::OnBaseUnloaded>(this);
     _baseMaterial->ParamsChanged.Bind<MaterialInstance, &MaterialInstance::OnBaseParamsChanged>(this);
 
+    // Sync parameters with the base parameters to ensure all data is valid for rendering (constants offset and resource register)
     MaterialParams& baseParams = _baseMaterial->Params;
-    bool validParams = Params.Count() == baseParams.Count();
-    for (int32 i = 0; validParams && i < Params.Count(); i++)
+    Params._versionHash = 0;
+    if (Params.Count() != baseParams.Count())
+    {
+        // Params changed
+        OnBaseParamsChanged();
+        return;
+    }
+    for (int32 i = 0; i < Params.Count(); i++)
     {
         MaterialParameter& param = Params[i];
         MaterialParameter& baseParam = baseParams[i];
 
         if (param.GetID() != baseParam.GetID() || param.GetParameterType() != baseParam.GetParameterType())
         {
-            validParams = false;
-            break;
+            // Params changed
+            OnBaseParamsChanged();
+            return;
         }
 
         param._isPublic = baseParam._isPublic;
@@ -41,21 +49,14 @@ void MaterialInstance::OnBaseSet()
         param._offset = baseParam._offset;
         param._name = baseParam._name;
     }
-    if (validParams)
-    {
-        Params._versionHash = baseParams._versionHash;
-        ParamsChanged();
-    }
-    else
-    {
-        OnBaseParamsChanged();
-    }
+
+    // Params are valid
+    Params._versionHash = baseParams._versionHash;
+    ParamsChanged();
 }
 
 void MaterialInstance::OnBaseUnset()
 {
-    ScopeLock lock(_baseMaterial->Locker);
-
     _baseMaterial->RemoveReference();
     _baseMaterial->OnUnloaded.Unbind<MaterialInstance, &MaterialInstance::OnBaseUnloaded>(this);
     _baseMaterial->ParamsChanged.Unbind<MaterialInstance, &MaterialInstance::OnBaseParamsChanged>(this);
diff --git a/Source/Engine/Graphics/Materials/MaterialParams.cpp b/Source/Engine/Graphics/Materials/MaterialParams.cpp
index 1f1d1bac6..81a630629 100644
--- a/Source/Engine/Graphics/Materials/MaterialParams.cpp
+++ b/Source/Engine/Graphics/Materials/MaterialParams.cpp
@@ -440,6 +440,67 @@ String MaterialParameter::ToString() const
     return String::Format(TEXT("\'{0}\' ({1}:{2}:{3})"), _name, ::ToString(_type), _paramId, _isPublic);
 }
 
+MaterialParameter* MaterialParams::Get(const Guid& id)
+{
+    MaterialParameter* result = nullptr;
+    for (int32 i = 0; i < Count(); i++)
+    {
+        if (At(i).GetParameterID() == id)
+        {
+            result = &At(i);
+            break;
+        }
+    }
+    return result;
+}
+
+MaterialParameter* MaterialParams::Get(const StringView& name)
+{
+    MaterialParameter* result = nullptr;
+    for (int32 i = 0; i < Count(); i++)
+    {
+        if (At(i).GetName() == name)
+        {
+            result = &At(i);
+            break;
+        }
+    }
+    return result;
+}
+
+int32 MaterialParams::Find(const Guid& id)
+{
+    int32 result = -1;
+    for (int32 i = 0; i < Count(); i++)
+    {
+        if (At(i).GetParameterID() == id)
+        {
+            result = i;
+            break;
+        }
+    }
+    return result;
+}
+
+int32 MaterialParams::Find(const StringView& name)
+{
+    int32 result = -1;
+    for (int32 i = 0; i < Count(); i++)
+    {
+        if (At(i).GetName() == name)
+        {
+            result = i;
+            break;
+        }
+    }
+    return result;
+}
+
+int32 MaterialParams::GetVersionHash() const
+{
+    return _versionHash;
+}
+
 void MaterialParams::Bind(MaterialParamsLink* link, MaterialParameter::BindMeta& meta)
 {
     ASSERT(link && link->This);
diff --git a/Source/Engine/Graphics/Materials/MaterialParams.h b/Source/Engine/Graphics/Materials/MaterialParams.h
index 997b18d34..1036e9402 100644
--- a/Source/Engine/Graphics/Materials/MaterialParams.h
+++ b/Source/Engine/Graphics/Materials/MaterialParams.h
@@ -366,72 +366,17 @@ private:
 
 public:
 
-    MaterialParameter* Get(const Guid& id)
-    {
-        MaterialParameter* result = nullptr;
-        for (int32 i = 0; i < Count(); i++)
-        {
-            if (At(i).GetParameterID() == id)
-            {
-                result = &At(i);
-                break;
-            }
-        }
-        return result;
-    }
-
-    MaterialParameter* Get(const StringView& name)
-    {
-        MaterialParameter* result = nullptr;
-        for (int32 i = 0; i < Count(); i++)
-        {
-            if (At(i).GetName() == name)
-            {
-                result = &At(i);
-                break;
-            }
-        }
-        return result;
-    }
-
-    int32 Find(const Guid& id)
-    {
-        int32 result = -1;
-        for (int32 i = 0; i < Count(); i++)
-        {
-            if (At(i).GetParameterID() == id)
-            {
-                result = i;
-                break;
-            }
-        }
-        return result;
-    }
-
-    int32 Find(const StringView& name)
-    {
-        int32 result = -1;
-        for (int32 i = 0; i < Count(); i++)
-        {
-            if (At(i).GetName() == name)
-            {
-                result = i;
-                break;
-            }
-        }
-        return result;
-    }
+    MaterialParameter* Get(const Guid& id);
+    MaterialParameter* Get(const StringView& name);
+    int32 Find(const Guid& id);
+    int32 Find(const StringView& name);
 
 public:
 
     /// <summary>
     /// Gets the parameters version hash. Every time the parameters are modified (loaded, edited, etc.) the hash changes. Can be used to sync instanced parameters collection.
     /// </summary>
-    /// <returns>The hash.</returns>
-    FORCE_INLINE int32 GetVersionHash() const
-    {
-        return _versionHash;
-    }
+    int32 GetVersionHash() const;
 
 public: