diff --git a/renderdoc/driver/vulkan/vk_resources.h b/renderdoc/driver/vulkan/vk_resources.h index b8c68afc1..734bf4aca 100644 --- a/renderdoc/driver/vulkan/vk_resources.h +++ b/renderdoc/driver/vulkan/vk_resources.h @@ -1080,22 +1080,14 @@ struct PipelineLayoutData struct MemMapState { - MemMapState() - : mapOffset(0), - mapSize(0), - needRefData(false), - mapFlushed(false), - mapCoherent(false), - mappedPtr(NULL), - refData(NULL) - { - } - VkDeviceSize mapOffset, mapSize; - bool needRefData; - bool mapFlushed; - bool mapCoherent; - byte *mappedPtr; - byte *refData; + VkDeviceSize mapOffset = 0, mapSize = 0; + bool needRefData = false; + bool mapFlushed = false; + bool mapCoherent = false; + // pointer to base of memory, may not be valid until after mapOffset bytes + byte *mappedPtr = NULL; + // this is map sized, not memory sized, rebased at the map offset. + byte *refData = NULL; Threading::CriticalSection mrLock; }; diff --git a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp index b102e41de..fc66a0499 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp @@ -1079,8 +1079,8 @@ VkResult WrappedVulkan::vkQueueSubmit(VkQueue queue, uint32_t submitCount, // if we have a previous set of data, compare. // otherwise just serialise it all if(state.refData) - found = FindDiffRange((byte *)state.mappedPtr, state.refData, (size_t)state.mapSize, - diffStart, diffEnd); + found = FindDiffRange(((byte *)state.mappedPtr) + state.mapOffset, state.refData, + (size_t)state.mapSize, diffStart, diffEnd); else #endif diffEnd = (size_t)state.mapSize; diff --git a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp index 3c748c251..0cefa12da 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp @@ -581,9 +581,19 @@ void WrappedVulkan::vkFreeMemory(VkDevice device, VkDeviceMemory memory, VkResult WrappedVulkan::vkMapMemory(VkDevice device, VkDeviceMemory mem, VkDeviceSize offset, VkDeviceSize size, VkMemoryMapFlags flags, void **ppData) { - void *realData = NULL; - VkResult ret = - ObjDisp(device)->MapMemory(Unwrap(device), Unwrap(mem), offset, size, flags, &realData); + // ensure we always map on a 16-byte boundary. This is for our own purposes so we can + // FindDiffRange against the mapped region. We adjust the pointer returned to the user but + // otherwise we act as if the mapped region was 16-byte aligned. Fortunately flushed regions in + // vkFlushMappedMemoryRanges are relative to the memory base, not the mapped region, so this + // offset effectively only modifies the returned pointer and has no other side-effects. + VkDeviceSize misalignedOffset = offset & 0xf; + offset &= ~0xf; + // need to adjust the size so the end-point is still the same! + size += misalignedOffset; + + byte *realData = NULL; + VkResult ret = ObjDisp(device)->MapMemory(Unwrap(device), Unwrap(mem), offset, size, flags, + (void **)&realData); if(ret == VK_SUCCESS && realData) { @@ -599,9 +609,11 @@ VkResult WrappedVulkan::vkMapMemory(VkDevice device, VkDeviceMemory mem, VkDevic MemMapState &state = *memrecord->memMapState; // ensure size is valid - RDCASSERT(size == VK_WHOLE_SIZE || (size > 0 && size <= memrecord->Length), GetResID(mem), - size, memrecord->Length); + RDCASSERT(size == VK_WHOLE_SIZE || (size > 0 && offset + size <= memrecord->Length), + GetResID(mem), size, memrecord->Length); + // flush range offsets are relative to the start of the memory so keep mappedPtr at that + // basis. We'll only access within the mapped range state.mappedPtr = (byte *)realData - (size_t)offset; state.refData = NULL; @@ -609,7 +621,7 @@ VkResult WrappedVulkan::vkMapMemory(VkDevice device, VkDeviceMemory mem, VkDevic state.mapSize = size == VK_WHOLE_SIZE ? (memrecord->Length - offset) : size; state.mapFlushed = false; - *ppData = realData; + *ppData = realData + misalignedOffset; if(state.mapCoherent) { @@ -619,7 +631,7 @@ VkResult WrappedVulkan::vkMapMemory(VkDevice device, VkDeviceMemory mem, VkDevic } else { - *ppData = realData; + *ppData = realData + misalignedOffset; } } else @@ -816,7 +828,7 @@ bool WrappedVulkan::Serialise_vkFlushMappedMemoryRanges(SerialiserType &ser, VkD if(!state->refData) { // if we're in this case, the range should be for the whole memory region. - RDCASSERT(MemRange.offset == 0 && memRangeSize == state->mapSize); + RDCASSERT(MemRange.offset == state->mapOffset && memRangeSize == state->mapSize); // allocate ref data so we can compare next time to minimise serialised data state->refData = AllocAlignedBuffer((size_t)state->mapSize); diff --git a/util/test/demos/vk/vk_misaligned_dirty.cpp b/util/test/demos/vk/vk_misaligned_dirty.cpp index 9c8e1b735..43a34bbbd 100644 --- a/util/test/demos/vk/vk_misaligned_dirty.cpp +++ b/util/test/demos/vk/vk_misaligned_dirty.cpp @@ -58,27 +58,39 @@ RD_TEST(VK_Misaligned_Dirty, VulkanGraphicsTest) const float val = 2.0f / 3.0f; - const DefaultA2V tri[4] = { + DefaultA2V tri[4] = { {Vec3f(-val, -val, val), Vec4f(0.0f, 1.0f, 0.0f, 1.0f), Vec2f(0.0f, 0.0f)}, {Vec3f(0.0f, val, val), Vec4f(0.0f, 1.0f, 0.0f, 1.0f), Vec2f(0.0f, 1.0f)}, {Vec3f(val, -val, val), Vec4f(0.0f, 1.0f, 0.0f, 1.0f), Vec2f(1.0f, 0.0f)}, {}, }; - AllocatedBuffer vb(this, vkh::BufferCreateInfo(sizeof(tri), VK_BUFFER_USAGE_VERTEX_BUFFER_BIT | - VK_BUFFER_USAGE_TRANSFER_DST_BIT), - VmaAllocationCreateInfo({0, VMA_MEMORY_USAGE_CPU_TO_GPU})); - - vb.upload(tri); - AllocatedBuffer copy_src( this, vkh::BufferCreateInfo( sizeof(tri), VK_BUFFER_USAGE_TRANSFER_SRC_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT), VmaAllocationCreateInfo({0, VMA_MEMORY_USAGE_CPU_TO_GPU})); + setName(copy_src.buffer, "copy_src"); + + AllocatedBuffer vb(this, vkh::BufferCreateInfo(sizeof(tri), VK_BUFFER_USAGE_VERTEX_BUFFER_BIT | + VK_BUFFER_USAGE_TRANSFER_DST_BIT), + VmaAllocationCreateInfo({0, VMA_MEMORY_USAGE_CPU_TO_GPU})); + + setName(vb.buffer, "vb"); + + vb.upload(tri); + + tri[0].pos = Vec3f(0.0f, 0.0f, 10.0f); + copy_src.upload(tri); - float *mapped = (float *)(copy_src.map() + sizeof(DefaultA2V) * 3); + VmaAllocationInfo alloc_info; + + vmaGetAllocationInfo(copy_src.allocator, copy_src.alloc, &alloc_info); + + float *mapped = NULL; + vkMapMemory(device, alloc_info.deviceMemory, alloc_info.offset + sizeof(DefaultA2V) * 3, + sizeof(Vec4f), 0, (void **)&mapped); float counter = 0; while(Running()) @@ -91,6 +103,7 @@ RD_TEST(VK_Misaligned_Dirty, VulkanGraphicsTest) // create a dummy submit which uses the memory. This will serialise the whole memory contents // (we don't create reference data until after this) vkBeginCommandBuffer(cmd, vkh::CommandBufferBeginInfo()); + setMarker(cmd, "First Submit"); vkCmdUpdateBuffer(cmd, copy_src.buffer, sizeof(Vec3f), sizeof(Vec4f), &tri[0].col); vkEndCommandBuffer(cmd); Submit(0, 2, {cmd}); @@ -110,10 +123,14 @@ RD_TEST(VK_Misaligned_Dirty, VulkanGraphicsTest) vkh::ImageSubresourceRange()); VkBufferCopy region = {}; - region.srcOffset = 3; - region.dstOffset = 3; + region.srcOffset = 3 + sizeof(DefaultA2V); + region.dstOffset = 3 + sizeof(DefaultA2V); region.size = 7; vkCmdCopyBuffer(cmd, copy_src.buffer, vb.buffer, 1, ®ion); + region.srcOffset = sizeof(DefaultA2V) * 3; + region.dstOffset = sizeof(DefaultA2V) * 3; + region.size = sizeof(DefaultA2V); + vkCmdCopyBuffer(cmd, copy_src.buffer, vb.buffer, 1, ®ion); vkCmdBeginRenderPass( cmd, vkh::RenderPassBeginInfo(mainWindow->rp, mainWindow->GetFB(), mainWindow->scissor), @@ -123,6 +140,7 @@ RD_TEST(VK_Misaligned_Dirty, VulkanGraphicsTest) vkCmdSetViewport(cmd, 0, 1, &mainWindow->viewport); vkCmdSetScissor(cmd, 0, 1, &mainWindow->scissor); vkh::cmdBindVertexBuffers(cmd, 0, {vb.buffer}, {0}); + setMarker(cmd, "Second Submit"); vkCmdDraw(cmd, 3, 1, 0, 0); vkCmdEndRenderPass(cmd); @@ -136,7 +154,7 @@ RD_TEST(VK_Misaligned_Dirty, VulkanGraphicsTest) Present(); } - copy_src.unmap(); + vkUnmapMemory(device, alloc_info.deviceMemory); return 0; } diff --git a/util/test/tests/Vulkan/VK_Misaligned_Dirty.py b/util/test/tests/Vulkan/VK_Misaligned_Dirty.py index a3f9c445f..58c6773d6 100644 --- a/util/test/tests/Vulkan/VK_Misaligned_Dirty.py +++ b/util/test/tests/Vulkan/VK_Misaligned_Dirty.py @@ -1,4 +1,5 @@ import renderdoc as rd +import struct import rdtest @@ -74,3 +75,37 @@ class VK_Misaligned_Dirty(rdtest.TestCase): self.check_pixel_value(tex, coord[0], coord[1], [0.0, 1.0, 0.0, 1.0]) rdtest.log.success("picked values are as expected") + + checkpoint1 = self.find_draw("First Submit") + checkpoint2 = self.find_draw("Second Submit") + + self.check(checkpoint1 is not None) + self.check(checkpoint2 is not None) + + resources = self.controller.GetResources() + + copy_src = None + vb = None + + for r in resources: + if r.name == 'copy_src': + copy_src = r.resourceId + elif r.name == 'vb': + vb = r.resourceId + + self.check(copy_src is not None) + self.check(vb is not None) + + self.controller.SetFrameEvent(checkpoint1.eventId, False) + + val = struct.unpack('f', self.controller.GetBufferData(copy_src, 116, 4)) + self.check(val[0] == 10.0) + + self.controller.SetFrameEvent(checkpoint2.eventId, False) + + val = struct.unpack('f', self.controller.GetBufferData(copy_src, 116, 4)) + self.check(val[0] == 11.0) + val = struct.unpack('f', self.controller.GetBufferData(vb, 116, 4)) + self.check(val[0] == 11.0) + + rdtest.log.success("buffers have correct values in both submits")