From 14999479b01d56d1ee4c84858e19bbe0642e3705 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Thu, 21 Nov 2024 22:47:58 +0000 Subject: [PATCH 1/2] [Impeller] Ensure that SnapshotControllerImpeller has a rendering context before creating the snapshot 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. --- shell/common/snapshot_controller_impeller.cc | 28 ++++++++++++++++--- .../android/android_surface_gl_impeller.cc | 19 ++++++++++++- shell/platform/embedder/fixtures/main.dart | 18 ++++++++++++ .../embedder/tests/embedder_gl_unittests.cc | 27 ++++++++++++++++++ 4 files changed, 87 insertions(+), 5 deletions(-) diff --git a/shell/common/snapshot_controller_impeller.cc b/shell/common/snapshot_controller_impeller.cc index 3da0f1961910f..5bfbd2113e39b 100644 --- a/shell/common/snapshot_controller_impeller.cc +++ b/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/shell/platform/android/android_surface_gl_impeller.cc b/shell/platform/android/android_surface_gl_impeller.cc index b2d0b69d9a9cf..405f7f2711f63 100644 --- a/shell/platform/android/android_surface_gl_impeller.cc +++ b/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/shell/platform/embedder/fixtures/main.dart b/shell/platform/embedder/fixtures/main.dart index adc7bb417b05c..e8597b78f1e9c 100644 --- a/shell/platform/embedder/fixtures/main.dart +++ b/shell/platform/embedder/fixtures/main.dart @@ -1595,3 +1595,21 @@ void render_impeller_text_test() { }; PlatformDispatcher.instance.scheduleFrame(); } + +@pragma('vm:entry-point') +// ignore: non_constant_identifier_names +void render_impeller_image_snapshot_test() async { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + const int alpha = 255; + const int blue = 123; + canvas.drawPaint(Paint()..color = Color.fromARGB(alpha, 0, 0, blue)); + 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) == alpha && ((pixel >> 8) & 0xFF) == blue; + notifyBoolValue(result); +} diff --git a/shell/platform/embedder/tests/embedder_gl_unittests.cc b/shell/platform/embedder/tests/embedder_gl_unittests.cc index 3592c3711ecb9..11a67df211ee2 100644 --- a/shell/platform/embedder/tests/embedder_gl_unittests.cc +++ b/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(); From 792d8803d7ad0f6b6aac56def614c870ed3a6ab3 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Fri, 22 Nov 2024 19:48:56 +0000 Subject: [PATCH 2/2] analyzer --- shell/platform/embedder/fixtures/main.dart | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/shell/platform/embedder/fixtures/main.dart b/shell/platform/embedder/fixtures/main.dart index e8597b78f1e9c..dabbb1e96ab7e 100644 --- a/shell/platform/embedder/fixtures/main.dart +++ b/shell/platform/embedder/fixtures/main.dart @@ -1598,18 +1598,17 @@ void render_impeller_text_test() { @pragma('vm:entry-point') // ignore: non_constant_identifier_names -void render_impeller_image_snapshot_test() async { +Future render_impeller_image_snapshot_test() async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); - const int alpha = 255; - const int blue = 123; - canvas.drawPaint(Paint()..color = Color.fromARGB(alpha, 0, 0, blue)); + 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) == alpha && ((pixel >> 8) & 0xFF) == blue; + final bool result = (pixel & 0xFF) == color.alpha && ((pixel >> 8) & 0xFF) == color.blue; notifyBoolValue(result); }