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

[Impeller] Support 'SyncMode' in 'CommandBuffer::SubmitCommands' #37772

Closed
wants to merge 7 commits into from
Closed

[Impeller] Support 'SyncMode' in 'CommandBuffer::SubmitCommands' #37772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1a1cc2a
[Impeller] Support ‘SyncMode’ in 'CommandBuffer:::SubmitCommands'
ColdPaleLight Nov 19, 2022
1b611f9
Clean code
ColdPaleLight Nov 19, 2022
ac84daa
Add a check
ColdPaleLight Nov 19, 2022
7f82399
Merge branch 'flutter:main' into support_sync_mode
ColdPaleLight Nov 22, 2022
347007b
Use CommandBuffer::SyncMode as param instead of bool
ColdPaleLight Nov 22, 2022
96f69a2
Clean code
ColdPaleLight Nov 22, 2022
b170635
Merge branch 'main' into support_sync_mode
ColdPaleLight Nov 24, 2022
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
6 changes: 4 additions & 2 deletions impeller/aiks/aiks_context.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,15 @@ const ContentContext& AiksContext::GetContentContext() const {
return *content_context_;
}

bool AiksContext::Render(const Picture& picture, RenderTarget& render_target) {
bool AiksContext::Render(const Picture& picture,
RenderTarget& render_target,
CommandBuffer::SyncMode sync_mode) {
if (!IsValid()) {
return false;
}

if (picture.pass) {
return picture.pass->Render(*content_context_, render_target);
return picture.pass->Render(*content_context_, render_target, sync_mode);
}

return true;
Expand Down
6 changes: 5 additions & 1 deletion impeller/aiks/aiks_context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "flutter/fml/macros.h"
#include "impeller/entity/contents/content_context.h"
#include "impeller/renderer/command_buffer.h"
#include "impeller/renderer/context.h"
#include "impeller/renderer/render_target.h"

Expand All @@ -28,7 +29,10 @@ class AiksContext {

const ContentContext& GetContentContext() const;

bool Render(const Picture& picture, RenderTarget& render_target);
bool Render(
const Picture& picture,
RenderTarget& render_target,
CommandBuffer::SyncMode sync_mode = CommandBuffer::SyncMode::kDontCare);

private:
std::shared_ptr<Context> context_;
Expand Down
11 changes: 7 additions & 4 deletions impeller/aiks/picture.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,21 @@ std::optional<Snapshot> Picture::Snapshot(AiksContext& context) {
.transform = Matrix::MakeTranslation(coverage.value().origin)};
}

std::shared_ptr<Image> Picture::ToImage(AiksContext& context, ISize size) {
std::shared_ptr<Image> Picture::ToImage(AiksContext& context,
ISize size,
CommandBuffer::SyncMode sync_mode) {
if (size.IsEmpty()) {
return nullptr;
}
auto texture = RenderToTexture(context, size);
auto texture = RenderToTexture(context, size, std::nullopt, sync_mode);
return texture ? std::make_shared<Image>(texture) : nullptr;
}

std::shared_ptr<Texture> Picture::RenderToTexture(
AiksContext& context,
ISize size,
std::optional<const Matrix> translate) {
std::optional<const Matrix> translate,
CommandBuffer::SyncMode sync_mode) {
FML_DCHECK(!size.IsEmpty());

pass->IterateAllEntities([&translate](auto& entity) -> bool {
Expand All @@ -64,7 +67,7 @@ std::shared_ptr<Texture> Picture::RenderToTexture(
return nullptr;
}

if (!context.Render(*this, target)) {
if (!context.Render(*this, target, sync_mode)) {
VALIDATION_LOG << "Could not render Picture to Texture.";
return nullptr;
}
Expand Down
8 changes: 6 additions & 2 deletions impeller/aiks/picture.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@ struct Picture {

std::optional<Snapshot> Snapshot(AiksContext& context);

std::shared_ptr<Image> ToImage(AiksContext& context, ISize size);
std::shared_ptr<Image> ToImage(
AiksContext& context,
ISize size,
CommandBuffer::SyncMode sync_mode = CommandBuffer::SyncMode::kDontCare);

private:
std::shared_ptr<Texture> RenderToTexture(
AiksContext& context,
ISize size,
std::optional<const Matrix> translate = std::nullopt);
std::optional<const Matrix> translate = std::nullopt,
CommandBuffer::SyncMode sync_mode = CommandBuffer::SyncMode::kDontCare);
};

} // namespace impeller
31 changes: 19 additions & 12 deletions impeller/entity/entity_pass.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ static RenderTarget CreateRenderTarget(ContentContext& renderer,
}

bool EntityPass::Render(ContentContext& renderer,
const RenderTarget& render_target) const {
const RenderTarget& render_target,
CommandBuffer::SyncMode sync_mode) const {
if (reads_from_pass_texture_ > 0) {
auto offscreen_target =
CreateRenderTarget(renderer, render_target.GetRenderTargetSize(), true);
Expand Down Expand Up @@ -214,15 +215,15 @@ bool EntityPass::Render(ContentContext& renderer,
if (!render_pass->EncodeCommands()) {
return false;
}
if (!command_buffer->SubmitCommands()) {
if (!command_buffer->SubmitCommands(sync_mode)) {
return false;
}

return true;
}

return OnRender(renderer, render_target.GetRenderTargetSize(), render_target,
Point(), Point(), 0);
Point(), Point(), 0, 0, nullptr, sync_mode);
}

EntityPass::EntityResult EntityPass::GetEntityForElement(
Expand Down Expand Up @@ -378,15 +379,15 @@ struct StencilLayer {
size_t stencil_depth;
};

bool EntityPass::OnRender(
ContentContext& renderer,
ISize root_pass_size,
const RenderTarget& render_target,
Point position,
Point parent_position,
uint32_t pass_depth,
size_t stencil_depth_floor,
std::shared_ptr<Contents> backdrop_filter_contents) const {
bool EntityPass::OnRender(ContentContext& renderer,
ISize root_pass_size,
const RenderTarget& render_target,
Point position,
Point parent_position,
uint32_t pass_depth,
size_t stencil_depth_floor,
std::shared_ptr<Contents> backdrop_filter_contents,
CommandBuffer::SyncMode sync_mode) const {
TRACE_EVENT0("impeller", "EntityPass::OnRender");

auto context = renderer.GetContext();
Expand Down Expand Up @@ -548,6 +549,12 @@ bool EntityPass::OnRender(
}
}

if (sync_mode != CommandBuffer::SyncMode::kDontCare) {
FML_DCHECK(reads_from_pass_texture_ == 0);
FML_DCHECK(pass_context.IsActive());
pass_context.EndPass(sync_mode);
}

return true;
}

Expand Down
9 changes: 6 additions & 3 deletions impeller/entity/entity_pass.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ class EntityPass {

EntityPass* GetSuperpass() const;

bool Render(ContentContext& renderer,
const RenderTarget& render_target) const;
bool Render(
ContentContext& renderer,
const RenderTarget& render_target,
CommandBuffer::SyncMode = CommandBuffer::SyncMode::kDontCare) const;

void IterateAllEntities(const std::function<bool(Entity&)>& iterator);

Expand Down Expand Up @@ -109,7 +111,8 @@ class EntityPass {
Point parent_position,
uint32_t pass_depth,
size_t stencil_depth_floor = 0,
std::shared_ptr<Contents> backdrop_filter_contents = nullptr) const;
std::shared_ptr<Contents> backdrop_filter_contents = nullptr,
CommandBuffer::SyncMode = CommandBuffer::SyncMode::kDontCare) const;

std::vector<Element> elements_;

Expand Down
4 changes: 2 additions & 2 deletions impeller/entity/inline_pass_context.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ std::shared_ptr<Texture> InlinePassContext::GetTexture() {
return render_target_.GetRenderTargetTexture();
}

bool InlinePassContext::EndPass() {
bool InlinePassContext::EndPass(CommandBuffer::SyncMode sync_mode) {
if (!IsActive()) {
return true;
}
Expand All @@ -48,7 +48,7 @@ bool InlinePassContext::EndPass() {
return false;
}

if (!command_buffer_->SubmitCommands()) {
if (!command_buffer_->SubmitCommands(sync_mode)) {
return false;
}

Expand Down
4 changes: 3 additions & 1 deletion impeller/entity/inline_pass_context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#pragma once

#include "impeller/renderer/command_buffer.h"
#include "impeller/renderer/context.h"
#include "impeller/renderer/render_pass.h"
#include "impeller/renderer/render_target.h"
Expand All @@ -25,7 +26,8 @@ class InlinePassContext {
bool IsValid() const;
bool IsActive() const;
std::shared_ptr<Texture> GetTexture();
bool EndPass();
bool EndPass(
CommandBuffer::SyncMode sync_mode = CommandBuffer::SyncMode::kDontCare);
const RenderTarget& GetRenderTarget() const;
uint32_t GetPassCount() const;

Expand Down
16 changes: 15 additions & 1 deletion impeller/renderer/backend/gles/command_buffer_gles.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,22 @@ bool CommandBufferGLES::IsValid() const {
}

// |CommandBuffer|
bool CommandBufferGLES::OnSubmitCommands(CompletionCallback callback) {
bool CommandBufferGLES::OnSubmitCommands(SyncMode sync_mode,
CompletionCallback callback) {
const auto result = reactor_->React();
if (result) {
const auto& gl = reactor_->GetProcTable();
switch (sync_mode) {
case CommandBuffer::SyncMode::kWaitUntilScheduled:
gl.Flush();
break;
case CommandBuffer::SyncMode::kWaitUntilCompleted:
gl.Finish();
Copy link
Member

@bdero bdero Nov 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

So looking at #37709, a couple of ideas come to mind:

  • For GLES: Since the blit pass uses glReadPixels, and GL commands are enqueued serially, is it actually necessary to call glFinish before downloading the texture? My understanding is that glReadPixels will synchronously block until any previous commands which attach the texture have completed on the GPU. In any case, doing the glFlush doesn't hurt and is probably a good idea.
  • This is a situation where it seems like the underlying rendering backend should own the choice about how to handle synchronization; we have an opportunity to avoid burdening users of the render layer API with an unnecessary choice and subtle bugs that are difficult to track down when used incorrectly:
    • For Metal (assuming waitUntilScheduled + hazard tracking isn't working as a sufficient stopgap for this), synchronization can be automatically managed by the renderer layer using native synchronization primitives:
      1. Have every TextureMTL track an optional MTLEvent, say TextureMTL::readable_event_.
      2. When a Metal RenderPassMTL encoding begins, create one new MTLEvent.
      3. For each attached texture in the render pass descriptor, set/overwrite the TextureMTL::readable_event_ to the new one.
      4. At the end of RenderPassMTL encoding, encode a signaling command for the event with encodeSignalEvent.
      5. Prior to encoding the Metal blit pass containing the device->host transfer, encode a wait command with encodeWaitForEvent against the source TextureMTL's event (if one has been set).
    • For Vulkan, we can create a timeline semaphore for every attachable VkImage being tracked which lasts the lifetime of the VkImage, incrementing the attached texture semaphores during RenderPassVK (but I'd recommend holding off on this until the Vulkan backend is further along, of course).

The above approach can be lightly tweaked later on if/when we support parallel command encoding (for example, by making the MTLEvents handle manipulation thread safe). Also, we can get rid of the kWaitUntilScheduled hack later on by extending this approach to track whether read events have finished, so that commands which write to a texture will wait until the reads are done regardless of the encoding order across command buffers.

Sorry about the runaround here! This aspect of our API is a bit unresolved as we haven't thrown together/documented a strategy for handling device resource synchronization comprehensively yet.

FYI @chinmaygarde, I'm thinking now might be the time to clean up my old notes about this and do a write up detailing how we can achieve maximal parallel encoding without exposing any explicit synchronization primitives above the render layer. It's feeling like the right time to address this since it's been popping up in the discussions around Vulkan too.

Copy link
Member Author

@ColdPaleLight ColdPaleLight Nov 28, 2022

Choose a reason for hiding this comment

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

@bdero It looks like you don't want to change the API, and expect to be able to solve the problem differently on different backends.

I guess we can further discuss the following two scenarios encountered on developing ui.Image.toByteData.

Scenario 1: Render the display list to texture on the Raster thread, and access the texture on the IO thread.

final ui.PictureRecorder recorder = ui.PictureRecorder();
final ui.Canvas canvas = ui.Canvas(recorder);
canvas.drawRect(const Rect.fromLTWH(0.0, 0.0, 100, 100), Paint()..color = Colors.red);
final ui.Picture picture = recorder.endRecording();
ui.Image image = await picture.toImage(100, 100); // render picture to texture on raster thread
ByteData? byteData = await image.toByteData(); // use texture on IO thread

OpenGLES

On the backend of OpenGLES, when the IO thread accesses the texture, we cannot know whether the command that the Raster thread renders the content to the texture is executed or not.

In this PR, I want to do a glFinish on the raster thread to solve this problem (I'm not sure if using glFlush here will also solve the problem). If we don't want developers to specify SyncMode explicitly, we need to introduce a synchronization mechanism, such as glFenceSync. However, these API are only available in GLES 3.0, not in GLES2.0.

Metal

In this scenario, it seems that Metal will not encounter this problem, because the commands corresponding to render to texture and access texture are all in the same CommandQueue, and the order in which they enter the queue in this scenario is determined (if I am wrong , please correct me).

Vulkan

TODO

Scenario 2: Blit operation to copy texture to buffer. We need to ensure that the cpu buffer has content after the blit operation API call ends.

OpenGLES

As you said, the implementation of OpenGLES uses glReadPixels, so there is no need to call glFinish.

Metal

We need to ensure that after the blit operation is completed, the cpu buffer already has data. This PR proposes to use SyncMode to ensure that if the API is not changed, we can also implement the corresponding functions through CompletionCallback, but it is not very convenient for developers to use.

Vulkan

TODO

Summarize

In scenario 1, if the OpenGLES backend does not have SyncMode, it seems that there is no way to ensure that the logic is correct, because OpenGLES 2.0 does not support glFenceSync.

In Scenario 2, Introducing SyncMode can make things sampler, otherwise CompletionCallback and fml::AutoResetWaitableEvent must be used which is a little complicated.

Do you have any thoughts on the above two questions? Thanks!

Copy link
Member

@bdero bdero Nov 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

On the backend of OpenGLES, when the IO thread accesses the texture, we cannot know whether the command that the Raster thread renders the content to the texture is executed or not.

The render pass and blit pass should both be encoded on the raster thread in serial. Would there be any issues with having picture.toImage and image.toByteData enqueue raster tasks with the same priority?

If we don't want developers to specify SyncMode explicitly, we need to introduce a synchronization mechanism, such as glFenceSync. However, these API are only available in GLES 3.0, not in GLES2.0.

Given the behavior of glReadPixels and the fact that GLES 2 commands should be encoded serially anyway (as the GL context is not guaranteed thread safe), I don't think there's a synchronization issue lurking between the framebuffer write and texture read that needs to be resolved.

The glReadPixels GLES 2 docs indicates that glReadPixels waits for completion of all previous commands that write to framebuffers:

glFinish does not return until the effects of all previously called GL commands are complete.
Such effects include all changes to GL state, all changes to connection state, and all
changes to the frame buffer contents.

Interestingly, the GLES 1.1 docs happen to be more explicit about it, but the description itself is really no different -- it would have been nice if this helpful note had survived to later specs. :)

glFinish is NOT required before a call to eglSwapBuffers or glReadPixels.
glFinish can take some time and for performance reasons it is best to use this function
infrequently and only when necessary.

Metal
We need to ensure that after the blit operation is completed, the cpu buffer already has data. This PR proposes to use SyncMode to ensure that if the API is not changed, we can also implement the corresponding functions through CompletionCallback, but it is not very convenient for developers to use.

While blocking the raster thread will be unavoidable for GL, using a completion callback for device->host transfers would allow us to avoid blocking the raster thread unnecessarily on Metal, Vulkan, and most other backends we're likely to implement over time (for Vulkan, we could invoke the callback from a worker thread after waiting on a fence, for example).

Copy link
Member Author

@ColdPaleLight ColdPaleLight Nov 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Given the behavior of glReadPixels and the fact that GLES 2 commands should be encoded serially anyway (as the GL context is not guaranteed thread safe), I don't think there's a synchronization issue lurking between the framebuffer write and texture read that needs to be resolved.

I think things are a little bit different here. There are two threads here, and each thread has its own EGLContext (the relationship between the two EGLContexts is a shared context). In this case, there is no way to guarantee that the render of the Raster thread must occur before the IO accesses the texture. And this is not just theoretical, in fact, I encountered this problem in the development of ui.Image.toByteData, it really happened.

That is to say, if the content is rendered on the raster thread, and then glReadPixels is called on the io thread, there is no guarantee that the expected content can be read.

Ahh, I see. You mean "if the render pass and blit pass both be encoded on the raster thread in serial, then there is no more synchronization issue".

Yes, that's right.

The render pass and blit pass should both be encoded on the raster thread in serial. Would there be any issues with having picture.toImage and image.toByteData enqueue raster tasks with the same priority?

That sounds good, I'll give it a try :)

Copy link
Member

@bdero bdero Nov 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

There are two threads here, and each thread has its own EGLContext (the relationship between the two EGLContexts is a shared context).

Ah, I wasn't aware we were making a second context on the IO thread. Do you foresee any downside to just using one context and performing all of the GLES activity on the raster thread given the lack of appropriate synchronization primitives in GLES 2? We could always add a multi-context solution later on that gets used in the presence of GLES 3 procs, but I suspect the overhead savings would be somewhat negligible.

Ahh, I see. You mean "if the render pass and blit pass both be encoded on the raster thread in serial, then there is no more synchronization issue".

Indeed!

Copy link
Member Author

@ColdPaleLight ColdPaleLight Nov 29, 2022

Choose a reason for hiding this comment

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

Do you foresee any downside to just using one context and performing all of the GLES activity on the raster thread given the lack of appropriate synchronization primitives in GLES 2?

Since the context of the raster thread is used for rendering, and the context of the io thread is used for texture uploading, I am worried that all gl activity on the raster thread will cause jank.

Copy link
Member

@bdero bdero Nov 29, 2022

Choose a reason for hiding this comment

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

Ah, right. Sorry, I meant more with respect to the device->host transfer. Performing new resource host->device transfers on the IO thread seems perfectly reasonable and safe.

break;
case CommandBuffer::SyncMode::kDontCare:
break;
}
}
if (callback) {
callback(result ? CommandBuffer::Status::kCompleted
: CommandBuffer::Status::kError);
Expand Down
3 changes: 2 additions & 1 deletion impeller/renderer/backend/gles/command_buffer_gles.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ class CommandBufferGLES final : public CommandBuffer {
bool IsValid() const override;

// |CommandBuffer|
bool OnSubmitCommands(CompletionCallback callback) override;
bool OnSubmitCommands(SyncMode sync_mode,
CompletionCallback callback) override;

// |CommandBuffer|
std::shared_ptr<RenderPass> OnCreateRenderPass(RenderTarget target) override;
Expand Down
4 changes: 3 additions & 1 deletion impeller/renderer/backend/gles/proc_table_gles.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ struct GLProc {
PROC(UseProgram); \
PROC(VertexAttribPointer); \
PROC(Viewport); \
PROC(ReadPixels);
PROC(ReadPixels); \
PROC(Flush); \
PROC(Finish);

#define FOR_EACH_IMPELLER_GLES3_PROC(PROC) PROC(BlitFramebuffer);

Expand Down
3 changes: 2 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 @@ -31,7 +31,8 @@ class CommandBufferMTL final : public CommandBuffer {
bool IsValid() const override;

// |CommandBuffer|
bool OnSubmitCommands(CompletionCallback callback) override;
bool OnSubmitCommands(SyncMode sync_mode,
CompletionCallback callback) override;

// |CommandBuffer|
std::shared_ptr<RenderPass> OnCreateRenderPass(RenderTarget target) override;
Expand Down
14 changes: 11 additions & 3 deletions 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 @@ -151,7 +151,8 @@ static bool LogMTLCommandBufferErrorIfPresent(id<MTLCommandBuffer> buffer) {
return CommandBufferMTL::Status::kError;
}

bool CommandBufferMTL::OnSubmitCommands(CompletionCallback callback) {
bool CommandBufferMTL::OnSubmitCommands(SyncMode sync_mode,
CompletionCallback callback) {
if (callback) {
[buffer_
addCompletedHandler:^(id<MTLCommandBuffer> buffer) {
Expand All @@ -162,9 +163,16 @@ static bool LogMTLCommandBufferErrorIfPresent(id<MTLCommandBuffer> buffer) {
callback(ToCommitResult(buffer.status));
}];
}

[buffer_ commit];
[buffer_ waitUntilScheduled];

switch (sync_mode) {
case SyncMode::kWaitUntilCompleted:
[buffer_ waitUntilCompleted];
case SyncMode::kWaitUntilScheduled:
case SyncMode::kDontCare:
[buffer_ waitUntilScheduled];
}

buffer_ = nil;
return true;
}
Expand Down
3 changes: 2 additions & 1 deletion impeller/renderer/backend/vulkan/command_buffer_vk.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ bool CommandBufferVK::IsValid() const {
return is_valid_;
}

bool CommandBufferVK::OnSubmitCommands(CompletionCallback callback) {
bool CommandBufferVK::OnSubmitCommands(SyncMode sync_mode,
CompletionCallback callback) {
bool submit = fenced_command_buffer_->Submit();
if (callback) {
callback(submit ? CommandBuffer::Status::kCompleted
Expand Down
3 changes: 2 additions & 1 deletion impeller/renderer/backend/vulkan/command_buffer_vk.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ class CommandBufferVK final : public CommandBuffer {
bool IsValid() const override;

// |CommandBuffer|
bool OnSubmitCommands(CompletionCallback callback) override;
bool OnSubmitCommands(SyncMode sync_mode,
CompletionCallback callback) override;

// |CommandBuffer|
std::shared_ptr<RenderPass> OnCreateRenderPass(RenderTarget target) override;
Expand Down
13 changes: 9 additions & 4 deletions impeller/renderer/command_buffer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ CommandBuffer::CommandBuffer(std::weak_ptr<const Context> context)

CommandBuffer::~CommandBuffer() = default;

bool CommandBuffer::SubmitCommands(const CompletionCallback& callback) {
bool CommandBuffer::SubmitCommands(SyncMode sync_mode,
const CompletionCallback& callback) {
TRACE_EVENT0("impeller", "CommandBuffer::SubmitCommands");
if (!IsValid()) {
// Already committed or was never valid. Either way, this is caller error.
Expand All @@ -25,11 +26,15 @@ bool CommandBuffer::SubmitCommands(const CompletionCallback& callback) {
}
return false;
}
return OnSubmitCommands(callback);
return OnSubmitCommands(sync_mode, callback);
}

bool CommandBuffer::SubmitCommands(const CompletionCallback& callback) {
return SubmitCommands(SyncMode::kDontCare, callback);
}

bool CommandBuffer::SubmitCommands() {
return SubmitCommands(nullptr);
bool CommandBuffer::SubmitCommands(SyncMode sync_mode) {
return SubmitCommands(sync_mode, nullptr);
}

std::shared_ptr<RenderPass> CommandBuffer::CreateRenderPass(
Expand Down
14 changes: 12 additions & 2 deletions impeller/renderer/command_buffer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ class CommandBuffer {
kCompleted,
};

enum class SyncMode {
kDontCare,
kWaitUntilScheduled,
kWaitUntilCompleted,
};

using CompletionCallback = std::function<void(Status)>;

virtual ~CommandBuffer();
Expand All @@ -61,9 +67,12 @@ class CommandBuffer {
///
/// @param[in] callback The completion callback.
///
[[nodiscard]] bool SubmitCommands(SyncMode sync_mode,
const CompletionCallback& callback);

[[nodiscard]] bool SubmitCommands(const CompletionCallback& callback);

[[nodiscard]] bool SubmitCommands();
[[nodiscard]] bool SubmitCommands(SyncMode sync_mode = SyncMode::kDontCare);

//----------------------------------------------------------------------------
/// @brief Create a render pass to record render commands into.
Expand Down Expand Up @@ -100,7 +109,8 @@ class CommandBuffer {

virtual std::shared_ptr<BlitPass> OnCreateBlitPass() const = 0;

[[nodiscard]] virtual bool OnSubmitCommands(CompletionCallback callback) = 0;
[[nodiscard]] virtual bool OnSubmitCommands(SyncMode sync_mode,
CompletionCallback callback) = 0;

virtual std::shared_ptr<ComputePass> OnCreateComputePass() const = 0;

Expand Down
3 changes: 2 additions & 1 deletion impeller/renderer/renderer_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,8 @@ TEST_P(RendererTest, CanBlitTextureToBuffer) {

pass->EncodeCommands(context->GetResourceAllocator());

if (!buffer->SubmitCommands()) {
if (!buffer->SubmitCommands(
CommandBuffer::SyncMode::kWaitUntilCompleted)) {
return false;
}
}
Expand Down
Loading