From 0ee540df9ca1773bd78fd39bbe626cde1fc6311b Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Tue, 22 Feb 2022 14:40:01 -0800 Subject: [PATCH] Vulkan: fix hack, allow SSAO to read from all mips. When we overhauled image layout transitions, we moved as many transitions as possible into the render pass. However the transition that we perform for "depth read only" layout needs to be applied to multiple miplevels, so we cannot use the render pass for that. --- filament/backend/src/vulkan/VulkanContext.h | 3 +- filament/backend/src/vulkan/VulkanDriver.cpp | 51 ++++++++++--------- filament/backend/src/vulkan/VulkanHandles.cpp | 2 +- filament/backend/src/vulkan/VulkanUtility.cpp | 1 + 4 files changed, 29 insertions(+), 28 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanContext.h b/filament/backend/src/vulkan/VulkanContext.h index 71649b884f..def3668a97 100644 --- a/filament/backend/src/vulkan/VulkanContext.h +++ b/filament/backend/src/vulkan/VulkanContext.h @@ -57,9 +57,8 @@ struct VulkanTimestamps { struct VulkanRenderPass { VkRenderPass renderPass; - uint32_t subpassMask; + RenderPassParams params; int currentSubpass; - VulkanTexture* depthFeedback; }; // For now we only support a single-device, single-instance scenario. Our concept of "context" is a diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 4f91d08d33..8fdb633b95 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -1020,6 +1020,9 @@ void VulkanDriver::beginRenderPass(Handle rth, const RenderPassP // Sometimes we need to permit the shader to sample the depth attachment by transitioning the // layout of all its subresources to a read-only layout. This is especially crucial for SSAO. // + // We cannot perform this transition using the render pass because the shaders in this render + // pass might sample from multiple miplevels. + // // We do not use GENERAL here due to the following validation message: // // The Vulkan spec states: Image subresources used as attachments in the current render pass @@ -1031,7 +1034,15 @@ void VulkanDriver::beginRenderPass(Handle rth, const RenderPassP // if (params.readOnlyDepthStencil & RenderPassParams::READONLY_DEPTH) { depthFeedback = depth.texture; - renderPassDepthLayout = VulkanDepthLayout::READ_ONLY; + VkImageSubresourceRange range = { + .aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT, + .baseMipLevel = 0, + .levelCount = depth.texture->levels, + .baseArrayLayer = 0, + .layerCount = depth.texture->depth, + }; + depth.texture->transitionLayout(cmdbuffer, range, VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL); + initialDepthLayout = renderPassDepthLayout = finalDepthLayout = VulkanDepthLayout::READ_ONLY; } if (depth.texture) { @@ -1175,9 +1186,8 @@ void VulkanDriver::beginRenderPass(Handle rth, const RenderPassP mContext.currentRenderPass = { .renderPass = renderPassInfo.renderPass, - .subpassMask = params.subpassMask, + .params = params, .currentSubpass = 0, - .depthFeedback = depthFeedback }; } @@ -1187,14 +1197,18 @@ void VulkanDriver::endRenderPass(int) { assert_invariant(mCurrentRenderTarget); - // In most cases, the image layout used during the render pass is the same as "finalLayout". - // i.e. there is no transition at the end. However one exception is depth, which can sometimes - // be transitioned from DEPTH_STENCIL_READ_ONLY_OPTIMAL to GENERAL. Here we detect this case - // and notify the texture wrapper for proper tracking. - VulkanTexture* depthFeedbackTexture = mContext.currentRenderPass.depthFeedback; - if (depthFeedbackTexture) { + // In some cases, depth needs to be transitioned from DEPTH_STENCIL_READ_ONLY_OPTIMAL back to + // GENERAL. We did not do this using the render pass because we need to change multiple mips. + if (mContext.currentRenderPass.params.readOnlyDepthStencil & RenderPassParams::READONLY_DEPTH) { const VulkanAttachment& depth = mCurrentRenderTarget->getDepth(); - depthFeedbackTexture->trackLayout(depth.level, depth.layer, + VkImageSubresourceRange range = { + .aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT, + .baseMipLevel = 0, + .levelCount = depth.texture->levels, + .baseArrayLayer = 0, + .layerCount = depth.texture->depth, + }; + depth.texture->transitionLayout(cmdbuffer, range, getDefaultImageLayout(TextureUsage::DEPTH_ATTACHMENT)); } @@ -1241,7 +1255,7 @@ void VulkanDriver::nextSubpass(int) { "Only two subpasses are currently supported."); assert_invariant(mCurrentRenderTarget); - assert_invariant(mContext.currentRenderPass.subpassMask); + assert_invariant(mContext.currentRenderPass.params.subpassMask); vkCmdNextSubpass(mContext.commands->get().cmdbuffer, VK_SUBPASS_CONTENTS_INLINE); @@ -1249,7 +1263,7 @@ void VulkanDriver::nextSubpass(int) { ++mContext.currentRenderPass.currentSubpass); for (uint32_t i = 0; i < VulkanPipelineCache::TARGET_BINDING_COUNT; i++) { - if ((1 << i) & mContext.currentRenderPass.subpassMask) { + if ((1 << i) & mContext.currentRenderPass.params.subpassMask) { VulkanAttachment subpassInput = mCurrentRenderTarget->getColor(i); VkDescriptorImageInfo info = { .imageView = subpassInput.getImageView(VK_IMAGE_ASPECT_COLOR_BIT), @@ -1787,19 +1801,6 @@ void VulkanDriver::draw(PipelineState pipelineState, Handle r .imageView = texture->getPrimaryImageView(), .imageLayout = texture->getPrimaryImageLayout() }; - - // TODO: it should not be necessary to use a custom image view here, instead we should - // transition all levels that need to be transitioned. - if (mContext.currentRenderPass.depthFeedback == texture) { - VkImageSubresourceRange range = { - .aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT, - .baseMipLevel = 0, - .levelCount = 1, - .baseArrayLayer = 0, - .layerCount = 1, - }; - iInfo[bindingPoint].imageView = texture->getImageView(range); - } } } diff --git a/filament/backend/src/vulkan/VulkanHandles.cpp b/filament/backend/src/vulkan/VulkanHandles.cpp index e637b798d3..1fdaccdd7f 100644 --- a/filament/backend/src/vulkan/VulkanHandles.cpp +++ b/filament/backend/src/vulkan/VulkanHandles.cpp @@ -214,7 +214,7 @@ int VulkanRenderTarget::getColorTargetCount(const VulkanRenderPass& pass) const continue; } // NOTE: This must be consistent with VkRenderPass construction (see VulkanFboCache). - if (!(pass.subpassMask & (1 << i)) || pass.currentSubpass == 1) { + if (!(pass.params.subpassMask & (1 << i)) || pass.currentSubpass == 1) { count++; } } diff --git a/filament/backend/src/vulkan/VulkanUtility.cpp b/filament/backend/src/vulkan/VulkanUtility.cpp index 00f5f0dc0c..3627f2d0af 100644 --- a/filament/backend/src/vulkan/VulkanUtility.cpp +++ b/filament/backend/src/vulkan/VulkanUtility.cpp @@ -601,6 +601,7 @@ VulkanLayoutTransition textureTransitionHelper(VulkanLayoutTransition transition break; case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: case VK_IMAGE_LAYOUT_GENERAL: + case VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL: transition.srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; transition.dstAccessMask = VK_ACCESS_SHADER_READ_BIT; transition.srcStage = VK_PIPELINE_STAGE_TRANSFER_BIT;