Merge pull request #4181 from aws-lumberyard-dev/Atom/rbarrand/MaterialDependenciesFix

Switch job dependency type to OrderOnce for material assets when property names are present.

Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp

@@ -30,6 +30,7 @@
 #include <AzCore/IO/IOUtils.h>
 #include <AzCore/IO/Path/Path.h>
 #include <AzCore/Serialization/Json/JsonSerialization.h>
+#include <AzCore/Settings/SettingsRegistry.h>
 
 namespace AZ
 {
@@ -46,7 +47,7 @@ namespace AZ
         {
             AssetBuilderSDK::AssetBuilderDesc materialBuilderDescriptor;
             materialBuilderDescriptor.m_name = JobKey;
-            materialBuilderDescriptor.m_version = 107; // ATOM-14918
+            materialBuilderDescriptor.m_version = 108; // Set materialtype dependency to OrderOnce
             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>();
@@ -66,21 +67,19 @@ namespace AZ
         //! Adds all relevant dependencies for a referenced source file, considering that the path might be relative to the original file location or a full asset path.
         //! This will usually include multiple source dependencies and a single job dependency, but will include only source dependencies if the file is not found.
         //! Note the AssetBuilderSDK::JobDependency::m_platformIdentifier will not be set by this function. The calling code must set this value before passing back
-        //! to the AssetBuilderSDK::CreateJobsResponse.
+        //! to the AssetBuilderSDK::CreateJobsResponse. If isOrderedOnceForMaterialTypes is true and the dependency is a materialtype file, the job dependency type
-        void AddPossibleDependencies(
+        //! will be set to JobDependencyType::OrderOnce.
-            AZStd::string_view currentFilePath, AZStd::string_view referencedParentPath,
+        void AddPossibleDependencies(AZStd::string_view currentFilePath,
-            AZStd::vector<AssetBuilderSDK::SourceFileDependency>& sourceFileDependencies,
+            AZStd::string_view referencedParentPath,
-            const char* jobKey, AZStd::vector<AssetBuilderSDK::JobDependency>& jobDependencies)
+            const char* jobKey,
+            AZStd::vector<AssetBuilderSDK::JobDependency>& jobDependencies,
+            bool isOrderedOnceForMaterialTypes = false)
         {
             bool dependencyFileFound = false;
 
             AZStd::vector<AZStd::string> possibleDependencies = RPI::AssetUtils::GetPossibleDepenencyPaths(currentFilePath, referencedParentPath);
             for (auto& file : possibleDependencies)
             {
-                AssetBuilderSDK::SourceFileDependency sourceFileDependency;
-                sourceFileDependency.m_sourceFileDependencyPath = file;
-                sourceFileDependencies.push_back(sourceFileDependency);
-
                 // The first path found is the highest priority, and will have a job dependency, as this is the one
                 // the builder will actually use
                 if (!dependencyFileFound)
@@ -93,8 +92,11 @@ namespace AZ
                     {
                         AssetBuilderSDK::JobDependency jobDependency;
                         jobDependency.m_jobKey = jobKey;
-                        jobDependency.m_type = AssetBuilderSDK::JobDependencyType::Order;
                         jobDependency.m_sourceFile.m_sourceFileDependencyPath = file;
+
+                        const bool isMaterialTypeFile = AzFramework::StringFunc::Path::IsExtension(file.c_str(), MaterialTypeSourceData::Extension);
+                        jobDependency.m_type = (isMaterialTypeFile && isOrderedOnceForMaterialTypes) ? AssetBuilderSDK::JobDependencyType::OrderOnce : AssetBuilderSDK::JobDependencyType::Order;
+
                         jobDependencies.push_back(jobDependency);
                     }
                 }
@@ -173,8 +175,9 @@ namespace AZ
 
                     for (auto& shader : materialTypeSourceData.GetValue().m_shaderCollection)
                     {
-                        AddPossibleDependencies(request.m_sourceFile, shader.m_shaderFilePath,
+                        AddPossibleDependencies(request.m_sourceFile,
-                            response.m_sourceFileDependencyList, "Shader Asset",
+                            shader.m_shaderFilePath,
+                            "Shader Asset",
                             outputJobDescriptor.m_jobDependencyList);
                     }
 
@@ -184,9 +187,10 @@ namespace AZ
 
                         for (const MaterialFunctorSourceData::AssetDependency& dependency : dependencies)
                         {
-                            AddPossibleDependencies(request.m_sourceFile, dependency.m_sourceFilePath,
+                            AddPossibleDependencies(request.m_sourceFile,
-                                response.m_sourceFileDependencyList,
+                                dependency.m_sourceFilePath,
-                                dependency.m_jobKey.c_str(), outputJobDescriptor.m_jobDependencyList);
+                                dependency.m_jobKey.c_str(),
+                                outputJobDescriptor.m_jobDependencyList);
                         }
                     }
                 }
@@ -219,11 +223,24 @@ namespace AZ
                         parentMaterialPath = materialTypePath;
                     }
 
+                    // If includeMaterialPropertyNames is false, then a job dependency is needed so the material builder can validate MaterialAsset properties
+                    // against the MaterialTypeAsset at asset build time.
+                    // If includeMaterialPropertyNames is true, the material properties will be validated at runtime when the material is loaded, so the job dependency
+                    // is needed only for first-time processing to set up the initial MaterialAsset. This speeds up AP processing time when a materialtype file
+                    // is edited (e.g. 10s when editing StandardPBR.materialtype on AtomTest project from 45s).
+                    bool includeMaterialPropertyNames = true;
+                    if (auto settingsRegistry = AZ::SettingsRegistry::Get(); settingsRegistry != nullptr)
+                    {
+                        settingsRegistry->Get(includeMaterialPropertyNames, "/O3DE/Atom/RPI/MaterialBuilder/IncludeMaterialPropertyNames");
+                    }
+
                     // Register dependency on the parent material source file so we can load it and use it's data to build this variant material.
                     // Note, we don't need a direct dependency on the material type because the parent material will depend on it.
-                    AddPossibleDependencies(request.m_sourceFile, parentMaterialPath,
+                    AddPossibleDependencies(request.m_sourceFile,
-                        response.m_sourceFileDependencyList,
+                        parentMaterialPath,
-                        JobKey, outputJobDescriptor.m_jobDependencyList);
+                        JobKey,
+                        outputJobDescriptor.m_jobDependencyList,
+                        includeMaterialPropertyNames);
                 }
             }
             

Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp

