[ATOM-5441] Shader Builders May Fail When Multiple New Files Are Added (#3862)

* [ATOM-5441] Shader Builders May Fail When Multiple
New Files Are Added

ShaderAssetBuilder::CreateJobs now recursively parses *.azsl files
looking for #include lines and builds the list of source dependencies
using a depth-first algorithm. It was using MCPP before but not anymore
(during CreateJobs).

The new algorithm may over prescribe, but fixes the issues
when multiple new shader related files are added, at once or out of order, to a game project
or Gem.

Overall the new ShaderAssetBuilder::CreateJobs() is around 40% faster
and, of course, handles source dependencies in a robust way.

* Added new test suite to AutomatedTesting project:
Gem/PythonTests/atom_renderer/test_Atom_ShaderBuildPipelineSuite.py

Bug fix to Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder.cpp
discovered thanks to the automated test suite. The idea is that
CreateJobs doesn't fail if the AZSL file doesn't exist. The failure is
deferred during ProcessJob. This way if the AZSL file exists the .shader
file is rebuilt automatically.

* For testability purposes and avoid memory leakage errors
during Unit Tests created the class ShaderBuilderUtility::IncludedFilesParser

Now accepts "#  include <file>" with space between '#' and 'include'.
Also now accepts the '-' character inside the file path.

Added Unit Test to validate all cases of "#include <file>" parsing.

* Fixed linux runtime issues for Unit Tests in Atom_Asset_Shader.Tests

Signed-off-by: garrieta <garrieta@amazon.com>

AutomatedTesting/Gem/PythonTests/atom_renderer/CMakeLists.txt

@@ -46,4 +46,15 @@ if(PAL_TRAIT_BUILD_HOST_TOOLS AND PAL_TRAIT_BUILD_TESTS_SUPPORTED AND AutomatedT
             AutomatedTesting.Assets
             Editor
     )
+    ly_add_pytest(
+        NAME AutomatedTesting::AtomRenderer_HydraTests_ShaderBuildPipeline
+        TEST_SUITE main
+        PATH ${CMAKE_CURRENT_LIST_DIR}/test_Atom_ShaderBuildPipelineSuite.py
+        TEST_SERIAL
+        TIMEOUT 600
+        RUNTIME_DEPENDENCIES
+            AssetProcessor
+            AutomatedTesting.Assets
+            Editor
+    )
 endif()

AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/TestAssets/ShaderAssetBuilder/DependencyValidation.azsl.txt

@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) Contributors to the Open 3D Engine Project.
+ * For complete copyright and license terms please see the LICENSE at the root of this distribution.
+ *
+ * SPDX-License-Identifier: Apache-2.0 OR MIT
+ *
+ */
+
+ /*
+    This is a dummy shader used to validate detection of "#included files"
+ */
+
+#include <Atom/Features/SrgSemantics.azsli>
+
+#include "Test1Color.azsli"
+#include <Test3Color.azsli>
+
+ShaderResourceGroup DummySrg : SRG_PerDraw
+{
+    float4 m_color;
+}
+
+struct VSInput
+{
+    float3 m_position : POSITION;
+    float4 m_color : COLOR0;
+};
+
+struct VSOutput
+{
+    float4 m_position : SV_Position;
+    float4 m_color : COLOR0;
+};
+
+VSOutput MainVS(VSInput vsInput)
+{
+    VSOutput OUT;
+    OUT.m_position = float4(vsInput.m_position, 1.0);
+    OUT.m_color = vsInput.m_color;
+    return OUT;
+}
+
+struct PSOutput
+{
+    float4 m_color : SV_Target0;
+};
+
+PSOutput MainPS(VSOutput vsOutput)
+{
+    PSOutput OUT;
+
+    OUT.m_color = GetTest1Color(DummySrg::m_color) + GetTest3Color(DummySrg::m_color);
+
+    return OUT;
+}

AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/TestAssets/ShaderAssetBuilder/DependencyValidation.shader.txt

@@ -0,0 +1,26 @@
+// This is a dummy shader used to validate detection of "#included files"
+{
+    "Source" : "DependencyValidation.azsl",
+
+    "DepthStencilState" : { 
+        "Depth" : { "Enable" : false, "CompareFunc" : "GreaterEqual" }
+    },
+
+    "DrawList" : "forward",
+
+    "ProgramSettings":
+    {
+      "EntryPoints":
+      [
+        {
+          "name": "MainVS",
+          "type": "Vertex"
+        },
+        {
+          "name": "MainPS",
+          "type": "Fragment"
+        }
+      ]
+    }
+    
+}

AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/TestAssets/ShaderAssetBuilder/Test1Color.azsli.txt

