diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/ConvertEmissiveUnitFunctorSourceData.h b/Gems/Atom/Feature/Common/Code/Source/Material/ConvertEmissiveUnitFunctorSourceData.h index 23219ce940..091351087f 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/ConvertEmissiveUnitFunctorSourceData.h +++ b/Gems/Atom/Feature/Common/Code/Source/Material/ConvertEmissiveUnitFunctorSourceData.h @@ -44,8 +44,8 @@ namespace AZ AZStd::string m_shaderInputName; // The indices of photometric units in the dropdown list - uint32_t m_ev100Index; - uint32_t m_nitIndex; + uint32_t m_ev100Index = 0; + uint32_t m_nitIndex = 1; // Minimum and Maximum value for different photometric units AZ::Vector2 m_ev100MinMax; 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 f669406172..8c78d4b491 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 @@ -34,7 +34,8 @@ namespace AZ explicit MaterialPropertyId(AZStd::string_view propertyName); MaterialPropertyId(AZStd::string_view groupName, AZStd::string_view propertyName); MaterialPropertyId(const Name& groupName, const Name& propertyName); - explicit MaterialPropertyId(const AZStd::span names); + explicit MaterialPropertyId(const AZStd::span names); + MaterialPropertyId(const AZStd::span groupNames, AZStd::string_view propertyName); AZ_DEFAULT_COPY_MOVE(MaterialPropertyId); 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 9333880594..79c73495e0 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 @@ -22,9 +22,12 @@ namespace AZ namespace RPI { class MaterialTypeAsset; + class MaterialTypeAssetCreator; class MaterialFunctorSourceDataHolder; + class JsonMaterialPropertySerializer; //! This is a simple data structure for serializing in/out material type source files. + //! Note that there may be a mixture of public and private members, as we are gradually introducing a proper API. class MaterialTypeSourceData final { public: @@ -69,13 +72,22 @@ namespace AZ struct PropertyDefinition { - AZ_TYPE_INFO(AZ::RPI::MaterialTypeSourceData::PropertyDefinition, "{E0DB3C0D-75DB-4ADB-9E79-30DA63FA18B7}"); + friend class JsonMaterialPropertySerializer; + 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; + + PropertyDefinition() = default; - AZStd::string m_name; //!< The name of the property within the property group. The full property ID will be groupName.propertyName. + explicit PropertyDefinition(AZStd::string_view name) : m_name(name) + { + } + + const AZStd::string& GetName() const { return m_name; } MaterialPropertyVisibility m_visibility = MaterialPropertyVisibility::Default; @@ -97,6 +109,58 @@ namespace AZ MaterialPropertyValue m_softMin; MaterialPropertyValue m_softMax; MaterialPropertyValue m_step; + + private: + + // We are gradually moving toward having a more proper API for MaterialTypeSourceData code, but we still some public members + // like above. However, it's important for m_name to be private because it is used as the key for lookups, collision validation, etc. + AZStd::string m_name; //!< The name of the property within the property group. The full property ID will be groupName.propertyName. + }; + + using PropertyList = AZStd::vector>; + + struct PropertyGroup + { + friend class MaterialTypeSourceData; + + AZ_CLASS_ALLOCATOR(PropertyGroup, SystemAllocator, 0); + AZ_TYPE_INFO(AZ::RPI::MaterialTypeSourceData::PropertyGroup, "{BA3AA0E4-C74D-4FD0-ADB2-00B060F06314}"); + + public: + + PropertyGroup() = default; + AZ_DISABLE_COPY(PropertyGroup) + + 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>& GetPropertyGroups() const { return m_propertyGroups; } + 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; } + + //! Add a new property to this PropertyGroup. + //! @param name a unique for the property. Must be a C-style identifier. + //! @return the new PropertyDefinition, or null if the name was not valid. + PropertyDefinition* AddProperty(AZStd::string_view name); + + //! Add a new nested PropertyGroup to this PropertyGroup. + //! @param name a unique for the property group. Must be a C-style identifier. + //! @return the new PropertyGroup, or null if the name was not valid. + PropertyGroup* AddPropertyGroup(AZStd::string_view name); + + private: + + static PropertyGroup* AddPropertyGroup(AZStd::string_view name, AZStd::vector>& toPropertyGroupList); + + AZStd::string m_name; + AZStd::string m_displayName; + AZStd::string m_description; + PropertyList m_properties; + AZStd::vector> m_propertyGroups; + AZStd::vector> m_materialFunctorSourceData; }; struct ShaderVariantReferenceData @@ -118,8 +182,6 @@ namespace AZ AZStd::unordered_map m_shaderOptionValues; }; - using PropertyList = AZStd::vector; - struct VersionUpdatesRenameOperationDefinition { AZ_TYPE_INFO(AZ::RPI::MaterialTypeSourceData::VersionUpdatesRenameOperationDefinition, "{F2295489-E15A-46CC-929F-8D42DEDBCF14}"); @@ -147,26 +209,31 @@ namespace AZ struct PropertyLayout { AZ_TYPE_INFO(AZ::RPI::MaterialTypeSourceData::PropertyLayout, "{AE53CF3F-5C3B-44F5-B2FB-306F0EB06393}"); - + + PropertyLayout() = default; + AZ_DISABLE_COPY(PropertyLayout) + //! This field is unused, and has been replaced by MaterialTypeSourceData::m_version below. It is kept for legacy file compatibility to suppress warnings and errors. uint32_t m_versionOld = 0; + //! [Deprecated] Use m_propertyGroups instead //! List of groups that will contain the available properties - AZStd::vector m_groups; + AZStd::vector m_groupsOld; + //! [Deprecated] Use m_propertyGroups instead + AZStd::map> m_propertiesOld; + //! Collection of all available user-facing properties - AZStd::map m_properties; + AZStd::vector> m_propertyGroups; }; - + AZStd::string m_description; //! Version 1 is the default and should not contain any version update. uint32_t m_version = 1; - + VersionUpdates m_versionUpdates; - PropertyLayout m_propertyLayout; - //! A list of shader variants that are always used at runtime; they cannot be turned off AZStd::vector m_shaderCollection; @@ -181,38 +248,94 @@ namespace AZ //! Copy over UV custom names to the properties enum values. void ResolveUvEnums(); - const GroupDefinition* FindGroup(AZStd::string_view groupName) const; + //! Add a new PropertyGroup for containing properties or other PropertyGroups. + //! @param propertyGroupId The ID of the new property group. To add as a nested PropertyGroup, use a full path ID like "levelA.levelB.levelC"; in this case a property group "levelA.levelB" must already exist. + //! @return a pointer to the new PropertyGroup or null if there was a problem (an AZ_Error will be reported). + PropertyGroup* AddPropertyGroup(AZStd::string_view propertyGroupId); - //! Searches for a specific property. - //! Note this function can find properties using old versions of the property name; in that case, - //! the name in the returned PropertyDefinition* will not match the @propertyName that was searched for. - //! @return the requested property, or null if it could not be found - const PropertyDefinition* FindProperty(AZStd::string_view groupName, AZStd::string_view propertyName) const; + //! Add a new property to a PropertyGroup. + //! @param propertyId The ID of the new property, like "layerBlend.factor" or "layer2.roughness.texture". The indicated property group must already exist. + //! @return a pointer to the new PropertyDefinition or null if there was a problem (an AZ_Error will be reported). + PropertyDefinition* AddProperty(AZStd::string_view propertyId); - //! 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; + //! Return the PropertyLayout containing the tree of property groups and property definitions. + const PropertyLayout& GetPropertyLayout() const { return m_propertyLayout; } - //! Call back function type used with the numeration functions - using EnumeratePropertiesCallback = AZStd::function 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); + + //! Call back function type used with the enumeration functions. + //! Return false to terminate the traversal. + using EnumeratePropertyGroupsCallback = 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; + //! Recursively traverses all of the property groups 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 EnumeratePropertyGroups(const EnumeratePropertyGroupsCallback& 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; + //! Call back function type used with the numeration functions. + //! Return false to terminate the traversal. + using EnumeratePropertiesCallback = AZStd::function; + + //! 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; Outcome> CreateMaterialTypeAsset(Data::AssetId assetId, AZStd::string_view materialTypeSourceFilePath = "", bool elevateWarnings = true) const; + + //! If the data was loaded from an old format file (i.e. where "groups" and "properties" were separate sections), + //! this converts to the new format where properties are listed inside property groups. + bool ConvertToNewDataFormat(); + + private: + + const PropertyGroup* FindPropertyGroup(AZStd::span parsedPropertyGroupId, AZStd::span> inPropertyGroupList) const; + PropertyGroup* FindPropertyGroup(AZStd::span parsedPropertyGroupId, AZStd::span> inPropertyGroupList); - //! Possibly renames @propertyId based on the material version update steps. - //! @return true if the property was renamed - bool ApplyPropertyRenames(MaterialPropertyId& propertyId) const; + const PropertyDefinition* FindProperty(AZStd::span parsedPropertyId, AZStd::span> inPropertyGroupList) const; + PropertyDefinition* FindProperty(AZStd::span parsedPropertyId, AZStd::span> inPropertyGroupList); + + // Function overloads for recursion, returns false to indicate that recursion should end. + bool EnumeratePropertyGroups(const EnumeratePropertyGroupsCallback& callback, AZStd::string propertyIdContext, const AZStd::vector>& inPropertyGroupList) const; + bool EnumerateProperties(const EnumeratePropertiesCallback& callback, AZStd::string propertyIdContext, const AZStd::vector>& inPropertyGroupList) const; + + //! Recursively populates a material asset with properties from the tree of material property groups. + //! @param materialTypeSourceFilePath path to the material type file that is being processed, used to look up relative paths + //! @param propertyNameContext the accumulated prefix that should be applied to any property names encountered in the current @propertyGroup + //! @param propertyGroup the current PropertyGroup that is being processed + //! @return false if errors are detected and processing should abort + bool BuildPropertyList( + const AZStd::string& materialTypeSourceFilePath, + MaterialTypeAssetCreator& materialTypeAssetCreator, + AZStd::vector& propertyNameContext, + const MaterialTypeSourceData::PropertyGroup* propertyGroup) 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. @@ -241,7 +364,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.Builders/Material/MaterialBuilder.cpp b/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp index cadb182d03..d55f202c03 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp @@ -52,7 +52,7 @@ namespace AZ { AssetBuilderSDK::AssetBuilderDesc materialBuilderDescriptor; materialBuilderDescriptor.m_name = JobKey; - materialBuilderDescriptor.m_version = 116; // more material dependency improvements + materialBuilderDescriptor.m_version = 117; // new material type file format materialBuilderDescriptor.m_patterns.push_back(AssetBuilderSDK::AssetBuilderPattern("*.material", AssetBuilderSDK::AssetBuilderPattern::PatternType::Wildcard)); materialBuilderDescriptor.m_patterns.push_back(AssetBuilderSDK::AssetBuilderPattern("*.materialtype", AssetBuilderSDK::AssetBuilderPattern::PatternType::Wildcard)); materialBuilderDescriptor.m_busId = azrtti_typeid(); 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 3b0d6d79b8..1f5581e417 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyId.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyId.cpp @@ -88,7 +88,37 @@ namespace AZ { } - MaterialPropertyId::MaterialPropertyId(const AZStd::span names) + MaterialPropertyId::MaterialPropertyId(const AZStd::span 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::MaterialPropertyId(const AZStd::span names) { for (const auto& name : names) { 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 9980d8f2ff..cb345e1d93 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyValueSerializer.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyValueSerializer.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp index 2fa4ff648e..ae7889aa93 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp @@ -217,14 +217,14 @@ namespace AZ return Failure(); } - MaterialTypeSourceData materialTypeSourceData; - if (!AZ::RPI::JsonUtils::LoadObjectFromFile(materialTypeSourcePath, materialTypeSourceData)) + auto materialTypeLoadOutcome = MaterialUtils::LoadMaterialTypeSourceData(materialTypeSourcePath); + if (!materialTypeLoadOutcome) { AZ_Error("MaterialSourceData", false, "Failed to load MaterialTypeSourceData: '%s'.", materialTypeSourcePath.c_str()); return Failure(); } - materialTypeSourceData.ResolveUvEnums(); + MaterialTypeSourceData materialTypeSourceData = materialTypeLoadOutcome.TakeValue(); const auto materialTypeAsset = materialTypeSourceData.CreateMaterialTypeAsset(materialTypeAssetId.GetValue(), materialTypeSourcePath, elevateWarnings); 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 3a5992f8dd..b9a5754732 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp @@ -57,7 +57,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() @@ -84,11 +88,22 @@ namespace AZ ->Field("options", &ShaderVariantReferenceData::m_shaderOptionValues) ; + serializeContext->Class() + ->Version(1) + ->Field("name", &PropertyGroup::m_name) + ->Field("displayName", &PropertyGroup::m_displayName) + ->Field("description", &PropertyGroup::m_description) + ->Field("properties", &PropertyGroup::m_properties) + ->Field("propertyGroups", &PropertyGroup::m_propertyGroups) + ->Field("functors", &PropertyGroup::m_materialFunctorSourceData) + ; + serializeContext->Class() - ->Version(2) // Material Version Update - ->Field("version", &PropertyLayout::m_versionOld) - ->Field("groups", &PropertyLayout::m_groups) - ->Field("properties", &PropertyLayout::m_properties) + ->Version(3) // Added propertyGroups + ->Field("version", &PropertyLayout::m_versionOld) //< Deprecated, preserved for backward compatibility, replaced by MaterialTypeSourceData::version + ->Field("groups", &PropertyLayout::m_groupsOld) //< Deprecated, preserved for backward compatibility, replaced by propertyGroups + ->Field("properties", &PropertyLayout::m_propertiesOld) //< Deprecated, preserved for backward compatibility, replaced by propertyGroups + ->Field("propertyGroups", &PropertyLayout::m_propertyGroups) ; serializeContext->RegisterGenericType(); @@ -112,89 +127,351 @@ 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; + + /*static*/ MaterialTypeSourceData::PropertyGroup* MaterialTypeSourceData::PropertyGroup::AddPropertyGroup(AZStd::string_view name, AZStd::vector>& toPropertyGroupList) + { + auto iter = AZStd::find_if(toPropertyGroupList.begin(), toPropertyGroupList.end(), [name](const AZStd::unique_ptr& existingPropertyGroup) + { + return existingPropertyGroup->m_name == name; + }); + + if (iter != toPropertyGroupList.end()) + { + AZ_Error("Material source data", false, "PropertyGroup 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; + } + + toPropertyGroupList.push_back(AZStd::make_unique()); + toPropertyGroupList.back()->m_name = name; + return toPropertyGroupList.back().get(); + } + + MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::PropertyGroup::AddProperty(AZStd::string_view name) + { + auto propertyIter = AZStd::find_if(m_properties.begin(), m_properties.end(), [name](const AZStd::unique_ptr& existingProperty) + { + return existingProperty->GetName() == name; + }); + + if (propertyIter != m_properties.end()) + { + AZ_Error("Material source data", false, "PropertyGroup '%s' already contains a property named '%.*s'", m_name.c_str(), AZ_STRING_ARG(name)); + return nullptr; + } + + auto propertyGroupIter = AZStd::find_if(m_propertyGroups.begin(), m_propertyGroups.end(), [name](const AZStd::unique_ptr& existingPropertyGroup) + { + return existingPropertyGroup->m_name == name; + }); + + if (propertyGroupIter != m_propertyGroups.end()) + { + AZ_Error("Material source data", false, "Property name '%.*s' collides with a PropertyGroup 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(name)); + return m_properties.back().get(); + } + + MaterialTypeSourceData::PropertyGroup* MaterialTypeSourceData::PropertyGroup::AddPropertyGroup(AZStd::string_view name) + { + auto iter = AZStd::find_if(m_properties.begin(), m_properties.end(), [name](const AZStd::unique_ptr& existingProperty) + { + return existingProperty->GetName() == name; + }); + + if (iter != m_properties.end()) + { + AZ_Error("Material source data", false, "PropertyGroup name '%.*s' collides with a Property of the same name", AZ_STRING_ARG(name)); + return nullptr; + } + + return AddPropertyGroup(name, m_propertyGroups); + } + + MaterialTypeSourceData::PropertyGroup* MaterialTypeSourceData::AddPropertyGroup(AZStd::string_view propertyGroupId) + { + AZStd::vector splitPropertyGroupId = SplitId(propertyGroupId); + + if (splitPropertyGroupId.size() == 1) + { + return PropertyGroup::AddPropertyGroup(propertyGroupId, m_propertyLayout.m_propertyGroups); + } + + PropertyGroup* parentPropertyGroup = FindPropertyGroup(splitPropertyGroupId[0]); + + if (!parentPropertyGroup) + { + AZ_Error("Material source data", false, "PropertyGroup '%.*s' does not exists", AZ_STRING_ARG(splitPropertyGroupId[0])); + return nullptr; + } + + return parentPropertyGroup->AddPropertyGroup(splitPropertyGroupId[1]); + } + + MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::AddProperty(AZStd::string_view propertyId) + { + AZStd::vector splitPropertyId = SplitId(propertyId); + + if (splitPropertyId.size() == 1) + { + AZ_Error("Material source data", false, "Property id '%.*s' is invalid. Properties must be added to a PropertyGroup (i.e. \"general.%.*s\").", AZ_STRING_ARG(propertyId), AZ_STRING_ARG(propertyId)); + return nullptr; + } + + PropertyGroup* parentPropertyGroup = FindPropertyGroup(splitPropertyId[0]); + + if (!parentPropertyGroup) + { + AZ_Error("Material source data", false, "PropertyGroup '%.*s' does not exists", AZ_STRING_ARG(splitPropertyId[0])); + return nullptr; + } - const MaterialTypeSourceData::GroupDefinition* MaterialTypeSourceData::FindGroup(AZStd::string_view groupName) const + return parentPropertyGroup->AddProperty(splitPropertyId[1]); + } + + const MaterialTypeSourceData::PropertyGroup* MaterialTypeSourceData::FindPropertyGroup(AZStd::span parsedPropertyGroupId, AZStd::span> inPropertyGroupList) const { - for (const GroupDefinition& group : m_propertyLayout.m_groups) + for (const auto& propertyGroup : inPropertyGroupList) { - if (group.m_name == groupName) + if (propertyGroup->m_name != parsedPropertyGroupId[0]) + { + continue; + } + else if (parsedPropertyGroupId.size() == 1) { - return &group; + return propertyGroup.get(); + } + else + { + AZStd::span subPath{parsedPropertyGroupId.begin() + 1, parsedPropertyGroupId.end()}; + + if (!subPath.empty()) + { + const MaterialTypeSourceData::PropertyGroup* propertySubset = FindPropertyGroup(subPath, propertyGroup->m_propertyGroups); + if (propertySubset) + { + return propertySubset; + } + } } } return nullptr; } + + MaterialTypeSourceData::PropertyGroup* MaterialTypeSourceData::FindPropertyGroup(AZStd::span parsedPropertyGroupId, AZStd::span> inPropertyGroupList) + { + return const_cast(const_cast(this)->FindPropertyGroup(parsedPropertyGroupId, inPropertyGroupList)); + } - bool MaterialTypeSourceData::ApplyPropertyRenames(MaterialPropertyId& propertyId) const + const MaterialTypeSourceData::PropertyGroup* MaterialTypeSourceData::FindPropertyGroup(AZStd::string_view propertyGroupId) const { - bool renamed = false; + AZStd::vector tokens = TokenizeId(propertyGroupId); + return FindPropertyGroup(tokens, m_propertyLayout.m_propertyGroups); + } - for (const VersionUpdateDefinition& versionUpdate : m_versionUpdates) + MaterialTypeSourceData::PropertyGroup* MaterialTypeSourceData::FindPropertyGroup(AZStd::string_view propertyGroupId) + { + AZStd::vector tokens = TokenizeId(propertyGroupId); + return FindPropertyGroup(tokens, m_propertyLayout.m_propertyGroups); + } + + const MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty( + AZStd::span parsedPropertyId, + AZStd::span> inPropertyGroupList) const + { + for (const auto& propertyGroup : inPropertyGroupList) { - for (const VersionUpdatesRenameOperationDefinition& action : versionUpdate.m_actions) + if (propertyGroup->m_name == parsedPropertyId[0]) { - if (action.m_operation == "rename") + AZStd::span subPath {parsedPropertyId.begin() + 1, parsedPropertyId.end()}; + + if (subPath.size() == 1) { - if (action.m_renameFrom == propertyId.GetStringView()) + for (AZStd::unique_ptr& property : propertyGroup->m_properties) { - propertyId = MaterialPropertyId::Parse(action.m_renameTo); - renamed = true; + if (property->GetName() == subPath[0]) + { + return property.get(); + } } } - else + else if(subPath.size() > 1) { - AZ_Warning("Material source data", false, "Unsupported material version update operation '%s'", action.m_operation.c_str()); + const MaterialTypeSourceData::PropertyDefinition* property = FindProperty(subPath, propertyGroup->m_propertyGroups); + if (property) + { + return property; + } } } } - return renamed; + return nullptr; + } + + MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::span parsedPropertyId, AZStd::span> inPropertyGroupList) + { + return const_cast(const_cast(this)->FindProperty(parsedPropertyId, inPropertyGroupList)); + } + + const MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::string_view propertyId) const + { + AZStd::vector tokens = TokenizeId(propertyId); + return FindProperty(tokens, m_propertyLayout.m_propertyGroups); + } + + MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::string_view propertyId) + { + AZStd::vector tokens = TokenizeId(propertyId); + return FindProperty(tokens, m_propertyLayout.m_propertyGroups); + } + + AZStd::vector MaterialTypeSourceData::TokenizeId(AZStd::string_view id) + { + AZStd::vector tokens; + + AzFramework::StringFunc::TokenizeVisitor(id, [&tokens](AZStd::string_view t) + { + tokens.push_back(t); + }, + "./", 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::EnumeratePropertyGroups(const EnumeratePropertyGroupsCallback& callback, AZStd::string propertyNameContext, const AZStd::vector>& inPropertyGroupList) const + { + for (auto& propertyGroup : inPropertyGroupList) + { + if (!callback(propertyNameContext, propertyGroup.get())) + { + return false; // Stop processing + } + + const AZStd::string propertyNameContext2 = propertyNameContext + propertyGroup->m_name + "."; + + if (!EnumeratePropertyGroups(callback, propertyNameContext2, propertyGroup->m_propertyGroups)) + { + return false; // Stop processing + } + } + + return true; } - const MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::string_view groupName, AZStd::string_view propertyName) const + bool MaterialTypeSourceData::EnumeratePropertyGroups(const EnumeratePropertyGroupsCallback& callback) const { - auto groupIter = m_propertyLayout.m_properties.find(groupName); - if (groupIter != m_propertyLayout.m_properties.end()) + if (!callback) { - for (const PropertyDefinition& property : groupIter->second) + return false; + } + + return EnumeratePropertyGroups(callback, {}, m_propertyLayout.m_propertyGroups); + } + + bool MaterialTypeSourceData::EnumerateProperties(const EnumeratePropertiesCallback& callback, AZStd::string propertyNameContext, const AZStd::vector>& inPropertyGroupList) const + { + + for (auto& propertyGroup : inPropertyGroupList) + { + const AZStd::string propertyNameContext2 = propertyNameContext + propertyGroup->m_name + "."; + + for (auto& property : propertyGroup->m_properties) { - if (property.m_name == propertyName) + if (!callback(propertyNameContext2, property.get())) { - return &property; + return false; // Stop processing } } + + if (!EnumerateProperties(callback, propertyNameContext2, propertyGroup->m_propertyGroups)) + { + return false; // Stop processing + } } - // Property has not been found, try looking for renames in the version history + return true; + } - MaterialPropertyId propertyId = MaterialPropertyId{groupName, propertyName}; - ApplyPropertyRenames(propertyId); + bool MaterialTypeSourceData::EnumerateProperties(const EnumeratePropertiesCallback& callback) const + { + if (!callback) + { + return false; + } - // Do the search again with the new names + return EnumerateProperties(callback, {}, m_propertyLayout.m_propertyGroups); + } - AZStd::vector tokens; - AZ::StringFunc::Tokenize(propertyId.GetStringView(), tokens, ".", true, true); - if (tokens.size() == 2) + bool MaterialTypeSourceData::ConvertToNewDataFormat() + { + for (const auto& group : GetOldFormatGroupDefinitionsInDisplayOrder()) { - groupIter = m_propertyLayout.m_properties.find(tokens[0]); - if (groupIter != m_propertyLayout.m_properties.end()) + auto propertyListItr = m_propertyLayout.m_propertiesOld.find(group.m_name); + if (propertyListItr != m_propertyLayout.m_propertiesOld.end()) { - for (const PropertyDefinition& property : groupIter->second) + const auto& propertyList = propertyListItr->second; + for (auto& propertyDefinition : propertyList) { - if (property.m_name == tokens[1]) + PropertyGroup* propertyGroup = FindPropertyGroup(group.m_name); + + if (!propertyGroup) { - return &property; + m_propertyLayout.m_propertyGroups.emplace_back(AZStd::make_unique()); + m_propertyLayout.m_propertyGroups.back()->m_name = group.m_name; + m_propertyLayout.m_propertyGroups.back()->m_displayName = group.m_displayName; + m_propertyLayout.m_propertyGroups.back()->m_description = group.m_description; + propertyGroup = m_propertyLayout.m_propertyGroups.back().get(); } + + PropertyDefinition* newProperty = propertyGroup->AddProperty(propertyDefinition.GetName()); + + *newProperty = propertyDefinition; } } } - return nullptr; + m_propertyLayout.m_groupsOld.clear(); + m_propertyLayout.m_propertiesOld.clear(); + + return true; } void MaterialTypeSourceData::ResolveUvEnums() @@ -205,27 +482,27 @@ 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()); + groupDefinitions.reserve(m_propertyLayout.m_propertiesOld.size()); // Some groups are defined explicitly in the .materialtype file's "groups" section. This is the primary way groups are sorted in the UI. AZStd::unordered_set foundGroups; - for (const auto& groupDefinition : m_propertyLayout.m_groups) + for (const auto& groupDefinition : m_propertyLayout.m_groupsOld) { if (foundGroups.insert(groupDefinition.m_name).second) { @@ -238,7 +515,7 @@ namespace AZ } // Some groups are defined implicitly, in the "properties" section where a group name is used but not explicitly defined in the "groups" section. - for (const auto& propertyListPair : m_propertyLayout.m_properties) + for (const auto& propertyListPair : m_propertyLayout.m_propertiesOld) { const AZStd::string& groupName = propertyListPair.first; if (foundGroups.insert(groupName).second) @@ -252,54 +529,183 @@ namespace AZ return groupDefinitions; } - void MaterialTypeSourceData::EnumerateProperties(const EnumeratePropertiesCallback& callback) const - { - if (!callback) + bool MaterialTypeSourceData::BuildPropertyList( + const AZStd::string& materialTypeSourceFilePath, + MaterialTypeAssetCreator& materialTypeAssetCreator, + AZStd::vector& propertyNameContext, + const MaterialTypeSourceData::PropertyGroup* propertyGroup) const + { + for (const AZStd::unique_ptr& property : propertyGroup->m_properties) { - return; - } + // Register the property... - for (const auto& propertyListPair : m_propertyLayout.m_properties) - { - const AZStd::string& groupName = propertyListPair.first; - const auto& propertyList = propertyListPair.second; - for (const auto& propertyDefinition : propertyList) + MaterialPropertyId propertyId{propertyNameContext, property->GetName()}; + + if (!propertyId.IsValid()) + { + // MaterialPropertyId reports an error message + return false; + } + + auto propertyGroupIter = AZStd::find_if(propertyGroup->GetPropertyGroups().begin(), propertyGroup->GetPropertyGroups().end(), + [&property](const AZStd::unique_ptr& existingPropertyGroup) + { + return existingPropertyGroup->GetName() == property->GetName(); + }); + + if (propertyGroupIter != propertyGroup->GetPropertyGroups().end()) + { + AZ_Error("Material source data", false, "Material property '%s' collides with a PropertyGroup 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) { - const AZStd::string& propertyName = propertyDefinition.m_name; - if (!callback(groupName, propertyName, propertyDefinition)) + switch (output.m_type) { - return; + 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; } } - } - } - void MaterialTypeSourceData::EnumeratePropertiesInDisplayOrder(const EnumeratePropertiesCallback& callback) const - { - if (!callback) + 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: + { + Data::Asset imageAsset; + + MaterialUtils::GetImageAssetResult result = MaterialUtils::GetImageAssetReference( + imageAsset, materialTypeSourceFilePath, property->m_value.GetValue()); + + if (result == MaterialUtils::GetImageAssetResult::Missing) + { + materialTypeAssetCreator.ReportError( + "Material property '%s': Could not find the image '%s'", propertyId.GetCStr(), + property->m_value.GetValue().data()); + } + else + { + materialTypeAssetCreator.SetPropertyValue(propertyId, imageAsset); + } + } + 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 : propertyGroup->m_propertyGroups) { - return; + propertyNameContext.push_back(propertySubset->m_name); + + bool success = BuildPropertyList( + materialTypeSourceFilePath, + materialTypeAssetCreator, + propertyNameContext, + propertySubset.get()); + + propertyNameContext.pop_back(); + + if (!success) + { + return false; + } } - for (const auto& groupDefinition : GetGroupDefinitionsInDisplayOrder()) + // 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 : propertyGroup->m_materialFunctorSourceData) { - const AZStd::string& groupName = groupDefinition.m_name; - const auto propertyListItr = m_propertyLayout.m_properties.find(groupName); - if (propertyListItr != m_propertyLayout.m_properties.end()) + MaterialFunctorSourceData::FunctorResult result = functorData->CreateFunctor( + MaterialFunctorSourceData::RuntimeContext( + materialTypeSourceFilePath, + materialTypeAssetCreator.GetMaterialPropertiesLayout(), + materialTypeAssetCreator.GetMaterialShaderResourceGroupLayout(), + materialTypeAssetCreator.GetShaderCollection() + ) + ); + + if (result.IsSuccess()) { - const auto& propertyList = propertyListItr->second; - for (const auto& propertyDefinition : propertyList) + Ptr& functor = result.GetValue(); + if (functor != nullptr) { - const AZStd::string& propertyName = propertyDefinition.m_name; - if (!callback(groupName, propertyName, propertyDefinition)) + materialTypeAssetCreator.AddMaterialFunctor(functor); + + for (const AZ::Name& optionName : functorData->GetActualSourceData()->GetShaderOptionDependencies()) { - return; + 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; @@ -388,108 +794,16 @@ namespace AZ return Failure(); } } - - for (auto& groupIter : m_propertyLayout.m_properties) + + for (const AZStd::unique_ptr& propertyGroup : m_propertyLayout.m_propertyGroups) { - const AZStd::string& groupName = groupIter.first; + AZStd::vector propertyNameContext; + propertyNameContext.push_back(propertyGroup->m_name); + bool success = BuildPropertyList(materialTypeSourceFilePath, materialTypeAssetCreator, propertyNameContext, propertyGroup.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: - { - Data::Asset imageAsset; - - MaterialUtils::GetImageAssetResult result = MaterialUtils::GetImageAssetReference( - imageAsset, materialTypeSourceFilePath, property.m_value.GetValue()); - - if (result == MaterialUtils::GetImageAssetResult::Missing) - { - materialTypeAssetCreator.ReportError( - "Material property '%s': Could not find the image '%s'", propertyId.GetCStr(), - property.m_value.GetValue().data()); - } - else - { - materialTypeAssetCreator.SetPropertyValue(propertyId, imageAsset); - } - } - 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 475c6ca219..b03771173f 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp @@ -102,6 +102,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/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/MaterialPropertySerializerTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialPropertySerializerTests.cpp index 9afc05a237..904860d4fc 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialPropertySerializerTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialPropertySerializerTests.cpp @@ -45,8 +45,7 @@ namespace JsonSerializationTests AZStd::shared_ptr CreatePartialDefaultInstance() override { - auto result = AZStd::make_shared(); - result->m_name = "testProperty"; + auto result = AZStd::make_shared("testProperty"); result->m_dataType = AZ::RPI::MaterialPropertyDataType::Float; result->m_step = 1.0f; result->m_value = 0.0f; @@ -65,8 +64,7 @@ namespace JsonSerializationTests AZStd::shared_ptr CreateFullySetInstance() override { - auto result = AZStd::make_shared(); - result->m_name = "testProperty"; + auto result = AZStd::make_shared("testProperty"); result->m_description = "description"; result->m_displayName = "display_name"; result->m_dataType = AZ::RPI::MaterialPropertyDataType::Float; @@ -135,7 +133,7 @@ namespace JsonSerializationTests const AZ::RPI::MaterialTypeSourceData::PropertyDefinition& lhs, const AZ::RPI::MaterialTypeSourceData::PropertyDefinition& rhs) override { - if (lhs.m_name != rhs.m_name) { return false; } + if (lhs.GetName() != rhs.GetName()) { return false; } if (lhs.m_description != rhs.m_description) { return false; } if (lhs.m_displayName != rhs.m_displayName) { return false; } if (lhs.m_dataType != rhs.m_dataType) { return false; } @@ -216,7 +214,7 @@ namespace UnitTest EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing()); EXPECT_EQ(AZ::JsonSerializationResult::Outcomes::PartialDefaults, loadResult.m_jsonResultCode.GetOutcome()); - EXPECT_EQ("testProperty", propertyData.m_name); + EXPECT_EQ("testProperty", propertyData.GetName()); EXPECT_EQ("Test Property", propertyData.m_displayName); EXPECT_EQ("This is a property description", propertyData.m_description); EXPECT_EQ(MaterialPropertyDataType::Float, propertyData.m_dataType); @@ -851,7 +849,7 @@ namespace UnitTest EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask()); EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing()); - EXPECT_EQ("testProperty", propertyData.m_name); + EXPECT_EQ("testProperty", propertyData.GetName()); EXPECT_EQ(1, propertyData.m_outputConnections.size()); EXPECT_EQ(MaterialPropertyOutputType::ShaderOption, propertyData.m_outputConnections[0].m_type); @@ -934,7 +932,7 @@ namespace UnitTest EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask()); EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing()); - EXPECT_EQ(propertyData.m_name, "testProperty"); + EXPECT_EQ(propertyData.GetName(), "testProperty"); EXPECT_EQ(propertyData.m_dataType, MaterialPropertyDataType::Float); EXPECT_EQ(propertyData.m_outputConnections.size(), 0); @@ -964,7 +962,7 @@ namespace UnitTest EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask()); EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing()); - EXPECT_EQ(propertyData.m_name, "testProperty"); + EXPECT_EQ(propertyData.GetName(), "testProperty"); EXPECT_EQ(propertyData.m_dataType, MaterialPropertyDataType::Float); EXPECT_EQ(propertyData.m_outputConnections.size(), 1); EXPECT_EQ(propertyData.m_outputConnections[0].m_fieldName, "o_foo"); diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp index 133592e72c..2bd257d55b 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp @@ -93,20 +93,23 @@ namespace UnitTest { "version": 10, "propertyLayout": { - "properties": { - "general": [ - {"name": "MyBool", "type": "bool"}, - {"name": "MyInt", "type": "Int"}, - {"name": "MyUInt", "type": "UInt"}, - {"name": "MyFloat", "type": "Float"}, - {"name": "MyFloat2", "type": "Vector2"}, - {"name": "MyFloat3", "type": "Vector3"}, - {"name": "MyFloat4", "type": "Vector4"}, - {"name": "MyColor", "type": "Color"}, - {"name": "MyImage", "type": "Image"}, - {"name": "MyEnum", "type": "Enum", "enumValues": ["Enum0", "Enum1", "Enum2"], "defaultValue": "Enum0"} - ] - } + "propertyGroups": [ + { + "name": "general", + "properties": [ + {"name": "MyBool", "type": "bool"}, + {"name": "MyInt", "type": "Int"}, + {"name": "MyUInt", "type": "UInt"}, + {"name": "MyFloat", "type": "Float"}, + {"name": "MyFloat2", "type": "Vector2"}, + {"name": "MyFloat3", "type": "Vector3"}, + {"name": "MyFloat4", "type": "Vector4"}, + {"name": "MyColor", "type": "Color"}, + {"name": "MyImage", "type": "Image"}, + {"name": "MyEnum", "type": "Enum", "enumValues": ["Enum0", "Enum1", "Enum2"], "defaultValue": "Enum0"} + ] + } + ] }, "shaders": [ { @@ -466,18 +469,22 @@ namespace UnitTest TEST_F(MaterialSourceDataTests, Load_MaterialTypeAfterPropertyList) { const AZStd::string simpleMaterialTypeJson = R"( - { - "propertyLayout": { - "properties": { - "general": [ + { + "propertyLayout": { + "propertyGroups": + [ { - "name": "testValue", - "type": "Float" + "name": "general", + "properties": [ + { + "name": "testValue", + "type": "Float" + } + ] } ] } } - } )"; const char* materialTypeFilePath = "@exefolder@/Temp/simpleMaterialType.materialtype"; diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp index 2fe7b1dbe5..206073e96a 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp @@ -226,6 +226,7 @@ namespace UnitTest { serializeContext->Class() ->Version(1) + ->Field("enableProperty", &SetShaderOptionFunctorSourceData::m_enablePropertyName) ; } } @@ -244,6 +245,8 @@ namespace UnitTest Ptr functor = aznew SetShaderOptionFunctor; return Success(Ptr(functor)); } + + AZStd::string m_enablePropertyName; }; ///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -351,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 groups and properties, using a variety of different Add functions, + // going through the MaterialTypeSourceData or going to the PropertyGroup directly. + + MaterialTypeSourceData::PropertyGroup* layer1 = sourceData.AddPropertyGroup("layer1"); + MaterialTypeSourceData::PropertyGroup* layer2 = sourceData.AddPropertyGroup("layer2"); + MaterialTypeSourceData::PropertyGroup* blend = sourceData.AddPropertyGroup("blend"); + + MaterialTypeSourceData::PropertyGroup* layer1_baseColor = layer1->AddPropertyGroup("baseColor"); + MaterialTypeSourceData::PropertyGroup* layer2_baseColor = layer2->AddPropertyGroup("baseColor"); + + MaterialTypeSourceData::PropertyGroup* layer1_roughness = sourceData.AddPropertyGroup("layer1.roughness"); + MaterialTypeSourceData::PropertyGroup* layer2_roughness = sourceData.AddPropertyGroup("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::PropertyGroup* layer2_clearCoat = layer2->AddPropertyGroup("clearCoat"); + MaterialTypeSourceData::PropertyGroup* layer2_clearCoat_roughness = layer2_clearCoat->AddPropertyGroup("roughness"); + MaterialTypeSourceData::PropertyGroup* layer2_clearCoat_normal = layer2_clearCoat->AddPropertyGroup("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 group, not a property + EXPECT_EQ(nullptr, sourceData.FindPropertyGroup("baseColor.texture")); // This is a property, not a property group + + EXPECT_EQ(layer1, sourceData.FindPropertyGroup("layer1")); + EXPECT_EQ(layer2, sourceData.FindPropertyGroup("layer2")); + EXPECT_EQ(blend, sourceData.FindPropertyGroup("blend")); + + EXPECT_EQ(layer1_baseColor, sourceData.FindPropertyGroup("layer1.baseColor")); + EXPECT_EQ(layer2_baseColor, sourceData.FindPropertyGroup("layer2.baseColor")); + + EXPECT_EQ(layer1_roughness, sourceData.FindPropertyGroup("layer1.roughness")); + EXPECT_EQ(layer2_roughness, sourceData.FindPropertyGroup("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.FindPropertyGroup("layer2.clearCoat")); + EXPECT_EQ(layer2_clearCoat_roughness, sourceData.FindPropertyGroup("layer2.clearCoat.roughness")); + EXPECT_EQ(layer2_clearCoat_normal, sourceData.FindPropertyGroup("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 EnumeratePropertyGroups + + struct EnumeratePropertyGroupsResult + { + AZStd::string m_propertyIdContext; + const MaterialTypeSourceData::PropertyGroup* m_propertyGroup; + + void Check(AZStd::string expectedIdContext, const MaterialTypeSourceData::PropertyGroup* expectedPropertyGroup) + { + EXPECT_EQ(expectedIdContext, m_propertyIdContext); + EXPECT_EQ(expectedPropertyGroup, m_propertyGroup); + } + }; + AZStd::vector enumeratePropertyGroupsResults; + + sourceData.EnumeratePropertyGroups([&enumeratePropertyGroupsResults](const AZStd::string& propertyIdContext, const MaterialTypeSourceData::PropertyGroup* propertyGroup) + { + enumeratePropertyGroupsResults.push_back(EnumeratePropertyGroupsResult{propertyIdContext, propertyGroup}); + return true; + }); + + int resultIndex = 0; + enumeratePropertyGroupsResults[resultIndex++].Check("", layer1); + enumeratePropertyGroupsResults[resultIndex++].Check("layer1.", layer1_baseColor); + enumeratePropertyGroupsResults[resultIndex++].Check("layer1.", layer1_roughness); + enumeratePropertyGroupsResults[resultIndex++].Check("", layer2); + enumeratePropertyGroupsResults[resultIndex++].Check("layer2.", layer2_baseColor); + enumeratePropertyGroupsResults[resultIndex++].Check("layer2.", layer2_roughness); + enumeratePropertyGroupsResults[resultIndex++].Check("layer2.", layer2_clearCoat); + enumeratePropertyGroupsResults[resultIndex++].Check("layer2.clearCoat.", layer2_clearCoat_roughness); + enumeratePropertyGroupsResults[resultIndex++].Check("layer2.clearCoat.", layer2_clearCoat_normal); + enumeratePropertyGroupsResults[resultIndex++].Check("", blend); + EXPECT_EQ(resultIndex, enumeratePropertyGroupsResults.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::PropertyGroup* propertyGroup = sourceData.AddPropertyGroup("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(propertyGroup->AddProperty("")); + EXPECT_FALSE(propertyGroup->AddProperty("main.")); + EXPECT_FALSE(sourceData.AddProperty("main.base-color")); + + EXPECT_TRUE(propertyGroup->GetProperties().empty()); + + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialTypeSourceDataTests, AddPropertyGroup_Error_InvalidName) + { + MaterialTypeSourceData sourceData; + + MaterialTypeSourceData::PropertyGroup* propertyGroup = sourceData.AddPropertyGroup("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(propertyGroup->AddPropertyGroup("")); + EXPECT_FALSE(sourceData.AddPropertyGroup("")); + EXPECT_FALSE(sourceData.AddPropertyGroup("base-color")); + EXPECT_FALSE(sourceData.AddPropertyGroup("general.look@it")); + + EXPECT_TRUE(propertyGroup->GetProperties().empty()); + + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialTypeSourceDataTests, AddProperty_Error_AddDuplicateProperty) + { + MaterialTypeSourceData sourceData; + + MaterialTypeSourceData::PropertyGroup* propertyGroup = sourceData.AddPropertyGroup("main"); + + ErrorMessageFinder errorMessageFinder; + errorMessageFinder.AddExpectedErrorMessage("PropertyGroup 'main' already contains a property named 'foo'", 2); + + EXPECT_TRUE(propertyGroup->AddProperty("foo")); + EXPECT_FALSE(propertyGroup->AddProperty("foo")); + EXPECT_FALSE(sourceData.AddProperty("main.foo")); + + EXPECT_EQ(propertyGroup->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 PropertyGroup"); + EXPECT_FALSE(sourceData.AddProperty("foo")); + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialTypeSourceDataTests, AddProperty_Error_PropertyGroupDoesNotExist ) + { + MaterialTypeSourceData sourceData; + ErrorMessageFinder errorMessageFinder("PropertyGroup 'DNE' does not exists"); + EXPECT_FALSE(sourceData.AddProperty("DNE.foo")); + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialTypeSourceDataTests, AddPropertyGroup_Error_PropertyGroupDoesNotExist ) + { + MaterialTypeSourceData sourceData; + ErrorMessageFinder errorMessageFinder("PropertyGroup 'DNE' does not exists"); + EXPECT_FALSE(sourceData.AddPropertyGroup("DNE.foo")); + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialTypeSourceDataTests, AddPropertyGroup_Error_AddDuplicatePropertyGroup) + { + MaterialTypeSourceData sourceData; + + MaterialTypeSourceData::PropertyGroup* propertyGroup = sourceData.AddPropertyGroup("main"); + sourceData.AddPropertyGroup("main.level2"); + + ErrorMessageFinder errorMessageFinder; + errorMessageFinder.AddExpectedErrorMessage("PropertyGroup named 'main' already exists", 1); + errorMessageFinder.AddExpectedErrorMessage("PropertyGroup named 'level2' already exists", 2); + + EXPECT_FALSE(sourceData.AddPropertyGroup("main")); + EXPECT_FALSE(sourceData.AddPropertyGroup("main.level2")); + EXPECT_FALSE(propertyGroup->AddPropertyGroup("level2")); + + errorMessageFinder.CheckExpectedErrorsFound(); + + EXPECT_EQ(sourceData.GetPropertyLayout().m_propertyGroups.size(), 1); + EXPECT_EQ(propertyGroup->GetPropertyGroups().size(), 1); + } + + TEST_F(MaterialTypeSourceDataTests, AddPropertyGroup_Error_NameCollidesWithProperty ) + { + MaterialTypeSourceData sourceData; + sourceData.AddPropertyGroup("main"); + sourceData.AddProperty("main.foo"); + + ErrorMessageFinder errorMessageFinder("PropertyGroup name 'foo' collides with a Property of the same name"); + EXPECT_FALSE(sourceData.AddPropertyGroup("main.foo")); + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialTypeSourceDataTests, AddProperty_Error_NameCollidesWithPropertyGroup ) + { + MaterialTypeSourceData sourceData; + sourceData.AddPropertyGroup("main"); + sourceData.AddPropertyGroup("main.foo"); + + ErrorMessageFinder errorMessageFinder("Property name 'foo' collides with a PropertyGroup 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.AddPropertyGroup("a"); + sourceData.AddPropertyGroup("a.b"); + sourceData.AddPropertyGroup("c"); + sourceData.AddPropertyGroup("c.d"); + sourceData.AddPropertyGroup("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; @@ -510,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::PropertyGroup* propertyGroup = sourceData.AddPropertyGroup("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertyGroup->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(); @@ -526,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); } @@ -535,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::PropertyGroup* propertyGroup = sourceData.AddPropertyGroup("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertyGroup->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(); @@ -556,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); } @@ -565,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::PropertyGroup* propertyGroup = sourceData.AddPropertyGroup("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertyGroup->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(); @@ -581,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); } @@ -590,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::PropertyGroup* propertyGroup = sourceData.AddPropertyGroup("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertyGroup->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); } @@ -613,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::PropertyGroup* propertyGroup = sourceData.AddPropertyGroup("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertyGroup->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" @@ -627,66 +932,140 @@ namespace UnitTest EXPECT_FALSE(materialTypeOutcome.IsSuccess()); } - TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_Error_InvalidGroupNameId) + TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_Error_InvalidGroupName) { - 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); + const AZStd::string inputJson = R"( + { + "propertyLayout": { + "propertyGroups": [ + { + "name": "not a valid name because it has spaces", + "properties": [ + { + "name": "foo", + "type": "Bool" + } + ] + } + ] + } + } + )"; - // 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; + MaterialTypeSourceData sourceData; + 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) { - 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); + const AZStd::string inputJson = R"( + { + "propertyLayout": { + "propertyGroups": [ + { + "name": "general", + "properties": [ + { + "name": "not a valid name because it has spaces", + "type": "Bool" + } + ] + } + ] + } + } + )"; - // 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; + MaterialTypeSourceData sourceData; + 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) { - 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); + const AZStd::string inputJson = R"( + { + "propertyLayout": { + "propertyGroups": [ + { + "name": "general", + "properties": [ + { + "name": "foo", + "type": "Bool" + }, + { + "name": "foo", + "type": "Bool" + } + ] + } + ] + } + } + )"; - // 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; + MaterialTypeSourceData sourceData; + 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_PropertyAndPropertyGroupNameCollision) + { + const AZStd::string inputJson = R"( + { + "propertyLayout": { + "propertyGroups": [ + { + "name": "general", + "properties": [ + { + "name": "foo", + "type": "Bool" + } + ], + "propertyGroups": [ + { + "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 PropertyGroup with the same ID"); + auto materialTypeOutcome = sourceData.CreateMaterialTypeAsset(Uuid::CreateRandom()); EXPECT_FALSE(materialTypeOutcome.IsSuccess()); + errorMessageFinder.CheckExpectedErrorsFound(); } TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_PropertyConnectedToMultipleOutputs) @@ -737,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::PropertyGroup* propertyGroup = sourceData.AddPropertyGroup("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertyGroup->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... @@ -796,15 +1175,15 @@ namespace UnitTest TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_PropertyWithShaderInputFunctor) { MaterialTypeSourceData sourceData; + + MaterialTypeSourceData::PropertyGroup* propertyGroup = sourceData.AddPropertyGroup("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertyGroup->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( @@ -812,7 +1191,7 @@ namespace UnitTest ( aznew MaterialFunctorSourceDataHolder ( - aznew Splat3FunctorSourceData{ "general.NonAliasFloat", "m_float3" } + aznew Splat3FunctorSourceData{ "general.floatForFunctor", "m_float3" } ) ) ); @@ -821,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()); @@ -842,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::PropertyGroup* propertyGroup = sourceData.AddPropertyGroup("general"); + MaterialTypeSourceData::PropertyDefinition* property1 = propertyGroup->AddProperty("EnableSpecialPassA"); + MaterialTypeSourceData::PropertyDefinition* property2 = propertyGroup->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 @@ -881,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()); @@ -901,13 +1278,13 @@ namespace UnitTest sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{TestShaderFilename}); sourceData.m_shaderCollection.push_back(MaterialTypeSourceData::ShaderVariantReferenceData{TestShaderFilename}); + + MaterialTypeSourceData::PropertyGroup* propertyGroup = sourceData.AddPropertyGroup("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertyGroup->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 ( @@ -929,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_FunctorIsInsidePropertyGroup) + { + MaterialTypeSourceData sourceData; + + MaterialTypeSourceData::PropertyGroup* propertyGroup = sourceData.AddPropertyGroup("general"); + MaterialTypeSourceData::PropertyDefinition* property = propertyGroup->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) { @@ -938,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; }; - - 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}); + + sourceData.AddPropertyGroup("general"); + + 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()); @@ -971,6 +1387,88 @@ namespace UnitTest CheckPropertyValue>(materialTypeAsset, Name{"general.MyImage"}, m_testImageAsset); } + TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_NestedPropertyGroups) + { + 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.AddPropertyGroup("layer1"); + sourceData.AddPropertyGroup("layer2"); + sourceData.AddPropertyGroup("blend"); + sourceData.AddPropertyGroup("layer1.baseColor"); + sourceData.AddPropertyGroup("layer2.baseColor"); + sourceData.AddPropertyGroup("layer1.roughness"); + sourceData.AddPropertyGroup("layer2.roughness"); + sourceData.AddPropertyGroup("layer2.clearCoat"); + sourceData.AddPropertyGroup("layer2.clearCoat.roughness"); + sourceData.AddPropertyGroup("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 @@ -989,46 +1487,92 @@ namespace UnitTest } ], "propertyLayout": { - "groups": [ + "propertyGroups": [ { "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 group", + "propertyGroups": [ + { + "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": [ { @@ -1049,17 +1593,9 @@ namespace UnitTest ], "functors": [ { - "type": "EnableShader", - "args": { - "enablePassProperty": "groupA.foo", - "shaderIndex": 1 - } - }, - { - "type": "Splat3", + "type": "SetShaderOption", "args": { - "floatPropertyInput": "groupB.foo", - "float3ShaderSettingOutput": "m_someFloat3" + "enableProperty": "groupA.foo" } } ] @@ -1071,6 +1607,7 @@ namespace UnitTest EXPECT_EQ(material.m_description, "This is a general description about the material"); + EXPECT_EQ(material.m_version, 2); EXPECT_EQ(material.m_versionUpdates.size(), 1); EXPECT_EQ(material.m_versionUpdates[0].m_toVersion, 2); @@ -1078,33 +1615,71 @@ namespace UnitTest EXPECT_EQ(material.m_versionUpdates[0].m_actions[0].m_renameFrom, "groupA.fooPrev"); EXPECT_EQ(material.m_versionUpdates[0].m_actions[0].m_renameTo, "groupA.foo"); - 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.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.GetPropertyLayout().m_propertyGroups.size(), 3); + EXPECT_TRUE(material.FindPropertyGroup("groupA") != nullptr); + EXPECT_TRUE(material.FindPropertyGroup("groupB") != nullptr); + EXPECT_TRUE(material.FindPropertyGroup("groupC") != nullptr); + EXPECT_TRUE(material.FindPropertyGroup("groupC.groupD") != nullptr); + EXPECT_TRUE(material.FindPropertyGroup("groupC.groupE") != nullptr); + EXPECT_EQ(material.FindPropertyGroup("groupA")->GetDisplayName(), "Property Group A"); + EXPECT_EQ(material.FindPropertyGroup("groupB")->GetDisplayName(), "Property Group B"); + EXPECT_EQ(material.FindPropertyGroup("groupC")->GetDisplayName(), "Property Group C"); + EXPECT_EQ(material.FindPropertyGroup("groupC.groupD")->GetDisplayName(), "Property Group D"); + EXPECT_EQ(material.FindPropertyGroup("groupC.groupE")->GetDisplayName(), "Property Group E"); + EXPECT_EQ(material.FindPropertyGroup("groupA")->GetDescription(), "Description of property group A"); + EXPECT_EQ(material.FindPropertyGroup("groupB")->GetDescription(), "Description of property group B"); + EXPECT_EQ(material.FindPropertyGroup("groupC")->GetDescription(), "Property group C has a nested property group"); + EXPECT_EQ(material.FindPropertyGroup("groupC.groupD")->GetDescription(), "Description of property group D"); + EXPECT_EQ(material.FindPropertyGroup("groupC.groupE")->GetDescription(), "Description of property group E"); + EXPECT_EQ(material.FindPropertyGroup("groupA")->GetProperties().size(), 2); + EXPECT_EQ(material.FindPropertyGroup("groupB")->GetProperties().size(), 2); + EXPECT_EQ(material.FindPropertyGroup("groupC")->GetProperties().size(), 0); + EXPECT_EQ(material.FindPropertyGroup("groupC.groupD")->GetProperties().size(), 1); + EXPECT_EQ(material.FindPropertyGroup("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.FindProperty("groupA.foo")->GetName(), "foo"); + EXPECT_EQ(material.FindProperty("groupA.bar")->GetName(), "bar"); + EXPECT_EQ(material.FindProperty("groupB.foo")->GetName(), "foo"); + EXPECT_EQ(material.FindProperty("groupB.bar")->GetName(), "bar"); + EXPECT_EQ(material.FindProperty("groupC.groupD.foo")->GetName(), "foo"); + EXPECT_EQ(material.FindProperty("groupC.groupE.bar")->GetName(), "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.FindPropertyGroup("groupA")->GetFunctors().size(), 1); + EXPECT_EQ(material.FindPropertyGroup("groupB")->GetFunctors().size(), 1); + Ptr functorA = material.FindPropertyGroup("groupA")->GetFunctors()[0]->GetActualSourceData(); + Ptr functorB = material.FindPropertyGroup("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"); @@ -1117,13 +1692,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); @@ -1134,6 +1706,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 unified property group definition const AZStd::string inputJson = R"( { @@ -1220,35 +1795,55 @@ 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_groupsOld.size(), 2); + EXPECT_EQ(material.GetPropertyLayout().m_propertiesOld.size(), 2); + EXPECT_EQ(material.GetPropertyLayout().m_propertyGroups.size(), 0); + + material.ConvertToNewDataFormat(); + + // After conversion to the new format, the data is in the new place + EXPECT_EQ(material.GetPropertyLayout().m_groupsOld.size(), 0); + EXPECT_EQ(material.GetPropertyLayout().m_propertiesOld.size(), 0); + EXPECT_EQ(material.GetPropertyLayout().m_propertyGroups.size(), 2); + EXPECT_EQ(material.m_description, "This is a general description about the material"); - 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.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_TRUE(material.FindPropertyGroup("groupA") != nullptr); + EXPECT_TRUE(material.FindPropertyGroup("groupB") != nullptr); + EXPECT_EQ(material.FindPropertyGroup("groupA")->GetDisplayName(), "Property Group A"); + EXPECT_EQ(material.FindPropertyGroup("groupB")->GetDisplayName(), "Property Group B"); + EXPECT_EQ(material.FindPropertyGroup("groupA")->GetDescription(), "Description of property group A"); + EXPECT_EQ(material.FindPropertyGroup("groupB")->GetDescription(), "Description of property group B"); + EXPECT_EQ(material.FindPropertyGroup("groupA")->GetProperties().size(), 2); + EXPECT_EQ(material.FindPropertyGroup("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.FindProperty("groupA.foo")->GetName(), "foo"); + EXPECT_EQ(material.FindProperty("groupA.bar")->GetName(), "bar"); + EXPECT_EQ(material.FindProperty("groupB.foo")->GetName(), "foo"); + EXPECT_EQ(material.FindProperty("groupB.bar")->GetName(), "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 group. The format conversion + // function doesn't know how to move the functors, and they will be left at the top level. + EXPECT_EQ(material.FindPropertyGroup("groupA")->GetFunctors().size(), 0); + EXPECT_EQ(material.FindPropertyGroup("groupB")->GetFunctors().size(), 0); EXPECT_EQ(material.m_shaderCollection.size(), 2); EXPECT_EQ(material.m_shaderCollection[0].m_shaderFilePath, "ForwardPass.shader"); @@ -1269,7 +1864,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]; @@ -1278,27 +1873,25 @@ namespace UnitTest { "description": "", "propertyLayout": { - "groups": [ + "propertyGroups": [ { "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" - } - ] - } + ] } } )", @@ -1318,157 +1911,13 @@ namespace UnitTest } - TEST_F(MaterialTypeSourceDataTests, FindPropertyUsingOldName) - { - const AZStd::string inputJson = R"( - { - "version": 10, - "versionUpdates": [ - { - "toVersion": 2, - "actions": [ - { "op": "rename", "from": "general.fooA", "to": "general.fooB" } - ] - }, - { - "toVersion": 4, - "actions": [ - { "op": "rename", "from": "general.barA", "to": "general.barB" } - ] - }, - { - "toVersion": 6, - "actions": [ - { "op": "rename", "from": "general.fooB", "to": "general.fooC" }, - { "op": "rename", "from": "general.barB", "to": "general.barC" } - ] - }, - { - "toVersion": 7, - "actions": [ - { "op": "rename", "from": "general.bazA", "to": "otherGroup.bazB" }, - { "op": "rename", "from": "onlyOneProperty.bopA", "to": "otherGroup.bopB" } // This tests a group 'onlyOneProperty' that no longer exists in the material type - ] - } - ], - "propertyLayout": { - "properties": { - "general": [ - { - "name": "fooC", - "type": "Bool" - }, - { - "name": "barC", - "type": "Float" - } - ], - "otherGroup": [ - { - "name": "dontMindMe", - "type": "Bool" - }, - { - "name": "bazB", - "type": "Float" - }, - { - "name": "bopB", - "type": "Float" - } - ] - } - } - } - )"; - - MaterialTypeSourceData materialType; - JsonTestResult loadResult = LoadTestDataFromJson(materialType, inputJson); - - EXPECT_EQ(materialType.m_version, 10); - - // First find the properties using their correct current names - const MaterialTypeSourceData::PropertyDefinition* foo = materialType.FindProperty("general", "fooC"); - const MaterialTypeSourceData::PropertyDefinition* bar = materialType.FindProperty("general", "barC"); - const MaterialTypeSourceData::PropertyDefinition* baz = materialType.FindProperty("otherGroup", "bazB"); - const MaterialTypeSourceData::PropertyDefinition* bop = materialType.FindProperty("otherGroup", "bopB"); - - EXPECT_TRUE(foo); - EXPECT_TRUE(bar); - EXPECT_TRUE(baz); - EXPECT_TRUE(bop); - EXPECT_EQ(foo->m_name, "fooC"); - EXPECT_EQ(bar->m_name, "barC"); - EXPECT_EQ(baz->m_name, "bazB"); - EXPECT_EQ(bop->m_name, "bopB"); - - // Now try doing the property lookup using old versions of the name and make sure the same property can be found - - EXPECT_EQ(foo, materialType.FindProperty("general", "fooA")); - EXPECT_EQ(foo, materialType.FindProperty("general", "fooB")); - EXPECT_EQ(bar, materialType.FindProperty("general", "barA")); - EXPECT_EQ(bar, materialType.FindProperty("general", "barB")); - EXPECT_EQ(baz, materialType.FindProperty("general", "bazA")); - EXPECT_EQ(bop, materialType.FindProperty("onlyOneProperty", "bopA")); - - EXPECT_EQ(nullptr, materialType.FindProperty("general", "fooX")); - EXPECT_EQ(nullptr, materialType.FindProperty("general", "barX")); - EXPECT_EQ(nullptr, materialType.FindProperty("general", "bazX")); - EXPECT_EQ(nullptr, materialType.FindProperty("general", "bazB")); - EXPECT_EQ(nullptr, materialType.FindProperty("otherGroup", "bazA")); - EXPECT_EQ(nullptr, materialType.FindProperty("onlyOneProperty", "bopB")); - EXPECT_EQ(nullptr, materialType.FindProperty("otherGroup", "bopA")); - } - - TEST_F(MaterialTypeSourceDataTests, FindPropertyUsingOldName_Error_UnsupportedVersionUpdate) - { - const AZStd::string inputJson = R"( - { - "version": 10, - "versionUpdates": [ - { - "toVersion": 2, - "actions": [ - { "op": "notRename", "from": "general.fooA", "to": "general.fooB" } - ] - } - ], - "propertyLayout": { - "properties": { - "general": [ - { - "name": "fooB", - "type": "Bool" - } - ] - } - } - } - )"; - - MaterialTypeSourceData materialType; - JsonTestResult loadResult = LoadTestDataFromJson(materialType, inputJson); - - ErrorMessageFinder errorMessageFinder; - errorMessageFinder.AddExpectedErrorMessage("Unsupported material version update operation 'notRename'"); - - - const MaterialTypeSourceData::PropertyDefinition* foo = materialType.FindProperty("general", "fooA"); - - EXPECT_EQ(nullptr, foo); - - errorMessageFinder.CheckExpectedErrorsFound(); - } - TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_Error_UnsupportedVersionUpdate) { MaterialTypeSourceData sourceData; - - MaterialTypeSourceData::PropertyDefinition propertySource; - propertySource.m_name = "a"; - propertySource.m_dataType = MaterialPropertyDataType::Int; - propertySource.m_value = 0; - sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource); + + MaterialTypeSourceData::PropertyDefinition* propertySource = sourceData.AddPropertyGroup("general")->AddProperty("a"); + propertySource->m_dataType = MaterialPropertyDataType::Int; + propertySource->m_value = 0; sourceData.m_version = 2; diff --git a/Gems/Atom/TestData/TestData/Materials/Types/MinimalPBR.materialtype b/Gems/Atom/TestData/TestData/Materials/Types/MinimalPBR.materialtype index 5d99737576..641b0089a5 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.", "version": 3, "propertyLayout": { - "groups": [ + "propertyGroups": [ { "name": "settings", - "displayName": "Settings" - } - ], - "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" + "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" + } } - } - ] - } + ] + } + ] }, "shaders": [ { diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Util/MaterialPropertyUtil.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Util/MaterialPropertyUtil.cpp index 568e701e3c..377fd46d69 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Util/MaterialPropertyUtil.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Util/MaterialPropertyUtil.cpp @@ -78,7 +78,7 @@ namespace AtomToolsFramework void ConvertToPropertyConfig(AtomToolsFramework::DynamicPropertyConfig& propertyConfig, const AZ::RPI::MaterialTypeSourceData::PropertyDefinition& propertyDefinition) { propertyConfig.m_dataType = ConvertToEditableType(propertyDefinition.m_dataType); - propertyConfig.m_name = propertyDefinition.m_name; + propertyConfig.m_name = propertyDefinition.GetName(); propertyConfig.m_displayName = propertyDefinition.m_displayName; propertyConfig.m_description = propertyDefinition.m_description; propertyConfig.m_defaultValue = ConvertToEditableType(propertyDefinition.m_value); diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index 6ec809e871..c441762710 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -585,9 +585,9 @@ namespace MaterialEditor bool result = true; // populate sourceData with properties that meet the filter - m_materialTypeSourceData.EnumerateProperties([&](const AZStd::string& groupName, const AZStd::string& propertyName, const auto& propertyDefinition) { + m_materialTypeSourceData.EnumerateProperties([&](const AZStd::string& propertyIdContext, const auto& propertyDefinition) { - const MaterialPropertyId propertyId(groupName, propertyName); + Name propertyId{propertyIdContext + propertyDefinition->GetName()}; const auto it = m_properties.find(propertyId); if (it != m_properties.end() && propertyFilter(it->second)) @@ -595,14 +595,16 @@ namespace MaterialEditor MaterialPropertyValue propertyValue = AtomToolsFramework::ConvertToRuntimeType(it->second.GetValue()); if (propertyValue.IsValid()) { - if (!AtomToolsFramework::ConvertToExportFormat(exportPath, propertyId, propertyDefinition, propertyValue)) + if (!AtomToolsFramework::ConvertToExportFormat(exportPath, propertyId, *propertyDefinition, propertyValue)) { AZ_Error("MaterialDocument", false, "Material document property could not be converted: '%s' in '%s'.", propertyId.GetCStr(), 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 groups, not just the top level. + const AZStd::string groupName = propertyId.GetStringView().substr(0, propertyId.GetStringView().size() - propertyDefinition->GetName().size() - 1); + sourceData.m_properties[groupName][propertyDefinition->GetName()].m_value = propertyValue; } } return true; @@ -679,7 +681,7 @@ namespace MaterialEditor AZ_Error("MaterialDocument", false, "Material type source data could not be loaded: '%s'.", m_materialSourceData.m_materialType.c_str()); return false; } - m_materialTypeSourceData = materialTypeOutcome.GetValue(); + m_materialTypeSourceData = materialTypeOutcome.TakeValue(); } else if (AzFramework::StringFunc::Path::IsExtension(m_absolutePath.c_str(), MaterialTypeSourceData::Extension)) { @@ -692,7 +694,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(); // We are storing absolute paths in the loaded version of the source data so that the files can be resolved at all times. m_materialSourceData.m_materialType = m_absolutePath; @@ -779,33 +781,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; + m_materialTypeSourceData.EnumeratePropertyGroups([this, &parentPropertyValues](const AZStd::string& propertyIdContext, const MaterialTypeSourceData::PropertyGroup* propertyGroup) + { + AtomToolsFramework::DynamicPropertyConfig propertyConfig; - // Assign id before conversion so it can be used in dynamic description - propertyConfig.m_id = MaterialPropertyId(groupName, propertyName); + for (const auto& propertyDefinition : propertyGroup->GetProperties()) + { + // Assign id before conversion so it can be used in dynamic description + propertyConfig.m_id = propertyIdContext + propertyGroup->GetName() + "." + propertyDefinition->GetName(); - 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()); + 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()]); - auto groupDefinition = m_materialTypeSourceData.FindGroup(groupName); - propertyConfig.m_groupName = groupDefinition ? groupDefinition->m_displayName : groupName; - m_properties[propertyConfig.m_id] = AtomToolsFramework::DynamicProperty(propertyConfig); - } - return true; - }); + 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 groups, not just the top level. + // (Does DynamicPropertyConfig really even need m_groupName?) + propertyConfig.m_groupName = propertyGroup->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 groups, not just the top level. + for (const AZStd::unique_ptr& propertyGroup : m_materialTypeSourceData.GetPropertyLayout().m_propertyGroups) { - m_propertyGroupVisibility[AZ::Name{group.m_name}] = true; + m_propertyGroupVisibility[AZ::Name{propertyGroup->GetName()}] = true; } // Adding properties for material type and parent as part of making dynamic @@ -867,6 +877,7 @@ namespace MaterialEditor m_properties[propertyConfig.m_id] = AtomToolsFramework::DynamicProperty(propertyConfig); } + // Add material functors that are in the top-level functors list. const MaterialFunctorSourceData::EditorContext editorContext = MaterialFunctorSourceData::EditorContext(m_materialSourceData.m_materialType, m_materialAsset->GetMaterialPropertiesLayout()); for (Ptr functorData : m_materialTypeSourceData.m_materialFunctorSourceData) @@ -887,6 +898,40 @@ namespace MaterialEditor return false; } } + + // Add any material functors that are located inside each property group. + bool enumerateResult = m_materialTypeSourceData.EnumeratePropertyGroups( + [this](const AZStd::string&, const MaterialTypeSourceData::PropertyGroup* propertyGroup) + { + const MaterialFunctorSourceData::EditorContext editorContext = MaterialFunctorSourceData::EditorContext( + m_materialSourceData.m_materialType, m_materialAsset->GetMaterialPropertiesLayout()); + + for (Ptr functorData : propertyGroup->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 0af051894d..1402ad9f24 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp @@ -170,28 +170,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 groups, not just the top level. + for (const AZStd::unique_ptr& propertyGroup : materialTypeSourceData->GetPropertyLayout().m_propertyGroups) { - 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 = propertyGroup->GetName(); + const AZStd::string& groupDisplayName = !propertyGroup->GetDisplayName().empty() ? propertyGroup->GetDisplayName() : groupName; + const AZStd::string& groupDescription = !propertyGroup->GetDescription().empty() ? propertyGroup->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(propertyGroup->GetProperties().size()); + for (const auto& propertyDefinition : propertyGroup->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->GetName())); + 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 b20fe5802d..dde81e77b5 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp @@ -293,44 +293,39 @@ namespace AZ void MaterialPropertyInspector::AddPropertiesGroup() { // 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 groups, not just the top level. + for (const AZStd::unique_ptr& propertyGroup : m_editData.m_materialTypeSourceData.GetPropertyLayout().m_propertyGroups) { - 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 = propertyGroup->GetName(); + const AZStd::string& groupDisplayName = !propertyGroup->GetDisplayName().empty() ? propertyGroup->GetDisplayName() : groupName; + const AZStd::string& groupDescription = !propertyGroup->GetDescription().empty() ? propertyGroup->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(propertyGroup->GetProperties().size()); + for (const auto& propertyDefinition : propertyGroup->GetProperties()) { - group.m_properties.reserve(propertyListItr->second.size()); - for (const auto& propertyDefinition : propertyListItr->second) - { - 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); - - AtomToolsFramework::ConvertToPropertyConfig(propertyConfig, propertyDefinition); - - const auto& propertyIndex = - m_editData.m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyConfig.m_id); - - propertyConfig.m_groupName = groupDisplayName; - propertyConfig.m_showThumbnail = true; - - propertyConfig.m_defaultValue = AtomToolsFramework::ConvertToEditableType( - m_editData.m_materialTypeAsset->GetDefaultPropertyValues()[propertyIndex.GetIndex()]); - - // There is no explicit parent material here. Material instance property overrides replace the values from the - // assigned material asset. Its values should be treated as parent, for comparison, in this case. - propertyConfig.m_parentValue = AtomToolsFramework::ConvertToEditableType( - m_editData.m_materialAsset->GetPropertyValues()[propertyIndex.GetIndex()]); - propertyConfig.m_originalValue = AtomToolsFramework::ConvertToEditableType( - m_editData.m_materialAsset->GetPropertyValues()[propertyIndex.GetIndex()]); - group.m_properties.emplace_back(propertyConfig); - } + AtomToolsFramework::DynamicPropertyConfig propertyConfig; + + // Assign id before conversion so it can be used in dynamic description + propertyConfig.m_id = AZ::RPI::MaterialPropertyId(groupName, propertyDefinition->GetName()); + + AtomToolsFramework::ConvertToPropertyConfig(propertyConfig, *propertyDefinition.get()); + + const auto& propertyIndex = + m_editData.m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyConfig.m_id); + + propertyConfig.m_groupName = groupDisplayName; + propertyConfig.m_showThumbnail = true; + propertyConfig.m_defaultValue = AtomToolsFramework::ConvertToEditableType( + m_editData.m_materialTypeAsset->GetDefaultPropertyValues()[propertyIndex.GetIndex()]); + + // There is no explicit parent material here. Material instance property overrides replace the values from the + // assigned material asset. Its values should be treated as parent, for comparison, in this case. + propertyConfig.m_parentValue = AtomToolsFramework::ConvertToEditableType( + m_editData.m_materialTypeAsset->GetDefaultPropertyValues()[propertyIndex.GetIndex()]); + propertyConfig.m_originalValue = AtomToolsFramework::ConvertToEditableType( + m_editData.m_materialAsset->GetPropertyValues()[propertyIndex.GetIndex()]); + group.m_properties.emplace_back(propertyConfig); } // 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 d4dd4f3a0e..dde9644c50 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp @@ -92,7 +92,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; } @@ -113,43 +113,46 @@ namespace AZ // Copy all of the properties from the material asset to the source data that will be exported bool 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()) - { - 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 (!AtomToolsFramework::ConvertToExportFormat(path, propertyId, 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) + editData.m_materialTypeSourceData.EnumerateProperties([&](const AZStd::string& propertyIdContext, const AZ::RPI::MaterialTypeSourceData::PropertyDefinition* propertyDefinition) { + AZ::Name propertyId(propertyIdContext + propertyDefinition->GetName()); + 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()) + { + 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 (!AtomToolsFramework::ConvertToExportFormat(path, propertyId, *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 groups, not just the top level. + const AZStd::string groupName = propertyId.GetStringView().substr(0, propertyId.GetStringView().size() - propertyDefinition->GetName().size() - 1); + exportData.m_properties[groupName][propertyDefinition->GetName()].m_value = propertyValue; return true; - } - - exportData.m_properties[groupName][propertyDefinition.m_name].m_value = propertyValue; - return true; - }); + }); return result && AZ::RPI::JsonUtils::SaveObjectToFile(path, exportData); }