Skip to content

Commit 2c8faca

Browse files
TheMostDiligentTheMostDiligent
committed
D3DShaderResourceLoader: fixed handling resource arrays with the optimized-out first element
1 parent 2e6e90b commit 2c8faca

File tree

4 files changed

+41
-46
lines changed

4 files changed

+41
-46
lines changed

‎Graphics/GraphicsEngineD3DBase/include/D3DShaderResourceLoader.hpp‎

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2019-2023 Diligent Graphics LLC
2+
* Copyright 2019-2024 Diligent Graphics LLC
33
* Copyright 2015-2019 Egor Yusov
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
@@ -36,6 +36,7 @@
3636
#include "ShaderToolsCommon.hpp"
3737
#include "D3DCommonTypeConversions.hpp"
3838
#include "EngineMemory.h"
39+
#include "ParsingTools.hpp"
3940

4041
/// \file
4142
/// D3D shader resource loading
@@ -185,12 +186,20 @@ void LoadD3DShaderResources(TShaderReflection* pShaderReflection,
185186
D3D_SHADER_INPUT_BIND_DESC BindingDesc = {};
186187
pShaderReflection->GetResourceBindingDesc(Res, &BindingDesc);
187188

189+
std::string Name;
190+
const auto ArrayIndex = Parsing::GetArrayIndex(BindingDesc.Name, Name);
191+
188192
if (BindingDesc.BindPoint == UINT32_MAX)
193+
{
189194
BindingDesc.BindPoint = D3DShaderResourceAttribs::InvalidBindPoint;
190-
191-
std::string Name{BindingDesc.Name};
192-
193-
SkipCount = 1;
195+
}
196+
else if (ArrayIndex > 0)
197+
{
198+
// Adjust bind point for array index
199+
VERIFY(BindingDesc.BindPoint >= static_cast<UINT>(ArrayIndex), "Resource '", BindingDesc.Name, "' uses bind point point ", BindingDesc.BindPoint,
200+
", which is invalid for its array index ", ArrayIndex);
201+
BindingDesc.BindPoint -= static_cast<UINT>(ArrayIndex);
202+
}
194203

195204
UINT BindCount = BindingDesc.BindCount;
196205
if (BindCount == UINT_MAX)
@@ -217,17 +226,11 @@ void LoadD3DShaderResources(TShaderReflection* pShaderReflection,
217226
//
218227
// Notice that if some array element is not used by the shader, it will not be enumerated
219228

220-
auto OpenBracketPos = Name.find('[');
221-
if (String::npos != OpenBracketPos)
229+
SkipCount = 1;
230+
if (ArrayIndex >= 0)
222231
{
223232
VERIFY(BindCount == 1, "When array elements are enumerated individually, BindCount is expected to always be 1");
224233

225-
// Name == "g_tex2DDiffuse[0]"
226-
// ^
227-
// OpenBracketPos
228-
Name.erase(OpenBracketPos, Name.length() - OpenBracketPos);
229-
// Name == "g_tex2DDiffuse"
230-
VERIFY_EXPR(Name.length() == OpenBracketPos);
231234
#ifdef DILIGENT_DEBUG
232235
for (const auto& ExistingRes : Resources)
233236
{
@@ -236,21 +239,23 @@ void LoadD3DShaderResources(TShaderReflection* pShaderReflection,
236239
#endif
237240
for (UINT ArrElem = Res + 1; ArrElem < shaderDesc.BoundResources; ++ArrElem)
238241
{
239-
D3D_SHADER_INPUT_BIND_DESC ArrElemBindingDesc = {};
240-
pShaderReflection->GetResourceBindingDesc(ArrElem, &ArrElemBindingDesc);
242+
D3D_SHADER_INPUT_BIND_DESC NextElemBindingDesc = {};
243+
pShaderReflection->GetResourceBindingDesc(ArrElem, &NextElemBindingDesc);
244+
245+
std::string NextElemName;
246+
const auto NextElemIndex = Parsing::GetArrayIndex(NextElemBindingDesc.Name, NextElemName);
241247

242248
// Make sure this case is handled correctly:
243249
// "g_tex2DDiffuse[.]" != "g_tex2DDiffuse2[.]"
244-
if (strncmp(Name.c_str(), ArrElemBindingDesc.Name, OpenBracketPos) == 0 && ArrElemBindingDesc.Name[OpenBracketPos] == '[')
250+
if (Name == NextElemName)
245251
{
246-
//g_tex2DDiffuse[2]
247-
// ^
248-
UINT Ind = atoi(ArrElemBindingDesc.Name + OpenBracketPos + 1);
249-
BindCount = std::max(BindCount, Ind + 1);
250-
VERIFY(ArrElemBindingDesc.BindPoint == BindingDesc.BindPoint + Ind,
252+
VERIFY_EXPR(NextElemIndex > 0);
253+
254+
BindCount = std::max(BindCount, static_cast<UINT>(NextElemIndex) + 1);
255+
VERIFY(NextElemBindingDesc.BindPoint == BindingDesc.BindPoint + NextElemIndex,
251256
"Array elements are expected to use contiguous bind points.\n",
252-
BindingDesc.Name, " uses slot ", BindingDesc.BindPoint, ", so ", ArrElemBindingDesc.Name,
253-
" is expected to use slot ", BindingDesc.BindPoint + Ind, " while ", ArrElemBindingDesc.BindPoint,
257+
BindingDesc.Name, " uses slot ", BindingDesc.BindPoint, ", so ", NextElemBindingDesc.Name,
258+
" is expected to use slot ", BindingDesc.BindPoint + NextElemIndex, " while ", NextElemBindingDesc.BindPoint,
254259
" is actually used");
255260

256261
// Note that skip count may not necessarily be the same as BindCount.

‎Graphics/GraphicsEngineOpenGL/src/ShaderResourcesGL.cpp‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2019-2022 Diligent Graphics LLC
2+
* Copyright 2019-2024 Diligent Graphics LLC
33
* Copyright 2015-2019 Egor Yusov
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
@@ -39,6 +39,7 @@
3939
#include "Align.hpp"
4040
#include "GLTypeConversions.hpp"
4141
#include "ShaderToolsCommon.hpp"
42+
#include "ParsingTools.hpp"
4243

4344
namespace Diligent
4445
{
@@ -284,20 +285,6 @@ static void AddUniformBufferVariable(GLuint
284285
}
285286
}
286287

287-
int GetArrayIndex(const char* VarName, std::string& NameWithoutBrackets)
288-
{
289-
NameWithoutBrackets = VarName;
290-
const auto* OpenBracketPtr = strchr(VarName, '[');
291-
if (OpenBracketPtr == nullptr)
292-
return -1;
293-
294-
auto Ind = atoi(OpenBracketPtr + 1);
295-
if (Ind >= 0)
296-
NameWithoutBrackets = std::string{VarName, OpenBracketPtr};
297-
298-
return Ind;
299-
}
300-
301288
static void ProcessUBVariable(ShaderCodeVariableDescX& Var, Uint32 BaseOffset)
302289
{
303290
// Re-sort by offset
@@ -343,7 +330,7 @@ static ShaderCodeBufferDescX PrepareUBReflection(std::vector<ShaderCodeVariableD
343330
// Dot
344331
std::string Prefix{Name, Dot}; // "s2[1]"
345332
std::string NameWithoutBrackets;
346-
const auto ArrayInd = GetArrayIndex(Prefix.c_str(), NameWithoutBrackets);
333+
const auto ArrayInd = Parsing::GetArrayIndex(Prefix, NameWithoutBrackets);
347334
// ArrayInd = 1
348335
// NameWithoutBrackets = "s2"
349336

@@ -414,7 +401,7 @@ static ShaderCodeBufferDescX PrepareUBReflection(std::vector<ShaderCodeVariableD
414401
// Name
415402

416403
std::string NameWithoutBrackets;
417-
const auto ArrayInd = GetArrayIndex(Name, NameWithoutBrackets);
404+
const auto ArrayInd = Parsing::GetArrayIndex(Name, NameWithoutBrackets);
418405
// ArrayInd = 2
419406
// NameWithoutBrackets = "f4"
420407
if (auto* pArray = pLevel->FindMember(NameWithoutBrackets.c_str()))

‎Tests/DiligentCoreAPITest/assets/shaders/ShaderResourceArrayTest.psh‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11

22
Texture2D<float4> g_tex2DTest[4];
3-
Texture2D<float4> g_tex2DTest2[2];
3+
Texture2D<float4> g_tex2DTest2[5]; // [0], [2] and [3] are not used
44
Texture2D<float4> g_tex2D[2];
55
SamplerState g_tex2DTest_sampler[4];
66
SamplerState g_tex2DTest2_sampler;
@@ -16,8 +16,8 @@ void main(in float4 f4Position : SV_Position,
1616
in VSOut vsOut,
1717
out float4 Color : SV_Target)
1818
{
19-
float4 Color0 = (g_tex2DTest[0].SampleLevel(g_tex2DTest_sampler[0], vsOut.f2UV, 0) + g_tex2DTest2[0].SampleLevel(g_tex2DTest2_sampler, vsOut.f2UV, 0))/2.0;
20-
float4 Color1 = (g_tex2DTest[0].SampleLevel(g_tex2DTest_sampler[0], vsOut.f2UV, 2) + g_tex2DTest2[1].SampleLevel(g_tex2DTest2_sampler, vsOut.f2UV, 0))/2.0;
19+
float4 Color0 = (g_tex2DTest[0].SampleLevel(g_tex2DTest_sampler[0], vsOut.f2UV, 0) + g_tex2DTest2[1].SampleLevel(g_tex2DTest2_sampler, vsOut.f2UV, 0))/2.0;
20+
float4 Color1 = (g_tex2DTest[0].SampleLevel(g_tex2DTest_sampler[0], vsOut.f2UV, 2) + g_tex2DTest2[4].SampleLevel(g_tex2DTest2_sampler, vsOut.f2UV, 0))/2.0;
2121
float4 Color2 = (g_tex2DTest[2].SampleLevel(g_tex2DTest_sampler[2], vsOut.f2UV, 4) + g_tex2D[0].SampleLevel(g_tex2D_sampler[0], vsOut.f2UV, 0))/2.0;
2222
float4 Color3 = (g_tex2DTest[3].SampleLevel(g_tex2DTest_sampler[3], vsOut.f2UV, 5) + g_tex2D[1].SampleLevel(g_tex2D_sampler[1], vsOut.f2UV, 0))/2.0;
2323
if( vsOut.f2UV.x < 0.5 && vsOut.f2UV.y < 0.5 )

‎Tests/DiligentCoreAPITest/src/ShaderResourceArrayTest.cpp‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2019-2023 Diligent Graphics LLC
2+
* Copyright 2019-2024 Diligent Graphics LLC
33
* Copyright 2015-2019 Egor Yusov
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
@@ -207,7 +207,10 @@ TEST(ShaderResourceLayout, ResourceArray)
207207
ResourceMappingEntry{"g_tex2DTest", pTextures[0]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 0},
208208
ResourceMappingEntry{"g_tex2DTest", pTextures[1]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 1},
209209
ResourceMappingEntry{"g_tex2DTest", pTextures[2]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 2},
210-
ResourceMappingEntry{"g_tex2DTest2", pTextures[5]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 0},
210+
ResourceMappingEntry{"g_tex2DTest2", pTextures[0]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 0}, // Unused
211+
ResourceMappingEntry{"g_tex2DTest2", pTextures[5]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 1},
212+
ResourceMappingEntry{"g_tex2DTest2", pTextures[0]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 2}, // Unused
213+
ResourceMappingEntry{"g_tex2DTest2", pTextures[0]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 3}, // Unused
211214
ResourceMappingEntry{"g_tex2D", pTextures[6]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 0}
212215
};
213216
// clang-format on
@@ -224,7 +227,7 @@ TEST(ShaderResourceLayout, ResourceArray)
224227
EXPECT_EQ(pSRB->CheckResources(SHADER_TYPE_VERTEX | SHADER_TYPE_PIXEL, pResMapping, BIND_SHADER_RESOURCES_UPDATE_ALL), SHADER_RESOURCE_VARIABLE_TYPE_FLAG_MUT_DYN);
225228
EXPECT_EQ(pSRB->CheckResources(SHADER_TYPE_VERTEX | SHADER_TYPE_PIXEL, pResMapping, BIND_SHADER_RESOURCES_VERIFY_ALL_RESOLVED), SHADER_RESOURCE_VARIABLE_TYPE_FLAG_MUT_DYN);
226229

227-
pPSO->GetStaticVariableByName(SHADER_TYPE_PIXEL, "g_tex2DTest2")->SetArray(ppSRVs, 1, 1);
230+
pPSO->GetStaticVariableByName(SHADER_TYPE_PIXEL, "g_tex2DTest2")->SetArray(ppSRVs, 4, 1);
228231

229232
pPSO->InitializeStaticSRBResources(pSRB);
230233
pSRB->BindResources(SHADER_TYPE_PIXEL, pResMapping, BIND_SHADER_RESOURCES_KEEP_EXISTING | BIND_SHADER_RESOURCES_UPDATE_MUTABLE | BIND_SHADER_RESOURCES_UPDATE_DYNAMIC);

0 commit comments

Comments
 (0)