Change Metal to not take ownership of objects
Prior to this change, Skia/Metal interfaces take ownership of the Metal
objects passed in (that is, the caller should count passing the object
to Skia as "freeing" the object).
Change this behavior so that Skia/Metal retains its own separate
ownership of the Metal objects.
Make GrBackendTexture and GrBackendRenderTarget maintain their own
references to the underlying MTLTexture by using the CFRetain/CFRelease
interfaces. Do this by adding a private GrMtlBackendSurfaceInfo.
Move GrMtlBackendSurfaceInfo (formerly GrMtlTextureInfo) out of the
union in GrBackendTexture and GrBackendRenderTarget because unions
cannot have nontrivial constructors and destructors (how fVkInfo isn't
causing a compile error is unclear).
Change-Id: Iae3719c0715825d86503d03c766e47f0f6015bdf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/215685
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
diff --git a/gn/gpu.gni b/gn/gpu.gni
index 23ffc43..4a225c6 100644
--- a/gn/gpu.gni
+++ b/gn/gpu.gni
@@ -701,6 +701,7 @@
skia_metal_sources = [
"$_include/gpu/mtl/GrMtlTypes.h",
+ "$_include/private/GrMtlTypesPriv.h",
"$_src/gpu/mtl/GrMtlBuffer.h",
"$_src/gpu/mtl/GrMtlBuffer.mm",
"$_src/gpu/mtl/GrMtlCaps.h",
@@ -738,6 +739,7 @@
"$_src/gpu/mtl/GrMtlTextureRenderTarget.mm",
"$_src/gpu/mtl/GrMtlTrampoline.h",
"$_src/gpu/mtl/GrMtlTrampoline.mm",
+ "$_src/gpu/mtl/GrMtlTypesPriv.cpp",
"$_src/gpu/mtl/GrMtlUniformHandler.h",
"$_src/gpu/mtl/GrMtlUniformHandler.mm",
"$_src/gpu/mtl/GrMtlUtil.h",
diff --git a/include/gpu/GrBackendSurface.h b/include/gpu/GrBackendSurface.h
index c42f7bc..58c7697 100644
--- a/include/gpu/GrBackendSurface.h
+++ b/include/gpu/GrBackendSurface.h
@@ -18,6 +18,7 @@
#ifdef SK_METAL
#include "include/gpu/mtl/GrMtlTypes.h"
+#include "include/private/GrMtlTypesPriv.h"
#endif
#if !SK_SUPPORT_GPU
@@ -145,6 +146,7 @@
const GrVkImageInfo& vkInfo);
#ifdef SK_METAL
+ // This will take a strong reference to the MTLTexture in mtlInfo.
GrBackendTexture(int width,
int height,
GrMipMapped,
@@ -253,11 +255,11 @@
union {
GrGLTextureInfo fGLInfo;
GrVkBackendSurfaceInfo fVkInfo;
-#ifdef SK_METAL
- GrMtlTextureInfo fMtlInfo;
-#endif
GrMockTextureInfo fMockInfo;
};
+#ifdef SK_METAL
+ GrMtlBackendSurfaceInfo fMtlInfo;
+#endif
};
class SK_API GrBackendRenderTarget {
@@ -281,6 +283,7 @@
GrBackendRenderTarget(int width, int height, int sampleCnt, const GrVkImageInfo& vkInfo);
#ifdef SK_METAL
+ // This will take a strong reference to the MTLTexture in mtlInfo.
GrBackendRenderTarget(int width,
int height,
int sampleCnt,
@@ -370,17 +373,16 @@
int fSampleCnt;
int fStencilBits;
GrPixelConfig fConfig;
-
GrBackendApi fBackend;
union {
GrGLFramebufferInfo fGLInfo;
GrVkBackendSurfaceInfo fVkInfo;
-#ifdef SK_METAL
- GrMtlTextureInfo fMtlInfo;
-#endif
GrMockRenderTargetInfo fMockInfo;
};
+#ifdef SK_METAL
+ GrMtlBackendSurfaceInfo fMtlInfo;
+#endif
};
#endif
diff --git a/include/gpu/GrContext.h b/include/gpu/GrContext.h
index 850e157..804f238 100644
--- a/include/gpu/GrContext.h
+++ b/include/gpu/GrContext.h
@@ -61,9 +61,8 @@
#ifdef SK_METAL
/**
* Makes a GrContext which uses Metal as the backend. The device parameter is an MTLDevice
- * and queue is an MTLCommandQueue which should be used by the backend. These objects must
- * have a ref on them which can be transferred to Ganesh which will release the ref when the
- * GrContext is destroyed.
+ * and queue is an MTLCommandQueue which should be used by the backend. These objects will
+ * be retained by Ganesh, and released when the GrContext is destroyed.
*/
static sk_sp<GrContext> MakeMetal(void* device, void* queue, const GrContextOptions& options);
static sk_sp<GrContext> MakeMetal(void* device, void* queue);
diff --git a/include/gpu/mtl/GrMtlTypes.h b/include/gpu/mtl/GrMtlTypes.h
index 5dffe25..8d96c85 100644
--- a/include/gpu/mtl/GrMtlTypes.h
+++ b/include/gpu/mtl/GrMtlTypes.h
@@ -17,8 +17,10 @@
///////////////////////////////////////////////////////////////////////////////
/**
- * Types for interacting with Metal resources created externally to Skia. Holds the MTLTexture as a
- * const void*. This is used by GrBackendObjects.
+ * Types for interacting with Metal resources created externally to Skia. Holds
+ * the MTLTexture as a weak const void*. This is used by GrBackendObjects. Note
+ * that GrBackendObjects created with a GrMtlTextureInfo will take a strong
+ * reference to the MTLTexture.
*/
struct GrMtlTextureInfo {
public:
diff --git a/include/private/GrMtlTypesPriv.h b/include/private/GrMtlTypesPriv.h
new file mode 100644
index 0000000..473cc0b
--- /dev/null
+++ b/include/private/GrMtlTypesPriv.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright 2019 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef GrMtlTypesPriv_DEFINED
+#define GrMtlTypesPriv_DEFINED
+
+#include "include/gpu/mtl/GrMtlTypes.h"
+
+// This struct is to used to store the the actual information about the Metal backend texture on
+// GrBackendTexture and GrBackendRenderTarget. This differs from the public GrMtlTextureInfo in
+// that it maintains a strong reference to the MTLTexture object.
+struct GrMtlBackendSurfaceInfo {
+ GrMtlBackendSurfaceInfo() {}
+ GrMtlBackendSurfaceInfo(const GrMtlBackendSurfaceInfo&) = delete;
+ GrMtlBackendSurfaceInfo& operator=(const GrMtlBackendSurfaceInfo&) = delete;
+ explicit GrMtlBackendSurfaceInfo(const GrMtlTextureInfo& info);
+ ~GrMtlBackendSurfaceInfo();
+
+ void cleanup();
+
+ // Assigns the passed in GrMtlBackendSurfaceInfo to this object. if isValid is true we will also
+ // attempt to unref the old fTexture.
+ void assign(const GrMtlBackendSurfaceInfo&, bool isValid);
+
+ GrMtlTextureInfo snapTextureInfo() const;
+
+#if GR_TEST_UTILS
+ bool operator==(const GrMtlBackendSurfaceInfo& that) const;
+#endif
+
+private:
+ // Strong reference to the MTLTexture, maintained by CFRetain and CFRelease.
+ const void* fTexture = nullptr;
+};
+
+#endif
diff --git a/src/gpu/GrBackendSurface.cpp b/src/gpu/GrBackendSurface.cpp
index dc58250..a2313d2 100644
--- a/src/gpu/GrBackendSurface.cpp
+++ b/src/gpu/GrBackendSurface.cpp
@@ -250,6 +250,11 @@
fVkInfo.cleanup();
}
#endif
+#ifdef SK_METAL
+ if (this->isValid() && GrBackendApi::kMetal == fBackend) {
+ fMtlInfo.cleanup();
+ }
+#endif
}
GrBackendTexture::GrBackendTexture(const GrBackendTexture& that) : fIsValid(false) {
@@ -279,7 +284,7 @@
break;
#ifdef SK_METAL
case GrBackendApi::kMetal:
- fMtlInfo = that.fMtlInfo;
+ fMtlInfo.assign(that.fMtlInfo, this->isValid());
break;
#endif
case GrBackendApi::kMock:
@@ -329,7 +334,7 @@
#ifdef SK_METAL
bool GrBackendTexture::getMtlTextureInfo(GrMtlTextureInfo* outInfo) const {
if (this->isValid() && GrBackendApi::kMetal == fBackend) {
- *outInfo = fMtlInfo;
+ *outInfo = fMtlInfo.snapTextureInfo();
return true;
}
return false;
@@ -376,7 +381,8 @@
#endif
#ifdef SK_METAL
case GrBackendApi::kMetal:
- return this->fMtlInfo.fTexture == that.fMtlInfo.fTexture;
+ return this->fMtlInfo.snapTextureInfo().fTexture ==
+ that.fMtlInfo.snapTextureInfo().fTexture;
#endif
case GrBackendApi::kMock:
return fMockInfo.fID == that.fMockInfo.fID;
@@ -548,6 +554,11 @@
fVkInfo.cleanup();
}
#endif
+#ifdef SK_METAL
+ if (this->isValid() && GrBackendApi::kMetal == fBackend) {
+ fMtlInfo.cleanup();
+ }
+#endif
}
GrBackendRenderTarget::GrBackendRenderTarget(const GrBackendRenderTarget& that) : fIsValid(false) {
@@ -578,7 +589,7 @@
break;
#ifdef SK_METAL
case GrBackendApi::kMetal:
- fMtlInfo = that.fMtlInfo;
+ fMtlInfo.assign(that.fMtlInfo, this->isValid());
break;
#endif
case GrBackendApi::kMock:
@@ -621,7 +632,7 @@
#ifdef SK_METAL
bool GrBackendRenderTarget::getMtlTextureInfo(GrMtlTextureInfo* outInfo) const {
if (this->isValid() && GrBackendApi::kMetal == fBackend) {
- *outInfo = fMtlInfo;
+ *outInfo = fMtlInfo.snapTextureInfo();
return true;
}
return false;
diff --git a/src/gpu/mtl/GrMtlGpu.mm b/src/gpu/mtl/GrMtlGpu.mm
index 8fae4b6..48af6ae 100644
--- a/src/gpu/mtl/GrMtlGpu.mm
+++ b/src/gpu/mtl/GrMtlGpu.mm
@@ -495,13 +495,12 @@
return std::move(tex);
}
-static id<MTLTexture> get_texture_from_backend(const GrBackendTexture& backendTex,
- GrWrapOwnership ownership) {
+static id<MTLTexture> get_texture_from_backend(const GrBackendTexture& backendTex) {
GrMtlTextureInfo textureInfo;
if (!backendTex.getMtlTextureInfo(&textureInfo)) {
return nil;
}
- return GrGetMTLTexture(textureInfo.fTexture, ownership);
+ return GrGetMTLTexture(textureInfo.fTexture);
}
static id<MTLTexture> get_texture_from_backend(const GrBackendRenderTarget& backendRT) {
@@ -509,7 +508,7 @@
if (!backendRT.getMtlTextureInfo(&textureInfo)) {
return nil;
}
- return GrGetMTLTexture(textureInfo.fTexture, GrWrapOwnership::kBorrow_GrWrapOwnership);
+ return GrGetMTLTexture(textureInfo.fTexture);
}
static inline void init_surface_desc(GrSurfaceDesc* surfaceDesc, id<MTLTexture> mtlTexture,
@@ -527,7 +526,7 @@
sk_sp<GrTexture> GrMtlGpu::onWrapBackendTexture(const GrBackendTexture& backendTex,
GrWrapOwnership ownership,
GrWrapCacheable cacheable, GrIOType ioType) {
- id<MTLTexture> mtlTexture = get_texture_from_backend(backendTex, ownership);
+ id<MTLTexture> mtlTexture = get_texture_from_backend(backendTex);
if (!mtlTexture) {
return nullptr;
}
@@ -542,7 +541,7 @@
int sampleCnt,
GrWrapOwnership ownership,
GrWrapCacheable cacheable) {
- id<MTLTexture> mtlTexture = get_texture_from_backend(backendTex, ownership);
+ id<MTLTexture> mtlTexture = get_texture_from_backend(backendTex);
if (!mtlTexture) {
return nullptr;
}
@@ -576,8 +575,7 @@
sk_sp<GrRenderTarget> GrMtlGpu::onWrapBackendTextureAsRenderTarget(
const GrBackendTexture& backendTex, int sampleCnt) {
- id<MTLTexture> mtlTexture = get_texture_from_backend(backendTex,
- GrWrapOwnership::kBorrow_GrWrapOwnership);
+ id<MTLTexture> mtlTexture = get_texture_from_backend(backendTex);
if (!mtlTexture) {
return nullptr;
}
@@ -697,7 +695,7 @@
[cmdBuffer waitUntilCompleted];
transferBuffer = nil;
- info->fTexture = GrReleaseId(testTexture);
+ info->fTexture = GrGetPtrFromId(testTexture);
return true;
}
@@ -802,12 +800,6 @@
void GrMtlGpu::deleteBackendTexture(const GrBackendTexture& tex) {
SkASSERT(GrBackendApi::kMetal == tex.fBackend);
-
- GrMtlTextureInfo info;
- if (tex.getMtlTextureInfo(&info)) {
- // Adopts the metal texture so that ARC will clean it up.
- GrGetMTLTexture(info.fTexture, GrWrapOwnership::kAdopt_GrWrapOwnership);
- }
}
#if GR_TEST_UTILS
@@ -818,8 +810,7 @@
if (!tex.getMtlTextureInfo(&info)) {
return false;
}
- id<MTLTexture> mtlTexture = GrGetMTLTexture(info.fTexture,
- GrWrapOwnership::kBorrow_GrWrapOwnership);
+ id<MTLTexture> mtlTexture = GrGetMTLTexture(info.fTexture);
if (!mtlTexture) {
return false;
}
@@ -855,8 +846,6 @@
GrMtlTextureInfo info;
if (rt.getMtlTextureInfo(&info)) {
this->testingOnly_flushGpuAndSync();
- // Adopts the metal texture so that ARC will clean it up.
- GrGetMTLTexture(info.fTexture, GrWrapOwnership::kAdopt_GrWrapOwnership);
}
}
diff --git a/src/gpu/mtl/GrMtlTrampoline.mm b/src/gpu/mtl/GrMtlTrampoline.mm
index c87223b..b73822e 100644
--- a/src/gpu/mtl/GrMtlTrampoline.mm
+++ b/src/gpu/mtl/GrMtlTrampoline.mm
@@ -17,9 +17,7 @@
const GrContextOptions& options,
void* device,
void* queue) {
- return GrMtlGpu::Make(context,
- options,
- (__bridge_transfer id<MTLDevice>)device,
- (__bridge_transfer id<MTLCommandQueue>)queue);
+ return GrMtlGpu::Make(
+ context, options, (__bridge id<MTLDevice>)device, (__bridge id<MTLCommandQueue>)queue);
}
diff --git a/src/gpu/mtl/GrMtlTypesPriv.cpp b/src/gpu/mtl/GrMtlTypesPriv.cpp
new file mode 100644
index 0000000..1302312
--- /dev/null
+++ b/src/gpu/mtl/GrMtlTypesPriv.cpp
@@ -0,0 +1,43 @@
+/*
+ * Copyright 2019 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "include/private/GrMtlTypesPriv.h"
+
+#include <CoreFoundation/CoreFoundation.h>
+
+GrMtlBackendSurfaceInfo::GrMtlBackendSurfaceInfo(const GrMtlTextureInfo& info) : fTexture(info.fTexture) {
+ CFRetain(fTexture);
+}
+
+GrMtlBackendSurfaceInfo::~GrMtlBackendSurfaceInfo() {
+ SkASSERT(!fTexture);
+}
+
+void GrMtlBackendSurfaceInfo::cleanup() {
+ CFRelease(fTexture);
+ fTexture = nullptr;
+}
+
+void GrMtlBackendSurfaceInfo::assign(const GrMtlBackendSurfaceInfo& that, bool isValid) {
+ CFRetain(that.fTexture);
+ if (isValid)
+ CFRelease(fTexture);
+ fTexture = that.fTexture;
+}
+
+GrMtlTextureInfo GrMtlBackendSurfaceInfo::snapTextureInfo() const {
+ GrMtlTextureInfo textureInfo;
+ textureInfo.fTexture = fTexture;
+ return textureInfo;
+}
+
+#if GR_TEST_UTILS
+bool GrMtlBackendSurfaceInfo::operator==(const GrMtlBackendSurfaceInfo& that) const {
+ return fTexture == that.fTexture;
+}
+#endif
+
diff --git a/src/gpu/mtl/GrMtlUtil.h b/src/gpu/mtl/GrMtlUtil.h
index 2026186..a305b91 100644
--- a/src/gpu/mtl/GrMtlUtil.h
+++ b/src/gpu/mtl/GrMtlUtil.h
@@ -30,28 +30,26 @@
#endif
#endif
+#if !__has_feature(objc_arc)
+#error This file must be compiled with Arc. Use -fobjc-arc flag
+#endif
+
/**
* Returns the Metal texture format for the given GrPixelConfig
*/
bool GrPixelConfigToMTLFormat(GrPixelConfig config, MTLPixelFormat* format);
/**
- * Returns a id<MTLTexture> to the MTLTexture pointed at by the const void*. Will use
- * __bridge_transfer if we are adopting ownership.
+ * Returns a id<MTLTexture> to the MTLTexture pointed at by the const void* (uses __bridge).
*/
-id<MTLTexture> GrGetMTLTexture(const void* mtlTexture, GrWrapOwnership);
+id<MTLTexture> GrGetMTLTexture(const void* mtlTexture);
/**
- * Returns a const void* to whatever the id object is pointing to. Always uses __bridge.
+ * Returns a weak const void* to whatever the id object is pointing to (uses __bridge).
*/
const void* GrGetPtrFromId(id idObject);
/**
- * Returns a const void* to whatever the id object is pointing to. Always uses __bridge_retained.
- */
-const void* GrReleaseId(id idObject);
-
-/**
* Returns a MTLTextureDescriptor which describes the MTLTexture. Useful when creating a duplicate
* MTLTexture without the same storage allocation.
*/
diff --git a/src/gpu/mtl/GrMtlUtil.mm b/src/gpu/mtl/GrMtlUtil.mm
index 3b2b3f3..f3414c9 100644
--- a/src/gpu/mtl/GrMtlUtil.mm
+++ b/src/gpu/mtl/GrMtlUtil.mm
@@ -109,22 +109,14 @@
return false;
}
-id<MTLTexture> GrGetMTLTexture(const void* mtlTexture, GrWrapOwnership wrapOwnership) {
- if (GrWrapOwnership::kAdopt_GrWrapOwnership == wrapOwnership) {
- return (__bridge_transfer id<MTLTexture>)mtlTexture;
- } else {
- return (__bridge id<MTLTexture>)mtlTexture;
- }
+id<MTLTexture> GrGetMTLTexture(const void* mtlTexture) {
+ return (__bridge id<MTLTexture>)mtlTexture;
}
const void* GrGetPtrFromId(id idObject) {
return (__bridge const void*)idObject;
}
-const void* GrReleaseId(id idObject) {
- return (__bridge_retained const void*)idObject;
-}
-
MTLTextureDescriptor* GrGetMTLTextureDescriptor(id<MTLTexture> mtlTexture) {
MTLTextureDescriptor* texDesc = [[MTLTextureDescriptor alloc] init];
texDesc.textureType = mtlTexture.textureType;
@@ -219,7 +211,6 @@
// CPP Utils
GrMTLPixelFormat GrGetMTLPixelFormatFromMtlTextureInfo(const GrMtlTextureInfo& info) {
- id<MTLTexture> mtlTexture = GrGetMTLTexture(info.fTexture,
- GrWrapOwnership::kBorrow_GrWrapOwnership);
+ id<MTLTexture> mtlTexture = GrGetMTLTexture(info.fTexture);
return static_cast<GrMTLPixelFormat>(mtlTexture.pixelFormat);
}
diff --git a/tools/gpu/mtl/MtlTestContext.mm b/tools/gpu/mtl/MtlTestContext.mm
index e822431..eeb1870 100644
--- a/tools/gpu/mtl/MtlTestContext.mm
+++ b/tools/gpu/mtl/MtlTestContext.mm
@@ -124,9 +124,7 @@
void finish() override {}
sk_sp<GrContext> makeGrContext(const GrContextOptions& options) override {
- return GrContext::MakeMetal((__bridge_retained void*)fDevice,
- (__bridge_retained void*)fQueue,
- options);
+ return GrContext::MakeMetal((__bridge void*)fDevice, (__bridge void*)fQueue, options);
}
id<MTLDevice> device() { return fDevice; }
