From 148f2e7ee4efca2bb84d9c951942169b772c30b7 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 8 Jan 2021 10:12:22 -0800 Subject: [PATCH 1/4] Started asserting that metal gpu contexts are shared, added ability for IOSContexts to store their skia contexts so we can make the OpenGL code work like the Metal code. --- .../ios/framework/Source/FlutterEngineTest_mrc.mm | 13 +++++++++++-- shell/platform/darwin/ios/ios_context.h | 8 ++++++++ shell/platform/darwin/ios/ios_context_gl.h | 3 +++ shell/platform/darwin/ios/ios_context_gl.mm | 10 ++++++++++ shell/platform/darwin/ios/ios_context_metal.h | 2 +- shell/platform/darwin/ios/ios_context_software.h | 3 +++ shell/platform/darwin/ios/ios_context_software.mm | 5 +++++ 7 files changed, 41 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest_mrc.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest_mrc.mm index cc2f46f9dd275..ac01d94c85d17 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest_mrc.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest_mrc.mm @@ -30,8 +30,17 @@ - (void)testSpawnsShareGpuContext { XCTAssertNotNil(spawn); XCTAssertTrue([engine iosPlatformView] != nullptr); XCTAssertTrue([spawn iosPlatformView] != nullptr); - XCTAssertEqual([engine iosPlatformView]->GetIosContext(), - [spawn iosPlatformView]->GetIosContext()); + 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 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 b/tbd. + XCTAssertTrue(engine_context->GetMainContext() != nullptr); + XCTAssertEqual(engine_context->GetMainContext(), spawn_context->GetMainContext()); + [engine release]; + [spawn release]; } @end diff --git a/shell/platform/darwin/ios/ios_context.h b/shell/platform/darwin/ios/ios_context.h index 6b2bf5e766c5b..b418342046e4d 100644 --- a/shell/platform/darwin/ios/ios_context.h +++ b/shell/platform/darwin/ios/ios_context.h @@ -106,6 +106,14 @@ class IOSContext { int64_t texture_id, fml::scoped_nsobject> texture) = 0; + //---------------------------------------------------------------------------- + /// @brief Accessor for the Skia context associated with IOSSurfaces. + /// + /// @attention The software context doesn't have a Skia context, so this + /// value will be nullptr. + /// + virtual sk_sp GetMainContext() const = 0; + protected: IOSContext(); diff --git a/shell/platform/darwin/ios/ios_context_gl.h b/shell/platform/darwin/ios/ios_context_gl.h index 69260e1903efa..823f13a14c59a 100644 --- a/shell/platform/darwin/ios/ios_context_gl.h +++ b/shell/platform/darwin/ios/ios_context_gl.h @@ -32,6 +32,9 @@ class IOSContextGL final : public IOSContext { // |IOSContext| sk_sp CreateResourceContext() override; + // |IOSContext| + sk_sp GetMainContext() const override; + // |IOSContext| std::unique_ptr MakeCurrent() override; diff --git a/shell/platform/darwin/ios/ios_context_gl.mm b/shell/platform/darwin/ios/ios_context_gl.mm index 4de8f11935ffa..ccf7ffea38344 100644 --- a/shell/platform/darwin/ios/ios_context_gl.mm +++ b/shell/platform/darwin/ios/ios_context_gl.mm @@ -43,6 +43,16 @@ GrBackend::kOpenGL_GrBackend, GPUSurfaceGLDelegate::GetDefaultPlatformGLInterface()); } +// |IOSContext| +sk_sp IOSContextGL::GetMainContext() const { + /// TODO(b/tbd): 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| std::unique_ptr IOSContextGL::MakeCurrent() { return std::make_unique( diff --git a/shell/platform/darwin/ios/ios_context_metal.h b/shell/platform/darwin/ios/ios_context_metal.h index 35fecb0507c2e..d99d7a0e1f7c7 100644 --- a/shell/platform/darwin/ios/ios_context_metal.h +++ b/shell/platform/darwin/ios/ios_context_metal.h @@ -24,7 +24,7 @@ class IOSContextMetal final : public IOSContext { fml::scoped_nsobject GetDarwinContext() const; - sk_sp GetMainContext() const; + sk_sp GetMainContext() const override; sk_sp GetResourceContext() const; diff --git a/shell/platform/darwin/ios/ios_context_software.h b/shell/platform/darwin/ios/ios_context_software.h index 4407be6033cc7..3731bd3ed6ed5 100644 --- a/shell/platform/darwin/ios/ios_context_software.h +++ b/shell/platform/darwin/ios/ios_context_software.h @@ -20,6 +20,9 @@ class IOSContextSoftware final : public IOSContext { // |IOSContext| sk_sp CreateResourceContext() override; + // |IOSContext| + sk_sp GetMainContext() const override; + // |IOSContext| std::unique_ptr MakeCurrent() override; diff --git a/shell/platform/darwin/ios/ios_context_software.mm b/shell/platform/darwin/ios/ios_context_software.mm index 261fdc44a450a..7005b5046406d 100644 --- a/shell/platform/darwin/ios/ios_context_software.mm +++ b/shell/platform/darwin/ios/ios_context_software.mm @@ -16,6 +16,11 @@ return nullptr; } +// |IOSContext| +sk_sp IOSContextSoftware::GetMainContext() const { + return nullptr; +} + // |IOSContext| std::unique_ptr IOSContextSoftware::MakeCurrent() { // This only makes sense for context that need to be bound to a specific thread. From 5f335adfb1242d518853209752c580e101e23936 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 11 Jan 2021 14:03:38 -0800 Subject: [PATCH 2/4] updated docs --- shell/platform/darwin/ios/ios_context.h | 7 +++++-- shell/platform/darwin/ios/ios_context_metal.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/shell/platform/darwin/ios/ios_context.h b/shell/platform/darwin/ios/ios_context.h index b418342046e4d..f6b19b7d440b4 100644 --- a/shell/platform/darwin/ios/ios_context.h +++ b/shell/platform/darwin/ios/ios_context.h @@ -107,10 +107,13 @@ class IOSContext { fml::scoped_nsobject> texture) = 0; //---------------------------------------------------------------------------- - /// @brief Accessor for the Skia context associated with IOSSurfaces. - /// + /// @brief Accessor for the Skia context associated with IOSSurfaces and + /// the raster thread. + /// @returns `nullptr` on failure. /// @attention The software context doesn't have a Skia context, so this /// value will be nullptr. + /// @see For contexts which are used for offscreen work like loading + /// textures see IOSContext::CreateResourceContext. /// virtual sk_sp GetMainContext() const = 0; diff --git a/shell/platform/darwin/ios/ios_context_metal.h b/shell/platform/darwin/ios/ios_context_metal.h index d99d7a0e1f7c7..ebda072bea0e9 100644 --- a/shell/platform/darwin/ios/ios_context_metal.h +++ b/shell/platform/darwin/ios/ios_context_metal.h @@ -24,6 +24,7 @@ class IOSContextMetal final : public IOSContext { fml::scoped_nsobject GetDarwinContext() const; + // |IOSContext| sk_sp GetMainContext() const override; sk_sp GetResourceContext() const; From e3e382a72a3e9eaa8efdf959ddf717feebdf65f9 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 11 Jan 2021 14:08:59 -0800 Subject: [PATCH 3/4] updated docstring --- shell/platform/darwin/ios/ios_context.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/shell/platform/darwin/ios/ios_context.h b/shell/platform/darwin/ios/ios_context.h index f6b19b7d440b4..c08b6b9c729d3 100644 --- a/shell/platform/darwin/ios/ios_context.h +++ b/shell/platform/darwin/ios/ios_context.h @@ -109,6 +109,9 @@ class IOSContext { //---------------------------------------------------------------------------- /// @brief Accessor for the Skia context associated with IOSSurfaces and /// the raster thread. + /// @details There can be any number of resource contexts but this is the + /// one context that will be used by surfaces to draw to the + /// screen from the raster thread. /// @returns `nullptr` on failure. /// @attention The software context doesn't have a Skia context, so this /// value will be nullptr. From 98571634f7840c5c564dd2c0ef0008c03167fa92 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 11 Jan 2021 14:27:06 -0800 Subject: [PATCH 4/4] linked refactor issue --- .../darwin/ios/framework/Source/FlutterEngineTest_mrc.mm | 2 +- shell/platform/darwin/ios/ios_context_gl.mm | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest_mrc.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest_mrc.mm index ac01d94c85d17..f5595bffb99d3 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest_mrc.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest_mrc.mm @@ -36,7 +36,7 @@ - (void)testSpawnsShareGpuContext { // 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 b/tbd. + // 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.mm b/shell/platform/darwin/ios/ios_context_gl.mm index ccf7ffea38344..d803174cba2ca 100644 --- a/shell/platform/darwin/ios/ios_context_gl.mm +++ b/shell/platform/darwin/ios/ios_context_gl.mm @@ -45,7 +45,7 @@ // |IOSContext| sk_sp IOSContextGL::GetMainContext() const { - /// TODO(b/tbd): Currently the GPUSurfaceGL creates the main context for + /// 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