From f49ad784ea8ca64ac5e384974643e8c3b6db51bd Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 28 Feb 2024 22:21:39 -0800 Subject: [PATCH] [Impeller] cache root render target and render pass. --- .../renderer/backend/vulkan/allocator_vk.cc | 1 + .../renderer/backend/vulkan/surface_vk.cc | 28 ++++++++----- .../backend/vulkan/swapchain_image_vk.cc | 13 ++++--- .../backend/vulkan/swapchain_image_vk.h | 12 ++++-- .../backend/vulkan/test/mock_vulkan.cc | 12 ++++++ .../vulkan/test/swapchain_unittests.cc | 39 +++++++++++++++++++ .../backend/vulkan/texture_source_vk.cc | 18 +++++++++ .../backend/vulkan/texture_source_vk.h | 26 +++++++++++++ .../renderer/backend/vulkan/texture_vk.cc | 8 ++-- impeller/renderer/backend/vulkan/texture_vk.h | 2 - 10 files changed, 133 insertions(+), 26 deletions(-) diff --git a/impeller/renderer/backend/vulkan/allocator_vk.cc b/impeller/renderer/backend/vulkan/allocator_vk.cc index 7147e3d9f16e6..360b126f835fc 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/impeller/renderer/backend/vulkan/allocator_vk.cc @@ -9,6 +9,7 @@ #include "flutter/fml/memory/ref_ptr.h" #include "flutter/fml/trace_event.h" #include "impeller/core/formats.h" +#include "impeller/core/texture_descriptor.h" #include "impeller/renderer/backend/vulkan/device_buffer_vk.h" #include "impeller/renderer/backend/vulkan/formats_vk.h" #include "impeller/renderer/backend/vulkan/texture_vk.h" diff --git a/impeller/renderer/backend/vulkan/surface_vk.cc b/impeller/renderer/backend/vulkan/surface_vk.cc index c5700a7783949..7df504dfa958f 100644 --- a/impeller/renderer/backend/vulkan/surface_vk.cc +++ b/impeller/renderer/backend/vulkan/surface_vk.cc @@ -7,6 +7,7 @@ #include "impeller/core/formats.h" #include "impeller/renderer/backend/vulkan/swapchain_image_vk.h" #include "impeller/renderer/backend/vulkan/texture_vk.h" +#include "impeller/renderer/render_target.h" #include "impeller/renderer/surface.h" namespace impeller { @@ -20,6 +21,12 @@ std::unique_ptr SurfaceVK::WrapSwapchainImage( return nullptr; } + if (swapchain_image->HasRenderTarget()) { + // The constructor is private. So make_unique may not be used. + return std::unique_ptr(new SurfaceVK( + swapchain_image->GetRenderTarget(), std::move(swap_callback))); + } + std::shared_ptr msaa_tex; if (enable_msaa) { TextureDescriptor msaa_tex_desc; @@ -30,16 +37,11 @@ std::unique_ptr SurfaceVK::WrapSwapchainImage( msaa_tex_desc.size = swapchain_image->GetSize(); msaa_tex_desc.usage = static_cast(TextureUsage::kRenderTarget); - if (!swapchain_image->HasMSAATexture()) { - msaa_tex = context->GetResourceAllocator()->CreateTexture(msaa_tex_desc); - msaa_tex->SetLabel("ImpellerOnscreenColorMSAA"); - if (!msaa_tex) { - VALIDATION_LOG << "Could not allocate MSAA color texture."; - return nullptr; - } - swapchain_image->SetMSAATexture(msaa_tex); - } else { - msaa_tex = swapchain_image->GetMSAATexture(); + msaa_tex = context->GetResourceAllocator()->CreateTexture(msaa_tex_desc); + msaa_tex->SetLabel("ImpellerOnscreenColorMSAA"); + if (!msaa_tex) { + VALIDATION_LOG << "Could not allocate MSAA color texture."; + return nullptr; } } @@ -78,6 +80,12 @@ std::unique_ptr SurfaceVK::WrapSwapchainImage( RenderTarget render_target_desc; render_target_desc.SetColorAttachment(color0, 0u); + render_target_desc.SetupDepthStencilAttachments( + *context, *context->GetResourceAllocator(), swapchain_image->GetSize(), + enable_msaa); + + swapchain_image->SetRenderTarget(render_target_desc); + // The constructor is private. So make_unique may not be used. return std::unique_ptr( new SurfaceVK(render_target_desc, std::move(swap_callback))); diff --git a/impeller/renderer/backend/vulkan/swapchain_image_vk.cc b/impeller/renderer/backend/vulkan/swapchain_image_vk.cc index c6fc383a4c2e9..6404f74abc3af 100644 --- a/impeller/renderer/backend/vulkan/swapchain_image_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_image_vk.cc @@ -35,16 +35,17 @@ bool SwapchainImageVK::IsValid() const { return is_valid_; } -std::shared_ptr SwapchainImageVK::GetMSAATexture() const { - return msaa_tex_; +RenderTarget SwapchainImageVK::GetRenderTarget() const { + return render_target_; } -bool SwapchainImageVK::HasMSAATexture() const { - return msaa_tex_ != nullptr; +bool SwapchainImageVK::HasRenderTarget() const { + return has_render_target_; } -void SwapchainImageVK::SetMSAATexture(std::shared_ptr msaa_tex) { - msaa_tex_ = std::move(msaa_tex); +void SwapchainImageVK::SetRenderTarget(const RenderTarget& render_target) { + has_render_target_ = true; + render_target_ = render_target; } PixelFormat SwapchainImageVK::GetPixelFormat() const { diff --git a/impeller/renderer/backend/vulkan/swapchain_image_vk.h b/impeller/renderer/backend/vulkan/swapchain_image_vk.h index 48673ebe4845d..d37ff1e1eaa19 100644 --- a/impeller/renderer/backend/vulkan/swapchain_image_vk.h +++ b/impeller/renderer/backend/vulkan/swapchain_image_vk.h @@ -9,6 +9,7 @@ #include "impeller/renderer/backend/vulkan/formats_vk.h" #include "impeller/renderer/backend/vulkan/texture_source_vk.h" #include "impeller/renderer/backend/vulkan/vk.h" +#include "impeller/renderer/render_target.h" #include "vulkan/vulkan_handles.hpp" namespace impeller { @@ -33,21 +34,24 @@ class SwapchainImageVK final : public TextureSourceVK { std::shared_ptr GetMSAATexture() const; - bool HasMSAATexture() const; + RenderTarget GetRenderTarget() const; + + bool HasRenderTarget() const; + + void SetRenderTarget(const RenderTarget& render_target); // |TextureSourceVK| vk::ImageView GetImageView() const override; vk::ImageView GetRenderTargetView() const override; - void SetMSAATexture(std::shared_ptr msaa_tex); - bool IsSwapchainImage() const override { return true; } private: vk::Image image_ = VK_NULL_HANDLE; vk::UniqueImageView image_view_ = {}; - std::shared_ptr msaa_tex_; + RenderTarget render_target_ = {}; + bool has_render_target_ = false; bool is_valid_ = false; SwapchainImageVK(const SwapchainImageVK&) = delete; diff --git a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc index 72a36804bda88..8ca18c33ebd62 100644 --- a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc +++ b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc @@ -40,6 +40,8 @@ struct MockSwapchainKHR {}; struct MockImage {}; +struct MockFramebuffer {}; + struct MockSemaphore {}; static ISize currentImageSize = ISize{1, 1}; @@ -686,6 +688,14 @@ VkResult vkAcquireNextImageKHR(VkDevice device, return VK_SUCCESS; } +VkResult vkCreateFramebuffer(VkDevice device, + const VkFramebufferCreateInfo* pCreateInfo, + const VkAllocationCallbacks* pAllocator, + VkFramebuffer* pFramebuffer) { + *pFramebuffer = reinterpret_cast(new MockFramebuffer()); + return VK_SUCCESS; +} + PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance, const char* pName) { if (strcmp("vkEnumerateInstanceExtensionProperties", pName) == 0) { @@ -814,6 +824,8 @@ PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance, return (PFN_vkVoidFunction)vkDestroySurfaceKHR; } else if (strcmp("vkAcquireNextImageKHR", pName) == 0) { return (PFN_vkVoidFunction)vkAcquireNextImageKHR; + } else if (strcmp("vkCreateFramebuffer", pName) == 0) { + return (PFN_vkVoidFunction)vkCreateFramebuffer; } return noop; } diff --git a/impeller/renderer/backend/vulkan/test/swapchain_unittests.cc b/impeller/renderer/backend/vulkan/test/swapchain_unittests.cc index a78d3187b58df..f0af23ed75b46 100644 --- a/impeller/renderer/backend/vulkan/test/swapchain_unittests.cc +++ b/impeller/renderer/backend/vulkan/test/swapchain_unittests.cc @@ -6,6 +6,7 @@ #include "gtest/gtest.h" #include "impeller/renderer/backend/vulkan/swapchain_vk.h" #include "impeller/renderer/backend/vulkan/test/mock_vulkan.h" +#include "impeller/renderer/backend/vulkan/texture_vk.h" #include "vulkan/vulkan_enums.hpp" namespace impeller { @@ -52,5 +53,43 @@ TEST(SwapchainTest, RecreateSwapchainWhenSizeChanges) { EXPECT_EQ(image_b->GetSize(), expected_size); } +TEST(SwapchainTest, CachesRenderPassOnSwapchainImage) { + auto const context = MockVulkanContextBuilder().Build(); + + auto surface = CreateSurface(*context); + auto swapchain = + SwapchainVK::Create(context, std::move(surface), ISize{1, 1}); + + EXPECT_TRUE(swapchain->IsValid()); + + // We should create 3 swapchain images with the current mock setup. However, + // we will only ever return the first image, so the render pass and + // framebuffer will be cached after one call to AcquireNextDrawable. + auto drawable = swapchain->AcquireNextDrawable(); + RenderTarget render_target = drawable->GetTargetRenderPassDescriptor(); + + auto texture = render_target.GetRenderTargetTexture(); + auto& texture_vk = TextureVK::Cast(*texture); + EXPECT_EQ(texture_vk.GetFramebuffer(), nullptr); + EXPECT_EQ(texture_vk.GetRenderPass(), nullptr); + + auto command_buffer = context->CreateCommandBuffer(); + auto render_pass = command_buffer->CreateRenderPass(render_target); + + render_pass->EncodeCommands(); + + EXPECT_NE(texture_vk.GetFramebuffer(), nullptr); + EXPECT_NE(texture_vk.GetRenderPass(), nullptr); + + { + auto drawable = swapchain->AcquireNextDrawable(); + auto texture = render_target.GetRenderTargetTexture(); + auto& texture_vk = TextureVK::Cast(*texture); + + EXPECT_NE(texture_vk.GetFramebuffer(), nullptr); + EXPECT_NE(texture_vk.GetRenderPass(), nullptr); + } +} + } // namespace testing } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/texture_source_vk.cc b/impeller/renderer/backend/vulkan/texture_source_vk.cc index 4a39bce151bd8..f200bc5778aaf 100644 --- a/impeller/renderer/backend/vulkan/texture_source_vk.cc +++ b/impeller/renderer/backend/vulkan/texture_source_vk.cc @@ -58,4 +58,22 @@ fml::Status TextureSourceVK::SetLayout(const BarrierVK& barrier) const { return {}; } +void TextureSourceVK::SetFramebuffer( + const SharedHandleVK& framebuffer) { + framebuffer_ = framebuffer; +} + +void TextureSourceVK::SetRenderPass( + const SharedHandleVK& render_pass) { + render_pass_ = render_pass; +} + +SharedHandleVK TextureSourceVK::GetFramebuffer() const { + return framebuffer_; +} + +SharedHandleVK TextureSourceVK::GetRenderPass() const { + return render_pass_; +} + } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/texture_source_vk.h b/impeller/renderer/backend/vulkan/texture_source_vk.h index 4f76a8067cf4f..9dd90df7cf67d 100644 --- a/impeller/renderer/backend/vulkan/texture_source_vk.h +++ b/impeller/renderer/backend/vulkan/texture_source_vk.h @@ -65,12 +65,38 @@ class TextureSourceVK { /// Whether or not this is a swapchain image. virtual bool IsSwapchainImage() const = 0; + /// Store the last framebuffer object used with this texture. + /// + /// This field is only set if this texture is used as the resolve texture + /// of a render pass. By construction, this framebuffer should be compatible + /// with any future render passes. + void SetFramebuffer(const SharedHandleVK& framebuffer); + + /// Store the last render pass object used with this texture. + /// + /// This field is only set if this texture is used as the resolve texture + /// of a render pass. By construction, this framebuffer should be compatible + /// with any future render passes. + void SetRenderPass(const SharedHandleVK& render_pass); + + /// Retrieve the last framebuffer object used with this texture. + /// + /// May be nullptr if no previous framebuffer existed. + SharedHandleVK GetFramebuffer() const; + + /// Retrieve the last render pass object used with this texture. + /// + /// May be nullptr if no previous render pass existed. + SharedHandleVK GetRenderPass() const; + protected: const TextureDescriptor desc_; explicit TextureSourceVK(TextureDescriptor desc); private: + SharedHandleVK framebuffer_; + SharedHandleVK render_pass_; mutable RWMutex layout_mutex_; mutable vk::ImageLayout layout_ IPLR_GUARDED_BY(layout_mutex_) = vk::ImageLayout::eUndefined; diff --git a/impeller/renderer/backend/vulkan/texture_vk.cc b/impeller/renderer/backend/vulkan/texture_vk.cc index b105d31b96e85..cd3d93ff7fb0d 100644 --- a/impeller/renderer/backend/vulkan/texture_vk.cc +++ b/impeller/renderer/backend/vulkan/texture_vk.cc @@ -175,20 +175,20 @@ vk::ImageView TextureVK::GetRenderTargetView() const { void TextureVK::SetFramebuffer( const SharedHandleVK& framebuffer) { - framebuffer_ = framebuffer; + source_->SetFramebuffer(framebuffer); } void TextureVK::SetRenderPass( const SharedHandleVK& render_pass) { - render_pass_ = render_pass; + source_->SetRenderPass(render_pass); } SharedHandleVK TextureVK::GetFramebuffer() const { - return framebuffer_; + return source_->GetFramebuffer(); } SharedHandleVK TextureVK::GetRenderPass() const { - return render_pass_; + return source_->GetRenderPass(); } } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/texture_vk.h b/impeller/renderer/backend/vulkan/texture_vk.h index 5826e3df78174..7a8be812eab31 100644 --- a/impeller/renderer/backend/vulkan/texture_vk.h +++ b/impeller/renderer/backend/vulkan/texture_vk.h @@ -73,8 +73,6 @@ class TextureVK final : public Texture, public BackendCast { private: std::weak_ptr context_; std::shared_ptr source_; - SharedHandleVK framebuffer_ = nullptr; - SharedHandleVK render_pass_ = nullptr; // |Texture| void SetLabel(std::string_view label) override;