Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
5aa9df6
Got rid of the black frame by synchronizing the main thread with the
gaaclarke Jun 18, 2019
28f23c8
Fixed return result for WaitForFrameRender and added docstring.
gaaclarke Jun 26, 2019
d93a5bc
Fixed formatting.
gaaclarke Jun 26, 2019
6b55244
Made the bool parameter atomic since it was getting written to from
gaaclarke Jun 26, 2019
d246253
Removed AwaitTask since it was controversial and not absolutely
gaaclarke Jul 1, 2019
700f062
Added asserts to avoid deadlock.
gaaclarke Jul 1, 2019
bc9cd95
Updated documenation for WaitForFrameRender.
gaaclarke Jul 1, 2019
ac7266b
Made the CL much simplier by just waiting for the first frame instead
gaaclarke Jul 1, 2019
040c146
Update docstring.
gaaclarke Jul 1, 2019
48db249
Merge branch 'master' into black-frame2
gaaclarke Jul 2, 2019
8d63518
Updated timeout message.
gaaclarke Jul 2, 2019
8a676f7
Switched preprocessor macro for determining timeout for first frame.
gaaclarke Jul 2, 2019
2915fe9
missing semicolon, doh
gaaclarke Jul 2, 2019
016c96d
Removed captures of 'this'.
gaaclarke Jul 2, 2019
d040af8
Added unit test for wait for first frame.
gaaclarke Jul 2, 2019
41406b9
tweaked unit test.
gaaclarke Jul 2, 2019
6068985
Fixed typo
gaaclarke Jul 2, 2019
1a7c9e1
Updated doxygen format to match.
gaaclarke Jul 9, 2019
82d19a7
updated doxygen format again
gaaclarke Jul 9, 2019
9df3a65
1) Added Status class and made WaitForFirstFrame use it
gaaclarke Jul 9, 2019
b9d4c14
Added timeout to unit test.
gaaclarke Jul 9, 2019
b6d35b5
Added class docstring
gaaclarke Jul 9, 2019
4e25624
grammar typo
gaaclarke Jul 9, 2019
2b88ff8
Fixed typos
gaaclarke Jul 9, 2019
f0b3a40
Added status to the license golden.
gaaclarke Jul 9, 2019
fdfa45c
Updated the docstring to update the Status returns.
gaaclarke Jul 9, 2019
aa12852
Fixed comment formatting.
gaaclarke Jul 9, 2019
ad944f3
Moved to std::string_view.
gaaclarke Jul 9, 2019
d55bf99
Removed raw_code from Status.
gaaclarke Jul 9, 2019
f7e2b8c
Removed thread_host from test.
gaaclarke Jul 10, 2019
9c3b023
Removed enumeration values for StatusCodes.
gaaclarke Jul 10, 2019
e713db8
Merge branch 'master' of github.com:flutter/engine into black-frame2
gaaclarke Jul 10, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ FILE: ../../../flutter/fml/platform/win/native_library_win.cc
FILE: ../../../flutter/fml/platform/win/paths_win.cc
FILE: ../../../flutter/fml/platform/win/wstring_conversion.h
FILE: ../../../flutter/fml/size.h
FILE: ../../../flutter/fml/status.h
FILE: ../../../flutter/fml/synchronization/atomic_object.h
FILE: ../../../flutter/fml/synchronization/count_down_latch.cc
FILE: ../../../flutter/fml/synchronization/count_down_latch.h
Expand Down
81 changes: 81 additions & 0 deletions fml/status.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef FLUTTER_FML_STATUS_H_
#define FLUTTER_FML_STATUS_H_

#include <string_view>

namespace fml {

enum class StatusCode {
kOk,
kCancelled,
kUnknown,
kInvalidArgument,
kDeadlineExceeded,
kNotFound,
kAlreadyExists,
kPermissionDenied,
kResourceExhausted,
kFailedPrecondition,
kAborted,
kOutOfRange,
kUnimplemented,
kInternal,
kUnavailable,
kDataLoss,
kUnauthenticated
};

/// Class that represents the resolution of the execution of a procedure. This
/// is used similarly to how exceptions might be used, typically as the return
/// value to a synchronous procedure or an argument to an asynchronous callback.
class Status final {
public:
/// Creates an 'ok' status.
Status();

Status(fml::StatusCode code, std::string_view message);

fml::StatusCode code() const;

/// A noop that helps with static analysis tools if you decide to ignore an
/// error.
void IgnoreError() const;

/// @return 'true' when the code is kOk.
bool ok() const;

std::string_view message() const;

private:
fml::StatusCode code_;
std::string_view message_;
};

inline Status::Status() : code_(fml::StatusCode::kOk), message_() {}

inline Status::Status(fml::StatusCode code, std::string_view message)
: code_(code), message_(message) {}

inline fml::StatusCode Status::code() const {
return code_;
}

inline void Status::IgnoreError() const {
// noop
}

inline bool Status::ok() const {
return code_ == fml::StatusCode::kOk;
}

inline std::string_view Status::message() const {
return message_;
}

} // namespace fml

