Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] add async command submission for blit pass. #48040

Merged
auto-submit merged 3 commits into from
Nov 15, 2023
Merged

[Impeller] add async command submission for blit pass. #48040

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bde0285
[Impeller] add async command submission for blit pass.
Nov 14, 2023
a6baa50
add unit test.
Nov 14, 2023
f11e4aa
Rename to encode and submit.
Nov 15, 2023
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
16 changes: 13 additions & 3 deletions impeller/aiks/aiks_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ TEST_P(AiksTest, CanRenderColorFilterWithInvertColorsDrawPaint) {
namespace {
bool GenerateMipmap(const std::shared_ptr<Context>& context,
std::shared_ptr<Texture> texture,
std::string label) {
std::string label,
bool async_submit) {
auto buffer = context->CreateCommandBuffer();
if (!buffer) {
return false;
Expand All @@ -174,18 +175,23 @@ bool GenerateMipmap(const std::shared_ptr<Context>& context,
return false;
}
pass->GenerateMipmap(std::move(texture), std::move(label));
if (async_submit) {
return buffer->SubmitCommandsAsync(pass, context->GetResourceAllocator());

@matanlurey matanlurey Nov 15, 2023

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.

Just based off of reading this, maybe it should be EncodeAndSubmitCommandsAsync.

@jonahwilliams jonahwilliams Nov 15, 2023

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.

Yeah that is a good suggestion, i will do that (not today, but when I update this tomrrow) 😄

@jonahwilliams jonahwilliams Nov 15, 2023

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

}

pass->EncodeCommands(context->GetResourceAllocator());
return buffer->SubmitCommands();
}

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");
GenerateMipmap(context, texture, "table_mountain_nx", async_submit);
Canvas canvas;
canvas.Scale(aiks_test->GetContentScale());
canvas.Translate({100.0f, 100.0f, 0});
Expand Down Expand Up @@ -232,6 +238,10 @@ 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 @@ -245,7 +255,7 @@ TEST_P(AiksTest, CanRenderTiledTextureDecal) {
}

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

Expand Down
5 changes: 0 additions & 5 deletions impeller/aiks/testing/context_mock.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ class CommandBufferMock : public CommandBuffer {

MOCK_METHOD(void, SetLabel, (const std::string& label), (const, override));

MOCK_METHOD(bool,
SubmitCommandsAsync,
(std::shared_ptr<RenderPass> render_pass),
(override));

MOCK_METHOD(std::shared_ptr<RenderPass>,
OnCreateRenderPass,
(RenderTarget render_target),
Expand Down
6 changes: 0 additions & 6 deletions impeller/aiks/testing/context_spy.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,6 @@ std::shared_ptr<ContextMock> ContextSpy::MakeContext(
return real_buffer->SetLabel(label);
});

ON_CALL(*spy, SubmitCommandsAsync)
.WillByDefault([real_buffer](
std::shared_ptr<RenderPass> render_pass) {
return real_buffer->SubmitCommandsAsync(std::move(render_pass));
});

ON_CALL(*spy, OnCreateRenderPass)
.WillByDefault(
[real_buffer, shared_this](const RenderTarget& render_target) {
Expand Down
10 changes: 3 additions & 7 deletions impeller/entity/entity_pass.cc
View file
Open in desktop

@matanlurey matanlurey Nov 15, 2023

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.

Why is this change unconditional? Do we always want to do this? I guess I'm confused here - is synchronous submission being removed after this lands?

@matanlurey matanlurey Nov 15, 2023

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.

^ What about this question? LGTM otherwise.

@jonahwilliams jonahwilliams Nov 15, 2023

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.

Ahh yes, we always want to do this. no, its not being removed because it is still useful for workloads that happen outside of the frame workload (like image upload/mipmaps).

There is also no way to create multiple render passes from a single cmd buffer and async submit them, which we don't need to do right now, but only the old API supports that technically.

Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,8 @@ bool EntityPass::Render(ContentContext& renderer,
offscreen_target.GetRenderTarget().GetRenderTargetTexture(),
root_render_target.GetRenderTargetTexture());

if (!blit_pass->EncodeCommands(
renderer.GetContext()->GetResourceAllocator())) {
if (!command_buffer->SubmitCommandsAsync(
blit_pass, renderer.GetContext()->GetResourceAllocator())) {
VALIDATION_LOG << "Failed to encode root pass blit command.";
return false;
}
Expand All @@ -399,15 +399,11 @@ bool EntityPass::Render(ContentContext& renderer,
}
}

if (!render_pass->EncodeCommands()) {
if (!command_buffer->SubmitCommandsAsync(render_pass)) {
VALIDATION_LOG << "Failed to encode root pass command buffer.";
return false;
}
}
if (!command_buffer->SubmitCommands()) {
VALIDATION_LOG << "Failed to submit root pass command buffer.";
return false;
}

return true;
}
Expand Down
9 changes: 8 additions & 1 deletion impeller/renderer/backend/metal/command_buffer_mtl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <Metal/Metal.h>

#include "flutter/fml/macros.h"
#include "impeller/core/allocator.h"
#include "impeller/renderer/command_buffer.h"

namespace impeller {
Expand Down Expand Up @@ -37,7 +38,13 @@ class CommandBufferMTL final : public CommandBuffer {
void OnWaitUntilScheduled() override;

// |CommandBuffer|
bool SubmitCommandsAsync(std::shared_ptr<RenderPass> render_pass) override;
bool SubmitCommandsAsync(
const std::shared_ptr<RenderPass>& render_pass) override;

// |CommandBuffer|
bool SubmitCommandsAsync(
const std::shared_ptr<BlitPass>& blit_ass,
const std::shared_ptr<Allocator>& allocator) override;

// |CommandBuffer|
std::shared_ptr<RenderPass> OnCreateRenderPass(RenderTarget target) override;
Expand Down
30 changes: 29 additions & 1 deletion impeller/renderer/backend/metal/command_buffer_mtl.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ static bool LogMTLCommandBufferErrorIfPresent(id<MTLCommandBuffer> buffer) {
}

bool CommandBufferMTL::SubmitCommandsAsync(
std::shared_ptr<RenderPass> render_pass) {
const std::shared_ptr<RenderPass>& render_pass) {
TRACE_EVENT0("impeller", "CommandBufferMTL::SubmitCommandsAsync");
if (!IsValid() || !render_pass->IsValid()) {
return false;
Expand Down Expand Up @@ -239,6 +239,34 @@ static bool LogMTLCommandBufferErrorIfPresent(id<MTLCommandBuffer> buffer) {
return true;
}

bool CommandBufferMTL::SubmitCommandsAsync(
const std::shared_ptr<BlitPass>& blit_pass,
const std::shared_ptr<Allocator>& allocator) {
if (!IsValid() || !blit_pass->IsValid()) {
return false;
}
auto context = context_.lock();
if (!context) {
return false;
}
[buffer_ enqueue];
auto buffer = buffer_;
buffer_ = nil;

auto worker_task_runner = ContextMTL::Cast(*context).GetWorkerTaskRunner();
auto task = fml::MakeCopyable(
[blit_pass, buffer, weak_context = context_, allocator]() {
auto context = weak_context.lock();
if (!blit_pass->EncodeCommands(allocator)) {
VALIDATION_LOG << "Failed to encode blit pass.";
return;
}
[buffer commit];
});
worker_task_runner->PostTask(task);
return true;
}

void CommandBufferMTL::OnWaitUntilScheduled() {}

std::shared_ptr<RenderPass> CommandBufferMTL::OnCreateRenderPass(
Expand Down
18 changes: 15 additions & 3 deletions impeller/renderer/command_buffer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@ void CommandBuffer::WaitUntilScheduled() {
}

bool CommandBuffer::SubmitCommandsAsync(
std::shared_ptr<RenderPass>
render_pass // NOLINT(performance-unnecessary-value-param)
) {
const std::shared_ptr<RenderPass>& render_pass) {
TRACE_EVENT0("impeller", "CommandBuffer::SubmitCommandsAsync");
if (!render_pass->IsValid() || !IsValid()) {
return false;
Expand All @@ -51,6 +49,20 @@ bool CommandBuffer::SubmitCommandsAsync(
return SubmitCommands(nullptr);
}

bool CommandBuffer::SubmitCommandsAsync(
const std::shared_ptr<BlitPass>& blit_pass,
const std::shared_ptr<Allocator>& allocator) {
TRACE_EVENT0("impeller", "CommandBuffer::SubmitCommandsAsync");
if (!blit_pass->IsValid() || !IsValid()) {
return false;
}
if (!blit_pass->EncodeCommands(allocator)) {
return false;
}

return SubmitCommands(nullptr);
}

std::shared_ptr<RenderPass> CommandBuffer::CreateRenderPass(
const RenderTarget& render_target) {
auto pass = OnCreateRenderPass(render_target);
Expand Down
14 changes: 13 additions & 1 deletion impeller/renderer/command_buffer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,19 @@ class CommandBuffer {
/// A command buffer may only be committed once.
///
[[nodiscard]] virtual bool SubmitCommandsAsync(
std::shared_ptr<RenderPass> render_pass);
const std::shared_ptr<RenderPass>& render_pass);

//----------------------------------------------------------------------------
/// @brief Schedule the command encoded by blit passes within this
/// command buffer on the GPU. The enqueing of this buffer is
/// performed immediately but encoding is pushed to a worker
/// thread if possible.
///
/// A command buffer may only be committed once.
///
[[nodiscard]] virtual bool SubmitCommandsAsync(
const std::shared_ptr<BlitPass>& blit_pass,
const std::shared_ptr<Allocator>& allocator);

//----------------------------------------------------------------------------
/// @brief Force execution of pending GPU commands.
Expand Down