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 5 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 @@ -147,6 +147,7 @@
../../../flutter/impeller/playground
../../../flutter/impeller/renderer/backend/vulkan/blit_command_vk_unittests.cc
../../../flutter/impeller/renderer/backend/vulkan/context_vk_unittests.cc
../../../flutter/impeller/renderer/backend/vulkan/pass_bindings_cache_unittests.cc
../../../flutter/impeller/renderer/backend/vulkan/test
../../../flutter/impeller/renderer/capabilities_unittests.cc
../../../flutter/impeller/renderer/compute_subgroup_unittests.cc
Expand Down
4 changes: 4 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -1520,6 +1520,8 @@ ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.cc + .
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/formats_vk.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/formats_vk.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/pass_bindings_cache.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/pass_bindings_cache.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/pipeline_cache_vk.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/pipeline_library_vk.cc + ../../../flutter/LICENSE
Expand Down Expand Up @@ -4195,6 +4197,8 @@ FILE: ../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.cc
FILE: ../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/formats_vk.cc
FILE: ../../../flutter/impeller/renderer/backend/vulkan/formats_vk.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/pass_bindings_cache.cc
FILE: ../../../flutter/impeller/renderer/backend/vulkan/pass_bindings_cache.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc
FILE: ../../../flutter/impeller/renderer/backend/vulkan/pipeline_cache_vk.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/pipeline_library_vk.cc
Expand Down
3 changes: 3 additions & 0 deletions impeller/renderer/backend/vulkan/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ impeller_component("vulkan_unittests") {
sources = [
"blit_command_vk_unittests.cc",
"context_vk_unittests.cc",
"pass_bindings_cache_unittests.cc",
"test/mock_vulkan.cc",
"test/mock_vulkan.h",
]
Expand Down Expand Up @@ -51,6 +52,8 @@ impeller_component("vulkan") {
"fence_waiter_vk.h",
"formats_vk.cc",
"formats_vk.h",
"pass_bindings_cache.cc",
"pass_bindings_cache.h",
"pipeline_cache_vk.cc",
"pipeline_cache_vk.h",
"pipeline_library_vk.cc",
Expand Down
26 changes: 6 additions & 20 deletions impeller/renderer/backend/vulkan/command_encoder_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,6 @@

namespace impeller {

namespace testing {
class BlitCommandVkTest_BlitCopyTextureToTextureCommandVK_Test;
class BlitCommandVkTest_BlitCopyTextureToBufferCommandVK_Test;
class BlitCommandVkTest_BlitCopyBufferToTextureCommandVK_Test;
class BlitCommandVkTest_BlitGenerateMipmapCommandVK_Test;
} // namespace testing

class ContextVK;
class DeviceBuffer;
class Buffer;
Expand All @@ -37,6 +30,12 @@ class CommandEncoderVK {
public:
using SubmitCallback = std::function<void(bool)>;

// Visible for testing.
CommandEncoderVK(const std::weak_ptr<const DeviceHolder>& device_holder,
const std::shared_ptr<QueueVK>& queue,
const std::shared_ptr<CommandPoolVK>& pool,
std::shared_ptr<FenceWaiterVK> fence_waiter);

~CommandEncoderVK();

bool IsValid() const;
Expand Down Expand Up @@ -68,26 +67,13 @@ class CommandEncoderVK {

private:
friend class ContextVK;
friend class ::impeller::testing::
BlitCommandVkTest_BlitCopyTextureToTextureCommandVK_Test;
friend class ::impeller::testing::
BlitCommandVkTest_BlitCopyTextureToBufferCommandVK_Test;
friend class ::impeller::testing::
BlitCommandVkTest_BlitGenerateMipmapCommandVK_Test;
friend class ::impeller::testing::
BlitCommandVkTest_BlitCopyBufferToTextureCommandVK_Test;

std::weak_ptr<const DeviceHolder> device_holder_;
std::shared_ptr<QueueVK> queue_;
std::shared_ptr<FenceWaiterVK> fence_waiter_;
std::shared_ptr<TrackedObjectsVK> tracked_objects_;
bool is_valid_ = false;

CommandEncoderVK(const std::weak_ptr<const DeviceHolder>& device_holder,
const std::shared_ptr<QueueVK>& queue,
const std::shared_ptr<CommandPoolVK>& pool,
std::shared_ptr<FenceWaiterVK> fence_waiter);

void Reset();

FML_DISALLOW_COPY_AND_ASSIGN(CommandEncoderVK);
Expand Down
72 changes: 72 additions & 0 deletions impeller/renderer/backend/vulkan/pass_bindings_cache.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// 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 "impeller/renderer/backend/vulkan/pass_bindings_cache.h"

namespace impeller {
void PassBindingsCache::bindPipeline(vk::CommandBuffer command_buffer,
vk::PipelineBindPoint pipeline_bind_point,
vk::Pipeline pipeline) {
switch (pipeline_bind_point) {
case vk::PipelineBindPoint::eGraphics:
if (graphics_pipeline_.has_value() &&
graphics_pipeline_.value() == pipeline) {
return;
}
graphics_pipeline_ = pipeline;
break;
case vk::PipelineBindPoint::eCompute:
if (compute_pipeline_.has_value() &&
compute_pipeline_.value() == pipeline) {
return;
}
compute_pipeline_ = pipeline;
break;
default:
break;
}
command_buffer.bindPipeline(pipeline_bind_point, pipeline);
}

void PassBindingsCache::setStencilReference(vk::CommandBuffer command_buffer,
vk::StencilFaceFlags face_mask,
uint32_t reference) {
if (stencil_face_flags_.has_value() &&
face_mask == stencil_face_flags_.value() &&
reference == stencil_reference_) {
return;
}
stencil_face_flags_ = face_mask;
stencil_reference_ = reference;
command_buffer.setStencilReference(face_mask, reference);
}

void PassBindingsCache::setScissor(vk::CommandBuffer command_buffer,
uint32_t first_scissor,
uint32_t scissor_count,
const vk::Rect2D* scissors) {
if (first_scissor == 0 && scissor_count == 1) {
if (scissors_.has_value() && scissors_.value() == scissors[0]) {
return;
}
scissors_ = scissors[0];
}
command_buffer.setScissor(first_scissor, scissor_count, scissors);
}

void PassBindingsCache::setViewport(vk::CommandBuffer command_buffer,
uint32_t first_viewport,
uint32_t viewport_count,
const vk::Viewport* viewports) {
if (first_viewport == 0 && viewport_count == 1) {
// Note that this is doing equality checks on floating point numbers.
if (viewport_.has_value() && viewport_.value() == viewports[0]) {
return;
}
viewport_ = viewports[0];
}
command_buffer.setViewport(first_viewport, viewport_count, viewports);
}

} // namespace impeller
46 changes: 46 additions & 0 deletions impeller/renderer/backend/vulkan/pass_bindings_cache.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// 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.

#pragma once

#include <optional>

#include "flutter/impeller/renderer/backend/vulkan/vk.h"

namespace impeller {

class PassBindingsCache {
public:
void bindPipeline(vk::CommandBuffer command_buffer,
Copy link
Member

Choose a reason for hiding this comment

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

Nit: CapitalCase.

Copy link
Member Author

Choose a reason for hiding this comment

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

I only did it so that they match verbatim the functions they are caching for (ie vk::CommandBuffer::bindPipeline).

Copy link
Member

Choose a reason for hiding this comment

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

I like being consistent in newly authored code. And besides, the stuff in libc++ is underscore_case. So having different conventions for code and calls in different components is a thing already.

In any case, I don't have a strong opinion about this. But if you are ambivalent about it, I'd rather go with sticking to the style guide.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

vk::PipelineBindPoint pipeline_bind_point,
vk::Pipeline pipeline);

void setStencilReference(vk::CommandBuffer command_buffer,
vk::StencilFaceFlags face_mask,
uint32_t reference);

void setScissor(vk::CommandBuffer command_buffer,
uint32_t first_scissor,
uint32_t scissor_count,
const vk::Rect2D* scissors);

void setViewport(vk::CommandBuffer command_buffer,
uint32_t first_viewport,
uint32_t viewport_count,
const vk::Viewport* viewports);

private:
// bindPipeline
std::optional<vk::Pipeline> graphics_pipeline_;
std::optional<vk::Pipeline> compute_pipeline_;
// setStencilReference
std::optional<vk::StencilFaceFlags> stencil_face_flags_;
uint32_t stencil_reference_ = 0;
// setScissor
std::optional<vk::Rect2D> scissors_;
// setViewport
std::optional<vk::Viewport> viewport_;
};

} // namespace impeller
94 changes: 94 additions & 0 deletions impeller/renderer/backend/vulkan/pass_bindings_cache_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
// 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 "impeller/renderer/backend/vulkan/command_encoder_vk.h"
#include "impeller/renderer/backend/vulkan/pass_bindings_cache.h"
#include "impeller/renderer/backend/vulkan/test/mock_vulkan.h"

namespace impeller {
namespace testing {

namespace {
int32_t CountStringViewInstances(const std::vector<std::string>& strings,
std::string_view target) {
int32_t count = 0;
for (const std::string& str : strings) {
if (str == target) {
count++;
}
}
return count;
}
} // namespace

TEST(PassBindingsCacheTest, bindPipeline) {
auto context = CreateMockVulkanContext();
PassBindingsCache cache;
auto pool = CommandPoolVK::GetThreadLocal(context.get());
CommandEncoderVK encoder(context->GetDeviceHolder(),
context->GetGraphicsQueue(), pool,
context->GetFenceWaiter());
auto buffer = encoder.GetCommandBuffer();
VkPipeline vk_pipeline = reinterpret_cast<VkPipeline>(0xfeedface);
vk::Pipeline pipeline(vk_pipeline);
cache.bindPipeline(buffer, vk::PipelineBindPoint::eGraphics, pipeline);
cache.bindPipeline(buffer, vk::PipelineBindPoint::eGraphics, pipeline);
std::shared_ptr<std::vector<std::string>> functions =
GetMockVulkanFunctions(context->GetDevice());
EXPECT_EQ(CountStringViewInstances(*functions, "vkCmdBindPipeline"), 1);
}

TEST(PassBindingsCacheTest, setStencilReference) {
auto context = CreateMockVulkanContext();
PassBindingsCache cache;
auto pool = CommandPoolVK::GetThreadLocal(context.get());
CommandEncoderVK encoder(context->GetDeviceHolder(),
context->GetGraphicsQueue(), pool,
context->GetFenceWaiter());
auto buffer = encoder.GetCommandBuffer();
cache.setStencilReference(
buffer, vk::StencilFaceFlagBits::eVkStencilFrontAndBack, 123);
cache.setStencilReference(
buffer, vk::StencilFaceFlagBits::eVkStencilFrontAndBack, 123);
std::shared_ptr<std::vector<std::string>> functions =
GetMockVulkanFunctions(context->GetDevice());
EXPECT_EQ(CountStringViewInstances(*functions, "vkCmdSetStencilReference"),
1);
}

TEST(PassBindingsCacheTest, setScissor) {
auto context = CreateMockVulkanContext();
PassBindingsCache cache;
auto pool = CommandPoolVK::GetThreadLocal(context.get());
CommandEncoderVK encoder(context->GetDeviceHolder(),
context->GetGraphicsQueue(), pool,
context->GetFenceWaiter());
auto buffer = encoder.GetCommandBuffer();
vk::Rect2D scissors;
cache.setScissor(buffer, 0, 1, &scissors);
cache.setScissor(buffer, 0, 1, &scissors);
std::shared_ptr<std::vector<std::string>> functions =
GetMockVulkanFunctions(context->GetDevice());
EXPECT_EQ(CountStringViewInstances(*functions, "vkCmdSetScissor"), 1);
}

TEST(PassBindingsCacheTest, setViewport) {
auto context = CreateMockVulkanContext();
PassBindingsCache cache;
auto pool = CommandPoolVK::GetThreadLocal(context.get());
CommandEncoderVK encoder(context->GetDeviceHolder(),
context->GetGraphicsQueue(), pool,
context->GetFenceWaiter());
auto buffer = encoder.GetCommandBuffer();
vk::Viewport viewports;
cache.setViewport(buffer, 0, 1, &viewports);
cache.setViewport(buffer, 0, 1, &viewports);
std::shared_ptr<std::vector<std::string>> functions =
GetMockVulkanFunctions(context->GetDevice());
EXPECT_EQ(CountStringViewInstances(*functions, "vkCmdSetViewport"), 1);
}

} // namespace testing
} // namespace impeller
19 changes: 11 additions & 8 deletions impeller/renderer/backend/vulkan/render_pass_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,7 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context,

static void SetViewportAndScissor(const Command& command,
const vk::CommandBuffer& cmd_buffer,
PassBindingsCache& cmd_buffer_cache,
const ISize& target_size) {
// Set the viewport.
const auto& vp = command.viewport.value_or<Viewport>(
Expand All @@ -427,20 +428,21 @@ static void SetViewportAndScissor(const Command& command,
.setY(vp.rect.size.height)
.setMinDepth(0.0f)
.setMaxDepth(1.0f);
cmd_buffer.setViewport(0, 1, &viewport);
cmd_buffer_cache.setViewport(cmd_buffer, 0, 1, &viewport);

// Set the scissor rect.
const auto& sc = command.scissor.value_or(IRect::MakeSize(target_size));
vk::Rect2D scissor =
vk::Rect2D()
.setOffset(vk::Offset2D(sc.origin.x, sc.origin.y))
.setExtent(vk::Extent2D(sc.size.width, sc.size.height));
cmd_buffer.setScissor(0, 1, &scissor);
cmd_buffer_cache.setScissor(cmd_buffer, 0, 1, &scissor);
}

static bool EncodeCommand(const Context& context,
const Command& command,
CommandEncoderVK& encoder,
PassBindingsCache& command_buffer_cache,
const ISize& target_size) {
if (command.vertex_count == 0u || command.instance_count == 0u) {
return true;
Expand All @@ -466,15 +468,15 @@ static bool EncodeCommand(const Context& context,
return false;
}

cmd_buffer.bindPipeline(vk::PipelineBindPoint::eGraphics,
pipeline_vk.GetPipeline());
command_buffer_cache.bindPipeline(
cmd_buffer, vk::PipelineBindPoint::eGraphics, pipeline_vk.GetPipeline());

// Set the viewport and scissors.
SetViewportAndScissor(command, cmd_buffer, target_size);
SetViewportAndScissor(command, cmd_buffer, command_buffer_cache, target_size);

// Set the stencil reference.
cmd_buffer.setStencilReference(
vk::StencilFaceFlagBits::eVkStencilFrontAndBack,
command_buffer_cache.setStencilReference(
cmd_buffer, vk::StencilFaceFlagBits::eVkStencilFrontAndBack,
command.stencil_reference);

// Configure vertex and index and buffers for binding.
Expand Down Expand Up @@ -619,7 +621,8 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const {
continue;
}

if (!EncodeCommand(context, command, *encoder, target_size)) {
if (!EncodeCommand(context, command, *encoder, pass_bindings_cache_,
target_size)) {
return false;
}
}
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 @@ -6,6 +6,7 @@

#include "flutter/fml/macros.h"
#include "impeller/renderer/backend/vulkan/context_vk.h"
#include "impeller/renderer/backend/vulkan/pass_bindings_cache.h"
#include "impeller/renderer/backend/vulkan/shared_object_vk.h"
#include "impeller/renderer/backend/vulkan/texture_vk.h"
#include "impeller/renderer/backend/vulkan/vk.h"
Expand All @@ -26,6 +27,7 @@ class RenderPassVK final : public RenderPass {
std::weak_ptr<CommandEncoderVK> encoder_;
std::string debug_label_;
bool is_valid_ = false;
mutable PassBindingsCache pass_bindings_cache_;

RenderPassVK(const std::shared_ptr<const Context>& context,
const RenderTarget& target,
Expand Down
Loading