Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller] Add interface for submitting multiple command buffers at once. #50139

Merged
merged 22 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from 8 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
8 changes: 8 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -5464,6 +5464,8 @@ ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/formats_vk.cc + ../../
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/formats_vk.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/gpu_tracer_vk.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/graphics_queue_vk.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/graphics_queue_vk.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 @@ -5527,6 +5529,8 @@ ORIGIN: ../../../flutter/impeller/renderer/compute_tessellator.cc + ../../../flu
ORIGIN: ../../../flutter/impeller/renderer/compute_tessellator.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/context.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/context.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/graphics_queue.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/graphics_queue.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/path_polyline.comp + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/pipeline.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/pipeline.h + ../../../flutter/LICENSE
Expand Down Expand Up @@ -8309,6 +8313,8 @@ FILE: ../../../flutter/impeller/renderer/backend/vulkan/formats_vk.cc
FILE: ../../../flutter/impeller/renderer/backend/vulkan/formats_vk.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc
FILE: ../../../flutter/impeller/renderer/backend/vulkan/gpu_tracer_vk.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/graphics_queue_vk.cc
FILE: ../../../flutter/impeller/renderer/backend/vulkan/graphics_queue_vk.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/limits_vk.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc
FILE: ../../../flutter/impeller/renderer/backend/vulkan/pipeline_cache_vk.h
Expand Down Expand Up @@ -8373,6 +8379,8 @@ FILE: ../../../flutter/impeller/renderer/compute_tessellator.cc
FILE: ../../../flutter/impeller/renderer/compute_tessellator.h
FILE: ../../../flutter/impeller/renderer/context.cc
FILE: ../../../flutter/impeller/renderer/context.h
FILE: ../../../flutter/impeller/renderer/graphics_queue.cc
FILE: ../../../flutter/impeller/renderer/graphics_queue.h
FILE: ../../../flutter/impeller/renderer/path_polyline.comp
FILE: ../../../flutter/impeller/renderer/pipeline.cc
FILE: ../../../flutter/impeller/renderer/pipeline.h
Expand Down
1 change: 1 addition & 0 deletions impeller/aiks/aiks_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ bool AiksContext::Render(const Picture& picture,
}

