Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Migrate FlutterUIPressProxy, ios_context*, rendering_api_selection, and a few other files to ARC #51633

Merged
merged 5 commits into from
Apr 16, 2024
Merged

Migrate FlutterUIPressProxy, ios_context*, rendering_api_selection, and a few other files to ARC #51633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d8d6ef6
Migrate a few "leaf import" MRC iOS files to ARC
jmagman Mar 22, 2024
86cb270
FlutterUIPressProxy
jmagman Apr 9, 2024
cedec0c
context
jmagman Apr 9, 2024
03f9b71
import order
jmagman Apr 11, 2024
d4899bb
Remove GetDarwinContext
jmagman Apr 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 25 additions & 23 deletions shell/platform/darwin/ios/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,32 @@ source_set("flutter_framework_source_arc") {
"framework/Source/FlutterTextInputPlugin.mm",
"framework/Source/FlutterTextureRegistryRelay.h",
"framework/Source/FlutterTextureRegistryRelay.mm",
"framework/Source/FlutterUIPressProxy.h",
"framework/Source/FlutterUIPressProxy.mm",
"framework/Source/KeyCodeMap.g.mm",
"framework/Source/KeyCodeMap_Internal.h",
"framework/Source/UIViewController+FlutterScreenAndSceneIfLoaded.h",
"framework/Source/UIViewController+FlutterScreenAndSceneIfLoaded.mm",
"framework/Source/connection_collection.h",
"framework/Source/connection_collection.mm",
"framework/Source/platform_message_response_darwin.h",
"framework/Source/platform_message_response_darwin.mm",
"framework/Source/profiler_metrics_ios.h",
"framework/Source/profiler_metrics_ios.mm",
"framework/Source/vsync_waiter_ios.h",
"framework/Source/vsync_waiter_ios.mm",
"ios_context.h",
"ios_context.mm",
"ios_context_metal_impeller.h",
"ios_context_metal_impeller.mm",
"ios_context_metal_skia.h",
"ios_context_metal_skia.mm",
"ios_context_software.h",
"ios_context_software.mm",
"ios_external_texture_metal.h",
"ios_external_texture_metal.mm",
"rendering_api_selection.h",
"rendering_api_selection.mm",
]

frameworks = [
Expand All @@ -92,11 +112,16 @@ source_set("flutter_framework_source_arc") {
]

deps += [
":ios_gpu_configuration",
"//flutter/common:common",
"//flutter/common/graphics",
"//flutter/lib/ui",
"//flutter/runtime",
"//flutter/shell/common",
"//flutter/shell/platform/darwin/common",
"//flutter/shell/platform/darwin/graphics",
"//flutter/shell/platform/embedder:embedder_as_internal_library",
"//flutter/shell/profiling:profiling",
]
}

