From c7894a662ae10b3a203c2c608aa5f5529478e3af Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 10 Mar 2023 16:39:04 -0800 Subject: [PATCH] Make the context current before accessing GL in MakeSkiaGpuImage (#40208) Make the context current before accessing GL in MakeSkiaGpuImage --- shell/common/BUILD.gn | 8 ++++ shell/common/fixtures/shell_test.dart | 4 ++ shell/common/rasterizer.cc | 17 +++++--- shell/common/shell_unittests.cc | 61 +++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 7 deletions(-) diff --git a/shell/common/BUILD.gn b/shell/common/BUILD.gn index 7fa0c1b23ae70..d0255379bd84a 100644 --- a/shell/common/BUILD.gn +++ b/shell/common/BUILD.gn @@ -328,5 +328,13 @@ if (enable_unittests) { deps += [ "//third_party/swiftshader" ] } } + + if (shell_enable_gl) { + deps += [ + "//third_party/angle:libEGL_static", + "//third_party/angle:libGLESv2_static", + "//third_party/swiftshader", + ] + } } } diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart index b88289a093f74..5f953fd64823d 100644 --- a/shell/common/fixtures/shell_test.dart +++ b/shell/common/fixtures/shell_test.dart @@ -354,6 +354,9 @@ void scene_with_red_box() { PlatformDispatcher.instance.scheduleFrame(); } +@pragma('vm:external-name', 'NativeOnBeforeToImageSync') +external void onBeforeToImageSync(); + @pragma('vm:entry-point') Future toImageSync() async { @@ -362,6 +365,7 @@ Future toImageSync() async { canvas.drawPaint(Paint()..color = const Color(0xFFAAAAAA)); final Picture picture = recorder.endRecording(); + onBeforeToImageSync(); final Image image = picture.toImageSync(20, 25); void expect(Object? a, Object? b) { if (a != b) { diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 478e5d0309393..d4a43e66b32da 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -289,17 +289,12 @@ std::unique_ptr Rasterizer::MakeSkiaGpuImage( TRACE_EVENT0("flutter", "Rasterizer::MakeGpuImage"); FML_DCHECK(display_list); -// TODO(dnfield): the Linux embedding is in a rough state right now and -// I can't seem to get the GPU path working on it. -// https://github.com/flutter/flutter/issues/108835 -#if FML_OS_LINUX - return MakeBitmapImage(display_list, image_info); -#endif - std::unique_ptr result; delegate_.GetIsGpuDisabledSyncSwitch()->Execute( fml::SyncSwitch::Handlers() .SetIfTrue([&result, &image_info, &display_list] { + // TODO(dnfield): This isn't safe if display_list contains any GPU + // resources like an SkImage_gpu. result = MakeBitmapImage(display_list, image_info); }) .SetIfFalse([&result, &image_info, &display_list, @@ -307,6 +302,14 @@ std::unique_ptr Rasterizer::MakeSkiaGpuImage( gpu_image_behavior = gpu_image_behavior_] { if (!surface || gpu_image_behavior == MakeGpuImageBehavior::kBitmap) { + // TODO(dnfield): This isn't safe if display_list contains any GPU + // resources like an SkImage_gpu. + result = MakeBitmapImage(display_list, image_info); + return; + } + + auto context_switch = surface->MakeRenderContextCurrent(); + if (!context_switch->GetResult()) { result = MakeBitmapImage(display_list, image_info); return; } diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 36c86463d8534..2d1b4f01f9259 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -11,6 +11,10 @@ #include #include +#if SHELL_ENABLE_GL +#include +#endif // SHELL_ENABLE_GL + #include "assets/directory_asset_bundle.h" #include "common/graphics/persistent_cache.h" #include "flutter/flow/layers/backdrop_filter_layer.h" @@ -3890,6 +3894,11 @@ TEST_F(ShellTest, PictureToImageSync) { ShellTestPlatformView::BackendType::kGLBackend // ); + AddNativeCallback("NativeOnBeforeToImageSync", + CREATE_NATIVE_ENTRY([&](auto args) { + // nop + })); + fml::CountDownLatch latch(2); AddNativeCallback("NotifyNative", CREATE_NATIVE_ENTRY([&](auto args) { // Teardown and set up rasterizer again. @@ -3912,6 +3921,58 @@ TEST_F(ShellTest, PictureToImageSync) { DestroyShell(std::move(shell)); } +#if SHELL_ENABLE_GL +// This test uses the GL backend and refers to symbols in egl.h +TEST_F(ShellTest, PictureToImageSyncWithTrampledContext) { + // make it easier to trample the GL context by running on a single task + // runner. + ThreadHost thread_host("io.flutter.test." + GetCurrentTestName() + ".", + ThreadHost::Type::Platform); + auto task_runner = thread_host.platform_thread->GetTaskRunner(); + TaskRunners task_runners("test", task_runner, task_runner, task_runner, + task_runner); + + auto settings = CreateSettingsForFixture(); + std::unique_ptr shell = + CreateShell(settings, // + task_runners, // + false, // + nullptr, // + false, // + ShellTestPlatformView::BackendType::kGLBackend // + ); + + AddNativeCallback( + "NativeOnBeforeToImageSync", CREATE_NATIVE_ENTRY([&](auto args) { + // Trample the GL context. If the rasterizer fails + // to make the right one current again, test will + // fail. + ::eglMakeCurrent(::eglGetCurrentDisplay(), NULL, NULL, NULL); + })); + + fml::CountDownLatch latch(2); + AddNativeCallback("NotifyNative", CREATE_NATIVE_ENTRY([&](auto args) { + // Teardown and set up rasterizer again. + PlatformViewNotifyDestroyed(shell.get()); + PlatformViewNotifyCreated(shell.get()); + latch.CountDown(); + })); + + ASSERT_NE(shell, nullptr); + ASSERT_TRUE(shell->IsSetup()); + auto configuration = RunConfiguration::InferFromSettings(settings); + PlatformViewNotifyCreated(shell.get()); + configuration.SetEntrypoint("toImageSync"); + RunEngine(shell.get(), std::move(configuration)); + PumpOneFrame(shell.get()); + + latch.Wait(); + + PlatformViewNotifyDestroyed(shell.get()); + DestroyShell(std::move(shell), task_runners); +} +#endif // SHELL_ENABLE_GL + TEST_F(ShellTest, PluginUtilitiesCallbackHandleErrorHandling) { auto settings = CreateSettingsForFixture(); std::unique_ptr shell =