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

Don't pause Animator when lifecycle state paused #30969

Merged
fluttergithubbot merged 1 commit into from
Jan 27, 2022
Merged

Don't pause Animator when lifecycle state paused #30969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 0 additions & 26 deletions shell/common/animator.cc
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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.
Copy link
Member

@chinmaygarde chinmaygarde Jan 27, 2022

Choose a reason for hiding this comment

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

Do we still need this since the framework ought to handle frame scheduling now. If it is redundant, we can get rid of the RunNowOrPostTask call completely. We can do this in a later patch if necessary.

Copy link
Contributor

@dnfield dnfield Jan 27, 2022

Choose a reason for hiding this comment

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

(@ColdPaleLight feel free to add the waiting for tree to go green label if you think this is good to go, I'm holding off on adding it in case you plan to address this now)

Copy link
Member Author

@ColdPaleLight ColdPaleLight Jan 27, 2022

Choose a reason for hiding this comment

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

I'll do this in a later patch.

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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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