Run the rasterizer on the platform thread #18841
Conversation
|
|
||
| class MessageLoop { | ||
| public: | ||
| FML_EMBEDDER_ONLY |
There was a problem hiding this comment.
This is to prevent the accidental use of fml::MessageLoop::GetCurrent() in a unit-test. The current message loop cannot be accessed from the cross platform engine components because message loop management is an embedder concern. But, if you want to use in in a unit-test, you can #define FML_USED_ON_EMBEDDER in your unit-test before including any other files.
But, you shouldn't have to do this in the first place. Instead of using raw threads, use fml::Thread which have their own task runners that you can access without resorting to removing this sanity check.
There was a problem hiding this comment.
TL;DR: Please don't remove this and use fml::Thread with the GetTaskRunner call on that thread to access task runners.
|
|
||
| class MessageLoop { | ||
| public: | ||
| FML_EMBEDDER_ONLY |
There was a problem hiding this comment.
This is to prevent the accidental use of fml::MessageLoop::GetCurrent() in a unit-test. The current message loop cannot be accessed from the cross platform engine components because message loop management is an embedder concern. But, if you want to use in in a unit-test, you can #define FML_USED_ON_EMBEDDER in your unit-test before including any other files.
But, you shouldn't have to do this in the first place. Instead of using raw threads, use fml::Thread which have their own task runners that you can access without resorting to removing this sanity check.
|
|
||
| class MessageLoop { | ||
| public: | ||
| FML_EMBEDDER_ONLY |
There was a problem hiding this comment.
TL;DR: Please don't remove this and use fml::Thread with the GetTaskRunner call on that thread to access task runners.
shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc
Outdated
Show resolved
Hide resolved
| TRACE_EVENT0("flutter", "AndroidExternalViewEmbedder::SubmitFrame"); | ||
| if (should_run_rasterizer_on_platform_thread_) { | ||
| // Don't submit the current frame if the frame will be resubmitted. | ||
| return false; |
There was a problem hiding this comment.
Is the result actually used? Or it is the same with iOS (the result is not used) If the result is not used, and we don't do anything based on a false result, I'd suggest to keep it as true.
There was a problem hiding this comment.
Done. SubmitFrame should return void. I will change that in a separate PR.
| // |ExternalViewEmbedder| | ||
| PostPrerollResult AndroidExternalViewEmbedder::PostPrerollAction( | ||
| fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) { | ||
| bool has_platform_views = composition_order_.size() > 0; |
There was a problem hiding this comment.
Does composition_order_ represent the platform views in this frame or existing platform views.
For example, if you have 2 platform views on the screen, and this frame is going to remove both platform views making a total of 0 platform views on the screen after this frame. Is the size of composition_order_ 0 or 2?
If it is 0, then we still need the threads to be merged. Because there are still 2 platform views on the screen and this frame is going to remove then, the removing process need to happen on the platform thread.
There was a problem hiding this comment.
Although having a kDefaultMergedLeaseDuration > 1 might already cover this case tho. Your call. On iOS, we sticked on the safe side to have the threads kept merged until we are sure there are no platform views both the previous frame and the current frame.
There was a problem hiding this comment.
I like that kDefaultMergedLeaseDuration > 1 covers this case.
There was a problem hiding this comment.
Cool. maybe we can put a comment on it.
Description
Runs the rasterizer on the platform thread if a platform view is in the current frame.
Related Issues
flutter/flutter#58292
Tests
I added the following tests: Unit tests
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.Breaking Change
Did any tests fail when you ran them? Please read handling breaking changes.