From fbcafa67111e79752b96866bf42fce31836ee442 Mon Sep 17 00:00:00 2001 From: Egor Yusov Date: Thu, 30 Aug 2018 09:31:11 -0700 Subject: Fixed shader parsing errors caused by incorrect representation of signed enums as bit fields --- Graphics/GLSLTools/include/SPIRVShaderResources.h | 9 ++- Graphics/GraphicsEngine/interface/Shader.h | 10 +-- .../src/ShaderResourceLayoutD3D11.cpp | 30 ++++---- .../include/ShaderVariableD3D12.h | 2 +- Graphics/GraphicsEngineD3D12/src/RootSignature.cpp | 4 +- .../src/ShaderResourceLayoutD3D12.cpp | 76 ++++++++++---------- .../include/D3DShaderResourceLoader.h | 15 ++-- .../include/ShaderResources.h | 81 ++++++++++++++-------- .../include/ShaderVariableD3DBase.h | 2 +- .../GraphicsEngineD3DBase/src/ShaderResources.cpp | 14 ++-- 10 files changed, 140 insertions(+), 103 deletions(-) (limited to 'Graphics') diff --git a/Graphics/GLSLTools/include/SPIRVShaderResources.h b/Graphics/GLSLTools/include/SPIRVShaderResources.h index 58e003e2..d87f1f39 100644 --- a/Graphics/GLSLTools/include/SPIRVShaderResources.h +++ b/Graphics/GLSLTools/include/SPIRVShaderResources.h @@ -86,10 +86,15 @@ struct SPIRVShaderResourceAttribs NumResourceTypes }; + static constexpr const Uint32 ResourceTypeBits = 4; + static constexpr const Uint32 VarTypeBits = 4; + static_assert(SHADER_VARIABLE_TYPE_NUM_TYPES < (1 << VarTypeBits), "Not enough bits to represent SHADER_VARIABLE_TYPE"); + static_assert(NumResourceTypes < (1 << ResourceTypeBits), "Not enough bits to represent ResourceType"); + const char *Name; const Uint16 ArraySize; - const ResourceType Type : 4; - const SHADER_VARIABLE_TYPE VarType : 4; + const ResourceType Type : ResourceTypeBits; + const SHADER_VARIABLE_TYPE VarType : VarTypeBits; const Int8 StaticSamplerInd; // Offset in SPIRV words (uint32_t) of binding & descriptor set decorations in SPIRV binary diff --git a/Graphics/GraphicsEngine/interface/Shader.h b/Graphics/GraphicsEngine/interface/Shader.h index b89d840c..2dcf65c5 100644 --- a/Graphics/GraphicsEngine/interface/Shader.h +++ b/Graphics/GraphicsEngine/interface/Shader.h @@ -43,7 +43,7 @@ static constexpr INTERFACE_ID IID_ShaderVariable = { 0xd57df3f, 0x977d, 0x4c8f, { 0xb6, 0x4c, 0x66, 0x75, 0x81, 0x4b, 0xc8, 0xc } }; /// Describes the shader type -enum SHADER_TYPE : Int32 +enum SHADER_TYPE : Uint32 { SHADER_TYPE_UNKNOWN = 0x000, ///< Unknown shader type SHADER_TYPE_VERTEX = 0x001, ///< Vertex shader @@ -54,7 +54,7 @@ enum SHADER_TYPE : Int32 SHADER_TYPE_COMPUTE = 0x020 ///< Compute shader }; -enum SHADER_PROFILE : Int32 +enum SHADER_PROFILE : Uint8 { SHADER_PROFILE_DEFAULT = 0, SHADER_PROFILE_DX_4_0, @@ -64,7 +64,7 @@ enum SHADER_PROFILE : Int32 }; /// Describes shader source code language -enum SHADER_SOURCE_LANGUAGE : Int32 +enum SHADER_SOURCE_LANGUAGE : Uint32 { /// Default language (GLSL for OpenGL/OpenGLES devices, HLSL for Direct3D11/Direct3D12 devices) SHADER_SOURCE_LANGUAGE_DEFAULT = 0, @@ -78,7 +78,7 @@ enum SHADER_SOURCE_LANGUAGE : Int32 /// Describes flags that can be supplied to IShader::BindResources() /// and IDeviceContext::BindShaderResources(). -enum BIND_SHADER_RESOURCES_FLAGS : Int32 +enum BIND_SHADER_RESOURCES_FLAGS : Uint32 { /// Reset all bindings. If this flag is specified, all existing bindings will be /// broken. By default all existing bindings are preserved. @@ -97,7 +97,7 @@ enum BIND_SHADER_RESOURCES_FLAGS : Int32 }; /// Describes shader variable type that is used by ShaderVariableDesc -enum SHADER_VARIABLE_TYPE : Int32 +enum SHADER_VARIABLE_TYPE : Uint8 { /// Shader variable is constant across all shader instances. /// It must be set *once* directly through IShader::BindResources() or through diff --git a/Graphics/GraphicsEngineD3D11/src/ShaderResourceLayoutD3D11.cpp b/Graphics/GraphicsEngineD3D11/src/ShaderResourceLayoutD3D11.cpp index 3d7f549e..62a3746c 100755 --- a/Graphics/GraphicsEngineD3D11/src/ShaderResourceLayoutD3D11.cpp +++ b/Graphics/GraphicsEngineD3D11/src/ShaderResourceLayoutD3D11.cpp @@ -161,7 +161,7 @@ void ShaderResourceLayoutD3D11::Initialize(const std::shared_ptr(CB.BindPoint + CB.BindCount)); @@ -169,7 +169,7 @@ void ShaderResourceLayoutD3D11::Initialize(const std::shared_ptrGetCB(Attribs.BindPoint + ArrayIndex); if( CachedCB.pBuff != nullptr && CachedCB.pBuff != pBuffD3D11Impl) { - auto VarTypeStr = GetShaderVariableTypeLiteralName(Attribs.VariableType); + auto VarTypeStr = GetShaderVariableTypeLiteralName(Attribs.GetVariableType()); LOG_ERROR_MESSAGE( "Non-null constant buffer is already bound to ", VarTypeStr, " shader variable \"", Attribs.GetPrintName(ArrayIndex), "\" in shader \"", m_ParentResLayout.GetShaderName(), "\". Attempting to bind another resource or null is an error and may cause unpredicted behavior. Use another shader resource binding instance or mark shader variable as dynamic." ); } } @@ -480,12 +480,12 @@ void ShaderResourceLayoutD3D11::TexAndSamplerBindInfo::BindResource( IDeviceObje if(pViewD3D11 && !dbgVerifyViewType("texture view", pViewD3D11.RawPtr(), Attribs, ArrayIndex, TEXTURE_VIEW_SHADER_RESOURCE, m_ParentResLayout.GetShaderName())) pViewD3D11.Release(); - if( Attribs.VariableType != SHADER_VARIABLE_TYPE_DYNAMIC) + if( Attribs.GetVariableType() != SHADER_VARIABLE_TYPE_DYNAMIC) { auto &CachedSRV = pResourceCache->GetSRV(Attribs.BindPoint + ArrayIndex); if( CachedSRV.pView != nullptr && CachedSRV.pView != pViewD3D11) { - auto VarTypeStr = GetShaderVariableTypeLiteralName(Attribs.VariableType); + auto VarTypeStr = GetShaderVariableTypeLiteralName(Attribs.GetVariableType()); LOG_ERROR_MESSAGE( "Non-null texture SRV is already bound to ", VarTypeStr, " shader variable \"", Attribs.GetPrintName(ArrayIndex), "\" in shader \"", m_ParentResLayout.GetShaderName(), "\". Attempting to bind another resource or null is an error and may cause unpredicted behavior. Use another shader resource binding instance or mark shader variable as dynamic." ); } } @@ -543,12 +543,12 @@ void ShaderResourceLayoutD3D11::BuffSRVBindInfo::BindResource( IDeviceObject* if(pViewD3D11 && !dbgVerifyViewType("buffer view", pViewD3D11.RawPtr(), Attribs, ArrayIndex, BUFFER_VIEW_SHADER_RESOURCE, m_ParentResLayout.GetShaderName())) pViewD3D11.Release(); - if( Attribs.VariableType != SHADER_VARIABLE_TYPE_DYNAMIC) + if( Attribs.GetVariableType() != SHADER_VARIABLE_TYPE_DYNAMIC) { auto &CachedSRV = pResourceCache->GetSRV(Attribs.BindPoint + ArrayIndex); if( CachedSRV.pView != nullptr && CachedSRV.pView != pViewD3D11) { - auto VarTypeStr = GetShaderVariableTypeLiteralName(Attribs.VariableType); + auto VarTypeStr = GetShaderVariableTypeLiteralName(Attribs.GetVariableType()); LOG_ERROR_MESSAGE( "Non-null buffer SRV is already bound to ", VarTypeStr, " shader variable \"", Attribs.GetPrintName(ArrayIndex), "\" in shader \"", m_ParentResLayout.GetShaderName(), "\". Attempting to bind another resource or null is an error and may cause unpredicted behavior. Use another shader resource binding instance or mark shader variable as dynamic." ); } } @@ -576,12 +576,12 @@ void ShaderResourceLayoutD3D11::TexUAVBindInfo::BindResource( IDeviceObject* if(pViewD3D11 && !dbgVerifyViewType("texture view", pViewD3D11.RawPtr(), Attribs, ArrayIndex, TEXTURE_VIEW_UNORDERED_ACCESS, m_ParentResLayout.GetShaderName())) pViewD3D11.Release(); - if( Attribs.VariableType != SHADER_VARIABLE_TYPE_DYNAMIC) + if( Attribs.GetVariableType() != SHADER_VARIABLE_TYPE_DYNAMIC) { auto &CachedUAV = pResourceCache->GetUAV(Attribs.BindPoint + ArrayIndex); if( CachedUAV.pView != nullptr && CachedUAV.pView != pViewD3D11) { - auto VarTypeStr = GetShaderVariableTypeLiteralName(Attribs.VariableType); + auto VarTypeStr = GetShaderVariableTypeLiteralName(Attribs.GetVariableType()); LOG_ERROR_MESSAGE( "Non-null texture UAV is already bound to ", VarTypeStr, " shader variable \"", Attribs.GetPrintName(ArrayIndex), "\" in shader \"", m_ParentResLayout.GetShaderName(), "\". Attempting to bind another resource or null is an error and may cause unpredicted behavior. Use another shader resource binding instance or mark shader variable as dynamic." ); } } @@ -609,12 +609,12 @@ void ShaderResourceLayoutD3D11::BuffUAVBindInfo::BindResource( IDeviceObject* if(pViewD3D11 && !dbgVerifyViewType("buffer view", pViewD3D11.RawPtr(), Attribs, ArrayIndex, BUFFER_VIEW_UNORDERED_ACCESS, m_ParentResLayout.GetShaderName()) ) pViewD3D11.Release(); - if( Attribs.VariableType != SHADER_VARIABLE_TYPE_DYNAMIC) + if( Attribs.GetVariableType() != SHADER_VARIABLE_TYPE_DYNAMIC) { auto &CachedUAV = pResourceCache->GetUAV(Attribs.BindPoint + ArrayIndex); if( CachedUAV.pView != nullptr && CachedUAV.pView != pViewD3D11) { - auto VarTypeStr = GetShaderVariableTypeLiteralName(Attribs.VariableType); + auto VarTypeStr = GetShaderVariableTypeLiteralName(Attribs.GetVariableType()); LOG_ERROR_MESSAGE( "Non-null buffer UAV is already bound to ", VarTypeStr, " shader variable \"", Attribs.GetPrintName(ArrayIndex), "\" in shader \"", m_ParentResLayout.GetShaderName(), "\". Attempting to bind another resource or null is an error and may cause unpredicted behavior. Use another shader resource binding instance or mark shader variable as dynamic." ); } } diff --git a/Graphics/GraphicsEngineD3D12/include/ShaderVariableD3D12.h b/Graphics/GraphicsEngineD3D12/include/ShaderVariableD3D12.h index ff2c3b9a..8f183161 100644 --- a/Graphics/GraphicsEngineD3D12/include/ShaderVariableD3D12.h +++ b/Graphics/GraphicsEngineD3D12/include/ShaderVariableD3D12.h @@ -163,7 +163,7 @@ public: virtual SHADER_VARIABLE_TYPE GetType()const override final { - return m_Resource.Attribs.VariableType; + return m_Resource.Attribs.GetVariableType(); } virtual void Set(IDeviceObject *pObject)override final diff --git a/Graphics/GraphicsEngineD3D12/src/RootSignature.cpp b/Graphics/GraphicsEngineD3D12/src/RootSignature.cpp index c927e9fa..e68d3682 100644 --- a/Graphics/GraphicsEngineD3D12/src/RootSignature.cpp +++ b/Graphics/GraphicsEngineD3D12/src/RootSignature.cpp @@ -297,12 +297,12 @@ void RootSignature::AllocateResourceSlot(SHADER_TYPE ShaderT OffsetFromTableStart = 0; // Add new root view to existing root parameters - m_RootParams.AddRootView(D3D12_ROOT_PARAMETER_TYPE_CBV, RootIndex, ShaderResAttribs.BindPoint, ShaderVisibility, ShaderResAttribs.VariableType); + m_RootParams.AddRootView(D3D12_ROOT_PARAMETER_TYPE_CBV, RootIndex, ShaderResAttribs.BindPoint, ShaderVisibility, ShaderResAttribs.GetVariableType()); } else { // Use the same table for static and mutable resources. Treat both as static - auto RootTableType = (ShaderResAttribs.VariableType == SHADER_VARIABLE_TYPE_DYNAMIC) ? SHADER_VARIABLE_TYPE_DYNAMIC : SHADER_VARIABLE_TYPE_STATIC; + auto RootTableType = (ShaderResAttribs.GetVariableType() == SHADER_VARIABLE_TYPE_DYNAMIC) ? SHADER_VARIABLE_TYPE_DYNAMIC : SHADER_VARIABLE_TYPE_STATIC; auto TableIndKey = ShaderInd * SHADER_VARIABLE_TYPE_NUM_TYPES + RootTableType; // Get the table array index (this is not the root index!) auto& RootTableArrayInd = (( RangeType == D3D12_DESCRIPTOR_RANGE_TYPE_SAMPLER ) ? m_SamplerRootTablesMap : m_SrvCbvUavRootTablesMap)[ TableIndKey ]; diff --git a/Graphics/GraphicsEngineD3D12/src/ShaderResourceLayoutD3D12.cpp b/Graphics/GraphicsEngineD3D12/src/ShaderResourceLayoutD3D12.cpp index 96a72f95..4c3e125f 100644 --- a/Graphics/GraphicsEngineD3D12/src/ShaderResourceLayoutD3D12.cpp +++ b/Graphics/GraphicsEngineD3D12/src/ShaderResourceLayoutD3D12.cpp @@ -138,19 +138,19 @@ void ShaderResourceLayoutD3D12::Initialize(ID3D12Device* [&](const D3DShaderResourceAttribs& CB, Uint32) { - VERIFY_EXPR(IsAllowedType(CB.VariableType, AllowedTypeBits)); - ++CbvSrvUavCount[CB.VariableType]; + VERIFY_EXPR(CB.IsAllowedType(AllowedTypeBits)); + ++CbvSrvUavCount[CB.GetVariableType()]; }, [&](const D3DShaderResourceAttribs& TexSRV, Uint32) { - auto VarType = TexSRV.VariableType; - VERIFY_EXPR(IsAllowedType(VarType, AllowedTypeBits)); + VERIFY_EXPR(TexSRV.IsAllowedType(AllowedTypeBits)); + auto VarType = TexSRV.GetVariableType(); ++CbvSrvUavCount[VarType]; if(TexSRV.IsValidSampler()) { - auto SamplerId = TexSRV.SamplerId; + auto SamplerId = TexSRV.GetSamplerId(); const auto& SamplerAttribs = m_pResources->GetSampler(SamplerId); - VERIFY(SamplerAttribs.VariableType == VarType, "Texture and sampler variable types are not conistent"); + VERIFY(SamplerAttribs.GetVariableType() == VarType, "Texture and sampler variable types are not conistent"); if(!SamplerAttribs.IsStaticSampler()) { ++SamplerCount[VarType]; @@ -159,18 +159,18 @@ void ShaderResourceLayoutD3D12::Initialize(ID3D12Device* }, [&](const D3DShaderResourceAttribs& TexUAV, Uint32) { - VERIFY_EXPR(IsAllowedType(TexUAV.VariableType, AllowedTypeBits)); - ++CbvSrvUavCount[TexUAV.VariableType]; + VERIFY_EXPR(TexUAV.IsAllowedType(AllowedTypeBits)); + ++CbvSrvUavCount[TexUAV.GetVariableType()]; }, [&](const D3DShaderResourceAttribs& BufSRV, Uint32) { - VERIFY_EXPR(IsAllowedType(BufSRV.VariableType, AllowedTypeBits)); - ++CbvSrvUavCount[BufSRV.VariableType]; + VERIFY_EXPR(BufSRV.IsAllowedType(AllowedTypeBits)); + ++CbvSrvUavCount[BufSRV.GetVariableType()]; }, [&](const D3DShaderResourceAttribs& BufUAV, Uint32) { - VERIFY_EXPR(IsAllowedType(BufUAV.VariableType, AllowedTypeBits)); - ++CbvSrvUavCount[BufUAV.VariableType]; + VERIFY_EXPR(BufUAV.IsAllowedType(AllowedTypeBits)); + ++CbvSrvUavCount[BufUAV.GetVariableType()]; } ); @@ -212,7 +212,7 @@ void ShaderResourceLayoutD3D12::Initialize(ID3D12Device* VERIFY(Offset != D3D12Resource::InvalidOffset, "Offset must be valid"); // Static samplers are never copied, and SamplerId == InvalidSamplerId - ::new (&GetSrvCbvUav(Attribs.VariableType, CurrCbvSrvUav[Attribs.VariableType]++)) D3D12Resource( *this, Attribs, ResType, RootIndex, Offset, SamplerId); + ::new (&GetSrvCbvUav(Attribs.GetVariableType(), CurrCbvSrvUav[Attribs.GetVariableType()]++)) D3D12Resource( *this, Attribs, ResType, RootIndex, Offset, SamplerId); }; @@ -221,23 +221,23 @@ void ShaderResourceLayoutD3D12::Initialize(ID3D12Device* [&](const D3DShaderResourceAttribs& CB, Uint32) { - VERIFY_EXPR( IsAllowedType(CB.VariableType, AllowedTypeBits) ); + VERIFY_EXPR( CB.IsAllowedType(AllowedTypeBits) ); AddResource(CB, CachedResourceType::CBV); }, [&](const D3DShaderResourceAttribs& TexSRV, Uint32) { - auto VarType = TexSRV.VariableType; - VERIFY_EXPR(IsAllowedType(VarType, AllowedTypeBits) ); + VERIFY_EXPR(TexSRV.IsAllowedType(AllowedTypeBits) ); + auto VarType = TexSRV.GetVariableType(); Uint32 SamplerId = D3D12Resource::InvalidSamplerId; if(TexSRV.IsValidSampler()) { - const auto &SrcSamplerAttribs = m_pResources->GetSampler(TexSRV.SamplerId); - VERIFY(SrcSamplerAttribs.VariableType == VarType, "Inconsistent texture and sampler variable types" ); + const auto& SrcSamplerAttribs = m_pResources->GetSampler(TexSRV.GetSamplerId()); + VERIFY(SrcSamplerAttribs.GetVariableType() == VarType, "Inconsistent texture and sampler variable types" ); if (SrcSamplerAttribs.IsStaticSampler()) { - if(pRootSig != nullptr) + if (pRootSig != nullptr) pRootSig->InitStaticSampler(m_pResources->GetShaderType(), TexSRV.Name, SrcSamplerAttribs); // Static samplers are never copied, and SamplerId == InvalidSamplerId @@ -279,17 +279,17 @@ void ShaderResourceLayoutD3D12::Initialize(ID3D12Device* }, [&](const D3DShaderResourceAttribs& TexUAV, Uint32) { - VERIFY_EXPR( IsAllowedType(TexUAV.VariableType, AllowedTypeBits) ); + VERIFY_EXPR( TexUAV.IsAllowedType(AllowedTypeBits) ); AddResource(TexUAV, CachedResourceType::TexUAV); }, [&](const D3DShaderResourceAttribs& BufSRV, Uint32) { - VERIFY_EXPR( IsAllowedType(BufSRV.VariableType, AllowedTypeBits) ); + VERIFY_EXPR( BufSRV.IsAllowedType(AllowedTypeBits) ); AddResource(BufSRV, CachedResourceType::BufSRV); }, [&](const D3DShaderResourceAttribs& BufUAV, Uint32) { - VERIFY_EXPR( IsAllowedType(BufUAV.VariableType, AllowedTypeBits) ); + VERIFY_EXPR( BufUAV.IsAllowedType(AllowedTypeBits) ); AddResource(BufUAV, CachedResourceType::BufUAV); } ); @@ -343,11 +343,11 @@ void ShaderResourceLayoutD3D12::D3D12Resource::CacheCB(IDeviceObject* { if( pBuffD3D12->GetDesc().BindFlags & BIND_UNIFORM_BUFFER ) { - if( Attribs.VariableType != SHADER_VARIABLE_TYPE_DYNAMIC && DstRes.pObject != nullptr ) + if( Attribs.GetVariableType() != SHADER_VARIABLE_TYPE_DYNAMIC && DstRes.pObject != nullptr ) { if(DstRes.pObject != pBuffD3D12) { - auto VarTypeStr = GetShaderVariableTypeLiteralName(Attribs.VariableType); + auto VarTypeStr = GetShaderVariableTypeLiteralName(Attribs.GetVariableType()); LOG_ERROR_MESSAGE( "Non-null constant buffer is already bound to ", VarTypeStr, " shader variable \"", Attribs.GetPrintName(ArrayInd), "\" in shader \"", ParentResLayout.GetShaderName(), "\". Attempring to bind another constant buffer is an error and will be ignored. Use another shader resource binding instance or mark shader variable as dynamic." ); } @@ -432,11 +432,11 @@ void ShaderResourceLayoutD3D12::D3D12Resource::CacheResourceView(IDeviceObject* return; } #endif - if( Attribs.VariableType != SHADER_VARIABLE_TYPE_DYNAMIC && DstRes.pObject != nullptr ) + if( Attribs.GetVariableType() != SHADER_VARIABLE_TYPE_DYNAMIC && DstRes.pObject != nullptr ) { if(DstRes.pObject != pViewD3D12) { - auto VarTypeStr = GetShaderVariableTypeLiteralName(Attribs.VariableType); + auto VarTypeStr = GetShaderVariableTypeLiteralName(Attribs.GetVariableType()); LOG_ERROR_MESSAGE( "Non-null resource is already bound to ", VarTypeStr, " shader variable \"", Attribs.GetPrintName(ArrayIndex), "\" in shader \"", ParentResLayout.GetShaderName(), "\". Attempting to bind another resource or null is an error and will be ignored. Use another shader resource binding instance or mark shader variable as dynamic." ); } @@ -481,11 +481,11 @@ void ShaderResourceLayoutD3D12::D3D12Resource::CacheSampler(ITextureViewD3D12* auto pSampler = pTexViewD3D12->GetSampler(); if( pSampler ) { - if( Attribs.VariableType != SHADER_VARIABLE_TYPE_DYNAMIC && DstSam.pObject != nullptr) + if( Attribs.GetVariableType() != SHADER_VARIABLE_TYPE_DYNAMIC && DstSam.pObject != nullptr) { if(DstSam.pObject != pSampler) { - auto VarTypeStr = GetShaderVariableTypeLiteralName(Attribs.VariableType); + auto VarTypeStr = GetShaderVariableTypeLiteralName(Attribs.GetVariableType()); LOG_ERROR_MESSAGE( "Non-null sampler is already bound to ", VarTypeStr, " shader variable \"", Attribs.GetPrintName(ArrayIndex), "\" in shader \"", ParentResLayout.GetShaderName(), "\". Attempting to bind another sampler is an error and will be ignored. Use another shader resource binding instance or mark shader variable as dynamic." ); } @@ -527,8 +527,8 @@ const ShaderResourceLayoutD3D12::D3D12Resource& ShaderResourceLayoutD3D12::GetAs { VERIFY(TexSrv.GetResType() == CachedResourceType::TexSRV, "Unexpected resource type: texture SRV is expected"); VERIFY(TexSrv.IsValidSampler(), "Texture SRV has no associated sampler"); - const auto& SamInfo = GetSampler(TexSrv.Attribs.VariableType, TexSrv.SamplerId); - VERIFY(SamInfo.Attribs.VariableType == TexSrv.Attribs.VariableType, "Inconsistent texture and sampler variable types"); + const auto& SamInfo = GetSampler(TexSrv.Attribs.GetVariableType(), TexSrv.SamplerId); + VERIFY(SamInfo.Attribs.GetVariableType() == TexSrv.Attribs.GetVariableType(), "Inconsistent texture and sampler variable types"); VERIFY(StrCmpSuff(SamInfo.Attribs.Name, TexSrv.Attribs.Name, D3DSamplerSuffix), "Sampler name \"", SamInfo.Attribs.Name, "\" does not match texture name \"", TexSrv.Attribs.Name, '\"'); return SamInfo; } @@ -562,7 +562,7 @@ void ShaderResourceLayoutD3D12::D3D12Resource::BindResource(IDeviceObject* } else { - if(Attribs.VariableType == SHADER_VARIABLE_TYPE_DYNAMIC) + if(Attribs.GetVariableType() == SHADER_VARIABLE_TYPE_DYNAMIC) VERIFY(ShdrVisibleHeapCPUDescriptorHandle.ptr == 0, "Dynamic resources of a shader resource binding should be assigned shader visible descriptor space at every draw call"); else VERIFY(ShdrVisibleHeapCPUDescriptorHandle.ptr != 0, "Non-dynamics resources of a shader resource binding must be assigned shader visible descriptor space"); @@ -602,7 +602,7 @@ void ShaderResourceLayoutD3D12::D3D12Resource::BindResource(IDeviceObject* } else if (ResourceCache.DbgGetContentType() == ShaderResourceCacheD3D12::DbgCacheContentType::SRBResources) { - if(Attribs.VariableType == SHADER_VARIABLE_TYPE_DYNAMIC) + if(Attribs.GetVariableType() == SHADER_VARIABLE_TYPE_DYNAMIC) VERIFY(ShdrVisibleSamplerHeapCPUDescriptorHandle.ptr == 0, "Dynamic resources of a shader resource binding should be assigned shader visible descriptor space at every draw call"); else VERIFY(ShdrVisibleSamplerHeapCPUDescriptorHandle.ptr != 0 || pTexView == nullptr, "Non-dynamics resources of a shader resource binding must be assigned shader visible descriptor space"); @@ -635,7 +635,7 @@ void ShaderResourceLayoutD3D12::D3D12Resource::BindResource(IDeviceObject* } else { - if (DstRes.pObject && Attribs.VariableType != SHADER_VARIABLE_TYPE_DYNAMIC) + if (DstRes.pObject && Attribs.GetVariableType() != SHADER_VARIABLE_TYPE_DYNAMIC) { LOG_ERROR_MESSAGE( "Shader variable \"", Attribs.Name, "\" in shader \"", ParentResLayout.GetShaderName(), "\" is not dynamic but being unbound. This is an error and may cause unpredicted behavior. Use another shader resource binding instance or mark shader variable as dynamic if you need to bind another resource." ); } @@ -782,7 +782,7 @@ void ShaderResourceLayoutD3D12::dvpVerifyBindings(ShaderResourceCacheD3D12& Reso for(Uint32 r=0; r < GetCbvSrvUavCount(VarType); ++r) { const auto &res = GetSrvCbvUav(VarType, r); - VERIFY(res.Attribs.VariableType == VarType, "Unexpected variable type"); + VERIFY(res.Attribs.GetVariableType() == VarType, "Unexpected variable type"); for(Uint32 ArrInd = 0; ArrInd < res.Attribs.BindCount; ++ArrInd) { @@ -795,7 +795,7 @@ void ShaderResourceLayoutD3D12::dvpVerifyBindings(ShaderResourceCacheD3D12& Reso if( !CachedRes.pObject || // Dynamic buffers do not have CPU descriptor handle as they do not keep D3D12 buffer, and space is allocated from the GPU ring buffer CachedRes.CPUDescriptorHandle.ptr == 0 && !(CachedRes.Type==CachedResourceType::CBV && CachedRes.pObject.RawPtr()->GetDesc().Usage == USAGE_DYNAMIC) ) - LOG_ERROR_MESSAGE( "No resource is bound to ", GetShaderVariableTypeLiteralName(res.Attribs.VariableType), " variable \"", res.Attribs.GetPrintName(ArrInd), "\" in shader \"", GetShaderName(), "\"" ); + LOG_ERROR_MESSAGE( "No resource is bound to ", GetShaderVariableTypeLiteralName(res.Attribs.GetVariableType()), " variable \"", res.Attribs.GetPrintName(ArrInd), "\" in shader \"", GetShaderName(), "\"" ); if (res.Attribs.BindCount > 1 && res.IsValidSampler()) { @@ -828,7 +828,7 @@ void ShaderResourceLayoutD3D12::dvpVerifyBindings(ShaderResourceCacheD3D12& Reso } else { - if(res.Attribs.VariableType == SHADER_VARIABLE_TYPE_DYNAMIC) + if(res.Attribs.GetVariableType() == SHADER_VARIABLE_TYPE_DYNAMIC) VERIFY(ShdrVisibleHeapCPUDescriptorHandle.ptr == 0, "Dynamic resources of a shader resource binding should be assigned shader visible descriptor space at every draw call"); else VERIFY(ShdrVisibleHeapCPUDescriptorHandle.ptr != 0, "Non-dynamics resources of a shader resource binding must be assigned shader visible descriptor space"); @@ -868,7 +868,7 @@ void ShaderResourceLayoutD3D12::dvpVerifyBindings(ShaderResourceCacheD3D12& Reso } else if (ResourceCache.DbgGetContentType() == ShaderResourceCacheD3D12::DbgCacheContentType::SRBResources) { - if(SamInfo.Attribs.VariableType == SHADER_VARIABLE_TYPE_DYNAMIC) + if(SamInfo.Attribs.GetVariableType() == SHADER_VARIABLE_TYPE_DYNAMIC) VERIFY(ShdrVisibleHeapCPUDescriptorHandle.ptr == 0, "Dynamic resources of a shader resource binding should be assigned shader visible descriptor space at every draw call"); else VERIFY(ShdrVisibleHeapCPUDescriptorHandle.ptr != 0, "Non-dynamics resources of a shader resource binding must be assigned shader visible descriptor space"); @@ -886,7 +886,7 @@ void ShaderResourceLayoutD3D12::dvpVerifyBindings(ShaderResourceCacheD3D12& Reso for(Uint32 s=0; s < GetSamplerCount(VarType); ++s) { const auto &sam = GetSampler(VarType, s); - VERIFY(sam.Attribs.VariableType == VarType, "Unexpected sampler variable type"); + VERIFY(sam.Attribs.GetVariableType() == VarType, "Unexpected sampler variable type"); for(Uint32 ArrInd = 0; ArrInd < sam.Attribs.BindCount; ++ArrInd) { diff --git a/Graphics/GraphicsEngineD3DBase/include/D3DShaderResourceLoader.h b/Graphics/GraphicsEngineD3DBase/include/D3DShaderResourceLoader.h index 3f268eae..75f880c0 100644 --- a/Graphics/GraphicsEngineD3DBase/include/D3DShaderResourceLoader.h +++ b/Graphics/GraphicsEngineD3DBase/include/D3DShaderResourceLoader.h @@ -212,7 +212,7 @@ namespace Diligent for (const auto& Res : Resources) { // Skip samplers as they are not handled as independent variables - if (Res.InputType != D3D_SIT_SAMPLER && strcmp(Res.Name, VarName) == 0) + if (Res.GetInputType() != D3D_SIT_SAMPLER && strcmp(Res.Name, VarName) == 0) { VariableFound = true; break; @@ -231,7 +231,7 @@ namespace Diligent for (const auto& Res : Resources) { - if ( Res.InputType == D3D_SIT_TEXTURE && Res.SRVDimension != D3D_SRV_DIMENSION_BUFFER && strcmp(Res.Name, TexName) == 0) + if ( Res.GetInputType() == D3D_SIT_TEXTURE && Res.GetSRVDimension() != D3D_SRV_DIMENSION_BUFFER && strcmp(Res.Name, TexName) == 0) { TextureFound = true; break; @@ -253,7 +253,7 @@ namespace Diligent for(size_t ResInd = 0; ResInd < Resources.size(); ++ResInd) { const auto& Res = Resources[ResInd]; - switch( Res.InputType ) + switch( Res.GetInputType() ) { case D3D_SIT_CBUFFER: { @@ -269,7 +269,7 @@ namespace Diligent case D3D_SIT_TEXTURE: { - if( Res.SRVDimension == D3D_SRV_DIMENSION_BUFFER ) + if( Res.GetSRVDimension() == D3D_SRV_DIMENSION_BUFFER ) { OnNewBuffSRV( Res ); } @@ -289,7 +289,7 @@ namespace Diligent case D3D_SIT_UAV_RWTYPED: { - if( Res.SRVDimension == D3D_SRV_DIMENSION_BUFFER ) + if( Res.GetSRVDimension() == D3D_SRV_DIMENSION_BUFFER ) { OnNewBuffUAV( Res ); } @@ -341,6 +341,11 @@ namespace Diligent UNSUPPORTED( "RW structured buffers with counter are not supported" ); break; } + + default: + { + UNEXPECTED("Unexpected resource input type"); + } } } diff --git a/Graphics/GraphicsEngineD3DBase/include/ShaderResources.h b/Graphics/GraphicsEngineD3DBase/include/ShaderResources.h index a8e9e46a..68925551 100644 --- a/Graphics/GraphicsEngineD3DBase/include/ShaderResources.h +++ b/Graphics/GraphicsEngineD3DBase/include/ShaderResources.h @@ -90,6 +90,7 @@ struct D3DShaderResourceAttribs const Uint16 BindPoint; const Uint16 BindCount; +private: // 4 3 4 20 1 // bit | 0 1 2 3 | 4 5 6 | 7 8 9 10 | 11 12 ... 30 | 31 | // | | | | | | @@ -101,21 +102,27 @@ struct D3DShaderResourceAttribs static constexpr const Uint32 StaticSamplerFlagBits = 1; static_assert(ShaderInputTypeBits + VariableTypeBits + SRVDimBits + SamplerIdBits + StaticSamplerFlagBits == 32, "Attributes are better be packed into 32 bits"); - const D3D_SHADER_INPUT_TYPE InputType : ShaderInputTypeBits; // Max value: D3D_SIT_UAV_RWSTRUCTURED_WITH_COUNTER == 11 - const SHADER_VARIABLE_TYPE VariableType : VariableTypeBits; // Max value: SHADER_VARIABLE_TYPE_DYNAMIC == 2 - const D3D_SRV_DIMENSION SRVDimension : SRVDimBits; // Max value: D3D_SRV_DIMENSION_BUFFEREX == 11 - const Uint32 SamplerId : SamplerIdBits; // Max value: 1048575 - const Uint32 StaticSamplerFlag : StaticSamplerFlagBits; // Needs to be Uint32, otherwise sizeof(D3DShaderResourceAttribs)==24 - // (https://stackoverflow.com/questions/308364/c-bitfield-packing-with-bools) + static_assert(D3D_SIT_UAV_RWSTRUCTURED_WITH_COUNTER < (1 << ShaderInputTypeBits), "Not enough bits to represent D3D_SHADER_INPUT_TYPE"); + static_assert(SHADER_VARIABLE_TYPE_NUM_TYPES < (1 << VariableTypeBits), "Not enough bits to represent SHADER_VARIABLE_TYPE"); + static_assert(D3D_SRV_DIMENSION_BUFFEREX < (1 << SRVDimBits), "Not enough bits to represent D3D_SRV_DIMENSION"); + + // We need to use Uint32 instead of the actual type for reliability and correctness. + // There originally was a problem when the type of InputType was D3D_SHADER_INPUT_TYPE: + // the value of D3D_SIT_UAV_RWBYTEADDRESS (8) was interpreted as -8 (as the underlying enum type + // is signed) causing errors + const Uint32 InputType : ShaderInputTypeBits; // Max value: D3D_SIT_UAV_RWSTRUCTURED_WITH_COUNTER == 11 + const Uint32 VariableType : VariableTypeBits; // Max value: SHADER_VARIABLE_TYPE_DYNAMIC == 2 + const Uint32 SRVDimension : SRVDimBits; // Max value: D3D_SRV_DIMENSION_BUFFEREX == 11 + const Uint32 SamplerId : SamplerIdBits; // Max value: 1048575 + const Uint32 StaticSamplerFlag : StaticSamplerFlagBits; // Needs to be Uint32, otherwise sizeof(D3DShaderResourceAttribs)==24 + // (https://stackoverflow.com/questions/308364/c-bitfield-packing-with-bools) +public: static constexpr const Uint32 InvalidSamplerId = (1 << SamplerIdBits) - 1; static constexpr const Uint16 InvalidBindPoint = std::numeric_limits::max(); static constexpr const Uint16 MaxBindPoint = InvalidBindPoint - 1; static constexpr const Uint16 MaxBindCount = std::numeric_limits::max(); - static_assert(D3D_SIT_UAV_RWSTRUCTURED_WITH_COUNTER < (1 << ShaderInputTypeBits), "Not enough bits to represent D3D_SHADER_INPUT_TYPE"); - static_assert(SHADER_VARIABLE_TYPE_NUM_TYPES < (1 << VariableTypeBits), "Not enough bits to represent SHADER_VARIABLE_TYPE"); - static_assert(D3D_SRV_DIMENSION_BUFFEREX < (1 << SRVDimBits), "Not enough bits to represent D3D_SRV_DIMENSION"); D3DShaderResourceAttribs(const char* _Name, UINT _BindPoint, @@ -126,11 +133,11 @@ struct D3DShaderResourceAttribs Uint32 _SamplerId, bool _IsStaticSampler)noexcept : Name (_Name), - BindPoint (static_cast(_BindPoint)), - BindCount (static_cast(_BindCount)), - InputType (_InputType), - VariableType (_VariableType), - SRVDimension (_SRVDimension), + BindPoint (static_cast (_BindPoint)), + BindCount (static_cast (_BindCount)), + InputType (static_cast (_InputType)), + VariableType (static_cast(_VariableType)), + SRVDimension (static_cast(_SRVDimension)), SamplerId (_SamplerId), StaticSamplerFlag(_IsStaticSampler ? 1 : 0) { @@ -141,7 +148,7 @@ struct D3DShaderResourceAttribs VERIFY(_SRVDimension < (1 << SRVDimBits), "SRV dimensions is out of expected range"); VERIFY(_SamplerId < (1 << SamplerIdBits), "SamplerId is out of representable range"); #ifdef _DEBUG - if(_InputType==D3D_SIT_SAMPLER) + if (_InputType==D3D_SIT_SAMPLER) VERIFY_EXPR(IsStaticSampler() == _IsStaticSampler); else VERIFY(!_IsStaticSampler, "Only samplers can be marked as static"); @@ -151,7 +158,7 @@ struct D3DShaderResourceAttribs else VERIFY(SamplerId == InvalidSamplerId, "Only textures can be assigned a valid texture sampler"); - if(_IsStaticSampler) + if (_IsStaticSampler) VERIFY( _InputType == D3D_SIT_SAMPLER, "Invalid input type: D3D_SIT_SAMPLER is expected" ); #endif } @@ -162,9 +169,9 @@ struct D3DShaderResourceAttribs NamesPool.CopyString(rhs.Name), rhs.BindPoint, rhs.BindCount, - rhs.InputType, - rhs.VariableType, - rhs.SRVDimension, + rhs.GetInputType(), + rhs.GetVariableType(), + rhs.GetSRVDimension(), SamplerId, false } @@ -178,9 +185,9 @@ struct D3DShaderResourceAttribs NamesPool.CopyString(rhs.Name), rhs.BindPoint, rhs.BindCount, - rhs.InputType, - rhs.VariableType, - rhs.SRVDimension, + rhs.GetInputType(), + rhs.GetVariableType(), + rhs.GetSRVDimension(), rhs.SamplerId, rhs.StaticSamplerFlag !=0 ? true : false } @@ -192,6 +199,21 @@ struct D3DShaderResourceAttribs D3DShaderResourceAttribs& operator = (const D3DShaderResourceAttribs& rhs) = delete; D3DShaderResourceAttribs& operator = ( D3DShaderResourceAttribs&& rhs) = delete; + D3D_SHADER_INPUT_TYPE GetInputType()const + { + return static_cast(InputType); + } + + SHADER_VARIABLE_TYPE GetVariableType()const + { + return static_cast(VariableType); + } + + D3D_SRV_DIMENSION GetSRVDimension()const + { + return static_cast(SRVDimension); + } + Uint32 GetSamplerId()const { VERIFY( InputType == D3D_SIT_TEXTURE, "Invalid input type: D3D_SIT_TEXTURE is expected" ); @@ -238,6 +260,11 @@ struct D3DShaderResourceAttribs { return ComputeHash(BindPoint, BindCount, InputType, VariableType, SRVDimension, SamplerId, StaticSamplerFlag); } + + bool IsAllowedType(Uint32 AllowedTypeBits)const + { + return Diligent::IsAllowedType(GetVariableType(), AllowedTypeBits); + } }; static_assert(sizeof(D3DShaderResourceAttribs) == sizeof(void*) + sizeof(Uint32)*2, "Unexpected sizeof(D3DShaderResourceAttribs)"); @@ -295,35 +322,35 @@ public: for(Uint32 n=0; n < GetNumCBs(); ++n) { const auto& CB = GetCB(n); - if( IsAllowedType(CB.VariableType, AllowedTypeBits) ) + if( CB.IsAllowedType(AllowedTypeBits) ) HandleCB(CB, n); } for(Uint32 n=0; n < GetNumTexSRV(); ++n) { const auto &TexSRV = GetTexSRV(n); - if( IsAllowedType(TexSRV.VariableType, AllowedTypeBits) ) + if( TexSRV.IsAllowedType(AllowedTypeBits) ) HandleTexSRV(TexSRV, n); } for(Uint32 n=0; n < GetNumTexUAV(); ++n) { const auto &TexUAV = GetTexUAV(n); - if( IsAllowedType(TexUAV.VariableType, AllowedTypeBits) ) + if( TexUAV.IsAllowedType(AllowedTypeBits) ) HandleTexUAV(TexUAV, n); } for(Uint32 n=0; n < GetNumBufSRV(); ++n) { const auto &BufSRV = GetBufSRV(n); - if( IsAllowedType(BufSRV.VariableType, AllowedTypeBits) ) + if( BufSRV.IsAllowedType(AllowedTypeBits) ) HandleBufSRV(BufSRV, n); } for(Uint32 n=0; n < GetNumBufUAV(); ++n) { const auto& BufUAV = GetBufUAV(n); - if( IsAllowedType(BufUAV.VariableType, AllowedTypeBits) ) + if( BufUAV.IsAllowedType(AllowedTypeBits) ) HandleBufUAV(BufUAV, n); } } diff --git a/Graphics/GraphicsEngineD3DBase/include/ShaderVariableD3DBase.h b/Graphics/GraphicsEngineD3DBase/include/ShaderVariableD3DBase.h index 3a3e706b..b0536ba6 100644 --- a/Graphics/GraphicsEngineD3DBase/include/ShaderVariableD3DBase.h +++ b/Graphics/GraphicsEngineD3DBase/include/ShaderVariableD3DBase.h @@ -69,7 +69,7 @@ namespace Diligent virtual SHADER_VARIABLE_TYPE GetType()const override final { - return Attribs.VariableType; + return Attribs.GetVariableType(); } virtual Uint32 GetArraySize()const override final diff --git a/Graphics/GraphicsEngineD3DBase/src/ShaderResources.cpp b/Graphics/GraphicsEngineD3DBase/src/ShaderResources.cpp index d2489ef2..f0585f9d 100644 --- a/Graphics/GraphicsEngineD3DBase/src/ShaderResources.cpp +++ b/Graphics/GraphicsEngineD3DBase/src/ShaderResources.cpp @@ -128,28 +128,28 @@ void ShaderResources::CountResources(const SHADER_VARIABLE_TYPE *AllowedVarTypes [&](const D3DShaderResourceAttribs &CB, Uint32) { - VERIFY_EXPR(IsAllowedType(CB.VariableType, AllowedTypeBits)); + VERIFY_EXPR(CB.IsAllowedType(AllowedTypeBits)); ++NumCBs; }, [&](const D3DShaderResourceAttribs& TexSRV, Uint32) { - VERIFY_EXPR(IsAllowedType(TexSRV.VariableType, AllowedTypeBits)); + VERIFY_EXPR(TexSRV.IsAllowedType(AllowedTypeBits)); ++NumTexSRVs; NumSamplers += TexSRV.IsValidSampler() ? 1 : 0; }, [&](const D3DShaderResourceAttribs &TexUAV, Uint32) { - VERIFY_EXPR(IsAllowedType(TexUAV.VariableType, AllowedTypeBits)); + VERIFY_EXPR(TexUAV.IsAllowedType(AllowedTypeBits)); ++NumTexUAVs; }, [&](const D3DShaderResourceAttribs &BufSRV, Uint32) { - VERIFY_EXPR(IsAllowedType(BufSRV.VariableType, AllowedTypeBits)); + VERIFY_EXPR(BufSRV.IsAllowedType(AllowedTypeBits)); ++NumBufSRVs; }, [&](const D3DShaderResourceAttribs &BufUAV, Uint32) { - VERIFY_EXPR(IsAllowedType(BufUAV.VariableType, AllowedTypeBits)); + VERIFY_EXPR(BufUAV.IsAllowedType(AllowedTypeBits)); ++NumBufUAVs; } ); @@ -158,14 +158,14 @@ void ShaderResources::CountResources(const SHADER_VARIABLE_TYPE *AllowedVarTypes Uint32 ShaderResources::FindAssignedSamplerId(const D3DShaderResourceAttribs& TexSRV)const { - VERIFY_EXPR(TexSRV.InputType == D3D_SIT_TEXTURE); + VERIFY_EXPR(TexSRV.GetInputType() == D3D_SIT_TEXTURE); auto NumSamplers = GetNumSamplers(); for (Uint32 s = 0; s < NumSamplers; ++s) { const auto &Sampler = GetSampler(s); if( StrCmpSuff(Sampler.Name, TexSRV.Name, D3DSamplerSuffix) ) { - VERIFY(Sampler.VariableType == TexSRV.VariableType, "Inconsistent texture and sampler variable types"); + VERIFY(Sampler.GetVariableType() == TexSRV.GetVariableType(), "Inconsistent texture and sampler variable types"); VERIFY(Sampler.BindCount == TexSRV.BindCount || Sampler.BindCount == 1, "Sampler assigned to array \"", TexSRV.Name, "\" is expected to be scalar or have the same dimension (",TexSRV.BindCount,"). Actual sampler array dimension : ", Sampler.BindCount); return s; } -- cgit v1.2.3