diff --git a/flow/frame_timings.cc b/flow/frame_timings.cc index 2943e850e17d0..339374d77b837 100644 --- a/flow/frame_timings.cc +++ b/flow/frame_timings.cc @@ -254,30 +254,8 @@ const char* FrameTimingsRecorder::GetFrameNumberTraceArg() const { return frame_number_trace_arg_val_.c_str(); } -static const char* StateToString(FrameTimingsRecorder::State state) { -#ifndef NDEBUG - switch (state) { - case FrameTimingsRecorder::State::kUninitialized: - return "kUninitialized"; - case FrameTimingsRecorder::State::kVsync: - return "kVsync"; - case FrameTimingsRecorder::State::kBuildStart: - return "kBuildStart"; - case FrameTimingsRecorder::State::kBuildEnd: - return "kBuildEnd"; - case FrameTimingsRecorder::State::kRasterStart: - return "kRasterStart"; - case FrameTimingsRecorder::State::kRasterEnd: - return "kRasterEnd"; - }; - FML_UNREACHABLE(); -#endif - return ""; -} - void FrameTimingsRecorder::AssertInState(State state) const { - FML_DCHECK(state_ == state) << "Expected state " << StateToString(state) - << ", actual state " << StateToString(state_); + FML_DCHECK(state_ == state); } } // namespace flutter diff --git a/flow/frame_timings.h b/flow/frame_timings.h index ac5a7e470215e..c81bcc3362298 100644 --- a/flow/frame_timings.h +++ b/flow/frame_timings.h @@ -31,7 +31,6 @@ class FrameTimingsRecorder { public: /// Various states that the recorder can be in. When created the recorder is /// in an unitialized state and transtions in sequential order of the states. - // After adding an item to this enum, modify StateToString accordingly. enum class State : uint32_t { kUninitialized, kVsync, @@ -122,8 +121,6 @@ class FrameTimingsRecorder { /// /// Instead of adding a `GetState` method and asserting on the result, this /// method prevents other logic from relying on the state. - /// - /// In opt builds, this call is a no-op. void AssertInState(State state) const; private: diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index bd954d821046a..72f56f443b79e 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -26,8 +26,8 @@ namespace impeller { DirectionalGaussianBlurFilterContents::DirectionalGaussianBlurFilterContents() = default; -DirectionalGaussianBlurFilterContents::~ -DirectionalGaussianBlurFilterContents() = default; +DirectionalGaussianBlurFilterContents:: + ~DirectionalGaussianBlurFilterContents() = default; void DirectionalGaussianBlurFilterContents::SetSigma(Sigma sigma) { blur_sigma_ = sigma; diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index 310de114e377a..976b373d29d08 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -98,7 +98,7 @@ typedef CanvasPath Path; V(NativeStringAttribute::initSpellOutStringAttribute, 3) \ V(PlatformConfigurationNativeApi::DefaultRouteName, 0) \ V(PlatformConfigurationNativeApi::ScheduleFrame, 0) \ - V(PlatformConfigurationNativeApi::Render, 2) \ + V(PlatformConfigurationNativeApi::Render, 1) \ V(PlatformConfigurationNativeApi::UpdateSemantics, 1) \ V(PlatformConfigurationNativeApi::SetNeedsReportTimings, 1) \ V(PlatformConfigurationNativeApi::SetIsolateDebugName, 1) \ diff --git a/lib/ui/painting/image_dispose_unittests.cc b/lib/ui/painting/image_dispose_unittests.cc index 93600ca83c93d..0f8bb6d027062 100644 --- a/lib/ui/painting/image_dispose_unittests.cc +++ b/lib/ui/painting/image_dispose_unittests.cc @@ -5,7 +5,6 @@ #define FML_USED_ON_EMBEDDER #include "flutter/common/task_runners.h" -#include "flutter/fml/synchronization/count_down_latch.h" #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/lib/ui/painting/canvas.h" #include "flutter/lib/ui/painting/image.h" @@ -58,10 +57,6 @@ TEST_F(ImageDisposeTest, ImageReleasedAfterFrameAndDisposePictureAndLayer) { }; Settings settings = CreateSettingsForFixture(); - fml::CountDownLatch frame_latch{2}; - settings.frame_rasterized_callback = [&frame_latch](const FrameTiming& t) { - frame_latch.CountDown(); - }; auto task_runner = CreateNewThread(); TaskRunners task_runners("test", // label GetCurrentTaskRunner(), // platform @@ -88,15 +83,12 @@ TEST_F(ImageDisposeTest, ImageReleasedAfterFrameAndDisposePictureAndLayer) { shell->RunEngine(std::move(configuration), [&](auto result) { ASSERT_EQ(result, Engine::RunStatus::Success); }); + message_latch_.Wait(); ASSERT_TRUE(current_display_list_); ASSERT_TRUE(current_image_); - // Wait for 2 frames to be rasterized. The 2nd frame releases resources of the - // 1st frame. - frame_latch.Wait(); - // Force a drain the SkiaUnrefQueue. The engine does this normally as frames // pump, but we force it here to make the test more deterministic. message_latch_.Reset(); diff --git a/lib/ui/window.dart b/lib/ui/window.dart index 022227be017c4..be761e548bb12 100644 --- a/lib/ui/window.dart +++ b/lib/ui/window.dart @@ -356,20 +356,18 @@ class FlutterView { void render(Scene scene) { // Duplicated calls or calls outside of onBeginFrame/onDrawFrame (indicated // by _debugRenderedViews being null) are ignored. See _debugRenderedViews. - // TODO(dkwingsmt): We should change this skip into an assertion. - // https://github.com/flutter/flutter/issues/137073 bool validRender = true; assert(() { validRender = platformDispatcher._debugRenderedViews?.add(this) ?? false; return true; }()); if (validRender) { - _render(viewId, scene as _NativeScene); + _render(scene as _NativeScene); } } - @Native)>(symbol: 'PlatformConfigurationNativeApi::Render') - external static void _render(int viewId, _NativeScene scene); + @Native)>(symbol: 'PlatformConfigurationNativeApi::Render') + external static void _render(_NativeScene scene); /// Change the retained semantics data about this [FlutterView]. /// diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index 082756d667976..d19c80a7a8028 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -449,10 +449,9 @@ void PlatformConfiguration::CompletePlatformMessageResponse( response->Complete(std::make_unique(std::move(data))); } -void PlatformConfigurationNativeApi::Render(int64_t view_id, Scene* scene) { +void PlatformConfigurationNativeApi::Render(Scene* scene) { UIDartState::ThrowIfUIOperationsProhibited(); - UIDartState::Current()->platform_configuration()->client()->Render(view_id, - scene); + UIDartState::Current()->platform_configuration()->client()->Render(scene); } void PlatformConfigurationNativeApi::SetNeedsReportTimings(bool value) { diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index 9c762b460c05c..7965ba10a7b91 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -69,7 +69,7 @@ class PlatformConfigurationClient { /// @brief Updates the client's rendering on the GPU with the newly /// provided Scene. /// - virtual void Render(int64_t view_id, Scene* scene) = 0; + virtual void Render(Scene* scene) = 0; //-------------------------------------------------------------------------- /// @brief Receives an updated semantics tree from the Framework. @@ -557,7 +557,7 @@ class PlatformConfigurationNativeApi { static void ScheduleFrame(); - static void Render(int64_t view_id, Scene* scene); + static void Render(Scene* scene); static void UpdateSemantics(SemanticsUpdate* update); diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index 916f47c7026e8..7410caeb66d6c 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -15,166 +15,8 @@ #include "flutter/shell/common/shell_test.h" #include "flutter/shell/common/thread_host.h" #include "flutter/testing/testing.h" -#include "gmock/gmock.h" namespace flutter { - -namespace { - -static constexpr int64_t kImplicitViewId = 0; - -static void PostSync(const fml::RefPtr& task_runner, - const fml::closure& task) { - fml::AutoResetWaitableEvent latch; - fml::TaskRunner::RunNowOrPostTask(task_runner, [&latch, &task] { - task(); - latch.Signal(); - }); - latch.Wait(); -} - -class MockRuntimeDelegate : public RuntimeDelegate { - public: - MOCK_METHOD(std::string, DefaultRouteName, (), (override)); - MOCK_METHOD(void, ScheduleFrame, (bool), (override)); - MOCK_METHOD(void, - Render, - (int64_t, std::unique_ptr, float), - (override)); - MOCK_METHOD(void, - UpdateSemantics, - (SemanticsNodeUpdates, CustomAccessibilityActionUpdates), - (override)); - MOCK_METHOD(void, - HandlePlatformMessage, - (std::unique_ptr), - (override)); - MOCK_METHOD(FontCollection&, GetFontCollection, (), (override)); - MOCK_METHOD(std::shared_ptr, GetAssetManager, (), (override)); - MOCK_METHOD(void, OnRootIsolateCreated, (), (override)); - MOCK_METHOD(void, - UpdateIsolateDescription, - (const std::string, int64_t), - (override)); - MOCK_METHOD(void, SetNeedsReportTimings, (bool), (override)); - MOCK_METHOD(std::unique_ptr>, - ComputePlatformResolvedLocale, - (const std::vector&), - (override)); - MOCK_METHOD(void, RequestDartDeferredLibrary, (intptr_t), (override)); - MOCK_METHOD(std::weak_ptr, - GetPlatformMessageHandler, - (), - (const, override)); - MOCK_METHOD(void, SendChannelUpdate, (std::string, bool), (override)); - MOCK_METHOD(double, - GetScaledFontSize, - (double font_size, int configuration_id), - (const, override)); -}; - -class MockPlatformMessageHandler : public PlatformMessageHandler { - public: - MOCK_METHOD(void, - HandlePlatformMessage, - (std::unique_ptr message), - (override)); - MOCK_METHOD(bool, - DoesHandlePlatformMessageOnPlatformThread, - (), - (const, override)); - MOCK_METHOD(void, - InvokePlatformMessageResponseCallback, - (int response_id, std::unique_ptr mapping), - (override)); - MOCK_METHOD(void, - InvokePlatformMessageEmptyResponseCallback, - (int response_id), - (override)); -}; - -// A class that can launch a RuntimeController with the specified -// RuntimeDelegate. -// -// To use this class, contruct this class with Create, call LaunchRootIsolate, -// and use the controller with ControllerTaskSync(). -class RuntimeControllerContext { - public: - using ControllerCallback = std::function; - - [[nodiscard]] static std::unique_ptr Create( - Settings settings, // - const TaskRunners& task_runners, // - RuntimeDelegate& client) { - auto [vm, isolate_snapshot] = Shell::InferVmInitDataFromSettings(settings); - FML_CHECK(vm) << "Must be able to initialize the VM."; - // Construct the class with `new` because `make_unique` has no access to the - // private constructor. - RuntimeControllerContext* raw_pointer = new RuntimeControllerContext( - settings, task_runners, client, std::move(vm), isolate_snapshot); - return std::unique_ptr(raw_pointer); - } - - ~RuntimeControllerContext() { - PostSync(task_runners_.GetUITaskRunner(), - [&]() { runtime_controller_.reset(); }); - } - - // Launch the root isolate. The post_launch callback will be executed in the - // same UI task, which can be used to create initial views. - void LaunchRootIsolate(RunConfiguration& configuration, - ControllerCallback post_launch) { - PostSync(task_runners_.GetUITaskRunner(), [&]() { - bool launch_success = runtime_controller_->LaunchRootIsolate( - settings_, // - []() {}, // - configuration.GetEntrypoint(), // - configuration.GetEntrypointLibrary(), // - configuration.GetEntrypointArgs(), // - configuration.TakeIsolateConfiguration()); // - ASSERT_TRUE(launch_success); - post_launch(*runtime_controller_); - }); - } - - // Run a task that operates the RuntimeController on the UI thread, and wait - // for the task to end. - void ControllerTaskSync(ControllerCallback task) { - ASSERT_TRUE(runtime_controller_); - ASSERT_TRUE(task); - PostSync(task_runners_.GetUITaskRunner(), - [&]() { task(*runtime_controller_); }); - } - - private: - RuntimeControllerContext(const Settings& settings, - const TaskRunners& task_runners, - RuntimeDelegate& client, - DartVMRef vm, - fml::RefPtr isolate_snapshot) - : settings_(settings), - task_runners_(task_runners), - isolate_snapshot_(std::move(isolate_snapshot)), - vm_(std::move(vm)), - runtime_controller_(std::make_unique( - client, - &vm_, - std::move(isolate_snapshot_), - settings.idle_notification_callback, // idle notification callback - flutter::PlatformData(), // platform data - settings.isolate_create_callback, // isolate create callback - settings.isolate_shutdown_callback, // isolate shutdown callback - settings.persistent_isolate_data, // persistent isolate data - UIDartState::Context{task_runners})) {} - - Settings settings_; - TaskRunners task_runners_; - fml::RefPtr isolate_snapshot_; - DartVMRef vm_; - std::unique_ptr runtime_controller_; -}; -} // namespace - namespace testing { class PlatformConfigurationTest : public ShellTest {}; diff --git a/runtime/dart_vm.cc b/runtime/dart_vm.cc index 23e59cf93b0f2..379243773c5ca 100644 --- a/runtime/dart_vm.cc +++ b/runtime/dart_vm.cc @@ -219,22 +219,22 @@ static std::vector ProfilingFlags(bool enable_profiling) { // flags. if (enable_profiling) { return { - // This is the default. But just be explicit. - "--profiler", - // This instructs the profiler to walk C++ frames, and to include - // them in the profile. - "--profile-vm", + // This is the default. But just be explicit. + "--profiler", + // This instructs the profiler to walk C++ frames, and to include + // them in the profile. + "--profile-vm", #if FML_OS_IOS && FML_ARCH_CPU_ARM_FAMILY && FML_ARCH_CPU_ARMEL - // Set the profiler interrupt period to 500Hz instead of the - // default 1000Hz on 32-bit iOS devices to reduce average and worst - // case frame build times. - // - // Note: profile_period is time in microseconds between sampling - // events, not frequency. Frequency is calculated 1/period (or - // 1,000,000 / 2,000 -> 500Hz in this case). - "--profile_period=2000", + // Set the profiler interrupt period to 500Hz instead of the + // default 1000Hz on 32-bit iOS devices to reduce average and worst + // case frame build times. + // + // Note: profile_period is time in microseconds between sampling + // events, not frequency. Frequency is calculated 1/period (or + // 1,000,000 / 2,000 -> 500Hz in this case). + "--profile_period=2000", #else - "--profile_period=1000", + "--profile_period=1000", #endif // FML_OS_IOS && FML_ARCH_CPU_ARM_FAMILY && FML_ARCH_CPU_ARMEL }; } else { diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 5ee24c9985dad..9dc6a944de7cc 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -341,14 +341,15 @@ void RuntimeController::ScheduleFrame() { } // |PlatformConfigurationClient| -void RuntimeController::Render(int64_t view_id, Scene* scene) { +void RuntimeController::Render(Scene* scene) { + // TODO(dkwingsmt): Currently only supports a single window. + int64_t view_id = kFlutterImplicitViewId; const ViewportMetrics* view_metrics = UIDartState::Current()->platform_configuration()->GetMetrics(view_id); if (view_metrics == nullptr) { return; } - client_.Render(view_id, - scene->takeLayerTree(view_metrics->physical_width, + client_.Render(scene->takeLayerTree(view_metrics->physical_width, view_metrics->physical_height), view_metrics->device_pixel_ratio); } diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 7a29a3616e3cb..7a65f2b074282 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -658,7 +658,7 @@ class RuntimeController : public PlatformConfigurationClient { void ScheduleFrame() override; // |PlatformConfigurationClient| - void Render(int64_t view_id, Scene* scene) override; + void Render(Scene* scene) override; // |PlatformConfigurationClient| void UpdateSemantics(SemanticsUpdate* update) override; diff --git a/runtime/runtime_delegate.h b/runtime/runtime_delegate.h index 809fa46ac0aff..bc3de031f4411 100644 --- a/runtime/runtime_delegate.h +++ b/runtime/runtime_delegate.h @@ -25,8 +25,7 @@ class RuntimeDelegate { virtual void ScheduleFrame(bool regenerate_layer_trees = true) = 0; - virtual void Render(int64_t view_id, - std::unique_ptr layer_tree, + virtual void Render(std::unique_ptr layer_tree, float device_pixel_ratio) = 0; virtual void UpdateSemantics(SemanticsNodeUpdates update, diff --git a/shell/common/animator.cc b/shell/common/animator.cc index 1696e5009db24..3dd925cee6213 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -60,10 +60,6 @@ void Animator::EnqueueTraceFlowId(uint64_t trace_flow_id) { void Animator::BeginFrame( std::unique_ptr frame_timings_recorder) { - // Both frame_timings_recorder_ and layer_trees_tasks_ must be empty if not - // between BeginFrame and EndFrame. - FML_DCHECK(frame_timings_recorder_ == nullptr); - FML_DCHECK(layer_trees_tasks_.empty()); TRACE_EVENT_ASYNC_END0("flutter", "Frame Request Pending", frame_request_number_); frame_request_number_++; @@ -116,33 +112,6 @@ void Animator::BeginFrame( dart_frame_deadline_ = frame_target_time.ToEpochDelta(); uint64_t frame_number = frame_timings_recorder_->GetFrameNumber(); delegate_.OnAnimatorBeginFrame(frame_target_time, frame_number); -} - -void Animator::EndFrame() { - FML_CHECK(frame_timings_recorder_ != nullptr); - if (!layer_trees_tasks_.empty()) { - // The build is completed in OnAnimatorBeginFrame. - frame_timings_recorder_->RecordBuildEnd(fml::TimePoint::Now()); - - delegate_.OnAnimatorUpdateLatestFrameTargetTime( - frame_timings_recorder_->GetVsyncTargetTime()); - - // Commit the pending continuation. - PipelineProduceResult result = - producer_continuation_.Complete(std::make_unique( - std::move(layer_trees_tasks_), std::move(frame_timings_recorder_))); - - if (!result.success) { - FML_DLOG(INFO) << "Failed to commit to the pipeline"; - } else if (!result.is_first_item) { - // Do nothing. It has been successfully pushed to the pipeline but not as - // the first item. Eventually the 'Rasterizer' will consume it, so we - // don't need to notify the delegate. - } else { - delegate_.OnAnimatorDraw(layer_tree_pipeline_); - } - } - frame_timings_recorder_ = nullptr; // Ensure it's cleared. if (!frame_scheduled_ && has_rendered_) { // Wait a tad more than 3 60hz frames before reporting a big idle period. @@ -170,33 +139,52 @@ void Animator::EndFrame() { }, kNotifyIdleTaskWaitTime); } - // Both frame_timings_recorder_ and layer_trees_tasks_ must be empty if not - // between BeginFrame and EndFrame. - FML_DCHECK(layer_trees_tasks_.empty()); - FML_DCHECK(frame_timings_recorder_ == nullptr); } -void Animator::Render(int64_t view_id, - std::unique_ptr layer_tree, +void Animator::Render(std::unique_ptr layer_tree, float device_pixel_ratio) { - // Animator::Render should be called between BeginFrame and EndFrame, - // which is indicated by frame_timings_recorder_ being non-null. - // This might happen on release build, and is guarded by PlatformDispatcher on - // debug build. - // TODO(dkwingsmt): We should change this skip into an assertion. - // https://github.com/flutter/flutter/issues/137073 - if (frame_timings_recorder_ == nullptr) { - return; - } - has_rendered_ = true; + if (!frame_timings_recorder_) { + // Framework can directly call render with a built scene. + frame_timings_recorder_ = std::make_unique(); + const fml::TimePoint placeholder_time = fml::TimePoint::Now(); + frame_timings_recorder_->RecordVsync(placeholder_time, placeholder_time); + frame_timings_recorder_->RecordBuildStart(placeholder_time); + } + TRACE_EVENT_WITH_FRAME_NUMBER(frame_timings_recorder_, "flutter", "Animator::Render", /*flow_id_count=*/0, /*flow_ids=*/nullptr); + frame_timings_recorder_->RecordBuildEnd(fml::TimePoint::Now()); - layer_trees_tasks_.push_back(std::make_unique( + delegate_.OnAnimatorUpdateLatestFrameTargetTime( + frame_timings_recorder_->GetVsyncTargetTime()); + + // TODO(dkwingsmt): Currently only supports a single window. + // See https://github.com/flutter/flutter/issues/135530, item 2. + int64_t view_id = kFlutterImplicitViewId; + std::vector> layer_trees_tasks; + layer_trees_tasks.push_back(std::make_unique( view_id, std::move(layer_tree), device_pixel_ratio)); + // Commit the pending continuation. + PipelineProduceResult result = + producer_continuation_.Complete(std::make_unique( + std::move(layer_trees_tasks), std::move(frame_timings_recorder_))); + + if (!result.success) { + FML_DLOG(INFO) << "No pending continuation to commit"; + return; + } + + if (!result.is_first_item) { + // It has been successfully pushed to the pipeline but not as the first + // item. Eventually the 'Rasterizer' will consume it, so we don't need to + // notify the delegate. + return; + } + + delegate_.OnAnimatorDraw(layer_tree_pipeline_); } const std::weak_ptr Animator::GetVsyncWaiter() const { @@ -268,7 +256,6 @@ void Animator::AwaitVSync() { self->DrawLastLayerTrees(std::move(frame_timings_recorder)); } else { self->BeginFrame(std::move(frame_timings_recorder)); - self->EndFrame(); } } }); diff --git a/shell/common/animator.h b/shell/common/animator.h index 5358ed6b14001..8a92705ede9e6 100644 --- a/shell/common/animator.h +++ b/shell/common/animator.h @@ -53,15 +53,7 @@ class Animator final { void RequestFrame(bool regenerate_layer_trees = true); - //-------------------------------------------------------------------------- - /// @brief Tells the Animator that this frame needs to render another view. - /// - /// This method must be called during a vsync callback, or - /// technically, between Animator::BeginFrame and Animator::EndFrame - /// (both private methods). Otherwise, this call will be ignored. - /// - void Render(int64_t view_id, - std::unique_ptr layer_tree, + void Render(std::unique_ptr layer_tree, float device_pixel_ratio); const std::weak_ptr GetVsyncWaiter() const; @@ -90,13 +82,7 @@ class Animator final { void EnqueueTraceFlowId(uint64_t trace_flow_id); private: - // Animator's work during a vsync is split into two methods, BeginFrame and - // EndFrame. The two methods should be called synchronously back-to-back to - // avoid being interrupted by a regular vsync. The reason to split them is to - // allow ShellTest::PumpOneFrame to insert a Render in between. - void BeginFrame(std::unique_ptr frame_timings_recorder); - void EndFrame(); bool CanReuseLastLayerTrees(); @@ -113,7 +99,6 @@ class Animator final { std::shared_ptr waiter_; std::unique_ptr frame_timings_recorder_; - std::vector> layer_trees_tasks_; uint64_t frame_request_number_ = 1; fml::TimeDelta dart_frame_deadline_; std::shared_ptr layer_tree_pipeline_; diff --git a/shell/common/animator_unittests.cc b/shell/common/animator_unittests.cc index 80fd30596b9b1..9190659fc6f55 100644 --- a/shell/common/animator_unittests.cc +++ b/shell/common/animator_unittests.cc @@ -23,8 +23,6 @@ namespace flutter { namespace testing { -constexpr int64_t kImplicitViewId = 0; - class FakeAnimatorDelegate : public Animator::Delegate { public: MOCK_METHOD(void, @@ -160,30 +158,20 @@ TEST_F(ShellTest, AnimatorDoesNotNotifyIdleBeforeRender) { latch.Wait(); ASSERT_FALSE(delegate.notify_idle_called_); - fml::AutoResetWaitableEvent render_latch; // Validate it has not notified idle and try to render. task_runners.GetUITaskRunner()->PostDelayedTask( [&] { ASSERT_FALSE(delegate.notify_idle_called_); - EXPECT_CALL(delegate, OnAnimatorBeginFrame).WillOnce([&] { - auto layer_tree = std::make_unique( - LayerTree::Config(), SkISize::Make(600, 800)); - animator->Render(kImplicitViewId, std::move(layer_tree), 1.0); - render_latch.Signal(); - }); - // Request a frame that builds a layer tree and renders a frame. - // When the frame is rendered, render_latch will be signaled. - animator->RequestFrame(true); + auto layer_tree = std::make_unique(LayerTree::Config(), + SkISize::Make(600, 800)); + animator->Render(std::move(layer_tree), 1.0); task_runners.GetPlatformTaskRunner()->PostTask(flush_vsync_task); }, // See kNotifyIdleTaskWaitTime in animator.cc. fml::TimeDelta::FromMilliseconds(60)); latch.Wait(); - render_latch.Wait(); - // A frame has been rendered, and the next frame request will notify idle. - // But at the moment there isn't another frame request, therefore it still - // hasn't notified idle. + // Still hasn't notified idle because there has been no frame request. task_runners.GetUITaskRunner()->PostTask([&] { ASSERT_FALSE(delegate.notify_idle_called_); // False to avoid getting cals to BeginFrame that will request more frames @@ -236,6 +224,11 @@ TEST_F(ShellTest, AnimatorDoesNotNotifyDelegateIfPipelineIsNotEmpty) { }); fml::AutoResetWaitableEvent begin_frame_latch; + EXPECT_CALL(delegate, OnAnimatorBeginFrame) + .WillRepeatedly( + [&](fml::TimePoint frame_target_time, uint64_t frame_number) { + begin_frame_latch.Signal(); + }); // It must always be called when the method 'Animator::Render' is called, // regardless of whether the pipeline is empty or not. EXPECT_CALL(delegate, OnAnimatorUpdateLatestFrameTargetTime).Times(2); @@ -246,16 +239,16 @@ TEST_F(ShellTest, AnimatorDoesNotNotifyDelegateIfPipelineIsNotEmpty) { for (int i = 0; i < 2; i++) { task_runners.GetUITaskRunner()->PostTask([&] { - EXPECT_CALL(delegate, OnAnimatorBeginFrame).WillOnce([&] { - auto layer_tree = std::make_unique(LayerTree::Config(), - SkISize::Make(600, 800)); - animator->Render(kImplicitViewId, std::move(layer_tree), 1.0); - begin_frame_latch.Signal(); - }); animator->RequestFrame(); task_runners.GetPlatformTaskRunner()->PostTask(flush_vsync_task); }); begin_frame_latch.Wait(); + + PostTaskSync(task_runners.GetUITaskRunner(), [&] { + auto layer_tree = std::make_unique(LayerTree::Config(), + SkISize::Make(600, 800)); + animator->Render(std::move(layer_tree), 1.0); + }); } PostTaskSync(task_runners.GetUITaskRunner(), [&] { animator.reset(); }); diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 1dcf9d4b128ee..ea0cb43d9be18 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -459,8 +459,7 @@ void Engine::ScheduleFrame(bool regenerate_layer_trees) { animator_->RequestFrame(regenerate_layer_trees); } -void Engine::Render(int64_t view_id, - std::unique_ptr layer_tree, +void Engine::Render(std::unique_ptr layer_tree, float device_pixel_ratio) { if (!layer_tree) { return; @@ -471,7 +470,7 @@ void Engine::Render(int64_t view_id, return; } - animator_->Render(view_id, std::move(layer_tree), device_pixel_ratio); + animator_->Render(std::move(layer_tree), device_pixel_ratio); } void Engine::UpdateSemantics(SemanticsNodeUpdates update, diff --git a/shell/common/engine.h b/shell/common/engine.h index 17ba3cc6fd884..a60d8b81f8ed4 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -959,8 +959,7 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { std::string DefaultRouteName() override; // |RuntimeDelegate| - void Render(int64_t view_id, - std::unique_ptr layer_tree, + void Render(std::unique_ptr layer_tree, float device_pixel_ratio) override; // |RuntimeDelegate| diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 7ffa0a6f7714a..b0393cb2d2490 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -7,8 +7,6 @@ #include #include "flutter/runtime/dart_vm_lifecycle.h" -#include "flutter/shell/common/shell.h" -#include "flutter/shell/common/shell_test.h" #include "flutter/shell/common/thread_host.h" #include "flutter/testing/fixture_test.h" #include "flutter/testing/testing.h" @@ -21,19 +19,6 @@ namespace flutter { namespace { -using ::testing::Invoke; -using ::testing::ReturnRef; - -static void PostSync(const fml::RefPtr& task_runner, - const fml::closure& task) { - fml::AutoResetWaitableEvent latch; - fml::TaskRunner::RunNowOrPostTask(task_runner, [&latch, &task] { - task(); - latch.Signal(); - }); - latch.Wait(); -} - class MockDelegate : public Engine::Delegate { public: MOCK_METHOD(void, @@ -80,7 +65,7 @@ class MockRuntimeDelegate : public RuntimeDelegate { MOCK_METHOD(void, ScheduleFrame, (bool), (override)); MOCK_METHOD(void, Render, - (int64_t, std::unique_ptr, float), + (std::unique_ptr, float), (override)); MOCK_METHOD(void, UpdateSemantics, @@ -132,51 +117,6 @@ class MockRuntimeController : public RuntimeController { MOCK_METHOD(bool, NotifyIdle, (fml::TimeDelta), (override)); }; -class MockAnimatorDelegate : public Animator::Delegate { - public: - /* Animator::Delegate */ - MOCK_METHOD(void, - OnAnimatorBeginFrame, - (fml::TimePoint frame_target_time, uint64_t frame_number), - (override)); - MOCK_METHOD(void, - OnAnimatorNotifyIdle, - (fml::TimeDelta deadline), - (override)); - MOCK_METHOD(void, - OnAnimatorUpdateLatestFrameTargetTime, - (fml::TimePoint frame_target_time), - (override)); - MOCK_METHOD(void, - OnAnimatorDraw, - (std::shared_ptr pipeline), - (override)); - MOCK_METHOD(void, - OnAnimatorDrawLastLayerTrees, - (std::unique_ptr frame_timings_recorder), - (override)); -}; - -class MockPlatformMessageHandler : public PlatformMessageHandler { - public: - MOCK_METHOD(void, - HandlePlatformMessage, - (std::unique_ptr message), - (override)); - MOCK_METHOD(bool, - DoesHandlePlatformMessageOnPlatformThread, - (), - (const, override)); - MOCK_METHOD(void, - InvokePlatformMessageResponseCallback, - (int response_id, std::unique_ptr mapping), - (override)); - MOCK_METHOD(void, - InvokePlatformMessageEmptyResponseCallback, - (int response_id), - (override)); -}; - std::unique_ptr MakePlatformMessage( const std::string& channel, const std::map& values, @@ -245,97 +185,6 @@ class EngineTest : public testing::FixtureTest { std::shared_ptr image_decoder_task_runner_; fml::TaskRunnerAffineWeakPtr snapshot_delegate_; }; - -// A class that can launch an Engine with the specified Engine::Delegate. -// -// To use this class, contruct this class with Create, call Run, and use the -// engine with EngineTaskSync(). -class EngineContext { - public: - using EngineCallback = std::function; - - [[nodiscard]] static std::unique_ptr Create( - Engine::Delegate& delegate, // - Settings settings, // - const TaskRunners& task_runners, // - std::unique_ptr animator) { - auto [vm, isolate_snapshot] = Shell::InferVmInitDataFromSettings(settings); - FML_CHECK(vm) << "Must be able to initialize the VM."; - // Construct the class with `new` because `make_unique` has no access to the - // private constructor. - EngineContext* raw_pointer = - new EngineContext(delegate, settings, task_runners, std::move(animator), - std::move(vm), isolate_snapshot); - return std::unique_ptr(raw_pointer); - } - - void Run(RunConfiguration configuration) { - PostSync(task_runners_.GetUITaskRunner(), [this, &configuration] { - Engine::RunStatus run_status = engine_->Run(std::move(configuration)); - FML_CHECK(run_status == Engine::RunStatus::Success) - << "Engine failed to run."; - (void)run_status; // Suppress unused-variable warning - }); - } - - // Run a task that operates the Engine on the UI thread, and wait for the - // task to end. - // - // If called on the UI thread, the task is executed synchronously. - void EngineTaskSync(EngineCallback task) { - ASSERT_TRUE(engine_); - ASSERT_TRUE(task); - auto runner = task_runners_.GetUITaskRunner(); - if (runner->RunsTasksOnCurrentThread()) { - task(*engine_); - } else { - PostSync(task_runners_.GetUITaskRunner(), [&]() { task(*engine_); }); - } - } - - ~EngineContext() { - PostSync(task_runners_.GetUITaskRunner(), [this] { engine_.reset(); }); - } - - private: - EngineContext(Engine::Delegate& delegate, // - Settings settings, // - const TaskRunners& task_runners, // - std::unique_ptr animator, // - DartVMRef vm, // - fml::RefPtr isolate_snapshot) - : task_runners_(task_runners), vm_(std::move(vm)) { - PostSync(task_runners.GetUITaskRunner(), [this, &settings, &animator, - &delegate, &isolate_snapshot] { - auto dispatcher_maker = - [](DefaultPointerDataDispatcher::Delegate& delegate) { - return std::make_unique(delegate); - }; - engine_ = std::make_unique( - /*delegate=*/delegate, - /*dispatcher_maker=*/dispatcher_maker, - /*vm=*/*&vm_, - /*isolate_snapshot=*/std::move(isolate_snapshot), - /*task_runners=*/task_runners_, - /*platform_data=*/PlatformData(), - /*settings=*/settings, - /*animator=*/std::move(animator), - /*io_manager=*/io_manager_, - /*unref_queue=*/nullptr, - /*snapshot_delegate=*/snapshot_delegate_, - /*volatile_path_tracker=*/nullptr, - /*gpu_disabled_switch=*/std::make_shared()); - }); - } - - TaskRunners task_runners_; - DartVMRef vm_; - std::unique_ptr engine_; - - fml::WeakPtr io_manager_; - fml::TaskRunnerAffineWeakPtr snapshot_delegate_; -}; - } // namespace TEST_F(EngineTest, Create) { @@ -569,68 +418,4 @@ TEST_F(EngineTest, PassesLoadDartDeferredLibraryErrorToRuntime) { }); } -TEST_F(EngineTest, AnimatorAcceptsMultipleRenders) { - MockAnimatorDelegate animator_delegate; - std::unique_ptr engine_context; - - std::shared_ptr platform_message_handler = - std::make_shared(); - EXPECT_CALL(delegate_, GetPlatformMessageHandler) - .WillOnce(ReturnRef(platform_message_handler)); - - fml::AutoResetWaitableEvent draw_latch; - EXPECT_CALL(animator_delegate, OnAnimatorDraw) - .WillOnce( - Invoke([&draw_latch](const std::shared_ptr& pipeline) { - auto status = - pipeline->Consume([&](std::unique_ptr item) { - EXPECT_EQ(item->layer_tree_tasks.size(), 2u); - EXPECT_EQ(item->layer_tree_tasks[0]->view_id, 1); - EXPECT_EQ(item->layer_tree_tasks[1]->view_id, 2); - }); - EXPECT_EQ(status, PipelineConsumeResult::Done); - draw_latch.Signal(); - })); - EXPECT_CALL(animator_delegate, OnAnimatorBeginFrame) - .WillOnce(Invoke([&engine_context](fml::TimePoint frame_target_time, - uint64_t frame_number) { - engine_context->EngineTaskSync([&](Engine& engine) { - engine.BeginFrame(frame_target_time, frame_number); - }); - })); - - static fml::AutoResetWaitableEvent callback_ready_latch; - callback_ready_latch.Reset(); - AddNativeCallback("NotifyNative", - [](auto args) { callback_ready_latch.Signal(); }); - - std::unique_ptr animator; - PostSync(task_runners_.GetUITaskRunner(), - [&animator, &animator_delegate, &task_runners = task_runners_] { - animator = std::make_unique( - animator_delegate, task_runners, - static_cast>( - std::make_unique( - task_runners))); - }); - - engine_context = EngineContext::Create(delegate_, settings_, task_runners_, - std::move(animator)); - - auto configuration = RunConfiguration::InferFromSettings(settings_); - configuration.SetEntrypoint("onBeginFrameRendersMultipleViews"); - engine_context->Run(std::move(configuration)); - - engine_context->EngineTaskSync([](Engine& engine) { - engine.AddView(1, {1, 10, 10, 22, 0}); - engine.AddView(2, {1, 10, 10, 22, 0}); - }); - - callback_ready_latch.Wait(); - - engine_context->EngineTaskSync( - [](Engine& engine) { engine.ScheduleFrame(); }); - draw_latch.Wait(); -} - } // namespace flutter diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart index cc6e32731d898..e4b0d98833f44 100644 --- a/shell/common/fixtures/shell_test.dart +++ b/shell/common/fixtures/shell_test.dart @@ -547,24 +547,3 @@ void testReportViewWidths() { nativeReportViewWidthsCallback(getCurrentViewWidths()); }; } - -@pragma('vm:entry-point') -void onBeginFrameRendersMultipleViews() { - PlatformDispatcher.instance.onBeginFrame = (Duration beginTime) { - for (final FlutterView view in PlatformDispatcher.instance.views) { - final SceneBuilder builder = SceneBuilder(); - final PictureRecorder recorder = PictureRecorder(); - final Canvas canvas = Canvas(recorder); - canvas.drawPaint(Paint()..color = const Color(0xFFABCDEF)); - final Picture picture = recorder.endRecording(); - builder.addPicture(Offset.zero, picture); - - final Scene scene = builder.build(); - view.render(scene); - - scene.dispose(); - picture.dispose(); - } - }; - notifyNative(); -} diff --git a/shell/common/input_events_unittests.cc b/shell/common/input_events_unittests.cc index 66a2b64a39e02..3824dc4d92bae 100644 --- a/shell/common/input_events_unittests.cc +++ b/shell/common/input_events_unittests.cc @@ -127,11 +127,11 @@ static void TestSimulatedInputEvents( ShellTest::DispatchFakePointerData(shell.get()); i += 1; } - ShellTest::VSyncFlush(shell.get(), &will_draw_new_frame); + ShellTest::VSyncFlush(shell.get(), will_draw_new_frame); } // Finally, issue a vsync for the pending event that may be generated duing // the last vsync. - ShellTest::VSyncFlush(shell.get(), &will_draw_new_frame); + ShellTest::VSyncFlush(shell.get(), will_draw_new_frame); }); simulation.wait(); @@ -345,7 +345,8 @@ TEST_F(ShellTest, CanCorrectlyPipePointerPacket) { CreateSimulatedPointerData(data, PointerData::Change::kRemove, 3.0, 4.0); packet->SetPointerData(5, data); ShellTest::DispatchPointerData(shell.get(), std::move(packet)); - ShellTest::VSyncFlush(shell.get()); + bool will_draw_new_frame; + ShellTest::VSyncFlush(shell.get(), will_draw_new_frame); reportLatch.Wait(); size_t expect_length = 6; @@ -406,7 +407,8 @@ TEST_F(ShellTest, CanCorrectlySynthesizePointerPacket) { CreateSimulatedPointerData(data, PointerData::Change::kRemove, 3.0, 4.0); packet->SetPointerData(3, data); ShellTest::DispatchPointerData(shell.get(), std::move(packet)); - ShellTest::VSyncFlush(shell.get()); + bool will_draw_new_frame; + ShellTest::VSyncFlush(shell.get(), will_draw_new_frame); reportLatch.Wait(); size_t expect_length = 6; diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 523bf53d350b6..0fc4d884c97bf 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -254,7 +254,6 @@ DrawStatus Rasterizer::Draw(const std::shared_ptr& pipeline) { bool should_resubmit_frame = ShouldResubmitFrame(draw_result); if (should_resubmit_frame) { - FML_CHECK(draw_result.resubmitted_item); auto front_continuation = pipeline->ProduceIfEmpty(); PipelineProduceResult pipeline_result = front_continuation.Complete(std::move(draw_result.resubmitted_item)); diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index bd952747ca58f..c673d4b429c9f 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -22,39 +22,6 @@ namespace testing { constexpr int64_t kImplicitViewId = 0; -FrameContent ViewContent::NoViews() { - return std::map(); -} - -FrameContent ViewContent::DummyView(double width, double height) { - FrameContent result; - result[kImplicitViewId] = ViewContent{ - .viewport_metrics = {1.0, width, height, 22, 0}, - .builder = {}, - }; - return result; -} - -FrameContent ViewContent::DummyView(flutter::ViewportMetrics viewport_metrics) { - FrameContent result; - result[kImplicitViewId] = ViewContent{ - .viewport_metrics = std::move(viewport_metrics), - .builder = {}, - }; - return result; -} - -FrameContent ViewContent::ImplicitView(double width, - double height, - LayerTreeBuilder builder) { - FrameContent result; - result[kImplicitViewId] = ViewContent{ - .viewport_metrics = {1.0, width, height, 22, 0}, - .builder = std::move(builder), - }; - return result; -} - ShellTest::ShellTest() : thread_host_("io.flutter.test." + GetCurrentTestName() + ".", ThreadHost::Type::Platform | ThreadHost::Type::IO | @@ -125,18 +92,16 @@ void ShellTest::RestartEngine(Shell* shell, RunConfiguration configuration) { ASSERT_TRUE(restarted.get_future().get()); } -void ShellTest::VSyncFlush(Shell* shell, bool* will_draw_new_frame) { +void ShellTest::VSyncFlush(Shell* shell, bool& will_draw_new_frame) { fml::AutoResetWaitableEvent latch; fml::TaskRunner::RunNowOrPostTask( shell->GetTaskRunners().GetPlatformTaskRunner(), - [shell, will_draw_new_frame, &latch] { + [shell, &will_draw_new_frame, &latch] { // The following UI task ensures that all previous UI tasks are flushed. fml::AutoResetWaitableEvent ui_latch; shell->GetTaskRunners().GetUITaskRunner()->PostTask( - [&ui_latch, will_draw_new_frame]() { - if (will_draw_new_frame != nullptr) { - *will_draw_new_frame = true; - } + [&ui_latch, &will_draw_new_frame]() { + will_draw_new_frame = true; ui_latch.Signal(); }); @@ -189,7 +154,6 @@ void ShellTest::SetViewportMetrics(Shell* shell, double width, double height) { std::make_unique(); recorder->RecordVsync(frame_begin_time, frame_end_time); engine->animator_->BeginFrame(std::move(recorder)); - engine->animator_->EndFrame(); } latch.Signal(); }); @@ -208,22 +172,23 @@ void ShellTest::NotifyIdle(Shell* shell, fml::TimeDelta deadline) { latch.Wait(); } -void ShellTest::PumpOneFrame(Shell* shell) { - PumpOneFrame(shell, ViewContent::DummyView()); +void ShellTest::PumpOneFrame(Shell* shell, + double width, + double height, + LayerTreeBuilder builder) { + PumpOneFrame(shell, {1.0, width, height, 22, 0}, std::move(builder)); } -void ShellTest::PumpOneFrame(Shell* shell, FrameContent frame_content) { +void ShellTest::PumpOneFrame(Shell* shell, + const flutter::ViewportMetrics& viewport_metrics, + LayerTreeBuilder builder) { // Set viewport to nonempty, and call Animator::BeginFrame to make the layer // tree pipeline nonempty. Without either of this, the layer tree below // won't be rasterized. fml::AutoResetWaitableEvent latch; - fml::WeakPtr runtime_delegate = shell->weak_engine_; shell->GetTaskRunners().GetUITaskRunner()->PostTask( - [&latch, engine = shell->weak_engine_, &frame_content, - runtime_delegate]() { - for (auto& [view_id, view_content] : frame_content) { - engine->SetViewportMetrics(view_id, view_content.viewport_metrics); - } + [&latch, engine = shell->weak_engine_, viewport_metrics]() { + engine->SetViewportMetrics(kImplicitViewId, viewport_metrics); const auto frame_begin_time = fml::TimePoint::Now(); const auto frame_end_time = frame_begin_time + fml::TimeDelta::FromSecondsF(1.0 / 60.0); @@ -231,28 +196,28 @@ void ShellTest::PumpOneFrame(Shell* shell, FrameContent frame_content) { std::make_unique(); recorder->RecordVsync(frame_begin_time, frame_end_time); engine->animator_->BeginFrame(std::move(recorder)); + latch.Signal(); + }); + latch.Wait(); - // The BeginFrame phase and the EndFrame phase must be performed in a - // single task, otherwise a normal vsync might be inserted in between, - // causing flaky assertion errors. - - for (auto& [view_id, view_content] : frame_content) { - SkMatrix identity; - identity.setIdentity(); - auto root_layer = std::make_shared(identity); - auto layer_tree = std::make_unique( - LayerTree::Config{.root_layer = root_layer}, - SkISize::Make(view_content.viewport_metrics.physical_width, - view_content.viewport_metrics.physical_height)); - float device_pixel_ratio = static_cast( - view_content.viewport_metrics.device_pixel_ratio); - if (view_content.builder) { - view_content.builder(root_layer); - } - runtime_delegate->Render(view_id, std::move(layer_tree), - device_pixel_ratio); + latch.Reset(); + // Call |Render| to rasterize a layer tree and trigger |OnFrameRasterized| + fml::WeakPtr runtime_delegate = shell->weak_engine_; + shell->GetTaskRunners().GetUITaskRunner()->PostTask( + [&latch, runtime_delegate, &builder, viewport_metrics]() { + SkMatrix identity; + identity.setIdentity(); + auto root_layer = std::make_shared(identity); + auto layer_tree = std::make_unique( + LayerTree::Config{.root_layer = root_layer}, + SkISize::Make(viewport_metrics.physical_width, + viewport_metrics.physical_height)); + float device_pixel_ratio = + static_cast(viewport_metrics.device_pixel_ratio); + if (builder) { + builder(root_layer); } - engine->animator_->EndFrame(); + runtime_delegate->Render(std::move(layer_tree), device_pixel_ratio); latch.Signal(); }); latch.Wait(); diff --git a/shell/common/shell_test.h b/shell/common/shell_test.h index c11ad1174dc88..7ded997fbcdc5 100644 --- a/shell/common/shell_test.h +++ b/shell/common/shell_test.h @@ -29,38 +29,6 @@ namespace flutter { namespace testing { -// The signature of ViewContent::builder. -using LayerTreeBuilder = - std::function root)>; -struct ViewContent; -// Defines the content to be rendered to all views of a frame in PumpOneFrame. -using FrameContent = std::map; -// Defines the content to be rendered to a view in PumpOneFrame. -struct ViewContent { - flutter::ViewportMetrics viewport_metrics; - // Given the root layer, this callback builds the layer tree to be rasterized - // in PumpOneFrame. - LayerTreeBuilder builder; - - // Build a frame with no views. This is useful when PumpOneFrame is used just - // to schedule the frame while the frame content is defined by other means. - static FrameContent NoViews(); - - // Build a frame with a single implicit view with the specific size and no - // content. - static FrameContent DummyView(double width = 1, double height = 1); - - // Build a frame with a single implicit view with the specific viewport - // metrics and no content. - static FrameContent DummyView(flutter::ViewportMetrics viewport_metrics); - - // Build a frame with a single implicit view with the specific size and - // content. - static FrameContent ImplicitView(double width, - double height, - LayerTreeBuilder builder); -}; - class ShellTest : public FixtureTest { public: struct Config { @@ -102,14 +70,24 @@ class ShellTest : public FixtureTest { static void RestartEngine(Shell* shell, RunConfiguration configuration); /// Issue as many VSYNC as needed to flush the UI tasks so far, and reset - /// the content of `will_draw_new_frame` to true if it's not nullptr. - static void VSyncFlush(Shell* shell, bool* will_draw_new_frame = nullptr); + /// the `will_draw_new_frame` to true. + static void VSyncFlush(Shell* shell, bool& will_draw_new_frame); + + /// Given the root layer, this callback builds the layer tree to be rasterized + /// in PumpOneFrame. + using LayerTreeBuilder = + std::function root)>; static void SetViewportMetrics(Shell* shell, double width, double height); static void NotifyIdle(Shell* shell, fml::TimeDelta deadline); - static void PumpOneFrame(Shell* shell); - static void PumpOneFrame(Shell* shell, FrameContent frame_content); + static void PumpOneFrame(Shell* shell, + double width = 1, + double height = 1, + LayerTreeBuilder = {}); + static void PumpOneFrame(Shell* shell, + const flutter::ViewportMetrics& viewport_metrics, + LayerTreeBuilder = {}); static void DispatchFakePointerData(Shell* shell); static void DispatchPointerData(Shell* shell, std::unique_ptr packet); diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 83c8bd3488c50..9914593728717 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -42,7 +42,6 @@ #include "flutter/shell/common/switches.h" #include "flutter/shell/common/thread_host.h" #include "flutter/shell/common/vsync_waiter_fallback.h" -#include "flutter/shell/common/vsync_waiters_test.h" #include "flutter/shell/version/version.h" #include "flutter/testing/mock_canvas.h" #include "flutter/testing/testing.h" @@ -876,7 +875,7 @@ TEST_F(ShellTest, ExternalEmbedderNoThreadMerger) { root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); + PumpOneFrame(shell.get(), 100, 100, builder); end_frame_latch.Wait(); ASSERT_TRUE(end_frame_called); @@ -950,7 +949,7 @@ TEST_F(ShellTest, PushBackdropFilterToVisitedPlatformViews) { backdrop_filter_layer->Add(platform_view_layer2); }; - PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); + PumpOneFrame(shell.get(), 100, 100, builder); end_frame_latch.Wait(); ASSERT_EQ(visited_platform_views, (std::vector{50, 75})); ASSERT_TRUE(stack_75.is_empty()); @@ -1011,7 +1010,7 @@ TEST_F(ShellTest, root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); + PumpOneFrame(shell.get(), 100, 100, builder); end_frame_latch.Wait(); ASSERT_TRUE(end_frame_called); @@ -1057,12 +1056,9 @@ TEST_F(ShellTest, OnPlatformViewDestroyDisablesThreadMerger) { root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); + PumpOneFrame(shell.get(), 100, 100, builder); auto result = shell->WaitForFirstFrame(fml::TimeDelta::Max()); - // Wait for the rasterizer to process the frame. WaitForFirstFrame only waits - // for the Animator, but end_frame_callback is called by the Rasterizer. - PostSync(shell->GetTaskRunners().GetRasterTaskRunner(), [] {}); ASSERT_TRUE(result.ok()) << "Result: " << static_cast(result.code()) << ": " << result.message(); @@ -1127,12 +1123,12 @@ TEST_F(ShellTest, OnPlatformViewDestroyAfterMergingThreads) { root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); + PumpOneFrame(shell.get(), 100, 100, builder); // Pump one frame to trigger thread merging. end_frame_latch.Wait(); // Pump another frame to ensure threads are merged and a regular layer tree is // submitted. - PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); + PumpOneFrame(shell.get(), 100, 100, builder); // Threads are merged here. PlatformViewNotifyDestroy should be executed // successfully. ASSERT_TRUE(fml::TaskRunnerChecker::RunsOnTheSameThread( @@ -1196,7 +1192,7 @@ TEST_F(ShellTest, OnPlatformViewDestroyWhenThreadsAreMerging) { root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); + PumpOneFrame(shell.get(), 100, 100, builder); // Pump one frame and threads aren't merged end_frame_latch.Wait(); ASSERT_FALSE(fml::TaskRunnerChecker::RunsOnTheSameThread( @@ -1207,7 +1203,7 @@ TEST_F(ShellTest, OnPlatformViewDestroyWhenThreadsAreMerging) { // threads external_view_embedder->UpdatePostPrerollResult( PostPrerollResult::kResubmitFrame); - PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); + PumpOneFrame(shell.get(), 100, 100, builder); // Now destroy the platform view immediately. // Two things can happen here: @@ -1263,7 +1259,7 @@ TEST_F(ShellTest, SkPoint::Make(10, 10), MakeSizedDisplayList(80, 80), false, false); root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); + PumpOneFrame(shell.get(), 100, 100, builder); end_frame_latch.Wait(); // Threads should not be merged. @@ -1302,7 +1298,7 @@ TEST_F(ShellTest, OnPlatformViewDestroyWithoutRasterThreadMerger) { SkPoint::Make(10, 10), MakeSizedDisplayList(80, 80), false, false); root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); + PumpOneFrame(shell.get(), 100, 100, builder); // Threads should not be merged. ASSERT_FALSE(fml::TaskRunnerChecker::RunsOnTheSameThread( @@ -1368,7 +1364,7 @@ TEST_F(ShellTest, OnPlatformViewDestroyWithStaticThreadMerging) { SkPoint::Make(10, 10), MakeSizedDisplayList(80, 80), false, false); root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); + PumpOneFrame(shell.get(), 100, 100, builder); end_frame_latch.Wait(); ValidateDestroyPlatformView(shell.get()); @@ -1414,7 +1410,7 @@ TEST_F(ShellTest, GetUsedThisFrameShouldBeSetBeforeEndFrame) { SkPoint::Make(10, 10), MakeSizedDisplayList(80, 80), false, false); root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); + PumpOneFrame(shell.get(), 100, 100, builder); end_frame_latch.Wait(); ASSERT_FALSE(used_this_frame); @@ -1564,11 +1560,10 @@ TEST_F(ShellTest, WaitForFirstFrameZeroSizeFrame) { configuration.SetEntrypoint("emptyMain"); RunEngine(shell.get(), std::move(configuration)); - PumpOneFrame(shell.get(), ViewContent::DummyView({1.0, 0.0, 0.0, 22, 0})); + PumpOneFrame(shell.get(), {1.0, 0.0, 0.0, 22, 0}); fml::Status result = shell->WaitForFirstFrame(fml::TimeDelta::Zero()); - EXPECT_FALSE(result.ok()); - EXPECT_EQ(result.message(), "timeout"); - EXPECT_EQ(result.code(), fml::StatusCode::kDeadlineExceeded); + ASSERT_FALSE(result.ok()); + ASSERT_EQ(result.code(), fml::StatusCode::kDeadlineExceeded); DestroyShell(std::move(shell)); } @@ -2084,7 +2079,6 @@ TEST_F(ShellTest, CanScheduleFrameFromPlatform) { TEST_F(ShellTest, SecondaryVsyncCallbackShouldBeCalledAfterVsyncCallback) { bool is_on_begin_frame_called = false; bool is_secondary_callback_called = false; - bool test_started = false; Settings settings = CreateSettingsForFixture(); TaskRunners task_runners = GetTaskRunnersForFixture(); fml::AutoResetWaitableEvent latch; @@ -2094,18 +2088,12 @@ TEST_F(ShellTest, SecondaryVsyncCallbackShouldBeCalledAfterVsyncCallback) { fml::CountDownLatch count_down_latch(2); AddNativeCallback("NativeOnBeginFrame", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { - if (!test_started) { - return; - } EXPECT_FALSE(is_on_begin_frame_called); EXPECT_FALSE(is_secondary_callback_called); is_on_begin_frame_called = true; count_down_latch.CountDown(); })); - std::unique_ptr shell = CreateShell({ - .settings = settings, - .task_runners = task_runners, - }); + std::unique_ptr shell = CreateShell(settings, task_runners); ASSERT_TRUE(shell->IsSetup()); auto configuration = RunConfiguration::InferFromSettings(settings); @@ -2118,16 +2106,12 @@ TEST_F(ShellTest, SecondaryVsyncCallbackShouldBeCalledAfterVsyncCallback) { fml::TaskRunner::RunNowOrPostTask( shell->GetTaskRunners().GetUITaskRunner(), [&]() { shell->GetEngine()->ScheduleSecondaryVsyncCallback(0, [&]() { - if (!test_started) { - return; - } EXPECT_TRUE(is_on_begin_frame_called); EXPECT_FALSE(is_secondary_callback_called); is_secondary_callback_called = true; count_down_latch.CountDown(); }); shell->GetEngine()->ScheduleFrame(); - test_started = true; }); count_down_latch.Wait(); EXPECT_TRUE(is_on_begin_frame_called); @@ -2172,7 +2156,7 @@ TEST_F(ShellTest, Screenshot) { root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); + PumpOneFrame(shell.get(), 100, 100, builder); firstFrameLatch.Wait(); std::promise screenshot_promise; @@ -2632,13 +2616,7 @@ TEST_F(ShellTest, OnServiceProtocolRenderFrameWithRasterStatsWorks) { configuration.SetEntrypoint("scene_with_red_box"); RunEngine(shell.get(), std::move(configuration)); - // Set a non-zero viewport metrics, otherwise the scene would be discarded. - PostSync(shell->GetTaskRunners().GetUITaskRunner(), - [engine = shell->GetEngine()]() { - engine->SetViewportMetrics(kImplicitViewId, - ViewportMetrics{1, 1, 1, 22, 0}); - }); - PumpOneFrame(shell.get(), ViewContent::NoViews()); + PumpOneFrame(shell.get()); ServiceProtocol::Handler::ServiceProtocolMap empty_params; rapidjson::Document document; @@ -2814,16 +2792,14 @@ TEST_F(ShellTest, DISABLED_DiscardLayerTreeOnResize) { RunEngine(shell.get(), std::move(configuration)); - PumpOneFrame(shell.get(), ViewContent::DummyView( - static_cast(wrong_size.width()), - static_cast(wrong_size.height()))); + PumpOneFrame(shell.get(), static_cast(wrong_size.width()), + static_cast(wrong_size.height())); end_frame_latch.Wait(); // Wrong size, no frames are submitted. ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount()); - PumpOneFrame(shell.get(), ViewContent::DummyView( - static_cast(expected_size.width()), - static_cast(expected_size.height()))); + PumpOneFrame(shell.get(), static_cast(expected_size.width()), + static_cast(expected_size.height())); end_frame_latch.Wait(); // Expected size, 1 frame submitted. ASSERT_EQ(1, external_view_embedder->GetSubmittedFrameCount()); @@ -2894,9 +2870,8 @@ TEST_F(ShellTest, DISABLED_DiscardResubmittedLayerTreeOnResize) { RunEngine(shell.get(), std::move(configuration)); - PumpOneFrame(shell.get(), ViewContent::DummyView( - static_cast(origin_size.width()), - static_cast(origin_size.height()))); + PumpOneFrame(shell.get(), static_cast(origin_size.width()), + static_cast(origin_size.height())); end_frame_latch.Wait(); ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount()); @@ -2917,9 +2892,8 @@ TEST_F(ShellTest, DISABLED_DiscardResubmittedLayerTreeOnResize) { ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount()); // Threads will be merged at the end of this frame. - PumpOneFrame(shell.get(), - ViewContent::DummyView(static_cast(new_size.width()), - static_cast(new_size.height()))); + PumpOneFrame(shell.get(), static_cast(new_size.width()), + static_cast(new_size.height())); end_frame_latch.Wait(); ASSERT_TRUE(raster_thread_merger_ref->IsMerged()); diff --git a/shell/platform/fuchsia/flutter/vulkan_surface_producer.cc b/shell/platform/fuchsia/flutter/vulkan_surface_producer.cc index 538827ef76cb5..f99c39a4d095e 100644 --- a/shell/platform/fuchsia/flutter/vulkan_surface_producer.cc +++ b/shell/platform/fuchsia/flutter/vulkan_surface_producer.cc @@ -235,21 +235,22 @@ bool VulkanSurfaceProducer::TransitionSurfacesToExternal( } VkImageMemoryBarrier image_barrier = { - .sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER, - .pNext = nullptr, - .srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT, - .dstAccessMask = 0, - .oldLayout = imageInfo.fImageLayout, + .sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER, + .pNext = nullptr, + .srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT, + .dstAccessMask = 0, + .oldLayout = imageInfo.fImageLayout, // Understand why this is causing issues on Intel. TODO(fxb/53449) #if defined(__aarch64__) - .newLayout = imageInfo.fImageLayout, + .newLayout = imageInfo.fImageLayout, #else - .newLayout = VK_IMAGE_LAYOUT_GENERAL, + .newLayout = VK_IMAGE_LAYOUT_GENERAL, #endif - .srcQueueFamilyIndex = 0, - .dstQueueFamilyIndex = VK_QUEUE_FAMILY_EXTERNAL_KHR, - .image = vk_surface->GetVkImage(), - .subresourceRange = {VK_IMAGE_ASPECT_COLOR_BIT, 0, 1, 0, 1}}; + .srcQueueFamilyIndex = 0, + .dstQueueFamilyIndex = VK_QUEUE_FAMILY_EXTERNAL_KHR, + .image = vk_surface->GetVkImage(), + .subresourceRange = {VK_IMAGE_ASPECT_COLOR_BIT, 0, 1, 0, 1} + }; if (!command_buffer->InsertPipelineBarrier( VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, diff --git a/shell/testing/tester_main.cc b/shell/testing/tester_main.cc index 4be39c4d70b41..607dc6a6a61a3 100644 --- a/shell/testing/tester_main.cc +++ b/shell/testing/tester_main.cc @@ -49,16 +49,17 @@ static std::vector> ShaderLibraryMappings() { return { - std::make_shared(impeller_entity_shaders_vk_data, - impeller_entity_shaders_vk_length), - std::make_shared(impeller_modern_shaders_vk_data, - impeller_modern_shaders_vk_length), + std::make_shared(impeller_entity_shaders_vk_data, + impeller_entity_shaders_vk_length), + std::make_shared( + impeller_modern_shaders_vk_data, impeller_modern_shaders_vk_length), #if IMPELLER_ENABLE_3D - std::make_shared(impeller_scene_shaders_vk_data, - impeller_scene_shaders_vk_length), + std::make_shared( + impeller_scene_shaders_vk_data, impeller_scene_shaders_vk_length), #endif // IMPELLER_ENABLE_3D - std::make_shared( - impeller_compute_shaders_vk_data, impeller_compute_shaders_vk_length), + std::make_shared( + impeller_compute_shaders_vk_data, + impeller_compute_shaders_vk_length), }; } diff --git a/testing/dart/platform_view_test.dart b/testing/dart/platform_view_test.dart index cd581a22d581d..146c865899952 100644 --- a/testing/dart/platform_view_test.dart +++ b/testing/dart/platform_view_test.dart @@ -10,12 +10,10 @@ void main() { test('PlatformView layers do not emit errors from tester', () async { final SceneBuilder builder = SceneBuilder(); builder.addPlatformView(1); - PlatformDispatcher.instance.onBeginFrame = (Duration duration) { - final Scene scene = builder.build(); - PlatformDispatcher.instance.implicitView!.render(scene); - scene.dispose(); - }; - PlatformDispatcher.instance.scheduleFrame(); + final Scene scene = builder.build(); + + PlatformDispatcher.instance.implicitView!.render(scene); + scene.dispose(); // Test harness asserts that this does not emit an error from the shell logs. }); }