From 1a99103999e132b21fc67ed4948401f788dc1c59 Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Thu, 30 Sep 2021 01:30:28 -0700 Subject: [PATCH] Overhauled the .materialtype file format to group all related properties data together. This prepares the way for a number of possible improvements, especially unlocking the ability to factor out material type configuration to be shared by multiple material types. Here we formalize the concept of a Property Set, which replaces property "groups", containing the group name and description, properties, and functors all in one place. The Property Set structure will allow arbitrarily deep nesting, whereas before you only had one level of grouping. This nesting is not fully supported yet throughout the system, particularly in the Material Editor. It was easier to go ahead and put in some of the nesting mechanims, parituclar in the implementation of MaterialTypeSourceData. This change is backward compatible, which is proved with unit tests, and by the fact that only MinimalPBR.materialtype has been updated to the new format. StandardPBR, EnhancedPBR, and others are still using the old format. (In a subsequent commit I'll update these as well, to prove that the new format works correctly). Other changes and improvements... - A new constructor for MaterialPropertyId - Improved API for MaterialTypeSourceData that hides a good deal more of it's data as private, with clear and convenient APIs. Especially AddProperty, AddPropertySet, FindProperty, FindPropertySet, EnumerateProperties, EnumeratePropertySets. - Added lots of new unit tests - Updated MinimalPBR.materialtype to the new format. Testing: - Updated unit tests. - Reprocessed Atom material assets. - Ran AtomSampleViewer material screenshot test. - Opened, edited, saved material in the Material Editor. - Opened a level, edited material property overrides, saved and reloaded. Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- .../RPI.Edit/Material/MaterialPropertyId.h | 4 + .../Material/MaterialTypeSourceData.h | 142 ++- .../RPI.Edit/Material/MaterialPropertyId.cpp | 35 + .../MaterialPropertyValueSerializer.cpp | 4 +- .../Material/MaterialSourceDataSerializer.cpp | 1 + .../Material/MaterialTypeSourceData.cpp | 749 ++++++++--- .../RPI.Edit/Material/MaterialUtils.cpp | 1 + .../Code/Tests/Common/ErrorMessageFinder.cpp | 2 +- .../Material/MaterialPropertyIdTests.cpp | 34 + .../Material/MaterialSourceDataTests.cpp | 117 +- .../Material/MaterialTypeSourceDataTests.cpp | 1134 +++++++++++++---- .../Materials/Types/MinimalPBR.materialtype | 80 +- .../Code/Source/Document/MaterialDocument.cpp | 113 +- .../MaterialInspector/MaterialInspector.cpp | 31 +- .../EditorMaterialComponentInspector.cpp | 42 +- .../Material/EditorMaterialComponentUtil.cpp | 70 +- 16 files changed, 1890 insertions(+), 669 deletions(-) diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyId.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyId.h index 4b6d78c092..496485d0d6 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyId.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyId.h @@ -35,6 +35,7 @@ namespace AZ MaterialPropertyId(AZStd::string_view groupName, AZStd::string_view propertyName); MaterialPropertyId(const Name& groupName, const Name& propertyName); MaterialPropertyId(const AZStd::array_view names); + MaterialPropertyId(const AZStd::array_view groupNames, AZStd::string_view propertyName); AZ_DEFAULT_COPY_MOVE(MaterialPropertyId); @@ -44,6 +45,9 @@ namespace AZ //! This is included for convenience so it can be used for error messages in the same way an AZ::Name is used. const char* GetCStr() const; + //! Wraps Name::GetStringView() for convenience. + AZStd::string_view GetStringView() const; + //! Returns a hash of the full name. This is needed for compatibility with NameIdReflectionMap. Name::Hash GetHash() const; 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 04b6222404..56f7c4612c 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 @@ -68,8 +68,9 @@ namespace AZ struct PropertyDefinition { + AZ_CLASS_ALLOCATOR(PropertyDefinition, SystemAllocator, 0); AZ_TYPE_INFO(AZ::RPI::MaterialTypeSourceData::PropertyDefinition, "{E0DB3C0D-75DB-4ADB-9E79-30DA63FA18B7}"); - + static const float DefaultMin; static const float DefaultMax; static const float DefaultStep; @@ -117,68 +118,159 @@ namespace AZ AZStd::unordered_map m_shaderOptionValues; }; - using PropertyList = AZStd::vector; + using PropertyList = AZStd::vector>; + + struct PropertySet + { + friend class MaterialTypeSourceData; + + AZ_CLASS_ALLOCATOR(PropertySet, SystemAllocator, 0); + AZ_TYPE_INFO(AZ::RPI::MaterialTypeSourceData::PropertySet, "{BA3AA0E4-C74D-4FD0-ADB2-00B060F06314}"); + + public: + + PropertySet() = default; + AZ_DISABLE_COPY(PropertySet) + + const AZStd::string& GetName() const { return m_name; } + const AZStd::string& GetDisplayName() const { return m_displayName; } + const AZStd::string& GetDescription() const { return m_description; } + const PropertyList& GetProperties() const { return m_properties; } + const AZStd::vector>& GetPropertySets() const { return m_propertySets; } + const AZStd::vector>& GetFunctors() const { return m_materialFunctorSourceData; } + + void SetDisplayName(AZStd::string_view displayName) { m_displayName = displayName; } + void SetDescription(AZStd::string_view description) { m_description = description; } + + PropertyDefinition* AddProperty(AZStd::string_view name); + PropertySet* AddPropertySet(AZStd::string_view name); + + private: + + static PropertySet* AddPropertySet(AZStd::string_view name, AZStd::vector>& toPropertySetList); + + AZStd::string m_name; + AZStd::string m_displayName; + AZStd::string m_description; + PropertyList m_properties; + AZStd::vector> m_propertySets; + AZStd::vector> m_materialFunctorSourceData; + }; + struct PropertyLayout { AZ_TYPE_INFO(AZ::RPI::MaterialTypeSourceData::PropertyLayout, "{AE53CF3F-5C3B-44F5-B2FB-306F0EB06393}"); + PropertyLayout() = default; + AZ_DISABLE_COPY(PropertyLayout) + //! Indicates the version of the set of available properties. Can be used to detect materials that might need to be updated. uint32_t m_version = 0; + //! [Deprecated] Use m_propertySets instead //! List of groups that will contain the available properties AZStd::vector m_groups; + //! [Deprecated] Use m_propertySets instead //! Collection of all available user-facing properties - AZStd::map m_properties; + AZStd::map> m_properties; + + AZStd::vector> m_propertySets; }; + + PropertySet* AddPropertySet(AZStd::string_view propertySetId); + //PropertySet* AddPropertySet(AZStd::string_view parentPropertySetId, AZStd::string_view name); + PropertyDefinition* AddProperty(AZStd::string_view propertyId); + //PropertyDefinition* AddProperty(AZStd::string_view parentPropertySetId, AZStd::string_view name); - AZStd::string m_description; + const PropertyLayout& GetPropertyLayout() const { return m_propertyLayout; } - PropertyLayout m_propertyLayout; + AZStd::string m_description; //< TODO: Make this private //! A list of shader variants that are always used at runtime; they cannot be turned off - AZStd::vector m_shaderCollection; + AZStd::vector m_shaderCollection; //< TODO: Make this private //! Material functors provide custom logic and calculations to configure shaders, render states, and more. See MaterialFunctor.h for details. - AZStd::vector> m_materialFunctorSourceData; + AZStd::vector> m_materialFunctorSourceData; //< TODO: Make this private //! Override names for UV input in the shaders of this material type. //! Using ordered map to sort names on loading. using UvNameMap = AZStd::map; - UvNameMap m_uvNameMap; + UvNameMap m_uvNameMap; //< TODO: Make this private //! Copy over UV custom names to the properties enum values. void ResolveUvEnums(); + + const PropertySet* FindPropertySet(AZStd::string_view propertySetId) const; - const GroupDefinition* FindGroup(AZStd::string_view groupName) const; + const PropertyDefinition* FindProperty(AZStd::string_view propertyId) const; - const PropertyDefinition* FindProperty(AZStd::string_view groupName, AZStd::string_view propertyName) const; + //! Tokenizes an ID string like "itemA.itemB.itemC" into a vector like ["itemA", "itemB", "itemC"] + static AZStd::vector TokenizeId(AZStd::string_view id); + + //! Splits an ID string like "itemA.itemB.itemC" into a vector like ["itemA.itemB", "itemC"] + static AZStd::vector SplitId(AZStd::string_view id); - //! Construct a complete list of group definitions, including implicit groups, arranged in the same order as the source data - //! Groups with the same name will be consolidated into a single entry - AZStd::vector GetGroupDefinitionsInDisplayOrder() const; + //! Call back function type used with the enumeration functions + using EnumeratePropertySetsCallback = AZStd::function; + //! Recursively traverses all of the property sets contained in the material type, executing a callback function for each. + //! @return false if the enumeration was terminated early by the callback returning false. + bool EnumeratePropertySets(const EnumeratePropertySetsCallback& callback) const; //! Call back function type used with the numeration functions using EnumeratePropertiesCallback = AZStd::function; - - //! Traverse all of the properties contained in the source data executing a callback function - //! Traversal will occur in group alphabetical order and stop once all properties have been enumerated or the callback function returns false - void EnumerateProperties(const EnumeratePropertiesCallback& callback) const; - - //! Traverse all of the properties in the source data in display/storage order executing a callback function - //! Traversal will stop once all properties have been enumerated or the callback function returns false - void EnumeratePropertiesInDisplayOrder(const EnumeratePropertiesCallback& callback) const; + + //! Recursively traverses all of the properties contained in the material type, executing a callback function for each. + //! @return false if the enumeration was terminated early by the callback returning false. + bool EnumerateProperties(const EnumeratePropertiesCallback& callback) const; //! Convert the property value into the format that will be stored in the source data //! This is primarily needed to support conversions of special types like enums and images bool ConvertPropertyValueToSourceDataFormat(const PropertyDefinition& propertyDefinition, MaterialPropertyValue& propertyValue) const; Outcome> CreateMaterialTypeAsset(Data::AssetId assetId, AZStd::string_view materialTypeSourceFilePath = "", bool elevateWarnings = true) const; + + bool ConvertToNewDataFormat(); + + private: + + //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) const; + + //PropertyDefinition* FindProperty(AZStd::array_view parsedPropertyId, PropertySet& inPropertySet); + //const PropertyDefinition* FindProperty(AZStd::array_view parsedPropertyId, const PropertySet& inPropertySet) const; + + // 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; + bool EnumerateProperties(const EnumeratePropertiesCallback& callback, AZStd::string propertyIdContext, const AZStd::vector>& inPropertySetList) const; + + //! Recursively populates a material asset with properties from the tree of material property sets. + //! @param materialTypeSourceFilePath path to the material type file that is being processed, used to look up relative paths + //! @param propertyNameContext the accumulated prefix that should be applied to any property names encountered in the current @propertySet + //! @param propertySet the current PropertySet that is being processed + //! @return false if errors are detected and processing should abort + bool BuildPropertyList( + const AZStd::string& materialTypeSourceFilePath, + MaterialTypeAssetCreator& materialTypeAssetCreator, + AZStd::vector& propertyNameContext, + const MaterialTypeSourceData::PropertySet* propertySet) const; + + //! Construct a complete list of group definitions, including implicit groups, arranged in the same order as the source data. + //! Groups with the same name will be consolidated into a single entry. + //! Operates on the old format PropertyLayout::m_groups, used for conversion to the new format. + AZStd::vector GetOldFormatGroupDefinitionsInDisplayOrder() const; + + PropertyLayout m_propertyLayout; }; //! The wrapper class for derived material functors. @@ -207,7 +299,7 @@ namespace AZ return m_actualSourceData ? m_actualSourceData->CreateFunctor(editorContext) : Failure(); } - const Ptr GetActualSourceData() const { return m_actualSourceData; } + Ptr GetActualSourceData() const { return m_actualSourceData; } private: Ptr m_actualSourceData = nullptr; // The derived material functor instance. }; diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyId.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyId.cpp index 1be5c4485c..47e41fa6fa 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyId.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyId.cpp @@ -103,6 +103,36 @@ namespace AZ AzFramework::StringFunc::Join(fullName, names.begin(), names.end(), "."); m_fullName = fullName; } + + MaterialPropertyId::MaterialPropertyId(const AZStd::array_view groupNames, AZStd::string_view propertyName) + { + for (const auto& name : groupNames) + { + if (!IsValidName(name)) + { + AZ_Error("MaterialPropertyId", false, "'%s' is not a valid identifier.", name.c_str()); + return; + } + } + + if (!IsValidName(propertyName)) + { + AZ_Error("MaterialPropertyId", false, "'%.*s' is not a valid identifier.", AZ_STRING_ARG(propertyName)); + return; + } + + if (groupNames.empty()) + { + m_fullName = propertyName; + } + else + { + AZStd::string fullName; + AzFramework::StringFunc::Join(fullName, groupNames.begin(), groupNames.end(), "."); + fullName = AZStd::string::format("%s.%.*s", fullName.c_str(), AZ_STRING_ARG(propertyName)); + m_fullName = fullName; + } + } MaterialPropertyId::operator const Name&() const { @@ -113,6 +143,11 @@ namespace AZ { return m_fullName.GetCStr(); } + + AZStd::string_view MaterialPropertyId::GetStringView() const + { + return m_fullName.GetStringView(); + } Name::Hash MaterialPropertyId::GetHash() const { diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyValueSerializer.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyValueSerializer.cpp index 3b2d36451a..5bf78d1639 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyValueSerializer.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyValueSerializer.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -63,6 +64,7 @@ namespace AZ } // Construct the full property name (groupName.propertyName) by parsing it from the JSON path string. + // Note we don't yet support full nested property sets, but eventually this should not be limited to just one group with one list of properties... size_t startPropertyName = context.GetPath().Get().rfind('/'); size_t startGroupName = context.GetPath().Get().rfind('/', startPropertyName-1); AZStd::string_view groupName = context.GetPath().Get().substr(startGroupName + 1, startPropertyName - startGroupName - 1); @@ -70,7 +72,7 @@ namespace AZ JSR::ResultCode result(JSR::Tasks::ReadField); - auto propertyDefinition = materialType->FindProperty(groupName, propertyName); + auto propertyDefinition = materialType->FindProperty(MaterialPropertyId{groupName, propertyName}.GetStringView()); if (!propertyDefinition) { AZStd::string message = AZStd::string::format("Property '%.*s.%.*s' not found in material type.", AZ_STRING_ARG(groupName), AZ_STRING_ARG(propertyName)); diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp index 5e4af07aae..e24655b7a6 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp @@ -103,6 +103,7 @@ namespace AZ settings.m_clearContainers = context.ShouldClearContainers(); JsonSerializationResult::ResultCode materialTypeLoadResult = JsonSerialization::Load(materialTypeData, materialTypeJson.GetValue(), settings); + materialTypeData.ConvertToNewDataFormat(); materialTypeData.ResolveUvEnums(); // Restore prior configuration 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 f69b082292..de55cbecb5 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp @@ -56,7 +56,11 @@ namespace AZ serializeContext->Class()->Version(3); serializeContext->Class()->Version(4); serializeContext->Class()->Version(1); - + + serializeContext->RegisterGenericType>(); + serializeContext->RegisterGenericType>(); + serializeContext->RegisterGenericType>>(); + serializeContext->RegisterGenericType>>(); serializeContext->RegisterGenericType(); serializeContext->Class() @@ -66,11 +70,22 @@ namespace AZ ->Field("options", &ShaderVariantReferenceData::m_shaderOptionValues) ; + serializeContext->Class() + ->Version(1) + ->Field("name", &PropertySet::m_name) + ->Field("displayName", &PropertySet::m_displayName) + ->Field("description", &PropertySet::m_description) + ->Field("properties", &PropertySet::m_properties) + ->Field("propertySets", &PropertySet::m_propertySets) + ->Field("functors", &PropertySet::m_materialFunctorSourceData) + ; + serializeContext->Class() ->Version(1) ->Field("version", &PropertyLayout::m_version) - ->Field("groups", &PropertyLayout::m_groups) - ->Field("properties", &PropertyLayout::m_properties) + ->Field("groups", &PropertyLayout::m_groups) //< Old, preserved for backward compatibility, replaced by propertySets + ->Field("properties", &PropertyLayout::m_properties) //< Old, preserved for backward compatibility, replaced by propertySets + ->Field("propertySets", &PropertyLayout::m_propertySets) ; serializeContext->RegisterGenericType(); @@ -92,43 +107,397 @@ namespace AZ , m_shaderIndex(shaderIndex) { } - + const float MaterialTypeSourceData::PropertyDefinition::DefaultMin = std::numeric_limits::lowest(); const float MaterialTypeSourceData::PropertyDefinition::DefaultMax = std::numeric_limits::max(); const float MaterialTypeSourceData::PropertyDefinition::DefaultStep = 0.1f; - - const MaterialTypeSourceData::GroupDefinition* MaterialTypeSourceData::FindGroup(AZStd::string_view groupName) const + + /*static*/ MaterialTypeSourceData::PropertySet* MaterialTypeSourceData::PropertySet::AddPropertySet(AZStd::string_view name, AZStd::vector>& toPropertySetList) { - for (const GroupDefinition& group : m_propertyLayout.m_groups) - { - if (group.m_name == groupName) + auto iter = AZStd::find_if(toPropertySetList.begin(), toPropertySetList.end(), [name](const AZStd::unique_ptr& existingPropertySet) { - return &group; + return existingPropertySet->m_name == name; + }); + + if (iter != toPropertySetList.end()) + { + AZ_Error("Material source data", false, "PropertySet named '%.*s' already exists", AZ_STRING_ARG(name)); + return nullptr; + } + + if (!MaterialPropertyId::IsValidName(name)) + { + AZ_Error("Material source data", false, "'%.*s' is not a valid identifier", AZ_STRING_ARG(name)); + return nullptr; + } + + toPropertySetList.push_back(AZStd::make_unique()); + toPropertySetList.back()->m_name = name; + return toPropertySetList.back().get(); + } + + MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::PropertySet::AddProperty(AZStd::string_view name) + { + auto propertyIter = AZStd::find_if(m_properties.begin(), m_properties.end(), [name](const AZStd::unique_ptr& existingProperty) + { + return existingProperty->m_name == name; + }); + + if (propertyIter != m_properties.end()) + { + AZ_Error("Material source data", false, "PropertySet '%s' already contains a property named '%.*s'", m_name.c_str(), AZ_STRING_ARG(name)); + return nullptr; + } + + auto propertySetIter = AZStd::find_if(m_propertySets.begin(), m_propertySets.end(), [name](const AZStd::unique_ptr& existingPropertySet) + { + return existingPropertySet->m_name == name; + }); + + if (propertySetIter != m_propertySets.end()) + { + AZ_Error("Material source data", false, "Property name '%.*s' collides with a PropertySet of the same name", AZ_STRING_ARG(name)); + return nullptr; + } + + if (!MaterialPropertyId::IsValidName(name)) + { + AZ_Error("Material source data", false, "'%.*s' is not a valid identifier", AZ_STRING_ARG(name)); + return nullptr; + } + + m_properties.emplace_back(AZStd::make_unique()); + m_properties.back()->m_name = name; + return m_properties.back().get(); + } + + MaterialTypeSourceData::PropertySet* MaterialTypeSourceData::PropertySet::AddPropertySet(AZStd::string_view name) + { + auto iter = AZStd::find_if(m_properties.begin(), m_properties.end(), [name](const AZStd::unique_ptr& existingProperty) + { + return existingProperty->m_name == name; + }); + + if (iter != m_properties.end()) + { + AZ_Error("Material source data", false, "PropertySet name '%.*s' collides with a Property of the same name", AZ_STRING_ARG(name)); + return nullptr; + } + + return AddPropertySet(name, m_propertySets); + } + + MaterialTypeSourceData::PropertySet* MaterialTypeSourceData::AddPropertySet(AZStd::string_view propertySetId) + { + AZStd::vector splitPropertySetId = SplitId(propertySetId); + + if (splitPropertySetId.size() == 1) + { + return PropertySet::AddPropertySet(propertySetId, m_propertyLayout.m_propertySets); + } + + // TODO: Delete + //return AddPropertySet(splitPropertySetId[0], splitPropertySetId[1]); + + PropertySet* parentPropertySet = const_cast(const_cast(this)->FindPropertySet(splitPropertySetId[0])); + + if (!parentPropertySet) + { + AZ_Error("Material source data", false, "PropertySet '%.*s' does not exists", AZ_STRING_ARG(splitPropertySetId[0])); + return nullptr; + } + + return parentPropertySet->AddPropertySet(splitPropertySetId[1]); + } + + // TODO: Delete + //MaterialTypeSourceData::PropertySet* MaterialTypeSourceData::AddPropertySet(AZStd::string_view parentPropertySetId, AZStd::string_view name) + //{ + // PropertySet* parentPropertySet = const_cast(const_cast(this)->FindPropertySet(parentPropertySetId)); + // + // if (!parentPropertySet) + // { + // AZ_Error("Material source data", false, "PropertySet '%.*s' does not exists", AZ_STRING_ARG(parentPropertySetId)); + // return nullptr; + // } + + // return parentPropertySet->AddPropertySet(name); + //} + + MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::AddProperty(AZStd::string_view propertyId) + { + AZStd::vector splitPropertyId = SplitId(propertyId); + //if (splitPropertyId.empty()) + //{ + // return nullptr; + //} + + if (splitPropertyId.size() == 1) + { + 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; + } + + // TODO: Delete + //return AddProperty(splitPropertyId[0], splitPropertyId[1]); + + PropertySet* parentPropertySet = const_cast(const_cast(this)->FindPropertySet(splitPropertyId[0])); + + if (!parentPropertySet) + { + AZ_Error("Material source data", false, "PropertySet '%.*s' does not exists", AZ_STRING_ARG(splitPropertyId[0])); + return nullptr; + } + + return parentPropertySet->AddProperty(splitPropertyId[1]); + } + + // TODO: Delete + //MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::AddProperty(AZStd::string_view parentPropertySetId, AZStd::string_view name) + //{ + // PropertySet* parentPropertySet = const_cast(const_cast(this)->FindPropertySet(parentPropertySetId)); + // + // if (!parentPropertySet) + // { + // AZ_Error("Material source data", false, "PropertySet '%.*s' does not exists", AZ_STRING_ARG(parentPropertySetId)); + // return nullptr; + // } + + // return parentPropertySet->AddProperty(name); + //} + + const MaterialTypeSourceData::PropertySet* MaterialTypeSourceData::FindPropertySet(AZStd::array_view parsedPropertySetId, AZStd::array_view> inPropertySetList) const + { + for (const auto& propertySet : inPropertySetList) + { + if (propertySet->m_name != parsedPropertySetId[0]) + { + continue; + } + else if (parsedPropertySetId.size() == 1) + { + return propertySet.get(); + } + else + { + AZStd::array_view subPath{parsedPropertySetId.begin() + 1, parsedPropertySetId.end()}; + + if (!subPath.empty()) + { + const MaterialTypeSourceData::PropertySet* propertySubset = FindPropertySet(subPath, propertySet->m_propertySets); + if (propertySubset) + { + return propertySubset; + } + } } } return nullptr; } - const MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::string_view groupName, AZStd::string_view propertyName) const - { - auto groupIter = m_propertyLayout.m_properties.find(groupName); - if (groupIter == m_propertyLayout.m_properties.end()) - { - return nullptr; - } + //MaterialTypeSourceData::PropertySet* MaterialTypeSourceData::FindPropertySet(AZStd::array_view parsedPropertySetId, AZStd::array_view> inPropertySetList) + //{ + // return const_cast(const_cast(this)->FindPropertySet(parsedPropertySetId, inPropertySetList)); + //} - for (const PropertyDefinition& property : groupIter->second) + const MaterialTypeSourceData::PropertySet* MaterialTypeSourceData::FindPropertySet(AZStd::string_view propertySetId) const + { + AZStd::vector tokens = TokenizeId(propertySetId); + return FindPropertySet(tokens, m_propertyLayout.m_propertySets); + } + + //MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::array_view parsedPropertyId, PropertySet& inPropertySet) + //{ + // if (parsedPropertyId.size() == 1) + // { + // for (AZStd::unique_ptr& property : inPropertySet.m_properties) + // { + // if (property->m_name == parsedPropertyId[0]) + // { + // return property.get(); + // } + // } + // } + + // return FindProperty(parsedPropertyId, inPropertySet.m_propertySets); + //} + + //const MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::array_view parsedPropertyId, const PropertySet& inPropertySet) const + //{ + // MaterialTypeSourceData* nonConstThis = const_cast(this); + // PropertySet& nonConstPropertySet = *const_cast(&inPropertySet); + // return const_cast(nonConstThis->FindProperty(parsedPropertyId, nonConstPropertySet)); + //} + + const MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty( + AZStd::array_view parsedPropertyId, + AZStd::array_view> inPropertySetList) const + { + for (const auto& propertySet : inPropertySetList) { - if (property.m_name == propertyName) + if (propertySet->m_name == parsedPropertyId[0]) { - return &property; + AZStd::array_view subPath {parsedPropertyId.begin() + 1, parsedPropertyId.end()}; + + if (subPath.size() == 1) + { + for (AZStd::unique_ptr& property : propertySet->m_properties) + { + if (property->m_name == subPath[0]) + { + return property.get(); + } + } + } + else if(subPath.size() > 1) + { + const MaterialTypeSourceData::PropertyDefinition* property = FindProperty(subPath, propertySet->m_propertySets); + if (property) + { + return property; + } + } } } 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); + } + + AZStd::vector MaterialTypeSourceData::TokenizeId(AZStd::string_view id) + { + AZStd::vector tokens; + AzFramework::StringFunc::Tokenize(id, tokens, "./", true, true); + return tokens; + } + + AZStd::vector MaterialTypeSourceData::SplitId(AZStd::string_view id) + { + AZStd::vector parts; + parts.reserve(2); + size_t lastDelim = id.rfind('.', id.size()-1); + if (lastDelim == AZStd::string::npos) + { + parts.push_back(id); + } + else + { + parts.push_back(AZStd::string_view{id.begin(), id.begin()+lastDelim}); + parts.push_back(AZStd::string_view{id.begin()+lastDelim+1, id.end()}); + } + + return parts; + } + + bool MaterialTypeSourceData::EnumeratePropertySets(const EnumeratePropertySetsCallback& callback, AZStd::string propertyNameContext, const AZStd::vector>& inPropertySetList) const + { + for (auto& propertySet : inPropertySetList) + { + if (!callback(propertyNameContext, propertySet.get())) + { + return false; // Stop processing + } + + const AZStd::string propertyNameContext2 = propertyNameContext + propertySet->m_name + "."; + + if (!EnumeratePropertySets(callback, propertyNameContext2, propertySet->m_propertySets)) + { + return false; // Stop processing + } + } + + return true; + } + + bool MaterialTypeSourceData::EnumeratePropertySets(const EnumeratePropertySetsCallback& callback) const + { + if (!callback) + { + return false; + } + + return EnumeratePropertySets(callback, {}, m_propertyLayout.m_propertySets); + } + + bool MaterialTypeSourceData::EnumerateProperties(const EnumeratePropertiesCallback& callback, AZStd::string propertyNameContext, const AZStd::vector>& inPropertySetList) const + { + + for (auto& propertySet : inPropertySetList) + { + const AZStd::string propertyNameContext2 = propertyNameContext + propertySet->m_name + "."; + + for (auto& property : propertySet->m_properties) + { + if (!callback(propertyNameContext2, property.get())) + { + return false; // Stop processing + } + } + + if (!EnumerateProperties(callback, propertyNameContext2, propertySet->m_propertySets)) + { + return false; // Stop processing + } + } + + return true; + } + + bool MaterialTypeSourceData::EnumerateProperties(const EnumeratePropertiesCallback& callback) const + { + if (!callback) + { + return false; + } + + return EnumerateProperties(callback, {}, m_propertyLayout.m_propertySets); + } + + bool MaterialTypeSourceData::ConvertToNewDataFormat() + { + for (const auto& group : GetOldFormatGroupDefinitionsInDisplayOrder()) + { + auto propertyListItr = m_propertyLayout.m_properties.find(group.m_name); + if (propertyListItr != m_propertyLayout.m_properties.end()) + { + const auto& propertyList = propertyListItr->second; + for (auto& propertyDefinition : propertyList) + { + PropertySet* propertySet = const_cast(const_cast(this)->FindPropertySet(group.m_name)); + + if (!propertySet) + { + m_propertyLayout.m_propertySets.emplace_back(AZStd::make_unique()); + m_propertyLayout.m_propertySets.back()->m_name = group.m_name; + m_propertyLayout.m_propertySets.back()->m_displayName = group.m_displayName; + m_propertyLayout.m_propertySets.back()->m_description = group.m_description; + propertySet = m_propertyLayout.m_propertySets.back().get(); + } + + PropertyDefinition* newProperty = propertySet->AddProperty(propertyDefinition.m_name); + + *newProperty = propertyDefinition; + } + } + } + + m_propertyLayout.m_groups.clear(); + m_propertyLayout.m_properties.clear(); + + return true; + } + void MaterialTypeSourceData::ResolveUvEnums() { AZStd::vector enumValues; @@ -137,20 +506,20 @@ namespace AZ { enumValues.push_back(uvNamePair.second); } - - for (auto& group : m_propertyLayout.m_properties) - { - for (PropertyDefinition& property : group.second) + + EnumerateProperties([&enumValues](const AZStd::string&, const MaterialTypeSourceData::PropertyDefinition* property) { - if (property.m_dataType == AZ::RPI::MaterialPropertyDataType::Enum && property.m_enumIsUv) + if (property->m_dataType == AZ::RPI::MaterialPropertyDataType::Enum && property->m_enumIsUv) { - property.m_enumValues = enumValues; + // const_cast is safe because this is internal to the MaterialTypeSourceData. It isn't worth complicating things + // by adding another version of EnumerateProperties. + const_cast(property)->m_enumValues = enumValues; } - } - } + return true; + }); } - AZStd::vector MaterialTypeSourceData::GetGroupDefinitionsInDisplayOrder() const + AZStd::vector MaterialTypeSourceData::GetOldFormatGroupDefinitionsInDisplayOrder() const { AZStd::vector groupDefinitions; groupDefinitions.reserve(m_propertyLayout.m_properties.size()); @@ -184,54 +553,7 @@ namespace AZ return groupDefinitions; } - void MaterialTypeSourceData::EnumerateProperties(const EnumeratePropertiesCallback& callback) const - { - if (!callback) - { - return; - } - - for (const auto& propertyListPair : m_propertyLayout.m_properties) - { - const AZStd::string& groupName = propertyListPair.first; - const auto& propertyList = propertyListPair.second; - for (const auto& propertyDefinition : propertyList) - { - const AZStd::string& propertyName = propertyDefinition.m_name; - if (!callback(groupName, propertyName, propertyDefinition)) - { - return; - } - } - } - } - - void MaterialTypeSourceData::EnumeratePropertiesInDisplayOrder(const EnumeratePropertiesCallback& callback) const - { - if (!callback) - { - return; - } - - for (const auto& groupDefinition : GetGroupDefinitionsInDisplayOrder()) - { - const AZStd::string& groupName = groupDefinition.m_name; - const auto propertyListItr = m_propertyLayout.m_properties.find(groupName); - if (propertyListItr != m_propertyLayout.m_properties.end()) - { - const auto& propertyList = propertyListItr->second; - for (const auto& propertyDefinition : propertyList) - { - const AZStd::string& propertyName = propertyDefinition.m_name; - if (!callback(groupName, propertyName, propertyDefinition)) - { - return; - } - } - } - } - } - + // TODO: It looks like this function doesn't operate on MaterialTypeSourceData data, it belongs in MaterialUtils bool MaterialTypeSourceData::ConvertPropertyValueToSourceDataFormat(const PropertyDefinition& propertyDefinition, MaterialPropertyValue& propertyValue) const { if (propertyDefinition.m_dataType == AZ::RPI::MaterialPropertyDataType::Enum && propertyValue.Is()) @@ -274,6 +596,178 @@ namespace AZ return true; } + bool MaterialTypeSourceData::BuildPropertyList( + const AZStd::string& materialTypeSourceFilePath, + MaterialTypeAssetCreator& materialTypeAssetCreator, + AZStd::vector& propertyNameContext, + const MaterialTypeSourceData::PropertySet* propertySet) const + { + for (const AZStd::unique_ptr& property : propertySet->m_properties) + { + // Register the property... + + MaterialPropertyId propertyId{propertyNameContext, property->m_name}; + + if (!propertyId.IsValid()) + { + // MaterialPropertyId reports an error message + return false; + } + + auto propertySetIter = AZStd::find_if(propertySet->GetPropertySets().begin(), propertySet->GetPropertySets().end(), + [&property](const AZStd::unique_ptr& existingPropertySet) + { + return existingPropertySet->GetName() == property->m_name; + }); + + if (propertySetIter != propertySet->GetPropertySets().end()) + { + AZ_Error("Material source data", false, "Material property '%s' collides with a PropertySet with the same ID.", propertyId.GetCStr()); + return false; + } + + materialTypeAssetCreator.BeginMaterialProperty(propertyId, property->m_dataType); + + if (property->m_dataType == MaterialPropertyDataType::Enum) + { + materialTypeAssetCreator.SetMaterialPropertyEnumNames(property->m_enumValues); + } + + for (auto& output : property->m_outputConnections) + { + switch (output.m_type) + { + case MaterialPropertyOutputType::ShaderInput: + { + materialTypeAssetCreator.ConnectMaterialPropertyToShaderInput(Name{output.m_fieldName}); + break; + } + case MaterialPropertyOutputType::ShaderOption: + { + if (output.m_shaderIndex >= 0) + { + materialTypeAssetCreator.ConnectMaterialPropertyToShaderOption(Name{output.m_fieldName}, output.m_shaderIndex); + } + else + { + materialTypeAssetCreator.ConnectMaterialPropertyToShaderOptions(Name{output.m_fieldName}); + } + break; + } + case MaterialPropertyOutputType::Invalid: + // Don't add any output mappings, this is the case when material functors are expected to process the property + break; + default: + AZ_Assert(false, "Unsupported MaterialPropertyOutputType"); + return false; + } + } + + materialTypeAssetCreator.EndMaterialProperty(); + + // Parse and set the property's value... + if (!property->m_value.IsValid()) + { + AZ_Warning("Material source data", false, "Source data for material property value is invalid."); + } + else + { + switch (property->m_dataType) + { + case MaterialPropertyDataType::Image: + { + Outcome> imageAssetResult = MaterialUtils::GetImageAssetReference(materialTypeSourceFilePath, property->m_value.GetValue()); + + if (imageAssetResult.IsSuccess()) + { + materialTypeAssetCreator.SetPropertyValue(propertyId, imageAssetResult.GetValue()); + } + else + { + materialTypeAssetCreator.ReportError("Material property '%s': Could not find the image '%s'", propertyId.GetCStr(), property->m_value.GetValue().data()); + } + } + break; + case MaterialPropertyDataType::Enum: + { + MaterialPropertyIndex propertyIndex = materialTypeAssetCreator.GetMaterialPropertiesLayout()->FindPropertyIndex(propertyId); + const MaterialPropertyDescriptor* propertyDescriptor = materialTypeAssetCreator.GetMaterialPropertiesLayout()->GetPropertyDescriptor(propertyIndex); + + AZ::Name enumName = AZ::Name(property->m_value.GetValue()); + uint32_t enumValue = propertyDescriptor->GetEnumValue(enumName); + if (enumValue == MaterialPropertyDescriptor::InvalidEnumValue) + { + materialTypeAssetCreator.ReportError("Enum value '%s' couldn't be found in the 'enumValues' list", enumName.GetCStr()); + } + else + { + materialTypeAssetCreator.SetPropertyValue(propertyId, enumValue); + } + } + break; + default: + materialTypeAssetCreator.SetPropertyValue(propertyId, property->m_value); + break; + } + } + } + + for (const AZStd::unique_ptr& propertySubset : propertySet->m_propertySets) + { + propertyNameContext.push_back(propertySubset->m_name); + + bool success = BuildPropertyList( + materialTypeSourceFilePath, + materialTypeAssetCreator, + propertyNameContext, + propertySubset.get()); + + propertyNameContext.pop_back(); + + if (!success) + { + return false; + } + } + + // We cannot create the MaterialFunctor until after all the properties are added because + // CreateFunctor() may need to look up properties in the MaterialPropertiesLayout + for (auto& functorData : propertySet->m_materialFunctorSourceData) + { + MaterialFunctorSourceData::FunctorResult result = functorData->CreateFunctor( + MaterialFunctorSourceData::RuntimeContext( + materialTypeSourceFilePath, + materialTypeAssetCreator.GetMaterialPropertiesLayout(), + materialTypeAssetCreator.GetMaterialShaderResourceGroupLayout(), + materialTypeAssetCreator.GetShaderCollection() + ) + ); + + if (result.IsSuccess()) + { + Ptr& functor = result.GetValue(); + if (functor != nullptr) + { + materialTypeAssetCreator.AddMaterialFunctor(functor); + + for (const AZ::Name& optionName : functorData->GetActualSourceData()->GetShaderOptionDependencies()) + { + materialTypeAssetCreator.ClaimShaderOptionOwnership(Name{optionName.GetCStr()}); + } + } + } + else + { + materialTypeAssetCreator.ReportError("Failed to create MaterialFunctor"); + return false; + } + } + + + return true; + } + + Outcome> MaterialTypeSourceData::CreateMaterialTypeAsset(Data::AssetId assetId, AZStd::string_view materialTypeSourceFilePath, bool elevateWarnings) const { MaterialTypeAssetCreator materialTypeAssetCreator; @@ -327,103 +821,16 @@ namespace AZ return Failure(); } } - - for (auto& groupIter : m_propertyLayout.m_properties) + + for (const AZStd::unique_ptr& propertySet : m_propertyLayout.m_propertySets) { - const AZStd::string& groupName = groupIter.first; + AZStd::vector propertyNameContext; + propertyNameContext.push_back(propertySet->m_name); + bool success = BuildPropertyList(materialTypeSourceFilePath, materialTypeAssetCreator, propertyNameContext, propertySet.get()); - for (const PropertyDefinition& property : groupIter.second) + if (!success) { - // Register the property... - - MaterialPropertyId propertyId{ groupName, property.m_name }; - - if (!propertyId.IsValid()) - { - materialTypeAssetCreator.ReportWarning("Cannot create material property with invalid ID '%s'.", propertyId.GetCStr()); - continue; - } - - materialTypeAssetCreator.BeginMaterialProperty(propertyId, property.m_dataType); - - if (property.m_dataType == MaterialPropertyDataType::Enum) - { - materialTypeAssetCreator.SetMaterialPropertyEnumNames(property.m_enumValues); - } - - for (auto& output : property.m_outputConnections) - { - switch (output.m_type) - { - case MaterialPropertyOutputType::ShaderInput: - materialTypeAssetCreator.ConnectMaterialPropertyToShaderInput(Name{ output.m_fieldName.data() }); - break; - case MaterialPropertyOutputType::ShaderOption: - if (output.m_shaderIndex >= 0) - { - materialTypeAssetCreator.ConnectMaterialPropertyToShaderOption(Name{ output.m_fieldName.data() }, output.m_shaderIndex); - } - else - { - materialTypeAssetCreator.ConnectMaterialPropertyToShaderOptions(Name{ output.m_fieldName.data() }); - } - break; - case MaterialPropertyOutputType::Invalid: - // Don't add any output mappings, this is the case when material functors are expected to process the property - break; - default: - AZ_Assert(false, "Unsupported MaterialPropertyOutputType"); - return Failure(); - } - } - - materialTypeAssetCreator.EndMaterialProperty(); - - // Parse and set the property's value... - if (!property.m_value.IsValid()) - { - AZ_Warning("Material source data", false, "Source data for material property value is invalid."); - } - else - { - switch (property.m_dataType) - { - case MaterialPropertyDataType::Image: - { - Outcome> imageAssetResult = MaterialUtils::GetImageAssetReference(materialTypeSourceFilePath, property.m_value.GetValue()); - - if (imageAssetResult.IsSuccess()) - { - materialTypeAssetCreator.SetPropertyValue(propertyId, imageAssetResult.GetValue()); - } - else - { - materialTypeAssetCreator.ReportError("Material property '%s': Could not find the image '%s'", propertyId.GetCStr(), property.m_value.GetValue().data()); - } - } - break; - case MaterialPropertyDataType::Enum: - { - MaterialPropertyIndex propertyIndex = materialTypeAssetCreator.GetMaterialPropertiesLayout()->FindPropertyIndex(propertyId); - const MaterialPropertyDescriptor* propertyDescriptor = materialTypeAssetCreator.GetMaterialPropertiesLayout()->GetPropertyDescriptor(propertyIndex); - - AZ::Name enumName = AZ::Name(property.m_value.GetValue()); - uint32_t enumValue = propertyDescriptor ? propertyDescriptor->GetEnumValue(enumName) : MaterialPropertyDescriptor::InvalidEnumValue; - if (enumValue == MaterialPropertyDescriptor::InvalidEnumValue) - { - materialTypeAssetCreator.ReportError("Enum value '%s' couldn't be found in the 'enumValues' list", enumName.GetCStr()); - } - else - { - materialTypeAssetCreator.SetPropertyValue(propertyId, enumValue); - } - } - break; - default: - materialTypeAssetCreator.SetPropertyValue(propertyId, property.m_value); - break; - } - } + return Failure(); } } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp index 90ce9e66ce..174a2e682a 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp @@ -90,6 +90,7 @@ namespace AZ settings.m_metadata.Add(fileLoadContext); JsonSerialization::Load(materialType, *document, settings); + materialType.ConvertToNewDataFormat(); materialType.ResolveUvEnums(); if (reportingHelper.ErrorsReported()) diff --git a/Gems/Atom/RPI/Code/Tests/Common/ErrorMessageFinder.cpp b/Gems/Atom/RPI/Code/Tests/Common/ErrorMessageFinder.cpp index fa88f145a6..06cdb073e9 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/RPI/Code/Tests/Material/MaterialPropertyIdTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialPropertyIdTests.cpp index 2b729ed8ef..80da59b430 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialPropertyIdTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialPropertyIdTests.cpp @@ -94,6 +94,40 @@ namespace UnitTest errorMessageFinder.CheckExpectedErrorsFound(); } + TEST_F(MaterialPropertyIdTests, TestConstructWithMultipleParentNamesSeparateFromPropertyName) + { + AZStd::vector names{"layer1", "clearCoat", "normal"}; + MaterialPropertyId id{names, "factor"}; + EXPECT_TRUE(id.IsValid()); + EXPECT_STREQ(id.GetCStr(), "layer1.clearCoat.normal.factor"); + AZ::Name idCastedToName = id; + EXPECT_EQ(idCastedToName, AZ::Name{"layer1.clearCoat.normal.factor"}); + } + + TEST_F(MaterialPropertyIdTests, TestConstructWithMultipleParentNamesSeparateFromPropertyName_BadParentName) + { + ErrorMessageFinder errorMessageFinder; + errorMessageFinder.AddExpectedErrorMessage("not a valid identifier"); + + AZStd::vector names{"layer1", "clear-coat", "normal"}; + MaterialPropertyId id{names, "factor"}; + EXPECT_FALSE(id.IsValid()); + + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialPropertyIdTests, TestConstructWithMultipleParentNamesSeparateFromPropertyName_BadPropertyName) + { + ErrorMessageFinder errorMessageFinder; + errorMessageFinder.AddExpectedErrorMessage("not a valid identifier"); + + AZStd::vector names{"layer1", "clearCoat", "normal"}; + MaterialPropertyId id{names, "#factor"}; + EXPECT_FALSE(id.IsValid()); + + errorMessageFinder.CheckExpectedErrorsFound(); + } + TEST_F(MaterialPropertyIdTests, TestParse) { MaterialPropertyId id = MaterialPropertyId::Parse("layer1.clearCoat.normal.factor"); diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp index dd3b3b2711..293289b00f 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp @@ -201,33 +201,38 @@ namespace UnitTest TEST_F(MaterialSourceDataTests, TestJsonRoundTrip) { const char* materialTypeJson = - "{ \n" - " \"propertyLayout\": { \n" - " \"version\": 1, \n" - " \"groups\": [ \n" - " { \"name\": \"groupA\" }, \n" - " { \"name\": \"groupB\" }, \n" - " { \"name\": \"groupC\" } \n" - " ], \n" - " \"properties\": { \n" - " \"groupA\": [ \n" - " {\"name\": \"MyBool\", \"type\": \"bool\"}, \n" - " {\"name\": \"MyInt\", \"type\": \"int\"}, \n" - " {\"name\": \"MyUInt\", \"type\": \"uint\"} \n" - " ], \n" - " \"groupB\": [ \n" - " {\"name\": \"MyFloat\", \"type\": \"float\"}, \n" - " {\"name\": \"MyFloat2\", \"type\": \"vector2\"}, \n" - " {\"name\": \"MyFloat3\", \"type\": \"vector3\"} \n" - " ], \n" - " \"groupC\": [ \n" - " {\"name\": \"MyFloat4\", \"type\": \"vector4\"}, \n" - " {\"name\": \"MyColor\", \"type\": \"color\"}, \n" - " {\"name\": \"MyImage\", \"type\": \"image\"} \n" - " ] \n" - " } \n" - " } \n" - "} \n"; + R"( + { + "propertyLayout": { + "propertySets": [ + { + "name": "groupA", + "properties": [ + {"name": "MyBool", "type": "bool"}, + {"name": "MyInt", "type": "int"}, + {"name": "MyUInt", "type": "uint"} + ] + }, + { + "name": "groupB", + "properties": [ + {"name": "MyFloat", "type": "float"}, + {"name": "MyFloat2", "type": "vector2"}, + {"name": "MyFloat3", "type": "vector3"} + ] + }, + { + "name": "groupC", + "properties": [ + {"name": "MyFloat4", "type": "vector4"}, + {"name": "MyColor", "type": "color"}, + {"name": "MyImage", "type": "image"} + ] + } + ] + } + } + )"; const char* materialTypeFilePath = "@exefolder@/Gems/Atom/RPI/Code/Tests/Material/Temp/roundTripTest.materialtype"; @@ -259,25 +264,29 @@ namespace UnitTest MaterialSourceData sourceDataCopy; JsonTestResult loadResult = LoadTestDataFromJson(sourceDataCopy, sourceDataSerialized); - + CheckEqual(sourceDataOriginal, sourceDataCopy); } TEST_F(MaterialSourceDataTests, Load_MaterialTypeAfterPropertyList) { const AZStd::string simpleMaterialTypeJson = R"( - { - "propertyLayout": { - "properties": { - "general": [ + { + "propertyLayout": { + "propertySets": + [ { - "name": "testColor", - "type": "color" + "name": "general", + "properties": [ + { + "name": "testColor", + "type": "color" + } + ] } ] } } - } )"; const char* materialTypeFilePath = "@exefolder@/Gems/Atom/RPI/Code/Tests/Material/Temp/simpleMaterialType.materialtype"; @@ -376,18 +385,22 @@ namespace UnitTest TEST_F(MaterialSourceDataTests, Load_MaterialTypeMessagesAreReported) { const AZStd::string simpleMaterialTypeJson = R"( - { - "propertyLayout": { - "properties": { - "general": [ + { + "propertyLayout": { + "propertySets": + [ { - "name": "testColor", - "type": "color" + "name": "general", + "properties": [ + { + "name": "testColor", + "type": "color" + } + ] } ] } } - } )"; const char* materialTypeFilePath = "@exefolder@/Gems/Atom/RPI/Code/Tests/Material/Temp/simpleMaterialType.materialtype"; @@ -416,24 +429,28 @@ namespace UnitTest EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing()); // propertyLayout is a field in the material type, not the material - EXPECT_TRUE(loadResult.ContainsMessage("[simpleMaterialType.materialtype]/propertyLayout/properties", "Successfully read")); + EXPECT_TRUE(loadResult.ContainsMessage("[simpleMaterialType.materialtype]/propertyLayout/propertySets", "Successfully read")); } TEST_F(MaterialSourceDataTests, Load_Error_PropertyNotFound) { const AZStd::string simpleMaterialTypeJson = R"( - { - "propertyLayout": { - "properties": { - "general": [ + { + "propertyLayout": { + "propertySets": + [ { - "name": "testColor", - "type": "color" + "name": "general", + "properties": [ + { + "name": "testColor", + "type": "color" + } + ] } ] } } - } )"; const char* materialTypeFilePath = "@exefolder@/Gems/Atom/RPI/Code/Tests/Material/Temp/simpleMaterialType.materialtype"; diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp index ba4a58b9ff..00523abbd8 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -225,6 +226,7 @@ namespace UnitTest { serializeContext->Class() ->Version(1) + ->Field("enableProperty", &SetShaderOptionFunctorSourceData::m_enablePropertyName) ; } } @@ -243,6 +245,8 @@ namespace UnitTest Ptr functor = aznew SetShaderOptionFunctor; return Success(Ptr(functor)); } + + AZStd::string m_enablePropertyName; }; ///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -350,9 +354,316 @@ namespace UnitTest EXPECT_EQ(propertyDescriptor->GetOutputConnections()[i].m_containerIndex.GetIndex(), expectedValues.m_outputConnections[i].m_shaderIndex); } } - }; + TEST_F(MaterialTypeSourceDataTests, PopulateAndSearchPropertyLayout) + { + MaterialTypeSourceData sourceData; + + // Here we are building up multiple layers of property sets and properties, using a variety of different Add functions, + // going through the MaterialTypeSourceData or going to the PropertySet directly. + + MaterialTypeSourceData::PropertySet* layer1 = sourceData.AddPropertySet("layer1"); + MaterialTypeSourceData::PropertySet* layer2 = sourceData.AddPropertySet("layer2"); + MaterialTypeSourceData::PropertySet* blend = sourceData.AddPropertySet("blend"); + + MaterialTypeSourceData::PropertySet* layer1_baseColor = layer1->AddPropertySet("baseColor"); + MaterialTypeSourceData::PropertySet* layer2_baseColor = layer2->AddPropertySet("baseColor"); + + MaterialTypeSourceData::PropertySet* layer1_roughness = sourceData.AddPropertySet("layer1.roughness"); + MaterialTypeSourceData::PropertySet* layer2_roughness = sourceData.AddPropertySet("layer2.roughness"); + + MaterialTypeSourceData::PropertyDefinition* layer1_baseColor_texture = layer1_baseColor->AddProperty("texture"); + MaterialTypeSourceData::PropertyDefinition* layer2_baseColor_texture = layer2_baseColor->AddProperty("texture"); + + MaterialTypeSourceData::PropertyDefinition* layer1_roughness_texture = sourceData.AddProperty("layer1.roughness.texture"); + MaterialTypeSourceData::PropertyDefinition* layer2_roughness_texture = sourceData.AddProperty("layer2.roughness.texture"); + + // We're doing clear coat only on layer2, for brevity + MaterialTypeSourceData::PropertySet* layer2_clearCoat = layer2->AddPropertySet("clearCoat"); + MaterialTypeSourceData::PropertySet* layer2_clearCoat_roughness = layer2_clearCoat->AddPropertySet("roughness"); + MaterialTypeSourceData::PropertySet* layer2_clearCoat_normal = layer2_clearCoat->AddPropertySet("normal"); + MaterialTypeSourceData::PropertyDefinition* layer2_clearCoat_enabled = layer2_clearCoat->AddProperty("enabled"); + MaterialTypeSourceData::PropertyDefinition* layer2_clearCoat_roughness_texture = layer2_clearCoat_roughness->AddProperty("texture"); + MaterialTypeSourceData::PropertyDefinition* layer2_clearCoat_normal_texture = layer2_clearCoat_normal->AddProperty("texture"); + MaterialTypeSourceData::PropertyDefinition* layer2_clearCoat_normal_factor = layer2_clearCoat_normal->AddProperty("factor"); + + MaterialTypeSourceData::PropertyDefinition* blend_factor = blend->AddProperty("factor"); + + // Check the available Find functions + + EXPECT_EQ(nullptr, sourceData.FindProperty("DoesNotExist")); + EXPECT_EQ(nullptr, sourceData.FindProperty("layer1.DoesNotExist")); + EXPECT_EQ(nullptr, sourceData.FindProperty("layer1.baseColor.DoesNotExist")); + EXPECT_EQ(nullptr, sourceData.FindProperty("baseColor.texture")); + EXPECT_EQ(nullptr, sourceData.FindProperty("baseColor")); // This is a property set, not a property + EXPECT_EQ(nullptr, sourceData.FindPropertySet("baseColor.texture")); // This is a property, not a property set + + EXPECT_EQ(layer1, sourceData.FindPropertySet("layer1")); + EXPECT_EQ(layer2, sourceData.FindPropertySet("layer2")); + EXPECT_EQ(blend, sourceData.FindPropertySet("blend")); + + EXPECT_EQ(layer1_baseColor, sourceData.FindPropertySet("layer1.baseColor")); + EXPECT_EQ(layer2_baseColor, sourceData.FindPropertySet("layer2.baseColor")); + + EXPECT_EQ(layer1_roughness, sourceData.FindPropertySet("layer1.roughness")); + EXPECT_EQ(layer2_roughness, sourceData.FindPropertySet("layer2.roughness")); + + EXPECT_EQ(layer1_baseColor_texture, sourceData.FindProperty("layer1.baseColor.texture")); + EXPECT_EQ(layer2_baseColor_texture, sourceData.FindProperty("layer2.baseColor.texture")); + EXPECT_EQ(layer1_roughness_texture, sourceData.FindProperty("layer1.roughness.texture")); + EXPECT_EQ(layer2_roughness_texture, sourceData.FindProperty("layer2.roughness.texture")); + + EXPECT_EQ(layer2_clearCoat, sourceData.FindPropertySet("layer2.clearCoat")); + EXPECT_EQ(layer2_clearCoat_roughness, sourceData.FindPropertySet("layer2.clearCoat.roughness")); + EXPECT_EQ(layer2_clearCoat_normal, sourceData.FindPropertySet("layer2.clearCoat.normal")); + + EXPECT_EQ(layer2_clearCoat_enabled, sourceData.FindProperty("layer2.clearCoat.enabled")); + EXPECT_EQ(layer2_clearCoat_roughness_texture, sourceData.FindProperty("layer2.clearCoat.roughness.texture")); + EXPECT_EQ(layer2_clearCoat_normal_texture, sourceData.FindProperty("layer2.clearCoat.normal.texture")); + EXPECT_EQ(layer2_clearCoat_normal_factor, sourceData.FindProperty("layer2.clearCoat.normal.factor")); + + EXPECT_EQ(blend_factor, sourceData.FindProperty("blend.factor")); + + // Check EnumeratePropertySets + + struct EnumeratePropertySetsResult + { + AZStd::string m_propertyIdContext; + const MaterialTypeSourceData::PropertySet* m_propertySet; + + void Check(AZStd::string expectedIdContext, const MaterialTypeSourceData::PropertySet* expectedPropertySet) + { + EXPECT_EQ(expectedIdContext, m_propertyIdContext); + EXPECT_EQ(expectedPropertySet, m_propertySet); + } + }; + AZStd::vector enumeratePropertySetsResults; + + sourceData.EnumeratePropertySets([&enumeratePropertySetsResults](const AZStd::string& propertyIdContext, const MaterialTypeSourceData::PropertySet* propertySet) + { + enumeratePropertySetsResults.push_back(EnumeratePropertySetsResult{propertyIdContext, propertySet}); + return true; + }); + + int resultIndex = 0; + enumeratePropertySetsResults[resultIndex++].Check("", layer1); + enumeratePropertySetsResults[resultIndex++].Check("layer1.", layer1_baseColor); + enumeratePropertySetsResults[resultIndex++].Check("layer1.", layer1_roughness); + enumeratePropertySetsResults[resultIndex++].Check("", layer2); + enumeratePropertySetsResults[resultIndex++].Check("layer2.", layer2_baseColor); + enumeratePropertySetsResults[resultIndex++].Check("layer2.", layer2_roughness); + enumeratePropertySetsResults[resultIndex++].Check("layer2.", layer2_clearCoat); + enumeratePropertySetsResults[resultIndex++].Check("layer2.clearCoat.", layer2_clearCoat_roughness); + enumeratePropertySetsResults[resultIndex++].Check("layer2.clearCoat.", layer2_clearCoat_normal); + enumeratePropertySetsResults[resultIndex++].Check("", blend); + EXPECT_EQ(resultIndex, enumeratePropertySetsResults.size()); + + // Check EnumerateProperties + + struct EnumeratePropertiesResult + { + AZStd::string m_propertyIdContext; + const MaterialTypeSourceData::PropertyDefinition* m_propertyDefinition; + + void Check(AZStd::string expectedIdContext, const MaterialTypeSourceData::PropertyDefinition* expectedPropertyDefinition) + { + EXPECT_EQ(expectedIdContext, m_propertyIdContext); + EXPECT_EQ(expectedPropertyDefinition, m_propertyDefinition); + } + }; + AZStd::vector enumeratePropertiesResults; + + sourceData.EnumerateProperties([&enumeratePropertiesResults](const AZStd::string& propertyIdContext, const MaterialTypeSourceData::PropertyDefinition* propertyDefinition) + { + enumeratePropertiesResults.push_back(EnumeratePropertiesResult{propertyIdContext, propertyDefinition}); + return true; + }); + + resultIndex = 0; + enumeratePropertiesResults[resultIndex++].Check("layer1.baseColor.", layer1_baseColor_texture); + enumeratePropertiesResults[resultIndex++].Check("layer1.roughness.", layer1_roughness_texture); + enumeratePropertiesResults[resultIndex++].Check("layer2.baseColor.", layer2_baseColor_texture); + enumeratePropertiesResults[resultIndex++].Check("layer2.roughness.", layer2_roughness_texture); + enumeratePropertiesResults[resultIndex++].Check("layer2.clearCoat.", layer2_clearCoat_enabled); + enumeratePropertiesResults[resultIndex++].Check("layer2.clearCoat.roughness.", layer2_clearCoat_roughness_texture); + enumeratePropertiesResults[resultIndex++].Check("layer2.clearCoat.normal.", layer2_clearCoat_normal_texture); + enumeratePropertiesResults[resultIndex++].Check("layer2.clearCoat.normal.", layer2_clearCoat_normal_factor); + enumeratePropertiesResults[resultIndex++].Check("blend.", blend_factor); + EXPECT_EQ(resultIndex, enumeratePropertiesResults.size()); + } + + TEST_F(MaterialTypeSourceDataTests, AddProperty_Error_AddPropertyWithInvalidName) + { + MaterialTypeSourceData sourceData; + + MaterialTypeSourceData::PropertySet* propertySet = sourceData.AddPropertySet("main"); + + ErrorMessageFinder errorMessageFinder; + errorMessageFinder.AddExpectedErrorMessage("'' is not a valid identifier"); + errorMessageFinder.AddExpectedErrorMessage("'main.' is not a valid identifier"); + errorMessageFinder.AddExpectedErrorMessage("'base-color' is not a valid identifier"); + + EXPECT_FALSE(propertySet->AddProperty("")); + EXPECT_FALSE(propertySet->AddProperty("main.")); + EXPECT_FALSE(sourceData.AddProperty("main.base-color")); + + EXPECT_TRUE(propertySet->GetProperties().empty()); + + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialTypeSourceDataTests, AddPropertySet_Error_InvalidName) + { + MaterialTypeSourceData sourceData; + + MaterialTypeSourceData::PropertySet* propertySet = sourceData.AddPropertySet("general"); + + ErrorMessageFinder errorMessageFinder; + errorMessageFinder.AddExpectedErrorMessage("'' is not a valid identifier", 2); + errorMessageFinder.AddExpectedErrorMessage("'base-color' is not a valid identifier"); + errorMessageFinder.AddExpectedErrorMessage("'look@it' is not a valid identifier"); + + EXPECT_FALSE(propertySet->AddPropertySet("")); + EXPECT_FALSE(sourceData.AddPropertySet("")); + EXPECT_FALSE(sourceData.AddPropertySet("base-color")); + EXPECT_FALSE(sourceData.AddPropertySet("general.look@it")); + + EXPECT_TRUE(propertySet->GetProperties().empty()); + + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialTypeSourceDataTests, AddProperty_Error_AddDuplicateProperty) + { + MaterialTypeSourceData sourceData; + + MaterialTypeSourceData::PropertySet* propertySet = sourceData.AddPropertySet("main"); + + ErrorMessageFinder errorMessageFinder; + errorMessageFinder.AddExpectedErrorMessage("PropertySet 'main' already contains a property named 'foo'", 2); + + EXPECT_TRUE(propertySet->AddProperty("foo")); + EXPECT_FALSE(propertySet->AddProperty("foo")); + EXPECT_FALSE(sourceData.AddProperty("main.foo")); + + EXPECT_EQ(propertySet->GetProperties().size(), 1); + + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialTypeSourceDataTests, AddProperty_Error_AddLooseProperty) + { + MaterialTypeSourceData sourceData; + ErrorMessageFinder errorMessageFinder("Property id 'foo' is invalid. Properties must be added to a PropertySet"); + EXPECT_FALSE(sourceData.AddProperty("foo")); + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialTypeSourceDataTests, AddProperty_Error_PropertySetDoesNotExist ) + { + MaterialTypeSourceData sourceData; + ErrorMessageFinder errorMessageFinder("PropertySet 'DNE' does not exists"); + EXPECT_FALSE(sourceData.AddProperty("DNE.foo")); + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialTypeSourceDataTests, AddPropertySet_Error_PropertySetDoesNotExist ) + { + MaterialTypeSourceData sourceData; + ErrorMessageFinder errorMessageFinder("PropertySet 'DNE' does not exists"); + EXPECT_FALSE(sourceData.AddPropertySet("DNE.foo")); + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialTypeSourceDataTests, AddPropertySet_Error_AddDuplicatePropertySet) + { + MaterialTypeSourceData sourceData; + + MaterialTypeSourceData::PropertySet* propertySet = sourceData.AddPropertySet("main"); + sourceData.AddPropertySet("main.level2"); + + ErrorMessageFinder errorMessageFinder; + errorMessageFinder.AddExpectedErrorMessage("PropertySet named 'main' already exists", 1); + errorMessageFinder.AddExpectedErrorMessage("PropertySet named 'level2' already exists", 2); + + EXPECT_FALSE(sourceData.AddPropertySet("main")); + EXPECT_FALSE(sourceData.AddPropertySet("main.level2")); + EXPECT_FALSE(propertySet->AddPropertySet("level2")); + + errorMessageFinder.CheckExpectedErrorsFound(); + + EXPECT_EQ(sourceData.GetPropertyLayout().m_propertySets.size(), 1); + EXPECT_EQ(propertySet->GetPropertySets().size(), 1); + } + + TEST_F(MaterialTypeSourceDataTests, AddPropertySet_Error_NameCollidesWithProperty ) + { + MaterialTypeSourceData sourceData; + sourceData.AddPropertySet("main"); + sourceData.AddProperty("main.foo"); + + ErrorMessageFinder errorMessageFinder("PropertySet name 'foo' collides with a Property of the same name"); + EXPECT_FALSE(sourceData.AddPropertySet("main.foo")); + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialTypeSourceDataTests, AddProperty_Error_NameCollidesWithPropertySet ) + { + MaterialTypeSourceData sourceData; + sourceData.AddPropertySet("main"); + sourceData.AddPropertySet("main.foo"); + + ErrorMessageFinder errorMessageFinder("Property name 'foo' collides with a PropertySet of the same name"); + EXPECT_FALSE(sourceData.AddProperty("main.foo")); + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialTypeSourceDataTests, ResolveUvStreamAsEnum) + { + MaterialTypeSourceData sourceData; + + sourceData.m_uvNameMap["UV0"] = "Tiled"; + sourceData.m_uvNameMap["UV1"] = "Unwrapped"; + sourceData.m_uvNameMap["UV2"] = "Other"; + + sourceData.AddPropertySet("a"); + sourceData.AddPropertySet("a.b"); + sourceData.AddPropertySet("c"); + sourceData.AddPropertySet("c.d"); + sourceData.AddPropertySet("c.d.e"); + + MaterialTypeSourceData::PropertyDefinition* enum1 = sourceData.AddProperty("a.enum1"); + MaterialTypeSourceData::PropertyDefinition* enum2 = sourceData.AddProperty("a.b.enum2"); + MaterialTypeSourceData::PropertyDefinition* enum3 = sourceData.AddProperty("c.d.e.enum3"); + MaterialTypeSourceData::PropertyDefinition* notEnum = sourceData.AddProperty("c.d.myFloat"); + + enum1->m_dataType = MaterialPropertyDataType::Enum; + enum2->m_dataType = MaterialPropertyDataType::Enum; + enum3->m_dataType = MaterialPropertyDataType::Enum; + notEnum->m_dataType = MaterialPropertyDataType::Float; + + enum1->m_enumIsUv = true; + enum2->m_enumIsUv = false; + enum3->m_enumIsUv = true; + + sourceData.ResolveUvEnums(); + + EXPECT_STREQ(enum1->m_enumValues[0].c_str(), "Tiled"); + EXPECT_STREQ(enum1->m_enumValues[1].c_str(), "Unwrapped"); + EXPECT_STREQ(enum1->m_enumValues[2].c_str(), "Other"); + + EXPECT_STREQ(enum3->m_enumValues[0].c_str(), "Tiled"); + EXPECT_STREQ(enum3->m_enumValues[1].c_str(), "Unwrapped"); + EXPECT_STREQ(enum3->m_enumValues[2].c_str(), "Other"); + + // enum2 is not a UV stream enum + EXPECT_EQ(enum2->m_enumValues.size(), 0); + + // myFloat is not even an enum + EXPECT_EQ(notEnum->m_enumValues.size(), 0); + } + TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_GetMaterialSrgAsset) { MaterialTypeSourceData sourceData; @@ -509,15 +820,14 @@ namespace UnitTest sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{ TestShaderFilename }); - MaterialTypeSourceData::PropertyDefinition propertySource; - propertySource.m_name = "MyBool"; - propertySource.m_displayName = "My Bool"; - propertySource.m_description = "This is a bool"; - propertySource.m_dataType = MaterialPropertyDataType::Bool; - propertySource.m_value = true; - propertySource.m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderInput, AZStd::string("m_bool") }); - sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource); - + MaterialTypeSourceData::PropertySet* propertySet = sourceData.AddPropertySet("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertySet->AddProperty("MyBool"); + property->m_displayName = "My Bool"; + property->m_description = "This is a bool"; + property->m_dataType = MaterialPropertyDataType::Bool; + property->m_value = true; + property->m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderInput, AZStd::string("m_bool") }); + auto materialTypeOutcome = sourceData.CreateMaterialTypeAsset(Uuid::CreateRandom()); EXPECT_TRUE(materialTypeOutcome.IsSuccess()); Data::Asset materialTypeAsset = materialTypeOutcome.GetValue(); @@ -525,7 +835,7 @@ namespace UnitTest const MaterialPropertyIndex propertyIndex = materialTypeAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(Name{ "general.MyBool" }); const MaterialPropertyDescriptor* propertyDescriptor = materialTypeAsset->GetMaterialPropertiesLayout()->GetPropertyDescriptor(propertyIndex); - ValidateCommonDescriptorFields(propertySource, propertyDescriptor); + ValidateCommonDescriptorFields(*property, propertyDescriptor); EXPECT_EQ(propertyDescriptor->GetOutputConnections()[0].m_itemIndex.GetIndex(), 7); } @@ -534,20 +844,19 @@ namespace UnitTest MaterialTypeSourceData sourceData; sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{ TestShaderFilename }); - - MaterialTypeSourceData::PropertyDefinition propertySource; - propertySource.m_name = "MyFloat"; - propertySource.m_displayName = "My Float"; - propertySource.m_description = "This is a float"; - propertySource.m_min = 0.0f; - propertySource.m_max = 1.0f; - propertySource.m_softMin = 0.2f; - propertySource.m_softMax = 1.0f; - propertySource.m_step = 0.01f; - propertySource.m_dataType = MaterialPropertyDataType::Float; - propertySource.m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderInput, AZStd::string("m_float") }); - sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource); - + + MaterialTypeSourceData::PropertySet* propertySet = sourceData.AddPropertySet("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertySet->AddProperty("MyFloat"); + property->m_displayName = "My Float"; + property->m_description = "This is a float"; + property->m_min = 0.0f; + property->m_max = 1.0f; + property->m_softMin = 0.2f; + property->m_softMax = 1.0f; + property->m_step = 0.01f; + property->m_dataType = MaterialPropertyDataType::Float; + property->m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderInput, AZStd::string("m_float") }); + auto materialTypeOutcome = sourceData.CreateMaterialTypeAsset(Uuid::CreateRandom()); EXPECT_TRUE(materialTypeOutcome.IsSuccess()); Data::Asset materialTypeAsset = materialTypeOutcome.GetValue(); @@ -555,7 +864,7 @@ namespace UnitTest const MaterialPropertyIndex propertyIndex = materialTypeAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(Name{"general.MyFloat" }); const MaterialPropertyDescriptor* propertyDescriptor = materialTypeAsset->GetMaterialPropertiesLayout()->GetPropertyDescriptor(propertyIndex); - ValidateCommonDescriptorFields(propertySource, propertyDescriptor); + ValidateCommonDescriptorFields(*property, propertyDescriptor); EXPECT_EQ(propertyDescriptor->GetOutputConnections()[0].m_itemIndex.GetIndex(), 1); } @@ -564,15 +873,14 @@ namespace UnitTest MaterialTypeSourceData sourceData; sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{ TestShaderFilename }); - - MaterialTypeSourceData::PropertyDefinition propertySource; - propertySource.m_name = "MyImage"; - propertySource.m_displayName = "My Image"; - propertySource.m_description = "This is an image"; - propertySource.m_dataType = MaterialPropertyDataType::Image; - propertySource.m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderInput, AZStd::string("m_image") }); - sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource); - + + MaterialTypeSourceData::PropertySet* propertySet = sourceData.AddPropertySet("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertySet->AddProperty("MyImage"); + property->m_displayName = "My Image"; + property->m_description = "This is an image"; + property->m_dataType = MaterialPropertyDataType::Image; + property->m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderInput, AZStd::string("m_image") }); + auto materialTypeOutcome = sourceData.CreateMaterialTypeAsset(Uuid::CreateRandom()); EXPECT_TRUE(materialTypeOutcome.IsSuccess()); Data::Asset materialTypeAsset = materialTypeOutcome.GetValue(); @@ -580,7 +888,7 @@ namespace UnitTest const MaterialPropertyIndex propertyIndex = materialTypeAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(Name{"general.MyImage" }); const MaterialPropertyDescriptor* propertyDescriptor = materialTypeAsset->GetMaterialPropertiesLayout()->GetPropertyDescriptor(propertyIndex); - ValidateCommonDescriptorFields(propertySource, propertyDescriptor); + ValidateCommonDescriptorFields(*property, propertyDescriptor); EXPECT_EQ(propertyDescriptor->GetOutputConnections()[0].m_itemIndex.GetIndex(), 0); } @@ -589,21 +897,20 @@ namespace UnitTest MaterialTypeSourceData sourceData; sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{TestShaderFilename}); - - MaterialTypeSourceData::PropertyDefinition propertySource; - propertySource.m_name = "MyInt"; - propertySource.m_displayName = "My Integer"; - propertySource.m_dataType = MaterialPropertyDataType::Int; - propertySource.m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{MaterialPropertyOutputType::ShaderOption, AZStd::string("o_foo"), 0}); - sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource); - + + MaterialTypeSourceData::PropertySet* propertySet = sourceData.AddPropertySet("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertySet->AddProperty("MyInt"); + property->m_displayName = "My Integer"; + property->m_dataType = MaterialPropertyDataType::Int; + property->m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{MaterialPropertyOutputType::ShaderOption, AZStd::string("o_foo"), 0}); + auto materialTypeOutcome = sourceData.CreateMaterialTypeAsset(Uuid::CreateRandom()); EXPECT_TRUE(materialTypeOutcome.IsSuccess()); Data::Asset materialTypeAsset = materialTypeOutcome.GetValue(); const MaterialPropertyDescriptor* propertyDescriptor = materialTypeAsset->GetMaterialPropertiesLayout()->GetPropertyDescriptor(MaterialPropertyIndex{0}); - ValidateCommonDescriptorFields(propertySource, propertyDescriptor); + ValidateCommonDescriptorFields(*property, propertyDescriptor); EXPECT_EQ(propertyDescriptor->GetOutputConnections()[0].m_itemIndex.GetIndex(), 1); } @@ -612,13 +919,12 @@ namespace UnitTest MaterialTypeSourceData sourceData; sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{TestShaderFilename}); - - MaterialTypeSourceData::PropertyDefinition propertySource; - propertySource.m_name = "MyInt"; - propertySource.m_dataType = MaterialPropertyDataType::Int; - propertySource.m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{MaterialPropertyOutputType::ShaderOption, AZStd::string("DoesNotExist"), 0}); - sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource); - + + MaterialTypeSourceData::PropertySet* propertySet = sourceData.AddPropertySet("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertySet->AddProperty("MyInt"); + property->m_dataType = MaterialPropertyDataType::Int; + property->m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{MaterialPropertyOutputType::ShaderOption, AZStd::string("DoesNotExist"), 0}); + AZ_TEST_START_TRACE_SUPPRESSION; auto materialTypeOutcome = sourceData.CreateMaterialTypeAsset(Uuid::CreateRandom()); AZ_TEST_STOP_TRACE_SUPPRESSION(2); // There happens to be an extra assert for "Cannot continue building MaterialAsset because 1 error(s) reported" @@ -626,66 +932,140 @@ namespace UnitTest EXPECT_FALSE(materialTypeOutcome.IsSuccess()); } - TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_Error_InvalidGroupNameId) + TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_Error_InvalidGroupName) { + const AZStd::string inputJson = R"( + { + "propertyLayout": { + "propertySets": [ + { + "name": "not a valid name because it has spaces", + "properties": [ + { + "name": "foo", + "type": "Bool" + } + ] + } + ] + } + } + )"; + MaterialTypeSourceData sourceData; - - MaterialTypeSourceData::PropertyDefinition propertySource; - propertySource.m_dataType = MaterialPropertyDataType::Int; - - propertySource.m_name = "a"; - sourceData.m_propertyLayout.m_properties["not a valid name because it has spaces"].push_back(propertySource); - - // Expected errors: - // Group name 'not a valid name because it has spaces' is not a valid identifier. - // Warning: Cannot create material property with invalid ID 'not a valid name because it has spaces'. - // Failed to build MaterialAsset because 1 warning(s) reported - AZ_TEST_START_TRACE_SUPPRESSION; + JsonTestResult loadResult = LoadTestDataFromJson(sourceData, inputJson); + EXPECT_EQ(loadResult.m_jsonResultCode.GetProcessing(), JsonSerializationResult::Processing::Completed); + + ErrorMessageFinder errorMessageFinder{"'not a valid name because it has spaces' is not a valid identifier"}; auto materialTypeOutcome = sourceData.CreateMaterialTypeAsset(Uuid::CreateRandom()); - AZ_TEST_STOP_TRACE_SUPPRESSION(2); - EXPECT_FALSE(materialTypeOutcome.IsSuccess()); + errorMessageFinder.CheckExpectedErrorsFound(); } - TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_Error_InvalidPropertyNameId) + TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_Error_InvalidPropertyName) { + const AZStd::string inputJson = R"( + { + "propertyLayout": { + "propertySets": [ + { + "name": "general", + "properties": [ + { + "name": "not a valid name because it has spaces", + "type": "Bool" + } + ] + } + ] + } + } + )"; + MaterialTypeSourceData sourceData; - - MaterialTypeSourceData::PropertyDefinition propertySource; - propertySource.m_dataType = MaterialPropertyDataType::Int; - - propertySource.m_name = "not a valid name because it has spaces"; - sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource); - - // Expected errors: - // Property name 'not a valid name because it has spaces' is not a valid identifier. - // Warning: Cannot create material property with invalid ID 'not a valid name because it has spaces'. - // Failed to build MaterialAsset because 1 warning(s) reported - AZ_TEST_START_TRACE_SUPPRESSION; + JsonTestResult loadResult = LoadTestDataFromJson(sourceData, inputJson); + EXPECT_EQ(loadResult.m_jsonResultCode.GetProcessing(), JsonSerializationResult::Processing::Completed); + + ErrorMessageFinder errorMessageFinder{"'not a valid name because it has spaces' is not a valid identifier"}; auto materialTypeOutcome = sourceData.CreateMaterialTypeAsset(Uuid::CreateRandom()); - AZ_TEST_STOP_TRACE_SUPPRESSION(2); - EXPECT_FALSE(materialTypeOutcome.IsSuccess()); + errorMessageFinder.CheckExpectedErrorsFound(); } TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_Error_DuplicatePropertyId) { + const AZStd::string inputJson = R"( + { + "propertyLayout": { + "propertySets": [ + { + "name": "general", + "properties": [ + { + "name": "foo", + "type": "Bool" + }, + { + "name": "foo", + "type": "Bool" + } + ] + } + ] + } + } + )"; + MaterialTypeSourceData sourceData; - - MaterialTypeSourceData::PropertyDefinition propertySource; - propertySource.m_dataType = MaterialPropertyDataType::Int; - propertySource.m_name = "a"; - sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource); - sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource); - - // Expected errors: - // Material property 'general.a': A property with this ID already exists. - // Cannot continue building MaterialAsset because 1 error(s) reported - AZ_TEST_START_TRACE_SUPPRESSION; + JsonTestResult loadResult = LoadTestDataFromJson(sourceData, inputJson); + EXPECT_EQ(loadResult.m_jsonResultCode.GetProcessing(), JsonSerializationResult::Processing::Completed); + + ErrorMessageFinder errorMessageFinder("Material property 'general.foo': A property with this ID already exists"); + errorMessageFinder.AddExpectedErrorMessage("Cannot continue building MaterialTypeAsset"); auto materialTypeOutcome = sourceData.CreateMaterialTypeAsset(Uuid::CreateRandom()); - AZ_TEST_STOP_TRACE_SUPPRESSION(2); - EXPECT_FALSE(materialTypeOutcome.IsSuccess()); + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_Error_PropertyAndPropertySetNameCollision) + { + const AZStd::string inputJson = R"( + { + "propertyLayout": { + "propertySets": [ + { + "name": "general", + "properties": [ + { + "name": "foo", + "type": "Bool" + } + ], + "propertySets": [ + { + "name": "foo", + "properties": [ + { + "name": "bar", + "type": "Bool" + } + ] + } + ] + } + ] + } + } + )"; + + MaterialTypeSourceData sourceData; + JsonTestResult loadResult = LoadTestDataFromJson(sourceData, inputJson); + EXPECT_EQ(loadResult.m_jsonResultCode.GetProcessing(), JsonSerializationResult::Processing::Completed); + + ErrorMessageFinder errorMessageFinder("Material property 'general.foo' collides with a PropertySet with the same ID"); + auto materialTypeOutcome = sourceData.CreateMaterialTypeAsset(Uuid::CreateRandom()); + EXPECT_FALSE(materialTypeOutcome.IsSuccess()); + errorMessageFinder.CheckExpectedErrorsFound(); } TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_PropertyConnectedToMultipleOutputs) @@ -736,25 +1116,25 @@ namespace UnitTest sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{ "shaderA.shader" }); sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{ "shaderB.shader" }); sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{ "shaderC.shader" }); + + MaterialTypeSourceData::PropertySet* propertySet = sourceData.AddPropertySet("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertySet->AddProperty("MyInt"); - MaterialTypeSourceData::PropertyDefinition propertySource; - propertySource.m_name = "MyInt"; - propertySource.m_displayName = "Integer"; - propertySource.m_description = "Integer property that is connected to multiple shader settings"; - propertySource.m_dataType = MaterialPropertyDataType::Int; + property->m_displayName = "Integer"; + property->m_description = "Integer property that is connected to multiple shader settings"; + property->m_dataType = MaterialPropertyDataType::Int; // The value maps to m_int in the SRG - propertySource.m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderInput, AZStd::string("m_int") }); + property->m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderInput, AZStd::string("m_int") }); // The value also maps to m_uint in the SRG - propertySource.m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderInput, AZStd::string("m_uint") }); + property->m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderInput, AZStd::string("m_uint") }); // The value also maps to the first shader's "o_speed" option - propertySource.m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderOption, AZStd::string("o_speed"), 0 }); + property->m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderOption, AZStd::string("o_speed"), 0 }); // The value also maps to the second shader's "o_speed" option - propertySource.m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderOption, AZStd::string("o_speed"), 1 }); + property->m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderOption, AZStd::string("o_speed"), 1 }); // This case doesn't specify an index, so it will apply to all shaders that have a "o_efficiency", which means it will create two outputs in the property descriptor. - propertySource.m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderOption, AZStd::string("o_efficiency") }); - - sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource); + property->m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderOption, AZStd::string("o_efficiency") }); + // Do the actual test... @@ -795,15 +1175,15 @@ namespace UnitTest TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_PropertyWithShaderInputFunctor) { MaterialTypeSourceData sourceData; + + MaterialTypeSourceData::PropertySet* propertySet = sourceData.AddPropertySet("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertySet->AddProperty("floatForFunctor"); - MaterialTypeSourceData::PropertyDefinition propertySource; - propertySource.m_name = "NonAliasFloat"; - propertySource.m_displayName = "Non-Alias Float"; - propertySource.m_description = "This float is processed by a functor, not with a direct alias"; - propertySource.m_dataType = MaterialPropertyDataType::Float; - // Note that we don't fill propertySource.m_aliasOutputId because this is not an aliased property - sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource); - + property->m_displayName = "Float for Functor"; + property->m_description = "This float is processed by a functor, not with a direct connection"; + property->m_dataType = MaterialPropertyDataType::Float; + // Note that we don't fill property->m_outputConnections because this is not an aliased property + sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{TestShaderFilename}); sourceData.m_materialFunctorSourceData.push_back( @@ -811,7 +1191,7 @@ namespace UnitTest ( aznew MaterialFunctorSourceDataHolder ( - aznew Splat3FunctorSourceData{ "general.NonAliasFloat", "m_float3" } + aznew Splat3FunctorSourceData{ "general.floatForFunctor", "m_float3" } ) ) ); @@ -820,10 +1200,10 @@ namespace UnitTest EXPECT_TRUE(materialTypeOutcome.IsSuccess()); Data::Asset materialTypeAsset = materialTypeOutcome.GetValue(); - const MaterialPropertyIndex propertyIndex = materialTypeAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(Name{"general.NonAliasFloat" }); + const MaterialPropertyIndex propertyIndex = materialTypeAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(Name{"general.floatForFunctor" }); const MaterialPropertyDescriptor* propertyDescriptor = materialTypeAsset->GetMaterialPropertiesLayout()->GetPropertyDescriptor(propertyIndex); - ValidateCommonDescriptorFields(propertySource, propertyDescriptor); + ValidateCommonDescriptorFields(*property, propertyDescriptor); EXPECT_EQ(1, materialTypeAsset->GetMaterialFunctors().size()); auto shaderInputFunctor = azrtti_cast(materialTypeAsset->GetMaterialFunctors()[0].get()); @@ -841,15 +1221,13 @@ namespace UnitTest sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{TestShaderFilename}); sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{TestShaderFilename}); - MaterialTypeSourceData::PropertyDefinition propertySource; - propertySource.m_name = "EnableSpecialPassA"; - propertySource.m_displayName = "Enable Special Pass"; - propertySource.m_description = "This is a bool to enable an extra shader/pass"; - propertySource.m_dataType = MaterialPropertyDataType::Bool; - // Note that we don't fill propertySource.m_outputConnections because this is not a direct-connected property - sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource); - propertySource.m_name = "EnableSpecialPassB"; - sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource); + MaterialTypeSourceData::PropertySet* propertySet = sourceData.AddPropertySet("general"); + MaterialTypeSourceData::PropertyDefinition* property1 = propertySet->AddProperty("EnableSpecialPassA"); + MaterialTypeSourceData::PropertyDefinition* property2 = propertySet->AddProperty("EnableSpecialPassB"); + + property1->m_displayName = property2->m_displayName = "Enable Special Pass"; + property1->m_description = property2->m_description = "This is a bool to enable an extra shader/pass"; + property1->m_dataType = property2->m_dataType = MaterialPropertyDataType::Bool; sourceData.m_materialFunctorSourceData.push_back( Ptr @@ -880,8 +1258,8 @@ namespace UnitTest const MaterialPropertyIndex propertyBIndex = materialTypeAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(Name{"general.EnableSpecialPassB"}); const MaterialPropertyDescriptor* propertyBDescriptor = materialTypeAsset->GetMaterialPropertiesLayout()->GetPropertyDescriptor(propertyBIndex); - ValidateCommonDescriptorFields(propertySource, propertyADescriptor); - ValidateCommonDescriptorFields(propertySource, propertyBDescriptor); + ValidateCommonDescriptorFields(*sourceData.FindProperty("general.EnableSpecialPassA"), propertyADescriptor); + ValidateCommonDescriptorFields(*sourceData.FindProperty("general.EnableSpecialPassB"), propertyBDescriptor); EXPECT_EQ(2, materialTypeAsset->GetMaterialFunctors().size()); auto functorA = azrtti_cast(materialTypeAsset->GetMaterialFunctors()[0].get()); @@ -900,13 +1278,13 @@ namespace UnitTest sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{TestShaderFilename}); sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{TestShaderFilename}); + + MaterialTypeSourceData::PropertySet* propertySet = sourceData.AddPropertySet("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertySet->AddProperty("MyProperty"); - MaterialTypeSourceData::PropertyDefinition propertySource; - propertySource.m_name = "MyProperty"; - propertySource.m_dataType = MaterialPropertyDataType::Bool; - // Note that we don't fill propertySource.m_outputConnections because this is not a direct-connected property - sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource); - + property->m_dataType = MaterialPropertyDataType::Bool; + // Note that we don't fill property->m_outputConnections because this is not a direct-connected property + sourceData.m_materialFunctorSourceData.push_back( Ptr ( @@ -928,6 +1306,45 @@ namespace UnitTest EXPECT_TRUE(materialTypeAsset->GetShaderCollection()[0].MaterialOwnsShaderOption(Name{"o_foo"})); EXPECT_TRUE(materialTypeAsset->GetShaderCollection()[0].MaterialOwnsShaderOption(Name{"o_bar"})); } + + TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_FunctorIsInsidePropertySet) + { + MaterialTypeSourceData sourceData; + + MaterialTypeSourceData::PropertySet* propertySet = sourceData.AddPropertySet("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertySet->AddProperty("floatForFunctor"); + + property->m_dataType = MaterialPropertyDataType::Float; + + sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{TestShaderFilename}); + + sourceData.m_materialFunctorSourceData.push_back( + Ptr + ( + aznew MaterialFunctorSourceDataHolder + ( + aznew Splat3FunctorSourceData{ "general.floatForFunctor", "m_float3" } + ) + ) + ); + + auto materialTypeOutcome = sourceData.CreateMaterialTypeAsset(Uuid::CreateRandom()); + EXPECT_TRUE(materialTypeOutcome.IsSuccess()); + Data::Asset materialTypeAsset = materialTypeOutcome.GetValue(); + + const MaterialPropertyIndex propertyIndex = materialTypeAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(Name{"general.floatForFunctor" }); + const MaterialPropertyDescriptor* propertyDescriptor = materialTypeAsset->GetMaterialPropertiesLayout()->GetPropertyDescriptor(propertyIndex); + + ValidateCommonDescriptorFields(*property, propertyDescriptor); + + EXPECT_EQ(1, materialTypeAsset->GetMaterialFunctors().size()); + auto shaderInputFunctor = azrtti_cast(materialTypeAsset->GetMaterialFunctors()[0].get()); + EXPECT_TRUE(nullptr != shaderInputFunctor); + EXPECT_EQ(propertyIndex, shaderInputFunctor->m_floatIndex); + + const RHI::ShaderInputConstantIndex expectedVector3Index = materialTypeAsset->GetMaterialSrgLayout()->FindShaderInputConstantIndex(Name{ "m_float3" }); + EXPECT_EQ(expectedVector3Index, shaderInputFunctor->m_vector3Index); + } TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_PropertyValues_AllTypes) { @@ -937,23 +1354,23 @@ namespace UnitTest auto addProperty = [&sourceData](MaterialPropertyDataType dateType, const char* propertyName, const char* srgConstantName, const AZ::RPI::MaterialPropertyValue& value) { - MaterialTypeSourceData::PropertyDefinition propertySource; - propertySource.m_name = propertyName; - propertySource.m_dataType = dateType; - propertySource.m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderInput, AZStd::string(srgConstantName) }); - propertySource.m_value = value; - sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource); + MaterialTypeSourceData::PropertyDefinition* property = sourceData.AddProperty(propertyName); + property->m_dataType = dateType; + property->m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ MaterialPropertyOutputType::ShaderInput, AZStd::string(srgConstantName) }); + property->m_value = value; }; + + sourceData.AddPropertySet("general"); - addProperty(MaterialPropertyDataType::Bool, "MyBool", "m_bool", true); - addProperty(MaterialPropertyDataType::Float, "MyFloat", "m_float", 1.2f); - addProperty(MaterialPropertyDataType::Int, "MyInt", "m_int", -12); - addProperty(MaterialPropertyDataType::UInt, "MyUInt", "m_uint", 12u); - addProperty(MaterialPropertyDataType::Vector2, "MyFloat2", "m_float2", AZ::Vector2{1.1f, 2.2f}); - addProperty(MaterialPropertyDataType::Vector3, "MyFloat3", "m_float3", AZ::Vector3{3.3f, 4.4f, 5.5f}); - addProperty(MaterialPropertyDataType::Vector4, "MyFloat4", "m_float4", AZ::Vector4{6.6f, 7.7f, 8.8f, 9.9f}); - addProperty(MaterialPropertyDataType::Color, "MyColor", "m_color", AZ::Color{0.1f, 0.2f, 0.3f, 0.4f}); - addProperty(MaterialPropertyDataType::Image, "MyImage", "m_image", AZStd::string{TestImageFilename}); + addProperty(MaterialPropertyDataType::Bool, "general.MyBool", "m_bool", true); + addProperty(MaterialPropertyDataType::Float, "general.MyFloat", "m_float", 1.2f); + addProperty(MaterialPropertyDataType::Int, "general.MyInt", "m_int", -12); + addProperty(MaterialPropertyDataType::UInt, "general.MyUInt", "m_uint", 12u); + addProperty(MaterialPropertyDataType::Vector2, "general.MyFloat2", "m_float2", AZ::Vector2{1.1f, 2.2f}); + addProperty(MaterialPropertyDataType::Vector3, "general.MyFloat3", "m_float3", AZ::Vector3{3.3f, 4.4f, 5.5f}); + addProperty(MaterialPropertyDataType::Vector4, "general.MyFloat4", "m_float4", AZ::Vector4{6.6f, 7.7f, 8.8f, 9.9f}); + addProperty(MaterialPropertyDataType::Color, "general.MyColor", "m_color", AZ::Color{0.1f, 0.2f, 0.3f, 0.4f}); + addProperty(MaterialPropertyDataType::Image, "general.MyImage", "m_image", AZStd::string{TestImageFilename}); auto materialTypeOutcome = sourceData.CreateMaterialTypeAsset(Uuid::CreateRandom()); EXPECT_TRUE(materialTypeOutcome.IsSuccess()); @@ -970,6 +1387,88 @@ namespace UnitTest CheckPropertyValue>(materialTypeAsset, Name{"general.MyImage"}, m_testImageAsset); } + TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_NestedPropertySets) + { + RHI::Ptr layeredMaterialSrgLayout = RHI::ShaderResourceGroupLayout::Create(); + layeredMaterialSrgLayout->SetName(Name{"MaterialSrg"}); + layeredMaterialSrgLayout->SetBindingSlot(SrgBindingSlot::Material); + layeredMaterialSrgLayout->AddShaderInput(RHI::ShaderInputImageDescriptor{ Name{ "m_layer1_baseColor_texture" }, RHI::ShaderInputImageAccess::Read, RHI::ShaderInputImageType::Image2D, 1, 1 }); + layeredMaterialSrgLayout->AddShaderInput(RHI::ShaderInputImageDescriptor{ Name{ "m_layer1_roughness_texture" }, RHI::ShaderInputImageAccess::Read, RHI::ShaderInputImageType::Image2D, 1, 1 }); + layeredMaterialSrgLayout->AddShaderInput(RHI::ShaderInputImageDescriptor{ Name{ "m_layer2_baseColor_texture" }, RHI::ShaderInputImageAccess::Read, RHI::ShaderInputImageType::Image2D, 1, 1 }); + layeredMaterialSrgLayout->AddShaderInput(RHI::ShaderInputImageDescriptor{ Name{ "m_layer2_roughness_texture" }, RHI::ShaderInputImageAccess::Read, RHI::ShaderInputImageType::Image2D, 1, 1 }); + layeredMaterialSrgLayout->AddShaderInput(RHI::ShaderInputImageDescriptor{ Name{ "m_layer2_clearCoat_roughness_texture" }, RHI::ShaderInputImageAccess::Read, RHI::ShaderInputImageType::Image2D, 1, 1 }); + layeredMaterialSrgLayout->AddShaderInput(RHI::ShaderInputImageDescriptor{ Name{ "m_layer2_clearCoat_normal_texture" }, RHI::ShaderInputImageAccess::Read, RHI::ShaderInputImageType::Image2D, 1, 1 }); + layeredMaterialSrgLayout->AddShaderInput(RHI::ShaderInputConstantDescriptor{ Name{ "m_layer2_clearCoat_normal_factor" }, 0, 4, 0 }); + layeredMaterialSrgLayout->AddShaderInput(RHI::ShaderInputConstantDescriptor{ Name{ "m_blendFactor" }, 4, 4, 0 }); + layeredMaterialSrgLayout->Finalize(); + + AZStd::vector boolOptionValues; + boolOptionValues.push_back({Name("False"), RPI::ShaderOptionValue(0)}); + boolOptionValues.push_back({Name("True"), RPI::ShaderOptionValue(1)}); + Ptr shaderOptionsLayout = ShaderOptionGroupLayout::Create(); + uint32_t order = 0; + shaderOptionsLayout->AddShaderOption(ShaderOptionDescriptor{Name{"o_layer2_clearCoat_enable"}, ShaderOptionType::Boolean, 0, order++, boolOptionValues, Name{"False"}}); + shaderOptionsLayout->Finalize(); + + Data::Asset layeredMaterialShaderAsset = CreateTestShaderAsset(Uuid::CreateRandom(), layeredMaterialSrgLayout, shaderOptionsLayout); + + Data::AssetInfo testShaderAssetInfo; + testShaderAssetInfo.m_assetId = layeredMaterialShaderAsset.GetId(); + m_assetSystemStub.RegisterSourceInfo("layeredMaterial.shader", testShaderAssetInfo, ""); + + MaterialTypeSourceData sourceData; + + sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{ "layeredMaterial.shader" }); + + auto addSrgProperty = [&sourceData](MaterialPropertyDataType dateType, MaterialPropertyOutputType connectionType, const char* propertyName, const char* srgConstantName, const AZ::RPI::MaterialPropertyValue& value) + { + MaterialTypeSourceData::PropertyDefinition* property = sourceData.AddProperty(propertyName); + property->m_dataType = dateType; + property->m_outputConnections.push_back(MaterialTypeSourceData::PropertyConnection{ connectionType, AZStd::string(srgConstantName) }); + property->m_value = value; + }; + + sourceData.AddPropertySet("layer1"); + sourceData.AddPropertySet("layer2"); + sourceData.AddPropertySet("blend"); + sourceData.AddPropertySet("layer1.baseColor"); + sourceData.AddPropertySet("layer2.baseColor"); + sourceData.AddPropertySet("layer1.roughness"); + sourceData.AddPropertySet("layer2.roughness"); + sourceData.AddPropertySet("layer2.clearCoat"); + sourceData.AddPropertySet("layer2.clearCoat.roughness"); + sourceData.AddPropertySet("layer2.clearCoat.normal"); + + addSrgProperty(MaterialPropertyDataType::Image, MaterialPropertyOutputType::ShaderInput, "layer1.baseColor.texture", "m_layer1_baseColor_texture", AZStd::string{TestImageFilename}); + addSrgProperty(MaterialPropertyDataType::Image, MaterialPropertyOutputType::ShaderInput, "layer1.roughness.texture", "m_layer1_roughness_texture", AZStd::string{TestImageFilename}); + addSrgProperty(MaterialPropertyDataType::Image, MaterialPropertyOutputType::ShaderInput, "layer2.baseColor.texture", "m_layer2_baseColor_texture", AZStd::string{TestImageFilename}); + addSrgProperty(MaterialPropertyDataType::Image, MaterialPropertyOutputType::ShaderInput, "layer2.roughness.texture", "m_layer2_roughness_texture", AZStd::string{TestImageFilename}); + addSrgProperty(MaterialPropertyDataType::Bool, MaterialPropertyOutputType::ShaderOption, "layer2.clearCoat.enabled", "o_layer2_clearCoat_enable", true); + addSrgProperty(MaterialPropertyDataType::Image, MaterialPropertyOutputType::ShaderInput, "layer2.clearCoat.roughness.texture", "m_layer2_clearCoat_roughness_texture", AZStd::string{TestImageFilename}); + addSrgProperty(MaterialPropertyDataType::Image, MaterialPropertyOutputType::ShaderInput, "layer2.clearCoat.normal.texture", "m_layer2_clearCoat_normal_texture", AZStd::string{TestImageFilename}); + addSrgProperty(MaterialPropertyDataType::Float, MaterialPropertyOutputType::ShaderInput, "layer2.clearCoat.normal.factor", "m_layer2_clearCoat_normal_factor", 0.4f); + addSrgProperty(MaterialPropertyDataType::Float, MaterialPropertyOutputType::ShaderInput, "blend.factor", "m_blendFactor", 0.5f); + + auto materialTypeOutcome = sourceData.CreateMaterialTypeAsset(Uuid::CreateRandom()); + EXPECT_TRUE(materialTypeOutcome.IsSuccess()); + Data::Asset materialTypeAsset = materialTypeOutcome.GetValue(); + + CheckPropertyValue>(materialTypeAsset, Name{"layer1.baseColor.texture"}, m_testImageAsset); + CheckPropertyValue>(materialTypeAsset, Name{"layer1.roughness.texture"}, m_testImageAsset); + CheckPropertyValue>(materialTypeAsset, Name{"layer2.baseColor.texture"}, m_testImageAsset); + CheckPropertyValue>(materialTypeAsset, Name{"layer2.roughness.texture"}, m_testImageAsset); + CheckPropertyValue(materialTypeAsset, Name{"layer2.clearCoat.enabled"}, true); + CheckPropertyValue>(materialTypeAsset, Name{"layer2.clearCoat.roughness.texture"}, m_testImageAsset); + CheckPropertyValue>(materialTypeAsset, Name{"layer2.clearCoat.normal.texture"}, m_testImageAsset); + CheckPropertyValue(materialTypeAsset, Name{"layer2.clearCoat.normal.factor"}, 0.4f); + CheckPropertyValue(materialTypeAsset, Name{"blend.factor"}, 0.5f); + + // Note it might be nice to check that the right property connections are prescribed in the final MaterialTypeAsset, + // but it's not really necessary because CreateMaterialTypeAsset reports errors when a connection target is not found + // in the shader options layout or SRG layout. If one of the output names like "m_layer2_roughness_texture" is wrong + // these errors will cause this test to fail. + } + TEST_F(MaterialTypeSourceDataTests, LoadAndStoreJson_AllFields) { // Note that serialization of individual fields within material properties is thoroughly tested in @@ -980,46 +1479,92 @@ namespace UnitTest "description": "This is a general description about the material", "propertyLayout": { "version": 2, - "groups": [ + "propertySets": [ { "name": "groupA", "displayName": "Property Group A", - "description": "Description of property group A" + "description": "Description of property group A", + "properties": [ + { + "name": "foo", + "type": "Bool", + "defaultValue": true + }, + { + "name": "bar", + "type": "Image", + "defaultValue": "Default.png", + "visibility": "Hidden" + } + ], + "functors": [ + { + "type": "EnableShader", + "args": { + "enablePassProperty": "foo", + "shaderIndex": 1 + } + } + ] }, { "name": "groupB", "displayName": "Property Group B", - "description": "Description of property group B" + "description": "Description of property group B", + "properties": [ + { + "name": "foo", + "type": "Float", + "defaultValue": 0.5 + }, + { + "name": "bar", + "type": "Color", + "defaultValue": [0.5, 0.5, 0.5], + "visibility": "Disabled" + } + ], + "functors": [ + { + "type": "Splat3", + "args": { + "floatPropertyInput": "foo", + "float3ShaderSettingOutput": "m_someFloat3" + } + } + ] + }, + { + "name": "groupC", + "displayName": "Property Group C", + "description": "Property group C has a nested property set", + "propertySets": [ + { + "name": "groupD", + "displayName": "Property Group D", + "description": "Description of property group D", + "properties": [ + { + "name": "foo", + "type": "Int", + "defaultValue": -1 + } + ] + }, + { + "name": "groupE", + "displayName": "Property Group E", + "description": "Description of property group E", + "properties": [ + { + "name": "bar", + "type": "UInt" + } + ] + } + ] } - ], - "properties": { - "groupA": [ - { - "name": "foo", - "type": "Bool", - "defaultValue": true - }, - { - "name": "bar", - "type": "Image", - "defaultValue": "Default.png", - "visibility": "Hidden" - } - ], - "groupB": [ - { - "name": "foo", - "type": "Float", - "defaultValue": 0.5 - }, - { - "name": "bar", - "type": "Color", - "defaultValue": [0.5, 0.5, 0.5], - "visibility": "Disabled" - } - ] - } + ] }, "shaders": [ { @@ -1040,17 +1585,9 @@ namespace UnitTest ], "functors": [ { - "type": "EnableShader", + "type": "SetShaderOption", "args": { - "enablePassProperty": "groupA.foo", - "shaderIndex": 1 - } - }, - { - "type": "Splat3", - "args": { - "floatPropertyInput": "groupB.foo", - "float3ShaderSettingOutput": "m_someFloat3" + "enableProperty": "groupA.foo" } } ] @@ -1062,35 +1599,72 @@ namespace UnitTest EXPECT_EQ(material.m_description, "This is a general description about the material"); - EXPECT_EQ(material.m_propertyLayout.m_version, 2); + EXPECT_EQ(material.GetPropertyLayout().m_version, 2); - EXPECT_EQ(material.m_propertyLayout.m_groups.size(), 2); - EXPECT_TRUE(material.FindGroup("groupA") != nullptr); - EXPECT_TRUE(material.FindGroup("groupB") != nullptr); - EXPECT_EQ(material.FindGroup("groupA")->m_displayName, "Property Group A"); - EXPECT_EQ(material.FindGroup("groupB")->m_displayName, "Property Group B"); - EXPECT_EQ(material.FindGroup("groupA")->m_description, "Description of property group A"); - EXPECT_EQ(material.FindGroup("groupB")->m_description, "Description of property group B"); + EXPECT_EQ(material.GetPropertyLayout().m_propertySets.size(), 3); + EXPECT_TRUE(material.FindPropertySet("groupA") != nullptr); + EXPECT_TRUE(material.FindPropertySet("groupB") != nullptr); + EXPECT_TRUE(material.FindPropertySet("groupC") != nullptr); + EXPECT_TRUE(material.FindPropertySet("groupC.groupD") != nullptr); + EXPECT_TRUE(material.FindPropertySet("groupC.groupE") != nullptr); + EXPECT_EQ(material.FindPropertySet("groupA")->GetDisplayName(), "Property Group A"); + EXPECT_EQ(material.FindPropertySet("groupB")->GetDisplayName(), "Property Group B"); + EXPECT_EQ(material.FindPropertySet("groupC")->GetDisplayName(), "Property Group C"); + EXPECT_EQ(material.FindPropertySet("groupC.groupD")->GetDisplayName(), "Property Group D"); + EXPECT_EQ(material.FindPropertySet("groupC.groupE")->GetDisplayName(), "Property Group E"); + EXPECT_EQ(material.FindPropertySet("groupA")->GetDescription(), "Description of property group A"); + EXPECT_EQ(material.FindPropertySet("groupB")->GetDescription(), "Description of property group B"); + EXPECT_EQ(material.FindPropertySet("groupC")->GetDescription(), "Property group C has a nested property set"); + EXPECT_EQ(material.FindPropertySet("groupC.groupD")->GetDescription(), "Description of property group D"); + EXPECT_EQ(material.FindPropertySet("groupC.groupE")->GetDescription(), "Description of property group E"); + EXPECT_EQ(material.FindPropertySet("groupA")->GetProperties().size(), 2); + EXPECT_EQ(material.FindPropertySet("groupB")->GetProperties().size(), 2); + EXPECT_EQ(material.FindPropertySet("groupC")->GetProperties().size(), 0); + EXPECT_EQ(material.FindPropertySet("groupC.groupD")->GetProperties().size(), 1); + EXPECT_EQ(material.FindPropertySet("groupC.groupE")->GetProperties().size(), 1); + + EXPECT_NE(material.FindProperty("groupA.foo"), nullptr); + EXPECT_NE(material.FindProperty("groupA.bar"), nullptr); + EXPECT_NE(material.FindProperty("groupB.foo"), nullptr); + EXPECT_NE(material.FindProperty("groupB.bar"), nullptr); + EXPECT_NE(material.FindProperty("groupC.groupD.foo"), nullptr); + EXPECT_NE(material.FindProperty("groupC.groupE.bar"), nullptr); - EXPECT_EQ(material.m_propertyLayout.m_properties.size(), 2); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"].size(), 2); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"].size(), 2); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"][0].m_name, "foo"); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"][1].m_name, "bar"); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"][0].m_name, "foo"); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"][1].m_name, "bar"); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"][0].m_dataType, MaterialPropertyDataType::Bool); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"][1].m_dataType, MaterialPropertyDataType::Image); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"][0].m_dataType, MaterialPropertyDataType::Float); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"][1].m_dataType, MaterialPropertyDataType::Color); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"][0].m_visibility, MaterialPropertyVisibility::Enabled); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"][1].m_visibility, MaterialPropertyVisibility::Hidden); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"][0].m_visibility, MaterialPropertyVisibility::Enabled); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"][1].m_visibility, MaterialPropertyVisibility::Disabled); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"][0].m_value, true); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"][1].m_value, AZStd::string{"Default.png"}); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"][0].m_value, 0.5f); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"][1].m_value, AZ::Color(0.5f, 0.5f, 0.5f, 1.0f)); + EXPECT_EQ(material.FindProperty("groupA.foo")->m_name, "foo"); + EXPECT_EQ(material.FindProperty("groupA.bar")->m_name, "bar"); + EXPECT_EQ(material.FindProperty("groupB.foo")->m_name, "foo"); + EXPECT_EQ(material.FindProperty("groupB.bar")->m_name, "bar"); + EXPECT_EQ(material.FindProperty("groupC.groupD.foo")->m_name, "foo"); + EXPECT_EQ(material.FindProperty("groupC.groupE.bar")->m_name, "bar"); + EXPECT_EQ(material.FindProperty("groupA.foo")->m_dataType, MaterialPropertyDataType::Bool); + EXPECT_EQ(material.FindProperty("groupA.bar")->m_dataType, MaterialPropertyDataType::Image); + EXPECT_EQ(material.FindProperty("groupB.foo")->m_dataType, MaterialPropertyDataType::Float); + EXPECT_EQ(material.FindProperty("groupB.bar")->m_dataType, MaterialPropertyDataType::Color); + EXPECT_EQ(material.FindProperty("groupC.groupD.foo")->m_dataType, MaterialPropertyDataType::Int); + EXPECT_EQ(material.FindProperty("groupC.groupE.bar")->m_dataType, MaterialPropertyDataType::UInt); + EXPECT_EQ(material.FindProperty("groupA.foo")->m_visibility, MaterialPropertyVisibility::Enabled); + EXPECT_EQ(material.FindProperty("groupA.bar")->m_visibility, MaterialPropertyVisibility::Hidden); + EXPECT_EQ(material.FindProperty("groupB.foo")->m_visibility, MaterialPropertyVisibility::Enabled); + EXPECT_EQ(material.FindProperty("groupB.bar")->m_visibility, MaterialPropertyVisibility::Disabled); + EXPECT_EQ(material.FindProperty("groupC.groupD.foo")->m_visibility, MaterialPropertyVisibility::Enabled); + EXPECT_EQ(material.FindProperty("groupC.groupE.bar")->m_visibility, MaterialPropertyVisibility::Enabled); + EXPECT_EQ(material.FindProperty("groupA.foo")->m_value, true); + EXPECT_EQ(material.FindProperty("groupA.bar")->m_value, AZStd::string{"Default.png"}); + EXPECT_EQ(material.FindProperty("groupB.foo")->m_value, 0.5f); + EXPECT_EQ(material.FindProperty("groupB.bar")->m_value, AZ::Color(0.5f, 0.5f, 0.5f, 1.0f)); + EXPECT_EQ(material.FindProperty("groupC.groupD.foo")->m_value, -1); + EXPECT_EQ(material.FindProperty("groupC.groupE.bar")->m_value, 0u); + + EXPECT_EQ(material.FindPropertySet("groupA")->GetFunctors().size(), 1); + EXPECT_EQ(material.FindPropertySet("groupB")->GetFunctors().size(), 1); + Ptr functorA = material.FindPropertySet("groupA")->GetFunctors()[0]->GetActualSourceData(); + Ptr functorB = material.FindPropertySet("groupB")->GetFunctors()[0]->GetActualSourceData(); + EXPECT_TRUE(azrtti_cast(functorA.get())); + EXPECT_EQ(azrtti_cast(functorA.get())->m_enablePassPropertyId, "foo"); + EXPECT_EQ(azrtti_cast(functorA.get())->m_shaderIndex, 1); + EXPECT_TRUE(azrtti_cast(functorB.get())); + EXPECT_EQ(azrtti_cast(functorB.get())->m_floatPropertyInputId, "foo"); + EXPECT_EQ(azrtti_cast(functorB.get())->m_float3ShaderSettingOutputId, "m_someFloat3"); EXPECT_EQ(material.m_shaderCollection.size(), 2); EXPECT_EQ(material.m_shaderCollection[0].m_shaderFilePath, "ForwardPass.shader"); @@ -1103,13 +1677,10 @@ namespace UnitTest EXPECT_EQ(material.m_shaderCollection[1].m_shaderOptionValues[Name{"o_optionD"}], Name{"2"}); EXPECT_EQ(material.m_shaderCollection[0].m_shaderTag, Name{"ForwardPass"}); - EXPECT_EQ(material.m_materialFunctorSourceData.size(), 2); - EXPECT_TRUE(azrtti_cast(material.m_materialFunctorSourceData[0]->GetActualSourceData().get())); - EXPECT_EQ(azrtti_cast(material.m_materialFunctorSourceData[0]->GetActualSourceData().get())->m_enablePassPropertyId, "groupA.foo"); - EXPECT_EQ(azrtti_cast(material.m_materialFunctorSourceData[0]->GetActualSourceData().get())->m_shaderIndex, 1); - EXPECT_TRUE(azrtti_cast(material.m_materialFunctorSourceData[1]->GetActualSourceData().get())); - EXPECT_EQ(azrtti_cast(material.m_materialFunctorSourceData[1]->GetActualSourceData().get())->m_floatPropertyInputId, "groupB.foo"); - EXPECT_EQ(azrtti_cast(material.m_materialFunctorSourceData[1]->GetActualSourceData().get())->m_float3ShaderSettingOutputId, "m_someFloat3"); + EXPECT_EQ(material.m_materialFunctorSourceData.size(), 1); + Ptr functorC = material.m_materialFunctorSourceData[0]->GetActualSourceData(); + EXPECT_TRUE(azrtti_cast(functorC.get())); + EXPECT_EQ(azrtti_cast(functorC.get())->m_enablePropertyName, "groupA.foo"); AZStd::string outputJson; JsonTestResult storeResult = StoreTestDataToJson(material, outputJson); @@ -1120,6 +1691,9 @@ namespace UnitTest { // The content of this test was copied from LoadAndStoreJson_AllFields to prove backward compatibility. // (The "store" part of the test was not included because the saved data will be the new format). + // Notable differences include: + // 1) the key "id" is used instead of "name" + // 2) the group metadata, property definitions, and functors are all defined in different sections rather than in a property set const AZStd::string inputJson = R"( { @@ -1206,37 +1780,57 @@ namespace UnitTest MaterialTypeSourceData material; JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); + // Before conversion to the new format, the data is in the old place + EXPECT_EQ(material.GetPropertyLayout().m_groups.size(), 2); + EXPECT_EQ(material.GetPropertyLayout().m_properties.size(), 2); + EXPECT_EQ(material.GetPropertyLayout().m_propertySets.size(), 0); + + material.ConvertToNewDataFormat(); + + // After conversion to the new format, the data is in the new place + EXPECT_EQ(material.GetPropertyLayout().m_groups.size(), 0); + EXPECT_EQ(material.GetPropertyLayout().m_properties.size(), 0); + EXPECT_EQ(material.GetPropertyLayout().m_propertySets.size(), 2); + EXPECT_EQ(material.m_description, "This is a general description about the material"); - EXPECT_EQ(material.m_propertyLayout.m_version, 2); + EXPECT_EQ(material.GetPropertyLayout().m_version, 2); - EXPECT_EQ(material.m_propertyLayout.m_groups.size(), 2); - EXPECT_TRUE(material.FindGroup("groupA") != nullptr); - EXPECT_TRUE(material.FindGroup("groupB") != nullptr); - EXPECT_EQ(material.FindGroup("groupA")->m_displayName, "Property Group A"); - EXPECT_EQ(material.FindGroup("groupB")->m_displayName, "Property Group B"); - EXPECT_EQ(material.FindGroup("groupA")->m_description, "Description of property group A"); - EXPECT_EQ(material.FindGroup("groupB")->m_description, "Description of property group B"); + EXPECT_TRUE(material.FindPropertySet("groupA") != nullptr); + EXPECT_TRUE(material.FindPropertySet("groupB") != nullptr); + EXPECT_EQ(material.FindPropertySet("groupA")->GetDisplayName(), "Property Group A"); + EXPECT_EQ(material.FindPropertySet("groupB")->GetDisplayName(), "Property Group B"); + EXPECT_EQ(material.FindPropertySet("groupA")->GetDescription(), "Description of property group A"); + EXPECT_EQ(material.FindPropertySet("groupB")->GetDescription(), "Description of property group B"); + EXPECT_EQ(material.FindPropertySet("groupA")->GetProperties().size(), 2); + EXPECT_EQ(material.FindPropertySet("groupB")->GetProperties().size(), 2); + + EXPECT_TRUE(material.FindProperty("groupA.foo") != nullptr); + EXPECT_TRUE(material.FindProperty("groupA.bar") != nullptr); + EXPECT_TRUE(material.FindProperty("groupB.foo") != nullptr); + EXPECT_TRUE(material.FindProperty("groupB.bar") != nullptr); - EXPECT_EQ(material.m_propertyLayout.m_properties.size(), 2); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"].size(), 2); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"].size(), 2); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"][0].m_name, "foo"); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"][1].m_name, "bar"); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"][0].m_name, "foo"); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"][1].m_name, "bar"); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"][0].m_dataType, MaterialPropertyDataType::Bool); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"][1].m_dataType, MaterialPropertyDataType::Image); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"][0].m_dataType, MaterialPropertyDataType::Float); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"][1].m_dataType, MaterialPropertyDataType::Color); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"][0].m_visibility, MaterialPropertyVisibility::Enabled); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"][1].m_visibility, MaterialPropertyVisibility::Hidden); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"][0].m_visibility, MaterialPropertyVisibility::Enabled); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"][1].m_visibility, MaterialPropertyVisibility::Disabled); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"][0].m_value, true); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupA"][1].m_value, AZStd::string{"Default.png"}); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"][0].m_value, 0.5f); - EXPECT_EQ(material.m_propertyLayout.m_properties["groupB"][1].m_value, AZ::Color(0.5f, 0.5f, 0.5f, 1.0f)); + EXPECT_EQ(material.FindProperty("groupA.foo")->m_name, "foo"); + EXPECT_EQ(material.FindProperty("groupA.bar")->m_name, "bar"); + EXPECT_EQ(material.FindProperty("groupB.foo")->m_name, "foo"); + EXPECT_EQ(material.FindProperty("groupB.bar")->m_name, "bar"); + EXPECT_EQ(material.FindProperty("groupA.foo")->m_dataType, MaterialPropertyDataType::Bool); + EXPECT_EQ(material.FindProperty("groupA.bar")->m_dataType, MaterialPropertyDataType::Image); + EXPECT_EQ(material.FindProperty("groupB.foo")->m_dataType, MaterialPropertyDataType::Float); + EXPECT_EQ(material.FindProperty("groupB.bar")->m_dataType, MaterialPropertyDataType::Color); + EXPECT_EQ(material.FindProperty("groupA.foo")->m_visibility, MaterialPropertyVisibility::Enabled); + EXPECT_EQ(material.FindProperty("groupA.bar")->m_visibility, MaterialPropertyVisibility::Hidden); + EXPECT_EQ(material.FindProperty("groupB.foo")->m_visibility, MaterialPropertyVisibility::Enabled); + EXPECT_EQ(material.FindProperty("groupB.bar")->m_visibility, MaterialPropertyVisibility::Disabled); + EXPECT_EQ(material.FindProperty("groupA.foo")->m_value, true); + EXPECT_EQ(material.FindProperty("groupA.bar")->m_value, AZStd::string{"Default.png"}); + EXPECT_EQ(material.FindProperty("groupB.foo")->m_value, 0.5f); + EXPECT_EQ(material.FindProperty("groupB.bar")->m_value, AZ::Color(0.5f, 0.5f, 0.5f, 1.0f)); + + // The functors can appear either at the top level or within each property set. The format conversion + // function doesn't know how to move the functors, and they will be left at the top level. + EXPECT_EQ(material.FindPropertySet("groupA")->GetFunctors().size(), 0); + EXPECT_EQ(material.FindPropertySet("groupB")->GetFunctors().size(), 0); EXPECT_EQ(material.m_shaderCollection.size(), 2); EXPECT_EQ(material.m_shaderCollection[0].m_shaderFilePath, "ForwardPass.shader"); @@ -1257,7 +1851,7 @@ namespace UnitTest EXPECT_EQ(azrtti_cast(material.m_materialFunctorSourceData[1]->GetActualSourceData().get())->m_floatPropertyInputId, "groupB.foo"); EXPECT_EQ(azrtti_cast(material.m_materialFunctorSourceData[1]->GetActualSourceData().get())->m_float3ShaderSettingOutputId, "m_someFloat3"); } - + TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_PropertyImagePath) { char inputJson[2048]; @@ -1267,27 +1861,25 @@ namespace UnitTest "description": "", "propertyLayout": { "version": 2, - "groups": [ + "propertySets": [ { "name": "general", "displayName": "General", - "description": "" + "description": "", + "properties": [ + { + "name": "absolute", + "type": "Image", + "defaultValue": "%s" + }, + { + "name": "relative", + "type": "Image", + "defaultValue": "%s" + } + ] } - ], - "properties": { - "general": [ - { - "name": "absolute", - "type": "Image", - "defaultValue": "%s" - }, - { - "name": "relative", - "type": "Image", - "defaultValue": "%s" - } - ] - } + ] } } )", diff --git a/Gems/Atom/TestData/TestData/Materials/Types/MinimalPBR.materialtype b/Gems/Atom/TestData/TestData/Materials/Types/MinimalPBR.materialtype index 81ebd63c28..ce59ec8b8e 100644 --- a/Gems/Atom/TestData/TestData/Materials/Types/MinimalPBR.materialtype +++ b/Gems/Atom/TestData/TestData/Materials/Types/MinimalPBR.materialtype @@ -2,50 +2,48 @@ "description": "Base Material with properties used to define Standard PBR, a metallic-roughness Physically-Based Rendering (PBR) material shading model.", "propertyLayout": { "version": 3, - "groups": [ + "propertySets": [ { "name": "settings", - "displayName": "Settings" + "displayName": "Settings", + "properties": [ + { + "name": "color", + "displayName": "Color", + "type": "Color", + "defaultValue": [ 1.0, 1.0, 1.0 ], + "connection": { + "type": "ShaderInput", + "name": "m_baseColor" + } + }, + { + "name": "metallic", + "displayName": "Metallic", + "type": "Float", + "defaultValue": 0.0, + "min": 0.0, + "max": 1.0, + "connection": { + "type": "ShaderInput", + "name": "m_metallic" + } + }, + { + "name": "roughness", + "displayName": "Roughness", + "type": "Float", + "defaultValue": 1.0, + "min": 0.0, + "max": 1.0, + "connection": { + "type": "ShaderInput", + "name": "m_roughness" + } + } + ] } - ], - "properties": { - "settings": [ - { - "name": "color", - "displayName": "Color", - "type": "Color", - "defaultValue": [ 1.0, 1.0, 1.0 ], - "connection": { - "type": "ShaderInput", - "name": "m_baseColor" - } - }, - { - "name": "metallic", - "displayName": "Metallic", - "type": "Float", - "defaultValue": 0.0, - "min": 0.0, - "max": 1.0, - "connection": { - "type": "ShaderInput", - "name": "m_metallic" - } - }, - { - "name": "roughness", - "displayName": "Roughness", - "type": "Float", - "defaultValue": 1.0, - "min": 0.0, - "max": 1.0, - "connection": { - "type": "ShaderInput", - "name": "m_roughness" - } - } - ] - } + ] }, "shaders": [ { diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index 3f4aa71a9c..cf3205a037 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -230,7 +230,7 @@ namespace MaterialEditor // create source data from properties MaterialSourceData sourceData; - sourceData.m_propertyLayoutVersion = m_materialTypeSourceData.m_propertyLayout.m_version; + sourceData.m_propertyLayoutVersion = m_materialTypeSourceData.GetPropertyLayout().m_version; sourceData.m_materialType = m_materialSourceData.m_materialType; sourceData.m_parentMaterial = m_materialSourceData.m_parentMaterial; @@ -302,7 +302,7 @@ namespace MaterialEditor // create source data from properties MaterialSourceData sourceData; - sourceData.m_propertyLayoutVersion = m_materialTypeSourceData.m_propertyLayout.m_version; + sourceData.m_propertyLayoutVersion = m_materialTypeSourceData.GetPropertyLayout().m_version; sourceData.m_materialType = m_materialSourceData.m_materialType; sourceData.m_parentMaterial = m_materialSourceData.m_parentMaterial; @@ -373,7 +373,7 @@ namespace MaterialEditor // create source data from properties MaterialSourceData sourceData; - sourceData.m_propertyLayoutVersion = m_materialTypeSourceData.m_propertyLayout.m_version; + sourceData.m_propertyLayoutVersion = m_materialTypeSourceData.GetPropertyLayout().m_version; sourceData.m_materialType = m_materialSourceData.m_materialType; // Only assign a parent path if the source was a .material @@ -592,24 +592,26 @@ namespace MaterialEditor bool result = true; // populate sourceData with properties that meet the filter - m_materialTypeSourceData.EnumerateProperties([this, &sourceData, &propertyFilter, &result](const AZStd::string& groupName, const AZStd::string& propertyName, const auto& propertyDefinition) { + m_materialTypeSourceData.EnumerateProperties([this, &sourceData, &propertyFilter, &result](const AZStd::string& propertyIdContext, const auto& propertyDefinition) { - const MaterialPropertyId propertyId(groupName, propertyName); + const AZStd::string propertyId = propertyIdContext + propertyDefinition->m_name; - const auto it = m_properties.find(propertyId); + const auto it = m_properties.find(Name{propertyId}); if (it != m_properties.end() && propertyFilter(it->second)) { MaterialPropertyValue propertyValue = AtomToolsFramework::ConvertToRuntimeType(it->second.GetValue()); if (propertyValue.IsValid()) { - if (!m_materialTypeSourceData.ConvertPropertyValueToSourceDataFormat(propertyDefinition, propertyValue)) + if (!m_materialTypeSourceData.ConvertPropertyValueToSourceDataFormat(*propertyDefinition, propertyValue)) { - AZ_Error("MaterialDocument", false, "Material document property could not be converted: '%s' in '%s'.", propertyId.GetCStr(), m_absolutePath.c_str()); + AZ_Error("MaterialDocument", false, "Material document property could not be converted: '%s' in '%s'.", propertyId.c_str(), m_absolutePath.c_str()); result = false; return false; } - - sourceData.m_properties[groupName][propertyName].m_value = propertyValue; + + // TODO: Support populating the Material Editor with nested property sets, not just the top level. + const AZStd::string groupName = propertyId.substr(0, propertyId.size() - propertyDefinition->m_name.size() - 1); + sourceData.m_properties[groupName][propertyDefinition->m_name].m_value = propertyValue; } } return true; @@ -678,7 +680,7 @@ namespace MaterialEditor AZ_Error("MaterialDocument", false, "Material type source data could not be loaded: '%s'.", materialTypeSourceFilePath.c_str()); return false; } - m_materialTypeSourceData = materialTypeOutcome.GetValue(); + m_materialTypeSourceData = materialTypeOutcome.TakeValue(); } else if (AzFramework::StringFunc::Path::IsExtension(m_absolutePath.c_str(), MaterialTypeSourceData::Extension)) { @@ -691,7 +693,7 @@ namespace MaterialEditor AZ_Error("MaterialDocument", false, "Material type source data could not be loaded: '%s'.", m_absolutePath.c_str()); return false; } - m_materialTypeSourceData = materialTypeOutcome.GetValue(); + m_materialTypeSourceData = materialTypeOutcome.TakeValue(); // The document represents a material, not a material type. // If the input data is a material type file we have to generate the material source data by referencing it. @@ -770,33 +772,41 @@ namespace MaterialEditor // 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 - m_materialTypeSourceData.EnumerateProperties([this, &parentPropertyValues](const AZStd::string& groupName, const AZStd::string& propertyName, const auto& propertyDefinition) { - AtomToolsFramework::DynamicPropertyConfig propertyConfig; - - // Assign id before conversion so it can be used in dynamic description - propertyConfig.m_id = MaterialPropertyId(groupName, propertyName); - - const auto& propertyIndex = m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyConfig.m_id); - const bool propertyIndexInBounds = propertyIndex.IsValid() && propertyIndex.GetIndex() < m_materialAsset->GetPropertyValues().size(); - AZ_Warning("MaterialDocument", propertyIndexInBounds, "Failed to add material property '%s' to document '%s'.", propertyConfig.m_id.GetCStr(), m_absolutePath.c_str()); - - if (propertyIndexInBounds) + m_materialTypeSourceData.EnumeratePropertySets([this, &parentPropertyValues](const AZStd::string& propertyIdContext, const MaterialTypeSourceData::PropertySet* propertySet) { - AtomToolsFramework::ConvertToPropertyConfig(propertyConfig, propertyDefinition); - propertyConfig.m_showThumbnail = true; - propertyConfig.m_originalValue = AtomToolsFramework::ConvertToEditableType(m_materialAsset->GetPropertyValues()[propertyIndex.GetIndex()]); - propertyConfig.m_parentValue = AtomToolsFramework::ConvertToEditableType(parentPropertyValues[propertyIndex.GetIndex()]); - auto groupDefinition = m_materialTypeSourceData.FindGroup(groupName); - propertyConfig.m_groupName = groupDefinition ? groupDefinition->m_displayName : groupName; - m_properties[propertyConfig.m_id] = AtomToolsFramework::DynamicProperty(propertyConfig); - } - return true; - }); + AtomToolsFramework::DynamicPropertyConfig propertyConfig; + + 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; + + const auto& propertyIndex = m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyConfig.m_id); + const bool propertyIndexInBounds = propertyIndex.IsValid() && propertyIndex.GetIndex() < m_materialAsset->GetPropertyValues().size(); + AZ_Warning("MaterialDocument", propertyIndexInBounds, "Failed to add material property '%s' to document '%s'.", propertyConfig.m_id.GetCStr(), m_absolutePath.c_str()); + + if (propertyIndexInBounds) + { + AtomToolsFramework::ConvertToPropertyConfig(propertyConfig, *propertyDefinition); + propertyConfig.m_showThumbnail = true; + propertyConfig.m_originalValue = AtomToolsFramework::ConvertToEditableType(m_materialAsset->GetPropertyValues()[propertyIndex.GetIndex()]); + propertyConfig.m_parentValue = AtomToolsFramework::ConvertToEditableType(parentPropertyValues[propertyIndex.GetIndex()]); + + // TODO: Support populating the Material Editor with nested property sets, not just the top level. + // (Does DynamicPropertyConfig really even need m_groupName?) + propertyConfig.m_groupName = propertySet->GetDisplayName(); + m_properties[propertyConfig.m_id] = AtomToolsFramework::DynamicProperty(propertyConfig); + } + } + + return true; + }); // Populate the property group visibility map - for (MaterialTypeSourceData::GroupDefinition& group : m_materialTypeSourceData.GetGroupDefinitionsInDisplayOrder()) + // TODO: Support populating the Material Editor with nested property sets, not just the top level. + for (const AZStd::unique_ptr& propertySet : m_materialTypeSourceData.GetPropertyLayout().m_propertySets) { - m_propertyGroupVisibility[AZ::Name{group.m_name}] = true; + m_propertyGroupVisibility[AZ::Name{propertySet->GetName()}] = true; } // Adding properties for material type and parent as part of making dynamic @@ -877,6 +887,39 @@ namespace MaterialEditor return false; } } + + bool enumerateResult = m_materialTypeSourceData.EnumeratePropertySets( + [this, &materialTypeSourceFilePath](const AZStd::string&, const MaterialTypeSourceData::PropertySet* propertySet) + { + const MaterialFunctorSourceData::EditorContext editorContext = MaterialFunctorSourceData::EditorContext( + materialTypeSourceFilePath, m_materialAsset->GetMaterialPropertiesLayout()); + + for (Ptr functorData : propertySet->GetFunctors()) + { + MaterialFunctorSourceData::FunctorResult result = functorData->CreateFunctor(editorContext); + + if (result.IsSuccess()) + { + Ptr& functor = result.GetValue(); + if (functor != nullptr) + { + m_editorFunctors.push_back(functor); + } + } + else + { + AZ_Error("MaterialDocument", false, "Material functors were not created: '%s'.", m_absolutePath.c_str()); + return false; + } + } + + return true; + }); + + if (!enumerateResult) + { + return false; + } AZ::RPI::MaterialPropertyFlags dirtyFlags; dirtyFlags.set(); // Mark all properties as dirty since we just loaded the material and need to initialize property visibility 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 f6b42bf13a..e279b7e5b0 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp @@ -163,28 +163,23 @@ namespace MaterialEditor const AZ::RPI::MaterialTypeSourceData* materialTypeSourceData = nullptr; MaterialDocumentRequestBus::EventResult( materialTypeSourceData, m_documentId, &MaterialDocumentRequestBus::Events::GetMaterialTypeSourceData); - - for (const auto& groupDefinition : materialTypeSourceData->GetGroupDefinitionsInDisplayOrder()) + + // TODO: Support populating the Material Editor with nested property sets, not just the top level. + for (const AZStd::unique_ptr& propertySet : materialTypeSourceData->GetPropertyLayout().m_propertySets) { - const AZStd::string& groupName = groupDefinition.m_name; - const AZStd::string& groupDisplayName = !groupDefinition.m_displayName.empty() ? groupDefinition.m_displayName : groupName; - const AZStd::string& groupDescription = - !groupDefinition.m_description.empty() ? groupDefinition.m_description : groupDisplayName; + const AZStd::string& groupName = propertySet->GetName(); + const AZStd::string& groupDisplayName = !propertySet->GetDisplayName().empty() ? propertySet->GetDisplayName() : groupName; + const AZStd::string& groupDescription = !propertySet->GetDescription().empty() ? propertySet->GetDescription() : groupDisplayName; auto& group = m_groups[groupName]; - const auto& propertyLayout = materialTypeSourceData->m_propertyLayout; - const auto& propertyListItr = propertyLayout.m_properties.find(groupName); - if (propertyListItr != propertyLayout.m_properties.end()) + group.m_properties.reserve(propertySet->GetProperties().size()); + for (const auto& propertyDefinition : propertySet->GetProperties()) { - group.m_properties.reserve(propertyListItr->second.size()); - for (const auto& propertyDefinition : propertyListItr->second) - { - AtomToolsFramework::DynamicProperty property; - AtomToolsFramework::AtomToolsDocumentRequestBus::EventResult( - property, m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::GetProperty, - AZ::RPI::MaterialPropertyId(groupName, propertyDefinition.m_name)); - group.m_properties.push_back(property); - } + AtomToolsFramework::DynamicProperty property; + AtomToolsFramework::AtomToolsDocumentRequestBus::EventResult( + property, m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::GetProperty, + AZ::RPI::MaterialPropertyId(groupName, propertyDefinition->m_name)); + group.m_properties.push_back(property); } // Passing in same group as main and comparison instance to enable custom value comparison for highlighting modified properties diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp index c05b84db39..86610ecc93 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp @@ -283,35 +283,31 @@ namespace AZ AddUvNamesGroup(); // Copy all of the properties from the material asset to the source data that will be exported - for (const auto& groupDefinition : m_editData.m_materialTypeSourceData.GetGroupDefinitionsInDisplayOrder()) + // TODO: Support populating the Material Editor with nested property sets, not just the top level. + for (const AZStd::unique_ptr& propertySet : m_editData.m_materialTypeSourceData.GetPropertyLayout().m_propertySets) { - const AZStd::string& groupName = groupDefinition.m_name; - const AZStd::string& groupDisplayName = !groupDefinition.m_displayName.empty() ? groupDefinition.m_displayName : groupName; - const AZStd::string& groupDescription = !groupDefinition.m_description.empty() ? groupDefinition.m_description : groupDisplayName; + const AZStd::string& groupName = propertySet->GetName(); + const AZStd::string& groupDisplayName = !propertySet->GetDisplayName().empty() ? propertySet->GetDisplayName() : groupName; + const AZStd::string& groupDescription = !propertySet->GetDescription().empty() ? propertySet->GetDescription() : groupDisplayName; auto& group = m_groups[groupName]; - - const auto& propertyLayout = m_editData.m_materialTypeSourceData.m_propertyLayout; - const auto& propertyListItr = propertyLayout.m_properties.find(groupName); - if (propertyListItr != propertyLayout.m_properties.end()) + + group.m_properties.reserve(propertySet->GetProperties().size()); + for (const auto& propertyDefinition : propertySet->GetProperties()) { - group.m_properties.reserve(propertyListItr->second.size()); - for (const auto& propertyDefinition : propertyListItr->second) - { - AtomToolsFramework::DynamicPropertyConfig propertyConfig; + 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); + // Assign id before conversion so it can be used in dynamic description + propertyConfig.m_id = AZ::RPI::MaterialPropertyId(groupName, propertyDefinition->m_name); - AtomToolsFramework::ConvertToPropertyConfig(propertyConfig, propertyDefinition); + AtomToolsFramework::ConvertToPropertyConfig(propertyConfig, *propertyDefinition.get()); - propertyConfig.m_groupName = groupDisplayName; - const auto& propertyIndex = m_editData.m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyConfig.m_id); - propertyConfig.m_showThumbnail = true; - propertyConfig.m_defaultValue = AtomToolsFramework::ConvertToEditableType(m_editData.m_materialTypeAsset->GetDefaultPropertyValues()[propertyIndex.GetIndex()]); - 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); - } + propertyConfig.m_groupName = groupDisplayName; + const auto& propertyIndex = m_editData.m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyConfig.m_id); + propertyConfig.m_showThumbnail = true; + propertyConfig.m_defaultValue = AtomToolsFramework::ConvertToEditableType(m_editData.m_materialTypeAsset->GetDefaultPropertyValues()[propertyIndex.GetIndex()]); + 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); } // Passing in same group as main and comparison instance to enable custom value comparison for highlighting modified properties diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp index e851bc5fce..2593a12f19 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp @@ -91,7 +91,7 @@ namespace AZ AZ_Error("AZ::Render::EditorMaterialComponentUtil", false, "Failed to load material type source data: %s", editData.m_materialTypeSourcePath.c_str()); return false; } - editData.m_materialTypeSourceData = materialTypeOutcome.GetValue(); + editData.m_materialTypeSourceData = materialTypeOutcome.TakeValue(); return true; } @@ -99,7 +99,7 @@ namespace AZ { // Construct the material source data object that will be exported AZ::RPI::MaterialSourceData exportData; - exportData.m_propertyLayoutVersion = editData.m_materialTypeSourceData.m_propertyLayout.m_version; + exportData.m_propertyLayoutVersion = editData.m_materialTypeSourceData.GetPropertyLayout().m_version; // Converting absolute material paths to relative paths bool result = false; @@ -137,42 +137,46 @@ namespace AZ // Copy all of the properties from the material asset to the source data that will be exported result = true; - editData.m_materialTypeSourceData.EnumerateProperties([&](const AZStd::string& groupName, const AZStd::string& propertyName, const auto& propertyDefinition) { - const AZ::RPI::MaterialPropertyId propertyId(groupName, propertyName); - const AZ::RPI::MaterialPropertyIndex propertyIndex = - editData.m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyId); - - AZ::RPI::MaterialPropertyValue propertyValue = editData.m_materialAsset->GetPropertyValues()[propertyIndex.GetIndex()]; - - AZ::RPI::MaterialPropertyValue propertyValueDefault = propertyDefinition.m_value; - if (editData.m_materialParentAsset.IsReady()) + editData.m_materialTypeSourceData.EnumerateProperties([&](const AZStd::string& propertyIdContext, const AZ::RPI::MaterialTypeSourceData::PropertyDefinition* propertyDefinition) { - propertyValueDefault = editData.m_materialParentAsset->GetPropertyValues()[propertyIndex.GetIndex()]; - } + AZ::Name propertyId(propertyIdContext + propertyDefinition->m_name); - // Check for and apply any property overrides before saving property values - auto propertyOverrideItr = editData.m_materialPropertyOverrideMap.find(propertyId); - if(propertyOverrideItr != editData.m_materialPropertyOverrideMap.end()) - { - propertyValue = AZ::RPI::MaterialPropertyValue::FromAny(propertyOverrideItr->second); - } + const AZ::RPI::MaterialPropertyIndex propertyIndex = + editData.m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyId); - if (!editData.m_materialTypeSourceData.ConvertPropertyValueToSourceDataFormat(propertyDefinition, propertyValue)) - { - AZ_Error("AZ::Render::EditorMaterialComponentUtil", false, "Failed to export: %s", path.c_str()); - result = false; - return false; - } + AZ::RPI::MaterialPropertyValue propertyValue = editData.m_materialAsset->GetPropertyValues()[propertyIndex.GetIndex()]; - // Don't export values if they are the same as the material type or parent - if (propertyValueDefault == propertyValue) - { + AZ::RPI::MaterialPropertyValue propertyValueDefault = propertyDefinition->m_value; + if (editData.m_materialParentAsset.IsReady()) + { + propertyValueDefault = editData.m_materialParentAsset->GetPropertyValues()[propertyIndex.GetIndex()]; + } + + // Check for and apply any property overrides before saving property values + auto propertyOverrideItr = editData.m_materialPropertyOverrideMap.find(propertyId); + if(propertyOverrideItr != editData.m_materialPropertyOverrideMap.end()) + { + propertyValue = AZ::RPI::MaterialPropertyValue::FromAny(propertyOverrideItr->second); + } + + if (!editData.m_materialTypeSourceData.ConvertPropertyValueToSourceDataFormat(*propertyDefinition, propertyValue)) + { + AZ_Error("AZ::Render::EditorMaterialComponentUtil", false, "Failed to export: %s", path.c_str()); + result = false; + return false; + } + + // Don't export values if they are the same as the material type or parent + if (propertyValueDefault == propertyValue) + { + 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; return true; - } - - exportData.m_properties[groupName][propertyDefinition.m_name].m_value = propertyValue; - return true; - }); + }); return result && AZ::RPI::JsonUtils::SaveObjectToFile(path, exportData); }