From 49e8a3e7be24c23ef337624af102e264d8763bd6 Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Thu, 6 Apr 2023 14:12:02 -0700 Subject: [PATCH] [Impeller] Removed requirement for multisample buffers from egl setup. (flutter/engine#40944) I reviewed the code and I don't think this requirement is necessary. Offscreen MSAA is already guarded by a flag and looking at the opengles code it doesn't seem to be using multisample buffers. When removing it the emulator no longer crashes and behaves identically to on device (which is rendering incorrect colors and crashing when pressing a the counter button). fixes https://github.com/flutter/flutter/issues/105323 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [ ] I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --- .../src/flutter/impeller/toolkit/egl/display.cc | 4 ++-- .../android/android_surface_gl_impeller.cc | 15 ++++++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/engine/src/flutter/impeller/toolkit/egl/display.cc b/engine/src/flutter/impeller/toolkit/egl/display.cc index b1c4fca1a3..7cabc314e6 100644 --- a/engine/src/flutter/impeller/toolkit/egl/display.cc +++ b/engine/src/flutter/impeller/toolkit/egl/display.cc @@ -141,9 +141,9 @@ std::unique_ptr Display::ChooseConfig(ConfigDescriptor config) const { if (sample_count > 1) { attributes.push_back(EGL_SAMPLE_BUFFERS); attributes.push_back(1); + attributes.push_back(EGL_SAMPLES); + attributes.push_back(sample_count); } - attributes.push_back(EGL_SAMPLES); - attributes.push_back(sample_count); } // termination sentinel must be present. 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 3200822776..ca4040ec5e 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 @@ -98,10 +98,19 @@ AndroidSurfaceGLImpeller::AndroidSurfaceGLImpeller( desc.samples = impeller::egl::Samples::kFour; desc.surface_type = impeller::egl::SurfaceType::kWindow; - auto onscreen_config = display->ChooseConfig(desc); + std::unique_ptr onscreen_config = + display->ChooseConfig(desc); if (!onscreen_config) { - FML_DLOG(ERROR) << "Could not choose onscreen config."; - return; + // Fallback for Android emulator. + desc.samples = impeller::egl::Samples::kOne; + onscreen_config = display->ChooseConfig(desc); + if (onscreen_config) { + FML_LOG(INFO) << "Warning: This device doesn't support MSAA for onscreen " + "framebuffers. Falling back to a single sample."; + } else { + FML_DLOG(ERROR) << "Could not choose onscreen config."; + return; + } } desc.surface_type = impeller::egl::SurfaceType::kPBuffer;