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

Commit 73ccb5b

Browse files
authored
iOS,macOS: Refactor TestMetalContext for ARC (#56510)
`TestMetalContext` cannot include any Objective-C types in its header file since that header is included in pure C++ translation units. Previously, we declared these fields as `void*`, which then requires manual `__bridge_retained` and `__bridge_transfer` casts to perform the necessary retain/relase manipulation while opting the object in and out of ARC management. Instead, we extract these to a struct in the implementation file and allow the fields to be managed by ARC. This is an interim measure consistent with the approach we've taken [elsewhere][1] until we implement the proper long-term solution. The proper long-term solution is to refactor `EmbedderTest` (see: flutter/flutter#157942) and other related code to split out backends into separate translation units, with the Metal backend code in Objective-C++ implementation files, which would then mean the headers could include Objective-C ivars. No changes to tests, since this change introduces no behaviour changes. Issue: flutter/flutter#157942 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent 980e4d2 commit 73ccb5b

File tree

2 files changed

+20
-25
lines changed

2 files changed

+20
-25
lines changed

testing/test_metal_context.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,16 @@
66
#define FLUTTER_TESTING_TEST_METAL_CONTEXT_H_
77

88
#include <map>
9+
#include <memory>
910
#include <mutex>
1011

1112
#include "third_party/skia/include/gpu/ganesh/GrDirectContext.h"
1213
#include "third_party/skia/include/ports/SkCFObject.h"
1314

1415
namespace flutter {
1516

17+
struct MetalObjCFields;
18+
1619
class TestMetalContext {
1720
public:
1821
struct TextureInfo {
@@ -38,9 +41,7 @@ class TestMetalContext {
3841
TextureInfo GetTextureInfo(int64_t texture_id);
3942

4043
private:
41-
// TODO(cbracken): https://github.com/flutter/flutter/issues/157942
42-
void* device_; // id<MTLDevice>
43-
void* command_queue_; // id<MTLCommandQueue>
44+
std::unique_ptr<MetalObjCFields> metal_;
4445
sk_sp<GrDirectContext> skia_context_;
4546
std::mutex textures_mutex_;
4647
int64_t texture_id_ctr_ = 1; // guarded by textures_mutex

testing/test_metal_context.mm

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@
1717

1818
namespace flutter {
1919

20+
// TOOD(cbracken): https://github.com/flutter/flutter/issues/157942
21+
struct MetalObjCFields {
22+
id<MTLDevice> device;
23+
id<MTLCommandQueue> command_queue;
24+
};
25+
2026
TestMetalContext::TestMetalContext() {
2127
id<MTLDevice> device = MTLCreateSystemDefaultDevice();
2228
if (!device) {
@@ -43,37 +49,21 @@
4349
"and command queue.";
4450
}
4551

46-
// Retain and transfer to non-ARC-managed pointers.
47-
// TODO(cbracken): https://github.com/flutter/flutter/issues/157942
48-
device_ = (__bridge_retained void*)device;
49-
command_queue_ = (__bridge_retained void*)command_queue;
52+
metal_ = std::make_unique<MetalObjCFields>(MetalObjCFields{device, command_queue});
5053
}
5154

5255
TestMetalContext::~TestMetalContext() {
5356
std::scoped_lock lock(textures_mutex_);
5457
textures_.clear();
55-
if (device_) {
56-
// Release and transfer to ARC-managed pointer.
57-
// TODO(cbracken): https://github.com/flutter/flutter/issues/157942
58-
id<MTLDevice> device = // NOLINT(clang-analyzer-deadcode.DeadStores)
59-
(__bridge_transfer id<MTLDevice>)device_;
60-
device = nil;
61-
}
62-
if (command_queue_) {
63-
// Release and transfer to ARC-managed pointer.
64-
// TODO(cbracken): https://github.com/flutter/flutter/issues/157942
65-
id<MTLCommandQueue> command_queue = // NOLINT(clang-analyzer-deadcode.DeadStores)
66-
(__bridge_transfer id<MTLCommandQueue>)command_queue_;
67-
command_queue = nil;
68-
}
58+
metal_.reset();
6959
}
7060

7161
void* TestMetalContext::GetMetalDevice() const {
72-
return device_;
62+
return metal_ ? (__bridge void*)metal_->device : nil;
7363
}
7464

7565
void* TestMetalContext::GetMetalCommandQueue() const {
76-
return command_queue_;
66+
return metal_ ? (__bridge void*)metal_->command_queue : nil;
7767
}
7868

7969
sk_sp<GrDirectContext> TestMetalContext::GetSkiaContext() const {
@@ -98,8 +88,12 @@
9888
return {.texture_id = -1, .texture = nullptr};
9989
}
10090

101-
id<MTLDevice> device = (__bridge id<MTLDevice>)GetMetalDevice();
102-
id<MTLTexture> texture = [device newTextureWithDescriptor:texture_descriptor];
91+
if (!metal_) {
92+
FML_CHECK(false) << "Invalid Metal device.";
93+
return {.texture_id = -1, .texture = nullptr};
94+
}
95+
96+
id<MTLTexture> texture = [metal_->device newTextureWithDescriptor:texture_descriptor];
10397
if (!texture) {
10498
FML_CHECK(false) << "Could not create texture from texture descriptor.";
10599
return {.texture_id = -1, .texture = nullptr};

0 commit comments

Comments
 (0)