From 0678dec64ef62f23cc49b3ce783ec55594684c5f Mon Sep 17 00:00:00 2001 From: galibzon <66021303+galibzon@users.noreply.github.com> Date: Wed, 26 May 2021 15:23:11 -0500 Subject: [PATCH] =?UTF-8?q?[ATOM-15618]=20Shader=20Build=20Pipeline:=20Add?= =?UTF-8?q?=20UnitTest=20To=20Validate=20Shader=20C=E2=80=A6=20(#918)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [ATOM-15618] Shader Build Pipeline: Add UnitTest To Validate Shader Compiler Argument Processing Introduced With The New Supervariant System - Added new test suite in Gems/Atom/Asset/Shader/Code/Tests/SupervariantCmdArgumentTests.cpp - Refactored and improved the previously existing classes: GlobalBuildOptions, PreprocessorOptions and ShaderCompilerArguments to work well with the new ShaderSourceData::SupervariantInfo. - Moved command line argument processing function out of ShaderCompilerArguments and into its own utility namespace in Atom/RHI.Edit/Utils.h Signed-off-by: garrieta --- Gems/Atom/Asset/Shader/Code/CMakeLists.txt | 33 ++ .../Editor/CommonFiles/Preprocessor.cpp | 34 +- .../Source/Editor/ShaderAssetBuilder2.cpp | 3 +- .../Tests/Common/ShaderBuilderTestFixture.cpp | 41 ++ .../Tests/Common/ShaderBuilderTestFixture.h | 34 ++ .../Tests/SupervariantCmdArgumentTests.cpp | 523 ++++++++++++++++++ ...om_asset_shader_builders_tests_files.cmake | 16 + .../Atom/RHI.Edit/ShaderCompilerArguments.h | 9 + .../RHI/Code/Include/Atom/RHI.Edit/Utils.h | 40 ++ .../RHI.Edit/ShaderCompilerArguments.cpp | 14 +- Gems/Atom/RHI/Code/Source/RHI.Edit/Utils.cpp | 59 ++ Gems/Atom/RPI/Code/CMakeLists.txt | 1 + .../RPI.Edit/Shader/ShaderSourceData.cpp | 87 ++- 13 files changed, 820 insertions(+), 74 deletions(-) create mode 100644 Gems/Atom/Asset/Shader/Code/Tests/Common/ShaderBuilderTestFixture.cpp create mode 100644 Gems/Atom/Asset/Shader/Code/Tests/Common/ShaderBuilderTestFixture.h create mode 100644 Gems/Atom/Asset/Shader/Code/Tests/SupervariantCmdArgumentTests.cpp create mode 100644 Gems/Atom/Asset/Shader/Code/atom_asset_shader_builders_tests_files.cmake diff --git a/Gems/Atom/Asset/Shader/Code/CMakeLists.txt b/Gems/Atom/Asset/Shader/Code/CMakeLists.txt index a06aa79d24..dd8afec534 100644 --- a/Gems/Atom/Asset/Shader/Code/CMakeLists.txt +++ b/Gems/Atom/Asset/Shader/Code/CMakeLists.txt @@ -101,3 +101,36 @@ ly_add_target( 3rdParty::SPIRVCross 3rdParty::azslc ) + +################################################################################ +# Tests +################################################################################ +if(PAL_TRAIT_BUILD_TESTS_SUPPORTED) + + ly_add_target( + NAME Atom_Asset_Shader.Tests ${PAL_TRAIT_TEST_TARGET_TYPE} + NAMESPACE Gem + FILES_CMAKE + atom_asset_shader_builders_tests_files.cmake + INCLUDE_DIRECTORIES + PRIVATE + . + Source/Editor + Tests + BUILD_DEPENDENCIES + PRIVATE + AZ::AtomCore + AZ::AzTest + AZ::AzFramework + AZ::AzToolsFramework + Legacy::CryCommon + Gem::Atom_RPI.Public + Gem::Atom_RHI.Public + Gem::Atom_RPI.Edit + Gem::Atom_Asset_Shader.Static + ) + ly_add_googletest( + NAME Gem::Atom_Asset_Shader.Tests + ) + +endif() diff --git a/Gems/Atom/Asset/Shader/Code/Source/Editor/CommonFiles/Preprocessor.cpp b/Gems/Atom/Asset/Shader/Code/Source/Editor/CommonFiles/Preprocessor.cpp index 1e471f8644..14193d774f 100644 --- a/Gems/Atom/Asset/Shader/Code/Source/Editor/CommonFiles/Preprocessor.cpp +++ b/Gems/Atom/Asset/Shader/Code/Source/Editor/CommonFiles/Preprocessor.cpp @@ -60,33 +60,31 @@ namespace AZ void PreprocessorOptions::RemovePredefinedMacros(const AZStd::vector& macroNames) { + for (const auto& macroName : macroNames) + { m_predefinedMacros.erase( AZStd::remove_if( m_predefinedMacros.begin(), m_predefinedMacros.end(), - [&](const AZStd::string& predefinedMacro) - { - for (const auto& macroName : macroNames) + [&](const AZStd::string& predefinedMacro) { + // Haystack, needle, bCaseSensitive + if (!AzFramework::StringFunc::StartsWith(predefinedMacro, macroName, true)) { - // Haystack, needle, bCaseSensitive - if (!AzFramework::StringFunc::StartsWith(predefinedMacro, macroName, true)) - { - return false; - } - // If found, let's make sure it is not just a substring. - if (predefinedMacro.size() == macroName.size()) - { - return true; - } - // The predefinedMacro can be a string like "macro=value". If we find '=' it is a match. - if (predefinedMacro.c_str()[macroName.size()] == '=') - { - return true; - } return false; } + // If found, let's make sure it is not just a substring. + if (predefinedMacro.size() == macroName.size()) + { + return true; + } + // The predefinedMacro can be a string like "macro=value". If we find '=' it is a match. + if (predefinedMacro.c_str()[macroName.size()] == '=') + { + return true; + } return false; }), m_predefinedMacros.end()); + } } //! Binder helper to Matsui C-Pre-Processor library diff --git a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder2.cpp b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder2.cpp index 1668b57866..5758db1da2 100644 --- a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder2.cpp +++ b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder2.cpp @@ -344,8 +344,7 @@ namespace AZ AZStd::string prependedAzslFilePath = RHI::PrependFile(args); if (prependedAzslFilePath == azslFullPath) { - // For some reason the combined azsl file was not created in the temporary - // directory assigned to this job. + // The specific error is already reported by RHI::PrependFile(). response.m_resultCode = AssetBuilderSDK::ProcessJobResult_Failed; return; } diff --git a/Gems/Atom/Asset/Shader/Code/Tests/Common/ShaderBuilderTestFixture.cpp b/Gems/Atom/Asset/Shader/Code/Tests/Common/ShaderBuilderTestFixture.cpp new file mode 100644 index 0000000000..276c225e05 --- /dev/null +++ b/Gems/Atom/Asset/Shader/Code/Tests/Common/ShaderBuilderTestFixture.cpp @@ -0,0 +1,41 @@ +/* +* All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or +* its licensors. +* +* For complete copyright and license terms please see the LICENSE at the root of this +* distribution (the "License"). All use of this software is governed by the License, +* or, if provided, by the license below or the license accompanying this file. Do not +* remove or modify any license notices. This file is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* +*/ + +#include "ShaderBuilderTestFixture.h" + +#include +#include + +namespace UnitTest +{ + void ShaderBuilderTestFixture::SetUp() + { + AllocatorsTestFixture::SetUp(); + + AZ::AllocatorInstance::Create(); + AZ::AllocatorInstance::Create(); + + AZ::NameDictionary::Create(); + } + + void ShaderBuilderTestFixture::TearDown() + { + AZ::NameDictionary::Destroy(); + + AZ::AllocatorInstance::Destroy(); + AZ::AllocatorInstance::Destroy(); + + AllocatorsTestFixture::TearDown(); + } + +} + diff --git a/Gems/Atom/Asset/Shader/Code/Tests/Common/ShaderBuilderTestFixture.h b/Gems/Atom/Asset/Shader/Code/Tests/Common/ShaderBuilderTestFixture.h new file mode 100644 index 0000000000..450cc6bde5 --- /dev/null +++ b/Gems/Atom/Asset/Shader/Code/Tests/Common/ShaderBuilderTestFixture.h @@ -0,0 +1,34 @@ +/* +* All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or +* its licensors. +* +* For complete copyright and license terms please see the LICENSE at the root of this +* distribution (the "License"). All use of this software is governed by the License, +* or, if provided, by the license below or the license accompanying this file. Do not +* remove or modify any license notices. This file is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* +*/ + +#pragma once + +#include + +namespace UnitTest +{ + /** + * Unit test fixture for setting up memory allocation pools and the AZ::Name dictionary. + * In the future will be extended as needed. + */ + class ShaderBuilderTestFixture + : public AllocatorsTestFixture + { + protected: + /////////////////////////////////////////////////////////////////////// + // AllocatorsTestFixture overrides + void SetUp() override; + void TearDown() override; + /////////////////////////////////////////////////////////////////////// + }; +} // namespace UnitTest + diff --git a/Gems/Atom/Asset/Shader/Code/Tests/SupervariantCmdArgumentTests.cpp b/Gems/Atom/Asset/Shader/Code/Tests/SupervariantCmdArgumentTests.cpp new file mode 100644 index 0000000000..2e5ee3fc09 --- /dev/null +++ b/Gems/Atom/Asset/Shader/Code/Tests/SupervariantCmdArgumentTests.cpp @@ -0,0 +1,523 @@ +/* +* All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or +* its licensors. +* +* For complete copyright and license terms please see the LICENSE at the root of this +* distribution (the "License"). All use of this software is governed by the License, +* or, if provided, by the license below or the license accompanying this file. Do not +* remove or modify any license notices. This file is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* +*/ + +#include +#include +#include + +#include +#include +#include + +#include + +#include "Common/ShaderBuilderTestFixture.h" + +namespace UnitTest +{ + using namespace AZ; + + struct KeyValueView + { + AZStd::string_view m_key; + AZStd::string_view m_value; + }; + + class SupervariantCmdArgumentTests : public ShaderBuilderTestFixture + { + protected: + static constexpr char MCPP_MACRO1[] = "MACRO1"; + static constexpr char MCPP_VALUE1[] = "VALUE1a"; + static constexpr char MCPP_NEW_VALUE1[] = "VALUE1b"; // Missing A is not a typo + + static constexpr char MCPP_MACRO2[] = "MACRO2"; + static constexpr char MCPP_VALUE2[] = "VALUE2"; + + static constexpr char MCPP_MACRO3[] = "MACRO3"; + static constexpr char MCPP_VALUE3[] = "VALUE3a"; + static constexpr char MCPP_NEW_VALUE3[] = "VALUE3b"; + + static constexpr char MCPP_MACRO4[] = "MACRO4"; + + static constexpr char MCPP_MACRO5[] = "MACRO5"; + + static constexpr char MCPP_MACRO6[] = "MACRO6"; + static constexpr char MCPP_VALUE6[] = "VALUE6"; + + static constexpr char AZSLC_ARG1[] = "--azsl1"; + + static constexpr char AZSLC_ARG2[] = "--azsl2"; + static constexpr char AZSLC_VAL2[] = "open,source"; + static constexpr char AZSLC_NEW_VAL2a[] = "closed,binary"; + static constexpr char AZSLC_NEW_VAL2b[] = "closed,source"; + + static constexpr char AZSLC_ARG3[] = "--azsl3"; + static constexpr char AZSLC_VAL3[] = "blue"; + + static constexpr char AZSLC_ARG4[] = "-azsl4"; + + static constexpr char AZSLC_ARG5[] = "--azsl5"; + static constexpr char AZSLC_VAL5[] = "smith,wick,john,45,-1,-1"; + static constexpr char AZSLC_NEW_VAL5[] = "apple,seed,crisp,-1,2,0"; + + static constexpr char AZSLC_ARG6[] = "--azsl6"; + + static constexpr char AZSLC_ARG7[] = "--azsl7"; + + //! Helper function. + //! Given an input list of {Key, Value} pairs returns a list of strings where each string is of the form: "Key=Value". + AZStd::vector CreateListOfStringsFromListOfKeyValues(AZStd::array_view listOfKeyValues) const + { + AZStd::vector listOfStrings; + for (const auto& keyValue : listOfKeyValues) + { + if (keyValue.m_value.empty()) + { + listOfStrings.push_back(keyValue.m_key); + } + else + { + listOfStrings.push_back(AZStd::string::format("%s=%s", keyValue.m_key.data(), keyValue.m_value.data())); + } + } + return listOfStrings; + } + + //! Helper function. + //! Given an input list of {Key, Value} pairs returns a list of strings where each string is of the form: "Key1", "Value1", "Key2", "Value2". + AZStd::vector CreateListOfSingleStringsFromListOfKeyValues(AZStd::array_view listOfKeyValues) const + { + AZStd::vector listOfStrings; + for (const auto& keyValue : listOfKeyValues) + { + listOfStrings.push_back(keyValue.m_key); + if (!keyValue.m_value.empty()) + { + listOfStrings.push_back(keyValue.m_value); + } + } + return listOfStrings; + } + + //! Helper function. + //! @param outputString: [out] The string " @argName" gets appended to it (The space is intentional). + //! Alternatively, if @argValue is NOT empty, then the string " @argName=@argValue" is + //! appended to it. + //! @param argName: A typical command line argument. "-p" or "--some". + //! @param argValue: A string representing the value that should be appended to @argName. + void AppendCmdLineArgument(AZStd::string& outputString, AZStd::string_view argName, AZStd::string_view argValue) const + { + if (argValue.empty()) + { + outputString += AZStd::string::format(" %s", argName.data()); + } + else + { + outputString += AZStd::string::format(" %s=%s", argName.data(), argValue.data()); + } + } + + //! Helper function. + //! Similar to above, but assumes that @argName refers to just the name of a macro definition so the appended string will always start + //! with "-D". + void AppendMacroDefinitionArgument(AZStd::string& outputString, AZStd::string_view argName, AZStd::string_view argValue) const + { + AppendCmdLineArgument(outputString, AZStd::string::format("-D%s", argName.data()), argValue); + } + + //! A helper made of helpers. + //! Returns a command line string that results of concatenating the input list of {Key, Value} pairs (with '='). + //! Example of a returned string: + //! "key1=value1 key2 key3 key4=value" + AZStd::string CreateCmdLineStringFromListOfKeyValues(AZStd::array_view listOfKeyValues) const + { + AZStd::string cmdLineString; + for (const auto& keyValueView : listOfKeyValues) + { + AppendCmdLineArgument(cmdLineString, keyValueView.m_key, keyValueView.m_value); + } + return cmdLineString; + } + + //! A helper made of helpers. + //! Returns a command line string of macro definitions that results of concatenating the input list of {Key, Value} pairs. + //! Example of a returned string: + //! "-Dkey1=value1 -Dkey2 -Dkey3 -Dkey4=value" + AZStd::string CreateMacroDefinitionCmdLineStringFromListOfKeyValues(AZStd::array_view listOfKeyValues) const + { + AZStd::string cmdLineString; + for (const auto& keyValueView : listOfKeyValues) + { + AppendMacroDefinitionArgument(cmdLineString, keyValueView.m_key, keyValueView.m_value); + } + return cmdLineString; + } + + //! @param includePaths A List of folder paths + //! @param predefinedMacros A List of strings with format: "name[=value]" + ShaderBuilder::PreprocessorOptions CreatePreprocessorOptions( + AZStd::array_view includePaths, AZStd::array_view predefinedMacros) const + { + ShaderBuilder::PreprocessorOptions preprocessorOptions; + + preprocessorOptions.m_projectIncludePaths.reserve(includePaths.size()); + for (const auto& path : includePaths) + { + preprocessorOptions.m_projectIncludePaths.push_back(path); + } + + preprocessorOptions.m_predefinedMacros.reserve(predefinedMacros.size()); + for (const auto& macro : predefinedMacros) + { + preprocessorOptions.m_predefinedMacros.push_back(macro); + } + + return preprocessorOptions; + } + + //! @param azslcAdditionalFreeArguments: A string representing series of command line arguments for AZSLc. + //! @param dxcAdditionalFreeArguments: A string representing series of command line arguments for DXC. + RHI::ShaderCompilerArguments CreateShaderCompilerArguments( + AZStd::string_view azslcAdditionalFreeArguments, AZStd::string_view dxcAdditionalFreeArguments) const + { + RHI::ShaderCompilerArguments shaderCompilerArguments; + shaderCompilerArguments.m_azslcWarningLevel = 1; + shaderCompilerArguments.m_azslcAdditionalFreeArguments = azslcAdditionalFreeArguments; + shaderCompilerArguments.m_defaultMatrixOrder = RHI::MatrixOrder::Row; + shaderCompilerArguments.m_dxcAdditionalFreeArguments = dxcAdditionalFreeArguments; + + return shaderCompilerArguments; + } + + + //! @param includePaths A List of folder paths + //! @param predefinedMacros A List of strings with format: "name[=value]" + //! @param azslcAdditionalFreeArguments A string representing series of command line arguments for AZSLc. + //! @param dxcAdditionalFreeArguments: A string representing series of command line arguments for DXC. + ShaderBuilder::GlobalBuildOptions CreateGlobalBuildOptions( + AZStd::array_view includePaths, + AZStd::array_view predefinedMacros, + AZStd::string_view azslcAdditionalFreeArguments, + AZStd::string_view dxcAdditionalFreeArguments) const + { + ShaderBuilder::GlobalBuildOptions globalBuildOptions; + globalBuildOptions.m_preprocessorSettings = CreatePreprocessorOptions(includePaths, predefinedMacros); + globalBuildOptions.m_compilerArguments = + CreateShaderCompilerArguments(azslcAdditionalFreeArguments, dxcAdditionalFreeArguments); + return globalBuildOptions; + } + + //! @param name Name of the supervariant. + //! @param plusArguments A string with command line arguments that contains both C-preprocessor macro definitions + //! and other command line arguments for AZSLc. + //! @param minusArguments A string with command line arguments that should be removed from the finalized command line arguments. + //! it can contain both, C-preprocessor macro definitions and other command line arguments for AZSLc. + RPI::ShaderSourceData::SupervariantInfo CreateSupervariantInfo( + AZStd::string_view name, AZStd::string_view plusArguments, AZStd::string_view minusArguments) const + { + RPI::ShaderSourceData::SupervariantInfo supervariantInfo; + supervariantInfo.m_name = name; + supervariantInfo.m_plusArguments = plusArguments; + supervariantInfo.m_minusArguments = minusArguments; + return supervariantInfo; + } + + bool StringContainsAllSubstrings(AZStd::string_view haystack, AZStd::array_view substrings) + { + return AZStd::all_of(AZ_BEGIN_END(substrings), + [&](AZStd::string_view needle) -> bool + { + return (haystack.find(needle) != AZStd::string::npos); + } + ); + } + + bool StringDoesNotContainAnyOneOfTheSubstrings(AZStd::string_view haystack, AZStd::array_view substrings) + { + return AZStd::all_of(AZ_BEGIN_END(substrings), [&](AZStd::string_view needle) -> bool { + return (haystack.find(needle) == AZStd::string::npos); + }); + } + + //! @returns: True if all strings in @substring appear in @vectorOfString. + //! @remark: Keep in mind that this is not the same as saying that all strings in @vectorOfStrings appear in @substrings. + bool VectorContainsAllSubstrings( + AZStd::array_view vectorOfStrings, AZStd::array_view substrings) + { + return AZStd::all_of( + AZ_BEGIN_END(substrings), + [&](AZStd::string_view needle) -> bool { + bool res = AZStd::any_of(AZ_BEGIN_END(vectorOfStrings), + [&](AZStd::string_view haystack) -> bool + { + return haystack.find(needle) != AZStd::string::npos; + } + ); + return res; + } + ); + } + + //! @returns: True only if None of the strings in @vectorOfStrings contains any of the strings in @substrings. + bool VectorDoesNotContainAnyOneOfTheSubstrings(AZStd::array_view vectorOfStrings, AZStd::array_view substrings) + { + return AZStd::all_of(AZ_BEGIN_END(vectorOfStrings), [&](AZStd::string_view haystack) -> bool { + return StringDoesNotContainAnyOneOfTheSubstrings(haystack, substrings); + }); + } + + }; // class SupervariantCmdArgumentTests + + + TEST_F(SupervariantCmdArgumentTests, CommandLineArgumentUtils_ValidateHelperFunctions) + { + // In this test the idea is to validate the static helper functions in AZ::RHI::ShaderCompilerArguments class + // that are useful for command line argument manipulation, etc. + AZStd::vector argumentList = { + {AZSLC_ARG1, ""}, {AZSLC_ARG2, AZSLC_VAL2}, {AZSLC_ARG3, AZSLC_VAL3}, {AZSLC_ARG4, ""}, {AZSLC_ARG5, AZSLC_VAL5}, + }; + + auto argumentsAsString = CreateCmdLineStringFromListOfKeyValues(argumentList); + auto listOfArgumentNames = AZ::RHI::CommandLineArgumentUtils::GetListOfArgumentNames(argumentsAsString); + + EXPECT_TRUE(AZStd::all_of(AZ_BEGIN_END(argumentList), [&](const KeyValueView& needle) -> bool { + return (AZStd::find(AZ_BEGIN_END(listOfArgumentNames), needle.m_key) != listOfArgumentNames.end()) && + // Make sure the values did not make into the expected list of keys. + (AZStd::find(AZ_BEGIN_END(listOfArgumentNames), needle.m_value) == listOfArgumentNames.end()); + })); + + AZStd::vector listOfArgumentsToRemove = { AZSLC_ARG4, AZSLC_ARG2 }; + auto stringWithRemovedArguments = + AZ::RHI::CommandLineArgumentUtils::RemoveArgumentsFromCommandLineString(listOfArgumentsToRemove, argumentsAsString); + EXPECT_TRUE(AZStd::all_of(AZ_BEGIN_END(listOfArgumentsToRemove), [&](const AZStd::string& needle) -> bool { + return stringWithRemovedArguments.find(needle) == AZStd::string::npos; + })); + + AZStd::vector listOfSurvivingArguments = {AZSLC_ARG1, AZSLC_ARG3, AZSLC_ARG5}; + EXPECT_TRUE(AZStd::all_of(AZ_BEGIN_END(listOfSurvivingArguments), [&](const AZStd::string& needle) -> bool { + return stringWithRemovedArguments.find(needle) != AZStd::string::npos; + })); + + auto stringWithoutExtraSpaces = + AZ::RHI::CommandLineArgumentUtils::RemoveExtraSpaces(" --arg1 -arg2 --arg3=foo --arg4=bar "); + EXPECT_EQ(stringWithoutExtraSpaces, AZStd::string("--arg1 -arg2 --arg3=foo --arg4=bar")); + + auto stringAsMergedArguments = + AZ::RHI::CommandLineArgumentUtils::MergeCommandLineArguments("--arg1 -arg2 --arg3=foo", "--arg3=bar --arg4"); + EXPECT_EQ(stringAsMergedArguments, AZStd::string("--arg1 -arg2 --arg3=bar --arg4")); + + EXPECT_TRUE(AZ::RHI::CommandLineArgumentUtils::HasMacroDefinitions("-DMACRO")); + EXPECT_TRUE(AZ::RHI::CommandLineArgumentUtils::HasMacroDefinitions("-D MACRO")); + EXPECT_TRUE(AZ::RHI::CommandLineArgumentUtils::HasMacroDefinitions("--help -D MACRO")); + EXPECT_TRUE(AZ::RHI::CommandLineArgumentUtils::HasMacroDefinitions("--help -p -DMACRO --more")); + EXPECT_TRUE(AZ::RHI::CommandLineArgumentUtils::HasMacroDefinitions("--help -p -D MACRO=VALUE --more")); + EXPECT_FALSE(AZ::RHI::CommandLineArgumentUtils::HasMacroDefinitions("--help -p --more")); + EXPECT_FALSE(AZ::RHI::CommandLineArgumentUtils::HasMacroDefinitions("--help -p --more --DFAKE")); + EXPECT_FALSE(AZ::RHI::CommandLineArgumentUtils::HasMacroDefinitions("--DFAKE1 --help -p --more --D FAKE2")); + } + + TEST_F(SupervariantCmdArgumentTests, ShaderCompilerArguments_ValidateCommandLineArgumentsMerge) + { + // In this test we validate that AZ::RHI::ShaderCompilerArguments::Merge() works as expected + // by merging AZSLC & DXC arguments giving higher priority to the arguments in the "right". + + auto shaderCompilerArgumentsLeft = CreateShaderCompilerArguments( + "--azsl1 --azsl2=avalue2a -azsl3 --azsl4=avalue4a", + "--dxc1=dvalue1a -dxc2 --dxc3=dvalue3a --dxc4"); + auto shaderCompilerArgumentsRight = CreateShaderCompilerArguments( + "--azsl1 --azsl2=avalue2b -azsl3 --azsl4=avalue4a --azsl5", + "--dxc1=dvalue1a -dxc2 --dxc3=dvalue3b --dxc4 --dxc5=dvalue5a"); + + shaderCompilerArgumentsLeft.Merge(shaderCompilerArgumentsRight); + EXPECT_EQ(shaderCompilerArgumentsLeft.m_azslcAdditionalFreeArguments, "--azsl1 --azsl2=avalue2b -azsl3 --azsl4=avalue4a --azsl5"); + EXPECT_EQ(shaderCompilerArgumentsLeft.m_dxcAdditionalFreeArguments, "--dxc1=dvalue1a -dxc2 --dxc3=dvalue3b --dxc4 --dxc5=dvalue5a"); + } + + + TEST_F(SupervariantCmdArgumentTests, SupervariantInfo_ValidateMemberFunctions) + { + // In this test all member functions of the ShaderSourceData::SupervariantInfo class + // are validated. + + AZStd::vector mcppMacrosList = { + {MCPP_MACRO1, MCPP_VALUE1}, + {MCPP_MACRO2, MCPP_VALUE2}, + {MCPP_MACRO3, MCPP_VALUE3}, + {MCPP_MACRO4, ""}, + }; + + AZStd::string argumentsToAddOrReplace; + AppendMacroDefinitionArgument(argumentsToAddOrReplace, MCPP_MACRO3, MCPP_NEW_VALUE3); + AppendCmdLineArgument(argumentsToAddOrReplace, AZSLC_ARG2, AZSLC_NEW_VAL2a); + AppendMacroDefinitionArgument(argumentsToAddOrReplace, MCPP_MACRO1, MCPP_NEW_VALUE1); + AppendCmdLineArgument(argumentsToAddOrReplace, AZSLC_ARG5, AZSLC_NEW_VAL5); + AppendMacroDefinitionArgument(argumentsToAddOrReplace, MCPP_MACRO5, ""); + AppendCmdLineArgument(argumentsToAddOrReplace, AZSLC_ARG6, ""); + + AZStd::string argumentsToRemove; + AppendCmdLineArgument(argumentsToRemove, AZSLC_ARG3, ""); + AppendMacroDefinitionArgument(argumentsToRemove, MCPP_MACRO2, ""); + AppendCmdLineArgument(argumentsToRemove, AZSLC_ARG4, ""); + AppendMacroDefinitionArgument(argumentsToRemove, MCPP_MACRO4, ""); + + auto supervariantInfo = CreateSupervariantInfo("Dummy", argumentsToAddOrReplace, argumentsToRemove); + + auto macroListToRemove = supervariantInfo.GetCombinedListOfMacroDefinitionNamesToRemove(); + AZStd::vector macroNamesToRemoveThatMustBePresent = { MCPP_MACRO1, MCPP_MACRO2, MCPP_MACRO3, MCPP_MACRO4, MCPP_MACRO5 }; + EXPECT_EQ(macroListToRemove.size(), macroNamesToRemoveThatMustBePresent.size()); + EXPECT_TRUE( + VectorContainsAllSubstrings(macroListToRemove, macroNamesToRemoveThatMustBePresent) + ); + + auto macroListToAdd = supervariantInfo.GetMacroDefinitionsToAdd(); + AZStd::vector macroNamesToAddThatMustBePresent = {MCPP_MACRO1, MCPP_MACRO3, MCPP_MACRO5}; + EXPECT_EQ(macroListToAdd.size(), macroNamesToAddThatMustBePresent.size()); + EXPECT_TRUE(VectorContainsAllSubstrings(macroListToAdd, macroNamesToAddThatMustBePresent)); + + // The result of GetCustomizedArgumentsForAzslc() is the most important value to test + AZStd::vector freeAzslcArgumentList = { + {AZSLC_ARG1, ""}, {AZSLC_ARG2, AZSLC_VAL2}, {AZSLC_ARG3, AZSLC_VAL3}, {AZSLC_ARG4, ""}, {AZSLC_ARG5, AZSLC_VAL5}, + }; + AZStd::string azslcArgs = CreateCmdLineStringFromListOfKeyValues(freeAzslcArgumentList); + AZStd::string customizedAzslcArgs = supervariantInfo.GetCustomizedArgumentsForAzslc(azslcArgs); + + AZStd::vector stringsThatMustBePresent = { + AZSLC_ARG1, AZSLC_ARG2, AZSLC_NEW_VAL2a, AZSLC_ARG5, AZSLC_NEW_VAL5, AZSLC_ARG6}; + EXPECT_TRUE(StringContainsAllSubstrings(customizedAzslcArgs, stringsThatMustBePresent)); + + AZStd::vector stringsThatCanNotBePresent = { AZSLC_ARG3, AZSLC_VAL3, AZSLC_ARG4, + // Because GetCustomizedArgumentsForAzslc() only returns arguments for AZSLc, none of the macro related + // arguments can be present + MCPP_MACRO1, MCPP_VALUE1, MCPP_NEW_VALUE1, + MCPP_MACRO2, MCPP_VALUE2, + MCPP_MACRO3, MCPP_VALUE3, MCPP_NEW_VALUE3, + MCPP_MACRO4, + MCPP_MACRO5 + }; + + EXPECT_TRUE( + StringDoesNotContainAnyOneOfTheSubstrings(customizedAzslcArgs, stringsThatCanNotBePresent) + ); + } + + + TEST_F(SupervariantCmdArgumentTests, ShaderAssetBuilder_ValidateInfluenceOfSupervariantInfoOnGlobalBuildOptions) + { + // In this test we validate how the ShaderAssetBuilder configure the commmand line arguments it passes + // to MCPP, AZSLc & DXC. It basically starts with a GlobalBuildOptions, that gets further customized by + // the ShaderCompilerArguments from ShaderSourceData(.shader file) and later further customized + // by each SupervariantInfo in ShaderSourceData. + + // The first step is to define the initial values of the GlobalBuildOptions. + AZStd::vector globalMcppMacrosList = { + {MCPP_MACRO1, MCPP_VALUE1}, + {MCPP_MACRO2, MCPP_VALUE2}, + {MCPP_MACRO3, MCPP_VALUE3}, + {MCPP_MACRO4, ""}, + }; + + AZStd::vector globalAzslArguments = { + {AZSLC_ARG1, ""}, + {AZSLC_ARG2, AZSLC_VAL2}, + {AZSLC_ARG3, AZSLC_VAL3}, + {AZSLC_ARG4, ""}, + {AZSLC_ARG5, AZSLC_VAL5}, + }; + + auto globalBuildOptions = CreateGlobalBuildOptions( + AZStd::vector(), CreateListOfStringsFromListOfKeyValues(globalMcppMacrosList), + CreateCmdLineStringFromListOfKeyValues(globalAzslArguments), + "" /* Don't care about DXC in this test */); + + // The second step is to load the Shader Compiler Arguments from the .shader file. + // These arguments will be merged in @globalBuildOptions, but the .shader arguments have + // higher priority. + AZStd::vector shaderAzslArguments = { + {AZSLC_ARG2, AZSLC_NEW_VAL2a}, + {AZSLC_ARG6, ""}, + }; + auto shaderCompilerArguments = CreateShaderCompilerArguments( + CreateCmdLineStringFromListOfKeyValues(shaderAzslArguments), "" /* Don't care about DXC in this test */); + globalBuildOptions.m_compilerArguments.Merge(shaderCompilerArguments); + + // Let's create the dummy supervariant. It will have some MCPP & AZSLc arguments to be added/replaced AND other MCPP & AZSLc arguments to be removed. + AZStd::vector supervariantAzslArgumentsToAdd = { + {AZSLC_ARG2, AZSLC_NEW_VAL2b}, + {AZSLC_ARG7, ""}, + }; + AZStd::vector supervariantMacroDefinitionsToAdd = { + {MCPP_MACRO1, MCPP_NEW_VALUE1}, + {MCPP_MACRO3, MCPP_NEW_VALUE3}, + {MCPP_MACRO5, ""}, + }; + auto supervariantArgumentsToAdd = CreateCmdLineStringFromListOfKeyValues(supervariantAzslArgumentsToAdd) + + CreateMacroDefinitionCmdLineStringFromListOfKeyValues(supervariantMacroDefinitionsToAdd); + + AZStd::vector supervariantAzslArgumentsToRemove = { + {AZSLC_ARG4, ""}, + {AZSLC_ARG1, ""}, + }; + AZStd::vector supervariantMacrosToRemove = { + {MCPP_MACRO2, ""}, + {MCPP_MACRO4, ""}, + }; + auto supervariantArgumentsToRemove = CreateCmdLineStringFromListOfKeyValues(supervariantAzslArgumentsToRemove) + + CreateMacroDefinitionCmdLineStringFromListOfKeyValues(supervariantMacrosToRemove); + + //CreateMacroDefinitionCmdLineStringFromListOfKeyValues + auto supervariantInfo = CreateSupervariantInfo("Dummy", + supervariantArgumentsToAdd, // These arguments will be added or replace existing ones. + supervariantArgumentsToRemove); // These arguments must be removed. + + AZStd::vector macroDefinitionNamesToRemove = supervariantInfo.GetCombinedListOfMacroDefinitionNamesToRemove(); + globalBuildOptions.m_preprocessorSettings.RemovePredefinedMacros(macroDefinitionNamesToRemove); + AZStd::vector macroDefinitionsToAdd = supervariantInfo.GetMacroDefinitionsToAdd(); + globalBuildOptions.m_preprocessorSettings.m_predefinedMacros.insert( + globalBuildOptions.m_preprocessorSettings.m_predefinedMacros.end(), macroDefinitionsToAdd.begin(), macroDefinitionsToAdd.end()); + + // Validate macro definitions that must be present. + EXPECT_TRUE( + VectorContainsAllSubstrings( + globalBuildOptions.m_preprocessorSettings.m_predefinedMacros, + AZStd::vector({MCPP_MACRO1, MCPP_NEW_VALUE1, MCPP_MACRO3, MCPP_NEW_VALUE3, MCPP_MACRO5})) + ); + + // Validate macro definitions that can't be present. + EXPECT_TRUE( + VectorDoesNotContainAnyOneOfTheSubstrings( + globalBuildOptions.m_preprocessorSettings.m_predefinedMacros, + AZStd::vector({MCPP_MACRO2, MCPP_VALUE3, MCPP_MACRO4})) + ); + + AZStd::string azslcArgsFromGlobalBuildOptions = globalBuildOptions.m_compilerArguments.MakeAdditionalAzslcCommandLineString(); + + // The result of GetCustomizedArgumentsForAzslc() is the most important value to test + AZStd::string customizedAzslcArgs = supervariantInfo.GetCustomizedArgumentsForAzslc(azslcArgsFromGlobalBuildOptions); + + EXPECT_TRUE( + StringContainsAllSubstrings(customizedAzslcArgs, CreateListOfSingleStringsFromListOfKeyValues(supervariantAzslArgumentsToAdd)) + ); + + EXPECT_TRUE( + StringDoesNotContainAnyOneOfTheSubstrings(customizedAzslcArgs, CreateListOfSingleStringsFromListOfKeyValues(supervariantAzslArgumentsToRemove)) + ); + + EXPECT_TRUE( + StringContainsAllSubstrings(customizedAzslcArgs, AZStd::vector({AZSLC_ARG3, AZSLC_VAL3, AZSLC_ARG5, AZSLC_VAL5})) + ); + } + + +} //namespace UnitTest + +AZ_UNIT_TEST_HOOK(DEFAULT_UNIT_TEST_ENV); + diff --git a/Gems/Atom/Asset/Shader/Code/atom_asset_shader_builders_tests_files.cmake b/Gems/Atom/Asset/Shader/Code/atom_asset_shader_builders_tests_files.cmake new file mode 100644 index 0000000000..9f22f9f632 --- /dev/null +++ b/Gems/Atom/Asset/Shader/Code/atom_asset_shader_builders_tests_files.cmake @@ -0,0 +1,16 @@ +# +# All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or +# its licensors. +# +# For complete copyright and license terms please see the LICENSE at the root of this +# distribution (the "License"). All use of this software is governed by the License, +# or, if provided, by the license below or the license accompanying this file. Do not +# remove or modify any license notices. This file is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# + +set(FILES + Tests/Common/ShaderBuilderTestFixture.h + Tests/Common/ShaderBuilderTestFixture.cpp + Tests/SupervariantCmdArgumentTests.cpp +) diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI.Edit/ShaderCompilerArguments.h b/Gems/Atom/RHI/Code/Include/Atom/RHI.Edit/ShaderCompilerArguments.h index 2073c48d25..a5d8f53573 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI.Edit/ShaderCompilerArguments.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI.Edit/ShaderCompilerArguments.h @@ -13,6 +13,8 @@ #include #include +#include +#include namespace AZ { @@ -30,9 +32,16 @@ namespace AZ static void Reflect(ReflectContext* context); + //! Returns true if either @m_azslcAdditionalFreeArguments or @m_dxcAdditionalFreeArguments contain + //! macro definitions, e.g. "-D MACRO" or "-D MACRO=VALUE" or "-DMACRO", "-DMACRO=VALUE". + //! It is used for validation to forbid macro definitions, because the idea is that this struct + //! is used inside GlobalBuildOptions which has a dedicated variable for macro definitions. + bool HasMacroDefinitionsInCommandLineArguments(); + //! Mix two instances of arguments, by or-ing bools, or by "if different, right hand side wins" void Merge(const ShaderCompilerArguments& right); + //! [GFX TODO] [ATOM-15472] Remove this function. //! Determine whether there is a rebuild-worthy difference in arguments for AZSLc bool HasDifferentAzslcArguments(const ShaderCompilerArguments& right) const; diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI.Edit/Utils.h b/Gems/Atom/RHI/Code/Include/Atom/RHI.Edit/Utils.h index 3e593794af..6f884eb358 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI.Edit/Utils.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI.Edit/Utils.h @@ -110,6 +110,46 @@ namespace AZ AZStd::string BuildFileNameWithExtension(const AZStd::string& shaderSourceFile, const AZStd::string& tempFolder, const char* outputExtension); + + namespace CommandLineArgumentUtils + { + //! @param commandLineString: A string with command line arguments of the form: + //! "- -- --[=] ..." + //! Example: "--use-spaces --namespace=vk -W1" + //! Returns: A list with just the [-|--]: + //! ["-", "--", "--arg3"] + //! For the example shown above it will return this vector: + //! ["--use-spaces", "--namespace", "-W1] + AZStd::vector GetListOfArgumentNames(AZStd::string_view commandLineString); + + //! Takes a list of names of command line arguments and removes those arguments from @commandLineString. + //! The core functionality of this function is that it searches by name in @commandLineString and removes + //! name and value if the name is found. + //! @param listOfArguments: This is a list of strings, usually generated by the helper function + //! ShaderCompilerArguments::GetListOfArgumentNames() + //! @param commandLineString: A single string made of several command line arguments + //! @returns A new string based on @commandLineString but with the matching arguments and their values + //! removed from it. + AZStd::string RemoveArgumentsFromCommandLineString( + AZStd::array_view listOfArguments, AZStd::string_view commandLineString); + + //! @param commandLineString: " --arg1 -arg2 --arg3=foo --arg4=bar " + //! @returns "--arg1 -arg2 --arg3=foo --arg4=bar" + AZStd::string RemoveExtraSpaces(AZStd::string_view commandLineString); + + //! Accepts two arbitrary strings that contain typical command line arguments and returns + //! a new string that combines the arguments were the arguments on the @right have precedence. + //! Example: + //! @param left: "--arg1 -arg2 --arg3=foo" + //! @param right: "--arg3=bar --arg4" + //! @returns: "--arg1 -arg2 --arg3=bar --arg4" + AZStd::string MergeCommandLineArguments(AZStd::string_view left, AZStd::string_view right); + + //! @param commandLineString: A string that contains a series of command line arguments. + //! @returns: true if @commandLineString contains macro definitions, e.g: + //! "-D MACRO" or "-D MACRO=VALUE" or "-DMACRO", "-DMACRO=VALUE". + bool HasMacroDefinitions(AZStd::string_view commandLineString); + } } } diff --git a/Gems/Atom/RHI/Code/Source/RHI.Edit/ShaderCompilerArguments.cpp b/Gems/Atom/RHI/Code/Source/RHI.Edit/ShaderCompilerArguments.cpp index 7304a351d4..3a04bf6b88 100644 --- a/Gems/Atom/RHI/Code/Source/RHI.Edit/ShaderCompilerArguments.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI.Edit/ShaderCompilerArguments.cpp @@ -12,6 +12,9 @@ #include #include +#include + +#include namespace AZ { @@ -49,6 +52,12 @@ namespace AZ } } + bool ShaderCompilerArguments::HasMacroDefinitionsInCommandLineArguments() + { + return CommandLineArgumentUtils::HasMacroDefinitions(m_azslcAdditionalFreeArguments) || + CommandLineArgumentUtils::HasMacroDefinitions(m_dxcAdditionalFreeArguments); + } + void ShaderCompilerArguments::Merge(const ShaderCompilerArguments& right) { if (right.m_azslcWarningLevel != LevelUnset) @@ -56,7 +65,7 @@ namespace AZ m_azslcWarningLevel = right.m_azslcWarningLevel; } m_azslcWarningAsError = m_azslcWarningAsError || right.m_azslcWarningAsError; - m_azslcAdditionalFreeArguments += " " + right.m_azslcAdditionalFreeArguments; + m_azslcAdditionalFreeArguments = CommandLineArgumentUtils::MergeCommandLineArguments(m_azslcAdditionalFreeArguments, right.m_azslcAdditionalFreeArguments); m_dxcDisableWarnings = m_dxcDisableWarnings || right.m_dxcDisableWarnings; m_dxcWarningAsError = m_dxcWarningAsError || right.m_dxcWarningAsError; m_dxcDisableOptimizations = m_dxcDisableOptimizations || right.m_dxcDisableOptimizations; @@ -65,13 +74,14 @@ namespace AZ { m_dxcOptimizationLevel = right.m_dxcOptimizationLevel; } - m_dxcAdditionalFreeArguments += " " + right.m_dxcAdditionalFreeArguments; + m_dxcAdditionalFreeArguments = CommandLineArgumentUtils::MergeCommandLineArguments(m_dxcAdditionalFreeArguments, right.m_dxcAdditionalFreeArguments); if (right.m_defaultMatrixOrder != MatrixOrder::Default) { m_defaultMatrixOrder = right.m_defaultMatrixOrder; } } + //! [GFX TODO] [ATOM-15472] Remove this function. bool ShaderCompilerArguments::HasDifferentAzslcArguments(const ShaderCompilerArguments& right) const { auto isSet = +[](uint8_t level) { return level != LevelUnset; }; diff --git a/Gems/Atom/RHI/Code/Source/RHI.Edit/Utils.cpp b/Gems/Atom/RHI/Code/Source/RHI.Edit/Utils.cpp index 00b5dada69..dc0efb7e9a 100644 --- a/Gems/Atom/RHI/Code/Source/RHI.Edit/Utils.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI.Edit/Utils.cpp @@ -494,5 +494,64 @@ namespace AZ AzFramework::StringFunc::Path::ReplaceExtension(outputFile, outputExtension); return outputFile; } + + namespace CommandLineArgumentUtils + { + AZStd::vector GetListOfArgumentNames(AZStd::string_view commandLineString) + { + AZStd::vector listOfTokens; + AzFramework::StringFunc::Tokenize(commandLineString, listOfTokens, " \t\n"); + AZStd::vector listOfArguments; + for (const AZStd::string& token : listOfTokens) + { + AZStd::vector splitArguments; + AzFramework::StringFunc::Tokenize(token, splitArguments, "="); + listOfArguments.push_back(splitArguments[0]); + } + return listOfArguments; + } + + AZStd::string RemoveArgumentsFromCommandLineString( + AZStd::array_view listOfArgumentsToRemove, AZStd::string_view commandLineString) + { + AZStd::string customizedArguments = commandLineString; + for (const AZStd::string& azslcArgumentName : listOfArgumentsToRemove) + { + AZStd::string regexStr = AZStd::string::format("%s(=\\S+)?", azslcArgumentName.c_str()); + AZStd::regex replaceRegex(regexStr, AZStd::regex::ECMAScript); + customizedArguments = AZStd::regex_replace(customizedArguments, replaceRegex, ""); + } + return customizedArguments; + } + + AZStd::string RemoveExtraSpaces(AZStd::string_view commandLineString) + { + AZStd::vector argumentList; + AzFramework::StringFunc::Tokenize(commandLineString, argumentList, " \t\n"); + AZStd::string cleanStringWithArguments; + AzFramework::StringFunc::Join(cleanStringWithArguments, argumentList.begin(), argumentList.end(), " "); + return cleanStringWithArguments; + } + + AZStd::string MergeCommandLineArguments(AZStd::string_view left, AZStd::string_view right) + { + auto listOfArgumentNamesFromRight = GetListOfArgumentNames(right); + auto leftWithRightArgumentsRemoved = RemoveArgumentsFromCommandLineString(listOfArgumentNamesFromRight, left); + AZStd::string combinedArguments = AZStd::string::format("%s %s", leftWithRightArgumentsRemoved.c_str(), right.data()); + return RemoveExtraSpaces(combinedArguments); + } + + bool HasMacroDefinitions(AZStd::string_view commandLineString) + { + const AZStd::regex macroRegex(R"((^-D\s*(\w+))|(\s+-D\s*(\w+)))", AZStd::regex::ECMAScript); + + AZStd::smatch match; + if (AZStd::regex_search(commandLineString.data(), match, macroRegex)) + { + return (match.size() >= 1); + } + return false; + } + } //namespace CommandLineArgumentUtils } // namespace RHI } // namespace AZ diff --git a/Gems/Atom/RPI/Code/CMakeLists.txt b/Gems/Atom/RPI/Code/CMakeLists.txt index f92213d9d7..d2b7fba071 100644 --- a/Gems/Atom/RPI/Code/CMakeLists.txt +++ b/Gems/Atom/RPI/Code/CMakeLists.txt @@ -69,6 +69,7 @@ if(PAL_TRAIT_BUILD_HOST_TOOLS) PRIVATE AZ::AtomCore AZ::AzToolsFramework + Gem::Atom_RHI.Edit Gem::Atom_RPI.Public ) diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Shader/ShaderSourceData.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Shader/ShaderSourceData.cpp index aac81a6e26..376399ff84 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Shader/ShaderSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Shader/ShaderSourceData.cpp @@ -11,6 +11,8 @@ */ #include +#include +#include #include #include @@ -57,7 +59,7 @@ namespace AZ bool ShaderSourceData::IsRhiBackendDisabled(const AZ::Name& rhiName) const { - return AZStd::any_of(m_disabledRhiBackends.begin(), m_disabledRhiBackends.end(), [&](const AZStd::string& currentRhiName) + return AZStd::any_of(AZ_BEGIN_END(m_disabledRhiBackends), [&](const AZStd::string& currentRhiName) { return currentRhiName == rhiName.GetStringView(); }); @@ -72,19 +74,32 @@ namespace AZ static void GetListOfMacroDefinitionNames( const AZStd::string& stringWithArguments, AZStd::vector& macroDefinitionNames) { - static const AZStd::regex macroRegex("-D\\s*(\\w+)", AZStd::regex::ECMAScript); + const AZStd::regex macroRegex(R"(-D\s*(\w+))", AZStd::regex::ECMAScript); - AZStd::cmatch match; - if (AZStd::regex_search(stringWithArguments.c_str(), match, macroRegex)) + AZStd::string hayStack(stringWithArguments); + AZStd::smatch match; + while (AZStd::regex_search(hayStack.c_str(), match, macroRegex)) { // First pattern is always the entire string for (unsigned i = 1; i < match.size(); ++i) { if (match[i].matched) { - macroDefinitionNames.push_back(match[i].str().c_str()); + AZStd::string macroToAdd(match[i].str().c_str()); + const bool isPresent = AZStd::any_of(AZ_BEGIN_END(macroDefinitionNames), + [&](AZStd::string_view macroName) -> bool + { + return macroToAdd == macroName; + } + ); + if (isPresent) + { + continue; + } + macroDefinitionNames.push_back(macroToAdd); } } + hayStack = match.suffix(); } } @@ -103,19 +118,22 @@ namespace AZ static void GetListOfMacroDefinitions( const AZStd::string& stringWithArguments, AZStd::vector& macroDefinitions) { - static const AZStd::regex macroRegex("-D\\s*(\\w+(=\\w+)?)", AZStd::regex::ECMAScript); + const AZStd::regex macroRegex(R"(-D\s*(\w+)(=\w+)?)", AZStd::regex::ECMAScript); - AZStd::cmatch match; - if (AZStd::regex_search(stringWithArguments.c_str(), match, macroRegex)) + AZStd::string hayStack(stringWithArguments); + AZStd::smatch match; + while (AZStd::regex_search(hayStack.c_str(), match, macroRegex)) { - // First pattern is always the entire string - for (unsigned i = 1; i < match.size(); ++i) + if (match.size() > 1) { - if (match[i].matched) + AZStd::string macro(match[1].str().c_str()); + if (match.size() > 2) { - macroDefinitions.push_back(match[i].str().c_str()); + macro += match[2].str().c_str(); } + macroDefinitions.push_back(macro); } + hayStack = match.suffix(); } } @@ -126,62 +144,27 @@ namespace AZ return parsedMacroDefinitions; } - - // Helper. - // @arguments: A string with command line arguments for a console application of the form: - // "- -- --[=] ..." - // Example: "--use-spaces --namespace=vk" - // Returns: A list with just the [-|--]: - // ["-", "--", "--arg3"] - // For the example shown above it will return this vector: - // ["--use-spaces", "--namespace"] - AZStd::vector GetListOfArgumentNames(const AZStd::string& arguments) - { - AZStd::vector listOfTokens; - AzFramework::StringFunc::Tokenize(arguments, listOfTokens); - AZStd::vector listOfArguments; - for (const AZStd::string& token : listOfTokens) - { - AZStd::vector splitArguments; - AzFramework::StringFunc::Tokenize(token, splitArguments, "="); - listOfArguments.push_back(splitArguments[0]); - } - return listOfArguments; - } - AZStd::string ShaderSourceData::SupervariantInfo::GetCustomizedArgumentsForAzslc( const AZStd::string& initialAzslcCompilerArguments) const { - static const AZStd::regex macroRegex("-D\\s*(\\w+(=\\S+)?)", AZStd::regex::ECMAScript); + const AZStd::regex macroRegex(R"(-D\s*(\w+(=\S+)?))", AZStd::regex::ECMAScript); // We are only concerned with AZSLc arguments. Let's remove the C-Preprocessor macro definitions // from @minusArguments. const AZStd::string minusArguments = AZStd::regex_replace(m_minusArguments, macroRegex, ""); const AZStd::string plusArguments = AZStd::regex_replace(m_plusArguments, macroRegex, ""); AZStd::string azslcArgumentsToRemove = minusArguments + " " + plusArguments; - AZStd::vector azslcArgumentNamesToRemove = GetListOfArgumentNames(azslcArgumentsToRemove); + AZStd::vector azslcArgumentNamesToRemove = RHI::CommandLineArgumentUtils::GetListOfArgumentNames(azslcArgumentsToRemove); // At this moment @azslcArgumentsToRemove contains arguments for AZSLc that can be of the form: // - // --[=] // We need to remove those from @initialAzslcCompilerArguments. - AZStd::string customizedArguments = initialAzslcCompilerArguments; - for (const AZStd::string& azslcArgumentName : azslcArgumentNamesToRemove) - { - AZStd::string regexStr = AZStd::string::format("%s(=\\S+)?", azslcArgumentName.c_str()); - AZStd::regex replaceRegex(regexStr, AZStd::regex::ECMAScript); - customizedArguments = AZStd::regex_replace(customizedArguments, replaceRegex, ""); - } - + AZStd::string customizedArguments = RHI::CommandLineArgumentUtils::RemoveArgumentsFromCommandLineString( + azslcArgumentNamesToRemove, initialAzslcCompilerArguments); customizedArguments += " " + plusArguments; - // Will contain the results that will be joined by a space. - // This is used to get a clean string to return without excess spaces. - AZStd::vector argumentList; - AzFramework::StringFunc::Tokenize(customizedArguments, argumentList, " \t\n"); - customizedArguments.clear(); // Need to clear because Join appends. - AzFramework::StringFunc::Join(customizedArguments, argumentList.begin(), argumentList.end(), " "); - return customizedArguments; + return RHI::CommandLineArgumentUtils::RemoveExtraSpaces(customizedArguments); }