Delay metal drawable acquisition till frame submission. (flutter#13367)

Uses the new `SkSurface::MakeFromCAMetalLayer` Skia API.
engine-flutter-autoroll · Oct 28, 2019 · 4044876 · 4044876
1 parent be14974
commit 4044876
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 50 deletions.
diff --git a/shell/gpu/gpu_surface_metal.h b/shell/gpu/gpu_surface_metal.h
@@ -31,6 +31,7 @@ class GPUSurfaceMetal : public Surface {
   fml::scoped_nsobject<CAMetalLayer> layer_;
   sk_sp<GrContext> context_;
   fml::scoped_nsprotocol<id<MTLCommandQueue>> command_queue_;
+  GrMTLHandle next_drawable_ = nullptr;
 
   // |Surface|
   bool IsValid() override;
@@ -50,6 +51,8 @@ class GPUSurfaceMetal : public Surface {
   // |Surface|
   bool MakeRenderContextCurrent() override;
 
+  void ReleaseUnusedDrawableIfNecessary();
+
   FML_DISALLOW_COPY_AND_ASSIGN(GPUSurfaceMetal);
 };
 

diff --git a/shell/gpu/gpu_surface_metal.mm b/shell/gpu/gpu_surface_metal.mm
@@ -10,6 +10,8 @@
 #include "third_party/skia/include/gpu/GrBackendSurface.h"
 #include "third_party/skia/include/ports/SkCFObject.h"
 
+static_assert(!__has_feature(objc_arc), "ARC must be disabled.");
+
 namespace flutter {
 
 GPUSurfaceMetal::GPUSurfaceMetal(GPUSurfaceDelegate* delegate,
@@ -68,7 +70,9 @@
   command_queue_ = metal_queue;
 }
 
-GPUSurfaceMetal::~GPUSurfaceMetal() = default;
+GPUSurfaceMetal::~GPUSurfaceMetal() {
+  ReleaseUnusedDrawableIfNecessary();
+}
 
 // |Surface|
 bool GPUSurfaceMetal::IsValid() {
@@ -93,65 +97,48 @@
     return nullptr;
   }
 
-  const auto scale = layer_.get().contentsScale;
+  ReleaseUnusedDrawableIfNecessary();
 
-  auto next_drawable = fml::scoped_nsprotocol<id<CAMetalDrawable>>([[layer_ nextDrawable] retain]);
-  if (!next_drawable) {
-    FML_LOG(ERROR) << "Could not acquire next metal drawable.";
-    return nullptr;
-  }
+  auto surface = SkSurface::MakeFromCAMetalLayer(context_.get(),            // context
+                                                 layer_.get(),              // layer
+                                                 kTopLeft_GrSurfaceOrigin,  // origin
+                                                 1,                         // sample count
+                                                 kBGRA_8888_SkColorType,    // color type
+                                                 nullptr,                   // colorspace
+                                                 nullptr,                   // surface properties
+                                                 &next_drawable_  // drawable (transfer out)
+  );
 
-  auto metal_texture = fml::scoped_nsprotocol<id<MTLTexture>>([next_drawable.get().texture retain]);
-  if (!metal_texture) {
-    FML_LOG(ERROR) << "Could not acquire metal texture from drawable.";
+  if (!surface) {
+    FML_LOG(ERROR) << "Could not create the SkSurface from the metal texture.";
     return nullptr;
   }
 
-  GrMtlTextureInfo metal_texture_info;
-  metal_texture_info.fTexture.reset(SkCFSafeRetain(metal_texture.get()));
-
-  GrBackendRenderTarget metal_render_target(bounds.width * scale,   // width
-                                            bounds.height * scale,  // height
-                                            1,                      // sample count
-                                            metal_texture_info      // metal texture info
-  );
+  auto submit_callback = [this](const SurfaceFrame& surface_frame, SkCanvas* canvas) -> bool {
+    canvas->flush();
 
-  auto command_buffer =
-      fml::scoped_nsprotocol<id<MTLCommandBuffer>>([[command_queue_.get() commandBuffer] retain]);
+    if (next_drawable_ == nullptr) {
+      FML_DLOG(ERROR) << "Could not acquire next Metal drawable from the SkSurface.";
+      return false;
+    }
 
-  SkSurface::RenderTargetReleaseProc release_proc = [](SkSurface::ReleaseContext context) {
-    [reinterpret_cast<id>(context) release];
-  };
+    const auto has_external_view_embedder = delegate_->GetExternalViewEmbedder() != nullptr;
 
-  auto surface =
-      SkSurface::MakeFromBackendRenderTarget(context_.get(),            // context
-                                             metal_render_target,       // backend render target
-                                             kTopLeft_GrSurfaceOrigin,  // origin
-                                             kBGRA_8888_SkColorType,    // color type
-                                             nullptr,                   // colorspace
-                                             nullptr,                   // surface properties
-                                             release_proc,              // release proc
-                                             metal_texture.release()    // release context (texture)
-      );
+    auto command_buffer =
+        fml::scoped_nsprotocol<id<MTLCommandBuffer>>([[command_queue_.get() commandBuffer] retain]);
 
-  if (!surface) {
-    FML_LOG(ERROR) << "Could not create the SkSurface from the metal texture.";
-    return nullptr;
-  }
+    fml::scoped_nsprotocol<id<CAMetalDrawable>> drawable(
+        reinterpret_cast<id<CAMetalDrawable>>(next_drawable_));
+    next_drawable_ = nullptr;
 
-  bool hasExternalViewEmbedder = delegate_->GetExternalViewEmbedder() != nullptr;
-
-  // External views need to present with transaction. When presenting with
-  // transaction, we have to block, otherwise we risk presenting the drawable
-  // after the CATransaction has completed.
-  // See:
-  // https://developer.apple.com/documentation/quartzcore/cametallayer/1478157-presentswithtransaction
-  // TODO(dnfield): only do this if transactions are actually being used.
-  // https://github.com/flutter/flutter/issues/24133
-  auto submit_callback = [drawable = next_drawable, command_buffer, hasExternalViewEmbedder](
-                             const SurfaceFrame& surface_frame, SkCanvas* canvas) -> bool {
-    canvas->flush();
-    if (!hasExternalViewEmbedder) {
+    // External views need to present with transaction. When presenting with
+    // transaction, we have to block, otherwise we risk presenting the drawable
+    // after the CATransaction has completed.
+    // See:
+    // https://developer.apple.com/documentation/quartzcore/cametallayer/1478157-presentswithtransaction
+    // TODO(dnfield): only do this if transactions are actually being used.
+    // https://github.com/flutter/flutter/issues/24133
+    if (!has_external_view_embedder) {
       [command_buffer.get() presentDrawable:drawable.get()];
       [command_buffer.get() commit];
     } else {
@@ -190,4 +177,16 @@ GrBackendRenderTarget metal_render_target(bounds.width * scale,   // width
   return true;
 }
 
+void GPUSurfaceMetal::ReleaseUnusedDrawableIfNecessary() {
+  // If the previous surface frame was not submitted before  a new one is acquired, the old drawable
+  // needs to be released. An RAII wrapper may not be used because this needs to interoperate with
+  // Skia APIs.
+  if (next_drawable_ == nullptr) {
+    return;
+  }
+
+  CFRelease(next_drawable_);
+  next_drawable_ = nullptr;
+}
+
 }  // namespace flutter