From 2e7bea138202ea8e5050fedb2f300761f42757c6 Mon Sep 17 00:00:00 2001 From: David Steele Date: Thu, 26 Sep 2024 19:06:21 +0100 Subject: [PATCH] Vulkan render pass uses ref counted handle As we are now sharing render pass between framebuffers of a swapchain, using a ref-counted handle will ensure that we can clean this up safely. Change-Id: Ic103c57314c39c2141883d688175664c8d30939d --- .../vulkan-impl/vulkan-command-buffer.cpp | 2 +- .../vulkan-impl/vulkan-framebuffer-impl.cpp | 34 +-- .../vulkan-impl/vulkan-framebuffer-impl.h | 28 +- .../vulkan-impl/vulkan-framebuffer.cpp | 2 +- .../graphics/vulkan-impl/vulkan-handle.h | 245 ++++++++++++++++++ .../vulkan-impl/vulkan-pipeline-impl.cpp | 2 +- .../vulkan-impl/vulkan-render-pass-impl.cpp | 7 +- .../vulkan-impl/vulkan-render-pass-impl.h | 16 +- .../vulkan-impl/vulkan-render-target.cpp | 2 +- .../vulkan-impl/vulkan-render-target.h | 8 +- .../vulkan-impl/vulkan-swapchain-impl.cpp | 12 +- .../graphics/vulkan-impl/vulkan-types.h | 53 ---- 12 files changed, 300 insertions(+), 111 deletions(-) create mode 100644 dali/internal/graphics/vulkan-impl/vulkan-handle.h diff --git a/dali/internal/graphics/vulkan-impl/vulkan-command-buffer.cpp b/dali/internal/graphics/vulkan-impl/vulkan-command-buffer.cpp index 1cba175a7..922bb391a 100644 --- a/dali/internal/graphics/vulkan-impl/vulkan-command-buffer.cpp +++ b/dali/internal/graphics/vulkan-impl/vulkan-command-buffer.cpp @@ -186,7 +186,7 @@ void CommandBuffer::BeginRenderPass(Graphics::RenderPass* gfxRenderPass auto surface = renderTarget->GetSurface(); auto& device = mController.GetGraphicsDevice(); FramebufferImpl* framebuffer; - RenderPassImpl* renderPassImpl; + RenderPassHandle renderPassImpl; if(surface) { auto window = static_cast(surface); diff --git a/dali/internal/graphics/vulkan-impl/vulkan-framebuffer-impl.cpp b/dali/internal/graphics/vulkan-impl/vulkan-framebuffer-impl.cpp index 6c281e774..4e5d556f5 100644 --- a/dali/internal/graphics/vulkan-impl/vulkan-framebuffer-impl.cpp +++ b/dali/internal/graphics/vulkan-impl/vulkan-framebuffer-impl.cpp @@ -119,13 +119,13 @@ bool FramebufferAttachment::IsValid() const FramebufferImpl* FramebufferImpl::New( Vulkan::Device& device, - RenderPassImpl* renderPass, + RenderPassHandle renderPass, OwnedAttachments& attachments, uint32_t width, uint32_t height, bool hasDepthAttachments) { - DALI_ASSERT_ALWAYS(renderPass != nullptr && "You require more render passes!"); + DALI_ASSERT_ALWAYS(renderPass && "You require more render passes!"); std::vector imageViewAttachments; for(auto& attachment : attachments) @@ -146,7 +146,7 @@ FramebufferImpl* FramebufferImpl::New( return new FramebufferImpl(device, attachments, vkFramebuffer, - *renderPass, + renderPass, width, height, hasDepthAttachments); @@ -154,7 +154,7 @@ FramebufferImpl* FramebufferImpl::New( FramebufferImpl* FramebufferImpl::New( Vulkan::Device& device, - RenderPassImpl* renderPass, + RenderPassHandle renderPass, OwnedAttachments& colorAttachments, std::unique_ptr& depthAttachment, uint32_t width, @@ -187,7 +187,7 @@ FramebufferImpl* FramebufferImpl::New( // This vector stores the attachments (vk::ImageViews) // Flag that indicates if the render pass is externally provided - if(renderPass == nullptr) + if(!renderPass) { // Create compatible vulkan render pass renderPass = RenderPassImpl::New(device, attachments, depthAttachment.get()); @@ -201,13 +201,13 @@ FramebufferImpl* FramebufferImpl::New( return FramebufferImpl::New(device, renderPass, ownedAttachments, width, height, hasDepth); } -FramebufferImpl::FramebufferImpl(Device& graphicsDevice, - OwnedAttachments& attachments, - vk::Framebuffer vkHandle, - const RenderPassImpl& renderPassImpl, - uint32_t width, - uint32_t height, - bool hasDepthAttachment) +FramebufferImpl::FramebufferImpl(Device& graphicsDevice, + OwnedAttachments& attachments, + vk::Framebuffer vkHandle, + RenderPassHandle renderPassImpl, + uint32_t width, + uint32_t height, + bool hasDepthAttachment) : mGraphicsDevice(&graphicsDevice), mWidth(width), mHeight(height), @@ -215,7 +215,7 @@ FramebufferImpl::FramebufferImpl(Device& graphicsDevice, mFramebuffer(vkHandle), mHasDepthAttachment(hasDepthAttachment) { - mRenderPasses.push_back(RenderPassMapElement{nullptr, const_cast(&renderPassImpl)}); + mRenderPasses.emplace_back(RenderPassMapElement{nullptr, renderPassImpl}); } void FramebufferImpl::Destroy() @@ -331,16 +331,16 @@ uint32_t FramebufferImpl::GetRenderPassCount() const return uint32_t(mRenderPasses.size()); } -RenderPassImpl* FramebufferImpl::GetRenderPass(uint32_t index) const +RenderPassHandle FramebufferImpl::GetRenderPass(uint32_t index) const { if(index < mRenderPasses.size()) { return mRenderPasses[index].renderPassImpl; } - return nullptr; + return RenderPassHandle{}; } -RenderPassImpl* FramebufferImpl::GetImplFromRenderPass(RenderPass* renderPass) +RenderPassHandle FramebufferImpl::GetImplFromRenderPass(RenderPass* renderPass) { auto attachments = renderPass->GetCreateInfo().attachments; auto matchLoadOp = attachments->front().loadOp; @@ -360,7 +360,7 @@ RenderPassImpl* FramebufferImpl::GetImplFromRenderPass(RenderPass* renderPass) } else { - DALI_ASSERT_DEBUG(element.renderPassImpl != nullptr && "Render pass list doesn't contain impl"); + DALI_ASSERT_DEBUG(element.renderPassImpl && "Render pass list doesn't contain impl"); auto createInfo = element.renderPassImpl->GetCreateInfo(); if(createInfo.attachmentDescriptions[0].loadOp == VkLoadOpType(matchLoadOp).loadOp && diff --git a/dali/internal/graphics/vulkan-impl/vulkan-framebuffer-impl.h b/dali/internal/graphics/vulkan-impl/vulkan-framebuffer-impl.h index 2d10f0c3f..16e4ea66b 100644 --- a/dali/internal/graphics/vulkan-impl/vulkan-framebuffer-impl.h +++ b/dali/internal/graphics/vulkan-impl/vulkan-framebuffer-impl.h @@ -21,11 +21,11 @@ #include #include +#include namespace Dali::Graphics::Vulkan { class RenderPass; -class RenderPassImpl; enum class AttachmentType { @@ -120,7 +120,7 @@ public: */ static FramebufferImpl* New( Vulkan::Device& device, - RenderPassImpl* renderPass, + RenderPassHandle renderPass, OwnedAttachments& attachments, uint32_t width, uint32_t height, @@ -140,7 +140,7 @@ public: */ static FramebufferImpl* New( Vulkan::Device& device, - RenderPassImpl* renderPass, + RenderPassHandle renderPass, OwnedAttachments& colorAttachments, std::unique_ptr& depthAttachment, uint32_t width, @@ -157,13 +157,13 @@ public: * @param[in] height Height of the framebuffer * @param[in] hasDepthAttachment True if the last attachment is a depth buffer */ - FramebufferImpl(Device& graphicsDevice, - OwnedAttachments& attachments, - vk::Framebuffer vkHandle, - const RenderPassImpl& renderPass, - uint32_t width, - uint32_t height, - bool hasDepthAttachment); + FramebufferImpl(Device& graphicsDevice, + OwnedAttachments& attachments, + vk::Framebuffer vkHandle, + RenderPassHandle renderPass, + uint32_t width, + uint32_t height, + bool hasDepthAttachment); void Destroy(); @@ -177,9 +177,9 @@ public: [[nodiscard]] uint32_t GetAttachmentCount(AttachmentType type) const; - [[nodiscard]] RenderPassImpl* GetImplFromRenderPass(RenderPass* renderPass); // May mutate mRenderPasses + [[nodiscard]] RenderPassHandle GetImplFromRenderPass(RenderPass* renderPass); // May mutate mRenderPasses - [[nodiscard]] RenderPassImpl* GetRenderPass(uint32_t index) const; + [[nodiscard]] RenderPassHandle GetRenderPass(uint32_t index) const; [[nodiscard]] uint32_t GetRenderPassCount() const; @@ -198,8 +198,8 @@ private: */ struct RenderPassMapElement { - RenderPass* renderPass{nullptr}; - RenderPassImpl* renderPassImpl{nullptr}; + RenderPass* renderPass{nullptr}; + RenderPassHandle renderPassImpl{nullptr}; }; using RenderPasses = std::vector; diff --git a/dali/internal/graphics/vulkan-impl/vulkan-framebuffer.cpp b/dali/internal/graphics/vulkan-impl/vulkan-framebuffer.cpp index 18d40723e..7209950b8 100644 --- a/dali/internal/graphics/vulkan-impl/vulkan-framebuffer.cpp +++ b/dali/internal/graphics/vulkan-impl/vulkan-framebuffer.cpp @@ -48,7 +48,7 @@ bool Framebuffer::InitializeResource() } auto& device = mController.GetGraphicsDevice(); - mFramebufferImpl = FramebufferImpl::New(device, nullptr, colorAttachments, depthStencilAttachment, mCreateInfo.size.width, mCreateInfo.size.height); + mFramebufferImpl = FramebufferImpl::New(device, RenderPassHandle{}, colorAttachments, depthStencilAttachment, mCreateInfo.size.width, mCreateInfo.size.height); return true; } diff --git a/dali/internal/graphics/vulkan-impl/vulkan-handle.h b/dali/internal/graphics/vulkan-impl/vulkan-handle.h new file mode 100644 index 000000000..53d8302f9 --- /dev/null +++ b/dali/internal/graphics/vulkan-impl/vulkan-handle.h @@ -0,0 +1,245 @@ +#pragma once + +/* + * Copyright (c) 2024 Samsung Electronics Co., Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +namespace Dali::Graphics::Vulkan +{ + +template +class Handle +{ +public: + Handle(); + + explicit Handle(T* object); + + Handle(const Handle& handle); + + Handle& operator=(const Handle& handle); + + Handle& operator=(Handle&& handle); + + Handle(Handle&& handle) noexcept; + + ~Handle(); + + operator bool() const; + + T* operator->() const + { + return mObject; + } + + uint32_t GetRefCount() const + { + return mObject->GetRefCount(); + } + + T& operator*() const + { + return *mObject; + } + + template + Handle StaticCast() + { + return Handle(static_cast(mObject)); + } + + template + bool operator==(const Handle& object) const + { + return mObject == &*object; + } + + template + bool operator!=(const Handle& object) const + { + return !(mObject == &*object); + } + + template + Handle DynamicCast(); + + void Reset() + { + if(mObject) + { + mObject->Release(); + mObject = nullptr; + } + } + +private: + T* mObject{nullptr}; +}; + +template +static Handle VkTypeCast(const Handle& inval) +{ + return Handle(static_cast(&*inval)); +} + +template +Handle::Handle(T* object) +: mObject(object) +{ + if(mObject) + { + mObject->Retain(); + } +} + +template +Handle::Handle() +: mObject(nullptr) +{ +} + +template +Handle::Handle(const Handle& handle) +{ + mObject = handle.mObject; + if(mObject) + { + mObject->Retain(); + } +} + +template +Handle::Handle(Handle&& handle) noexcept +{ + mObject = handle.mObject; + handle.mObject = nullptr; +} + +template +Handle::operator bool() const +{ + return mObject != nullptr; +} + +template +Handle& Handle::operator=(Handle&& handle) +{ + if(mObject) + { + mObject->Release(); + } + mObject = handle.mObject; + handle.mObject = nullptr; + return *this; +} + +template +Handle& Handle::operator=(const Handle& handle) +{ + mObject = handle.mObject; + if(mObject) + { + mObject->Retain(); + } + return *this; +} + +template +Handle::~Handle() +{ + if(mObject) + { + mObject->Release(); + } +} + +template +template +Handle Handle::DynamicCast() +{ + auto val = dynamic_cast(mObject); + if(val) + { + return Handle(val); + } + return Handle(); +} + +template +Handle MakeRef(Args&&... args) +{ + return Handle(new T(std::forward(args)...)); +} + +template +Handle NewRef(Args&&... args) +{ + return Handle(T::New(std::forward(args)...)); +} + +class VkSharedResource // E.g. render pass +{ +public: + VkSharedResource() = default; + + virtual ~VkSharedResource() = default; + + void Release() + { + OnRelease(--mRefCount); + + if(mRefCount == 0) + { + // orphaned + if(!Destroy()) + { + delete this; + } + } + } + + void Retain() + { + OnRetain(++mRefCount); + } + + uint32_t GetRefCount() + { + return mRefCount; + } + + virtual bool Destroy() + { + return OnDestroy(); + } + + virtual void OnRetain(uint32_t refcount) + { + } + + virtual void OnRelease(uint32_t refcount) + { + } + + virtual bool OnDestroy() + { + return false; + } + +private: + std::atomic_uint mRefCount{0u}; +}; + +} // namespace Dali::Graphics::Vulkan diff --git a/dali/internal/graphics/vulkan-impl/vulkan-pipeline-impl.cpp b/dali/internal/graphics/vulkan-impl/vulkan-pipeline-impl.cpp index becd5d085..03d928098 100644 --- a/dali/internal/graphics/vulkan-impl/vulkan-pipeline-impl.cpp +++ b/dali/internal/graphics/vulkan-impl/vulkan-pipeline-impl.cpp @@ -284,7 +284,7 @@ void PipelineImpl::InitializePipeline() auto renderPassCount = fbImpl->GetRenderPassCount(); for(auto i = 0u; i < renderPassCount; ++i) { - RenderPassImpl* impl = fbImpl->GetRenderPass(i); + RenderPassHandle impl = fbImpl->GetRenderPass(i); gfxPipelineInfo.renderPass = impl->GetVkHandle(); gfxPipelineInfo.subpass = 0; diff --git a/dali/internal/graphics/vulkan-impl/vulkan-render-pass-impl.cpp b/dali/internal/graphics/vulkan-impl/vulkan-render-pass-impl.cpp index c9b2ebd04..9c5d9a3c3 100644 --- a/dali/internal/graphics/vulkan-impl/vulkan-render-pass-impl.cpp +++ b/dali/internal/graphics/vulkan-impl/vulkan-render-pass-impl.cpp @@ -29,13 +29,13 @@ extern Debug::Filter* gVulkanFilter; namespace Dali::Graphics::Vulkan { -RenderPassImpl* RenderPassImpl::New( +RenderPassHandle RenderPassImpl::New( Vulkan::Device& device, const std::vector& colorAttachments, FramebufferAttachment* depthAttachment) { auto renderPass = new RenderPassImpl(device, colorAttachments, depthAttachment); - return renderPass; + return RenderPassHandle(renderPass); } RenderPassImpl::RenderPassImpl(Vulkan::Device& device, @@ -49,7 +49,7 @@ RenderPassImpl::RenderPassImpl(Vulkan::Device& device RenderPassImpl::~RenderPassImpl() = default; -void RenderPassImpl::Destroy() +bool RenderPassImpl::OnDestroy() { if(mVkRenderPass) { @@ -62,6 +62,7 @@ void RenderPassImpl::Destroy() mVkRenderPass = nullptr; } + return true; } vk::RenderPass RenderPassImpl::GetVkHandle() diff --git a/dali/internal/graphics/vulkan-impl/vulkan-render-pass-impl.h b/dali/internal/graphics/vulkan-impl/vulkan-render-pass-impl.h index ade80a0b6..571cffc0a 100644 --- a/dali/internal/graphics/vulkan-impl/vulkan-render-pass-impl.h +++ b/dali/internal/graphics/vulkan-impl/vulkan-render-pass-impl.h @@ -18,6 +18,7 @@ */ #include +#include #include #include @@ -26,6 +27,9 @@ namespace Dali::Graphics::Vulkan class Device; class VulkanGraphicsController; class RenderTarget; +class RenderPassImpl; + +using RenderPassHandle = Handle; /** * Holder class for Vulkan RenderPass object. @@ -41,7 +45,7 @@ class RenderTarget; * FramebufferImpl will create a separate compatible RenderPassImpl if a matching * render pass is NOT found. */ -class RenderPassImpl +class RenderPassImpl : public VkSharedResource { public: struct CreateInfo @@ -54,16 +58,15 @@ public: vk::RenderPassCreateInfo createInfo; }; - static RenderPassImpl* New( - Vulkan::Device& device, - const std::vector& colorAttachments, - FramebufferAttachment* depthAttachment); + static RenderPassHandle New(Vulkan::Device& device, + const std::vector& colorAttachments, + FramebufferAttachment* depthAttachment); RenderPassImpl(Vulkan::Device& device, const std::vector& colorAttachments, FramebufferAttachment* depthAttachment); ~RenderPassImpl(); - void Destroy(); + bool OnDestroy() override; vk::RenderPass GetVkHandle(); @@ -87,6 +90,7 @@ private: vk::RenderPass mVkRenderPass; std::vector mAttachments{}; }; + } // namespace Dali::Graphics::Vulkan #endif // DALI_INTERNAL_GRAPHICS_VULKAN_RENDER_PASS_IMPL_H diff --git a/dali/internal/graphics/vulkan-impl/vulkan-render-target.cpp b/dali/internal/graphics/vulkan-impl/vulkan-render-target.cpp index 1dbd4c207..b69c3c475 100644 --- a/dali/internal/graphics/vulkan-impl/vulkan-render-target.cpp +++ b/dali/internal/graphics/vulkan-impl/vulkan-render-target.cpp @@ -87,7 +87,7 @@ Vulkan::FramebufferImpl* RenderTarget::GetCurrentFramebufferImpl() const return fbImpl; } -Vulkan::RenderPassImpl* RenderTarget::GetRenderPass(const Graphics::RenderPass* gfxRenderPass) const +Vulkan::RenderPassHandle RenderTarget::GetRenderPass(const Graphics::RenderPass* gfxRenderPass) const { auto renderPass = const_cast(static_cast(gfxRenderPass)); auto framebufferImpl = GetCurrentFramebufferImpl(); diff --git a/dali/internal/graphics/vulkan-impl/vulkan-render-target.h b/dali/internal/graphics/vulkan-impl/vulkan-render-target.h index 15dcb0889..f08983e8b 100644 --- a/dali/internal/graphics/vulkan-impl/vulkan-render-target.h +++ b/dali/internal/graphics/vulkan-impl/vulkan-render-target.h @@ -23,12 +23,12 @@ #include #include #include +#include namespace Dali::Graphics::Vulkan { class Framebuffer; class Surface; -class RenderPassImpl; using RenderTargetResource = Resource; @@ -99,11 +99,7 @@ public: * @param[in] renderPass A render pass to search for * @return a matching render pass implementation from the current framebuffer */ - [[nodiscard]] Vulkan::RenderPassImpl* GetRenderPass(const Graphics::RenderPass* renderPass) const; - // Get Swapchain? - -private: - // UniquePtr mSwapchain; + [[nodiscard]] Vulkan::RenderPassHandle GetRenderPass(const Graphics::RenderPass* renderPass) const; }; } // namespace Dali::Graphics::Vulkan diff --git a/dali/internal/graphics/vulkan-impl/vulkan-swapchain-impl.cpp b/dali/internal/graphics/vulkan-impl/vulkan-swapchain-impl.cpp index 87ea40c62..095132c2e 100644 --- a/dali/internal/graphics/vulkan-impl/vulkan-swapchain-impl.cpp +++ b/dali/internal/graphics/vulkan-impl/vulkan-swapchain-impl.cpp @@ -163,8 +163,7 @@ void Swapchain::CreateVkSwapchain( auto presentModes = surface->GetSurfacePresentModes(); auto found = std::find_if(presentModes.begin(), presentModes.end(), - [&](vk::PresentModeKHR mode) - { + [&](vk::PresentModeKHR mode) { return presentMode == mode; }); @@ -239,7 +238,7 @@ void Swapchain::CreateFramebuffers() // // CREATE FRAMEBUFFERS // - RenderPassImpl* compatibleRenderPass = nullptr; + RenderPassHandle compatibleRenderPass{}; for(auto&& image : images) { auto colorImage = mGraphicsDevice.CreateImageFromExternal(image, @@ -262,18 +261,15 @@ void Swapchain::CreateFramebuffers() depthAttachment, mSwapchainCreateInfoKHR.imageExtent.width, mSwapchainCreateInfoKHR.imageExtent.height), - [](FramebufferImpl* framebuffer1) - { + [](FramebufferImpl* framebuffer1) { framebuffer1->Destroy(); delete framebuffer1; }); mFramebuffers.push_back(std::move(framebuffer)); - if(compatibleRenderPass == nullptr) + if(!compatibleRenderPass) { // use common renderpass for all framebuffers. - // @todo Swapchain should own _these_ renderpasses, not framebuffer. - // @todo fix renderpass ownership model compatibleRenderPass = mFramebuffers.back()->GetRenderPass(0); } } diff --git a/dali/internal/graphics/vulkan-impl/vulkan-types.h b/dali/internal/graphics/vulkan-impl/vulkan-types.h index 1810932db..c4dd38219 100644 --- a/dali/internal/graphics/vulkan-impl/vulkan-types.h +++ b/dali/internal/graphics/vulkan-impl/vulkan-types.h @@ -192,59 +192,6 @@ struct VkStoreOpType vk::AttachmentStoreOp storeOp{vk::AttachmentStoreOp::eDontCare}; }; -class VkSharedResource // E.g. render pass -{ -public: - VkSharedResource() = default; - - virtual ~VkSharedResource() = default; - - void Release() - { - OnRelease(--mRefCount); - - if(mRefCount == 0) - { - // orphaned - if(!Destroy()) - { - delete this; - } - } - } - - void Retain() - { - OnRetain(++mRefCount); - } - - uint32_t GetRefCount() - { - return mRefCount; - } - - virtual bool Destroy() - { - return OnDestroy(); - } - - virtual void OnRetain(uint32_t refcount) - { - } - - virtual void OnRelease(uint32_t refcount) - { - } - - virtual bool OnDestroy() - { - return false; - } - -private: - std::atomic_uint mRefCount{0u}; -}; - } // namespace Vulkan } // namespace Dali::Graphics -- 2.34.1