From 58b1186362fe5f4ebe33ed5db01381fe246ebdd5 Mon Sep 17 00:00:00 2001 From: Steve Karolewics Date: Thu, 13 Feb 2020 22:03:27 -0800 Subject: [PATCH] Fix resource bindings when reflection info is stripped The logical identifiers, rather than shader registers, were used to populate reflection info, causing the wrong slots in large descriptor tables to be referenced for the pipeline state. --- .../driver/shaders/dxbc/dxbc_bytecode.cpp | 68 +++++++------------ .../d3d12/d3d12_resource_mapping_zoo.cpp | 23 +++---- 2 files changed, 37 insertions(+), 54 deletions(-) diff --git a/renderdoc/driver/shaders/dxbc/dxbc_bytecode.cpp b/renderdoc/driver/shaders/dxbc/dxbc_bytecode.cpp index 5e7b402ac..de5eb2e2c 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_bytecode.cpp +++ b/renderdoc/driver/shaders/dxbc/dxbc_bytecode.cpp @@ -45,6 +45,25 @@ Program::Program(const byte *bytes, size_t length) FetchTypeVersion(); } +void HandleResourceArrayIndices(const rdcarray &indices, + DXBC::ShaderInputBind &desc) +{ + // If there are 3 indices, we're using SM5.1 and this binding may be a resource array + if(indices.size() == 3) + { + // With SM5.1, the first index is the logical identifier, + // and the 2nd index is the starting shader register + desc.reg = (uint32_t)indices[1].index; + + // Start/end registers are inclusive, so one resource will have the same start/end register + desc.bindCount = uint32_t(indices[2].index - indices[1].index + 1); + + // If it's an unbounded resource array, mark the bind count as 0 + if(indices[2].index == 0xffffffff) + desc.bindCount = 0; + } +} + DXBC::Reflection *Program::GuessReflection() { DisassembleHexDump(); @@ -79,12 +98,7 @@ DXBC::Reflection *Program::GuessReflection() desc.dimension = DXBC::ShaderInputBind::DIM_UNKNOWN; desc.numSamples = 0; - if(dcl.operand.indices.size() == 3) - { - desc.bindCount = uint32_t(dcl.operand.indices[2].index - dcl.operand.indices[1].index); - if(dcl.operand.indices[2].index == 0xffffffff) - desc.bindCount = 0; - } + HandleResourceArrayIndices(dcl.operand.indices, desc); ret->Samplers.push_back(desc); @@ -148,12 +162,7 @@ DXBC::Reflection *Program::GuessReflection() RDCASSERT(desc.dimension != DXBC::ShaderInputBind::DIM_UNKNOWN); - if(dcl.operand.indices.size() == 3) - { - desc.bindCount = uint32_t(dcl.operand.indices[2].index - dcl.operand.indices[1].index); - if(dcl.operand.indices[2].index == 0xffffffff) - desc.bindCount = 0; - } + HandleResourceArrayIndices(dcl.operand.indices, desc); ret->SRVs.push_back(desc); @@ -184,12 +193,7 @@ DXBC::Reflection *Program::GuessReflection() desc.dimension = DXBC::ShaderInputBind::DIM_BUFFER; desc.numSamples = 0; - if(dcl.operand.indices.size() == 3) - { - desc.bindCount = uint32_t(dcl.operand.indices[2].index - dcl.operand.indices[1].index); - if(dcl.operand.indices[2].index == 0xffffffff) - desc.bindCount = 0; - } + HandleResourceArrayIndices(dcl.operand.indices, desc); if(dcl.operand.type == TYPE_RESOURCE) ret->SRVs.push_back(desc); @@ -218,12 +222,7 @@ DXBC::Reflection *Program::GuessReflection() desc.dimension = DXBC::ShaderInputBind::DIM_BUFFER; desc.numSamples = dcl.stride; - if(dcl.operand.indices.size() == 3) - { - desc.bindCount = uint32_t(dcl.operand.indices[2].index - dcl.operand.indices[1].index); - if(dcl.operand.indices[2].index == 0xffffffff) - desc.bindCount = 0; - } + HandleResourceArrayIndices(dcl.operand.indices, desc); ret->SRVs.push_back(desc); @@ -254,12 +253,7 @@ DXBC::Reflection *Program::GuessReflection() desc.dimension = DXBC::ShaderInputBind::DIM_BUFFER; desc.numSamples = dcl.stride; - if(dcl.operand.indices.size() == 3) - { - desc.bindCount = uint32_t(dcl.operand.indices[2].index - dcl.operand.indices[1].index); - if(dcl.operand.indices[2].index == 0xffffffff) - desc.bindCount = 0; - } + HandleResourceArrayIndices(dcl.operand.indices, desc); ret->UAVs.push_back(desc); @@ -317,12 +311,7 @@ DXBC::Reflection *Program::GuessReflection() } desc.numSamples = (uint32_t)-1; - if(dcl.operand.indices.size() == 3) - { - desc.bindCount = uint32_t(dcl.operand.indices[2].index - dcl.operand.indices[1].index); - if(dcl.operand.indices[2].index == 0xffffffff) - desc.bindCount = 0; - } + HandleResourceArrayIndices(dcl.operand.indices, desc); ret->UAVs.push_back(desc); @@ -355,12 +344,7 @@ DXBC::Reflection *Program::GuessReflection() desc.dimension = DXBC::ShaderInputBind::DIM_UNKNOWN; desc.numSamples = 0; - if(dcl.operand.indices.size() == 3) - { - desc.bindCount = uint32_t(dcl.operand.indices[2].index - dcl.operand.indices[1].index + 1); - if(dcl.operand.indices[2].index == 0xffffffff) - desc.bindCount = 0; - } + HandleResourceArrayIndices(dcl.operand.indices, desc); DXBC::CBuffer cb; diff --git a/util/test/demos/d3d12/d3d12_resource_mapping_zoo.cpp b/util/test/demos/d3d12/d3d12_resource_mapping_zoo.cpp index d0e1a39ef..126c7d80d 100644 --- a/util/test/demos/d3d12/d3d12_resource_mapping_zoo.cpp +++ b/util/test/demos/d3d12/d3d12_resource_mapping_zoo.cpp @@ -52,9 +52,8 @@ float4 main() : SV_Target0 std::string pixel_5_1 = R"EOSHADER( -// TODO: Once SRV mappings with 5.1 are fixed, change these registers to test gaps in registers -Texture2D res1 : register(t0); -Texture2D res2 : register(t1); +Texture2D res1 : register(t6); +Texture2D res2 : register(t7); // TODO: Add UAV writes and test gaps in those mappings @@ -108,14 +107,14 @@ float4 main() : SV_Target0 cbufferarray[x][y].col = Vec4f(x / 1.0f, y / 1.0f, 0.5f, 0.5f); ID3D12ResourcePtr cbArray = MakeBuffer().Data(cbufferarray).Size(sizeof(AlignedCB) * 12); for(uint32_t i = 0; i < 12; ++i) - MakeCBV(cbArray).SizeBytes(256).Offset(i * sizeof(AlignedCB)).CreateGPU(3 + i); + MakeCBV(cbArray).SizeBytes(256).Offset(i * sizeof(AlignedCB)).CreateGPU(i); ID3D12ResourcePtr res1 = MakeTexture(DXGI_FORMAT_R8G8B8A8_UNORM, 2, 2).Mips(1).InitialState(D3D12_RESOURCE_STATE_COPY_DEST); - MakeSRV(res1).CreateGPU(0); + MakeSRV(res1).CreateGPU(18); ID3D12ResourcePtr res2 = MakeTexture(DXGI_FORMAT_R8G8B8A8_UNORM, 2, 2).Mips(1).InitialState(D3D12_RESOURCE_STATE_COPY_DEST); - MakeSRV(res2).CreateGPU(2); + MakeSRV(res2).CreateGPU(19); ID3D12ResourcePtr uploadBuf = MakeBuffer().Size(1024 * 1024).Upload(); { @@ -209,16 +208,17 @@ float4 main() : SV_Target0 GPUSync(); } + // Test the same resource mappings both with explicitly specified resources, + // and a bindless style table param ID3D12RootSignaturePtr sig_5_0 = MakeSig({ cbvParam(D3D12_SHADER_VISIBILITY_PIXEL, 0, 3), - tableParam(D3D12_SHADER_VISIBILITY_PIXEL, D3D12_DESCRIPTOR_RANGE_TYPE_SRV, 0, 0, 1, 0), - tableParam(D3D12_SHADER_VISIBILITY_PIXEL, D3D12_DESCRIPTOR_RANGE_TYPE_SRV, 0, 2, 1, 2), + tableParam(D3D12_SHADER_VISIBILITY_PIXEL, D3D12_DESCRIPTOR_RANGE_TYPE_SRV, 0, 0, 1, 18), + tableParam(D3D12_SHADER_VISIBILITY_PIXEL, D3D12_DESCRIPTOR_RANGE_TYPE_SRV, 0, 2, 1, 19), }); ID3D12RootSignaturePtr sig_5_1 = MakeSig({ cbvParam(D3D12_SHADER_VISIBILITY_PIXEL, 0, 3), - tableParam(D3D12_SHADER_VISIBILITY_PIXEL, D3D12_DESCRIPTOR_RANGE_TYPE_SRV, 0, 0, 1, 0), - tableParam(D3D12_SHADER_VISIBILITY_PIXEL, D3D12_DESCRIPTOR_RANGE_TYPE_SRV, 0, 1, 1, 2), - tableParam(D3D12_SHADER_VISIBILITY_PIXEL, D3D12_DESCRIPTOR_RANGE_TYPE_CBV, 0, 4, 12, 3), + tableParam(D3D12_SHADER_VISIBILITY_PIXEL, D3D12_DESCRIPTOR_RANGE_TYPE_CBV, 0, 4, 12, 0), + tableParam(D3D12_SHADER_VISIBILITY_PIXEL, D3D12_DESCRIPTOR_RANGE_TYPE_SRV, 0, 0, UINT_MAX, 12), }); ID3D12PipelineStatePtr pso_5_0 = MakePSO() @@ -283,7 +283,6 @@ float4 main() : SV_Target0 cmd->SetGraphicsRootConstantBufferView(0, cb->GetGPUVirtualAddress()); cmd->SetGraphicsRootDescriptorTable(1, m_CBVUAVSRV->GetGPUDescriptorHandleForHeapStart()); cmd->SetGraphicsRootDescriptorTable(2, m_CBVUAVSRV->GetGPUDescriptorHandleForHeapStart()); - cmd->SetGraphicsRootDescriptorTable(3, m_CBVUAVSRV->GetGPUDescriptorHandleForHeapStart()); cmd->DrawInstanced(3, 1, 0, 0); FinishUsingBackbuffer(cmd, D3D12_RESOURCE_STATE_RENDER_TARGET);