@@ -0,0 +1,18 @@
+/*
+ * Copyright (c) Contributors to the Open 3D Engine Project.
+ * For complete copyright and license terms please see the LICENSE at the root of this distribution.
+ *
+ * SPDX-License-Identifier: Apache-2.0 OR MIT
+ *
+ */
+
+ /*
+    This is a dummy shader used to validate detection of "#included files"
+ */
+
+#include "Test2Color.azsli"
+
+float4 GetTest1Color(float4 color)
+{
+    return color + GetTest2Color(color);
+}

AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/TestAssets/ShaderAssetBuilder/Test2Color.azsli.txt

@@ -0,0 +1,16 @@
+/*
+ * Copyright (c) Contributors to the Open 3D Engine Project.
+ * For complete copyright and license terms please see the LICENSE at the root of this distribution.
+ *
+ * SPDX-License-Identifier: Apache-2.0 OR MIT
+ *
+ */
+
+ /*
+    This is a dummy shader used to validate detection of "#included files"
+ */
+
+float4 GetTest2Color(float4 color)
+{
+    return color * 0.5; 
+}

AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/TestAssets/ShaderAssetBuilder/Test3Color.azsli.txt

@@ -0,0 +1,16 @@
+/*
+ * Copyright (c) Contributors to the Open 3D Engine Project.
+ * For complete copyright and license terms please see the LICENSE at the root of this distribution.
+ *
+ * SPDX-License-Identifier: Apache-2.0 OR MIT
+ *
+ */
+
+ /*
+    This is a dummy shader used to validate detection of "#included files"
+ */
+
+float4 GetTest3Color(float4 color)
+{
+    return color * 0.13; 
+}

AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/hydra_ShaderAssetBuilder_RecompilesShaderAsChainOfDependenciesChanges.py