Expand Down Expand Up @@ -132,8 +157,6 @@ source_set("flutter_framework_source") {
"framework/Source/FlutterSemanticsScrollView.mm",
"framework/Source/FlutterSpellCheckPlugin.h",
"framework/Source/FlutterSpellCheckPlugin.mm",
"framework/Source/FlutterUIPressProxy.h",
"framework/Source/FlutterUIPressProxy.mm",
"framework/Source/FlutterUndoManagerDelegate.h",
"framework/Source/FlutterUndoManagerPlugin.h",
"framework/Source/FlutterUndoManagerPlugin.mm",
Expand All @@ -147,22 +170,6 @@ source_set("flutter_framework_source") {
"framework/Source/accessibility_bridge.mm",
"framework/Source/accessibility_text_entry.h",
"framework/Source/accessibility_text_entry.mm",
"framework/Source/connection_collection.h",
"framework/Source/connection_collection.mm",
"framework/Source/platform_message_response_darwin.h",
"framework/Source/platform_message_response_darwin.mm",
"framework/Source/profiler_metrics_ios.h",
"framework/Source/profiler_metrics_ios.mm",
"ios_context.h",
"ios_context.mm",
"ios_context_metal_impeller.h",
"ios_context_metal_impeller.mm",
"ios_context_metal_skia.h",
"ios_context_metal_skia.mm",
"ios_context_software.h",
"ios_context_software.mm",
"ios_external_texture_metal.h",
"ios_external_texture_metal.mm",
"ios_external_view_embedder.h",
"ios_external_view_embedder.mm",
"ios_surface.h",
Expand All @@ -177,8 +184,6 @@ source_set("flutter_framework_source") {
"platform_message_handler_ios.mm",
"platform_view_ios.h",
"platform_view_ios.mm",
"rendering_api_selection.h",
"rendering_api_selection.mm",
]

sources += _flutter_framework_headers
Expand All @@ -191,17 +196,14 @@ source_set("flutter_framework_source") {
deps += [
":ios_gpu_configuration",
"//flutter/common",
"//flutter/common/graphics",
"//flutter/flow",
"//flutter/fml",
"//flutter/lib/ui",
"//flutter/runtime",
"//flutter/runtime:libdart",
"//flutter/shell/common",
"//flutter/shell/platform/common:common_cpp_input",
"//flutter/shell/platform/darwin/common",
"//flutter/shell/platform/darwin/common:framework_common",
"//flutter/shell/platform/darwin/graphics",
"//flutter/shell/platform/embedder:embedder_as_internal_library",
"//flutter/shell/profiling:profiling",
"//flutter/skia",
Expand Down
6 changes: 5 additions & 1 deletion shell/platform/darwin/ios/framework/Source/FlutterUIPressProxy.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterUIPressProxy.h"

#import <UIKit/UIKit.h>

#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterUIPressProxy.h"
#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h"

FLUTTER_ASSERT_ARC

@interface FlutterUIPressProxy ()
@property(nonatomic, readonly) UIPress* press;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were these properties accidentally weak before and it just hadn't happened to break anything?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess? The usages look like this:

engine/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm

Lines 1885 to 1889 in fef8499

[self handlePressEvent:[[[FlutterUIPressProxy alloc] initWithPress:press
withEvent:event] autorelease]
nextAction:^() {
[super pressesBegan:[NSSet setWithObject:press] withEvent:event];
}];

so maybe press and event are being retained in the nextAction block? I don't see any obvious bugs about it...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting - yeah seems like this should previously have been explicitly strong, and maybe worked because (I assume) it's retained in that map we create to pointer events being sent to the framework until we get a response? (Didn't look but IIRC we hold onto them there)

Expand Down
4 changes: 4 additions & 0 deletions shell/platform/darwin/ios/framework/Source/connection_collection.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

#import "flutter/shell/platform/darwin/ios/framework/Source/connection_collection.h"

#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h"

FLUTTER_ASSERT_ARC

namespace flutter {
ConnectionCollection::Connection ConnectionCollection::AquireConnection(const std::string& name) {
Connection nextConnection = ++counter_;
Expand Down
4 changes: 4 additions & 0 deletions shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

#import "flutter/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.h"

#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h"

FLUTTER_ASSERT_ARC

namespace flutter {

PlatformMessageResponseDarwin::PlatformMessageResponseDarwin(
Expand Down
3 changes: 3 additions & 0 deletions shell/platform/darwin/ios/framework/Source/profiler_metrics_ios.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@

#import <Foundation/Foundation.h>

#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/IOKit.h"

FLUTTER_ASSERT_ARC

namespace {

// RAII holder for `thread_array_t` this is so any early returns in
Expand Down
2 changes: 2 additions & 0 deletions shell/platform/darwin/ios/ios_context.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#include "flutter/shell/platform/darwin/ios/ios_context_metal_skia.h"
#include "flutter/shell/platform/darwin/ios/ios_context_software.h"

FLUTTER_ASSERT_ARC

namespace flutter {

IOSContext::IOSContext(MsaaSampleCount msaa_samples) : msaa_samples_(msaa_samples) {}
Expand Down
2 changes: 0 additions & 2 deletions shell/platform/darwin/ios/ios_context_metal_impeller.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ class IOSContextMetalImpeller final : public IOSContext {

~IOSContextMetalImpeller();

fml::scoped_nsobject<FlutterDarwinContextMetalSkia> GetDarwinContext() const;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Hmm, why is IOSContextMetalImpeller returning FlutterDarwinContextMetalSkia?"
It's dead code.


IOSRenderingBackend GetBackend() const override;

// |IOSContext|
Expand Down
13 changes: 5 additions & 8 deletions shell/platform/darwin/ios/ios_context_metal_impeller.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#include "flutter/impeller/entity/mtl/entity_shaders.h"
#import "flutter/shell/platform/darwin/ios/ios_external_texture_metal.h"

FLUTTER_ASSERT_ARC

namespace flutter {

IOSContextMetalImpeller::IOSContextMetalImpeller(
Expand All @@ -16,11 +18,6 @@

IOSContextMetalImpeller::~IOSContextMetalImpeller() = default;

fml::scoped_nsobject<FlutterDarwinContextMetalSkia> IOSContextMetalImpeller::GetDarwinContext()
const {
return fml::scoped_nsobject<FlutterDarwinContextMetalSkia>{};
}

IOSRenderingBackend IOSContextMetalImpeller::GetBackend() const {
return IOSRenderingBackend::kImpeller;
}
Expand Down Expand Up @@ -54,9 +51,9 @@
int64_t texture_id,
fml::scoped_nsobject<NSObject<FlutterTexture>> texture) {
return std::make_unique<IOSExternalTextureMetal>(
fml::scoped_nsobject<FlutterDarwinExternalTextureMetal>{
[[darwin_context_metal_impeller_ createExternalTextureWithIdentifier:texture_id
texture:texture] retain]});
fml::scoped_nsobject<FlutterDarwinExternalTextureMetal>{[darwin_context_metal_impeller_
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need the scoped_nsobject here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally the answer should be no, but since scoped_nsobject is ARC-friendly, we should be safe to do those in a followup patch if preferred.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ios_context_->CreateExternalTexture is only called from mrc platform_view_ios:

engine/shell/platform/darwin/ios/platform_view_ios.mm

Lines 139 to 140 in a9a4496

RegisterTexture(ios_context_->CreateExternalTexture(
texture_id, fml::scoped_nsobject<NSObject<FlutterTexture>>{[texture retain]}));

I'm going to hold off on removing scoped_nsobject from this signature until I can remove it there as well. Do you foresee any issues with that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fine to keep scoped_nsobjects since we have the multi-mode compliant version.

createExternalTextureWithIdentifier:texture_id
texture:texture]});
}

} // namespace flutter
5 changes: 3 additions & 2 deletions shell/platform/darwin/ios/ios_context_metal_skia.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#import "flutter/shell/platform/darwin/ios/ios_external_texture_metal.h"
#include "third_party/skia/include/gpu/GrContextOptions.h"

FLUTTER_ASSERT_ARC

namespace flutter {

IOSContextMetalSkia::IOSContextMetalSkia(MsaaSampleCount msaa_samples) : IOSContext(msaa_samples) {
Expand Down Expand Up @@ -52,8 +54,7 @@
fml::scoped_nsobject<NSObject<FlutterTexture>> texture) {
return std::make_unique<IOSExternalTextureMetal>(
fml::scoped_nsobject<FlutterDarwinExternalTextureMetal>{
[[darwin_context_metal_ createExternalTextureWithIdentifier:texture_id
texture:texture] retain]});
[darwin_context_metal_ createExternalTextureWithIdentifier:texture_id texture:texture]});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question here.

}

} // namespace flutter
2 changes: 2 additions & 0 deletions shell/platform/darwin/ios/ios_context_software.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#import "flutter/shell/platform/darwin/ios/ios_context_software.h"
#include "ios_context.h"

FLUTTER_ASSERT_ARC

namespace flutter {

IOSContextSoftware::IOSContextSoftware() : IOSContext(MsaaSampleCount::kNone) {}
Expand Down
2 changes: 2 additions & 0 deletions shell/platform/darwin/ios/ios_external_texture_metal.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#import "flutter/shell/platform/darwin/ios/ios_external_texture_metal.h"
#include "flow/layers/layer.h"

FLUTTER_ASSERT_ARC

namespace flutter {

IOSExternalTextureMetal::IOSExternalTextureMetal(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we convert this scoped_nsobject to a simple Obj-C object property now?

Expand Down
6 changes: 4 additions & 2 deletions shell/platform/darwin/ios/rendering_api_selection.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,18 @@

#include "flutter/fml/logging.h"

#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h"
#include "flutter/shell/platform/darwin/ios/framework/Source/FlutterMetalLayer.h"

FLUTTER_ASSERT_ARC

namespace flutter {

bool ShouldUseMetalRenderer() {
bool ios_version_supports_metal = false;
if (@available(iOS METAL_IOS_VERSION_BASELINE, *)) {
auto device = MTLCreateSystemDefaultDevice();
id<MTLDevice> device = MTLCreateSystemDefaultDevice();
ios_version_supports_metal = [device supportsFeatureSet:MTLFeatureSet_iOS_GPUFamily1_v3];
[device release];
}
return ios_version_supports_metal;
}
Expand Down