Merge "Fix TextureView glitch and memory leak"

libs/hwui/renderthread/RenderThread.h

@@ -41,6 +41,7 @@
 namespace android {
 
 class Bitmap;
+class AutoBackendTextureRelease;
 
 namespace uirenderer {
 
@@ -135,6 +136,7 @@ private:
     friend class DispatchFrameCallbacks;
     friend class RenderProxy;
     friend class DummyVsyncSource;
+    friend class android::AutoBackendTextureRelease;
     friend class android::uirenderer::TestUtils;
     friend class android::uirenderer::WebViewFunctor;
     friend class android::uirenderer::skiapipeline::VkFunctorDrawHandler;

libs/hwui/surfacetexture/ImageConsumer.cpp

@@ -24,13 +24,17 @@
 #include "renderthread/VulkanManager.h"
 #include "utils/Color.h"
 #include <GrAHardwareBufferUtils.h>
+#include <GrBackendSurface.h>
 
 // Macro for including the SurfaceTexture name in log messages
 #define IMG_LOGE(x, ...) ALOGE("[%s] " x, st.mName.string(), ##__VA_ARGS__)
 
+using namespace android::uirenderer::renderthread;
+
 namespace android {
 
 void ImageConsumer::onFreeBufferLocked(int slotIndex) {
+    // This callback may be invoked on any thread.
     mImageSlots[slotIndex].clear();
 }
 
@@ -46,55 +50,141 @@ void ImageConsumer::onReleaseBufferLocked(int buf) {
     mImageSlots[buf].eglFence() = EGL_NO_SYNC_KHR;
 }
 
+/**
+ * AutoBackendTextureRelease manages EglImage/VkImage lifetime. It is a ref-counted object
+ * that keeps GPU resources alive until the last SKImage object using them is destroyed.
+ */
+class AutoBackendTextureRelease {
+public:
+    static void releaseProc(SkImage::ReleaseContext releaseContext);
+
+    AutoBackendTextureRelease(GrContext* context, GraphicBuffer* buffer);
+
+    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 =
+        0 != (buffer->getUsage() & GraphicBuffer::USAGE_PROTECTED);
+    GrBackendFormat backendFormat = GrAHardwareBufferUtils::GetBackendFormat(
+        context,
+        reinterpret_cast<AHardwareBuffer*>(buffer),
+        buffer->getPixelFormat(),
+        false);
+    mBackendTexture = GrAHardwareBufferUtils::MakeBackendTexture(
+        context,
+        reinterpret_cast<AHardwareBuffer*>(buffer),
+        buffer->getWidth(),
+        buffer->getHeight(),
+        &mDeleteProc,
+        &mDeleteCtx,
+        createProtectedImage,
+        backendFormat,
+        false);
+}
+
+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(
+        graphicBuffer->getPixelFormat());
+    mImage = SkImage::MakeFromTexture(context,
+        mBackendTexture,
+        kTopLeft_GrSurfaceOrigin,
+        colorType,
+        kPremul_SkAlphaType,
+        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 (!mImage.get() || dataspace != mDataspace || forceCreate) {
+    if (!mTextureRelease || !mTextureRelease->getImage().get() || dataspace != mDataspace
+            || forceCreate) {
         if (!graphicBuffer.get()) {
             clear();
             return;
         }
 
-        if (!mBackendTexture.isValid()) {
-            clear();
-            bool createProtectedImage =
-                0 != (graphicBuffer->getUsage() & GraphicBuffer::USAGE_PROTECTED);
-            GrBackendFormat backendFormat = GrAHardwareBufferUtils::GetBackendFormat(
-                context,
-                reinterpret_cast<AHardwareBuffer*>(graphicBuffer.get()),
-                graphicBuffer->getPixelFormat(),
-                false);
-            mBackendTexture = GrAHardwareBufferUtils::MakeBackendTexture(
-                context,
-                reinterpret_cast<AHardwareBuffer*>(graphicBuffer.get()),
-                graphicBuffer->getWidth(),
-                graphicBuffer->getHeight(),
-                &mDeleteProc,
-                &mDeleteCtx,
-                createProtectedImage,
-                backendFormat,
-                false);
+        if (!mTextureRelease) {
+            mTextureRelease = new AutoBackendTextureRelease(context, graphicBuffer.get());
         }
+
         mDataspace = dataspace;
-        SkColorType colorType = GrAHardwareBufferUtils::GetSkColorTypeFromBufferFormat(
-            graphicBuffer->getPixelFormat());
-        mImage = SkImage::MakeFromTexture(context,
-            mBackendTexture,
-            kTopLeft_GrSurfaceOrigin,
-            colorType,
-            kPremul_SkAlphaType,
-            uirenderer::DataSpaceToColorSpace(dataspace));
+        mTextureRelease->makeImage(graphicBuffer, dataspace, context);
     }
 }
 
 void ImageConsumer::ImageSlot::clear() {
-    mImage.reset();
-    if (mBackendTexture.isValid()) {
-        mDeleteProc(mDeleteCtx);
-        mBackendTexture = {};
+    if (mTextureRelease) {
+        // The following unref counteracts the initial mUsageCount of 1, set by default initializer.
+        mTextureRelease->unref(true);
+        mTextureRelease = nullptr;
     }
 }
 
+sk_sp<SkImage> ImageConsumer::ImageSlot::getImage() {
+    return mTextureRelease ? mTextureRelease->getImage() : nullptr;
+}
+
 sk_sp<SkImage> ImageConsumer::dequeueImage(bool* queueEmpty, SurfaceTexture& st,
                                            uirenderer::RenderState& renderState) {
     BufferItem item;

libs/hwui/surfacetexture/ImageConsumer.h

@@ -25,7 +25,6 @@
 #include <cutils/compiler.h>
 #include <gui/BufferItem.h>
 #include <system/graphics.h>
-#include <GrBackendSurface.h>
 
 namespace GrAHardwareBufferUtils {
 typedef void* DeleteImageCtx;
@@ -38,6 +37,7 @@ namespace uirenderer {
 class RenderState;
 }
 
+class AutoBackendTextureRelease;
 class SurfaceTexture;
 
 /*
@@ -81,16 +81,14 @@ private:
 
         void createIfNeeded(sp<GraphicBuffer> graphicBuffer, android_dataspace dataspace,
                             bool forceCreate, GrContext* context);
+
         void clear();
 
         inline EGLSyncKHR& eglFence() { return mEglFence; }
 
-        inline sk_sp<SkImage> getImage() { return mImage; }
+        sk_sp<SkImage> getImage();
 
     private:
-        // mImage is the SkImage created from mGraphicBuffer.
-        sk_sp<SkImage> mImage;
-
         // the dataspace associated with the current image
         android_dataspace mDataspace;
 
@@ -100,11 +98,11 @@ private:
          */
         EGLSyncKHR mEglFence;
 
-        GrBackendTexture mBackendTexture;
-
-        GrAHardwareBufferUtils::DeleteImageProc mDeleteProc;
-
-        GrAHardwareBufferUtils::DeleteImageCtx mDeleteCtx;
+        /**
+         * mTextureRelease may outlive ImageConsumer, if the last ref is held by an SkImage.
+         * ImageConsumer holds one ref to mTextureRelease, which is decremented by "clear".
+         */
+        AutoBackendTextureRelease* mTextureRelease = nullptr;
     };
 
     /**