@@ -0,0 +1,188 @@
+"""
+Copyright (c) Contributors to the Open 3D Engine Project.
+For complete copyright and license terms please see the LICENSE at the root of this distribution.
+
+SPDX-License-Identifier: Apache-2.0 OR MIT
+
+"""
+
+import os
+import shutil
+
+def _copy_file(src_file, src_path, target_file, target_path):
+    # type: (str, str, str, str) -> None
+    """
+    Copies the [src_file] located in [src_path] to the [target_file] located at [target_path].
+    Leaves the [target_file] unlocked for reading and writing privileges
+    :param src_file: The source file to copy (file name)
+    :param src_path: The source file's path
+    :param target_file: The target file to copy into (file name)
+    :param target_path: The target file's path
+    :return: None
+    """
+    target_file_path = os.path.join(target_path, target_file)
+    src_file_path = os.path.join(src_path, src_file)
+    if os.path.exists(target_file_path):
+        fs.unlock_file(target_file_path)
+    shutil.copyfile(src_file_path, target_file_path)
+
+def _copy_tmp_files_in_order(src_directory, file_list, dst_directory, wait_time_in_between = 0.0):
+    # type: (str, list, str, float) -> None
+    """
+    This function assumes that for each file name listed in @file_list
+    there's file named "@filename.txt" which the original source file
+    but they will be copied with just the @filename (.txt removed).
+    """
+    for filename in file_list:
+        src_name = f"{filename}.txt"
+        _copy_file(src_name, src_directory, filename, dst_directory)
+        if wait_time_in_between > 0.0: 
+            print(f"Created {filename} in {dst_directory}")
+            general.idle_wait(wait_time_in_between)
+
+
+def _remove_file(src_file, src_path):
+    # type: (str, str) -> None
+    """
+    Removes the [src_file] located in [src_path].
+    :param src_file: The source file to copy (file name)
+    :param src_path: The source file's path
+    :return: None
+    """
+    src_file_path = os.path.join(src_path, src_file)
+    if os.path.exists(src_file_path):
+        fs.unlock_file(src_file_path)
+        os.remove(src_file_path)
+
+
+def _remove_files(directory, file_list):
+    for filename in file_list:
+        _remove_file(filename, directory)
+
+
+def _asset_exists(cache_relative_path):
+    asset_id = azasset.AssetCatalogRequestBus(azbus.Broadcast, "GetAssetIdByPath", cache_relative_path, azmath.Uuid(), False)
+    return asset_id.is_valid()
+
+# List of results that we want to check, this is not 100% necessary but it's a good
+# practice to make it easier to debug tests.
+# Here we define a tuple of tests
+class Results():
+    azshader_was_removed          = ("azshader was removed",        "Failed to remove azshader")
+    azshader_was_compiled         = ("azshader was compiled",       "Failed to compile azshader")
+
+
+def ShaderAssetBuilder_RecompilesShaderAsChainOfDependenciesChanges():
+    """
+    This test validates [ATOM-5441] Shader Builders May Fail When Multiple New Files Are Added
+    It creates source assets to compile a particular shader.
+    1- The first phase generates the source assets out of order and slowly. The AP should
+       wakeup each time one of the source dependencies appears but will fail each time. Only when the
+       last dependency appears then the shader should build successfully. 
+    2- The second phase is similar as above, except that all source assets will be created
+       at once and We also expect that in the end the shader is built successfully.
+    """
+    # Required for automated tests
+    helper.init_idle()
+
+    game_root_path = os.path.normpath(general.get_game_folder())
+    game_asset_path = os.path.join(game_root_path, "Assets")
+
+    base_dir = os.path.dirname(__file__)
+    src_assets_subdir = os.path.join(base_dir, "TestAssets", "ShaderAssetBuilder") 
+
+    with Tracer() as error_tracer:
+        # The script drives the execution of the test, to return the flow back to the editor,
+        # we will tick it one time
+        general.idle_wait_frames(1)
+ 
+        # This is the order in which the source assets should be deployed
+        # to avoid source dependency issues with the old MCPP-based CreateJobs. 
+        file_list = [
+            "Test2Color.azsli",
+            "Test3Color.azsli",
+            "Test1Color.azsli",
+            "DependencyValidation.azsl",
+            "DependencyValidation.shader"
+        ]
+
+        reverse_file_list = file_list[::-1]
+
+        # Remove files in reverse order
+        _remove_files(game_asset_path, reverse_file_list)
+
+        # Wait here until the azshader doesn't exist anymore.
+        azshader_name = "assets/dependencyvalidation.azshader"
+        helper.wait_for_condition(lambda: not _asset_exists(azshader_name), 5.0)
+
+        Report.critical_result(Results.azshader_was_removed, not _asset_exists(azshader_name))
+
+        _copy_tmp_files_in_order(src_assets_subdir, file_list, game_asset_path, 1.0)
+
+        # Give enough time to AP to compile the shader
+        helper.wait_for_condition(lambda: _asset_exists(azshader_name), 60.0)
+
+        Report.critical_result(Results.azshader_was_compiled, _asset_exists(azshader_name))
+
+        # The first part was about compiling the shader under normal conditions.
+        # Let's remove the files from the previous phase and will proceed
+        # to make the source files visible to the AP in reverse order. The
+        # ShaderAssetBuilder will only succeed when the last file becomes visible.
+        _remove_files(game_asset_path, reverse_file_list)
+        helper.wait_for_condition(lambda: not _asset_exists(azshader_name), 5.0)
+        Report.critical_result(Results.azshader_was_removed, not _asset_exists(azshader_name))
+
+        # Remark, if you are running this test manually from the Editor with "pyRunFile",
+        # You'll notice how the AP issues notifications that it fails to compile the shader
+        # as the source files are being copied to the "Assets" subfolder.
+        # Those errors are OK and also expected because We need the AP to wake up as each
+        # reported source dependency exists. Once the last file is copied then all source
+        # dependencies are fully satisfied and the shader should compile successfully.
+        # And this summarizes the importance of this Test: The previous version
+        # of ShaderAssetBuilder::CreateJobs was incapable of compiling the shader under the conditions
+        # presented in this test, but with the new version of ShaderAssetBuilder::CreateJobs, which
+        # doesn't use MCPP for #include files discovery, it should eventually compile the shader
+        # once all the source files are in place.
+        _copy_tmp_files_in_order(src_assets_subdir, reverse_file_list, game_asset_path, 3.0)
+
+        # Give enough time to AP to compile the shader
+        helper.wait_for_condition(lambda: _asset_exists(azshader_name), 60.0)
+
+        Report.critical_result(Results.azshader_was_compiled, _asset_exists(azshader_name))
+
+        # The last phase of the test puts stress on potential race conditions
+        # when all required files appear as soon as possible.
+
+        # First Clean up.
+        # Remove left over files.
+        _remove_files(game_asset_path, reverse_file_list)
+        helper.wait_for_condition(lambda: not _asset_exists(azshader_name), 5.0)
+        Report.critical_result(Results.azshader_was_removed, not _asset_exists(azshader_name))
+
+        # Now let's copy all the source files to the "Assets" folder as fast as possible.
+        _copy_tmp_files_in_order(src_assets_subdir, reverse_file_list, game_asset_path)
+
+        # Give enough time to AP to compile the shader
+        helper.wait_for_condition(lambda: _asset_exists(azshader_name), 60.0)
+
+        Report.critical_result(Results.azshader_was_compiled, _asset_exists(azshader_name))
+
+        # All good, let's cleanup leftover files before closing the test.
+        _remove_files(game_asset_path, reverse_file_list)
+        helper.wait_for_condition(lambda: not _asset_exists(azshader_name), 5.0)
+
+
+if __name__ == "__main__":
+    # All exposed python bindings are in azlmbr
+    import azlmbr.legacy.general as general
+    import azlmbr.bus as azbus
+    import azlmbr.asset as azasset
+    import azlmbr.math as azmath
+
+    # Import report and test helper utilities
+    from editor_python_test_tools.utils import Report
+    from editor_python_test_tools.utils import TestHelper as helper
+    from editor_python_test_tools.utils import Tracer
+    import ly_test_tools.environment.file_system as fs
+
+    Report.start_test(ShaderAssetBuilder_RecompilesShaderAsChainOfDependenciesChanges)

