From d6095e5be33dac90b8b9e1a96c5bf58cfb01f17d Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Fri, 22 Nov 2024 12:46:39 -0800 Subject: [PATCH] [Impeller] Ensure that SnapshotControllerImpeller has a rendering context before creating the snapshot (flutter/engine#56743) The Skia snapshot controller will activate the delegate's surface render context on the current thread. If the delegate has no surface, then it will use the snapshot surface producer to create a temporary surface. The Impeller snapshot controller needs to do the same in order to support OpenGL/GLES scenarios where the thread does not currently have an EGL context. --- .../common/snapshot_controller_impeller.cc | 28 ++++++++++++++++--- .../android/android_surface_gl_impeller.cc | 19 ++++++++++++- .../platform/embedder/fixtures/main.dart | 17 +++++++++++ .../embedder/tests/embedder_gl_unittests.cc | 27 ++++++++++++++++++ 4 files changed, 86 insertions(+), 5 deletions(-) diff --git a/engine/src/flutter/shell/common/snapshot_controller_impeller.cc b/engine/src/flutter/shell/common/snapshot_controller_impeller.cc index 3da0f19619..5bfbd2113e 100644 --- a/engine/src/flutter/shell/common/snapshot_controller_impeller.cc +++ b/engine/src/flutter/shell/common/snapshot_controller_impeller.cc @@ -18,6 +18,7 @@ namespace flutter { namespace { + sk_sp DoMakeRasterSnapshot( const sk_sp& display_list, SkISize size, @@ -53,6 +54,27 @@ sk_sp DoMakeRasterSnapshot( DlImage::OwningContext::kRaster); } +sk_sp DoMakeRasterSnapshot( + const sk_sp& display_list, + SkISize size, + const SnapshotController::Delegate& delegate) { + // Ensure that the current thread has a rendering context. This must be done + // before calling GetAiksContext because constructing the AiksContext may + // invoke graphics APIs. + std::unique_ptr pbuffer_surface; + if (delegate.GetSurface()) { + delegate.GetSurface()->MakeRenderContextCurrent(); + } else if (delegate.GetSnapshotSurfaceProducer()) { + pbuffer_surface = + delegate.GetSnapshotSurfaceProducer()->CreateSnapshotSurface(); + if (pbuffer_surface) { + pbuffer_surface->MakeRenderContextCurrent(); + } + } + + return DoMakeRasterSnapshot(display_list, size, delegate.GetAiksContext()); +} + sk_sp DoMakeRasterSnapshot( sk_sp display_list, SkISize picture_size, @@ -112,16 +134,14 @@ void SnapshotControllerImpeller::MakeRasterSnapshot( } #endif callback(DoMakeRasterSnapshot(display_list, picture_size, - GetDelegate().GetAiksContext())); + GetDelegate())); })); } sk_sp SnapshotControllerImpeller::MakeRasterSnapshotSync( sk_sp display_list, SkISize picture_size) { - return DoMakeRasterSnapshot(display_list, picture_size, - GetDelegate().GetIsGpuDisabledSyncSwitch(), - GetDelegate().GetAiksContext()); + return DoMakeRasterSnapshot(display_list, picture_size, GetDelegate()); } void SnapshotControllerImpeller::CacheRuntimeStage( diff --git a/engine/src/flutter/shell/platform/android/android_surface_gl_impeller.cc b/engine/src/flutter/shell/platform/android/android_surface_gl_impeller.cc index b2d0b69d9a..405f7f2711 100644 --- a/engine/src/flutter/shell/platform/android/android_surface_gl_impeller.cc +++ b/engine/src/flutter/shell/platform/android/android_surface_gl_impeller.cc @@ -81,7 +81,24 @@ bool AndroidSurfaceGLImpeller::SetNativeWindow( // |AndroidSurface| std::unique_ptr AndroidSurfaceGLImpeller::CreateSnapshotSurface() { - FML_UNREACHABLE(); + if (!onscreen_surface_ || !onscreen_surface_->IsValid()) { + onscreen_surface_ = android_context_->CreateOffscreenSurface(); + if (!onscreen_surface_) { + FML_DLOG(ERROR) << "Could not create offscreen surface for snapshot."; + return nullptr; + } + } + // Make the snapshot surface current because constucting a + // GPUSurfaceGLImpeller and its AiksContext may invoke graphics APIs. + if (!android_context_->OnscreenContextMakeCurrent(onscreen_surface_.get())) { + FML_DLOG(ERROR) << "Could not make snapshot surface current."; + return nullptr; + } + return std::make_unique( + this, // delegate + android_context_->GetImpellerContext(), // context + true // render to surface + ); } // |AndroidSurface| diff --git a/engine/src/flutter/shell/platform/embedder/fixtures/main.dart b/engine/src/flutter/shell/platform/embedder/fixtures/main.dart index adc7bb417b..dabbb1e96a 100644 --- a/engine/src/flutter/shell/platform/embedder/fixtures/main.dart +++ b/engine/src/flutter/shell/platform/embedder/fixtures/main.dart @@ -1595,3 +1595,20 @@ void render_impeller_text_test() { }; PlatformDispatcher.instance.scheduleFrame(); } + +@pragma('vm:entry-point') +// ignore: non_constant_identifier_names +Future render_impeller_image_snapshot_test() async { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + const Color color = Color.fromARGB(255, 0, 0, 123); + canvas.drawPaint(Paint()..color = color); + final Picture picture = recorder.endRecording(); + + final Image image = await picture.toImage(100, 100); + final ByteData? imageData = await image.toByteData(); + final int pixel = imageData!.getInt32(0); + + final bool result = (pixel & 0xFF) == color.alpha && ((pixel >> 8) & 0xFF) == color.blue; + notifyBoolValue(result); +} diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_gl_unittests.cc b/engine/src/flutter/shell/platform/embedder/tests/embedder_gl_unittests.cc index 3592c3711e..11a67df211 100644 --- a/engine/src/flutter/shell/platform/embedder/tests/embedder_gl_unittests.cc +++ b/engine/src/flutter/shell/platform/embedder/tests/embedder_gl_unittests.cc @@ -4814,6 +4814,33 @@ TEST_F(EmbedderTest, CanRenderWithImpellerOpenGL) { ASSERT_FALSE(present_called); } +TEST_F(EmbedderTest, ImpellerOpenGLImageSnapshot) { + auto& context = GetEmbedderContext(); + + bool result = false; + fml::AutoResetWaitableEvent latch; + context.AddNativeCallback("NotifyBoolValue", + CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + result = tonic::DartConverter::FromDart( + Dart_GetNativeArgument(args, 0)); + latch.Signal(); + })); + + EmbedderConfigBuilder builder(context); + builder.AddCommandLineArgument("--enable-impeller"); + builder.SetDartEntrypoint("render_impeller_image_snapshot_test"); + builder.SetSurface(SkISize::Make(800, 600)); + builder.SetCompositor(); + builder.SetRenderTargetType( + EmbedderTestBackingStoreProducer::RenderTargetType::kOpenGLFramebuffer); + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + latch.Wait(); + + ASSERT_TRUE(result); +} + TEST_F(EmbedderTest, CompositorMustBeAbleToRenderToOpenGLSurface) { auto& context = GetEmbedderContext();