Merge pull request #7228 from aws-lumberyard-dev/Atom/santorac/RemixableMaterialTypes3

Reorganized Material Type Properties and Groups

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;

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<const AZStd::string> names);
+            explicit MaterialPropertyId(const AZStd::span<AZStd::string> names);
+            MaterialPropertyId(const AZStd::span<AZStd::string> groupNames, AZStd::string_view propertyName);
 
             AZ_DEFAULT_COPY_MOVE(MaterialPropertyId);
 

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<AZStd::unique_ptr<PropertyDefinition>>;
+
+            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<AZStd::unique_ptr<PropertyGroup>>& GetPropertyGroups() const { return m_propertyGroups; }
+                const AZStd::vector<Ptr<MaterialFunctorSourceDataHolder>>& 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<AZStd::unique_ptr<PropertyGroup>>& toPropertyGroupList);
+
+                AZStd::string m_name;
+                AZStd::string m_displayName;
+                AZStd::string m_description;
+                PropertyList m_properties;
+                AZStd::vector<AZStd::unique_ptr<PropertyGroup>> m_propertyGroups;
+                AZStd::vector<Ptr<MaterialFunctorSourceDataHolder>> m_materialFunctorSourceData;
             };
 
             struct ShaderVariantReferenceData
@@ -118,8 +182,6 @@ namespace AZ
                 AZStd::unordered_map<Name/*shaderOption*/, Name/*value*/> m_shaderOptionValues;
             };
 
-            using PropertyList = AZStd::vector<PropertyDefinition>;
-
             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<GroupDefinition> m_groups;
+                AZStd::vector<GroupDefinition> m_groupsOld;
 
+                //! [Deprecated] Use m_propertyGroups instead
+                AZStd::map<AZStd::string /*group name*/, AZStd::vector<PropertyDefinition>> m_propertiesOld;
+                
                 //! Collection of all available user-facing properties
-                AZStd::map<AZStd::string /*group name*/, PropertyList> m_properties;
+                AZStd::vector<AZStd::unique_ptr<PropertyGroup>> 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<ShaderVariantReferenceData> 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<GroupDefinition> 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<bool(
-                const AZStd::string&, // The name of the group containing the property
-                const AZStd::string&, // The name of the property
-                const PropertyDefinition& // the property definition object that corresponds to the group and property names
+            //! Find the PropertyGroup with the given ID.
+            //! @param propertyGroupId The full ID of a property group to find, like "levelA.levelB.levelC".
+            //! @return the found PropertyGroup or null if it doesn't exist.
+            const PropertyGroup* FindPropertyGroup(AZStd::string_view propertyGroupId) const;
+            PropertyGroup* FindPropertyGroup(AZStd::string_view propertyGroupId);
+            
+            //! Find the definition for a property with the given ID.
+            //! @param propertyId The full ID of a property to find, like "baseColor.texture".
+            //! @return the found PropertyDefinition or null if it doesn't exist.
+            const PropertyDefinition* FindProperty(AZStd::string_view propertyId) const;
+            PropertyDefinition* FindProperty(AZStd::string_view propertyId);
+
+            //! Tokenizes an ID string like "itemA.itemB.itemC" into a vector like ["itemA", "itemB", "itemC"].
+            static AZStd::vector<AZStd::string_view> TokenizeId(AZStd::string_view id);
+            
+            //! Splits an ID string like "itemA.itemB.itemC" into a vector like ["itemA.itemB", "itemC"].
+            static AZStd::vector<AZStd::string_view> SplitId(AZStd::string_view id);
+
+            //! Call back function type used with the enumeration functions.
+            //! Return false to terminate the traversal.
+            using EnumeratePropertyGroupsCallback = AZStd::function<bool(
+                const AZStd::string&, // The property ID context (i.e. "levelA.levelB.")
+                const PropertyGroup* // the next property group in the tree
                 )>;
 
-            //! 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<bool(
+                const AZStd::string&, // The property ID context (i.e. "levelA.levelB."
+                const PropertyDefinition* // the property definition object 
+                )>;
+            
+            //! 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<Data::Asset<MaterialTypeAsset>> 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<const AZStd::string_view> parsedPropertyGroupId, AZStd::span<const AZStd::unique_ptr<PropertyGroup>> inPropertyGroupList) const;
+            PropertyGroup* FindPropertyGroup(AZStd::span<AZStd::string_view> parsedPropertyGroupId, AZStd::span<AZStd::unique_ptr<PropertyGroup>> 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<const AZStd::string_view> parsedPropertyId, AZStd::span<const AZStd::unique_ptr<PropertyGroup>> inPropertyGroupList) const;
+            PropertyDefinition* FindProperty(AZStd::span<AZStd::string_view> parsedPropertyId, AZStd::span<AZStd::unique_ptr<PropertyGroup>> inPropertyGroupList);
+            
+            // Function overloads for recursion, returns false to indicate that recursion should end.
+            bool EnumeratePropertyGroups(const EnumeratePropertyGroupsCallback& callback, AZStd::string propertyIdContext, const AZStd::vector<AZStd::unique_ptr<PropertyGroup>>& inPropertyGroupList) const;
+            bool EnumerateProperties(const EnumeratePropertiesCallback& callback, AZStd::string propertyIdContext, const AZStd::vector<AZStd::unique_ptr<PropertyGroup>>& 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<AZStd::string>& 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<GroupDefinition> 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<MaterialFunctorSourceData> GetActualSourceData() const { return m_actualSourceData; }
+            Ptr<MaterialFunctorSourceData> GetActualSourceData() const { return m_actualSourceData; }
         private:
             Ptr<MaterialFunctorSourceData> m_actualSourceData = nullptr; // The derived material functor instance.
         };

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<MaterialBuilder>();

Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyId.cpp

