-
Notifications
You must be signed in to change notification settings - Fork 6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Android] update fallback and rendering state to combine impeller + android backend. #51008
Changes from 2 commits
abdf41f
b19e86c
c405e38
ed8e432
8547119
37ae3b0
2de48e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,7 +27,6 @@ class MessageLoopImpl; | |
/// \see fml::Wakeable | ||
class MessageLoop { | ||
public: | ||
FML_EMBEDDER_ONLY | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @chinmaygarde not sure what to do here. This macro is flagging usages of this API in flutter_main.cc, it looks like those hadn't been touched in a while. Is there an alternatie usage of this API I can switch to? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the If the embedder knows that it is safe to access the TLS message loop slot, it can Ideally, we should get rid of GetCurrent altogether but that is a migration. |
||
static MessageLoop& GetCurrent(); | ||
|
||
void Run(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jonahwilliams You are running into the message loop error above because this define is not at the top. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤦 Thanks! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While you are at it, https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes for the include order. These includes would go below the related header. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
|
||
#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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this should fault instead of falling back on releases? Let's ask the team. |
||
// 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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can implement ImpellerVulkan -> Skia fallback by changing this value (or adding another configuration setting). |
||
} | ||
return AndroidRenderingAPI::kImpellerVulkan; | ||
} | ||
|
||
return AndroidRenderingAPI::kSkiaOpenGLES; | ||
} | ||
|
||
} // namespace flutter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Android"RenderingAPI going in common is unfortunate. I suppose no more so that common itself. Couldn't this go in
//flutter/shell/common
instead?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not unless common/settings.h can import flutter/common.