From 217506869fa0f345e3014a98d0842ddfced00445 Mon Sep 17 00:00:00 2001 From: baldurk Date: Tue, 4 Jun 2019 18:17:26 +0100 Subject: [PATCH] Only patch vulkan image layouts at the last second * Instead of patching e.g. PRESENT to GENERAL early, we keep the 'real' layout even if that's UNDEFINED or PRESENT or whatever. We then do a last-second patchup whenever we're actually transitioning images in vulkan itself, to set the right layout. * This requires us to do the patching in a few more places - anywhere like texture rendering or initial states where we want to go from current state -> custom state -> back to current state. * This also allows us to more gracefully handle PREINITIALIZED image layouts. We internally promote them to GENERAL as soon as possible, but keep them around as PREINITIALIZED for display. --- renderdoc/driver/vulkan/vk_common.cpp | 25 +- renderdoc/driver/vulkan/vk_common.h | 3 +- renderdoc/driver/vulkan/vk_core.cpp | 17 +- renderdoc/driver/vulkan/vk_initstate.cpp | 33 +++ renderdoc/driver/vulkan/vk_manager.cpp | 50 +++- renderdoc/driver/vulkan/vk_rendertexture.cpp | 3 + renderdoc/driver/vulkan/vk_replay.cpp | 5 + renderdoc/driver/vulkan/vk_resources.h | 1 + .../driver/vulkan/wrappers/vk_cmd_funcs.cpp | 20 +- .../driver/vulkan/wrappers/vk_misc_funcs.cpp | 10 +- .../driver/vulkan/wrappers/vk_queue_funcs.cpp | 7 +- .../vulkan/wrappers/vk_resource_funcs.cpp | 6 +- .../driver/vulkan/wrappers/vk_sync_funcs.cpp | 17 +- .../driver/vulkan/wrappers/vk_wsi_funcs.cpp | 2 + util/test/demos/CMakeLists.txt | 1 + util/test/demos/demos.vcxproj | 1 + util/test/demos/demos.vcxproj.filters | 3 + util/test/demos/vk/vk_image_layouts.cpp | 239 ++++++++++++++++++ util/test/tests/Vulkan/VK_Image_Layouts.py | 69 +++++ 19 files changed, 471 insertions(+), 41 deletions(-) create mode 100644 util/test/demos/vk/vk_image_layouts.cpp create mode 100644 util/test/tests/Vulkan/VK_Image_Layouts.py diff --git a/renderdoc/driver/vulkan/vk_common.cpp b/renderdoc/driver/vulkan/vk_common.cpp index 608c563da..51593d557 100644 --- a/renderdoc/driver/vulkan/vk_common.cpp +++ b/renderdoc/driver/vulkan/vk_common.cpp @@ -328,10 +328,33 @@ VkAccessFlags MakeAccessMask(VkImageLayout layout) return VkAccessFlags(0); } -void ReplacePresentableImageLayout(VkImageLayout &layout) +void SanitiseOldImageLayout(VkImageLayout &layout) { + // we don't replay with present layouts since we don't create actual swapchains. So change any + // present layouts to general layouts if(layout == VK_IMAGE_LAYOUT_PRESENT_SRC_KHR || layout == VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR) layout = VK_IMAGE_LAYOUT_GENERAL; + + // we can't transition to PREINITIALIZED, so instead use GENERAL. This allows host access so we + // can still replay maps of the image's memory. In theory we can still transition from + // PREINITIALIZED on replay, but consider that we need to be able to reset layouts and suddenly we + // have a problem transitioning from PREINITIALIZED more than once - so for that reason we + // instantly promote any images that are PREINITIALIZED to GENERAL at the start of the frame + // capture, and from then on treat it as the same + if(layout == VK_IMAGE_LAYOUT_PREINITIALIZED) + layout = VK_IMAGE_LAYOUT_GENERAL; +} + +void SanitiseNewImageLayout(VkImageLayout &layout) +{ + // apply any general image layout sanitisation + SanitiseOldImageLayout(layout); + + // we also can't transition to UNDEFINED, so go to GENERAL instead. This is safe since if the + // layout was supposed to be undefined before then the only valid transition *from* the state is + // UNDEFINED, which will work silently. + if(layout == VK_IMAGE_LAYOUT_UNDEFINED) + layout = VK_IMAGE_LAYOUT_GENERAL; } int SampleCount(VkSampleCountFlagBits countFlag) diff --git a/renderdoc/driver/vulkan/vk_common.h b/renderdoc/driver/vulkan/vk_common.h index 4554fc632..5120d1d0b 100644 --- a/renderdoc/driver/vulkan/vk_common.h +++ b/renderdoc/driver/vulkan/vk_common.h @@ -102,7 +102,8 @@ StencilOperation MakeStencilOp(VkStencilOp op); // set conservative access bits for this image layout VkAccessFlags MakeAccessMask(VkImageLayout layout); -void ReplacePresentableImageLayout(VkImageLayout &layout); +void SanitiseOldImageLayout(VkImageLayout &layout); +void SanitiseNewImageLayout(VkImageLayout &layout); void DoPipelineBarrier(VkCommandBuffer cmd, uint32_t count, VkImageMemoryBarrier *barriers); void DoPipelineBarrier(VkCommandBuffer cmd, uint32_t count, VkBufferMemoryBarrier *barriers); diff --git a/renderdoc/driver/vulkan/vk_core.cpp b/renderdoc/driver/vulkan/vk_core.cpp index 8d77c8f34..006179c97 100644 --- a/renderdoc/driver/vulkan/vk_core.cpp +++ b/renderdoc/driver/vulkan/vk_core.cpp @@ -1313,27 +1313,23 @@ bool WrappedVulkan::Serialise_BeginCaptureFrame(SerialiserType &ser) if(IsLoading(m_State)) { - // for the first load, ensure all images are in a non-undefined layout. Any images that don't - // have an initial layout to transition back into were likely created mid-frame so their state - // is expected to be transitioned from undefined in the capture itself. + // for the first load, promote any PREINITIALIZED images to GENERAL here since we treat + // PREINIT as if it was GENERAL. for(auto it = m_ImageLayouts.begin(); it != m_ImageLayouts.end(); ++it) { for(auto stit = it->second.subresourceStates.begin(); stit != it->second.subresourceStates.end(); ++stit) { - if(stit->newLayout == VK_IMAGE_LAYOUT_UNDEFINED && + if(stit->newLayout == VK_IMAGE_LAYOUT_PREINITIALIZED && GetResourceManager()->HasCurrentResource(it->first)) { VkImage img = GetResourceManager()->GetCurrentHandle(it->first); - if(GetResID(img) != GetResourceManager()->GetOriginalID(GetResID(img))) { VkImageMemoryBarrier barrier = {}; - stit->newLayout = VK_IMAGE_LAYOUT_GENERAL; - barrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER; - barrier.oldLayout = VK_IMAGE_LAYOUT_UNDEFINED; + barrier.oldLayout = VK_IMAGE_LAYOUT_PREINITIALIZED; barrier.newLayout = VK_IMAGE_LAYOUT_GENERAL; barrier.srcQueueFamilyIndex = m_QueueFamilyIdx; barrier.dstQueueFamilyIndex = m_QueueFamilyIdx; @@ -1351,6 +1347,11 @@ bool WrappedVulkan::Serialise_BeginCaptureFrame(SerialiserType &ser) { for(size_t i = 0; i < imgBarriers.size(); i++) { + // sanitise the layouts before passing to Vulkan + if(!IsLoading(m_State)) + SanitiseOldImageLayout(imgBarriers[i].oldLayout); + SanitiseNewImageLayout(imgBarriers[i].newLayout); + imgBarriers[i].srcAccessMask = MakeAccessMask(imgBarriers[i].oldLayout); imgBarriers[i].dstAccessMask = MakeAccessMask(imgBarriers[i].newLayout); } diff --git a/renderdoc/driver/vulkan/vk_initstate.cpp b/renderdoc/driver/vulkan/vk_initstate.cpp index 8764139d9..a0179147a 100644 --- a/renderdoc/driver/vulkan/vk_initstate.cpp +++ b/renderdoc/driver/vulkan/vk_initstate.cpp @@ -331,6 +331,9 @@ bool WrappedVulkan::Prepare_InitialState(WrappedVkRes *res) { srcimBarrier.subresourceRange = layout->subresourceStates[si].subresourceRange; srcimBarrier.oldLayout = layout->subresourceStates[si].newLayout; + + SanitiseOldImageLayout(srcimBarrier.oldLayout); + DoPipelineBarrier(cmd, 1, &srcimBarrier); if(srcimBarrier.srcQueueFamilyIndex != srcimBarrier.dstQueueFamilyIndex) @@ -489,6 +492,9 @@ bool WrappedVulkan::Prepare_InitialState(WrappedVkRes *res) srcimBarrier.subresourceRange = layout->subresourceStates[si].subresourceRange; srcimBarrier.newLayout = layout->subresourceStates[si].newLayout; srcimBarrier.dstAccessMask = MakeAccessMask(srcimBarrier.newLayout); + + SanitiseNewImageLayout(srcimBarrier.newLayout); + DoPipelineBarrier(cmd, 1, &srcimBarrier); if(srcimBarrier.srcQueueFamilyIndex != srcimBarrier.dstQueueFamilyIndex) @@ -1424,6 +1430,9 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten { barrier.subresourceRange = m_ImageLayouts[id].subresourceStates[si].subresourceRange; barrier.oldLayout = m_ImageLayouts[id].subresourceStates[si].newLayout; + + SanitiseOldImageLayout(barrier.oldLayout); + DoPipelineBarrier(cmd, 1, &barrier); if(extQCmd != VK_NULL_HANDLE) @@ -1464,6 +1473,9 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten barrier.subresourceRange = m_ImageLayouts[id].subresourceStates[si].subresourceRange; barrier.newLayout = m_ImageLayouts[id].subresourceStates[si].newLayout; barrier.dstAccessMask |= MakeAccessMask(barrier.newLayout); + + SanitiseNewImageLayout(barrier.newLayout); + DoPipelineBarrier(cmd, 1, &barrier); if(extQCmd != VK_NULL_HANDLE) @@ -1528,6 +1540,9 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten { barrier.subresourceRange = m_ImageLayouts[id].subresourceStates[si].subresourceRange; barrier.oldLayout = m_ImageLayouts[id].subresourceStates[si].newLayout; + + SanitiseOldImageLayout(barrier.oldLayout); + DoPipelineBarrier(cmd, 1, &barrier); if(extQCmd != VK_NULL_HANDLE) @@ -1568,6 +1583,9 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten { barrier.subresourceRange = m_ImageLayouts[id].subresourceStates[si].subresourceRange; barrier.newLayout = m_ImageLayouts[id].subresourceStates[si].newLayout; + + SanitiseNewImageLayout(barrier.newLayout); + DoPipelineBarrier(cmd, 1, &barrier); if(extQCmd != VK_NULL_HANDLE) @@ -1652,6 +1670,9 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten { barrier.subresourceRange = m_ImageLayouts[id].subresourceStates[si].subresourceRange; barrier.oldLayout = m_ImageLayouts[id].subresourceStates[si].newLayout; + + SanitiseOldImageLayout(barrier.oldLayout); + DoPipelineBarrier(cmd, 1, &barrier); if(extQCmd != VK_NULL_HANDLE) @@ -1698,7 +1719,11 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten { barrier.subresourceRange = m_ImageLayouts[id].subresourceStates[si].subresourceRange; barrier.newLayout = m_ImageLayouts[id].subresourceStates[si].newLayout; + + SanitiseNewImageLayout(barrier.newLayout); + barrier.dstAccessMask |= MakeAccessMask(barrier.newLayout); + DoPipelineBarrier(cmd, 1, &barrier); if(extQCmd != VK_NULL_HANDLE) @@ -1834,7 +1859,11 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten { dstimBarrier.subresourceRange = m_ImageLayouts[id].subresourceStates[si].subresourceRange; dstimBarrier.oldLayout = m_ImageLayouts[id].subresourceStates[si].newLayout; + + SanitiseOldImageLayout(dstimBarrier.oldLayout); + dstimBarrier.srcAccessMask = VK_ACCESS_ALL_WRITE_BITS | MakeAccessMask(dstimBarrier.oldLayout); + DoPipelineBarrier(cmd, 1, &dstimBarrier); if(extQCmd != VK_NULL_HANDLE) @@ -1945,7 +1974,11 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten { dstimBarrier.subresourceRange = m_ImageLayouts[id].subresourceStates[si].subresourceRange; dstimBarrier.newLayout = m_ImageLayouts[id].subresourceStates[si].newLayout; + + SanitiseNewImageLayout(dstimBarrier.newLayout); + dstimBarrier.dstAccessMask |= MakeAccessMask(dstimBarrier.newLayout); + DoPipelineBarrier(cmd, 1, &dstimBarrier); if(extQCmd != VK_NULL_HANDLE) diff --git a/renderdoc/driver/vulkan/vk_manager.cpp b/renderdoc/driver/vulkan/vk_manager.cpp index cd8612fee..5eb883743 100644 --- a/renderdoc/driver/vulkan/vk_manager.cpp +++ b/renderdoc/driver/vulkan/vk_manager.cpp @@ -256,6 +256,8 @@ void VulkanResourceManager::SerialiseImageStates(SerialiserType &ser, std::vector > vec; + std::set updatedState; + for(uint32_t i = 0; i < NumImages; i++) { SERIALISE_ELEMENT_LOCAL(Image, (ResourceId)(srcit->first)).TypedAs("VkImage"_lit); @@ -267,6 +269,8 @@ void VulkanResourceManager::SerialiseImageStates(SerialiserType &ser, if(IsReplayingAndReading() && liveid != ResourceId()) { + updatedState.insert(liveid); + for(ImageRegionState &state : ImageState.subresourceStates) { VkImageMemoryBarrier t; @@ -284,15 +288,10 @@ void VulkanResourceManager::SerialiseImageStates(SerialiserType &ser, t.dstQueueFamilyIndex = t.srcQueueFamilyIndex = m_Core->GetQueueFamilyIndex(); state.dstQueueFamilyIndex = t.dstQueueFamilyIndex; t.image = Unwrap(GetCurrentHandle(liveid)); + t.oldLayout = VK_IMAGE_LAYOUT_UNDEFINED; t.newLayout = state.newLayout; - // sanitise the new layout - ReplacePresentableImageLayout(state.newLayout); - if(t.newLayout == VK_IMAGE_LAYOUT_UNDEFINED) - t.newLayout = VK_IMAGE_LAYOUT_GENERAL; - if(state.newLayout == VK_IMAGE_LAYOUT_UNDEFINED) - state.newLayout = VK_IMAGE_LAYOUT_GENERAL; t.subresourceRange = state.subresourceRange; auto stit = states.find(liveid); @@ -309,6 +308,45 @@ void VulkanResourceManager::SerialiseImageStates(SerialiserType &ser, srcit++; } + // on replay, any images from the capture which didn't get touched above were created mid-frame so + // we reset them to their initialLayout. + if(IsReplayingAndReading()) + { + for(auto it = states.begin(); it != states.end(); ++it) + { + ResourceId liveid = it->first; + + if(GetOriginalID(liveid) != liveid && updatedState.find(liveid) == updatedState.end()) + { + for(ImageRegionState &state : it->second.subresourceStates) + { + VkImageMemoryBarrier t = {}; + t.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER; + t.srcQueueFamilyIndex = it->second.queueFamilyIndex; + t.dstQueueFamilyIndex = it->second.queueFamilyIndex; + m_Core->RemapQueueFamilyIndices(t.srcQueueFamilyIndex, t.dstQueueFamilyIndex); + if(t.dstQueueFamilyIndex == VK_QUEUE_FAMILY_IGNORED) + t.dstQueueFamilyIndex = t.srcQueueFamilyIndex = m_Core->GetQueueFamilyIndex(); + state.dstQueueFamilyIndex = t.dstQueueFamilyIndex; + t.image = Unwrap(GetCurrentHandle(liveid)); + + t.oldLayout = VK_IMAGE_LAYOUT_UNDEFINED; + state.newLayout = t.newLayout = it->second.initialLayout; + + t.subresourceRange = state.subresourceRange; + + auto stit = states.find(liveid); + + if(stit == states.end() || stit->second.memoryBound) + { + barriers.push_back(t); + vec.push_back(make_rdcpair(liveid, state)); + } + } + } + } + } + // we don't have to specify a queue here because all of the images have a specific queue above ApplyBarriers(VK_QUEUE_FAMILY_IGNORED, vec, states); diff --git a/renderdoc/driver/vulkan/vk_rendertexture.cpp b/renderdoc/driver/vulkan/vk_rendertexture.cpp index 50e86db30..09c8d5169 100644 --- a/renderdoc/driver/vulkan/vk_rendertexture.cpp +++ b/renderdoc/driver/vulkan/vk_rendertexture.cpp @@ -513,6 +513,9 @@ bool VulkanReplay::RenderTextureInternal(TextureDisplay cfg, VkRenderPassBeginIn srcimBarrier.subresourceRange = layouts.subresourceStates[si].subresourceRange; srcimBarrier.oldLayout = layouts.subresourceStates[si].newLayout; srcimBarrier.srcAccessMask = VK_ACCESS_ALL_WRITE_BITS | MakeAccessMask(srcimBarrier.oldLayout); + + SanitiseOldImageLayout(srcimBarrier.oldLayout); + DoPipelineBarrier(cmd, 1, &srcimBarrier); if(extQCmd != VK_NULL_HANDLE) diff --git a/renderdoc/driver/vulkan/vk_replay.cpp b/renderdoc/driver/vulkan/vk_replay.cpp index 91e5b8a20..462dd218d 100644 --- a/renderdoc/driver/vulkan/vk_replay.cpp +++ b/renderdoc/driver/vulkan/vk_replay.cpp @@ -1671,6 +1671,9 @@ void VulkanReplay::SavePipelineState(uint32_t eventId) { VKPipe::ImageData &img = m_VulkanPipelineState.images[i]; + if(rm->GetOriginalID(it->first) == it->first) + continue; + img.resourceId = rm->GetOriginalID(it->first); img.layouts.resize(it->second.subresourceStates.size()); @@ -1691,6 +1694,8 @@ void VulkanReplay::SavePipelineState(uint32_t eventId) i++; } + + m_VulkanPipelineState.images.resize(i); } if(state.conditionalRendering.buffer != ResourceId()) diff --git a/renderdoc/driver/vulkan/vk_resources.h b/renderdoc/driver/vulkan/vk_resources.h index 265dd084e..ae56a8527 100644 --- a/renderdoc/driver/vulkan/vk_resources.h +++ b/renderdoc/driver/vulkan/vk_resources.h @@ -1331,6 +1331,7 @@ struct ImageLayouts VkFormat format; VkImageType imageType; bool memoryBound = false; + VkImageLayout initialLayout = VK_IMAGE_LAYOUT_UNDEFINED; }; DECLARE_REFLECTION_STRUCT(ImageLayouts); diff --git a/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp index b7f79fd84..de7087bae 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp @@ -207,9 +207,6 @@ std::vector WrappedVulkan::GetImplicitRenderPassBarriers(u } } - ReplacePresentableImageLayout(barrier.oldLayout); - ReplacePresentableImageLayout(barrier.newLayout); - ret.push_back(barrier); } @@ -2559,8 +2556,6 @@ bool WrappedVulkan::Serialise_vkCmdPipelineBarrier( { imgBarriers.push_back(pImageMemoryBarriers[i]); imgBarriers.back().image = Unwrap(imgBarriers.back().image); - ReplacePresentableImageLayout(imgBarriers.back().oldLayout); - ReplacePresentableImageLayout(imgBarriers.back().newLayout); RemapQueueFamilyIndices(imgBarriers.back().srcQueueFamilyIndex, imgBarriers.back().dstQueueFamilyIndex); @@ -2584,14 +2579,21 @@ bool WrappedVulkan::Serialise_vkCmdPipelineBarrier( if(commandBuffer != VK_NULL_HANDLE) { + ResourceId cmd = GetResID(commandBuffer); + GetResourceManager()->RecordBarriers(m_BakedCmdBufferInfo[cmd].imgbarriers, m_ImageLayouts, + (uint32_t)imgBarriers.size(), imgBarriers.data()); + + // now sanitise layouts before passing to vulkan + for(VkImageMemoryBarrier &barrier : imgBarriers) + { + SanitiseOldImageLayout(barrier.oldLayout); + SanitiseNewImageLayout(barrier.newLayout); + } + ObjDisp(commandBuffer) ->CmdPipelineBarrier(Unwrap(commandBuffer), srcStageMask, destStageMask, dependencyFlags, memoryBarrierCount, pMemoryBarriers, (uint32_t)bufBarriers.size(), bufBarriers.data(), (uint32_t)imgBarriers.size(), imgBarriers.data()); - - ResourceId cmd = GetResID(commandBuffer); - GetResourceManager()->RecordBarriers(m_BakedCmdBufferInfo[cmd].imgbarriers, m_ImageLayouts, - (uint32_t)imgBarriers.size(), imgBarriers.data()); } } diff --git a/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp index 64f70000c..95877babb 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp @@ -757,9 +757,9 @@ bool WrappedVulkan::Serialise_vkCreateRenderPass(SerialiserType &ser, VkDevice d if(att[i].stencilLoadOp == VK_ATTACHMENT_LOAD_OP_DONT_CARE) att[i].stencilLoadOp = VK_ATTACHMENT_LOAD_OP_LOAD; - // renderpass can't start or end in presentable layout on replay - ReplacePresentableImageLayout(att[i].initialLayout); - ReplacePresentableImageLayout(att[i].finalLayout); + // sanitise the actual layouts used to create the renderpass + SanitiseOldImageLayout(att[i].initialLayout); + SanitiseNewImageLayout(att[i].finalLayout); } VkResult ret = ObjDisp(device)->CreateRenderPass(Unwrap(device), &CreateInfo, NULL, &rp); @@ -988,8 +988,8 @@ bool WrappedVulkan::Serialise_vkCreateRenderPass2KHR(SerialiserType &ser, VkDevi att[i].stencilLoadOp = VK_ATTACHMENT_LOAD_OP_LOAD; // renderpass can't start or end in presentable layout on replay - ReplacePresentableImageLayout(att[i].initialLayout); - ReplacePresentableImageLayout(att[i].finalLayout); + SanitiseOldImageLayout(att[i].initialLayout); + SanitiseNewImageLayout(att[i].finalLayout); } VkResult ret = ObjDisp(device)->CreateRenderPass2KHR(Unwrap(device), &CreateInfo, NULL, &rp); diff --git a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp index 8b22d33b5..3f97fe891 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp @@ -244,13 +244,14 @@ bool WrappedVulkan::Serialise_vkQueueSubmit(SerialiserType &ser, VkQueue queue, for(uint32_t c = 0; c < submitInfo.commandBufferCount; c++) { - ResourceId cmd = - GetResourceManager()->GetOriginalID(GetResID(submitInfo.pCommandBuffers[c])); + ResourceId liveCmd = GetResID(submitInfo.pCommandBuffers[c]); + ResourceId cmd = GetResourceManager()->GetOriginalID(liveCmd); BakedCmdBufferInfo &cmdBufInfo = m_BakedCmdBufferInfo[cmd]; GetResourceManager()->ApplyBarriers(m_CreationInfo.m_Queue[GetResID(queue)], - m_BakedCmdBufferInfo[cmd].imgbarriers, m_ImageLayouts); + m_BakedCmdBufferInfo[liveCmd].imgbarriers, + m_ImageLayouts); std::string name = StringFormat::Fmt("=> %s[%u]: vkBeginCommandBuffer(%s)", basename.c_str(), c, ToStr(cmd).c_str()); diff --git a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp index 73c0563be..92084d6c0 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp @@ -1475,6 +1475,7 @@ bool WrappedVulkan::Serialise_vkCreateImage(SerialiserType &ser, VkDevice device layouts.extent = CreateInfo.extent; layouts.format = CreateInfo.format; layouts.imageType = CreateInfo.imageType; + layouts.initialLayout = CreateInfo.initialLayout; range.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; if(IsDepthOnlyFormat(CreateInfo.format)) @@ -1485,7 +1486,7 @@ bool WrappedVulkan::Serialise_vkCreateImage(SerialiserType &ser, VkDevice device range.aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT; layouts.subresourceStates.push_back(ImageRegionState( - VK_QUEUE_FAMILY_IGNORED, range, UNKNOWN_PREV_IMG_LAYOUT, VK_IMAGE_LAYOUT_UNDEFINED)); + VK_QUEUE_FAMILY_IGNORED, range, UNKNOWN_PREV_IMG_LAYOUT, CreateInfo.initialLayout)); } const char *prefix = "Image"; @@ -1771,6 +1772,7 @@ VkResult WrappedVulkan::vkCreateImage(VkDevice device, const VkImageCreateInfo * layout->extent = pCreateInfo->extent; layout->format = pCreateInfo->format; layout->imageType = pCreateInfo->imageType; + layout->initialLayout = pCreateInfo->initialLayout; layout->subresourceStates.clear(); @@ -1783,7 +1785,7 @@ VkResult WrappedVulkan::vkCreateImage(VkDevice device, const VkImageCreateInfo * range.aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT; layout->subresourceStates.push_back(ImageRegionState( - VK_QUEUE_FAMILY_IGNORED, range, UNKNOWN_PREV_IMG_LAYOUT, VK_IMAGE_LAYOUT_UNDEFINED)); + VK_QUEUE_FAMILY_IGNORED, range, UNKNOWN_PREV_IMG_LAYOUT, pCreateInfo->initialLayout)); } return ret; diff --git a/renderdoc/driver/vulkan/wrappers/vk_sync_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_sync_funcs.cpp index 627fcba0a..2d180b1ab 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_sync_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_sync_funcs.cpp @@ -779,8 +779,6 @@ bool WrappedVulkan::Serialise_vkCmdWaitEvents( { imgBarriers.push_back(pImageMemoryBarriers[i]); imgBarriers.back().image = Unwrap(imgBarriers.back().image); - ReplacePresentableImageLayout(imgBarriers.back().oldLayout); - ReplacePresentableImageLayout(imgBarriers.back().newLayout); RemapQueueFamilyIndices(imgBarriers.back().srcQueueFamilyIndex, imgBarriers.back().dstQueueFamilyIndex); @@ -816,18 +814,25 @@ bool WrappedVulkan::Serialise_vkCmdWaitEvents( m_PersistentEvents.push_back(ev); } + ResourceId cmd = GetResID(commandBuffer); + GetResourceManager()->RecordBarriers(m_BakedCmdBufferInfo[cmd].imgbarriers, m_ImageLayouts, + (uint32_t)imgBarriers.size(), &imgBarriers[0]); + if(commandBuffer != VK_NULL_HANDLE) { + // now sanitise layouts before passing to vulkan + for(VkImageMemoryBarrier &barrier : imgBarriers) + { + SanitiseOldImageLayout(barrier.oldLayout); + SanitiseNewImageLayout(barrier.newLayout); + } + ObjDisp(commandBuffer)->CmdSetEvent(Unwrap(commandBuffer), ev, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT); ObjDisp(commandBuffer) ->CmdWaitEvents(Unwrap(commandBuffer), 1, &ev, srcStageMask, dstStageMask, memoryBarrierCount, pMemoryBarriers, (uint32_t)bufBarriers.size(), bufBarriers.data(), (uint32_t)imgBarriers.size(), imgBarriers.data()); } - - ResourceId cmd = GetResID(commandBuffer); - GetResourceManager()->RecordBarriers(m_BakedCmdBufferInfo[cmd].imgbarriers, m_ImageLayouts, - (uint32_t)imgBarriers.size(), &imgBarriers[0]); } return true; diff --git a/renderdoc/driver/vulkan/wrappers/vk_wsi_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_wsi_funcs.cpp index 61774476a..fbeae5052 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_wsi_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_wsi_funcs.cpp @@ -450,6 +450,7 @@ bool WrappedVulkan::Serialise_vkCreateSwapchainKHR(SerialiserType &ser, VkDevice m_ImageLayouts[liveId].format = iminfo.format; m_ImageLayouts[liveId].imageType = iminfo.type; m_ImageLayouts[liveId].memoryBound = true; + m_ImageLayouts[liveId].initialLayout = VK_IMAGE_LAYOUT_UNDEFINED; m_ImageLayouts[liveId].subresourceStates.clear(); m_ImageLayouts[liveId].subresourceStates.push_back(ImageRegionState( @@ -588,6 +589,7 @@ void WrappedVulkan::WrapAndProcessCreatedSwapchain(VkDevice device, m_ImageLayouts[imid].format = pCreateInfo->imageFormat; m_ImageLayouts[imid].imageType = VK_IMAGE_TYPE_2D; m_ImageLayouts[imid].memoryBound = true; + m_ImageLayouts[imid].initialLayout = VK_IMAGE_LAYOUT_UNDEFINED; m_ImageLayouts[imid].subresourceStates.clear(); m_ImageLayouts[imid].subresourceStates.push_back(ImageRegionState( diff --git a/util/test/demos/CMakeLists.txt b/util/test/demos/CMakeLists.txt index 7e4891a10..07f38f728 100644 --- a/util/test/demos/CMakeLists.txt +++ b/util/test/demos/CMakeLists.txt @@ -12,6 +12,7 @@ set(VULKAN_SRC vk/vk_descriptor_index.cpp vk/vk_discard_rects.cpp vk/vk_draw_zoo.cpp + vk/vk_image_layouts.cpp vk/vk_indirect.cpp vk/vk_multi_thread_windows.cpp vk/vk_overlay_test.cpp diff --git a/util/test/demos/demos.vcxproj b/util/test/demos/demos.vcxproj index 7edb2c6b1..0d927c0cd 100644 --- a/util/test/demos/demos.vcxproj +++ b/util/test/demos/demos.vcxproj @@ -208,6 +208,7 @@ + diff --git a/util/test/demos/demos.vcxproj.filters b/util/test/demos/demos.vcxproj.filters index de22f0559..506bd0e35 100644 --- a/util/test/demos/demos.vcxproj.filters +++ b/util/test/demos/demos.vcxproj.filters @@ -306,6 +306,9 @@ D3D11\demos + + Vulkan\demos + diff --git a/util/test/demos/vk/vk_image_layouts.cpp b/util/test/demos/vk/vk_image_layouts.cpp new file mode 100644 index 000000000..8d0d4559a --- /dev/null +++ b/util/test/demos/vk/vk_image_layouts.cpp @@ -0,0 +1,239 @@ +/****************************************************************************** + * The MIT License (MIT) + * + * Copyright (c) 2018-2019 Baldur Karlsson + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + ******************************************************************************/ + +#include "vk_test.h" + +TEST(VK_Image_Layouts, VulkanGraphicsTest) +{ + static constexpr const char *Description = + "Tests edge-cases of image layout transitions, such as images being in UNDEFINED, " + "PREINITIALIZED or PRESENT_SRC at the start of the frame."; + + std::string common = R"EOSHADER( + +#version 420 core + +struct v2f +{ + vec4 pos; + vec4 col; + vec4 uv; +}; + +)EOSHADER"; + + const std::string vertex = R"EOSHADER( + +layout(location = 0) in vec3 Position; +layout(location = 1) in vec4 Color; +layout(location = 2) in vec2 UV; + +layout(location = 0) out v2f vertOut; + +void main() +{ + vertOut.pos = vec4(Position.xyz*vec3(1,-1,1), 1); + gl_Position = vertOut.pos; + vertOut.col = Color; + vertOut.uv = vec4(UV.xy, 0, 1); +} + +)EOSHADER"; + + const std::string pixel = R"EOSHADER( + +layout(location = 0) in v2f vertIn; + +layout(location = 0, index = 0) out vec4 Color; + +void main() +{ + Color = vertIn.col; +} + +)EOSHADER"; + + int main() + { + // initialise, create window, create context, etc + if(!Init()) + return 3; + + VkPipelineLayout layout = createPipelineLayout(vkh::PipelineLayoutCreateInfo()); + + vkh::GraphicsPipelineCreateInfo pipeCreateInfo; + + pipeCreateInfo.layout = layout; + pipeCreateInfo.renderPass = mainWindow->rp; + + pipeCreateInfo.vertexInputState.vertexBindingDescriptions = {vkh::vertexBind(0, DefaultA2V)}; + pipeCreateInfo.vertexInputState.vertexAttributeDescriptions = { + vkh::vertexAttr(0, 0, DefaultA2V, pos), vkh::vertexAttr(1, 0, DefaultA2V, col), + vkh::vertexAttr(2, 0, DefaultA2V, uv), + }; + + pipeCreateInfo.stages = { + CompileShaderModule(common + vertex, ShaderLang::glsl, ShaderStage::vert, "main"), + CompileShaderModule(common + pixel, ShaderLang::glsl, ShaderStage::frag, "main"), + }; + + VkPipeline pipe = createGraphicsPipeline(pipeCreateInfo); + + AllocatedBuffer vb( + allocator, vkh::BufferCreateInfo(sizeof(DefaultTri), VK_BUFFER_USAGE_VERTEX_BUFFER_BIT | + VK_BUFFER_USAGE_TRANSFER_DST_BIT), + VmaAllocationCreateInfo({0, VMA_MEMORY_USAGE_CPU_TO_GPU})); + + vb.upload(DefaultTri); + + vkh::ImageCreateInfo preinitInfo(4, 4, 0, VK_FORMAT_R8G8B8A8_UNORM, + VK_IMAGE_USAGE_TRANSFER_SRC_BIT); + preinitInfo.tiling = VK_IMAGE_TILING_LINEAR; + preinitInfo.initialLayout = VK_IMAGE_LAYOUT_PREINITIALIZED; + + const VkPhysicalDeviceMemoryProperties *props = NULL; + vmaGetMemoryProperties(allocator, &props); + + while(Running()) + { + VkImage preinitImg = VK_NULL_HANDLE; + VkDeviceMemory preinitMem = VK_NULL_HANDLE; + + vkCreateImage(device, preinitInfo, NULL, &preinitImg); + + setName(preinitImg, "Image:Preinitialised"); + + AllocatedImage undefImg(allocator, vkh::ImageCreateInfo(4, 4, 0, VK_FORMAT_R8G8B8A8_UNORM, + VK_IMAGE_USAGE_TRANSFER_DST_BIT | + VK_IMAGE_USAGE_SAMPLED_BIT), + VmaAllocationCreateInfo({0, VMA_MEMORY_USAGE_GPU_ONLY})); + + setName(undefImg.image, "Image:Undefined"); + + { + VkMemoryRequirements mrq; + vkGetImageMemoryRequirements(device, preinitImg, &mrq); + + VkMemoryAllocateInfo info = {}; + info.sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO; + info.allocationSize = mrq.size; + info.memoryTypeIndex = 100; + + for(uint32_t i = 0; i < props->memoryTypeCount; i++) + { + if(mrq.memoryTypeBits & (1 << i) && + (props->memoryTypes[i].propertyFlags & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT)) + { + info.memoryTypeIndex = i; + break; + } + } + + TEST_ASSERT(info.memoryTypeIndex != 100, "Couldn't find compatible memory type"); + + vkAllocateMemory(device, &info, NULL, &preinitMem); + vkBindImageMemory(device, preinitImg, preinitMem, 0); + + void *data = NULL; + vkMapMemory(device, preinitMem, 0, mrq.size, 0, &data); + memset(data, 0x40, mrq.size); + vkUnmapMemory(device, preinitMem); + } + + VkCommandBuffer cmd = GetCommandBuffer(); + + vkBeginCommandBuffer(cmd, vkh::CommandBufferBeginInfo()); + + VkImage swapimg = mainWindow->GetImage(); + if(curFrame <= mainWindow->GetCount()) + setName(swapimg, "Image:Swapchain"); + + setMarker(cmd, "Before Transition"); + + // after the first N frames, we expect the swapchain to be in PRESENT_SRC + vkh::cmdPipelineBarrier(cmd, { + vkh::ImageMemoryBarrier(0, VK_ACCESS_TRANSFER_WRITE_BIT, + curFrame <= mainWindow->GetCount() + ? VK_IMAGE_LAYOUT_UNDEFINED + : VK_IMAGE_LAYOUT_PRESENT_SRC_KHR, + VK_IMAGE_LAYOUT_GENERAL, swapimg), + }); + + vkCmdClearColorImage(cmd, swapimg, VK_IMAGE_LAYOUT_GENERAL, + vkh::ClearColorValue(0.4f, 0.5f, 0.6f, 1.0f), 1, + vkh::ImageSubresourceRange()); + + // the manual images are transitioned into general for copying, from pre-initialised and + // undefined + vkh::cmdPipelineBarrier( + cmd, { + vkh::ImageMemoryBarrier(VK_ACCESS_HOST_WRITE_BIT, VK_ACCESS_TRANSFER_READ_BIT, + VK_IMAGE_LAYOUT_PREINITIALIZED, + VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, preinitImg), + vkh::ImageMemoryBarrier(0, VK_ACCESS_TRANSFER_WRITE_BIT, VK_IMAGE_LAYOUT_UNDEFINED, + VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, undefImg.image), + }); + + VkImageCopy region = { + {VK_IMAGE_ASPECT_COLOR_BIT, 0, 0, 1}, + {0, 0, 0}, + {VK_IMAGE_ASPECT_COLOR_BIT, 0, 0, 1}, + {0, 0, 0}, + {4, 4, 1}, + }; + + vkCmdCopyImage(cmd, preinitImg, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, undefImg.image, + VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, ®ion); + + vkCmdBeginRenderPass( + cmd, vkh::RenderPassBeginInfo(mainWindow->rp, mainWindow->GetFB(), mainWindow->scissor), + VK_SUBPASS_CONTENTS_INLINE); + + vkCmdBindPipeline(cmd, VK_PIPELINE_BIND_POINT_GRAPHICS, pipe); + vkCmdSetViewport(cmd, 0, 1, &mainWindow->viewport); + vkCmdSetScissor(cmd, 0, 1, &mainWindow->scissor); + vkh::cmdBindVertexBuffers(cmd, 0, {vb.buffer}, {0}); + vkCmdDraw(cmd, 3, 1, 0, 0); + + vkCmdEndRenderPass(cmd); + + FinishUsingBackbuffer(cmd, VK_ACCESS_TRANSFER_WRITE_BIT, VK_IMAGE_LAYOUT_GENERAL); + + vkEndCommandBuffer(cmd); + + Submit(0, 1, {cmd}); + + Present(); + + vkDeviceWaitIdle(device); + + vkDestroyImage(device, preinitImg, NULL); + vkFreeMemory(device, preinitMem, NULL); + } + + return 0; + } +}; + +REGISTER_TEST(); diff --git a/util/test/tests/Vulkan/VK_Image_Layouts.py b/util/test/tests/Vulkan/VK_Image_Layouts.py new file mode 100644 index 000000000..45446e04b --- /dev/null +++ b/util/test/tests/Vulkan/VK_Image_Layouts.py @@ -0,0 +1,69 @@ +import rdtest +import renderdoc as rd + + +class VK_Image_Layouts(rdtest.TestCase): + demos_test_name = 'VK_Image_Layouts' + + def check_capture(self): + self.controller.SetFrameEvent(0, False) + + pipe: rd.VKState = self.controller.GetVulkanPipelineState() + + # Check that the layout is reported correctly at the start of the frame + for img in pipe.images: + img: rd.VKImageData + res = self.get_resource(img.resourceId) + if res.name == "Image:Preinitialised": + if img.layouts[0].name != "VK_IMAGE_LAYOUT_PREINITIALIZED": + raise rdtest.TestFailureException("Pre-initialised image is in {} layout".format(img.layouts[0].name)) + elif res.name == "Image:Undefined": + if img.layouts[0].name != "VK_IMAGE_LAYOUT_UNDEFINED": + raise rdtest.TestFailureException("Undefined image is in {} layout".format(img.layouts[0].name)) + elif res.name == "Image:Swapchain": + if img.layouts[0].name != "VK_IMAGE_LAYOUT_PRESENT_SRC_KHR": + raise rdtest.TestFailureException("Swapchain image is in {} layout".format(img.layouts[0].name)) + + draw = self.find_draw("Before Transition") + + self.check(draw is not None) + + self.controller.SetFrameEvent(draw.eventId, False) + + pipe: rd.VKState = self.controller.GetVulkanPipelineState() + + # Check that the layout is reported correctly before transitions still + for img in pipe.images: + img: rd.VKImageData + res = self.get_resource(img.resourceId) + if res.name == "Image:Preinitialised": + if img.layouts[0].name != "VK_IMAGE_LAYOUT_PREINITIALIZED": + raise rdtest.TestFailureException("Pre-initialised image is in {} layout".format(img.layouts[0].name)) + elif res.name == "Image:Undefined": + if img.layouts[0].name != "VK_IMAGE_LAYOUT_UNDEFINED": + raise rdtest.TestFailureException("Undefined image is in {} layout".format(img.layouts[0].name)) + elif res.name == "Image:Swapchain": + if img.layouts[0].name != "VK_IMAGE_LAYOUT_PRESENT_SRC_KHR": + raise rdtest.TestFailureException("Swapchain image is in {} layout".format(img.layouts[0].name)) + + draw = self.find_draw("vkCmdDraw") + + self.check(draw is not None) + + self.controller.SetFrameEvent(draw.eventId, False) + + pipe: rd.VKState = self.controller.GetVulkanPipelineState() + + # Check that after transitions, the images are in the right state + for img in pipe.images: + img: rd.VKImageData + res = self.get_resource(img.resourceId) + if res.name == "Image:Preinitialised": + if img.layouts[0].name != "VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL": + raise rdtest.TestFailureException("Pre-initialised image is in {} layout".format(img.layouts[0].name)) + elif res.name == "Image:Undefined": + if img.layouts[0].name != "VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL": + raise rdtest.TestFailureException("Undefined image is in {} layout".format(img.layouts[0].name)) + elif img.resourceId == pipe.currentPass.framebuffer.attachments[0].imageResourceId: + if img.layouts[0].name != "VK_IMAGE_LAYOUT_GENERAL": + raise rdtest.TestFailureException("Rendered swapchain image is in {} layout".format(img.layouts[0].name))