AutomatedTesting/Gem/PythonTests/atom_renderer/test_Atom_ShaderBuildPipelineSuite.py

@@ -0,0 +1,19 @@
+"""
+Copyright (c) Contributors to the Open 3D Engine Project.
+For complete copyright and license terms please see the LICENSE at the root of this distribution.
+
+SPDX-License-Identifier: Apache-2.0 OR MIT
+
+Main suite tests for the Shader Build Pipeline.
+"""
+import pytest
+from ly_test_tools import LAUNCHERS
+from ly_test_tools.o3de.editor_test import EditorTestSuite, EditorSingleTest
+
+@pytest.mark.parametrize("project", ["AutomatedTesting"])
+@pytest.mark.parametrize("launcher_platform", ['windows_editor'])
+class TestShaderBuildPipelineMain(EditorTestSuite):
+    """Holds tests for Shader Build Pipeline validation"""
+
+    class ShaderAssetBuilder_RecompilesShaderAsChainOfDependenciesChanges(EditorSingleTest):
+        from .atom_hydra_scripts import hydra_ShaderAssetBuilder_RecompilesShaderAsChainOfDependenciesChanges as test_module

Gems/Atom/Asset/Shader/Code/CMakeLists.txt

@@ -65,6 +65,7 @@ ly_add_target(
             AZ::AzFramework
             AZ::AzToolsFramework
             Gem::Atom_RHI.Edit
+            Gem::Atom_RPI.Edit
             Gem::Atom_RPI.Public
 )
 

Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder.cpp

@@ -61,10 +61,99 @@ namespace AZ
         static constexpr char ShaderAssetBuilderName[] = "ShaderAssetBuilder";
         static constexpr uint32_t ShaderAssetBuildTimestampParam = 0;
 