@@ -88,7 +88,37 @@ namespace AZ
         {
         }
         
-        MaterialPropertyId::MaterialPropertyId(const AZStd::span<const AZStd::string> names)
+        MaterialPropertyId::MaterialPropertyId(const AZStd::span<AZStd::string> 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<AZStd::string> names)
         {
             for (const auto& name : names)
             {

Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyValueSerializer.cpp

@@ -9,6 +9,7 @@
 #include <Atom/RPI.Edit/Material/MaterialPropertyValueSerializer.h>
 #include <Atom/RPI.Edit/Material/MaterialTypeSourceData.h>
 #include <Atom/RPI.Edit/Material/MaterialPropertySerializer.h>
+#include <Atom/RPI.Edit/Material/MaterialPropertyId.h>
 
 #include <AzCore/Math/Color.h>
 #include <AzCore/Math/Vector2.h>

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);

Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp

@@ -57,7 +57,11 @@ namespace AZ
                 serializeContext->Class<PropertyConnection>()->Version(3);
                 serializeContext->Class<GroupDefinition>()->Version(4);
                 serializeContext->Class<PropertyDefinition>()->Version(1);
-
+                
+                serializeContext->RegisterGenericType<AZStd::unique_ptr<PropertyGroup>>();
+                serializeContext->RegisterGenericType<AZStd::unique_ptr<PropertyDefinition>>();
+                serializeContext->RegisterGenericType<AZStd::vector<AZStd::unique_ptr<PropertyGroup>>>();
+                serializeContext->RegisterGenericType<AZStd::vector<AZStd::unique_ptr<PropertyDefinition>>>();
                 serializeContext->RegisterGenericType<PropertyConnectionList>();
 
                 serializeContext->Class<VersionUpdatesRenameOperationDefinition>()
@@ -84,11 +88,22 @@ namespace AZ
                     ->Field("options", &ShaderVariantReferenceData::m_shaderOptionValues)
                     ;
 
+                serializeContext->Class<PropertyGroup>()
+                    ->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<PropertyLayout>()
-                    ->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<UvNameMap>();
@@ -112,83 +127,148 @@ namespace AZ
             , m_shaderIndex(shaderIndex)
         {
         }
-
+        
         const float MaterialTypeSourceData::PropertyDefinition::DefaultMin = std::numeric_limits<float>::lowest();
         const float MaterialTypeSourceData::PropertyDefinition::DefaultMax = std::numeric_limits<float>::max();
         const float MaterialTypeSourceData::PropertyDefinition::DefaultStep = 0.1f;
-
-        const MaterialTypeSourceData::GroupDefinition* MaterialTypeSourceData::FindGroup(AZStd::string_view groupName) const
+        
+        /*static*/ MaterialTypeSourceData::PropertyGroup* MaterialTypeSourceData::PropertyGroup::AddPropertyGroup(AZStd::string_view name, AZStd::vector<AZStd::unique_ptr<PropertyGroup>>& toPropertyGroupList)
         {
-            for (const GroupDefinition& group : m_propertyLayout.m_groups)
-            {
-                if (group.m_name == groupName)
+            auto iter = AZStd::find_if(toPropertyGroupList.begin(), toPropertyGroupList.end(), [name](const AZStd::unique_ptr<PropertyGroup>& existingPropertyGroup)
                 {
-                    return &group;
-                }
+                    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;
             }
 
-            return nullptr;
+            toPropertyGroupList.push_back(AZStd::make_unique<PropertyGroup>());
+            toPropertyGroupList.back()->m_name = name;
+            return toPropertyGroupList.back().get();
         }
 
-        bool MaterialTypeSourceData::ApplyPropertyRenames(MaterialPropertyId& propertyId) const
+        MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::PropertyGroup::AddProperty(AZStd::string_view name)
         {
-            bool renamed = false;
-
-            for (const VersionUpdateDefinition& versionUpdate : m_versionUpdates)
-            {
-                for (const VersionUpdatesRenameOperationDefinition& action : versionUpdate.m_actions)
+            auto propertyIter = AZStd::find_if(m_properties.begin(), m_properties.end(), [name](const AZStd::unique_ptr<PropertyDefinition>& existingProperty)
                 {
-                    if (action.m_operation == "rename")
-                    {
-                        if (action.m_renameFrom == propertyId.GetStringView())
-                        {
-                            propertyId = MaterialPropertyId::Parse(action.m_renameTo);
-                            renamed = true;
-                        }
-                    }
-                    else
-                    {
-                        AZ_Warning("Material source data", false, "Unsupported material version update operation '%s'", action.m_operation.c_str());
-                    }
-                }
+                    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<PropertyGroup>& 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;
             }
 
-            return renamed;
+            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<PropertyDefinition>(name));
+            return m_properties.back().get();
         }
 
-        const MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::string_view groupName, AZStd::string_view propertyName) const
+        MaterialTypeSourceData::PropertyGroup* MaterialTypeSourceData::PropertyGroup::AddPropertyGroup(AZStd::string_view name)
         {
-            auto groupIter = m_propertyLayout.m_properties.find(groupName);
-            if (groupIter != m_propertyLayout.m_properties.end())
-            {
-                for (const PropertyDefinition& property : groupIter->second)
+            auto iter = AZStd::find_if(m_properties.begin(), m_properties.end(), [name](const AZStd::unique_ptr<PropertyDefinition>& existingProperty)
                 {
-                    if (property.m_name == propertyName)
-                    {
-                        return &property;
-                    }
-                }
+                    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;
             }
 
-            // Property has not been found, try looking for renames in the version history
+            return AddPropertyGroup(name, m_propertyGroups);
+        }
+        
+        MaterialTypeSourceData::PropertyGroup* MaterialTypeSourceData::AddPropertyGroup(AZStd::string_view propertyGroupId)
+        {
+            AZStd::vector<AZStd::string_view> splitPropertyGroupId = SplitId(propertyGroupId);
 
-            MaterialPropertyId propertyId = MaterialPropertyId{groupName, propertyName};
-            ApplyPropertyRenames(propertyId);
-
-            // Do the search again with the new names
-
-            AZStd::vector<AZStd::string> tokens;
-            AZ::StringFunc::Tokenize(propertyId.GetStringView(), tokens, ".", true, true);
-            if (tokens.size() == 2)
+            if (splitPropertyGroupId.size() == 1)
             {
-                groupIter = m_propertyLayout.m_properties.find(tokens[0]);
-                if (groupIter != m_propertyLayout.m_properties.end())
+                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<AZStd::string_view> 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;
+            }
+
+            return parentPropertyGroup->AddProperty(splitPropertyId[1]);
+        }
+        
+        const MaterialTypeSourceData::PropertyGroup* MaterialTypeSourceData::FindPropertyGroup(AZStd::span<const AZStd::string_view> parsedPropertyGroupId, AZStd::span<const AZStd::unique_ptr<PropertyGroup>> inPropertyGroupList) const
+        {
+            for (const auto& propertyGroup : inPropertyGroupList)
+            {
+                if (propertyGroup->m_name != parsedPropertyGroupId[0])
                 {
-                    for (const PropertyDefinition& property : groupIter->second)
+                    continue;
+                }
+                else if (parsedPropertyGroupId.size() == 1)
+                {
+                    return propertyGroup.get();
+                }
+                else
+                {
+                    AZStd::span<const AZStd::string_view> subPath{parsedPropertyGroupId.begin() + 1, parsedPropertyGroupId.end()};
+
+                    if (!subPath.empty())
                     {
-                        if (property.m_name == tokens[1])
+                        const MaterialTypeSourceData::PropertyGroup* propertySubset = FindPropertyGroup(subPath, propertyGroup->m_propertyGroups);
+                        if (propertySubset)
                         {
-                            return &property;
+                            return propertySubset;
                         }
                     }
                 }
@@ -196,6 +276,203 @@ namespace AZ
 
             return nullptr;
         }
+        
+        MaterialTypeSourceData::PropertyGroup* MaterialTypeSourceData::FindPropertyGroup(AZStd::span<AZStd::string_view> parsedPropertyGroupId, AZStd::span<AZStd::unique_ptr<PropertyGroup>> inPropertyGroupList)
+        {
+            return const_cast<PropertyGroup*>(const_cast<const MaterialTypeSourceData*>(this)->FindPropertyGroup(parsedPropertyGroupId, inPropertyGroupList));
+        }
+
+        const MaterialTypeSourceData::PropertyGroup* MaterialTypeSourceData::FindPropertyGroup(AZStd::string_view propertyGroupId) const
+        {
+            AZStd::vector<AZStd::string_view> tokens = TokenizeId(propertyGroupId);
+            return FindPropertyGroup(tokens, m_propertyLayout.m_propertyGroups);
+        }
+
+        MaterialTypeSourceData::PropertyGroup* MaterialTypeSourceData::FindPropertyGroup(AZStd::string_view propertyGroupId)
+        {
+            AZStd::vector<AZStd::string_view> tokens = TokenizeId(propertyGroupId);
+            return FindPropertyGroup(tokens, m_propertyLayout.m_propertyGroups);
+        }
+        
+        const MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(
+            AZStd::span<const AZStd::string_view> parsedPropertyId,
+            AZStd::span<const AZStd::unique_ptr<PropertyGroup>> inPropertyGroupList) const
+        {
+            for (const auto& propertyGroup : inPropertyGroupList)
+            {
+                if (propertyGroup->m_name == parsedPropertyId[0])
+                {
+                    AZStd::span<const AZStd::string_view> subPath {parsedPropertyId.begin() + 1, parsedPropertyId.end()};
+
+                    if (subPath.size() == 1)
+                    {
+                        for (AZStd::unique_ptr<PropertyDefinition>& property : propertyGroup->m_properties)
+                        {
+                            if (property->GetName() == subPath[0])
+                            {
+                                return property.get();
+                            }
+                        }
+                    }
+                    else if(subPath.size() > 1)
+                    {
+                        const MaterialTypeSourceData::PropertyDefinition* property = FindProperty(subPath, propertyGroup->m_propertyGroups);
+                        if (property)
+                        {
+                            return property;
+                        }
+                    }
+                }
+            }
+
+            return nullptr;
+        }
+        
+        MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::span<AZStd::string_view> parsedPropertyId, AZStd::span<AZStd::unique_ptr<PropertyGroup>> inPropertyGroupList)
+        {
+            return const_cast<MaterialTypeSourceData::PropertyDefinition*>(const_cast<const MaterialTypeSourceData*>(this)->FindProperty(parsedPropertyId, inPropertyGroupList));
+        }
+
+        const MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::string_view propertyId) const
+        {
+            AZStd::vector<AZStd::string_view> tokens = TokenizeId(propertyId);
+            return FindProperty(tokens, m_propertyLayout.m_propertyGroups);
+        }
+        
+        MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::string_view propertyId)
+        {
+            AZStd::vector<AZStd::string_view> tokens = TokenizeId(propertyId);
+            return FindProperty(tokens, m_propertyLayout.m_propertyGroups);
+        }
+
+        AZStd::vector<AZStd::string_view> MaterialTypeSourceData::TokenizeId(AZStd::string_view id)
+        {
+            AZStd::vector<AZStd::string_view> tokens;
+
+            AzFramework::StringFunc::TokenizeVisitor(id, [&tokens](AZStd::string_view t)
+                {
+                    tokens.push_back(t);
+                },
+                "./", true, true);
+
+            return tokens;
+        }
+        
+        AZStd::vector<AZStd::string_view> MaterialTypeSourceData::SplitId(AZStd::string_view id)
+        {
+            AZStd::vector<AZStd::string_view> 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<AZStd::unique_ptr<PropertyGroup>>& 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;
+        }
+
+        bool MaterialTypeSourceData::EnumeratePropertyGroups(const EnumeratePropertyGroupsCallback& callback) const
+        {
+            if (!callback)
+            {
+                return false;
+            }
+
+            return EnumeratePropertyGroups(callback, {}, m_propertyLayout.m_propertyGroups);
+        }
+
+        bool MaterialTypeSourceData::EnumerateProperties(const EnumeratePropertiesCallback& callback, AZStd::string propertyNameContext, const AZStd::vector<AZStd::unique_ptr<PropertyGroup>>& inPropertyGroupList) const
+        {
+
+            for (auto& propertyGroup : inPropertyGroupList)
+            {
+                const AZStd::string propertyNameContext2 = propertyNameContext + propertyGroup->m_name + ".";
+
+                for (auto& property : propertyGroup->m_properties)
+                {
+                    if (!callback(propertyNameContext2, property.get()))
+                    {
+                        return false; // Stop processing
+                    }
+                }
+
+                if (!EnumerateProperties(callback, propertyNameContext2, propertyGroup->m_propertyGroups))
+                {
+                    return false; // Stop processing
+                }
+            }
+
+            return true;
+        }
+
+        bool MaterialTypeSourceData::EnumerateProperties(const EnumeratePropertiesCallback& callback) const
+        {
+            if (!callback)
+            {
+                return false;
+            }
+
+            return EnumerateProperties(callback, {}, m_propertyLayout.m_propertyGroups);
+        }
+
+        bool MaterialTypeSourceData::ConvertToNewDataFormat()
+        {            
+            for (const auto& group : GetOldFormatGroupDefinitionsInDisplayOrder())
+            {
+                auto propertyListItr = m_propertyLayout.m_propertiesOld.find(group.m_name);
+                if (propertyListItr != m_propertyLayout.m_propertiesOld.end())
+                {
+                    const auto& propertyList = propertyListItr->second;
+                    for (auto& propertyDefinition : propertyList)
+                    {
+                        PropertyGroup* propertyGroup = FindPropertyGroup(group.m_name);
+
+                        if (!propertyGroup)
+                        {
+                            m_propertyLayout.m_propertyGroups.emplace_back(AZStd::make_unique<PropertyGroup>());
+                            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; 
+                    }
+                }
+            }
+
+            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<MaterialTypeSourceData::PropertyDefinition*>(property)->m_enumValues = enumValues;
                     }
-                }
-            }
+                    return true;
+                });
         }
 