#endif // FLUTTER_FML_SIZE_H_
57 changes: 45 additions & 12 deletions shell/common/shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -454,18 +454,22 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {
// This is a synchronous operation because certain platforms depend on
// setup/suspension of all activities that may be interacting with the GPU in
// a synchronous fashion.

fml::AutoResetWaitableEvent latch;
auto gpu_task = fml::MakeCopyable([rasterizer = rasterizer_->GetWeakPtr(), //
surface = std::move(surface), //
&latch]() mutable {
if (rasterizer) {
rasterizer->Setup(std::move(surface));
}
// Step 3: All done. Signal the latch that the platform thread is waiting
// on.
latch.Signal();
});
auto gpu_task =
fml::MakeCopyable([& waiting_for_first_frame = waiting_for_first_frame_,
rasterizer = rasterizer_->GetWeakPtr(), //
surface = std::move(surface), //
&latch]() mutable {
if (rasterizer) {
rasterizer->Setup(std::move(surface));
}

waiting_for_first_frame.store(true);

// Step 3: All done. Signal the latch that the platform thread is
// waiting on.
latch.Signal();
});

// The normal flow executed by this method is that the platform thread is
// starting the sequence and waiting on the latch. Later the UI thread posts
Expand Down Expand Up @@ -787,10 +791,17 @@ void Shell::OnAnimatorDraw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline) {
FML_DCHECK(is_setup_);

task_runners_.GetGPUTaskRunner()->PostTask(
[rasterizer = rasterizer_->GetWeakPtr(),
[& waiting_for_first_frame = waiting_for_first_frame_,
&waiting_for_first_frame_condition = waiting_for_first_frame_condition_,
rasterizer = rasterizer_->GetWeakPtr(),
pipeline = std::move(pipeline)]() {
if (rasterizer) {
rasterizer->Draw(pipeline);

if (waiting_for_first_frame.load()) {
waiting_for_first_frame.store(false);
waiting_for_first_frame_condition.notify_all();
}
}
});
}
Expand Down Expand Up @@ -1226,4 +1237,26 @@ Rasterizer::Screenshot Shell::Screenshot(
return screenshot;
}

fml::Status Shell::WaitForFirstFrame(fml::TimeDelta timeout) {
FML_DCHECK(is_setup_);
if (task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread() ||
task_runners_.GetGPUTaskRunner()->RunsTasksOnCurrentThread()) {
return fml::Status(fml::StatusCode::kFailedPrecondition,
"WaitForFirstFrame called from thread that can't wait "
"because it is responsible for generating the frame.");
}

std::unique_lock<std::mutex> lock(waiting_for_first_frame_mutex_);
bool success = waiting_for_first_frame_condition_.wait_for(
lock, std::chrono::milliseconds(timeout.ToMilliseconds()),
[& waiting_for_first_frame = waiting_for_first_frame_] {
return !waiting_for_first_frame.load();
});
if (success) {
return fml::Status();
} else {
return fml::Status(fml::StatusCode::kDeadlineExceeded, "timeout");
}
}

} // namespace flutter
13 changes: 13 additions & 0 deletions shell/common/shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "flutter/fml/memory/ref_ptr.h"
#include "flutter/fml/memory/thread_checker.h"
#include "flutter/fml/memory/weak_ptr.h"
#include "flutter/fml/status.h"
#include "flutter/fml/synchronization/thread_annotations.h"
#include "flutter/fml/synchronization/waitable_event.h"
#include "flutter/fml/thread.h"
Expand Down Expand Up @@ -243,6 +244,15 @@ class Shell final : public PlatformView::Delegate,
Rasterizer::Screenshot Screenshot(Rasterizer::ScreenshotType type,
bool base64_encode);

//----------------------------------------------------------------------------
/// @brief Pauses the calling thread until the first frame is presented.
///
/// @return 'kOk' when the first frame has been presented before the timeout
/// successfully, 'kFailedPrecondition' if called from the GPU or UI
/// thread, 'kDeadlineExceeded' if there is a timeout.
///
fml::Status WaitForFirstFrame(fml::TimeDelta timeout);

