From 4fd366fad9bb63aa3bfa7462e527f278bf2e5e0a Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Tue, 31 Aug 2021 17:26:11 +0800 Subject: [PATCH 01/14] Started providing the GPU sync switch to Rasterizer.Draw() --- flow/surface.cc | 4 ++++ flow/surface.h | 2 ++ shell/common/rasterizer.cc | 19 +++++++++++++++++-- shell/gpu/gpu_surface_gl.cc | 5 +++++ shell/gpu/gpu_surface_gl.h | 3 +++ shell/gpu/gpu_surface_gl_delegate.cc | 4 ++++ shell/gpu/gpu_surface_gl_delegate.h | 3 +++ shell/gpu/gpu_surface_metal.h | 3 +++ shell/gpu/gpu_surface_metal.mm | 4 ++++ shell/gpu/gpu_surface_metal_delegate.cc | 4 ++++ shell/gpu/gpu_surface_metal_delegate.h | 5 +++++ shell/platform/darwin/ios/ios_surface_gl.h | 3 +++ shell/platform/darwin/ios/ios_surface_gl.mm | 5 +++++ shell/platform/darwin/ios/ios_surface_metal.h | 3 +++ .../platform/darwin/ios/ios_surface_metal.mm | 5 +++++ 15 files changed, 70 insertions(+), 2 deletions(-) diff --git a/flow/surface.cc b/flow/surface.cc index 7dbe56c1e8e78..db92fe19c9c9c 100644 --- a/flow/surface.cc +++ b/flow/surface.cc @@ -18,4 +18,8 @@ bool Surface::ClearRenderContext() { return false; } +bool Surface::IsAllowDrawingToSurfaceWhenGpuDisabled() const { + return true; +} + } // namespace flutter diff --git a/flow/surface.h b/flow/surface.h index 21b248c074d4c..437d110245824 100644 --- a/flow/surface.h +++ b/flow/surface.h @@ -33,6 +33,8 @@ class Surface { virtual bool ClearRenderContext(); + virtual bool IsAllowDrawingToSurfaceWhenGpuDisabled() const; + private: FML_DISALLOW_COPY_AND_ASSIGN(Surface); }; diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 502e04c4416ae..5c0dbe0a6522f 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -172,8 +172,23 @@ void Rasterizer::Draw( if (discardCallback(*layer_tree.get())) { raster_status = RasterStatus::kDiscarded; } else { - raster_status = - DoDraw(std::move(frame_timings_recorder), std::move(layer_tree)); + if (!layer_tree || !surface_) { + raster_status = RasterStatus::kFailed; + } + + if (surface_->IsAllowDrawingToSurfaceWhenGpuDisabled()) { + raster_status = DoDraw(std::move(frame_timings_recorder), + std::move(layer_tree)); + } else { + delegate_.GetIsGpuDisabledSyncSwitch()->Execute( + fml::SyncSwitch::Handlers() + .SetIfTrue( + [&] { raster_status = RasterStatus::kDiscarded; }) + .SetIfFalse([&] { + raster_status = DoDraw(std::move(frame_timings_recorder), + std::move(layer_tree)); + })); + } } }; diff --git a/shell/gpu/gpu_surface_gl.cc b/shell/gpu/gpu_surface_gl.cc index e78135663764b..72276344816d2 100644 --- a/shell/gpu/gpu_surface_gl.cc +++ b/shell/gpu/gpu_surface_gl.cc @@ -318,4 +318,9 @@ bool GPUSurfaceGL::ClearRenderContext() { return delegate_->GLContextClearCurrent(); } +// |Surface| +bool GPUSurfaceGL::IsAllowDrawingToSurfaceWhenGpuDisabled() const { + return delegate_->IsAllowDrawingToSurfaceWhenGpuDisabled(); +} + } // namespace flutter diff --git a/shell/gpu/gpu_surface_gl.h b/shell/gpu/gpu_surface_gl.h index f1be74c495b2d..51622f68660d1 100644 --- a/shell/gpu/gpu_surface_gl.h +++ b/shell/gpu/gpu_surface_gl.h @@ -50,6 +50,9 @@ class GPUSurfaceGL : public Surface { // |Surface| bool ClearRenderContext() override; + // |Surface| + bool IsAllowDrawingToSurfaceWhenGpuDisabled() const override; + private: GPUSurfaceGLDelegate* delegate_; sk_sp context_; diff --git a/shell/gpu/gpu_surface_gl_delegate.cc b/shell/gpu/gpu_surface_gl_delegate.cc index 66f9df261f0ab..4ff0f08ea2a0e 100644 --- a/shell/gpu/gpu_surface_gl_delegate.cc +++ b/shell/gpu/gpu_surface_gl_delegate.cc @@ -99,4 +99,8 @@ GPUSurfaceGLDelegate::GetDefaultPlatformGLInterface() { return CreateGLInterface(nullptr); } +bool GPUSurfaceGLDelegate::IsAllowDrawingToSurfaceWhenGpuDisabled() const { + return true; +} + } // namespace flutter diff --git a/shell/gpu/gpu_surface_gl_delegate.h b/shell/gpu/gpu_surface_gl_delegate.h index fff96945e00a8..614c0826cbf6d 100644 --- a/shell/gpu/gpu_surface_gl_delegate.h +++ b/shell/gpu/gpu_surface_gl_delegate.h @@ -69,6 +69,9 @@ class GPUSurfaceGLDelegate { // instrumentation to specific GL calls can specify custom GL functions // here. virtual GLProcResolver GetGLProcResolver() const; + + // Whether to allow drawing to the surface when the GPU is disabled + virtual bool IsAllowDrawingToSurfaceWhenGpuDisabled() const; }; } // namespace flutter diff --git a/shell/gpu/gpu_surface_metal.h b/shell/gpu/gpu_surface_metal.h index bd715ecb407d9..3a64143e8f9a8 100644 --- a/shell/gpu/gpu_surface_metal.h +++ b/shell/gpu/gpu_surface_metal.h @@ -49,6 +49,9 @@ class SK_API_AVAILABLE_CA_METAL_LAYER GPUSurfaceMetal : public Surface { // |Surface| std::unique_ptr MakeRenderContextCurrent() override; + // |Surface| + bool IsAllowDrawingToSurfaceWhenGpuDisabled() const override; + std::unique_ptr AcquireFrameFromCAMetalLayer( const SkISize& frame_info); diff --git a/shell/gpu/gpu_surface_metal.mm b/shell/gpu/gpu_surface_metal.mm index ea40dda5893dd..afdaf4de5e912 100644 --- a/shell/gpu/gpu_surface_metal.mm +++ b/shell/gpu/gpu_surface_metal.mm @@ -184,6 +184,10 @@ return std::make_unique(true); } +bool GPUSurfaceMetal::IsAllowDrawingToSurfaceWhenGpuDisabled() const { + return delegate_->IsAllowDrawingToSurfaceWhenGpuDisabled(); +} + void GPUSurfaceMetal::ReleaseUnusedDrawableIfNecessary() { // If the previous surface frame was not submitted before a new one is acquired, the old drawable // needs to be released. An RAII wrapper may not be used because this needs to interoperate with diff --git a/shell/gpu/gpu_surface_metal_delegate.cc b/shell/gpu/gpu_surface_metal_delegate.cc index 427614e1b41d8..1ea305c93eca4 100644 --- a/shell/gpu/gpu_surface_metal_delegate.cc +++ b/shell/gpu/gpu_surface_metal_delegate.cc @@ -16,4 +16,8 @@ MTLRenderTargetType GPUSurfaceMetalDelegate::GetRenderTargetType() { return render_target_type_; } +bool GPUSurfaceMetalDelegate::IsAllowDrawingToSurfaceWhenGpuDisabled() const { + return true; +} + } // namespace flutter diff --git a/shell/gpu/gpu_surface_metal_delegate.h b/shell/gpu/gpu_surface_metal_delegate.h index b11a622bd7489..941556084d51b 100644 --- a/shell/gpu/gpu_surface_metal_delegate.h +++ b/shell/gpu/gpu_surface_metal_delegate.h @@ -89,6 +89,11 @@ class GPUSurfaceMetalDelegate { /// virtual bool PresentTexture(GPUMTLTextureInfo texture) const = 0; + //------------------------------------------------------------------------------ + /// @brief Whether to allow drawing to the surface when the GPU is disabled + /// + virtual bool IsAllowDrawingToSurfaceWhenGpuDisabled() const; + MTLRenderTargetType GetRenderTargetType(); private: diff --git a/shell/platform/darwin/ios/ios_surface_gl.h b/shell/platform/darwin/ios/ios_surface_gl.h index 0172e225a6bd8..a70eb82c51a04 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.h +++ b/shell/platform/darwin/ios/ios_surface_gl.h @@ -46,6 +46,9 @@ class IOSSurfaceGL final : public IOSSurface, public GPUSurfaceGLDelegate { // |GPUSurfaceGLDelegate| bool SurfaceSupportsReadback() const override; + // |GPUSurfaceGLDelegate| + bool IsAllowDrawingToSurfaceWhenGpuDisabled() const override; + private: std::unique_ptr render_target_; diff --git a/shell/platform/darwin/ios/ios_surface_gl.mm b/shell/platform/darwin/ios/ios_surface_gl.mm index 34983f3a5bb21..bdcf013fd8763 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.mm +++ b/shell/platform/darwin/ios/ios_surface_gl.mm @@ -89,4 +89,9 @@ return IsValid() && render_target_->PresentRenderBuffer(); } +// |GPUSurfaceGLDelegate| +bool IOSSurfaceGL::IsAllowDrawingToSurfaceWhenGpuDisabled() const { + return false; +} + } // namespace flutter diff --git a/shell/platform/darwin/ios/ios_surface_metal.h b/shell/platform/darwin/ios/ios_surface_metal.h index c2e9b8c50d0da..ffbc00b1c2c16 100644 --- a/shell/platform/darwin/ios/ios_surface_metal.h +++ b/shell/platform/darwin/ios/ios_surface_metal.h @@ -49,6 +49,9 @@ class SK_API_AVAILABLE_CA_METAL_LAYER IOSSurfaceMetal final : public IOSSurface, // |GPUSurfaceMetalDelegate| bool PresentTexture(GPUMTLTextureInfo texture) const override; + // |GPUSurfaceMetalDelegate| + bool IsAllowDrawingToSurfaceWhenGpuDisabled() const override; + FML_DISALLOW_COPY_AND_ASSIGN(IOSSurfaceMetal); }; diff --git a/shell/platform/darwin/ios/ios_surface_metal.mm b/shell/platform/darwin/ios/ios_surface_metal.mm index 64909e0b15b1b..d7d311d67e3eb 100644 --- a/shell/platform/darwin/ios/ios_surface_metal.mm +++ b/shell/platform/darwin/ios/ios_surface_metal.mm @@ -98,4 +98,9 @@ return false; } +// |GPUSurfaceMetalDelegate| +bool IOSSurfaceMetal::IsAllowDrawingToSurfaceWhenGpuDisabled() const { + return false; +} + } // namespace flutter From b85350ec86d798b93e1296e2516f2803ec3b0e98 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Tue, 31 Aug 2021 19:32:15 +0800 Subject: [PATCH 02/14] Return early when raster status is failed --- shell/common/rasterizer.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 5c0dbe0a6522f..0be75932cab55 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -174,6 +174,7 @@ void Rasterizer::Draw( } else { if (!layer_tree || !surface_) { raster_status = RasterStatus::kFailed; + return; } if (surface_->IsAllowDrawingToSurfaceWhenGpuDisabled()) { From d1875bda49a2702f093fc29ad20765ac2401ed3c Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Tue, 31 Aug 2021 23:29:54 +0800 Subject: [PATCH 03/14] Make SyncSwitch recursive --- fml/synchronization/sync_switch.h | 2 +- fml/synchronization/sync_switch_unittest.cc | 32 +++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/fml/synchronization/sync_switch.h b/fml/synchronization/sync_switch.h index 08451c7476f89..78d748154cb41 100644 --- a/fml/synchronization/sync_switch.h +++ b/fml/synchronization/sync_switch.h @@ -53,7 +53,7 @@ class SyncSwitch { void SetSwitch(bool value); private: - mutable std::mutex mutex_; + mutable std::recursive_mutex mutex_; bool value_; FML_DISALLOW_COPY_AND_ASSIGN(SyncSwitch); diff --git a/fml/synchronization/sync_switch_unittest.cc b/fml/synchronization/sync_switch_unittest.cc index 09994496cf907..4efa9e406209f 100644 --- a/fml/synchronization/sync_switch_unittest.cc +++ b/fml/synchronization/sync_switch_unittest.cc @@ -22,6 +22,38 @@ TEST(SyncSwitchTest, Basic) { EXPECT_TRUE(switchValue); } +TEST(SyncSwitchTest, Recursive) { + SyncSwitch syncSwitch; + bool switchValue = false; + syncSwitch.Execute( + SyncSwitch::Handlers() + .SetIfTrue([&] { + syncSwitch.Execute(SyncSwitch::Handlers() + .SetIfTrue([&] { switchValue = true; }) + .SetIfFalse([&] { switchValue = false; })); + }) + .SetIfFalse([&] { + syncSwitch.Execute(SyncSwitch::Handlers() + .SetIfTrue([&] { switchValue = true; }) + .SetIfFalse([&] { switchValue = false; })); + })); + EXPECT_FALSE(switchValue); + syncSwitch.SetSwitch(true); + syncSwitch.Execute( + SyncSwitch::Handlers() + .SetIfTrue([&] { + syncSwitch.Execute(SyncSwitch::Handlers() + .SetIfTrue([&] { switchValue = true; }) + .SetIfFalse([&] { switchValue = false; })); + }) + .SetIfFalse([&] { + syncSwitch.Execute(SyncSwitch::Handlers() + .SetIfTrue([&] { switchValue = true; }) + .SetIfFalse([&] { switchValue = false; })); + })); + EXPECT_TRUE(switchValue); +} + TEST(SyncSwitchTest, NoopIfUndefined) { SyncSwitch syncSwitch; bool switchValue = false; From 4816654f8c0386a9c6183dd41d5f3d2401d1e630 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 1 Sep 2021 05:28:52 +0800 Subject: [PATCH 04/14] Replace IsAllowDrawingToSurfaceWhenGpuDisabled with AllowsDrawingWhenGpuDisabled --- flow/surface.cc | 2 +- flow/surface.h | 2 +- shell/common/rasterizer.cc | 2 +- shell/gpu/gpu_surface_gl.cc | 4 ++-- shell/gpu/gpu_surface_gl.h | 2 +- shell/gpu/gpu_surface_gl_delegate.cc | 2 +- shell/gpu/gpu_surface_gl_delegate.h | 2 +- shell/gpu/gpu_surface_metal.h | 2 +- shell/gpu/gpu_surface_metal.mm | 4 ++-- shell/gpu/gpu_surface_metal_delegate.cc | 2 +- shell/gpu/gpu_surface_metal_delegate.h | 2 +- shell/platform/darwin/ios/ios_surface_gl.h | 2 +- shell/platform/darwin/ios/ios_surface_gl.mm | 2 +- shell/platform/darwin/ios/ios_surface_metal.h | 2 +- shell/platform/darwin/ios/ios_surface_metal.mm | 2 +- 15 files changed, 17 insertions(+), 17 deletions(-) diff --git a/flow/surface.cc b/flow/surface.cc index db92fe19c9c9c..79c8c8d7245d8 100644 --- a/flow/surface.cc +++ b/flow/surface.cc @@ -18,7 +18,7 @@ bool Surface::ClearRenderContext() { return false; } -bool Surface::IsAllowDrawingToSurfaceWhenGpuDisabled() const { +bool Surface::AllowsDrawingWhenGpuDisabled() const { return true; } diff --git a/flow/surface.h b/flow/surface.h index 437d110245824..fc8daaebfa771 100644 --- a/flow/surface.h +++ b/flow/surface.h @@ -33,7 +33,7 @@ class Surface { virtual bool ClearRenderContext(); - virtual bool IsAllowDrawingToSurfaceWhenGpuDisabled() const; + virtual bool AllowsDrawingWhenGpuDisabled() const; private: FML_DISALLOW_COPY_AND_ASSIGN(Surface); diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 0be75932cab55..e7d29ce899dca 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -177,7 +177,7 @@ void Rasterizer::Draw( return; } - if (surface_->IsAllowDrawingToSurfaceWhenGpuDisabled()) { + if (surface_->AllowsDrawingWhenGpuDisabled()) { raster_status = DoDraw(std::move(frame_timings_recorder), std::move(layer_tree)); } else { diff --git a/shell/gpu/gpu_surface_gl.cc b/shell/gpu/gpu_surface_gl.cc index 72276344816d2..9d06aa9d35715 100644 --- a/shell/gpu/gpu_surface_gl.cc +++ b/shell/gpu/gpu_surface_gl.cc @@ -319,8 +319,8 @@ bool GPUSurfaceGL::ClearRenderContext() { } // |Surface| -bool GPUSurfaceGL::IsAllowDrawingToSurfaceWhenGpuDisabled() const { - return delegate_->IsAllowDrawingToSurfaceWhenGpuDisabled(); +bool GPUSurfaceGL::AllowsDrawingWhenGpuDisabled() const { + return delegate_->AllowsDrawingWhenGpuDisabled(); } } // namespace flutter diff --git a/shell/gpu/gpu_surface_gl.h b/shell/gpu/gpu_surface_gl.h index 51622f68660d1..e4b04d31f9ed5 100644 --- a/shell/gpu/gpu_surface_gl.h +++ b/shell/gpu/gpu_surface_gl.h @@ -51,7 +51,7 @@ class GPUSurfaceGL : public Surface { bool ClearRenderContext() override; // |Surface| - bool IsAllowDrawingToSurfaceWhenGpuDisabled() const override; + bool AllowsDrawingWhenGpuDisabled() const override; private: GPUSurfaceGLDelegate* delegate_; diff --git a/shell/gpu/gpu_surface_gl_delegate.cc b/shell/gpu/gpu_surface_gl_delegate.cc index 4ff0f08ea2a0e..70bdfc03ba4bb 100644 --- a/shell/gpu/gpu_surface_gl_delegate.cc +++ b/shell/gpu/gpu_surface_gl_delegate.cc @@ -99,7 +99,7 @@ GPUSurfaceGLDelegate::GetDefaultPlatformGLInterface() { return CreateGLInterface(nullptr); } -bool GPUSurfaceGLDelegate::IsAllowDrawingToSurfaceWhenGpuDisabled() const { +bool GPUSurfaceGLDelegate::AllowsDrawingWhenGpuDisabled() const { return true; } diff --git a/shell/gpu/gpu_surface_gl_delegate.h b/shell/gpu/gpu_surface_gl_delegate.h index 614c0826cbf6d..fb13c8212390c 100644 --- a/shell/gpu/gpu_surface_gl_delegate.h +++ b/shell/gpu/gpu_surface_gl_delegate.h @@ -71,7 +71,7 @@ class GPUSurfaceGLDelegate { virtual GLProcResolver GetGLProcResolver() const; // Whether to allow drawing to the surface when the GPU is disabled - virtual bool IsAllowDrawingToSurfaceWhenGpuDisabled() const; + virtual bool AllowsDrawingWhenGpuDisabled() const; }; } // namespace flutter diff --git a/shell/gpu/gpu_surface_metal.h b/shell/gpu/gpu_surface_metal.h index 3a64143e8f9a8..8899089383df5 100644 --- a/shell/gpu/gpu_surface_metal.h +++ b/shell/gpu/gpu_surface_metal.h @@ -50,7 +50,7 @@ class SK_API_AVAILABLE_CA_METAL_LAYER GPUSurfaceMetal : public Surface { std::unique_ptr MakeRenderContextCurrent() override; // |Surface| - bool IsAllowDrawingToSurfaceWhenGpuDisabled() const override; + bool AllowsDrawingWhenGpuDisabled() const override; std::unique_ptr AcquireFrameFromCAMetalLayer( const SkISize& frame_info); diff --git a/shell/gpu/gpu_surface_metal.mm b/shell/gpu/gpu_surface_metal.mm index afdaf4de5e912..d3ec678d8bb5e 100644 --- a/shell/gpu/gpu_surface_metal.mm +++ b/shell/gpu/gpu_surface_metal.mm @@ -184,8 +184,8 @@ return std::make_unique(true); } -bool GPUSurfaceMetal::IsAllowDrawingToSurfaceWhenGpuDisabled() const { - return delegate_->IsAllowDrawingToSurfaceWhenGpuDisabled(); +bool GPUSurfaceMetal::AllowsDrawingWhenGpuDisabled() const { + return delegate_->AllowsDrawingWhenGpuDisabled(); } void GPUSurfaceMetal::ReleaseUnusedDrawableIfNecessary() { diff --git a/shell/gpu/gpu_surface_metal_delegate.cc b/shell/gpu/gpu_surface_metal_delegate.cc index 1ea305c93eca4..7a22d8de88a89 100644 --- a/shell/gpu/gpu_surface_metal_delegate.cc +++ b/shell/gpu/gpu_surface_metal_delegate.cc @@ -16,7 +16,7 @@ MTLRenderTargetType GPUSurfaceMetalDelegate::GetRenderTargetType() { return render_target_type_; } -bool GPUSurfaceMetalDelegate::IsAllowDrawingToSurfaceWhenGpuDisabled() const { +bool GPUSurfaceMetalDelegate::AllowsDrawingWhenGpuDisabled() const { return true; } diff --git a/shell/gpu/gpu_surface_metal_delegate.h b/shell/gpu/gpu_surface_metal_delegate.h index 941556084d51b..56112042fbc1d 100644 --- a/shell/gpu/gpu_surface_metal_delegate.h +++ b/shell/gpu/gpu_surface_metal_delegate.h @@ -92,7 +92,7 @@ class GPUSurfaceMetalDelegate { //------------------------------------------------------------------------------ /// @brief Whether to allow drawing to the surface when the GPU is disabled /// - virtual bool IsAllowDrawingToSurfaceWhenGpuDisabled() const; + virtual bool AllowsDrawingWhenGpuDisabled() const; MTLRenderTargetType GetRenderTargetType(); diff --git a/shell/platform/darwin/ios/ios_surface_gl.h b/shell/platform/darwin/ios/ios_surface_gl.h index a70eb82c51a04..a541e28e7c21c 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.h +++ b/shell/platform/darwin/ios/ios_surface_gl.h @@ -47,7 +47,7 @@ class IOSSurfaceGL final : public IOSSurface, public GPUSurfaceGLDelegate { bool SurfaceSupportsReadback() const override; // |GPUSurfaceGLDelegate| - bool IsAllowDrawingToSurfaceWhenGpuDisabled() const override; + bool AllowsDrawingWhenGpuDisabled() const override; private: std::unique_ptr render_target_; diff --git a/shell/platform/darwin/ios/ios_surface_gl.mm b/shell/platform/darwin/ios/ios_surface_gl.mm index bdcf013fd8763..e1c5bde675499 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.mm +++ b/shell/platform/darwin/ios/ios_surface_gl.mm @@ -90,7 +90,7 @@ } // |GPUSurfaceGLDelegate| -bool IOSSurfaceGL::IsAllowDrawingToSurfaceWhenGpuDisabled() const { +bool IOSSurfaceGL::AllowsDrawingWhenGpuDisabled() const { return false; } diff --git a/shell/platform/darwin/ios/ios_surface_metal.h b/shell/platform/darwin/ios/ios_surface_metal.h index ffbc00b1c2c16..2424ceafca3e6 100644 --- a/shell/platform/darwin/ios/ios_surface_metal.h +++ b/shell/platform/darwin/ios/ios_surface_metal.h @@ -50,7 +50,7 @@ class SK_API_AVAILABLE_CA_METAL_LAYER IOSSurfaceMetal final : public IOSSurface, bool PresentTexture(GPUMTLTextureInfo texture) const override; // |GPUSurfaceMetalDelegate| - bool IsAllowDrawingToSurfaceWhenGpuDisabled() const override; + bool AllowsDrawingWhenGpuDisabled() const override; FML_DISALLOW_COPY_AND_ASSIGN(IOSSurfaceMetal); }; diff --git a/shell/platform/darwin/ios/ios_surface_metal.mm b/shell/platform/darwin/ios/ios_surface_metal.mm index d7d311d67e3eb..2df3fd0840897 100644 --- a/shell/platform/darwin/ios/ios_surface_metal.mm +++ b/shell/platform/darwin/ios/ios_surface_metal.mm @@ -99,7 +99,7 @@ } // |GPUSurfaceMetalDelegate| -bool IOSSurfaceMetal::IsAllowDrawingToSurfaceWhenGpuDisabled() const { +bool IOSSurfaceMetal::AllowsDrawingWhenGpuDisabled() const { return false; } From 01ee1986104a8f5deb4092fad97b0d0098f91ab9 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 1 Sep 2021 06:16:28 +0800 Subject: [PATCH 05/14] Remove early return in DoDraw --- shell/common/rasterizer.cc | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index e7d29ce899dca..91ded3fb77919 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -174,10 +174,7 @@ void Rasterizer::Draw( } else { if (!layer_tree || !surface_) { raster_status = RasterStatus::kFailed; - return; - } - - if (surface_->AllowsDrawingWhenGpuDisabled()) { + } else if (surface_->AllowsDrawingWhenGpuDisabled()) { raster_status = DoDraw(std::move(frame_timings_recorder), std::move(layer_tree)); } else { @@ -384,10 +381,6 @@ RasterStatus Rasterizer::DoDraw( .GetRasterTaskRunner() ->RunsTasksOnCurrentThread()); - if (!layer_tree || !surface_) { - return RasterStatus::kFailed; - } - frame_timings_recorder->RecordRasterStart(fml::TimePoint::Now()); PersistentCache* persistent_cache = PersistentCache::GetCacheForProcess(); From 290836f6e3f5810a9de0fb22048968750f1619cc Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 1 Sep 2021 10:38:41 +0800 Subject: [PATCH 06/14] Move sync switch to DrawToSurface --- shell/common/rasterizer.cc | 45 +++++++++++++++++++++++++------------- shell/common/rasterizer.h | 3 +++ 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 91ded3fb77919..6adab7416d0ef 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -172,21 +172,8 @@ void Rasterizer::Draw( if (discardCallback(*layer_tree.get())) { raster_status = RasterStatus::kDiscarded; } else { - if (!layer_tree || !surface_) { - raster_status = RasterStatus::kFailed; - } else if (surface_->AllowsDrawingWhenGpuDisabled()) { - raster_status = DoDraw(std::move(frame_timings_recorder), - std::move(layer_tree)); - } else { - delegate_.GetIsGpuDisabledSyncSwitch()->Execute( - fml::SyncSwitch::Handlers() - .SetIfTrue( - [&] { raster_status = RasterStatus::kDiscarded; }) - .SetIfFalse([&] { - raster_status = DoDraw(std::move(frame_timings_recorder), - std::move(layer_tree)); - })); - } + raster_status = + DoDraw(std::move(frame_timings_recorder), std::move(layer_tree)); } }; @@ -381,6 +368,10 @@ RasterStatus Rasterizer::DoDraw( .GetRasterTaskRunner() ->RunsTasksOnCurrentThread()); + if (!layer_tree || !surface_) { + return RasterStatus::kFailed; + } + frame_timings_recorder->RecordRasterStart(fml::TimePoint::Now()); PersistentCache* persistent_cache = PersistentCache::GetCacheForProcess(); @@ -394,6 +385,8 @@ RasterStatus Rasterizer::DoDraw( raster_status == RasterStatus::kSkipAndRetry) { resubmitted_layer_tree_ = std::move(layer_tree); return raster_status; + } else if (raster_status == RasterStatus::kDiscarded) { + return raster_status; } if (persistent_cache->IsDumpingSkp() && @@ -474,6 +467,28 @@ RasterStatus Rasterizer::DrawToSurface( TRACE_EVENT0("flutter", "Rasterizer::DrawToSurface"); FML_DCHECK(surface_); + RasterStatus raster_status; + if (surface_->AllowsDrawingWhenGpuDisabled()) { + raster_status = DrawToSurfaceUnsafe(frame_timings_recorder, layer_tree); + } else { + delegate_.GetIsGpuDisabledSyncSwitch()->Execute( + fml::SyncSwitch::Handlers() + .SetIfTrue([&] { raster_status = RasterStatus::kDiscarded; }) + .SetIfFalse([&] { + raster_status = + DrawToSurfaceUnsafe(frame_timings_recorder, layer_tree); + })); + } + + return raster_status; +} + +RasterStatus Rasterizer::DrawToSurfaceUnsafe( + FrameTimingsRecorder& frame_timings_recorder, + flutter::LayerTree& layer_tree) { + TRACE_EVENT0("flutter", "Rasterizer::DrawToSurfaceUnsafe"); + FML_DCHECK(surface_); + compositor_context_->ui_time().SetLapTime( frame_timings_recorder.GetBuildDuration()); diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index 15beadf5511a5..995573a27f664 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -492,6 +492,9 @@ class Rasterizer final : public SnapshotDelegate { RasterStatus DrawToSurface(FrameTimingsRecorder& frame_timings_recorder, flutter::LayerTree& layer_tree); + RasterStatus DrawToSurfaceUnsafe(FrameTimingsRecorder& frame_timings_recorder, + flutter::LayerTree& layer_tree); + void FireNextFrameCallbackIfPresent(); static bool NoDiscard(const flutter::LayerTree& layer_tree) { return false; } From 4d1b9b5b1275aca4dfe06132d9736d2547101f22 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 1 Sep 2021 11:22:27 +0800 Subject: [PATCH 07/14] Revert "started providing the GPU sync switch to external view embedders (#22302)" This reverts commit 1c3bc02649176159fd024c7f85b6951ba406fd0c. --- flow/embedded_views.cc | 6 ++---- flow/embedded_views.h | 7 ++----- shell/common/rasterizer.cc | 5 ++--- shell/common/rasterizer_unittests.cc | 6 ++---- .../shell_test_external_view_embedder.cc | 3 +-- .../shell_test_external_view_embedder.h | 4 +--- .../external_view_embedder.cc | 3 +-- .../external_view_embedder.h | 4 +--- .../external_view_embedder_unittests.cc | 14 +++++++------- .../framework/Source/FlutterPlatformViews.mm | 19 +++---------------- .../Source/FlutterPlatformViewsTest.mm | 14 ++++++-------- .../Source/FlutterPlatformViews_Internal.h | 13 +++++++------ .../darwin/ios/ios_external_view_embedder.h | 4 +--- .../darwin/ios/ios_external_view_embedder.mm | 9 +++------ .../embedder_external_view_embedder.cc | 3 +-- .../embedder_external_view_embedder.h | 4 +--- .../flutter/fuchsia_external_view_embedder.cc | 3 +-- .../flutter/fuchsia_external_view_embedder.h | 4 +--- .../fuchsia/flutter/platform_view_unittest.cc | 4 +--- 19 files changed, 44 insertions(+), 85 deletions(-) diff --git a/flow/embedded_views.cc b/flow/embedded_views.cc index bf4ccaad319fb..9441c8dc9470c 100644 --- a/flow/embedded_views.cc +++ b/flow/embedded_views.cc @@ -6,10 +6,8 @@ namespace flutter { -void ExternalViewEmbedder::SubmitFrame( - GrDirectContext* context, - std::unique_ptr frame, - const std::shared_ptr& gpu_disable_sync_switch) { +void ExternalViewEmbedder::SubmitFrame(GrDirectContext* context, + std::unique_ptr frame) { frame->Submit(); }; diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 8f8e228636169..c94d671752e0c 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -10,7 +10,6 @@ #include "flutter/flow/surface_frame.h" #include "flutter/fml/memory/ref_counted.h" #include "flutter/fml/raster_thread_merger.h" -#include "flutter/fml/synchronization/sync_switch.h" #include "third_party/skia/include/core/SkCanvas.h" #include "third_party/skia/include/core/SkPath.h" #include "third_party/skia/include/core/SkPoint.h" @@ -313,10 +312,8 @@ class ExternalViewEmbedder { // This method can mutate the root Skia canvas before submitting the frame. // // It can also allocate frames for overlay surfaces to compose hybrid views. - virtual void SubmitFrame( - GrDirectContext* context, - std::unique_ptr frame, - const std::shared_ptr& gpu_disable_sync_switch); + virtual void SubmitFrame(GrDirectContext* context, + std::unique_ptr frame); // This method provides the embedder a way to do additional tasks after // |SubmitFrame|. For example, merge task runners if `should_resubmit_frame` diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 6adab7416d0ef..68f0973d1fd9f 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -536,9 +536,8 @@ RasterStatus Rasterizer::DrawToSurfaceUnsafe( if (external_view_embedder_ && (!raster_thread_merger_ || raster_thread_merger_->IsMerged())) { FML_DCHECK(!frame->IsSubmitted()); - external_view_embedder_->SubmitFrame( - surface_->GetContext(), std::move(frame), - delegate_.GetIsGpuDisabledSyncSwitch()); + external_view_embedder_->SubmitFrame(surface_->GetContext(), + std::move(frame)); } else { frame->Submit(); } diff --git a/shell/common/rasterizer_unittests.cc b/shell/common/rasterizer_unittests.cc index 3bd3f82465285..af4b17089a437 100644 --- a/shell/common/rasterizer_unittests.cc +++ b/shell/common/rasterizer_unittests.cc @@ -63,11 +63,9 @@ class MockExternalViewEmbedder : public ExternalViewEmbedder { fml::RefPtr raster_thread_merger)); MOCK_METHOD0(GetCurrentCanvases, std::vector()); MOCK_METHOD1(CompositeEmbeddedView, SkCanvas*(int view_id)); - MOCK_METHOD3(SubmitFrame, + MOCK_METHOD2(SubmitFrame, void(GrDirectContext* context, - std::unique_ptr frame, - const std::shared_ptr& - gpu_disable_sync_switch)); + std::unique_ptr frame)); MOCK_METHOD2(EndFrame, void(bool should_resubmit_frame, fml::RefPtr raster_thread_merger)); diff --git a/shell/common/shell_test_external_view_embedder.cc b/shell/common/shell_test_external_view_embedder.cc index 300fc6a57328f..5a8edc19fb45a 100644 --- a/shell/common/shell_test_external_view_embedder.cc +++ b/shell/common/shell_test_external_view_embedder.cc @@ -63,8 +63,7 @@ SkCanvas* ShellTestExternalViewEmbedder::CompositeEmbeddedView(int view_id) { // |ExternalViewEmbedder| void ShellTestExternalViewEmbedder::SubmitFrame( GrDirectContext* context, - std::unique_ptr frame, - const std::shared_ptr& gpu_disable_sync_switch) { + std::unique_ptr frame) { frame->Submit(); if (frame && frame->SkiaSurface()) { last_submitted_frame_size_ = SkISize::Make(frame->SkiaSurface()->width(), diff --git a/shell/common/shell_test_external_view_embedder.h b/shell/common/shell_test_external_view_embedder.h index cd4d6c6ae4baf..72c101ed1f4bc 100644 --- a/shell/common/shell_test_external_view_embedder.h +++ b/shell/common/shell_test_external_view_embedder.h @@ -63,9 +63,7 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder { // |ExternalViewEmbedder| void SubmitFrame(GrDirectContext* context, - std::unique_ptr frame, - const std::shared_ptr& - gpu_disable_sync_switch) override; + std::unique_ptr frame) override; // |ExternalViewEmbedder| void EndFrame( 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 dd7c45148ef17..08596ba8ad925 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -75,8 +75,7 @@ SkRect AndroidExternalViewEmbedder::GetViewRect(int view_id) const { // |ExternalViewEmbedder| void AndroidExternalViewEmbedder::SubmitFrame( GrDirectContext* context, - std::unique_ptr frame, - const std::shared_ptr& gpu_disable_sync_switch) { + std::unique_ptr frame) { TRACE_EVENT0("flutter", "AndroidExternalViewEmbedder::SubmitFrame"); if (!FrameHasPlatformLayers()) { 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 f456674be9a94..a0386167418d1 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -47,9 +47,7 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { // |ExternalViewEmbedder| void SubmitFrame(GrDirectContext* context, - std::unique_ptr frame, - const std::shared_ptr& - gpu_disable_sync_switch) override; + std::unique_ptr frame) override; // |ExternalViewEmbedder| PostPrerollResult PostPrerollAction( 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 24d0063c81714..0b07e6c3a9f2e 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 @@ -334,7 +334,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame) { return true; }); - embedder->SubmitFrame(gr_context.get(), std::move(surface_frame), nullptr); + embedder->SubmitFrame(gr_context.get(), std::move(surface_frame)); // Submits frame if no Android view in the current frame. EXPECT_TRUE(did_submit_frame); // Doesn't resubmit frame. @@ -401,7 +401,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame) { return true; }); - embedder->SubmitFrame(gr_context.get(), std::move(surface_frame), nullptr); + embedder->SubmitFrame(gr_context.get(), std::move(surface_frame)); // Doesn't submit frame if there aren't Android views in the previous frame. EXPECT_FALSE(did_submit_frame); // Resubmits frame. @@ -465,7 +465,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame) { } return true; }); - embedder->SubmitFrame(gr_context.get(), std::move(surface_frame), nullptr); + embedder->SubmitFrame(gr_context.get(), std::move(surface_frame)); // Submits frame if there are Android views in the previous frame. EXPECT_TRUE(did_submit_frame); // Doesn't resubmit frame. @@ -573,7 +573,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame__overlayComposition) { return true; }); - embedder->SubmitFrame(gr_context.get(), std::move(surface_frame), nullptr); + embedder->SubmitFrame(gr_context.get(), std::move(surface_frame)); EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); @@ -640,7 +640,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame__platformViewWithoutAnyOverlay) { return true; }); - embedder->SubmitFrame(gr_context.get(), std::move(surface_frame), nullptr); + embedder->SubmitFrame(gr_context.get(), std::move(surface_frame)); EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); @@ -734,7 +734,7 @@ TEST(AndroidExternalViewEmbedder, DestroyOverlayLayersOnSizeChange) { std::make_unique(SkSurface::MakeNull(1000, 1000), false, [](const SurfaceFrame& surface_frame, SkCanvas* canvas) { return true; }); - embedder->SubmitFrame(gr_context.get(), std::move(surface_frame), nullptr); + embedder->SubmitFrame(gr_context.get(), std::move(surface_frame)); EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); @@ -816,7 +816,7 @@ TEST(AndroidExternalViewEmbedder, DoesNotDestroyOverlayLayersOnSizeChange) { std::make_unique(SkSurface::MakeNull(1000, 1000), false, [](const SurfaceFrame& surface_frame, SkCanvas* canvas) { return true; }); - embedder->SubmitFrame(gr_context.get(), std::move(surface_frame), nullptr); + embedder->SubmitFrame(gr_context.get(), std::move(surface_frame)); EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index ed622f070733b..fdd540c2aa658 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -462,22 +462,9 @@ ); } -bool FlutterPlatformViewsController::SubmitFrame( - GrDirectContext* gr_context, - std::shared_ptr ios_context, - std::unique_ptr frame, - const std::shared_ptr& gpu_disable_sync_switch) { - bool result = false; - gpu_disable_sync_switch->Execute( - fml::SyncSwitch::Handlers().SetIfTrue([&] { result = false; }).SetIfFalse([&] { - result = SubmitFrameGpuSafe(gr_context, ios_context, std::move(frame)); - })); - return result; -} - -bool FlutterPlatformViewsController::SubmitFrameGpuSafe(GrDirectContext* gr_context, - std::shared_ptr ios_context, - std::unique_ptr frame) { +bool FlutterPlatformViewsController::SubmitFrame(GrDirectContext* gr_context, + std::shared_ptr ios_context, + std::unique_ptr frame) { // Any UIKit related code has to run on main thread. FML_DCHECK([[NSThread currentThread] isMainThread]); if (flutter_view_ == nullptr) { diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm index 6cd7c349c8dfe..cd0f867181c05 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm @@ -968,10 +968,8 @@ - (void)testFlutterPlatformViewControllerSubmitFrameWithoutFlutterViewNotCrashin auto mock_surface = std::make_unique( nullptr, true, [](const flutter::SurfaceFrame& surface_frame, SkCanvas* canvas) { return false; }); - auto is_gpu_disabled = std::make_shared(); - is_gpu_disabled->SetSwitch(false); - XCTAssertFalse(flutterPlatformViewsController->SubmitFrame( - nullptr, nullptr, std::move(mock_surface), is_gpu_disabled)); + XCTAssertFalse( + flutterPlatformViewsController->SubmitFrame(nullptr, nullptr, std::move(mock_surface))); auto embeddedViewParams_2 = std::make_unique(finalMatrix, SkSize::Make(300, 300), stack); @@ -980,10 +978,10 @@ - (void)testFlutterPlatformViewControllerSubmitFrameWithoutFlutterViewNotCrashin auto mock_surface_submit_false = std::make_unique( nullptr, true, [](const flutter::SurfaceFrame& surface_frame, SkCanvas* canvas) { return true; }); - auto gpu_is_disabled = std::make_shared(); - gpu_is_disabled->SetSwitch(false); - XCTAssertTrue(flutterPlatformViewsController->SubmitFrame( - nullptr, nullptr, std::move(mock_surface_submit_false), gpu_is_disabled)); + XCTAssertTrue(flutterPlatformViewsController->SubmitFrame(nullptr, nullptr, + std::move(mock_surface_submit_false))); + + flutterPlatformViewsController->Reset(); } - (void) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index c195f870653c2..d1ccccb3092a7 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -173,8 +173,13 @@ class FlutterPlatformViewsController { bool SubmitFrame(GrDirectContext* gr_context, std::shared_ptr ios_context, - std::unique_ptr frame, - const std::shared_ptr& gpu_disable_sync_switch); + std::unique_ptr frame); + + // Invoked at the very end of a frame. + // After invoking this method, nothing should happen on the current TaskRunner during the same + // frame. + void EndFrame(bool should_resubmit_frame, + fml::RefPtr raster_thread_merger); void OnMethodCall(FlutterMethodCall* call, FlutterResult& result); @@ -300,10 +305,6 @@ class FlutterPlatformViewsController { // Commit a CATransaction if |BeginCATransaction| has been called during the frame. void CommitCATransactionIfNeeded(); - bool SubmitFrameGpuSafe(GrDirectContext* gr_context, - std::shared_ptr ios_context, - std::unique_ptr frame); - // Resets the state of the frame. void ResetFrameState(); diff --git a/shell/platform/darwin/ios/ios_external_view_embedder.h b/shell/platform/darwin/ios/ios_external_view_embedder.h index b6ca3b6e6f948..6c023d1b793e1 100644 --- a/shell/platform/darwin/ios/ios_external_view_embedder.h +++ b/shell/platform/darwin/ios/ios_external_view_embedder.h @@ -54,9 +54,7 @@ class IOSExternalViewEmbedder : public ExternalViewEmbedder { // |ExternalViewEmbedder| void SubmitFrame(GrDirectContext* context, - std::unique_ptr frame, - const std::shared_ptr& - gpu_disable_sync_switch) override; + std::unique_ptr frame) override; // |ExternalViewEmbedder| void EndFrame( diff --git a/shell/platform/darwin/ios/ios_external_view_embedder.mm b/shell/platform/darwin/ios/ios_external_view_embedder.mm index 391044d7e90ab..a4921af254af9 100644 --- a/shell/platform/darwin/ios/ios_external_view_embedder.mm +++ b/shell/platform/darwin/ios/ios_external_view_embedder.mm @@ -72,14 +72,11 @@ } // |ExternalViewEmbedder| -void IOSExternalViewEmbedder::SubmitFrame( - GrDirectContext* context, - std::unique_ptr frame, - const std::shared_ptr& gpu_disable_sync_switch) { +void IOSExternalViewEmbedder::SubmitFrame(GrDirectContext* context, + std::unique_ptr frame) { TRACE_EVENT0("flutter", "IOSExternalViewEmbedder::SubmitFrame"); FML_CHECK(platform_views_controller_); - platform_views_controller_->SubmitFrame(std::move(context), ios_context_, std::move(frame), - gpu_disable_sync_switch); + platform_views_controller_->SubmitFrame(std::move(context), ios_context_, std::move(frame)); TRACE_EVENT0("flutter", "IOSExternalViewEmbedder::DidSubmitFrame"); } diff --git a/shell/platform/embedder/embedder_external_view_embedder.cc b/shell/platform/embedder/embedder_external_view_embedder.cc index 1d816ab64e274..a890260887e26 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.cc +++ b/shell/platform/embedder/embedder_external_view_embedder.cc @@ -135,8 +135,7 @@ static FlutterBackingStoreConfig MakeBackingStoreConfig( // |ExternalViewEmbedder| void EmbedderExternalViewEmbedder::SubmitFrame( GrDirectContext* context, - std::unique_ptr frame, - const std::shared_ptr& gpu_disable_sync_switch) { + std::unique_ptr frame) { auto [matched_render_targets, pending_keys] = render_target_cache_.GetExistingTargetsInCache(pending_views_); diff --git a/shell/platform/embedder/embedder_external_view_embedder.h b/shell/platform/embedder/embedder_external_view_embedder.h index 62b4540c92a2c..98fe8b2a56639 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.h +++ b/shell/platform/embedder/embedder_external_view_embedder.h @@ -98,9 +98,7 @@ class EmbedderExternalViewEmbedder final : public ExternalViewEmbedder { // |ExternalViewEmbedder| void SubmitFrame(GrDirectContext* context, - std::unique_ptr frame, - const std::shared_ptr& - gpu_disable_sync_switch) override; + std::unique_ptr frame) override; // |ExternalViewEmbedder| SkCanvas* GetRootCanvas() override; diff --git a/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.cc b/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.cc index 639dd3c1b11c6..c485c04edb482 100644 --- a/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.cc +++ b/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.cc @@ -244,8 +244,7 @@ void FuchsiaExternalViewEmbedder::EndFrame( void FuchsiaExternalViewEmbedder::SubmitFrame( GrDirectContext* context, - std::unique_ptr frame, - const std::shared_ptr& gpu_disable_sync_switch) { + std::unique_ptr frame) { TRACE_EVENT0("flutter", "FuchsiaExternalViewEmbedder::SubmitFrame"); std::vector> frame_surfaces; std::unordered_map frame_surface_indices; diff --git a/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h b/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h index 125c8f843ec56..a5eb09d23d442 100644 --- a/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h +++ b/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h @@ -104,9 +104,7 @@ class FuchsiaExternalViewEmbedder final : public flutter::ExternalViewEmbedder { // |ExternalViewEmbedder| void SubmitFrame(GrDirectContext* context, - std::unique_ptr frame, - const std::shared_ptr& - gpu_disable_sync_switch) override; + std::unique_ptr frame) override; // |ExternalViewEmbedder| void CancelFrame() override { Reset(); } diff --git a/shell/platform/fuchsia/flutter/platform_view_unittest.cc b/shell/platform/fuchsia/flutter/platform_view_unittest.cc index 28501ad4e5cb0..6db634d16595f 100644 --- a/shell/platform/fuchsia/flutter/platform_view_unittest.cc +++ b/shell/platform/fuchsia/flutter/platform_view_unittest.cc @@ -53,9 +53,7 @@ class MockExternalViewEmbedder : public flutter::ExternalViewEmbedder { double device_pixel_ratio, fml::RefPtr raster_thread_merger) override {} void SubmitFrame(GrDirectContext* context, - std::unique_ptr frame, - const std::shared_ptr& - gpu_disable_sync_switch) override { + std::unique_ptr frame) override { return; } From 93eaa3772d3a2df36fadace696a99c2be382bd1c Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 1 Sep 2021 17:58:14 +0800 Subject: [PATCH 08/14] Add rasterizer unit tests --- shell/common/rasterizer_unittests.cc | 182 +++++++++++++++++++++++++++ 1 file changed, 182 insertions(+) diff --git a/shell/common/rasterizer_unittests.cc b/shell/common/rasterizer_unittests.cc index af4b17089a437..bd1da287d8eee 100644 --- a/shell/common/rasterizer_unittests.cc +++ b/shell/common/rasterizer_unittests.cc @@ -37,6 +37,8 @@ class MockDelegate : public Rasterizer::Delegate { class MockSurface : public Surface { public: + + MockSurface(bool allows_drawing_when_gpu_disabled = true):allows_drawing_when_gpu_disabled_(allows_drawing_when_gpu_disabled) {}; MOCK_METHOD0(IsValid, bool()); MOCK_METHOD1(AcquireFrame, std::unique_ptr(const SkISize& size)); @@ -45,6 +47,11 @@ class MockSurface : public Surface { MOCK_METHOD0(GetExternalViewEmbedder, ExternalViewEmbedder*()); MOCK_METHOD0(MakeRenderContextCurrent, std::unique_ptr()); MOCK_METHOD0(ClearRenderContext, bool()); + bool AllowsDrawingWhenGpuDisabled() const override { + return allows_drawing_when_gpu_disabled_; + } + private: + const bool allows_drawing_when_gpu_disabled_; }; class MockExternalViewEmbedder : public ExternalViewEmbedder { @@ -323,4 +330,179 @@ TEST(RasterizerTest, externalViewEmbedderDoesntEndFrameWhenNoSurfaceIsSet) { }); latch.Wait(); } + + +TEST(RasterizerTest, + drawWithGpuEnabledAndSurfaceAllowsDrawingWhenGpuDisabledDoesAcquireFrame) { + std::string test_name = + ::testing::UnitTest::GetInstance()->current_test_info()->name(); + ThreadHost thread_host("io.flutter.test." + test_name + ".", + ThreadHost::Type::Platform | ThreadHost::Type::RASTER | + ThreadHost::Type::IO | ThreadHost::Type::UI); + TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(), + thread_host.raster_thread->GetTaskRunner(), + thread_host.ui_thread->GetTaskRunner(), + thread_host.io_thread->GetTaskRunner()); + MockDelegate delegate; + EXPECT_CALL(delegate, GetTaskRunners()) + .WillRepeatedly(ReturnRef(task_runners)); + EXPECT_CALL(delegate, OnFrameRasterized(_)); + + auto rasterizer = std::make_unique(delegate); + auto surface = std::make_unique(true); + auto is_gpu_disabled_sync_switch = std::make_shared(false); + + auto surface_frame = std::make_unique( + /*surface=*/nullptr, /*supports_readback=*/true, + /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); + EXPECT_CALL(delegate, GetIsGpuDisabledSyncSwitch()).Times(0); + EXPECT_CALL(*surface, AcquireFrame(SkISize())) + .WillOnce(Return(ByMove(std::move(surface_frame)))); + EXPECT_CALL(*surface, MakeRenderContextCurrent()) + .WillOnce(Return(ByMove(std::make_unique(true)))); + + rasterizer->Setup(std::move(surface)); + fml::AutoResetWaitableEvent latch; + thread_host.raster_thread->GetTaskRunner()->PostTask([&] { + auto pipeline = std::make_shared>(/*depth=*/10); + auto layer_tree = std::make_unique(/*frame_size=*/SkISize(), + /*device_pixel_ratio=*/2.0f); + bool result = pipeline->Produce().Complete(std::move(layer_tree)); + EXPECT_TRUE(result); + auto no_discard = [](LayerTree&) { return false; }; + rasterizer->Draw(CreateFinishedBuildRecorder(), pipeline, no_discard); + latch.Signal(); + }); + latch.Wait(); +} + +TEST(RasterizerTest, + drawWithGpuDisabledAndSurfaceAllowsDrawingWhenGpuDisabledDoesAcquireFrame) { + std::string test_name = + ::testing::UnitTest::GetInstance()->current_test_info()->name(); + ThreadHost thread_host("io.flutter.test." + test_name + ".", + ThreadHost::Type::Platform | ThreadHost::Type::RASTER | + ThreadHost::Type::IO | ThreadHost::Type::UI); + TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(), + thread_host.raster_thread->GetTaskRunner(), + thread_host.ui_thread->GetTaskRunner(), + thread_host.io_thread->GetTaskRunner()); + MockDelegate delegate; + EXPECT_CALL(delegate, GetTaskRunners()) + .WillRepeatedly(ReturnRef(task_runners)); + EXPECT_CALL(delegate, OnFrameRasterized(_)); + auto rasterizer = std::make_unique(delegate); + auto surface = std::make_unique(true); + auto is_gpu_disabled_sync_switch = std::make_shared(true); + + auto surface_frame = std::make_unique( + /*surface=*/nullptr, /*supports_readback=*/true, + /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); + EXPECT_CALL(delegate, GetIsGpuDisabledSyncSwitch()).Times(0); + EXPECT_CALL(*surface, AcquireFrame(SkISize())) + .WillOnce(Return(ByMove(std::move(surface_frame)))); + EXPECT_CALL(*surface, MakeRenderContextCurrent()) + .WillOnce(Return(ByMove(std::make_unique(true)))); + + rasterizer->Setup(std::move(surface)); + fml::AutoResetWaitableEvent latch; + thread_host.raster_thread->GetTaskRunner()->PostTask([&] { + auto pipeline = std::make_shared>(/*depth=*/10); + auto layer_tree = std::make_unique(/*frame_size=*/SkISize(), + /*device_pixel_ratio=*/2.0f); + bool result = pipeline->Produce().Complete(std::move(layer_tree)); + EXPECT_TRUE(result); + auto no_discard = [](LayerTree&) { return false; }; + rasterizer->Draw(CreateFinishedBuildRecorder(), pipeline, no_discard); + latch.Signal(); + }); + latch.Wait(); +} + +TEST(RasterizerTest, + drawWithGpuEnabledAndSurfaceDisallowsDrawingWhenGpuDisabledDoesAcquireFrame) { + std::string test_name = + ::testing::UnitTest::GetInstance()->current_test_info()->name(); + ThreadHost thread_host("io.flutter.test." + test_name + ".", + ThreadHost::Type::Platform | ThreadHost::Type::RASTER | + ThreadHost::Type::IO | ThreadHost::Type::UI); + TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(), + thread_host.raster_thread->GetTaskRunner(), + thread_host.ui_thread->GetTaskRunner(), + thread_host.io_thread->GetTaskRunner()); + MockDelegate delegate; + EXPECT_CALL(delegate, GetTaskRunners()) + .WillRepeatedly(ReturnRef(task_runners)); + EXPECT_CALL(delegate, OnFrameRasterized(_)); + auto rasterizer = std::make_unique(delegate); + auto surface = std::make_unique(false); + auto is_gpu_disabled_sync_switch = std::make_shared(false); + + auto surface_frame = std::make_unique( + /*surface=*/nullptr, /*supports_readback=*/true, + /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); + EXPECT_CALL(delegate, GetIsGpuDisabledSyncSwitch()).WillOnce(Return(is_gpu_disabled_sync_switch)); + EXPECT_CALL(*surface, AcquireFrame(SkISize())) + .WillOnce(Return(ByMove(std::move(surface_frame)))); + EXPECT_CALL(*surface, MakeRenderContextCurrent()) + .WillOnce(Return(ByMove(std::make_unique(true)))); + + rasterizer->Setup(std::move(surface)); + fml::AutoResetWaitableEvent latch; + thread_host.raster_thread->GetTaskRunner()->PostTask([&] { + auto pipeline = std::make_shared>(/*depth=*/10); + auto layer_tree = std::make_unique(/*frame_size=*/SkISize(), + /*device_pixel_ratio=*/2.0f); + bool result = pipeline->Produce().Complete(std::move(layer_tree)); + EXPECT_TRUE(result); + auto no_discard = [](LayerTree&) { return false; }; + rasterizer->Draw(CreateFinishedBuildRecorder(), pipeline, no_discard); + latch.Signal(); + }); + latch.Wait(); +} + + +TEST(RasterizerTest, + drawWithGpuDisabledAndSurfaceDisallowsDrawingWhenGpuDisabledDoesntAcquireFrame) { + std::string test_name = + ::testing::UnitTest::GetInstance()->current_test_info()->name(); + ThreadHost thread_host("io.flutter.test." + test_name + ".", + ThreadHost::Type::Platform | ThreadHost::Type::RASTER | + ThreadHost::Type::IO | ThreadHost::Type::UI); + TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(), + thread_host.raster_thread->GetTaskRunner(), + thread_host.ui_thread->GetTaskRunner(), + thread_host.io_thread->GetTaskRunner()); + MockDelegate delegate; + EXPECT_CALL(delegate, GetTaskRunners()) + .WillRepeatedly(ReturnRef(task_runners)); + EXPECT_CALL(delegate, OnFrameRasterized(_)).Times(0); + auto rasterizer = std::make_unique(delegate); + auto surface = std::make_unique(false); + auto is_gpu_disabled_sync_switch = std::make_shared(true); + + auto surface_frame = std::make_unique( + /*surface=*/nullptr, /*supports_readback=*/true, + /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); + EXPECT_CALL(delegate, GetIsGpuDisabledSyncSwitch()).WillOnce(Return(is_gpu_disabled_sync_switch)); + EXPECT_CALL(*surface, AcquireFrame(SkISize())).Times(0); + EXPECT_CALL(*surface, MakeRenderContextCurrent()) + .WillOnce(Return(ByMove(std::make_unique(true)))); + + rasterizer->Setup(std::move(surface)); + fml::AutoResetWaitableEvent latch; + thread_host.raster_thread->GetTaskRunner()->PostTask([&] { + auto pipeline = std::make_shared>(/*depth=*/10); + auto layer_tree = std::make_unique(/*frame_size=*/SkISize(), + /*device_pixel_ratio=*/2.0f); + bool result = pipeline->Produce().Complete(std::move(layer_tree)); + EXPECT_TRUE(result); + auto no_discard = [](LayerTree&) { return false; }; + rasterizer->Draw(CreateFinishedBuildRecorder(), pipeline, no_discard); + latch.Signal(); + }); + latch.Wait(); +} + } // namespace flutter From 314e22f4f42b8942ce9d0073b7c9fc69709b948c Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 1 Sep 2021 18:00:24 +0800 Subject: [PATCH 09/14] Update FlatlandExternalViewEmbedder --- .../fuchsia/flutter/flatland_external_view_embedder.cc | 3 +-- .../fuchsia/flutter/flatland_external_view_embedder.h | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/shell/platform/fuchsia/flutter/flatland_external_view_embedder.cc b/shell/platform/fuchsia/flutter/flatland_external_view_embedder.cc index e84bdf852cac2..1071d4e881dbb 100644 --- a/shell/platform/fuchsia/flutter/flatland_external_view_embedder.cc +++ b/shell/platform/fuchsia/flutter/flatland_external_view_embedder.cc @@ -105,8 +105,7 @@ void FlatlandExternalViewEmbedder::EndFrame( void FlatlandExternalViewEmbedder::SubmitFrame( GrDirectContext* context, - std::unique_ptr frame, - const std::shared_ptr& gpu_disable_sync_switch) { + std::unique_ptr frame) { TRACE_EVENT0("flutter", "FlatlandExternalViewEmbedder::SubmitFrame"); std::vector> frame_surfaces; std::unordered_map frame_surface_indices; diff --git a/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h b/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h index 0084efc819c65..9ee0c42dcdff3 100644 --- a/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h +++ b/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h @@ -86,9 +86,7 @@ class FlatlandExternalViewEmbedder final // |ExternalViewEmbedder| void SubmitFrame(GrDirectContext* context, - std::unique_ptr frame, - const std::shared_ptr& - gpu_disable_sync_switch) override; + std::unique_ptr frame) override; // |ExternalViewEmbedder| void CancelFrame() override { Reset(); } From cfd563f2cb84fcd4a36309322e6124efb0600187 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 1 Sep 2021 18:01:04 +0800 Subject: [PATCH 10/14] Format rasterizer_unittests.cc --- shell/common/rasterizer_unittests.cc | 46 ++++++++++++++++------------ 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/shell/common/rasterizer_unittests.cc b/shell/common/rasterizer_unittests.cc index bd1da287d8eee..f8341a2d724ad 100644 --- a/shell/common/rasterizer_unittests.cc +++ b/shell/common/rasterizer_unittests.cc @@ -37,8 +37,8 @@ class MockDelegate : public Rasterizer::Delegate { class MockSurface : public Surface { public: - - MockSurface(bool allows_drawing_when_gpu_disabled = true):allows_drawing_when_gpu_disabled_(allows_drawing_when_gpu_disabled) {}; + MockSurface(bool allows_drawing_when_gpu_disabled = true) + : allows_drawing_when_gpu_disabled_(allows_drawing_when_gpu_disabled){}; MOCK_METHOD0(IsValid, bool()); MOCK_METHOD1(AcquireFrame, std::unique_ptr(const SkISize& size)); @@ -48,10 +48,11 @@ class MockSurface : public Surface { MOCK_METHOD0(MakeRenderContextCurrent, std::unique_ptr()); MOCK_METHOD0(ClearRenderContext, bool()); bool AllowsDrawingWhenGpuDisabled() const override { - return allows_drawing_when_gpu_disabled_; + return allows_drawing_when_gpu_disabled_; } + private: - const bool allows_drawing_when_gpu_disabled_; + const bool allows_drawing_when_gpu_disabled_; }; class MockExternalViewEmbedder : public ExternalViewEmbedder { @@ -331,7 +332,6 @@ TEST(RasterizerTest, externalViewEmbedderDoesntEndFrameWhenNoSurfaceIsSet) { latch.Wait(); } - TEST(RasterizerTest, drawWithGpuEnabledAndSurfaceAllowsDrawingWhenGpuDisabledDoesAcquireFrame) { std::string test_name = @@ -350,7 +350,8 @@ TEST(RasterizerTest, auto rasterizer = std::make_unique(delegate); auto surface = std::make_unique(true); - auto is_gpu_disabled_sync_switch = std::make_shared(false); + auto is_gpu_disabled_sync_switch = + std::make_shared(false); auto surface_frame = std::make_unique( /*surface=*/nullptr, /*supports_readback=*/true, @@ -376,8 +377,9 @@ TEST(RasterizerTest, latch.Wait(); } -TEST(RasterizerTest, - drawWithGpuDisabledAndSurfaceAllowsDrawingWhenGpuDisabledDoesAcquireFrame) { +TEST( + RasterizerTest, + drawWithGpuDisabledAndSurfaceAllowsDrawingWhenGpuDisabledDoesAcquireFrame) { std::string test_name = ::testing::UnitTest::GetInstance()->current_test_info()->name(); ThreadHost thread_host("io.flutter.test." + test_name + ".", @@ -393,7 +395,8 @@ TEST(RasterizerTest, EXPECT_CALL(delegate, OnFrameRasterized(_)); auto rasterizer = std::make_unique(delegate); auto surface = std::make_unique(true); - auto is_gpu_disabled_sync_switch = std::make_shared(true); + auto is_gpu_disabled_sync_switch = + std::make_shared(true); auto surface_frame = std::make_unique( /*surface=*/nullptr, /*supports_readback=*/true, @@ -419,8 +422,9 @@ TEST(RasterizerTest, latch.Wait(); } -TEST(RasterizerTest, - drawWithGpuEnabledAndSurfaceDisallowsDrawingWhenGpuDisabledDoesAcquireFrame) { +TEST( + RasterizerTest, + drawWithGpuEnabledAndSurfaceDisallowsDrawingWhenGpuDisabledDoesAcquireFrame) { std::string test_name = ::testing::UnitTest::GetInstance()->current_test_info()->name(); ThreadHost thread_host("io.flutter.test." + test_name + ".", @@ -436,12 +440,14 @@ TEST(RasterizerTest, EXPECT_CALL(delegate, OnFrameRasterized(_)); auto rasterizer = std::make_unique(delegate); auto surface = std::make_unique(false); - auto is_gpu_disabled_sync_switch = std::make_shared(false); + auto is_gpu_disabled_sync_switch = + std::make_shared(false); auto surface_frame = std::make_unique( /*surface=*/nullptr, /*supports_readback=*/true, /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); - EXPECT_CALL(delegate, GetIsGpuDisabledSyncSwitch()).WillOnce(Return(is_gpu_disabled_sync_switch)); + EXPECT_CALL(delegate, GetIsGpuDisabledSyncSwitch()) + .WillOnce(Return(is_gpu_disabled_sync_switch)); EXPECT_CALL(*surface, AcquireFrame(SkISize())) .WillOnce(Return(ByMove(std::move(surface_frame)))); EXPECT_CALL(*surface, MakeRenderContextCurrent()) @@ -462,9 +468,9 @@ TEST(RasterizerTest, latch.Wait(); } - -TEST(RasterizerTest, - drawWithGpuDisabledAndSurfaceDisallowsDrawingWhenGpuDisabledDoesntAcquireFrame) { +TEST( + RasterizerTest, + drawWithGpuDisabledAndSurfaceDisallowsDrawingWhenGpuDisabledDoesntAcquireFrame) { std::string test_name = ::testing::UnitTest::GetInstance()->current_test_info()->name(); ThreadHost thread_host("io.flutter.test." + test_name + ".", @@ -480,13 +486,15 @@ TEST(RasterizerTest, EXPECT_CALL(delegate, OnFrameRasterized(_)).Times(0); auto rasterizer = std::make_unique(delegate); auto surface = std::make_unique(false); - auto is_gpu_disabled_sync_switch = std::make_shared(true); + auto is_gpu_disabled_sync_switch = + std::make_shared(true); auto surface_frame = std::make_unique( /*surface=*/nullptr, /*supports_readback=*/true, /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); - EXPECT_CALL(delegate, GetIsGpuDisabledSyncSwitch()).WillOnce(Return(is_gpu_disabled_sync_switch)); - EXPECT_CALL(*surface, AcquireFrame(SkISize())).Times(0); + EXPECT_CALL(delegate, GetIsGpuDisabledSyncSwitch()) + .WillOnce(Return(is_gpu_disabled_sync_switch)); + EXPECT_CALL(*surface, AcquireFrame(SkISize())).Times(0); EXPECT_CALL(*surface, MakeRenderContextCurrent()) .WillOnce(Return(ByMove(std::make_unique(true)))); From 7c1f729acdc6d8c1c755fea10d5a39306ddbcadf Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 1 Sep 2021 18:36:52 +0800 Subject: [PATCH 11/14] Revert "Make SyncSwitch recursive" This reverts commit d1875bda49a2702f093fc29ad20765ac2401ed3c. --- fml/synchronization/sync_switch.h | 2 +- fml/synchronization/sync_switch_unittest.cc | 32 --------------------- 2 files changed, 1 insertion(+), 33 deletions(-) diff --git a/fml/synchronization/sync_switch.h b/fml/synchronization/sync_switch.h index 78d748154cb41..08451c7476f89 100644 --- a/fml/synchronization/sync_switch.h +++ b/fml/synchronization/sync_switch.h @@ -53,7 +53,7 @@ class SyncSwitch { void SetSwitch(bool value); private: - mutable std::recursive_mutex mutex_; + mutable std::mutex mutex_; bool value_; FML_DISALLOW_COPY_AND_ASSIGN(SyncSwitch); diff --git a/fml/synchronization/sync_switch_unittest.cc b/fml/synchronization/sync_switch_unittest.cc index 4efa9e406209f..09994496cf907 100644 --- a/fml/synchronization/sync_switch_unittest.cc +++ b/fml/synchronization/sync_switch_unittest.cc @@ -22,38 +22,6 @@ TEST(SyncSwitchTest, Basic) { EXPECT_TRUE(switchValue); } -TEST(SyncSwitchTest, Recursive) { - SyncSwitch syncSwitch; - bool switchValue = false; - syncSwitch.Execute( - SyncSwitch::Handlers() - .SetIfTrue([&] { - syncSwitch.Execute(SyncSwitch::Handlers() - .SetIfTrue([&] { switchValue = true; }) - .SetIfFalse([&] { switchValue = false; })); - }) - .SetIfFalse([&] { - syncSwitch.Execute(SyncSwitch::Handlers() - .SetIfTrue([&] { switchValue = true; }) - .SetIfFalse([&] { switchValue = false; })); - })); - EXPECT_FALSE(switchValue); - syncSwitch.SetSwitch(true); - syncSwitch.Execute( - SyncSwitch::Handlers() - .SetIfTrue([&] { - syncSwitch.Execute(SyncSwitch::Handlers() - .SetIfTrue([&] { switchValue = true; }) - .SetIfFalse([&] { switchValue = false; })); - }) - .SetIfFalse([&] { - syncSwitch.Execute(SyncSwitch::Handlers() - .SetIfTrue([&] { switchValue = true; }) - .SetIfFalse([&] { switchValue = false; })); - })); - EXPECT_TRUE(switchValue); -} - TEST(SyncSwitchTest, NoopIfUndefined) { SyncSwitch syncSwitch; bool switchValue = false; From d4b500cae1d9388fa257c6171b33d90ea8641daf Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 1 Sep 2021 19:39:22 +0800 Subject: [PATCH 12/14] Let GetIsGpuDisabledSyncSwitch will return is_gpu_disabled_sync_switch by default --- shell/common/rasterizer_unittests.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/shell/common/rasterizer_unittests.cc b/shell/common/rasterizer_unittests.cc index f8341a2d724ad..1a3b5824ccff0 100644 --- a/shell/common/rasterizer_unittests.cc +++ b/shell/common/rasterizer_unittests.cc @@ -356,6 +356,8 @@ TEST(RasterizerTest, auto surface_frame = std::make_unique( /*surface=*/nullptr, /*supports_readback=*/true, /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); + ON_CALL(delegate, GetIsGpuDisabledSyncSwitch()) + .WillByDefault(Return(is_gpu_disabled_sync_switch)); EXPECT_CALL(delegate, GetIsGpuDisabledSyncSwitch()).Times(0); EXPECT_CALL(*surface, AcquireFrame(SkISize())) .WillOnce(Return(ByMove(std::move(surface_frame)))); @@ -401,6 +403,8 @@ TEST( auto surface_frame = std::make_unique( /*surface=*/nullptr, /*supports_readback=*/true, /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); + ON_CALL(delegate, GetIsGpuDisabledSyncSwitch()) + .WillByDefault(Return(is_gpu_disabled_sync_switch)); EXPECT_CALL(delegate, GetIsGpuDisabledSyncSwitch()).Times(0); EXPECT_CALL(*surface, AcquireFrame(SkISize())) .WillOnce(Return(ByMove(std::move(surface_frame)))); From 34224304b0ab2c0cc2b39aa620d0e82575fe1c88 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 1 Sep 2021 20:30:51 +0800 Subject: [PATCH 13/14] Delete unused code --- .../darwin/ios/framework/Source/FlutterPlatformViewsTest.mm | 2 -- .../ios/framework/Source/FlutterPlatformViews_Internal.h | 6 ------ 2 files changed, 8 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm index cd0f867181c05..0bcb1b483846b 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm @@ -980,8 +980,6 @@ - (void)testFlutterPlatformViewControllerSubmitFrameWithoutFlutterViewNotCrashin [](const flutter::SurfaceFrame& surface_frame, SkCanvas* canvas) { return true; }); XCTAssertTrue(flutterPlatformViewsController->SubmitFrame(nullptr, nullptr, std::move(mock_surface_submit_false))); - - flutterPlatformViewsController->Reset(); } - (void) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index d1ccccb3092a7..f886bd0df7df3 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -175,12 +175,6 @@ class FlutterPlatformViewsController { std::shared_ptr ios_context, std::unique_ptr frame); - // Invoked at the very end of a frame. - // After invoking this method, nothing should happen on the current TaskRunner during the same - // frame. - void EndFrame(bool should_resubmit_frame, - fml::RefPtr raster_thread_merger); - void OnMethodCall(FlutterMethodCall* call, FlutterResult& result); private: From 60260aa16062c243ee53f4ee72d0dcfb068152ed Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 1 Sep 2021 10:10:15 -0700 Subject: [PATCH 14/14] Tweaked the tests to assert the frame is discarded and added some documentation --- flow/compositor_context.h | 8 +++++-- shell/common/rasterizer.cc | 9 +++++-- shell/common/rasterizer.h | 7 +++--- shell/common/rasterizer_unittests.cc | 36 ++++++++++++++++------------ 4 files changed, 38 insertions(+), 22 deletions(-) diff --git a/flow/compositor_context.h b/flow/compositor_context.h index ca1bb662f68cf..7c10f3b147e58 100644 --- a/flow/compositor_context.h +++ b/flow/compositor_context.h @@ -46,8 +46,12 @@ enum class RasterStatus { kEnqueuePipeline, // Failed to rasterize the frame. kFailed, - // Layer tree was discarded due to LayerTreeDiscardCallback - kDiscarded + // Layer tree was discarded due to LayerTreeDiscardCallback or inability to + // access the GPU. + kDiscarded, + // Drawing was yielded to allow the correct thread to draw as a result of the + // RasterThreadMerger. + kYielded, }; class CompositorContext { diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 68f0973d1fd9f..fe399580ed306 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -147,7 +147,7 @@ void Rasterizer::DrawLastLayerTree( DrawToSurface(*frame_timings_recorder, *last_layer_tree_); } -void Rasterizer::Draw( +RasterStatus Rasterizer::Draw( std::unique_ptr frame_timings_recorder, std::shared_ptr> pipeline, LayerTreeDiscardCallback discardCallback) { @@ -156,7 +156,7 @@ void Rasterizer::Draw( if (raster_thread_merger_ && !raster_thread_merger_->IsOnRasterizingThread()) { // we yield and let this frame be serviced on the right thread. - return; + return RasterStatus::kYielded; } FML_DCHECK(delegate_.GetTaskRunners() .GetRasterTaskRunner() @@ -217,6 +217,8 @@ void Rasterizer::Draw( default: break; } + + return raster_status; } namespace { @@ -483,6 +485,9 @@ RasterStatus Rasterizer::DrawToSurface( return raster_status; } +/// Unsafe because it assumes we have access to the GPU which isn't the case +/// when iOS is backgrounded, for example. +/// \see Rasterizer::DrawToSurface RasterStatus Rasterizer::DrawToSurfaceUnsafe( FrameTimingsRecorder& frame_timings_recorder, flutter::LayerTree& layer_tree) { diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index 995573a27f664..2829f0c8dc487 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -233,9 +233,10 @@ class Rasterizer final : public SnapshotDelegate { /// @param[in] discardCallback if specified and returns true, the layer tree /// is discarded instead of being rendered /// - void Draw(std::unique_ptr frame_timings_recorder, - std::shared_ptr> pipeline, - LayerTreeDiscardCallback discardCallback = NoDiscard); + RasterStatus Draw( + std::unique_ptr frame_timings_recorder, + std::shared_ptr> pipeline, + LayerTreeDiscardCallback discardCallback = NoDiscard); //---------------------------------------------------------------------------- /// @brief The type of the screenshot to obtain of the previously diff --git a/shell/common/rasterizer_unittests.cc b/shell/common/rasterizer_unittests.cc index 1a3b5824ccff0..5d082a68eb493 100644 --- a/shell/common/rasterizer_unittests.cc +++ b/shell/common/rasterizer_unittests.cc @@ -37,8 +37,6 @@ class MockDelegate : public Rasterizer::Delegate { class MockSurface : public Surface { public: - MockSurface(bool allows_drawing_when_gpu_disabled = true) - : allows_drawing_when_gpu_disabled_(allows_drawing_when_gpu_disabled){}; MOCK_METHOD0(IsValid, bool()); MOCK_METHOD1(AcquireFrame, std::unique_ptr(const SkISize& size)); @@ -47,12 +45,7 @@ class MockSurface : public Surface { MOCK_METHOD0(GetExternalViewEmbedder, ExternalViewEmbedder*()); MOCK_METHOD0(MakeRenderContextCurrent, std::unique_ptr()); MOCK_METHOD0(ClearRenderContext, bool()); - bool AllowsDrawingWhenGpuDisabled() const override { - return allows_drawing_when_gpu_disabled_; - } - - private: - const bool allows_drawing_when_gpu_disabled_; + MOCK_CONST_METHOD0(AllowsDrawingWhenGpuDisabled, bool()); }; class MockExternalViewEmbedder : public ExternalViewEmbedder { @@ -148,6 +141,7 @@ TEST(RasterizerTest, auto surface_frame = std::make_unique( /*surface=*/nullptr, /*supports_readback=*/true, /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); + EXPECT_CALL(*surface, AllowsDrawingWhenGpuDisabled()).WillOnce(Return(true)); EXPECT_CALL(*surface, AcquireFrame(SkISize())) .WillOnce(Return(ByMove(std::move(surface_frame)))); EXPECT_CALL(*surface, MakeRenderContextCurrent()) @@ -208,6 +202,7 @@ TEST( auto surface_frame = std::make_unique( /*surface=*/nullptr, /*supports_readback=*/true, /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); + EXPECT_CALL(*surface, AllowsDrawingWhenGpuDisabled()).WillOnce(Return(true)); EXPECT_CALL(*surface, AcquireFrame(SkISize())) .WillOnce(Return(ByMove(std::move(surface_frame)))); EXPECT_CALL(*surface, MakeRenderContextCurrent()) @@ -268,6 +263,7 @@ TEST( auto surface_frame = std::make_unique( /*surface=*/nullptr, /*supports_readback=*/true, /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); + EXPECT_CALL(*surface, AllowsDrawingWhenGpuDisabled()).WillOnce(Return(true)); EXPECT_CALL(*surface, AcquireFrame(SkISize())) .WillOnce(Return(ByMove(std::move(surface_frame)))); EXPECT_CALL(*surface, MakeRenderContextCurrent()) @@ -349,13 +345,14 @@ TEST(RasterizerTest, EXPECT_CALL(delegate, OnFrameRasterized(_)); auto rasterizer = std::make_unique(delegate); - auto surface = std::make_unique(true); + auto surface = std::make_unique(); auto is_gpu_disabled_sync_switch = std::make_shared(false); auto surface_frame = std::make_unique( /*surface=*/nullptr, /*supports_readback=*/true, /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); + EXPECT_CALL(*surface, AllowsDrawingWhenGpuDisabled()).WillOnce(Return(true)); ON_CALL(delegate, GetIsGpuDisabledSyncSwitch()) .WillByDefault(Return(is_gpu_disabled_sync_switch)); EXPECT_CALL(delegate, GetIsGpuDisabledSyncSwitch()).Times(0); @@ -396,13 +393,14 @@ TEST( .WillRepeatedly(ReturnRef(task_runners)); EXPECT_CALL(delegate, OnFrameRasterized(_)); auto rasterizer = std::make_unique(delegate); - auto surface = std::make_unique(true); + auto surface = std::make_unique(); auto is_gpu_disabled_sync_switch = std::make_shared(true); auto surface_frame = std::make_unique( /*surface=*/nullptr, /*supports_readback=*/true, /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); + EXPECT_CALL(*surface, AllowsDrawingWhenGpuDisabled()).WillOnce(Return(true)); ON_CALL(delegate, GetIsGpuDisabledSyncSwitch()) .WillByDefault(Return(is_gpu_disabled_sync_switch)); EXPECT_CALL(delegate, GetIsGpuDisabledSyncSwitch()).Times(0); @@ -420,7 +418,9 @@ TEST( bool result = pipeline->Produce().Complete(std::move(layer_tree)); EXPECT_TRUE(result); auto no_discard = [](LayerTree&) { return false; }; - rasterizer->Draw(CreateFinishedBuildRecorder(), pipeline, no_discard); + RasterStatus status = + rasterizer->Draw(CreateFinishedBuildRecorder(), pipeline, no_discard); + EXPECT_EQ(status, RasterStatus::kSuccess); latch.Signal(); }); latch.Wait(); @@ -443,13 +443,14 @@ TEST( .WillRepeatedly(ReturnRef(task_runners)); EXPECT_CALL(delegate, OnFrameRasterized(_)); auto rasterizer = std::make_unique(delegate); - auto surface = std::make_unique(false); + auto surface = std::make_unique(); auto is_gpu_disabled_sync_switch = std::make_shared(false); auto surface_frame = std::make_unique( /*surface=*/nullptr, /*supports_readback=*/true, /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); + EXPECT_CALL(*surface, AllowsDrawingWhenGpuDisabled()).WillOnce(Return(false)); EXPECT_CALL(delegate, GetIsGpuDisabledSyncSwitch()) .WillOnce(Return(is_gpu_disabled_sync_switch)); EXPECT_CALL(*surface, AcquireFrame(SkISize())) @@ -466,7 +467,9 @@ TEST( bool result = pipeline->Produce().Complete(std::move(layer_tree)); EXPECT_TRUE(result); auto no_discard = [](LayerTree&) { return false; }; - rasterizer->Draw(CreateFinishedBuildRecorder(), pipeline, no_discard); + RasterStatus status = + rasterizer->Draw(CreateFinishedBuildRecorder(), pipeline, no_discard); + EXPECT_EQ(status, RasterStatus::kSuccess); latch.Signal(); }); latch.Wait(); @@ -489,13 +492,14 @@ TEST( .WillRepeatedly(ReturnRef(task_runners)); EXPECT_CALL(delegate, OnFrameRasterized(_)).Times(0); auto rasterizer = std::make_unique(delegate); - auto surface = std::make_unique(false); + auto surface = std::make_unique(); auto is_gpu_disabled_sync_switch = std::make_shared(true); auto surface_frame = std::make_unique( /*surface=*/nullptr, /*supports_readback=*/true, /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); + EXPECT_CALL(*surface, AllowsDrawingWhenGpuDisabled()).WillOnce(Return(false)); EXPECT_CALL(delegate, GetIsGpuDisabledSyncSwitch()) .WillOnce(Return(is_gpu_disabled_sync_switch)); EXPECT_CALL(*surface, AcquireFrame(SkISize())).Times(0); @@ -511,7 +515,9 @@ TEST( bool result = pipeline->Produce().Complete(std::move(layer_tree)); EXPECT_TRUE(result); auto no_discard = [](LayerTree&) { return false; }; - rasterizer->Draw(CreateFinishedBuildRecorder(), pipeline, no_discard); + RasterStatus status = + rasterizer->Draw(CreateFinishedBuildRecorder(), pipeline, no_discard); + EXPECT_EQ(status, RasterStatus::kDiscarded); latch.Signal(); }); latch.Wait();