-        AZStd::vector<MaterialTypeSourceData::GroupDefinition> MaterialTypeSourceData::GetGroupDefinitionsInDisplayOrder() const
+        AZStd::vector<MaterialTypeSourceData::GroupDefinition> MaterialTypeSourceData::GetOldFormatGroupDefinitionsInDisplayOrder() const
         {
             AZStd::vector<MaterialTypeSourceData::GroupDefinition> 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<AZStd::string> 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<AZStd::string>& propertyNameContext,
+            const MaterialTypeSourceData::PropertyGroup* propertyGroup) const
+        {            
+            for (const AZStd::unique_ptr<PropertyDefinition>& 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())
                 {
-                    const AZStd::string& propertyName = propertyDefinition.m_name;
-                    if (!callback(groupName, propertyName, propertyDefinition))
+                    // MaterialPropertyId reports an error message
+                    return false;
+                }
+
+                auto propertyGroupIter = AZStd::find_if(propertyGroup->GetPropertyGroups().begin(), propertyGroup->GetPropertyGroups().end(),
+                    [&property](const AZStd::unique_ptr<PropertyGroup>& existingPropertyGroup)
                     {
-                        return;
+                        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)
+                {
+                    switch (output.m_type)
+                    {
+                    case MaterialPropertyOutputType::ShaderInput:
+                    {
+                        materialTypeAssetCreator.ConnectMaterialPropertyToShaderInput(Name{output.m_fieldName});
+                        break;
+                    }
+                    case MaterialPropertyOutputType::ShaderOption:
+                    {
+                        if (output.m_shaderIndex >= 0)
+                        {
+                            materialTypeAssetCreator.ConnectMaterialPropertyToShaderOption(Name{output.m_fieldName}, output.m_shaderIndex);
+                        }
+                        else
+                        {
+                            materialTypeAssetCreator.ConnectMaterialPropertyToShaderOptions(Name{output.m_fieldName});
+                        }
+                        break;
+                    }
+                    case MaterialPropertyOutputType::Invalid:
+                        // Don't add any output mappings, this is the case when material functors are expected to process the property
+                        break;
+                    default:
+                        AZ_Assert(false, "Unsupported MaterialPropertyOutputType");
+                        return false;
+                    }
+                }
+
+                materialTypeAssetCreator.EndMaterialProperty();
+
+                // Parse and set the property's value...
+                if (!property->m_value.IsValid())
+                {
+                    AZ_Warning("Material source data", false, "Source data for material property value is invalid.");
+                }
+                else
+                {
+                    switch (property->m_dataType)
+                    {
+                    case MaterialPropertyDataType::Image:
+                    {
+                        Data::Asset<ImageAsset> imageAsset;
+
+                        MaterialUtils::GetImageAssetResult result = MaterialUtils::GetImageAssetReference(
+                            imageAsset, materialTypeSourceFilePath, property->m_value.GetValue<AZStd::string>());
+
+                        if (result == MaterialUtils::GetImageAssetResult::Missing)
+                        {
+                            materialTypeAssetCreator.ReportError(
+                                "Material property '%s': Could not find the image '%s'", propertyId.GetCStr(),
+                                property->m_value.GetValue<AZStd::string>().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<AZStd::string>());
+                        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;
                     }
                 }
             }
-        }
-
-        void MaterialTypeSourceData::EnumeratePropertiesInDisplayOrder(const EnumeratePropertiesCallback& callback) const
-        {
-            if (!callback)
+            
+            for (const AZStd::unique_ptr<PropertyGroup>& 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<MaterialFunctor>& 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<Data::Asset<MaterialTypeAsset>> 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>& propertyGroup : m_propertyLayout.m_propertyGroups)
             {
-                const AZStd::string& groupName = groupIter.first;
+                AZStd::vector<AZStd::string> 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> imageAsset;
-
-                            MaterialUtils::GetImageAssetResult result = MaterialUtils::GetImageAssetReference(
-                                imageAsset, materialTypeSourceFilePath, property.m_value.GetValue<AZStd::string>());
-
-                            if (result == MaterialUtils::GetImageAssetResult::Missing)
-                            {
-                                materialTypeAssetCreator.ReportError(
-                                    "Material property '%s': Could not find the image '%s'", propertyId.GetCStr(),
-                                    property.m_value.GetValue<AZStd::string>().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<AZStd::string>());
-                            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();
                 }
             }
 

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())

Gems/Atom/RPI/Code/Tests/Material/MaterialPropertyIdTests.cpp

@@ -94,6 +94,40 @@ namespace UnitTest
         errorMessageFinder.CheckExpectedErrorsFound();
     }
     
+    TEST_F(MaterialPropertyIdTests, TestConstructWithMultipleParentNamesSeparateFromPropertyName)
+    {
+        AZStd::vector<AZStd::string> 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<AZStd::string> 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<AZStd::string> 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");

Gems/Atom/RPI/Code/Tests/Material/MaterialPropertySerializerTests.cpp

@@ -45,8 +45,7 @@ namespace JsonSerializationTests
 
         AZStd::shared_ptr<AZ::RPI::MaterialTypeSourceData::PropertyDefinition> CreatePartialDefaultInstance() override
         {
-            auto result = AZStd::make_shared<AZ::RPI::MaterialTypeSourceData::PropertyDefinition>();
-            result->m_name = "testProperty";
+            auto result = AZStd::make_shared<AZ::RPI::MaterialTypeSourceData::PropertyDefinition>("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<AZ::RPI::MaterialTypeSourceData::PropertyDefinition> CreateFullySetInstance() override
         {
-            auto result = AZStd::make_shared<AZ::RPI::MaterialTypeSourceData::PropertyDefinition>();
-            result->m_name = "testProperty";
+            auto result = AZStd::make_shared<AZ::RPI::MaterialTypeSourceData::PropertyDefinition>("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");

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";

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"
+                "displayName": "Settings",
+                "properties": [
+                    {
+                        "name": "color",
+                        "displayName": "Color",
+                        "type": "Color",
+                        "defaultValue": [ 1.0, 1.0, 1.0 ],
+                        "connection": {
+                            "type": "ShaderInput",
+                            "name": "m_baseColor"
+                        }
+                    },
+                    {
+                        "name": "metallic",
+                        "displayName": "Metallic",
+                        "type": "Float",
+                        "defaultValue": 0.0,
+                        "min": 0.0,
+                        "max": 1.0,
+                        "connection": {
+                            "type": "ShaderInput",
+                            "name": "m_metallic"
+                        }
+                    },
+                    {
+                        "name": "roughness",
+                        "displayName": "Roughness",
+                        "type": "Float",
+                        "defaultValue": 1.0,
+                        "min": 0.0,
+                        "max": 1.0,
+                        "connection": {
+                            "type": "ShaderInput",
+                            "name": "m_roughness"
+                        }
+                    }
+                ]
             }
-        ],
-        "properties": {
-            "settings": [
-                {
-                    "name": "color",
-                    "displayName": "Color",
-                    "type": "Color",
-                    "defaultValue": [ 1.0, 1.0, 1.0 ],
-                    "connection": {
-                        "type": "ShaderInput",
-                        "name": "m_baseColor"
-                    }
-                },
-                {
-                    "name": "metallic",
-                    "displayName": "Metallic",
-                    "type": "Float",
-                    "defaultValue": 0.0,
-                    "min": 0.0,
-                    "max": 1.0,
-                    "connection": {
-                        "type": "ShaderInput",
-                        "name": "m_metallic"
-                    }
-                },
-                {
-                    "name": "roughness",
-                    "displayName": "Roughness",
-                    "type": "Float",
-                    "defaultValue": 1.0,
-                    "min": 0.0,
-                    "max": 1.0,
-                    "connection": {
-                        "type": "ShaderInput",
-                        "name": "m_roughness"
-                    }
-                }
-            ]
-        }
+        ]
     },
     "shaders": [
         {

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);

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;
-
-            // Assign id before conversion so it can be used in dynamic description
-            propertyConfig.m_id = MaterialPropertyId(groupName, propertyName);
-
-            const auto& propertyIndex = m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyConfig.m_id);
-            const bool propertyIndexInBounds = propertyIndex.IsValid() && propertyIndex.GetIndex() < m_materialAsset->GetPropertyValues().size();
-            AZ_Warning("MaterialDocument", propertyIndexInBounds, "Failed to add material property '%s' to document '%s'.", propertyConfig.m_id.GetCStr(), m_absolutePath.c_str());
-
-            if (propertyIndexInBounds)
+        m_materialTypeSourceData.EnumeratePropertyGroups([this, &parentPropertyValues](const AZStd::string& propertyIdContext, const MaterialTypeSourceData::PropertyGroup* propertyGroup)
             {
-                AtomToolsFramework::ConvertToPropertyConfig(propertyConfig, propertyDefinition);
-                propertyConfig.m_showThumbnail = true;
-                propertyConfig.m_originalValue = AtomToolsFramework::ConvertToEditableType(m_materialAsset->GetPropertyValues()[propertyIndex.GetIndex()]);
-                propertyConfig.m_parentValue = AtomToolsFramework::ConvertToEditableType(parentPropertyValues[propertyIndex.GetIndex()]);
-                auto groupDefinition = m_materialTypeSourceData.FindGroup(groupName);
-                propertyConfig.m_groupName = groupDefinition ? groupDefinition->m_displayName : groupName;
-                m_properties[propertyConfig.m_id] = AtomToolsFramework::DynamicProperty(propertyConfig);
-            }
-            return true;
-        });
+                AtomToolsFramework::DynamicPropertyConfig propertyConfig;
+
+                for (const auto& propertyDefinition : 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());
+
+                    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<MaterialTypeSourceData::PropertyGroup>& 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<MaterialFunctorSourceDataHolder> 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<MaterialFunctorSourceDataHolder> functorData : propertyGroup->GetFunctors())
+                {
+                    MaterialFunctorSourceData::FunctorResult result = functorData->CreateFunctor(editorContext);
+
+                    if (result.IsSuccess())
+                    {
+                        Ptr<MaterialFunctor>& 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

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<AZ::RPI::MaterialTypeSourceData::PropertyGroup>& 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

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<AZ::RPI::MaterialTypeSourceData::PropertyGroup>& 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;
+                        AtomToolsFramework::DynamicPropertyConfig propertyConfig;
 
-                            // Assign id before conversion so it can be used in dynamic description
-                            propertyConfig.m_id = AZ::RPI::MaterialPropertyId(groupName, propertyDefinition.m_name);
+                        // Assign id before conversion so it can be used in dynamic description
+                        propertyConfig.m_id = AZ::RPI::MaterialPropertyId(groupName, propertyDefinition->GetName());
 
-                            AtomToolsFramework::ConvertToPropertyConfig(propertyConfig, propertyDefinition);
+                        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_materialAsset->GetPropertyValues()[propertyIndex.GetIndex()]);
-                            propertyConfig.m_originalValue = AtomToolsFramework::ConvertToEditableType(
-                                m_editData.m_materialAsset->GetPropertyValues()[propertyIndex.GetIndex()]);
-                            group.m_properties.emplace_back(propertyConfig);
-                        }
+                        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

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())
+                editData.m_materialTypeSourceData.EnumerateProperties([&](const AZStd::string& propertyIdContext, const AZ::RPI::MaterialTypeSourceData::PropertyDefinition* propertyDefinition)
                     {
-                        propertyValueDefault = editData.m_materialParentAsset->GetPropertyValues()[propertyIndex.GetIndex()];
-                    }
+                        AZ::Name propertyId(propertyIdContext + propertyDefinition->GetName());
+                        const AZ::RPI::MaterialPropertyIndex propertyIndex =
+                            editData.m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyId);
 
-                    // 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);
-                    }
+                        AZ::RPI::MaterialPropertyValue propertyValue =
+                            editData.m_materialAsset->GetPropertyValues()[propertyIndex.GetIndex()];
 
-                    if (!AtomToolsFramework::ConvertToExportFormat(path, propertyId, propertyDefinition, propertyValue))
-                    {
-                        AZ_Error("AZ::Render::EditorMaterialComponentUtil", false, "Failed to export: %s", path.c_str());
-                        result = false;
-                        return false;
-                    }
+                        AZ::RPI::MaterialPropertyValue propertyValueDefault = propertyDefinition->m_value;
+                        if (editData.m_materialParentAsset.IsReady())
+                        {
+                            propertyValueDefault = editData.m_materialParentAsset->GetPropertyValues()[propertyIndex.GetIndex()];
+                        }
 
-                    // Don't export values if they are the same as the material type or parent
-                    if (propertyValueDefault == propertyValue)
-                    {
+                        // 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);
             }