Skip to content

Commit

Permalink
D3DShaderResourceLoader: fixed handling resource arrays with the opti…
Browse files Browse the repository at this point in the history 
…mized-out first element
  • Loading branch information
@TheMostDiligent
TheMostDiligent committed Jan 22, 2024
1 parent 2e6e90b commit 2c8faca
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 46 deletions.
51 changes: 28 additions & 23 deletions Graphics/GraphicsEngineD3DBase/include/D3DShaderResourceLoader.hpp
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2019-2023 Diligent Graphics LLC
* Copyright 2019-2024 Diligent Graphics LLC
* Copyright 2015-2019 Egor Yusov
*
* Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -36,6 +36,7 @@
#include "ShaderToolsCommon.hpp"
#include "D3DCommonTypeConversions.hpp"
#include "EngineMemory.h"
#include "ParsingTools.hpp"

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

std::string Name;
const auto ArrayIndex = Parsing::GetArrayIndex(BindingDesc.Name, Name);

if (BindingDesc.BindPoint == UINT32_MAX)
{
BindingDesc.BindPoint = D3DShaderResourceAttribs::InvalidBindPoint;

std::string Name{BindingDesc.Name};

SkipCount = 1;
}
else if (ArrayIndex > 0)
{
// Adjust bind point for array index
VERIFY(BindingDesc.BindPoint >= static_cast<UINT>(ArrayIndex), "Resource '", BindingDesc.Name, "' uses bind point point ", BindingDesc.BindPoint,
", which is invalid for its array index ", ArrayIndex);
BindingDesc.BindPoint -= static_cast<UINT>(ArrayIndex);
}

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

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

