Skip to content

Commit 179f633

Browse files
TheMostDiligentTheMostDiligent
committed
Move resource merge verification into DefaultSignatureDescBuilder
1 parent 0f882b5 commit 179f633

5 files changed

Lines changed: 31 additions & 45 deletions

File tree

‎Graphics/GraphicsEngine/include/PipelineStateBase.hpp‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1163,27 +1163,34 @@ class PipelineStateBase : public DeviceObjectBase<typename EngineImplTraits::Pip
11631163
template <typename AttribsType>
11641164
struct DefaultSignatureDescBuilder
11651165
{
1166-
const char* const PSOName;
1167-
const PipelineResourceLayoutDesc& ResourceLayout;
1166+
1167+
const char* const PSOName;
1168+
const PipelineResourceLayoutDesc& ResourceLayout;
1169+
1170+
using VerifyMergeFn = void (*)(const char*, const AttribsType&, const AttribsType&);
1171+
VerifyMergeFn const VerifyMerge;
1172+
11681173
PipelineResourceSignatureDescWrapper& SignDesc;
11691174

11701175
struct UniqueResourceInfo
11711176
{
11721177
const AttribsType& Attribs;
11731178
const Uint32 ResIdx; // Index in SignDesc
11741179
};
1175-
std::unordered_map<ShaderResourceHashKey, UniqueResourceInfo, ShaderResourceHashKey::Hasher> UniqueResources;
1180+
std::unordered_map<ShaderResourceHashKey, UniqueResourceInfo, ShaderResourceHashKey::Hasher> UniqueResources = {};
11761181

11771182
DefaultSignatureDescBuilder(const char* _PSOName,
11781183
const PipelineResourceLayoutDesc& _ResourceLayout,
1184+
VerifyMergeFn _VerifyMerge,
11791185
PipelineResourceSignatureDescWrapper& _SignDesc) :
11801186
PSOName{_PSOName},
11811187
ResourceLayout{_ResourceLayout},
1188+
VerifyMerge{_VerifyMerge},
11821189
SignDesc{_SignDesc}
11831190
{
11841191
}
11851192

1186-
auto AddResource(const char* Name,
1193+
void AddResource(const char* Name,
11871194
const AttribsType& Attribs,
11881195
const ShaderResourceVariableDesc& VarDesc,
11891196
Uint32 ArraySize,
@@ -1192,25 +1199,24 @@ class PipelineStateBase : public DeviceObjectBase<typename EngineImplTraits::Pip
11921199
WebGPUResourceAttribs WebGPUAttribs = {})
11931200
{
11941201
// Note that Attribs.Name != VarDesc.Name for combined samplers
1195-
auto it_assigned = UniqueResources.emplace(ShaderResourceHashKey{VarDesc.ShaderStages, Name},
1196-
UniqueResourceInfo{Attribs, SignDesc.GetNumResources()});
1197-
if (it_assigned.second)
1202+
auto [it, assigned] = UniqueResources.emplace(ShaderResourceHashKey{VarDesc.ShaderStages, Name},
1203+
UniqueResourceInfo{Attribs, SignDesc.GetNumResources()});
1204+
if (assigned)
11981205
{
11991206
SignDesc.AddResource(VarDesc.ShaderStages, Name, ArraySize, ResType, VarDesc.Type, ResFlags, WebGPUAttribs);
12001207
}
12011208
else
12021209
{
12031210
if (ResFlags & PIPELINE_RESOURCE_FLAG_INLINE_CONSTANTS)
12041211
{
1205-
PipelineResourceDesc& InlineCB = SignDesc.GetResource(it_assigned.first->second.ResIdx);
1212+
PipelineResourceDesc& InlineCB = SignDesc.GetResource(it->second.ResIdx);
12061213
VERIFY_EXPR(InlineCB.Flags & PIPELINE_RESOURCE_FLAG_INLINE_CONSTANTS);
12071214
//VERIFY_EXPR(InlineCB.ShaderStages & ShaderType);
12081215
// Use the maximum number of constants across all shaders
12091216
InlineCB.ArraySize = (std::max)(InlineCB.ArraySize, ArraySize);
12101217
}
1218+
VerifyMerge(PSOName, it->second.Attribs, Attribs);
12111219
}
1212-
1213-
return it_assigned;
12141220
}
12151221
};
12161222

‎Graphics/GraphicsEngineD3DBase/include/PipelineStateD3DBase.hpp‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class PipelineStateD3DBase : public PipelineStateBase<EngineImplTraits>
6969
const PipelineResourceLayoutDesc& _ResourceLayout,
7070
const LocalRootSignatureType* _pLocalRootSig,
7171
PipelineResourceSignatureDescWrapper& _SignDesc) :
72-
TBaseBuilder{_PSOName, _ResourceLayout, _SignDesc},
72+
TBaseBuilder{_PSOName, _ResourceLayout, VerifyD3DResourceMerge, _SignDesc},
7373
pLocalRootSig{_pLocalRootSig}
7474
{}
7575

