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

Commit 75f3a9c

Browse files
authored
Multiview pipeline Pt. 1: Skip illegal render calls (#49266)
This is one of a series of changes to reland #47239. This PR changes `Animator` so that if `Render` is not called after a `BeginFrame`, this call is ignored. Note that this is slightly different from #47239. Instead of saying that we should ultimately change this skip to an assertion, this PR aims to keep the skip as the final shape. This is because a while ago we (with @goderbauer and @loic-sharma) decided that `PlatformDispatcher` should contain as little logic as possible to allow testing, and instead serve as a minimal native function binding, which means that we should eventually move the code that validates calling convention to the engine. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent b404502 commit 75f3a9c

File tree

2 files changed

+25
-18
lines changed

2 files changed

+25
-18
lines changed

shell/common/animator.cc

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -143,15 +143,12 @@ void Animator::BeginFrame(
143143

144144
void Animator::Render(std::unique_ptr<flutter::LayerTree> layer_tree,
145145
float device_pixel_ratio) {
146-
has_rendered_ = true;
147-
148-
if (!frame_timings_recorder_) {
149-
// Framework can directly call render with a built scene.
150-
frame_timings_recorder_ = std::make_unique<FrameTimingsRecorder>();
151-
const fml::TimePoint placeholder_time = fml::TimePoint::Now();
152-
frame_timings_recorder_->RecordVsync(placeholder_time, placeholder_time);
153-
frame_timings_recorder_->RecordBuildStart(placeholder_time);
146+
// Animator::Render should be called after BeginFrame, which is indicated by
147+
// frame_timings_recorder_ being non-null. Otherwise, this call is ignored.
148+
if (frame_timings_recorder_ == nullptr) {
149+
return;
154150
}
151+
has_rendered_ = true;
155152

156153
TRACE_EVENT_WITH_FRAME_NUMBER(frame_timings_recorder_, "flutter",
157154
"Animator::Render", /*flow_id_count=*/0,

shell/common/animator_unittests.cc

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -158,20 +158,30 @@ TEST_F(ShellTest, AnimatorDoesNotNotifyIdleBeforeRender) {
158158
latch.Wait();
159159
ASSERT_FALSE(delegate.notify_idle_called_);
160160

161+
fml::AutoResetWaitableEvent render_latch;
161162
// Validate it has not notified idle and try to render.
162163
task_runners.GetUITaskRunner()->PostDelayedTask(
163164
[&] {
164165
ASSERT_FALSE(delegate.notify_idle_called_);
165-
auto layer_tree = std::make_unique<LayerTree>(LayerTree::Config(),
166-
SkISize::Make(600, 800));
167-
animator->Render(std::move(layer_tree), 1.0);
166+
EXPECT_CALL(delegate, OnAnimatorBeginFrame).WillOnce([&] {
167+
auto layer_tree = std::make_unique<LayerTree>(
168+
LayerTree::Config(), SkISize::Make(600, 800));
169+
animator->Render(std::move(layer_tree), 1.0);
170+
render_latch.Signal();
171+
});
172+
// Request a frame that builds a layer tree and renders a frame.
173+
// When the frame is rendered, render_latch will be signaled.
174+
animator->RequestFrame(true);
168175
task_runners.GetPlatformTaskRunner()->PostTask(flush_vsync_task);
169176
},
170177
// See kNotifyIdleTaskWaitTime in animator.cc.
171178
fml::TimeDelta::FromMilliseconds(60));
172179
latch.Wait();
180+
render_latch.Wait();
173181

174-
// Still hasn't notified idle because there has been no frame request.
182+
// A frame has been rendered, and the next frame request will notify idle.
183+
// But at the moment there isn't another frame request, therefore it still
184+
// hasn't notified idle.
175185
task_runners.GetUITaskRunner()->PostTask([&] {
176186
ASSERT_FALSE(delegate.notify_idle_called_);
177187
// False to avoid getting cals to BeginFrame that will request more frames
@@ -239,16 +249,16 @@ TEST_F(ShellTest, AnimatorDoesNotNotifyDelegateIfPipelineIsNotEmpty) {
239249

240250
for (int i = 0; i < 2; i++) {
241251
task_runners.GetUITaskRunner()->PostTask([&] {
252+
EXPECT_CALL(delegate, OnAnimatorBeginFrame).WillOnce([&] {
253+
auto layer_tree = std::make_unique<LayerTree>(LayerTree::Config(),
254+
SkISize::Make(600, 800));
255+
animator->Render(std::move(layer_tree), 1.0);
256+
begin_frame_latch.Signal();
257+
});
242258
animator->RequestFrame();
243259
task_runners.GetPlatformTaskRunner()->PostTask(flush_vsync_task);
244260
});
245261
begin_frame_latch.Wait();
246-
247-
PostTaskSync(task_runners.GetUITaskRunner(), [&] {
248-
auto layer_tree = std::make_unique<LayerTree>(LayerTree::Config(),
249-
SkISize::Make(600, 800));
250-
animator->Render(std::move(layer_tree), 1.0);
251-
});
252262
}
253263

254264
PostTaskSync(task_runners.GetUITaskRunner(), [&] { animator.reset(); });

0 commit comments

Comments
 (0)