From d9b46ebfb1802d31a91dc3163d8559ea198ebc22 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 5 Mar 2024 09:25:45 -0800 Subject: [PATCH 1/9] [Impeller] more efficient use of transient onscreen attachments. --- .../renderer/backend/vulkan/surface_vk.cc | 7 +++-- .../backend/vulkan/swapchain_image_vk.cc | 15 ++++++--- .../backend/vulkan/swapchain_image_vk.h | 9 ++++-- .../backend/vulkan/swapchain_impl_vk.cc | 31 +++++++++++++++++++ impeller/renderer/render_target.cc | 16 +++++++--- impeller/renderer/render_target.h | 12 ++----- 6 files changed, 65 insertions(+), 25 deletions(-) diff --git a/impeller/renderer/backend/vulkan/surface_vk.cc b/impeller/renderer/backend/vulkan/surface_vk.cc index c5700a7783949..949da581be0f2 100644 --- a/impeller/renderer/backend/vulkan/surface_vk.cc +++ b/impeller/renderer/backend/vulkan/surface_vk.cc @@ -30,14 +30,13 @@ std::unique_ptr SurfaceVK::WrapSwapchainImage( msaa_tex_desc.size = swapchain_image->GetSize(); msaa_tex_desc.usage = static_cast(TextureUsage::kRenderTarget); - if (!swapchain_image->HasMSAATexture()) { + if (!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; } - swapchain_image->SetMSAATexture(msaa_tex); } else { msaa_tex = swapchain_image->GetMSAATexture(); } @@ -77,6 +76,10 @@ 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, "Onscreen", RenderTarget::kDefaultStencilAttachmentConfig, + swapchain_image->GetDepthStencilTexture()); // The constructor is private. So make_unique may not be used. return std::unique_ptr( diff --git a/impeller/renderer/backend/vulkan/swapchain_image_vk.cc b/impeller/renderer/backend/vulkan/swapchain_image_vk.cc index c6fc383a4c2e9..fc749b0ba72e3 100644 --- a/impeller/renderer/backend/vulkan/swapchain_image_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_image_vk.cc @@ -36,15 +36,20 @@ bool SwapchainImageVK::IsValid() const { } std::shared_ptr SwapchainImageVK::GetMSAATexture() const { - return msaa_tex_; + return msaa_texture_; } -bool SwapchainImageVK::HasMSAATexture() const { - return msaa_tex_ != nullptr; +std::shared_ptr SwapchainImageVK::GetDepthStencilTexture() const { + return depth_stencil_texture_; } -void SwapchainImageVK::SetMSAATexture(std::shared_ptr msaa_tex) { - msaa_tex_ = std::move(msaa_tex); +void SwapchainImageVK::SetMSAATexture(std::shared_ptr texture) { + msaa_texture_ = std::move(texture); +} + +void SwapchainImageVK::SetDepthStencilTexture( + std::shared_ptr texture) { + depth_stencil_texture_ = std::move(texture); } 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..026f1cda7b8c2 100644 --- a/impeller/renderer/backend/vulkan/swapchain_image_vk.h +++ b/impeller/renderer/backend/vulkan/swapchain_image_vk.h @@ -33,21 +33,24 @@ class SwapchainImageVK final : public TextureSourceVK { std::shared_ptr GetMSAATexture() const; - bool HasMSAATexture() const; + std::shared_ptr GetDepthStencilTexture() const; // |TextureSourceVK| vk::ImageView GetImageView() const override; vk::ImageView GetRenderTargetView() const override; - void SetMSAATexture(std::shared_ptr msaa_tex); + void SetMSAATexture(std::shared_ptr texture); + + void SetDepthStencilTexture(std::shared_ptr texture); bool IsSwapchainImage() const override { return true; } private: vk::Image image_ = VK_NULL_HANDLE; vk::UniqueImageView image_view_ = {}; - std::shared_ptr msaa_tex_; + std::shared_ptr msaa_texture_; + std::shared_ptr depth_stencil_texture_; bool is_valid_ = false; SwapchainImageVK(const SwapchainImageVK&) = delete; diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 76411732c004a..6538e28ea3364 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -228,6 +228,35 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, texture_desc.size = ISize::MakeWH(swapchain_info.imageExtent.width, swapchain_info.imageExtent.height); + // Allocate a single onscreen MSAA texture and Depth+Stencil Texture to + // be shared by all swapchain images. + TextureDescriptor msaa_desc; + msaa_desc.storage_mode = StorageMode::kDeviceTransient; + msaa_desc.type = TextureType::kTexture2DMultisample; + msaa_desc.sample_count = SampleCount::kCount4; + msaa_desc.format = texture_desc.format; + msaa_desc.size = texture_desc.size; + msaa_desc.usage = static_cast(TextureUsage::kRenderTarget); + + TextureDescriptor depth_stencil_desc; + depth_stencil_desc.storage_mode = StorageMode::kDeviceTransient; + depth_stencil_desc.type = TextureType::kTexture2DMultisample; + depth_stencil_desc.sample_count = SampleCount::kCount4; + depth_stencil_desc.format = + context->GetCapabilities()->GetDefaultDepthStencilFormat(); + depth_stencil_desc.size = texture_desc.size; + depth_stencil_desc.usage = static_cast(TextureUsage::kRenderTarget); + + std::shared_ptr msaa_texture = + context->GetResourceAllocator()->CreateTexture(msaa_desc); + std::shared_ptr depth_stencil_texture = + context->GetResourceAllocator()->CreateTexture(depth_stencil_desc); + if (!msaa_texture || !depth_stencil_texture) { + VALIDATION_LOG + << "Failed to allocate onscreen MSAA or depth+stencil texture."; + return; + } + std::vector> swapchain_images; for (const auto& image : images) { auto swapchain_image = @@ -239,6 +268,8 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, VALIDATION_LOG << "Could not create swapchain image."; return; } + swapchain_image->SetMSAATexture(msaa_texture); + swapchain_image->SetDepthStencilTexture(depth_stencil_texture); ContextVK::SetDebugName( vk_context.GetDevice(), swapchain_image->GetImage(), diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc index 9cab7db94db69..8985e7609b4d9 100644 --- a/impeller/renderer/render_target.cc +++ b/impeller/renderer/render_target.cc @@ -398,7 +398,8 @@ void RenderTarget::SetupDepthStencilAttachments( ISize size, bool msaa, const std::string& label, - RenderTarget::AttachmentConfig stencil_attachment_config) { + RenderTarget::AttachmentConfig stencil_attachment_config, + const std::shared_ptr& depth_stencil_texture) { TextureDescriptor depth_stencil_texture_desc; depth_stencil_texture_desc.storage_mode = stencil_attachment_config.storage_mode; @@ -412,8 +413,13 @@ void RenderTarget::SetupDepthStencilAttachments( depth_stencil_texture_desc.usage = static_cast(TextureUsage::kRenderTarget); - auto depth_stencil_texture = - allocator.CreateTexture(depth_stencil_texture_desc); + std::shared_ptr new_depth_stencil_texture = + depth_stencil_texture == nullptr + ? allocator.CreateTexture(depth_stencil_texture_desc) + : depth_stencil_texture; + FML_DCHECK(depth_stencil_texture == nullptr || + depth_stencil_texture->GetTextureDescriptor() == + depth_stencil_texture_desc); if (!depth_stencil_texture) { return; // Error messages are handled by `Allocator::CreateTexture`. } @@ -422,13 +428,13 @@ void RenderTarget::SetupDepthStencilAttachments( depth0.load_action = stencil_attachment_config.load_action; depth0.store_action = stencil_attachment_config.store_action; depth0.clear_depth = 0u; - depth0.texture = depth_stencil_texture; + depth0.texture = new_depth_stencil_texture; StencilAttachment stencil0; stencil0.load_action = stencil_attachment_config.load_action; stencil0.store_action = stencil_attachment_config.store_action; stencil0.clear_stencil = 0u; - stencil0.texture = std::move(depth_stencil_texture); + stencil0.texture = std::move(new_depth_stencil_texture); stencil0.texture->SetLabel( SPrintF("%s Depth+Stencil Texture", label.c_str())); diff --git a/impeller/renderer/render_target.h b/impeller/renderer/render_target.h index 0bfce5137c8fa..2a1accc6b653e 100644 --- a/impeller/renderer/render_target.h +++ b/impeller/renderer/render_target.h @@ -84,7 +84,8 @@ class RenderTarget final { bool msaa, const std::string& label = "Offscreen", RenderTarget::AttachmentConfig stencil_attachment_config = - RenderTarget::kDefaultStencilAttachmentConfig); + RenderTarget::kDefaultStencilAttachmentConfig, + const std::shared_ptr& depth_stencil_texture = nullptr); SampleCount GetSampleCount() const; @@ -176,15 +177,6 @@ class RenderTargetAllocator { virtual void End(); private: - void SetupDepthStencilAttachments( - Allocator& allocator, - const Context& context, - ISize size, - bool msaa, - const std::string& label = "Offscreen", - RenderTarget::AttachmentConfig stencil_attachment_config = - RenderTarget::kDefaultStencilAttachmentConfig); - std::shared_ptr allocator_; }; From 2819e3652dc0106536156ab35015b9177d52633c Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 5 Mar 2024 09:25:45 -0800 Subject: [PATCH 2/9] [Impeller] more efficient use of transient onscreen attachments. --- .../renderer/backend/vulkan/surface_vk.cc | 7 +++-- impeller/renderer/backend/vulkan/surface_vk.h | 1 - .../backend/vulkan/swapchain_image_vk.cc | 15 ++++++--- .../backend/vulkan/swapchain_image_vk.h | 9 ++++-- .../backend/vulkan/swapchain_impl_vk.cc | 31 +++++++++++++++++++ impeller/renderer/render_target.cc | 18 +++++++---- impeller/renderer/render_target.h | 12 ++----- 7 files changed, 66 insertions(+), 27 deletions(-) diff --git a/impeller/renderer/backend/vulkan/surface_vk.cc b/impeller/renderer/backend/vulkan/surface_vk.cc index c5700a7783949..949da581be0f2 100644 --- a/impeller/renderer/backend/vulkan/surface_vk.cc +++ b/impeller/renderer/backend/vulkan/surface_vk.cc @@ -30,14 +30,13 @@ std::unique_ptr SurfaceVK::WrapSwapchainImage( msaa_tex_desc.size = swapchain_image->GetSize(); msaa_tex_desc.usage = static_cast(TextureUsage::kRenderTarget); - if (!swapchain_image->HasMSAATexture()) { + if (!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; } - swapchain_image->SetMSAATexture(msaa_tex); } else { msaa_tex = swapchain_image->GetMSAATexture(); } @@ -77,6 +76,10 @@ 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, "Onscreen", RenderTarget::kDefaultStencilAttachmentConfig, + swapchain_image->GetDepthStencilTexture()); // The constructor is private. So make_unique may not be used. return std::unique_ptr( diff --git a/impeller/renderer/backend/vulkan/surface_vk.h b/impeller/renderer/backend/vulkan/surface_vk.h index ea4203135324c..73bbf2b7319f4 100644 --- a/impeller/renderer/backend/vulkan/surface_vk.h +++ b/impeller/renderer/backend/vulkan/surface_vk.h @@ -7,7 +7,6 @@ #include -#include "flutter/fml/macros.h" #include "impeller/renderer/backend/vulkan/context_vk.h" #include "impeller/renderer/backend/vulkan/swapchain_image_vk.h" #include "impeller/renderer/surface.h" diff --git a/impeller/renderer/backend/vulkan/swapchain_image_vk.cc b/impeller/renderer/backend/vulkan/swapchain_image_vk.cc index c6fc383a4c2e9..fc749b0ba72e3 100644 --- a/impeller/renderer/backend/vulkan/swapchain_image_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_image_vk.cc @@ -36,15 +36,20 @@ bool SwapchainImageVK::IsValid() const { } std::shared_ptr SwapchainImageVK::GetMSAATexture() const { - return msaa_tex_; + return msaa_texture_; } -bool SwapchainImageVK::HasMSAATexture() const { - return msaa_tex_ != nullptr; +std::shared_ptr SwapchainImageVK::GetDepthStencilTexture() const { + return depth_stencil_texture_; } -void SwapchainImageVK::SetMSAATexture(std::shared_ptr msaa_tex) { - msaa_tex_ = std::move(msaa_tex); +void SwapchainImageVK::SetMSAATexture(std::shared_ptr texture) { + msaa_texture_ = std::move(texture); +} + +void SwapchainImageVK::SetDepthStencilTexture( + std::shared_ptr texture) { + depth_stencil_texture_ = std::move(texture); } 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..026f1cda7b8c2 100644 --- a/impeller/renderer/backend/vulkan/swapchain_image_vk.h +++ b/impeller/renderer/backend/vulkan/swapchain_image_vk.h @@ -33,21 +33,24 @@ class SwapchainImageVK final : public TextureSourceVK { std::shared_ptr GetMSAATexture() const; - bool HasMSAATexture() const; + std::shared_ptr GetDepthStencilTexture() const; // |TextureSourceVK| vk::ImageView GetImageView() const override; vk::ImageView GetRenderTargetView() const override; - void SetMSAATexture(std::shared_ptr msaa_tex); + void SetMSAATexture(std::shared_ptr texture); + + void SetDepthStencilTexture(std::shared_ptr texture); bool IsSwapchainImage() const override { return true; } private: vk::Image image_ = VK_NULL_HANDLE; vk::UniqueImageView image_view_ = {}; - std::shared_ptr msaa_tex_; + std::shared_ptr msaa_texture_; + std::shared_ptr depth_stencil_texture_; bool is_valid_ = false; SwapchainImageVK(const SwapchainImageVK&) = delete; diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 76411732c004a..6538e28ea3364 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -228,6 +228,35 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, texture_desc.size = ISize::MakeWH(swapchain_info.imageExtent.width, swapchain_info.imageExtent.height); + // Allocate a single onscreen MSAA texture and Depth+Stencil Texture to + // be shared by all swapchain images. + TextureDescriptor msaa_desc; + msaa_desc.storage_mode = StorageMode::kDeviceTransient; + msaa_desc.type = TextureType::kTexture2DMultisample; + msaa_desc.sample_count = SampleCount::kCount4; + msaa_desc.format = texture_desc.format; + msaa_desc.size = texture_desc.size; + msaa_desc.usage = static_cast(TextureUsage::kRenderTarget); + + TextureDescriptor depth_stencil_desc; + depth_stencil_desc.storage_mode = StorageMode::kDeviceTransient; + depth_stencil_desc.type = TextureType::kTexture2DMultisample; + depth_stencil_desc.sample_count = SampleCount::kCount4; + depth_stencil_desc.format = + context->GetCapabilities()->GetDefaultDepthStencilFormat(); + depth_stencil_desc.size = texture_desc.size; + depth_stencil_desc.usage = static_cast(TextureUsage::kRenderTarget); + + std::shared_ptr msaa_texture = + context->GetResourceAllocator()->CreateTexture(msaa_desc); + std::shared_ptr depth_stencil_texture = + context->GetResourceAllocator()->CreateTexture(depth_stencil_desc); + if (!msaa_texture || !depth_stencil_texture) { + VALIDATION_LOG + << "Failed to allocate onscreen MSAA or depth+stencil texture."; + return; + } + std::vector> swapchain_images; for (const auto& image : images) { auto swapchain_image = @@ -239,6 +268,8 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, VALIDATION_LOG << "Could not create swapchain image."; return; } + swapchain_image->SetMSAATexture(msaa_texture); + swapchain_image->SetDepthStencilTexture(depth_stencil_texture); ContextVK::SetDebugName( vk_context.GetDevice(), swapchain_image->GetImage(), diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc index 9cab7db94db69..f14609fa077b6 100644 --- a/impeller/renderer/render_target.cc +++ b/impeller/renderer/render_target.cc @@ -398,7 +398,8 @@ void RenderTarget::SetupDepthStencilAttachments( ISize size, bool msaa, const std::string& label, - RenderTarget::AttachmentConfig stencil_attachment_config) { + RenderTarget::AttachmentConfig stencil_attachment_config, + const std::shared_ptr& depth_stencil_texture) { TextureDescriptor depth_stencil_texture_desc; depth_stencil_texture_desc.storage_mode = stencil_attachment_config.storage_mode; @@ -412,9 +413,14 @@ void RenderTarget::SetupDepthStencilAttachments( depth_stencil_texture_desc.usage = static_cast(TextureUsage::kRenderTarget); - auto depth_stencil_texture = - allocator.CreateTexture(depth_stencil_texture_desc); - if (!depth_stencil_texture) { + std::shared_ptr new_depth_stencil_texture = + depth_stencil_texture == nullptr + ? allocator.CreateTexture(depth_stencil_texture_desc) + : depth_stencil_texture; + FML_DCHECK(depth_stencil_texture == nullptr || + depth_stencil_texture->GetTextureDescriptor() == + depth_stencil_texture_desc); + if (!new_depth_stencil_texture) { return; // Error messages are handled by `Allocator::CreateTexture`. } @@ -422,13 +428,13 @@ void RenderTarget::SetupDepthStencilAttachments( depth0.load_action = stencil_attachment_config.load_action; depth0.store_action = stencil_attachment_config.store_action; depth0.clear_depth = 0u; - depth0.texture = depth_stencil_texture; + depth0.texture = new_depth_stencil_texture; StencilAttachment stencil0; stencil0.load_action = stencil_attachment_config.load_action; stencil0.store_action = stencil_attachment_config.store_action; stencil0.clear_stencil = 0u; - stencil0.texture = std::move(depth_stencil_texture); + stencil0.texture = std::move(new_depth_stencil_texture); stencil0.texture->SetLabel( SPrintF("%s Depth+Stencil Texture", label.c_str())); diff --git a/impeller/renderer/render_target.h b/impeller/renderer/render_target.h index 0bfce5137c8fa..2a1accc6b653e 100644 --- a/impeller/renderer/render_target.h +++ b/impeller/renderer/render_target.h @@ -84,7 +84,8 @@ class RenderTarget final { bool msaa, const std::string& label = "Offscreen", RenderTarget::AttachmentConfig stencil_attachment_config = - RenderTarget::kDefaultStencilAttachmentConfig); + RenderTarget::kDefaultStencilAttachmentConfig, + const std::shared_ptr& depth_stencil_texture = nullptr); SampleCount GetSampleCount() const; @@ -176,15 +177,6 @@ class RenderTargetAllocator { virtual void End(); private: - void SetupDepthStencilAttachments( - Allocator& allocator, - const Context& context, - ISize size, - bool msaa, - const std::string& label = "Offscreen", - RenderTarget::AttachmentConfig stencil_attachment_config = - RenderTarget::kDefaultStencilAttachmentConfig); - std::shared_ptr allocator_; }; From 143711475b59625c0ca23bbc02d7fefcf2fc2b8d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 5 Mar 2024 11:49:50 -0800 Subject: [PATCH 3/9] fold in RP caching. --- .../vulkan/test/swapchain_unittests.cc | 39 +++++++++++++++++++ .../backend/vulkan/texture_source_vk.cc | 18 +++++++++ .../backend/vulkan/texture_source_vk.h | 28 +++++++++++++ .../renderer/backend/vulkan/texture_vk.cc | 8 ++-- impeller/renderer/backend/vulkan/texture_vk.h | 2 - 5 files changed, 89 insertions(+), 6 deletions(-) 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..eecb3bf9d88a0 100644 --- a/impeller/renderer/backend/vulkan/texture_source_vk.h +++ b/impeller/renderer/backend/vulkan/texture_source_vk.h @@ -65,12 +65,40 @@ class TextureSourceVK { /// Whether or not this is a swapchain image. virtual bool IsSwapchainImage() const = 0; + // These methods should only be used by render_pass_vk.h + + /// 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; From 743899cbe8c0f148fe45141f18eef0560fba8526 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 5 Mar 2024 12:07:47 -0800 Subject: [PATCH 4/9] update mock vulkan. --- .../backend/vulkan/test/mock_vulkan.cc | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc index 4c590620701fb..da5031b45615d 100644 --- a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc +++ b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc @@ -44,6 +44,8 @@ struct MockSwapchainKHR { struct MockSemaphore {}; +struct MockFramebuffer {}; + static ISize currentImageSize = ISize{1, 1}; class MockDevice final { @@ -719,6 +721,20 @@ 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; +} + +void vkDestroyFramebuffer(VkDevice device, + VkFramebuffer framebuffer, + const VkAllocationCallbacks* pAllocator) { + delete reinterpret_cast(framebuffer); +} + PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance, const char* pName) { if (strcmp("vkEnumerateInstanceExtensionProperties", pName) == 0) { @@ -855,6 +871,10 @@ 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; + } else if (strcmp("vkDestroyFramebuffer", pName) == 0) { + return (PFN_vkVoidFunction)vkDestroyFramebuffer; } return noop; } From 4432aa124b3123d3a4622f134c518d35c746e1b3 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 5 Mar 2024 13:35:37 -0800 Subject: [PATCH 5/9] ++ --- .../backend/vulkan/swapchain_impl_vk.cc | 21 +++++++++++-------- impeller/renderer/render_target.cc | 1 + 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 6538e28ea3364..dfa43ff75a32d 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -6,6 +6,7 @@ #include "fml/synchronization/semaphore.h" #include "impeller/base/validation.h" +#include "impeller/core/formats.h" #include "impeller/renderer/backend/vulkan/command_buffer_vk.h" #include "impeller/renderer/backend/vulkan/command_encoder_vk.h" #include "impeller/renderer/backend/vulkan/context_vk.h" @@ -240,22 +241,24 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, TextureDescriptor depth_stencil_desc; depth_stencil_desc.storage_mode = StorageMode::kDeviceTransient; - depth_stencil_desc.type = TextureType::kTexture2DMultisample; - depth_stencil_desc.sample_count = SampleCount::kCount4; + if (enable_msaa) { + depth_stencil_desc.type = TextureType::kTexture2DMultisample; + depth_stencil_desc.sample_count = SampleCount::kCount4; + } else { + depth_stencil_desc.type = TextureType::kTexture2D; + depth_stencil_desc.sample_count = SampleCount::kCount1; + } depth_stencil_desc.format = context->GetCapabilities()->GetDefaultDepthStencilFormat(); depth_stencil_desc.size = texture_desc.size; depth_stencil_desc.usage = static_cast(TextureUsage::kRenderTarget); - std::shared_ptr msaa_texture = - context->GetResourceAllocator()->CreateTexture(msaa_desc); + std::shared_ptr msaa_texture; + if (enable_msaa) { + context->GetResourceAllocator()->CreateTexture(msaa_desc); + } std::shared_ptr depth_stencil_texture = context->GetResourceAllocator()->CreateTexture(depth_stencil_desc); - if (!msaa_texture || !depth_stencil_texture) { - VALIDATION_LOG - << "Failed to allocate onscreen MSAA or depth+stencil texture."; - return; - } std::vector> swapchain_images; for (const auto& image : images) { diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc index f14609fa077b6..7f4992b14e66a 100644 --- a/impeller/renderer/render_target.cc +++ b/impeller/renderer/render_target.cc @@ -11,6 +11,7 @@ #include "impeller/core/allocator.h" #include "impeller/core/formats.h" #include "impeller/core/texture.h" +#include "impeller/core/texture_descriptor.h" #include "impeller/renderer/context.h" namespace impeller { From 634bee052780c6203d036cc9ceef610e05a9819e Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 6 Mar 2024 12:38:07 -0800 Subject: [PATCH 6/9] merge fix. --- impeller/renderer/backend/vulkan/test/mock_vulkan.cc | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc index 358912403ccda..2c6f47462d753 100644 --- a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc +++ b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc @@ -44,11 +44,9 @@ struct MockSwapchainKHR { struct MockSemaphore {}; -<<<<<<< HEAD struct MockFramebuffer {}; -======= + struct MockBuffer {}; ->>>>>>> 5bbac1a5c5762e93c130c08123e0e856efc73996 static ISize currentImageSize = ISize{1, 1}; @@ -739,7 +737,6 @@ VkResult vkAcquireNextImageKHR(VkDevice device, return VK_SUCCESS; } -<<<<<<< HEAD VkResult vkCreateFramebuffer(VkDevice device, const VkFramebufferCreateInfo* pCreateInfo, const VkAllocationCallbacks* pAllocator, @@ -752,7 +749,8 @@ void vkDestroyFramebuffer(VkDevice device, VkFramebuffer framebuffer, const VkAllocationCallbacks* pAllocator) { delete reinterpret_cast(framebuffer); -======= +} + VkResult vkFlushMappedMemoryRanges(VkDevice device, uint32_t memoryRangeCount, const VkMappedMemoryRange* pMemoryRanges) { @@ -770,7 +768,6 @@ VkResult vkMapMemory(VkDevice device, MockDevice* mock_device = reinterpret_cast(device); mock_device->AddCalledFunction("vkMapMemory"); return VK_SUCCESS; ->>>>>>> 5bbac1a5c5762e93c130c08123e0e856efc73996 } PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance, @@ -909,19 +906,16 @@ PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance, return (PFN_vkVoidFunction)vkDestroySurfaceKHR; } else if (strcmp("vkAcquireNextImageKHR", pName) == 0) { return (PFN_vkVoidFunction)vkAcquireNextImageKHR; -<<<<<<< HEAD } else if (strcmp("vkCreateFramebuffer", pName) == 0) { return (PFN_vkVoidFunction)vkCreateFramebuffer; } else if (strcmp("vkDestroyFramebuffer", pName) == 0) { return (PFN_vkVoidFunction)vkDestroyFramebuffer; -======= } else if (strcmp("vkFlushMappedMemoryRanges", pName) == 0) { return (PFN_vkVoidFunction)vkFlushMappedMemoryRanges; } else if (strcmp("vkDestroyBuffer", pName) == 0) { return (PFN_vkVoidFunction)vkDestroyBuffer; } else if (strcmp("vkMapMemory", pName) == 0) { return (PFN_vkVoidFunction)vkMapMemory; ->>>>>>> 5bbac1a5c5762e93c130c08123e0e856efc73996 } return noop; } From 0c5743d315d09e4e3a064c5d739ca0826502492e Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 6 Mar 2024 15:41:55 -0800 Subject: [PATCH 7/9] ++ --- impeller/renderer/backend/vulkan/swapchain_impl_vk.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index dfa43ff75a32d..61fea57025b70 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -255,7 +255,7 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, std::shared_ptr msaa_texture; if (enable_msaa) { - context->GetResourceAllocator()->CreateTexture(msaa_desc); + msaa_texture = context->GetResourceAllocator()->CreateTexture(msaa_desc); } std::shared_ptr depth_stencil_texture = context->GetResourceAllocator()->CreateTexture(depth_stencil_desc); From e7b3a1b632a0f69f4a7362c27dd35403a54ae55e Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 7 Mar 2024 09:14:34 -0800 Subject: [PATCH 8/9] matanl review. --- .../vulkan/render_pass_cache_unittests.cc | 8 +-- .../renderer/backend/vulkan/render_pass_vk.cc | 10 +-- .../renderer/backend/vulkan/surface_vk.cc | 12 +++- impeller/renderer/backend/vulkan/surface_vk.h | 5 ++ .../backend/vulkan/swapchain_impl_vk.cc | 3 + .../backend/vulkan/test/mock_vulkan.cc | 6 +- .../vulkan/test/swapchain_unittests.cc | 67 ++++++++++++++----- .../backend/vulkan/texture_source_vk.cc | 8 +-- .../backend/vulkan/texture_source_vk.h | 8 +-- .../renderer/backend/vulkan/texture_vk.cc | 16 ++--- impeller/renderer/backend/vulkan/texture_vk.h | 8 +-- 11 files changed, 101 insertions(+), 50 deletions(-) diff --git a/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc b/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc index 21b8bd9d05505..055c91a3ccb10 100644 --- a/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc +++ b/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc @@ -27,14 +27,14 @@ TEST_P(RendererTest, CachesRenderPassAndFramebuffer) { render_target.GetColorAttachments().find(0u)->second.resolve_texture; auto& texture_vk = TextureVK::Cast(*resolve_texture); - EXPECT_EQ(texture_vk.GetFramebuffer(), nullptr); - EXPECT_EQ(texture_vk.GetRenderPass(), nullptr); + EXPECT_EQ(texture_vk.GetCachedFramebuffer(), nullptr); + EXPECT_EQ(texture_vk.GetCachedRenderPass(), nullptr); auto buffer = GetContext()->CreateCommandBuffer(); auto render_pass = buffer->CreateRenderPass(render_target); - EXPECT_NE(texture_vk.GetFramebuffer(), nullptr); - EXPECT_NE(texture_vk.GetRenderPass(), nullptr); + EXPECT_NE(texture_vk.GetCachedFramebuffer(), nullptr); + EXPECT_NE(texture_vk.GetCachedRenderPass(), nullptr); render_pass->EncodeCommands(); GetContext()->GetCommandQueue()->Submit({buffer}); diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index bc2ed6c5115d7..d8c0c4ce82e01 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -164,8 +164,10 @@ RenderPassVK::RenderPassVK(const std::shared_ptr& context, SharedHandleVK recycled_render_pass; SharedHandleVK recycled_framebuffer; if (resolve_image_vk_) { - recycled_render_pass = TextureVK::Cast(*resolve_image_vk_).GetRenderPass(); - recycled_framebuffer = TextureVK::Cast(*resolve_image_vk_).GetFramebuffer(); + recycled_render_pass = + TextureVK::Cast(*resolve_image_vk_).GetCachedRenderPass(); + recycled_framebuffer = + TextureVK::Cast(*resolve_image_vk_).GetCachedFramebuffer(); } const auto& target_size = render_target_.GetRenderTargetSize(); @@ -192,8 +194,8 @@ RenderPassVK::RenderPassVK(const std::shared_ptr& context, return; } if (resolve_image_vk_) { - TextureVK::Cast(*resolve_image_vk_).SetFramebuffer(framebuffer); - TextureVK::Cast(*resolve_image_vk_).SetRenderPass(render_pass_); + TextureVK::Cast(*resolve_image_vk_).SetCachedFramebuffer(framebuffer); + TextureVK::Cast(*resolve_image_vk_).SetCachedRenderPass(render_pass_); } auto clear_values = GetVKClearValues(render_target_); diff --git a/impeller/renderer/backend/vulkan/surface_vk.cc b/impeller/renderer/backend/vulkan/surface_vk.cc index 949da581be0f2..1ea9a8fed84fb 100644 --- a/impeller/renderer/backend/vulkan/surface_vk.cc +++ b/impeller/renderer/backend/vulkan/surface_vk.cc @@ -77,9 +77,15 @@ 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, "Onscreen", RenderTarget::kDefaultStencilAttachmentConfig, - swapchain_image->GetDepthStencilTexture()); + /*context=*/*context, // + /*allocator=*/*context->GetResourceAllocator(), // + /*size=*/swapchain_image->GetSize(), // + /*msaa=*/enable_msaa, // + /*label=*/"Onscreen", // + /*stencil_attachment_config=*/ + RenderTarget::kDefaultStencilAttachmentConfig, // + /*depth_stencil_texture=*/swapchain_image->GetDepthStencilTexture() // + ); // The constructor is private. So make_unique may not be used. return std::unique_ptr( diff --git a/impeller/renderer/backend/vulkan/surface_vk.h b/impeller/renderer/backend/vulkan/surface_vk.h index 73bbf2b7319f4..61ecb198f1efb 100644 --- a/impeller/renderer/backend/vulkan/surface_vk.h +++ b/impeller/renderer/backend/vulkan/surface_vk.h @@ -17,6 +17,11 @@ class SurfaceVK final : public Surface { public: using SwapCallback = std::function; + /// @brief Wrap the swapchain image in a Surface, which provides the + /// additional configuration required for usage as on onscreen render + /// target by Impeller. + /// + /// This creates the associated MSAA and depth+stencil texture. static std::unique_ptr WrapSwapchainImage( const std::shared_ptr& context, std::shared_ptr& swapchain_image, diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 61fea57025b70..26b7c578fcc94 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -239,6 +239,9 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, msaa_desc.size = texture_desc.size; msaa_desc.usage = static_cast(TextureUsage::kRenderTarget); + // The depth+stencil configuration matches the configuration used by + // RenderTarget::SetupDepthStencilAttachments and matching the swapchain + // image dimensions and sample count. TextureDescriptor depth_stencil_desc; depth_stencil_desc.storage_mode = StorageMode::kDeviceTransient; if (enable_msaa) { diff --git a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc index da5031b45615d..cc865c0408d00 100644 --- a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc +++ b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc @@ -9,7 +9,6 @@ #include #include -#include "fml/macros.h" #include "impeller/base/thread_safety.h" #include "impeller/renderer/backend/vulkan/vk.h" // IWYU pragma: keep. #include "third_party/swiftshader/include/vulkan/vulkan_core.h" @@ -40,6 +39,7 @@ struct MockImage {}; struct MockSwapchainKHR { std::array images; + size_t current_image = 0; }; struct MockSemaphore {}; @@ -717,7 +717,9 @@ VkResult vkAcquireNextImageKHR(VkDevice device, VkSemaphore semaphore, VkFence fence, uint32_t* pImageIndex) { - *pImageIndex = 0; + auto current_index = + reinterpret_cast(swapchain)->current_image++; + *pImageIndex = (current_index + 1) % 3u; return VK_SUCCESS; } diff --git a/impeller/renderer/backend/vulkan/test/swapchain_unittests.cc b/impeller/renderer/backend/vulkan/test/swapchain_unittests.cc index f0af23ed75b46..10904b3f245ff 100644 --- a/impeller/renderer/backend/vulkan/test/swapchain_unittests.cc +++ b/impeller/renderer/backend/vulkan/test/swapchain_unittests.cc @@ -8,6 +8,7 @@ #include "impeller/renderer/backend/vulkan/test/mock_vulkan.h" #include "impeller/renderer/backend/vulkan/texture_vk.h" #include "vulkan/vulkan_enums.hpp" +#include "vulkan/vulkan_handles.hpp" namespace impeller { namespace testing { @@ -62,32 +63,64 @@ TEST(SwapchainTest, CachesRenderPassOnSwapchainImage) { 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(); + // The mock swapchain will always create 3 images, verify each one starts + // out with the same MSAA and depth+stencil texture, and no cached + // framebuffer. + std::vector> msaa_textures; + std::vector> depth_stencil_textures; + for (auto i = 0u; i < 3u; i++) { + auto drawable = swapchain->AcquireNextDrawable(); + RenderTarget render_target = drawable->GetTargetRenderPassDescriptor(); + + auto texture = render_target.GetRenderTargetTexture(); + auto& texture_vk = TextureVK::Cast(*texture); + EXPECT_EQ(texture_vk.GetCachedFramebuffer(), nullptr); + EXPECT_EQ(texture_vk.GetCachedRenderPass(), nullptr); + + auto command_buffer = context->CreateCommandBuffer(); + auto render_pass = command_buffer->CreateRenderPass(render_target); + render_pass->EncodeCommands(); + + depth_stencil_textures.push_back( + render_target.GetDepthAttachment()->texture); + msaa_textures.push_back( + render_target.GetColorAttachments().find(0u)->second.texture); + } - auto texture = render_target.GetRenderTargetTexture(); - auto& texture_vk = TextureVK::Cast(*texture); - EXPECT_EQ(texture_vk.GetFramebuffer(), nullptr); - EXPECT_EQ(texture_vk.GetRenderPass(), nullptr); + for (auto i = 1; i < 3; i++) { + EXPECT_EQ(msaa_textures[i - 1], msaa_textures[i]); + EXPECT_EQ(depth_stencil_textures[i - 1], depth_stencil_textures[i]); + } - auto command_buffer = context->CreateCommandBuffer(); - auto render_pass = command_buffer->CreateRenderPass(render_target); + // After each images has been acquired once and the render pass presented, + // each should have a cached framebuffer and render pass. - render_pass->EncodeCommands(); + std::vector> framebuffers; + std::vector> render_passes; + for (auto i = 0u; i < 3u; i++) { + auto drawable = swapchain->AcquireNextDrawable(); + RenderTarget render_target = drawable->GetTargetRenderPassDescriptor(); - EXPECT_NE(texture_vk.GetFramebuffer(), nullptr); - EXPECT_NE(texture_vk.GetRenderPass(), nullptr); + auto texture = render_target.GetRenderTargetTexture(); + auto& texture_vk = TextureVK::Cast(*texture); - { + EXPECT_NE(texture_vk.GetCachedFramebuffer(), nullptr); + EXPECT_NE(texture_vk.GetCachedRenderPass(), nullptr); + framebuffers.push_back(texture_vk.GetCachedFramebuffer()); + render_passes.push_back(texture_vk.GetCachedRenderPass()); + } + + // Iterate through once more to verify render passes and framebuffers are + // unchanged. + for (auto i = 0u; i < 3u; i++) { auto drawable = swapchain->AcquireNextDrawable(); + RenderTarget render_target = drawable->GetTargetRenderPassDescriptor(); + auto texture = render_target.GetRenderTargetTexture(); auto& texture_vk = TextureVK::Cast(*texture); - EXPECT_NE(texture_vk.GetFramebuffer(), nullptr); - EXPECT_NE(texture_vk.GetRenderPass(), nullptr); + EXPECT_EQ(texture_vk.GetCachedFramebuffer(), framebuffers[i]); + EXPECT_EQ(texture_vk.GetCachedRenderPass(), render_passes[i]); } } diff --git a/impeller/renderer/backend/vulkan/texture_source_vk.cc b/impeller/renderer/backend/vulkan/texture_source_vk.cc index 9d9c965b13c2b..7fdf5caf924be 100644 --- a/impeller/renderer/backend/vulkan/texture_source_vk.cc +++ b/impeller/renderer/backend/vulkan/texture_source_vk.cc @@ -62,21 +62,21 @@ fml::Status TextureSourceVK::SetLayout(const BarrierVK& barrier) const { return {}; } -void TextureSourceVK::SetFramebuffer( +void TextureSourceVK::SetCachedFramebuffer( const SharedHandleVK& framebuffer) { framebuffer_ = framebuffer; } -void TextureSourceVK::SetRenderPass( +void TextureSourceVK::SetCachedRenderPass( const SharedHandleVK& render_pass) { render_pass_ = render_pass; } -SharedHandleVK TextureSourceVK::GetFramebuffer() const { +SharedHandleVK TextureSourceVK::GetCachedFramebuffer() const { return framebuffer_; } -SharedHandleVK TextureSourceVK::GetRenderPass() const { +SharedHandleVK TextureSourceVK::GetCachedRenderPass() const { return render_pass_; } diff --git a/impeller/renderer/backend/vulkan/texture_source_vk.h b/impeller/renderer/backend/vulkan/texture_source_vk.h index 0b072dbd6a5a0..374907cae13a6 100644 --- a/impeller/renderer/backend/vulkan/texture_source_vk.h +++ b/impeller/renderer/backend/vulkan/texture_source_vk.h @@ -132,24 +132,24 @@ class TextureSourceVK { /// 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); + void SetCachedFramebuffer(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); + void SetCachedRenderPass(const SharedHandleVK& render_pass); /// Retrieve the last framebuffer object used with this texture. /// /// May be nullptr if no previous framebuffer existed. - SharedHandleVK GetFramebuffer() const; + SharedHandleVK GetCachedFramebuffer() const; /// Retrieve the last render pass object used with this texture. /// /// May be nullptr if no previous render pass existed. - SharedHandleVK GetRenderPass() const; + SharedHandleVK GetCachedRenderPass() const; protected: const TextureDescriptor desc_; diff --git a/impeller/renderer/backend/vulkan/texture_vk.cc b/impeller/renderer/backend/vulkan/texture_vk.cc index b11a3add35329..e34bb89e76ba6 100644 --- a/impeller/renderer/backend/vulkan/texture_vk.cc +++ b/impeller/renderer/backend/vulkan/texture_vk.cc @@ -174,22 +174,22 @@ vk::ImageView TextureVK::GetRenderTargetView() const { return source_->GetRenderTargetView(); } -void TextureVK::SetFramebuffer( +void TextureVK::SetCachedFramebuffer( const SharedHandleVK& framebuffer) { - source_->SetFramebuffer(framebuffer); + source_->SetCachedFramebuffer(framebuffer); } -void TextureVK::SetRenderPass( +void TextureVK::SetCachedRenderPass( const SharedHandleVK& render_pass) { - source_->SetRenderPass(render_pass); + source_->SetCachedRenderPass(render_pass); } -SharedHandleVK TextureVK::GetFramebuffer() const { - return source_->GetFramebuffer(); +SharedHandleVK TextureVK::GetCachedFramebuffer() const { + return source_->GetCachedFramebuffer(); } -SharedHandleVK TextureVK::GetRenderPass() const { - return source_->GetRenderPass(); +SharedHandleVK TextureVK::GetCachedRenderPass() const { + return source_->GetCachedRenderPass(); } void TextureVK::SetMipMapGenerated() { diff --git a/impeller/renderer/backend/vulkan/texture_vk.h b/impeller/renderer/backend/vulkan/texture_vk.h index e4d0d9150d703..fe243dc3e6f31 100644 --- a/impeller/renderer/backend/vulkan/texture_vk.h +++ b/impeller/renderer/backend/vulkan/texture_vk.h @@ -55,24 +55,24 @@ class TextureVK final : public Texture, public BackendCast { /// 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); + void SetCachedFramebuffer(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); + void SetCachedRenderPass(const SharedHandleVK& render_pass); /// Retrieve the last framebuffer object used with this texture. /// /// May be nullptr if no previous framebuffer existed. - SharedHandleVK GetFramebuffer() const; + SharedHandleVK GetCachedFramebuffer() const; /// Retrieve the last render pass object used with this texture. /// /// May be nullptr if no previous render pass existed. - SharedHandleVK GetRenderPass() const; + SharedHandleVK GetCachedRenderPass() const; private: std::weak_ptr context_; From 2dd355a60e5690f73b707ed6d266f1bba2ed0dab Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 7 Mar 2024 09:54:33 -0800 Subject: [PATCH 9/9] ++ --- impeller/renderer/backend/vulkan/test/swapchain_unittests.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/impeller/renderer/backend/vulkan/test/swapchain_unittests.cc b/impeller/renderer/backend/vulkan/test/swapchain_unittests.cc index 10904b3f245ff..844a460d18f5f 100644 --- a/impeller/renderer/backend/vulkan/test/swapchain_unittests.cc +++ b/impeller/renderer/backend/vulkan/test/swapchain_unittests.cc @@ -81,8 +81,9 @@ TEST(SwapchainTest, CachesRenderPassOnSwapchainImage) { auto render_pass = command_buffer->CreateRenderPass(render_target); render_pass->EncodeCommands(); - depth_stencil_textures.push_back( - render_target.GetDepthAttachment()->texture); + auto& depth = render_target.GetDepthAttachment(); + depth_stencil_textures.push_back(depth.has_value() ? depth->texture + : nullptr); msaa_textures.push_back( render_target.GetColorAttachments().find(0u)->second.texture); }