From 61683c2ed7b92f1f20a5437e31a32e87a8879509 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Fri, 21 Jan 2022 23:55:59 +0800 Subject: [PATCH] Don't pause Animator when lifecycle state paused --- shell/common/animator.cc | 26 ----------------- shell/common/animator.h | 8 ------ shell/common/animator_unittests.cc | 30 ------------------- shell/common/engine.cc | 46 ++---------------------------- shell/common/engine.h | 34 ---------------------- shell/common/shell.cc | 22 ++------------ shell/common/shell_test.cc | 17 ----------- 7 files changed, 6 insertions(+), 177 deletions(-) diff --git a/shell/common/animator.cc b/shell/common/animator.cc index 8171990ca9114..48bcd6bfc4f69 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -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(), @@ -179,10 +160,6 @@ void Animator::BeginFrame( void Animator::Render(std::unique_ptr 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_) { @@ -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 diff --git a/shell/common/animator.h b/shell/common/animator.h index 1ae308bdbbf3f..b63fd29b85903 100644 --- a/shell/common/animator.h +++ b/shell/common/animator.h @@ -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. @@ -112,11 +106,9 @@ class Animator final { std::shared_ptr 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 trace_flow_ids_; bool has_rendered_ = false; diff --git a/shell/common/animator_unittests.cc b/shell/common/animator_unittests.cc index f23437f1acd99..fea09475bad9c 100644 --- a/shell/common/animator_unittests.cc +++ b/shell/common/animator_unittests.cc @@ -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. @@ -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 = { @@ -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); @@ -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(); diff --git a/shell/common/engine.cc b/shell/common/engine.cc index d1647ed232037..cccf646abd7f3 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -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)), @@ -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(); } } @@ -339,20 +318,12 @@ bool Engine::HandleLifecyclePlatformMessage(PlatformMessage* message) { const auto& data = message->data(); std::string state(reinterpret_cast(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); @@ -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_; @@ -473,7 +434,6 @@ std::string Engine::DefaultRouteName() { } void Engine::ScheduleFrame(bool regenerate_layer_tree) { - StartAnimatorIfPossible(); animator_->RequestFrame(regenerate_layer_tree); } diff --git a/shell/common/engine.h b/shell/common/engine.h index e140732d0cd6b..faa25e4c38144 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -658,36 +658,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { /// std::optional 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 @@ -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( diff --git a/shell/common/shell.cc b/shell/common/shell.cc index bc664f34ac605..f75fde53031e2 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -762,12 +762,9 @@ void Shell::OnPlatformViewCreated(std::unique_ptr 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(); } }; @@ -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); diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index 25cde8d470c39..4a01c96da7e7a 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -385,23 +385,6 @@ void ShellTest::DestroyShell(std::unique_ptr 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 tracker) { return std::count_if(