From d4b8e0c30214065e63d428be3219f6db18e02a2d Mon Sep 17 00:00:00 2001 From: assiduous Date: Mon, 23 Dec 2019 12:33:55 -0800 Subject: Fixed staging buffers in Vulkan and D3D12 backends (https://github.com/DiligentGraphics/DiligentEngine/issues/69). Added buffer access test. --- .../src/DeviceContextD3D12Impl.cpp | 22 ++++++++++------ .../GraphicsEngineVulkan/include/BufferVkImpl.h | 10 +++++++- Graphics/GraphicsEngineVulkan/src/BufferVkImpl.cpp | 6 ++--- .../src/DeviceContextVkImpl.cpp | 29 +++++++++++++--------- 4 files changed, 43 insertions(+), 24 deletions(-) (limited to 'Graphics') diff --git a/Graphics/GraphicsEngineD3D12/src/DeviceContextD3D12Impl.cpp b/Graphics/GraphicsEngineD3D12/src/DeviceContextD3D12Impl.cpp index 12de666b..5d35eb82 100644 --- a/Graphics/GraphicsEngineD3D12/src/DeviceContextD3D12Impl.cpp +++ b/Graphics/GraphicsEngineD3D12/src/DeviceContextD3D12Impl.cpp @@ -1083,10 +1083,16 @@ void DeviceContextD3D12Impl::MapBuffer(IBuffer* pBuffer, MAP_TYPE MapType, MAP_F if (MapType == MAP_READ) { - LOG_WARNING_MESSAGE_ONCE("Mapping CPU buffer for reading on D3D12 currently requires flushing context and idling GPU"); - Flush(); - m_pDevice->IdleGPU(); - VERIFY(BuffDesc.Usage == USAGE_STAGING, "Buffer must be created as USAGE_STAGING to be mapped for reading"); + DEV_CHECK_ERR(BuffDesc.Usage == USAGE_STAGING, "Buffer must be created as USAGE_STAGING to be mapped for reading"); + DEV_CHECK_ERR(pd3d12Resource != nullptr, "USAGE_STAGING buffer must intialize D3D12 resource"); + + if ((MapFlags & MAP_FLAG_DO_NOT_WAIT) == 0) + { + LOG_WARNING_MESSAGE("D3D12 backend never waits for GPU when mapping staging buffers for reading. " + "Applications must use fences or other synchronization methods to explicitly synchronize " + "access and use MAP_FLAG_DO_NOT_WAIT flag."); + } + D3D12_RANGE MapRange; MapRange.Begin = 0; MapRange.End = BuffDesc.uiSizeInBytes; @@ -1096,7 +1102,7 @@ void DeviceContextD3D12Impl::MapBuffer(IBuffer* pBuffer, MAP_TYPE MapType, MAP_F { if (BuffDesc.Usage == USAGE_STAGING) { - VERIFY(pd3d12Resource != nullptr, "USAGE_STAGING buffer mapped for writing must intialize D3D12 resource"); + DEV_CHECK_ERR(pd3d12Resource != nullptr, "USAGE_STAGING buffer mapped for writing must intialize D3D12 resource"); if (MapFlags & MAP_FLAG_DISCARD) { } @@ -1104,7 +1110,7 @@ void DeviceContextD3D12Impl::MapBuffer(IBuffer* pBuffer, MAP_TYPE MapType, MAP_F } else if (BuffDesc.Usage == USAGE_DYNAMIC) { - VERIFY((MapFlags & (MAP_FLAG_DISCARD | MAP_FLAG_NO_OVERWRITE)) != 0, "D3D12 buffer must be mapped for writing with MAP_FLAG_DISCARD or MAP_FLAG_NO_OVERWRITE flag"); + DEV_CHECK_ERR((MapFlags & (MAP_FLAG_DISCARD | MAP_FLAG_NO_OVERWRITE)) != 0, "D3D12 buffer must be mapped for writing with MAP_FLAG_DISCARD or MAP_FLAG_NO_OVERWRITE flag"); auto& DynamicData = pBufferD3D12->m_DynamicData[m_ContextId]; if ((MapFlags & MAP_FLAG_DISCARD) != 0 || DynamicData.CPUAddress == nullptr) { @@ -1564,9 +1570,9 @@ void DeviceContextD3D12Impl::MapTextureSubresource(ITexture* pTe { if ((MapFlags & MAP_FLAG_DO_NOT_WAIT) == 0) { - LOG_WARNING_MESSAGE("Mapping staging textures for reading never blocks or waits for GPU in D3D12 backend. " + LOG_WARNING_MESSAGE("D3D12 backend never waits for GPU when mapping staging textures for reading. " "Applications must use fences or other synchronization methods to explicitly synchronize " - "access and map texture with MAP_FLAG_DO_NOT_WAIT flag."); + "access and use MAP_FLAG_DO_NOT_WAIT flag."); } DEV_CHECK_ERR((TexDesc.CPUAccessFlags & CPU_ACCESS_READ), "Texture '", TexDesc.Name, "' was not created with CPU_ACCESS_READ flag and can't be mapped for reading"); diff --git a/Graphics/GraphicsEngineVulkan/include/BufferVkImpl.h b/Graphics/GraphicsEngineVulkan/include/BufferVkImpl.h index cf8923f3..30f8206f 100644 --- a/Graphics/GraphicsEngineVulkan/include/BufferVkImpl.h +++ b/Graphics/GraphicsEngineVulkan/include/BufferVkImpl.h @@ -108,13 +108,21 @@ public: return (GetAccessFlags() & AccessFlags) == AccessFlags; } + void* GetStagingCPUAddress() + { + VERIFY_EXPR(m_Desc.Usage == USAGE_STAGING); + return reinterpret_cast(m_MemoryAllocation.Page->GetCPUMemory()) + m_BufferMemoryAlignedOffset; + } + private: friend class DeviceContextVkImpl; virtual void CreateViewInternal(const struct BufferViewDesc& ViewDesc, IBufferView** ppView, bool bIsDefaultView) override; VulkanUtilities::BufferViewWrapper CreateView(struct BufferViewDesc& ViewDesc); - Uint32 m_DynamicOffsetAlignment = 0; + + Uint32 m_DynamicOffsetAlignment = 0; + VkDeviceSize m_BufferMemoryAlignedOffset = 0; std::vector> m_DynamicAllocations; diff --git a/Graphics/GraphicsEngineVulkan/src/BufferVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/BufferVkImpl.cpp index 4b34ca5f..65737f0b 100644 --- a/Graphics/GraphicsEngineVulkan/src/BufferVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/BufferVkImpl.cpp @@ -189,10 +189,10 @@ BufferVkImpl::BufferVkImpl(IReferenceCounters* pRefCounters, VERIFY(IsPowerOfTwo(MemReqs.alignment), "Alignment is not power of 2!"); m_MemoryAllocation = pRenderDeviceVk->AllocateMemory(MemReqs, BufferMemoryFlags); - auto AlignedOffset = Align(VkDeviceSize{m_MemoryAllocation.UnalignedOffset}, MemReqs.alignment); - VERIFY(m_MemoryAllocation.Size >= MemReqs.size + (AlignedOffset - m_MemoryAllocation.UnalignedOffset), "Size of memory allocation is too small"); + m_BufferMemoryAlignedOffset = Align(VkDeviceSize{m_MemoryAllocation.UnalignedOffset}, MemReqs.alignment); + VERIFY(m_MemoryAllocation.Size >= MemReqs.size + (m_BufferMemoryAlignedOffset - m_MemoryAllocation.UnalignedOffset), "Size of memory allocation is too small"); auto Memory = m_MemoryAllocation.Page->GetVkMemory(); - auto err = LogicalDevice.BindBufferMemory(m_VulkanBuffer, Memory, AlignedOffset); + auto err = LogicalDevice.BindBufferMemory(m_VulkanBuffer, Memory, m_BufferMemoryAlignedOffset); CHECK_VK_ERROR_AND_THROW(err, "Failed to bind buffer memory"); bool bInitializeBuffer = (pBuffData != nullptr && pBuffData->pData != nullptr && pBuffData->DataSize > 0); diff --git a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp index 690b0589..6122ffda 100644 --- a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp @@ -1297,15 +1297,22 @@ void DeviceContextVkImpl::MapBuffer(IBuffer* pBuffer, MAP_TYPE MapType, MAP_FLAG if (MapType == MAP_READ) { - LOG_ERROR("Mapping buffer for reading is not yet imlemented in Vulkan backend"); - UNSUPPORTED("Mapping buffer for reading is not yet imlemented in Vulkan backend"); + DEV_CHECK_ERR(BuffDesc.Usage == USAGE_STAGING, "Buffer must be created as USAGE_STAGING to be mapped for reading"); + + if ((MapFlags & MAP_FLAG_DO_NOT_WAIT) == 0) + { + LOG_WARNING_MESSAGE("Vulkan backend never waits for GPU when mapping staging buffers for reading. " + "Applications must use fences or other synchronization methods to explicitly synchronize " + "access and use MAP_FLAG_DO_NOT_WAIT flag."); + } + + pMappedData = pBufferVk->GetStagingCPUAddress(); } else if (MapType == MAP_WRITE) { if (BuffDesc.Usage == USAGE_STAGING) { - LOG_ERROR("Not implemented"); - UNSUPPORTED("Not implemented"); + pMappedData = pBufferVk->GetStagingCPUAddress(); } else if (BuffDesc.Usage == USAGE_DYNAMIC) { @@ -1340,11 +1347,11 @@ void DeviceContextVkImpl::MapBuffer(IBuffer* pBuffer, MAP_TYPE MapType, MAP_FLAG } else if (MapType == MAP_READ_WRITE) { - LOG_ERROR("MAP_READ_WRITE is not supported on Vk"); + LOG_ERROR("MAP_READ_WRITE is not supported in Vulkan backend"); } else { - LOG_ERROR("Only MAP_WRITE_DISCARD and MAP_READ are currently implemented in Vk"); + UNEXPECTED("Unknown map type"); } } @@ -1356,15 +1363,13 @@ void DeviceContextVkImpl::UnmapBuffer(IBuffer* pBuffer, MAP_TYPE MapType) if (MapType == MAP_READ) { - LOG_ERROR("This map type is not yet supported"); - UNSUPPORTED("This map type is not yet supported"); + // We are currently using cache-coherent memory, so there is no need to invalidated mapped range } else if (MapType == MAP_WRITE) { if (BuffDesc.Usage == USAGE_STAGING) { - LOG_ERROR("This map type is not yet supported"); - UNSUPPORTED("This map type is not yet supported"); + // We are currently using cache-coherent memory, so there is no need to flush mapped range } else if (BuffDesc.Usage == USAGE_DYNAMIC) { @@ -1864,9 +1869,9 @@ void DeviceContextVkImpl::MapTextureSubresource(ITexture* pTextu { if ((MapFlags & MAP_FLAG_DO_NOT_WAIT) == 0) { - LOG_WARNING_MESSAGE("Mapping staging textures for reading never blocks or waits for GPU in Vulkan backend. " + LOG_WARNING_MESSAGE("Vulkan backend never waits for GPU when mapping staging textures for reading. " "Applications must use fences or other synchronization methods to explicitly synchronize " - "access and map texture with MAP_FLAG_DO_NOT_WAIT flag."); + "access and use MAP_FLAG_DO_NOT_WAIT flag."); } DEV_CHECK_ERR((TexDesc.CPUAccessFlags & CPU_ACCESS_READ), "Texture '", TexDesc.Name, "' was not created with CPU_ACCESS_READ flag and can't be mapped for reading"); -- cgit v1.2.3