Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ci/licenses_golden/excluded_files
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@
../../../flutter/impeller/renderer/backend/vulkan/context_vk_unittests.cc
../../../flutter/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc
../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk_unittests.cc
../../../flutter/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc
../../../flutter/impeller/renderer/backend/vulkan/resource_manager_vk_unittests.cc
../../../flutter/impeller/renderer/backend/vulkan/test
../../../flutter/impeller/renderer/blit_pass_unittests.cc
Expand Down
2 changes: 2 additions & 0 deletions impeller/renderer/backend/vulkan/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ impeller_component("vulkan_unittests") {
"context_vk_unittests.cc",
"descriptor_pool_vk_unittests.cc",
"fence_waiter_vk_unittests.cc",
"render_pass_cache_unittests.cc",
"resource_manager_vk_unittests.cc",
"test/gpu_tracer_unittests.cc",
"test/mock_vulkan.cc",
Expand All @@ -23,6 +24,7 @@ impeller_component("vulkan_unittests") {
]
deps = [
":vulkan",
"../../../playground:playground_test",
"//flutter/testing:testing_lib",
]
}
Expand Down
51 changes: 51 additions & 0 deletions impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "flutter/testing/testing.h"
#include "gtest/gtest.h"
#include "impeller/playground/playground_test.h"
#include "impeller/renderer/backend/vulkan/texture_vk.h"
#include "impeller/renderer/render_target.h"

namespace impeller {
namespace testing {

using RendererTest = PlaygroundTest;

TEST_P(RendererTest, CachesRenderPassAndFramebuffer) {
if (GetBackend() != PlaygroundBackend::kVulkan) {
GTEST_SKIP() << "Test only applies to Vulkan";
}

auto allocator = std::make_shared<RenderTargetAllocator>(
GetContext()->GetResourceAllocator());

auto render_target = RenderTarget::CreateOffscreenMSAA(
*GetContext(), *allocator, {100, 100}, 1);
auto resolve_texture =
render_target.GetColorAttachments().find(0u)->second.resolve_texture;
auto& texture_vk = TextureVK::Cast(*resolve_texture);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test appears to run with validations locally. There were no validation errors I believe

EXPECT_EQ(texture_vk.GetFramebuffer(), nullptr);
EXPECT_EQ(texture_vk.GetRenderPass(), nullptr);

auto buffer = GetContext()->CreateCommandBuffer();
auto render_pass = buffer->CreateRenderPass(render_target);

EXPECT_NE(texture_vk.GetFramebuffer(), nullptr);
EXPECT_NE(texture_vk.GetRenderPass(), nullptr);

render_pass->EncodeCommands();
GetContext()->GetCommandQueue()->Submit({buffer});

// Can be reused without error.
auto buffer_2 = GetContext()->CreateCommandBuffer();
auto render_pass_2 = buffer_2->CreateRenderPass(render_target);

EXPECT_TRUE(render_pass_2->EncodeCommands());
EXPECT_TRUE(GetContext()->GetCommandQueue()->Submit({buffer_2}).ok());
}

} // namespace testing
} // namespace impeller
38 changes: 32 additions & 6 deletions impeller/renderer/backend/vulkan/render_pass_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "impeller/renderer/backend/vulkan/shared_object_vk.h"
#include "impeller/renderer/backend/vulkan/texture_vk.h"
#include "impeller/renderer/backend/vulkan/vk.h"
#include "vulkan/vulkan_handles.hpp"

