From 5c603882b348bdf220eda73e90c2491217744079 Mon Sep 17 00:00:00 2001 From: dmcdiarmid-ly <63674186+dmcdiarmid-ly@users.noreply.github.com> Date: Thu, 13 Jan 2022 19:57:22 -0700 Subject: [PATCH 1/2] Changed the Shader instanceId to the combined assetId and supervariant index Signed-off-by: dmcdiarmid-ly <63674186+dmcdiarmid-ly@users.noreply.github.com> --- .../Include/Atom/RPI.Public/Shader/Shader.h | 5 --- .../Code/Source/RPI.Public/Shader/Shader.cpp | 44 ++++++++++--------- 2 files changed, 24 insertions(+), 25 deletions(-) diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Shader/Shader.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Shader/Shader.h index 92b67c3a7a..be20d89d65 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Shader/Shader.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Shader/Shader.h @@ -136,11 +136,6 @@ namespace AZ //! This tag corresponds to the ShaderAsset object's DrawListName. RHI::DrawListTag GetDrawListTag() const; - //! Changes the supervariant of the shader to the specified supervariantIndex. - //! [GFX TODO][ATOM-15813]: this can be removed when the shader InstanceDatabase can support multiple shader - //! instances with different supervariants. - void ChangeSupervariant(SupervariantIndex supervariantIndex); - private: explicit Shader(const SupervariantIndex& supervariantIndex) : m_supervariantIndex(supervariantIndex){}; Shader() = delete; diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Shader/Shader.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Shader/Shader.cpp index b1ae460af6..3830d8ee42 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Shader/Shader.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Shader/Shader.cpp @@ -25,21 +25,34 @@ namespace AZ Data::Instance Shader::FindOrCreate(const Data::Asset& shaderAsset, const Name& supervariantName) { auto anySupervariantName = AZStd::any(supervariantName); - Data::Instance shaderInstance = Data::InstanceDatabase::Instance().FindOrCreate( - Data::InstanceId::CreateFromAssetId(shaderAsset.GetId()), shaderAsset, &anySupervariantName); - if (shaderInstance) + // retrieve the supervariant index from the shader asset + SupervariantIndex supervariantIndex = shaderAsset->GetSupervariantIndex(supervariantName); + if (!supervariantIndex.IsValid()) { - // [GFX TODO][ATOM-15813] Change InstanceDatabase to support multiple instances with different supervariants. - // At this time we do not support multiple supervariants loaded for a shader asset simultaneously, so if this shader - // is referring to the wrong supervariant we need to change it to the correct one. - SupervariantIndex supervariantIndex = shaderAsset->GetSupervariantIndex(supervariantName); - if (supervariantIndex.IsValid() && shaderInstance->GetSupervariantIndex() != supervariantIndex) - { - shaderInstance->ChangeSupervariant(supervariantIndex); - } + AZ_Error("Shader", false, "Supervariant with name %s, was not found in shader %s", supervariantName.GetCStr(), shaderAsset->GetName().GetCStr()); + return nullptr; } + // create the InstanceId from the combined assetId and supervariantIndex + const Data::AssetId& assetId = shaderAsset.GetId(); + uint32_t shaderSupervariantIndex = supervariantIndex.GetIndex(); + + const uint32_t instanceIdDataSize = sizeof(assetId.m_guid) + sizeof(assetId.m_subId) + sizeof(shaderSupervariantIndex); + uint8_t instanceIdData[instanceIdDataSize]; + uint8_t* instanceIdDataPtr = instanceIdData; + + memcpy(instanceIdDataPtr, &assetId.m_guid, sizeof(assetId.m_guid)); + instanceIdDataPtr += sizeof(assetId.m_guid); + memcpy(instanceIdDataPtr, &assetId.m_subId, sizeof(assetId.m_subId)); + instanceIdDataPtr += sizeof(assetId.m_subId); + memcpy(instanceIdDataPtr, &shaderSupervariantIndex, sizeof(shaderSupervariantIndex)); + + Data::InstanceId instanceId = Data::InstanceId::CreateData(instanceIdData, instanceIdDataSize); + + // retrieve the shader instance from the Instance database + Data::Instance shaderInstance = Data::InstanceDatabase::Instance().FindOrCreate(instanceId, shaderAsset, &anySupervariantName); + return shaderInstance; } @@ -478,14 +491,5 @@ namespace AZ return m_drawListTag; } - void Shader::ChangeSupervariant(SupervariantIndex supervariantIndex) - { - if (supervariantIndex != m_supervariantIndex) - { - m_supervariantIndex = supervariantIndex; - Init(*m_asset); - } - } - } // namespace RPI } // namespace AZ From 1ef868437a67ec87057df8b87cedb16907df84ee Mon Sep 17 00:00:00 2001 From: dmcdiarmid-ly <63674186+dmcdiarmid-ly@users.noreply.github.com> Date: Fri, 14 Jan 2022 17:28:14 -0700 Subject: [PATCH 2/2] Added supervariantIndex check in Shader::OnShaderVariantAssetReady Signed-off-by: dmcdiarmid-ly <63674186+dmcdiarmid-ly@users.noreply.github.com> --- .../Include/Atom/RPI.Reflect/Shader/ShaderVariantAsset.h | 1 + Gems/Atom/RPI/Code/Source/RPI.Public/Shader/Shader.cpp | 6 ++++++ .../Code/Source/RPI.Reflect/Shader/ShaderVariantAsset.cpp | 5 +++++ 3 files changed, 12 insertions(+) diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Shader/ShaderVariantAsset.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Shader/ShaderVariantAsset.h index 5146ae370a..b65d9837f7 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Shader/ShaderVariantAsset.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Shader/ShaderVariantAsset.h @@ -50,6 +50,7 @@ namespace AZ RPI::ShaderVariantStableId GetStableId() const { return m_stableId; } const ShaderVariantId& GetShaderVariantId() const { return m_shaderVariantId; } + uint32_t GetSupervariantIndex() const; //! Returns the shader stage function associated with the provided stage enum value. const RHI::ShaderStageFunction* GetShaderStageFunction(RHI::ShaderStage shaderStage) const; diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Shader/Shader.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Shader/Shader.cpp index 3830d8ee42..ed9d3430d9 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Shader/Shader.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Shader/Shader.cpp @@ -253,6 +253,12 @@ namespace AZ AZ_Assert(shaderVariantAsset, "Reloaded ShaderVariantAsset is null"); const ShaderVariantStableId stableId = shaderVariantAsset->GetStableId(); + // check the supervariantIndex of the ShaderVariantAsset to make sure it matches the supervariantIndex of this shader instance + if (shaderVariantAsset->GetSupervariantIndex() != m_supervariantIndex.GetIndex()) + { + return; + } + // We make a copy of the updated variant because OnShaderVariantReinitialized must not be called inside // m_variantCacheMutex or deadlocks may occur. // Or if there is an error, we leave this object in its default state to indicate there was an error. diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Shader/ShaderVariantAsset.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Shader/ShaderVariantAsset.cpp index ec1a65a6d5..f9f046ac8a 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Shader/ShaderVariantAsset.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Shader/ShaderVariantAsset.cpp @@ -65,6 +65,11 @@ namespace AZ return m_buildTimestamp; } + uint32_t ShaderVariantAsset::GetSupervariantIndex() const + { + return (m_assetId.m_subId >> SupervariantIndexBitPosition) & SupervariantIndexMaxValue; + } + const RHI::ShaderStageFunction* ShaderVariantAsset::GetShaderStageFunction(RHI::ShaderStage shaderStage) const { return m_functionsByStage[static_cast(shaderStage)].get();