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

[metal] Darwin unified external metal textures #24157

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 2 additions & 2 deletions common/graphics/texture.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ namespace flutter {

class Texture {
public:
Texture(int64_t id); // Called from UI or raster thread.
virtual ~Texture(); // Called from raster thread.
explicit Texture(int64_t id); // Called from UI or raster thread.
virtual ~Texture(); // Called from raster thread.

// Called from raster thread.
virtual void Paint(SkCanvas& canvas,
Expand Down
8 changes: 6 additions & 2 deletions shell/platform/darwin/graphics/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,23 @@ assert(is_ios || is_mac)
import("//flutter/common/config.gni")

source_set("graphics") {
cflags_objc = flutter_cflags_objc
cflags_objcc = flutter_cflags_objcc
cflags_objcc = [ "-fobjc-arc" ]
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be in addition to the flutter_cflags_objc? I am not sure what those flags currently are but we are no longer applying them to this target.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that the other targets that enable arc, only had this will verify.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created a new high level param for arc targets and unified it.


sources = [
"FlutterDarwinContextMetal.h",
"FlutterDarwinContextMetal.mm",
"FlutterDarwinExternalTextureMetal.h",
"FlutterDarwinExternalTextureMetal.mm",
]

deps = [
"//flutter/common/graphics",
"//flutter/fml",
"//flutter/shell/platform/darwin/common:framework_shared",
]

libs = [ "CoreVideo.framework" ]

public_deps = [ "//third_party/skia" ]

public_configs = [ "//flutter:config" ]
Expand Down
14 changes: 14 additions & 0 deletions shell/platform/darwin/graphics/FlutterDarwinContextMetal.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@
#ifndef SHELL_PLATFORM_DARWIN_GRAPHICS_DARWIN_CONTEXT_METAL_H_
#define SHELL_PLATFORM_DARWIN_GRAPHICS_DARWIN_CONTEXT_METAL_H_

#import <CoreVideo/CVMetalTextureCache.h>
#import <Foundation/Foundation.h>
#import <Metal/Metal.h>

#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterTexture.h"
#import "flutter/shell/platform/darwin/graphics/FlutterDarwinExternalTextureMetal.h"
#include "third_party/skia/include/gpu/GrDirectContext.h"

NS_ASSUME_NONNULL_BEGIN
Expand All @@ -29,6 +32,12 @@ NS_ASSUME_NONNULL_BEGIN
- (instancetype)initWithMTLDevice:(id<MTLDevice>)device
commandQueue:(id<MTLCommandQueue>)commandQueue;

/**
* Creates an external texture with the specified ID and contents.
*/
- (FlutterDarwinExternalTextureMetal*)externalTextureWithID:(int64_t)textureID
Copy link
Member

Choose a reason for hiding this comment

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

IMO createExternalTexture:withIdentifier: seems more idiomatic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

texture:(NSObject<FlutterTexture>*)texture;

/**
* MTLDevice that is backing this context.s
*/
Expand All @@ -50,6 +59,11 @@ NS_ASSUME_NONNULL_BEGIN
*/
@property(nonatomic, readonly) sk_sp<GrDirectContext> resourceContext;

/*
* Texture cache for external textures.
*/
@property(nonatomic, readonly) CVMetalTextureCacheRef textureCache;

@end

NS_ASSUME_NONNULL_END
Expand Down
28 changes: 21 additions & 7 deletions shell/platform/darwin/graphics/FlutterDarwinContextMetal.mm
Original file line number Diff line number Diff line change
Expand Up @@ -33,32 +33,39 @@ - (instancetype)initWithMTLDevice:(id<MTLDevice>)device

if (!_device) {
FML_DLOG(ERROR) << "Could not acquire Metal device.";
[self release];
return nil;
Copy link
Member

Choose a reason for hiding this comment

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

Can we decorate translation units with FLUTTER_ASSERT_ARC and FLUTTER_ASSERT_NON_ARC now that we are mixing the two in the engine?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added it to the ARC targets that i've touched.

}

_commandQueue = commandQueue;

if (!_commandQueue) {
FML_DLOG(ERROR) << "Could not create Metal command queue.";
[self release];
return nil;
}

[_commandQueue setLabel:@"Flutter Main Queue"];

CVReturn cvReturn = CVMetalTextureCacheCreate(kCFAllocatorDefault, // allocator
nil, // cache attributes (nil default)
_device, // metal device
nil, // texture attributes (nil default)
&_textureCache // [out] cache
Copy link
Member

Choose a reason for hiding this comment

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

CoreFoundation objects need to be explicitly released. IIRC, this would leak the texture cache in the dealloc unless __bridge_transfered

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was worried about the same. I will try to add a test to see that everything that was allocated gets collected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't able to come up with a way to test de-alloc in a nice way. I audited the code and made sure that are CoreFoundation objects are released and retained as necessary.

);
if (cvReturn != kCVReturnSuccess) {
FML_DLOG(ERROR) << "Could not create Metal texture cache.";
return nil;
}

auto contextOptions = CreateMetalGrContextOptions();

// Skia expect arguments to `MakeMetal` transfer ownership of the reference in for release later
// when the GrDirectContext is collected.
_mainContext =
GrDirectContext::MakeMetal([_device retain], [_commandQueue retain], contextOptions);
_resourceContext =
GrDirectContext::MakeMetal([_device retain], [_commandQueue retain], contextOptions);
_mainContext = GrDirectContext::MakeMetal(
(__bridge_retained void*)_device, (__bridge_retained void*)_commandQueue, contextOptions);
_resourceContext = _mainContext;

if (!_mainContext || !_resourceContext) {
FML_DLOG(ERROR) << "Could not create Skia Metal contexts.";
[self release];
return nil;
}

Expand All @@ -67,4 +74,11 @@ - (instancetype)initWithMTLDevice:(id<MTLDevice>)device
return self;
}

- (FlutterDarwinExternalTextureMetal*)externalTextureWithID:(int64_t)textureID
texture:(NSObject<FlutterTexture>*)texture {
return [[FlutterDarwinExternalTextureMetal alloc] initWithTextureCache:_textureCache
textureID:textureID
texture:texture];
}

@end
34 changes: 34 additions & 0 deletions shell/platform/darwin/graphics/FlutterDarwinExternalTextureMetal.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// 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 <Foundation/Foundation.h>
#import <Metal/Metal.h>

#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterTexture.h"
#include "third_party/skia/include/core/SkCanvas.h"
#include "third_party/skia/include/gpu/GrDirectContext.h"

@interface FlutterDarwinExternalTextureMetal : NSObject

- (nullable instancetype)initWithTextureCache:(nonnull CVMetalTextureCacheRef)textureCache
textureID:(int64_t)textureID
texture:(nonnull NSObject<FlutterTexture>*)texture;

- (void)paint:(SkCanvas&)canvas
bounds:(const SkRect&)bounds
freeze:(BOOL)freeze
grContext:(nonnull GrDirectContext*)grContext
sampling:(const SkSamplingOptions&)sampling;

- (void)onGrContextCreated;

- (void)onGrContextDestroyed;

- (void)markNewFrameAvailable;

- (void)onTextureUnregistered;

@property(nonatomic, readonly) int64_t textureID;

@end
Loading