Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 90276c86 authored by Stan Iliev's avatar Stan Iliev
Browse files

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
parent 912ca402

libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp

+16 −1
Original line number Diff line number Diff line
@@ -42,7 +42,13 @@ namespace uirenderer { 
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 @@ Frame SkiaVulkanPipeline::getFrame() { 
                        "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 @@ sk_sp<Bitmap> SkiaVulkanPipeline::allocateHardwareBitmap(renderthread::RenderThr 
    return nullptr;

}



void SkiaVulkanPipeline::onContextDestroyed() {

    if (mVkSurface) {

        mVkManager.destroySurface(mVkSurface);

        mVkSurface = nullptr;

    }

}



} /* namespace skiapipeline */

} /* namespace uirenderer */

} /* namespace android */

libs/hwui/pipeline/skia/SkiaVulkanPipeline.h

+7 −2
Original line number Diff line number Diff line
@@ -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 @@ public: 
    static sk_sp<Bitmap> allocateHardwareBitmap(renderthread::RenderThread& thread,

                                                SkBitmap& skBitmap);



protected:

    void onContextDestroyed() override;



private:

    renderthread::VulkanManager& mVkManager;

    renderthread::VulkanSurface* mVkSurface = nullptr;

libs/hwui/renderthread/RenderThread.cpp

+10 −4
Original line number Diff line number Diff line
@@ -203,12 +203,18 @@ void RenderThread::requireGlContext() { 


void RenderThread::destroyRenderingContext() {

    mFunctorManager.onContextDestroyed();

    if (Properties::getRenderPipelineType() == RenderPipelineType::SkiaGL) {

        if (mEglManager->hasEglContext()) {

            setGrContext(nullptr);

            mEglManager->destroy();

        }

    } else {

        if (vulkanManager().hasVkContext()) {

            setGrContext(nullptr);

            vulkanManager().destroy();

        }

    }

}



void RenderThread::dumpGraphicsMemory(int fd) {

    globalProfileData()->dump(fd);

libs/hwui/renderthread/VulkanManager.cpp

+21 −65
Original line number Diff line number Diff line
@@ -89,7 +89,7 @@ void VulkanManager::destroy() { 
    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 @@ bool VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe 


        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 @@ bool VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe 
                hasKHRAndroidSurfaceExtension = true;

            }

        }

        if (!hasKHRSurfaceExtension || !hasKHRAndroidSurfaceExtension) {

            this->destroy();

            return false;

        }

        LOG_ALWAYS_FATAL_IF(!hasKHRSurfaceExtension || !hasKHRAndroidSurfaceExtension);

    }



    const VkInstanceCreateInfo instance_create = {
@@ -146,10 +139,7 @@ bool VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe 


    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 @@ bool VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe 
    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 @@ bool VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe 
            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 @@ bool VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe 
        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 @@ bool VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe 
                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 @@ bool VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe 
    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 @@ bool VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe 
        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 @@ bool VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe 
    GET_DEV_PROC(DestroyFence);

    GET_DEV_PROC(WaitForFences);

    GET_DEV_PROC(ResetFences);



    return true;

}



void VulkanManager::initialize() {
@@ -381,7 +334,7 @@ void VulkanManager::initialize() { 
    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 @@ void VulkanManager::initialize() { 


    if (!setupDummyCommandBuffer()) {

        this->destroy();

        return;

        // Pass through will crash on next line.

    }

    LOG_ALWAYS_FATAL_IF(mDummyCB == VK_NULL_HANDLE);
@@ -520,6 +473,9 @@ SkSurface* VulkanManager::getBackbufferSurface(VulkanSurface** surfaceOut) { 
        destroySurface(surface);

        *surfaceOut = createSurface(window, colorMode, colorSpace, colorType);

        surface = *surfaceOut;

        if (!surface) {

            return nullptr;

        }

    }



    VulkanSurface::BackbufferInfo* backbuffer = getAvailableBackbuffer(surface);

libs/hwui/renderthread/VulkanManager.h

+1 −1
Original line number Diff line number Diff line
@@ -151,7 +151,7 @@ private: 


    // 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);