Skip to content

Commit

Permalink
Revert "Make the context current before accessing GL in MakeSkiaGpuIm…
Browse files Browse the repository at this point in the history
…age (flutter#40208)"

This reverts commit c7894a6.
  • Loading branch information
swift-kim committed May 18, 2023
1 parent 6531113 commit d8f927a
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 130 deletions.
8 changes: 0 additions & 8 deletions shell/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -325,13 +325,5 @@ if (enable_unittests) {
sources += [ "shell_io_manager_unittests.cc" ]
deps += [ "//third_party/swiftshader" ]
}

if (shell_enable_gl) {
deps += [
"//third_party/angle:libEGL_static",
"//third_party/angle:libGLESv2_static",
"//third_party/swiftshader",
]
}
}
}
4 changes: 0 additions & 4 deletions shell/common/fixtures/shell_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -354,9 +354,6 @@ void scene_with_red_box() {
PlatformDispatcher.instance.scheduleFrame();
}

@pragma('vm:external-name', 'NativeOnBeforeToImageSync')
external void onBeforeToImageSync();


@pragma('vm:entry-point')
Future<void> toImageSync() async {
Expand All @@ -365,7 +362,6 @@ Future<void> 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) {
Expand Down
17 changes: 7 additions & 10 deletions shell/common/rasterizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -294,27 +294,24 @@ std::unique_ptr<Rasterizer::GpuImageResult> 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<SnapshotDelegate::GpuImageResult> 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,
surface = surface_.get(),
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;
}
Expand Down
108 changes: 0 additions & 108 deletions shell/common/shell_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@
#include <utility>
#include <vector>

#if SHELL_ENABLE_GL
#include <EGL/egl.h>
#endif // SHELL_ENABLE_GL

#include "assets/directory_asset_bundle.h"
#include "common/graphics/persistent_cache.h"
#include "flutter/flow/layers/backdrop_filter_layer.h"
Expand Down Expand Up @@ -3959,11 +3955,6 @@ 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.
Expand All @@ -3986,105 +3977,6 @@ TEST_F(ShellTest, PictureToImageSync) {
DestroyShell(std::move(shell));
}

TEST_F(ShellTest, PictureToImageSyncImpellerNoSurface) {
#if !SHELL_ENABLE_METAL
// This test uses the Metal backend.
GTEST_SKIP();
#endif // !SHELL_ENABLE_METAL
auto settings = CreateSettingsForFixture();
settings.enable_impeller = true;
std::unique_ptr<Shell> shell =
CreateShell(settings, //
GetTaskRunnersForFixture(), //
false, //
nullptr, //
false, //
ShellTestPlatformView::BackendType::kMetalBackend //
);

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.
PlatformViewNotifyDestroyed(shell.get());
PlatformViewNotifyCreated(shell.get());
latch.CountDown();
}));

ASSERT_NE(shell, nullptr);
ASSERT_TRUE(shell->IsSetup());
auto configuration = RunConfiguration::InferFromSettings(settings);

// Important: Do not create the platform view yet!
// This test is making sure that the rasterizer can create the texture
// as expected without a surface.

configuration.SetEntrypoint("toImageSync");
RunEngine(shell.get(), std::move(configuration));
PumpOneFrame(shell.get());

latch.Wait();

PlatformViewNotifyDestroyed(shell.get());
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> 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> shell =
Expand Down

0 comments on commit d8f927a

Please sign in to comment.