From 6b84b8eae330d317d610cc330a236971ea777a71 Mon Sep 17 00:00:00 2001 From: Lin Zhang Date: Wed, 8 Sep 2021 22:30:51 +0800 Subject: [PATCH 1/4] Fix OpenGL leak on Android and iOS --- shell/platform/android/android_surface_gl.cc | 1 + shell/platform/darwin/ios/ios_surface_gl.mm | 1 + 2 files changed, 2 insertions(+) diff --git a/shell/platform/android/android_surface_gl.cc b/shell/platform/android/android_surface_gl.cc index 224b2280d22e2..e9fd661ec178e 100644 --- a/shell/platform/android/android_surface_gl.cc +++ b/shell/platform/android/android_surface_gl.cc @@ -57,6 +57,7 @@ std::unique_ptr AndroidSurfaceGL::CreateGPUSurface( if (!main_skia_context) { main_skia_context = GPUSurfaceGL::MakeGLContext(this); GLContextPtr()->SetMainSkiaContext(main_skia_context); + return std::make_unique(main_skia_context, true); } return std::make_unique(main_skia_context, this, true); } diff --git a/shell/platform/darwin/ios/ios_surface_gl.mm b/shell/platform/darwin/ios/ios_surface_gl.mm index e1c5bde675499..dbcb4f9298412 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.mm +++ b/shell/platform/darwin/ios/ios_surface_gl.mm @@ -44,6 +44,7 @@ if (!context) { context = GPUSurfaceGL::MakeGLContext(this); gl_context->SetMainContext(context); + return std::make_unique(context, true); } return std::make_unique(context, this, true); From 9aa70314b4d63280dd91f33fc5f21916b552af74 Mon Sep 17 00:00:00 2001 From: Lin Zhang Date: Thu, 9 Sep 2021 02:38:13 +0800 Subject: [PATCH 2/4] Revert "Fix OpenGL leak on Android and iOS" This reverts commit 6b84b8eae330d317d610cc330a236971ea777a71. --- shell/platform/android/android_surface_gl.cc | 1 - shell/platform/darwin/ios/ios_surface_gl.mm | 1 - 2 files changed, 2 deletions(-) diff --git a/shell/platform/android/android_surface_gl.cc b/shell/platform/android/android_surface_gl.cc index e9fd661ec178e..224b2280d22e2 100644 --- a/shell/platform/android/android_surface_gl.cc +++ b/shell/platform/android/android_surface_gl.cc @@ -57,7 +57,6 @@ std::unique_ptr AndroidSurfaceGL::CreateGPUSurface( if (!main_skia_context) { main_skia_context = GPUSurfaceGL::MakeGLContext(this); GLContextPtr()->SetMainSkiaContext(main_skia_context); - return std::make_unique(main_skia_context, true); } return std::make_unique(main_skia_context, this, true); } diff --git a/shell/platform/darwin/ios/ios_surface_gl.mm b/shell/platform/darwin/ios/ios_surface_gl.mm index dbcb4f9298412..e1c5bde675499 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.mm +++ b/shell/platform/darwin/ios/ios_surface_gl.mm @@ -44,7 +44,6 @@ if (!context) { context = GPUSurfaceGL::MakeGLContext(this); gl_context->SetMainContext(context); - return std::make_unique(context, true); } return std::make_unique(context, this, true); From a621ce48e5de17eb1a10fc5f7000c5dfff198423 Mon Sep 17 00:00:00 2001 From: Lin Zhang Date: Thu, 9 Sep 2021 02:40:44 +0800 Subject: [PATCH 3/4] Call GrDirectContext::releaseResourcesAndAbandonContext() explictly on rasterizer task runner when destructing Shell --- shell/common/platform_view.cc | 4 ++++ shell/common/platform_view.h | 10 ++++++++++ shell/common/shell.cc | 16 ++++++++++------ .../platform/android/context/android_context.cc | 13 ++++++++----- shell/platform/android/context/android_context.h | 5 +++++ shell/platform/android/platform_view_android.cc | 8 ++++++++ shell/platform/android/platform_view_android.h | 3 +++ shell/platform/darwin/ios/ios_context.h | 7 +++++++ shell/platform/darwin/ios/ios_context.mm | 4 ++++ shell/platform/darwin/ios/ios_context_gl.h | 3 +++ shell/platform/darwin/ios/ios_context_gl.mm | 14 +++++++++----- shell/platform/darwin/ios/platform_view_ios.h | 3 +++ shell/platform/darwin/ios/platform_view_ios.mm | 8 ++++++++ 13 files changed, 82 insertions(+), 16 deletions(-) diff --git a/shell/common/platform_view.cc b/shell/common/platform_view.cc index d48442604ff5c..cef4d99848808 100644 --- a/shell/common/platform_view.cc +++ b/shell/common/platform_view.cc @@ -141,6 +141,10 @@ std::unique_ptr PlatformView::CreateRenderingSurface() { return nullptr; } +void PlatformView::ReleaseSurfaceContext() { + // Do nothing by default +} + std::shared_ptr PlatformView::CreateExternalViewEmbedder() { FML_DLOG(WARNING) diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h index 1cbf73ce4abea..28fd5994e06b9 100644 --- a/shell/common/platform_view.h +++ b/shell/common/platform_view.h @@ -812,6 +812,16 @@ class PlatformView { virtual std::unique_ptr CreateSnapshotSurfaceProducer(); + //---------------------------------------------------------------------------- + /// @brief Used by the shell to notify the embedder that all the + /// rendering surface previously obtained via a call to + /// `CreateRenderingSurface()` are being collected. The embedder + /// should collect the shared context of surfaces + /// + /// @attention This should be called on the rasterizer task runner. + /// + virtual void ReleaseSurfaceContext(); + protected: PlatformView::Delegate& delegate_; const TaskRunners task_runners_; diff --git a/shell/common/shell.cc b/shell/common/shell.cc index f9666183608b6..bd39a7b226a3c 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -443,12 +443,16 @@ Shell::~Shell() { fml::TaskRunner::RunNowOrPostTask( task_runners_.GetRasterTaskRunner(), - fml::MakeCopyable( - [this, rasterizer = std::move(rasterizer_), &gpu_latch]() mutable { - rasterizer.reset(); - this->weak_factory_gpu_.reset(); - gpu_latch.Signal(); - })); + fml::MakeCopyable([this, rasterizer = std::move(rasterizer_), + platform_view = platform_view_.get(), + &gpu_latch]() mutable { + rasterizer.reset(); + this->weak_factory_gpu_.reset(); + if (platform_view) { + platform_view->ReleaseSurfaceContext(); + } + gpu_latch.Signal(); + })); gpu_latch.Wait(); fml::TaskRunner::RunNowOrPostTask( diff --git a/shell/platform/android/context/android_context.cc b/shell/platform/android/context/android_context.cc index 1d4badf9543b7..f6197253c3e50 100644 --- a/shell/platform/android/context/android_context.cc +++ b/shell/platform/android/context/android_context.cc @@ -9,11 +9,7 @@ namespace flutter { AndroidContext::AndroidContext(AndroidRenderingAPI rendering_api) : rendering_api_(rendering_api) {} -AndroidContext::~AndroidContext() { - if (main_context_) { - main_context_->releaseResourcesAndAbandonContext(); - } -}; +AndroidContext::~AndroidContext() = default; AndroidRenderingAPI AndroidContext::RenderingApi() const { return rendering_api_; @@ -28,6 +24,13 @@ void AndroidContext::SetMainSkiaContext( main_context_ = main_context; } +void AndroidContext::ReleaseMainSkiaContext() { + if (main_context_) { + main_context_->releaseResourcesAndAbandonContext(); + main_context_ = nullptr; + } +} + sk_sp AndroidContext::GetMainSkiaContext() const { return main_context_; } diff --git a/shell/platform/android/context/android_context.h b/shell/platform/android/context/android_context.h index 61662c5c81376..57694759f84f0 100644 --- a/shell/platform/android/context/android_context.h +++ b/shell/platform/android/context/android_context.h @@ -40,6 +40,11 @@ class AndroidContext { /// void SetMainSkiaContext(const sk_sp& main_context); + /// @brief Release the Skia context used by subsequent AndroidSurfaces. + /// + /// @attention This should be called on the rasterizer task runner. + void ReleaseMainSkiaContext(); + //---------------------------------------------------------------------------- /// @brief Accessor for the Skia context associated with AndroidSurfaces /// and the raster thread. diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 899870961c639..0d713c5566838 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -297,6 +297,14 @@ std::unique_ptr PlatformViewAndroid::CreateRenderingSurface() { android_context_->GetMainSkiaContext().get()); } +// |PlatformView| +void PlatformViewAndroid::ReleaseSurfaceContext() { + FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread()); + if (android_context_) { + android_context_->ReleaseMainSkiaContext(); + } +} + // |PlatformView| std::shared_ptr PlatformViewAndroid::CreateExternalViewEmbedder() { diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h index 3bb4195126373..62553f4ac38fe 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -149,6 +149,9 @@ class PlatformViewAndroid final : public PlatformView { // |PlatformView| std::unique_ptr CreateRenderingSurface() override; + // |PlatformView| + void ReleaseSurfaceContext() override; + // |PlatformView| std::shared_ptr CreateExternalViewEmbedder() override; diff --git a/shell/platform/darwin/ios/ios_context.h b/shell/platform/darwin/ios/ios_context.h index c08b6b9c729d3..9a694fba843a6 100644 --- a/shell/platform/darwin/ios/ios_context.h +++ b/shell/platform/darwin/ios/ios_context.h @@ -120,6 +120,13 @@ class IOSContext { /// virtual sk_sp GetMainContext() const = 0; + //---------------------------------------------------------------------------- + /// @brief Release the Skia context associated with IOSSurfaces and + /// the raster thread. + /// @attention The should be called on the rasterizer task runner. + /// + virtual void ReleaseMainContext(); + protected: IOSContext(); diff --git a/shell/platform/darwin/ios/ios_context.mm b/shell/platform/darwin/ios/ios_context.mm index 3752235cbda33..c2c0159e41a70 100644 --- a/shell/platform/darwin/ios/ios_context.mm +++ b/shell/platform/darwin/ios/ios_context.mm @@ -35,4 +35,8 @@ return nullptr; } +void IOSContext::ReleaseMainContext() { + // Do nothing by default +} + } // namespace flutter diff --git a/shell/platform/darwin/ios/ios_context_gl.h b/shell/platform/darwin/ios/ios_context_gl.h index 9ed70072ef009..ee466e41063d2 100644 --- a/shell/platform/darwin/ios/ios_context_gl.h +++ b/shell/platform/darwin/ios/ios_context_gl.h @@ -41,6 +41,9 @@ class IOSContextGL final : public IOSContext { // |IOSContext| sk_sp GetMainContext() const override; + // |IOSContext| + void ReleaseMainContext() override; + private: fml::scoped_nsobject context_; fml::scoped_nsobject resource_context_; diff --git a/shell/platform/darwin/ios/ios_context_gl.mm b/shell/platform/darwin/ios/ios_context_gl.mm index ef469fad5ab7d..30541ce6e42d3 100644 --- a/shell/platform/darwin/ios/ios_context_gl.mm +++ b/shell/platform/darwin/ios/ios_context_gl.mm @@ -25,11 +25,7 @@ } } -IOSContextGL::~IOSContextGL() { - if (main_context_) { - main_context_->releaseResourcesAndAbandonContext(); - } -} +IOSContextGL::~IOSContextGL() = default; std::unique_ptr IOSContextGL::CreateRenderTarget( fml::scoped_nsobject layer) { @@ -57,6 +53,14 @@ main_context_ = main_context; } +// |IOSContext| +void IOSContextGL::ReleaseMainContext() { + if (main_context_) { + main_context_->releaseResourcesAndAbandonContext(); + main_context_ = nullptr; + } +} + // |IOSContext| std::unique_ptr IOSContextGL::MakeCurrent() { return std::make_unique( diff --git a/shell/platform/darwin/ios/platform_view_ios.h b/shell/platform/darwin/ios/platform_view_ios.h index 4010da254bd1d..d3dc35940c0fe 100644 --- a/shell/platform/darwin/ios/platform_view_ios.h +++ b/shell/platform/darwin/ios/platform_view_ios.h @@ -152,6 +152,9 @@ class PlatformViewIOS final : public PlatformView { // |PlatformView| std::unique_ptr CreateRenderingSurface() override; + // |PlatformView| + void ReleaseSurfaceContext() override; + // |PlatformView| std::shared_ptr CreateExternalViewEmbedder() override; diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index 081bbe0db130a..f7b6287012905 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -151,6 +151,14 @@ return ios_surface_->CreateGPUSurface(ios_context_->GetMainContext().get()); } +// |PlatformView| +void PlatformViewIOS::ReleaseSurfaceContext() { + FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread()); + if (ios_context_) { + ios_context_->ReleaseMainContext(); + } +} + // |PlatformView| std::shared_ptr PlatformViewIOS::CreateExternalViewEmbedder() { return std::make_shared(platform_views_controller_, ios_context_); From 854c30bc4ae0adbc130eafabc0a71e3effd36104 Mon Sep 17 00:00:00 2001 From: Lin Zhang Date: Thu, 9 Sep 2021 12:59:58 +0800 Subject: [PATCH 4/4] Release GrDirectContext depends on shared_ptr --- shell/platform/android/context/android_context.cc | 13 +++++-------- shell/platform/android/context/android_context.h | 5 ----- shell/platform/android/platform_view_android.cc | 7 +++++-- shell/platform/darwin/ios/ios_context.h | 7 ------- shell/platform/darwin/ios/ios_context.mm | 4 ---- shell/platform/darwin/ios/ios_context_gl.h | 3 --- shell/platform/darwin/ios/ios_context_gl.mm | 14 +++++--------- shell/platform/darwin/ios/platform_view_ios.mm | 7 ++++++- 8 files changed, 21 insertions(+), 39 deletions(-) diff --git a/shell/platform/android/context/android_context.cc b/shell/platform/android/context/android_context.cc index f6197253c3e50..1d4badf9543b7 100644 --- a/shell/platform/android/context/android_context.cc +++ b/shell/platform/android/context/android_context.cc @@ -9,7 +9,11 @@ namespace flutter { AndroidContext::AndroidContext(AndroidRenderingAPI rendering_api) : rendering_api_(rendering_api) {} -AndroidContext::~AndroidContext() = default; +AndroidContext::~AndroidContext() { + if (main_context_) { + main_context_->releaseResourcesAndAbandonContext(); + } +}; AndroidRenderingAPI AndroidContext::RenderingApi() const { return rendering_api_; @@ -24,13 +28,6 @@ void AndroidContext::SetMainSkiaContext( main_context_ = main_context; } -void AndroidContext::ReleaseMainSkiaContext() { - if (main_context_) { - main_context_->releaseResourcesAndAbandonContext(); - main_context_ = nullptr; - } -} - sk_sp AndroidContext::GetMainSkiaContext() const { return main_context_; } diff --git a/shell/platform/android/context/android_context.h b/shell/platform/android/context/android_context.h index 57694759f84f0..61662c5c81376 100644 --- a/shell/platform/android/context/android_context.h +++ b/shell/platform/android/context/android_context.h @@ -40,11 +40,6 @@ class AndroidContext { /// void SetMainSkiaContext(const sk_sp& main_context); - /// @brief Release the Skia context used by subsequent AndroidSurfaces. - /// - /// @attention This should be called on the rasterizer task runner. - void ReleaseMainSkiaContext(); - //---------------------------------------------------------------------------- /// @brief Accessor for the Skia context associated with AndroidSurfaces /// and the raster thread. diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 0d713c5566838..02a03102e1f5d 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -35,9 +35,11 @@ AndroidSurfaceFactoryImpl::~AndroidSurfaceFactoryImpl() = default; std::unique_ptr AndroidSurfaceFactoryImpl::CreateSurface() { switch (android_context_->RenderingApi()) { case AndroidRenderingAPI::kSoftware: + FML_DCHECK(android_context_); return std::make_unique(android_context_, jni_facade_); case AndroidRenderingAPI::kOpenGLES: + FML_DCHECK(android_context_); return std::make_unique(android_context_, jni_facade_); default: FML_DCHECK(false); @@ -290,7 +292,7 @@ std::unique_ptr PlatformViewAndroid::CreateVSyncWaiter() { // |PlatformView| std::unique_ptr PlatformViewAndroid::CreateRenderingSurface() { - if (!android_surface_) { + if (!android_surface_ || !android_context_) { return nullptr; } return android_surface_->CreateGPUSurface( @@ -301,13 +303,14 @@ std::unique_ptr PlatformViewAndroid::CreateRenderingSurface() { void PlatformViewAndroid::ReleaseSurfaceContext() { FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread()); if (android_context_) { - android_context_->ReleaseMainSkiaContext(); + android_context_.reset(); } } // |PlatformView| std::shared_ptr PlatformViewAndroid::CreateExternalViewEmbedder() { + FML_DCHECK(android_context_); return std::make_shared( *android_context_, jni_facade_, surface_factory_); } diff --git a/shell/platform/darwin/ios/ios_context.h b/shell/platform/darwin/ios/ios_context.h index 9a694fba843a6..c08b6b9c729d3 100644 --- a/shell/platform/darwin/ios/ios_context.h +++ b/shell/platform/darwin/ios/ios_context.h @@ -120,13 +120,6 @@ class IOSContext { /// virtual sk_sp GetMainContext() const = 0; - //---------------------------------------------------------------------------- - /// @brief Release the Skia context associated with IOSSurfaces and - /// the raster thread. - /// @attention The should be called on the rasterizer task runner. - /// - virtual void ReleaseMainContext(); - protected: IOSContext(); diff --git a/shell/platform/darwin/ios/ios_context.mm b/shell/platform/darwin/ios/ios_context.mm index c2c0159e41a70..3752235cbda33 100644 --- a/shell/platform/darwin/ios/ios_context.mm +++ b/shell/platform/darwin/ios/ios_context.mm @@ -35,8 +35,4 @@ return nullptr; } -void IOSContext::ReleaseMainContext() { - // Do nothing by default -} - } // namespace flutter diff --git a/shell/platform/darwin/ios/ios_context_gl.h b/shell/platform/darwin/ios/ios_context_gl.h index ee466e41063d2..9ed70072ef009 100644 --- a/shell/platform/darwin/ios/ios_context_gl.h +++ b/shell/platform/darwin/ios/ios_context_gl.h @@ -41,9 +41,6 @@ class IOSContextGL final : public IOSContext { // |IOSContext| sk_sp GetMainContext() const override; - // |IOSContext| - void ReleaseMainContext() override; - private: fml::scoped_nsobject context_; fml::scoped_nsobject resource_context_; diff --git a/shell/platform/darwin/ios/ios_context_gl.mm b/shell/platform/darwin/ios/ios_context_gl.mm index 30541ce6e42d3..ef469fad5ab7d 100644 --- a/shell/platform/darwin/ios/ios_context_gl.mm +++ b/shell/platform/darwin/ios/ios_context_gl.mm @@ -25,7 +25,11 @@ } } -IOSContextGL::~IOSContextGL() = default; +IOSContextGL::~IOSContextGL() { + if (main_context_) { + main_context_->releaseResourcesAndAbandonContext(); + } +} std::unique_ptr IOSContextGL::CreateRenderTarget( fml::scoped_nsobject layer) { @@ -53,14 +57,6 @@ main_context_ = main_context; } -// |IOSContext| -void IOSContextGL::ReleaseMainContext() { - if (main_context_) { - main_context_->releaseResourcesAndAbandonContext(); - main_context_ = nullptr; - } -} - // |IOSContext| std::unique_ptr IOSContextGL::MakeCurrent() { return std::make_unique( diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index f7b6287012905..44a37f1c77935 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -113,6 +113,7 @@ void PlatformViewIOS::attachView() { FML_DCHECK(owner_controller_); + FML_DCHECK(ios_context_); FML_DCHECK(owner_controller_.get().isViewLoaded) << "FlutterViewController's view should be loaded " "before attaching to PlatformViewIOS."; @@ -135,6 +136,7 @@ void PlatformViewIOS::RegisterExternalTexture(int64_t texture_id, NSObject* texture) { + FML_DCHECK(ios_context_); RegisterTexture(ios_context_->CreateExternalTexture( texture_id, fml::scoped_nsobject>{[texture retain]})); } @@ -142,6 +144,7 @@ // |PlatformView| std::unique_ptr PlatformViewIOS::CreateRenderingSurface() { FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread()); + FML_DCHECK(ios_context_); std::lock_guard guard(ios_surface_mutex_); if (!ios_surface_) { FML_DLOG(INFO) << "Could not CreateRenderingSurface, this PlatformViewIOS " @@ -155,17 +158,19 @@ void PlatformViewIOS::ReleaseSurfaceContext() { FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread()); if (ios_context_) { - ios_context_->ReleaseMainContext(); + ios_context_.reset(); } } // |PlatformView| std::shared_ptr PlatformViewIOS::CreateExternalViewEmbedder() { + FML_DCHECK(ios_context_); return std::make_shared(platform_views_controller_, ios_context_); } // |PlatformView| sk_sp PlatformViewIOS::CreateResourceContext() const { + FML_DCHECK(ios_context_); return ios_context_->CreateResourceContext(); }