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

Commit b830772f authored by Derek Sollenberger's avatar Derek Sollenberger
Browse files

Fix crash where trim memory resulted in HWUI deleting the surface. 

When HWUI is given the TRIM_MEMORY_COMPLETE signal it deletes all
of the associated GPU resources. In doing so it also deleted the
GL/Vk handle to the the ANativeWindow. So if the app attempted to
recover and draw new content it would crash because we no longer
had a surface to reinitialize and render into.

Test: atest hwui_unit_tests
Bug: 233388563
Change-Id: I54f3f0dadc3dfc89d2b1f567d36611f6a966c4f0
parent b61d0153

libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp

+11 −3
Original line number Diff line number Diff line
@@ -53,8 +53,12 @@ SkiaOpenGLPipeline::~SkiaOpenGLPipeline() { 
}



MakeCurrentResult SkiaOpenGLPipeline::makeCurrent() {

    // TODO: Figure out why this workaround is needed, see b/13913604

    // In the meantime this matches the behavior of GLRenderer, so it is not a regression

    // In case the surface was destroyed (e.g. a previous trimMemory call) we

    // need to recreate it here.

    if (!isSurfaceReady() && mNativeWindow) {

        setSurface(mNativeWindow.get(), mSwapBehavior);

    }



    EGLint error = 0;

    if (!mEglManager.makeCurrent(mEglSurface, &error)) {

        return MakeCurrentResult::AlreadyCurrent;
@@ -166,6 +170,9 @@ void SkiaOpenGLPipeline::onStop() { 
}



bool SkiaOpenGLPipeline::setSurface(ANativeWindow* surface, SwapBehavior swapBehavior) {

    mNativeWindow = surface;

    mSwapBehavior = swapBehavior;



    if (mEglSurface != EGL_NO_SURFACE) {

        mEglManager.destroySurface(mEglSurface);

        mEglSurface = EGL_NO_SURFACE;
@@ -182,7 +189,8 @@ bool SkiaOpenGLPipeline::setSurface(ANativeWindow* surface, SwapBehavior swapBeh 


    if (mEglSurface != EGL_NO_SURFACE) {

        const bool preserveBuffer = (swapBehavior != SwapBehavior::kSwap_discardBuffer);

        mBufferPreserved = mEglManager.setPreserveBuffer(mEglSurface, preserveBuffer);

        const bool isPreserved = mEglManager.setPreserveBuffer(mEglSurface, preserveBuffer);

        ALOGE_IF(preserveBuffer != isPreserved, "Unable to match the desired swap behavior.");

        return true;

    }

libs/hwui/pipeline/skia/SkiaOpenGLPipeline.h

+2 −1
Original line number Diff line number Diff line
@@ -61,7 +61,8 @@ protected: 
private:

    renderthread::EglManager& mEglManager;

    EGLSurface mEglSurface = EGL_NO_SURFACE;

    bool mBufferPreserved = false;

    sp<ANativeWindow> mNativeWindow;

    renderthread::SwapBehavior mSwapBehavior = renderthread::SwapBehavior::kSwap_discardBuffer;

};



} /* namespace skiapipeline */

libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp

+11 −2
Original line number Diff line number Diff line
@@ -55,7 +55,12 @@ VulkanManager& SkiaVulkanPipeline::vulkanManager() { 
}



MakeCurrentResult SkiaVulkanPipeline::makeCurrent() {

    return MakeCurrentResult::AlreadyCurrent;

    // In case the surface was destroyed (e.g. a previous trimMemory call) we

    // need to recreate it here.

    if (!isSurfaceReady() && mNativeWindow) {

        setSurface(mNativeWindow.get(), SwapBehavior::kSwap_default);

    }

    return isContextReady() ? MakeCurrentResult::AlreadyCurrent : MakeCurrentResult::Failed;

}



Frame SkiaVulkanPipeline::getFrame() {
@@ -132,7 +137,11 @@ DeferredLayerUpdater* SkiaVulkanPipeline::createTextureLayer() { 


void SkiaVulkanPipeline::onStop() {}



bool SkiaVulkanPipeline::setSurface(ANativeWindow* surface, SwapBehavior swapBehavior) {

// We can safely ignore the swap behavior because VkManager will always operate

// in a mode equivalent to EGLManager::SwapBehavior::kBufferAge

bool SkiaVulkanPipeline::setSurface(ANativeWindow* surface, SwapBehavior /*swapBehavior*/) {

    mNativeWindow = surface;



    if (mVkSurface) {

        vulkanManager().destroySurface(mVkSurface);

        mVkSurface = nullptr;

libs/hwui/pipeline/skia/SkiaVulkanPipeline.h

+1 −0
Original line number Diff line number Diff line
@@ -66,6 +66,7 @@ private: 
    renderthread::VulkanManager& vulkanManager();



    renderthread::VulkanSurface* mVkSurface = nullptr;

    sp<ANativeWindow> mNativeWindow;

};



} /* namespace skiapipeline */

libs/hwui/tests/unit/SkiaPipelineTests.cpp

+3 −1
Original line number Diff line number Diff line
@@ -404,7 +404,9 @@ RENDERTHREAD_SKIA_PIPELINE_TEST(SkiaPipeline, context_lost) { 
    EXPECT_TRUE(pipeline->isSurfaceReady());

    renderThread.destroyRenderingContext();

    EXPECT_FALSE(pipeline->isSurfaceReady());

    LOG_ALWAYS_FATAL_IF(pipeline->isSurfaceReady());



    pipeline->makeCurrent();

    EXPECT_TRUE(pipeline->isSurfaceReady());

}



RENDERTHREAD_SKIA_PIPELINE_TEST(SkiaPipeline, pictureCallback) {