From b67862a1331b6f31c50ffeb8cd7fadc8ce24ab4f Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Fri, 11 Feb 2022 09:37:12 -0800 Subject: [PATCH] Minor updates from code review feedback. Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- .../Atom/RPI.Edit/Material/MaterialFunctorSourceData.h | 4 ++-- .../Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h | 3 ++- .../Source/RPI.Edit/Material/LuaMaterialFunctorSourceData.cpp | 2 +- .../RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp | 2 ++ .../MaterialEditor/Code/Source/Document/MaterialDocument.cpp | 3 +++ .../Code/Source/Material/EditorMaterialComponentInspector.cpp | 3 +++ 6 files changed, 13 insertions(+), 4 deletions(-) diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialFunctorSourceData.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialFunctorSourceData.h index 8bf88342c3..d26e70831b 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialFunctorSourceData.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialFunctorSourceData.h @@ -112,7 +112,7 @@ namespace AZ //! Returns the name context for the functor. //! It acts like a namespace for any names that the MaterialFunctorSourceData might reference. The namespace //! is automatically applied by the other relevant functions of this RuntimeContext class. - //! Not that by default the MaterialNameContext is not saved as part of the final MaterialFunctor class + //! Note that by default the MaterialNameContext is not saved as part of the final MaterialFunctor class //! (most CreateFunctor() implementations should convert names to indexes anyway) but CreateFunctor() can //! copy it to the created MaterialFunctor for use at runtime if needed. const MaterialNameContext* GetNameContext() const { return m_materialNameContext; } @@ -148,7 +148,7 @@ namespace AZ //! Returns the name context for the functor. //! It acts like a namespace for any names that the MaterialFunctorSourceData might reference. The namespace //! is automatically applied by the other relevant functions of this RuntimeContext class. - //! Not that by default the MaterialNameContext is not saved as part of the final MaterialFunctor class + //! Note that by default the MaterialNameContext is not saved as part of the final MaterialFunctor class //! (most CreateFunctor() implementations should convert names to indexes anyway) but CreateFunctor() can //! copy it to the created MaterialFunctor for use at runtime if needed. const MaterialNameContext* GetNameContext() const { return m_materialNameContext; } diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h index ee2a464290..f1e92a70ea 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h @@ -331,7 +331,8 @@ namespace AZ //! Recursively populates a material type asset with properties from the tree of material property groups. //! @param materialTypeSourceFilePath path to the material type file that is being processed, used to look up relative paths - //! @param propertyIdContext the accumulated prefix that should be applied to any property names encountered in the current @propertyGroup + //! @param materialTypeAssetCreator properties will be added to this creator + //! @param materialNameContext the accumulated name context that should be applied to any property names or connection names encountered in the current @propertyGroup //! @param propertyGroup the current PropertyGroup that is being processed //! @return false if errors are detected and processing should abort bool BuildPropertyList( diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/LuaMaterialFunctorSourceData.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/LuaMaterialFunctorSourceData.cpp index 2477360b8c..790b721fb7 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/LuaMaterialFunctorSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/LuaMaterialFunctorSourceData.cpp @@ -198,7 +198,7 @@ namespace AZ for (const Name& materialProperty : materialPropertyDependencies.GetValue()) { - Name propertyName{materialProperty.GetCStr()}; + Name propertyName{materialProperty}; functor->m_materialNameContext.ContextualizeProperty(propertyName); MaterialPropertyIndex index = propertiesLayout->FindPropertyIndex(propertyName); diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp index 67b468d5bb..85d1a4ccd9 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp @@ -273,6 +273,8 @@ namespace UnitTest using AZ::RPI::MaterialFunctor::Process; void Process(AZ::RPI::MaterialFunctor::RuntimeContext&) override { + // Intentionally empty, this is where the functor would do it's normal processing, + // but all this test functor does is store the MaterialNameContext. } MaterialNameContext m_nameContext; diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index d122a7fff4..13b2519368 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -566,6 +566,9 @@ namespace MaterialEditor AZStd::vector groupNameVector; AZStd::vector groupDisplayNameVector; + + groupNameVector.reserve(propertyGroupStack.size()); + groupDisplayNameVector.reserve(propertyGroupStack.size()); for (auto& group : propertyGroupStack) { diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp index 37ca44fa43..ff31f3382f 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp @@ -289,6 +289,9 @@ namespace AZ AZStd::vector groupNameVector; AZStd::vector groupDisplayNameVector; + + groupNameVector.reserve(propertyGroupStack.size()); + groupDisplayNameVector.reserve(propertyGroupStack.size()); for (auto& nextGroup : propertyGroupStack) {