+        //! The search will start in @currentFolderPath.
+        //! if the file is not found then it searches in order of appearence in @includeDirectories.
+        //! If the search yields no existing file it returns an empty string.
+        static AZStd::string DiscoverFullPath(AZStd::string_view normalizedRelativePath, AZStd::string_view currentFolderPath, const AZStd::vector<AZStd::string>& includeDirectories)
+        {
+            AZStd::string fullPath;
+            AzFramework::StringFunc::Path::Join(currentFolderPath.data(), normalizedRelativePath.data(), fullPath);
+            if (AZ::IO::SystemFile::Exists(fullPath.c_str()))
+            {
+                return fullPath;
+            }
+
+            for (const auto &includeDir : includeDirectories)
+            {
+                AzFramework::StringFunc::Path::Join(includeDir.c_str(), normalizedRelativePath.data(), fullPath);
+                if (AZ::IO::SystemFile::Exists(fullPath.c_str()))
+                {
+                    return fullPath;
+                }
+            }
+
+            return "";
+        }
+
+        // Appends to @includedFiles normalized paths of possible future locations of the file @normalizedRelativePath.
+        // The future locations are each directory listed in @includeDirectories joined with @normalizedRelativePath.
+        // This function is called when an included file doesn't exist but We need to declare source dependency so a .shader
+        // asset is rebuilt when the missing file appears in the future.
+        static void AppendListOfPossibleFutureLocations(AZStd::unordered_set<AZStd::string>& includedFiles, AZStd::string_view normalizedRelativePath, AZStd::string_view currentFolderPath, const AZStd::vector<AZStd::string>& includeDirectories)
+        {
+            AZStd::string fullPath;
+            AzFramework::StringFunc::Path::Join(currentFolderPath.data(), normalizedRelativePath.data(), fullPath);
+            includedFiles.insert(fullPath);
+            for (const auto &includeDir : includeDirectories)
+            {
+                AzFramework::StringFunc::Path::Join(includeDir.c_str(), normalizedRelativePath.data(), fullPath);
+                includedFiles.insert(fullPath);
+            }
+        }
+
+        //! Parses, using depth-first recursive approach, azsl files. Looks for '#include <foo/bar/blah.h>' or '#include "foo/bar/blah.h"' lines
+        //! and in turn parses the included files.
+        //! The included files are searched in the directories listed in @includeDirectories. Basically it's a similar approach
+        //! as how most C-preprocessors would find included files.
+        static void GetListOfIncludedFiles(AZStd::string_view sourceFilePath, const AZStd::vector<AZStd::string>& includeDirectories,
+            const ShaderBuilderUtility::IncludedFilesParser& includedFilesParser, AZStd::unordered_set<AZStd::string>& includedFiles)
+        {
+            auto outcome = includedFilesParser.ParseFileAndGetIncludedFiles(sourceFilePath);
+            if (!outcome.IsSuccess())
+            {
+                AZ_Warning(ShaderAssetBuilderName, false, outcome.GetError().c_str());
+                return;
+            }
+
+            // Cache the path of the folder where @sourceFilePath is located.
+            AZStd::string sourceFileFolderPath;
+            {
+                AZStd::string drive;
+                AzFramework::StringFunc::Path::Split(sourceFilePath.data(), &drive, &sourceFileFolderPath);
+                if (!drive.empty())
+                {
+                    AzFramework::StringFunc::Path::Join(drive.c_str(), sourceFileFolderPath.c_str(), sourceFileFolderPath);
+                }
+            }
+
+            auto listOfRelativePaths = outcome.TakeValue();
+            for (auto relativePath : listOfRelativePaths)
+            {
+                auto fullPath = DiscoverFullPath(relativePath, sourceFileFolderPath, includeDirectories);
+                if (fullPath.empty())
+                {
+                    // The file doesn't exist in any of the includeDirectories. It doesn't exist in @sourceFileFolderPath either.
+                    // The file may appear in the future in one of those directories, We must build an exhaustive list
+                    // of full file paths where the file may appear in the future.
+                    AppendListOfPossibleFutureLocations(includedFiles, relativePath, sourceFileFolderPath, includeDirectories);
+                    continue;
+                }
+
+                // Add the file to the list and keep parsing recursively.
+                if (includedFiles.count(fullPath))
+                {
+                    continue;
+                }
+                includedFiles.insert(fullPath);
+                GetListOfIncludedFiles(fullPath, includeDirectories, includedFilesParser, includedFiles);
+            }
+        }
+
         void ShaderAssetBuilder::CreateJobs(const AssetBuilderSDK::CreateJobsRequest& request, AssetBuilderSDK::CreateJobsResponse& response) const
         {
             AZStd::string fullPath;
             AzFramework::StringFunc::Path::ConstructFull(request.m_watchFolder.data(), request.m_sourceFile.data(), fullPath, true);
+            ShaderBuilderUtility::IncludedFilesParser includedFilesParser;
 
             AZ_TracePrintf(ShaderAssetBuilderName, "CreateJobs for Shader \"%s\"\n", fullPath.data());
 
@@ -90,36 +179,6 @@ namespace AZ
 
             AZStd::string azslFullPath;
             ShaderBuilderUtility::GetAbsolutePathToAzslFile(fullPath, shaderSourceData.m_source, azslFullPath);
-            if (!IO::FileIOBase::GetInstance()->Exists(azslFullPath.c_str()))
-            {
-                AZ_Error(
-                    ShaderAssetBuilderName, false, "Shader program listed as the source entry does not exist: %s.", azslFullPath.c_str());
-                response.m_result = AssetBuilderSDK::CreateJobsResultCode::Failed;
-                return;
-            }
-
-
-            GlobalBuildOptions buildOptions = ReadBuildOptions(ShaderAssetBuilderName);
-
-            // [GFX TODO] [ATOM-14966] In principle, based on macro definitions, included files can change per supervariant.
-            //                         So, the list of source asset dependencies must be collected by running MCPP on each supervariant.
-            //                         For now, we will run MCPP only once because CreateJobs() should be as light as possible.
-            // 
-            // Regardless of the PlatformInfo and enabled ShaderPlatformInterfaces, the azsl file will be preprocessed
-            // with the sole purpose of extracting all included files. For each included file a SourceDependency will be declared.
-            PreprocessorData output;
-            buildOptions.m_compilerArguments.Merge(shaderSourceData.m_compiler);
-            PreprocessFile(azslFullPath, output, buildOptions.m_preprocessorSettings, true, true);
-            for (auto includePath : output.includedPaths)
-            {
-                // m_sourceFileDependencyList does not support paths with "." or ".." for relative lookup, but the preprocessor
-                // may produce path strings like "C:/a/b/c/../../d/file.azsli" so we have to normalize
-                AzFramework::StringFunc::Path::Normalize(includePath);
-
-                AssetBuilderSDK::SourceFileDependency includeFileDependency;
-                includeFileDependency.m_sourceFileDependencyPath = includePath;
-                response.m_sourceFileDependencyList.emplace_back(includeFileDependency);
-            }
 
             {
                 // Add the AZSL as source dependency
@@ -128,6 +187,26 @@ namespace AZ
                 response.m_sourceFileDependencyList.emplace_back(azslFileDependency);
             }
 
+            if (!IO::FileIOBase::GetInstance()->Exists(azslFullPath.c_str()))
+            {
+                AZ_Error(
+                    ShaderAssetBuilderName, false, "Shader program listed as the source entry does not exist: %s.", azslFullPath.c_str());
+                // Treat as success, so when the azsl file shows up the AP will try to recompile.
+                response.m_result = AssetBuilderSDK::CreateJobsResultCode::Success;
+                return;
+            }
+
+            GlobalBuildOptions buildOptions = ReadBuildOptions(ShaderAssetBuilderName);
+
+            AZStd::unordered_set<AZStd::string> includedFiles;
+            GetListOfIncludedFiles(azslFullPath, buildOptions.m_preprocessorSettings.m_projectIncludePaths, includedFilesParser, includedFiles);
+            for (auto includePath : includedFiles)
+            {
+                AssetBuilderSDK::SourceFileDependency includeFileDependency;
+                includeFileDependency.m_sourceFileDependencyPath = includePath;
+                response.m_sourceFileDependencyList.emplace_back(includeFileDependency);
+            }
+
             for (const AssetBuilderSDK::PlatformInfo& platformInfo : request.m_enabledPlatforms)
             {
                 AZ_TraceContext("For platform", platformInfo.m_identifier.data());
@@ -149,6 +228,10 @@ namespace AZ
                 response.m_createJobOutputs.push_back(jobDescriptor);
             }  // for all request.m_enabledPlatforms
 
+            const AZStd::sys_time_t createJobsEndStamp = AZStd::GetTimeNowMicroSecond();
+            const u64 createJobDurationMicros = createJobsEndStamp - shaderAssetBuildTimestamp;
+            AZ_TracePrintf(ShaderAssetBuilderName, "CreateJobs for %s took %llu microseconds", fullPath.c_str(), createJobDurationMicros );
+
             response.m_result = AssetBuilderSDK::CreateJobsResultCode::Success;
         }
 
