From 6cf12db4826c1cc4c2d481d44b1eb6de1e39b774 Mon Sep 17 00:00:00 2001 From: Egor Yusov Date: Sat, 5 May 2018 15:00:32 -0700 Subject: Fixed GLSL shader compilation to generate bindings and descriptor sets; updated SPIRV resource loading --- Graphics/GLSLTools/include/SPIRVShaderResources.h | 44 ++++---- Graphics/GLSLTools/src/GLSL2SPIRV.cpp | 111 +++++++++++++++++++-- Graphics/GLSLTools/src/SPIRVShaderResources.cpp | 99 +++++++++++------- .../GraphicsEngineVulkan/include/PipelineLayout.h | 12 --- .../include/PipelineStateVkImpl.h | 5 +- .../GraphicsEngineVulkan/src/PipelineLayout.cpp | 6 -- .../src/PipelineStateVkImpl.cpp | 9 +- 7 files changed, 195 insertions(+), 91 deletions(-) (limited to 'Graphics') diff --git a/Graphics/GLSLTools/include/SPIRVShaderResources.h b/Graphics/GLSLTools/include/SPIRVShaderResources.h index 0b740b14..9d952a97 100644 --- a/Graphics/GLSLTools/include/SPIRVShaderResources.h +++ b/Graphics/GLSLTools/include/SPIRVShaderResources.h @@ -39,6 +39,12 @@ #include "STDAllocator.h" #include "HashUtils.h" +namespace spirv_cross +{ +class Compiler; +struct Resource; +} + namespace Diligent { @@ -73,25 +79,19 @@ struct SPIRVShaderResourceAttribs }; const String Name; - const Uint8 Binding; - const Uint8 Location; + + const Uint16 Binding; const Uint16 ArraySize; - const ResourceType Type; - const SHADER_VARIABLE_TYPE VarType : 8; - - SPIRVShaderResourceAttribs(String _Name, - Uint8 _Binding, - Uint8 _Location, - Uint16 _ArraySize, - ResourceType _Type, - SHADER_VARIABLE_TYPE _VarType) : - Name(std::move(_Name)), - Binding(_Binding), - Location(_Location), - ArraySize(_ArraySize), - Type(_Type), - VarType(_VarType) - {} + + // offset in SPIRV words (uint32_t) for a decoration which was originally declared in the SPIRV binary + const uint32_t BindingDecorationOffset; + const uint32_t DescriptorSetDecorationOffset; + + const Uint8 DescriptorSet; + const ResourceType Type : 4; + const SHADER_VARIABLE_TYPE VarType : 4; + + SPIRVShaderResourceAttribs(const spirv_cross::Compiler &Compiler, const spirv_cross::Resource &Res, ResourceType _Type, SHADER_VARIABLE_TYPE _VarType); String GetPrintName(Uint32 ArrayInd)const { @@ -104,10 +104,10 @@ struct SPIRVShaderResourceAttribs bool IsCompatibleWith(const SPIRVShaderResourceAttribs& Attibs)const { - return Binding == Attibs.Binding && - Location == Attibs.Location && - ArraySize == Attibs.ArraySize && - Type == Attibs.Type; + return Binding == Attibs.Binding && + DescriptorSet == Attibs.DescriptorSet && + ArraySize == Attibs.ArraySize && + Type == Attibs.Type; } }; diff --git a/Graphics/GLSLTools/src/GLSL2SPIRV.cpp b/Graphics/GLSLTools/src/GLSL2SPIRV.cpp index 69cadd2c..81f2009a 100644 --- a/Graphics/GLSLTools/src/GLSL2SPIRV.cpp +++ b/Graphics/GLSLTools/src/GLSL2SPIRV.cpp @@ -173,10 +173,102 @@ TBuiltInResource InitResources() return Resources; } -std::vector GLSLtoSPIRV(const SHADER_TYPE ShaderType, const char *ShaderSource) +class IoMapResolver : public glslang::TIoMapResolver { - std::vector spirv; +public: + // Should return true if the resulting/current binding would be okay. + // Basic idea is to do aliasing binding checks with this. + virtual bool validateBinding(EShLanguage stage, const char* name, const glslang::TType& type, bool is_live) + { + return true; + } + + // Should return a value >= 0 if the current binding should be overridden. + // Return -1 if the current binding (including no binding) should be kept. + virtual int resolveBinding(EShLanguage stage, const char* name, const glslang::TType& type, bool is_live) + { + // We do not care about actual binding value here. + // We only need decoration to be present in SPIRV + return 0; + } + + // Should return a value >= 0 if the current set should be overridden. + // Return -1 if the current set (including no set) should be kept. + virtual int resolveSet(EShLanguage stage, const char* name, const glslang::TType& type, bool is_live) + { + // We do not care about actual descriptor set value here. + // We only need decoration to be present in SPIRV + return 0; + } + + // Should return a value >= 0 if the current location should be overridden. + // Return -1 if the current location (including no location) should be kept. + virtual int resolveUniformLocation(EShLanguage stage, const char* name, const glslang::TType& type, bool is_live) + { + return -1; + } + + // Should return true if the resulting/current setup would be okay. + // Basic idea is to do aliasing checks and reject invalid semantic names. + virtual bool validateInOut(EShLanguage stage, const char* name, const glslang::TType& type, bool is_live) + { + return true; + } + + // Should return a value >= 0 if the current location should be overridden. + // Return -1 if the current location (including no location) should be kept. + virtual int resolveInOutLocation(EShLanguage stage, const char* name, const glslang::TType& type, bool is_live) + { + return -1; + } + + // Should return a value >= 0 if the current component index should be overridden. + // Return -1 if the current component index (including no index) should be kept. + virtual int resolveInOutComponent(EShLanguage stage, const char* name, const glslang::TType& type, bool is_live) + { + return -1; + } + + // Should return a value >= 0 if the current color index should be overridden. + // Return -1 if the current color index (including no index) should be kept. + virtual int resolveInOutIndex(EShLanguage stage, const char* name, const glslang::TType& type, bool is_live) + { + return -1; + } + // Notification of a uniform variable + virtual void notifyBinding(EShLanguage stage, const char* name, const glslang::TType& type, bool is_live) + { + } + + // Notification of a in or out variable + virtual void notifyInOut(EShLanguage stage, const char* name, const glslang::TType& type, bool is_live) + { + } + + // Called by mapIO when it has finished the notify pass + virtual void endNotifications(EShLanguage stage) + { + } + + // Called by mapIO when it starts its notify pass for the given stage + virtual void beginNotifications(EShLanguage stage) + { + } + + // Called by mipIO when it starts its resolve pass for the given stage + virtual void beginResolve(EShLanguage stage) + { + } + + // Called by mapIO when it has finished the resolve pass + virtual void endResolve(EShLanguage stage) + { + } +}; + +std::vector GLSLtoSPIRV(const SHADER_TYPE ShaderType, const char *ShaderSource) +{ #if PLATFORM_ANDROID // On Android, use shaderc instead. @@ -187,6 +279,7 @@ std::vector GLSLtoSPIRV(const SHADER_TYPE ShaderType, const char * LOGE("Error: Id=%d, Msg=%s", module.GetCompilationStatus(), module.GetErrorMessage().c_str()); return false; } + std::vector spirv; spirv.assign(module.cbegin(), module.cend()); #else @@ -202,25 +295,25 @@ std::vector GLSLtoSPIRV(const SHADER_TYPE ShaderType, const char * Shader.setStrings(ShaderStrings, 1); Shader.setAutoMapBindings(true); - Shader.setAutoMapLocations(true); if (!Shader.parse(&Resources, 100, false, messages)) { LOG_ERROR_MESSAGE("Failed to parse shader source: \n", Shader.getInfoLog(), '\n', Shader.getInfoDebugLog()); - return std::move(spirv); + return {}; } glslang::TProgram Program; Program.addShader(&Shader); - - // - // Program-level processing... - // if (!Program.link(messages)) { LOG_ERROR_MESSAGE("Failed to link program: \n", Shader.getInfoLog(), '\n', Shader.getInfoDebugLog()); - return std::move(spirv); + return {}; } + IoMapResolver Resovler; + // This step is essential to set bindings and descriptor sets + Program.mapIO(&Resovler); + + std::vector spirv; glslang::GlslangToSpv(*Program.getIntermediate(ShLang), spirv); #endif diff --git a/Graphics/GLSLTools/src/SPIRVShaderResources.cpp b/Graphics/GLSLTools/src/SPIRVShaderResources.cpp index 36fd9d4f..1e2a5865 100644 --- a/Graphics/GLSLTools/src/SPIRVShaderResources.cpp +++ b/Graphics/GLSLTools/src/SPIRVShaderResources.cpp @@ -27,11 +27,57 @@ namespace Diligent { +template +Type GetResourceArraySize(const spirv_cross::Compiler &Compiler, + const spirv_cross::Resource &Res) +{ + const auto& type = Compiler.get_type(Res.type_id); + uint32_t arrSize = !type.array.empty() ? type.array[0] : 1; + VERIFY(arrSize <= std::numeric_limits::max(), "Array size exceeds maximum representable value ", std::numeric_limits::max()); + return static_cast(arrSize); +} + +static uint32_t GetDecorationOffset(const spirv_cross::Compiler &Compiler, + const spirv_cross::Resource &Res, + spv::Decoration Decoration) +{ + VERIFY(Compiler.has_decoration(Res.id, Decoration), "Res \'", Res.name, "\' has no requested decoration"); + uint32_t offset = 0; + auto declared = Compiler.get_binary_offset_for_decoration(Res.id, Decoration, offset); + VERIFY(declared, "Requested decoration is not declared"); + return offset; +} + +template +static Type GetDecoration(const spirv_cross::Compiler &Compiler, + const spirv_cross::Resource &Res, + spv::Decoration Decoration) +{ + auto dec = Compiler.get_decoration(Res.id, Decoration); + VERIFY(dec <= std::numeric_limits::max(), "Decoration value exceeds maximum representable value ", std::numeric_limits::max()); + return static_cast(dec); +} + +SPIRVShaderResourceAttribs::SPIRVShaderResourceAttribs(const spirv_cross::Compiler &Compiler, + const spirv_cross::Resource &Res, + ResourceType _Type, + SHADER_VARIABLE_TYPE _VarType) : + Name(Res.name), + Binding(GetDecoration(Compiler, Res, spv::DecorationBinding)), + ArraySize(GetResourceArraySize(Compiler, Res)), + BindingDecorationOffset(GetDecorationOffset(Compiler, Res, spv::Decoration::DecorationBinding)), + DescriptorSetDecorationOffset(GetDecorationOffset(Compiler, Res, spv::Decoration::DecorationDescriptorSet)), + DescriptorSet(GetDecoration(Compiler,Res, spv::DecorationDescriptorSet)), + Type(_Type), + VarType(_VarType) +{ +} + SPIRVShaderResources::SPIRVShaderResources(IMemoryAllocator &Allocator, SHADER_TYPE ShaderType, std::vector spirv_binary) : m_MemoryBuffer(nullptr, STDDeleterRawMem(Allocator)), m_ShaderType(ShaderType) { - spirv_cross::Compiler Compiler(std::move(spirv_binary)); + spirv_cross::Compiler Compiler(spirv_binary); // The SPIR-V is now parsed, and we can perform reflection on it. spirv_cross::ShaderResources resources = Compiler.get_shader_resources(); @@ -46,68 +92,49 @@ SPIRVShaderResources::SPIRVShaderResources(IMemoryAllocator &Allocator, SHADER_T static_cast(resources.separate_samplers.size()) ); + Uint32 CurrUB = 0, CurrSB = 0, CurrImg = 0, CurrSmplImg = 0, CurrAC = 0, CurrSepImg = 0, CurrSepSmpl = 0; for (const auto &UB : resources.uniform_buffers) { - UB.name; - unsigned location = Compiler.get_decoration(UB.id, spv::DecorationLocation); - unsigned binding = Compiler.get_decoration(UB.id, spv::DecorationBinding); - - int a=0; + new (&GetUB(CurrUB++)) SPIRVShaderResourceAttribs(Compiler, UB, SPIRVShaderResourceAttribs::ResourceType::UniformBuffer, SHADER_VARIABLE_TYPE_STATIC); } for (const auto &SB : resources.storage_buffers) { - SB.name; - unsigned location = Compiler.get_decoration(SB.id, spv::DecorationLocation); - unsigned binding = Compiler.get_decoration(SB.id, spv::DecorationBinding); - - int a = 0; + new (&GetSB(CurrSB++)) SPIRVShaderResourceAttribs(Compiler, SB, SPIRVShaderResourceAttribs::ResourceType::StorageBuffer, SHADER_VARIABLE_TYPE_STATIC); } for (const auto &SmplImg : resources.sampled_images) { - SmplImg.name; - unsigned location = Compiler.get_decoration(SmplImg.id, spv::DecorationLocation); - unsigned binding = Compiler.get_decoration(SmplImg.id, spv::DecorationBinding); - - int a = 0; + new (&GetSmplImg(CurrSmplImg++)) SPIRVShaderResourceAttribs(Compiler, SmplImg, SPIRVShaderResourceAttribs::ResourceType::SampledImage, SHADER_VARIABLE_TYPE_STATIC); } for (const auto &Img : resources.storage_images) { - Img.name; - unsigned location = Compiler.get_decoration(Img.id, spv::DecorationLocation); - unsigned binding = Compiler.get_decoration(Img.id, spv::DecorationBinding); - - int a = 0; + new (&GetImg(CurrImg++)) SPIRVShaderResourceAttribs(Compiler, Img, SPIRVShaderResourceAttribs::ResourceType::StorageImage, SHADER_VARIABLE_TYPE_STATIC); } for (const auto &AC : resources.atomic_counters) { - AC.name; - unsigned location = Compiler.get_decoration(AC.id, spv::DecorationLocation); - unsigned binding = Compiler.get_decoration(AC.id, spv::DecorationBinding); - - int a = 0; + new (&GetAC(CurrAC++)) SPIRVShaderResourceAttribs(Compiler, AC, SPIRVShaderResourceAttribs::ResourceType::AtomicCounter, SHADER_VARIABLE_TYPE_STATIC); } for (const auto &SepImg : resources.separate_images) { - SepImg.name; - unsigned location = Compiler.get_decoration(SepImg.id, spv::DecorationLocation); - unsigned binding = Compiler.get_decoration(SepImg.id, spv::DecorationBinding); - - int a = 0; + new (&GetSepImg(CurrSepImg++)) SPIRVShaderResourceAttribs(Compiler, SepImg, SPIRVShaderResourceAttribs::ResourceType::SeparateImage, SHADER_VARIABLE_TYPE_STATIC); } for (const auto &SepSam : resources.separate_samplers) { - SepSam.name; - unsigned location = Compiler.get_decoration(SepSam.id, spv::DecorationLocation); - unsigned binding = Compiler.get_decoration(SepSam.id, spv::DecorationBinding); - - int a = 0; + new (&GetSepSmpl(CurrSepSmpl++)) SPIRVShaderResourceAttribs(Compiler, SepSam, SPIRVShaderResourceAttribs::ResourceType::SeparateSampler, SHADER_VARIABLE_TYPE_STATIC); } + + VERIFY_EXPR(CurrUB == GetNumUBs()); + VERIFY_EXPR(CurrSB == GetNumSBs()); + VERIFY_EXPR(CurrImg == GetNumImgs()); + VERIFY_EXPR(CurrSmplImg == GetNumSmplImgs()); + VERIFY_EXPR(CurrAC == GetNumACs()); + VERIFY_EXPR(CurrSepImg == GetNumSepImgs()); + VERIFY_EXPR(CurrSepSmpl == GetNumSepSmpls()); } SPIRVShaderResources::SPIRVShaderResources(IMemoryAllocator &Allocator, diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h b/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h index efe4caef..625e1f3d 100644 --- a/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h +++ b/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h @@ -160,18 +160,6 @@ private: uint8_t m_ActiveSets = 0; }; -#if 0 - static constexpr Uint8 InvalidRootTableIndex = static_cast(-1); - - // The array below contains array index of a CBV/SRV/UAV root table - // in m_RootParams (NOT the Root Index!), for every variable type - // (static, mutable, dynamic) and every shader type, - // or -1, if the table is not yet assigned to the combination - Uint8 m_SrvCbvUavRootTablesMap[SHADER_VARIABLE_TYPE_NUM_TYPES * 6]; - // This array contains the same data for Sampler root table - Uint8 m_SamplerRootTablesMap[SHADER_VARIABLE_TYPE_NUM_TYPES * 6]; -#endif - IMemoryAllocator &m_MemAllocator; DescriptorSetLayoutManager m_LayoutMgr; diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineStateVkImpl.h b/Graphics/GraphicsEngineVulkan/include/PipelineStateVkImpl.h index 0ebfe80c..5ec8d260 100644 --- a/Graphics/GraphicsEngineVulkan/include/PipelineStateVkImpl.h +++ b/Graphics/GraphicsEngineVulkan/include/PipelineStateVkImpl.h @@ -33,6 +33,7 @@ #include "ShaderResourceLayoutVk.h" #include "AdaptiveFixedBlockAllocator.h" #include "VulkanUtilities/VulkanObjectWrappers.h" +#include "PipelineLayout.h" /// Namespace for the Direct3D11 implementation of the graphics engine namespace Diligent @@ -127,8 +128,8 @@ private: #endif VulkanUtilities::RenderPassWrapper m_RenderPass; VulkanUtilities::PipelineWrapper m_Pipeline; - VulkanUtilities::PipelineLayoutWrapper m_PipelineLayout; - + PipelineLayout m_PipelineLayout; + VulkanUtilities::PipelineLayoutWrapper m_TmpPipelineLayout; }; } diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp index ab69114e..c7a80ace 100644 --- a/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp +++ b/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp @@ -301,12 +301,6 @@ PipelineLayout::PipelineLayout() : m_StaticSamplers( STD_ALLOCATOR_RAW_MEM(StaticSamplerAttribs, GetRawAllocator(), "Allocator for vector") ) */ { -#if 0 - for(size_t i=0; i < _countof(m_SrvCbvUavRootTablesMap); ++i) - m_SrvCbvUavRootTablesMap[i] = InvalidRootTableIndex; - for(size_t i=0; i < _countof(m_SamplerRootTablesMap); ++i) - m_SamplerRootTablesMap[i] = InvalidRootTableIndex; -#endif } void PipelineLayout::Release(RenderDeviceVkImpl *pDeviceVkImpl) diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp index 8fccd1e8..17f4bcda 100644 --- a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp @@ -144,7 +144,7 @@ void PipelineStateVkImpl::CreateRenderPass(const VulkanUtilities::VulkanLogicalD PipelineStateVkImpl :: PipelineStateVkImpl(IReferenceCounters *pRefCounters, RenderDeviceVkImpl *pDeviceVk, const PipelineStateDesc &PipelineDesc) : TPipelineStateBase(pRefCounters, pDeviceVk, PipelineDesc)/*, - m_DummyVar(*this), + m_DummyVar(*this) m_ResourceCacheDataAllocator(GetRawAllocator(), PipelineDesc.SRBAllocationGranularity), m_pDefaultShaderResBinding(nullptr, STDDeleter(pDeviceVk->GetSRBAllocator()) ) */ @@ -353,8 +353,8 @@ PipelineStateVkImpl :: PipelineStateVkImpl(IReferenceCounters *pRefCounters, Ren PipelineLayoutCI.pSetLayouts = nullptr; PipelineLayoutCI.pushConstantRangeCount = 0; PipelineLayoutCI.pPushConstantRanges = nullptr; - m_PipelineLayout = LogicalDevice.CreatePipelineLayout(PipelineLayoutCI); - PipelineCI.layout = m_PipelineLayout; + m_TmpPipelineLayout = LogicalDevice.CreatePipelineLayout(PipelineLayoutCI); + PipelineCI.layout = m_TmpPipelineLayout; PipelineCI.renderPass = m_RenderPass; PipelineCI.subpass = 0; @@ -423,7 +423,8 @@ PipelineStateVkImpl::~PipelineStateVkImpl() pDeviceVkImpl->GetFramebufferCache().OnDestroyRenderPass(m_RenderPass); pDeviceVkImpl->SafeReleaseVkObject(std::move(m_RenderPass)); pDeviceVkImpl->SafeReleaseVkObject(std::move(m_Pipeline)); - pDeviceVkImpl->SafeReleaseVkObject(std::move(m_PipelineLayout)); + m_PipelineLayout.Release(pDeviceVkImpl); + //pDeviceVkImpl->SafeReleaseVkObject(std::move(m_TmpPipelineLayout)); #if 0 auto &ShaderResLayoutAllocator = GetRawAllocator(); -- cgit v1.2.3