Reland "Use a Pbuffer surface when the onscreen surface is not available for snapshotting on Android (flutter#27141)" (flutter#27607)"

This reverts commit 0c121d3.

Author: dnfield

BUILD.gn

@@ -121,6 +121,11 @@ group("flutter") {
     public_deps += [ "//flutter/testing/scenario_app" ]
   }
 
+  if (is_android && flutter_runtime_mode == "profile" &&
+      current_cpu == "arm64") {
+    public_deps += [ "//flutter/testing/android_background_image" ]
+  }
+
   # Compile all unittests targets if enabled.
   if (enable_unittests) {
     public_deps += [

ci/firebase_testlab.py

@@ -19,28 +19,28 @@
 
 
 def RunFirebaseTest(apk, results_dir):
-  try:
-    # game-loop tests are meant for OpenGL apps.
-    # This type of test will give the application a handle to a file, and
-    # we'll write the timeline JSON to that file.
-    # See https://firebase.google.com/docs/test-lab/android/game-loop
-    # Pixel 4. As of this commit, this is a highly available device in FTL.
-    subprocess.check_output([
-        'gcloud',
-        '--project', 'flutter-infra',
-        'firebase', 'test', 'android', 'run',
-        '--type', 'game-loop',
-        '--app', apk,
-        '--timeout', '2m',
-        '--results-bucket', bucket,
-        '--results-dir', results_dir,
-        '--device', 'model=flame,version=29',
-    ])
-  except subprocess.CalledProcessError as ex:
-    print(ex.output)
-    # Recipe will retry return codes from firebase that indicate an infra
-    # failure.
-    sys.exit(ex.returncode)
+  # game-loop tests are meant for OpenGL apps.
+  # This type of test will give the application a handle to a file, and
+  # we'll write the timeline JSON to that file.
+  # See https://firebase.google.com/docs/test-lab/android/game-loop
+  # Pixel 4. As of this commit, this is a highly available device in FTL.
+  process = subprocess.Popen(
+    [
+      'gcloud',
+      '--project', 'flutter-infra',
+      'firebase', 'test', 'android', 'run',
+      '--type', 'game-loop',
+      '--app', apk,
+      '--timeout', '2m',
+      '--results-bucket', bucket,
+      '--results-dir', results_dir,
+      '--device', 'model=flame,version=29',
+    ],
+    stdout=subprocess.PIPE,
+    stderr=subprocess.STDOUT,
+    universal_newlines=True,
+  )
+  return process
 
 
 def CheckLogcat(results_dir):
@@ -87,13 +87,25 @@ def main():
   git_revision = subprocess.check_output(
       ['git', 'rev-parse', 'HEAD'], cwd=script_dir).strip()
 
+  results = []
   for apk in apks:
     results_dir = '%s/%s/%s' % (os.path.basename(apk), git_revision, args.build_id)
-
-    RunFirebaseTest(apk, results_dir)
+    process = RunFirebaseTest(apk, results_dir)
+    results.append((results_dir, process))
+
+  for results_dir, process in results:
+    for line in iter(process.stdout.readline, ""):
+      print(line.strip())
+    return_code = process.wait()
+    if return_code != 0:
+      print('Firebase test failed ' + returncode)
+      sys.exit(process.returncode)
+
+    print('Checking logcat for %s' % results_dir)
     CheckLogcat(results_dir)
     # scenario_app produces a timeline, but the android image test does not.
     if 'scenario' in apk:
+      print('Checking timeline for %s' % results_dir)
       CheckTimeline(results_dir)
 
   return 0

ci/licenses_golden/licenses_flutter

@@ -725,6 +725,7 @@ FILE: ../../../flutter/shell/common/shell_unittests.cc
 FILE: ../../../flutter/shell/common/skia_event_tracer_impl.cc
 FILE: ../../../flutter/shell/common/skia_event_tracer_impl.h
 FILE: ../../../flutter/shell/common/skp_shader_warmup_unittests.cc
+FILE: ../../../flutter/shell/common/snapshot_surface_producer.h
 FILE: ../../../flutter/shell/common/switches.cc
 FILE: ../../../flutter/shell/common/switches.h
 FILE: ../../../flutter/shell/common/thread_host.cc
@@ -932,6 +933,8 @@ FILE: ../../../flutter/shell/platform/android/surface/android_surface.cc
 FILE: ../../../flutter/shell/platform/android/surface/android_surface.h
 FILE: ../../../flutter/shell/platform/android/surface/android_surface_mock.cc
 FILE: ../../../flutter/shell/platform/android/surface/android_surface_mock.h
+FILE: ../../../flutter/shell/platform/android/surface/snapshot_surface_producer.cc
+FILE: ../../../flutter/shell/platform/android/surface/snapshot_surface_producer.h
 FILE: ../../../flutter/shell/platform/android/vsync_waiter_android.cc
 FILE: ../../../flutter/shell/platform/android/vsync_waiter_android.h
 FILE: ../../../flutter/shell/platform/common/accessibility_bridge.cc

shell/common/BUILD.gn

@@ -89,6 +89,7 @@ source_set("common") {
     "shell_io_manager.h",
     "skia_event_tracer_impl.cc",
     "skia_event_tracer_impl.h",
+    "snapshot_surface_producer.h",
     "switches.cc",
     "switches.h",
     "thread_host.cc",

shell/common/platform_view.cc

@@ -67,7 +67,6 @@ void PlatformView::SetViewportMetrics(const ViewportMetrics& metrics) {
 
 void PlatformView::NotifyCreated() {
   std::unique_ptr<Surface> surface;
-
   // Threading: We want to use the platform view on the non-platform thread.
   // Using the weak pointer is illegal. But, we are going to introduce a latch
   // so that the platform view is not collected till the surface is obtained.
@@ -182,4 +181,9 @@ void PlatformView::UpdateAssetResolverByType(
   delegate_.UpdateAssetResolverByType(std::move(updated_asset_resolver), type);
 }
 
+std::unique_ptr<SnapshotSurfaceProducer>
+PlatformView::CreateSnapshotSurfaceProducer() {
+  return nullptr;
+}
+
 }  // namespace flutter

shell/common/platform_view.h

@@ -62,7 +62,7 @@ class PlatformView {
     ///             Metal, Vulkan) specific. This is usually a sign to the
     ///             rasterizer to set up and begin rendering to that surface.
     ///
-    /// @param[in]  surface  The surface
+    /// @param[in]  surface           The surface
     ///
     virtual void OnPlatformViewCreated(std::unique_ptr<Surface> surface) = 0;
 
@@ -796,6 +796,22 @@ class PlatformView {
       std::unique_ptr<AssetResolver> updated_asset_resolver,
       AssetResolver::AssetResolverType type);
 
+  //--------------------------------------------------------------------------
+  /// @brief      Creates an object that produces surfaces suitable for raster
+  ///             snapshotting. The rasterizer will request this surface if no
+  ///             on screen surface is currently available when an application
+  ///             requests a snapshot, e.g. if `Scene.toImage` or
+  ///             `Picture.toImage` are called while the application is in the
+  ///             background.
+  ///
+  ///             Not all backends support this kind of surface usage, and the
+  ///             default implementation returns nullptr. Platforms should
+  ///             override this if they can support GPU operations in the
+  ///             background and support GPU resource context usage.
+  ///
+  virtual std::unique_ptr<SnapshotSurfaceProducer>
+  CreateSnapshotSurfaceProducer();
+
  protected:
   PlatformView::Delegate& delegate_;
   const TaskRunners task_runners_;
@@ -804,8 +820,7 @@ class PlatformView {
   SkISize size_;
   fml::WeakPtrFactory<PlatformView> weak_factory_;
 
-  // Unlike all other methods on the platform view, this is called on the
-  // raster task runner.
+  // This is the only method called on the raster task runner.
   virtual std::unique_ptr<Surface> CreateRenderingSurface();
 
  private:

shell/common/rasterizer.cc

@@ -258,11 +258,22 @@ sk_sp<SkImage> Rasterizer::DoMakeRasterSnapshot(
   sk_sp<SkImage> result;
   SkImageInfo image_info = SkImageInfo::MakeN32Premul(
       size.width(), size.height(), SkColorSpace::MakeSRGB());
-  if (surface_ == nullptr || surface_->GetContext() == nullptr) {
+
+  std::unique_ptr<Surface> pbuffer_surface;
+  Surface* snapshot_surface = nullptr;
+  if (surface_ && surface_->GetContext()) {
+    snapshot_surface = surface_.get();
+  } else if (snapshot_surface_producer_) {
+    pbuffer_surface = snapshot_surface_producer_->CreateSnapshotSurface();
+    if (pbuffer_surface && pbuffer_surface->GetContext())
+      snapshot_surface = pbuffer_surface.get();
+  }
+
+  if (!snapshot_surface) {
     // Raster surface is fine if there is no on screen surface. This might
     // happen in case of software rendering.
-    sk_sp<SkSurface> surface = SkSurface::MakeRaster(image_info);
-    result = DrawSnapshot(surface, draw_callback);
+    sk_sp<SkSurface> sk_surface = SkSurface::MakeRaster(image_info);
+    result = DrawSnapshot(sk_surface, draw_callback);
   } else {
     delegate_.GetIsGpuDisabledSyncSwitch()->Execute(
         fml::SyncSwitch::Handlers()
@@ -271,12 +282,14 @@ sk_sp<SkImage> Rasterizer::DoMakeRasterSnapshot(
               result = DrawSnapshot(surface, draw_callback);
             })
             .SetIfFalse([&] {
-              auto context_switch = surface_->MakeRenderContextCurrent();
+              FML_DCHECK(snapshot_surface);
+              auto context_switch =
+                  snapshot_surface->MakeRenderContextCurrent();
               if (!context_switch->GetResult()) {
                 return;
               }
 
-              GrRecordingContext* context = surface_->GetContext();
+              GrRecordingContext* context = snapshot_surface->GetContext();
               auto max_size = context->maxRenderTargetSize();
               double scale_factor = std::min(
                   1.0, static_cast<double>(max_size) /
@@ -294,19 +307,19 @@ sk_sp<SkImage> Rasterizer::DoMakeRasterSnapshot(
 
               // When there is an on screen surface, we need a render target
               // SkSurface because we want to access texture backed images.
-              sk_sp<SkSurface> surface =
+              sk_sp<SkSurface> sk_surface =
                   SkSurface::MakeRenderTarget(context,          // context
                                               SkBudgeted::kNo,  // budgeted
                                               image_info        // image info
                   );
-              if (!surface) {
+              if (!sk_surface) {
                 FML_LOG(ERROR)
                     << "DoMakeRasterSnapshot can not create GPU render target";
                 return;
               }
 
-              surface->getCanvas()->scale(scale_factor, scale_factor);
-              result = DrawSnapshot(surface, draw_callback);
+              sk_surface->getCanvas()->scale(scale_factor, scale_factor);
+              result = DrawSnapshot(sk_surface, draw_callback);
             }));
   }
 
@@ -700,6 +713,11 @@ void Rasterizer::SetExternalViewEmbedder(
   external_view_embedder_ = view_embedder;
 }
 
+void Rasterizer::SetSnapshotSurfaceProducer(
+    std::unique_ptr<SnapshotSurfaceProducer> producer) {
+  snapshot_surface_producer_ = std::move(producer);
+}
+
 void Rasterizer::FireNextFrameCallbackIfPresent() {
   if (!next_frame_callback_) {
     return;

shell/common/rasterizer.h

@@ -24,6 +24,7 @@
 #include "flutter/fml/time/time_point.h"
 #include "flutter/lib/ui/snapshot_delegate.h"
 #include "flutter/shell/common/pipeline.h"
+#include "flutter/shell/common/snapshot_surface_producer.h"
 
 namespace flutter {
 
@@ -98,7 +99,7 @@ class Rasterizer final : public SnapshotDelegate {
   ///             currently only created by the shell (which also sets itself up
   ///             as the rasterizer delegate).
   ///
-  /// @param[in]  delegate            The rasterizer delegate.
+  /// @param[in]  delegate                   The rasterizer delegate.
   ///
   Rasterizer(Delegate& delegate);
 
@@ -349,6 +350,17 @@ class Rasterizer final : public SnapshotDelegate {
   void SetExternalViewEmbedder(
       const std::shared_ptr<ExternalViewEmbedder>& view_embedder);
 
+  //----------------------------------------------------------------------------
+  /// @brief Set the snapshot surface producer. This is done on shell
+  ///        initialization. This is non-null on platforms that support taking
+  ///        GPU accelerated raster snapshots in the background.
+  ///
+  /// @param[in]  producer  A surface producer for raster snapshotting when the
+  ///                       onscreen surface is not available.
+  ///
+  void SetSnapshotSurfaceProducer(
+      std::unique_ptr<SnapshotSurfaceProducer> producer);
+
   //----------------------------------------------------------------------------
   /// @brief      Returns a pointer to the compositor context used by this
   ///             rasterizer. This pointer will never be `nullptr`.
@@ -434,6 +446,7 @@ class Rasterizer final : public SnapshotDelegate {
  private:
   Delegate& delegate_;
   std::unique_ptr<Surface> surface_;
+  std::unique_ptr<SnapshotSurfaceProducer> snapshot_surface_producer_;
   std::unique_ptr<flutter::CompositorContext> compositor_context_;
   // This is the last successfully rasterized layer tree.
   std::unique_ptr<flutter::LayerTree> last_layer_tree_;

shell/common/rasterizer_unittests.cc

@@ -30,6 +30,7 @@ class MockDelegate : public Rasterizer::Delegate {
   MOCK_CONST_METHOD0(GetTaskRunners, const TaskRunners&());
   MOCK_CONST_METHOD0(GetIsGpuDisabledSyncSwitch,
                      std::shared_ptr<const fml::SyncSwitch>());
+  MOCK_METHOD0(CreateSnapshotSurface, std::unique_ptr<Surface>());
 };
 
 class MockSurface : public Surface {

shell/common/shell.cc

@@ -634,6 +634,8 @@ bool Shell::Setup(std::unique_ptr<PlatformView> platform_view,
   // Set the external view embedder for the rasterizer.
   auto view_embedder = platform_view_->CreateExternalViewEmbedder();
   rasterizer_->SetExternalViewEmbedder(view_embedder);
+  rasterizer_->SetSnapshotSurfaceProducer(
+      platform_view_->CreateSnapshotSurfaceProducer());
 
   // The weak ptr must be generated in the platform thread which owns the unique
   // ptr.