From 8cdc5165d86bd4b60b85ea714b54933f55f8e5a3 Mon Sep 17 00:00:00 2001 From: George Wright Date: Fri, 30 Aug 2019 16:06:07 -0700 Subject: [PATCH 01/17] Manage resource and onscreen contexts using separate IOSGLContext objects --- .../ios/framework/Source/FlutterOverlayView.h | 3 +- .../framework/Source/FlutterOverlayView.mm | 5 +-- .../framework/Source/FlutterPlatformViews.mm | 12 ++++--- .../Source/FlutterPlatformViews_Internal.h | 5 +-- .../ios/framework/Source/FlutterView.mm | 10 ++++-- shell/platform/darwin/ios/ios_gl_context.h | 8 +---- shell/platform/darwin/ios/ios_gl_context.mm | 32 +++++++++---------- .../darwin/ios/ios_gl_render_target.h | 9 +++--- .../darwin/ios/ios_gl_render_target.mm | 23 +++++++------ shell/platform/darwin/ios/ios_surface_gl.h | 12 +++++-- shell/platform/darwin/ios/ios_surface_gl.mm | 20 ++++++------ .../darwin/ios/ios_surface_software.mm | 2 +- shell/platform/darwin/ios/platform_view_ios.h | 2 +- .../platform/darwin/ios/platform_view_ios.mm | 6 ++-- 14 files changed, 80 insertions(+), 69 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h b/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h index fac783f87f33f..12238fc99d311 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h @@ -25,7 +25,8 @@ - (instancetype)init NS_DESIGNATED_INITIALIZER; - (instancetype)initWithContentsScale:(CGFloat)contentsScale; - (std::unique_ptr)createSurface: - (std::shared_ptr)gl_context; + (std::shared_ptr)onscreen_gl_context + resourceContext:(std::shared_ptr)resource_gl_context; @end diff --git a/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm b/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm index 414029023061d..43bd5052735ac 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm @@ -79,14 +79,15 @@ + (Class)layerClass { } - (std::unique_ptr)createSurface: - (std::shared_ptr)gl_context { + (std::shared_ptr)onscreen_gl_context + resourceContext:(std::shared_ptr)resource_gl_context { if ([self.layer isKindOfClass:[CAEAGLLayer class]]) { fml::scoped_nsobject eagl_layer( reinterpret_cast([self.layer retain])); if (@available(iOS 9.0, *)) { eagl_layer.get().presentsWithTransaction = YES; } - return std::make_unique(std::move(eagl_layer), gl_context); + return std::make_unique(std::move(eagl_layer), onscreen_gl_context, resource_gl_context); #if FLUTTER_SHELL_ENABLE_METAL } else if ([self.layer isKindOfClass:[CAMetalLayer class]]) { fml::scoped_nsobject metalLayer( diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 00a7c9ecda5bd..cde4ab9b84629 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -362,12 +362,13 @@ } bool FlutterPlatformViewsController::SubmitFrame(GrContext* gr_context, - std::shared_ptr gl_context) { + std::shared_ptr onscreen_gl_context, + std::shared_ptr resource_gl_context) { DisposeViews(); bool did_submit = true; for (int64_t view_id : composition_order_) { - EnsureOverlayInitialized(view_id, std::move(gl_context), gr_context); + EnsureOverlayInitialized(view_id, std::move(onscreen_gl_context), std::move(resource_gl_context), gr_context); auto frame = overlays_[view_id]->surface->AcquireFrame(frame_size_); SkCanvas* canvas = frame->SkiaCanvas(); canvas->drawPicture(picture_recorders_[view_id]->finishRecordingAsPicture()); @@ -451,7 +452,8 @@ void FlutterPlatformViewsController::EnsureOverlayInitialized( int64_t overlay_id, - std::shared_ptr gl_context, + std::shared_ptr onscreen_gl_context, + std::shared_ptr resource_gl_context, GrContext* gr_context) { FML_DCHECK(flutter_view_); @@ -467,7 +469,7 @@ overlay_view.get().frame = flutter_view_.get().bounds; overlay_view.get().autoresizingMask = (UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight); - std::unique_ptr ios_surface = [overlay_view.get() createSurface:nil]; + std::unique_ptr ios_surface = [overlay_view.get() createSurface:nil resourceContext:nil]; std::unique_ptr surface = ios_surface->CreateGPUSurface(); overlays_[overlay_id] = std::make_unique( std::move(overlay_view), std::move(ios_surface), std::move(surface)); @@ -492,7 +494,7 @@ overlay_view.get().autoresizingMask = (UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight); std::unique_ptr ios_surface = - [overlay_view.get() createSurface:std::move(gl_context)]; + [overlay_view.get() createSurface:std::move(onscreen_gl_context) resourceContext:std::move(resource_gl_context)]; std::unique_ptr surface = ios_surface->CreateGPUSurface(gr_context); overlays_[overlay_id] = std::make_unique( std::move(overlay_view), std::move(ios_surface), std::move(surface)); diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index f9f0eefece601..e3c3fd1b322f4 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -100,7 +100,7 @@ class FlutterPlatformViewsController { // Discards all platform views instances and auxiliary resources. void Reset(); - bool SubmitFrame(GrContext* gr_context, std::shared_ptr gl_context); + bool SubmitFrame(GrContext* gr_context, std::shared_ptr onscreen_gl_context, std::shared_ptr resource_gl_context); void OnMethodCall(FlutterMethodCall* call, FlutterResult& result); @@ -162,7 +162,8 @@ class FlutterPlatformViewsController { // Dispose the views in `views_to_dispose_`. void DisposeViews(); void EnsureOverlayInitialized(int64_t overlay_id, - std::shared_ptr gl_context, + std::shared_ptr onscreen_gl_context, + std::shared_ptr resource_gl_context, GrContext* gr_context); // This will return true after pre-roll if any of the embedded views diff --git a/shell/platform/darwin/ios/framework/Source/FlutterView.mm b/shell/platform/darwin/ios/framework/Source/FlutterView.mm index 4e4acdfc0438f..c590aaef27a7c 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterView.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterView.mm @@ -13,6 +13,7 @@ #include "flutter/shell/common/platform_view.h" #include "flutter/shell/common/rasterizer.h" #include "flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h" +#include "flutter/shell/platform/darwin/ios/ios_gl_context.h" #include "flutter/shell/platform/darwin/ios/ios_surface_gl.h" #include "flutter/shell/platform/darwin/ios/ios_surface_software.h" #include "third_party/skia/include/utils/mac/SkCGUtils.h" @@ -24,6 +25,7 @@ @implementation FlutterView id _delegate; +std::shared_ptr _onscreen_context; - (instancetype)init { @throw([NSException exceptionWithName:@"FlutterView must initWithDelegate" @@ -52,6 +54,10 @@ - (instancetype)initWithDelegate:(id)delegate opaque: self.layer.opaque = opaque; } +#if !TARGET_IPHONE_SIMULATOR + _onscreen_context = std::make_shared(); +#endif // !TARGET_IPHONE_SIMULATOR + return self; } @@ -94,7 +100,7 @@ + (Class)layerClass { } - (std::unique_ptr)createSurface: - (std::shared_ptr)context { + (std::shared_ptr)resource_context { if ([self.layer isKindOfClass:[CAEAGLLayer class]]) { fml::scoped_nsobject eagl_layer( reinterpret_cast([self.layer retain])); @@ -105,7 +111,7 @@ + (Class)layerClass { eagl_layer.get().presentsWithTransaction = YES; } } - return std::make_unique(context, std::move(eagl_layer), + return std::make_unique(_onscreen_context, resource_context, std::move(eagl_layer), [_delegate platformViewsController]); #if FLUTTER_SHELL_ENABLE_METAL } else if ([self.layer isKindOfClass:[CAMetalLayer class]]) { diff --git a/shell/platform/darwin/ios/ios_gl_context.h b/shell/platform/darwin/ios/ios_gl_context.h index 232645d9c8592..44d5444c6987a 100644 --- a/shell/platform/darwin/ios/ios_gl_context.h +++ b/shell/platform/darwin/ios/ios_gl_context.h @@ -13,7 +13,6 @@ #include "flutter/fml/macros.h" #include "flutter/fml/platform/darwin/scoped_nsobject.h" #include "flutter/shell/common/platform_view.h" -#include "ios_gl_render_target.h" namespace flutter { @@ -23,18 +22,13 @@ class IOSGLContext { ~IOSGLContext(); - std::unique_ptr CreateRenderTarget( - fml::scoped_nsobject layer); - bool MakeCurrent(); - bool ResourceMakeCurrent(); - + bool BindRenderbufferStorage(NSUInteger target, fml::scoped_nsobject layer); sk_sp ColorSpace() const { return color_space_; } private: fml::scoped_nsobject context_; - fml::scoped_nsobject resource_context_; sk_sp color_space_; FML_DISALLOW_COPY_AND_ASSIGN(IOSGLContext); diff --git a/shell/platform/darwin/ios/ios_gl_context.mm b/shell/platform/darwin/ios/ios_gl_context.mm index 52fb85f8f19a9..b31585b4e347e 100644 --- a/shell/platform/darwin/ios/ios_gl_context.mm +++ b/shell/platform/darwin/ios/ios_gl_context.mm @@ -12,15 +12,19 @@ namespace flutter { +static fml::scoped_nsobject sharegroup_; + IOSGLContext::IOSGLContext() { - resource_context_.reset([[EAGLContext alloc] initWithAPI:kEAGLRenderingAPIOpenGLES3]); - if (resource_context_ != nullptr) { - context_.reset([[EAGLContext alloc] initWithAPI:kEAGLRenderingAPIOpenGLES3 - sharegroup:resource_context_.get().sharegroup]); - } else { - resource_context_.reset([[EAGLContext alloc] initWithAPI:kEAGLRenderingAPIOpenGLES2]); + context_.reset([[EAGLContext alloc] initWithAPI:kEAGLRenderingAPIOpenGLES3 + sharegroup:sharegroup_.get()]); + + if (!context_) { context_.reset([[EAGLContext alloc] initWithAPI:kEAGLRenderingAPIOpenGLES2 - sharegroup:resource_context_.get().sharegroup]); + sharegroup:sharegroup_.get()]); + } + + if (sharegroup_ == nullptr) { + sharegroup_.reset(context_.get().sharegroup); } // TODO: @@ -44,20 +48,14 @@ } } -IOSGLContext::~IOSGLContext() = default; - -std::unique_ptr IOSGLContext::CreateRenderTarget( - fml::scoped_nsobject layer) { - return std::make_unique(std::move(layer), context_.get(), - resource_context_.get()); +bool IOSGLContext::BindRenderbufferStorage(NSUInteger target, fml::scoped_nsobject layer) { + return [context_.get() renderbufferStorage:GL_RENDERBUFFER fromDrawable:layer.get()]; } +IOSGLContext::~IOSGLContext() = default; + bool IOSGLContext::MakeCurrent() { return [EAGLContext setCurrentContext:context_.get()]; } -bool IOSGLContext::ResourceMakeCurrent() { - return [EAGLContext setCurrentContext:resource_context_.get()]; -} - } // namespace flutter diff --git a/shell/platform/darwin/ios/ios_gl_render_target.h b/shell/platform/darwin/ios/ios_gl_render_target.h index b2eafe16e0950..6145f7139d4a4 100644 --- a/shell/platform/darwin/ios/ios_gl_render_target.h +++ b/shell/platform/darwin/ios/ios_gl_render_target.h @@ -13,14 +13,15 @@ #include "flutter/fml/macros.h" #include "flutter/fml/platform/darwin/scoped_nsobject.h" #include "flutter/shell/common/platform_view.h" +#include "flutter/shell/platform/darwin/ios/ios_gl_context.h" namespace flutter { class IOSGLRenderTarget { public: IOSGLRenderTarget(fml::scoped_nsobject layer, - EAGLContext* context, - EAGLContext* resource_context); + std::shared_ptr onscreen_context, + std::shared_ptr resource_context); ~IOSGLRenderTarget(); @@ -40,8 +41,8 @@ class IOSGLRenderTarget { private: fml::scoped_nsobject layer_; - fml::scoped_nsobject context_; - fml::scoped_nsobject resource_context_; + std::shared_ptr onscreen_context_; + std::shared_ptr resource_context_; GLuint framebuffer_; GLuint colorbuffer_; GLint storage_size_width_; diff --git a/shell/platform/darwin/ios/ios_gl_render_target.mm b/shell/platform/darwin/ios/ios_gl_render_target.mm index a57ba9c46b414..cf3727f1d5a77 100644 --- a/shell/platform/darwin/ios/ios_gl_render_target.mm +++ b/shell/platform/darwin/ios/ios_gl_render_target.mm @@ -11,23 +11,22 @@ #include "third_party/skia/include/gpu/gl/GrGLInterface.h" namespace flutter { - IOSGLRenderTarget::IOSGLRenderTarget(fml::scoped_nsobject layer, - EAGLContext* context, - EAGLContext* resource_context) + std::shared_ptr onscreen_context, + std::shared_ptr resource_context) : layer_(std::move(layer)), - context_([context retain]), - resource_context_([resource_context retain]), + onscreen_context_(onscreen_context.get()), + resource_context_(resource_context.get()), framebuffer_(GL_NONE), colorbuffer_(GL_NONE), storage_size_width_(0), storage_size_height_(0), valid_(false) { FML_DCHECK(layer_ != nullptr); - FML_DCHECK(context_ != nullptr); + FML_DCHECK(onscreen_context_ != nullptr); FML_DCHECK(resource_context_ != nullptr); - bool context_current = [EAGLContext setCurrentContext:context_]; + bool context_current = onscreen_context_->MakeCurrent(); FML_DCHECK(context_current); FML_DCHECK(glGetError() == GL_NO_ERROR); @@ -63,7 +62,7 @@ IOSGLRenderTarget::~IOSGLRenderTarget() { EAGLContext* context = EAGLContext.currentContext; - [EAGLContext setCurrentContext:context_]; + onscreen_context_->MakeCurrent(); FML_DCHECK(glGetError() == GL_NO_ERROR); // Deletes on GL_NONEs are ignored @@ -105,7 +104,7 @@ FML_DCHECK(glGetError() == GL_NO_ERROR); - if (![EAGLContext setCurrentContext:context_]) { + if (!onscreen_context_->MakeCurrent()) { return false; } @@ -116,7 +115,7 @@ glBindRenderbuffer(GL_RENDERBUFFER, colorbuffer_); FML_DCHECK(glGetError() == GL_NO_ERROR); - if (![context_.get() renderbufferStorage:GL_RENDERBUFFER fromDrawable:layer_.get()]) { + if (!onscreen_context_->BindRenderbufferStorage(GL_RENDERBUFFER, std::move(layer_))) { return false; } @@ -133,11 +132,11 @@ } bool IOSGLRenderTarget::MakeCurrent() { - return UpdateStorageSizeIfNecessary() && [EAGLContext setCurrentContext:context_.get()]; + return UpdateStorageSizeIfNecessary() && onscreen_context_->MakeCurrent(); } bool IOSGLRenderTarget::ResourceMakeCurrent() { - return [EAGLContext setCurrentContext:resource_context_.get()]; + return resource_context_->MakeCurrent(); } } // namespace flutter diff --git a/shell/platform/darwin/ios/ios_surface_gl.h b/shell/platform/darwin/ios/ios_surface_gl.h index 964cc7adb692e..e6170dea7ec20 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.h +++ b/shell/platform/darwin/ios/ios_surface_gl.h @@ -20,11 +20,14 @@ class IOSSurfaceGL final : public IOSSurface, public GPUSurfaceGLDelegate, public ExternalViewEmbedder { public: - IOSSurfaceGL(std::shared_ptr context, + IOSSurfaceGL(std::shared_ptr onscreen_context, + std::shared_ptr resource_context, fml::scoped_nsobject layer, FlutterPlatformViewsController* platform_views_controller); - IOSSurfaceGL(fml::scoped_nsobject layer, std::shared_ptr context); + IOSSurfaceGL(fml::scoped_nsobject layer, + std::shared_ptr onscreen_context, + std::shared_ptr resource_context); ~IOSSurfaceGL() override; @@ -79,7 +82,10 @@ class IOSSurfaceGL final : public IOSSurface, bool SubmitFrame(GrContext* context) override; private: - std::shared_ptr context_; + std::shared_ptr onscreen_context_; + + std::shared_ptr resource_context_; + std::unique_ptr render_target_; FML_DISALLOW_COPY_AND_ASSIGN(IOSSurfaceGL); diff --git a/shell/platform/darwin/ios/ios_surface_gl.mm b/shell/platform/darwin/ios/ios_surface_gl.mm index 9e838e200df34..674a06a60b9c2 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.mm +++ b/shell/platform/darwin/ios/ios_surface_gl.mm @@ -9,17 +9,19 @@ namespace flutter { -IOSSurfaceGL::IOSSurfaceGL(std::shared_ptr context, +IOSSurfaceGL::IOSSurfaceGL(std::shared_ptr onscreen_context, + std::shared_ptr resource_context, fml::scoped_nsobject layer, FlutterPlatformViewsController* platform_views_controller) - : IOSSurface(platform_views_controller), context_(context) { - render_target_ = context_->CreateRenderTarget(std::move(layer)); + : IOSSurface(platform_views_controller), onscreen_context_(onscreen_context), resource_context_(resource_context) { + render_target_ = std::make_unique(std::move(layer), onscreen_context_, resource_context_); } IOSSurfaceGL::IOSSurfaceGL(fml::scoped_nsobject layer, - std::shared_ptr context) - : IOSSurface(nullptr), context_(context) { - render_target_ = context_->CreateRenderTarget(std::move(layer)); + std::shared_ptr onscreen_context, + std::shared_ptr resource_context) + : IOSSurface(nullptr), onscreen_context_(onscreen_context), resource_context_(resource_context) { + render_target_ = std::make_unique(std::move(layer), onscreen_context_, resource_context_); } IOSSurfaceGL::~IOSSurfaceGL() = default; @@ -29,7 +31,7 @@ } bool IOSSurfaceGL::ResourceContextMakeCurrent() { - return context_->ResourceMakeCurrent(); + return resource_context_->MakeCurrent(); } void IOSSurfaceGL::UpdateStorageSizeIfNecessary() { @@ -60,7 +62,7 @@ if (!IsValid()) { return false; } - return render_target_->UpdateStorageSizeIfNecessary() && context_->MakeCurrent(); + return render_target_->UpdateStorageSizeIfNecessary() && onscreen_context_->MakeCurrent(); } bool IOSSurfaceGL::GLContextClearCurrent() { @@ -145,7 +147,7 @@ return true; } - bool submitted = platform_views_controller->SubmitFrame(std::move(context), context_); + bool submitted = platform_views_controller->SubmitFrame(std::move(context), onscreen_context_, resource_context_); [CATransaction commit]; return submitted; } diff --git a/shell/platform/darwin/ios/ios_surface_software.mm b/shell/platform/darwin/ios/ios_surface_software.mm index 4566bbf3f3624..10595359c650c 100644 --- a/shell/platform/darwin/ios/ios_surface_software.mm +++ b/shell/platform/darwin/ios/ios_surface_software.mm @@ -184,7 +184,7 @@ if (platform_views_controller == nullptr) { return true; } - return platform_views_controller->SubmitFrame(nullptr, nullptr); + return platform_views_controller->SubmitFrame(nullptr, nullptr, nullptr); } } // namespace flutter diff --git a/shell/platform/darwin/ios/platform_view_ios.h b/shell/platform/darwin/ios/platform_view_ios.h index 38fc2a9f30b28..574983bc0040a 100644 --- a/shell/platform/darwin/ios/platform_view_ios.h +++ b/shell/platform/darwin/ios/platform_view_ios.h @@ -47,7 +47,7 @@ class PlatformViewIOS final : public PlatformView { private: fml::WeakPtr owner_controller_; std::unique_ptr ios_surface_; - std::shared_ptr gl_context_; + std::shared_ptr gl_resource_context_; PlatformMessageRouter platform_message_router_; std::unique_ptr accessibility_bridge_; fml::scoped_nsprotocol text_input_plugin_; diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index c80ae377c03a1..c62ae99e4ca72 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -23,7 +23,7 @@ flutter::TaskRunners task_runners) : PlatformView(delegate, std::move(task_runners)) { #if !TARGET_IPHONE_SIMULATOR - gl_context_ = std::make_shared(); + gl_resource_context_ = std::make_shared(); #endif // !TARGET_IPHONE_SIMULATOR } @@ -51,7 +51,7 @@ owner_controller_ = owner_controller; if (owner_controller_) { ios_surface_ = - [static_cast(owner_controller.get().view) createSurface:gl_context_]; + [static_cast(owner_controller.get().view) createSurface:gl_resource_context_]; FML_DCHECK(ios_surface_ != nullptr); if (accessibility_bridge_) { @@ -83,7 +83,7 @@ new AccessibilityBridge(static_cast(owner_controller_.get().view), // |PlatformView| sk_sp PlatformViewIOS::CreateResourceContext() const { - if (!gl_context_ || !gl_context_->ResourceMakeCurrent()) { + if (!gl_resource_context_ || !gl_resource_context_->MakeCurrent()) { FML_DLOG(INFO) << "Could not make resource context current on IO thread. " "Async texture uploads will be disabled. On Simulators, " "this is expected."; From 9f34f1ab4febc063cdf40ed9e75875e3cbc49594 Mon Sep 17 00:00:00 2001 From: George Wright Date: Fri, 30 Aug 2019 20:05:55 -0700 Subject: [PATCH 02/17] formatting --- .../ios/framework/Source/FlutterOverlayView.h | 4 ++-- .../ios/framework/Source/FlutterOverlayView.mm | 7 ++++--- .../framework/Source/FlutterPlatformViews.mm | 16 ++++++++++------ .../Source/FlutterPlatformViews_Internal.h | 4 +++- .../darwin/ios/framework/Source/FlutterView.mm | 3 ++- shell/platform/darwin/ios/ios_gl_context.mm | 3 ++- shell/platform/darwin/ios/ios_surface_gl.mm | 17 ++++++++++++----- 7 files changed, 35 insertions(+), 19 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h b/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h index 12238fc99d311..d47df4a532041 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h @@ -24,8 +24,8 @@ - (instancetype)init NS_DESIGNATED_INITIALIZER; - (instancetype)initWithContentsScale:(CGFloat)contentsScale; -- (std::unique_ptr)createSurface: - (std::shared_ptr)onscreen_gl_context +- (std::unique_ptr) + createSurface:(std::shared_ptr)onscreen_gl_context resourceContext:(std::shared_ptr)resource_gl_context; @end diff --git a/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm b/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm index 43bd5052735ac..8dc5391bdc5d0 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm @@ -78,8 +78,8 @@ + (Class)layerClass { #endif // TARGET_IPHONE_SIMULATOR } -- (std::unique_ptr)createSurface: - (std::shared_ptr)onscreen_gl_context +- (std::unique_ptr) + createSurface:(std::shared_ptr)onscreen_gl_context resourceContext:(std::shared_ptr)resource_gl_context { if ([self.layer isKindOfClass:[CAEAGLLayer class]]) { fml::scoped_nsobject eagl_layer( @@ -87,7 +87,8 @@ + (Class)layerClass { if (@available(iOS 9.0, *)) { eagl_layer.get().presentsWithTransaction = YES; } - return std::make_unique(std::move(eagl_layer), onscreen_gl_context, resource_gl_context); + return std::make_unique(std::move(eagl_layer), onscreen_gl_context, + resource_gl_context); #if FLUTTER_SHELL_ENABLE_METAL } else if ([self.layer isKindOfClass:[CAMetalLayer class]]) { fml::scoped_nsobject metalLayer( diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index cde4ab9b84629..d43cf55b7a71d 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -361,14 +361,16 @@ views_to_recomposite_.clear(); } -bool FlutterPlatformViewsController::SubmitFrame(GrContext* gr_context, - std::shared_ptr onscreen_gl_context, - std::shared_ptr resource_gl_context) { +bool FlutterPlatformViewsController::SubmitFrame( + GrContext* gr_context, + std::shared_ptr onscreen_gl_context, + std::shared_ptr resource_gl_context) { DisposeViews(); bool did_submit = true; for (int64_t view_id : composition_order_) { - EnsureOverlayInitialized(view_id, std::move(onscreen_gl_context), std::move(resource_gl_context), gr_context); + EnsureOverlayInitialized(view_id, std::move(onscreen_gl_context), + std::move(resource_gl_context), gr_context); auto frame = overlays_[view_id]->surface->AcquireFrame(frame_size_); SkCanvas* canvas = frame->SkiaCanvas(); canvas->drawPicture(picture_recorders_[view_id]->finishRecordingAsPicture()); @@ -469,7 +471,8 @@ overlay_view.get().frame = flutter_view_.get().bounds; overlay_view.get().autoresizingMask = (UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight); - std::unique_ptr ios_surface = [overlay_view.get() createSurface:nil resourceContext:nil]; + std::unique_ptr ios_surface = [overlay_view.get() createSurface:nil + resourceContext:nil]; std::unique_ptr surface = ios_surface->CreateGPUSurface(); overlays_[overlay_id] = std::make_unique( std::move(overlay_view), std::move(ios_surface), std::move(surface)); @@ -494,7 +497,8 @@ overlay_view.get().autoresizingMask = (UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight); std::unique_ptr ios_surface = - [overlay_view.get() createSurface:std::move(onscreen_gl_context) resourceContext:std::move(resource_gl_context)]; + [overlay_view.get() createSurface:std::move(onscreen_gl_context) + resourceContext:std::move(resource_gl_context)]; std::unique_ptr surface = ios_surface->CreateGPUSurface(gr_context); overlays_[overlay_id] = std::make_unique( std::move(overlay_view), std::move(ios_surface), std::move(surface)); diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index e3c3fd1b322f4..e161b5b622ab4 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -100,7 +100,9 @@ class FlutterPlatformViewsController { // Discards all platform views instances and auxiliary resources. void Reset(); - bool SubmitFrame(GrContext* gr_context, std::shared_ptr onscreen_gl_context, std::shared_ptr resource_gl_context); + bool SubmitFrame(GrContext* gr_context, + std::shared_ptr onscreen_gl_context, + std::shared_ptr resource_gl_context); void OnMethodCall(FlutterMethodCall* call, FlutterResult& result); diff --git a/shell/platform/darwin/ios/framework/Source/FlutterView.mm b/shell/platform/darwin/ios/framework/Source/FlutterView.mm index c590aaef27a7c..4742ca84a57b4 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterView.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterView.mm @@ -111,7 +111,8 @@ + (Class)layerClass { eagl_layer.get().presentsWithTransaction = YES; } } - return std::make_unique(_onscreen_context, resource_context, std::move(eagl_layer), + return std::make_unique(_onscreen_context, resource_context, + std::move(eagl_layer), [_delegate platformViewsController]); #if FLUTTER_SHELL_ENABLE_METAL } else if ([self.layer isKindOfClass:[CAMetalLayer class]]) { diff --git a/shell/platform/darwin/ios/ios_gl_context.mm b/shell/platform/darwin/ios/ios_gl_context.mm index b31585b4e347e..48d7942f8b50c 100644 --- a/shell/platform/darwin/ios/ios_gl_context.mm +++ b/shell/platform/darwin/ios/ios_gl_context.mm @@ -48,7 +48,8 @@ } } -bool IOSGLContext::BindRenderbufferStorage(NSUInteger target, fml::scoped_nsobject layer) { +bool IOSGLContext::BindRenderbufferStorage(NSUInteger target, + fml::scoped_nsobject layer) { return [context_.get() renderbufferStorage:GL_RENDERBUFFER fromDrawable:layer.get()]; } diff --git a/shell/platform/darwin/ios/ios_surface_gl.mm b/shell/platform/darwin/ios/ios_surface_gl.mm index 674a06a60b9c2..ec8ef4693b25f 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.mm +++ b/shell/platform/darwin/ios/ios_surface_gl.mm @@ -13,15 +13,21 @@ std::shared_ptr resource_context, fml::scoped_nsobject layer, FlutterPlatformViewsController* platform_views_controller) - : IOSSurface(platform_views_controller), onscreen_context_(onscreen_context), resource_context_(resource_context) { - render_target_ = std::make_unique(std::move(layer), onscreen_context_, resource_context_); + : IOSSurface(platform_views_controller), + onscreen_context_(onscreen_context), + resource_context_(resource_context) { + render_target_ = + std::make_unique(std::move(layer), onscreen_context_, resource_context_); } IOSSurfaceGL::IOSSurfaceGL(fml::scoped_nsobject layer, std::shared_ptr onscreen_context, std::shared_ptr resource_context) - : IOSSurface(nullptr), onscreen_context_(onscreen_context), resource_context_(resource_context) { - render_target_ = std::make_unique(std::move(layer), onscreen_context_, resource_context_); + : IOSSurface(nullptr), + onscreen_context_(onscreen_context), + resource_context_(resource_context) { + render_target_ = + std::make_unique(std::move(layer), onscreen_context_, resource_context_); } IOSSurfaceGL::~IOSSurfaceGL() = default; @@ -147,7 +153,8 @@ return true; } - bool submitted = platform_views_controller->SubmitFrame(std::move(context), onscreen_context_, resource_context_); + bool submitted = platform_views_controller->SubmitFrame(std::move(context), onscreen_context_, + resource_context_); [CATransaction commit]; return submitted; } From f62387b43003f7ee0ca93ab283586f4d9fa0ead7 Mon Sep 17 00:00:00 2001 From: George Wright Date: Tue, 3 Sep 2019 11:06:25 -0700 Subject: [PATCH 03/17] get rid of sharegroup static, plumb it through at time of surface creation. minor style updates. release onscreenContext at FlutterView destruction --- .../darwin/ios/framework/Source/FlutterView.mm | 17 +++++++++++------ shell/platform/darwin/ios/ios_gl_context.h | 5 ++++- shell/platform/darwin/ios/ios_gl_context.mm | 16 +++++++--------- shell/platform/darwin/ios/platform_view_ios.mm | 2 +- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterView.mm b/shell/platform/darwin/ios/framework/Source/FlutterView.mm index 4742ca84a57b4..08ba0af935bc8 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterView.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterView.mm @@ -25,7 +25,7 @@ @implementation FlutterView id _delegate; -std::shared_ptr _onscreen_context; +std::shared_ptr _onscreenContext; - (instancetype)init { @throw([NSException exceptionWithName:@"FlutterView must initWithDelegate" @@ -45,6 +45,11 @@ - (instancetype)initWithCoder:(NSCoder*)aDecoder { userInfo:nil]); } +- (void)dealloc { + _onscreenContext = nullptr; + [super dealloc]; +} + - (instancetype)initWithDelegate:(id)delegate opaque:(BOOL)opaque { FML_DCHECK(delegate) << "Delegate must not be nil."; self = [super initWithFrame:CGRectNull]; @@ -54,10 +59,6 @@ - (instancetype)initWithDelegate:(id)delegate opaque: self.layer.opaque = opaque; } -#if !TARGET_IPHONE_SIMULATOR - _onscreen_context = std::make_shared(); -#endif // !TARGET_IPHONE_SIMULATOR - return self; } @@ -101,6 +102,10 @@ + (Class)layerClass { - (std::unique_ptr)createSurface: (std::shared_ptr)resource_context { +#if !TARGET_IPHONE_SIMULATOR + _onscreenContext = std::make_shared(resource_context->Sharegroup()); +#endif // !TARGET_IPHONE_SIMULATOR + if ([self.layer isKindOfClass:[CAEAGLLayer class]]) { fml::scoped_nsobject eagl_layer( reinterpret_cast([self.layer retain])); @@ -111,7 +116,7 @@ + (Class)layerClass { eagl_layer.get().presentsWithTransaction = YES; } } - return std::make_unique(_onscreen_context, resource_context, + return std::make_unique(_onscreenContext, resource_context, std::move(eagl_layer), [_delegate platformViewsController]); #if FLUTTER_SHELL_ENABLE_METAL diff --git a/shell/platform/darwin/ios/ios_gl_context.h b/shell/platform/darwin/ios/ios_gl_context.h index 44d5444c6987a..305e3df6e50be 100644 --- a/shell/platform/darwin/ios/ios_gl_context.h +++ b/shell/platform/darwin/ios/ios_gl_context.h @@ -18,13 +18,16 @@ namespace flutter { class IOSGLContext { public: - IOSGLContext(); + IOSGLContext(fml::scoped_nsobject sharegroup); ~IOSGLContext(); bool MakeCurrent(); bool BindRenderbufferStorage(NSUInteger target, fml::scoped_nsobject layer); + + fml::scoped_nsobject Sharegroup(); + sk_sp ColorSpace() const { return color_space_; } private: diff --git a/shell/platform/darwin/ios/ios_gl_context.mm b/shell/platform/darwin/ios/ios_gl_context.mm index 48d7942f8b50c..f09d52e5f5249 100644 --- a/shell/platform/darwin/ios/ios_gl_context.mm +++ b/shell/platform/darwin/ios/ios_gl_context.mm @@ -12,19 +12,13 @@ namespace flutter { -static fml::scoped_nsobject sharegroup_; - -IOSGLContext::IOSGLContext() { +IOSGLContext::IOSGLContext(fml::scoped_nsobject sharegroup) { context_.reset([[EAGLContext alloc] initWithAPI:kEAGLRenderingAPIOpenGLES3 - sharegroup:sharegroup_.get()]); + sharegroup:sharegroup.get()]); if (!context_) { context_.reset([[EAGLContext alloc] initWithAPI:kEAGLRenderingAPIOpenGLES2 - sharegroup:sharegroup_.get()]); - } - - if (sharegroup_ == nullptr) { - sharegroup_.reset(context_.get().sharegroup); + sharegroup:sharegroup.get()]); } // TODO: @@ -48,6 +42,10 @@ } } +fml::scoped_nsobject IOSGLContext::Sharegroup() { + return fml::scoped_nsobject(context_.get().sharegroup); +} + bool IOSGLContext::BindRenderbufferStorage(NSUInteger target, fml::scoped_nsobject layer) { return [context_.get() renderbufferStorage:GL_RENDERBUFFER fromDrawable:layer.get()]; diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index c62ae99e4ca72..bd7580980d8ef 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -23,7 +23,7 @@ flutter::TaskRunners task_runners) : PlatformView(delegate, std::move(task_runners)) { #if !TARGET_IPHONE_SIMULATOR - gl_resource_context_ = std::make_shared(); + gl_resource_context_ = std::make_shared(nullptr); #endif // !TARGET_IPHONE_SIMULATOR } From 78ef3a2f490d00bcef14a6b6b50a5b096f7c22e4 Mon Sep 17 00:00:00 2001 From: George Wright Date: Tue, 3 Sep 2019 14:13:32 -0700 Subject: [PATCH 04/17] review updates - camelCase for objc variables - switch to using a factory for creating contexts using the same sharedgroup -> hides implementation details of shared contexts from the caller --- .../darwin/ios/framework/Source/FlutterView.mm | 6 +++--- shell/platform/darwin/ios/ios_gl_context.h | 8 +++++--- shell/platform/darwin/ios/ios_gl_context.mm | 16 +++++++++------- shell/platform/darwin/ios/platform_view_ios.mm | 2 +- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterView.mm b/shell/platform/darwin/ios/framework/Source/FlutterView.mm index 08ba0af935bc8..86ad89d06536f 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterView.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterView.mm @@ -101,9 +101,9 @@ + (Class)layerClass { } - (std::unique_ptr)createSurface: - (std::shared_ptr)resource_context { + (std::shared_ptr)resourceContext { #if !TARGET_IPHONE_SIMULATOR - _onscreenContext = std::make_shared(resource_context->Sharegroup()); + _onscreenContext = resourceContext->MakeSharedContext(); #endif // !TARGET_IPHONE_SIMULATOR if ([self.layer isKindOfClass:[CAEAGLLayer class]]) { @@ -116,7 +116,7 @@ + (Class)layerClass { eagl_layer.get().presentsWithTransaction = YES; } } - return std::make_unique(_onscreenContext, resource_context, + return std::make_unique(_onscreenContext, resourceContext, std::move(eagl_layer), [_delegate platformViewsController]); #if FLUTTER_SHELL_ENABLE_METAL diff --git a/shell/platform/darwin/ios/ios_gl_context.h b/shell/platform/darwin/ios/ios_gl_context.h index 305e3df6e50be..c668932804c8b 100644 --- a/shell/platform/darwin/ios/ios_gl_context.h +++ b/shell/platform/darwin/ios/ios_gl_context.h @@ -18,7 +18,7 @@ namespace flutter { class IOSGLContext { public: - IOSGLContext(fml::scoped_nsobject sharegroup); + IOSGLContext(); ~IOSGLContext(); @@ -26,11 +26,13 @@ class IOSGLContext { bool BindRenderbufferStorage(NSUInteger target, fml::scoped_nsobject layer); - fml::scoped_nsobject Sharegroup(); - sk_sp ColorSpace() const { return color_space_; } + std::shared_ptr MakeSharedContext(); + private: + IOSGLContext(EAGLSharegroup* sharegroup); + fml::scoped_nsobject context_; sk_sp color_space_; diff --git a/shell/platform/darwin/ios/ios_gl_context.mm b/shell/platform/darwin/ios/ios_gl_context.mm index f09d52e5f5249..a816b4bb0485d 100644 --- a/shell/platform/darwin/ios/ios_gl_context.mm +++ b/shell/platform/darwin/ios/ios_gl_context.mm @@ -12,13 +12,15 @@ namespace flutter { -IOSGLContext::IOSGLContext(fml::scoped_nsobject sharegroup) { +IOSGLContext::IOSGLContext() : IOSGLContext(nullptr) {} + +IOSGLContext::IOSGLContext(EAGLSharegroup* sharegroup) { context_.reset([[EAGLContext alloc] initWithAPI:kEAGLRenderingAPIOpenGLES3 - sharegroup:sharegroup.get()]); + sharegroup:sharegroup]); if (!context_) { context_.reset([[EAGLContext alloc] initWithAPI:kEAGLRenderingAPIOpenGLES2 - sharegroup:sharegroup.get()]); + sharegroup:sharegroup]); } // TODO: @@ -42,10 +44,6 @@ } } -fml::scoped_nsobject IOSGLContext::Sharegroup() { - return fml::scoped_nsobject(context_.get().sharegroup); -} - bool IOSGLContext::BindRenderbufferStorage(NSUInteger target, fml::scoped_nsobject layer) { return [context_.get() renderbufferStorage:GL_RENDERBUFFER fromDrawable:layer.get()]; @@ -57,4 +55,8 @@ return [EAGLContext setCurrentContext:context_.get()]; } +std::shared_ptr IOSGLContext::MakeSharedContext() { + return std::shared_ptr(new IOSGLContext(context_.get().sharegroup)); +} + } // namespace flutter diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index bd7580980d8ef..c62ae99e4ca72 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -23,7 +23,7 @@ flutter::TaskRunners task_runners) : PlatformView(delegate, std::move(task_runners)) { #if !TARGET_IPHONE_SIMULATOR - gl_resource_context_ = std::make_shared(nullptr); + gl_resource_context_ = std::make_shared(); #endif // !TARGET_IPHONE_SIMULATOR } From 2b9e022f286c29352696c4695fcd8be1b0810f7f Mon Sep 17 00:00:00 2001 From: George Wright Date: Wed, 4 Sep 2019 14:33:53 -0700 Subject: [PATCH 05/17] Ensure we properly destroy the IOSSurface when the FlutterView goes away. Refcounting fixes. --- shell/platform/darwin/ios/framework/Source/FlutterView.h | 2 +- shell/platform/darwin/ios/framework/Source/FlutterView.mm | 8 +++++--- shell/platform/darwin/ios/ios_gl_context.h | 2 +- shell/platform/darwin/ios/ios_gl_context.mm | 2 +- shell/platform/darwin/ios/ios_gl_render_target.mm | 8 ++++---- shell/platform/darwin/ios/ios_surface_gl.mm | 8 ++++---- shell/platform/darwin/ios/platform_view_ios.h | 2 ++ shell/platform/darwin/ios/platform_view_ios.mm | 5 ++++- 8 files changed, 22 insertions(+), 15 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterView.h b/shell/platform/darwin/ios/framework/Source/FlutterView.h index 0da2d5b0bc71d..5e7495a508a62 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterView.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterView.h @@ -34,7 +34,7 @@ - (instancetype)initWithDelegate:(id)delegate opaque:(BOOL)opaque NS_DESIGNATED_INITIALIZER; - (std::unique_ptr)createSurface: - (std::shared_ptr)context; + (std::shared_ptr)resourceContext; @end diff --git a/shell/platform/darwin/ios/framework/Source/FlutterView.mm b/shell/platform/darwin/ios/framework/Source/FlutterView.mm index 86ad89d06536f..53348f05e1c95 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterView.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterView.mm @@ -46,7 +46,7 @@ - (instancetype)initWithCoder:(NSCoder*)aDecoder { } - (void)dealloc { - _onscreenContext = nullptr; + _onscreenContext.reset(); [super dealloc]; } @@ -103,7 +103,9 @@ + (Class)layerClass { - (std::unique_ptr)createSurface: (std::shared_ptr)resourceContext { #if !TARGET_IPHONE_SIMULATOR - _onscreenContext = resourceContext->MakeSharedContext(); + if (!_onscreenContext) { + _onscreenContext = resourceContext->MakeSharedContext(); + } #endif // !TARGET_IPHONE_SIMULATOR if ([self.layer isKindOfClass:[CAEAGLLayer class]]) { @@ -116,7 +118,7 @@ + (Class)layerClass { eagl_layer.get().presentsWithTransaction = YES; } } - return std::make_unique(_onscreenContext, resourceContext, + return std::make_unique(_onscreenContext, std::move(resourceContext), std::move(eagl_layer), [_delegate platformViewsController]); #if FLUTTER_SHELL_ENABLE_METAL diff --git a/shell/platform/darwin/ios/ios_gl_context.h b/shell/platform/darwin/ios/ios_gl_context.h index c668932804c8b..0e91072a66857 100644 --- a/shell/platform/darwin/ios/ios_gl_context.h +++ b/shell/platform/darwin/ios/ios_gl_context.h @@ -19,6 +19,7 @@ namespace flutter { class IOSGLContext { public: IOSGLContext(); + IOSGLContext(EAGLSharegroup* sharegroup); ~IOSGLContext(); @@ -31,7 +32,6 @@ class IOSGLContext { std::shared_ptr MakeSharedContext(); private: - IOSGLContext(EAGLSharegroup* sharegroup); fml::scoped_nsobject context_; sk_sp color_space_; diff --git a/shell/platform/darwin/ios/ios_gl_context.mm b/shell/platform/darwin/ios/ios_gl_context.mm index a816b4bb0485d..bbd270835b473 100644 --- a/shell/platform/darwin/ios/ios_gl_context.mm +++ b/shell/platform/darwin/ios/ios_gl_context.mm @@ -56,7 +56,7 @@ } std::shared_ptr IOSGLContext::MakeSharedContext() { - return std::shared_ptr(new IOSGLContext(context_.get().sharegroup)); + return std::make_shared(context_.get().sharegroup); } } // namespace flutter diff --git a/shell/platform/darwin/ios/ios_gl_render_target.mm b/shell/platform/darwin/ios/ios_gl_render_target.mm index cf3727f1d5a77..9c8028aded221 100644 --- a/shell/platform/darwin/ios/ios_gl_render_target.mm +++ b/shell/platform/darwin/ios/ios_gl_render_target.mm @@ -15,16 +15,16 @@ std::shared_ptr onscreen_context, std::shared_ptr resource_context) : layer_(std::move(layer)), - onscreen_context_(onscreen_context.get()), - resource_context_(resource_context.get()), + onscreen_context_(std::move(onscreen_context)), + resource_context_(std::move(resource_context)), framebuffer_(GL_NONE), colorbuffer_(GL_NONE), storage_size_width_(0), storage_size_height_(0), valid_(false) { FML_DCHECK(layer_ != nullptr); - FML_DCHECK(onscreen_context_ != nullptr); - FML_DCHECK(resource_context_ != nullptr); + FML_DCHECK(onscreen_context_); + FML_DCHECK(resource_context_); bool context_current = onscreen_context_->MakeCurrent(); diff --git a/shell/platform/darwin/ios/ios_surface_gl.mm b/shell/platform/darwin/ios/ios_surface_gl.mm index ec8ef4693b25f..95c5ff7bd3e47 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.mm +++ b/shell/platform/darwin/ios/ios_surface_gl.mm @@ -14,8 +14,8 @@ fml::scoped_nsobject layer, FlutterPlatformViewsController* platform_views_controller) : IOSSurface(platform_views_controller), - onscreen_context_(onscreen_context), - resource_context_(resource_context) { + onscreen_context_(std::move(onscreen_context)), + resource_context_(std::move(resource_context)) { render_target_ = std::make_unique(std::move(layer), onscreen_context_, resource_context_); } @@ -24,8 +24,8 @@ std::shared_ptr onscreen_context, std::shared_ptr resource_context) : IOSSurface(nullptr), - onscreen_context_(onscreen_context), - resource_context_(resource_context) { + onscreen_context_(std::move(onscreen_context)), + resource_context_(std::move(resource_context)) { render_target_ = std::make_unique(std::move(layer), onscreen_context_, resource_context_); } diff --git a/shell/platform/darwin/ios/platform_view_ios.h b/shell/platform/darwin/ios/platform_view_ios.h index 574983bc0040a..f3bca12eb9c01 100644 --- a/shell/platform/darwin/ios/platform_view_ios.h +++ b/shell/platform/darwin/ios/platform_view_ios.h @@ -75,6 +75,8 @@ class PlatformViewIOS final : public PlatformView { // |PlatformView| void OnPreEngineRestart() const override; + void NotifyDestroyed() override; + FML_DISALLOW_COPY_AND_ASSIGN(PlatformViewIOS); }; diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index c62ae99e4ca72..bc375eaf8d1a1 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -42,10 +42,13 @@ return owner_controller_; } +void PlatformViewIOS::NotifyDestroyed() { + PlatformView::NotifyDestroyed(); + ios_surface_.reset(); +} void PlatformViewIOS::SetOwnerViewController(fml::WeakPtr owner_controller) { if (ios_surface_ || !owner_controller) { NotifyDestroyed(); - ios_surface_.reset(); accessibility_bridge_.reset(); } owner_controller_ = owner_controller; From ff01e4bbcd0f5bfc9587a6bb282a53a2be1969e3 Mon Sep 17 00:00:00 2001 From: George Wright Date: Wed, 4 Sep 2019 14:59:41 -0700 Subject: [PATCH 06/17] formatting --- shell/platform/darwin/ios/ios_gl_context.h | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/darwin/ios/ios_gl_context.h b/shell/platform/darwin/ios/ios_gl_context.h index 0e91072a66857..50621623c6092 100644 --- a/shell/platform/darwin/ios/ios_gl_context.h +++ b/shell/platform/darwin/ios/ios_gl_context.h @@ -32,7 +32,6 @@ class IOSGLContext { std::shared_ptr MakeSharedContext(); private: - fml::scoped_nsobject context_; sk_sp color_space_; From a8026638b93443fc7c7c417f89177c2332d6b32b Mon Sep 17 00:00:00 2001 From: George Wright Date: Thu, 5 Sep 2019 10:49:35 -0700 Subject: [PATCH 07/17] Review updates - Use WeakPtr to reference the onscreen/resource contexts outside of FlutterView - FlutterView now explicitly owns the onscreen context through a unique_ptr - (hacky for now - need to assess whether it's the right call) add a nullptr_t constructor for fml::WeakPtr - ObjC naming convention nits --- fml/memory/weak_ptr.h | 2 ++ .../ios/framework/Source/FlutterOverlayView.h | 4 ++-- .../framework/Source/FlutterOverlayView.mm | 4 ++-- .../framework/Source/FlutterPlatformViews.mm | 22 +++++++++---------- .../Source/FlutterPlatformViews_Internal.h | 8 +++---- .../darwin/ios/framework/Source/FlutterView.h | 2 +- .../ios/framework/Source/FlutterView.mm | 21 +++++++----------- shell/platform/darwin/ios/ios_gl_context.h | 6 ++++- shell/platform/darwin/ios/ios_gl_context.mm | 11 +++++++--- .../darwin/ios/ios_gl_render_target.h | 8 +++---- .../darwin/ios/ios_gl_render_target.mm | 4 ++-- shell/platform/darwin/ios/ios_surface_gl.h | 12 +++++----- shell/platform/darwin/ios/ios_surface_gl.mm | 8 +++---- shell/platform/darwin/ios/platform_view_ios.h | 2 +- .../platform/darwin/ios/platform_view_ios.mm | 6 ++--- 15 files changed, 62 insertions(+), 58 deletions(-) diff --git a/fml/memory/weak_ptr.h b/fml/memory/weak_ptr.h index 09cc10a116b35..907139e10aae6 100644 --- a/fml/memory/weak_ptr.h +++ b/fml/memory/weak_ptr.h @@ -45,6 +45,8 @@ class WeakPtr { // Copy constructor. WeakPtr(const WeakPtr& r) = default; + WeakPtr(std::nullptr_t r) : ptr_(r) {} + template WeakPtr(const WeakPtr& r) : ptr_(static_cast(r.ptr_)), flag_(r.flag_), checker_(r.checker_) {} diff --git a/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h b/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h index d47df4a532041..85d11efc7cb10 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h @@ -25,8 +25,8 @@ - (instancetype)init NS_DESIGNATED_INITIALIZER; - (instancetype)initWithContentsScale:(CGFloat)contentsScale; - (std::unique_ptr) - createSurface:(std::shared_ptr)onscreen_gl_context - resourceContext:(std::shared_ptr)resource_gl_context; + createSurfaceWithOnscreenContext:(fml::WeakPtr)onscreen_gl_context + resourceContext:(fml::WeakPtr)resource_gl_context; @end diff --git a/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm b/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm index 8dc5391bdc5d0..ec14f06b1331b 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm @@ -79,8 +79,8 @@ + (Class)layerClass { } - (std::unique_ptr) - createSurface:(std::shared_ptr)onscreen_gl_context - resourceContext:(std::shared_ptr)resource_gl_context { + createSurfaceWithOnscreenContext:(fml::WeakPtr)onscreen_gl_context + resourceContext:(fml::WeakPtr)resource_gl_context { if ([self.layer isKindOfClass:[CAEAGLLayer class]]) { fml::scoped_nsobject eagl_layer( reinterpret_cast([self.layer retain])); diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index d43cf55b7a71d..6b533a07f95d5 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -361,16 +361,14 @@ views_to_recomposite_.clear(); } -bool FlutterPlatformViewsController::SubmitFrame( - GrContext* gr_context, - std::shared_ptr onscreen_gl_context, - std::shared_ptr resource_gl_context) { +bool FlutterPlatformViewsController::SubmitFrame(GrContext* gr_context, + fml::WeakPtr onscreen_gl_context, + fml::WeakPtr resource_gl_context) { DisposeViews(); bool did_submit = true; for (int64_t view_id : composition_order_) { - EnsureOverlayInitialized(view_id, std::move(onscreen_gl_context), - std::move(resource_gl_context), gr_context); + EnsureOverlayInitialized(view_id, onscreen_gl_context, resource_gl_context, gr_context); auto frame = overlays_[view_id]->surface->AcquireFrame(frame_size_); SkCanvas* canvas = frame->SkiaCanvas(); canvas->drawPicture(picture_recorders_[view_id]->finishRecordingAsPicture()); @@ -454,8 +452,8 @@ void FlutterPlatformViewsController::EnsureOverlayInitialized( int64_t overlay_id, - std::shared_ptr onscreen_gl_context, - std::shared_ptr resource_gl_context, + fml::WeakPtr onscreen_gl_context, + fml::WeakPtr resource_gl_context, GrContext* gr_context) { FML_DCHECK(flutter_view_); @@ -471,8 +469,8 @@ overlay_view.get().frame = flutter_view_.get().bounds; overlay_view.get().autoresizingMask = (UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight); - std::unique_ptr ios_surface = [overlay_view.get() createSurface:nil - resourceContext:nil]; + std::unique_ptr ios_surface = + [overlay_view.get() createSurfaceWithOnscreenContext:nil resourceContext:nil]; std::unique_ptr surface = ios_surface->CreateGPUSurface(); overlays_[overlay_id] = std::make_unique( std::move(overlay_view), std::move(ios_surface), std::move(surface)); @@ -497,8 +495,8 @@ overlay_view.get().autoresizingMask = (UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight); std::unique_ptr ios_surface = - [overlay_view.get() createSurface:std::move(onscreen_gl_context) - resourceContext:std::move(resource_gl_context)]; + [overlay_view.get() createSurfaceWithOnscreenContext:std::move(onscreen_gl_context) + resourceContext:std::move(resource_gl_context)]; std::unique_ptr surface = ios_surface->CreateGPUSurface(gr_context); overlays_[overlay_id] = std::make_unique( std::move(overlay_view), std::move(ios_surface), std::move(surface)); diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index e161b5b622ab4..181723fd80286 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -101,8 +101,8 @@ class FlutterPlatformViewsController { void Reset(); bool SubmitFrame(GrContext* gr_context, - std::shared_ptr onscreen_gl_context, - std::shared_ptr resource_gl_context); + fml::WeakPtr onscreen_gl_context, + fml::WeakPtr resource_gl_context); void OnMethodCall(FlutterMethodCall* call, FlutterResult& result); @@ -164,8 +164,8 @@ class FlutterPlatformViewsController { // Dispose the views in `views_to_dispose_`. void DisposeViews(); void EnsureOverlayInitialized(int64_t overlay_id, - std::shared_ptr onscreen_gl_context, - std::shared_ptr resource_gl_context, + fml::WeakPtr onscreen_gl_context, + fml::WeakPtr resource_gl_context, GrContext* gr_context); // This will return true after pre-roll if any of the embedded views diff --git a/shell/platform/darwin/ios/framework/Source/FlutterView.h b/shell/platform/darwin/ios/framework/Source/FlutterView.h index 5e7495a508a62..858b05f3110a0 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterView.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterView.h @@ -34,7 +34,7 @@ - (instancetype)initWithDelegate:(id)delegate opaque:(BOOL)opaque NS_DESIGNATED_INITIALIZER; - (std::unique_ptr)createSurface: - (std::shared_ptr)resourceContext; + (fml::WeakPtr)resourceContext; @end diff --git a/shell/platform/darwin/ios/framework/Source/FlutterView.mm b/shell/platform/darwin/ios/framework/Source/FlutterView.mm index 53348f05e1c95..cf3ee17b3a897 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterView.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterView.mm @@ -22,10 +22,10 @@ #include "flutter/shell/platform/darwin/ios/ios_surface_metal.h" #endif // FLUTTER_SHELL_ENABLE_METAL -@implementation FlutterView - -id _delegate; -std::shared_ptr _onscreenContext; +@implementation FlutterView { + id _delegate; + std::unique_ptr _onscreenContext; +} - (instancetype)init { @throw([NSException exceptionWithName:@"FlutterView must initWithDelegate" @@ -45,11 +45,6 @@ - (instancetype)initWithCoder:(NSCoder*)aDecoder { userInfo:nil]); } -- (void)dealloc { - _onscreenContext.reset(); - [super dealloc]; -} - - (instancetype)initWithDelegate:(id)delegate opaque:(BOOL)opaque { FML_DCHECK(delegate) << "Delegate must not be nil."; self = [super initWithFrame:CGRectNull]; @@ -101,7 +96,7 @@ + (Class)layerClass { } - (std::unique_ptr)createSurface: - (std::shared_ptr)resourceContext { + (fml::WeakPtr)resourceContext { #if !TARGET_IPHONE_SIMULATOR if (!_onscreenContext) { _onscreenContext = resourceContext->MakeSharedContext(); @@ -118,9 +113,9 @@ + (Class)layerClass { eagl_layer.get().presentsWithTransaction = YES; } } - return std::make_unique(_onscreenContext, std::move(resourceContext), - std::move(eagl_layer), - [_delegate platformViewsController]); + return std::make_unique( + _onscreenContext->WeakPtr(), std::move(resourceContext), std::move(eagl_layer), + [_delegate platformViewsController]); #if FLUTTER_SHELL_ENABLE_METAL } else if ([self.layer isKindOfClass:[CAMetalLayer class]]) { fml::scoped_nsobject metalLayer( diff --git a/shell/platform/darwin/ios/ios_gl_context.h b/shell/platform/darwin/ios/ios_gl_context.h index 50621623c6092..cd0b6e50c8403 100644 --- a/shell/platform/darwin/ios/ios_gl_context.h +++ b/shell/platform/darwin/ios/ios_gl_context.h @@ -29,12 +29,16 @@ class IOSGLContext { sk_sp ColorSpace() const { return color_space_; } - std::shared_ptr MakeSharedContext(); + std::unique_ptr MakeSharedContext(); + + fml::WeakPtr WeakPtr(); private: fml::scoped_nsobject context_; sk_sp color_space_; + std::unique_ptr> weak_factory_; + FML_DISALLOW_COPY_AND_ASSIGN(IOSGLContext); }; diff --git a/shell/platform/darwin/ios/ios_gl_context.mm b/shell/platform/darwin/ios/ios_gl_context.mm index bbd270835b473..c37cdcdb2afc5 100644 --- a/shell/platform/darwin/ios/ios_gl_context.mm +++ b/shell/platform/darwin/ios/ios_gl_context.mm @@ -14,7 +14,8 @@ IOSGLContext::IOSGLContext() : IOSGLContext(nullptr) {} -IOSGLContext::IOSGLContext(EAGLSharegroup* sharegroup) { +IOSGLContext::IOSGLContext(EAGLSharegroup* sharegroup) + : weak_factory_(std::make_unique>(this)) { context_.reset([[EAGLContext alloc] initWithAPI:kEAGLRenderingAPIOpenGLES3 sharegroup:sharegroup]); @@ -44,6 +45,10 @@ } } +fml::WeakPtr IOSGLContext::WeakPtr() { + return weak_factory_->GetWeakPtr(); +} + bool IOSGLContext::BindRenderbufferStorage(NSUInteger target, fml::scoped_nsobject layer) { return [context_.get() renderbufferStorage:GL_RENDERBUFFER fromDrawable:layer.get()]; @@ -55,8 +60,8 @@ return [EAGLContext setCurrentContext:context_.get()]; } -std::shared_ptr IOSGLContext::MakeSharedContext() { - return std::make_shared(context_.get().sharegroup); +std::unique_ptr IOSGLContext::MakeSharedContext() { + return std::make_unique(context_.get().sharegroup); } } // namespace flutter diff --git a/shell/platform/darwin/ios/ios_gl_render_target.h b/shell/platform/darwin/ios/ios_gl_render_target.h index 6145f7139d4a4..c189350d8b1d1 100644 --- a/shell/platform/darwin/ios/ios_gl_render_target.h +++ b/shell/platform/darwin/ios/ios_gl_render_target.h @@ -20,8 +20,8 @@ namespace flutter { class IOSGLRenderTarget { public: IOSGLRenderTarget(fml::scoped_nsobject layer, - std::shared_ptr onscreen_context, - std::shared_ptr resource_context); + fml::WeakPtr onscreen_context, + fml::WeakPtr resource_context); ~IOSGLRenderTarget(); @@ -41,8 +41,8 @@ class IOSGLRenderTarget { private: fml::scoped_nsobject layer_; - std::shared_ptr onscreen_context_; - std::shared_ptr resource_context_; + fml::WeakPtr onscreen_context_; + fml::WeakPtr resource_context_; GLuint framebuffer_; GLuint colorbuffer_; GLint storage_size_width_; diff --git a/shell/platform/darwin/ios/ios_gl_render_target.mm b/shell/platform/darwin/ios/ios_gl_render_target.mm index 9c8028aded221..e91b2e2f481d9 100644 --- a/shell/platform/darwin/ios/ios_gl_render_target.mm +++ b/shell/platform/darwin/ios/ios_gl_render_target.mm @@ -12,8 +12,8 @@ namespace flutter { IOSGLRenderTarget::IOSGLRenderTarget(fml::scoped_nsobject layer, - std::shared_ptr onscreen_context, - std::shared_ptr resource_context) + fml::WeakPtr onscreen_context, + fml::WeakPtr resource_context) : layer_(std::move(layer)), onscreen_context_(std::move(onscreen_context)), resource_context_(std::move(resource_context)), diff --git a/shell/platform/darwin/ios/ios_surface_gl.h b/shell/platform/darwin/ios/ios_surface_gl.h index e6170dea7ec20..ebd9159733fdf 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.h +++ b/shell/platform/darwin/ios/ios_surface_gl.h @@ -20,14 +20,14 @@ class IOSSurfaceGL final : public IOSSurface, public GPUSurfaceGLDelegate, public ExternalViewEmbedder { public: - IOSSurfaceGL(std::shared_ptr onscreen_context, - std::shared_ptr resource_context, + IOSSurfaceGL(fml::WeakPtr onscreen_context, + fml::WeakPtr resource_context, fml::scoped_nsobject layer, FlutterPlatformViewsController* platform_views_controller); IOSSurfaceGL(fml::scoped_nsobject layer, - std::shared_ptr onscreen_context, - std::shared_ptr resource_context); + fml::WeakPtr onscreen_context, + fml::WeakPtr resource_context); ~IOSSurfaceGL() override; @@ -82,9 +82,9 @@ class IOSSurfaceGL final : public IOSSurface, bool SubmitFrame(GrContext* context) override; private: - std::shared_ptr onscreen_context_; + fml::WeakPtr onscreen_context_; - std::shared_ptr resource_context_; + fml::WeakPtr resource_context_; 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 95c5ff7bd3e47..addf991033a2c 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.mm +++ b/shell/platform/darwin/ios/ios_surface_gl.mm @@ -9,8 +9,8 @@ namespace flutter { -IOSSurfaceGL::IOSSurfaceGL(std::shared_ptr onscreen_context, - std::shared_ptr resource_context, +IOSSurfaceGL::IOSSurfaceGL(fml::WeakPtr onscreen_context, + fml::WeakPtr resource_context, fml::scoped_nsobject layer, FlutterPlatformViewsController* platform_views_controller) : IOSSurface(platform_views_controller), @@ -21,8 +21,8 @@ } IOSSurfaceGL::IOSSurfaceGL(fml::scoped_nsobject layer, - std::shared_ptr onscreen_context, - std::shared_ptr resource_context) + fml::WeakPtr onscreen_context, + fml::WeakPtr resource_context) : IOSSurface(nullptr), onscreen_context_(std::move(onscreen_context)), resource_context_(std::move(resource_context)) { diff --git a/shell/platform/darwin/ios/platform_view_ios.h b/shell/platform/darwin/ios/platform_view_ios.h index f3bca12eb9c01..14e2cf6ab999e 100644 --- a/shell/platform/darwin/ios/platform_view_ios.h +++ b/shell/platform/darwin/ios/platform_view_ios.h @@ -47,7 +47,7 @@ class PlatformViewIOS final : public PlatformView { private: fml::WeakPtr owner_controller_; std::unique_ptr ios_surface_; - std::shared_ptr gl_resource_context_; + std::unique_ptr gl_resource_context_; PlatformMessageRouter platform_message_router_; std::unique_ptr accessibility_bridge_; fml::scoped_nsprotocol text_input_plugin_; diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index bc375eaf8d1a1..cfb7410dc208a 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -23,7 +23,7 @@ flutter::TaskRunners task_runners) : PlatformView(delegate, std::move(task_runners)) { #if !TARGET_IPHONE_SIMULATOR - gl_resource_context_ = std::make_shared(); + gl_resource_context_ = std::make_unique(); #endif // !TARGET_IPHONE_SIMULATOR } @@ -53,8 +53,8 @@ } owner_controller_ = owner_controller; if (owner_controller_) { - ios_surface_ = - [static_cast(owner_controller.get().view) createSurface:gl_resource_context_]; + ios_surface_ = [static_cast(owner_controller.get().view) + createSurface:gl_resource_context_->WeakPtr()]; FML_DCHECK(ios_surface_ != nullptr); if (accessibility_bridge_) { From 531e86a263be77d6d3453cde0cf796a829b640f2 Mon Sep 17 00:00:00 2001 From: George Wright Date: Thu, 5 Sep 2019 10:58:49 -0700 Subject: [PATCH 08/17] WeakPtr -> GetWeakPtr --- shell/platform/darwin/ios/framework/Source/FlutterView.mm | 2 +- shell/platform/darwin/ios/ios_gl_context.h | 2 +- shell/platform/darwin/ios/ios_gl_context.mm | 2 +- shell/platform/darwin/ios/platform_view_ios.mm | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterView.mm b/shell/platform/darwin/ios/framework/Source/FlutterView.mm index cf3ee17b3a897..a55fd982a9fea 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterView.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterView.mm @@ -114,7 +114,7 @@ + (Class)layerClass { } } return std::make_unique( - _onscreenContext->WeakPtr(), std::move(resourceContext), std::move(eagl_layer), + _onscreenContext->GetWeakPtr(), std::move(resourceContext), std::move(eagl_layer), [_delegate platformViewsController]); #if FLUTTER_SHELL_ENABLE_METAL } else if ([self.layer isKindOfClass:[CAMetalLayer class]]) { diff --git a/shell/platform/darwin/ios/ios_gl_context.h b/shell/platform/darwin/ios/ios_gl_context.h index cd0b6e50c8403..6e48620a3ea85 100644 --- a/shell/platform/darwin/ios/ios_gl_context.h +++ b/shell/platform/darwin/ios/ios_gl_context.h @@ -31,7 +31,7 @@ class IOSGLContext { std::unique_ptr MakeSharedContext(); - fml::WeakPtr WeakPtr(); + fml::WeakPtr GetWeakPtr(); private: fml::scoped_nsobject context_; diff --git a/shell/platform/darwin/ios/ios_gl_context.mm b/shell/platform/darwin/ios/ios_gl_context.mm index c37cdcdb2afc5..011b48f4647eb 100644 --- a/shell/platform/darwin/ios/ios_gl_context.mm +++ b/shell/platform/darwin/ios/ios_gl_context.mm @@ -45,7 +45,7 @@ } } -fml::WeakPtr IOSGLContext::WeakPtr() { +fml::WeakPtr IOSGLContext::GetWeakPtr() { return weak_factory_->GetWeakPtr(); } diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index cfb7410dc208a..0f5bdd8e14164 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -54,7 +54,7 @@ owner_controller_ = owner_controller; if (owner_controller_) { ios_surface_ = [static_cast(owner_controller.get().view) - createSurface:gl_resource_context_->WeakPtr()]; + createSurface:gl_resource_context_->GetWeakPtr()]; FML_DCHECK(ios_surface_ != nullptr); if (accessibility_bridge_) { From 1fee1600be8820f300227118ab098e1f21fb44ad Mon Sep 17 00:00:00 2001 From: George Wright Date: Thu, 5 Sep 2019 11:04:32 -0700 Subject: [PATCH 09/17] get rid of nullptr_t ctor and explicitly construct an empty WeakRef instead, as per existing convention --- fml/memory/weak_ptr.h | 2 -- .../darwin/ios/framework/Source/FlutterPlatformViews.mm | 3 ++- shell/platform/darwin/ios/ios_surface_software.mm | 3 ++- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/fml/memory/weak_ptr.h b/fml/memory/weak_ptr.h index 907139e10aae6..09cc10a116b35 100644 --- a/fml/memory/weak_ptr.h +++ b/fml/memory/weak_ptr.h @@ -45,8 +45,6 @@ class WeakPtr { // Copy constructor. WeakPtr(const WeakPtr& r) = default; - WeakPtr(std::nullptr_t r) : ptr_(r) {} - template WeakPtr(const WeakPtr& r) : ptr_(static_cast(r.ptr_)), flag_(r.flag_), checker_(r.checker_) {} diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 6b533a07f95d5..56daa7f8fe1df 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -470,7 +470,8 @@ overlay_view.get().autoresizingMask = (UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight); std::unique_ptr ios_surface = - [overlay_view.get() createSurfaceWithOnscreenContext:nil resourceContext:nil]; + [overlay_view.get() createSurfaceWithOnscreenContext:fml::WeakPtr() + resourceContext:fml::WeakPtr()]; std::unique_ptr surface = ios_surface->CreateGPUSurface(); overlays_[overlay_id] = std::make_unique( std::move(overlay_view), std::move(ios_surface), std::move(surface)); diff --git a/shell/platform/darwin/ios/ios_surface_software.mm b/shell/platform/darwin/ios/ios_surface_software.mm index 10595359c650c..253c8645d2308 100644 --- a/shell/platform/darwin/ios/ios_surface_software.mm +++ b/shell/platform/darwin/ios/ios_surface_software.mm @@ -184,7 +184,8 @@ if (platform_views_controller == nullptr) { return true; } - return platform_views_controller->SubmitFrame(nullptr, nullptr, nullptr); + return platform_views_controller->SubmitFrame(nullptr, fml::WeakPtr(), + fml::WeakPtr()); } } // namespace flutter From 30ec6c865b1e2c509048c4c51be2adaa6739fa6c Mon Sep 17 00:00:00 2001 From: George Wright Date: Thu, 5 Sep 2019 13:29:45 -0700 Subject: [PATCH 10/17] review comments - minor style updates and remove errant std::move --- .../ios/framework/Source/FlutterOverlayView.h | 4 ++-- .../framework/Source/FlutterOverlayView.mm | 8 +++---- .../framework/Source/FlutterPlatformViews.mm | 8 +++---- .../darwin/ios/framework/Source/FlutterView.h | 2 +- .../ios/framework/Source/FlutterView.mm | 10 ++++---- shell/platform/darwin/ios/ios_gl_context.h | 2 +- shell/platform/darwin/ios/ios_gl_context.mm | 3 +-- .../darwin/ios/ios_gl_render_target.h | 4 ++-- .../darwin/ios/ios_gl_render_target.mm | 24 +++++++++---------- 9 files changed, 32 insertions(+), 33 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h b/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h index 85d11efc7cb10..903b9c02ca9a6 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h @@ -25,8 +25,8 @@ - (instancetype)init NS_DESIGNATED_INITIALIZER; - (instancetype)initWithContentsScale:(CGFloat)contentsScale; - (std::unique_ptr) - createSurfaceWithOnscreenContext:(fml::WeakPtr)onscreen_gl_context - resourceContext:(fml::WeakPtr)resource_gl_context; + createSurfaceWithOnscreenGLContext:(fml::WeakPtr)onscreenGLContext + resourceGLContext:(fml::WeakPtr)resourceGLContext; @end diff --git a/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm b/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm index ec14f06b1331b..9cdf862c8c078 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm @@ -79,16 +79,16 @@ + (Class)layerClass { } - (std::unique_ptr) - createSurfaceWithOnscreenContext:(fml::WeakPtr)onscreen_gl_context - resourceContext:(fml::WeakPtr)resource_gl_context { + createSurfaceWithOnscreenGLContext:(fml::WeakPtr)onscreenGLContext + resourceGLContext:(fml::WeakPtr)resourceGLContext { if ([self.layer isKindOfClass:[CAEAGLLayer class]]) { fml::scoped_nsobject eagl_layer( reinterpret_cast([self.layer retain])); if (@available(iOS 9.0, *)) { eagl_layer.get().presentsWithTransaction = YES; } - return std::make_unique(std::move(eagl_layer), onscreen_gl_context, - resource_gl_context); + return std::make_unique(std::move(eagl_layer), onscreenGLContext, + resourceGLContext); #if FLUTTER_SHELL_ENABLE_METAL } else if ([self.layer isKindOfClass:[CAMetalLayer class]]) { fml::scoped_nsobject metalLayer( diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 56daa7f8fe1df..db706afefe5bf 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -470,8 +470,8 @@ overlay_view.get().autoresizingMask = (UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight); std::unique_ptr ios_surface = - [overlay_view.get() createSurfaceWithOnscreenContext:fml::WeakPtr() - resourceContext:fml::WeakPtr()]; + [overlay_view.get() createSurfaceWithOnscreenGLContext:fml::WeakPtr() + resourceGLContext:fml::WeakPtr()]; std::unique_ptr surface = ios_surface->CreateGPUSurface(); overlays_[overlay_id] = std::make_unique( std::move(overlay_view), std::move(ios_surface), std::move(surface)); @@ -496,8 +496,8 @@ overlay_view.get().autoresizingMask = (UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight); std::unique_ptr ios_surface = - [overlay_view.get() createSurfaceWithOnscreenContext:std::move(onscreen_gl_context) - resourceContext:std::move(resource_gl_context)]; + [overlay_view.get() createSurfaceWithOnscreenGLContext:std::move(onscreen_gl_context) + resourceGLContext:std::move(resource_gl_context)]; std::unique_ptr surface = ios_surface->CreateGPUSurface(gr_context); overlays_[overlay_id] = std::make_unique( std::move(overlay_view), std::move(ios_surface), std::move(surface)); diff --git a/shell/platform/darwin/ios/framework/Source/FlutterView.h b/shell/platform/darwin/ios/framework/Source/FlutterView.h index 858b05f3110a0..c92ac9fc01fe0 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterView.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterView.h @@ -34,7 +34,7 @@ - (instancetype)initWithDelegate:(id)delegate opaque:(BOOL)opaque NS_DESIGNATED_INITIALIZER; - (std::unique_ptr)createSurface: - (fml::WeakPtr)resourceContext; + (fml::WeakPtr)resourceGLContext; @end diff --git a/shell/platform/darwin/ios/framework/Source/FlutterView.mm b/shell/platform/darwin/ios/framework/Source/FlutterView.mm index a55fd982a9fea..c5e82c5a5b0bb 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterView.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterView.mm @@ -24,7 +24,7 @@ @implementation FlutterView { id _delegate; - std::unique_ptr _onscreenContext; + std::unique_ptr _onscreenGLContext; } - (instancetype)init { @@ -96,10 +96,10 @@ + (Class)layerClass { } - (std::unique_ptr)createSurface: - (fml::WeakPtr)resourceContext { + (fml::WeakPtr)resourceGLContext { #if !TARGET_IPHONE_SIMULATOR - if (!_onscreenContext) { - _onscreenContext = resourceContext->MakeSharedContext(); + if (!_onscreenGLContext) { + _onscreenGLContext = resourceGLContext->MakeSharedContext(); } #endif // !TARGET_IPHONE_SIMULATOR @@ -114,7 +114,7 @@ + (Class)layerClass { } } return std::make_unique( - _onscreenContext->GetWeakPtr(), std::move(resourceContext), std::move(eagl_layer), + _onscreenGLContext->GetWeakPtr(), std::move(resourceGLContext), std::move(eagl_layer), [_delegate platformViewsController]); #if FLUTTER_SHELL_ENABLE_METAL } else if ([self.layer isKindOfClass:[CAMetalLayer class]]) { diff --git a/shell/platform/darwin/ios/ios_gl_context.h b/shell/platform/darwin/ios/ios_gl_context.h index 6e48620a3ea85..1f1bebcfb85a3 100644 --- a/shell/platform/darwin/ios/ios_gl_context.h +++ b/shell/platform/darwin/ios/ios_gl_context.h @@ -25,7 +25,7 @@ class IOSGLContext { bool MakeCurrent(); - bool BindRenderbufferStorage(NSUInteger target, fml::scoped_nsobject layer); + bool BindRenderbufferStorage(fml::scoped_nsobject layer); sk_sp ColorSpace() const { return color_space_; } diff --git a/shell/platform/darwin/ios/ios_gl_context.mm b/shell/platform/darwin/ios/ios_gl_context.mm index 011b48f4647eb..8398ce0a5efd6 100644 --- a/shell/platform/darwin/ios/ios_gl_context.mm +++ b/shell/platform/darwin/ios/ios_gl_context.mm @@ -49,8 +49,7 @@ return weak_factory_->GetWeakPtr(); } -bool IOSGLContext::BindRenderbufferStorage(NSUInteger target, - fml::scoped_nsobject layer) { +bool IOSGLContext::BindRenderbufferStorage(fml::scoped_nsobject layer) { return [context_.get() renderbufferStorage:GL_RENDERBUFFER fromDrawable:layer.get()]; } diff --git a/shell/platform/darwin/ios/ios_gl_render_target.h b/shell/platform/darwin/ios/ios_gl_render_target.h index c189350d8b1d1..b26713e8ea707 100644 --- a/shell/platform/darwin/ios/ios_gl_render_target.h +++ b/shell/platform/darwin/ios/ios_gl_render_target.h @@ -41,8 +41,8 @@ class IOSGLRenderTarget { private: fml::scoped_nsobject layer_; - fml::WeakPtr onscreen_context_; - fml::WeakPtr resource_context_; + fml::WeakPtr onscreen_gl_context_; + fml::WeakPtr resource_gl_context_; GLuint framebuffer_; GLuint colorbuffer_; GLint storage_size_width_; diff --git a/shell/platform/darwin/ios/ios_gl_render_target.mm b/shell/platform/darwin/ios/ios_gl_render_target.mm index e91b2e2f481d9..e0ce5e7bf8292 100644 --- a/shell/platform/darwin/ios/ios_gl_render_target.mm +++ b/shell/platform/darwin/ios/ios_gl_render_target.mm @@ -12,21 +12,21 @@ namespace flutter { IOSGLRenderTarget::IOSGLRenderTarget(fml::scoped_nsobject layer, - fml::WeakPtr onscreen_context, - fml::WeakPtr resource_context) + fml::WeakPtr onscreen_gl_context, + fml::WeakPtr resource_gl_context) : layer_(std::move(layer)), - onscreen_context_(std::move(onscreen_context)), - resource_context_(std::move(resource_context)), + onscreen_gl_context_(std::move(onscreen_gl_context)), + resource_gl_context_(std::move(resource_gl_context)), framebuffer_(GL_NONE), colorbuffer_(GL_NONE), storage_size_width_(0), storage_size_height_(0), valid_(false) { FML_DCHECK(layer_ != nullptr); - FML_DCHECK(onscreen_context_); - FML_DCHECK(resource_context_); + FML_DCHECK(onscreen_gl_context_); + FML_DCHECK(resource_gl_context_); - bool context_current = onscreen_context_->MakeCurrent(); + bool context_current = onscreen_gl_context_->MakeCurrent(); FML_DCHECK(context_current); FML_DCHECK(glGetError() == GL_NO_ERROR); @@ -62,7 +62,7 @@ IOSGLRenderTarget::~IOSGLRenderTarget() { EAGLContext* context = EAGLContext.currentContext; - onscreen_context_->MakeCurrent(); + onscreen_gl_context_->MakeCurrent(); FML_DCHECK(glGetError() == GL_NO_ERROR); // Deletes on GL_NONEs are ignored @@ -104,7 +104,7 @@ FML_DCHECK(glGetError() == GL_NO_ERROR); - if (!onscreen_context_->MakeCurrent()) { + if (!onscreen_gl_context_->MakeCurrent()) { return false; } @@ -115,7 +115,7 @@ glBindRenderbuffer(GL_RENDERBUFFER, colorbuffer_); FML_DCHECK(glGetError() == GL_NO_ERROR); - if (!onscreen_context_->BindRenderbufferStorage(GL_RENDERBUFFER, std::move(layer_))) { + if (!onscreen_gl_context_->BindRenderbufferStorage(layer_)) { return false; } @@ -132,11 +132,11 @@ } bool IOSGLRenderTarget::MakeCurrent() { - return UpdateStorageSizeIfNecessary() && onscreen_context_->MakeCurrent(); + return UpdateStorageSizeIfNecessary() && onscreen_gl_context_->MakeCurrent(); } bool IOSGLRenderTarget::ResourceMakeCurrent() { - return resource_context_->MakeCurrent(); + return resource_gl_context_->MakeCurrent(); } } // namespace flutter From 7f8ca9cc8f31378bf41736cb6c2977a4b3c09042 Mon Sep 17 00:00:00 2001 From: George Wright Date: Thu, 5 Sep 2019 13:36:23 -0700 Subject: [PATCH 11/17] more nits --- .../framework/Source/FlutterPlatformViews.mm | 6 ++-- shell/platform/darwin/ios/ios_surface_gl.h | 12 +++---- shell/platform/darwin/ios/ios_surface_gl.mm | 32 +++++++++---------- shell/platform/darwin/ios/platform_view_ios.h | 2 +- .../platform/darwin/ios/platform_view_ios.mm | 6 ++-- 5 files changed, 29 insertions(+), 29 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index db706afefe5bf..7cc3c44c04ff0 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -469,9 +469,9 @@ overlay_view.get().frame = flutter_view_.get().bounds; overlay_view.get().autoresizingMask = (UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight); - std::unique_ptr ios_surface = - [overlay_view.get() createSurfaceWithOnscreenGLContext:fml::WeakPtr() - resourceGLContext:fml::WeakPtr()]; + std::unique_ptr ios_surface = [overlay_view.get() + createSurfaceWithOnscreenGLContext:fml::WeakPtr() + resourceGLContext:fml::WeakPtr()]; std::unique_ptr surface = ios_surface->CreateGPUSurface(); overlays_[overlay_id] = std::make_unique( std::move(overlay_view), std::move(ios_surface), std::move(surface)); diff --git a/shell/platform/darwin/ios/ios_surface_gl.h b/shell/platform/darwin/ios/ios_surface_gl.h index ebd9159733fdf..7dc815dbba879 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.h +++ b/shell/platform/darwin/ios/ios_surface_gl.h @@ -20,14 +20,14 @@ class IOSSurfaceGL final : public IOSSurface, public GPUSurfaceGLDelegate, public ExternalViewEmbedder { public: - IOSSurfaceGL(fml::WeakPtr onscreen_context, - fml::WeakPtr resource_context, + IOSSurfaceGL(fml::WeakPtr onscreen_gl_context, + fml::WeakPtr resource_gl_context, fml::scoped_nsobject layer, FlutterPlatformViewsController* platform_views_controller); IOSSurfaceGL(fml::scoped_nsobject layer, - fml::WeakPtr onscreen_context, - fml::WeakPtr resource_context); + fml::WeakPtr onscreen_gl_context, + fml::WeakPtr resource_gl_context); ~IOSSurfaceGL() override; @@ -82,9 +82,9 @@ class IOSSurfaceGL final : public IOSSurface, bool SubmitFrame(GrContext* context) override; private: - fml::WeakPtr onscreen_context_; + fml::WeakPtr onscreen_gl_context_; - fml::WeakPtr resource_context_; + fml::WeakPtr resource_gl_context_; 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 addf991033a2c..b278e8170cd89 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.mm +++ b/shell/platform/darwin/ios/ios_surface_gl.mm @@ -9,25 +9,25 @@ namespace flutter { -IOSSurfaceGL::IOSSurfaceGL(fml::WeakPtr onscreen_context, - fml::WeakPtr resource_context, +IOSSurfaceGL::IOSSurfaceGL(fml::WeakPtr onscreen_gl_context, + fml::WeakPtr resource_gl_context, fml::scoped_nsobject layer, FlutterPlatformViewsController* platform_views_controller) : IOSSurface(platform_views_controller), - onscreen_context_(std::move(onscreen_context)), - resource_context_(std::move(resource_context)) { - render_target_ = - std::make_unique(std::move(layer), onscreen_context_, resource_context_); + onscreen_gl_context_(std::move(onscreen_gl_context)), + resource_gl_context_(std::move(resource_gl_context)) { + render_target_ = std::make_unique(std::move(layer), onscreen_gl_context_, + resource_gl_context_); } IOSSurfaceGL::IOSSurfaceGL(fml::scoped_nsobject layer, - fml::WeakPtr onscreen_context, - fml::WeakPtr resource_context) + fml::WeakPtr onscreen_gl_context, + fml::WeakPtr resource_gl_context) : IOSSurface(nullptr), - onscreen_context_(std::move(onscreen_context)), - resource_context_(std::move(resource_context)) { - render_target_ = - std::make_unique(std::move(layer), onscreen_context_, resource_context_); + onscreen_gl_context_(std::move(onscreen_gl_context)), + resource_gl_context_(std::move(resource_gl_context)) { + render_target_ = std::make_unique(std::move(layer), onscreen_gl_context_, + resource_gl_context_); } IOSSurfaceGL::~IOSSurfaceGL() = default; @@ -37,7 +37,7 @@ } bool IOSSurfaceGL::ResourceContextMakeCurrent() { - return resource_context_->MakeCurrent(); + return resource_gl_context_->MakeCurrent(); } void IOSSurfaceGL::UpdateStorageSizeIfNecessary() { @@ -68,7 +68,7 @@ if (!IsValid()) { return false; } - return render_target_->UpdateStorageSizeIfNecessary() && onscreen_context_->MakeCurrent(); + return render_target_->UpdateStorageSizeIfNecessary() && onscreen_gl_context_->MakeCurrent(); } bool IOSSurfaceGL::GLContextClearCurrent() { @@ -153,8 +153,8 @@ return true; } - bool submitted = platform_views_controller->SubmitFrame(std::move(context), onscreen_context_, - resource_context_); + bool submitted = platform_views_controller->SubmitFrame(std::move(context), onscreen_gl_context_, + resource_gl_context_); [CATransaction commit]; return submitted; } diff --git a/shell/platform/darwin/ios/platform_view_ios.h b/shell/platform/darwin/ios/platform_view_ios.h index 14e2cf6ab999e..064e57c754f66 100644 --- a/shell/platform/darwin/ios/platform_view_ios.h +++ b/shell/platform/darwin/ios/platform_view_ios.h @@ -47,7 +47,7 @@ class PlatformViewIOS final : public PlatformView { private: fml::WeakPtr owner_controller_; std::unique_ptr ios_surface_; - std::unique_ptr gl_resource_context_; + std::unique_ptr resource_gl_context_; PlatformMessageRouter platform_message_router_; std::unique_ptr accessibility_bridge_; fml::scoped_nsprotocol text_input_plugin_; diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index 0f5bdd8e14164..aca2df5723722 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -23,7 +23,7 @@ flutter::TaskRunners task_runners) : PlatformView(delegate, std::move(task_runners)) { #if !TARGET_IPHONE_SIMULATOR - gl_resource_context_ = std::make_unique(); + resource_gl_context_ = std::make_unique(); #endif // !TARGET_IPHONE_SIMULATOR } @@ -54,7 +54,7 @@ owner_controller_ = owner_controller; if (owner_controller_) { ios_surface_ = [static_cast(owner_controller.get().view) - createSurface:gl_resource_context_->GetWeakPtr()]; + createSurface:resource_gl_context_->GetWeakPtr()]; FML_DCHECK(ios_surface_ != nullptr); if (accessibility_bridge_) { @@ -86,7 +86,7 @@ new AccessibilityBridge(static_cast(owner_controller_.get().view), // |PlatformView| sk_sp PlatformViewIOS::CreateResourceContext() const { - if (!gl_resource_context_ || !gl_resource_context_->MakeCurrent()) { + if (!resource_gl_context_ || !resource_gl_context_->MakeCurrent()) { FML_DLOG(INFO) << "Could not make resource context current on IO thread. " "Async texture uploads will be disabled. On Simulators, " "this is expected."; From a897c51fe94c8a52676c88470ff4237d684b8ef4 Mon Sep 17 00:00:00 2001 From: George Wright Date: Thu, 5 Sep 2019 14:06:53 -0700 Subject: [PATCH 12/17] objc naming update --- shell/platform/darwin/ios/framework/Source/FlutterView.h | 2 +- shell/platform/darwin/ios/framework/Source/FlutterView.mm | 2 +- shell/platform/darwin/ios/platform_view_ios.mm | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterView.h b/shell/platform/darwin/ios/framework/Source/FlutterView.h index c92ac9fc01fe0..ad75d817f6800 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterView.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterView.h @@ -33,7 +33,7 @@ - (instancetype)initWithDelegate:(id)delegate opaque:(BOOL)opaque NS_DESIGNATED_INITIALIZER; -- (std::unique_ptr)createSurface: +- (std::unique_ptr)createSurfaceWithResourceGLContext: (fml::WeakPtr)resourceGLContext; @end diff --git a/shell/platform/darwin/ios/framework/Source/FlutterView.mm b/shell/platform/darwin/ios/framework/Source/FlutterView.mm index c5e82c5a5b0bb..8891648da52f4 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterView.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterView.mm @@ -95,7 +95,7 @@ + (Class)layerClass { #endif // TARGET_IPHONE_SIMULATOR } -- (std::unique_ptr)createSurface: +- (std::unique_ptr)createSurfaceWithResourceGLContext: (fml::WeakPtr)resourceGLContext { #if !TARGET_IPHONE_SIMULATOR if (!_onscreenGLContext) { diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index aca2df5723722..d6f17f6fb6c94 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -54,7 +54,7 @@ owner_controller_ = owner_controller; if (owner_controller_) { ios_surface_ = [static_cast(owner_controller.get().view) - createSurface:resource_gl_context_->GetWeakPtr()]; + createSurfaceWithResourceGLContext:resource_gl_context_->GetWeakPtr()]; FML_DCHECK(ios_surface_ != nullptr); if (accessibility_bridge_) { From 230a40c58b7ca9995acfca2c4b1405088604003e Mon Sep 17 00:00:00 2001 From: George Wright Date: Fri, 6 Sep 2019 15:11:58 -0700 Subject: [PATCH 13/17] unit test --- .../Scenarios.xcodeproj/project.pbxproj | 4 ++ .../ScenariosTests/FlutterViewTest.m | 58 +++++++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m diff --git a/testing/scenario_app/ios/Scenarios/Scenarios.xcodeproj/project.pbxproj b/testing/scenario_app/ios/Scenarios/Scenarios.xcodeproj/project.pbxproj index c7fe99847ea68..bf19f4e482c9e 100644 --- a/testing/scenario_app/ios/Scenarios/Scenarios.xcodeproj/project.pbxproj +++ b/testing/scenario_app/ios/Scenarios/Scenarios.xcodeproj/project.pbxproj @@ -27,6 +27,7 @@ 24D47D1B230C79840069DD5E /* golden_platform_view_D211AP.png in Resources */ = {isa = PBXBuildFile; fileRef = 24D47D1A230C79840069DD5E /* golden_platform_view_D211AP.png */; }; 24D47D1D230CA2700069DD5E /* golden_platform_view_iPhone SE_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 24D47D1C230CA2700069DD5E /* golden_platform_view_iPhone SE_simulator.png */; }; 24F1FB89230B4579005ACE7C /* TextPlatformView.m in Sources */ = {isa = PBXBuildFile; fileRef = 24F1FB87230B4579005ACE7C /* TextPlatformView.m */; }; + D678B7C82322EFDA00792FDC /* FlutterViewTest.m in Sources */ = {isa = PBXBuildFile; fileRef = D678B7C72322EFDA00792FDC /* FlutterViewTest.m */; settings = {COMPILER_FLAGS = "-fno-objc-arc"; }; }; /* End PBXBuildFile section */ /* Begin PBXContainerItemProxy section */ @@ -105,6 +106,7 @@ 24D47D1E230CA4480069DD5E /* README.md */ = {isa = PBXFileReference; lastKnownFileType = net.daringfireball.markdown; path = README.md; sourceTree = ""; }; 24F1FB87230B4579005ACE7C /* TextPlatformView.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = TextPlatformView.m; sourceTree = ""; }; 24F1FB88230B4579005ACE7C /* TextPlatformView.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TextPlatformView.h; sourceTree = ""; }; + D678B7C72322EFDA00792FDC /* FlutterViewTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = FlutterViewTest.m; sourceTree = ""; }; /* End PBXFileReference section */ /* Begin PBXFrameworksBuildPhase section */ @@ -176,6 +178,7 @@ children = ( 248FDFC322FE7CD0009CC7CD /* FlutterEngineTest.m */, 0DB781FC22EA2C0300E9B371 /* FlutterViewControllerTest.m */, + D678B7C72322EFDA00792FDC /* FlutterViewTest.m */, 248D76E522E388380012F0C1 /* Info.plist */, ); path = ScenariosTests; @@ -349,6 +352,7 @@ files = ( 0DB7820222EA493B00E9B371 /* FlutterViewControllerTest.m in Sources */, 248FDFC422FE7CD0009CC7CD /* FlutterEngineTest.m in Sources */, + D678B7C82322EFDA00792FDC /* FlutterViewTest.m in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m b/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m new file mode 100644 index 0000000000000..27f3c944de8b1 --- /dev/null +++ b/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m @@ -0,0 +1,58 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#import +#import +#import "AppDelegate.h" + +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wundeclared-selector" + +@interface FlutterViewGLContextTest : XCTestCase +@property(nonatomic, strong) FlutterViewController* flutterViewController; +@end + +@implementation FlutterViewGLContextTest + +- (void)setUp { + [super setUp]; + self.continueAfterFailure = NO; +} + +- (void)tearDown { + if (self.flutterViewController) { + [self.flutterViewController removeFromParentViewController]; + } + [super tearDown]; +} + +// Ideally we want to also test that the IOSurface backed by a FlutterView is +// destroyed as well. Unfortunately due to test harness limitations we can't +// check that without adding test-only APIs to get access to internal objects. +// +// TODO: Test for the IOSurface destruction as well. +- (void)testFlutterViewDestroyed { + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"testGL" project:nil]; + [engine runWithEntrypoint:nil]; + self.flutterViewController = [[FlutterViewController alloc] initWithEngine:engine + nibName:nil + bundle:nil]; + + AppDelegate* appDelegate = (AppDelegate*)UIApplication.sharedApplication.delegate; + UIViewController* rootVC = appDelegate.window.rootViewController; + [rootVC presentViewController:self.flutterViewController animated:NO completion:nil]; + + __weak id flutterView = [self.flutterViewController performSelector:NSSelectorFromString(@"flutterView")]; + XCTAssertNotNil(flutterView); + + [self.flutterViewController dismissViewControllerAnimated:NO completion:^{ + __weak id flutterView = [self.flutterViewController performSelector:NSSelectorFromString(@"flutterView")]; + XCTAssertNil(flutterView); + }]; +} + +@end + +#pragma clang diagnostic pop + From 73a7c9a90544e839b41d757b1fa9744c668d2370 Mon Sep 17 00:00:00 2001 From: George Wright Date: Fri, 6 Sep 2019 15:41:58 -0700 Subject: [PATCH 14/17] unit tests with surface checking --- .../ios/framework/Source/FlutterEngine_Internal.h | 2 ++ .../ios/framework/Source/FlutterViewController.mm | 4 ++++ shell/platform/darwin/ios/platform_view_ios.h | 2 ++ shell/platform/darwin/ios/platform_view_ios.mm | 5 +++++ .../ios/Scenarios/ScenariosTests/FlutterViewTest.m | 10 ++++------ 5 files changed, 17 insertions(+), 6 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h index 868d2176a309b..82d5fcc1d3a2e 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h @@ -36,6 +36,8 @@ - (fml::WeakPtr)platformView; +- (flutter::PlatformViewIOS*)iosPlatformView; + - (flutter::Rasterizer::Screenshot)screenshot:(flutter::Rasterizer::ScreenshotType)type base64Encode:(bool)base64Encode; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 3cea602f86bad..2e82f68e6e7a1 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -1152,4 +1152,8 @@ - (NSObject*)valuePublishedByPlugin:(NSString*)pluginKey { return [_engine.get() valuePublishedByPlugin:pluginKey]; } +- (BOOL)hasOnscreenSurface { + return [_engine.get() iosPlatformView] -> HasOnscreenSurface(); +} + @end diff --git a/shell/platform/darwin/ios/platform_view_ios.h b/shell/platform/darwin/ios/platform_view_ios.h index 064e57c754f66..f9461719ba329 100644 --- a/shell/platform/darwin/ios/platform_view_ios.h +++ b/shell/platform/darwin/ios/platform_view_ios.h @@ -44,6 +44,8 @@ class PlatformViewIOS final : public PlatformView { // |PlatformView| void SetSemanticsEnabled(bool enabled) override; + bool HasOnscreenSurface() const; + private: fml::WeakPtr owner_controller_; std::unique_ptr ios_surface_; diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index d6f17f6fb6c94..34407de9aac30 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -46,6 +46,11 @@ PlatformView::NotifyDestroyed(); ios_surface_.reset(); } + +bool PlatformViewIOS::HasOnscreenSurface() const { + return !!ios_surface_; +} + void PlatformViewIOS::SetOwnerViewController(fml::WeakPtr owner_controller) { if (ios_surface_ || !owner_controller) { NotifyDestroyed(); diff --git a/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m b/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m index 27f3c944de8b1..516f9e6d1fb0a 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m @@ -27,11 +27,6 @@ - (void)tearDown { [super tearDown]; } -// Ideally we want to also test that the IOSurface backed by a FlutterView is -// destroyed as well. Unfortunately due to test harness limitations we can't -// check that without adding test-only APIs to get access to internal objects. -// -// TODO: Test for the IOSurface destruction as well. - (void)testFlutterViewDestroyed { FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"testGL" project:nil]; [engine runWithEntrypoint:nil]; @@ -43,12 +38,15 @@ - (void)testFlutterViewDestroyed { UIViewController* rootVC = appDelegate.window.rootViewController; [rootVC presentViewController:self.flutterViewController animated:NO completion:nil]; + // TODO: refactor this to not rely on private test-only APIs __weak id flutterView = [self.flutterViewController performSelector:NSSelectorFromString(@"flutterView")]; XCTAssertNotNil(flutterView); - + XCTAssertTrue([self.flutterViewController performSelector:NSSelectorFromString(@"hasOnscreenSurface")]); + [self.flutterViewController dismissViewControllerAnimated:NO completion:^{ __weak id flutterView = [self.flutterViewController performSelector:NSSelectorFromString(@"flutterView")]; XCTAssertNil(flutterView); + XCTAssertFalse([self.flutterViewController performSelector:NSSelectorFromString(@"hasOnscreenSurface")]); }]; } From 89fdd6bff7e1097293e6fd9878d316b1057db8a0 Mon Sep 17 00:00:00 2001 From: George Wright Date: Fri, 6 Sep 2019 15:45:56 -0700 Subject: [PATCH 15/17] style updates --- .../framework/Source/FlutterViewController.mm | 2 +- .../ScenariosTests/FlutterViewTest.m | 23 +++++++++++-------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 2e82f68e6e7a1..05eac19d4a83b 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -1153,7 +1153,7 @@ - (NSObject*)valuePublishedByPlugin:(NSString*)pluginKey { } - (BOOL)hasOnscreenSurface { - return [_engine.get() iosPlatformView] -> HasOnscreenSurface(); + return [_engine.get() iosPlatformView] -> HasOnscreenSurface(); } @end diff --git a/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m b/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m index 516f9e6d1fb0a..d41c486186a7a 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m @@ -39,18 +39,23 @@ - (void)testFlutterViewDestroyed { [rootVC presentViewController:self.flutterViewController animated:NO completion:nil]; // TODO: refactor this to not rely on private test-only APIs - __weak id flutterView = [self.flutterViewController performSelector:NSSelectorFromString(@"flutterView")]; + __weak id flutterView = + [self.flutterViewController performSelector:NSSelectorFromString(@"flutterView")]; XCTAssertNotNil(flutterView); - XCTAssertTrue([self.flutterViewController performSelector:NSSelectorFromString(@"hasOnscreenSurface")]); - - [self.flutterViewController dismissViewControllerAnimated:NO completion:^{ - __weak id flutterView = [self.flutterViewController performSelector:NSSelectorFromString(@"flutterView")]; - XCTAssertNil(flutterView); - XCTAssertFalse([self.flutterViewController performSelector:NSSelectorFromString(@"hasOnscreenSurface")]); - }]; + XCTAssertTrue( + [self.flutterViewController performSelector:NSSelectorFromString(@"hasOnscreenSurface")]); + + [self.flutterViewController + dismissViewControllerAnimated:NO + completion:^{ + __weak id flutterView = [self.flutterViewController + performSelector:NSSelectorFromString(@"flutterView")]; + XCTAssertNil(flutterView); + XCTAssertFalse([self.flutterViewController + performSelector:NSSelectorFromString(@"hasOnscreenSurface")]); + }]; } @end #pragma clang diagnostic pop - From 97f41c4d78f674fa834bbcc2d92a57818496f90b Mon Sep 17 00:00:00 2001 From: George Wright Date: Fri, 6 Sep 2019 16:02:13 -0700 Subject: [PATCH 16/17] call selector directly instead of using performSelector --- .../Scenarios/ScenariosTests/FlutterViewTest.m | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m b/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m index d41c486186a7a..ff40f9352d5c9 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m @@ -6,9 +6,6 @@ #import #import "AppDelegate.h" -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wundeclared-selector" - @interface FlutterViewGLContextTest : XCTestCase @property(nonatomic, strong) FlutterViewController* flutterViewController; @end @@ -39,23 +36,17 @@ - (void)testFlutterViewDestroyed { [rootVC presentViewController:self.flutterViewController animated:NO completion:nil]; // TODO: refactor this to not rely on private test-only APIs - __weak id flutterView = - [self.flutterViewController performSelector:NSSelectorFromString(@"flutterView")]; + __weak id flutterView = [self.flutterViewController flutterView]; XCTAssertNotNil(flutterView); - XCTAssertTrue( - [self.flutterViewController performSelector:NSSelectorFromString(@"hasOnscreenSurface")]); + XCTAssertTrue([self.flutterViewController hasOnscreenSurface]); [self.flutterViewController dismissViewControllerAnimated:NO completion:^{ - __weak id flutterView = [self.flutterViewController - performSelector:NSSelectorFromString(@"flutterView")]; + __weak id flutterView = [self.flutterViewController flutterView]; XCTAssertNil(flutterView); - XCTAssertFalse([self.flutterViewController - performSelector:NSSelectorFromString(@"hasOnscreenSurface")]); + XCTAssertFalse([self.flutterViewController hasOnscreenSurface]); }]; } @end - -#pragma clang diagnostic pop From 43864782af88b6e23035893639aae94690231d34 Mon Sep 17 00:00:00 2001 From: George Wright Date: Fri, 6 Sep 2019 16:10:35 -0700 Subject: [PATCH 17/17] ensure we manually release the engine/viewcontroller now that ARC is off --- .../ios/Scenarios/ScenariosTests/FlutterViewTest.m | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m b/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m index ff40f9352d5c9..27c1f7702868e 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewTest.m @@ -8,6 +8,7 @@ @interface FlutterViewGLContextTest : XCTestCase @property(nonatomic, strong) FlutterViewController* flutterViewController; +@property(nonatomic, strong) FlutterEngine* flutterEngine; @end @implementation FlutterViewGLContextTest @@ -20,14 +21,18 @@ - (void)setUp { - (void)tearDown { if (self.flutterViewController) { [self.flutterViewController removeFromParentViewController]; + [self.flutterViewController release]; + } + if (self.flutterEngine) { + [self.flutterEngine release]; } [super tearDown]; } - (void)testFlutterViewDestroyed { - FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"testGL" project:nil]; - [engine runWithEntrypoint:nil]; - self.flutterViewController = [[FlutterViewController alloc] initWithEngine:engine + self.flutterEngine = [[FlutterEngine alloc] initWithName:@"testGL" project:nil]; + [self.flutterEngine runWithEntrypoint:nil]; + self.flutterViewController = [[FlutterViewController alloc] initWithEngine:self.flutterEngine nibName:nil bundle:nil];