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/VulkanInterface.cpp b/libs/renderengine/skia/VulkanInterface.cpp
index 49b9f1e..bd3cca7 100644
--- a/libs/renderengine/skia/VulkanInterface.cpp
+++ b/libs/renderengine/skia/VulkanInterface.cpp
@@ -557,13 +557,16 @@
ALOGD("%s: Success init Vulkan interface in %f ms", __func__, initTimeMs);
}
-// TODO: b/293371537 - Iterate on this.
-// Currently unused, but copied over from its original location for potential future use. This
-// should likely be improved to walk the pNext chain of mPhysicalDeviceFeatures2 and free everything
-// like HWUI's VulkanManager. Also, not all fields are being reset.
-void VulkanInterface::teardown() {
- mInitialized = false;
+bool VulkanInterface::takeOwnership() {
+ if (!isInitialized() || mIsOwned) {
+ return false;
+ }
+ mIsOwned = true;
+ return true;
+}
+void VulkanInterface::teardown() {
+ // Core resources that must be destroyed using Vulkan functions.
if (mDevice != VK_NULL_HANDLE) {
mFuncs.vkDeviceWaitIdle(mDevice);
mFuncs.vkDestroyDevice(mDevice, nullptr);
@@ -574,26 +577,42 @@
mInstance = VK_NULL_HANDLE;
}
+ // Optional features that can be deleted directly.
+ // TODO: b/293371537 - This section should likely be improved to walk the pNext chain of
+ // mPhysicalDeviceFeatures2 and free everything like HWUI's VulkanManager.
if (mProtectedMemoryFeatures) {
delete mProtectedMemoryFeatures;
+ mProtectedMemoryFeatures = nullptr;
}
-
if (mSamplerYcbcrConversionFeatures) {
delete mSamplerYcbcrConversionFeatures;
+ mSamplerYcbcrConversionFeatures = nullptr;
}
-
if (mPhysicalDeviceFeatures2) {
delete mPhysicalDeviceFeatures2;
+ mPhysicalDeviceFeatures2 = nullptr;
}
-
if (mDeviceFaultFeatures) {
delete mDeviceFaultFeatures;
+ mDeviceFaultFeatures = nullptr;
}
- mSamplerYcbcrConversionFeatures = nullptr;
- mPhysicalDeviceFeatures2 = nullptr;
- mProtectedMemoryFeatures = nullptr;
- mDeviceFaultFeatures = nullptr;
+ // Misc. fields that can be trivially reset without special deletion:
+ mInitialized = false;
+ mIsOwned = false;
+ mPhysicalDevice = VK_NULL_HANDLE; // Implicitly destroyed by destroying mInstance.
+ mQueue = VK_NULL_HANDLE; // Implicitly destroyed by destroying mDevice.
+ mQueueIndex = 0;
+ mApiVersion = 0;
+ mGrExtensions = GrVkExtensions();
+ mGrGetProc = nullptr;
+ mIsProtected = false;
+ mIsRealtimePriority = false;
+
+ mFuncs = VulkanFuncs();
+
+ mInstanceExtensionNames.clear();
+ mDeviceExtensionNames.clear();
}
} // namespace skia