From abdf41f20c84494a73e5362a4b88b236e0653a12 Mon Sep 17 00:00:00 2001 From: jonahwilliams <jonahwilliams@google.com> Date: Tue, 27 Feb 2024 10:06:05 -0800 Subject: [PATCH 1/4] [Android] update fallback and renderng state to combine impeller + android backend. --- common/settings.h | 16 +- fml/message_loop.h | 1 - shell/common/switches.cc | 2 +- .../android/android_context_gl_impeller.cc | 2 +- .../android/android_context_gl_skia.cc | 3 +- .../android/android_context_gl_skia.h | 3 +- .../android/android_context_gl_unittests.cc | 26 +-- .../android_context_vulkan_impeller.cc | 24 +-- .../android/android_context_vulkan_impeller.h | 4 +- shell/platform/android/context/BUILD.gn | 1 + .../android/context/android_context.h | 10 +- shell/platform/android/flutter_main.cc | 67 ++++++++ shell/platform/android/flutter_main.h | 3 + .../platform/android/platform_view_android.cc | 150 ++++++------------ .../platform_view_android_unittests.cc | 18 ++- 15 files changed, 187 insertions(+), 143 deletions(-) diff --git a/common/settings.h b/common/settings.h index ec4e8f5a842fa..439f6aca09382 100644 --- a/common/settings.h +++ b/common/settings.h @@ -22,6 +22,14 @@ namespace flutter { +// +enum class AndroidRenderingAPI { + kSoftware, + kImpellerOpenGLES, + kImpellerVulkan, + kSkiaOpenGLES +}; + class FrameTiming { public: enum Phase { @@ -221,8 +229,12 @@ struct Settings { bool enable_impeller = false; #endif - // Requests a particular backend to be used (ex "opengles" or "vulkan") - std::optional<std::string> impeller_backend; + // The selected Android rendering API. + AndroidRenderingAPI android_rendering_api = + AndroidRenderingAPI::kSkiaOpenGLES; + + // Requests a specific rendering backend. + std::optional<std::string> requested_rendering_backend; // Enable Vulkan validation on backends that support it. The validation layers // must be available to the application. diff --git a/fml/message_loop.h b/fml/message_loop.h index ccc1f70b93b8d..28ce4a8a43f9c 100644 --- a/fml/message_loop.h +++ b/fml/message_loop.h @@ -27,7 +27,6 @@ class MessageLoopImpl; /// \see fml::Wakeable class MessageLoop { public: - FML_EMBEDDER_ONLY static MessageLoop& GetCurrent(); void Run(); diff --git a/shell/common/switches.cc b/shell/common/switches.cc index 0c28e81d5af1c..fd3d9508fb8c9 100644 --- a/shell/common/switches.cc +++ b/shell/common/switches.cc @@ -459,7 +459,7 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) { if (command_line.GetOptionValue(FlagForSwitch(Switch::ImpellerBackend), &impeller_backend_value)) { if (!impeller_backend_value.empty()) { - settings.impeller_backend = impeller_backend_value; + settings.requested_rendering_backend = impeller_backend_value; } } } diff --git a/shell/platform/android/android_context_gl_impeller.cc b/shell/platform/android/android_context_gl_impeller.cc index 73cec9af7d9e1..d96e55e878055 100644 --- a/shell/platform/android/android_context_gl_impeller.cc +++ b/shell/platform/android/android_context_gl_impeller.cc @@ -91,7 +91,7 @@ static std::shared_ptr<impeller::Context> CreateImpellerContext( AndroidContextGLImpeller::AndroidContextGLImpeller( std::unique_ptr<impeller::egl::Display> display, bool enable_gpu_tracing) - : AndroidContext(AndroidRenderingAPI::kOpenGLES), + : AndroidContext(AndroidRenderingAPI::kImpellerOpenGLES), reactor_worker_(std::shared_ptr<ReactorWorker>(new ReactorWorker())), display_(std::move(display)) { if (!display_ || !display_->IsValid()) { diff --git a/shell/platform/android/android_context_gl_skia.cc b/shell/platform/android/android_context_gl_skia.cc index a784c6d2259bd..8ad49cf82c5e8 100644 --- a/shell/platform/android/android_context_gl_skia.cc +++ b/shell/platform/android/android_context_gl_skia.cc @@ -65,11 +65,10 @@ static bool TeardownContext(EGLDisplay display, EGLContext context) { } AndroidContextGLSkia::AndroidContextGLSkia( - AndroidRenderingAPI rendering_api, fml::RefPtr<AndroidEnvironmentGL> environment, const TaskRunners& task_runners, uint8_t msaa_samples) - : AndroidContext(AndroidRenderingAPI::kOpenGLES), + : AndroidContext(AndroidRenderingAPI::kSkiaOpenGLES), environment_(std::move(environment)), task_runners_(task_runners) { if (!environment_->IsValid()) { diff --git a/shell/platform/android/android_context_gl_skia.h b/shell/platform/android/android_context_gl_skia.h index 1c9e092ff3a33..af19ef789d8de 100644 --- a/shell/platform/android/android_context_gl_skia.h +++ b/shell/platform/android/android_context_gl_skia.h @@ -27,8 +27,7 @@ class AndroidEGLSurface; /// class AndroidContextGLSkia : public AndroidContext { public: - AndroidContextGLSkia(AndroidRenderingAPI rendering_api, - fml::RefPtr<AndroidEnvironmentGL> environment, + AndroidContextGLSkia(fml::RefPtr<AndroidEnvironmentGL> environment, const TaskRunners& taskRunners, uint8_t msaa_samples); diff --git a/shell/platform/android/android_context_gl_unittests.cc b/shell/platform/android/android_context_gl_unittests.cc index ec6690b912548..bd28354383727 100644 --- a/shell/platform/android/android_context_gl_unittests.cc +++ b/shell/platform/android/android_context_gl_unittests.cc @@ -104,8 +104,8 @@ TEST(AndroidContextGl, Create) { thread_label, ThreadHost::Type::kUi | ThreadHost::Type::kRaster | ThreadHost::Type::kIo)); TaskRunners task_runners = MakeTaskRunners(thread_label, thread_host); - auto context = std::make_unique<AndroidContextGLSkia>( - AndroidRenderingAPI::kOpenGLES, environment, task_runners, 0); + auto context = + std::make_unique<AndroidContextGLSkia>(environment, task_runners, 0); context->SetMainSkiaContext(main_context); EXPECT_NE(context.get(), nullptr); context.reset(); @@ -115,7 +115,7 @@ TEST(AndroidContextGl, Create) { TEST(AndroidContextGl, CreateImpeller) { auto impeller_context = std::make_shared<TestImpellerContext>(); auto android_context = std::make_unique<TestAndroidContext>( - impeller_context, AndroidRenderingAPI::kOpenGLES); + impeller_context, AndroidRenderingAPI::kImpellerOpenGLES); EXPECT_FALSE(impeller_context->did_shutdown); android_context.reset(); @@ -136,8 +136,8 @@ TEST(AndroidContextGl, CreateSingleThread) { TaskRunners task_runners = TaskRunners(thread_label, platform_runner, platform_runner, platform_runner, platform_runner); - auto context = std::make_unique<AndroidContextGLSkia>( - AndroidRenderingAPI::kOpenGLES, environment, task_runners, 0); + auto context = + std::make_unique<AndroidContextGLSkia>(environment, task_runners, 0); context->SetMainSkiaContext(main_context); EXPECT_NE(context.get(), nullptr); context.reset(); @@ -155,8 +155,8 @@ TEST(AndroidSurfaceGL, CreateSnapshopSurfaceWhenOnscreenSurfaceIsNotNull) { thread_label, ThreadHost::Type::kUi | ThreadHost::Type::kRaster | ThreadHost::Type::kIo)); TaskRunners task_runners = MakeTaskRunners(thread_label, thread_host); - auto android_context = std::make_shared<AndroidContextGLSkia>( - AndroidRenderingAPI::kOpenGLES, environment, task_runners, 0); + auto android_context = + std::make_shared<AndroidContextGLSkia>(environment, task_runners, 0); auto android_surface = std::make_unique<AndroidSurfaceGLSkia>(android_context); auto window = fml::MakeRefCounted<AndroidNativeWindow>( @@ -182,8 +182,8 @@ TEST(AndroidSurfaceGL, CreateSnapshopSurfaceWhenOnscreenSurfaceIsNull) { ThreadHost thread_host(host_config); TaskRunners task_runners = MakeTaskRunners(thread_label, thread_host); - auto android_context = std::make_shared<AndroidContextGLSkia>( - AndroidRenderingAPI::kOpenGLES, environment, task_runners, 0); + auto android_context = + std::make_shared<AndroidContextGLSkia>(environment, task_runners, 0); auto android_surface = std::make_unique<AndroidSurfaceGLSkia>(android_context); EXPECT_EQ(android_surface->GetOnscreenSurface(), nullptr); @@ -204,8 +204,8 @@ TEST(AndroidContextGl, DISABLED_MSAAx4) { thread_label, ThreadHost::Type::kUi | ThreadHost::Type::kRaster | ThreadHost::Type::kIo)); TaskRunners task_runners = MakeTaskRunners(thread_label, thread_host); - auto context = std::make_unique<AndroidContextGLSkia>( - AndroidRenderingAPI::kOpenGLES, environment, task_runners, 4); + auto context = + std::make_unique<AndroidContextGLSkia>(environment, task_runners, 4); context->SetMainSkiaContext(main_context); EGLint sample_count; @@ -226,8 +226,8 @@ TEST(AndroidContextGl, EnsureMakeCurrentChecksCurrentContextStatus) { thread_label, ThreadHost::Type::kUi | ThreadHost::Type::kRaster | ThreadHost::Type::kIo)); TaskRunners task_runners = MakeTaskRunners(thread_label, thread_host); - auto context = std::make_unique<AndroidContextGLSkia>( - AndroidRenderingAPI::kOpenGLES, environment, task_runners, 0); + auto context = + std::make_unique<AndroidContextGLSkia>(environment, task_runners, 0); auto pbuffer_surface = context->CreatePbufferSurface(); auto status = pbuffer_surface->MakeCurrent(); diff --git a/shell/platform/android/android_context_vulkan_impeller.cc b/shell/platform/android/android_context_vulkan_impeller.cc index d9d23ebe31122..ecbeea6405f5d 100644 --- a/shell/platform/android/android_context_vulkan_impeller.cc +++ b/shell/platform/android/android_context_vulkan_impeller.cc @@ -19,7 +19,8 @@ namespace flutter { static std::shared_ptr<impeller::Context> CreateImpellerContext( const fml::RefPtr<fml::NativeLibrary>& vulkan_dylib, bool enable_vulkan_validation, - bool enable_gpu_tracing) { + bool enable_gpu_tracing, + bool quiet) { if (!vulkan_dylib) { VALIDATION_LOG << "Could not open the Vulkan dylib."; return nullptr; @@ -57,12 +58,14 @@ static std::shared_ptr<impeller::Context> CreateImpellerContext( auto context = impeller::ContextVK::Create(std::move(settings)); - if (context && impeller::CapabilitiesVK::Cast(*context->GetCapabilities()) - .AreValidationsEnabled()) { - FML_LOG(IMPORTANT) << "Using the Impeller rendering backend (Vulkan with " - "Validation Layers)."; - } else { - FML_LOG(IMPORTANT) << "Using the Impeller rendering backend (Vulkan)."; + if (!quiet) { + if (context && impeller::CapabilitiesVK::Cast(*context->GetCapabilities()) + .AreValidationsEnabled()) { + FML_LOG(IMPORTANT) << "Using the Impeller rendering backend (Vulkan with " + "Validation Layers)."; + } else { + FML_LOG(IMPORTANT) << "Using the Impeller rendering backend (Vulkan)."; + } } return context; @@ -70,11 +73,12 @@ static std::shared_ptr<impeller::Context> CreateImpellerContext( AndroidContextVulkanImpeller::AndroidContextVulkanImpeller( bool enable_validation, - bool enable_gpu_tracing) - : AndroidContext(AndroidRenderingAPI::kVulkan), + bool enable_gpu_tracing, + bool quiet) + : AndroidContext(AndroidRenderingAPI::kImpellerVulkan), vulkan_dylib_(fml::NativeLibrary::Create("libvulkan.so")) { auto impeller_context = CreateImpellerContext( - vulkan_dylib_, enable_validation, enable_gpu_tracing); + vulkan_dylib_, enable_validation, enable_gpu_tracing, quiet); SetImpellerContext(impeller_context); is_valid_ = !!impeller_context; } diff --git a/shell/platform/android/android_context_vulkan_impeller.h b/shell/platform/android/android_context_vulkan_impeller.h index 266843d534f15..cd9be6946ad76 100644 --- a/shell/platform/android/android_context_vulkan_impeller.h +++ b/shell/platform/android/android_context_vulkan_impeller.h @@ -14,7 +14,9 @@ namespace flutter { class AndroidContextVulkanImpeller : public AndroidContext { public: - AndroidContextVulkanImpeller(bool enable_validation, bool enable_gpu_tracing); + AndroidContextVulkanImpeller(bool enable_validation, + bool enable_gpu_tracing, + bool quiet = false); ~AndroidContextVulkanImpeller(); diff --git a/shell/platform/android/context/BUILD.gn b/shell/platform/android/context/BUILD.gn index 1de85c8144376..31c5f0ecb95ab 100644 --- a/shell/platform/android/context/BUILD.gn +++ b/shell/platform/android/context/BUILD.gn @@ -14,6 +14,7 @@ source_set("context") { public_configs = [ "//flutter:config" ] deps = [ + "//flutter/common", "//flutter/fml", "//flutter/impeller/renderer", "//flutter/skia", diff --git a/shell/platform/android/context/android_context.h b/shell/platform/android/context/android_context.h index e49ddcf58ffd6..598787fd70117 100644 --- a/shell/platform/android/context/android_context.h +++ b/shell/platform/android/context/android_context.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_SHELL_PLATFORM_ANDROID_CONTEXT_ANDROID_CONTEXT_H_ #define FLUTTER_SHELL_PLATFORM_ANDROID_CONTEXT_ANDROID_CONTEXT_H_ +#include "common/settings.h" #include "flutter/fml/macros.h" #include "flutter/fml/task_runner.h" #include "flutter/impeller/renderer/context.h" @@ -12,15 +13,6 @@ namespace flutter { -enum class AndroidRenderingAPI { - kSoftware, - kOpenGLES, - kVulkan, - /// @brief Attempt to create a Vulkan surface, if that fails then fall back - /// to GLES. - kAutoselect, -}; - //------------------------------------------------------------------------------ /// @brief Holds state that is shared across Android surfaces. /// diff --git a/shell/platform/android/flutter_main.cc b/shell/platform/android/flutter_main.cc index 6ad57a7ca04cc..e018ed5c1a53b 100644 --- a/shell/platform/android/flutter_main.cc +++ b/shell/platform/android/flutter_main.cc @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "common/settings.h" +#include "impeller/base/validation.h" #define FML_USED_ON_EMBEDDER #include "flutter/shell/platform/android/flutter_main.h" @@ -13,6 +15,7 @@ #include "flutter/fml/command_line.h" #include "flutter/fml/file.h" +#include "flutter/fml/logging.h" #include "flutter/fml/macros.h" #include "flutter/fml/message_loop.h" #include "flutter/fml/native_library.h" @@ -25,11 +28,14 @@ #include "flutter/runtime/dart_vm.h" #include "flutter/shell/common/shell.h" #include "flutter/shell/common/switches.h" +#include "flutter/shell/platform/android/android_context_vulkan_impeller.h" #include "third_party/dart/runtime/include/dart_tools_api.h" #include "txt/platform.h" namespace flutter { +constexpr int kMinimumAndroidApiLevelForVulkan = 29; + extern "C" { #if FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG // Used for debugging dart:* sources. @@ -91,6 +97,18 @@ void FlutterMain::Init(JNIEnv* env, } } + settings.android_rendering_api = SelectedRenderingAPI(settings); + switch (settings.android_rendering_api) { + case AndroidRenderingAPI::kSoftware: + case AndroidRenderingAPI::kSkiaOpenGLES: + settings.enable_impeller = false; + break; + case AndroidRenderingAPI::kImpellerOpenGLES: + case AndroidRenderingAPI::kImpellerVulkan: + settings.enable_impeller = true; + break; + } + #if FLUTTER_RELEASE // On most platforms the timeline is always disabled in release mode. // On Android, enable it in release mode only when using systrace. @@ -211,4 +229,53 @@ bool FlutterMain::Register(JNIEnv* env) { return env->RegisterNatives(clazz, methods, fml::size(methods)) == 0; } +// static +AndroidRenderingAPI FlutterMain::SelectedRenderingAPI( + const flutter::Settings& settings) { + if (settings.enable_software_rendering) { + FML_CHECK(!settings.enable_impeller) + << "Impeller does not support software rendering. Either disable " + "software rendering or disable impeller."; + return AndroidRenderingAPI::kSoftware; + } + // Debug/Profile only functionality for testing a specific + // backend configuration. +#ifndef FLUTTER_RELEASE + if (settings.requested_rendering_backend == "opengles" & + settings.enable_impeller) { + return AndroidRenderingAPI::kImpellerOpenGLES; + } + if (settings.requested_rendering_backend == "vulkan" && + settings.enable_impeller) { + return AndroidRenderingAPI::kImpellerVulkan; + } +#endif + + if (settings.enable_impeller) { + // Vulkan must only be used on API level 29+, as older API levels do not + // have requisite features to support platform views. + // + // Even if this check returns true, Impeller may determine it cannot use + // Vulkan for some other reason, such as a missing required extension or + // feature. + int api_level = android_get_device_api_level(); + if (api_level < kMinimumAndroidApiLevelForVulkan) { + return AndroidRenderingAPI::kImpellerOpenGLES; + } + // Determine if Vulkan is supported by creating a Vulkan context and + // checking if it is valid. + impeller::ScopedValidationDisable disable_validation; + auto vulkan_backend = std::make_unique<AndroidContextVulkanImpeller>( + /*enable_vulkan_validation=*/false, + /*enable_vulkan_gpu_tracing=*/false, + /*quiet=*/true); + if (!vulkan_backend->IsValid()) { + return AndroidRenderingAPI::kImpellerOpenGLES; + } + return AndroidRenderingAPI::kImpellerVulkan; + } + + return AndroidRenderingAPI::kSkiaOpenGLES; +} + } // namespace flutter diff --git a/shell/platform/android/flutter_main.h b/shell/platform/android/flutter_main.h index 5bd9f298a1c5a..c572b458fbc29 100644 --- a/shell/platform/android/flutter_main.h +++ b/shell/platform/android/flutter_main.h @@ -23,6 +23,9 @@ class FlutterMain { const flutter::Settings& GetSettings() const; + static AndroidRenderingAPI SelectedRenderingAPI( + const flutter::Settings& settings); + private: const flutter::Settings settings_; DartServiceIsolate::CallbackHandle vm_service_uri_callback_ = 0; diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index b8d1f80fafe8b..4dea4b432e38c 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -20,6 +20,7 @@ #include "flutter/shell/platform/android/android_surface_software.h" #include "flutter/shell/platform/android/image_external_texture_gl.h" #include "flutter/shell/platform/android/surface_texture_external_texture_gl.h" +#include "fml/logging.h" #if IMPELLER_ENABLE_VULKAN // b/258506856 for why this is behind an if #include "flutter/shell/platform/android/android_surface_vulkan_impeller.h" #include "flutter/shell/platform/android/image_external_texture_vk.h" @@ -47,100 +48,46 @@ std::unique_ptr<AndroidSurface> AndroidSurfaceFactoryImpl::CreateSurface() { switch (android_context_->RenderingApi()) { case AndroidRenderingAPI::kSoftware: return std::make_unique<AndroidSurfaceSoftware>(); - case AndroidRenderingAPI::kOpenGLES: - if (enable_impeller_) { - return std::make_unique<AndroidSurfaceGLImpeller>( - std::static_pointer_cast<AndroidContextGLImpeller>( - android_context_)); - } else { - return std::make_unique<AndroidSurfaceGLSkia>( - std::static_pointer_cast<AndroidContextGLSkia>(android_context_)); - } - case AndroidRenderingAPI::kVulkan: - FML_DCHECK(enable_impeller_); + case AndroidRenderingAPI::kImpellerOpenGLES: + return std::make_unique<AndroidSurfaceGLImpeller>( + std::static_pointer_cast<AndroidContextGLImpeller>(android_context_)); + case AndroidRenderingAPI::kSkiaOpenGLES: + return std::make_unique<AndroidSurfaceGLSkia>( + std::static_pointer_cast<AndroidContextGLSkia>(android_context_)); + case AndroidRenderingAPI::kImpellerVulkan: return std::make_unique<AndroidSurfaceVulkanImpeller>( std::static_pointer_cast<AndroidContextVulkanImpeller>( android_context_)); - default: - FML_DCHECK(false); - return nullptr; } + FML_UNREACHABLE(); } static std::shared_ptr<flutter::AndroidContext> CreateAndroidContext( bool use_software_rendering, const flutter::TaskRunners& task_runners, uint8_t msaa_samples, - bool enable_impeller, - const std::optional<std::string>& impeller_backend, + AndroidRenderingAPI android_rendering_api, bool enable_vulkan_validation, bool enable_opengl_gpu_tracing, bool enable_vulkan_gpu_tracing) { - if (use_software_rendering) { - FML_DCHECK(!enable_impeller); - return std::make_shared<AndroidContext>(AndroidRenderingAPI::kSoftware); - } - if (enable_impeller) { - // Vulkan must only be used on API level 29+, as older API levels do not - // have requisite features to support platform views. - // - // Even if this check returns true, Impeller may determine it cannot use - // Vulkan for some other reason, such as a missing required extension or - // feature. - int api_level = android_get_device_api_level(); - if (api_level < kMinimumAndroidApiLevelForVulkan) { - if (impeller_backend.value_or("") == "vulkan") { - FML_LOG(WARNING) - << "Impeller Vulkan requested, but detected Android API level " - << api_level - << " does not support required features for Vulkan with platform " - "views. Falling back to OpenGLES."; - } + switch (android_rendering_api) { + case AndroidRenderingAPI::kSoftware: + return std::make_shared<AndroidContext>(AndroidRenderingAPI::kSoftware); + case AndroidRenderingAPI::kImpellerOpenGLES: return std::make_unique<AndroidContextGLImpeller>( std::make_unique<impeller::egl::Display>(), enable_opengl_gpu_tracing); - } - - // Default value is Vulkan with GLES fallback. - AndroidRenderingAPI backend = AndroidRenderingAPI::kAutoselect; - if (impeller_backend.has_value()) { - if (impeller_backend.value() == "opengles") { - backend = AndroidRenderingAPI::kOpenGLES; - } else if (impeller_backend.value() == "vulkan") { - backend = AndroidRenderingAPI::kVulkan; - } else { - FML_CHECK(impeller_backend.value() == "vulkan" || - impeller_backend.value() == "opengles"); - } - } - switch (backend) { - case AndroidRenderingAPI::kOpenGLES: - return std::make_unique<AndroidContextGLImpeller>( - std::make_unique<impeller::egl::Display>(), - enable_opengl_gpu_tracing); - case AndroidRenderingAPI::kVulkan: - return std::make_unique<AndroidContextVulkanImpeller>( - enable_vulkan_validation, enable_vulkan_gpu_tracing); - case AndroidRenderingAPI::kAutoselect: { - auto vulkan_backend = std::make_unique<AndroidContextVulkanImpeller>( - enable_vulkan_validation, enable_vulkan_gpu_tracing); - if (!vulkan_backend->IsValid()) { - return std::make_unique<AndroidContextGLImpeller>( - std::make_unique<impeller::egl::Display>(), - enable_opengl_gpu_tracing); - } - return vulkan_backend; - } - default: - FML_UNREACHABLE(); - } + case AndroidRenderingAPI::kImpellerVulkan: + return std::make_unique<AndroidContextVulkanImpeller>( + enable_vulkan_validation, enable_vulkan_gpu_tracing); + case AndroidRenderingAPI::kSkiaOpenGLES: + return std::make_unique<AndroidContextGLSkia>( + fml::MakeRefCounted<AndroidEnvironmentGL>(), // + task_runners, // + msaa_samples // + ); } - return std::make_unique<AndroidContextGLSkia>( - AndroidRenderingAPI::kOpenGLES, // - fml::MakeRefCounted<AndroidEnvironmentGL>(), // - task_runners, // - msaa_samples // - ); + FML_UNREACHABLE(); } PlatformViewAndroid::PlatformViewAndroid( @@ -157,8 +104,7 @@ PlatformViewAndroid::PlatformViewAndroid( use_software_rendering, task_runners, msaa_samples, - delegate.OnPlatformViewGetSettings().enable_impeller, - delegate.OnPlatformViewGetSettings().impeller_backend, + delegate.OnPlatformViewGetSettings().android_rendering_api, delegate.OnPlatformViewGetSettings().enable_vulkan_validation, delegate.OnPlatformViewGetSettings().enable_opengl_gpu_tracing, delegate.OnPlatformViewGetSettings().enable_vulkan_gpu_tracing)) { @@ -333,48 +279,56 @@ void PlatformViewAndroid::UpdateSemantics( void PlatformViewAndroid::RegisterExternalTexture( int64_t texture_id, const fml::jni::ScopedJavaGlobalRef<jobject>& surface_texture) { - if (android_context_->RenderingApi() == AndroidRenderingAPI::kOpenGLES) { - if (android_context_->GetImpellerContext()) { + switch (android_context_->RenderingApi()) { + case AndroidRenderingAPI::kImpellerOpenGLES: // Impeller GLES. RegisterTexture(std::make_shared<SurfaceTextureExternalTextureImpellerGL>( std::static_pointer_cast<impeller::ContextGLES>( android_context_->GetImpellerContext()), texture_id, surface_texture, jni_facade_)); - } else { + break; + case AndroidRenderingAPI::kSkiaOpenGLES: // Legacy GL. RegisterTexture(std::make_shared<SurfaceTextureExternalTextureGL>( texture_id, surface_texture, jni_facade_)); - } - } else { - FML_LOG(INFO) << "Attempted to use a SurfaceTextureExternalTexture with an " - "unsupported rendering API."; + break; + case AndroidRenderingAPI::kSoftware: + case AndroidRenderingAPI::kImpellerVulkan: + FML_LOG(INFO) + << "Attempted to use a SurfaceTextureExternalTexture with an " + "unsupported rendering API."; + break; } } void PlatformViewAndroid::RegisterImageTexture( int64_t texture_id, const fml::jni::ScopedJavaGlobalRef<jobject>& image_texture_entry) { - if (android_context_->RenderingApi() == AndroidRenderingAPI::kOpenGLES) { - if (android_context_->GetImpellerContext()) { + switch (android_context_->RenderingApi()) { + case AndroidRenderingAPI::kImpellerOpenGLES: // Impeller GLES. RegisterTexture(std::make_shared<ImageExternalTextureGLImpeller>( std::static_pointer_cast<impeller::ContextGLES>( android_context_->GetImpellerContext()), texture_id, image_texture_entry, jni_facade_)); - } else { + break; + case AndroidRenderingAPI::kSkiaOpenGLES: // Legacy GL. RegisterTexture(std::make_shared<ImageExternalTextureGLSkia>( std::static_pointer_cast<AndroidContextGLSkia>(android_context_), texture_id, image_texture_entry, jni_facade_)); - } - } else if (android_context_->RenderingApi() == AndroidRenderingAPI::kVulkan) { - RegisterTexture(std::make_shared<ImageExternalTextureVK>( - std::static_pointer_cast<impeller::ContextVK>( - android_context_->GetImpellerContext()), - texture_id, image_texture_entry, jni_facade_)); - } else { - FML_LOG(INFO) << "Attempted to use a HardwareBuffer texture with an " - "unsupported rendering API."; + break; + case AndroidRenderingAPI::kImpellerVulkan: + RegisterTexture(std::make_shared<ImageExternalTextureVK>( + std::static_pointer_cast<impeller::ContextVK>( + android_context_->GetImpellerContext()), + texture_id, image_texture_entry, jni_facade_)); + break; + case AndroidRenderingAPI::kSoftware: + FML_LOG(INFO) + << "Attempted to use a SurfaceTextureExternalTexture with an " + "unsupported rendering API."; + break; } } diff --git a/shell/platform/android/platform_view_android_unittests.cc b/shell/platform/android/platform_view_android_unittests.cc index c757c71001508..2aec13434d599 100644 --- a/shell/platform/android/platform_view_android_unittests.cc +++ b/shell/platform/android/platform_view_android_unittests.cc @@ -6,6 +6,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include "shell/platform/android/context/android_context.h" +#include "shell/platform/android/flutter_main.h" #include "third_party/googletest/googlemock/include/gmock/gmock-nice-strict.h" namespace flutter { @@ -107,7 +108,7 @@ TEST(AndroidPlatformView, SelectsVulkanBasedOnApiLevel) { Settings settings; settings.enable_software_rendering = false; settings.enable_impeller = true; - settings.impeller_backend = "vulkan"; + settings.requested_rendering_backend = "vulkan"; NiceMock<MockPlatformViewDelegate> mock_delegate; EXPECT_CALL(mock_delegate, OnPlatformViewGetSettings) .WillRepeatedly(ReturnRef(settings)); @@ -123,11 +124,22 @@ TEST(AndroidPlatformView, SelectsVulkanBasedOnApiLevel) { int api_level = android_get_device_api_level(); EXPECT_GT(api_level, 0); if (api_level >= 29) { - EXPECT_TRUE(context->RenderingApi() == AndroidRenderingAPI::kVulkan); + EXPECT_TRUE(context->RenderingApi() == + AndroidRenderingAPI::kImpellerVulkan); } else { - EXPECT_TRUE(context->RenderingApi() == AndroidRenderingAPI::kOpenGLES); + EXPECT_TRUE(context->RenderingApi() == + AndroidRenderingAPI::kImpellerOpenGLES); } } +TEST(AndroidPlatformView, SoftwareRenderingNotSupportedWithImpeller) { + Settings settings; + settings.enable_software_rendering = true; + settings.enable_impeller = true; + + ASSERT_DEATH(FlutterMain::SelectedRenderingAPI(settings), + "Impeller does not support software rendering"); +} + } // namespace testing } // namespace flutter From b19e86cd0352a8a21e89352efe473c550472d601 Mon Sep 17 00:00:00 2001 From: jonahwilliams <jonahwilliams@google.com> Date: Tue, 27 Feb 2024 10:17:04 -0800 Subject: [PATCH 2/4] add doc comment --- common/settings.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/settings.h b/common/settings.h index 439f6aca09382..a62f1b44a82cb 100644 --- a/common/settings.h +++ b/common/settings.h @@ -22,7 +22,7 @@ namespace flutter { -// +// The combination of targeted graphics API and Impeller support. enum class AndroidRenderingAPI { kSoftware, kImpellerOpenGLES, From c405e38ec35b9ee3e07dbe175904a8072e17323c Mon Sep 17 00:00:00 2001 From: jonahwilliams <jonahwilliams@google.com> Date: Tue, 27 Feb 2024 10:44:18 -0800 Subject: [PATCH 3/4] chinmay review and test cleanup. --- fml/message_loop.h | 1 + shell/platform/android/flutter_main.cc | 8 +- .../platform_view_android_unittests.cc | 114 +----------------- 3 files changed, 8 insertions(+), 115 deletions(-) diff --git a/fml/message_loop.h b/fml/message_loop.h index 28ce4a8a43f9c..ccc1f70b93b8d 100644 --- a/fml/message_loop.h +++ b/fml/message_loop.h @@ -27,6 +27,7 @@ class MessageLoopImpl; /// \see fml::Wakeable class MessageLoop { public: + FML_EMBEDDER_ONLY static MessageLoop& GetCurrent(); void Run(); diff --git a/shell/platform/android/flutter_main.cc b/shell/platform/android/flutter_main.cc index e018ed5c1a53b..d3ad35ee6b23b 100644 --- a/shell/platform/android/flutter_main.cc +++ b/shell/platform/android/flutter_main.cc @@ -2,17 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "common/settings.h" -#include "impeller/base/validation.h" #define FML_USED_ON_EMBEDDER -#include "flutter/shell/platform/android/flutter_main.h" - #include <android/log.h> - #include <optional> #include <vector> +#include "common/settings.h" #include "flutter/fml/command_line.h" #include "flutter/fml/file.h" #include "flutter/fml/logging.h" @@ -29,6 +25,8 @@ #include "flutter/shell/common/shell.h" #include "flutter/shell/common/switches.h" #include "flutter/shell/platform/android/android_context_vulkan_impeller.h" +#include "flutter/shell/platform/android/flutter_main.h" +#include "impeller/base/validation.h" #include "third_party/dart/runtime/include/dart_tools_api.h" #include "txt/platform.h" diff --git a/shell/platform/android/platform_view_android_unittests.cc b/shell/platform/android/platform_view_android_unittests.cc index 2aec13434d599..f61700b08c51c 100644 --- a/shell/platform/android/platform_view_android_unittests.cc +++ b/shell/platform/android/platform_view_android_unittests.cc @@ -2,133 +2,27 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/shell/platform/android/platform_view_android.h" #include "gmock/gmock.h" #include "gtest/gtest.h" -#include "shell/platform/android/context/android_context.h" #include "shell/platform/android/flutter_main.h" #include "third_party/googletest/googlemock/include/gmock/gmock-nice-strict.h" namespace flutter { namespace testing { -using ::testing::NiceMock; -using ::testing::ReturnRef; - -namespace { -class MockPlatformViewDelegate : public PlatformView::Delegate { - public: - MOCK_METHOD(void, - OnPlatformViewCreated, - (std::unique_ptr<Surface> surface), - (override)); - - MOCK_METHOD(void, OnPlatformViewDestroyed, (), (override)); - - MOCK_METHOD(void, OnPlatformViewScheduleFrame, (), (override)); - - MOCK_METHOD(void, - OnPlatformViewSetNextFrameCallback, - (const fml::closure& closure), - (override)); - - MOCK_METHOD(void, - OnPlatformViewSetViewportMetrics, - (int64_t view_id, const ViewportMetrics& metrics), - (override)); - - MOCK_METHOD(void, - OnPlatformViewDispatchPlatformMessage, - (std::unique_ptr<PlatformMessage> message), - (override)); - - MOCK_METHOD(void, - OnPlatformViewDispatchPointerDataPacket, - (std::unique_ptr<PointerDataPacket> packet), - (override)); - - MOCK_METHOD(void, - OnPlatformViewDispatchSemanticsAction, - (int32_t id, SemanticsAction action, fml::MallocMapping args), - (override)); - - MOCK_METHOD(void, - OnPlatformViewSetSemanticsEnabled, - (bool enabled), - (override)); - - MOCK_METHOD(void, - OnPlatformViewSetAccessibilityFeatures, - (int32_t flags), - (override)); - - MOCK_METHOD(void, - OnPlatformViewRegisterTexture, - (std::shared_ptr<Texture> texture), - (override)); - - MOCK_METHOD(void, - OnPlatformViewUnregisterTexture, - (int64_t texture_id), - (override)); - - MOCK_METHOD(void, - OnPlatformViewMarkTextureFrameAvailable, - (int64_t texture_id), - (override)); - - MOCK_METHOD(const Settings&, - OnPlatformViewGetSettings, - (), - (const, override)); - - MOCK_METHOD(void, - LoadDartDeferredLibrary, - (intptr_t loading_unit_id, - std::unique_ptr<const fml::Mapping> snapshot_data, - std::unique_ptr<const fml::Mapping> snapshot_instructions), - (override)); - - MOCK_METHOD(void, - LoadDartDeferredLibraryError, - (intptr_t loading_unit_id, - const std::string error_message, - bool transient), - (override)); - - MOCK_METHOD(void, - UpdateAssetResolverByType, - (std::unique_ptr<AssetResolver> updated_asset_resolver, - AssetResolver::AssetResolverType type), - (override)); -}; -} // namespace - TEST(AndroidPlatformView, SelectsVulkanBasedOnApiLevel) { Settings settings; settings.enable_software_rendering = false; settings.enable_impeller = true; - settings.requested_rendering_backend = "vulkan"; - NiceMock<MockPlatformViewDelegate> mock_delegate; - EXPECT_CALL(mock_delegate, OnPlatformViewGetSettings) - .WillRepeatedly(ReturnRef(settings)); - TaskRunners task_runners("test", nullptr, nullptr, nullptr, nullptr); - PlatformViewAndroid platform_view(/*delegate=*/mock_delegate, - /*task_runners=*/task_runners, - /*jni_facade=*/nullptr, - /*use_software_rendering=*/false, - /*msaa_samples=*/1); - auto context = platform_view.GetAndroidContext(); - EXPECT_TRUE(context); int api_level = android_get_device_api_level(); EXPECT_GT(api_level, 0); if (api_level >= 29) { - EXPECT_TRUE(context->RenderingApi() == - AndroidRenderingAPI::kImpellerVulkan); + EXPECT_EQ(FlutterMain::SelectedRenderingAPI(settings), + AndroidRenderingAPI::kImpellerVulkan); } else { - EXPECT_TRUE(context->RenderingApi() == - AndroidRenderingAPI::kImpellerOpenGLES); + EXPECT_EQ(FlutterMain::SelectedRenderingAPI(settings), + AndroidRenderingAPI::kImpellerOpenGLES); } } From ed8e43219c382afa77fd77d179cc094525fc38b1 Mon Sep 17 00:00:00 2001 From: jonahwilliams <jonahwilliams@google.com> Date: Tue, 27 Feb 2024 10:48:30 -0800 Subject: [PATCH 4/4] remove unused code. --- shell/platform/android/platform_view_android.cc | 2 -- shell/platform/android/platform_view_android_unittests.cc | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 4dea4b432e38c..15abb985bb73a 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -35,8 +35,6 @@ namespace flutter { -constexpr int kMinimumAndroidApiLevelForVulkan = 29; - AndroidSurfaceFactoryImpl::AndroidSurfaceFactoryImpl( const std::shared_ptr<AndroidContext>& context, bool enable_impeller) diff --git a/shell/platform/android/platform_view_android_unittests.cc b/shell/platform/android/platform_view_android_unittests.cc index f61700b08c51c..fc1c2d9d1b17d 100644 --- a/shell/platform/android/platform_view_android_unittests.cc +++ b/shell/platform/android/platform_view_android_unittests.cc @@ -31,8 +31,7 @@ TEST(AndroidPlatformView, SoftwareRenderingNotSupportedWithImpeller) { settings.enable_software_rendering = true; settings.enable_impeller = true; - ASSERT_DEATH(FlutterMain::SelectedRenderingAPI(settings), - "Impeller does not support software rendering"); + ASSERT_DEATH(FlutterMain::SelectedRenderingAPI(settings), ""); } } // namespace testing