From 0c933dc874b8c19c9b59adbc651b0d7214602612 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Thu, 13 Aug 2020 11:29:56 -0700 Subject: [PATCH 1/6] only create raster_thread_merge when necessary --- flow/embedded_views.cc | 4 ++++ flow/embedded_views.h | 2 ++ shell/common/rasterizer.cc | 3 ++- shell/common/shell_test_external_view_embedder.cc | 4 ++++ shell/common/shell_test_external_view_embedder.h | 3 +++ shell/platform/darwin/ios/ios_surface.h | 3 +++ shell/platform/darwin/ios/ios_surface.mm | 5 +++++ 7 files changed, 23 insertions(+), 1 deletion(-) diff --git a/flow/embedded_views.cc b/flow/embedded_views.cc index 07a484999ff51..36a7423f10a29 100644 --- a/flow/embedded_views.cc +++ b/flow/embedded_views.cc @@ -60,4 +60,8 @@ const std::vector>::const_iterator MutatorsStack::End() return vector_.end(); }; +bool ExternalViewEmbedder::SupportDynamicThreadMerging() { + return false; +} + } // namespace flutter diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 455eb91511801..43bd929f8f614 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -310,6 +310,8 @@ class ExternalViewEmbedder { bool should_resubmit_frame, fml::RefPtr raster_thread_merger) {} + virtual bool SupportDynamicThreadMerging(); + FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder); }; // ExternalViewEmbedder diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 709d5495ba626..f402a37be66f5 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -81,7 +81,8 @@ void Rasterizer::Setup(std::unique_ptr surface) { #if !defined(OS_FUCHSIA) // TODO(sanjayc77): https://github.com/flutter/flutter/issues/53179. Add // support for raster thread merger for Fuchsia. - if (surface_->GetExternalViewEmbedder()) { + if (surface_->GetExternalViewEmbedder() && + surface_->GetExternalViewEmbedder()->SupportDynamicThreadMerging()) { const auto platform_id = task_runners_.GetPlatformTaskRunner()->GetTaskQueueId(); const auto gpu_id = task_runners_.GetRasterTaskRunner()->GetTaskQueueId(); diff --git a/shell/common/shell_test_external_view_embedder.cc b/shell/common/shell_test_external_view_embedder.cc index f3a5a0a37d27d..33658b3fa03d1 100644 --- a/shell/common/shell_test_external_view_embedder.cc +++ b/shell/common/shell_test_external_view_embedder.cc @@ -53,4 +53,8 @@ SkCanvas* ShellTestExternalViewEmbedder::GetRootCanvas() { return nullptr; } +bool ShellTestExternalViewEmbedder::SupportDynamicThreadMerging() { + return true; +} + } // namespace flutter diff --git a/shell/common/shell_test_external_view_embedder.h b/shell/common/shell_test_external_view_embedder.h index c67772df74f9a..92924365ef0a8 100644 --- a/shell/common/shell_test_external_view_embedder.h +++ b/shell/common/shell_test_external_view_embedder.h @@ -62,6 +62,9 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder { // |ExternalViewEmbedder| SkCanvas* GetRootCanvas() override; + // |ExternalViewEmbedder| + bool SupportDynamicThreadMerging() override; + const EndFrameCallBack end_frame_call_back_; const PostPrerollResult post_preroll_result_; diff --git a/shell/platform/darwin/ios/ios_surface.h b/shell/platform/darwin/ios/ios_surface.h index 041e3bacedb32..b6e1645f65250 100644 --- a/shell/platform/darwin/ios/ios_surface.h +++ b/shell/platform/darwin/ios/ios_surface.h @@ -88,6 +88,9 @@ class IOSSurface : public ExternalViewEmbedder { void EndFrame(bool should_resubmit_frame, fml::RefPtr raster_thread_merger) override; + // |ExternalViewEmbedder| + bool SupportDynamicThreadMerging() override; + public: FML_DISALLOW_COPY_AND_ASSIGN(IOSSurface); }; diff --git a/shell/platform/darwin/ios/ios_surface.mm b/shell/platform/darwin/ios/ios_surface.mm index de330fb58fc42..32cc614c6af6f 100644 --- a/shell/platform/darwin/ios/ios_surface.mm +++ b/shell/platform/darwin/ios/ios_surface.mm @@ -155,4 +155,9 @@ bool IsIosEmbeddedViewsPreviewEnabled() { return platform_views_controller_->EndFrame(should_resubmit_frame, raster_thread_merger); } +// |ExternalViewEmbedder| +bool IOSSurface::SupportDynamicThreadMerging() { + return true; +} + } // namespace flutter From 31b16d26d400a2a023ec5513518b98724b0332e7 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Thu, 13 Aug 2020 11:55:03 -0700 Subject: [PATCH 2/6] formatting --- .../shell_test_external_view_embedder.cc | 4 +- .../shell_test_external_view_embedder.h | 11 +++- shell/common/shell_unittests.cc | 63 +++++++++++++++++-- 3 files changed, 67 insertions(+), 11 deletions(-) diff --git a/shell/common/shell_test_external_view_embedder.cc b/shell/common/shell_test_external_view_embedder.cc index 33658b3fa03d1..cf16a19f95f07 100644 --- a/shell/common/shell_test_external_view_embedder.cc +++ b/shell/common/shell_test_external_view_embedder.cc @@ -45,7 +45,7 @@ void ShellTestExternalViewEmbedder::SubmitFrame( void ShellTestExternalViewEmbedder::EndFrame( bool should_resubmit_frame, fml::RefPtr raster_thread_merger) { - end_frame_call_back_(should_resubmit_frame); + end_frame_call_back_(should_resubmit_frame, raster_thread_merger); } // |ExternalViewEmbedder| @@ -54,7 +54,7 @@ SkCanvas* ShellTestExternalViewEmbedder::GetRootCanvas() { } bool ShellTestExternalViewEmbedder::SupportDynamicThreadMerging() { - return true; + return support_thread_merging_; } } // namespace flutter diff --git a/shell/common/shell_test_external_view_embedder.h b/shell/common/shell_test_external_view_embedder.h index 92924365ef0a8..0d06e537e69c9 100644 --- a/shell/common/shell_test_external_view_embedder.h +++ b/shell/common/shell_test_external_view_embedder.h @@ -15,12 +15,15 @@ namespace flutter { /// class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder { public: - using EndFrameCallBack = std::function; + using EndFrameCallBack = + std::function)>; ShellTestExternalViewEmbedder(const EndFrameCallBack& end_frame_call_back, - PostPrerollResult post_preroll_result) + PostPrerollResult post_preroll_result, + bool support_thread_merging) : end_frame_call_back_(end_frame_call_back), - post_preroll_result_(post_preroll_result) {} + post_preroll_result_(post_preroll_result), + support_thread_merging_(support_thread_merging) {} ~ShellTestExternalViewEmbedder() = default; @@ -68,6 +71,8 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder { const EndFrameCallBack end_frame_call_back_; const PostPrerollResult post_preroll_result_; + bool support_thread_merging_; + FML_DISALLOW_COPY_AND_ASSIGN(ShellTestExternalViewEmbedder); }; diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index d1ce6ecfbaf0d..68dc10f59cd4a 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -478,18 +478,69 @@ TEST_F(ShellTest, FrameRasterizedCallbackIsCalled) { #if !defined(OS_FUCHSIA) // TODO(sanjayc77): https://github.com/flutter/flutter/issues/53179. Add // support for raster thread merger for Fuchsia. +TEST_F(ShellTest, ExternalEmbedderNoThreadMerger) { + auto settings = CreateSettingsForFixture(); + fml::AutoResetWaitableEvent endFrameLatch; + bool end_frame_called = false; + auto end_frame_callback = + [&](bool should_resubmit_frame, + fml::RefPtr raster_thread_merger) { + ASSERT_TRUE(raster_thread_merger.get() == nullptr); + ASSERT_FALSE(should_resubmit_frame); + end_frame_called = true; + endFrameLatch.Signal(); + }; + auto external_view_embedder = std::make_shared( + end_frame_callback, PostPrerollResult::kResubmitFrame, false); + auto shell = CreateShell(std::move(settings), GetTaskRunnersForFixture(), + false, external_view_embedder); + + // Create the surface needed by rasterizer + PlatformViewNotifyCreated(shell.get()); + + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("emptyMain"); + + RunEngine(shell.get(), std::move(configuration)); + + LayerTreeBuilder builder = [&](std::shared_ptr root) { + SkPictureRecorder recorder; + SkCanvas* recording_canvas = + recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80)); + recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80), + SkPaint(SkColor4f::FromColor(SK_ColorRED))); + auto sk_picture = recorder.finishRecordingAsPicture(); + fml::RefPtr queue = fml::MakeRefCounted( + this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0)); + auto picture_layer = std::make_shared( + SkPoint::Make(10, 10), + flutter::SkiaGPUObject({sk_picture, queue}), false, false); + root->Add(picture_layer); + }; + + PumpOneFrame(shell.get(), 100, 100, builder); + endFrameLatch.Wait(); + + ASSERT_TRUE(end_frame_called); + + DestroyShell(std::move(shell)); +} + TEST_F(ShellTest, ExternalEmbedderEndFrameIsCalledWhenPostPrerollResultIsResubmit) { auto settings = CreateSettingsForFixture(); fml::AutoResetWaitableEvent endFrameLatch; bool end_frame_called = false; - auto end_frame_callback = [&](bool should_resubmit_frame) { - ASSERT_TRUE(should_resubmit_frame); - end_frame_called = true; - endFrameLatch.Signal(); - }; + auto end_frame_callback = + [&](bool should_resubmit_frame, + fml::RefPtr raster_thread_merger) { + ASSERT_TRUE(raster_thread_merger.get() != nullptr); + ASSERT_TRUE(should_resubmit_frame); + end_frame_called = true; + endFrameLatch.Signal(); + }; auto external_view_embedder = std::make_shared( - end_frame_callback, PostPrerollResult::kResubmitFrame); + end_frame_callback, PostPrerollResult::kResubmitFrame, true); auto shell = CreateShell(std::move(settings), GetTaskRunnersForFixture(), false, external_view_embedder); From ecbbf38746d45c70d31e8e0fea58b8a18284e5c7 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Thu, 13 Aug 2020 13:33:09 -0700 Subject: [PATCH 3/6] docs --- flow/embedded_views.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 43bd929f8f614..8a140ece2ea37 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -266,6 +266,10 @@ class ExternalViewEmbedder { // sets the stage for the next pre-roll. virtual void CancelFrame() = 0; + // Indicates the begining of a frame. + // + // The `raster_thread_merger` will be null if |SupportDynamicThreadMerging| + // returns false. virtual void BeginFrame( SkISize frame_size, GrDirectContext* context, @@ -306,10 +310,18 @@ class ExternalViewEmbedder { // A new frame on the platform thread starts immediately. If the GPU thread // still has some task running, there could be two frames being rendered // concurrently, which causes undefined behaviors. + // + // The `raster_thread_merger` will be null if |SupportDynamicThreadMerging| + // returns false. virtual void EndFrame( bool should_resubmit_frame, fml::RefPtr raster_thread_merger) {} + // Whether the embedder should support dynamic thread merging. + // + // Returning `true` results a |RasterThreadMerger| instance to be created. + // * See also |BegineFrame| and |EndFrame| for getting the + // |RasterThreadMerger| instance. virtual bool SupportDynamicThreadMerging(); FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder); From 2f75d1cea2c58d9f4565bdd48423c1a9bb9f8bda Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Thu, 13 Aug 2020 13:33:36 -0700 Subject: [PATCH 4/6] rename --- flow/embedded_views.cc | 2 +- flow/embedded_views.h | 6 +++--- shell/common/rasterizer.cc | 2 +- shell/common/shell_test_external_view_embedder.cc | 2 +- shell/common/shell_test_external_view_embedder.h | 2 +- shell/platform/darwin/ios/ios_surface.h | 2 +- shell/platform/darwin/ios/ios_surface.mm | 2 +- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/flow/embedded_views.cc b/flow/embedded_views.cc index 36a7423f10a29..9441c8dc9470c 100644 --- a/flow/embedded_views.cc +++ b/flow/embedded_views.cc @@ -60,7 +60,7 @@ const std::vector>::const_iterator MutatorsStack::End() return vector_.end(); }; -bool ExternalViewEmbedder::SupportDynamicThreadMerging() { +bool ExternalViewEmbedder::SupportsDynamicThreadMerging() { return false; } diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 8a140ece2ea37..cbfde228786a3 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -268,7 +268,7 @@ class ExternalViewEmbedder { // Indicates the begining of a frame. // - // The `raster_thread_merger` will be null if |SupportDynamicThreadMerging| + // The `raster_thread_merger` will be null if |SupportsDynamicThreadMerging| // returns false. virtual void BeginFrame( SkISize frame_size, @@ -311,7 +311,7 @@ class ExternalViewEmbedder { // still has some task running, there could be two frames being rendered // concurrently, which causes undefined behaviors. // - // The `raster_thread_merger` will be null if |SupportDynamicThreadMerging| + // The `raster_thread_merger` will be null if |SupportsDynamicThreadMerging| // returns false. virtual void EndFrame( bool should_resubmit_frame, @@ -322,7 +322,7 @@ class ExternalViewEmbedder { // Returning `true` results a |RasterThreadMerger| instance to be created. // * See also |BegineFrame| and |EndFrame| for getting the // |RasterThreadMerger| instance. - virtual bool SupportDynamicThreadMerging(); + virtual bool SupportsDynamicThreadMerging(); FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder); diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index f402a37be66f5..0cdaa6a80415a 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -82,7 +82,7 @@ void Rasterizer::Setup(std::unique_ptr surface) { // TODO(sanjayc77): https://github.com/flutter/flutter/issues/53179. Add // support for raster thread merger for Fuchsia. if (surface_->GetExternalViewEmbedder() && - surface_->GetExternalViewEmbedder()->SupportDynamicThreadMerging()) { + surface_->GetExternalViewEmbedder()->SupportsDynamicThreadMerging()) { const auto platform_id = task_runners_.GetPlatformTaskRunner()->GetTaskQueueId(); const auto gpu_id = task_runners_.GetRasterTaskRunner()->GetTaskQueueId(); diff --git a/shell/common/shell_test_external_view_embedder.cc b/shell/common/shell_test_external_view_embedder.cc index cf16a19f95f07..105f109dbab31 100644 --- a/shell/common/shell_test_external_view_embedder.cc +++ b/shell/common/shell_test_external_view_embedder.cc @@ -53,7 +53,7 @@ SkCanvas* ShellTestExternalViewEmbedder::GetRootCanvas() { return nullptr; } -bool ShellTestExternalViewEmbedder::SupportDynamicThreadMerging() { +bool ShellTestExternalViewEmbedder::SupportsDynamicThreadMerging() { return support_thread_merging_; } diff --git a/shell/common/shell_test_external_view_embedder.h b/shell/common/shell_test_external_view_embedder.h index 0d06e537e69c9..8b855a0a7f4f6 100644 --- a/shell/common/shell_test_external_view_embedder.h +++ b/shell/common/shell_test_external_view_embedder.h @@ -66,7 +66,7 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder { SkCanvas* GetRootCanvas() override; // |ExternalViewEmbedder| - bool SupportDynamicThreadMerging() override; + bool SupportsDynamicThreadMerging() override; const EndFrameCallBack end_frame_call_back_; const PostPrerollResult post_preroll_result_; diff --git a/shell/platform/darwin/ios/ios_surface.h b/shell/platform/darwin/ios/ios_surface.h index b6e1645f65250..d5dfca5526c51 100644 --- a/shell/platform/darwin/ios/ios_surface.h +++ b/shell/platform/darwin/ios/ios_surface.h @@ -89,7 +89,7 @@ class IOSSurface : public ExternalViewEmbedder { fml::RefPtr raster_thread_merger) override; // |ExternalViewEmbedder| - bool SupportDynamicThreadMerging() override; + bool SupportsDynamicThreadMerging() override; public: FML_DISALLOW_COPY_AND_ASSIGN(IOSSurface); diff --git a/shell/platform/darwin/ios/ios_surface.mm b/shell/platform/darwin/ios/ios_surface.mm index 32cc614c6af6f..30327582a2599 100644 --- a/shell/platform/darwin/ios/ios_surface.mm +++ b/shell/platform/darwin/ios/ios_surface.mm @@ -156,7 +156,7 @@ bool IsIosEmbeddedViewsPreviewEnabled() { } // |ExternalViewEmbedder| -bool IOSSurface::SupportDynamicThreadMerging() { +bool IOSSurface::SupportsDynamicThreadMerging() { return true; } From 40865b5fbb7f66992467c6e8df9a6579c1bd3e21 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Thu, 13 Aug 2020 13:49:40 -0700 Subject: [PATCH 5/6] enable dyanmic thraed merging on android --- .../external_view_embedder/external_view_embedder.cc | 5 +++++ .../external_view_embedder/external_view_embedder.h | 2 ++ .../external_view_embedder_unittests.cc | 8 ++++++++ 3 files changed, 15 insertions(+) diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.cc b/shell/platform/android/external_view_embedder/external_view_embedder.cc index 9a32dc171eabf..b5479284aa0ef 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -295,4 +295,9 @@ void AndroidExternalViewEmbedder::EndFrame( } } +// |ExternalViewEmbedder| +bool AndroidExternalViewEmbedder::SupportsDynamicThreadMerging() { + return true; +} + } // namespace flutter diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.h b/shell/platform/android/external_view_embedder/external_view_embedder.h index 9e827b83b0ee0..ce755f252a8ac 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -70,6 +70,8 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { bool should_resubmit_frame, fml::RefPtr raster_thread_merger) override; + bool SupportsDynamicThreadMerging() override; + // Gets the rect based on the device pixel ratio of a platform view displayed // on the screen. SkRect GetViewRect(int view_id) const; diff --git a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc index 93356347ae322..05518d10e08e5 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc @@ -556,5 +556,13 @@ TEST(AndroidExternalViewEmbedder, DestroyOverlayLayersOnSizeChange) { raster_thread_merger); } +TEST(AndroidExternalViewEmbedder, SupportsDynamicThreadMerging) { + auto jni_mock = std::make_shared(); + + auto embedder = + std::make_unique(nullptr, jni_mock, nullptr); + ASSERT_TRUE(embedder->SupportDynamicThreadMerging(), true); +} + } // namespace testing } // namespace flutter From a99a7e8658e003b3d791d344153ada3e3e08585c Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Sat, 15 Aug 2020 10:11:51 -0700 Subject: [PATCH 6/6] fix test --- .../external_view_embedder/external_view_embedder_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc index 05518d10e08e5..bdb8ca1c6c286 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc @@ -561,7 +561,7 @@ TEST(AndroidExternalViewEmbedder, SupportsDynamicThreadMerging) { auto embedder = std::make_unique(nullptr, jni_mock, nullptr); - ASSERT_TRUE(embedder->SupportDynamicThreadMerging(), true); + ASSERT_TRUE(embedder->SupportsDynamicThreadMerging()); } } // namespace testing