From 73a175c890f366d910877abec2845ca256768e2c Mon Sep 17 00:00:00 2001
From: Chinmay Garde <chinmaygarde@google.com>
Date: Sun, 22 Mar 2020 17:28:56 -0700
Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20depend=20on=20an=20implicit=20t?=
 =?UTF-8?q?ransaction=20when=20no=20external=20view=20embedder=20is=20pres?=
 =?UTF-8?q?ent.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

All Metal layers have their presentsWithTransaction property set to true.
However, when an external view embedder is not present, there is no mechanism to
ensure that the command buffer commit is within transaction scope. This works in
most cases as there there is usually an implicit (possibly nested) transaction
in place during rendering. However, when there isn’t, rendering will look paused
at an incorrect size. This code now works similar to OpenGL but will be
refactored for ease of understanding and consistency between the various
backends.
---
 shell/gpu/gpu_surface_metal.mm | 40 +++++++++-------------------------
 1 file changed, 10 insertions(+), 30 deletions(-)

diff --git a/shell/gpu/gpu_surface_metal.mm b/shell/gpu/gpu_surface_metal.mm
index 154e951319bdb..5ca8b6b4a42d5 100644
--- a/shell/gpu/gpu_surface_metal.mm
+++ b/shell/gpu/gpu_surface_metal.mm
@@ -39,32 +39,25 @@
 }
 
 // |Surface|
-std::unique_ptr<SurfaceFrame> GPUSurfaceMetal::AcquireFrame(const SkISize& size) {
+std::unique_ptr<SurfaceFrame> GPUSurfaceMetal::AcquireFrame(const SkISize& frame_size) {
   if (!IsValid()) {
     FML_LOG(ERROR) << "Metal surface was invalid.";
     return nullptr;
   }
 
-  if (size.isEmpty()) {
+  if (frame_size.isEmpty()) {
     FML_LOG(ERROR) << "Metal surface was asked for an empty frame.";
     return nullptr;
   }
 
-  const auto bounds = layer_.get().bounds.size;
-  const auto scale = layer_.get().contentsScale;
-  if (bounds.width <= 0.0 || bounds.height <= 0.0 || scale <= 0.0) {
-    FML_LOG(ERROR) << "Metal layer bounds were invalid.";
-    return nullptr;
-  }
+  const auto drawable_size = CGSizeMake(frame_size.width(), frame_size.height());
 
-  const auto scaled_bounds = CGSizeMake(bounds.width * scale, bounds.height * scale);
+  if (!CGSizeEqualToSize(drawable_size, layer_.get().drawableSize)) {
+    layer_.get().drawableSize = drawable_size;
+  }
 
   ReleaseUnusedDrawableIfNecessary();
 
-  if (!CGSizeEqualToSize(scaled_bounds, layer_.get().drawableSize)) {
-    layer_.get().drawableSize = scaled_bounds;
-  }
-
   auto surface = SkSurface::MakeFromCAMetalLayer(context_.get(),            // context
                                                  layer_.get(),              // layer
                                                  kTopLeft_GrSurfaceOrigin,  // origin
@@ -89,8 +82,6 @@
       return false;
     }
 
-    const auto has_external_view_embedder = delegate_->GetExternalViewEmbedder() != nullptr;
-
     auto command_buffer =
         fml::scoped_nsprotocol<id<MTLCommandBuffer>>([[command_queue_.get() commandBuffer] retain]);
 
@@ -98,21 +89,10 @@
         reinterpret_cast<id<CAMetalDrawable>>(next_drawable_));
     next_drawable_ = 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
-    if (!has_external_view_embedder) {
-      [command_buffer.get() presentDrawable:drawable.get()];
-      [command_buffer.get() commit];
-    } else {
-      [command_buffer.get() commit];
-      [command_buffer.get() waitUntilScheduled];
-      [drawable.get() present];
-    }
+    [command_buffer.get() commit];
+    [command_buffer.get() waitUntilScheduled];
+    [drawable.get() present];
+
     return true;
   };