From 5d51b524c677f30fb62f0eeb08d2e24af360d20d Mon Sep 17 00:00:00 2001 From: baldurk Date: Fri, 22 Jul 2022 19:07:00 +0100 Subject: [PATCH] Fix interpreting and declaring buffer formats that include pointers --- qrenderdoc/Code/BufferFormatter.cpp | 65 ++++++++++++------- qrenderdoc/Code/QRDUtils.cpp | 6 +- qrenderdoc/Code/QRDUtils.h | 18 +++-- qrenderdoc/Windows/BufferViewer.cpp | 11 ++-- .../D3D11PipelineStateViewer.cpp | 4 +- .../D3D12PipelineStateViewer.cpp | 4 +- .../PipelineState/GLPipelineStateViewer.cpp | 5 +- .../VulkanPipelineStateViewer.cpp | 3 +- 8 files changed, 73 insertions(+), 43 deletions(-) diff --git a/qrenderdoc/Code/BufferFormatter.cpp b/qrenderdoc/Code/BufferFormatter.cpp index 24174b655..a8552d65b 100644 --- a/qrenderdoc/Code/BufferFormatter.cpp +++ b/qrenderdoc/Code/BufferFormatter.cpp @@ -161,7 +161,8 @@ static bool MatchBaseTypeDeclaration(QString basetype, const bool isUnsigned, Sh return true; } -void BufferFormatter::EstimatePackingRules(Packing::Rules &pack, const ShaderConstant &constant) +void BufferFormatter::EstimatePackingRules(Packing::Rules &pack, ResourceId shader, + const ShaderConstant &constant) { // see if this constant violates any of the packing rules we are currently checking for. // We can't *prove* a rule is followed just from one example, we can only see if it is never @@ -230,16 +231,24 @@ void BufferFormatter::EstimatePackingRules(Packing::Rules &pack, const ShaderCon pack.tight_arrays = true; } - EstimatePackingRules(pack, constant.type.members); + EstimatePackingRules(pack, shader, constant.type.members); } -void BufferFormatter::EstimatePackingRules(Packing::Rules &pack, +void BufferFormatter::EstimatePackingRules(Packing::Rules &pack, ResourceId shader, const rdcarray &members) { for(size_t i = 0; i < members.size(); i++) { // check this constant - EstimatePackingRules(pack, members[i]); + EstimatePackingRules(pack, shader, members[i]); + + // when pointers are in use, follow the type and estimate with those too + if(members[i].type.pointerTypeID != ~0U) + { + const ShaderConstantType &ptrType = + PointerTypeRegistry::GetTypeDescriptor(shader, members[i].type.pointerTypeID); + EstimatePackingRules(pack, shader, ptrType.members); + } // check for trailing array/struct use if(i > 0) @@ -262,7 +271,8 @@ void BufferFormatter::EstimatePackingRules(Packing::Rules &pack, } } -Packing::Rules BufferFormatter::EstimatePackingRules(const rdcarray &members) +Packing::Rules BufferFormatter::EstimatePackingRules(ResourceId shader, + const rdcarray &members) { Packing::Rules pack; @@ -277,7 +287,7 @@ Packing::Rules BufferFormatter::EstimatePackingRules(const rdcarray declaredStructs; QMap anonStructs; - format = DeclareStruct(pack, declaredStructs, anonStructs, structName, res.variableType.members, - 0, QString()); + format = DeclareStruct(pack, shader, declaredStructs, anonStructs, structName, + res.variableType.members, 0, QString()); format = QFormatStr("%1\n\n%2").arg(DeclarePacking(pack)).arg(format); } else @@ -2577,7 +2588,8 @@ uint32_t BufferFormatter::GetUnpaddedStructAdvance(Packing::Rules pack, return lastMemberStart + GetVarAdvance(pack, *lastChild); } -QString BufferFormatter::DeclareStruct(Packing::Rules pack, QList &declaredStructs, +QString BufferFormatter::DeclareStruct(Packing::Rules pack, ResourceId shader, + QList &declaredStructs, QMap &anonStructs, const QString &name, const rdcarray &members, uint32_t requiredByteStride, QString innerSkippedPrefixString) @@ -2641,7 +2653,7 @@ QString BufferFormatter::DeclareStruct(Packing::Rules pack, QList &decl if(members[i].type.pointerTypeID != ~0U) { const ShaderConstantType &pointeeType = - PointerTypeRegistry::GetTypeDescriptor(members[i].type.pointerTypeID); + PointerTypeRegistry::GetTypeDescriptor(shader, members[i].type.pointerTypeID); varTypeName = pointeeType.name; @@ -2651,7 +2663,7 @@ QString BufferFormatter::DeclareStruct(Packing::Rules pack, QList &decl if(!declaredStructs.contains(varTypeName)) { declaredStructs.push_back(varTypeName); - declarations += DeclareStruct(pack, declaredStructs, anonStructs, varTypeName, + declarations += DeclareStruct(pack, shader, declaredStructs, anonStructs, varTypeName, pointeeType.members, pointeeType.arrayByteStride, QString()) + lit("\n"); } @@ -2685,8 +2697,8 @@ QString BufferFormatter::DeclareStruct(Packing::Rules pack, QList &decl { declaredStructs.push_back(varTypeName); declarations += - DeclareStruct(pack, declaredStructs, anonStructs, varTypeName, members[i].type.members, - members[i].type.arrayByteStride, QString()) + + DeclareStruct(pack, shader, declaredStructs, anonStructs, varTypeName, + members[i].type.members, members[i].type.arrayByteStride, QString()) + lit("\n"); } } @@ -2746,13 +2758,13 @@ QString BufferFormatter::DeclareStruct(Packing::Rules pack, QList &decl return declarations + ret; } -QString BufferFormatter::DeclareStruct(Packing::Rules pack, const QString &name, +QString BufferFormatter::DeclareStruct(Packing::Rules pack, ResourceId shader, const QString &name, const rdcarray &members, uint32_t requiredByteStride) { QList declaredStructs; QMap anonStructs; - QString structDef = DeclareStruct(pack, declaredStructs, anonStructs, name, members, + QString structDef = DeclareStruct(pack, shader, declaredStructs, anonStructs, name, members, requiredByteStride, QString()); return QFormatStr("%1\n\n%2").arg(DeclarePacking(pack)).arg(structDef); @@ -2785,6 +2797,13 @@ ResourceFormat GetInterpretedResourceFormat(const ShaderConstant &elem) else format.compCount = elem.type.rows; + if(elem.type.baseType == VarType::GPUPointer) + { + format.compCount = 1; + format.compType = CompType::UInt; + format.compByteWidth = 8; + } + return format; } @@ -2842,10 +2861,9 @@ static void FillShaderVarData(ShaderVariable &var, const ShaderConstant &elem, c case VarType::SByte: var.value.u8v[dst] = (int8_t)qBound((int)INT8_MIN, o.toInt(), (int)INT8_MAX); break; - case VarType::Enum: + case VarType::Enum: var.value.u64v[dst] = o.value().val; break; case VarType::GPUPointer: - // treat this as a 64-bit unsigned integer - var.value.u64v[dst] = o.value().val; + var.SetTypedPointer(o.toULongLong(), ResourceId(), elem.type.pointerTypeID); break; case VarType::ConstantBlock: case VarType::ReadOnlyResource: @@ -3726,7 +3744,8 @@ TEST_CASE("round-trip via format", "[formatter]") // std140 packing parsed = BufferFormatter::ParseFormatString( - BufferFormatter::GetBufferFormatString(BufferFormatter::EstimatePackingRules(members), res, fmt), + BufferFormatter::GetBufferFormatString( + BufferFormatter::EstimatePackingRules(ResourceId(), members), ResourceId(), res, fmt), 0, true); CHECK((parsed.fixed.type.members == members)); @@ -3735,7 +3754,8 @@ TEST_CASE("round-trip via format", "[formatter]") members.back().type.arrayByteStride = 4; parsed = BufferFormatter::ParseFormatString( - BufferFormatter::GetBufferFormatString(BufferFormatter::EstimatePackingRules(members), res, fmt), + BufferFormatter::GetBufferFormatString( + BufferFormatter::EstimatePackingRules(ResourceId(), members), ResourceId(), res, fmt), 0, true); CHECK((parsed.fixed.type.members == members)); @@ -3746,7 +3766,8 @@ TEST_CASE("round-trip via format", "[formatter]") members.back().type.arrayByteStride = 12; parsed = BufferFormatter::ParseFormatString( - BufferFormatter::GetBufferFormatString(BufferFormatter::EstimatePackingRules(members), res, fmt), + BufferFormatter::GetBufferFormatString( + BufferFormatter::EstimatePackingRules(ResourceId(), members), ResourceId(), res, fmt), 0, true); CHECK((parsed.fixed.type.members == members)); diff --git a/qrenderdoc/Code/QRDUtils.cpp b/qrenderdoc/Code/QRDUtils.cpp index bd65da02b..9bd5f3783 100644 --- a/qrenderdoc/Code/QRDUtils.cpp +++ b/qrenderdoc/Code/QRDUtils.cpp @@ -98,6 +98,8 @@ void PointerTypeRegistry::Init() uint32_t PointerTypeRegistry::GetTypeID(ResourceId shader, uint32_t pointerTypeId) { + if(pointerTypeId & TypeIDBit) + shader = ResourceId(); return typeMapping[qMakePair(shader, pointerTypeId)]; } @@ -1083,8 +1085,8 @@ bool RichResourceTextMouseEvent(const QWidget *owner, const QVariant &var, QRect if(!ptrType.members.isEmpty()) formatter = BufferFormatter::DeclareStruct( - BufferFormatter::EstimatePackingRules(ptrType.members), ptrType.name, - ptrType.members, ptrType.arrayByteStride); + BufferFormatter::EstimatePackingRules(ResourceId(), ptrType.members), ResourceId(), + ptrType.name, ptrType.members, ptrType.arrayByteStride); IBufferViewer *view = ctx.ViewBuffer(ptr->offset, ~0ULL, ptr->base, formatter); diff --git a/qrenderdoc/Code/QRDUtils.h b/qrenderdoc/Code/QRDUtils.h index 8bf6cc7ef..948e5b990 100644 --- a/qrenderdoc/Code/QRDUtils.h +++ b/qrenderdoc/Code/QRDUtils.h @@ -203,7 +203,8 @@ private: static bool ContainsUnbounded(const ShaderConstant &structType, rdcpair *found = NULL); - static QString DeclareStruct(Packing::Rules pack, QList &declaredStructs, + static QString DeclareStruct(Packing::Rules pack, ResourceId shader, + QList &declaredStructs, QMap &anonStructs, const QString &name, const rdcarray &members, uint32_t requiredByteStride, QString innerSkippedPrefixString); @@ -214,8 +215,10 @@ private: static uint32_t GetVarStraddleSize(const ShaderConstant &var); static uint32_t GetVarSizeAndTrail(const ShaderConstant &var); - static void EstimatePackingRules(Packing::Rules &pack, const ShaderConstant &constant); - static void EstimatePackingRules(Packing::Rules &pack, const rdcarray &members); + static void EstimatePackingRules(Packing::Rules &pack, ResourceId shader, + const ShaderConstant &constant); + static void EstimatePackingRules(Packing::Rules &pack, ResourceId shader, + const rdcarray &members); static QString DeclarePacking(Packing::Rules pack); public: @@ -225,13 +228,14 @@ public: static ParsedFormat ParseFormatString(const QString &formatString, uint64_t maxLen, bool cbuffer); static uint32_t GetVarAdvance(Packing::Rules pack, const ShaderConstant &var); - static Packing::Rules EstimatePackingRules(const rdcarray &members); + static Packing::Rules EstimatePackingRules(ResourceId shader, + const rdcarray &members); static QString GetTextureFormatString(const TextureDescription &tex); - static QString GetBufferFormatString(Packing::Rules pack, const ShaderResource &res, - const ResourceFormat &viewFormat); + static QString GetBufferFormatString(Packing::Rules pack, ResourceId shader, + const ShaderResource &res, const ResourceFormat &viewFormat); - static QString DeclareStruct(Packing::Rules pack, const QString &name, + static QString DeclareStruct(Packing::Rules pack, ResourceId shader, const QString &name, const rdcarray &members, uint32_t requiredByteStride); }; diff --git a/qrenderdoc/Windows/BufferViewer.cpp b/qrenderdoc/Windows/BufferViewer.cpp index 83ed355ff..2ba5b33d5 100644 --- a/qrenderdoc/Windows/BufferViewer.cpp +++ b/qrenderdoc/Windows/BufferViewer.cpp @@ -2921,8 +2921,8 @@ void BufferViewer::OnEventChanged(uint32_t eventId) if(IsD3D(m_Ctx.APIProps().pipelineType)) bufdata->vsinConfig.packing = Packing::D3DCB; else - bufdata->vsinConfig.packing = - BufferFormatter::EstimatePackingRules(bufdata->vsinConfig.fixedVars.type.members); + bufdata->vsinConfig.packing = BufferFormatter::EstimatePackingRules( + reflection->resourceId, bufdata->vsinConfig.fixedVars.type.members); } } @@ -4970,12 +4970,13 @@ void BufferViewer::on_setFormat_toggled(bool checked) if(IsD3D(m_Ctx.APIProps().pipelineType)) ui->formatSpecifier->setAutoFormat(BufferFormatter::DeclareStruct( - Packing::D3DCB, reflection->constantBlocks[m_CBufferSlot.slot].name, + Packing::D3DCB, reflection->resourceId, reflection->constantBlocks[m_CBufferSlot.slot].name, reflection->constantBlocks[m_CBufferSlot.slot].variables, 0)); else ui->formatSpecifier->setAutoFormat(BufferFormatter::DeclareStruct( - BufferFormatter::EstimatePackingRules(reflection->constantBlocks[m_CBufferSlot.slot].variables), - reflection->constantBlocks[m_CBufferSlot.slot].name, + BufferFormatter::EstimatePackingRules( + reflection->resourceId, reflection->constantBlocks[m_CBufferSlot.slot].variables), + reflection->resourceId, reflection->constantBlocks[m_CBufferSlot.slot].name, reflection->constantBlocks[m_CBufferSlot.slot].variables, 0)); } diff --git a/qrenderdoc/Windows/PipelineState/D3D11PipelineStateViewer.cpp b/qrenderdoc/Windows/PipelineState/D3D11PipelineStateViewer.cpp index f151ba25f..1d8d2f229 100644 --- a/qrenderdoc/Windows/PipelineState/D3D11PipelineStateViewer.cpp +++ b/qrenderdoc/Windows/PipelineState/D3D11PipelineStateViewer.cpp @@ -2162,8 +2162,8 @@ void D3D11PipelineStateViewer::resource_itemActivated(RDTreeWidgetItem *item, in if(shaderRes) { - format = - BufferFormatter::GetBufferFormatString(Packing::D3DUAV, *shaderRes, view.res.viewFormat); + format = BufferFormatter::GetBufferFormatString(Packing::D3DUAV, stage->resourceId, + *shaderRes, view.res.viewFormat); if(view.res.bufferFlags & D3DBufferViewFlags::Raw) format = lit("xint"); diff --git a/qrenderdoc/Windows/PipelineState/D3D12PipelineStateViewer.cpp b/qrenderdoc/Windows/PipelineState/D3D12PipelineStateViewer.cpp index ba6879281..3913548a3 100644 --- a/qrenderdoc/Windows/PipelineState/D3D12PipelineStateViewer.cpp +++ b/qrenderdoc/Windows/PipelineState/D3D12PipelineStateViewer.cpp @@ -2274,8 +2274,8 @@ void D3D12PipelineStateViewer::resource_itemActivated(RDTreeWidgetItem *item, in if(shaderRes) { - format = - BufferFormatter::GetBufferFormatString(Packing::D3DUAV, *shaderRes, view.res.viewFormat); + format = BufferFormatter::GetBufferFormatString(Packing::D3DUAV, stage->resourceId, + *shaderRes, view.res.viewFormat); if(view.res.bufferFlags & D3DBufferViewFlags::Raw) format = lit("xint"); diff --git a/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.cpp b/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.cpp index ae61e9c71..50d38a216 100644 --- a/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.cpp +++ b/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.cpp @@ -2347,8 +2347,9 @@ void GLPipelineStateViewer::resource_itemActivated(RDTreeWidgetItem *item, int c return; QString format = BufferFormatter::GetBufferFormatString( - BufferFormatter::EstimatePackingRules(shaderRes->variableType.members), *shaderRes, - ResourceFormat()); + BufferFormatter::EstimatePackingRules(stage->shaderResourceId, + shaderRes->variableType.members), + stage->shaderResourceId, *shaderRes, ResourceFormat()); if(buf.ID != ResourceId()) { diff --git a/qrenderdoc/Windows/PipelineState/VulkanPipelineStateViewer.cpp b/qrenderdoc/Windows/PipelineState/VulkanPipelineStateViewer.cpp index 3be030624..9e9b10a9c 100644 --- a/qrenderdoc/Windows/PipelineState/VulkanPipelineStateViewer.cpp +++ b/qrenderdoc/Windows/PipelineState/VulkanPipelineStateViewer.cpp @@ -3039,7 +3039,8 @@ void VulkanPipelineStateViewer::resource_itemActivated(RDTreeWidgetItem *item, i : stage->reflection->readOnlyResources[buf.bindPoint]; format = BufferFormatter::GetBufferFormatString( - BufferFormatter::EstimatePackingRules(shaderRes.variableType.members), shaderRes, buf.fmt); + BufferFormatter::EstimatePackingRules(stage->resourceId, shaderRes.variableType.members), + stage->resourceId, shaderRes, buf.fmt); } if(buf.ID != ResourceId())