From ee9465e3e5dfeea1dd6cc5a1f179fb0d094cebc6 Mon Sep 17 00:00:00 2001 From: Yong He Date: Wed, 8 Jan 2025 21:19:33 -0800 Subject: [PATCH] Fix Metal type layout reflection for nested parameter blocks. --- source/slang/slang-type-layout.cpp | 34 +++++++++++++++++-- source/slang/slang-type-layout.h | 2 ++ .../nested-parameter-block-reflection.slang | 18 ++++++++++ 3 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 tests/metal/nested-parameter-block-reflection.slang diff --git a/source/slang/slang-type-layout.cpp b/source/slang/slang-type-layout.cpp index 98a324d962..eb91a67653 100644 --- a/source/slang/slang-type-layout.cpp +++ b/source/slang/slang-type-layout.cpp @@ -845,6 +845,8 @@ struct GLSLObjectLayoutRulesImpl : ObjectLayoutRulesImpl { case ShaderParameterKind::SubpassInput: return SimpleLayoutInfo(LayoutResourceKind::InputAttachmentIndex, slotCount); + case ShaderParameterKind::ParameterBlock: + return SimpleLayoutInfo(LayoutResourceKind::SubElementRegisterSpace, 1); default: break; } @@ -891,7 +893,8 @@ struct HLSLObjectLayoutRulesImpl : ObjectLayoutRulesImpl { case ShaderParameterKind::ConstantBuffer: return SimpleLayoutInfo(LayoutResourceKind::ConstantBuffer, 1); - + case ShaderParameterKind::ParameterBlock: + return SimpleLayoutInfo(LayoutResourceKind::SubElementRegisterSpace, 1); case ShaderParameterKind::TextureUniformBuffer: case ShaderParameterKind::StructuredBuffer: case ShaderParameterKind::RawBuffer: @@ -1180,6 +1183,7 @@ struct CPUObjectLayoutRulesImpl : ObjectLayoutRulesImpl switch (kind) { case ShaderParameterKind::ConstantBuffer: + case ShaderParameterKind::ParameterBlock: // It's a pointer to the actual uniform data return SimpleLayoutInfo( LayoutResourceKind::Uniform, @@ -1262,6 +1266,7 @@ struct CUDAObjectLayoutRulesImpl : CPUObjectLayoutRulesImpl switch (kind) { case ShaderParameterKind::ConstantBuffer: + case ShaderParameterKind::ParameterBlock: { // It's a pointer to the actual uniform data return SimpleLayoutInfo( @@ -1856,6 +1861,7 @@ struct MetalObjectLayoutRulesImpl : ObjectLayoutRulesImpl switch (kind) { case ShaderParameterKind::ConstantBuffer: + case ShaderParameterKind::ParameterBlock: case ShaderParameterKind::StructuredBuffer: case ShaderParameterKind::MutableStructuredBuffer: case ShaderParameterKind::RawBuffer: @@ -1917,6 +1923,7 @@ struct MetalArgumentBufferElementLayoutRulesImpl : ObjectLayoutRulesImpl, Defaul switch (kind) { case ShaderParameterKind::ConstantBuffer: + case ShaderParameterKind::ParameterBlock: case ShaderParameterKind::StructuredBuffer: case ShaderParameterKind::MutableStructuredBuffer: case ShaderParameterKind::RawBuffer: @@ -2341,6 +2348,10 @@ static SimpleLayoutInfo _getParameterGroupLayoutInfo( } else if (as(type)) { + auto info = + rules->GetObjectLayout(ShaderParameterKind::ParameterBlock, context.objectLayoutOptions) + .getSimple(); + // Note: we default to consuming zero register spces here, because // a parameter block might not contain anything (or all it contains // is other blocks), and so it won't get a space allocated. @@ -2351,7 +2362,9 @@ static SimpleLayoutInfo _getParameterGroupLayoutInfo( // // TODO: wouldn't it be any different to just allocate this // as an empty `SimpleLayoutInfo` of any other kind? - return SimpleLayoutInfo(LayoutResourceKind::SubElementRegisterSpace, 0); + if (info.kind == LayoutResourceKind::SubElementRegisterSpace) + info.size = 0; + return info; } // TODO: the vertex-input and fragment-output cases should @@ -4049,6 +4062,21 @@ RefPtr StructTypeLayoutBuilder::addField( RefPtr fieldTypeLayout = fieldResult.layout; UniformLayoutInfo fieldInfo = fieldResult.info.getUniformLayout(); + if (fieldTypeLayout->resourceInfos.getCount() == 0) + { + if (auto paramGroupTypeLayout = as(fieldTypeLayout)) + { + // If field type layout is a parameter block and it has a size that is not just a space, + // we need to count for it in the struct layout. + auto containerTypeLayout = paramGroupTypeLayout->containerVarLayout->getTypeLayout(); + if (containerTypeLayout->FindResourceInfo( + LayoutResourceKind::SubElementRegisterSpace) == nullptr) + { + fieldTypeLayout = containerTypeLayout; + } + } + } + // Note: we don't add any zero-size fields // when computing structure layout, just // to avoid having a resource type impact @@ -4079,7 +4107,7 @@ RefPtr StructTypeLayoutBuilder::addField( // for each field of the structure. RefPtr fieldLayout = new VarLayout(); fieldLayout->varDecl = field; - fieldLayout->typeLayout = fieldTypeLayout; + fieldLayout->typeLayout = fieldResult.layout; m_typeLayout->fields.add(fieldLayout); if (field) diff --git a/source/slang/slang-type-layout.h b/source/slang/slang-type-layout.h index 2cdb4b3866..91132c76a2 100644 --- a/source/slang/slang-type-layout.h +++ b/source/slang/slang-type-layout.h @@ -975,6 +975,8 @@ enum class ShaderParameterKind AtomicUint, SubpassInput, + + ParameterBlock, }; struct SimpleLayoutRulesImpl diff --git a/tests/metal/nested-parameter-block-reflection.slang b/tests/metal/nested-parameter-block-reflection.slang new file mode 100644 index 0000000000..8282f55731 --- /dev/null +++ b/tests/metal/nested-parameter-block-reflection.slang @@ -0,0 +1,18 @@ +//TEST:REFLECTION(filecheck=CHECK): -target metal + +// CHECK:"name": "tex", +// CHECK:"binding": {"kind": "metalArgumentBufferElement", "index": 1} + +struct Data { int3 content; } +struct Params +{ + ParameterBlock pdata; + Texture2D tex; +} +ParameterBlock gParams; +RWStructuredBuffer output; +[numthreads(1,1,1)] +void computeMain() +{ + output[0] = gParams.tex.Load(gParams.pdata.content); +} \ No newline at end of file