@@ -286,6 +369,13 @@ namespace AZ
                     return;
                 }
             }
+            else
+            {
+                // CreateJobs was not successful if there's no timestamp property in m_jobParameters.
+                response.m_resultCode = AssetBuilderSDK::ProcessJobResult_Failed;
+                AZ_Assert(false, "Missing ShaderAssetBuildTimestampParam");
+                return;
+            }
 
             auto supervariantList = ShaderBuilderUtility::GetSupervariantListFromShaderSourceData(shaderSourceData);
 

Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderBuilderUtility.cpp

@@ -20,6 +20,7 @@
 #include <AzCore/IO/IOUtils.h>
 #include <AzCore/IO/FileIO.h>
 #include <AzCore/Settings/SettingsRegistryMergeUtils.h>
+#include <AzCore/std/string/regex.h>
 
 #include <AzCore/Serialization/Json/JsonUtils.h>
 
@@ -814,6 +815,51 @@ namespace AZ
                 return success;
             }
 
+            IncludedFilesParser::IncludedFilesParser()
+            {
+                AZStd::regex regex(R"(#\s*include\s+[<|"]([\w|/|\\|\.|-]+)[>|"])", AZStd::regex::ECMAScript);
+                m_includeRegex.swap(regex);
+            }
+
+            AZStd::vector<AZStd::string> IncludedFilesParser::ParseStringAndGetIncludedFiles(AZStd::string_view haystack) const
+            {
+                AZStd::vector<AZStd::string> listOfFilePaths;
+                AZStd::smatch match;
+                AZStd::string::const_iterator searchStart(haystack.cbegin());
+                while (AZStd::regex_search(searchStart, haystack.cend(), match, m_includeRegex))
+                {
+                    if (match.size() > 1)
+                    {
+                        AZStd::string relativeFilePath(match[1].str().c_str());
+                        AzFramework::StringFunc::Path::Normalize(relativeFilePath);
+                        listOfFilePaths.push_back(relativeFilePath);
+                    }
+                    searchStart = match.suffix().first;
+                }
+                return listOfFilePaths;
+            }
+
+            AZ::Outcome<AZStd::vector<AZStd::string>, AZStd::string> IncludedFilesParser::ParseFileAndGetIncludedFiles(AZStd::string_view sourceFilePath) const
+            {
+                AZ::IO::FileIOStream stream(sourceFilePath.data(), AZ::IO::OpenMode::ModeRead);
+                if (!stream.IsOpen())
+                {
+                    return AZ::Failure(AZStd::string::format("\"%s\" source file could not be opened.", sourceFilePath.data()));
+                }
+
+                if (!stream.CanRead())
+                {
+                    return AZ::Failure(AZStd::string::format("\"%s\" source file could not be read.", sourceFilePath.data()));
+                }
+
+                AZStd::string hayStack;
+                hayStack.resize_no_construct(stream.GetLength());
+                stream.Read(stream.GetLength(), hayStack.data());
+
+                auto listOfFilePaths = ParseStringAndGetIncludedFiles(hayStack);
+                return AZ::Success(AZStd::move(listOfFilePaths));
+            }
+
         }  // namespace ShaderBuilderUtility
     } // namespace ShaderBuilder
 } // AZ

Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderBuilderUtility.h

