From f177f671ac14d24ac46d10df835aa611df4a8f30 Mon Sep 17 00:00:00 2001 From: Tommy Walton <82672795+amzn-tommy@users.noreply.github.com> Date: Wed, 7 Jul 2021 15:09:14 -0700 Subject: [PATCH] Fix for LYN-4594: [Atom][EMFX] Loading a saved level with entity and an actor component causes the editor to freeze. (DCO fixup) (#1878) * Queue shader loads and register with the dynamic draw context after the shader is loaded to avoid a deadlock when there are multiple scenes processing at the same time. Signed-off-by: amzn-tommy * Fixed AzCore case in AtomFont.h and two other files I found while searching Signed-off-by: amzn-tommy * Switched to a utility that will both get the assetId and create the Asset, without calling GetAsset explicitely Signed-off-by: amzn-tommy * Fixing typos in error message Signed-off-by: amzn-tommy --- .../AtomLyIntegration/AtomFont/AtomFont.h | 5 ++ .../AtomFont/Code/Source/AtomFont.cpp | 56 +++++++++++++------ ...tomViewportDisplayIconsSystemComponent.cpp | 39 +++++++++---- .../AtomViewportDisplayIconsSystemComponent.h | 4 ++ Gems/Blast/Code/Include/Blast/BlastDebug.h | 2 +- .../Code/Source/Family/ActorRenderManager.h | 2 +- 6 files changed, 78 insertions(+), 30 deletions(-) diff --git a/Gems/AtomLyIntegration/AtomFont/Code/Include/AtomLyIntegration/AtomFont/AtomFont.h b/Gems/AtomLyIntegration/AtomFont/Code/Include/AtomLyIntegration/AtomFont/AtomFont.h index 9fe3f2c25b..9e6c38a939 100644 --- a/Gems/AtomLyIntegration/AtomFont/Code/Include/AtomLyIntegration/AtomFont/AtomFont.h +++ b/Gems/AtomLyIntegration/AtomFont/Code/Include/AtomLyIntegration/AtomFont/AtomFont.h @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -38,6 +39,7 @@ namespace AZ class AtomFont : public ICryFont , public AzFramework::FontQueryInterface + , private Data::AssetBus::Handler { friend class FFont; @@ -122,6 +124,9 @@ namespace AZ //! \param outputFullPath Full path to loaded font family, may need resolving with PathUtil::MakeGamePath. XmlNodeRef LoadFontFamilyXml(const char* fontFamilyName, string& outputDirectory, string& outputFullPath); + // Data::AssetBus::Handler overrides... + void OnAssetReady(Data::Asset asset) override; + private: AzFramework::ISceneSystem::SceneEvent::Handler m_sceneEventHandler; diff --git a/Gems/AtomLyIntegration/AtomFont/Code/Source/AtomFont.cpp b/Gems/AtomLyIntegration/AtomFont/Code/Source/AtomFont.cpp index 114dbf62bb..cb157b95f6 100644 --- a/Gems/AtomLyIntegration/AtomFont/Code/Source/AtomFont.cpp +++ b/Gems/AtomLyIntegration/AtomFont/Code/Source/AtomFont.cpp @@ -29,6 +29,7 @@ #include #include +#include // Static member definitions const AZ::AtomFont::GlyphSize AZ::AtomFont::defaultGlyphSize = AZ::AtomFont::GlyphSize(ICryFont::defaultGlyphSizeX, ICryFont::defaultGlyphSizeY); @@ -349,30 +350,18 @@ AZ::AtomFont::AtomFont(ISystem* system) #endif AZ::Interface::Register(this); - // register font per viewport dynamic draw context. + // Queue a load for the font per viewport dynamic draw context shader, and wait for it to load static const char* shaderFilepath = "Shaders/SimpleTextured.azshader"; - AZ::AtomBridge::PerViewportDynamicDraw::Get()->RegisterDynamicDrawContext( - AZ::Name(AZ::AtomFontDynamicDrawContextName), - [](RPI::Ptr drawContext) - { - Data::Instance shader = AZ::RPI::LoadShader(shaderFilepath); - AZ::RPI::ShaderOptionList shaderOptions; - shaderOptions.push_back(AZ::RPI::ShaderOption(AZ::Name("o_useColorChannels"), AZ::Name("false"))); - shaderOptions.push_back(AZ::RPI::ShaderOption(AZ::Name("o_clamp"), AZ::Name("true"))); - drawContext->InitShaderWithVariant(shader, &shaderOptions); - drawContext->InitVertexFormat( - { - {"POSITION", RHI::Format::R32G32B32_FLOAT}, - {"COLOR", RHI::Format::B8G8R8A8_UNORM}, - {"TEXCOORD0", RHI::Format::R32G32_FLOAT} - }); - drawContext->EndInit(); - }); + Data::Asset shaderAsset = RPI::AssetUtils::GetAssetByProductPath(shaderFilepath, RPI::AssetUtils::TraceLevel::Assert); + shaderAsset.QueueLoad(); + Data::AssetBus::Handler::BusConnect(shaderAsset.GetId()); } AZ::AtomFont::~AtomFont() { + Data::AssetBus::Handler::BusDisconnect(); + AZ::Interface::Unregister(this); m_defaultFontDrawInterface = nullptr; @@ -864,5 +853,36 @@ XmlNodeRef AZ::AtomFont::LoadFontFamilyXml(const char* fontFamilyName, string& o return root; } + +void AZ::AtomFont::OnAssetReady(Data::Asset asset) +{ + Data::Asset shaderAsset = asset; + + AZ::AtomBridge::PerViewportDynamicDraw::Get()->RegisterDynamicDrawContext( + AZ::Name(AZ::AtomFontDynamicDrawContextName), + [shaderAsset](RPI::Ptr drawContext) + { + AZ_Assert(shaderAsset->IsReady(), "Attempting to register the AtomFont" + " dynamic draw context before the shader asset is loaded. The shader should be loaded first" + " to avoid a blocking asset load and potential deadlock, since the DynamicDrawContext lambda" + " will be executed during scene processing and there may be multiple scenes executing in parallel."); + + Data::Instance shader = RPI::Shader::FindOrCreate(shaderAsset); + AZ::RPI::ShaderOptionList shaderOptions; + shaderOptions.push_back(AZ::RPI::ShaderOption(AZ::Name("o_useColorChannels"), AZ::Name("false"))); + shaderOptions.push_back(AZ::RPI::ShaderOption(AZ::Name("o_clamp"), AZ::Name("true"))); + drawContext->InitShaderWithVariant(shader, &shaderOptions); + drawContext->InitVertexFormat( + { + {"POSITION", RHI::Format::R32G32B32_FLOAT}, + {"COLOR", RHI::Format::B8G8R8A8_UNORM}, + {"TEXCOORD0", RHI::Format::R32G32_FLOAT} + }); + drawContext->EndInit(); + }); + + Data::AssetBus::Handler::BusDisconnect(); +} + #endif diff --git a/Gems/AtomLyIntegration/AtomViewportDisplayIcons/Code/Source/AtomViewportDisplayIconsSystemComponent.cpp b/Gems/AtomLyIntegration/AtomViewportDisplayIcons/Code/Source/AtomViewportDisplayIconsSystemComponent.cpp index 305fad8ad0..f3f06a2efc 100644 --- a/Gems/AtomLyIntegration/AtomViewportDisplayIcons/Code/Source/AtomViewportDisplayIconsSystemComponent.cpp +++ b/Gems/AtomLyIntegration/AtomViewportDisplayIcons/Code/Source/AtomViewportDisplayIconsSystemComponent.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -87,6 +88,7 @@ namespace AZ::Render void AtomViewportDisplayIconsSystemComponent::Deactivate() { + Data::AssetBus::Handler::BusDisconnect(); Bootstrap::NotificationBus::Handler::BusDisconnect(); auto perViewportDynamicDrawInterface = AtomBridge::PerViewportDynamicDraw::Get(); @@ -338,15 +340,32 @@ namespace AZ::Render void AtomViewportDisplayIconsSystemComponent::OnBootstrapSceneReady([[maybe_unused]]AZ::RPI::Scene* bootstrapScene) { - AtomBridge::PerViewportDynamicDraw::Get()->RegisterDynamicDrawContext(m_drawContextName, [](RPI::Ptr drawContext) - { - auto shader = RPI::LoadShader(DrawContextShaderPath); - drawContext->InitShader(shader); - drawContext->InitVertexFormat( - {{"POSITION", RHI::Format::R32G32B32_FLOAT}, - {"COLOR", RHI::Format::R8G8B8A8_UNORM}, - {"TEXCOORD", RHI::Format::R32G32_FLOAT}}); - drawContext->EndInit(); - }); + // Queue a load for the draw context shader, and wait for it to load + Data::Asset shaderAsset = RPI::AssetUtils::GetAssetByProductPath(DrawContextShaderPath, RPI::AssetUtils::TraceLevel::Assert); + shaderAsset.QueueLoad(); + Data::AssetBus::Handler::BusConnect(shaderAsset.GetId()); + } + + void AtomViewportDisplayIconsSystemComponent::OnAssetReady(Data::Asset asset) + { + // Once the shader is loaded, register it with the dynamic draw context + Data::Asset shaderAsset = asset; + AtomBridge::PerViewportDynamicDraw::Get()->RegisterDynamicDrawContext(m_drawContextName, [shaderAsset](RPI::Ptr drawContext) + { + AZ_Assert(shaderAsset->IsReady(), "Attempting to register the AtomViewportDisplayIconsSystemComponent" + " dynamic draw context before the shader asset is loaded. The shader should be loaded first" + " to avoid a blocking asset load and potential deadlock, since the DynamicDrawContext lambda" + " will be executed during scene processing and there may be multiple scenes executing in parallel."); + + Data::Instance shader = RPI::Shader::FindOrCreate(shaderAsset); + drawContext->InitShader(shader); + drawContext->InitVertexFormat( + { {"POSITION", RHI::Format::R32G32B32_FLOAT}, + {"COLOR", RHI::Format::R8G8B8A8_UNORM}, + {"TEXCOORD", RHI::Format::R32G32_FLOAT} }); + drawContext->EndInit(); + }); + + Data::AssetBus::Handler::BusDisconnect(); } } // namespace AZ::Render diff --git a/Gems/AtomLyIntegration/AtomViewportDisplayIcons/Code/Source/AtomViewportDisplayIconsSystemComponent.h b/Gems/AtomLyIntegration/AtomViewportDisplayIcons/Code/Source/AtomViewportDisplayIconsSystemComponent.h index 8cb268d36a..cd6fddf745 100644 --- a/Gems/AtomLyIntegration/AtomViewportDisplayIcons/Code/Source/AtomViewportDisplayIconsSystemComponent.h +++ b/Gems/AtomLyIntegration/AtomViewportDisplayIcons/Code/Source/AtomViewportDisplayIconsSystemComponent.h @@ -27,6 +27,7 @@ namespace AZ : public AZ::Component , public AzToolsFramework::EditorViewportIconDisplayInterface , public AZ::Render::Bootstrap::NotificationBus::Handler + , private Data::AssetBus::Handler { public: AZ_COMPONENT(AtomViewportDisplayIconsSystemComponent, "{AEC1D3E1-1D9A-437A-B4C6-CFAEE620C160}"); @@ -51,6 +52,9 @@ namespace AZ // AZ::Render::Bootstrap::NotificationBus::Handler overrides... void OnBootstrapSceneReady(AZ::RPI::Scene* bootstrapScene) override; + // Data::AssetBus::Handler overrides... + void OnAssetReady(Data::Asset asset) override; + private: static constexpr const char* DrawContextShaderPath = "Shaders/TexturedIcon.azshader"; static constexpr QSize MinimumRenderedSvgSize = QSize(128, 128); diff --git a/Gems/Blast/Code/Include/Blast/BlastDebug.h b/Gems/Blast/Code/Include/Blast/BlastDebug.h index 2530eb4217..9613b94ff5 100644 --- a/Gems/Blast/Code/Include/Blast/BlastDebug.h +++ b/Gems/Blast/Code/Include/Blast/BlastDebug.h @@ -8,7 +8,7 @@ #include #include -#include +#include namespace Blast { diff --git a/Gems/Blast/Code/Source/Family/ActorRenderManager.h b/Gems/Blast/Code/Source/Family/ActorRenderManager.h index c4037bb638..c0cdda158c 100644 --- a/Gems/Blast/Code/Source/Family/ActorRenderManager.h +++ b/Gems/Blast/Code/Source/Family/ActorRenderManager.h @@ -8,7 +8,7 @@ #include #include -#include +#include namespace Blast {