Fixed ATOM-14613 Baseviewer MatertialHotReloadTest fails to change the color after turning blending on and off

The problem was...
After a MaterialAsset reload, there could be two different versions of the MaterialAsset in memory: the old one and the reloaded one. The old one is still connected to buses and can send reinitialization messages when other things reload or reinitialize. So when the shader asset reloaded, both the old and new MaterialAsset were sending reinitialization messages. Material::OnMaterialAssetReinitialized was using the materialAsset parameter to initialize the Material, and the latest call to OnMaterialAssetReinitialized was for the *old* MaterialAsset.

The solution is to use the m_materialAsset member when reinitializing the Material. I also added checks in a couple places to skip unnecessary reinitialization, and added comments in the bus headers to warn developers about this issue.

Testing: Added a new step to ASV's MaterialHotReloadTest.bv.lua script for the error scenario, and this now passes. Ran ASV full test suite, both dx12 and vulkan, only known issues occurred.

Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Material/MaterialReloadNotificationBus.h

@@ -24,6 +24,11 @@ namespace AZ
 
         //! Connect to this EBus to get notifications whenever material objects reload.
         //! The bus address is the AssetId of the MaterialAsset or MaterialTypeAsset.
+        //! 
+        //! Be careful when using the parameters provided by these functions. The bus ID is an AssetId, and it's possible for the system to have
+        //! both *old* versions and *new reloaded* versions of the asset in memory at the same time, and they will have the same AssetId. Therefore
+        //! your bus Handlers could receive Reinitialized messages from multiple sources. It may be necessary to check the memory addresses of these
+        //! parameters against local members before using this data.
         class MaterialReloadNotifications
             : public EBusTraits
         {

Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Shader/ShaderReloadNotificationBus.h

@@ -27,6 +27,11 @@ namespace AZ
         /**
          * Connect to this EBus to get notifications whenever a shader system class reinitializes itself.
          * The bus address is the AssetId of the ShaderAsset, even when the thing being reinitialized is a ShaderVariant or other shader related class.
+         * 
+         * Be careful when using the parameters provided by these functions. The bus ID is an AssetId, and it's possible for the system to have
+         * both *old* versions and *new reloaded* versions of the asset in memory at the same time, and they will have the same AssetId. Therefore
+         * your bus Handlers could receive Reinitialized messages from multiple sources. It may be necessary to check the memory addresses of these
+         * parameters against local members before using this data.
          */
         class ShaderReloadNotifications
             : public EBusTraits

Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp

@@ -242,8 +242,16 @@ namespace AZ
         // MaterialReloadNotificationBus overrides...
         void Material::OnMaterialAssetReinitialized(const Data::Asset<MaterialAsset>& materialAsset)
         {
-            ShaderReloadDebugTracker::ScopedSection reloadSection("{%p}->Material::OnMaterialAssetReinitialized %s", this, materialAsset.GetHint().c_str());
-            OnAssetReloaded(materialAsset);
+            // It's important that we don't just pass materialAsset to Init() because when reloads occur,
+            // it's possible for old Asset objects to hang around and report reinitialization, so materialAsset
+            // might be stale data.
+
+            if (materialAsset.Get() == m_materialAsset.Get())
+            {
+                ShaderReloadDebugTracker::ScopedSection reloadSection("{%p}->Material::OnMaterialAssetReinitialized %s", this, materialAsset.GetHint().c_str());
+
+                OnAssetReloaded(m_materialAsset);
+            }
         }
 
         ///////////////////////////////////////////////////////////////////
@@ -259,8 +267,6 @@ namespace AZ
 
         void Material::OnShaderAssetReinitialized(const Data::Asset<ShaderAsset>& shaderAsset)
         {
-            // TODO: I think we should make Shader handle OnShaderAssetReinitialized and treat it just like the shader reloaded.
-
             ShaderReloadDebugTracker::ScopedSection reloadSection("{%p}->Material::OnShaderAssetReinitialized %s", this, shaderAsset.GetHint().c_str());
             // Note that it might not be strictly necessary to reinitialize the entire material, we might be able to get away with
             // just bumping the m_currentChangeId or some other minor updates. But it's pretty hard to know what exactly needs to be

Gems/Atom/RPI/Code/Source/RPI.Public/Shader/Shader.cpp

@@ -213,10 +213,15 @@ namespace AZ
         // ShaderReloadNotificationBus overrides...
         void Shader::OnShaderAssetReinitialized(const Data::Asset<ShaderAsset>& shaderAsset)
         {
-            ShaderReloadDebugTracker::ScopedSection reloadSection("{%p}->Shader::OnShaderAssetReinitialized %s", this, shaderAsset.GetHint().c_str());
+            // When reloads occur, it's possible for old Asset objects to hang around and report reinitialization,
+            // so we can reduce unnecessary reinitialization in that case.
+            if (shaderAsset.Get() == m_asset.Get())
+            {
+                ShaderReloadDebugTracker::ScopedSection reloadSection("{%p}->Shader::OnShaderAssetReinitialized %s", this, shaderAsset.GetHint().c_str());
             
-            Init(*m_asset.Get());
-            ShaderReloadNotificationBus::Event(shaderAsset.GetId(), &ShaderReloadNotificationBus::Events::OnShaderReinitialized, *this);
+                Init(*m_asset.Get());
+                ShaderReloadNotificationBus::Event(shaderAsset.GetId(), &ShaderReloadNotificationBus::Events::OnShaderReinitialized, *this);
+            }
         }
         ///////////////////////////////////////////////////////////////////
 

Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp

@@ -115,12 +115,19 @@ namespace AZ
             }
         }
 
-        void MaterialAsset::OnMaterialTypeAssetReinitialized(const Data::Asset<MaterialTypeAsset>&)
+        void MaterialAsset::OnMaterialTypeAssetReinitialized(const Data::Asset<MaterialTypeAsset>& materialTypeAsset)
         {
-            // MaterialAsset doesn't need to reinitialize any of its own data when MaterialTypeAsset reinitializes,
-            // because all it depends on is the MaterialTypeAsset reference, rather than the data inside it.
-            // Ultimately it's the Material that cares about these changes, so we just forward any signal we get.
-            MaterialReloadNotificationBus::Event(GetId(), &MaterialReloadNotifications::OnMaterialAssetReinitialized, Data::Asset<MaterialAsset>{this, AZ::Data::AssetLoadBehavior::PreLoad});
+            // When reloads occur, it's possible for old Asset objects to hang around and report reinitialization,
+            // so we can reduce unnecessary reinitialization in that case.
+            if (materialTypeAsset.Get() == m_materialTypeAsset.Get()) 
+            {
+                ShaderReloadDebugTracker::ScopedSection reloadSection("{%p}->MaterialAsset::OnMaterialTypeAssetReinitialized %s", this, materialTypeAsset.GetHint().c_str());
+
+                // MaterialAsset doesn't need to reinitialize any of its own data when MaterialTypeAsset reinitializes,
+                // because all it depends on is the MaterialTypeAsset reference, rather than the data inside it.
+                // Ultimately it's the Material that cares about these changes, so we just forward any signal we get.
+                MaterialReloadNotificationBus::Event(GetId(), &MaterialReloadNotifications::OnMaterialAssetReinitialized, Data::Asset<MaterialAsset>{this, AZ::Data::AssetLoadBehavior::PreLoad});
+            }
         }
         
         void MaterialAsset::ReinitializeMaterialTypeAsset(Data::Asset<Data::AssetData> asset)