@@ -141,6 +141,29 @@ namespace AZ
                 const uint32_t rhiUniqueIndex, const AZStd::string& platformIdentifier, const AZStd::string& shaderJsonPath,
                 const uint32_t supervariantIndex, RPI::ShaderAssetSubId shaderAssetSubId);
 
+
+            class IncludedFilesParser
+            {
+            public:
+                IncludedFilesParser();
+                ~IncludedFilesParser() = default;
+
+                //! This static function was made public for testability purposes only.
+                //! Parses the string @haystack, looking for "#include file" lines with a regular expression.
+                //! Returns the list of relative paths as included by the file.
+                //! REMARK: The algorithm may over prescribe what files to include because it doesn't discern between comments, etc.
+                //!         Also, a #include line may be protected by #ifdef macros but this algorithm doesn't care.
+                //! Over prescribing is not a real problem, albeit potential waste in processing. Under prescribing would be a real problem.
+                AZStd::vector<AZStd::string> ParseStringAndGetIncludedFiles(AZStd::string_view haystack) const;
+
+                //! This static function was made public for testability purposes only.
+                //! Opens the file @sourceFilePath, loads the content into a string and returns ParseStringAndGetIncludedFiles(content)
+                AZ::Outcome<AZStd::vector<AZStd::string>, AZStd::string> ParseFileAndGetIncludedFiles(AZStd::string_view sourceFilePath) const;
+
+            private:
+                AZStd::regex m_includeRegex;
+            };
+
         }  // ShaderBuilderUtility namespace
     } // ShaderBuilder namespace
 } // AZ

Gems/Atom/Asset/Shader/Code/Tests/ShaderBuilderUtilityTests.cpp

