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

Conversation

@ColdPaleLight
Copy link
Member

@ColdPaleLight ColdPaleLight commented Nov 17, 2022

I'm working on flutter/flutter#110829, and I found that Impeller doesn't have an API to ensure that all commands are completed.

Let's take the following code as an example. In the following piece of code, we will draw the DisplayList into a Texture in the Raster thread, and then blit the Texture into the CPU Buffer on the IO thread. So we need to ensure that when we do the blit operation on the IO thread, the commands about the DisplayList drawn to the Texture have been completed in the GPU.

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);
ByteData? byteData = await image.toByteData();

Another place where we need to use this new API is "blit texture to buffer". We need to ensure that the blit operation has been executed on the gpu before using the cpu buffer.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the [CLA].
  • All existing and new tests are passing.

@ColdPaleLight ColdPaleLight requested review from bdero and chinmaygarde and removed request for chinmaygarde November 17, 2022 06:08

virtual std::shared_ptr<CommandBuffer> CreateCommandBuffer() const = 0;

virtual bool WaitUntilCommandsCompleted() = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this belongs on Context - you want to know when the commands from a particular buffer are completed right?

For example, should this wait for all commands from compute to finish as well?

Copy link
Member Author

@ColdPaleLight ColdPaleLight Nov 17, 2022

Choose a reason for hiding this comment

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

I tried adding a parameter to the SubmitCommands method of CommandBuffer to solve the problem. But I found that in some scenarios, developers can't call SubmitCommands themselves, or even they can't find the Command they want to wait for.

for example

std::shared_ptr<impeller::Image> image =
picture.ToImage(*context, render_target_size);
context->GetContext()->WaitUntilCommandsCompleted();

Copy link
Member Author

Choose a reason for hiding this comment

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

@dnfield Do you have any thoughts on this example of not being able to access the specific CommandsBuffer? Thanks!
Also cc @chinmaygarde

Copy link
Contributor

Choose a reason for hiding this comment

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

When you call EntityPass::Render from AiksContext::Render maybe there could be some parameter like a callback for when the texture is done?

Copy link
Contributor

Choose a reason for hiding this comment

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

EntityPass::Render knows the command buffer, and the command buffer has SubmitCommands(const CompletionCallback& callback).

Copy link
Member Author

@ColdPaleLight ColdPaleLight Nov 19, 2022

Choose a reason for hiding this comment

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

@dnfield Thank you very much! I file a new PR #37772, Any thoughts?

@chinmaygarde chinmaygarde changed the title Add a new API 'Context::WaitUntilCommandsCompleted' to ensure that commands are completed [Impeller] Add a new API 'Context::WaitUntilCommandsCompleted' to ensure that commands are completed. Nov 18, 2022
@ColdPaleLight
Copy link
Member Author

Closing it since there is a better solution #37772

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants