Fix RE's VulkanInterface destruction & unnecessary initialization
SurfaceFlinger's initialization of RE now:
- Only attempts to check for VK support if either the GaneshVk or
GraphiteVk flag is set.
- Caches the VulkanInterface created to check for VK support until a VK
instance of RE is created.
- Tears down a partially initialized VulkanInterface if some required
feature is unsupported.
Additionally, SkiaVkRenderEngine's destructor now tears down the static
VulkanInterfaces it uses, which necessitates:
- Caching whether VK is supported.
- Ensuring all Skia resources are destroyed *before* the VK resources
managed by VulkanInterface that they rely on are torn down. This
involves ensuring all textures are destroyed, and all Skia
context-like objects are destroyed.
This latter change means that tests in librenderengine_test that are
parameterized by Skia backend must now recreate RE's VulkanInterfaces
twice for each test case (once for GaneshVk, and again for GraphiteVk),
which results in a minor regression in test duration. However, this is
necessary because holding on to a VulkanInterface while attempting to
set up a test for GaneshGL will cause contention over the real-time
GPU context priority, which can only be held exclusively by either a GL
context OR a VK context on some hardware.
Many thanks to joseph.cheng@imgtec.com for raising the issue of SF's
init of RE not tearing down the VulkanInterface used to check for
support, and for proposing an initial patch (b/333477752) which this
change builds upon. And thanks to scroggo@google.com for proposing the
reordering of SF's flag vs. VK support checks.
Bug: b/293371537
Bug: b/333477752
Test: librenderengine_test && manual boot validation across 3 backends
Flag: com.android.graphics.surfaceflinger.flags.graphite_renderengine-READ-ONLY
Change-Id: I289c3f7699d16707d1462179f4d5e8c54e4bb049
diff --git a/libs/renderengine/skia/SkiaVkRenderEngine.cpp b/libs/renderengine/skia/SkiaVkRenderEngine.cpp
index 406fd81..bd50107 100644
--- a/libs/renderengine/skia/SkiaVkRenderEngine.cpp
+++ b/libs/renderengine/skia/SkiaVkRenderEngine.cpp
@@ -69,12 +69,38 @@
case GraphicsApi::GL:
return true;
case GraphicsApi::VK: {
- if (!sVulkanInterface.isInitialized()) {
- sVulkanInterface.init(false /* no protected content */);
- ALOGD("%s: initialized == %s.", __func__,
- sVulkanInterface.isInitialized() ? "true" : "false");
+ // Static local variables are initialized once, on first invocation of the function.
+ static const bool canSupportVulkan = []() {
+ if (!sVulkanInterface.isInitialized()) {
+ sVulkanInterface.init(false /* no protected content */);
+ ALOGD("%s: initialized == %s.", __func__,
+ sVulkanInterface.isInitialized() ? "true" : "false");
+ if (!sVulkanInterface.isInitialized()) {
+ sVulkanInterface.teardown();
+ return false;
+ }
+ }
+ return true;
+ }();
+ return canSupportVulkan;
+ }
+ }
+}
+
+void RenderEngine::teardown(GraphicsApi graphicsApi) {
+ switch (graphicsApi) {
+ case GraphicsApi::GL:
+ break;
+ case GraphicsApi::VK: {
+ if (sVulkanInterface.isInitialized()) {
+ sVulkanInterface.teardown();
+ ALOGD("Tearing down the unprotected VulkanInterface.");
}
- return sVulkanInterface.isInitialized();
+ if (sProtectedContentVulkanInterface.isInitialized()) {
+ sProtectedContentVulkanInterface.teardown();
+ ALOGD("Tearing down the protected VulkanInterface.");
+ }
+ break;
}
}
}
@@ -88,11 +114,27 @@
args.blurAlgorithm) {}
SkiaVkRenderEngine::~SkiaVkRenderEngine() {
- finishRenderingAndAbandonContext();
+ finishRenderingAndAbandonContexts();
+ // Teardown VulkanInterfaces after Skia contexts have been abandoned
+ teardown(GraphicsApi::VK);
}
SkiaRenderEngine::Contexts SkiaVkRenderEngine::createContexts() {
sSetupVulkanInterface();
+ // More work would need to be done in order to have multiple RenderEngine instances. In
+ // particular, they would not be able to share the same VulkanInterface(s).
+ LOG_ALWAYS_FATAL_IF(!sVulkanInterface.takeOwnership(),
+ "SkiaVkRenderEngine couldn't take ownership of existing unprotected "
+ "VulkanInterface! Only one SkiaVkRenderEngine instance may exist at a "
+ "time.");
+ if (sProtectedContentVulkanInterface.isInitialized()) {
+ // takeOwnership fails on an uninitialized VulkanInterface, but protected content support is
+ // optional.
+ LOG_ALWAYS_FATAL_IF(!sProtectedContentVulkanInterface.takeOwnership(),
+ "SkiaVkRenderEngine couldn't take ownership of existing protected "
+ "VulkanInterface! Only one SkiaVkRenderEngine instance may exist at a "
+ "time.");
+ }
SkiaRenderEngine::Contexts contexts;
contexts.first = createContext(sVulkanInterface);