@@ -109,11 +109,7 @@ class PipelineStateD3DBase : public PipelineStateBase<EngineImplTraits>
109109
"INLINE_CONSTANTS flag cannot be combined with other flags.");
110110

111111
// Note that Attribs.Name != VarDesc.Name for combined samplers
112-
auto [it, assigned] = this->AddResource(Attribs.Name, Attribs, VarDesc, ArraySize, ResType, ResFlags);
113-
if (!assigned)
114-
{
115-
VerifyD3DResourceMerge(this->PSOName, it->second.Attribs, Attribs);
116-
}
112+
this->AddResource(Attribs.Name, Attribs, VarDesc, ArraySize, ResType, ResFlags);
117113
} //
118114
);
119115

‎Graphics/GraphicsEngineOpenGL/src/PipelineStateGLImpl.cpp‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,15 @@ namespace Diligent
4545

4646
constexpr INTERFACE_ID PipelineStateGLImpl::IID_InternalImpl;
4747

48-
static void VerifyResourceMerge(const PipelineStateDesc& PSODesc,
48+
static void VerifyResourceMerge(const char* PSOName,
4949
const ShaderResourcesGL::GLResourceAttribs& ExistingRes,
5050
const ShaderResourcesGL::GLResourceAttribs& NewResAttribs)
5151
{
52-
#define LOG_RESOURCE_MERGE_ERROR_AND_THROW(PropertyName) \
53-
LOG_ERROR_AND_THROW("Shader variable '", NewResAttribs.Name, \
54-
"' is shared between multiple shaders in pipeline '", (PSODesc.Name ? PSODesc.Name : ""), \
55-
"', but its " PropertyName " varies. A variable shared between multiple shaders " \
56-
"must be defined identically in all shaders. Either use separate variables for " \
52+
#define LOG_RESOURCE_MERGE_ERROR_AND_THROW(PropertyName) \
53+
LOG_ERROR_AND_THROW("Shader variable '", NewResAttribs.Name, \
54+
"' is shared between multiple shaders in pipeline '", (PSOName ? PSOName : ""), \
55+
"', but its " PropertyName " varies. A variable shared between multiple shaders " \
56+
"must be defined identically in all shaders. Either use separate variables for " \
5757
"different shader stages, change resource name or make sure that " PropertyName " is consistent.");
5858

5959
if (ExistingRes.ResourceType != NewResAttribs.ResourceType)
@@ -112,18 +112,14 @@ PipelineResourceSignatureDescWrapper PipelineStateGLImpl::GetDefaultSignatureDes
112112

113113
PipelineResourceSignatureDescWrapper SignDesc{m_Desc.Name, ResourceLayout, m_Desc.SRBAllocationGranularity};
114114
SignDesc.SetCombinedSamplerSuffix(PipelineResourceSignatureDesc{}.CombinedSamplerSuffix);
115-
DefaultSignatureDescBuilder<ShaderResourcesGL::GLResourceAttribs> Builder{m_Desc.Name, ResourceLayout, SignDesc};
115+
DefaultSignatureDescBuilder<ShaderResourcesGL::GLResourceAttribs> Builder{m_Desc.Name, ResourceLayout, VerifyResourceMerge, SignDesc};
116116

117117
const auto HandleResource = [&](const ShaderResourcesGL::GLResourceAttribs& Attribs) //
118118
{
119119
const ShaderResourceVariableDesc VarDesc = FindPipelineResourceLayoutVariable(ResourceLayout, Attribs.Name, Attribs.ShaderStages, nullptr);
120120
const PIPELINE_RESOURCE_FLAGS Flags = Attribs.ResourceFlags | ShaderVariableFlagsToPipelineResourceFlags(VarDesc.Flags);
121121

122-
auto [it, assigned] = Builder.AddResource(Attribs.Name, Attribs, VarDesc, Attribs.ArraySize, Attribs.ResourceType, Flags);
123-
if (!assigned)
124-
{
125-
VerifyResourceMerge(m_Desc, it->second.Attribs, Attribs);
126-
}
122+
Builder.AddResource(Attribs.Name, Attribs, VarDesc, Attribs.ArraySize, Attribs.ResourceType, Flags);
127123
};
128124

129125
// Specialized handler for uniform buffers to handle inline constants correctly
@@ -136,11 +132,7 @@ PipelineResourceSignatureDescWrapper PipelineStateGLImpl::GetDefaultSignatureDes
136132
UB.GetInlineConstantCountOrThrow() :
137133
UB.ArraySize;
138134

139-
auto [it, assigned] = Builder.AddResource(UB.Name, UB, VarDesc, ArraySize, UB.ResourceType, Flags);
140-
if (!assigned)
141-
{
142-
VerifyResourceMerge(m_Desc, it->second.Attribs, UB);
143-
}
135+
Builder.AddResource(UB.Name, UB, VarDesc, ArraySize, UB.ResourceType, Flags);
144136
};
145137

146138
if (m_IsProgramPipelineSupported)

‎Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,7 @@ PipelineResourceSignatureDescWrapper PipelineStateVkImpl::GetDefaultResourceSign
637637
Uint32 SRBAllocationGranularity) noexcept(false)
638638
{
639639
PipelineResourceSignatureDescWrapper SignDesc{PSOName, ResourceLayout, SRBAllocationGranularity};
640-
DefaultSignatureDescBuilder<SPIRVShaderResourceAttribs> Builder{PSOName, ResourceLayout, SignDesc};
640+
DefaultSignatureDescBuilder<SPIRVShaderResourceAttribs> Builder{PSOName, ResourceLayout, VerifyResourceMerge, SignDesc};
641641

642642
MergedPushConstantMapType MergedPushConstants;
643643

@@ -703,11 +703,7 @@ PipelineResourceSignatureDescWrapper PipelineStateVkImpl::GetDefaultResourceSign
703703
"INLINE_CONSTANTS flag cannot be combined with other flags.");
704704

705705
// Note that Attribs.Name != VarDesc.Name for combined samplers
706-
auto [it, assigned] = Builder.AddResource(Attribs.Name, Attribs, VarDesc, ArraySize, ResType, Flags);
707-
if (!assigned)
708-
{
709-
VerifyResourceMerge(PSOName, it->second.Attribs, Attribs);
710-
}
706+
Builder.AddResource(Attribs.Name, Attribs, VarDesc, ArraySize, ResType, Flags);
711707
});
712708

