From a4346de65845b26fc827f3e5aea036c27f872f03 Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Wed, 26 Jan 2022 23:54:25 -0800 Subject: [PATCH] Code cleanup. New comments. Added some non-const find functions to MaterialTypeSourceData. Fixed places where I forgot to change m_name to GetName(). Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- .../Material/MaterialTypeSourceData.h | 16 ++++++++-- .../Material/MaterialTypeSourceData.cpp | 30 ++++++++++++++++--- .../Code/Tests/Common/ErrorMessageFinder.cpp | 2 +- .../Code/Source/Util/MaterialPropertyUtil.cpp | 2 +- .../Code/Source/Document/MaterialDocument.cpp | 12 ++++---- .../MaterialInspector/MaterialInspector.cpp | 2 +- .../EditorMaterialComponentInspector.cpp | 3 +- .../Material/EditorMaterialComponentUtil.cpp | 7 ++--- 8 files changed, 54 insertions(+), 20 deletions(-) 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 0daa0a4c24..6c439195f1 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 @@ -140,7 +140,14 @@ namespace AZ void SetDisplayName(AZStd::string_view displayName) { m_displayName = displayName; } void SetDescription(AZStd::string_view description) { m_description = description; } + //! Add a new property to this PropertySet. + //! @param name a unique for the property. Must be a C-style identifier. + //! @return the new PropertyDefinition, or null if the name was not valid. PropertyDefinition* AddProperty(AZStd::string_view name); + + //! Add a new nested PropertySet to this PropertySet. + //! @param name a unique for the property set. Must be a C-style identifier. + //! @return the new PropertySet, or null if the name was not valid. PropertySet* AddPropertySet(AZStd::string_view name); private: @@ -213,9 +220,9 @@ namespace AZ AZStd::vector m_groupsOld; //! [Deprecated] Use m_propertySets instead - //! Collection of all available user-facing properties AZStd::map> m_propertiesOld; - + + //! Collection of all available user-facing properties AZStd::vector> m_propertySets; }; @@ -257,11 +264,13 @@ namespace AZ //! @param propertySetId The full ID of a property set to find, like "levelA.levelB.levelC". //! @return the found PropertySet or null if it doesn't exist. const PropertySet* FindPropertySet(AZStd::string_view propertySetId) const; + PropertySet* FindPropertySet(AZStd::string_view propertySetId); //! Find the definition for a property with the given ID. //! @param propertyId The full ID of a property to find, like "baseColor.texture". //! @return the found PropertyDefinition or null if it doesn't exist. const PropertyDefinition* FindProperty(AZStd::string_view propertyId) const; + PropertyDefinition* FindProperty(AZStd::string_view propertyId); //! Tokenizes an ID string like "itemA.itemB.itemC" into a vector like ["itemA", "itemB", "itemC"]. static AZStd::vector TokenizeId(AZStd::string_view id); @@ -300,7 +309,10 @@ namespace AZ private: const PropertySet* FindPropertySet(AZStd::array_view parsedPropertySetId, AZStd::array_view> inPropertySetList) const; + PropertySet* FindPropertySet(AZStd::array_view parsedPropertySetId, AZStd::array_view> inPropertySetList); + const PropertyDefinition* FindProperty(AZStd::array_view parsedPropertyId, AZStd::array_view> inPropertySetList) const; + PropertyDefinition* FindProperty(AZStd::array_view parsedPropertyId, AZStd::array_view> inPropertySetList); // Function overloads for recursion, returns false to indicate that recursion should end. bool EnumeratePropertySets(const EnumeratePropertySetsCallback& callback, AZStd::string propertyIdContext, const AZStd::vector>& inPropertySetList) const; diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp index 046dee9507..032d36636e 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp @@ -215,7 +215,7 @@ namespace AZ return PropertySet::AddPropertySet(propertySetId, m_propertyLayout.m_propertySets); } - PropertySet* parentPropertySet = const_cast(const_cast(this)->FindPropertySet(splitPropertySetId[0])); + PropertySet* parentPropertySet = FindPropertySet(splitPropertySetId[0]); if (!parentPropertySet) { @@ -235,8 +235,8 @@ namespace AZ AZ_Error("Material source data", false, "Property id '%.*s' is invalid. Properties must be added to a PropertySet (i.e. \"general.%.*s\").", AZ_STRING_ARG(propertyId), AZ_STRING_ARG(propertyId)); return nullptr; } - - PropertySet* parentPropertySet = const_cast(const_cast(this)->FindPropertySet(splitPropertyId[0])); + + PropertySet* parentPropertySet = FindPropertySet(splitPropertyId[0]); if (!parentPropertySet) { @@ -276,12 +276,23 @@ namespace AZ return nullptr; } + + MaterialTypeSourceData::PropertySet* MaterialTypeSourceData::FindPropertySet(AZStd::array_view parsedPropertySetId, AZStd::array_view> inPropertySetList) + { + return const_cast(const_cast(this)->FindPropertySet(parsedPropertySetId, inPropertySetList)); + } const MaterialTypeSourceData::PropertySet* MaterialTypeSourceData::FindPropertySet(AZStd::string_view propertySetId) const { AZStd::vector tokens = TokenizeId(propertySetId); return FindPropertySet(tokens, m_propertyLayout.m_propertySets); } + + MaterialTypeSourceData::PropertySet* MaterialTypeSourceData::FindPropertySet(AZStd::string_view propertySetId) + { + AZStd::vector tokens = TokenizeId(propertySetId); + return FindPropertySet(tokens, m_propertyLayout.m_propertySets); + } const MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty( AZStd::array_view parsedPropertyId, @@ -316,12 +327,23 @@ namespace AZ return nullptr; } + + MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::array_view parsedPropertyId, AZStd::array_view> inPropertySetList) + { + return const_cast(const_cast(this)->FindProperty(parsedPropertyId, inPropertySetList)); + } const MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::string_view propertyId) const { AZStd::vector tokens = TokenizeId(propertyId); return FindProperty(tokens, m_propertyLayout.m_propertySets); } + + MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::string_view propertyId) + { + AZStd::vector tokens = TokenizeId(propertyId); + return FindProperty(tokens, m_propertyLayout.m_propertySets); + } AZStd::vector MaterialTypeSourceData::TokenizeId(AZStd::string_view id) { @@ -428,7 +450,7 @@ namespace AZ const auto& propertyList = propertyListItr->second; for (auto& propertyDefinition : propertyList) { - PropertySet* propertySet = const_cast(const_cast(this)->FindPropertySet(group.m_name)); + PropertySet* propertySet = FindPropertySet(group.m_name); if (!propertySet) { diff --git a/Gems/Atom/RPI/Code/Tests/Common/ErrorMessageFinder.cpp b/Gems/Atom/RPI/Code/Tests/Common/ErrorMessageFinder.cpp index 06cdb073e9..fa88f145a6 100644 --- a/Gems/Atom/RPI/Code/Tests/Common/ErrorMessageFinder.cpp +++ b/Gems/Atom/RPI/Code/Tests/Common/ErrorMessageFinder.cpp @@ -84,7 +84,7 @@ namespace UnitTest } } - m_checked = true; + m_checked = true; } void ErrorMessageFinder::ReportFailure(const AZStd::string& failureMessage) diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Util/MaterialPropertyUtil.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Util/MaterialPropertyUtil.cpp index 568e701e3c..377fd46d69 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Util/MaterialPropertyUtil.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Util/MaterialPropertyUtil.cpp @@ -78,7 +78,7 @@ namespace AtomToolsFramework void ConvertToPropertyConfig(AtomToolsFramework::DynamicPropertyConfig& propertyConfig, const AZ::RPI::MaterialTypeSourceData::PropertyDefinition& propertyDefinition) { propertyConfig.m_dataType = ConvertToEditableType(propertyDefinition.m_dataType); - propertyConfig.m_name = propertyDefinition.m_name; + propertyConfig.m_name = propertyDefinition.GetName(); propertyConfig.m_displayName = propertyDefinition.m_displayName; propertyConfig.m_description = propertyDefinition.m_description; propertyConfig.m_defaultValue = ConvertToEditableType(propertyDefinition.m_value); diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index c9ae970215..aa14b02bbe 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -587,7 +587,7 @@ namespace MaterialEditor // populate sourceData with properties that meet the filter m_materialTypeSourceData.EnumerateProperties([&](const AZStd::string& propertyIdContext, const auto& propertyDefinition) { - Name propertyId{propertyIdContext + propertyDefinition->m_name}; + Name propertyId{propertyIdContext + propertyDefinition->GetName()}; const auto it = m_properties.find(propertyId); if (it != m_properties.end() && propertyFilter(it->second)) @@ -603,8 +603,8 @@ namespace MaterialEditor } // TODO: Support populating the Material Editor with nested property sets, not just the top level. - const AZStd::string groupName = propertyId.GetStringView().substr(0, propertyId.GetStringView().size() - propertyDefinition->m_name.size() - 1); - sourceData.m_properties[groupName][propertyDefinition->m_name].m_value = propertyValue; + const AZStd::string groupName = propertyId.GetStringView().substr(0, propertyId.GetStringView().size() - propertyDefinition->GetName().size() - 1); + sourceData.m_properties[groupName][propertyDefinition->GetName()].m_value = propertyValue; } } return true; @@ -788,7 +788,7 @@ namespace MaterialEditor for (const auto& propertyDefinition : propertySet->GetProperties()) { // Assign id before conversion so it can be used in dynamic description - propertyConfig.m_id = propertyIdContext + propertySet->GetName() + "." + propertyDefinition->m_name; + propertyConfig.m_id = propertyIdContext + propertySet->GetName() + "." + propertyDefinition->GetName(); const auto& propertyIndex = m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyConfig.m_id); const bool propertyIndexInBounds = propertyIndex.IsValid() && propertyIndex.GetIndex() < m_materialAsset->GetPropertyValues().size(); @@ -877,6 +877,7 @@ namespace MaterialEditor m_properties[propertyConfig.m_id] = AtomToolsFramework::DynamicProperty(propertyConfig); } + // Add material functors that are in the top-level functors list. const MaterialFunctorSourceData::EditorContext editorContext = MaterialFunctorSourceData::EditorContext(m_materialSourceData.m_materialType, m_materialAsset->GetMaterialPropertiesLayout()); for (Ptr functorData : m_materialTypeSourceData.m_materialFunctorSourceData) @@ -897,7 +898,8 @@ namespace MaterialEditor return false; } } - + + // Add any material functors that are located inside each property set. bool enumerateResult = m_materialTypeSourceData.EnumeratePropertySets( [this](const AZStd::string&, const MaterialTypeSourceData::PropertySet* propertySet) { diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp index 98e4ede2a9..3208683bf0 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp @@ -185,7 +185,7 @@ namespace MaterialEditor AtomToolsFramework::DynamicProperty property; AtomToolsFramework::AtomToolsDocumentRequestBus::EventResult( property, m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::GetProperty, - AZ::RPI::MaterialPropertyId(groupName, propertyDefinition->m_name)); + AZ::RPI::MaterialPropertyId(groupName, propertyDefinition->GetName())); group.m_properties.push_back(property); } diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp index 9e990e2fc8..37fba346b8 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp @@ -307,7 +307,7 @@ namespace AZ AtomToolsFramework::DynamicPropertyConfig propertyConfig; // Assign id before conversion so it can be used in dynamic description - propertyConfig.m_id = AZ::RPI::MaterialPropertyId(groupName, propertyDefinition->m_name); + propertyConfig.m_id = AZ::RPI::MaterialPropertyId(groupName, propertyDefinition->GetName()); AtomToolsFramework::ConvertToPropertyConfig(propertyConfig, *propertyDefinition.get()); @@ -323,7 +323,6 @@ namespace AZ // assigned material asset. Its values should be treated as parent, for comparison, in this case. propertyConfig.m_parentValue = AtomToolsFramework::ConvertToEditableType( m_editData.m_materialTypeAsset->GetDefaultPropertyValues()[propertyIndex.GetIndex()]); - propertyConfig.m_originalValue = AtomToolsFramework::ConvertToEditableType( m_editData.m_materialAsset->GetPropertyValues()[propertyIndex.GetIndex()]); group.m_properties.emplace_back(propertyConfig); diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp index d6342a2507..62982e4b4d 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp @@ -115,7 +115,7 @@ namespace AZ bool result = true; editData.m_materialTypeSourceData.EnumerateProperties([&](const AZStd::string& propertyIdContext, const AZ::RPI::MaterialTypeSourceData::PropertyDefinition* propertyDefinition) { - AZ::Name propertyId(propertyIdContext + propertyDefinition->m_name); + AZ::Name propertyId(propertyIdContext + propertyDefinition->GetName()); const AZ::RPI::MaterialPropertyIndex propertyIndex = editData.m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyId); @@ -148,10 +148,9 @@ namespace AZ return true; } - // TODO: Support populating the Material Editor with nested property sets, not just the top level. - const AZStd::string groupName = propertyId.GetStringView().substr(0, propertyId.GetStringView().size() - propertyDefinition->m_name.size() - 1); - exportData.m_properties[groupName][propertyDefinition->m_name].m_value = propertyValue; + const AZStd::string groupName = propertyId.GetStringView().substr(0, propertyId.GetStringView().size() - propertyDefinition->GetName().size() - 1); + exportData.m_properties[groupName][propertyDefinition->GetName()].m_value = propertyValue; return true; });