Material System Reports Warnings About PSO-Impacting Changes

Merge pull request #4102 from aws-lumberyard-dev/Atom/santorac/WarnOnMaterialPsoChanges These changes make material system report warnings when gameplay scripts attempt to change PSO-impacting material properties at runtime. So far the material system has always allowed any properties to be changed at runtime, including those that affect Pipeline State Objects (PSOs), as this is supported on several platforms. But some platforms require that Pipeline State Objects be pre-compiled and shipped with the game. At some point we will need to add new restrictions that limit what material properties can be changed at runtime. In the meantime, these warnings should alert users to avoid this, as the functionality likely won't be supported in the future. - Made the Material and LuaMaterialFunctor classes configurable to report errors or warnings when material properties modify Pipeline State Objects. This is controlled by a new "MaterialPropertyPsoHandling" enum. - Made the EditorMaterialComponent override PSO handling as Enabled, to prevent warnings when the user is editing material instance property overrides. This requried a new MaterialComponentNotificationBus bus message "OnMaterialInstanceCreated". - Added a new ScopedValue utility class that simply sets a value when it goes out of scope. - Removed unnecessary GetMaterialPropertyDependencies member from material functor context classes, as this is already available as part of the functor itself. - Made Material::SetPropertyValue return early when the property value hadn't actually changed. Besides being more efficientn, this prevents unnecessary spamming of the new warning. - Made a couple imporvements to material_find_overrides_demo.lua: - Made the target material slot name configurable through an exposed component property. - Fixed a timing issue where the assignmentId was invalid if FindMaterialAssignmentId is called too early.
2021-09-15 15:11:01 -07:00
parent 28b18a33bd 346e2d9f66
commit 5b04183b4e
18 changed files with 368 additions and 59 deletions
@@ -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<typename Type>
            bool SetPropertyValue(MaterialPropertyIndex index, const Type& value);

@@ -81,12 +82,15 @@ namespace AZ
            template<typename Type>
            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<MaterialPropertyValue>& GetPropertyValues() const;
+            
+            //! Gets flags indicating which properties have been modified.
+            const MaterialPropertyFlags& GetPropertyDirtyFlags() const;

            //! Gets the material properties layout.
            RHI::ConstPtr<MaterialPropertiesLayout> 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<uint32_t> 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<MaterialAsset>& 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
@@ -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<MaterialPropertiesLayout> 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<bool(ShaderOptionGroup*, ShaderOptionIndex)> 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);
@@ -28,6 +28,24 @@ namespace AZ
        class MaterialPropertiesLayout;

        using MaterialPropertyFlags = AZStd::bitset<Limits::Material::PropertyCountMax>;
+        
+        //! 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> 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<MaterialPropertyValue>& m_materialPropertyValues;
                RHI::ConstPtr<MaterialPropertiesLayout> 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