diff --git a/shell/gpu/gpu_surface_gl.cc b/shell/gpu/gpu_surface_gl.cc index 435618bf91fc0..388a22e352dd5 100644 --- a/shell/gpu/gpu_surface_gl.cc +++ b/shell/gpu/gpu_surface_gl.cc @@ -34,13 +34,16 @@ static const int kGrCacheMaxCount = 8192; // system channel. static const size_t kGrCacheMaxByteSize = 24 * (1 << 20); -sk_sp GPUSurfaceGL::MakeGLContext( - GPUSurfaceGLDelegate* delegate) { - auto context_switch = delegate->GLContextMakeCurrent(); +GPUSurfaceGL::GPUSurfaceGL(GPUSurfaceGLDelegate* delegate, + bool render_to_surface) + : delegate_(delegate), + render_to_surface_(render_to_surface), + weak_factory_(this) { + auto context_switch = delegate_->GLContextMakeCurrent(); if (!context_switch->GetResult()) { FML_LOG(ERROR) << "Could not make the context current to setup the gr context."; - return nullptr; + return; } GrContextOptions options; @@ -61,31 +64,32 @@ sk_sp GPUSurfaceGL::MakeGLContext( // TODO(goderbauer): remove option when skbug.com/7523 is fixed. // A similar work-around is also used in shell/common/io_manager.cc. options.fDisableGpuYUVConversion = true; - auto context = GrDirectContext::MakeGL(delegate->GetGLInterface(), options); - if (!context) { + auto context = GrDirectContext::MakeGL(delegate_->GetGLInterface(), options); + + if (context == nullptr) { FML_LOG(ERROR) << "Failed to setup Skia Gr context."; - return nullptr; + return; } - context->setResourceCacheLimits(kGrCacheMaxCount, kGrCacheMaxByteSize); + context_ = std::move(context); + + context_->setResourceCacheLimits(kGrCacheMaxCount, kGrCacheMaxByteSize); + + context_owner_ = true; + + valid_ = true; std::vector caches = PersistentCache::GetCacheForProcess()->LoadSkSLs(); int compiled_count = 0; for (const auto& cache : caches) { - compiled_count += context->precompileShader(*cache.first, *cache.second); + compiled_count += context_->precompileShader(*cache.first, *cache.second); } FML_LOG(INFO) << "Found " << caches.size() << " SkSL shaders; precompiled " << compiled_count; - return context; -} - -GPUSurfaceGL::GPUSurfaceGL(GPUSurfaceGLDelegate* delegate, - bool render_to_surface) - : GPUSurfaceGL(MakeGLContext(delegate), delegate, render_to_surface) { - context_owner_ = true; + delegate_->GLContextClearCurrent(); } GPUSurfaceGL::GPUSurfaceGL(sk_sp gr_context, @@ -93,7 +97,6 @@ GPUSurfaceGL::GPUSurfaceGL(sk_sp gr_context, bool render_to_surface) : delegate_(delegate), context_(gr_context), - context_owner_(false), render_to_surface_(render_to_surface), weak_factory_(this) { auto context_switch = delegate_->GLContextMakeCurrent(); @@ -106,6 +109,7 @@ GPUSurfaceGL::GPUSurfaceGL(sk_sp gr_context, delegate_->GLContextClearCurrent(); valid_ = true; + context_owner_ = false; } GPUSurfaceGL::~GPUSurfaceGL() { diff --git a/shell/gpu/gpu_surface_gl.h b/shell/gpu/gpu_surface_gl.h index f1be74c495b2d..e7fa001123e52 100644 --- a/shell/gpu/gpu_surface_gl.h +++ b/shell/gpu/gpu_surface_gl.h @@ -20,8 +20,6 @@ namespace flutter { class GPUSurfaceGL : public Surface { public: - static sk_sp MakeGLContext(GPUSurfaceGLDelegate* delegate); - GPUSurfaceGL(GPUSurfaceGLDelegate* delegate, bool render_to_surface); // Creates a new GL surface reusing an existing GrDirectContext. @@ -55,13 +53,13 @@ class GPUSurfaceGL : public Surface { sk_sp context_; sk_sp onscreen_surface_; /// FBO backing the current `onscreen_surface_`. - uint32_t fbo_id_ = 0; - bool context_owner_ = false; + uint32_t fbo_id_; + bool context_owner_; // TODO(38466): Refactor GPU surface APIs take into account the fact that an // external view embedder may want to render to the root surface. This is a // hack to make avoid allocating resources for the root surface when an // external view embedder is present. - const bool render_to_surface_ = true; + const bool render_to_surface_; bool valid_ = false; fml::TaskRunnerAffineWeakPtrFactory weak_factory_; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest_mrc.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest_mrc.mm index 46a7e4abac965..f5595bffb99d3 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest_mrc.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest_mrc.mm @@ -33,8 +33,10 @@ - (void)testSpawnsShareGpuContext { std::shared_ptr engine_context = [engine iosPlatformView]->GetIosContext(); std::shared_ptr spawn_context = [spawn iosPlatformView]->GetIosContext(); XCTAssertEqual(engine_context, spawn_context); - // If this assert fails it means we may be using the software. For software rendering, this is - // expected to be nullptr. + // If this assert fails it means we may be using the software or OpenGL + // renderer when we were expecting Metal. For software rendering, this is + // expected to be nullptr. For OpenGL, implementing this is an outstanding + // change see https://github.com/flutter/flutter/issues/73744. XCTAssertTrue(engine_context->GetMainContext() != nullptr); XCTAssertEqual(engine_context->GetMainContext(), spawn_context->GetMainContext()); [engine release]; diff --git a/shell/platform/darwin/ios/ios_context_gl.h b/shell/platform/darwin/ios/ios_context_gl.h index 9ed70072ef009..823f13a14c59a 100644 --- a/shell/platform/darwin/ios/ios_context_gl.h +++ b/shell/platform/darwin/ios/ios_context_gl.h @@ -25,11 +25,16 @@ class IOSContextGL final : public IOSContext { std::unique_ptr CreateRenderTarget(fml::scoped_nsobject layer); - void SetMainContext(const sk_sp& main_context); + private: + fml::scoped_nsobject context_; + fml::scoped_nsobject resource_context_; // |IOSContext| sk_sp CreateResourceContext() override; + // |IOSContext| + sk_sp GetMainContext() const override; + // |IOSContext| std::unique_ptr MakeCurrent() override; @@ -38,14 +43,6 @@ class IOSContextGL final : public IOSContext { int64_t texture_id, fml::scoped_nsobject> texture) override; - // |IOSContext| - sk_sp GetMainContext() const override; - - private: - fml::scoped_nsobject context_; - fml::scoped_nsobject resource_context_; - sk_sp main_context_; - FML_DISALLOW_COPY_AND_ASSIGN(IOSContextGL); }; diff --git a/shell/platform/darwin/ios/ios_context_gl.mm b/shell/platform/darwin/ios/ios_context_gl.mm index ef469fad5ab7d..d803174cba2ca 100644 --- a/shell/platform/darwin/ios/ios_context_gl.mm +++ b/shell/platform/darwin/ios/ios_context_gl.mm @@ -7,7 +7,6 @@ #import #include "flutter/shell/common/shell_io_manager.h" -#include "flutter/shell/gpu/gpu_surface_gl.h" #include "flutter/shell/gpu/gpu_surface_gl_delegate.h" #import "flutter/shell/platform/darwin/ios/ios_external_texture_gl.h" @@ -25,11 +24,7 @@ } } -IOSContextGL::~IOSContextGL() { - if (main_context_) { - main_context_->releaseResourcesAndAbandonContext(); - } -} +IOSContextGL::~IOSContextGL() = default; std::unique_ptr IOSContextGL::CreateRenderTarget( fml::scoped_nsobject layer) { @@ -50,11 +45,12 @@ // |IOSContext| sk_sp IOSContextGL::GetMainContext() const { - return main_context_; -} - -void IOSContextGL::SetMainContext(const sk_sp& main_context) { - main_context_ = main_context; + /// TODO(73744): Currently the GPUSurfaceGL creates the main context for + /// OpenGL. With Metal the IOSContextMetal creates the main context and is + /// shared across surfaces. We should refactor the OpenGL Context/Surfaces to + /// behave like the Metal equivalents. Until then engines in the same group + /// will have a heavier memory cost if they are using OpenGL. + return nullptr; } // |IOSContext| diff --git a/shell/platform/darwin/ios/ios_surface_gl.mm b/shell/platform/darwin/ios/ios_surface_gl.mm index 34983f3a5bb21..637674417514f 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.mm +++ b/shell/platform/darwin/ios/ios_surface_gl.mm @@ -38,16 +38,8 @@ std::unique_ptr IOSSurfaceGL::CreateGPUSurface(GrDirectContext* gr_context) { if (gr_context) { return std::make_unique(sk_ref_sp(gr_context), this, true); - } else { - IOSContextGL* gl_context = CastToGLContext(GetContext()); - sk_sp context = gl_context->GetMainContext(); - if (!context) { - context = GPUSurfaceGL::MakeGLContext(this); - gl_context->SetMainContext(context); - } - - return std::make_unique(context, this, true); } + return std::make_unique(this, true); } // |GPUSurfaceGLDelegate| diff --git a/shell/platform/darwin/ios/ios_surface_metal.mm b/shell/platform/darwin/ios/ios_surface_metal.mm index 64909e0b15b1b..915fc6eada20c 100644 --- a/shell/platform/darwin/ios/ios_surface_metal.mm +++ b/shell/platform/darwin/ios/ios_surface_metal.mm @@ -40,10 +40,10 @@ } // |IOSSurface| -std::unique_ptr IOSSurfaceMetal::CreateGPUSurface(GrDirectContext* context) { - FML_DCHECK(context); - return std::make_unique(this, // layer - sk_ref_sp(context) // context +std::unique_ptr IOSSurfaceMetal::CreateGPUSurface(GrDirectContext* /* unused */) { + auto metal_context = CastToMetalContext(GetContext()); + return std::make_unique(this, // layer + metal_context->GetMainContext() // context ); } diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index 17974a0700d1b..6dfb00ddd546d 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -148,7 +148,7 @@ "has no ViewController."; return nullptr; } - return ios_surface_->CreateGPUSurface(ios_context_->GetMainContext().get()); + return ios_surface_->CreateGPUSurface(); } // |PlatformView| diff --git a/shell/platform/embedder/embedder_surface_gl.cc b/shell/platform/embedder/embedder_surface_gl.cc index 69c75cb2e7462..53b0e9c3e3b85 100644 --- a/shell/platform/embedder/embedder_surface_gl.cc +++ b/shell/platform/embedder/embedder_surface_gl.cc @@ -80,6 +80,7 @@ std::unique_ptr EmbedderSurfaceGL::CreateGPUSurface() { const bool render_to_surface = !external_view_embedder_; return std::make_unique(this, // GPU surface GL delegate render_to_surface // render to surface + ); }