-
Notifications
You must be signed in to change notification settings - Fork 6k
EndFrame should be always called by rasterizer #19257
Conversation
@cyanglaz I verified this fixes the perf issue you were seeing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo nits. For testing, we need to make sure the existing tests related to platform views don't fail.
flow/embedded_views.h
Outdated
// This method provides the embedder a way to do additional tasks after | ||
// |SubmitFrame|. After invoking this method, the current task on the | ||
// TaskRunner should end immediately. | ||
// |SubmitFrame|. For example, merge tasks if `should_resubmit_frame` is true. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nits: merge task runners?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
TRACE_EVENT0("flutter", "IOSSurface::EndFrame"); | ||
FML_CHECK(platform_views_controller_ != nullptr); | ||
return platform_views_controller_->EndFrame(raster_thread_merger); | ||
return platform_views_controller_->EndFrame(raster_status, raster_thread_merger); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
platform_views_controller_->EndFrame(should_resubmit_frame, raster_thread_merger);
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
// causes|RasterStatus::kResubmit|. Check to make sure. | ||
FML_DCHECK(external_view_embedder != nullptr); | ||
external_view_embedder->EndFrame(raster_thread_merger_); | ||
if (surface_ != nullptr && surface_->GetExternalViewEmbedder() != nullptr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe also modify the https://github.com/flutter/engine/blob/master/shell/common/shell_unittests.cc#L472 test to make sure the should_resubmit_frame
flag is passed correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, didn't know there was that test
Description
EndFrame
should be called by the rasterizer as it is used to restore up state or recycle the overlay surfaces.Related Issues
Fixes flutter/flutter#60124
Tests
I added the following tests: updated 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.