@@ -44,7 +44,7 @@ namespace AZ
             if (auto* serialize = azrtti_cast<SerializeContext*>(context))
             {
                 serialize->Class<MaterialAssetDependenciesComponent, Component>()
-                    ->Version(4)
+                    ->Version(5) // Set materialtype dependency to OrderOnce
                     ->Attribute(Edit::Attributes::SystemComponentTags, AZStd::vector<Crc32>({ AssetBuilderSDK::ComponentTags::AssetBuilder }));
             }
         }
@@ -78,10 +78,7 @@ namespace AZ
             AZStd::string materialTypePath;
             RPI::MaterialConverterBus::BroadcastResult(materialTypePath, &RPI::MaterialConverterBus::Events::GetMaterialTypePath);
 
-            bool includeMaterialPropertyNames = true;
+            if (conversionEnabled && !materialTypePath.empty())
-            RPI::MaterialConverterBus::BroadcastResult(includeMaterialPropertyNames, &RPI::MaterialConverterBus::Events::ShouldIncludeMaterialPropertyNames);
-            // TODO: Use includeMaterialPropertyNames to break materialtype dependency on fbx files. Materialasset's dependency on materialtypeasset will need to be decoupled first
-            if (conversionEnabled && !materialTypePath.empty() /*&& !includeMaterialPropertyNames*/)
             {
                 AssetBuilderSDK::SourceFileDependency materialTypeSource;
                 materialTypeSource.m_sourceFileDependencyPath = materialTypePath;
@@ -90,7 +87,15 @@ namespace AZ
                 jobDependency.m_jobKey = "Atom Material Builder";
                 jobDependency.m_sourceFile = materialTypeSource;
                 jobDependency.m_platformIdentifier = platformIdentifier;
-                jobDependency.m_type = AssetBuilderSDK::JobDependencyType::Order;
+
+                // If includeMaterialPropertyNames is false, then a job dependency is needed so the material builder can validate
+                // MaterialAsset properties against the MaterialTypeAsset at asset build time. If includeMaterialPropertyNames is true, the
+                // material properties will be validated at runtime when the material is loaded, so the job dependency is needed only for
+                // first-time processing to set up the initial MaterialAsset. This speeds up AP processing time when a materialtype file is
+                // edited (e.g. 10s when editing StandardPBR.materialtype on AtomTest project from 45s).
+                bool includeMaterialPropertyNames = true;
+                RPI::MaterialConverterBus::BroadcastResult(includeMaterialPropertyNames, &RPI::MaterialConverterBus::Events::ShouldIncludeMaterialPropertyNames);
+                jobDependency.m_type = includeMaterialPropertyNames ? AssetBuilderSDK::JobDependencyType::OrderOnce : AssetBuilderSDK::JobDependencyType::Order;
 
                 jobDependencyList.push_back(jobDependency);
             }