From a1f22f68238aae636b9a0d0ec3def89b3440e4d4 Mon Sep 17 00:00:00 2001 From: mrieggeramzn Date: Thu, 30 Sep 2021 10:19:26 -0700 Subject: [PATCH 1/7] Adding some code to cache the material in the editor component to avoid flickering Signed-off-by: mrieggeramzn --- .../Code/Source/Decals/EditorDecalComponent.cpp | 16 ++++++++++++++++ .../Code/Source/Decals/EditorDecalComponent.h | 6 ++++++ 2 files changed, 22 insertions(+) diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.cpp index c5d61dd54a..02a2e1c0c9 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.cpp @@ -15,6 +15,11 @@ namespace AZ { namespace Render { + static AZ::Data::Asset QueueLoad(const AZ::Data::AssetId id) + { + return AZ::Data::AssetManager::Instance().GetAsset(id, AZ::Data::AssetLoadBehavior::QueueLoad); + } + EditorDecalComponent::EditorDecalComponent(const DecalComponentConfig& config) : BaseClass(config) { @@ -87,16 +92,26 @@ namespace AZ AzFramework::EntityDebugDisplayEventBus::Handler::BusConnect(GetEntityId()); AzToolsFramework::EditorComponentSelectionRequestsBus::Handler::BusConnect(GetEntityId()); AzFramework::BoundsRequestBus::Handler::BusConnect(GetEntityId()); + CacheMaterial(); } void EditorDecalComponent::Deactivate() { + m_cachedMaterial = {}; AzFramework::BoundsRequestBus::Handler::BusDisconnect(); AzToolsFramework::EditorComponentSelectionRequestsBus::Handler::BusDisconnect(); AzFramework::EntityDebugDisplayEventBus::Handler::BusDisconnect(); BaseClass::Deactivate(); } + void EditorDecalComponent::CacheMaterial() + { + DecalComponentConfig decalComponentConfig; + GetConfiguration(decalComponentConfig); + const auto& materialAsset = decalComponentConfig.m_materialAsset; + m_cachedMaterial = QueueLoad(materialAsset.GetId()); + } + AZ::Transform EditorDecalComponent::GetWorldTransform() const { AZ::Transform transform = AZ::Transform::CreateIdentity(); @@ -193,6 +208,7 @@ namespace AZ u32 EditorDecalComponent::OnConfigurationChanged() { BaseClass::OnConfigurationChanged(); + CacheMaterial(); return Edit::PropertyRefreshLevels::AttributesAndValues; } diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.h b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.h index 1ca88d23fc..9878e72da0 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.h +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.h @@ -64,6 +64,12 @@ namespace AZ //! EditorRenderComponentAdapter overrides ... u32 OnConfigurationChanged() override; + + // Hold onto the material for the lifespan of the EditorDecalComponent to smooth out performance. This is so we can avoid + // duplicate loads as the DecalTextureArrayFeatureProcessor will unload the materials after texture packing. + void CacheMaterial(); + + AZ::Data::Asset m_cachedMaterial; }; } // namespace Render } // namespace AZ From 25a7b70440d79dae0f379bc3d859676729b822c5 Mon Sep 17 00:00:00 2001 From: mrieggeramzn Date: Thu, 30 Sep 2021 10:36:09 -0700 Subject: [PATCH 2/7] Getting rid of some tabs Signed-off-by: mrieggeramzn --- .../CommonFeatures/Code/Source/Decals/EditorDecalComponent.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.h b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.h index 9878e72da0..1a3d8f3844 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.h +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.h @@ -65,7 +65,7 @@ namespace AZ //! EditorRenderComponentAdapter overrides ... u32 OnConfigurationChanged() override; - // Hold onto the material for the lifespan of the EditorDecalComponent to smooth out performance. This is so we can avoid + // Hold onto the material for the lifespan of the EditorDecalComponent to smooth out performance. This is so we can avoid // duplicate loads as the DecalTextureArrayFeatureProcessor will unload the materials after texture packing. void CacheMaterial(); From d9a3048a6d8d33093934721f65f0896e88889e20 Mon Sep 17 00:00:00 2001 From: mrieggeramzn Date: Thu, 30 Sep 2021 11:04:02 -0700 Subject: [PATCH 3/7] Adding comment Signed-off-by: mrieggeramzn --- .../Common/Assets/ShaderLib/Atom/Features/PBR/Decals.azsli | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Gems/Atom/Feature/Common/Assets/ShaderLib/Atom/Features/PBR/Decals.azsli b/Gems/Atom/Feature/Common/Assets/ShaderLib/Atom/Features/PBR/Decals.azsli index 1bd8e0b9a5..1d7a2d6dd0 100644 --- a/Gems/Atom/Feature/Common/Assets/ShaderLib/Atom/Features/PBR/Decals.azsli +++ b/Gems/Atom/Feature/Common/Assets/ShaderLib/Atom/Features/PBR/Decals.azsli @@ -67,6 +67,9 @@ void ApplyDecal(uint currDecalIndex, inout Surface surface) float3 decalSample; float4 baseMap = 0; + + // Each texture array handles a size permutation. + // e.g. it could be that tex array 0 handles 256x256 and tex array 1 handles 512x64, etc. switch(textureArrayIndex) { case 0: From 65e375bb422017a01fb5d94c883ee493ad917ded Mon Sep 17 00:00:00 2001 From: mrieggeramzn Date: Thu, 30 Sep 2021 13:44:06 -0700 Subject: [PATCH 4/7] Guthries excellent recommendation Signed-off-by: mrieggeramzn --- .../DecalTextureArrayFeatureProcessor.cpp | 4 +++- .../Source/Decals/EditorDecalComponent.cpp | 18 +----------------- .../Code/Source/Decals/EditorDecalComponent.h | 6 ------ 3 files changed, 4 insertions(+), 24 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp index c0b8f3315a..3975df52b9 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp @@ -303,7 +303,9 @@ namespace AZ if (material.IsValid()) { - AZ_Assert(m_decalData.GetData(handle.GetIndex()).m_textureArrayIndex == DecalData::UnusedIndex, "Setting Material on a decal more than once is not currently supported."); + AZ_Assert( + m_decalData.GetData(handle.GetIndex()).m_textureArrayIndex == DecalData::UnusedIndex || GetMaterialUsedByDecal(handle) == material, + "Setting Material on a decal more than once is not currently supported."); const auto iter = m_materialToTextureArrayLookupTable.find(material); if (iter != m_materialToTextureArrayLookupTable.end()) diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.cpp index 02a2e1c0c9..f8865e8d34 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.cpp @@ -15,11 +15,6 @@ namespace AZ { namespace Render { - static AZ::Data::Asset QueueLoad(const AZ::Data::AssetId id) - { - return AZ::Data::AssetManager::Instance().GetAsset(id, AZ::Data::AssetLoadBehavior::QueueLoad); - } - EditorDecalComponent::EditorDecalComponent(const DecalComponentConfig& config) : BaseClass(config) { @@ -92,26 +87,16 @@ namespace AZ AzFramework::EntityDebugDisplayEventBus::Handler::BusConnect(GetEntityId()); AzToolsFramework::EditorComponentSelectionRequestsBus::Handler::BusConnect(GetEntityId()); AzFramework::BoundsRequestBus::Handler::BusConnect(GetEntityId()); - CacheMaterial(); } void EditorDecalComponent::Deactivate() { - m_cachedMaterial = {}; AzFramework::BoundsRequestBus::Handler::BusDisconnect(); AzToolsFramework::EditorComponentSelectionRequestsBus::Handler::BusDisconnect(); AzFramework::EntityDebugDisplayEventBus::Handler::BusDisconnect(); BaseClass::Deactivate(); } - void EditorDecalComponent::CacheMaterial() - { - DecalComponentConfig decalComponentConfig; - GetConfiguration(decalComponentConfig); - const auto& materialAsset = decalComponentConfig.m_materialAsset; - m_cachedMaterial = QueueLoad(materialAsset.GetId()); - } - AZ::Transform EditorDecalComponent::GetWorldTransform() const { AZ::Transform transform = AZ::Transform::CreateIdentity(); @@ -207,8 +192,7 @@ namespace AZ u32 EditorDecalComponent::OnConfigurationChanged() { - BaseClass::OnConfigurationChanged(); - CacheMaterial(); + m_controller.ConfigurationChanged(); return Edit::PropertyRefreshLevels::AttributesAndValues; } diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.h b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.h index 1a3d8f3844..1ca88d23fc 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.h +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.h @@ -64,12 +64,6 @@ namespace AZ //! EditorRenderComponentAdapter overrides ... u32 OnConfigurationChanged() override; - - // Hold onto the material for the lifespan of the EditorDecalComponent to smooth out performance. This is so we can avoid - // duplicate loads as the DecalTextureArrayFeatureProcessor will unload the materials after texture packing. - void CacheMaterial(); - - AZ::Data::Asset m_cachedMaterial; }; } // namespace Render } // namespace AZ From b3d7de1e78c447d44f3cb3702a1b084e6c319aaf Mon Sep 17 00:00:00 2001 From: mrieggeramzn Date: Fri, 1 Oct 2021 13:03:52 -0700 Subject: [PATCH 5/7] Improvements. You can now call SetDecalMaterial() with any permutation and it works as expected Signed-off-by: mrieggeramzn --- .../ShaderLib/Atom/Features/PBR/Decals.azsli | 2 + .../DecalTextureArrayFeatureProcessor.cpp | 45 ++++++++++++++----- .../DecalTextureArrayFeatureProcessor.h | 1 + .../Decals/DecalComponentController.cpp | 2 +- .../Source/Decals/EditorDecalComponent.cpp | 2 +- 5 files changed, 38 insertions(+), 14 deletions(-) diff --git a/Gems/Atom/Feature/Common/Assets/ShaderLib/Atom/Features/PBR/Decals.azsli b/Gems/Atom/Feature/Common/Assets/ShaderLib/Atom/Features/PBR/Decals.azsli index 5e7088617b..f9ead72ce4 100644 --- a/Gems/Atom/Feature/Common/Assets/ShaderLib/Atom/Features/PBR/Decals.azsli +++ b/Gems/Atom/Feature/Common/Assets/ShaderLib/Atom/Features/PBR/Decals.azsli @@ -70,6 +70,8 @@ void ApplyDecal(uint currDecalIndex, inout Surface surface) float4 baseMap = 0; float2 normalMap = 0; + // Each texture array handles a size permutation. + // e.g. it could be that tex array 0 handles 256x256 and tex array 1 handles 512x64, etc. switch(textureArrayIndex) { case 0: diff --git a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp index e5bf0bd9fa..0323282f5c 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp @@ -301,23 +301,44 @@ namespace AZ return; } - if (material.IsValid()) + if (GetMaterialUsedByDecal(handle) == material) + return; + + const auto decalIndex = handle.GetIndex(); + + const bool isValidMaterialBeingUsedCurrently = m_decalData.GetData(decalIndex).m_textureArrayIndex != DecalData::UnusedIndex; + if (isValidMaterialBeingUsedCurrently) { - AZ_Assert(m_decalData.GetData(handle.GetIndex()).m_textureArrayIndex == DecalData::UnusedIndex, "Setting Material on a decal more than once is not currently supported."); + RemoveMaterialFromDecal(decalIndex); + } - const auto iter = m_materialToTextureArrayLookupTable.find(material); - if (iter != m_materialToTextureArrayLookupTable.end()) - { - // This material is already loaded and registered with this feature processor - iter->second.m_useCount++; - SetDecalTextureLocation(handle, iter->second.m_location); - return; - } + if (!material.IsValid()) + return; - // Material not loaded so queue it up for loading. - QueueMaterialLoadForDecal(material, handle); + const auto iter = m_materialToTextureArrayLookupTable.find(material); + if (iter != m_materialToTextureArrayLookupTable.end()) + { + // This material is already loaded and registered with this feature processor + iter->second.m_useCount++; + SetDecalTextureLocation(handle, iter->second.m_location); return; } + + // Material not loaded so queue it up for loading. + QueueMaterialLoadForDecal(material, handle); + } + + void DecalTextureArrayFeatureProcessor::RemoveMaterialFromDecal(const uint16_t decalIndex) + { + DecalLocation decalLocation; + decalLocation.textureArrayIndex = m_decalData.GetData(decalIndex).m_textureArrayIndex; + decalLocation.textureIndex = m_decalData.GetData(decalIndex).m_textureIndex; + RemoveDecalFromTextureArrays(decalLocation); + + m_decalData.GetData(decalIndex).m_textureArrayIndex = DecalData::UnusedIndex; + m_decalData.GetData(decalIndex).m_textureIndex = DecalData::UnusedIndex; + + m_deviceBufferNeedsUpdate = true; } void DecalTextureArrayFeatureProcessor::CacheShaderIndices() diff --git a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.h b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.h index 13a6682629..fd535bbe64 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.h +++ b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.h @@ -114,6 +114,7 @@ namespace AZ AZStd::optional AddMaterialToTextureArrays(const AZ::RPI::MaterialAsset* materialAsset); int FindTextureArrayWithSize(const RHI::Size& size) const; + void RemoveMaterialFromDecal(const uint16_t decalIndex); void SetDecalTextureLocation(const DecalHandle& handle, const DecalLocation location); void QueueMaterialLoadForDecal(const AZ::Data::AssetId material, const DecalHandle handle); bool RemoveDecalFromTextureArrays(const DecalLocation decalLocation); diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/DecalComponentController.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/DecalComponentController.cpp index 5d16164d92..dd8266c1ca 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/DecalComponentController.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/DecalComponentController.cpp @@ -230,7 +230,7 @@ namespace AZ { DecalNotificationBus::Event(m_entityId, &DecalNotifications::OnMaterialChanged, m_configuration.m_materialAsset); - if (m_featureProcessor && m_configuration.m_materialAsset.GetId().IsValid()) + if (m_featureProcessor) { m_featureProcessor->SetDecalMaterial(m_handle, m_configuration.m_materialAsset.GetId()); } diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.cpp index c5d61dd54a..f8865e8d34 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Decals/EditorDecalComponent.cpp @@ -192,7 +192,7 @@ namespace AZ u32 EditorDecalComponent::OnConfigurationChanged() { - BaseClass::OnConfigurationChanged(); + m_controller.ConfigurationChanged(); return Edit::PropertyRefreshLevels::AttributesAndValues; } From 76de21940ddd02eb08bee151b3b8b852e5b69abc Mon Sep 17 00:00:00 2001 From: mrieggeramzn Date: Fri, 1 Oct 2021 13:29:52 -0700 Subject: [PATCH 6/7] Fixup merge Signed-off-by: mrieggeramzn --- .../Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp index 46b00ef47f..0323282f5c 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp @@ -309,8 +309,8 @@ namespace AZ const bool isValidMaterialBeingUsedCurrently = m_decalData.GetData(decalIndex).m_textureArrayIndex != DecalData::UnusedIndex; if (isValidMaterialBeingUsedCurrently) { - AZ_Assert( - AZ_Assert(m_decalData.GetData(handle.GetIndex()).m_textureArrayIndex == DecalData::UnusedIndex, "Setting Material on a decal more than once is not currently supported."); + RemoveMaterialFromDecal(decalIndex); + } if (!material.IsValid()) return; From ba23583d5f464faa22ee385a670563fda609d6a7 Mon Sep 17 00:00:00 2001 From: mrieggeramzn Date: Fri, 1 Oct 2021 15:54:49 -0700 Subject: [PATCH 7/7] Adding gadams feedback Signed-off-by: mrieggeramzn --- .../Decals/DecalTextureArrayFeatureProcessor.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp index 0323282f5c..472dbff5c8 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp @@ -302,7 +302,9 @@ namespace AZ } if (GetMaterialUsedByDecal(handle) == material) + { return; + } const auto decalIndex = handle.GetIndex(); @@ -313,7 +315,9 @@ namespace AZ } if (!material.IsValid()) + { return; + } const auto iter = m_materialToTextureArrayLookupTable.find(material); if (iter != m_materialToTextureArrayLookupTable.end()) @@ -330,13 +334,15 @@ namespace AZ void DecalTextureArrayFeatureProcessor::RemoveMaterialFromDecal(const uint16_t decalIndex) { + auto& decalData = m_decalData.GetData(decalIndex); + DecalLocation decalLocation; - decalLocation.textureArrayIndex = m_decalData.GetData(decalIndex).m_textureArrayIndex; - decalLocation.textureIndex = m_decalData.GetData(decalIndex).m_textureIndex; + decalLocation.textureArrayIndex = decalData.m_textureArrayIndex; + decalLocation.textureIndex = decalData.m_textureIndex; RemoveDecalFromTextureArrays(decalLocation); - m_decalData.GetData(decalIndex).m_textureArrayIndex = DecalData::UnusedIndex; - m_decalData.GetData(decalIndex).m_textureIndex = DecalData::UnusedIndex; + decalData.m_textureArrayIndex = DecalData::UnusedIndex; + decalData.m_textureIndex = DecalData::UnusedIndex; m_deviceBufferNeedsUpdate = true; }