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

Commit ee3754ab authored by Stan Iliev's avatar Stan Iliev
Browse files

Fix TextureView glitch and memory leak

Keep EglImage/VkImage alive until the last SkImage object using
it is destroyed.
Fix memory leak with GL pipeline only, caused by destroying
EGLImage on the wrong thread.
Make sure cached SkImage dtor is invoked on RenderThread.

Bug: 128523183
Bug: 128688107
Test: Ran settings app, systrace and CTS
Change-Id: I32c14c7174a2a97e54fbfaa49dffb4e9a160f90d
parent d4b81682
Loading
Loading
Loading
Loading
+2 −0
Original line number Original line Diff line number Diff line
@@ -41,6 +41,7 @@
namespace android {
namespace android {


class Bitmap;
class Bitmap;
class AutoBackendTextureRelease;


namespace uirenderer {
namespace uirenderer {


@@ -135,6 +136,7 @@ private:
    friend class DispatchFrameCallbacks;
    friend class DispatchFrameCallbacks;
    friend class RenderProxy;
    friend class RenderProxy;
    friend class DummyVsyncSource;
    friend class DummyVsyncSource;
    friend class android::AutoBackendTextureRelease;
    friend class android::uirenderer::TestUtils;
    friend class android::uirenderer::TestUtils;
    friend class android::uirenderer::WebViewFunctor;
    friend class android::uirenderer::WebViewFunctor;
    friend class android::uirenderer::skiapipeline::VkFunctorDrawHandler;
    friend class android::uirenderer::skiapipeline::VkFunctorDrawHandler;
+122 −32
Original line number Original line Diff line number Diff line
@@ -24,13 +24,17 @@
#include "renderthread/VulkanManager.h"
#include "renderthread/VulkanManager.h"
#include "utils/Color.h"
#include "utils/Color.h"
#include <GrAHardwareBufferUtils.h>
#include <GrAHardwareBufferUtils.h>
#include <GrBackendSurface.h>


// Macro for including the SurfaceTexture name in log messages
// Macro for including the SurfaceTexture name in log messages
#define IMG_LOGE(x, ...) ALOGE("[%s] " x, st.mName.string(), ##__VA_ARGS__)
#define IMG_LOGE(x, ...) ALOGE("[%s] " x, st.mName.string(), ##__VA_ARGS__)


using namespace android::uirenderer::renderthread;

namespace android {
namespace android {


void ImageConsumer::onFreeBufferLocked(int slotIndex) {
void ImageConsumer::onFreeBufferLocked(int slotIndex) {
    // This callback may be invoked on any thread.
    mImageSlots[slotIndex].clear();
    mImageSlots[slotIndex].clear();
}
}


@@ -46,36 +50,94 @@ void ImageConsumer::onReleaseBufferLocked(int buf) {
    mImageSlots[buf].eglFence() = EGL_NO_SYNC_KHR;
    mImageSlots[buf].eglFence() = EGL_NO_SYNC_KHR;
}
}


void ImageConsumer::ImageSlot::createIfNeeded(sp<GraphicBuffer> graphicBuffer,
/**
                                              android_dataspace dataspace, bool forceCreate,
 * AutoBackendTextureRelease manages EglImage/VkImage lifetime. It is a ref-counted object
                                              GrContext* context) {
 * that keeps GPU resources alive until the last SKImage object using them is destroyed.
    if (!mImage.get() || dataspace != mDataspace || forceCreate) {
 */
        if (!graphicBuffer.get()) {
class AutoBackendTextureRelease {
            clear();
public:
            return;
    static void releaseProc(SkImage::ReleaseContext releaseContext);
        }


        if (!mBackendTexture.isValid()) {
    AutoBackendTextureRelease(GrContext* context, GraphicBuffer* buffer);
            clear();

    const GrBackendTexture& getTexture() const { return mBackendTexture; }

    void ref() { mUsageCount++; }

    void unref(bool releaseImage);

    inline sk_sp<SkImage> getImage() { return mImage; }

    void makeImage(sp<GraphicBuffer>& graphicBuffer, android_dataspace dataspace,
                   GrContext* context);

private:
    // The only way to invoke dtor is with unref, when mUsageCount is 0.
    ~AutoBackendTextureRelease() {}

    GrBackendTexture mBackendTexture;
    GrAHardwareBufferUtils::DeleteImageProc mDeleteProc;
    GrAHardwareBufferUtils::DeleteImageCtx mDeleteCtx;

    // Starting with refcount 1, because the first ref is held by SurfaceTexture. Additional refs
    // are held by SkImages.
    int mUsageCount = 1;

    // mImage is the SkImage created from mBackendTexture.
    sk_sp<SkImage> mImage;
};

AutoBackendTextureRelease::AutoBackendTextureRelease(GrContext* context, GraphicBuffer* buffer) {
    bool createProtectedImage =
    bool createProtectedImage =
                0 != (graphicBuffer->getUsage() & GraphicBuffer::USAGE_PROTECTED);
        0 != (buffer->getUsage() & GraphicBuffer::USAGE_PROTECTED);
    GrBackendFormat backendFormat = GrAHardwareBufferUtils::GetBackendFormat(
    GrBackendFormat backendFormat = GrAHardwareBufferUtils::GetBackendFormat(
        context,
        context,
                reinterpret_cast<AHardwareBuffer*>(graphicBuffer.get()),
        reinterpret_cast<AHardwareBuffer*>(buffer),
                graphicBuffer->getPixelFormat(),
        buffer->getPixelFormat(),
        false);
        false);
    mBackendTexture = GrAHardwareBufferUtils::MakeBackendTexture(
    mBackendTexture = GrAHardwareBufferUtils::MakeBackendTexture(
        context,
        context,
                reinterpret_cast<AHardwareBuffer*>(graphicBuffer.get()),
        reinterpret_cast<AHardwareBuffer*>(buffer),
                graphicBuffer->getWidth(),
        buffer->getWidth(),
                graphicBuffer->getHeight(),
        buffer->getHeight(),
        &mDeleteProc,
        &mDeleteProc,
        &mDeleteCtx,
        &mDeleteCtx,
        createProtectedImage,
        createProtectedImage,
        backendFormat,
        backendFormat,
        false);
        false);
}
}
        mDataspace = dataspace;

void AutoBackendTextureRelease::unref(bool releaseImage) {
    if (!RenderThread::isCurrent()) {
        // EGLImage needs to be destroyed on RenderThread to prevent memory leak.
        // ~SkImage dtor for both pipelines needs to be invoked on RenderThread, because it is not
        // thread safe.
        RenderThread::getInstance().queue().post([this, releaseImage]() { unref(releaseImage); });
        return;
    }

    if (releaseImage) {
        mImage.reset();
    }

    mUsageCount--;
    if (mUsageCount <= 0) {
        if (mBackendTexture.isValid()) {
            mDeleteProc(mDeleteCtx);
            mBackendTexture = {};
        }
        delete this;
    }
}

void AutoBackendTextureRelease::releaseProc(SkImage::ReleaseContext releaseContext) {
    AutoBackendTextureRelease* textureRelease =
        reinterpret_cast<AutoBackendTextureRelease*>(releaseContext);
    textureRelease->unref(false);
}

void AutoBackendTextureRelease::makeImage(sp<GraphicBuffer>& graphicBuffer,
                                          android_dataspace dataspace, GrContext* context) {
    SkColorType colorType = GrAHardwareBufferUtils::GetSkColorTypeFromBufferFormat(
    SkColorType colorType = GrAHardwareBufferUtils::GetSkColorTypeFromBufferFormat(
        graphicBuffer->getPixelFormat());
        graphicBuffer->getPixelFormat());
    mImage = SkImage::MakeFromTexture(context,
    mImage = SkImage::MakeFromTexture(context,
@@ -83,16 +145,44 @@ void ImageConsumer::ImageSlot::createIfNeeded(sp<GraphicBuffer> graphicBuffer,
        kTopLeft_GrSurfaceOrigin,
        kTopLeft_GrSurfaceOrigin,
        colorType,
        colorType,
        kPremul_SkAlphaType,
        kPremul_SkAlphaType,
            uirenderer::DataSpaceToColorSpace(dataspace));
        uirenderer::DataSpaceToColorSpace(dataspace),
        releaseProc,
        this);
    if (mImage.get()) {
        // The following ref will be counteracted by releaseProc, when SkImage is discarded.
        ref();
    }
}

void ImageConsumer::ImageSlot::createIfNeeded(sp<GraphicBuffer> graphicBuffer,
                                              android_dataspace dataspace, bool forceCreate,
                                              GrContext* context) {
    if (!mTextureRelease || !mTextureRelease->getImage().get() || dataspace != mDataspace
            || forceCreate) {
        if (!graphicBuffer.get()) {
            clear();
            return;
        }

        if (!mTextureRelease) {
            mTextureRelease = new AutoBackendTextureRelease(context, graphicBuffer.get());
        }

        mDataspace = dataspace;
        mTextureRelease->makeImage(graphicBuffer, dataspace, context);
    }
    }
}
}


void ImageConsumer::ImageSlot::clear() {
void ImageConsumer::ImageSlot::clear() {
    mImage.reset();
    if (mTextureRelease) {
    if (mBackendTexture.isValid()) {
        // The following unref counteracts the initial mUsageCount of 1, set by default initializer.
        mDeleteProc(mDeleteCtx);
        mTextureRelease->unref(true);
        mBackendTexture = {};
        mTextureRelease = nullptr;
    }
}
}

sk_sp<SkImage> ImageConsumer::ImageSlot::getImage() {
    return mTextureRelease ? mTextureRelease->getImage() : nullptr;
}
}


sk_sp<SkImage> ImageConsumer::dequeueImage(bool* queueEmpty, SurfaceTexture& st,
sk_sp<SkImage> ImageConsumer::dequeueImage(bool* queueEmpty, SurfaceTexture& st,
+8 −10
Original line number Original line Diff line number Diff line
@@ -25,7 +25,6 @@
#include <cutils/compiler.h>
#include <cutils/compiler.h>
#include <gui/BufferItem.h>
#include <gui/BufferItem.h>
#include <system/graphics.h>
#include <system/graphics.h>
#include <GrBackendSurface.h>


namespace GrAHardwareBufferUtils {
namespace GrAHardwareBufferUtils {
typedef void* DeleteImageCtx;
typedef void* DeleteImageCtx;
@@ -38,6 +37,7 @@ namespace uirenderer {
class RenderState;
class RenderState;
}
}


class AutoBackendTextureRelease;
class SurfaceTexture;
class SurfaceTexture;


/*
/*
@@ -81,16 +81,14 @@ private:


        void createIfNeeded(sp<GraphicBuffer> graphicBuffer, android_dataspace dataspace,
        void createIfNeeded(sp<GraphicBuffer> graphicBuffer, android_dataspace dataspace,
                            bool forceCreate, GrContext* context);
                            bool forceCreate, GrContext* context);

        void clear();
        void clear();


        inline EGLSyncKHR& eglFence() { return mEglFence; }
        inline EGLSyncKHR& eglFence() { return mEglFence; }


        inline sk_sp<SkImage> getImage() { return mImage; }
        sk_sp<SkImage> getImage();


    private:
    private:
        // mImage is the SkImage created from mGraphicBuffer.
        sk_sp<SkImage> mImage;

        // the dataspace associated with the current image
        // the dataspace associated with the current image
        android_dataspace mDataspace;
        android_dataspace mDataspace;


@@ -100,11 +98,11 @@ private:
         */
         */
        EGLSyncKHR mEglFence;
        EGLSyncKHR mEglFence;


        GrBackendTexture mBackendTexture;
        /**

         * mTextureRelease may outlive ImageConsumer, if the last ref is held by an SkImage.
        GrAHardwareBufferUtils::DeleteImageProc mDeleteProc;
         * ImageConsumer holds one ref to mTextureRelease, which is decremented by "clear".

         */
        GrAHardwareBufferUtils::DeleteImageCtx mDeleteCtx;
        AutoBackendTextureRelease* mTextureRelease = nullptr;
    };
    };


    /**
    /**