From 3989df07fc373e2ca9728fbc26dd561527a80278 Mon Sep 17 00:00:00 2001 From: assiduous Date: Wed, 3 Feb 2021 20:47:43 -0800 Subject: Some refactoring of DXBCUtils --- Graphics/ShaderTools/CMakeLists.txt | 25 +++-- Graphics/ShaderTools/include/DXBCUtils.hpp | 13 ++- Graphics/ShaderTools/src/DXBCUtils.cpp | 145 ++++++++++++++++------------- 3 files changed, 101 insertions(+), 82 deletions(-) (limited to 'Graphics/ShaderTools') diff --git a/Graphics/ShaderTools/CMakeLists.txt b/Graphics/ShaderTools/CMakeLists.txt index 61dc51ac..0921eaf4 100644 --- a/Graphics/ShaderTools/CMakeLists.txt +++ b/Graphics/ShaderTools/CMakeLists.txt @@ -29,14 +29,14 @@ if(ENABLE_HLSL) list(APPEND INCLUDE include/HLSLUtils.hpp) endif() -#if(D3D11_SUPPORTED OR D3D12_SUPPORTED) - list(APPEND SOURCE - src/DXBCUtils.cpp - ../../ThirdParty/GPUOpenShaderUtils/DXBCChecksum.cpp) - list(APPEND INCLUDE - include/DXBCUtils.hpp - ../../ThirdParty/GPUOpenShaderUtils/DXBCChecksum.h) -#endif() +if(D3D11_SUPPORTED OR D3D12_SUPPORTED) + set(DXBC_CHECKSUM_SOURCE + ../../ThirdParty/GPUOpenShaderUtils/DXBCChecksum.cpp + ../../ThirdParty/GPUOpenShaderUtils/DXBCChecksum.h + ) + list(APPEND SOURCE src/DXBCUtils.cpp) + list(APPEND INCLUDE include/DXBCUtils.hpp) +endif() if((PLATFORM_WIN32 AND NOT MINGW_BUILD) OR PLATFORM_UNIVERSAL_WINDOWS OR PLATFORM_LINUX) set(DXC_SUPPORTED TRUE) @@ -92,7 +92,7 @@ if(ENABLE_SPIRV) endif() endif() -add_library(Diligent-ShaderTools STATIC ${SOURCE} ${INCLUDE}) +add_library(Diligent-ShaderTools STATIC ${SOURCE} ${INCLUDE} ${DXBC_CHECKSUM_SOURCE}) target_include_directories(Diligent-ShaderTools PUBLIC @@ -105,10 +105,6 @@ if (DXC_SUPPORTED) target_include_directories(Diligent-ShaderTools PUBLIC ../../ThirdParty/DirectXShaderCompiler) endif() -#if(D3D11_SUPPORTED OR D3D12_SUPPORTED) - target_include_directories(Diligent-ShaderTools PRIVATE ../../ThirdParty/GPUOpenShaderUtils) -#endif() - target_link_libraries(Diligent-ShaderTools PRIVATE Diligent-BuildSettings @@ -152,6 +148,9 @@ set_common_target_properties(Diligent-ShaderTools) source_group("src" FILES ${SOURCE}) source_group("include" FILES ${INCLUDE}) source_group("interface" FILES ${INTERFACE}) +if (DXBC_CHECKSUM_SOURCE) + source_group("ThirdParty\\DXBCChecksum" FILES ${DXBC_CHECKSUM_SOURCE}) +endif() set_target_properties(Diligent-ShaderTools PROPERTIES FOLDER DiligentCore/Graphics diff --git a/Graphics/ShaderTools/include/DXBCUtils.hpp b/Graphics/ShaderTools/include/DXBCUtils.hpp index 75768c5c..bb509908 100644 --- a/Graphics/ShaderTools/include/DXBCUtils.hpp +++ b/Graphics/ShaderTools/include/DXBCUtils.hpp @@ -27,11 +27,9 @@ #pragma once -#include #include -#include "Constants.h" -#include "Shader.h" +#include "BasicTypes.h" #include "HashUtils.hpp" namespace Diligent @@ -48,11 +46,16 @@ struct DXBCUtils /// A mapping from the resource name to the binding (shader register). using TResourceBindingMap = std::unordered_map; + + /// Remaps resource bindings in the given DXBC byte code. + /// \param [in] ResourceMap - Resource binding map. For every resource in the /// byte code it must define the binding (shader register). - /// \param [inout] pBytecode - Byte code that will be patched. + /// \param [inout] pBytecode - Pointer to the byte code to be patched. + /// \param [in] Size - The byte code size, in bytes. static bool RemapDXBCResources(const TResourceBindingMap& ResourceMap, - ID3DBlob* pBytecode); + void* pBytecode, + size_t Size); }; } // namespace Diligent diff --git a/Graphics/ShaderTools/src/DXBCUtils.cpp b/Graphics/ShaderTools/src/DXBCUtils.cpp index 40409178..08f94fb2 100644 --- a/Graphics/ShaderTools/src/DXBCUtils.cpp +++ b/Graphics/ShaderTools/src/DXBCUtils.cpp @@ -26,13 +26,16 @@ */ #include + #include "DXBCUtils.hpp" -#include "DXBCChecksum.h" +#include "../../../ThirdParty/GPUOpenShaderUtils/DXBCChecksum.h" namespace Diligent { + namespace { + struct DXBCHeader { Uint32 Magic; // 0..3 "DXBC" @@ -41,7 +44,7 @@ struct DXBCHeader Uint32 TotalSize; // 24..27 Uint32 ChunkCount; // 28..31 }; -static_assert(sizeof(DXBCHeader) == 32, ""); +static_assert(sizeof(DXBCHeader) == 32, "The size of DXBC header must be 32 bytes"); struct ChunkHeader { @@ -61,9 +64,9 @@ struct ResourceDefChunkHeader : ChunkHeader Uint32 Flags; // 28..31 Uint32 CreatorStringOffset; // 32..35, from start of chunk data }; -static_assert(sizeof(ResourceDefChunkHeader) == 36, ""); +static_assert(sizeof(ResourceDefChunkHeader) == 36, "The size of resource definition chunk header must be 36 bytes"); -struct ResourceBindingInfo11 +struct ResourceBindingInfo50 { Uint32 NameOffset; // 0..3, from start of chunk data D3D_SHADER_INPUT_TYPE ShaderInputType; // 4..7 @@ -74,9 +77,9 @@ struct ResourceBindingInfo11 Uint32 BindCount; // 24..27 D3D_SHADER_INPUT_FLAGS ShaderInputFlags; // 28..31 }; -static_assert(sizeof(ResourceBindingInfo11) == 32, ""); +static_assert(sizeof(ResourceBindingInfo50) == 32, "The size of SM50 resource binding info struct must be 32 bytes"); -struct ResourceBindingInfo12 +struct ResourceBindingInfo51 { Uint32 NameOffset; // 0..3, from start of chunk data D3D_SHADER_INPUT_TYPE ShaderInputType; // 4..7 @@ -89,7 +92,7 @@ struct ResourceBindingInfo12 Uint32 Space; // 32..35 Uint32 Reserved; // 36..39 }; -static_assert(sizeof(ResourceBindingInfo12) == 40, ""); +static_assert(sizeof(ResourceBindingInfo51) == 40, "The size of SM51 resource binding info struct must be 40 bytes"); #define FOURCC(a, b, c, d) (Uint32{(d) << 24} | Uint32{(c) << 16} | Uint32{(b) << 8} | Uint32{a}) @@ -97,132 +100,146 @@ constexpr Uint32 DXBCFourCC = FOURCC('D', 'X', 'B', 'C'); constexpr Uint32 RDEFFourCC = FOURCC('R', 'D', 'E', 'F'); +inline bool PatchSpace(ResourceBindingInfo51& Res, Uint32 Space) +{ + Res.Space = Space; + return true; +} + +inline bool PatchSpace(ResourceBindingInfo50&, Uint32 Space) +{ + return Space == 0 || Space == ~0U; +} + +template bool RemapShaderResources(const DXBCUtils::TResourceBindingMap& ResourceMap, const void* EndPtr, ResourceDefChunkHeader* RDEFHeader) { VERIFY_EXPR(RDEFHeader->Magic == RDEFFourCC); - VERIFY_EXPR((RDEFHeader->MajorVersion == 5 && RDEFHeader->MinorVersion == 0) || RDEFHeader->MajorVersion < 5); auto* Ptr = reinterpret_cast(RDEFHeader) + sizeof(ChunkHeader); - auto* ResBinding = reinterpret_cast(Ptr + RDEFHeader->ResBindingOffset); - VERIFY((ResBinding + RDEFHeader->ResBindingCount) <= EndPtr, "Resource bindings is outside of buffer range."); + auto* ResBinding = reinterpret_cast(Ptr + RDEFHeader->ResBindingOffset); + if (ResBinding + RDEFHeader->ResBindingCount > EndPtr) + { + LOG_ERROR_MESSAGE("Resource binding data is outside of the specified byte code range. The byte code may be corrupted."); + return false; + } for (Uint32 r = 0; r < RDEFHeader->ResBindingCount; ++r) { auto& Res = ResBinding[r]; const char* Name = Ptr + Res.NameOffset; - VERIFY(Name < EndPtr, "Resource name pointer is outside of buffer range."); - - auto Iter = ResourceMap.find(HashMapStringKey{Name}); - if (Iter == ResourceMap.end()) + if (Name + 1 > EndPtr) { - LOG_ERROR("Failed to find '", Name, "' in ResourceMap."); + LOG_ERROR_MESSAGE("Resource name pointer is outside of the specified byte code range. The byte code may be corrupted."); return false; } - if (Iter->second.Space != 0 && Iter->second.Space != ~0u) + auto Iter = ResourceMap.find(HashMapStringKey{Name}); + if (Iter == ResourceMap.end()) { - LOG_ERROR("Can not change space for resource '", Name, "' because shader is not compiled for SM 5.1"); + LOG_ERROR_MESSAGE("Failed to find '", Name, "' in the resource mapping."); return false; } Res.BindPoint = Iter->second.BindPoint; - } - return true; -} - -bool RemapShaderResourcesSM51(const DXBCUtils::TResourceBindingMap& ResourceMap, const void* EndPtr, ResourceDefChunkHeader* RDEFHeader) -{ - VERIFY_EXPR(RDEFHeader->Magic == RDEFFourCC); - VERIFY_EXPR(RDEFHeader->MajorVersion == 5 && RDEFHeader->MinorVersion == 1); - - auto* Ptr = reinterpret_cast(RDEFHeader) + sizeof(ChunkHeader); - auto* ResBinding = reinterpret_cast(Ptr + RDEFHeader->ResBindingOffset); - VERIFY((ResBinding + RDEFHeader->ResBindingCount) <= EndPtr, "Resource bindings is outside of buffer range."); - - for (Uint32 r = 0; r < RDEFHeader->ResBindingCount; ++r) - { - auto& Res = ResBinding[r]; - const char* Name = Ptr + Res.NameOffset; - VERIFY(Name < EndPtr, "Resource name pointer is outside of buffer range."); - - auto Iter = ResourceMap.find(HashMapStringKey{Name}); - if (Iter == ResourceMap.end()) + if (!PatchSpace(Res, Iter->second.Space)) { - LOG_ERROR("Failed to find '", Name, "' in ResourceMap."); + LOG_ERROR_MESSAGE("Can not change space for resource '", Name, "' because the shader was not compiled for SM 5.1."); return false; } - - Res.BindPoint = Iter->second.BindPoint; - Res.Space = Iter->second.Space; } return true; } + } // namespace bool DXBCUtils::RemapDXBCResources(const TResourceBindingMap& ResourceMap, - ID3DBlob* pBytecode) + void* pBytecode, + size_t Size) { if (pBytecode == nullptr) { - LOG_ERROR("pBytecode must not be null."); + LOG_ERROR_MESSAGE("pBytecode must not be null."); return false; } - char* const Ptr = static_cast(pBytecode->GetBufferPointer()); - const auto Size = pBytecode->GetBufferSize(); + auto* const Ptr = static_cast(pBytecode); const void* const EndPtr = Ptr + Size; if (Size < sizeof(DXBCHeader)) { - LOG_ERROR("Size of bytecode is too small."); + LOG_ERROR_MESSAGE("The size of the byte code (", Size, ") is too small to contain the DXBC header. The byte code may be corrupted."); return false; } auto& Header = *reinterpret_cast(Ptr); - VERIFY_EXPR(Header.TotalSize == Size); + if (Header.TotalSize != Size) + { + LOG_ERROR_MESSAGE("The byte code size (", Header.TotalSize, ") specified in the header does not match the actual size (", Size, + "). The byte code may be corrupted."); + return false; + } -#ifdef DILIGENT_DEBUG +#ifdef DILIGENT_DEVELOPMENT { DWORD Checksum[4] = {}; CalculateDXBCChecksum(reinterpret_cast(Ptr), static_cast(Size), Checksum); - VERIFY_EXPR(Checksum[0] == Header.Checksum[0]); - VERIFY_EXPR(Checksum[1] == Header.Checksum[1]); - VERIFY_EXPR(Checksum[2] == Header.Checksum[2]); - VERIFY_EXPR(Checksum[3] == Header.Checksum[3]); + DEV_CHECK_ERR(Checksum[0] == Header.Checksum[0] && + Checksum[1] == Header.Checksum[1] && + Checksum[2] == Header.Checksum[2] && + Checksum[3] == Header.Checksum[3], + "Unexpected checksum. The byte code may be corrupted or the container format may have changed."); } #endif if (Header.Magic != DXBCFourCC) { - LOG_ERROR("Bytecode header must contain 'DXBC' magic number."); + LOG_ERROR_MESSAGE("Bytecode header does not contain the 'DXBC' magic number. The byte code may be corrupted."); return false; } const Uint32* Chunks = reinterpret_cast(Ptr + sizeof(Header)); - bool Result = true; + bool RemappingOK = false; for (Uint32 i = 0; i < Header.ChunkCount; ++i) { - auto& Chunk = *reinterpret_cast(Ptr + Chunks[i]); - VERIFY((&Chunk + 1) <= EndPtr, "Pointer to chunk is outside of buffer range."); + auto* pChunk = reinterpret_cast(Ptr + Chunks[i]); + if (pChunk + 1 > EndPtr) + { + LOG_ERROR_MESSAGE("Not enough space for the chunk header. The byte code may be corrupted."); + return false; + } - if (Chunk.Magic == RDEFFourCC) + if (pChunk->Magic == RDEFFourCC) { - auto* RDEFHeader = reinterpret_cast(&Chunk); + auto* RDEFHeader = reinterpret_cast(pChunk); if (RDEFHeader->MajorVersion == 5 && RDEFHeader->MinorVersion == 1) - Result = RemapShaderResourcesSM51(ResourceMap, EndPtr, RDEFHeader); + { + RemappingOK = RemapShaderResources(ResourceMap, EndPtr, RDEFHeader); + } + else if (RDEFHeader->MajorVersion == 5 && RDEFHeader->MinorVersion == 0 || RDEFHeader->MajorVersion < 5) + { + RemappingOK = RemapShaderResources(ResourceMap, EndPtr, RDEFHeader); + } else - Result = RemapShaderResources(ResourceMap, EndPtr, RDEFHeader); + { + LOG_ERROR_MESSAGE("Unexpected shader model: ", RDEFHeader->MajorVersion, '.', RDEFHeader->MinorVersion); + RemappingOK = false; + } + + if (!RemappingOK) + return false; + break; } } - if (!Result) + if (!RemappingOK) { - LOG_ERROR("Failed to find chunk 'RDEF' with resource definition."); + LOG_ERROR_MESSAGE("Failed to find 'RDEF' chunk with the resource definition."); return false; } @@ -230,7 +247,7 @@ bool DXBCUtils::RemapDXBCResources(const TResourceBindingMap& ResourceMap, DWORD Checksum[4] = {}; CalculateDXBCChecksum(reinterpret_cast(Ptr), static_cast(Size), Checksum); - static_assert(sizeof(Header.Checksum) == sizeof(Checksum), ""); + static_assert(sizeof(Header.Checksum) == sizeof(Checksum), "Unexpected checksum size"); memcpy(Header.Checksum, Checksum, sizeof(Header.Checksum)); return true; -- cgit v1.2.3