From 9bbd42b285f6ece1ae11681b151dd2f94e6cc888 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 15 Sep 2020 16:55:11 +0200 Subject: [PATCH 1/4] Add LayerTreeDiscardCallback parameter to Rasterizer::Draw If this callback returns true for given layer tree, the layer tree will be discarded and not rasterized This is useful after the resize event, when layer tree with certain size is expected, but there is a tree with previous size still in pipeline --- flow/compositor_context.h | 4 +++- shell/common/rasterizer.cc | 9 +++++++-- shell/common/rasterizer.h | 9 ++++++++- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/flow/compositor_context.h b/flow/compositor_context.h index afe19271795d3..7631e7e41ee2a 100644 --- a/flow/compositor_context.h +++ b/flow/compositor_context.h @@ -45,7 +45,9 @@ enum class RasterStatus { // only used when thread configuration change occurs. kEnqueuePipeline, // Failed to rasterize the frame. - kFailed + kFailed, + // Layer tree was discarded due to LayerTreeDiscardCallback + kDiscarded }; class CompositorContext { diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index c3f8001ee24e4..ab8f0d1fdad51 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -152,7 +152,8 @@ void Rasterizer::DrawLastLayerTree() { DrawToSurface(*last_layer_tree_); } -void Rasterizer::Draw(fml::RefPtr> pipeline) { +void Rasterizer::Draw(fml::RefPtr> pipeline, + LayerTreeDiscardCallback discardCallback) { TRACE_EVENT0("flutter", "GPURasterizer::Draw"); if (raster_thread_merger_ && !raster_thread_merger_->IsOnRasterizingThread()) { @@ -166,7 +167,11 @@ void Rasterizer::Draw(fml::RefPtr> pipeline) { RasterStatus raster_status = RasterStatus::kFailed; Pipeline::Consumer consumer = [&](std::unique_ptr layer_tree) { - raster_status = DoDraw(std::move(layer_tree)); + if (discardCallback(*layer_tree.get())) { + raster_status = RasterStatus::kDiscarded; + } else { + raster_status = DoDraw(std::move(layer_tree)); + } }; PipelineConsumeResult consume_result = pipeline->Consume(consumer); diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index e400ff16b2e23..faf0f5ee9b895 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -204,6 +204,8 @@ class Rasterizer final : public SnapshotDelegate { /// flutter::TextureRegistry* GetTextureRegistry(); + using LayerTreeDiscardCallback = std::function; + //---------------------------------------------------------------------------- /// @brief Takes the next item from the layer tree pipeline and executes /// the raster thread frame workload for that pipeline item to @@ -232,8 +234,11 @@ class Rasterizer final : public SnapshotDelegate { /// /// @param[in] pipeline The layer tree pipeline to take the next layer tree /// to render from. + /// @param[in] discardCallback if specified and returns true, the layer tree + /// is discarded instead of being rendered /// - void Draw(fml::RefPtr> pipeline); + void Draw(fml::RefPtr> pipeline, + LayerTreeDiscardCallback discardCallback = NoDiscard); //---------------------------------------------------------------------------- /// @brief The type of the screenshot to obtain of the previously @@ -454,6 +459,8 @@ class Rasterizer final : public SnapshotDelegate { void FireNextFrameCallbackIfPresent(); + static bool NoDiscard(const flutter::LayerTree& layer_tree) { return false; } + FML_DISALLOW_COPY_AND_ASSIGN(Rasterizer); }; From 9dab1f2c3387c7e3f12a98a024519d8a4cf3f671 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 15 Sep 2020 17:14:13 +0200 Subject: [PATCH 2/4] Do not rasterize wrong size layer tree after viewport metrics change --- shell/common/shell.cc | 18 +++++++++++++++--- shell/common/shell.h | 7 +++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/shell/common/shell.cc b/shell/common/shell.cc index b8b0686de54dc..6bd27c260e82d 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -832,6 +832,12 @@ void Shell::OnPlatformViewSetViewportMetrics(const ViewportMetrics& metrics) { engine->SetViewportMetrics(metrics); } }); + + { + std::scoped_lock lock(resize_mutex_); + expected_frame_size_ = + SkISize::Make(metrics.physical_width, metrics.physical_height); + } } // |PlatformView::Delegate| @@ -1021,13 +1027,19 @@ void Shell::OnAnimatorDraw(fml::RefPtr> pipeline, } } + auto discard_callback = [this](flutter::LayerTree& tree) { + std::scoped_lock lock(resize_mutex_); + return !expected_frame_size_.isEmpty() && + tree.frame_size() != expected_frame_size_; + }; + task_runners_.GetRasterTaskRunner()->PostTask( [&waiting_for_first_frame = waiting_for_first_frame_, &waiting_for_first_frame_condition = waiting_for_first_frame_condition_, - rasterizer = rasterizer_->GetWeakPtr(), - pipeline = std::move(pipeline)]() { + rasterizer = rasterizer_->GetWeakPtr(), pipeline = std::move(pipeline), + discard_callback = std::move(discard_callback)]() { if (rasterizer) { - rasterizer->Draw(pipeline); + rasterizer->Draw(pipeline, std::move(discard_callback)); if (waiting_for_first_frame.load()) { waiting_for_first_frame.store(false); diff --git a/shell/common/shell.h b/shell/common/shell.h index c8dad14e02193..368e55116778b 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -425,6 +425,13 @@ class Shell final : public PlatformView::Delegate, // and read from the raster thread. std::atomic display_refresh_rate_ = 0.0f; + // protects expected_frame_size_ which is set on platform thread and read on + // raster thread + std::mutex resize_mutex_; + + // used to discard wrong size layer tree produced during interactive resizing + SkISize expected_frame_size_ = SkISize::MakeEmpty(); + // How many frames have been timed since last report. size_t UnreportedFramesCount() const; From 8370a9ebd3031fc2d89f8c43a8e59dccac437f5a Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 15 Sep 2020 22:55:06 +0200 Subject: [PATCH 3/4] Provide frame size of last submitted frame --- shell/common/BUILD.gn | 9 ++++++++- shell/common/shell_test_external_view_embedder.cc | 10 ++++++++++ shell/common/shell_test_external_view_embedder.h | 4 ++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/shell/common/BUILD.gn b/shell/common/BUILD.gn index f377d643ff1c5..a3093d4f44c58 100644 --- a/shell/common/BUILD.gn +++ b/shell/common/BUILD.gn @@ -181,6 +181,13 @@ if (enable_unittests) { ] } + config("shell_test_fixture_sources_config") { + defines = [ + # Required for MSVC STL + "_ENABLE_ATOMIC_ALIGNMENT_FIX", + ] + } + source_set("shell_test_fixture_sources") { testonly = true @@ -214,7 +221,7 @@ if (enable_unittests) { "//third_party/skia", ] - public_configs = [] + public_configs = [ ":shell_test_fixture_sources_config" ] # SwiftShader only supports x86/x64_64 if (target_cpu == "x86" || target_cpu == "x64") { diff --git a/shell/common/shell_test_external_view_embedder.cc b/shell/common/shell_test_external_view_embedder.cc index ba47306afbad8..5a8edc19fb45a 100644 --- a/shell/common/shell_test_external_view_embedder.cc +++ b/shell/common/shell_test_external_view_embedder.cc @@ -24,6 +24,10 @@ int ShellTestExternalViewEmbedder::GetSubmittedFrameCount() { return submitted_frame_count_; } +SkISize ShellTestExternalViewEmbedder::GetLastSubmittedFrameSize() { + return last_submitted_frame_size_; +} + // |ExternalViewEmbedder| void ShellTestExternalViewEmbedder::CancelFrame() {} @@ -61,6 +65,12 @@ void ShellTestExternalViewEmbedder::SubmitFrame( GrDirectContext* context, std::unique_ptr frame) { frame->Submit(); + if (frame && frame->SkiaSurface()) { + last_submitted_frame_size_ = SkISize::Make(frame->SkiaSurface()->width(), + frame->SkiaSurface()->height()); + } else { + last_submitted_frame_size_ = SkISize::MakeEmpty(); + } submitted_frame_count_++; } diff --git a/shell/common/shell_test_external_view_embedder.h b/shell/common/shell_test_external_view_embedder.h index 7220b175a8a84..72c101ed1f4bc 100644 --- a/shell/common/shell_test_external_view_embedder.h +++ b/shell/common/shell_test_external_view_embedder.h @@ -32,6 +32,9 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder { // the external view embedder. int GetSubmittedFrameCount(); + // Returns the size of last submitted frame surface + SkISize GetLastSubmittedFrameSize(); + private: // |ExternalViewEmbedder| void CancelFrame() override; @@ -80,6 +83,7 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder { bool support_thread_merging_; std::atomic submitted_frame_count_; + std::atomic last_submitted_frame_size_; FML_DISALLOW_COPY_AND_ASSIGN(ShellTestExternalViewEmbedder); }; From 9fccda2dd8a90a131551163ae25539bc22a16db2 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 15 Sep 2020 22:56:27 +0200 Subject: [PATCH 4/4] Add ShellTest.DiscardLayerTreeOnResize --- shell/common/shell_unittests.cc | 55 +++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index c6351316ad770..2e46dd762565c 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -2014,5 +2014,60 @@ TEST_F(ShellTest, OnServiceProtocolEstimateRasterCacheMemoryWorks) { DestroyShell(std::move(shell)); } +TEST_F(ShellTest, DiscardLayerTreeOnResize) { + auto settings = CreateSettingsForFixture(); + + SkISize wrong_size = SkISize::Make(400, 100); + SkISize expected_size = SkISize::Make(400, 200); + + fml::AutoResetWaitableEvent end_frame_latch; + + auto end_frame_callback = [&](bool, fml::RefPtr) { + end_frame_latch.Signal(); + }; + + std::shared_ptr external_view_embedder = + std::make_shared( + std::move(end_frame_callback), PostPrerollResult::kSuccess, true); + + std::unique_ptr shell = CreateShell( + settings, GetTaskRunnersForFixture(), false, external_view_embedder); + + // Create the surface needed by rasterizer + PlatformViewNotifyCreated(shell.get()); + + fml::TaskRunner::RunNowOrPostTask( + shell->GetTaskRunners().GetPlatformTaskRunner(), + [&shell, &expected_size]() { + shell->GetPlatformView()->SetViewportMetrics( + {1.0, static_cast(expected_size.width()), + static_cast(expected_size.height())}); + }); + + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("emptyMain"); + + RunEngine(shell.get(), std::move(configuration)); + + fml::WeakPtr runtime_delegate = shell->GetEngine(); + + PumpOneFrame(shell.get(), static_cast(wrong_size.width()), + static_cast(wrong_size.height())); + + end_frame_latch.Wait(); + + ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount()); + + PumpOneFrame(shell.get(), static_cast(expected_size.width()), + static_cast(expected_size.height())); + + end_frame_latch.Wait(); + + ASSERT_EQ(1, external_view_embedder->GetSubmittedFrameCount()); + ASSERT_EQ(expected_size, external_view_embedder->GetLastSubmittedFrameSize()); + + DestroyShell(std::move(shell)); +} + } // namespace testing } // namespace flutter