From 67447b7b7e3344835d34a99e42e4f75564dd04e3 Mon Sep 17 00:00:00 2001 From: Steve Karolewics Date: Sat, 14 Mar 2020 12:29:43 -0700 Subject: [PATCH] Improve handling of resource arrays for D3D12 When getting read only resources from the pipe state, stitch back up according to the bindpoint mappings. When displaying resources in UI, don't traverse unbounded arrays. Fix resource swizzle on load/sample/ gather instructions, which happens on fetch result, not on the source operand. Added more tests for unbounded arrays and different ways to index into arrays. --- qrenderdoc/Windows/ShaderViewer.cpp | 26 ++++++++- renderdoc/api/replay/pipestate.inl | 44 ++++++++------ renderdoc/driver/shaders/dxbc/dxbc_debug.cpp | 54 ++++++++++++++--- .../d3d12/d3d12_resource_mapping_zoo.cpp | 58 +++++++++++++++++-- util/test/demos/d3d12/d3d12_test.cpp | 10 ++-- .../tests/D3D12/D3D12_Resource_Mapping_Zoo.py | 11 ++++ 6 files changed, 166 insertions(+), 37 deletions(-) diff --git a/qrenderdoc/Windows/ShaderViewer.cpp b/qrenderdoc/Windows/ShaderViewer.cpp index 9b6f5937f..a4ce224a1 100644 --- a/qrenderdoc/Windows/ShaderViewer.cpp +++ b/qrenderdoc/Windows/ShaderViewer.cpp @@ -2269,6 +2269,14 @@ void ShaderViewer::updateDebugState() VariableTag(DebugVariableReference(DebugVariableType::ReadOnlyResource, ro.name)))); ui->constants->addTopLevelItem(node); } + else if(bind.arraySize == ~0U) + { + RDTreeWidgetItem *node = new RDTreeWidgetItem( + {m_ShaderDetails->readOnlyResources[i].name, ro.name, lit("[unbounded]"), QString()}); + node->setTag(QVariant::fromValue( + VariableTag(DebugVariableReference(DebugVariableType::ReadOnlyResource, ro.name)))); + ui->constants->addTopLevelItem(node); + } else { RDTreeWidgetItem *node = @@ -2325,7 +2333,15 @@ void ShaderViewer::updateDebugState() new RDTreeWidgetItem({m_ShaderDetails->readWriteResources[i].name, rw.name, lit("Resource"), ToQStr(rwBind.resources[0].resourceId)}); node->setTag(QVariant::fromValue( - VariableTag(DebugVariableReference(DebugVariableType::ReadOnlyResource, rw.name)))); + VariableTag(DebugVariableReference(DebugVariableType::ReadWriteResource, rw.name)))); + ui->constants->addTopLevelItem(node); + } + else if(bind.arraySize == ~0U) + { + RDTreeWidgetItem *node = new RDTreeWidgetItem( + {m_ShaderDetails->readWriteResources[i].name, rw.name, lit("[unbounded]"), QString()}); + node->setTag(QVariant::fromValue( + VariableTag(DebugVariableReference(DebugVariableType::ReadWriteResource, rw.name)))); ui->constants->addTopLevelItem(node); } else @@ -2334,7 +2350,7 @@ void ShaderViewer::updateDebugState() new RDTreeWidgetItem({m_ShaderDetails->readWriteResources[i].name, rw.name, QFormatStr("[%1]").arg(bind.arraySize), QString()}); node->setTag(QVariant::fromValue( - VariableTag(DebugVariableReference(DebugVariableType::ReadOnlyResource, rw.name)))); + VariableTag(DebugVariableReference(DebugVariableType::ReadWriteResource, rw.name)))); for(uint32_t a = 0; a < bind.arraySize; a++) { @@ -2764,6 +2780,12 @@ RDTreeWidgetItem *ShaderViewer::makeSourceVariableNode(const SourceVariableMappi { value = ToQStr(res.resources[0].resourceId); } + else if(bind.arraySize == ~0U) + { + regNames = QString(); + typeName = lit("[unbounded]"); + value = QString(); + } else { for(uint32_t a = 0; a < bind.arraySize; a++) diff --git a/renderdoc/api/replay/pipestate.inl b/renderdoc/api/replay/pipestate.inl index cc311be9a..2bf1c8de5 100644 --- a/renderdoc/api/replay/pipestate.inl +++ b/renderdoc/api/replay/pipestate.inl @@ -1034,31 +1034,37 @@ rdcarray PipeState::GetReadOnlyResources(ShaderStage stage) { const D3D12Pipe::Shader &s = GetD3D12Stage(stage); - size_t size = 0; - for(int space = 0; space < s.spaces.count(); space++) - size += s.spaces[space].srvs.size(); - + size_t size = s.bindpointMapping.readOnlyResources.size(); ret.reserve(size); - for(int space = 0; space < s.spaces.count(); space++) + for(size_t bp = 0; bp < size; bp++) { - for(int reg = 0; reg < s.spaces[space].srvs.count(); reg++) + const Bindpoint &bind = s.bindpointMapping.readOnlyResources[bp]; + ret.push_back(BoundResourceArray()); + ret.back().bindPoint = bind; + + uint32_t count = bind.arraySize == ~0U ? 1 : bind.arraySize; + rdcarray &val = ret.back().resources; + val.resize(count); + + int spaceIndex = 0; + for(int space = 0; space < s.spaces.count(); space++) { - const D3D12Pipe::View &bind = s.spaces[space].srvs[reg]; - Bindpoint key(s.spaces[space].spaceIndex, reg); - BoundResource val; + if(s.spaces[space].spaceIndex == (uint32_t)bind.bindset) + { + spaceIndex = space; + break; + } + } - // consider this register to not exist - it's in a gap defined by sparse root signature - // elements - if(bind.rootElement == ~0U) - continue; + for(uint32_t i = 0; i < count; i++) + { + const D3D12Pipe::View &view = s.spaces[spaceIndex].srvs[bind.bind + i]; - val.resourceId = bind.resourceId; - val.firstMip = (int)bind.firstMip; - val.firstSlice = (int)bind.firstSlice; - val.typeCast = bind.viewFormat.compType; - - ret.push_back(BoundResourceArray(key, {val})); + val[i].resourceId = view.resourceId; + val[i].firstMip = (int)view.firstMip; + val[i].firstSlice = (int)view.firstSlice; + val[i].typeCast = view.viewFormat.compType; } } diff --git a/renderdoc/driver/shaders/dxbc/dxbc_debug.cpp b/renderdoc/driver/shaders/dxbc/dxbc_debug.cpp index 93660bf96..751af81bf 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_debug.cpp +++ b/renderdoc/driver/shaders/dxbc/dxbc_debug.cpp @@ -445,6 +445,38 @@ bool OperationFlushing(const DXBCBytecode::OpcodeType &op) return false; } +bool OperandSwizzle(const Operation &op, const Operand &oper) +{ + switch(op.operation) + { + case OPCODE_SAMPLE: + case OPCODE_SAMPLE_L: + case OPCODE_SAMPLE_B: + case OPCODE_SAMPLE_D: + case OPCODE_SAMPLE_C: + case OPCODE_SAMPLE_C_LZ: + case OPCODE_LD: + case OPCODE_LD_MS: + case OPCODE_GATHER4: + case OPCODE_GATHER4_C: + case OPCODE_GATHER4_PO: + case OPCODE_GATHER4_PO_C: + case OPCODE_LOD: + { + // Per HLSL docs, "the swizzle on srcResource determines how to swizzle the 4-component result + // coming back from the texture sample/filter". That swizzle should not be handled when + // fetching the src operand + if(oper.type == TYPE_RESOURCE) + return false; + } + break; + + default: break; + } + + return true; +} + void DoubleSet(ShaderVariable &var, const double in[2]) { var.value.d.x = in[0]; @@ -1623,16 +1655,24 @@ ShaderVariable ThreadState::GetSrc(const Operand &oper, const Operation &op, boo } } - // perform swizzling - v.value.uv[0] = s.value.uv[oper.comps[0] == 0xff ? 0 : oper.comps[0]]; - v.value.uv[1] = s.value.uv[oper.comps[1] == 0xff ? 1 : oper.comps[1]]; - v.value.uv[2] = s.value.uv[oper.comps[2] == 0xff ? 2 : oper.comps[2]]; - v.value.uv[3] = s.value.uv[oper.comps[3] == 0xff ? 3 : oper.comps[3]]; + if(OperandSwizzle(op, oper)) + { + // perform swizzling + v.value.uv[0] = s.value.uv[oper.comps[0] == 0xff ? 0 : oper.comps[0]]; + v.value.uv[1] = s.value.uv[oper.comps[1] == 0xff ? 1 : oper.comps[1]]; + v.value.uv[2] = s.value.uv[oper.comps[2] == 0xff ? 2 : oper.comps[2]]; + v.value.uv[3] = s.value.uv[oper.comps[3] == 0xff ? 3 : oper.comps[3]]; - if(oper.comps[0] != 0xff && oper.comps[1] == 0xff && oper.comps[2] == 0xff && oper.comps[3] == 0xff) - v.columns = 1; + if(oper.comps[0] != 0xff && oper.comps[1] == 0xff && oper.comps[2] == 0xff && + oper.comps[3] == 0xff) + v.columns = 1; + else + v.columns = 4; + } else + { v.columns = 4; + } if(oper.modifier == OPERAND_MODIFIER_ABS || oper.modifier == OPERAND_MODIFIER_ABSNEG) { diff --git a/util/test/demos/d3d12/d3d12_resource_mapping_zoo.cpp b/util/test/demos/d3d12/d3d12_resource_mapping_zoo.cpp index db6facb7e..cf12e60b7 100644 --- a/util/test/demos/d3d12/d3d12_resource_mapping_zoo.cpp +++ b/util/test/demos/d3d12/d3d12_resource_mapping_zoo.cpp @@ -81,11 +81,40 @@ float4 main() : SV_Target0 Texture2DArray resArray[4] : register(t10, space1); +cbuffer consts : register(b3) +{ + float4 test; +}; + float4 main(float4 pos : SV_Position) : SV_Target0 { + // Test resource array access with a constant, uniform, and non-uniform uint2 indices = ((uint2)pos.xy) % uint2(4, 4); - float arrayVal = resArray[NonUniformResourceIndex(indices.x)].Load(uint4(0, 0, indices.y, 0)); - return float4(arrayVal, arrayVal, arrayVal, 1.0f); + float arrayVal1 = resArray[1].Load(uint4(0, 0, indices.y, 0)); + float arrayVal2 = resArray[test.x].Load(uint4(0, 0, indices.y, 0)); + float arrayVal3 = resArray[NonUniformResourceIndex(indices.x)].Load(uint4(0, 0, indices.y, 0)); + return float4(arrayVal1, arrayVal2, arrayVal3, 1.0f); +} + +)EOSHADER"; + + std::string pixel_bindless = R"EOSHADER( + +Texture2DArray resArray[] : register(t0); + +cbuffer consts : register(b3) +{ + float4 test; +}; + +float4 main(float4 pos : SV_Position) : SV_Target0 +{ + // Test resource array access with a constant, uniform, and non-uniform + uint2 indices = ((uint2)pos.xy) % uint2(4, 4); + float arrayVal1 = resArray[1].Load(uint4(0, 0, indices.y, 0)); + float arrayVal2 = resArray[test.x].Load(uint4(0, 0, indices.y, 0)); + float arrayVal3 = resArray[NonUniformResourceIndex(indices.x)].Load(uint4(0, 0, indices.y, 0)); + return float4(arrayVal1, arrayVal2, arrayVal3, 1.0f); } )EOSHADER"; @@ -162,8 +191,9 @@ float4 main(float4 pos : SV_Position) : SV_Target0 ID3DBlobPtr psblob_5_0 = Compile(pixel_5_0, "main", "ps_5_0"); ID3DBlobPtr psblob_5_1 = Compile(pixel_5_1, "main", "ps_5_1"); ID3DBlobPtr psblob_resArray = Compile(pixel_resArray, "main", "ps_5_1"); + ID3DBlobPtr psblob_bindless = Compile(pixel_bindless, "main", "ps_5_1"); - Vec4f cbufferdata = Vec4f(25.0f, 50.0f, 75.0f, 100.0f); + Vec4f cbufferdata = Vec4f(3.0f, 50.0f, 75.0f, 100.0f); ID3D12ResourcePtr vb = MakeBuffer().Data(DefaultTri); ID3D12ResourcePtr cb = MakeBuffer().Data(&cbufferdata); @@ -221,8 +251,13 @@ float4 main(float4 pos : SV_Position) : SV_Target0 tableParam(D3D12_SHADER_VISIBILITY_PIXEL, D3D12_DESCRIPTOR_RANGE_TYPE_SRV, 0, 0, UINT_MAX, 50), }); ID3D12RootSignaturePtr sig_resArray = MakeSig({ + cbvParam(D3D12_SHADER_VISIBILITY_PIXEL, 0, 3), tableParam(D3D12_SHADER_VISIBILITY_PIXEL, D3D12_DESCRIPTOR_RANGE_TYPE_SRV, 1, 10, 4, 30), }); + ID3D12RootSignaturePtr sig_bindless = MakeSig({ + cbvParam(D3D12_SHADER_VISIBILITY_PIXEL, 0, 3), + tableParam(D3D12_SHADER_VISIBILITY_PIXEL, D3D12_DESCRIPTOR_RANGE_TYPE_SRV, 0, 0, UINT_MAX, 30), + }); ID3D12PipelineStatePtr pso_5_0 = MakePSO() .RootSig(sig_5_0) @@ -242,6 +277,12 @@ float4 main(float4 pos : SV_Position) : SV_Target0 .VS(vsblob) .PS(psblob_resArray) .RTVs({DXGI_FORMAT_R32G32B32A32_FLOAT}); + ID3D12PipelineStatePtr pso_bindless = MakePSO() + .RootSig(sig_bindless) + .InputLayout() + .VS(vsblob) + .PS(psblob_bindless) + .RTVs({DXGI_FORMAT_R32G32B32A32_FLOAT}); ResourceBarrier(vb, D3D12_RESOURCE_STATE_COMMON, D3D12_RESOURCE_STATE_VERTEX_AND_CONSTANT_BUFFER); ResourceBarrier(cb, D3D12_RESOURCE_STATE_COMMON, D3D12_RESOURCE_STATE_VERTEX_AND_CONSTANT_BUFFER); @@ -298,7 +339,16 @@ float4 main(float4 pos : SV_Position) : SV_Target0 cmd->SetPipelineState(pso_resArray); cmd->SetGraphicsRootSignature(sig_resArray); cmd->SetDescriptorHeaps(1, &m_CBVUAVSRV.GetInterfacePtr()); - cmd->SetGraphicsRootDescriptorTable(0, m_CBVUAVSRV->GetGPUDescriptorHandleForHeapStart()); + cmd->SetGraphicsRootConstantBufferView(0, cb->GetGPUVirtualAddress()); + cmd->SetGraphicsRootDescriptorTable(1, m_CBVUAVSRV->GetGPUDescriptorHandleForHeapStart()); + cmd->DrawInstanced(3, 1, 0, 0); + + setMarker(cmd, "Bindless"); + cmd->SetPipelineState(pso_bindless); + cmd->SetGraphicsRootSignature(sig_bindless); + cmd->SetDescriptorHeaps(1, &m_CBVUAVSRV.GetInterfacePtr()); + cmd->SetGraphicsRootConstantBufferView(0, cb->GetGPUVirtualAddress()); + cmd->SetGraphicsRootDescriptorTable(1, m_CBVUAVSRV->GetGPUDescriptorHandleForHeapStart()); cmd->DrawInstanced(3, 1, 0, 0); FinishUsingBackbuffer(cmd, D3D12_RESOURCE_STATE_RENDER_TARGET); diff --git a/util/test/demos/d3d12/d3d12_test.cpp b/util/test/demos/d3d12/d3d12_test.cpp index e0ec027e8..d1d838125 100644 --- a/util/test/demos/d3d12/d3d12_test.cpp +++ b/util/test/demos/d3d12/d3d12_test.cpp @@ -906,11 +906,11 @@ ID3DBlobPtr D3D12GraphicsTest::Compile(std::string src, std::string entry, std:: ID3DBlobPtr blob = NULL; ID3DBlobPtr error = NULL; - HRESULT hr = - dyn_D3DCompile(src.c_str(), src.length(), "", NULL, NULL, entry.c_str(), profile.c_str(), - D3DCOMPILE_WARNINGS_ARE_ERRORS | D3DCOMPILE_DEBUG | - D3DCOMPILE_SKIP_OPTIMIZATION | D3DCOMPILE_OPTIMIZATION_LEVEL0, - 0, &blob, &error); + HRESULT hr = dyn_D3DCompile( + src.c_str(), src.length(), "", NULL, NULL, entry.c_str(), profile.c_str(), + D3DCOMPILE_WARNINGS_ARE_ERRORS | D3DCOMPILE_DEBUG | D3DCOMPILE_SKIP_OPTIMIZATION | + D3DCOMPILE_OPTIMIZATION_LEVEL0 | D3DCOMPILE_ENABLE_UNBOUNDED_DESCRIPTOR_TABLES, + 0, &blob, &error); if(FAILED(hr)) { diff --git a/util/test/tests/D3D12/D3D12_Resource_Mapping_Zoo.py b/util/test/tests/D3D12/D3D12_Resource_Mapping_Zoo.py index aef921e90..5f767ce88 100644 --- a/util/test/tests/D3D12/D3D12_Resource_Mapping_Zoo.py +++ b/util/test/tests/D3D12/D3D12_Resource_Mapping_Zoo.py @@ -58,6 +58,17 @@ class D3D12_Resource_Mapping_Zoo(rdtest.TestCase): rdtest.log.end_section("Resource array tests") + rdtest.log.begin_section("Bindless tests") + test_marker: rd.DrawcallDescription = self.find_draw("Bindless") + draw = test_marker.next + self.controller.SetFrameEvent(draw.eventId, False) + + for y in range(4): + for x in range(4): + failed = not self.test_debug_pixel(200 + x, 200 + y, "Bindless({},{})".format(x, y)) or failed + + rdtest.log.end_section("Bindless tests") + if failed: raise rdtest.TestFailureException("Some tests were not as expected")