From 875b2959673ad83f78fc6d2de0c781516342cc64 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Thu, 10 Oct 2024 12:31:20 -0700 Subject: [PATCH] vk: refactor texture default layout (#8183) Previously, default layout is based on usage, but this actually has two paths (Filament's TextureUsage and the computed VkTextureUsage) that do not always agree. We simplify so that default layout is stored in the texture itself. Also remove some unnecessary code that is no longer necessary. In particular, we shouldn't be doing a flush and wait for the transition to complete before updating a sampler descriptor. We just need to make sure the layout before it is accessed is correctly given in the update struct. --- filament/backend/src/vulkan/VulkanBlitter.cpp | 8 +- filament/backend/src/vulkan/VulkanDriver.cpp | 10 -- filament/backend/src/vulkan/VulkanDriver.h | 1 - .../backend/src/vulkan/VulkanImageUtility.h | 30 ----- .../backend/src/vulkan/VulkanReadPixels.cpp | 4 +- filament/backend/src/vulkan/VulkanTexture.cpp | 114 ++++++++++++------ filament/backend/src/vulkan/VulkanTexture.h | 24 ++-- .../caching/VulkanDescriptorSetManager.cpp | 2 +- 8 files changed, 91 insertions(+), 102 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanBlitter.cpp b/filament/backend/src/vulkan/VulkanBlitter.cpp index 7100b026352..db68b94b58b 100644 --- a/filament/backend/src/vulkan/VulkanBlitter.cpp +++ b/filament/backend/src/vulkan/VulkanBlitter.cpp @@ -66,10 +66,10 @@ inline void blitFast(VulkanCommandBuffer* commands, VkImageAspectFlags aspect, V 1, blitRegions, filter); if (oldSrcLayout == VulkanLayout::UNDEFINED) { - oldSrcLayout = imgutil::getDefaultLayout(src.texture->usage); + oldSrcLayout = src.texture->getDefaultLayout(); } if (oldDstLayout == VulkanLayout::UNDEFINED) { - oldDstLayout = imgutil::getDefaultLayout(dst.texture->usage); + oldDstLayout = dst.texture->getDefaultLayout(); } src.texture->transitionLayout(commands, srcRange, oldSrcLayout); dst.texture->transitionLayout(commands, dstRange, oldDstLayout); @@ -109,10 +109,10 @@ inline void resolveFast(VulkanCommandBuffer* commands, VkImageAspectFlags aspect 1, resolveRegions); if (oldSrcLayout == VulkanLayout::UNDEFINED) { - oldSrcLayout = imgutil::getDefaultLayout(src.texture->usage); + oldSrcLayout = src.texture->getDefaultLayout(); } if (oldDstLayout == VulkanLayout::UNDEFINED) { - oldDstLayout = imgutil::getDefaultLayout(dst.texture->usage); + oldDstLayout = dst.texture->getDefaultLayout(); } src.texture->transitionLayout(commands, srcRange, oldSrcLayout); dst.texture->transitionLayout(commands, dstRange, oldDstLayout); diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index f1a41cc7a62..18f6bf53596 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -404,14 +404,6 @@ void VulkanDriver::updateDescriptorSetTexture( SamplerParams params) { VulkanDescriptorSet* set = mResourceAllocator.handle_cast(dsh); VulkanTexture* texture = mResourceAllocator.handle_cast(th); - - // We need to make sure the initial layout transition has been completed before we can write - // the sampler descriptor. We flush and wait until the transition has been completed. - if (!texture->transitionReady()) { - mCommands.flush(); - mCommands.wait(); - } - VkSampler const vksampler = mSamplerCache.getSampler(params); mDescriptorSetManager.updateSampler(set, binding, texture, vksampler); } @@ -1360,8 +1352,6 @@ void VulkanDriver::beginRenderPass(Handle rth, const RenderPassP VulkanTexture* texture = attachment.texture; if (texture->samples == 1) { - mRenderPassFboInfo.hasColorResolve = true; - auto const& range = attachment.getSubresourceRange(); attachment.texture->transitionLayout(&commands, range, VulkanLayout::COLOR_ATTACHMENT); diff --git a/filament/backend/src/vulkan/VulkanDriver.h b/filament/backend/src/vulkan/VulkanDriver.h index a3a9456a82b..7dd95209342 100644 --- a/filament/backend/src/vulkan/VulkanDriver.h +++ b/filament/backend/src/vulkan/VulkanDriver.h @@ -152,7 +152,6 @@ class VulkanDriver final : public DriverBase { struct { using AttachmentArray = CappedArray; AttachmentArray attachments; - bool hasColorResolve = false; } mRenderPassFboInfo = {}; bool const mIsSRGBSwapChainSupported; diff --git a/filament/backend/src/vulkan/VulkanImageUtility.h b/filament/backend/src/vulkan/VulkanImageUtility.h index 82ff436c163..f8027d96975 100644 --- a/filament/backend/src/vulkan/VulkanImageUtility.h +++ b/filament/backend/src/vulkan/VulkanImageUtility.h @@ -76,36 +76,6 @@ inline VkImageViewType getViewType(SamplerType target) { } } -inline VulkanLayout getDefaultLayout(TextureUsage usage) { - if (any(usage & TextureUsage::DEPTH_ATTACHMENT)) { - if (any(usage & TextureUsage::SAMPLEABLE)) { - return VulkanLayout::DEPTH_SAMPLER; - } else { - return VulkanLayout::DEPTH_ATTACHMENT; - } - } - - if (any(usage & TextureUsage::COLOR_ATTACHMENT)) { - return VulkanLayout::COLOR_ATTACHMENT; - } - // Finally, the layout for an immutable texture is optimal read-only. - return VulkanLayout::READ_ONLY; -} - -inline VulkanLayout getDefaultLayout(VkImageUsageFlags vkusage) { - TextureUsage usage{}; - if (vkusage & VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) { - usage = usage | TextureUsage::DEPTH_ATTACHMENT; - } - if (vkusage & VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT) { - usage = usage | TextureUsage::COLOR_ATTACHMENT; - } - if (vkusage & VK_IMAGE_USAGE_SAMPLED_BIT) { - usage = usage | TextureUsage::SAMPLEABLE; - } - return getDefaultLayout(usage); -} - constexpr inline VkImageLayout getVkLayout(VulkanLayout layout) { switch (layout) { case VulkanLayout::UNDEFINED: diff --git a/filament/backend/src/vulkan/VulkanReadPixels.cpp b/filament/backend/src/vulkan/VulkanReadPixels.cpp index bef08811c6d..418f6771961 100644 --- a/filament/backend/src/vulkan/VulkanReadPixels.cpp +++ b/filament/backend/src/vulkan/VulkanReadPixels.cpp @@ -232,7 +232,7 @@ void VulkanReadPixels::run(VulkanRenderTarget* srcTarget, uint32_t const x, uint VulkanAttachment const srcAttachment = srcTarget->getColor(0); VkImageSubresourceRange const srcRange = srcAttachment.getSubresourceRange(); - srcTexture->transitionLayout(cmdbuffer, {}, srcRange, VulkanLayout::TRANSFER_SRC); + srcTexture->transitionLayout(cmdbuffer, srcRange, VulkanLayout::TRANSFER_SRC); VkImageCopy const imageCopyRegion = { .srcSubresource = { @@ -268,7 +268,7 @@ void VulkanReadPixels::run(VulkanRenderTarget* srcTarget, uint32_t const x, uint imgutil::getVkLayout(VulkanLayout::TRANSFER_DST), 1, &imageCopyRegion); // Restore the source image layout. - srcTexture->transitionLayout(cmdbuffer, {}, srcRange, VulkanLayout::COLOR_ATTACHMENT); + srcTexture->transitionLayout(cmdbuffer, srcRange, srcTexture->getDefaultLayout()); vkEndCommandBuffer(cmdbuffer); diff --git a/filament/backend/src/vulkan/VulkanTexture.cpp b/filament/backend/src/vulkan/VulkanTexture.cpp index e33fd6faa00..2358d6ea0de 100644 --- a/filament/backend/src/vulkan/VulkanTexture.cpp +++ b/filament/backend/src/vulkan/VulkanTexture.cpp @@ -113,25 +113,52 @@ VkComponentMapping composeSwizzle(VkComponentMapping const& prev, VkComponentMap }; } -} // anonymous namespace +inline VulkanLayout getDefaultLayoutImpl(TextureUsage usage) { + if (any(usage & TextureUsage::DEPTH_ATTACHMENT)) { + if (any(usage & TextureUsage::SAMPLEABLE)) { + return VulkanLayout::DEPTH_SAMPLER; + } else { + return VulkanLayout::DEPTH_ATTACHMENT; + } + } -VulkanTextureState::VulkanTextureState( - VkDevice device, VmaAllocator allocator, VulkanCommands* commands, - VulkanStagePool& stagePool, - VkFormat format, VkImageViewType viewType, uint8_t levels, uint8_t layerCount) - : VulkanResource(VulkanResourceType::HEAP_ALLOCATED), - mVkFormat(format), - mViewType(viewType), - mFullViewRange { - filament::backend::getImageAspect(format), 0, levels, 0, layerCount - }, - mStagePool(stagePool), - mDevice(device), - mAllocator(allocator), - mCommands(commands), - mIsTransientAttachment(false) { + if (any(usage & TextureUsage::COLOR_ATTACHMENT)) { + return VulkanLayout::COLOR_ATTACHMENT; + } + // Finally, the layout for an immutable texture is optimal read-only. + return VulkanLayout::READ_ONLY; +} + +inline VulkanLayout getDefaultLayoutImpl(VkImageUsageFlags vkusage) { + TextureUsage usage{}; + if (vkusage & VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) { + usage = usage | TextureUsage::DEPTH_ATTACHMENT; + } + if (vkusage & VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT) { + usage = usage | TextureUsage::COLOR_ATTACHMENT; + } + if (vkusage & VK_IMAGE_USAGE_SAMPLED_BIT) { + usage = usage | TextureUsage::SAMPLEABLE; + } + return getDefaultLayoutImpl(usage); } +} // anonymous namespace + +VulkanTextureState::VulkanTextureState(VkDevice device, VmaAllocator allocator, + VulkanCommands* commands, VulkanStagePool& stagePool, VkFormat format, + VkImageViewType viewType, uint8_t levels, uint8_t layerCount, VulkanLayout defaultLayout) + : VulkanResource(VulkanResourceType::HEAP_ALLOCATED), + mVkFormat(format), + mViewType(viewType), + mFullViewRange{filament::backend::getImageAspect(format), 0, levels, 0, layerCount}, + mDefaultLayout(defaultLayout), + mStagePool(stagePool), + mDevice(device), + mAllocator(allocator), + mCommands(commands), + mIsTransientAttachment(false) {} + VulkanTextureState* VulkanTexture::getSharedState() { VulkanTextureState* state = mAllocator->handle_cast(mState); return state; @@ -142,6 +169,7 @@ VulkanTextureState const* VulkanTexture::getSharedState() const { return state; } +// Constructor for internally passed VkImage VulkanTexture::VulkanTexture( VkDevice device, VmaAllocator allocator, VulkanCommands* commands, VulkanResourceAllocator* handleAllocator, @@ -154,12 +182,14 @@ VulkanTexture::VulkanTexture( mAllocator(handleAllocator), mState(handleAllocator->initHandle( device, allocator, commands, stagePool, - format, imgutil::getViewType(SamplerType::SAMPLER_2D), 1, 1)) { + format, imgutil::getViewType(SamplerType::SAMPLER_2D), 1, 1, + getDefaultLayoutImpl(tusage))) { auto* const state = getSharedState(); state->mTextureImage = image; mPrimaryViewRange = state->mFullViewRange; } +// Constructor for user facing texture VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice, VulkanContext const& context, VmaAllocator allocator, VulkanCommands* commands, VulkanResourceAllocator* handleAllocator, SamplerType target, uint8_t levels, @@ -171,7 +201,7 @@ VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice, mAllocator(handleAllocator), mState(handleAllocator->initHandle(device, allocator, commands, stagePool, backend::getVkFormat(tformat), imgutil::getViewType(target), levels, - getLayerCount(target, depth))) { + getLayerCount(target, depth), VulkanLayout::UNDEFINED)) { auto* const state = getSharedState(); // Create an appropriately-sized device-only VkImage, but do not fill it yet. @@ -326,18 +356,20 @@ VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice, VulkanCommandBuffer& commandsBuf = state->mCommands->get(); commandsBuf.acquire(this); - transitionLayout(&commandsBuf, mPrimaryViewRange, imgutil::getDefaultLayout(imageInfo.usage)); + + auto const defaultLayout = state->mDefaultLayout = getDefaultLayoutImpl(imageInfo.usage); + transitionLayout(&commandsBuf, mPrimaryViewRange, defaultLayout); } +// Constructor for creating a texture view VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice, VulkanContext const& context, VmaAllocator allocator, VulkanCommands* commands, - VulkanResourceAllocator* handleAllocator, - VulkanTexture const* src, uint8_t baseLevel, uint8_t levelCount) - : HwTexture(src->target, src->levels, src->samples, src->width, src->height, src->depth, - src->format, src->usage), - VulkanResource(VulkanResourceType::TEXTURE), - mAllocator(handleAllocator) -{ + VulkanResourceAllocator* handleAllocator, VulkanTexture const* src, uint8_t baseLevel, + uint8_t levelCount) + : HwTexture(src->target, src->levels, src->samples, src->width, src->height, src->depth, + src->format, src->usage), + VulkanResource(VulkanResourceType::TEXTURE), + mAllocator(handleAllocator) { mState = src->mState; auto* state = getSharedState(); @@ -347,14 +379,15 @@ VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice, mPrimaryViewRange.levelCount = levelCount; } +// Constructor for creating a texture view with swizzle VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice, VulkanContext const& context, VmaAllocator allocator, VulkanCommands* commands, - VulkanResourceAllocator* handleAllocator, - VulkanTexture const* src, VkComponentMapping swizzle) - : HwTexture(src->target, src->levels, src->samples, src->width, src->height, src->depth, - src->format, src->usage), - VulkanResource(VulkanResourceType::TEXTURE), - mAllocator(handleAllocator) { + VulkanResourceAllocator* handleAllocator, VulkanTexture const* src, + VkComponentMapping swizzle) + : HwTexture(src->target, src->levels, src->samples, src->width, src->height, src->depth, + src->format, src->usage), + VulkanResource(VulkanResourceType::TEXTURE), + mAllocator(handleAllocator) { mState = src->mState; auto* state = getSharedState(); state->refs++; @@ -457,7 +490,7 @@ void VulkanTexture::updateImage(const PixelBufferDescriptor& data, uint32_t widt VkImageLayout const newVkLayout = imgutil::getVkLayout(newLayout); if (nextLayout == VulkanLayout::UNDEFINED) { - nextLayout = imgutil::getDefaultLayout(this->usage); + nextLayout = getDefaultLayout(); } transitionLayout(&commands, transitionRange, newLayout); @@ -508,6 +541,11 @@ void VulkanTexture::updateImageWithBlit(const PixelBufferDescriptor& hostData, u transitionLayout(&commands, range, oldLayout); } +VulkanLayout VulkanTexture::getDefaultLayout() const { + auto* const state = getSharedState(); + return state->mDefaultLayout; +} + VkImageView VulkanTexture::getAttachmentView(VkImageSubresourceRange range) { range.levelCount = 1; range.layerCount = 1; @@ -553,13 +591,11 @@ VkImageAspectFlags VulkanTexture::getImageAspect() const { } bool VulkanTexture::transitionLayout(VulkanCommandBuffer* commands, - const VkImageSubresourceRange& range, VulkanLayout newLayout) { - return transitionLayout(commands->buffer(), commands->fence, range, newLayout); + VkImageSubresourceRange const& range, VulkanLayout newLayout) { + return transitionLayout(commands->buffer(), range, newLayout); } -bool VulkanTexture::transitionLayout( - VkCommandBuffer cmdbuf, std::shared_ptr fence, - const VkImageSubresourceRange& range, +bool VulkanTexture::transitionLayout(VkCommandBuffer cmdbuf, VkImageSubresourceRange const& range, VulkanLayout newLayout) { auto* const state = getSharedState(); VulkanLayout const oldLayout = getLayout(range.baseArrayLayer, range.baseMipLevel); @@ -619,8 +655,6 @@ bool VulkanTexture::transitionLayout( setLayout(range, newLayout); if (hasTransitions) { - state->mTransitionFence = fence; - #if FVK_ENABLED(FVK_DEBUG_LAYOUT_TRANSITION) FVK_LOGD << "transition texture=" << state->mTextureImage << " (" << range.baseArrayLayer << "," << range.baseMipLevel << ")" << " count=(" << range.layerCount << "," diff --git a/filament/backend/src/vulkan/VulkanTexture.h b/filament/backend/src/vulkan/VulkanTexture.h index 4b76eb5329e..acc58846709 100644 --- a/filament/backend/src/vulkan/VulkanTexture.h +++ b/filament/backend/src/vulkan/VulkanTexture.h @@ -34,8 +34,8 @@ class VulkanResourceAllocator; struct VulkanTextureState : public VulkanResource { VulkanTextureState(VkDevice device, VmaAllocator allocator, VulkanCommands* commands, - VulkanStagePool& stagePool, - VkFormat format, VkImageViewType viewType, uint8_t levels, uint8_t layerCount); + VulkanStagePool& stagePool, VkFormat format, VkImageViewType viewType, uint8_t levels, + uint8_t layerCount, VulkanLayout defaultLayout); struct ImageViewKey { VkImageSubresourceRange range; // 4 * 5 bytes @@ -67,8 +67,8 @@ struct VulkanTextureState : public VulkanResource { VkFormat const mVkFormat; VkImageViewType const mViewType; VkImageSubresourceRange const mFullViewRange; - VkImage mTextureImage = VK_NULL_HANDLE; + VulkanLayout mDefaultLayout; // Track the image layout of each subresource using a sparse range map. utils::RangeMap mSubresourceLayouts; @@ -78,7 +78,6 @@ struct VulkanTextureState : public VulkanResource { VkDevice mDevice; VmaAllocator mAllocator; VulkanCommands* mCommands; - std::shared_ptr mTransitionFence; bool mIsTransientAttachment; }; @@ -135,6 +134,10 @@ struct VulkanTexture : public HwTexture, VulkanResource { return getLayout(mPrimaryViewRange.baseArrayLayer, mPrimaryViewRange.baseMipLevel); } + // Returns the layout for the intended use of this texture (and not the expected layout at the + // time of the call. + VulkanLayout getDefaultLayout() const; + // Gets or creates a cached VkImageView for a single subresource that can be used as a render // target attachment. Unlike the primary image view, this always has type VK_IMAGE_VIEW_TYPE_2D // and the identity swizzle. @@ -176,11 +179,11 @@ struct VulkanTexture : public HwTexture, VulkanResource { return state->mIsTransientAttachment; } - bool transitionLayout(VulkanCommandBuffer* commands, const VkImageSubresourceRange& range, + bool transitionLayout(VulkanCommandBuffer* commands, VkImageSubresourceRange const& range, VulkanLayout newLayout); - bool transitionLayout(VkCommandBuffer cmdbuf, std::shared_ptr fence, - VkImageSubresourceRange const& range, VulkanLayout newLayout); + bool transitionLayout(VkCommandBuffer cmdbuf, VkImageSubresourceRange const& range, + VulkanLayout newLayout); void attachmentToSamplerBarrier(VulkanCommandBuffer* commands, VkImageSubresourceRange const& range); @@ -196,13 +199,6 @@ struct VulkanTexture : public HwTexture, VulkanResource { // manually (outside of calls to transitionLayout). void setLayout(VkImageSubresourceRange const& range, VulkanLayout newLayout); - bool transitionReady() { - VulkanTextureState* state = getSharedState(); - auto res = !state->mTransitionFence || state->mTransitionFence->getStatus() == VK_SUCCESS; - state->mTransitionFence.reset(); - return res; - } - #if FVK_ENABLED(FVK_DEBUG_TEXTURE) void print() const; #endif diff --git a/filament/backend/src/vulkan/caching/VulkanDescriptorSetManager.cpp b/filament/backend/src/vulkan/caching/VulkanDescriptorSetManager.cpp index 660001083e8..6ffaa8ae07d 100644 --- a/filament/backend/src/vulkan/caching/VulkanDescriptorSetManager.cpp +++ b/filament/backend/src/vulkan/caching/VulkanDescriptorSetManager.cpp @@ -514,7 +514,7 @@ void VulkanDescriptorSetManager::updateSampler(VulkanDescriptorSet* set, uint8_t } else { info.imageView = texture->getViewForType(range, expectedType); } - info.imageLayout = imgutil::getVkLayout(texture->getPrimaryImageLayout()); + info.imageLayout = imgutil::getVkLayout(texture->getDefaultLayout()); VkWriteDescriptorSet const descriptorWrite = { .sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET, .pNext = nullptr,