@@ -0,0 +1,86 @@
+/*
+ * Copyright (c) Contributors to the Open 3D Engine Project.
+ * For complete copyright and license terms please see the LICENSE at the root of this distribution.
+ *
+ * SPDX-License-Identifier: Apache-2.0 OR MIT
+ *
+ */
+
+#include <AzTest/AzTest.h>
+#include <AzCore/UnitTest/TestTypes.h>
+
+#include "Common/ShaderBuilderTestFixture.h"
+
+#include <ShaderBuilderUtility.h>
+
+namespace UnitTest
+{
+    using namespace AZ;
+
+    // The main purpose of this class is to test ShaderBuilderUtility functions
+    class ShaderBuilderUtilityTests : public ShaderBuilderTestFixture
+    {
+    }; // class ShaderBuilderUtilityTests
+
+
+    TEST_F(ShaderBuilderUtilityTests, IncludedFilesParser_ParseStringAndGetIncludedFiles)
+    {
+        AZStd::string haystack(
+            "Some content to parse\n"
+            "#include <valid_file1.azsli>\n"
+            "// #include <valid_file2.azsli>\n"
+            "blah # include \"valid_file3.azsli\"\n"
+            "bar include <a\\dire-ctory\\invalid-file4.azsli>\n"
+            "foo #   include \"a/directory/valid-file5.azsli\"\n"
+            "# include <a\\dire-ctory\\valid-file6.azsli>\n"
+            "#includ \"a\\dire-ctory\\invalid-file7.azsli\"\n"
+        );
+
+        AZ::ShaderBuilder::ShaderBuilderUtility::IncludedFilesParser includedFilesParser;
+        auto fileList = includedFilesParser.ParseStringAndGetIncludedFiles(haystack);
+        EXPECT_EQ(fileList.size(), 5);
+
+        auto it = AZStd::find(fileList.begin(), fileList.end(), "valid_file1.azsli");
+        EXPECT_TRUE(it != fileList.end());
+
+        it = AZStd::find(fileList.begin(), fileList.end(), "valid_file2.azsli");
+        EXPECT_TRUE(it != fileList.end());
+
+        it = AZStd::find(fileList.begin(), fileList.end(), "valid_file3.azsli");
+        EXPECT_TRUE(it != fileList.end());
+
+        // Remark: From now on We must normalize because internally AZ::ShaderBuilder::ShaderBuilderUtility::IncludedFilesParser
+        // always returns normalized paths.
+        {
+            AZStd::string fileName("a\\dire-ctory\\invalid-file4.azsli");
+            AzFramework::StringFunc::Path::Normalize(fileName);
+            it = AZStd::find(fileList.begin(), fileList.end(), fileName);
+            EXPECT_TRUE(it == fileList.end());
+        }
+
+        {
+            AZStd::string fileName("a\\directory\\valid-file5.azsli");
+            AzFramework::StringFunc::Path::Normalize(fileName);
+            it = AZStd::find(fileList.begin(), fileList.end(), fileName);
+            EXPECT_TRUE(it != fileList.end());
+        }
+
+        {
+            AZStd::string fileName("a\\dire-ctory\\valid-file6.azsli");
+            AzFramework::StringFunc::Path::Normalize(fileName);
+            it = AZStd::find(fileList.begin(), fileList.end(), fileName);
+            EXPECT_TRUE(it != fileList.end());
+        }
+
+        {
+            AZStd::string fileName("a\\dire-ctory\\invalid-file7.azsli");
+            AzFramework::StringFunc::Path::Normalize(fileName);
+            it = AZStd::find(fileList.begin(), fileList.end(), fileName);
+            EXPECT_TRUE(it == fileList.end());
+        }
+    }
+
+} //namespace UnitTest
+
+//AZ_UNIT_TEST_HOOK(DEFAULT_UNIT_TEST_ENV);
+

Gems/Atom/Asset/Shader/Code/atom_asset_shader_builders_tests_files.cmake

@@ -11,4 +11,5 @@ set(FILES
     Tests/Common/ShaderBuilderTestFixture.cpp
     Tests/SupervariantCmdArgumentTests.cpp
     Tests/McppBinderTests.cpp
+    Tests/ShaderBuilderUtilityTests.cpp
 )

Gems/Atom/Feature/Common/Assets/Shaders/PostProcessing/FastDepthAwareBlurHor.azsl

@@ -55,7 +55,7 @@ int GetLdsIndex(int2 ldsPosition)
 
 // --- Common file start ---
 
-// #include <Atom/Features/PostProcessing/FastDepthAwareBlurCommon.azsli>
+// include <Atom/Features/PostProcessing/FastDepthAwareBlurCommon.azsli> ('#' symbol before 'include' was removed on purpose to avoid parsing this azsli file during ShaderAssetBuilder::CreateJobs)
 // This include fails with the asset processor when generating the .shader for this file
 // Everything below this is copy pasted from FastDepthAwareBlurCommon.azsli up until the
 // "Common file end" marker below

Gems/Atom/Feature/Common/Assets/Shaders/PostProcessing/FastDepthAwareBlurVer.azsl

@@ -55,7 +55,7 @@ int GetLdsIndex(int2 ldsPosition)
 
 // --- Common file start ---
 
-// #include <Atom/Features/PostProcessing/FastDepthAwareBlurCommon.azsli>
+// include <Atom/Features/PostProcessing/FastDepthAwareBlurCommon.azsli> ('#' symbol before 'include' was removed on purpose to avoid parsing this azsli file during ShaderAssetBuilder::CreateJobs)
 // This include fails with the asset processor when generating the .shader for this file
 // Everything below this is copy pasted from FastDepthAwareBlurCommon.azsli up until the
 // "Common file end" marker below

Gems/Atom/Feature/Common/Assets/Shaders/PostProcessing/SMAA.azsli

@@ -157,7 +157,7 @@
  *         #define SMAA_RT_METRICS float4(1.0 / 1280.0, 1.0 / 720.0, 1280.0, 720.0)
  *         #define SMAA_HLSL_4
  *         #define SMAA_PRESET_HIGH
- *         #include "SMAA.h"
+ *         include "SMAA.h" ('#' symbol before 'include' was removed on purpose to avoid parsing this azsli file during ShaderAssetBuilder::CreateJobs)
  *
  *     Note that SMAA_RT_METRICS doesn't need to be a macro, it can be a
  *     uniform variable. The code is designed to minimize the impact of not