[Proposal] Refactor AndroidRenderingAPI to encapsulate Impeller/Skia config #137798
Description
Related to, but non-blocking: #137639.
Note
If you are an assignee of this issue, I'm looking to get your input:
- Should we make this change
- If yes, do you have opinions about the outcome
android_context.h has an enum that looks like this:
enum class AndroidRenderingAPI {
kSoftware,
kOpenGLES,
kVulkan,
/// @brief Attempt to create a Vulkan surface, if that fails then fall back
/// to GLES.
kAutoselect,
};After talking a bit with @jonahwilliams, it appears this has some missing historical context - i.e. it potentially precedes Impeller (flutter/engine#18670), and doesn't encapsulate valid state - for example, it's invalid to have a software backend on Impeller, or a Vulkan backend on Skia.
I'd like to have this encapsulate:
- What backend to use (either Impeller or Skia)
- If Skia, whether to use the (standard) GL backend, or Software
- If Impeller, whether to use our auto-selection logic (Vulkan w/ fallback), Vulkan or fail, or OpenGL.
- Etc
Aside from the best way to do this (unions? std::variant? dumb old struct?) I wanted a quick consensus this is a good change to make and I understand the details. For example, I'd like to be able to change CreateAndroidContext:
// These could be a proper class/struct instead of an increasingly long list of parameters
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,
bool enable_vulkan_validation,
bool enable_opengl_gpu_tracing)We'd refactor some of the logic here:
if (use_software_rendering) {
return std::make_shared<AndroidContext>(AndroidRenderingAPI::kSoftware);
}
if (enable_impeller) {
// 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);
case AndroidRenderingAPI::kAutoselect: {
auto vulkan_backend = std::make_unique<AndroidContextVulkanImpeller>(
enable_vulkan_validation);
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();
}
}The actual impact seems fairly limited (~50, all static, usages of AndroidRenderingAPI across the engine, the vast majority in tests).