Fix crash when VulkanSurface is no longer valid
SkiaVulkanPipeline::mVkSurface can become obsolete if
RenderThread destroys Vulkan context. This CL enables
RenderThread to notify active Vulkan pipelines that their
surface is invalid.
Improve error handling, when trying to draw a frame with null
VulkanSurface.
Bug: 123640274
Bug: 123541940
Test: Ran several apps
Change-Id: If7fba00713d097192c96179df36e90b54f4f8090
diff --git a/libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp b/libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp
index d0fe022..15f53f2 100644
--- a/libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp
+++ b/libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp
@@ -42,7 +42,13 @@
namespace skiapipeline {
SkiaVulkanPipeline::SkiaVulkanPipeline(renderthread::RenderThread& thread)
- : SkiaPipeline(thread), mVkManager(thread.vulkanManager()) {}
+ : SkiaPipeline(thread), mVkManager(thread.vulkanManager()) {
+ thread.renderState().registerContextCallback(this);
+}
+
+SkiaVulkanPipeline::~SkiaVulkanPipeline() {
+ mRenderThread.renderState().removeContextCallback(this);
+}
MakeCurrentResult SkiaVulkanPipeline::makeCurrent() {
return MakeCurrentResult::AlreadyCurrent;
@@ -53,6 +59,8 @@
"drawRenderNode called on a context with no surface!");
SkSurface* backBuffer = mVkManager.getBackbufferSurface(&mVkSurface);
+ LOG_ALWAYS_FATAL_IF(mVkSurface == nullptr,
+ "drawRenderNode called on a context with an invalid surface");
if (backBuffer == nullptr) {
SkDebugf("failed to get backbuffer");
return Frame(-1, -1, 0);
@@ -153,6 +161,13 @@
return nullptr;
}
+void SkiaVulkanPipeline::onContextDestroyed() {
+ if (mVkSurface) {
+ mVkManager.destroySurface(mVkSurface);
+ mVkSurface = nullptr;
+ }
+}
+
} /* namespace skiapipeline */
} /* namespace uirenderer */
} /* namespace android */
diff --git a/libs/hwui/pipeline/skia/SkiaVulkanPipeline.h b/libs/hwui/pipeline/skia/SkiaVulkanPipeline.h
index 9343076..2c24edd 100644
--- a/libs/hwui/pipeline/skia/SkiaVulkanPipeline.h
+++ b/libs/hwui/pipeline/skia/SkiaVulkanPipeline.h
@@ -19,14 +19,16 @@
#include "SkiaPipeline.h"
#include "renderthread/VulkanManager.h"
+#include "renderstate/RenderState.h"
+
namespace android {
namespace uirenderer {
namespace skiapipeline {
-class SkiaVulkanPipeline : public SkiaPipeline {
+class SkiaVulkanPipeline : public SkiaPipeline, public IGpuContextCallback {
public:
explicit SkiaVulkanPipeline(renderthread::RenderThread& thread);
- virtual ~SkiaVulkanPipeline() {}
+ virtual ~SkiaVulkanPipeline();
renderthread::MakeCurrentResult makeCurrent() override;
renderthread::Frame getFrame() override;
@@ -49,6 +51,9 @@
static sk_sp<Bitmap> allocateHardwareBitmap(renderthread::RenderThread& thread,
SkBitmap& skBitmap);
+protected:
+ void onContextDestroyed() override;
+
private:
renderthread::VulkanManager& mVkManager;
renderthread::VulkanSurface* mVkSurface = nullptr;
diff --git a/libs/hwui/renderthread/RenderThread.cpp b/libs/hwui/renderthread/RenderThread.cpp
index 8bef359..3b37c83 100644
--- a/libs/hwui/renderthread/RenderThread.cpp
+++ b/libs/hwui/renderthread/RenderThread.cpp
@@ -203,11 +203,17 @@
void RenderThread::destroyRenderingContext() {
mFunctorManager.onContextDestroyed();
- if (mEglManager->hasEglContext()) {
- setGrContext(nullptr);
- mEglManager->destroy();
+ if (Properties::getRenderPipelineType() == RenderPipelineType::SkiaGL) {
+ if (mEglManager->hasEglContext()) {
+ setGrContext(nullptr);
+ mEglManager->destroy();
+ }
+ } else {
+ if (vulkanManager().hasVkContext()) {
+ setGrContext(nullptr);
+ vulkanManager().destroy();
+ }
}
- vulkanManager().destroy();
}
void RenderThread::dumpGraphicsMemory(int fd) {
diff --git a/libs/hwui/renderthread/VulkanManager.cpp b/libs/hwui/renderthread/VulkanManager.cpp
index 582d51e..90397fd 100644
--- a/libs/hwui/renderthread/VulkanManager.cpp
+++ b/libs/hwui/renderthread/VulkanManager.cpp
@@ -89,7 +89,7 @@
mPhysicalDeviceFeatures2 = {};
}
-bool VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFeatures2& features) {
+void VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFeatures2& features) {
VkResult err;
constexpr VkApplicationInfo app_info = {
@@ -107,15 +107,11 @@
uint32_t extensionCount = 0;
err = mEnumerateInstanceExtensionProperties(nullptr, &extensionCount, nullptr);
- if (VK_SUCCESS != err) {
- return false;
- }
+ LOG_ALWAYS_FATAL_IF(VK_SUCCESS != err);
std::unique_ptr<VkExtensionProperties[]> extensions(
new VkExtensionProperties[extensionCount]);
err = mEnumerateInstanceExtensionProperties(nullptr, &extensionCount, extensions.get());
- if (VK_SUCCESS != err) {
- return false;
- }
+ LOG_ALWAYS_FATAL_IF(VK_SUCCESS != err);
bool hasKHRSurfaceExtension = false;
bool hasKHRAndroidSurfaceExtension = false;
for (uint32_t i = 0; i < extensionCount; ++i) {
@@ -127,10 +123,7 @@
hasKHRAndroidSurfaceExtension = true;
}
}
- if (!hasKHRSurfaceExtension || !hasKHRAndroidSurfaceExtension) {
- this->destroy();
- return false;
- }
+ LOG_ALWAYS_FATAL_IF(!hasKHRSurfaceExtension || !hasKHRAndroidSurfaceExtension);
}
const VkInstanceCreateInfo instance_create = {
@@ -146,10 +139,7 @@
GET_PROC(CreateInstance);
err = mCreateInstance(&instance_create, nullptr, &mInstance);
- if (err < 0) {
- this->destroy();
- return false;
- }
+ LOG_ALWAYS_FATAL_IF(err < 0);
GET_INST_PROC(DestroyInstance);
GET_INST_PROC(EnumeratePhysicalDevices);
@@ -166,39 +156,23 @@
GET_INST_PROC(GetPhysicalDeviceSurfacePresentModesKHR);
uint32_t gpuCount;
- err = mEnumeratePhysicalDevices(mInstance, &gpuCount, nullptr);
- if (err) {
- this->destroy();
- return false;
- }
- if (!gpuCount) {
- this->destroy();
- return false;
- }
+ LOG_ALWAYS_FATAL_IF(mEnumeratePhysicalDevices(mInstance, &gpuCount, nullptr));
+ LOG_ALWAYS_FATAL_IF(!gpuCount);
// Just returning the first physical device instead of getting the whole array. Since there
// should only be one device on android.
gpuCount = 1;
err = mEnumeratePhysicalDevices(mInstance, &gpuCount, &mPhysicalDevice);
// VK_INCOMPLETE is returned when the count we provide is less than the total device count.
- if (err && VK_INCOMPLETE != err) {
- this->destroy();
- return false;
- }
+ LOG_ALWAYS_FATAL_IF(err && VK_INCOMPLETE != err);
VkPhysicalDeviceProperties physDeviceProperties;
mGetPhysicalDeviceProperties(mPhysicalDevice, &physDeviceProperties);
- if (physDeviceProperties.apiVersion < VK_MAKE_VERSION(1, 1, 0)) {
- this->destroy();
- return false;
- }
+ LOG_ALWAYS_FATAL_IF(physDeviceProperties.apiVersion < VK_MAKE_VERSION(1, 1, 0));
// query to get the initial queue props size
uint32_t queueCount;
mGetPhysicalDeviceQueueFamilyProperties(mPhysicalDevice, &queueCount, nullptr);
- if (!queueCount) {
- this->destroy();
- return false;
- }
+ LOG_ALWAYS_FATAL_IF(!queueCount);
// now get the actual queue props
std::unique_ptr<VkQueueFamilyProperties[]> queueProps(new VkQueueFamilyProperties[queueCount]);
@@ -212,10 +186,7 @@
break;
}
}
- if (mGraphicsQueueIndex == queueCount) {
- this->destroy();
- return false;
- }
+ LOG_ALWAYS_FATAL_IF(mGraphicsQueueIndex == queueCount);
// All physical devices and queue families on Android must be capable of
// presentation with any native window. So just use the first one.
@@ -225,18 +196,12 @@
uint32_t extensionCount = 0;
err = mEnumerateDeviceExtensionProperties(mPhysicalDevice, nullptr, &extensionCount,
nullptr);
- if (VK_SUCCESS != err) {
- this->destroy();
- return false;
- }
+ LOG_ALWAYS_FATAL_IF(VK_SUCCESS != err);
std::unique_ptr<VkExtensionProperties[]> extensions(
new VkExtensionProperties[extensionCount]);
err = mEnumerateDeviceExtensionProperties(mPhysicalDevice, nullptr, &extensionCount,
extensions.get());
- if (VK_SUCCESS != err) {
- this->destroy();
- return false;
- }
+ LOG_ALWAYS_FATAL_IF(VK_SUCCESS != err);
bool hasKHRSwapchainExtension = false;
for (uint32_t i = 0; i < extensionCount; ++i) {
mDeviceExtensions.push_back(extensions[i].extensionName);
@@ -244,10 +209,7 @@
hasKHRSwapchainExtension = true;
}
}
- if (!hasKHRSwapchainExtension) {
- this->destroy();
- return false;
- }
+ LOG_ALWAYS_FATAL_IF(!hasKHRSwapchainExtension);
}
auto getProc = [] (const char* proc_name, VkInstance instance, VkDevice device) {
@@ -259,10 +221,7 @@
grExtensions.init(getProc, mInstance, mPhysicalDevice, mInstanceExtensions.size(),
mInstanceExtensions.data(), mDeviceExtensions.size(), mDeviceExtensions.data());
- if (!grExtensions.hasExtension(VK_KHR_EXTERNAL_SEMAPHORE_FD_EXTENSION_NAME, 1)) {
- this->destroy();
- return false;
- }
+ LOG_ALWAYS_FATAL_IF(!grExtensions.hasExtension(VK_KHR_EXTERNAL_SEMAPHORE_FD_EXTENSION_NAME, 1));
memset(&features, 0, sizeof(VkPhysicalDeviceFeatures2));
features.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2;
@@ -332,11 +291,7 @@
nullptr, // ppEnabledFeatures
};
- err = mCreateDevice(mPhysicalDevice, &deviceInfo, nullptr, &mDevice);
- if (err) {
- this->destroy();
- return false;
- }
+ LOG_ALWAYS_FATAL_IF(mCreateDevice(mPhysicalDevice, &deviceInfo, nullptr, &mDevice));
GET_DEV_PROC(GetDeviceQueue);
GET_DEV_PROC(DeviceWaitIdle);
@@ -366,8 +321,6 @@
GET_DEV_PROC(DestroyFence);
GET_DEV_PROC(WaitForFences);
GET_DEV_PROC(ResetFences);
-
- return true;
}
void VulkanManager::initialize() {
@@ -381,7 +334,7 @@
LOG_ALWAYS_FATAL_IF(instanceVersion < VK_MAKE_VERSION(1, 1, 0));
GrVkExtensions extensions;
- LOG_ALWAYS_FATAL_IF(!this->setupDevice(extensions, mPhysicalDeviceFeatures2));
+ this->setupDevice(extensions, mPhysicalDeviceFeatures2);
mGetDeviceQueue(mDevice, mGraphicsQueueIndex, 0, &mGraphicsQueue);
@@ -419,7 +372,7 @@
if (!setupDummyCommandBuffer()) {
this->destroy();
- return;
+ // Pass through will crash on next line.
}
LOG_ALWAYS_FATAL_IF(mDummyCB == VK_NULL_HANDLE);
@@ -520,6 +473,9 @@
destroySurface(surface);
*surfaceOut = createSurface(window, colorMode, colorSpace, colorType);
surface = *surfaceOut;
+ if (!surface) {
+ return nullptr;
+ }
}
VulkanSurface::BackbufferInfo* backbuffer = getAvailableBackbuffer(surface);
diff --git a/libs/hwui/renderthread/VulkanManager.h b/libs/hwui/renderthread/VulkanManager.h
index 6426fe2..1fe6c65 100644
--- a/libs/hwui/renderthread/VulkanManager.h
+++ b/libs/hwui/renderthread/VulkanManager.h
@@ -151,7 +151,7 @@
// Sets up the VkInstance and VkDevice objects. Also fills out the passed in
// VkPhysicalDeviceFeatures struct.
- bool setupDevice(GrVkExtensions&, VkPhysicalDeviceFeatures2&);
+ void setupDevice(GrVkExtensions&, VkPhysicalDeviceFeatures2&);
void destroyBuffers(VulkanSurface* surface);