From 3b48de6b2dca8967e9c525b93abb6a4fe89ad069 Mon Sep 17 00:00:00 2001 From: Benson Joeris Date: Fri, 8 Nov 2019 11:19:37 -0500 Subject: [PATCH] Vulkan: Don't split depth/stencil aspects when initializing images For images with both depth and stencil aspects, `VkImageMemoryBarrier`s must include both depth and stencil aspects--e.g. you cannot transition the layout of the depth alone. This can be violated by the barriers in RenderDoc replay that transition image layouts before and after initialization. This change ensures that, if depth or stencil needs to be initialized, then beth will be initialized, and the image layout transition will be applied to the two aspects together. Specifically, `ImgRefs` stores a separate FrameRefType for each aspect. This can cause the depth and stencil aspects to have different reset requirements, which can cause a barrier to contain only one of the depth/stencil aspects. Change-Id: I8a16a0c450bd8e6cb0d60c1d688b01b75df86915 --- renderdoc/driver/vulkan/vk_initstate.cpp | 127 ++++++++++++++++------- renderdoc/driver/vulkan/vk_resources.cpp | 63 +++++++++-- renderdoc/driver/vulkan/vk_resources.h | 2 + 3 files changed, 146 insertions(+), 46 deletions(-) diff --git a/renderdoc/driver/vulkan/vk_initstate.cpp b/renderdoc/driver/vulkan/vk_initstate.cpp index 5526f958e..5257739f9 100644 --- a/renderdoc/driver/vulkan/vk_initstate.cpp +++ b/renderdoc/driver/vulkan/vk_initstate.cpp @@ -1362,6 +1362,19 @@ void WrappedVulkan::ImageInitializationBarriers(ResourceId id, WrappedVkRes *liv ToHandle(live), imageLayouts.subresourceStates[si].subresourceRange, }; + if(IsDepthAndStencilFormat(imageLayouts.imageInfo.format)) + { + if(barrier.subresourceRange.aspectMask == VK_IMAGE_ASPECT_STENCIL_BIT) + { + // There will be a different subresourceState with DEPTH aspect, which is when we will + // handle both the depth and stencil aspects. + continue; + } + else + { + barrier.subresourceRange.aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT; + } + } VkImageMemoryBarrier revBarrier = barrier; revBarrier.oldLayout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL; revBarrier.newLayout = imageLayouts.subresourceStates[si].newLayout; @@ -1386,9 +1399,26 @@ void WrappedVulkan::ImageInitializationBarriers(ResourceId id, WrappedVkRes *liv imgRefs->SubresourceRangeInitReqs(barrier.subresourceRange, policy, initialized); for(auto initIt = initReqs.begin(); initIt != initReqs.end(); ++initIt) { - if(initIt->second != eInitReq_None) + barrier.subresourceRange = revBarrier.subresourceRange = initIt->first; + InitReqType initReq = initIt->second; + if(IsDepthAndStencilFormat(imageLayouts.imageInfo.format)) + { + if(barrier.subresourceRange.aspectMask == VK_IMAGE_ASPECT_STENCIL_BIT) + { + // There will be a different subresourceState with DEPTH aspect, which is when we will + // handle both the depth and stencil aspects. + continue; + } + else if(barrier.subresourceRange.aspectMask == VK_IMAGE_ASPECT_DEPTH_BIT) + { + barrier.subresourceRange.aspectMask = revBarrier.subresourceRange.aspectMask = + VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT; + initReq = + imgRefs->SubresourceRangeMaxInitReq(barrier.subresourceRange, policy, initialized); + } + } + if(initReq != eInitReq_None) { - barrier.subresourceRange = revBarrier.subresourceRange = initIt->first; setupBarriers.push_back(barrier); cleanupBarriers.push_back(revBarrier); } @@ -2005,22 +2035,6 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten std::vector copyRegions; std::vector clearRegions; -#define INIT_REGION() \ - if(!initialized) \ - { \ - copyRegions.push_back(region); \ - } \ - else \ - { \ - InitReqType initReq = imgRefs->SubresourceInitReq( \ - imgRefs->AspectIndex((VkImageAspectFlagBits)region.imageSubresource.aspectMask), m, a, \ - policy, initialized); \ - if(initReq == eInitReq_Copy) \ - copyRegions.push_back(region); \ - else if(initReq == eInitReq_Clear) \ - clearRegions.push_back(ImageRange(region.imageSubresource)); \ - } - // copy each slice/mip individually for(int a = 0; a < m_CreationInfo.m_Image[id].arrayLayers; a++) { @@ -2038,6 +2052,8 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten }, extent, }; + VkImageSubresourceRange range = ImageRange(region.imageSubresource); + InitReqType initReq; if(planeCount > 1) { @@ -2058,34 +2074,71 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten bufOffset += GetPlaneByteSize(extent.width, extent.height, extent.depth, fmt, 0, i); - INIT_REGION(); + if(!initialized) + initReq = eInitReq_Copy; + else + initReq = imgRefs->SubresourceRangeMaxInitReq(range, policy, initialized); + if(initReq == eInitReq_Copy) + copyRegions.push_back(region); + else if(initReq == eInitReq_Clear) + clearRegions.push_back(range); } } + else if(IsDepthAndStencilFormat(fmt)) + { + bufOffset = AlignUp(bufOffset, bufAlignment); + + range.aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT; + + if(!initialized) + initReq = eInitReq_Copy; + else + initReq = imgRefs->SubresourceRangeMaxInitReq(range, policy, initialized); + if(initReq == eInitReq_None) + continue; + + region.bufferOffset = bufOffset; + region.imageSubresource.aspectMask = range.aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT; + + VkFormat sizeFormat = GetDepthOnlyFormat(fmt); + + // pass 0 for mip since we've already pre-downscaled extent + bufOffset += GetByteSize(extent.width, extent.height, extent.depth, sizeFormat, 0); + if(initReq == eInitReq_Copy) + copyRegions.push_back(region); + else if(initReq == eInitReq_Clear) + clearRegions.push_back(range); + + // we removed stencil from the format, copy that separately now. + bufOffset = AlignUp(bufOffset, bufAlignment); + + region.bufferOffset = bufOffset; + region.imageSubresource.aspectMask = range.aspectMask = VK_IMAGE_ASPECT_STENCIL_BIT; + + bufOffset += GetByteSize(extent.width, extent.height, extent.depth, VK_FORMAT_S8_UINT, 0); + + if(initReq == eInitReq_Copy) + copyRegions.push_back(region); + else if(initReq == eInitReq_Clear) + clearRegions.push_back(range); + } else { bufOffset = AlignUp(bufOffset, bufAlignment); region.bufferOffset = bufOffset; - VkFormat sizeFormat = GetDepthOnlyFormat(fmt); - // pass 0 for mip since we've already pre-downscaled extent - bufOffset += GetByteSize(extent.width, extent.height, extent.depth, sizeFormat, 0); + bufOffset += GetByteSize(extent.width, extent.height, extent.depth, fmt, 0); - INIT_REGION(); - - if(sizeFormat != fmt) - { - // if we removed stencil from the format, copy that separately now. - bufOffset = AlignUp(bufOffset, bufAlignment); - - region.bufferOffset = bufOffset; - region.imageSubresource.aspectMask = VK_IMAGE_ASPECT_STENCIL_BIT; - - bufOffset += GetByteSize(extent.width, extent.height, extent.depth, VK_FORMAT_S8_UINT, 0); - - INIT_REGION(); - } + if(!initialized) + initReq = eInitReq_Copy; + else + initReq = imgRefs->SubresourceRangeMaxInitReq(range, policy, initialized); + if(initReq == eInitReq_Copy) + copyRegions.push_back(region); + else if(initReq == eInitReq_Clear) + clearRegions.push_back(range); } // update the extent for the next mip @@ -2095,8 +2148,6 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten } } -#undef INIT_REGION - if(copyRegions.size() > 0) ObjDisp(cmd)->CmdCopyBufferToImage(Unwrap(cmd), Unwrap(buf), ToHandle(live), VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, diff --git a/renderdoc/driver/vulkan/vk_resources.cpp b/renderdoc/driver/vulkan/vk_resources.cpp index f7412ed13..57958b848 100644 --- a/renderdoc/driver/vulkan/vk_resources.cpp +++ b/renderdoc/driver/vulkan/vk_resources.cpp @@ -3023,23 +3023,70 @@ int ImgRefs::SubresourceIndex(int aspectIndex, int level, int layer) const return (aspectIndex * splitLevelCount + level) * splitLayerCount + layer; } +InitReqType ImgRefs::SubresourceRangeMaxInitReq(VkImageSubresourceRange range, InitPolicy policy, + bool initialized) const +{ + InitReqType initReq = eInitReq_None; + std::vector splitAspectIndices; + if(areAspectsSplit) + { + int aspectIndex = 0; + for(auto aspectIt = ImageAspectFlagIter::begin(aspectMask); + aspectIt != ImageAspectFlagIter::end(); ++aspectIt, ++aspectIndex) + { + if(((*aspectIt) & range.aspectMask) != 0) + splitAspectIndices.push_back(aspectIndex); + } + } + else + { + splitAspectIndices.push_back(0); + } + + int splitLevelCount = 1; + if(areLevelsSplit || range.baseMipLevel != 0 || range.levelCount < (uint32_t)imageInfo.levelCount) + { + splitLevelCount = range.levelCount; + } + int splitLayerCount = 1; + if(areLayersSplit || range.baseArrayLayer != 0 || range.layerCount < (uint32_t)imageInfo.layerCount) + { + splitLayerCount = range.layerCount; + } + for(auto aspectIndexIt = splitAspectIndices.begin(); aspectIndexIt != splitAspectIndices.end(); + ++aspectIndexIt) + { + for(int level = range.baseMipLevel; level < splitLevelCount; ++level) + { + for(int layer = range.baseArrayLayer; layer < splitLayerCount; ++layer) + { + initReq = + RDCMAX(initReq, SubresourceInitReq(*aspectIndexIt, level, layer, policy, initialized)); + } + } + } + return initReq; +} + std::vector > ImgRefs::SubresourceRangeInitReqs( VkImageSubresourceRange range, InitPolicy policy, bool initialized) const { VkImageSubresourceRange out(range); std::vector > res; - std::vector splitAspects; + std::vector > splitAspects; if(areAspectsSplit) { - for(auto aspectIt = ImageAspectFlagIter::begin(aspectMask & range.aspectMask); - aspectIt != ImageAspectFlagIter::end(); ++aspectIt) + int aspectIndex = 0; + for(auto aspectIt = ImageAspectFlagIter::begin(aspectMask); + aspectIt != ImageAspectFlagIter::end(); ++aspectIt, ++aspectIndex) { - splitAspects.push_back(*aspectIt); + if(((*aspectIt) & range.aspectMask) != 0) + splitAspects.push_back({aspectIndex, (VkImageAspectFlags)*aspectIt}); } } else { - splitAspects.push_back(range.aspectMask); + splitAspects.push_back({0, aspectMask}); } int splitLevelCount = 1; @@ -3054,10 +3101,10 @@ std::vector > ImgRefs::Subresource splitLayerCount = range.layerCount; out.layerCount = 1; } - int aspectIndex = 0; - for(auto aspectIt = splitAspects.begin(); aspectIt != splitAspects.end(); ++aspectIt, ++aspectIndex) + for(auto aspectIt = splitAspects.begin(); aspectIt != splitAspects.end(); ++aspectIt) { - out.aspectMask = *aspectIt; + int aspectIndex = aspectIt->first; + out.aspectMask = aspectIt->second; for(int level = range.baseMipLevel; level < splitLevelCount; ++level) { out.baseMipLevel = level; diff --git a/renderdoc/driver/vulkan/vk_resources.h b/renderdoc/driver/vulkan/vk_resources.h index 9cef261a3..d7aaef7b4 100644 --- a/renderdoc/driver/vulkan/vk_resources.h +++ b/renderdoc/driver/vulkan/vk_resources.h @@ -1146,6 +1146,8 @@ struct ImgRefs { return InitReq(SubresourceRef(aspectIndex, level, layer), policy, initialized); } + InitReqType SubresourceRangeMaxInitReq(VkImageSubresourceRange range, InitPolicy policy, + bool initialized) const; std::vector > SubresourceRangeInitReqs( VkImageSubresourceRange range, InitPolicy policy, bool initialized) const; void Split(bool splitAspects, bool splitLevels, bool splitLayers);