Commit Graph

6 Commits (7dabe8b6e966273940e7d98284966104064b783e)

Author SHA1 Message Date
Chris Santora b176697ce9 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.
5 years ago
Chris Santora c158ca178f Added new shader reinitialization signaling.
This was done while working on "ATOM-15728 Shader Hot Reload Fails in Debug Build", but it turned out these changes did not actually fix the issue (or any other known hot-reload issue). Still, these improvements are appropriate as they correct logical oversights.

ShaderVariant was not listening to asset reloads. It needs to know when the ShaderVariantAsset reload happens so it can reinitialize it's members as well as propagate reinitialization messages. I added a member for the ShaderAsset as the class needs this to reinitialize itself. So now the class listens for reloads of both the ShaderVariantAsset and the ShaderAsset.

Shader was not listening for ShaderAsset reinitialization events.

Updated the API for ShaderReloadNotificationBus's OnShaderVariantReinitialized to include the ShaderVariant which is the most relevant information (the other information wasn't really being used anyway).
5 years ago
Chris Santora 982406d4d5 Fixed race conditions that could prevent successful hot-reload of shaders.
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.
5 years ago
Santora 00062430a2 Updated the usage of ShaderReloadDebugTracker to include the address of the object. This is necessary for debugging where multiple assets may be reloading at the same time. Also added a Printf function for generic messages at the current indent level.
This is specifically in support of:
ATOM-14613 Baseviewer MatertialHotReloadTest fails to change the color after turning blending on and off
ATOM-15728 Shader Hot Reload Fails in Debug Build
5 years ago
alexpete 75dc720198 Integrating latest 47acbe8 5 years ago
alexpete a10351f38d Initial commit 5 years ago