birkeh/o3de
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fixed race conditions that could prevent successful hot-reload of shaders.

Browse Source 
ATOM-15728 Shader Hot Reload Fails in Debug Build

The main change was to add OnAssetReady handlers to each of the asset classes. See comments in ShaderAsset::OnAssetReady for a detailed explanation. In short, OnAssetReloaded gets missed while assets are being reloaded at the same time on multiple threads, but OnAssetReady is always called whenever connecting to the AssetBus because of its AssetConnectionPolicy.

The above change required the addition of a new AssetInitBus to call the PostLoadInit() functions. Because OnAssetReady connects to buses that are not mutex-protected, they have to be connected on the main thread. AssetInitBus::PostLoadInit is called every frame in RPISystem::SimulationTick. All Atom's asset handlers that need to do post-load initialization must connect to the AssetInitBus, and the asset will disconnect itself after initialization is complete.

We also need the Shader class to handle OnShaderAssetReinitialized to properly handle the shader reload.

With these changes I can click back and forth between "Blending On" and "Blending Off" many times (like 20 times) without issue.
This commit is contained in:
Chris Santora
2021-06-14 20:36:12 -07:00
parent 406792606b
commit 982406d4d5
12 changed files with 179 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Gems/Atom/RPI/Code/Source/RPI.Reflect/Shader/ShaderAsset.cpp
+30 -23
View File
@@ -85,6 +85,7 @@ namespace AZ
{
Data::AssetBus::Handler::BusDisconnect();
ShaderVariantFinderNotificationBus::Handler::BusDisconnect();
AssetInitBus::Handler::BusDisconnect();
}
const Name& ShaderAsset::GetName() const
@@ -332,7 +333,7 @@ namespace AZ
return m_perAPIShaderData[0];
}
bool ShaderAsset::FinalizeAfterLoad()
bool ShaderAsset::PostLoadInit()
{
// Use the current RHI that is active to select which shader data to use.
// We don't assert if the Factory is not available because this method could be called during build time,
@@ -373,25 +374,43 @@ namespace AZ
// Once the ShaderAsset is loaded, it is necessary to listen for changes in the Root Variant Asset.
Data::AssetBus::Handler::BusConnect(GetRootVariant().GetId());
ShaderVariantFinderNotificationBus::Handler::BusConnect(GetId());
AssetInitBus::Handler::BusDisconnect();
return true;
}
void ShaderAsset::ReinitializeRootShaderVariant(Data::Asset<Data::AssetData> asset)
{
Data::Asset<ShaderVariantAsset> shaderVariantAsset = { asset.GetAs<ShaderVariantAsset>(), AZ::Data::AssetLoadBehavior::PreLoad };
AZ_Assert(shaderVariantAsset->GetStableId() == RootShaderVariantStableId, "Was expecting to update the root variant");
GetCurrentShaderApiData().m_rootShaderVariantAsset = asset;
ShaderReloadNotificationBus::Event(GetId(), &ShaderReloadNotificationBus::Events::OnShaderAssetReinitialized, Data::Asset<ShaderAsset>{ this, AZ::Data::AssetLoadBehavior::PreLoad } );
}
///////////////////////////////////////////////////////////////////////
// AssetBus overrides...
void ShaderAsset::OnAssetReloaded(Data::Asset<Data::AssetData> asset)
{
ShaderReloadDebugTracker::ScopedSection reloadSection("{%p}->ShaderAsset::OnAssetReloaded %s", this, asset.GetHint().c_str());
ReinitializeRootShaderVariant(asset);
}
void ShaderAsset::OnAssetReady(Data::Asset<Data::AssetData> asset)
{
// We have to listen to OnAssetReady, OnAssetReloaded isn't enough, because of the following scenario:
// The user changes a .shader file, which causes the AP to rebuild the ShaderAsset and root ShaderVariantAsset.
// 1) Thread A creates the new ShaderAsset, loads it, and gets the old ShaderVariantAsset.
// 2) Thread B creates the new ShaderVariantAsset, loads it, and calls OnAssetReloaded.
// 3) Main thread calls ShaderAsset::PostLoadInit which connects to the AssetBus but it's too late to receive OnAssetReloaded,
// so it continues using the old ShaderVariantAsset instead of the new one.
// The OnAssetReady bus function is called automatically whenever a connection to AssetBus is made, so listening to this gives
// us the opportunity to assign the appropriate ShaderVariantAsset.
Data::Asset<ShaderVariantAsset> shaderVariantAsset = { asset.GetAs<ShaderVariantAsset>(), AZ::Data::AssetLoadBehavior::PreLoad };
AZ_Assert(shaderVariantAsset->GetStableId() == RootShaderVariantStableId,
"Was expecting to update the root variant");
GetCurrentShaderApiData().m_rootShaderVariantAsset = asset;
ShaderReloadNotificationBus::Event(GetId(), &ShaderReloadNotificationBus::Events::OnShaderAssetReinitialized, Data::Asset<ShaderAsset>{ this, AZ::Data::AssetLoadBehavior::PreLoad } );
ShaderReloadDebugTracker::ScopedSection reloadSection("{%p}->ShaderAsset::OnAssetReady %s", this, asset.GetHint().c_str());
ReinitializeRootShaderVariant(asset);
}
///////////////////////////////////////////////////////////////////////
///////////////////////////////////////////////////////////////////
/// ShaderVariantFinderNotificationBus overrides
void ShaderAsset::OnShaderVariantTreeAssetReady(Data::Asset<ShaderVariantTreeAsset> shaderVariantTreeAsset, bool isError)
@@ -425,25 +444,13 @@ namespace AZ
{
if (Base::LoadAssetData(asset, stream, assetLoadFilterCB) == Data::AssetHandler::LoadResult::LoadComplete)
{
return PostLoadInit(asset);
}
return Data::AssetHandler::LoadResult::Error;
}
Data::AssetHandler::LoadResult ShaderAssetHandler::PostLoadInit(const Data::Asset<Data::AssetData>& asset)
{
if (ShaderAsset* shaderAsset = asset.GetAs<ShaderAsset>())
{
if (!shaderAsset->FinalizeAfterLoad())
{
AZ_Error("ShaderAssetHandler", false, "Shader asset failed to finalize.");
return Data::AssetHandler::LoadResult::Error;
}
asset.GetAs<ShaderAsset>()->AssetInitBus::Handler::BusConnect();
return Data::AssetHandler::LoadResult::LoadComplete;
}
return Data::AssetHandler::LoadResult::Error;
}
///////////////////////////////////////////////////////////////////////
} // namespace RPI