From 8cd23b043cf16b6604fa6b5d204f421e6d93a59d Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Mon, 7 Oct 2024 16:04:16 -0700 Subject: [PATCH] vk: refactor texture default layout 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 ----- filament/backend/src/vulkan/VulkanTexture.cpp | 104 ++++++++++++------ filament/backend/src/vulkan/VulkanTexture.h | 18 ++- .../caching/VulkanDescriptorSetManager.cpp | 2 +- 7 files changed, 82 insertions(+), 91 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 c90760e9ba7..106edeb031c 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); } @@ -1350,8 +1342,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/VulkanTexture.cpp b/filament/backend/src/vulkan/VulkanTexture.cpp index 2181ee59053..877ddbaf83a 100644 --- a/filament/backend/src/vulkan/VulkanTexture.cpp +++ b/filament/backend/src/vulkan/VulkanTexture.cpp @@ -113,24 +113,51 @@ 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) { + 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) {} + VulkanTextureState* VulkanTexture::getSharedState() { VulkanTextureState* state = mAllocator->handle_cast(mState); return state; @@ -141,6 +168,7 @@ VulkanTextureState const* VulkanTexture::getSharedState() const { return state; } +// Constructor for internally passed VkImage VulkanTexture::VulkanTexture( VkDevice device, VmaAllocator allocator, VulkanCommands* commands, VulkanResourceAllocator* handleAllocator, @@ -153,12 +181,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, @@ -170,7 +200,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. @@ -309,18 +339,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(); @@ -330,14 +362,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++; @@ -440,7 +473,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); @@ -491,6 +524,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; @@ -602,8 +640,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 e3d1babbbd0..dc82a3d743f 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; }; @@ -134,6 +133,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. @@ -190,13 +193,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,