[Embedder] Only call removeview callback when raster thread is done with the view #164571
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
7c315e7 to
5a238af
Compare
ddcc3ad to
30c93ac
Compare
| EmbedderConfigBuilder builder(context); | ||
| std::mutex engine_mutex; | ||
| UniqueEngine engine; | ||
| auto render_thread = CreateNewThread("custom_render_thread"); |
There was a problem hiding this comment.
Is this mimicking the UI thread? Should we call it the UI thread instead?
There was a problem hiding this comment.
It's mimicking raster thread:
builder.SetRenderTaskRunner(
&render_task_runner.GetFlutterTaskRunnerDescription());There was a problem hiding this comment.
This looks good to me, but please also get an approval from @dkwingsmt if they have the cycles to review this.
|
Interestingly, I remember that the reason I had to make the callback earlier than the raster thread is because I must not let the main thread block on raster threads, which is further because macOS's rendering mechanism makes raster threads block on main threads ( |
To elaborate on that: MacOS no longer blocks the raster thread. But even if it did, the waiting would simply need to account for it. Linux for example will block the raster thread during present once it has separate raster thread, but we have way to ensure raster thread progress while blocking main thread, so this can be done without deadlocking. The callback will post a "unblock" message on main thread and the main thread will be pumping flutter messages (ignoring platform main thread messages) until the "unblock" message comes. All desktop platforms now have resize synchronization implemented in unified way where we are able to only process flutter posted messages on main loop (ignoring other system messages). This makes it possible to block on |
…tests.cc Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>
4c538a5 to
25b73c0
Compare
|
Thank you for the explanation! That sounds great! |
|
(Feel free to add auto-submit when you feel ready.) |
I think want to test this on Linux first after separate raster thread landed. |
Fixes #164564 (comment)
This would ensure that raster thread is completely done with the view, i.e. it won't try to use the opengl context, which might be associated with view window. So the client can know for sure, that when the callback returns, it is safe to destroy the view and container window.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.