Revert "Change Metal to not take ownership of objects"

This reverts commit 186a295.

Reason for revert: Metal bots failing

Original change's description:
> 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 <[email protected]>
> Reviewed-by: Jim Van Verth <[email protected]>

[email protected],[email protected],[email protected],[email protected]

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: Ie569fe1938857706b5413876a9480ef1eb3314ea
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/216221
Reviewed-by: Jim Van Verth <[email protected]>
Commit-Queue: Jim Van Verth <[email protected]>

Author: jvanverth

gn/gpu.gni

@@ -701,7 +701,6 @@ skia_vk_sources = [
 
 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",
@@ -739,7 +738,6 @@ skia_metal_sources = [
   "$_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",

include/gpu/GrBackendSurface.h

@@ -18,7 +18,6 @@ class GrVkImageLayout;
 
 #ifdef SK_METAL
 #include "include/gpu/mtl/GrMtlTypes.h"
-#include "include/private/GrMtlTypesPriv.h"
 #endif
 
 #if !SK_SUPPORT_GPU
@@ -146,7 +145,6 @@ class SK_API GrBackendTexture {
                      const GrVkImageInfo& vkInfo);
 
 #ifdef SK_METAL
-    // This will take a strong reference to the MTLTexture in mtlInfo.
     GrBackendTexture(int width,
                      int height,
                      GrMipMapped,
@@ -255,11 +253,11 @@ class SK_API GrBackendTexture {
     union {
         GrGLTextureInfo fGLInfo;
         GrVkBackendSurfaceInfo fVkInfo;
-        GrMockTextureInfo fMockInfo;
-    };
 #ifdef SK_METAL
-    GrMtlBackendSurfaceInfo fMtlInfo;
+        GrMtlTextureInfo fMtlInfo;
 #endif
+        GrMockTextureInfo fMockInfo;
+    };
 };
 
 class SK_API GrBackendRenderTarget {
@@ -283,7 +281,6 @@ class SK_API GrBackendRenderTarget {
     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,
@@ -373,16 +370,17 @@ class SK_API GrBackendRenderTarget {
     int fSampleCnt;
     int fStencilBits;
     GrPixelConfig fConfig;
+
     GrBackendApi fBackend;
 
     union {
         GrGLFramebufferInfo fGLInfo;
         GrVkBackendSurfaceInfo fVkInfo;
-        GrMockRenderTargetInfo fMockInfo;
-    };
 #ifdef SK_METAL
-    GrMtlBackendSurfaceInfo fMtlInfo;
+        GrMtlTextureInfo fMtlInfo;
 #endif
+        GrMockRenderTargetInfo fMockInfo;
+    };
 };
 
 #endif

include/gpu/GrContext.h

@@ -61,8 +61,9 @@ class SK_API GrContext : public GrRecordingContext {
 #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 will
-     * be retained by Ganesh, and released when the GrContext is destroyed.
+     * 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.
      */
     static sk_sp<GrContext> MakeMetal(void* device, void* queue, const GrContextOptions& options);
     static sk_sp<GrContext> MakeMetal(void* device, void* queue);

include/gpu/mtl/GrMtlTypes.h

@@ -17,10 +17,8 @@ typedef unsigned int GrMTLPixelFormat;
 
 ///////////////////////////////////////////////////////////////////////////////
 /**
- * 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.
+ * Types for interacting with Metal resources created externally to Skia. Holds the MTLTexture as a
+ * const void*. This is used by GrBackendObjects.
  */
 struct GrMtlTextureInfo {
 public:

include/private/GrMtlTypesPriv.h

@@ -250,11 +250,6 @@ void GrBackendTexture::cleanup() {
         fVkInfo.cleanup();
     }
 #endif
-#ifdef SK_METAL
-    if (this->isValid() && GrBackendApi::kMetal == fBackend) {
-        fMtlInfo.cleanup();
-    }
-#endif
 }
 
 GrBackendTexture::GrBackendTexture(const GrBackendTexture& that) : fIsValid(false) {
@@ -284,7 +279,7 @@ GrBackendTexture& GrBackendTexture::operator=(const GrBackendTexture& that) {
             break;
 #ifdef SK_METAL
         case GrBackendApi::kMetal:
-            fMtlInfo.assign(that.fMtlInfo, this->isValid());
+            fMtlInfo = that.fMtlInfo;
             break;
 #endif
         case GrBackendApi::kMock:
@@ -334,7 +329,7 @@ sk_sp<GrVkImageLayout> GrBackendTexture::getGrVkImageLayout() const {
 #ifdef SK_METAL
 bool GrBackendTexture::getMtlTextureInfo(GrMtlTextureInfo* outInfo) const {
     if (this->isValid() && GrBackendApi::kMetal == fBackend) {
-        *outInfo = fMtlInfo.snapTextureInfo();
+        *outInfo = fMtlInfo;
         return true;
     }
     return false;
@@ -381,8 +376,7 @@ bool GrBackendTexture::isSameTexture(const GrBackendTexture& that) {
 #endif
 #ifdef SK_METAL
         case GrBackendApi::kMetal:
-            return this->fMtlInfo.snapTextureInfo().fTexture ==
-                   that.fMtlInfo.snapTextureInfo().fTexture;
+            return this->fMtlInfo.fTexture == that.fMtlInfo.fTexture;
 #endif
         case GrBackendApi::kMock:
             return fMockInfo.fID == that.fMockInfo.fID;
@@ -554,11 +548,6 @@ void GrBackendRenderTarget::cleanup() {
         fVkInfo.cleanup();
     }
 #endif
-#ifdef SK_METAL
-    if (this->isValid() && GrBackendApi::kMetal == fBackend) {
-        fMtlInfo.cleanup();
-    }
-#endif
 }
 
 GrBackendRenderTarget::GrBackendRenderTarget(const GrBackendRenderTarget& that) : fIsValid(false) {
@@ -589,7 +578,7 @@ GrBackendRenderTarget& GrBackendRenderTarget::operator=(const GrBackendRenderTar
             break;
 #ifdef SK_METAL
         case GrBackendApi::kMetal:
-            fMtlInfo.assign(that.fMtlInfo, this->isValid());
+            fMtlInfo = that.fMtlInfo;
             break;
 #endif
         case GrBackendApi::kMock:
@@ -632,7 +621,7 @@ sk_sp<GrVkImageLayout> GrBackendRenderTarget::getGrVkImageLayout() const {
 #ifdef SK_METAL
 bool GrBackendRenderTarget::getMtlTextureInfo(GrMtlTextureInfo* outInfo) const {
     if (this->isValid() && GrBackendApi::kMetal == fBackend) {
-        *outInfo = fMtlInfo.snapTextureInfo();
+        *outInfo = fMtlInfo;
         return true;
     }
     return false;

src/gpu/GrBackendSurface.cpp

@@ -495,20 +495,21 @@ static bool check_max_blit_width(int widthInPixels) {
     return std::move(tex);
 }
 
-static id<MTLTexture> get_texture_from_backend(const GrBackendTexture& backendTex) {
+static id<MTLTexture> get_texture_from_backend(const GrBackendTexture& backendTex,
+                                               GrWrapOwnership ownership) {
     GrMtlTextureInfo textureInfo;
     if (!backendTex.getMtlTextureInfo(&textureInfo)) {
         return nil;
     }
-    return GrGetMTLTexture(textureInfo.fTexture);
+    return GrGetMTLTexture(textureInfo.fTexture, ownership);
 }
 
 static id<MTLTexture> get_texture_from_backend(const GrBackendRenderTarget& backendRT) {
     GrMtlTextureInfo textureInfo;
     if (!backendRT.getMtlTextureInfo(&textureInfo)) {
         return nil;
     }
-    return GrGetMTLTexture(textureInfo.fTexture);
+    return GrGetMTLTexture(textureInfo.fTexture, GrWrapOwnership::kBorrow_GrWrapOwnership);
 }
 
 static inline void init_surface_desc(GrSurfaceDesc* surfaceDesc, id<MTLTexture> mtlTexture,
@@ -526,7 +527,7 @@ static inline void init_surface_desc(GrSurfaceDesc* surfaceDesc, id<MTLTexture>
 sk_sp<GrTexture> GrMtlGpu::onWrapBackendTexture(const GrBackendTexture& backendTex,
                                                 GrWrapOwnership ownership,
                                                 GrWrapCacheable cacheable, GrIOType ioType) {
-    id<MTLTexture> mtlTexture = get_texture_from_backend(backendTex);
+    id<MTLTexture> mtlTexture = get_texture_from_backend(backendTex, ownership);
     if (!mtlTexture) {
         return nullptr;
     }
@@ -541,7 +542,7 @@ static inline void init_surface_desc(GrSurfaceDesc* surfaceDesc, id<MTLTexture>
                                                           int sampleCnt,
                                                           GrWrapOwnership ownership,
                                                           GrWrapCacheable cacheable) {
-    id<MTLTexture> mtlTexture = get_texture_from_backend(backendTex);
+    id<MTLTexture> mtlTexture = get_texture_from_backend(backendTex, ownership);
     if (!mtlTexture) {
         return nullptr;
     }
@@ -575,7 +576,8 @@ static inline void init_surface_desc(GrSurfaceDesc* surfaceDesc, id<MTLTexture>
 
 sk_sp<GrRenderTarget> GrMtlGpu::onWrapBackendTextureAsRenderTarget(
         const GrBackendTexture& backendTex, int sampleCnt) {
-    id<MTLTexture> mtlTexture = get_texture_from_backend(backendTex);
+    id<MTLTexture> mtlTexture = get_texture_from_backend(backendTex,
+                                                         GrWrapOwnership::kBorrow_GrWrapOwnership);
     if (!mtlTexture) {
         return nullptr;
     }
@@ -695,7 +697,7 @@ static inline void init_surface_desc(GrSurfaceDesc* surfaceDesc, id<MTLTexture>
     [cmdBuffer waitUntilCompleted];
     transferBuffer = nil;
 
-    info->fTexture = GrGetPtrFromId(testTexture);
+    info->fTexture = GrReleaseId(testTexture);
 
     return true;
 }
@@ -800,6 +802,12 @@ static bool mtl_format_to_pixel_config(MTLPixelFormat format, GrPixelConfig* con
 
 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
@@ -810,7 +818,8 @@ static bool mtl_format_to_pixel_config(MTLPixelFormat format, GrPixelConfig* con
     if (!tex.getMtlTextureInfo(&info)) {
         return false;
     }
-    id<MTLTexture> mtlTexture = GrGetMTLTexture(info.fTexture);
+    id<MTLTexture> mtlTexture = GrGetMTLTexture(info.fTexture,
+                                                GrWrapOwnership::kBorrow_GrWrapOwnership);
     if (!mtlTexture) {
         return false;
     }
@@ -846,6 +855,8 @@ static bool mtl_format_to_pixel_config(MTLPixelFormat format, GrPixelConfig* con
     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);
     }
 }
 

src/gpu/mtl/GrMtlGpu.mm

@@ -17,7 +17,9 @@
                                       const GrContextOptions& options,
                                       void* device,
                                       void* queue) {
-    return GrMtlGpu::Make(
-            context, options, (__bridge id<MTLDevice>)device, (__bridge id<MTLCommandQueue>)queue);
+    return GrMtlGpu::Make(context,
+                          options,
+                          (__bridge_transfer id<MTLDevice>)device,
+                          (__bridge_transfer id<MTLCommandQueue>)queue);
 }
 

src/gpu/mtl/GrMtlTrampoline.mm

@@ -30,25 +30,27 @@ class GrSurface;
 #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* (uses __bridge).
+ * Returns a id<MTLTexture> to the MTLTexture pointed at by the const void*. Will use
+ * __bridge_transfer if we are adopting ownership.
  */
-id<MTLTexture> GrGetMTLTexture(const void* mtlTexture);
+id<MTLTexture> GrGetMTLTexture(const void* mtlTexture, GrWrapOwnership);
 
 /**
- * Returns a weak const void* to whatever the id object is pointing to (uses __bridge).
+ * Returns a const void* to whatever the id object is pointing to. Always 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.