Skip to content

Commit

Permalink
Don't pause Animator when lifecycle state paused (flutter#30969)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ColdPaleLight
ColdPaleLight authored Jan 27, 2022
1 parent c5ee2e4 commit 46bea73
Show file tree
Hide file tree
Showing 7 changed files with 6 additions and 177 deletions.
26 changes: 0 additions & 26 deletions shell/common/animator.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,25 +45,6 @@ Animator::Animator(Delegate& delegate,

Animator::~Animator() = default;

void Animator::Stop() {
paused_ = true;
}

void Animator::Start() {
if (!paused_) {
return;
}

paused_ = false;
RequestFrame();
}

// Indicate that screen dimensions will be changing in order to force rendering
// of an updated frame even if the animator is currently paused.
void Animator::SetDimensionChangePending() {
dimension_change_pending_ = true;
}

void Animator::EnqueueTraceFlowId(uint64_t trace_flow_id) {
fml::TaskRunner::RunNowOrPostTask(
task_runners_.GetUITaskRunner(),
Expand Down Expand Up @@ -179,10 +160,6 @@ void Animator::BeginFrame(

void Animator::Render(std::unique_ptr<flutter::LayerTree> layer_tree) {
has_rendered_ = true;
if (dimension_change_pending_ &&
layer_tree->frame_size() != last_layer_tree_size_) {
dimension_change_pending_ = false;
}
last_layer_tree_size_ = layer_tree->frame_size();

if (!frame_timings_recorder_) {
Expand Down Expand Up @@ -232,9 +209,6 @@ void Animator::RequestFrame(bool regenerate_layer_tree) {
if (regenerate_layer_tree) {
regenerate_layer_tree_ = true;
}
if (paused_ && !dimension_change_pending_) {
return;
}

if (!pending_frame_semaphore_.TryWait()) {
// Multiple calls to Animator::RequestFrame will still result in a
Expand Down
8 changes: 0 additions & 8 deletions shell/common/animator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,6 @@ class Animator final {
void ScheduleSecondaryVsyncCallback(uintptr_t id,
const fml::closure& callback);

void Start();

void Stop();

void SetDimensionChangePending();

// Enqueue |trace_flow_id| into |trace_flow_ids_|. The flow event will be
// ended at either the next frame, or the next vsync interval with no active
// active rendering.
Expand Down Expand Up @@ -112,11 +106,9 @@ class Animator final {
std::shared_ptr<LayerTreePipeline> layer_tree_pipeline_;
fml::Semaphore pending_frame_semaphore_;
LayerTreePipeline::ProducerContinuation producer_continuation_;
bool paused_ = true;
bool regenerate_layer_tree_ = false;
bool frame_scheduled_ = false;
int notify_idle_task_id_ = 0;
bool dimension_change_pending_ = false;
SkISize last_layer_tree_size_ = {0, 0};
std::deque<uint64_t> trace_flow_ids_;
bool has_rendered_ = false;
Expand Down
30 changes: 0 additions & 30 deletions shell/common/animator_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,6 @@ TEST_F(ShellTest, VSyncTargetTime) {
fml::TaskRunner::RunNowOrPostTask(shell->GetTaskRunners().GetUITaskRunner(),
[engine = shell->GetEngine()]() {
if (engine) {
// Engine needs a surface for frames to
// be scheduled.
engine->OnOutputSurfaceCreated();
// this implies we can re-use the last
// frame to trigger begin frame rather
// than re-generating the layer tree.
Expand All @@ -116,30 +113,6 @@ TEST_F(ShellTest, VSyncTargetTime) {
ASSERT_FALSE(DartVMRef::IsInstanceRunning());
}

TEST_F(ShellTest, AnimatorStartsPaused) {
// Create all te prerequisites for a shell.
ASSERT_FALSE(DartVMRef::IsInstanceRunning());
auto settings = CreateSettingsForFixture();
TaskRunners task_runners = GetTaskRunnersForFixture();

auto shell = CreateShell(std::move(settings), task_runners,
/* simulate_vsync=*/true);
ASSERT_TRUE(DartVMRef::IsInstanceRunning());

auto configuration = RunConfiguration::InferFromSettings(settings);
ASSERT_TRUE(configuration.IsValid());

configuration.SetEntrypoint("emptyMain");

RunEngine(shell.get(), std::move(configuration));

ASSERT_FALSE(IsAnimatorRunning(shell.get()));

// teardown.
DestroyShell(std::move(shell), std::move(task_runners));
ASSERT_FALSE(DartVMRef::IsInstanceRunning());
}

TEST_F(ShellTest, AnimatorDoesNotNotifyIdleBeforeRender) {
FakeAnimatorDelegate delegate;
TaskRunners task_runners = {
Expand Down Expand Up @@ -176,7 +149,6 @@ TEST_F(ShellTest, AnimatorDoesNotNotifyIdleBeforeRender) {
// Validate it has not notified idle and start it. This will request a frame.
task_runners.GetUITaskRunner()->PostTask([&] {
ASSERT_FALSE(delegate.notify_idle_called_);
animator->Start();
// Immediately request a frame saying it can reuse the last layer tree to
// avoid more calls to BeginFrame by the animator.
animator->RequestFrame(false);
Expand Down Expand Up @@ -211,8 +183,6 @@ TEST_F(ShellTest, AnimatorDoesNotNotifyIdleBeforeRender) {
// Now it should notify idle. Make sure it is destroyed on the UI thread.
ASSERT_TRUE(delegate.notify_idle_called_);

// Stop and do one more flush so we can safely clean up on the UI thread.
animator->Stop();
task_runners.GetPlatformTaskRunner()->PostTask(flush_vsync_task);
latch.Wait();

Expand Down
46 changes: 3 additions & 43 deletions shell/common/engine.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ Engine::Engine(
settings_(std::move(settings)),
animator_(std::move(animator)),
runtime_controller_(std::move(runtime_controller)),
activity_running_(true),
have_surface_(false),
font_collection_(font_collection),
image_decoder_(task_runners, image_decoder_task_runner, io_manager),
task_runners_(std::move(task_runners)),
Expand Down Expand Up @@ -280,30 +278,11 @@ tonic::DartErrorHandleType Engine::GetUIIsolateLastError() {
return runtime_controller_->GetLastError();
}

void Engine::OnOutputSurfaceCreated() {
have_surface_ = true;
ScheduleFrame();
}

void Engine::OnOutputSurfaceDestroyed() {
have_surface_ = false;
StopAnimator();
}

void Engine::SetViewportMetrics(const ViewportMetrics& metrics) {
bool dimensions_changed =
viewport_metrics_.physical_height != metrics.physical_height ||
viewport_metrics_.physical_width != metrics.physical_width ||
viewport_metrics_.device_pixel_ratio != metrics.device_pixel_ratio;
viewport_metrics_ = metrics;
runtime_controller_->SetViewportMetrics(viewport_metrics_);
if (animator_) {
if (dimensions_changed) {
animator_->SetDimensionChangePending();
}
if (have_surface_) {
ScheduleFrame();
}
ScheduleFrame();
}
}

Expand Down Expand Up @@ -339,20 +318,12 @@ bool Engine::HandleLifecyclePlatformMessage(PlatformMessage* message) {
const auto& data = message->data();
std::string state(reinterpret_cast<const char*>(data.GetMapping()),
data.GetSize());
if (state == "AppLifecycleState.paused" ||
state == "AppLifecycleState.detached") {
activity_running_ = false;
StopAnimator();
} else if (state == "AppLifecycleState.resumed" ||
state == "AppLifecycleState.inactive") {
activity_running_ = true;
StartAnimatorIfPossible();
}

// Always schedule a frame when the app does become active as per API
// recommendation
// https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1622956-applicationdidbecomeactive?language=objc
if (state == "AppLifecycleState.resumed" && have_surface_) {
if (state == "AppLifecycleState.resumed" ||
state == "AppLifecycleState.inactive") {
ScheduleFrame();
}
runtime_controller_->SetLifecycleState(state);
Expand Down Expand Up @@ -455,16 +426,6 @@ void Engine::SetAccessibilityFeatures(int32_t flags) {
runtime_controller_->SetAccessibilityFeatures(flags);
}

void Engine::StopAnimator() {
animator_->Stop();
}

void Engine::StartAnimatorIfPossible() {
if (activity_running_ && have_surface_) {
animator_->Start();
}
}

std::string Engine::DefaultRouteName() {
if (!initial_route_.empty()) {
return initial_route_;
Expand All @@ -473,7 +434,6 @@ std::string Engine::DefaultRouteName() {
}

void Engine::ScheduleFrame(bool regenerate_layer_tree) {
StartAnimatorIfPossible();
animator_->RequestFrame(regenerate_layer_tree);
}

Expand Down
34 changes: 0 additions & 34 deletions shell/common/engine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -658,36 +658,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
///
std::optional<uint32_t> GetUIIsolateReturnCode();

//----------------------------------------------------------------------------
/// @brief Indicates to the Flutter application that it has obtained a
/// rendering surface. This is a good opportunity for the engine
/// to start servicing any outstanding frame requests from the
/// Flutter applications. Flutter application that have no
/// rendering concerns may never get a rendering surface. In such
/// cases, while their root isolate can perform as normal, any
/// frame requests made by them will never be serviced and layer
/// trees produced outside of frame workloads will be dropped.
///
/// Very close to when this call is made, the application can
/// expect the updated viewport metrics. Rendering only begins
/// when the Flutter application gets an output surface and a
/// valid set of viewport metrics.
///
/// @see `OnOutputSurfaceDestroyed`
///
void OnOutputSurfaceCreated();

//----------------------------------------------------------------------------
/// @brief Indicates to the Flutter application that a previously
/// acquired rendering surface has been lost. Further frame
/// requests will no longer be serviced and any layer tree
/// submitted for rendering will be dropped. If/when a new surface
/// is acquired, a new layer tree must be generated.
///
/// @see `OnOutputSurfaceCreated`
///
void OnOutputSurfaceDestroyed();

//----------------------------------------------------------------------------
/// @brief Updates the viewport metrics for the currently running Flutter
/// application. The viewport metrics detail the size of the
Expand Down Expand Up @@ -932,10 +902,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {

void SetNeedsReportTimings(bool value) override;

void StopAnimator();

void StartAnimatorIfPossible();

bool HandleLifecyclePlatformMessage(PlatformMessage* message);

bool HandleNavigationPlatformMessage(
Expand Down
22 changes: 3 additions & 19 deletions shell/common/shell.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -762,12 +762,9 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {
waiting_for_first_frame.store(true);
});

// TODO(91717): This probably isn't necessary. The engine should be able to
// handle things here via normal lifecycle messages.
// https://github.com/flutter/flutter/issues/91717
auto ui_task = [engine = engine_->GetWeakPtr()] {
if (engine) {
engine->OnOutputSurfaceCreated();
engine->ScheduleFrame();
}
};

Expand Down Expand Up @@ -863,24 +860,11 @@ void Shell::OnPlatformViewDestroyed() {
rasterizer->EnableThreadMergerIfNeeded();
rasterizer->Teardown();
}
// Step 3: Tell the IO thread to complete its remaining work.
// Step 2: Tell the IO thread to complete its remaining work.
fml::TaskRunner::RunNowOrPostTask(io_task_runner, io_task);
};

// TODO(91717): This probably isn't necessary. The engine should be able to
// handle things here via normal lifecycle messages.
// https://github.com/flutter/flutter/issues/91717
auto ui_task = [engine = engine_->GetWeakPtr()]() {
if (engine) {
engine->OnOutputSurfaceDestroyed();
}
};

// Step 1: Post a task onto the UI thread to tell the engine that its output
// surface is about to go away.
fml::TaskRunner::RunNowOrPostTask(task_runners_.GetUITaskRunner(), ui_task);

// Step 2: Post a task to the Raster thread (possibly this thread) to tell the
// Step 1: Post a task to the Raster thread (possibly this thread) to tell the
// rasterizer the output surface is going away.
fml::TaskRunner::RunNowOrPostTask(task_runners_.GetRasterTaskRunner(),
raster_task);
Expand Down
17 changes: 0 additions & 17 deletions shell/common/shell_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,23 +385,6 @@ void ShellTest::DestroyShell(std::unique_ptr<Shell> shell,
latch.Wait();
}

bool ShellTest::IsAnimatorRunning(Shell* shell) {
fml::AutoResetWaitableEvent latch;
bool running = false;
if (!shell) {
return running;
}
fml::TaskRunner::RunNowOrPostTask(
shell->GetTaskRunners().GetUITaskRunner(), [shell, &running, &latch]() {
if (shell && shell->engine_ && shell->engine_->animator_) {
running = !shell->engine_->animator_->paused_;
}
latch.Signal();
});
latch.Wait();
return running;
}

size_t ShellTest::GetLiveTrackedPathCount(
std::shared_ptr<VolatilePathTracker> tracker) {
return std::count_if(
Expand Down

0 comments on commit 46bea73

Please sign in to comment.