namespace impeller {

Expand Down Expand Up @@ -79,6 +80,7 @@ static std::vector<vk::ClearValue> GetVKClearValues(

SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
const ContextVK& context,
const SharedHandleVK<vk::RenderPass>& recycled_renderpass,
const std::shared_ptr<CommandBufferVK>& command_buffer) const {
BarrierVK barrier;
barrier.new_layout = vk::ImageLayout::eGeneral;
Expand Down Expand Up @@ -127,6 +129,15 @@ SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
TextureVK::Cast(*stencil->texture).SetLayout(barrier);
}

// There may exist a previous recycled render pass that we can continue using.
// This is probably compatible with the render pass we are about to construct,
// but I have not conclusively proven this. If there are scenarios that
// produce validation warnings, we could use them to determine if we need
// additional checks at this point to determine reusability.
if (recycled_renderpass != nullptr) {
return recycled_renderpass;
}

auto pass = builder.Build(context.GetDevice());

if (!pass) {
Expand All @@ -143,6 +154,11 @@ RenderPassVK::RenderPassVK(const std::shared_ptr<const Context>& context,
const RenderTarget& target,
std::shared_ptr<CommandBufferVK> command_buffer)
: RenderPass(context, target), command_buffer_(std::move(command_buffer)) {
color_image_vk_ =
render_target_.GetColorAttachments().find(0u)->second.texture;
resolve_image_vk_ =
render_target_.GetColorAttachments().find(0u)->second.resolve_texture;

const auto& vk_context = ContextVK::Cast(*context);
const std::shared_ptr<CommandEncoderVK>& encoder =
command_buffer_->GetEncoder();
Expand All @@ -154,16 +170,26 @@ RenderPassVK::RenderPassVK(const std::shared_ptr<const Context>& context,
return true;
});

SharedHandleVK<vk::RenderPass> recycled_render_pass;
SharedHandleVK<vk::Framebuffer> recycled_framebuffer;
if (resolve_image_vk_) {
recycled_render_pass = TextureVK::Cast(*resolve_image_vk_).GetRenderPass();
recycled_framebuffer = TextureVK::Cast(*resolve_image_vk_).GetFramebuffer();
}

const auto& target_size = render_target_.GetRenderTargetSize();

render_pass_ = CreateVKRenderPass(vk_context, command_buffer_);
render_pass_ =
CreateVKRenderPass(vk_context, recycled_render_pass, command_buffer_);
if (!render_pass_) {
VALIDATION_LOG << "Could not create renderpass.";
is_valid_ = false;
return;
}

auto framebuffer = CreateVKFramebuffer(vk_context, *render_pass_);
auto framebuffer = (recycled_framebuffer == nullptr)
? CreateVKFramebuffer(vk_context, *render_pass_)
: recycled_framebuffer;
if (!framebuffer) {
VALIDATION_LOG << "Could not create framebuffer.";
is_valid_ = false;
Expand All @@ -174,6 +200,10 @@ RenderPassVK::RenderPassVK(const std::shared_ptr<const Context>& context,
is_valid_ = false;
return;
}
if (resolve_image_vk_) {
TextureVK::Cast(*resolve_image_vk_).SetFramebuffer(framebuffer);
TextureVK::Cast(*resolve_image_vk_).SetRenderPass(render_pass_);
}

auto clear_values = GetVKClearValues(render_target_);

Expand Down Expand Up @@ -205,10 +235,6 @@ RenderPassVK::RenderPassVK(const std::shared_ptr<const Context>& context,
.setExtent(vk::Extent2D(sc.GetWidth(), sc.GetHeight()));
command_buffer_vk_.setScissor(0, 1, &scissor);

color_image_vk_ =
render_target_.GetColorAttachments().find(0u)->second.texture;
resolve_image_vk_ =
render_target_.GetColorAttachments().find(0u)->second.resolve_texture;
is_valid_ = true;
}

Expand Down
2 changes: 2 additions & 0 deletions impeller/renderer/backend/vulkan/render_pass_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "impeller/renderer/backend/vulkan/shared_object_vk.h"
#include "impeller/renderer/render_pass.h"
#include "impeller/renderer/render_target.h"
#include "vulkan/vulkan_handles.hpp"

namespace impeller {

Expand Down Expand Up @@ -123,6 +124,7 @@ class RenderPassVK final : public RenderPass {

SharedHandleVK<vk::RenderPass> CreateVKRenderPass(
const ContextVK& context,
const SharedHandleVK<vk::RenderPass>& recycled_renderpass,
const std::shared_ptr<CommandBufferVK>& command_buffer) const;

SharedHandleVK<vk::Framebuffer> CreateVKFramebuffer(
Expand Down
1 change: 1 addition & 0 deletions impeller/renderer/backend/vulkan/swapchain_image_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 "vulkan/vulkan_handles.hpp"

namespace impeller {

Expand Down
2 changes: 2 additions & 0 deletions impeller/renderer/backend/vulkan/texture_source_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
#include "impeller/core/texture_descriptor.h"
#include "impeller/renderer/backend/vulkan/barrier_vk.h"
#include "impeller/renderer/backend/vulkan/formats_vk.h"
#include "impeller/renderer/backend/vulkan/shared_object_vk.h"
#include "impeller/renderer/backend/vulkan/vk.h"
#include "vulkan/vulkan_handles.hpp"

namespace impeller {

Expand Down
18 changes: 18 additions & 0 deletions impeller/renderer/backend/vulkan/texture_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -173,4 +173,22 @@ vk::ImageView TextureVK::GetRenderTargetView() const {
return source_->GetRenderTargetView();
}

void TextureVK::SetFramebuffer(
const SharedHandleVK<vk::Framebuffer>& framebuffer) {
framebuffer_ = framebuffer;
}

void TextureVK::SetRenderPass(
const SharedHandleVK<vk::RenderPass>& render_pass) {
render_pass_ = render_pass;
}

SharedHandleVK<vk::Framebuffer> TextureVK::GetFramebuffer() const {
return framebuffer_;
}

SharedHandleVK<vk::RenderPass> TextureVK::GetRenderPass() const {
return render_pass_;
}

} // namespace impeller
28 changes: 28 additions & 0 deletions impeller/renderer/backend/vulkan/texture_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,37 @@ class TextureVK final : public Texture, public BackendCast<TextureVK, Texture> {

bool IsSwapchainImage() const { return source_->IsSwapchainImage(); }

// These methods should only be used by render_pass_vk.h
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments look good 👍


/// 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<vk::Framebuffer>& framebuffer);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to save this on the RenderTarget, not the Texture? This isn't applicable to every Texture, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The RenderTarget itself is const/immutable whereas the Texture is stateful.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not a problem though. We can make the frame buffer and render pass when we create the RenderTarget. We shouldn't ever have a RenderTarget without a render pass and it will never change, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately that is all constructed above the backend layer, and there are no metal or gles analogs for these objects.


/// 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<vk::RenderPass>& render_pass);

/// Retrieve the last framebuffer object used with this texture.
///
/// May be nullptr if no previous framebuffer existed.
SharedHandleVK<vk::Framebuffer> GetFramebuffer() const;

/// Retrieve the last render pass object used with this texture.
///
/// May be nullptr if no previous render pass existed.
SharedHandleVK<vk::RenderPass> GetRenderPass() const;

private:
std::weak_ptr<Context> context_;
std::shared_ptr<TextureSourceVK> source_;
SharedHandleVK<vk::Framebuffer> framebuffer_ = nullptr;
SharedHandleVK<vk::RenderPass> render_pass_ = nullptr;

// |Texture|
void SetLabel(std::string_view label) override;
Expand Down