diff --git a/Code/Framework/AtomCore/AtomCore/Utils/ScopedValue.h b/Code/Framework/AtomCore/AtomCore/Utils/ScopedValue.h new file mode 100644 index 0000000000..f6ed6d6df4 --- /dev/null +++ b/Code/Framework/AtomCore/AtomCore/Utils/ScopedValue.h @@ -0,0 +1,36 @@ +/* + * Copyright (c) Contributors to the Open 3D Engine Project. + * For complete copyright and license terms please see the LICENSE at the root of this distribution. + * + * SPDX-License-Identifier: Apache-2.0 OR MIT + * + */ +#pragma once + +#include + +namespace AZ +{ + //! Sets a variable upon construction and again when the object goes out of scope. + template + class ScopedValue + { + private: + T* m_ptr; + T m_finalValue; + + public: + ScopedValue(T* ptr, T initialValue, T finalValue) : + m_ptr(ptr), m_finalValue(finalValue) + { + AZ_Assert(m_ptr, "ScopedValue::m_ptr is null"); + *m_ptr = initialValue; + } + + ~ScopedValue() + { + *m_ptr = m_finalValue; + } + }; + +} // namespace AZ diff --git a/Code/Framework/AtomCore/AtomCore/atomcore_files.cmake b/Code/Framework/AtomCore/AtomCore/atomcore_files.cmake index c90263468f..9167c1e645 100644 --- a/Code/Framework/AtomCore/AtomCore/atomcore_files.cmake +++ b/Code/Framework/AtomCore/AtomCore/atomcore_files.cmake @@ -19,4 +19,5 @@ set(FILES std/containers/vector_set.h std/containers/vector_set_base.h std/parallel/concurrency_checker.h + Utils/ScopedValue.h ) diff --git a/Code/Framework/AtomCore/Tests/ScopedValueTest.cpp b/Code/Framework/AtomCore/Tests/ScopedValueTest.cpp new file mode 100644 index 0000000000..9578cb2329 --- /dev/null +++ b/Code/Framework/AtomCore/Tests/ScopedValueTest.cpp @@ -0,0 +1,37 @@ +/* + * Copyright (c) Contributors to the Open 3D Engine Project. + * For complete copyright and license terms please see the LICENSE at the root of this distribution. + * + * SPDX-License-Identifier: Apache-2.0 OR MIT + * + */ + +#include +#include + +namespace UnitTest +{ + TEST(ScopedValueTest, TestBoolValue) + { + bool localValue = false; + + { + AZ::ScopedValue scopedValue(&localValue, true, false); + EXPECT_EQ(true, localValue); + } + + EXPECT_EQ(false, localValue); + } + + TEST(ScopedValueTest, TestIntValue) + { + int localValue = 0; + + { + AZ::ScopedValue scopedValue(&localValue, 1, 2); + EXPECT_EQ(1, localValue); + } + + EXPECT_EQ(2, localValue); + } +} diff --git a/Code/Framework/AtomCore/Tests/atomcore_tests_files.cmake b/Code/Framework/AtomCore/Tests/atomcore_tests_files.cmake index 0f5fcb441d..4522a4b7b6 100644 --- a/Code/Framework/AtomCore/Tests/atomcore_tests_files.cmake +++ b/Code/Framework/AtomCore/Tests/atomcore_tests_files.cmake @@ -12,5 +12,6 @@ set(FILES InstanceDatabase.cpp lru_cache.cpp Main.cpp + ScopedValueTest.cpp vector_set.cpp ) diff --git a/Gems/Atom/Feature/Common/Assets/Scripts/material_find_overrides_demo.lua b/Gems/Atom/Feature/Common/Assets/Scripts/material_find_overrides_demo.lua index 41df35e355..4a1b8c8f06 100644 --- a/Gems/Atom/Feature/Common/Assets/Scripts/material_find_overrides_demo.lua +++ b/Gems/Atom/Feature/Common/Assets/Scripts/material_find_overrides_demo.lua @@ -22,6 +22,7 @@ local FindMaterialAssignmentTest = "materials/presets/macbeth/12_orange_yellow_srgb.tif.streamingimage", "materials/presets/macbeth/17_magenta_srgb.tif.streamingimage" }, + MaterialSlotFilter = "" }, } @@ -50,18 +51,6 @@ function FindMaterialAssignmentTest:OnActivate() self.colors = {} self.lerpDirs = {} - self.assignmentIds = - { - MaterialComponentRequestBus.Event.FindMaterialAssignmentId(self.entityId, -1, "lambert"), - } - - for index = 1, #self.assignmentIds do - local id = self.assignmentIds[index] - if (id ~= nil) then - self.colors[index] = randomColor() - self.lerpDirs[index] = randomDir() - end - end self.tickBusHandler = TickBus.Connect(self); end @@ -144,10 +133,33 @@ function FindMaterialAssignmentTest:lerpColors(deltaTime) end function FindMaterialAssignmentTest:OnTick(deltaTime, timePoint) + + + if(nil == self.assignmentIds) then + + local originalAssignments = MaterialComponentRequestBus.Event.GetOriginalMaterialAssignments(self.entityId) + if(nil == originalAssignments or #originalAssignments <= 1) then -- There is always 1 entry for the default assignment; a loaded model will have at least 2 assignments + return + end + + self.assignmentIds = + { + MaterialComponentRequestBus.Event.FindMaterialAssignmentId(self.entityId, -1, self.Properties.MaterialSlotFilter), + } + + for index = 1, #self.assignmentIds do + local id = self.assignmentIds[index] + if (id ~= nil) then + self.colors[index] = randomColor() + self.lerpDirs[index] = randomDir() + end + end + end + self.timer = self.timer + deltaTime self.totalTime = self.totalTime + deltaTime self:lerpColors(deltaTime) - + if (self.timer > self.timeUpdate and self.totalTime < self.totalTimeMax) then self.timer = self.timer - self.timeUpdate self:UpdateProperties() @@ -155,6 +167,7 @@ function FindMaterialAssignmentTest:OnTick(deltaTime, timePoint) self:ClearProperties() self.tickBusHandler:Disconnect(self); end + end -return FindMaterialAssignmentTest \ No newline at end of file +return FindMaterialAssignmentTest diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Material/Material.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Material/Material.h index 59c5d571fc..3976ef989b 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Material/Material.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Material/Material.h @@ -74,6 +74,7 @@ namespace AZ MaterialPropertyIndex FindPropertyIndex(const Name& name) const; //! Sets the value of a material property. The template data type must match the property's data type. + //! @return true if property value was changed template bool SetPropertyValue(MaterialPropertyIndex index, const Type& value); @@ -81,12 +82,15 @@ namespace AZ template const Type& GetPropertyValue(MaterialPropertyIndex index) const; - //! Gets flags indicating which properties have been modified. - const MaterialPropertyFlags& GetPropertyDirtyFlags() const; - + //! Sets the value of a material property. The @value data type must match the property's data type. + //! @return true if property value was changed bool SetPropertyValue(MaterialPropertyIndex index, const MaterialPropertyValue& value); + const MaterialPropertyValue& GetPropertyValue(MaterialPropertyIndex index) const; const AZStd::vector& GetPropertyValues() const; + + //! Gets flags indicating which properties have been modified. + const MaterialPropertyFlags& GetPropertyDirtyFlags() const; //! Gets the material properties layout. RHI::ConstPtr GetMaterialPropertiesLayout() const; @@ -111,6 +115,12 @@ namespace AZ //! @param return the number of shader options that were updated, or Failure if the material owns the indicated shader option. AZ::Outcome SetSystemShaderOption(const Name& shaderOptionName, RPI::ShaderOptionValue value); + //! Override the material's default PSO handling setting. + //! This is normally used in tools like Asset Processor or Material Editor to allow changes that impact + //! Pipeline State Objects which is not allowed at runtime. See MaterialPropertyPsoHandling for more details. + //! Do not set this in the shipping runtime unless you know what you are doing. + void SetPsoHandlingOverride(MaterialPropertyPsoHandling psoHandlingOverride); + const RHI::ShaderResourceGroup* GetRHIShaderResourceGroup() const; const Data::Asset& GetAsset() const; @@ -189,6 +199,10 @@ namespace AZ //! Records the m_currentChangeId when the material was last compiled. ChangeId m_compiledChangeId = DEFAULT_CHANGE_ID; + + bool m_isInitializing = false; + + MaterialPropertyPsoHandling m_psoHandling = MaterialPropertyPsoHandling::Warning; }; } // namespace RPI diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/LuaMaterialFunctor.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/LuaMaterialFunctor.h index 026e9f7f18..5f4dce40e5 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/LuaMaterialFunctor.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/LuaMaterialFunctor.h @@ -83,14 +83,19 @@ namespace AZ AZ_TYPE_INFO(AZ::RPI::LuaMaterialFunctorCommonContext, "{2CCCB9A9-AD4F-447C-B587-E7A91CEA8088}"); explicit LuaMaterialFunctorCommonContext(MaterialFunctor::RuntimeContext* runtimeContextImpl, + const MaterialPropertyFlags* materialPropertyDependencies, const AZStd::string& propertyNamePrefix, const AZStd::string& srgNamePrefix, const AZStd::string& optionsNamePrefix); explicit LuaMaterialFunctorCommonContext(MaterialFunctor::EditorContext* editorContextImpl, + const MaterialPropertyFlags* materialPropertyDependencies, const AZStd::string& propertyNamePrefix, const AZStd::string& srgNamePrefix, const AZStd::string& optionsNamePrefix); + + //! Returns false if PSO changes are not allowed, and may report errors or warnings + bool CheckPsoChangesAllowed(); protected: @@ -100,6 +105,12 @@ namespace AZ MaterialPropertyIndex GetMaterialPropertyIndex(const char* name, const char* functionName) const; const MaterialPropertyValue& GetMaterialPropertyValue(MaterialPropertyIndex propertyIndex) const; + + MaterialPropertyPsoHandling GetMaterialPropertyPsoHandling() const; + + RHI::ConstPtr GetMaterialPropertiesLayout() const; + + AZStd::string GetMaterialPropertyDependenciesString() const; // These are prefix strings that will be applied to every name lookup in the lua functor. // This allows the lua script to be reused in different contexts. @@ -112,6 +123,8 @@ namespace AZ // Only one of these will be valid MaterialFunctor::RuntimeContext* m_runtimeContextImpl = nullptr; MaterialFunctor::EditorContext* m_editorContextImpl = nullptr; + const MaterialPropertyFlags* m_materialPropertyDependencies = nullptr; + bool m_psoChangesReported = false; //!< errors/warnings about PSO changes will only be reported once per execution of the functor }; //! Wraps RHI::RenderStates for LuaMaterialFunctor access @@ -241,7 +254,11 @@ namespace AZ static void Reflect(BehaviorContext* behaviorContext); - explicit LuaMaterialFunctorShaderItem(ShaderCollection::Item* shaderItem) : m_shaderItem(shaderItem) {} + LuaMaterialFunctorShaderItem() : + m_context(nullptr), m_shaderItem(nullptr) {} + + explicit LuaMaterialFunctorShaderItem(LuaMaterialFunctorCommonContext* context, ShaderCollection::Item* shaderItem) : + m_context(context), m_shaderItem(shaderItem) {} LuaMaterialFunctorRenderStates GetRenderStatesOverride(); void SetEnabled(bool enable); @@ -253,6 +270,7 @@ namespace AZ private: void SetShaderOptionValue(const Name& name, AZStd::function setValueCommand); + LuaMaterialFunctorCommonContext* m_context = nullptr; ShaderCollection::Item* m_shaderItem = nullptr; }; @@ -265,6 +283,7 @@ namespace AZ static void Reflect(BehaviorContext* behaviorContext); explicit LuaMaterialFunctorRuntimeContext(MaterialFunctor::RuntimeContext* runtimeContextImpl, + const MaterialPropertyFlags* materialPropertyDependencies, const AZStd::string& propertyNamePrefix, const AZStd::string& srgNamePrefix, const AZStd::string& optionsNamePrefix); @@ -304,6 +323,7 @@ namespace AZ static void Reflect(BehaviorContext* behaviorContext); explicit LuaMaterialFunctorEditorContext(MaterialFunctor::EditorContext* editorContextImpl, + const MaterialPropertyFlags* materialPropertyDependencies, const AZStd::string& propertyNamePrefix, const AZStd::string& srgNamePrefix, const AZStd::string& optionsNamePrefix); diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialFunctor.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialFunctor.h index 682183f70c..09d10bf538 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialFunctor.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialFunctor.h @@ -28,6 +28,24 @@ namespace AZ class MaterialPropertiesLayout; using MaterialPropertyFlags = AZStd::bitset; + + //! Indicates how the material system should respond to any material property changes that + //! impact Pipeline State Object configuration. This is significant because some platforms + //! require that PSOs be pre-compiled and shipped with the game. + enum class MaterialPropertyPsoHandling + { + //! PSO-impacting property changes are not allowed, are ignored, and will report an error. + //! This should be used at runtime. It is recommended to do this on all platforms, not just the restricted ones, + //! to encourage best-practices. However, if a game project is not shipping on any restricted platforms, + //! then the team could decide to allow PSO changes. + Error, + + //! PSO-impacting property changes are allowed, but produce a warning message. + Warning, + + //! PSO-impacting property changes are allowed. This can be used during asset processing, in developer tools, or on platforms that don't restrict PSO changes. + Allowed + }; //! MaterialFunctor objects provide custom logic and calculations to configure shaders, render states, //! editor metadata, and more. @@ -81,6 +99,8 @@ namespace AZ const MaterialPropertyValue& GetMaterialPropertyValue(const MaterialPropertyIndex& index) const; const MaterialPropertiesLayout* GetMaterialPropertiesLayout() const { return m_materialPropertiesLayout.get(); } + + MaterialPropertyPsoHandling GetMaterialPropertyPsoHandling() const { return m_psoHandling; } //! Set the value of a shader option //! @param shaderIndex the index of a shader in the material's ShaderCollection @@ -126,16 +146,18 @@ namespace AZ RHI::ConstPtr materialPropertiesLayout, ShaderCollection* shaderCollection, ShaderResourceGroup* shaderResourceGroup, - const MaterialPropertyFlags* materialPropertyDependencies + const MaterialPropertyFlags* materialPropertyDependencies, + MaterialPropertyPsoHandling psoHandling ); private: bool SetShaderOptionValue(ShaderCollection::Item& shaderItem, ShaderOptionIndex optionIndex, ShaderOptionValue value); const AZStd::vector& m_materialPropertyValues; RHI::ConstPtr m_materialPropertiesLayout; - ShaderCollection* m_shaderCollection; - ShaderResourceGroup* m_shaderResourceGroup; + ShaderCollection* m_shaderCollection; + ShaderResourceGroup* m_shaderResourceGroup; const MaterialPropertyFlags* m_materialPropertyDependencies = nullptr; + MaterialPropertyPsoHandling m_psoHandling = MaterialPropertyPsoHandling::Error; }; class EditorContext @@ -144,7 +166,7 @@ namespace AZ public: const MaterialPropertyDynamicMetadata* GetMaterialPropertyMetadata(const Name& propertyName) const; const MaterialPropertyDynamicMetadata* GetMaterialPropertyMetadata(const MaterialPropertyIndex& index) const; - + const MaterialPropertyGroupDynamicMetadata* GetMaterialPropertyGroupMetadata(const Name& propertyName) const; //! Get the property value. The type must be one of those in MaterialPropertyValue. @@ -158,6 +180,8 @@ namespace AZ const MaterialPropertyValue& GetMaterialPropertyValue(const MaterialPropertyIndex& index) const; const MaterialPropertiesLayout* GetMaterialPropertiesLayout() const { return m_materialPropertiesLayout.get(); } + + MaterialPropertyPsoHandling GetMaterialPropertyPsoHandling() const { return MaterialPropertyPsoHandling::Allowed; } //! Set the visibility dynamic metadata of a material property. bool SetMaterialPropertyVisibility(const Name& propertyName, MaterialPropertyVisibility visibility); @@ -177,7 +201,7 @@ namespace AZ bool SetMaterialPropertySoftMaxValue(const Name& propertyName, const MaterialPropertyValue& max); bool SetMaterialPropertySoftMaxValue(const MaterialPropertyIndex& index, const MaterialPropertyValue& max); - + bool SetMaterialPropertyGroupVisibility(const Name& propertyGroupName, MaterialPropertyGroupVisibility visibility); // [GFX TODO][ATOM-4168] Replace the workaround for unlink-able RPI.Public classes in MaterialFunctor diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp index 3b82114a69..2dcaa70deb 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp @@ -20,6 +20,7 @@ #include #include +#include namespace AZ { @@ -58,6 +59,8 @@ namespace AZ { AZ_TRACE_METHOD(); + ScopedValue isInitializing(&m_isInitializing, true, false); + m_materialAsset = { &materialAsset, AZ::Data::AssetLoadBehavior::PreLoad }; // Cache off pointers to some key data structures from the material type... @@ -198,6 +201,11 @@ namespace AZ return AZ::Success(appliedCount); } + void Material::SetPsoHandlingOverride(MaterialPropertyPsoHandling psoHandlingOverride) + { + m_psoHandling = psoHandlingOverride; + } + const RHI::ShaderResourceGroup* Material::GetRHIShaderResourceGroup() const { return m_rhiShaderResourceGroup; @@ -310,6 +318,16 @@ namespace AZ if (NeedsCompile() && CanCompile()) { + // On some platforms, PipelineStateObjects must be pre-compiled and shipped with the game; they cannot be compiled at runtime. So at some + // point the material system needs to be smart about when it allows PSO changes and when it doesn't. There is a task scheduled to + // thoroughly address this in 2022, but for now we just report a warning to alert users who are using the engine in a way that might + // not be supported for much longer. PSO changes should only be allowed in developer tools (though we could also expose a way for users to + // enable dynamic PSO changes if their project only targets platforms that support this). + // PSO modifications are allowed during initialization because that's using the stored asset data, which the asset system can + // access to pre-compile the necessary PSOs. + MaterialPropertyPsoHandling psoHandling = m_isInitializing ? MaterialPropertyPsoHandling::Allowed : m_psoHandling; + + AZ_PROFILE_BEGIN(RPI, "Material::Compile() Processing Functors"); for (const Ptr& functor : m_materialAsset->GetMaterialFunctors()) { @@ -325,7 +343,8 @@ namespace AZ m_layout, &m_shaderCollection, m_shaderResourceGroup.get(), - &materialPropertyDependencies + &materialPropertyDependencies, + psoHandling ); @@ -484,6 +503,13 @@ namespace AZ } MaterialPropertyValue& savedPropertyValue = m_propertyValues[index.GetIndex()]; + + // If the property value didn't actually change, don't waste time running functors and compiling the changes + if (savedPropertyValue == value) + { + return false; + } + savedPropertyValue = value; m_propertyDirtyFlags.set(index.GetIndex()); m_propertyOverrideFlags.set(index.GetIndex()); diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/LuaMaterialFunctor.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/LuaMaterialFunctor.cpp index 77902093f0..9338d52cb2 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/LuaMaterialFunctor.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/LuaMaterialFunctor.cpp @@ -128,7 +128,7 @@ namespace AZ if (m_scriptStatus == ScriptStatus::Ready) { - LuaMaterialFunctorRuntimeContext luaContext{&context, m_propertyNamePrefix, m_srgNamePrefix, m_optionsNamePrefix}; + LuaMaterialFunctorRuntimeContext luaContext{&context, &GetMaterialPropertyDependencies(), m_propertyNamePrefix, m_srgNamePrefix, m_optionsNamePrefix}; AZ::ScriptDataContext call; if (m_scriptContext->Call("Process", call)) { @@ -146,7 +146,7 @@ namespace AZ if (m_scriptStatus == ScriptStatus::Ready) { - LuaMaterialFunctorEditorContext luaContext{&context, m_propertyNamePrefix, m_srgNamePrefix, m_optionsNamePrefix}; + LuaMaterialFunctorEditorContext luaContext{&context, &GetMaterialPropertyDependencies(), m_propertyNamePrefix, m_srgNamePrefix, m_optionsNamePrefix}; AZ::ScriptDataContext call; if (m_scriptContext->Call("ProcessEditor", call)) { @@ -157,10 +157,12 @@ namespace AZ } LuaMaterialFunctorCommonContext::LuaMaterialFunctorCommonContext(MaterialFunctor::RuntimeContext* runtimeContextImpl, + const MaterialPropertyFlags* materialPropertyDependencies, const AZStd::string& propertyNamePrefix, const AZStd::string& srgNamePrefix, const AZStd::string& optionsNamePrefix) : m_runtimeContextImpl(runtimeContextImpl) + , m_materialPropertyDependencies(materialPropertyDependencies) , m_propertyNamePrefix(propertyNamePrefix) , m_srgNamePrefix(srgNamePrefix) , m_optionsNamePrefix(optionsNamePrefix) @@ -168,34 +170,96 @@ namespace AZ } LuaMaterialFunctorCommonContext::LuaMaterialFunctorCommonContext(MaterialFunctor::EditorContext* editorContextImpl, + const MaterialPropertyFlags* materialPropertyDependencies, const AZStd::string& propertyNamePrefix, const AZStd::string& srgNamePrefix, const AZStd::string& optionsNamePrefix) : m_editorContextImpl(editorContextImpl) + , m_materialPropertyDependencies(materialPropertyDependencies) , m_propertyNamePrefix(propertyNamePrefix) , m_srgNamePrefix(srgNamePrefix) , m_optionsNamePrefix(optionsNamePrefix) { } + + MaterialPropertyPsoHandling LuaMaterialFunctorCommonContext::GetMaterialPropertyPsoHandling() const + { + if (m_runtimeContextImpl) + { + return m_runtimeContextImpl->GetMaterialPropertyPsoHandling(); + } + else + { + return m_editorContextImpl->GetMaterialPropertyPsoHandling(); + } + } + + RHI::ConstPtr LuaMaterialFunctorCommonContext::GetMaterialPropertiesLayout() const + { + if (m_runtimeContextImpl) + { + return m_runtimeContextImpl->GetMaterialPropertiesLayout(); + } + else + { + return m_editorContextImpl->GetMaterialPropertiesLayout(); + } + } + + AZStd::string LuaMaterialFunctorCommonContext::GetMaterialPropertyDependenciesString() const + { + AZStd::vector propertyList; + for (size_t i = 0; i < m_materialPropertyDependencies->size(); ++i) + { + if ((*m_materialPropertyDependencies)[i]) + { + propertyList.push_back(GetMaterialPropertiesLayout()->GetPropertyDescriptor(MaterialPropertyIndex{i})->GetName().GetStringView()); + } + } + + AZStd::string propertyListString; + AzFramework::StringFunc::Join(propertyListString, propertyList.begin(), propertyList.end(), ", "); + + return propertyListString; + } + + bool LuaMaterialFunctorCommonContext::CheckPsoChangesAllowed() + { + if (GetMaterialPropertyPsoHandling() == MaterialPropertyPsoHandling::Error) + { + if (!m_psoChangesReported) + { + LuaMaterialFunctorUtilities::Script_Error( + AZStd::string::format( + "The following material properties must not be changed at runtime because they impact Pipeline State Objects: %s", GetMaterialPropertyDependenciesString().c_str())); + + m_psoChangesReported = true; + } + + return false; + } + else if (GetMaterialPropertyPsoHandling() == MaterialPropertyPsoHandling::Warning) + { + if (!m_psoChangesReported) + { + LuaMaterialFunctorUtilities::Script_Warning( + AZStd::string::format( + "The following material properties should not be changed at runtime because they impact Pipeline State Objects: %s", GetMaterialPropertyDependenciesString().c_str())); + + m_psoChangesReported = true; + } + } + + return true; + } MaterialPropertyIndex LuaMaterialFunctorCommonContext::GetMaterialPropertyIndex(const char* name, const char* functionName) const { MaterialPropertyIndex propertyIndex; Name propertyFullName{m_propertyNamePrefix + name}; - - if (m_runtimeContextImpl) - { - propertyIndex = m_runtimeContextImpl->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyFullName); - } - else if (m_editorContextImpl) - { - propertyIndex = m_editorContextImpl->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyFullName); - } - else - { - AZ_Assert(false, "Context not initialized properly"); - } + + propertyIndex = GetMaterialPropertiesLayout()->FindPropertyIndex(propertyFullName); if (!propertyIndex.IsValid()) { @@ -297,10 +361,11 @@ namespace AZ } LuaMaterialFunctorRuntimeContext::LuaMaterialFunctorRuntimeContext(MaterialFunctor::RuntimeContext* runtimeContextImpl, + const MaterialPropertyFlags* materialPropertyDependencies, const AZStd::string& propertyNamePrefix, const AZStd::string& srgNamePrefix, const AZStd::string& optionsNamePrefix) - : LuaMaterialFunctorCommonContext(runtimeContextImpl, propertyNamePrefix, srgNamePrefix, optionsNamePrefix) + : LuaMaterialFunctorCommonContext(runtimeContextImpl, materialPropertyDependencies, propertyNamePrefix, srgNamePrefix, optionsNamePrefix) , m_runtimeContextImpl(runtimeContextImpl) { } @@ -331,7 +396,7 @@ namespace AZ if (!shaderItem.MaterialOwnsShaderOption(optionIndex)) { - LuaMaterialFunctorUtilities::Script_Error(AZStd::string::format("Shader option '%s' is not owned by this material.", fullOptionName.GetCStr()).c_str()); + LuaMaterialFunctorUtilities::Script_Error(AZStd::string::format("Shader option '%s' is not owned by this material.", fullOptionName.GetCStr())); break; } @@ -398,12 +463,12 @@ namespace AZ { if (index < GetShaderCount()) { - return LuaMaterialFunctorShaderItem{&(*m_runtimeContextImpl->m_shaderCollection)[index]}; + return LuaMaterialFunctorShaderItem{this, &(*m_runtimeContextImpl->m_shaderCollection)[index]}; } else { LuaMaterialFunctorUtilities::Script_Error(AZStd::string::format("GetShader(%zu) is invalid.", index)); - return LuaMaterialFunctorShaderItem{nullptr}; + return {}; } } @@ -412,13 +477,13 @@ namespace AZ const AZ::Name tag{shaderTag}; if (m_runtimeContextImpl->m_shaderCollection->HasShaderTag(tag)) { - return LuaMaterialFunctorShaderItem{&(*m_runtimeContextImpl->m_shaderCollection)[tag]}; + return LuaMaterialFunctorShaderItem{this, &(*m_runtimeContextImpl->m_shaderCollection)[tag]}; } else { LuaMaterialFunctorUtilities::Script_Error(AZStd::string::format( "GetShaderByTag('%s') is invalid: Could not find a shader with the tag '%s'.", tag.GetCStr(), tag.GetCStr())); - return LuaMaterialFunctorShaderItem{nullptr}; + return {}; } } @@ -459,10 +524,11 @@ namespace AZ } LuaMaterialFunctorEditorContext::LuaMaterialFunctorEditorContext(MaterialFunctor::EditorContext* editorContextImpl, + const MaterialPropertyFlags* materialPropertyDependencies, const AZStd::string& propertyNamePrefix, const AZStd::string& srgNamePrefix, const AZStd::string& optionsNamePrefix) - : LuaMaterialFunctorCommonContext(editorContextImpl, propertyNamePrefix, srgNamePrefix, optionsNamePrefix) + : LuaMaterialFunctorCommonContext(editorContextImpl, materialPropertyDependencies, propertyNamePrefix, srgNamePrefix, optionsNamePrefix) , m_editorContextImpl(editorContextImpl) { } @@ -595,7 +661,7 @@ namespace AZ LuaMaterialFunctorRenderStates LuaMaterialFunctorShaderItem::GetRenderStatesOverride() { - if (m_shaderItem) + if (m_context->CheckPsoChangesAllowed() && m_shaderItem) { return LuaMaterialFunctorRenderStates{m_shaderItem->GetRenderStatesOverlay()}; } @@ -638,8 +704,7 @@ namespace AZ { LuaMaterialFunctorUtilities::Script_Error( AZStd::string::format( - "Shader option '%s' is not owned by the shader '%s'.", name.GetCStr(), m_shaderItem->GetShaderTag().GetCStr()) - .c_str()); + "Shader option '%s' is not owned by the shader '%s'.", name.GetCStr(), m_shaderItem->GetShaderTag().GetCStr())); return; } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialFunctor.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialFunctor.cpp index d08fa3e58e..0f70d0af35 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialFunctor.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialFunctor.cpp @@ -35,13 +35,15 @@ namespace AZ RHI::ConstPtr materialPropertiesLayout, ShaderCollection* shaderCollection, ShaderResourceGroup* shaderResourceGroup, - const MaterialPropertyFlags* materialPropertyDependencies + const MaterialPropertyFlags* materialPropertyDependencies, + MaterialPropertyPsoHandling psoHandling ) : m_materialPropertyValues(propertyValues) , m_materialPropertiesLayout(materialPropertiesLayout) , m_shaderCollection(shaderCollection) , m_shaderResourceGroup(shaderResourceGroup) , m_materialPropertyDependencies(materialPropertyDependencies) + , m_psoHandling(psoHandling) {} bool MaterialFunctor::RuntimeContext::SetShaderOptionValue(ShaderCollection::Item& shaderItem, ShaderOptionIndex optionIndex, ShaderOptionValue value) diff --git a/Gems/Atom/RPI/Code/Tests/Material/LuaMaterialFunctorTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/LuaMaterialFunctorTests.cpp index 99efa38c3a..37d0930d97 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/LuaMaterialFunctorTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/LuaMaterialFunctorTests.cpp @@ -1039,6 +1039,42 @@ namespace UnitTest drawListTagRegistry->ReleaseTag(tag); } + + TEST_F(LuaMaterialFunctorTests, LuaMaterialFunctor_RuntimeContext_PsoChangesNotAllowed_Error) + { + using namespace AZ::RPI; + + const char* functorScript = + R"( + function GetMaterialPropertyDependencies() + return {"general.MyBool"} + end + + function GetShaderOptionDependencies() + return {} + end + + function Process(context) + local boolValue = context:GetMaterialPropertyValue_bool("general.MyBool") + if(boolValue) then + context:GetShader(0):GetRenderStatesOverride():SetFillMode(FillMode_Wireframe) + else + context:GetShader(0):GetRenderStatesOverride():ClearFillMode() + end + end + )"; + + TestMaterialData testData; + testData.Setup(MaterialPropertyDataType::Bool, "general.MyBool", functorScript); + + testData.GetMaterial()->SetPropertyValue(testData.GetMaterialPropertyIndex(), MaterialPropertyValue{true}); + + ErrorMessageFinder errorMessageFinder; + + errorMessageFinder.AddExpectedErrorMessage("not be changed at runtime because they impact Pipeline State Objects: general.MyBool"); + EXPECT_TRUE(testData.GetMaterial()->Compile()); + errorMessageFinder.CheckExpectedErrorsFound(); + } TEST_F(LuaMaterialFunctorTests, LuaMaterialFunctor_RuntimeContext_MultisampleCustomPositionCountIndex_Error) { @@ -1067,6 +1103,7 @@ namespace UnitTest TestMaterialData testData; testData.Setup(MaterialPropertyDataType::Bool, "general.MyBool", functorScript); + testData.GetMaterial()->SetPsoHandlingOverride(AZ::RPI::MaterialPropertyPsoHandling::Allowed); testData.GetMaterial()->SetPropertyValue(testData.GetMaterialPropertyIndex(), MaterialPropertyValue{true}); ErrorMessageFinder errorMessageFinder; @@ -1107,7 +1144,8 @@ namespace UnitTest errorMessageFinder.AddExpectedErrorMessage("ClearMultisampleCustomPosition(18,...) index is out of range. Must be less than 16."); testData.Setup(MaterialPropertyDataType::Bool, "general.MyBool", functorScript); errorMessageFinder.CheckExpectedErrorsFound(); - + + testData.GetMaterial()->SetPsoHandlingOverride(AZ::RPI::MaterialPropertyPsoHandling::Allowed); testData.GetMaterial()->SetPropertyValue(testData.GetMaterialPropertyIndex(), MaterialPropertyValue{true}); errorMessageFinder.AddExpectedErrorMessage("SetMultisampleCustomPosition(17,...) index is out of range. Must be less than 16."); @@ -1146,7 +1184,8 @@ namespace UnitTest errorMessageFinder.AddExpectedErrorMessage("ClearBlendEnabled(10,...) index is out of range. Must be less than 8."); testData.Setup(MaterialPropertyDataType::Bool, "general.MyBool", functorScript); errorMessageFinder.CheckExpectedErrorsFound(); - + + testData.GetMaterial()->SetPsoHandlingOverride(AZ::RPI::MaterialPropertyPsoHandling::Allowed); testData.GetMaterial()->SetPropertyValue(testData.GetMaterialPropertyIndex(), MaterialPropertyValue{true}); errorMessageFinder.AddExpectedErrorMessage("SetBlendEnabled(9,...) index is out of range. Must be less than 8."); diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialFunctorTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialFunctorTests.cpp index e1e8c91944..ff5d24ff9a 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialFunctorTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialFunctorTests.cpp @@ -168,7 +168,8 @@ namespace UnitTest materialTypeAsset->GetMaterialPropertiesLayout(), &shaderCollectionCopy, unusedSrg, - &testFunctorSetOptionA.GetMaterialPropertyDependencies() + &testFunctorSetOptionA.GetMaterialPropertyDependencies(), + AZ::RPI::MaterialPropertyPsoHandling::Allowed }; testFunctorSetOptionA.Process(runtimeContext); EXPECT_TRUE(testFunctorSetOptionA.GetProcessResult()); @@ -184,7 +185,8 @@ namespace UnitTest materialTypeAsset->GetMaterialPropertiesLayout(), &shaderCollectionCopy, unusedSrg, - &testFunctorSetOptionB.GetMaterialPropertyDependencies() + &testFunctorSetOptionB.GetMaterialPropertyDependencies(), + AZ::RPI::MaterialPropertyPsoHandling::Allowed }; testFunctorSetOptionB.Process(runtimeContext); EXPECT_TRUE(testFunctorSetOptionB.GetProcessResult()); @@ -201,7 +203,8 @@ namespace UnitTest materialTypeAsset->GetMaterialPropertiesLayout(), &shaderCollectionCopy, unusedSrg, - &testFunctorSetOptionC.GetMaterialPropertyDependencies() + &testFunctorSetOptionC.GetMaterialPropertyDependencies(), + AZ::RPI::MaterialPropertyPsoHandling::Allowed }; testFunctorSetOptionC.Process(runtimeContext); EXPECT_FALSE(testFunctorSetOptionC.GetProcessResult()); @@ -216,7 +219,8 @@ namespace UnitTest materialTypeAsset->GetMaterialPropertiesLayout(), &shaderCollectionCopy, unusedSrg, - &testFunctorSetOptionInvalid.GetMaterialPropertyDependencies() + &testFunctorSetOptionInvalid.GetMaterialPropertyDependencies(), + AZ::RPI::MaterialPropertyPsoHandling::Allowed }; testFunctorSetOptionInvalid.Process(runtimeContext); EXPECT_FALSE(testFunctorSetOptionInvalid.GetProcessResult()); diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index 11834beb73..d032f35e38 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -763,6 +763,10 @@ namespace MaterialEditor return false; } + // Pipeline State Object changes are always allowed in the material editor because it only runs on developer systems + // where such changes are supported at runtime. + m_materialInstance->SetPsoHandlingOverride(AZ::RPI::MaterialPropertyPsoHandling::Allowed); + // Populate the property map from a combination of source data and assets // Assets must still be used for now because they contain the final accumulated value after all other materials // in the hierarchy are applied diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Include/AtomLyIntegration/CommonFeatures/Material/MaterialComponentBus.h b/Gems/AtomLyIntegration/CommonFeatures/Code/Include/AtomLyIntegration/CommonFeatures/Material/MaterialComponentBus.h index 01c87fa2fb..1ed8469749 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Include/AtomLyIntegration/CommonFeatures/Material/MaterialComponentBus.h +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Include/AtomLyIntegration/CommonFeatures/Material/MaterialComponentBus.h @@ -105,8 +105,15 @@ namespace AZ : public ComponentBus { public: - virtual void OnMaterialsUpdated([[maybe_unused]] const MaterialAssignmentMap& materials) {} + + //! This message is sent every time a material property is updated. virtual void OnMaterialsEdited([[maybe_unused]] const MaterialAssignmentMap& materials) {} + + //! This message is sent when one or more material property changes have been applied, at most once per frame. + virtual void OnMaterialsUpdated([[maybe_unused]] const MaterialAssignmentMap& materials) {} + + //! This message is sent when the component has created the material instance to be used for rendering. + virtual void OnMaterialInstanceCreated([[maybe_unused]] const MaterialAssignment& materialAssignment) {} }; using MaterialComponentNotificationBus = EBus; diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponent.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponent.cpp index a4e058561e..4249ed0c3b 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponent.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponent.cpp @@ -257,6 +257,16 @@ namespace AZ &AzToolsFramework::ToolsApplicationEvents::InvalidatePropertyDisplay, AzToolsFramework::Refresh_AttributesAndValues); } + + void EditorMaterialComponent::OnMaterialInstanceCreated(const MaterialAssignment& materialAssignment) + { + // PSO-impacting property changes are allowed in the editor + // because the saved slice data can be analyzed to pre-compile the necessary PSOs. + if (materialAssignment.m_materialInstance) + { + materialAssignment.m_materialInstance->SetPsoHandlingOverride(AZ::RPI::MaterialPropertyPsoHandling::Allowed); + } + } void EditorMaterialComponent::UpdateConfiguration(const MaterialComponentConfig& config) { @@ -280,19 +290,19 @@ namespace AZ { continue; } + + MaterialAssignment& materialAssignment = config.m_materials[materialSlot->m_id]; // Only material slots with a valid asset IDs or property overrides will be copied // to minimize the amount of data stored in the controller and game component if (materialSlot->m_materialAsset.GetId().IsValid()) { - MaterialAssignment& materialAssignment = config.m_materials[materialSlot->m_id]; materialAssignment.m_materialAsset = materialSlot->m_materialAsset; materialAssignment.m_propertyOverrides = materialSlot->m_propertyOverrides; materialAssignment.m_matModUvOverrides = materialSlot->m_matModUvOverrides; } else if (!materialSlot->m_propertyOverrides.empty() || !materialSlot->m_matModUvOverrides.empty()) { - MaterialAssignment& materialAssignment = config.m_materials[materialSlot->m_id]; materialAssignment.m_materialAsset = materialSlot->m_defaultMaterialAsset; materialAssignment.m_propertyOverrides = materialSlot->m_propertyOverrides; materialAssignment.m_matModUvOverrides = materialSlot->m_matModUvOverrides; diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponent.h b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponent.h index 88ffef9366..0c468b10a6 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponent.h +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponent.h @@ -51,6 +51,7 @@ namespace AZ //! MaterialComponentNotificationBus::Handler overrides... void OnMaterialsEdited(const MaterialAssignmentMap& materials) override; + void OnMaterialInstanceCreated(const MaterialAssignment& materialAssignment) override; // Apply a material component configuration to the active controller void UpdateConfiguration(const MaterialComponentConfig& config); diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp index 6e1e710282..97e8301bc9 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp @@ -249,6 +249,7 @@ namespace AZ for (auto& materialPair : m_configuration.m_materials) { materialPair.second.RebuildInstance(); + MaterialComponentNotificationBus::Event(m_entityId, &MaterialComponentNotifications::OnMaterialInstanceCreated, materialPair.second); QueuePropertyChanges(materialPair.first); } QueueMaterialUpdateNotification(); @@ -364,6 +365,7 @@ namespace AZ { materialAssignment.m_propertyOverrides[AZ::Name(propertyName)] = value; materialAssignment.RebuildInstance(); + MaterialComponentNotificationBus::Event(m_entityId, &MaterialComponentNotifications::OnMaterialInstanceCreated, materialAssignment); QueueMaterialUpdateNotification(); } else @@ -561,6 +563,7 @@ namespace AZ if (materialIt->second.m_propertyOverrides.empty()) { materialIt->second.RebuildInstance(); + MaterialComponentNotificationBus::Event(m_entityId, &MaterialComponentNotifications::OnMaterialInstanceCreated, materialIt->second); QueueMaterialUpdateNotification(); } @@ -581,6 +584,7 @@ namespace AZ { materialIt->second.m_propertyOverrides = {}; materialIt->second.RebuildInstance(); + MaterialComponentNotificationBus::Event(m_entityId, &MaterialComponentNotifications::OnMaterialInstanceCreated, materialIt->second); QueueMaterialUpdateNotification(); MaterialComponentNotificationBus::Event(m_entityId, &MaterialComponentNotifications::OnMaterialsEdited, m_configuration.m_materials); } @@ -595,6 +599,7 @@ namespace AZ { materialPair.second.m_propertyOverrides = {}; materialPair.second.RebuildInstance(); + MaterialComponentNotificationBus::Event(m_entityId, &MaterialComponentNotifications::OnMaterialInstanceCreated, materialPair.second); QueueMaterialUpdateNotification(); cleared = true; }