// Name == "g_tex2DDiffuse[0]"
// ^
// OpenBracketPos
Name.erase(OpenBracketPos, Name.length() - OpenBracketPos);
// Name == "g_tex2DDiffuse"
VERIFY_EXPR(Name.length() == OpenBracketPos);
#ifdef DILIGENT_DEBUG
for (const auto& ExistingRes : Resources)
{
Expand All @@ -236,21 +239,23 @@ void LoadD3DShaderResources(TShaderReflection* pShaderReflection,
#endif
for (UINT ArrElem = Res + 1; ArrElem < shaderDesc.BoundResources; ++ArrElem)
{
D3D_SHADER_INPUT_BIND_DESC ArrElemBindingDesc = {};
pShaderReflection->GetResourceBindingDesc(ArrElem, &ArrElemBindingDesc);
D3D_SHADER_INPUT_BIND_DESC NextElemBindingDesc = {};
pShaderReflection->GetResourceBindingDesc(ArrElem, &NextElemBindingDesc);

std::string NextElemName;
const auto NextElemIndex = Parsing::GetArrayIndex(NextElemBindingDesc.Name, NextElemName);

// Make sure this case is handled correctly:
// "g_tex2DDiffuse[.]" != "g_tex2DDiffuse2[.]"
if (strncmp(Name.c_str(), ArrElemBindingDesc.Name, OpenBracketPos) == 0 && ArrElemBindingDesc.Name[OpenBracketPos] == '[')
if (Name == NextElemName)
{
//g_tex2DDiffuse[2]
// ^
UINT Ind = atoi(ArrElemBindingDesc.Name + OpenBracketPos + 1);
BindCount = std::max(BindCount, Ind + 1);
VERIFY(ArrElemBindingDesc.BindPoint == BindingDesc.BindPoint + Ind,
VERIFY_EXPR(NextElemIndex > 0);

BindCount = std::max(BindCount, static_cast<UINT>(NextElemIndex) + 1);
VERIFY(NextElemBindingDesc.BindPoint == BindingDesc.BindPoint + NextElemIndex,
"Array elements are expected to use contiguous bind points.\n",
BindingDesc.Name, " uses slot ", BindingDesc.BindPoint, ", so ", ArrElemBindingDesc.Name,
" is expected to use slot ", BindingDesc.BindPoint + Ind, " while ", ArrElemBindingDesc.BindPoint,
BindingDesc.Name, " uses slot ", BindingDesc.BindPoint, ", so ", NextElemBindingDesc.Name,
" is expected to use slot ", BindingDesc.BindPoint + NextElemIndex, " while ", NextElemBindingDesc.BindPoint,
" is actually used");

// Note that skip count may not necessarily be the same as BindCount.
Expand Down
21 changes: 4 additions & 17 deletions Graphics/GraphicsEngineOpenGL/src/ShaderResourcesGL.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2019-2022 Diligent Graphics LLC
* Copyright 2019-2024 Diligent Graphics LLC
* Copyright 2015-2019 Egor Yusov
*
* Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -39,6 +39,7 @@
#include "Align.hpp"
#include "GLTypeConversions.hpp"
#include "ShaderToolsCommon.hpp"
#include "ParsingTools.hpp"

namespace Diligent
{
Expand Down Expand Up @@ -284,20 +285,6 @@ static void AddUniformBufferVariable(GLuint
}
}

int GetArrayIndex(const char* VarName, std::string& NameWithoutBrackets)
{
NameWithoutBrackets = VarName;
const auto* OpenBracketPtr = strchr(VarName, '[');
if (OpenBracketPtr == nullptr)
return -1;

auto Ind = atoi(OpenBracketPtr + 1);
if (Ind >= 0)
NameWithoutBrackets = std::string{VarName, OpenBracketPtr};

return Ind;
}

static void ProcessUBVariable(ShaderCodeVariableDescX& Var, Uint32 BaseOffset)
{
// Re-sort by offset
Expand Down Expand Up @@ -343,7 +330,7 @@ static ShaderCodeBufferDescX PrepareUBReflection(std::vector<ShaderCodeVariableD
// Dot
std::string Prefix{Name, Dot}; // "s2[1]"
std::string NameWithoutBrackets;
const auto ArrayInd = GetArrayIndex(Prefix.c_str(), NameWithoutBrackets);
const auto ArrayInd = Parsing::GetArrayIndex(Prefix, NameWithoutBrackets);
// ArrayInd = 1
// NameWithoutBrackets = "s2"

Expand Down Expand Up @@ -414,7 +401,7 @@ static ShaderCodeBufferDescX PrepareUBReflection(std::vector<ShaderCodeVariableD
// Name

std::string NameWithoutBrackets;
const auto ArrayInd = GetArrayIndex(Name, NameWithoutBrackets);
const auto ArrayInd = Parsing::GetArrayIndex(Name, NameWithoutBrackets);
// ArrayInd = 2
// NameWithoutBrackets = "f4"
if (auto* pArray = pLevel->FindMember(NameWithoutBrackets.c_str()))
Expand Down
6 changes: 3 additions & 3 deletions Tests/DiligentCoreAPITest/assets/shaders/ShaderResourceArrayTest.psh
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

Texture2D<float4> g_tex2DTest[4];
Texture2D<float4> g_tex2DTest2[2];
Texture2D<float4> g_tex2DTest2[5]; // [0], [2] and [3] are not used
Texture2D<float4> g_tex2D[2];
SamplerState g_tex2DTest_sampler[4];
SamplerState g_tex2DTest2_sampler;
Expand All @@ -16,8 +16,8 @@ void main(in float4 f4Position : SV_Position,
in VSOut vsOut,
out float4 Color : SV_Target)
{
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;
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;
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;
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;
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;
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;
if( vsOut.f2UV.x < 0.5 && vsOut.f2UV.y < 0.5 )
Expand Down
9 changes: 6 additions & 3 deletions Tests/DiligentCoreAPITest/src/ShaderResourceArrayTest.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2019-2023 Diligent Graphics LLC
* Copyright 2019-2024 Diligent Graphics LLC
* Copyright 2015-2019 Egor Yusov
*
* Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -207,7 +207,10 @@ TEST(ShaderResourceLayout, ResourceArray)
ResourceMappingEntry{"g_tex2DTest", pTextures[0]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 0},
ResourceMappingEntry{"g_tex2DTest", pTextures[1]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 1},
ResourceMappingEntry{"g_tex2DTest", pTextures[2]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 2},
ResourceMappingEntry{"g_tex2DTest2", pTextures[5]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 0},
ResourceMappingEntry{"g_tex2DTest2", pTextures[0]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 0}, // Unused
ResourceMappingEntry{"g_tex2DTest2", pTextures[5]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 1},
ResourceMappingEntry{"g_tex2DTest2", pTextures[0]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 2}, // Unused
ResourceMappingEntry{"g_tex2DTest2", pTextures[0]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 3}, // Unused
ResourceMappingEntry{"g_tex2D", pTextures[6]->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE), 0}
};
// clang-format on
Expand All @@ -224,7 +227,7 @@ TEST(ShaderResourceLayout, ResourceArray)
EXPECT_EQ(pSRB->CheckResources(SHADER_TYPE_VERTEX | SHADER_TYPE_PIXEL, pResMapping, BIND_SHADER_RESOURCES_UPDATE_ALL), SHADER_RESOURCE_VARIABLE_TYPE_FLAG_MUT_DYN);
EXPECT_EQ(pSRB->CheckResources(SHADER_TYPE_VERTEX | SHADER_TYPE_PIXEL, pResMapping, BIND_SHADER_RESOURCES_VERIFY_ALL_RESOLVED), SHADER_RESOURCE_VARIABLE_TYPE_FLAG_MUT_DYN);

pPSO->GetStaticVariableByName(SHADER_TYPE_PIXEL, "g_tex2DTest2")->SetArray(ppSRVs, 1, 1);
pPSO->GetStaticVariableByName(SHADER_TYPE_PIXEL, "g_tex2DTest2")->SetArray(ppSRVs, 4, 1);

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

0 comments on commit 2c8faca

Please sign in to comment.