From 859eb02ccdca5c2a69935e011810490b986cb719 Mon Sep 17 00:00:00 2001 From: assiduous Date: Mon, 22 Feb 2021 18:01:50 -0800 Subject: Implemented committed resource validation in d3d12 --- .../include/PipelineResourceSignatureVkImpl.hpp | 16 +-- .../src/PipelineResourceSignatureVkImpl.cpp | 128 +++++++++++---------- .../src/PipelineStateVkImpl.cpp | 3 +- 3 files changed, 73 insertions(+), 74 deletions(-) (limited to 'Graphics/GraphicsEngineVulkan') diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp b/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp index b873d828..cabfa010 100644 --- a/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp +++ b/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp @@ -185,12 +185,6 @@ public: return m_pResourceAttribs[ResIndex]; } - const PipelineResourceDesc& GetResourceDesc(Uint32 ResIndex) const - { - VERIFY_EXPR(ResIndex < m_Desc.NumResources); - return m_Desc.Resources[ResIndex]; - } - struct ImmutableSamplerAttribs { RefCntAutoPtr Ptr; @@ -205,12 +199,6 @@ public: return m_ImmutableSamplers[SampIndex]; } - const ImmutableSamplerDesc& GetImmutableSamplerDesc(Uint32 SampIndex) const - { - VERIFY_EXPR(SampIndex < m_Desc.NumImmutableSamplers); - return m_Desc.ImmutableSamplers[SampIndex]; - } - VkDescriptorSetLayout GetVkDescriptorSetLayout(DESCRIPTOR_SET_ID SetId) const { return m_VkDescrSetLayouts[SetId]; } bool HasDescriptorSet(DESCRIPTOR_SET_ID SetId) const { return m_VkDescrSetLayouts[SetId] != VK_NULL_HANDLE; } @@ -278,7 +266,9 @@ public: /// Verifies committed resource attribs using the SPIRV resource attributes from the PSO. bool DvpValidateCommittedResource(const SPIRVShaderResourceAttribs& SPIRVAttribs, Uint32 ResIndex, - ShaderResourceCacheVk& ResourceCache) const; + const ShaderResourceCacheVk& ResourceCache, + const char* ShaderName, + const char* PSOName) const; #endif // Returns the descriptor set index in the resource cache diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp index ddd59ae1..cedeaddc 100644 --- a/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp @@ -189,10 +189,10 @@ inline TEXTURE_VIEW_TYPE DescriptorTypeToTextureView(DescriptorType Type) } } -Int32 FindImmutableSampler(const PipelineResourceDesc& Res, - DescriptorType DescType, - const PipelineResourceSignatureDesc& Desc, - const char* SamplerSuffix) +Uint32 FindImmutableSamplerVk(const PipelineResourceDesc& Res, + DescriptorType DescType, + const PipelineResourceSignatureDesc& Desc, + const char* SamplerSuffix) { if (DescType == DescriptorType::CombinedImageSampler) { @@ -206,10 +206,10 @@ Int32 FindImmutableSampler(const PipelineResourceDesc& Res, else { UNEXPECTED("Immutable sampler can only be assigned to a sampled image or separate sampler"); - return -1; + return InvalidImmutableSamplerIndex; } - return Diligent::FindImmutableSampler(Desc.ImmutableSamplers, Desc.NumImmutableSamplers, Res.ShaderStages, Res.Name, SamplerSuffix); + return FindImmutableSampler(Desc.ImmutableSamplers, Desc.NumImmutableSamplers, Res.ShaderStages, Res.Name, SamplerSuffix); } } // namespace @@ -443,8 +443,8 @@ void PipelineResourceSignatureVkImpl::CreateSetLayouts(const CacheOffsetsType& C // Only search for immutable sampler for combined image samplers and separate samplers. // Note that for DescriptorType::SeparateImage with immutable sampler, we will initialize // a separate immutable sampler below. It will not be assigned to the image variable. - const auto SrcImmutableSamplerInd = FindImmutableSampler(ResDesc, DescrType, m_Desc, GetCombinedSamplerSuffix()); - if (SrcImmutableSamplerInd >= 0) + const auto SrcImmutableSamplerInd = FindImmutableSamplerVk(ResDesc, DescrType, m_Desc, GetCombinedSamplerSuffix()); + if (SrcImmutableSamplerInd != InvalidImmutableSamplerIndex) { auto& ImmutableSampler = m_ImmutableSamplers[SrcImmutableSamplerInd]; const auto& ImmutableSamplerDesc = m_Desc.ImmutableSamplers[SrcImmutableSamplerInd].Desc; @@ -1592,14 +1592,17 @@ bool PipelineResourceSignatureVkImpl::IsBound(Uint32 Array #ifdef DILIGENT_DEVELOPMENT bool PipelineResourceSignatureVkImpl::DvpValidateCommittedResource(const SPIRVShaderResourceAttribs& SPIRVAttribs, Uint32 ResIndex, - ShaderResourceCacheVk& ResourceCache) const + const ShaderResourceCacheVk& ResourceCache, + const char* ShaderName, + const char* PSOName) const { VERIFY_EXPR(ResIndex < m_Desc.NumResources); - const auto& ResInfo = m_Desc.Resources[ResIndex]; + const auto& ResDesc = m_Desc.Resources[ResIndex]; const auto& ResAttribs = m_pResourceAttribs[ResIndex]; - VERIFY(strcmp(ResInfo.Name, SPIRVAttribs.Name) == 0, "Inconsistent resource names"); + VERIFY(strcmp(ResDesc.Name, SPIRVAttribs.Name) == 0, "Inconsistent resource names"); - bool BindingsOK = true; + if (ResDesc.ResourceType == SHADER_RESOURCE_TYPE_SAMPLER && ResAttribs.IsImmutableSamplerAssigned()) + return true; // Skip immutable separate samplers const auto& DescrSetResources = ResourceCache.GetDescriptorSet(ResAttribs.DescrSet); const auto CacheType = ResourceCache.GetContentType(); @@ -1607,16 +1610,43 @@ bool PipelineResourceSignatureVkImpl::DvpValidateCommittedResource(const SPIRVSh VERIFY_EXPR(SPIRVAttribs.ArraySize <= ResAttribs.ArraySize); - switch (ResAttribs.GetDescriptorType()) + bool BindingsOK = true; + for (Uint32 ArrIndex = 0; ArrIndex < SPIRVAttribs.ArraySize; ++ArrIndex) { - case DescriptorType::UniformBuffer: - case DescriptorType::UniformBufferDynamic: + if (!IsBound(ArrIndex, ResIndex, ResourceCache)) { - VERIFY_EXPR(ResInfo.ResourceType == SHADER_RESOURCE_TYPE_CONSTANT_BUFFER); - for (Uint32 i = 0; i < SPIRVAttribs.ArraySize; ++i) + LOG_ERROR_MESSAGE("No resource is bound to variable '", GetShaderResourcePrintName(SPIRVAttribs, ArrIndex), + "' in shader '", ShaderName, "' of PSO '", PSOName, "'"); + BindingsOK = false; + continue; + } + + if (ResAttribs.IsCombinedWithSampler()) + { + auto& SamplerResDesc = GetResourceDesc(ResAttribs.SamplerInd); + auto& SamplerAttribs = GetResourceAttribs(ResAttribs.SamplerInd); + VERIFY_EXPR(SamplerResDesc.ResourceType == SHADER_RESOURCE_TYPE_SAMPLER); + VERIFY_EXPR(SamplerResDesc.ArraySize == 1 || SamplerResDesc.ArraySize == ResDesc.ArraySize); + if (!SamplerAttribs.IsImmutableSamplerAssigned()) { - const auto& Res = DescrSetResources.GetResource(CacheOffset + i); + if (ArrIndex < SamplerResDesc.ArraySize) + { + if (!IsBound(ArrIndex, ResAttribs.SamplerInd, ResourceCache)) + { + LOG_ERROR_MESSAGE("No sampler is bound to sampler variable '", GetShaderResourcePrintName(SamplerResDesc, ArrIndex), + "' combined with texture '", SPIRVAttribs.Name, "' in shader '", ShaderName, "' of PSO '", PSOName, "'."); + BindingsOK = false; + } + } + } + } + const auto& Res = DescrSetResources.GetResource(CacheOffset + ArrIndex); + switch (ResAttribs.GetDescriptorType()) + { + case DescriptorType::UniformBuffer: + case DescriptorType::UniformBufferDynamic: + VERIFY_EXPR(ResDesc.ResourceType == SHADER_RESOURCE_TYPE_CONSTANT_BUFFER); // When can use raw cast here because the dynamic type is verified when the resource // is bound. It will be null if the type is incorrect. if (const auto* pBufferVk = Res.pObject.RawPtr()) @@ -1626,29 +1656,18 @@ bool PipelineResourceSignatureVkImpl::DvpValidateCommittedResource(const SPIRVSh // It is OK if robustBufferAccess feature is enabled, otherwise access outside of buffer range may lead to crash or undefined behavior. LOG_WARNING_MESSAGE("The size of uniform buffer '", pBufferVk->GetDesc().Name, "' bound to shader variable '", - GetShaderResourcePrintName(ResInfo, i), "' is ", pBufferVk->GetDesc().uiSizeInBytes, + GetShaderResourcePrintName(SPIRVAttribs, ArrIndex), "' is ", pBufferVk->GetDesc().uiSizeInBytes, " bytes, but the shader expects at least ", SPIRVAttribs.BufferStaticSize, " bytes."); } } - else - { - // Missing resource error is logged by BindResourceHelper::CacheUniformBuffer - } - } - } - break; - - case DescriptorType::StorageBuffer: - case DescriptorType::StorageBuffer_ReadOnly: - case DescriptorType::StorageBufferDynamic: - case DescriptorType::StorageBufferDynamic_ReadOnly: - { - VERIFY_EXPR(ResInfo.ResourceType == SHADER_RESOURCE_TYPE_BUFFER_UAV || ResInfo.ResourceType == SHADER_RESOURCE_TYPE_BUFFER_SRV); - for (Uint32 i = 0; i < SPIRVAttribs.ArraySize; ++i) - { - const auto& Res = DescrSetResources.GetResource(CacheOffset + i); + break; + case DescriptorType::StorageBuffer: + case DescriptorType::StorageBuffer_ReadOnly: + case DescriptorType::StorageBufferDynamic: + case DescriptorType::StorageBufferDynamic_ReadOnly: + VERIFY_EXPR(ResDesc.ResourceType == SHADER_RESOURCE_TYPE_BUFFER_UAV || ResDesc.ResourceType == SHADER_RESOURCE_TYPE_BUFFER_SRV); // When can use raw cast here because the dynamic type is verified when the resource // is bound. It will be null if the type is incorrect. if (auto* pBufferViewVk = Res.pObject.RawPtr()) @@ -1663,7 +1682,7 @@ bool PipelineResourceSignatureVkImpl::DvpValidateCommittedResource(const SPIRVSh { // It is OK if robustBufferAccess feature is enabled, otherwise access outside of buffer range may lead to crash or undefined behavior. LOG_WARNING_MESSAGE("The size of buffer view '", ViewDesc.Name, "' of buffer '", BuffDesc.Name, "' bound to shader variable '", - GetShaderResourcePrintName(ResInfo, i), "' is ", ViewDesc.ByteWidth, " bytes, but the shader expects at least ", + GetShaderResourcePrintName(SPIRVAttribs, ArrIndex), "' is ", ViewDesc.ByteWidth, " bytes, but the shader expects at least ", SPIRVAttribs.BufferStaticSize, " bytes."); } } @@ -1675,44 +1694,33 @@ bool PipelineResourceSignatureVkImpl::DvpValidateCommittedResource(const SPIRVSh // Element stride in the shader may be differ than in the code. Here we check that the buffer size is exactly the same as the array with N elements. LOG_WARNING_MESSAGE("The size (", ViewDesc.ByteWidth, ") and stride (", BuffDesc.ElementByteStride, ") of buffer view '", ViewDesc.Name, "' of buffer '", BuffDesc.Name, "' bound to shader variable '", - GetShaderResourcePrintName(ResInfo, i), "' are incompatible with what the shader expects. This may be the result of the array element size mismatch."); + GetShaderResourcePrintName(SPIRVAttribs, ArrIndex), "' are incompatible with what the shader expects. This may be the result of the array element size mismatch."); } } } - else - { - // Missing resource error is logged by BindResourceHelper::CacheStorageBuffer - } - } - } - break; + break; - case DescriptorType::StorageImage: - case DescriptorType::SeparateImage: - case DescriptorType::CombinedImageSampler: - { - VERIFY_EXPR(ResInfo.ResourceType == SHADER_RESOURCE_TYPE_TEXTURE_SRV || ResInfo.ResourceType == SHADER_RESOURCE_TYPE_TEXTURE_UAV); - for (Uint32 i = 0; i < SPIRVAttribs.ArraySize; ++i) - { - const auto& Res = DescrSetResources.GetResource(CacheOffset + i); + case DescriptorType::StorageImage: + case DescriptorType::SeparateImage: + case DescriptorType::CombinedImageSampler: + VERIFY_EXPR(ResDesc.ResourceType == SHADER_RESOURCE_TYPE_TEXTURE_SRV || ResDesc.ResourceType == SHADER_RESOURCE_TYPE_TEXTURE_UAV); // When can use raw cast here because the dynamic type is verified when the resource // is bound. It will be null if the type is incorrect. if (const auto* pTexViewVk = Res.pObject.RawPtr()) { - if (!ValidateResourceViewDimension(ResInfo.Name, ResInfo.ArraySize, i, pTexViewVk, SPIRVAttribs.GetResourceDimension(), SPIRVAttribs.IsMultisample())) + if (!ValidateResourceViewDimension(SPIRVAttribs.Name, SPIRVAttribs.ArraySize, ArrIndex, pTexViewVk, SPIRVAttribs.GetResourceDimension(), SPIRVAttribs.IsMultisample())) BindingsOK = false; } else { // Missing resource error is logged by BindResourceHelper::CacheImage } - } - } - break; + break; - default: - break; - // Nothing to do + default: + break; + // Nothing to do + } } return BindingsOK; diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp index bab4d247..93262266 100644 --- a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp @@ -1109,7 +1109,8 @@ void PipelineStateVkImpl::DvpVerifySRBResources(SRBArray& SRBs) const const auto SignBindIndex = SignDesc.BindingIndex; if (auto* pSRB = SRBs[SignBindIndex]) { - res_info->Signature->DvpValidateCommittedResource(ResAttribs, res_info->ResIndex, pSRB->GetResourceCache()); + res_info->Signature->DvpValidateCommittedResource(ResAttribs, res_info->ResIndex, pSRB->GetResourceCache(), + pResources->GetShaderName(), m_Desc.Name); } else { -- cgit v1.2.3