From 7fea80e6bbd3bf438ed195b140b6033cb885ae30 Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Thu, 31 Oct 2024 16:24:56 -0700 Subject: [PATCH] iOS: Refactor ShellTestPlatformViewMetal Migrates DarwinContextMetal to a plain C struct, eliminating the need for constructor, getters, etc. since it's only used in this translation unit. The fields themselves cannot be inlined as fields on ShellTestPlatformViewMetal because the header in which that class is defined is included in plain C++ (non-Obj-C++) translation units and therefore cannot contain Obj-C types. This simplifies the code and simultaneously fixes complicated ARC behaviour in which the const "DarwinContextMetal::context()" getter caused refcount to be incremented on the underlying context_ pointer, but not decremented even if never unassigned. In particular the line ```objc FML_CHECK([metal_context->context() mainContext])); ``` appeared to cause refcount to be incremented but never decremented. Regardless of the ARC issue, this significantly simplifies the code. This also eliminates the last remaining use of fml::scoped_nsobject in Flutter's codebase. That class will be removed in a followup PR. Issue: https://github.com/flutter/flutter/issues/137801 --- shell/common/BUILD.gn | 3 + shell/common/shell_test_platform_view_metal.h | 2 +- .../common/shell_test_platform_view_metal.mm | 93 ++++++++----------- 3 files changed, 42 insertions(+), 56 deletions(-) diff --git a/shell/common/BUILD.gn b/shell/common/BUILD.gn index 8e2d9f2174b3a..a2cd2be483465 100644 --- a/shell/common/BUILD.gn +++ b/shell/common/BUILD.gn @@ -301,6 +301,9 @@ if (enable_unittests) { "shell_test_platform_view_metal.mm", ] + cflags_objc = flutter_cflags_objc_arc + cflags_objcc = flutter_cflags_objcc_arc + public_deps += [ "//flutter/shell/platform/darwin/graphics" ] } } diff --git a/shell/common/shell_test_platform_view_metal.h b/shell/common/shell_test_platform_view_metal.h index c5c0a8d454d04..afebef88eaedf 100644 --- a/shell/common/shell_test_platform_view_metal.h +++ b/shell/common/shell_test_platform_view_metal.h @@ -12,7 +12,7 @@ namespace flutter { namespace testing { -class DarwinContextMetal; +struct DarwinContextMetal; class ShellTestPlatformViewMetal final : public ShellTestPlatformView, public GPUSurfaceMetalDelegate { diff --git a/shell/common/shell_test_platform_view_metal.mm b/shell/common/shell_test_platform_view_metal.mm index 94345527bcc31..17080d8d4f964 100644 --- a/shell/common/shell_test_platform_view_metal.mm +++ b/shell/common/shell_test_platform_view_metal.mm @@ -8,64 +8,50 @@ #include -#include "flutter/fml/platform/darwin/scoped_nsobject.h" #include "flutter/shell/gpu/gpu_surface_metal_impeller.h" #include "flutter/shell/gpu/gpu_surface_metal_skia.h" #include "flutter/shell/platform/darwin/graphics/FlutterDarwinContextMetalImpeller.h" #include "flutter/shell/platform/darwin/graphics/FlutterDarwinContextMetalSkia.h" +FLUTTER_ASSERT_ARC + namespace flutter { namespace testing { -static fml::scoped_nsprotocol> CreateOffscreenTexture(id device) { +// This is out of the header so that shell_test_platform_view_metal.h can be included in +// non-Objective-C TUs. +struct DarwinContextMetal { + FlutterDarwinContextMetalSkia* context; + FlutterDarwinContextMetalImpeller* impeller_context; + id offscreen_texture; +}; + +static std::unique_ptr CreateDarwinContext( + bool impeller, + const std::shared_ptr& is_gpu_disabled_sync_switch) { + FlutterDarwinContextMetalSkia* skia_context = nil; + FlutterDarwinContextMetalImpeller* impeller_context = nil; + id device = nil; + if (impeller) { + impeller_context = [[FlutterDarwinContextMetalImpeller alloc] init:is_gpu_disabled_sync_switch]; + FML_CHECK(impeller_context.context); + device = impeller_context.context->GetMTLDevice(); + } else { + skia_context = [[FlutterDarwinContextMetalSkia alloc] initWithDefaultMTLDevice]; + FML_CHECK(skia_context.mainContext); + device = skia_context.device; + } auto descriptor = [MTLTextureDescriptor texture2DDescriptorWithPixelFormat:MTLPixelFormatBGRA8Unorm width:800 height:600 mipmapped:NO]; descriptor.usage = MTLTextureUsageRenderTarget | MTLTextureUsageShaderRead; - return fml::scoped_nsprotocol>{[device newTextureWithDescriptor:descriptor]}; + id offscreen_texture = [device newTextureWithDescriptor:descriptor]; + return std::make_unique( + DarwinContextMetal{skia_context, impeller_context, offscreen_texture}); } -// This is out of the header so that shell_test_platform_view_metal.h can be included in -// non-Objective-C TUs. -class DarwinContextMetal { - public: - explicit DarwinContextMetal( - bool impeller, - const std::shared_ptr& is_gpu_disabled_sync_switch) - : context_(impeller ? nil : [[FlutterDarwinContextMetalSkia alloc] initWithDefaultMTLDevice]), - impeller_context_( - impeller ? [[FlutterDarwinContextMetalImpeller alloc] init:is_gpu_disabled_sync_switch] - : nil), - offscreen_texture_(CreateOffscreenTexture( - impeller ? [impeller_context_ context]->GetMTLDevice() : [context_ device])) {} - - ~DarwinContextMetal() = default; - - fml::scoped_nsobject impeller_context() const { - return impeller_context_; - } - - fml::scoped_nsobject context() const { return context_; } - - fml::scoped_nsprotocol> offscreen_texture() const { return offscreen_texture_; } - - GPUMTLTextureInfo offscreen_texture_info() const { - GPUMTLTextureInfo info = {}; - info.texture_id = 0; - info.texture = reinterpret_cast(offscreen_texture_.get()); - return info; - } - - private: - const fml::scoped_nsobject context_; - const fml::scoped_nsobject impeller_context_; - const fml::scoped_nsprotocol> offscreen_texture_; - - FML_DISALLOW_COPY_AND_ASSIGN(DarwinContextMetal); -}; - ShellTestPlatformViewMetal::ShellTestPlatformViewMetal( PlatformView::Delegate& delegate, const TaskRunners& task_runners, @@ -75,17 +61,11 @@ GPUMTLTextureInfo offscreen_texture_info() const { const std::shared_ptr& is_gpu_disabled_sync_switch) : ShellTestPlatformView(delegate, task_runners), GPUSurfaceMetalDelegate(MTLRenderTargetType::kMTLTexture), - metal_context_(std::make_unique(GetSettings().enable_impeller, - is_gpu_disabled_sync_switch)), + metal_context_( + CreateDarwinContext(GetSettings().enable_impeller, is_gpu_disabled_sync_switch)), create_vsync_waiter_(std::move(create_vsync_waiter)), vsync_clock_(std::move(vsync_clock)), - shell_test_external_view_embedder_(std::move(shell_test_external_view_embedder)) { - if (GetSettings().enable_impeller) { - FML_CHECK([metal_context_->impeller_context() context] != nil); - } else { - FML_CHECK([metal_context_->context() mainContext] != nil); - } -} + shell_test_external_view_embedder_(std::move(shell_test_external_view_embedder)) {} ShellTestPlatformViewMetal::~ShellTestPlatformViewMetal() = default; @@ -113,16 +93,16 @@ GPUMTLTextureInfo offscreen_texture_info() const { // |PlatformView| std::unique_ptr ShellTestPlatformViewMetal::CreateRenderingSurface() { if (GetSettings().enable_impeller) { - auto context = [metal_context_->impeller_context() context]; + auto context = metal_context_->impeller_context.context; return std::make_unique( this, std::make_shared(context, nullptr)); } - return std::make_unique(this, [metal_context_->context() mainContext]); + return std::make_unique(this, metal_context_->context.mainContext); } // |PlatformView| std::shared_ptr ShellTestPlatformViewMetal::GetImpellerContext() const { - return [metal_context_->impeller_context() context]; + return metal_context_->impeller_context.context; } // |GPUSurfaceMetalDelegate| @@ -141,7 +121,10 @@ GPUMTLTextureInfo offscreen_texture_info() const { // |GPUSurfaceMetalDelegate| GPUMTLTextureInfo ShellTestPlatformViewMetal::GetMTLTexture(const SkISize& frame_info) const { - return metal_context_->offscreen_texture_info(); + GPUMTLTextureInfo info = {}; + info.texture_id = 0; + info.texture = (__bridge GPUMTLTextureHandle)metal_context_->offscreen_texture; + return info; } // |GPUSurfaceMetalDelegate|