713709
// Merge combined sampler suffixes

‎Graphics/GraphicsEngineWebGPU/src/PipelineStateWebGPUImpl.cpp‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,7 @@ PipelineResourceSignatureDescWrapper PipelineStateWebGPUImpl::GetDefaultResource
749749
Uint32 SRBAllocationGranularity)
750750
{
751751
PipelineResourceSignatureDescWrapper SignDesc{PSOName, ResourceLayout, SRBAllocationGranularity};
752-
DefaultSignatureDescBuilder<WGSLShaderResourceAttribs> Builder{PSOName, ResourceLayout, SignDesc};
752+
DefaultSignatureDescBuilder<WGSLShaderResourceAttribs> Builder{PSOName, ResourceLayout, VerifyResourceMerge, SignDesc};
753753

754754
for (const ShaderStageInfo& Stage : ShaderStages)
755755
{
@@ -780,11 +780,7 @@ PipelineResourceSignatureDescWrapper PipelineStateWebGPUImpl::GetDefaultResource
780780
Attribs.ArraySize;
781781

782782
// Note that Attribs.Name != VarDesc.Name for combined samplers
783-
auto [it, assigned] = Builder.AddResource(Attribs.Name, Attribs, VarDesc, ArraySize, ResType, Flags, WebGPUAttribs);
784-
if (!assigned)
785-
{
786-
VerifyResourceMerge(PSOName, it->second.Attribs, Attribs);
787-
}
783+
Builder.AddResource(Attribs.Name, Attribs, VarDesc, ArraySize, ResType, Flags, WebGPUAttribs);
788784
});
789785

790786
// Merge combined sampler suffixes

0 commit comments

Comments
 (0)