From f756cf363e78d97853f88bddc9eabbc73a640c8d Mon Sep 17 00:00:00 2001 From: baldurk Date: Wed, 19 Oct 2016 12:26:08 +0200 Subject: [PATCH] Remove legacy DXBC padding of cbuffers array. Also stub reg spaces * Historically there was no bindpoint mapping so it was convenient to just pad the cbuffers array so that the elements in it were indexed by their bind point. It doesn't make any sense anymore especially with D3D12's bind model, so just remove it and roll bind points into a struct member. --- renderdoc/api/replay/shader_types.h | 7 +- renderdoc/driver/d3d11/d3d11_common.cpp | 4 +- renderdoc/driver/d3d11/d3d11_renderstate.cpp | 4 +- renderdoc/driver/d3d12/d3d12_common.cpp | 8 ++- .../driver/shaders/dxbc/dxbc_inspect.cpp | 68 ++++++++++--------- renderdoc/driver/shaders/dxbc/dxbc_inspect.h | 6 +- .../PipelineState/D3D11PipelineStateViewer.cs | 45 +++++++++--- .../PipelineState/D3D12PipelineStateViewer.cs | 9 +-- 8 files changed, 93 insertions(+), 58 deletions(-) diff --git a/renderdoc/api/replay/shader_types.h b/renderdoc/api/replay/shader_types.h index 5153f6201..8d1861b7a 100644 --- a/renderdoc/api/replay/shader_types.h +++ b/renderdoc/api/replay/shader_types.h @@ -253,11 +253,10 @@ struct ShaderReflection rdctype::array InputSig; rdctype::array OutputSig; - rdctype::array ConstantBlocks; // sparse - index indicates bind point + rdctype::array ConstantBlocks; - rdctype::array ReadOnlyResources; // non-sparse, since bind points can overlap. - rdctype::array - ReadWriteResources; // non-sparse, since bind points can overlap. + rdctype::array ReadOnlyResources; + rdctype::array ReadWriteResources; // TODO expand this to encompass shader subroutines. rdctype::array Interfaces; diff --git a/renderdoc/driver/d3d11/d3d11_common.cpp b/renderdoc/driver/d3d11/d3d11_common.cpp index 0793994b8..031da29bc 100644 --- a/renderdoc/driver/d3d11/d3d11_common.cpp +++ b/renderdoc/driver/d3d11/d3d11_common.cpp @@ -153,7 +153,7 @@ ShaderReflection *MakeShaderReflection(DXBC::DXBCFile *dxbc) cb.name = dxbc->m_CBuffers[i].name; cb.bufferBacked = dxbc->m_CBuffers[i].descriptor.type == DXBC::CBuffer::Descriptor::TYPE_CBUFFER; cb.byteSize = dxbc->m_CBuffers[i].descriptor.byteSize; - cb.bindPoint = (uint32_t)i; + cb.bindPoint = dxbc->m_CBuffers[i].reg; create_array_uninit(cb.variables, dxbc->m_CBuffers[i].variables.size()); for(size_t v = 0; v < dxbc->m_CBuffers[i].variables.size(); v++) @@ -198,7 +198,7 @@ ShaderReflection *MakeShaderReflection(DXBC::DXBCFile *dxbc) continue; ShaderResource res; - res.bindPoint = r.bindPoint; + res.bindPoint = r.reg; res.name = r.name; res.IsSampler = (r.type == DXBC::ShaderInputBind::TYPE_SAMPLER); diff --git a/renderdoc/driver/d3d11/d3d11_renderstate.cpp b/renderdoc/driver/d3d11/d3d11_renderstate.cpp index 6840d1b1f..f2915369d 100644 --- a/renderdoc/driver/d3d11/d3d11_renderstate.cpp +++ b/renderdoc/driver/d3d11/d3d11_renderstate.cpp @@ -1514,7 +1514,7 @@ bool D3D11RenderState::shader::Used_SRV(uint32_t slot) const for(size_t i = 0; i < dxbc->m_Resources.size(); i++) { - if(dxbc->m_Resources[i].bindPoint == slot && + if(dxbc->m_Resources[i].reg == slot && (dxbc->m_Resources[i].type == DXBC::ShaderInputBind::TYPE_TEXTURE || dxbc->m_Resources[i].type == DXBC::ShaderInputBind::TYPE_STRUCTURED || dxbc->m_Resources[i].type == DXBC::ShaderInputBind::TYPE_TBUFFER || @@ -1542,7 +1542,7 @@ bool D3D11RenderState::shader::Used_UAV(uint32_t slot) const for(size_t i = 0; i < dxbc->m_Resources.size(); i++) { - if(dxbc->m_Resources[i].bindPoint == slot && + if(dxbc->m_Resources[i].reg == slot && (dxbc->m_Resources[i].type == DXBC::ShaderInputBind::TYPE_UAV_APPEND_STRUCTURED || dxbc->m_Resources[i].type == DXBC::ShaderInputBind::TYPE_UAV_CONSUME_STRUCTURED || dxbc->m_Resources[i].type == DXBC::ShaderInputBind::TYPE_UAV_RWBYTEADDRESS || diff --git a/renderdoc/driver/d3d12/d3d12_common.cpp b/renderdoc/driver/d3d12/d3d12_common.cpp index d57d0d058..1dd080547 100644 --- a/renderdoc/driver/d3d12/d3d12_common.cpp +++ b/renderdoc/driver/d3d12/d3d12_common.cpp @@ -168,11 +168,12 @@ void MakeShaderReflection(DXBC::DXBCFile *dxbc, ShaderReflection *refl, cb.name = dxbc->m_CBuffers[i].name; cb.bufferBacked = true; cb.byteSize = dxbc->m_CBuffers[i].descriptor.byteSize; - cb.bindPoint = (uint32_t)c; + cb.bindPoint = (uint32_t)i; BindpointMap map; map.arraySize = 1; - map.bind = (int32_t)i; + map.bindset = dxbc->m_CBuffers[i].space; + map.bind = dxbc->m_CBuffers[i].reg; map.used = true; mapping->ConstantBlocks[c] = map; @@ -307,7 +308,8 @@ void MakeShaderReflection(DXBC::DXBCFile *dxbc, ShaderReflection *refl, BindpointMap map; map.arraySize = 1; - map.bind = r.bindPoint; + map.bindset = r.space; + map.bind = r.reg; map.used = true; if(IsReadWrite) diff --git a/renderdoc/driver/shaders/dxbc/dxbc_inspect.cpp b/renderdoc/driver/shaders/dxbc/dxbc_inspect.cpp index 7cfdebb93..5600d5a80 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_inspect.cpp +++ b/renderdoc/driver/shaders/dxbc/dxbc_inspect.cpp @@ -597,15 +597,7 @@ DXBCFile::DXBCFile(const void *ByteCode, size_t ByteCodeLength) m_Resources.reserve(h->resources.count); - // we have to use this map to match up cbuffers to their bind point, as - // it's not guaranteed that the resources and cbuffers will come in the - // same order. However it's possible for two cbuffers to have the same - // name, so in that case we assume they will come in matching order - // and just append _ to subsequent cbuffers with the same name. - map cbufferSlots; - uint32_t maxCBufferSlot = 0; - - set cbuffernames; + map > cbufferslots; for(int32_t i = 0; i < h->resources.count; i++) { @@ -616,7 +608,8 @@ DXBCFile::DXBCFile(const void *ByteCode, size_t ByteCodeLength) desc.name = chunkContents + res->nameOffset; desc.type = (ShaderInputBind::InputType)res->type; - desc.bindPoint = res->bindPoint; + desc.space = 0; + desc.reg = res->bindPoint; desc.bindCount = res->bindCount; desc.flags = res->flags; desc.retType = (ShaderInputBind::RetType)res->retType; @@ -631,17 +624,17 @@ DXBCFile::DXBCFile(const void *ByteCode, size_t ByteCodeLength) desc.numSamples = 1 + ((desc.flags & 0xC) >> 2); } + // for cbuffers the names can be duplicated, so handle this by assuming + // the order will match between binding declaration and cbuffer declaration + // and append _s onto each subsequent buffer name if(desc.type == ShaderInputBind::TYPE_CBUFFER) { string cname = desc.name; - while(cbuffernames.find(cname) != cbuffernames.end()) + while(cbufferslots.find(cname) != cbufferslots.end()) cname += "_"; - cbuffernames.insert(cname); - - cbufferSlots[cname] = desc.bindPoint; - maxCBufferSlot = RDCMAX(maxCBufferSlot, desc.bindPoint); + cbufferslots[cname] = std::make_pair(0, desc.reg); } m_Resources.push_back(desc); @@ -669,7 +662,7 @@ DXBCFile::DXBCFile(const void *ByteCode, size_t ByteCodeLength) { desc.name = StringFormat::Fmt("%s[%u]", rname.c_str(), a); m_Resources.push_back(desc); - desc.bindPoint++; + desc.reg++; } // continue from the i'th element again since @@ -680,10 +673,7 @@ DXBCFile::DXBCFile(const void *ByteCode, size_t ByteCodeLength) i++; } - cbuffernames.clear(); - - if(h->cbuffers.count > 0) - m_CBuffers.resize(maxCBufferSlot + 1); + set cbuffernames; for(int32_t i = 0; i < h->cbuffers.count; i++) { @@ -771,10 +761,12 @@ DXBCFile::DXBCFile(const void *ByteCode, size_t ByteCodeLength) cbuffernames.insert(cname); + cb.space = cbufferslots[cname].first; + cb.reg = cbufferslots[cname].second; + if(cb.descriptor.type == CBuffer::Descriptor::TYPE_CBUFFER) { - RDCASSERT(cbufferSlots.find(cname) != cbufferSlots.end()); - m_CBuffers[cbufferSlots[cname]] = cb; + m_CBuffers.push_back(cb); } else if(cb.descriptor.type == CBuffer::Descriptor::TYPE_RESOURCE_BIND_INFO) { @@ -1026,7 +1018,8 @@ void DXBCFile::GuessResources() desc.name = buf; desc.type = ShaderInputBind::TYPE_SAMPLER; - desc.bindPoint = idx; + desc.space = 0; + desc.reg = idx; desc.bindCount = 1; desc.flags = dcl.samplerMode == SAMPLER_MODE_COMPARISON ? 2 : 0; desc.retType = ShaderInputBind::RETTYPE_UNKNOWN; @@ -1051,7 +1044,8 @@ void DXBCFile::GuessResources() desc.name = buf; desc.type = ShaderInputBind::TYPE_TEXTURE; - desc.bindPoint = idx; + desc.space = 0; + desc.reg = idx; desc.bindCount = 1; desc.flags = 0; desc.retType = (ShaderInputBind::RetType)dcl.resType[0]; @@ -1105,7 +1099,8 @@ void DXBCFile::GuessResources() desc.name = buf; desc.type = dcl.operand.type == TYPE_RESOURCE ? ShaderInputBind::TYPE_BYTEADDRESS : ShaderInputBind::TYPE_UAV_RWBYTEADDRESS; - desc.bindPoint = idx; + desc.space = 0; + desc.reg = idx; desc.bindCount = 1; desc.flags = 0; desc.retType = ShaderInputBind::RETTYPE_MIXED; @@ -1130,7 +1125,8 @@ void DXBCFile::GuessResources() desc.name = buf; desc.type = ShaderInputBind::TYPE_STRUCTURED; - desc.bindPoint = idx; + desc.space = 0; + desc.reg = idx; desc.bindCount = 1; desc.flags = 0; desc.retType = ShaderInputBind::RETTYPE_MIXED; @@ -1159,7 +1155,8 @@ void DXBCFile::GuessResources() // rwstructured if(dcl.hasCounter) desc.type = ShaderInputBind::TYPE_UAV_RWSTRUCTURED_WITH_COUNTER; - desc.bindPoint = idx; + desc.space = 0; + desc.reg = idx; desc.bindCount = 1; desc.flags = 0; desc.retType = ShaderInputBind::RETTYPE_MIXED; @@ -1184,7 +1181,8 @@ void DXBCFile::GuessResources() desc.name = buf; desc.type = ShaderInputBind::TYPE_UAV_RWTYPED; - desc.bindPoint = idx; + desc.space = 0; + desc.reg = idx; desc.bindCount = 1; desc.flags = 0; desc.retType = (ShaderInputBind::RetType) int(dcl.resType[0]); // enums match @@ -1229,7 +1227,8 @@ void DXBCFile::GuessResources() desc.name = buf; desc.type = ShaderInputBind::TYPE_CBUFFER; - desc.bindPoint = idx; + desc.space = 0; + desc.reg = idx; desc.bindCount = 1; desc.flags = 1; desc.retType = ShaderInputBind::RETTYPE_UNKNOWN; @@ -1242,6 +1241,9 @@ void DXBCFile::GuessResources() cb.name = desc.name; + cb.space = 0; + cb.reg = idx; + cb.descriptor.name = cb.name; cb.descriptor.byteSize = numVecs * 4 * sizeof(float); cb.descriptor.type = CBuffer::Descriptor::TYPE_CBUFFER; @@ -1254,7 +1256,10 @@ void DXBCFile::GuessResources() { CBufferVariable var; - StringFormat::snprintf(buf, 63, "cb%u_v%u", desc.bindPoint, v); + if(desc.space > 0) + StringFormat::snprintf(buf, 63, "cb%u_%u_v%u", desc.space, desc.reg, v); + else + StringFormat::snprintf(buf, 63, "cb%u_v%u", desc.reg, v); var.name = buf; @@ -1281,8 +1286,7 @@ void DXBCFile::GuessResources() cb.variables.push_back(var); } - m_CBuffers.resize(RDCMAX((uint32_t)m_CBuffers.size(), desc.bindPoint + 1)); - m_CBuffers[desc.bindPoint] = cb; + m_CBuffers.push_back(cb); break; } diff --git a/renderdoc/driver/shaders/dxbc/dxbc_inspect.h b/renderdoc/driver/shaders/dxbc/dxbc_inspect.h index ca23638f4..2f399d1e1 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_inspect.h +++ b/renderdoc/driver/shaders/dxbc/dxbc_inspect.h @@ -131,7 +131,8 @@ struct ShaderInputBind TYPE_UAV_RWSTRUCTURED_WITH_COUNTER, } type; - uint32_t bindPoint; + uint32_t space; + uint32_t reg; uint32_t bindCount; uint32_t flags; @@ -283,6 +284,9 @@ struct CBuffer { string name; + uint32_t space; + uint32_t reg; + struct Descriptor { string name; diff --git a/renderdocui/Windows/PipelineState/D3D11PipelineStateViewer.cs b/renderdocui/Windows/PipelineState/D3D11PipelineStateViewer.cs index 0fd344e2d..54f0b1617 100644 --- a/renderdocui/Windows/PipelineState/D3D11PipelineStateViewer.cs +++ b/renderdocui/Windows/PipelineState/D3D11PipelineStateViewer.cs @@ -595,8 +595,17 @@ namespace renderdocui.Windows.PipelineState { ConstantBlock shaderCBuf = null; - if (shaderDetails != null && i < shaderDetails.ConstantBlocks.Length && shaderDetails.ConstantBlocks[i].name.Length > 0) - shaderCBuf = shaderDetails.ConstantBlocks[i]; + if (shaderDetails != null) + { + foreach (var cb in shaderDetails.ConstantBlocks) + { + if (cb.bindPoint == i) + { + shaderCBuf = cb; + break; + } + } + } bool filledSlot = (b.Buffer != ResourceId.Null); bool usedSlot = (shaderCBuf != null); @@ -2434,7 +2443,7 @@ namespace renderdocui.Windows.PipelineState { if (cbuf.name.Length > 0 && cbuf.variables.Length > 0) { - cbuffers += String.Format("cbuffer {0} : register(b{1}) {{", cbuf.name, cbufIdx) + nl; + cbuffers += String.Format("cbuffer {0} : register(b{1}) {{", cbuf.name, cbuf.bindPoint) + nl; MakeShaderVariablesHLSL(true, cbuf.variables, ref cbuffers, ref hlsl); cbuffers += "};" + nl2; } @@ -2600,20 +2609,36 @@ namespace renderdocui.Windows.PipelineState sv.Show(m_DockContent.DockPanel); } - private void ShowCBuffer(D3D11PipelineState.ShaderStage stage, UInt32 slot) + private void ShowCBuffer(D3D11PipelineState.ShaderStage stage, UInt32 bindPoint) { - if (stage.ShaderDetails != null && - (stage.ShaderDetails.ConstantBlocks.Length <= slot || - stage.ShaderDetails.ConstantBlocks[slot].name.Length == 0) - ) + bool found = false; + UInt32 slot = UInt32.MaxValue; + + if (stage.ShaderDetails != null) + { + UInt32 i = 0; + foreach (var cb in stage.ShaderDetails.ConstantBlocks) + { + if (cb.bindPoint == bindPoint) + { + slot = i; + found = true; + break; + } + + i++; + } + } + + if (!found) { // unused cbuffer, open regular buffer viewer var viewer = new BufferViewer(m_Core, false); - if (stage.ConstantBuffers.Length < slot) + if (stage.ConstantBuffers.Length < bindPoint) return; - var buf = stage.ConstantBuffers[slot]; + var buf = stage.ConstantBuffers[bindPoint]; viewer.ViewRawBuffer(true, buf.VecOffset * 4 * sizeof(float), buf.VecCount * 4 * sizeof(float), buf.Buffer); viewer.Show(m_DockContent.DockPanel); diff --git a/renderdocui/Windows/PipelineState/D3D12PipelineStateViewer.cs b/renderdocui/Windows/PipelineState/D3D12PipelineStateViewer.cs index 20d10d030..9a5b9c196 100644 --- a/renderdocui/Windows/PipelineState/D3D12PipelineStateViewer.cs +++ b/renderdocui/Windows/PipelineState/D3D12PipelineStateViewer.cs @@ -2075,16 +2075,17 @@ namespace renderdocui.Windows.PipelineState string cbuffers = ""; - int cbufIdx = 0; - foreach (var cbuf in shaderDetails.ConstantBlocks) + for (int i = 0; i < stage.BindpointMapping.ConstantBlocks.Length; i++) { + var bd = stage.BindpointMapping.ConstantBlocks[i]; + var cbuf = stage.ShaderDetails.ConstantBlocks[i]; + if (cbuf.name.Length > 0 && cbuf.variables.Length > 0) { - cbuffers += String.Format("cbuffer {0} : register(b{1}) {{", cbuf.name, cbufIdx) + nl; + cbuffers += String.Format("cbuffer {0} : register(space{1}, b{2}) {{", cbuf.name, bd.bindset, bd.bind) + nl; MakeShaderVariablesHLSL(true, cbuf.variables, ref cbuffers, ref hlsl); cbuffers += "};" + nl2; } - cbufIdx++; } hlsl += cbuffers + nl2;