From bbcf97efa28653b1510bc7f9fa79f715dd67a1fc Mon Sep 17 00:00:00 2001 From: baldurk Date: Tue, 4 Jun 2019 15:16:38 +0100 Subject: [PATCH] Use nonCoherentAtomSize as an additional alignment for Vulkan buffers * This isn't technically true but it makes the code much simpler to align offsets and sizes to it, so that then map flushes and invalidates all happen on multiples of the atom size. --- renderdoc/driver/vulkan/vk_common.cpp | 16 ++++++++++++---- renderdoc/driver/vulkan/vk_memory.cpp | 5 +++++ renderdoc/driver/vulkan/vk_outputwindow.cpp | 2 +- renderdoc/driver/vulkan/vk_rendertext.cpp | 2 +- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/renderdoc/driver/vulkan/vk_common.cpp b/renderdoc/driver/vulkan/vk_common.cpp index 07201c9f0..608c563da 100644 --- a/renderdoc/driver/vulkan/vk_common.cpp +++ b/renderdoc/driver/vulkan/vk_common.cpp @@ -134,10 +134,13 @@ void GPUBuffer::Create(WrappedVulkan *driver, VkDevice dev, VkDeviceSize size, u align = (VkDeviceSize)driver->GetDeviceProps().limits.minUniformBufferOffsetAlignment; + // for simplicity, consider the non-coherent atom size also an alignment requirement + align = AlignUp(align, driver->GetDeviceProps().limits.nonCoherentAtomSize); + sz = size; // offset must be aligned, so ensure we have at least ringSize // copies accounting for that - totalsize = ringSize == 1 ? size : AlignUp(size, align) * ringSize; + totalsize = AlignUp(size, align) * RDCMAX(1U, ringSize); curoffset = 0; ringCount = ringSize; @@ -208,11 +211,16 @@ void *GPUBuffer::Map(uint32_t *bindoffset, VkDeviceSize usedsize) VkDeviceSize offset = bindoffset ? curoffset : 0; VkDeviceSize size = usedsize > 0 ? usedsize : sz; - // wrap around the ring, assuming the ring is large enough - // that this memory is now free + // align the size so we always consume coherent atoms + size = AlignUp(size, align); + + // wrap around the ring as soon as the 'sz' would overflow. This is because if we're using dynamic + // offsets in the descriptor the range is still set to that fixed size and the validation + // complains if we go off the end (even if it's unused). Rather than constantly update the + // descriptor, we just conservatively wrap and waste the last bit of space. if(offset + sz > totalsize) offset = 0; - RDCASSERT(offset + sz <= totalsize); + RDCASSERT(offset + size <= totalsize); // offset must be aligned curoffset = AlignUp(offset + size, align); diff --git a/renderdoc/driver/vulkan/vk_memory.cpp b/renderdoc/driver/vulkan/vk_memory.cpp index e6047b001..19892f200 100644 --- a/renderdoc/driver/vulkan/vk_memory.cpp +++ b/renderdoc/driver/vulkan/vk_memory.cpp @@ -178,11 +178,16 @@ void WrappedVulkan::RemapMemoryIndices(VkPhysicalDeviceMemoryProperties *memProp MemoryAllocation WrappedVulkan::AllocateMemoryForResource(bool buffer, VkMemoryRequirements mrq, MemoryScope scope, MemoryType type) { + const VkDeviceSize nonCoherentAtomSize = GetDeviceProps().limits.nonCoherentAtomSize; + MemoryAllocation ret; ret.scope = scope; ret.type = type; ret.buffer = buffer; ret.size = AlignUp(mrq.size, mrq.alignment); + // for ease, ensure all allocations are multiples of the non-coherent atom size, so we can + // invalidate/flush safely. This is at most 256 bytes which is likely already satisfied. + ret.size = AlignUp(ret.size, nonCoherentAtomSize); RDCDEBUG("Allocating 0x%llx with alignment 0x%llx in 0x%x for a %s (%s in %s)", ret.size, mrq.alignment, mrq.memoryTypeBits, buffer ? "buffer" : "image", ToStr(type).c_str(), diff --git a/renderdoc/driver/vulkan/vk_outputwindow.cpp b/renderdoc/driver/vulkan/vk_outputwindow.cpp index 86cc91a16..f7233a62e 100644 --- a/renderdoc/driver/vulkan/vk_outputwindow.cpp +++ b/renderdoc/driver/vulkan/vk_outputwindow.cpp @@ -721,7 +721,7 @@ void VulkanReplay::GetOutputWindowData(uint64_t id, bytebuf &retData) RDCASSERT(pData != NULL); VkMappedMemoryRange range = { - VK_STRUCTURE_TYPE_MAPPED_MEMORY_RANGE, NULL, readbackMem, 0, bufInfo.size, + VK_STRUCTURE_TYPE_MAPPED_MEMORY_RANGE, NULL, readbackMem, 0, VK_WHOLE_SIZE, }; vkr = vt->InvalidateMappedMemoryRanges(Unwrap(device), 1, &range); diff --git a/renderdoc/driver/vulkan/vk_rendertext.cpp b/renderdoc/driver/vulkan/vk_rendertext.cpp index 3ae416f6a..a596a2605 100644 --- a/renderdoc/driver/vulkan/vk_rendertext.cpp +++ b/renderdoc/driver/vulkan/vk_rendertext.cpp @@ -263,7 +263,7 @@ VulkanTextRenderer::VulkanTextRenderer(WrappedVulkan *driver) // we only use a subset of the [MAX_SINGLE_LINE_LENGTH] array needed for each line, so this ring // can be smaller - m_TextStringUBO.Create(driver, dev, 4096, 10, 0); + m_TextStringUBO.Create(driver, dev, 4096, 20, 0); RDCCOMPILE_ASSERT(sizeof(StringUBOData) <= 4096, "font uniforms size"); rm->SetInternalResource(GetResID(m_TextStringUBO.buf));