From b3fec8bd40e80115086281bce74774617aa95e23 Mon Sep 17 00:00:00 2001 From: assiduous Date: Wed, 14 Oct 2020 01:04:05 -0700 Subject: Added buffer mode validation when binding buffer views --- Graphics/GraphicsEngine/include/BufferBase.hpp | 10 ++++- Graphics/GraphicsEngine/include/TextureBase.hpp | 3 +- .../src/ShaderResourceLayoutD3D11.cpp | 2 + .../src/ShaderResourceLayoutD3D12.cpp | 12 ++++++ .../include/ShaderVariableD3DBase.hpp | 49 ++++++++++++++++++++++ .../src/GLPipelineResourceLayout.cpp | 30 +++++++++++++ .../src/ShaderResourceLayoutVk.cpp | 20 +++++++++ 7 files changed, 124 insertions(+), 2 deletions(-) (limited to 'Graphics') diff --git a/Graphics/GraphicsEngine/include/BufferBase.hpp b/Graphics/GraphicsEngine/include/BufferBase.hpp index 9ae6db92..605bc2ed 100644 --- a/Graphics/GraphicsEngine/include/BufferBase.hpp +++ b/Graphics/GraphicsEngine/include/BufferBase.hpp @@ -30,12 +30,14 @@ /// \file /// Implementation of the Diligent::BufferBase template class +#include + #include "Buffer.h" #include "GraphicsTypes.h" #include "DeviceObjectBase.hpp" #include "GraphicsAccessories.hpp" #include "STDAllocator.hpp" -#include +#include "FormatString.hpp" namespace Diligent { @@ -232,6 +234,9 @@ void BufferBasem_Desc.Name, '\''); + ViewDesc.Name = UAVName.c_str(); + IBufferView* pUAV = nullptr; CreateViewInternal(ViewDesc, &pUAV, true); m_pDefaultUAV.reset(static_cast(pUAV)); @@ -242,6 +247,9 @@ void BufferBasem_Desc.Name, '\''); + ViewDesc.Name = SRVName.c_str(); + IBufferView* pSRV = nullptr; CreateViewInternal(ViewDesc, &pSRV, true); m_pDefaultSRV.reset(static_cast(pSRV)); diff --git a/Graphics/GraphicsEngine/include/TextureBase.hpp b/Graphics/GraphicsEngine/include/TextureBase.hpp index 1805f7d3..559df1ea 100644 --- a/Graphics/GraphicsEngine/include/TextureBase.hpp +++ b/Graphics/GraphicsEngine/include/TextureBase.hpp @@ -30,13 +30,14 @@ /// \file /// Implementation of the Diligent::TextureBase template class +#include + #include "Texture.h" #include "GraphicsTypes.h" #include "DeviceObjectBase.hpp" #include "GraphicsAccessories.hpp" #include "STDAllocator.hpp" #include "FormatString.hpp" -#include namespace Diligent { diff --git a/Graphics/GraphicsEngineD3D11/src/ShaderResourceLayoutD3D11.cpp b/Graphics/GraphicsEngineD3D11/src/ShaderResourceLayoutD3D11.cpp index fd6c7299..63b34f44 100755 --- a/Graphics/GraphicsEngineD3D11/src/ShaderResourceLayoutD3D11.cpp +++ b/Graphics/GraphicsEngineD3D11/src/ShaderResourceLayoutD3D11.cpp @@ -553,6 +553,7 @@ void ShaderResourceLayoutD3D11::BuffSRVBindInfo::BindResource(IDeviceObject* pVi { auto& CachedSRV = ResourceCache.GetSRV(m_Attribs.BindPoint + ArrayIndex); VerifyResourceViewBinding(m_Attribs, GetType(), ArrayIndex, pView, pViewD3D11.RawPtr(), {BUFFER_VIEW_SHADER_RESOURCE}, CachedSRV.pView.RawPtr(), m_ParentResLayout.GetShaderName()); + VerifyBufferViewModeD3D(pViewD3D11.RawPtr(), m_Attribs, m_ParentResLayout.GetShaderName()); } #endif ResourceCache.SetBufSRV(m_Attribs.BindPoint + ArrayIndex, std::move(pViewD3D11)); @@ -593,6 +594,7 @@ void ShaderResourceLayoutD3D11::BuffUAVBindInfo::BindResource(IDeviceObject* pVi { auto& CachedUAV = ResourceCache.GetUAV(m_Attribs.BindPoint + ArrayIndex); VerifyResourceViewBinding(m_Attribs, GetType(), ArrayIndex, pView, pViewD3D11.RawPtr(), {BUFFER_VIEW_UNORDERED_ACCESS}, CachedUAV.pView.RawPtr(), m_ParentResLayout.GetShaderName()); + VerifyBufferViewModeD3D(pViewD3D11.RawPtr(), m_Attribs, m_ParentResLayout.GetShaderName()); } #endif ResourceCache.SetBufUAV(m_Attribs.BindPoint + ArrayIndex, std::move(pViewD3D11)); diff --git a/Graphics/GraphicsEngineD3D12/src/ShaderResourceLayoutD3D12.cpp b/Graphics/GraphicsEngineD3D12/src/ShaderResourceLayoutD3D12.cpp index 6c313882..f00d62e7 100644 --- a/Graphics/GraphicsEngineD3D12/src/ShaderResourceLayoutD3D12.cpp +++ b/Graphics/GraphicsEngineD3D12/src/ShaderResourceLayoutD3D12.cpp @@ -38,6 +38,7 @@ #include "RootSignature.hpp" #include "PipelineStateD3D12Impl.hpp" #include "ShaderResourceVariableBase.hpp" +#include "ShaderVariableD3DBase.hpp" namespace Diligent { @@ -429,6 +430,11 @@ template <> struct ResourceViewTraits { static const INTERFACE_ID& IID; + + static bool VerifyView(ITextureViewD3D12* pViewD3D12, const D3DShaderResourceAttribs& Attribs, const char* ShaderName) + { + return true; + } }; const INTERFACE_ID& ResourceViewTraits::IID = IID_TextureViewD3D12; @@ -436,6 +442,11 @@ template <> struct ResourceViewTraits { static const INTERFACE_ID& IID; + + static bool VerifyView(IBufferViewD3D12* pViewD3D12, const D3DShaderResourceAttribs& Attribs, const char* ShaderName) + { + return VerifyBufferViewModeD3D(pViewD3D12, Attribs, ShaderName); + } }; const INTERFACE_ID& ResourceViewTraits::IID = IID_BufferViewD3D12; @@ -454,6 +465,7 @@ void ShaderResourceLayoutD3D12::D3D12Resource::CacheResourceView(IDeviceObject* RefCntAutoPtr pViewD3D12(pView, ResourceViewTraits::IID); #ifdef DILIGENT_DEVELOPMENT VerifyResourceViewBinding(Attribs, GetVariableType(), ArrayIndex, pView, pViewD3D12.RawPtr(), {dbgExpectedViewType}, DstRes.pObject.RawPtr(), ParentResLayout.GetShaderName()); + ResourceViewTraits::VerifyView(pViewD3D12, Attribs, ParentResLayout.GetShaderName()); #endif if (pViewD3D12) { diff --git a/Graphics/GraphicsEngineD3DBase/include/ShaderVariableD3DBase.hpp b/Graphics/GraphicsEngineD3DBase/include/ShaderVariableD3DBase.hpp index 0ea6e397..2497b17d 100644 --- a/Graphics/GraphicsEngineD3DBase/include/ShaderVariableD3DBase.hpp +++ b/Graphics/GraphicsEngineD3DBase/include/ShaderVariableD3DBase.hpp @@ -86,4 +86,53 @@ protected: const SHADER_RESOURCE_VARIABLE_TYPE m_VariableType; }; + +template +bool VerifyBufferViewModeD3D(BufferViewImplType* pViewD3D11, const D3DShaderResourceAttribs& Attribs, const char* ShaderName) +{ + if (pViewD3D11 == nullptr) + return true; + + const auto& ViewDesc = pViewD3D11->GetDesc(); + const auto& BuffDesc = pViewD3D11->GetBuffer()->GetDesc(); + + auto LogBufferBindingError = [&](const char* Msg) // + { + LOG_ERROR_MESSAGE("Error binding buffer view '", ViewDesc.Name, "' of buffer '", BuffDesc.Name, + "' to shader variable '", Attribs.Name, "' in shader '", ShaderName, "': ", Msg); + }; + + switch (Attribs.GetInputType()) + { + case D3D_SIT_TEXTURE: + case D3D_SIT_UAV_RWTYPED: + if (BuffDesc.Mode != BUFFER_MODE_FORMATTED || ViewDesc.Format.ValueType == VT_UNDEFINED) + { + LogBufferBindingError("formatted buffer view is expected."); + return false; + } + break; + + case D3D_SIT_STRUCTURED: + case D3D_SIT_UAV_RWSTRUCTURED: + if (BuffDesc.Mode != BUFFER_MODE_STRUCTURED) + { + LogBufferBindingError("structured buffer view is expected."); + return false; + } + break; + + case D3D_SIT_BYTEADDRESS: + case D3D_SIT_UAV_RWBYTEADDRESS: + if (BuffDesc.Mode != BUFFER_MODE_RAW) + { + LogBufferBindingError("raw buffer view is expected."); + return false; + } + break; + } + + return true; +} + } // namespace Diligent diff --git a/Graphics/GraphicsEngineOpenGL/src/GLPipelineResourceLayout.cpp b/Graphics/GraphicsEngineOpenGL/src/GLPipelineResourceLayout.cpp index 0955a54f..914a09bb 100644 --- a/Graphics/GraphicsEngineOpenGL/src/GLPipelineResourceLayout.cpp +++ b/Graphics/GraphicsEngineOpenGL/src/GLPipelineResourceLayout.cpp @@ -307,6 +307,16 @@ void GLPipelineResourceLayout::SamplerBindInfo::BindResource(IDeviceObject* pVie { auto& CachedBuffSampler = ResourceCache.GetConstSampler(m_Attribs.Binding + ArrayIndex); VerifyResourceViewBinding(m_Attribs, GetType(), ArrayIndex, pView, pViewGL.RawPtr(), {BUFFER_VIEW_SHADER_RESOURCE}, CachedBuffSampler.pView.RawPtr()); + if (pViewGL != nullptr) + { + const auto& ViewDesc = pViewGL->GetDesc(); + const auto& BuffDesc = pViewGL->GetBuffer()->GetDesc(); + if (!(BuffDesc.Mode == BUFFER_MODE_FORMATTED && ViewDesc.Format.ValueType != VT_UNDEFINED || BuffDesc.Mode == BUFFER_MODE_RAW)) + { + LOG_ERROR_MESSAGE("Error binding buffer view '", ViewDesc.Name, "' of buffer '", BuffDesc.Name, "' to shader variable '", + m_Attribs.Name, ": formatted buffer view is expected."); + } + } } #endif ResourceCache.SetBufSampler(m_Attribs.Binding + ArrayIndex, std::move(pViewGL)); @@ -347,6 +357,16 @@ void GLPipelineResourceLayout::ImageBindInfo::BindResource(IDeviceObject* pView, { auto& CachedUAV = ResourceCache.GetConstImage(m_Attribs.Binding + ArrayIndex); VerifyResourceViewBinding(m_Attribs, GetType(), ArrayIndex, pView, pViewGL.RawPtr(), {BUFFER_VIEW_UNORDERED_ACCESS}, CachedUAV.pView.RawPtr()); + if (pViewGL != nullptr) + { + const auto& ViewDesc = pViewGL->GetDesc(); + const auto& BuffDesc = pViewGL->GetBuffer()->GetDesc(); + if (!(BuffDesc.Mode == BUFFER_MODE_FORMATTED && ViewDesc.Format.ValueType != VT_UNDEFINED || BuffDesc.Mode == BUFFER_MODE_RAW)) + { + LOG_ERROR_MESSAGE("Error binding buffer view '", ViewDesc.Name, "' of buffer '", BuffDesc.Name, "' to shader variable '", + m_Attribs.Name, ": formatted buffer view is expected."); + } + } } #endif ResourceCache.SetBufImage(m_Attribs.Binding + ArrayIndex, std::move(pViewGL)); @@ -375,6 +395,16 @@ void GLPipelineResourceLayout::StorageBufferBindInfo::BindResource(IDeviceObject auto& CachedSSBO = ResourceCache.GetConstSSBO(m_Attribs.Binding + ArrayIndex); // HLSL structured buffers are mapped to SSBOs in GLSL VerifyResourceViewBinding(m_Attribs, GetType(), ArrayIndex, pView, pViewGL.RawPtr(), {BUFFER_VIEW_SHADER_RESOURCE, BUFFER_VIEW_UNORDERED_ACCESS}, CachedSSBO.pBufferView.RawPtr()); + if (pViewGL != nullptr) + { + const auto& ViewDesc = pViewGL->GetDesc(); + const auto& BuffDesc = pViewGL->GetBuffer()->GetDesc(); + if (BuffDesc.Mode != BUFFER_MODE_STRUCTURED && BuffDesc.Mode != BUFFER_MODE_RAW) + { + LOG_ERROR_MESSAGE("Error binding buffer view '", ViewDesc.Name, "' of buffer '", BuffDesc.Name, "' to shader variable '", + m_Attribs.Name, ": structured buffer view is expected."); + } + } } #endif ResourceCache.SetSSBO(m_Attribs.Binding + ArrayIndex, std::move(pViewGL)); diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp index 8b00c394..7f726e69 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp @@ -702,6 +702,16 @@ void ShaderResourceLayoutVk::VkResource::CacheStorageBuffer(IDeviceObject* // HLSL buffer SRVs are mapped to storge buffers in GLSL auto RequiredViewType = SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::ROStorageBuffer ? BUFFER_VIEW_SHADER_RESOURCE : BUFFER_VIEW_UNORDERED_ACCESS; VerifyResourceViewBinding(SpirvAttribs, GetVariableType(), ArrayInd, pBufferView, pBufferViewVk.RawPtr(), {RequiredViewType}, DstRes.pObject.RawPtr(), ParentResLayout.GetShaderName()); + if (pBufferViewVk != nullptr) + { + const auto& ViewDesc = pBufferViewVk->GetDesc(); + const auto& BuffDesc = pBufferViewVk->GetBuffer()->GetDesc(); + if (BuffDesc.Mode != BUFFER_MODE_STRUCTURED && BuffDesc.Mode != BUFFER_MODE_RAW) + { + LOG_ERROR_MESSAGE("Error binding buffer view '", ViewDesc.Name, "' of buffer '", BuffDesc.Name, "' to shader variable '", + SpirvAttribs.Name, "' in shader '", ParentResLayout.GetShaderName(), "': structured buffer view is expected."); + } + } } #endif @@ -748,6 +758,16 @@ void ShaderResourceLayoutVk::VkResource::CacheTexelBuffer(IDeviceObject* // HLSL buffer SRVs are mapped to storge buffers in GLSL auto RequiredViewType = SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::StorageTexelBuffer ? BUFFER_VIEW_UNORDERED_ACCESS : BUFFER_VIEW_SHADER_RESOURCE; VerifyResourceViewBinding(SpirvAttribs, GetVariableType(), ArrayInd, pBufferView, pBufferViewVk.RawPtr(), {RequiredViewType}, DstRes.pObject.RawPtr(), ParentResLayout.GetShaderName()); + if (pBufferViewVk != nullptr) + { + const auto& ViewDesc = pBufferViewVk->GetDesc(); + const auto& BuffDesc = pBufferViewVk->GetBuffer()->GetDesc(); + if (!(BuffDesc.Mode == BUFFER_MODE_FORMATTED && ViewDesc.Format.ValueType != VT_UNDEFINED || BuffDesc.Mode == BUFFER_MODE_RAW)) + { + LOG_ERROR_MESSAGE("Error binding buffer view '", ViewDesc.Name, "' of buffer '", BuffDesc.Name, "' to shader variable '", + SpirvAttribs.Name, "' in shader '", ParentResLayout.GetShaderName(), "': formatted buffer view is expected."); + } + } } #endif -- cgit v1.2.3