private:
using ServiceProtocolHandler =
std::function<bool(const ServiceProtocol::Handler::ServiceProtocolMap&,
Expand Down Expand Up @@ -271,6 +281,9 @@ class Shell final : public PlatformView::Delegate,
uint64_t next_pointer_flow_id_ = 0;

bool first_frame_rasterized_ = false;
std::atomic<bool> waiting_for_first_frame_ = true;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: std::atomic_bool

Copy link
Member Author

@gaaclarke gaaclarke Jul 9, 2019

Choose a reason for hiding this comment

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

std::atomic<bool> is used later in the file.

std::mutex waiting_for_first_frame_mutex_;
std::condition_variable waiting_for_first_frame_condition_;

// Written in the UI thread and read from the GPU thread. Hence make it
// atomic.
Expand Down
82 changes: 82 additions & 0 deletions shell/common/shell_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -518,5 +518,87 @@ TEST_F(ShellTest, ReportTimingsIsCalledImmediatelyAfterTheFirstFrame) {
ASSERT_EQ(timestamps.size(), FrameTiming::kCount);
}

TEST_F(ShellTest, WaitForFirstFrame) {
Copy link
Member

Choose a reason for hiding this comment

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

Please create a unit test for a shell in a single threaded configuration as well as one in which the platform and GPU task runners are the same. The former configuration is used by the test runners and the latter by iOS when there is a platform view composited inline.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

auto settings = CreateSettingsForFixture();
std::unique_ptr<Shell> shell = CreateShell(settings);

// Create the surface needed by rasterizer
PlatformViewNotifyCreated(shell.get());

auto configuration = RunConfiguration::InferFromSettings(settings);
configuration.SetEntrypoint("emptyMain");

RunEngine(shell.get(), std::move(configuration));
PumpOneFrame(shell.get());
fml::Status result =
shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(1000));
ASSERT_TRUE(result.ok());
}

TEST_F(ShellTest, WaitForFirstFrameTimeout) {
auto settings = CreateSettingsForFixture();
std::unique_ptr<Shell> shell = CreateShell(settings);

// Create the surface needed by rasterizer
PlatformViewNotifyCreated(shell.get());

auto configuration = RunConfiguration::InferFromSettings(settings);
configuration.SetEntrypoint("emptyMain");

RunEngine(shell.get(), std::move(configuration));
fml::Status result =
shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(10));
ASSERT_EQ(result.code(), fml::StatusCode::kDeadlineExceeded);
}

TEST_F(ShellTest, WaitForFirstFrameMultiple) {
auto settings = CreateSettingsForFixture();
std::unique_ptr<Shell> shell = CreateShell(settings);

// Create the surface needed by rasterizer
PlatformViewNotifyCreated(shell.get());

auto configuration = RunConfiguration::InferFromSettings(settings);
configuration.SetEntrypoint("emptyMain");

RunEngine(shell.get(), std::move(configuration));
PumpOneFrame(shell.get());
fml::Status result =
shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(1000));
ASSERT_TRUE(result.ok());
for (int i = 0; i < 100; ++i) {
result = shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(1));
ASSERT_TRUE(result.ok());
}
}

/// Makes sure that WaitForFirstFrame works if we rendered a frame with the
/// single-thread setup.
TEST_F(ShellTest, WaitForFirstFrameInlined) {
Settings settings = CreateSettingsForFixture();
auto task_runner = GetThreadTaskRunner();
TaskRunners task_runners("test", task_runner, task_runner, task_runner,
task_runner);
std::unique_ptr<Shell> shell =
CreateShell(std::move(settings), std::move(task_runners));

// Create the surface needed by rasterizer
PlatformViewNotifyCreated(shell.get());

auto configuration = RunConfiguration::InferFromSettings(settings);
configuration.SetEntrypoint("emptyMain");

RunEngine(shell.get(), std::move(configuration));
PumpOneFrame(shell.get());
fml::AutoResetWaitableEvent event;
task_runner->PostTask([&shell, &event] {
fml::Status result =
shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(1000));
ASSERT_EQ(result.code(), fml::StatusCode::kFailedPrecondition);
event.Signal();
});
ASSERT_FALSE(event.WaitWithTimeout(fml::TimeDelta::FromMilliseconds(1000)));
}

} // namespace testing
} // namespace flutter
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,9 @@ - (void)viewWillAppear:(BOOL)animated {

// Only recreate surface on subsequent appearances when viewport metrics are known.
// First time surface creation is done on viewDidLayoutSubviews.
if (_viewportMetrics.physical_width)
if (_viewportMetrics.physical_width) {
[self surfaceUpdated:YES];
}
[[_engine.get() lifecycleChannel] sendMessage:@"AppLifecycleState.inactive"];

[super viewWillAppear:animated];
Expand Down Expand Up @@ -698,8 +699,22 @@ - (void)viewDidLayoutSubviews {

// This must run after updateViewportMetrics so that the surface creation tasks are queued after
// the viewport metrics update tasks.
if (firstViewBoundsUpdate)
if (firstViewBoundsUpdate) {
[self surfaceUpdated:YES];

flutter::Shell& shell = [_engine.get() shell];
fml::TimeDelta waitTime =
#if FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG
fml::TimeDelta::FromMilliseconds(200);
#else
fml::TimeDelta::FromMilliseconds(100);
#endif
if (shell.WaitForFirstFrame(waitTime).code() == fml::StatusCode::kDeadlineExceeded) {
FML_LOG(INFO) << "Timeout waiting for the first frame to render. This may happen in "
<< "unoptimized builds. If this is a release build, you should load a less "
<< "complex frame to avoid the timeout.";
}
}
}

- (void)viewSafeAreaInsetsDidChange {
Expand Down