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 1 commit
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
2 changes: 2 additions & 0 deletions impeller/renderer/backend/vulkan/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ impeller_component("vulkan_unittests") {
testonly = true
sources = [
"blit_command_vk_unittests.cc",
"command_buffer_cache_unittests.cc",
"context_vk_unittests.cc",
"test/mock_vulkan.cc",
"test/mock_vulkan.h",
Expand All @@ -29,6 +30,7 @@ impeller_component("vulkan") {
"blit_pass_vk.h",
"capabilities_vk.cc",
"capabilities_vk.h",
"command_buffer_cache.h",
"command_buffer_vk.cc",
"command_buffer_vk.h",
"command_encoder_vk.cc",
Expand Down
93 changes: 93 additions & 0 deletions impeller/renderer/backend/vulkan/command_buffer_cache.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// 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 {

template <typename CommandBuffer>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The only reason this is templated is to serve the mock. This is unfortunate as it affects our ability to OOL this if needed and makes the code harder to read. Barring another way to test it that doesn't use templates, I'd rather leave the test out and make this simpler (and OOLed). If we mess up our bindings, I'm sure higher level tests will catch this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

class CommandBufferCache {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would call this PassBindingsCache like the Metal backend. This class doesn't seem to cache command buffers at all.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

This class doesn't seem to cache command buffers at all.

"Pass bindings" isn't really accurate either since it is caching anything that is added to the command buffer, not just bindings. I agree, lets keep it consistent though.

public:
void bindPipeline(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 setStencilReference(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 setScissor(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 setViewport(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);
}

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
95 changes: 95 additions & 0 deletions impeller/renderer/backend/vulkan/command_buffer_cache_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// 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_buffer_cache.h"

namespace impeller {
namespace testing {

namespace {
struct Tallies {
int32_t bindPipeline_count = 0;
int32_t setStencilReference_count = 0;
int32_t setScissor_count = 0;
int32_t setViewport_count = 0;
};

class MockCommandBuffer {
public:
MockCommandBuffer() : tallies_(new Tallies()) {}

void bindPipeline(vk::PipelineBindPoint pipeline_bind_point,
vk::Pipeline pipeline) {
tallies_->bindPipeline_count += 1;
}

void setStencilReference(vk::StencilFaceFlags face_mask, uint32_t reference) {
tallies_->setStencilReference_count += 1;
}

void setScissor(uint32_t first_scissor,
uint32_t scissor_count,
const vk::Rect2D* scissors) {
tallies_->setScissor_count += 1;
}

void setViewport(uint32_t first_viewport,
uint32_t viewport_count,
const vk::Viewport* viewports) {
tallies_->setViewport_count += 1;
}

std::shared_ptr<Tallies> tallies_;
};
} // namespace

TEST(CommandBufferCacheTest, bindPipeline) {
CommandBufferCache<MockCommandBuffer> cache;
MockCommandBuffer buffer;
VkPipeline vk_pipeline = reinterpret_cast<VkPipeline>(0xfeedface);
vk::Pipeline pipeline(vk_pipeline);
ASSERT_EQ(buffer.tallies_->bindPipeline_count, 0);
cache.bindPipeline(buffer, vk::PipelineBindPoint::eGraphics, pipeline);
ASSERT_EQ(buffer.tallies_->bindPipeline_count, 1);
cache.bindPipeline(buffer, vk::PipelineBindPoint::eGraphics, pipeline);
ASSERT_EQ(buffer.tallies_->bindPipeline_count, 1);
}

TEST(CommandBufferCacheTest, setStencilReference) {
CommandBufferCache<MockCommandBuffer> cache;
MockCommandBuffer buffer;
ASSERT_EQ(buffer.tallies_->setStencilReference_count, 0);
cache.setStencilReference(
buffer, vk::StencilFaceFlagBits::eVkStencilFrontAndBack, 123);
ASSERT_EQ(buffer.tallies_->setStencilReference_count, 1);
cache.setStencilReference(
buffer, vk::StencilFaceFlagBits::eVkStencilFrontAndBack, 123);
ASSERT_EQ(buffer.tallies_->setStencilReference_count, 1);
}

TEST(CommandBufferCacheTest, setScissor) {
CommandBufferCache<MockCommandBuffer> cache;
MockCommandBuffer buffer;
vk::Rect2D scissors;
ASSERT_EQ(buffer.tallies_->setScissor_count, 0);
cache.setScissor(buffer, 0, 1, &scissors);
ASSERT_EQ(buffer.tallies_->setScissor_count, 1);
cache.setScissor(buffer, 0, 1, &scissors);
ASSERT_EQ(buffer.tallies_->setScissor_count, 1);
}

TEST(CommandBufferCacheTest, setViewport) {
CommandBufferCache<MockCommandBuffer> cache;
MockCommandBuffer buffer;
vk::Viewport viewports;
ASSERT_EQ(buffer.tallies_->setViewport_count, 0);
cache.setViewport(buffer, 0, 1, &viewports);
ASSERT_EQ(buffer.tallies_->setViewport_count, 1);
cache.setViewport(buffer, 0, 1, &viewports);
ASSERT_EQ(buffer.tallies_->setViewport_count, 1);
}

} // namespace testing
} // namespace impeller
35 changes: 20 additions & 15 deletions impeller/renderer/backend/vulkan/render_pass_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -415,9 +415,11 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context,
return true;
}

static void SetViewportAndScissor(const Command& command,
const vk::CommandBuffer& cmd_buffer,
const ISize& target_size) {
static void SetViewportAndScissor(
const Command& command,
const vk::CommandBuffer& cmd_buffer,
CommandBufferCache<vk::CommandBuffer>& cmd_buffer_cache,
const ISize& target_size) {
// Set the viewport.
const auto& vp = command.viewport.value_or<Viewport>(
{.rect = Rect::MakeSize(target_size)});
Expand All @@ -427,21 +429,23 @@ 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,
const ISize& target_size) {
static bool EncodeCommand(
const Context& context,
const Command& command,
CommandEncoderVK& encoder,
CommandBufferCache<vk::CommandBuffer>& command_buffer_cache,
const ISize& target_size) {
if (command.vertex_count == 0u || command.instance_count == 0u) {
return true;
}
Expand All @@ -466,15 +470,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 +623,8 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const {
continue;
}

if (!EncodeCommand(context, command, *encoder, target_size)) {
if (!EncodeCommand(context, command, *encoder, command_buffer_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 @@ -5,6 +5,7 @@
#pragma once

#include "flutter/fml/macros.h"
#include "impeller/renderer/backend/vulkan/command_buffer_cache.h"
#include "impeller/renderer/backend/vulkan/context_vk.h"
#include "impeller/renderer/backend/vulkan/shared_object_vk.h"
#include "impeller/renderer/backend/vulkan/texture_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 CommandBufferCache<vk::CommandBuffer> command_buffer_cache_;

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