From a3cc8845dd12d6fa8de34322e7d10c9fd21c23fe Mon Sep 17 00:00:00 2001 From: Jeremy Ong Date: Mon, 27 Sep 2021 01:21:31 -0600 Subject: [PATCH] Disable Vulkan PDB generation and fix D3D12 PDBs Vulkan PDBs are currently broken because debug data isn't stripped from the shader binary. Furthermore, sub-id hashing is broken because sub-ids count from 1 which generates a collision on the first sub-object. Changing the default value for the sub-id argument resolves this. Signed-off-by: Jeremy Ong --- .../Code/Source/RHI.Edit/ShaderCompilerArguments.cpp | 5 ----- .../Source/RHI.Builders/ShaderPlatformInterface.cpp | 12 ++++++++---- .../Atom/RPI.Reflect/Shader/ShaderVariantAsset.h | 2 +- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/Gems/Atom/RHI/Code/Source/RHI.Edit/ShaderCompilerArguments.cpp b/Gems/Atom/RHI/Code/Source/RHI.Edit/ShaderCompilerArguments.cpp index ab45d2239b..ef330e7902 100644 --- a/Gems/Atom/RHI/Code/Source/RHI.Edit/ShaderCompilerArguments.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI.Edit/ShaderCompilerArguments.cpp @@ -155,11 +155,6 @@ namespace AZ { arguments += " -Zpr"; } - if (m_dxcGenerateDebugInfo) - { - arguments += " -Zi"; // Generate debug information - arguments += " -Zss"; // Compute Shader Hash considering source information - } // strip spaces at both sides AZStd::string dxcAdditionalFreeArguments = m_dxcAdditionalFreeArguments; AzFramework::StringFunc::TrimWhiteSpace(dxcAdditionalFreeArguments, true, true); diff --git a/Gems/Atom/RHI/DX12/Code/Source/RHI.Builders/ShaderPlatformInterface.cpp b/Gems/Atom/RHI/DX12/Code/Source/RHI.Builders/ShaderPlatformInterface.cpp index 89ae3ede46..f30fc72ace 100644 --- a/Gems/Atom/RHI/DX12/Code/Source/RHI.Builders/ShaderPlatformInterface.cpp +++ b/Gems/Atom/RHI/DX12/Code/Source/RHI.Builders/ShaderPlatformInterface.cpp @@ -255,6 +255,11 @@ namespace AZ // Compilation parameters AZStd::string params = shaderCompilerArguments.MakeAdditionalDxcCommandLineString(); + if (BuildHasDebugInfo(shaderCompilerArguments)) + { + params += " -Zi"; // Generate debug information + params += " -Zss"; // Compute Shader Hash considering source information + } // Enable half precision types when shader model >= 6.2 int shaderModelMajor = 0; @@ -281,12 +286,11 @@ namespace AZ AZStd::string symbolDatabaseFileCliArgument{" "}; // when not debug: still insert a space between 5.dxil and 7.hlsl-in if (BuildHasDebugInfo(shaderCompilerArguments)) { - // prepare .ldd filename: + // prepare .pdb filename: AZStd::string md5hex = RHI::ByteToHexString(md5); AZStd::string symbolDatabaseFilePath = dxcInputFile.c_str(); // mutate from source - AZStd::string lldFileName = md5hex // lld is like pdb but it's the default symbol database extension in dxc - + "-" + profileIt->second; // concatenate the shader profile to disambiguate vs/ps... - AzFramework::StringFunc::Path::ReplaceFullName(symbolDatabaseFilePath, lldFileName.c_str(), "lld"); + AZStd::string pdbFileName = md5hex + "-" + profileIt->second; // concatenate the shader profile to disambiguate vs/ps... + AzFramework::StringFunc::Path::ReplaceFullName(symbolDatabaseFilePath, pdbFileName.c_str(), "pdb"); // it is possible that another activated platform/profile, already exported that file. (since it's hashed on the source file) // dxc returns an error in such case. we get less surprising effets by just not mentionning an -Fd argument if (AZ::IO::SystemFile::Exists(symbolDatabaseFilePath.c_str())) diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Shader/ShaderVariantAsset.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Shader/ShaderVariantAsset.h index 1462490884..66bbd7b188 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Shader/ShaderVariantAsset.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Shader/ShaderVariantAsset.h @@ -40,7 +40,7 @@ namespace AZ //! by ShaderVariantAssetBuilder this is 1+. static uint32_t MakeAssetProductSubId( uint32_t rhiApiUniqueIndex, uint32_t supervariantIndex, ShaderVariantStableId variantStableId, - uint32_t subProductType = ShaderVariantAssetSubProductType); + uint32_t subProductType = 0); ShaderVariantAsset() = default; ~ShaderVariantAsset() = default;