From efd2fd46aa4b9d7bb121e3312da65d2053f0fa7f Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Fri, 1 May 2020 13:05:23 -0700 Subject: [PATCH] Don't depend on an implicit transaction when presenting drawables on the raster thread. (flutter/engine#18076) The way transactions were added changed in https://github.com/flutter/engine/commit/8500bd4156c1db9da729c3facc51756f2909f65e. This broke rendering using both Metal and OpenGL when no implicit transaction was present on the transaction stack. The failure models differ based on Metal vs. OpenGL and iOS/device versions. On older versions of iOS, rendering would consume memory till exhaustion. On newer iOS versions, rendering would be stuck (till a timeout). This patch brings transaction management back in line with as it was earlier and also makes the Metal backend resilient to transactions being present on the transaction stack at all. Since this is still quite brittle, transaction management must be moved to IOSSurface as a followup. Fixes https://github.com/flutter/flutter/issues/55784. --- .../flutter/shell/gpu/gpu_surface_metal.mm | 5 +++++ .../shell/platform/darwin/ios/ios_surface.mm | 20 ++----------------- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/engine/src/flutter/shell/gpu/gpu_surface_metal.mm b/engine/src/flutter/shell/gpu/gpu_surface_metal.mm index 5ca8b6b4a42..ec96de928c9 100644 --- a/engine/src/flutter/shell/gpu/gpu_surface_metal.mm +++ b/engine/src/flutter/shell/gpu/gpu_surface_metal.mm @@ -58,6 +58,11 @@ std::unique_ptr GPUSurfaceMetal::AcquireFrame(const SkISize& frame ReleaseUnusedDrawableIfNecessary(); + // When there are platform views in the scene, the drawable needs to be presented in the same + // transaction as the one created for platform views. When the drawable are being presented from + // the raster thread, there is no such transaction. + layer_.get().presentsWithTransaction = [[NSThread currentThread] isMainThread]; + auto surface = SkSurface::MakeFromCAMetalLayer(context_.get(), // context layer_.get(), // layer kTopLeft_GrSurfaceOrigin, // origin diff --git a/engine/src/flutter/shell/platform/darwin/ios/ios_surface.mm b/engine/src/flutter/shell/platform/darwin/ios/ios_surface.mm index 5f2725273e0..bb630c3d9be 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/ios_surface.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/ios_surface.mm @@ -90,12 +90,7 @@ void IOSSurface::CancelFrame() { platform_views_controller_->CancelFrame(); // Committing the current transaction as |BeginFrame| will create a nested // CATransaction otherwise. - if ([[NSThread currentThread] isMainThread]) { - // The only time we need to commit the `CATranscation` is when - // there are platform views in the scene, which has to be run on the - // main thread. - [CATransaction commit]; - } + [CATransaction commit]; } // |ExternalViewEmbedder| @@ -103,12 +98,7 @@ void IOSSurface::BeginFrame(SkISize frame_size, GrContext* context, double devic TRACE_EVENT0("flutter", "IOSSurface::BeginFrame"); FML_CHECK(platform_views_controller_ != nullptr); platform_views_controller_->SetFrameSize(frame_size); - if ([[NSThread currentThread] isMainThread]) { - // The only time we need to commit the `CATranscation` is when - // there are platform views in the scene, which has to be run on the - // main thread. - [CATransaction begin]; - } + [CATransaction begin]; } // |ExternalViewEmbedder| @@ -160,12 +150,6 @@ void IOSSurface::EndFrame(fml::RefPtr raster_thread_mer // |ExternalViewEmbedder| void IOSSurface::FinishFrame() { TRACE_EVENT0("flutter", "IOSSurface::DidSubmitFrame"); - if (![[NSThread currentThread] isMainThread]) { - return; - } - // The only time we need to commit the `CATranscation` is when - // there are platform views in the scene, which has to be run on the - // main thread. [CATransaction commit]; } } // namespace flutter