fml::ScopedCleanupClosure closure([&]() {
content_context_->FlushCommandBuffers();
if (reset_host_buffer) {
content_context_->GetTransientsBuffer().Reset();
}
Expand Down
17 changes: 4 additions & 13 deletions impeller/aiks/aiks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,7 @@ TEST_P(AiksTest, CanRenderColorFilterWithInvertColorsDrawPaint) {
namespace {
bool GenerateMipmap(const std::shared_ptr<Context>& context,
std::shared_ptr<Texture> texture,
std::string label,
bool async_submit) {
std::string label) {
auto buffer = context->CreateCommandBuffer();
if (!buffer) {
return false;
Expand All @@ -152,23 +151,19 @@ bool GenerateMipmap(const std::shared_ptr<Context>& context,
return false;
}
pass->GenerateMipmap(std::move(texture), std::move(label));
if (async_submit) {
return buffer->EncodeAndSubmit(pass, context->GetResourceAllocator());
}

pass->EncodeCommands(context->GetResourceAllocator());
return buffer->SubmitCommands();
return context->GetCommandQueue()->Submit({buffer}).ok();
}

void CanRenderTiledTexture(AiksTest* aiks_test,
Entity::TileMode tile_mode,
bool async_submit = false,
Matrix local_matrix = {}) {
auto context = aiks_test->GetContext();
ASSERT_TRUE(context);
auto texture = aiks_test->CreateTextureForFixture("table_mountain_nx.png",
/*enable_mipmapping=*/true);
GenerateMipmap(context, texture, "table_mountain_nx", async_submit);
GenerateMipmap(context, texture, "table_mountain_nx");
Canvas canvas;
canvas.Scale(aiks_test->GetContentScale());
canvas.Translate({100.0f, 100.0f, 0});
Expand Down Expand Up @@ -215,10 +210,6 @@ TEST_P(AiksTest, CanRenderTiledTextureClamp) {
CanRenderTiledTexture(this, Entity::TileMode::kClamp);
}

TEST_P(AiksTest, CanRenderTiledTextureClampAsync) {
CanRenderTiledTexture(this, Entity::TileMode::kClamp, /*async_submit=*/true);
}

TEST_P(AiksTest, CanRenderTiledTextureRepeat) {
CanRenderTiledTexture(this, Entity::TileMode::kRepeat);
}
Expand All @@ -232,7 +223,7 @@ TEST_P(AiksTest, CanRenderTiledTextureDecal) {
}

TEST_P(AiksTest, CanRenderTiledTextureClampWithTranslate) {
CanRenderTiledTexture(this, Entity::TileMode::kClamp, /*async_submit=*/false,
CanRenderTiledTexture(this, Entity::TileMode::kClamp,
Matrix::MakeTranslation({172.f, 172.f, 0.f}));
}

Expand Down
5 changes: 5 additions & 0 deletions impeller/aiks/testing/context_mock.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ class ContextMock : public Context {
(),
(const, override));

MOCK_METHOD(const std::shared_ptr<CommandQueue>&,
GetCommandQueue,
(),
(const override));

MOCK_METHOD(void, Shutdown, (), (override));
};

Expand Down
4 changes: 4 additions & 0 deletions impeller/aiks/testing/context_spy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ std::shared_ptr<ContextMock> ContextSpy::MakeContext(
return real_context->GetPipelineLibrary();
});

ON_CALL(*mock_context, GetCommandQueue).WillByDefault([real_context]() {
return real_context->GetCommandQueue();
});

ON_CALL(*mock_context, CreateCommandBuffer)
.WillByDefault([real_context, shared_this]() {
auto real_buffer = real_context->CreateCommandBuffer();
Expand Down
17 changes: 15 additions & 2 deletions impeller/entity/contents/content_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,8 @@ ContentContext::ContentContext(
? std::make_shared<RenderTargetCache>(
context_->GetResourceAllocator())
: std::move(render_target_allocator)),
host_buffer_(HostBuffer::Create(context_->GetResourceAllocator())) {
host_buffer_(HostBuffer::Create(context_->GetResourceAllocator())),
pending_command_buffers_(std::make_unique<PendingCommandBuffers>()) {
if (!context_ || !context_->IsValid()) {
return;
}
Expand Down Expand Up @@ -477,9 +478,10 @@ fml::StatusOr<RenderTarget> ContentContext::MakeSubpass(
return fml::Status(fml::StatusCode::kUnknown, "");
}

if (!sub_command_buffer->EncodeAndSubmit(sub_renderpass)) {
if (!sub_renderpass->EncodeCommands()) {
return fml::Status(fml::StatusCode::kUnknown, "");
}
RecordCommandBuffer(std::move(sub_command_buffer));

return subpass_target;
}
Expand Down Expand Up @@ -532,4 +534,15 @@ void ContentContext::ClearCachedRuntimeEffectPipeline(
}
}

void ContentContext::RecordCommandBuffer(
std::shared_ptr<CommandBuffer> command_buffer) const {
pending_command_buffers_->command_buffers.push_back(
std::move(command_buffer));
}

void ContentContext::FlushCommandBuffers() const {
auto buffers = std::move(pending_command_buffers_->command_buffers);
GetContext()->GetCommandQueue()->Submit(buffers);
}

} // namespace impeller
12 changes: 12 additions & 0 deletions impeller/entity/contents/content_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "impeller/core/host_buffer.h"
#include "impeller/entity/entity.h"
#include "impeller/renderer/capabilities.h"
#include "impeller/renderer/command_buffer.h"
#include "impeller/renderer/pipeline.h"
#include "impeller/renderer/pipeline_descriptor.h"
#include "impeller/renderer/render_target.h"
Expand Down Expand Up @@ -268,6 +269,12 @@ using TiledTextureExternalPipeline =
TiledTextureFillExternalFragmentShader>;
#endif // IMPELLER_ENABLE_OPENGLES

// A struct used to isolate command buffer storage from the content
// context options to preserve const-ness.
struct PendingCommandBuffers {
std::vector<std::shared_ptr<CommandBuffer>> command_buffers;
};

/// Pipeline state configuration.
///
/// Each unique combination of these options requires a different pipeline state
Expand Down Expand Up @@ -715,6 +722,10 @@ class ContentContext {

void SetWireframe(bool wireframe);

void RecordCommandBuffer(std::shared_ptr<CommandBuffer> command_buffer) const;

void FlushCommandBuffers() const;

using SubpassCallback =
std::function<bool(const ContentContext&, RenderPass&)>;

Expand Down Expand Up @@ -1005,6 +1016,7 @@ class ContentContext {
#endif // IMPELLER_ENABLE_3D
std::shared_ptr<RenderTargetAllocator> render_target_cache_;
std::shared_ptr<HostBuffer> host_buffer_;
std::unique_ptr<PendingCommandBuffers> pending_command_buffers_;
bool wireframe_ = false;

ContentContext(const ContentContext&) = delete;
Expand Down
5 changes: 5 additions & 0 deletions impeller/entity/contents/content_context_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@

#include <cstdint>

#include "fml/logging.h"
#include "gtest/gtest.h"

#include "impeller/core/allocator.h"
#include "impeller/core/device_buffer_descriptor.h"
#include "impeller/entity/contents/content_context.h"
#include "impeller/geometry/color.h"
#include "impeller/renderer/capabilities.h"
#include "impeller/renderer/command_queue.h"
#include "impeller/renderer/pipeline.h"
#include "impeller/renderer/pipeline_descriptor.h"

Expand Down Expand Up @@ -52,6 +54,9 @@ class FakeContext : public Context {
std::shared_ptr<PipelineLibrary> GetPipelineLibrary() const {
return nullptr;
}
const std::shared_ptr<CommandQueue>& GetCommandQueue() const {
FML_UNREACHABLE();
}
std::shared_ptr<CommandBuffer> CreateCommandBuffer() const { return nullptr; }
void Shutdown() {}

Expand Down
11 changes: 6 additions & 5 deletions impeller/entity/entity_pass.cc
Original file line number Diff line number Diff line change
Expand Up @@ -387,11 +387,12 @@ bool EntityPass::Render(ContentContext& renderer,
blit_pass->AddCopy(
offscreen_target.GetRenderTarget().GetRenderTargetTexture(),
root_render_target.GetRenderTargetTexture());
if (!command_buffer->EncodeAndSubmit(
blit_pass, renderer.GetContext()->GetResourceAllocator())) {
if (!blit_pass->EncodeCommands(
renderer.GetContext()->GetResourceAllocator())) {
VALIDATION_LOG << "Failed to encode root pass blit command.";
return false;
}
renderer.RecordCommandBuffer(std::move(command_buffer));
} else {
auto render_pass = command_buffer->CreateRenderPass(root_render_target);
render_pass->SetLabel("EntityPass Root Render Pass");
Expand All @@ -415,10 +416,11 @@ bool EntityPass::Render(ContentContext& renderer,
}
}

if (!command_buffer->EncodeAndSubmit(render_pass)) {
if (!render_pass->EncodeCommands()) {
VALIDATION_LOG << "Failed to encode root pass command buffer.";
return false;
}
renderer.RecordCommandBuffer(std::move(command_buffer));
}

return true;
Expand Down Expand Up @@ -855,8 +857,7 @@ bool EntityPass::OnRender(
collapsed_parent_pass) const {
TRACE_EVENT0("impeller", "EntityPass::OnRender");

const std::shared_ptr<Context>& context = renderer.GetContext();
InlinePassContext pass_context(context, pass_target,
InlinePassContext pass_context(renderer, pass_target,
GetTotalPassReads(renderer), GetElementCount(),
collapsed_parent_pass);
if (!pass_context.IsValid()) {
Expand Down
3 changes: 2 additions & 1 deletion impeller/entity/geometry/point_field_geometry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,10 @@ GeometryResult PointFieldGeometry::GetPositionBufferGPU(
output = geometry_uv_buffer;
}

if (!compute_pass->EncodeCommands() || !cmd_buffer->SubmitCommands()) {
if (!compute_pass->EncodeCommands()) {
return {};
}
renderer.RecordCommandBuffer(std::move(cmd_buffer));

return {
.type = PrimitiveType::kTriangle,
Expand Down
19 changes: 9 additions & 10 deletions impeller/entity/inline_pass_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,20 @@
#include "impeller/base/allocation.h"
#include "impeller/base/validation.h"
#include "impeller/core/formats.h"
#include "impeller/entity/contents/content_context.h"
#include "impeller/entity/entity_pass_target.h"
#include "impeller/renderer/command_buffer.h"
#include "impeller/renderer/texture_mipmap.h"

namespace impeller {

InlinePassContext::InlinePassContext(
std::shared_ptr<Context> context,
const ContentContext& renderer,
EntityPassTarget& pass_target,
uint32_t pass_texture_reads,
uint32_t entity_count,
std::optional<RenderPassResult> collapsed_parent_pass)
: context_(std::move(context)),
: renderer_(renderer),
pass_target_(pass_target),
entity_count_(entity_count),
is_collapsed_(collapsed_parent_pass.has_value()) {
Expand Down Expand Up @@ -67,15 +68,13 @@ bool InlinePassContext::EndPass() {
const std::shared_ptr<Texture>& target_texture =
GetPassTarget().GetRenderTarget().GetRenderTargetTexture();
if (target_texture->GetMipCount() > 1) {
fml::Status mip_status =
AddMipmapGeneration(command_buffer_, context_, target_texture);
fml::Status mip_status = AddMipmapGeneration(
command_buffer_, renderer_.GetContext(), target_texture);
if (!mip_status.ok()) {
return false;
}
}
if (!command_buffer_->SubmitCommands()) {
return false;
}
renderer_.RecordCommandBuffer(std::move(command_buffer_));

pass_ = nullptr;
command_buffer_ = nullptr;
Expand All @@ -97,7 +96,7 @@ InlinePassContext::RenderPassResult InlinePassContext::GetRenderPass(
/// time this method is called, but it'll also run if the pass has been
/// previously ended via `EndPass`.

command_buffer_ = context_->CreateCommandBuffer();
command_buffer_ = renderer_.GetContext()->CreateCommandBuffer();
if (!command_buffer_) {
VALIDATION_LOG << "Could not create command buffer.";
return {};
Expand All @@ -122,7 +121,7 @@ InlinePassContext::RenderPassResult InlinePassContext::GetRenderPass(
->second.resolve_texture != nullptr;
if (pass_count_ > 0 && is_msaa) {
result.backdrop_texture =
pass_target_.Flip(*context_->GetResourceAllocator());
pass_target_.Flip(*renderer_.GetContext()->GetResourceAllocator());
if (!result.backdrop_texture) {
VALIDATION_LOG << "Could not flip the EntityPass render target.";
}
Expand Down Expand Up @@ -174,7 +173,7 @@ InlinePassContext::RenderPassResult InlinePassContext::GetRenderPass(

result.pass = pass_;

if (!context_->GetCapabilities()->SupportsReadFromResolve() &&
if (!renderer_.GetContext()->GetCapabilities()->SupportsReadFromResolve() &&
result.backdrop_texture ==
result.pass->GetRenderTarget().GetRenderTargetTexture()) {
VALIDATION_LOG << "EntityPass backdrop restore configuration is not valid "
Expand Down
6 changes: 3 additions & 3 deletions impeller/entity/inline_pass_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

#include <cstdint>

#include "impeller/entity/contents/content_context.h"
#include "impeller/entity/entity_pass_target.h"
#include "impeller/renderer/context.h"
#include "impeller/renderer/render_pass.h"
#include "impeller/renderer/render_target.h"

namespace impeller {

Expand All @@ -22,7 +22,7 @@ class InlinePassContext {
};

InlinePassContext(
std::shared_ptr<Context> context,
const ContentContext& renderer,
EntityPassTarget& pass_target,
uint32_t pass_texture_reads,
uint32_t entity_count,
Expand All @@ -45,7 +45,7 @@ class InlinePassContext {
RenderPassResult GetRenderPass(uint32_t pass_depth);

private:
std::shared_ptr<Context> context_;
const ContentContext& renderer_;
EntityPassTarget& pass_target_;
std::shared_ptr<CommandBuffer> command_buffer_;
std::shared_ptr<RenderPass> pass_;
Expand Down
11 changes: 7 additions & 4 deletions impeller/golden_tests/vulkan_screenshotter.mm
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,13 @@

fml::AutoResetWaitableEvent latch;
success =
command_buffer->SubmitCommands([&latch](CommandBuffer::Status status) {
FML_CHECK(status == CommandBuffer::Status::kCompleted);
latch.Signal();
});
surface_context->GetCommandQueue()
->Submit({command_buffer},
[&latch](CommandBuffer::Status status) {
FML_CHECK(status == CommandBuffer::Status::kCompleted);
latch.Signal();
})
.ok();
FML_CHECK(success);
latch.Wait();

Expand Down
Loading