Clean up Renderengine caching semantics

* Add unbindExternalTextureBuffer, so that callers such as
BufferLayerConsumer can help manage the cache directly, which improves
system utilization.
* Remove the CacheHint, as it's no longer needed.
* Remove the backing image cache in BufferLayerConsumer, as it has not
been used. We still need a shadow array to properly callback into
RenderEngine, but we no longer need to store EGLImages here.

Bug: 123107664
Test: systrace
Test: open and close apps with GL composition forced, check for buffer
leakage through dumpsys
Change-Id: Ie48f99868dd46a4e18b5d659eea520e97a9eb300
diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp
index 6badc73..756ca42 100644
--- a/services/surfaceflinger/BufferLayer.cpp
+++ b/services/surfaceflinger/BufferLayer.cpp
@@ -163,9 +163,6 @@
         layer.source.buffer.buffer = mActiveBuffer;
         layer.source.buffer.isOpaque = isOpaque(s);
         layer.source.buffer.fence = mActiveBufferFence;
-        layer.source.buffer.cacheHint = useCachedBufferForClientComposition()
-                ? renderengine::Buffer::CachingHint::USE_CACHE
-                : renderengine::Buffer::CachingHint::NO_CACHE;
         layer.source.buffer.textureName = mTextureName;
         layer.source.buffer.usePremultipliedAlpha = getPremultipledAlpha();
         layer.source.buffer.isY410BT2020 = isHdrY410();
diff --git a/services/surfaceflinger/BufferLayer.h b/services/surfaceflinger/BufferLayer.h
index c48146f..e9dbdea 100644
--- a/services/surfaceflinger/BufferLayer.h
+++ b/services/surfaceflinger/BufferLayer.h
@@ -161,10 +161,6 @@
 
     bool mRefreshPending{false};
 
-    // Returns true if, when drawing the active buffer during gpu compositon, we
-    // should use a cached buffer or not.
-    virtual bool useCachedBufferForClientComposition() const = 0;
-
     // prepareClientLayer - constructs a RenderEngine layer for GPU composition.
     bool prepareClientLayer(const RenderArea& renderArea, const Region& clip,
                             bool useIdentityTransform, Region& clearRegion,
diff --git a/services/surfaceflinger/BufferLayerConsumer.cpp b/services/surfaceflinger/BufferLayerConsumer.cpp
index 6866e5c..7d2dcba 100644
--- a/services/surfaceflinger/BufferLayerConsumer.cpp
+++ b/services/surfaceflinger/BufferLayerConsumer.cpp
@@ -178,7 +178,8 @@
         return;
     }
 
-    auto buffer = mPendingRelease.isPending ? mPendingRelease.graphicBuffer : mCurrentTextureBuffer;
+    auto buffer = mPendingRelease.isPending ? mPendingRelease.graphicBuffer
+                                            : mCurrentTextureBuffer->graphicBuffer();
     auto err = addReleaseFence(slot, buffer, fence);
     if (err != OK) {
         BLC_LOGE("setReleaseFence: failed to add the fence: %s (%d)", strerror(-err), err);
@@ -214,9 +215,9 @@
     }
 
     // If item->mGraphicBuffer is not null, this buffer has not been acquired
-    // before.
+    // before, so we need to clean up old references.
     if (item->mGraphicBuffer != nullptr) {
-        mImages[item->mSlot] = nullptr;
+        mImages[item->mSlot] = std::make_shared<Image>(item->mGraphicBuffer, mRE);
     }
 
     return NO_ERROR;
@@ -229,18 +230,21 @@
     int slot = item.mSlot;
 
     BLC_LOGV("updateAndRelease: (slot=%d buf=%p) -> (slot=%d buf=%p)", mCurrentTexture,
-             mCurrentTextureBuffer != nullptr ? mCurrentTextureBuffer->handle : 0, slot,
-             mSlots[slot].mGraphicBuffer->handle);
+             (mCurrentTextureBuffer != nullptr && mCurrentTextureBuffer->graphicBuffer() != nullptr)
+                     ? mCurrentTextureBuffer->graphicBuffer()->handle
+                     : 0,
+             slot, mSlots[slot].mGraphicBuffer->handle);
 
     // Hang onto the pointer so that it isn't freed in the call to
     // releaseBufferLocked() if we're in shared buffer mode and both buffers are
     // the same.
-    sp<GraphicBuffer> nextTextureBuffer = mSlots[slot].mGraphicBuffer;
+    std::shared_ptr<Image> nextTextureBuffer = mImages[slot];
 
     // release old buffer
     if (mCurrentTexture != BufferQueue::INVALID_BUFFER_SLOT) {
         if (pendingRelease == nullptr) {
-            status_t status = releaseBufferLocked(mCurrentTexture, mCurrentTextureBuffer);
+            status_t status =
+                    releaseBufferLocked(mCurrentTexture, mCurrentTextureBuffer->graphicBuffer());
             if (status < NO_ERROR) {
                 BLC_LOGE("updateAndRelease: failed to release buffer: %s (%d)", strerror(-status),
                          status);
@@ -249,7 +253,7 @@
             }
         } else {
             pendingRelease->currentTexture = mCurrentTexture;
-            pendingRelease->graphicBuffer = mCurrentTextureBuffer;
+            pendingRelease->graphicBuffer = mCurrentTextureBuffer->graphicBuffer();
             pendingRelease->isPending = true;
         }
     }
@@ -257,8 +261,6 @@
     // Update the BufferLayerConsumer state.
     mCurrentTexture = slot;
     mCurrentTextureBuffer = nextTextureBuffer;
-    mCurrentTextureBufferStaleForGpu = false;
-    mCurrentTextureImageFreed = nullptr;
     mCurrentCrop = item.mCrop;
     mCurrentTransform = item.mTransform;
     mCurrentScalingMode = item.mScalingMode;
@@ -280,7 +282,12 @@
 status_t BufferLayerConsumer::bindTextureImageLocked() {
     ATRACE_CALL();
 
-    return mRE.bindExternalTextureBuffer(mTexName, mCurrentTextureBuffer, mCurrentFence, false);
+    if (mCurrentTextureBuffer != nullptr && mCurrentTextureBuffer->graphicBuffer() != nullptr) {
+        return mRE.bindExternalTextureBuffer(mTexName, mCurrentTextureBuffer->graphicBuffer(),
+                                             mCurrentFence);
+    }
+
+    return NO_INIT;
 }
 
 void BufferLayerConsumer::getTransformMatrix(float mtx[16]) {
@@ -308,12 +315,15 @@
 
 void BufferLayerConsumer::computeCurrentTransformMatrixLocked() {
     BLC_LOGV("computeCurrentTransformMatrixLocked");
-    if (mCurrentTextureBuffer == nullptr) {
+    if (mCurrentTextureBuffer == nullptr || mCurrentTextureBuffer->graphicBuffer() == nullptr) {
         BLC_LOGD("computeCurrentTransformMatrixLocked: "
                  "mCurrentTextureBuffer is nullptr");
     }
-    GLConsumer::computeTransformMatrix(mCurrentTransformMatrix, mCurrentTextureBuffer, mCurrentCrop,
-                                       mCurrentTransform, mFilteringEnabled);
+    GLConsumer::computeTransformMatrix(mCurrentTransformMatrix,
+                                       mCurrentTextureBuffer == nullptr
+                                               ? nullptr
+                                               : mCurrentTextureBuffer->graphicBuffer(),
+                                       mCurrentCrop, mCurrentTransform, mFilteringEnabled);
 }
 
 nsecs_t BufferLayerConsumer::getTimestamp() {
@@ -365,16 +375,7 @@
         *outFence = mCurrentFence;
     }
 
-    return mCurrentTextureBuffer;
-}
-
-bool BufferLayerConsumer::getAndSetCurrentBufferCacheHint() {
-    Mutex::Autolock lock(mMutex);
-    bool useCache = mCurrentTextureBufferStaleForGpu;
-    // Set the staleness bit here, as this function is only called during a
-    // client composition path.
-    mCurrentTextureBufferStaleForGpu = true;
-    return useCache;
+    return mCurrentTextureBuffer == nullptr ? nullptr : mCurrentTextureBuffer->graphicBuffer();
 }
 
 Rect BufferLayerConsumer::getCurrentCrop() const {
@@ -432,10 +433,8 @@
     BLC_LOGV("freeBufferLocked: slotIndex=%d", slotIndex);
     if (slotIndex == mCurrentTexture) {
         mCurrentTexture = BufferQueue::INVALID_BUFFER_SLOT;
-        mCurrentTextureImageFreed = std::move(mImages[slotIndex]);
-    } else {
-        mImages[slotIndex] = nullptr;
     }
+    mImages[slotIndex] = nullptr;
     ConsumerBase::freeBufferLocked(slotIndex);
 }
 
@@ -474,7 +473,10 @@
 
 void BufferLayerConsumer::abandonLocked() {
     BLC_LOGV("abandonLocked");
-    mCurrentTextureBuffer.clear();
+    mCurrentTextureBuffer = nullptr;
+    for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) {
+        mImages[i] = nullptr;
+    }
     ConsumerBase::abandonLocked();
 }
 
@@ -492,4 +494,11 @@
     ConsumerBase::dumpLocked(result, prefix);
 }
 
+BufferLayerConsumer::Image::~Image() {
+    if (mGraphicBuffer != nullptr) {
+        ALOGE("Destroying buffer: %" PRId64, mGraphicBuffer->getId());
+        mRE.unbindExternalTextureBuffer(mGraphicBuffer->getId());
+    }
+}
+
 }; // namespace android
diff --git a/services/surfaceflinger/BufferLayerConsumer.h b/services/surfaceflinger/BufferLayerConsumer.h
index e2a6d2e..32ccfbb 100644
--- a/services/surfaceflinger/BufferLayerConsumer.h
+++ b/services/surfaceflinger/BufferLayerConsumer.h
@@ -149,11 +149,6 @@
     // for use with bilinear filtering.
     void setFilteringEnabled(bool enabled);
 
-    // Sets mCurrentTextureBufferStaleForGpu to true to indicate that the
-    // buffer is now "stale" for GPU composition, and returns the old staleness
-    // bit as a caching hint.
-    bool getAndSetCurrentBufferCacheHint();
-
     // getCurrentBuffer returns the buffer associated with the current image.
     // When outSlot is not nullptr, the current buffer slot index is also
     // returned. Simiarly, when outFence is not nullptr, the current output
@@ -218,6 +213,22 @@
     status_t bindTextureImageLocked();
 
 private:
+    // Utility class for managing GraphicBuffer references into renderengine
+    class Image {
+    public:
+        Image(sp<GraphicBuffer> graphicBuffer, renderengine::RenderEngine& engine)
+              : mGraphicBuffer(graphicBuffer), mRE(engine) {}
+        virtual ~Image();
+        const sp<GraphicBuffer>& graphicBuffer() { return mGraphicBuffer; }
+
+    private:
+        // mGraphicBuffer is the buffer that was used to create this image.
+        sp<GraphicBuffer> mGraphicBuffer;
+        // Back-reference into renderengine to initiate cleanup.
+        renderengine::RenderEngine& mRE;
+        DISALLOW_COPY_AND_ASSIGN(Image);
+    };
+
     // freeBufferLocked frees up the given buffer slot. If the slot has been
     // initialized this will release the reference to the GraphicBuffer in
     // that slot.  Otherwise it has no effect.
@@ -248,14 +259,10 @@
     // consume buffers as hardware textures.
     static const uint64_t DEFAULT_USAGE_FLAGS = GraphicBuffer::USAGE_HW_TEXTURE;
 
-    // mCurrentTextureImage is the buffer containing the current texture. It's
+    // mCurrentTextureBuffer is the buffer containing the current texture. It's
     // possible that this buffer is not associated with any buffer slot, so we
     // must track it separately in order to support the getCurrentBuffer method.
-    sp<GraphicBuffer> mCurrentTextureBuffer;
-
-    // True if the buffer was used for the previous client composition frame,
-    // and false otherwise.
-    bool mCurrentTextureBufferStaleForGpu;
+    std::shared_ptr<Image> mCurrentTextureBuffer;
 
     // mCurrentCrop is the crop rectangle that applies to the current texture.
     // It gets set each time updateTexImage is called.
@@ -333,16 +340,8 @@
     // reset mCurrentTexture to INVALID_BUFFER_SLOT.
     int mCurrentTexture;
 
-    // Cached image used for rendering the current texture through GPU
-    // composition, which contains the cached image after freeBufferLocked is
-    // called on the current buffer. Whenever latchBuffer is called, this is
-    // expected to be cleared. Then, if bindTexImage is called before the next
-    // buffer is acquired, then this image is bound.
-    std::unique_ptr<renderengine::Image> mCurrentTextureImageFreed;
-
-    // Cached images used for rendering the current texture through GPU
-    // composition.
-    std::unique_ptr<renderengine::Image> mImages[BufferQueueDefs::NUM_BUFFER_SLOTS];
+    // Shadow buffer cache for cleaning up renderengine references.
+    std::shared_ptr<Image> mImages[BufferQueueDefs::NUM_BUFFER_SLOTS];
 
     // A release that is pending on the receipt of a new release fence from
     // presentDisplay
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp
index 215dea1..16e8e95 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -342,10 +342,6 @@
     return NO_ERROR;
 }
 
-bool BufferQueueLayer::useCachedBufferForClientComposition() const {
-    return mConsumer->getAndSetCurrentBufferCacheHint();
-}
-
 status_t BufferQueueLayer::updateFrameNumber(nsecs_t latchTime) {
     mPreviousFrameNumber = mCurrentFrameNumber;
     mCurrentFrameNumber = mConsumer->getFrameNumber();
diff --git a/services/surfaceflinger/BufferQueueLayer.h b/services/surfaceflinger/BufferQueueLayer.h
index fbb8c14..a2aad17 100644
--- a/services/surfaceflinger/BufferQueueLayer.h
+++ b/services/surfaceflinger/BufferQueueLayer.h
@@ -62,9 +62,6 @@
 public:
     bool fenceHasSignaled() const override;
 
-protected:
-    bool useCachedBufferForClientComposition() const override;
-
 private:
     nsecs_t getDesiredPresentTime() override;
     std::shared_ptr<FenceTime> getCurrentFenceTime() const override;
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index f6b69eb..6390e85 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -48,7 +48,12 @@
     mOverrideScalingMode = NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW;
     mCurrentState.dataspace = ui::Dataspace::V0_SRGB;
 }
-BufferStateLayer::~BufferStateLayer() = default;
+BufferStateLayer::~BufferStateLayer() {
+    if (mActiveBuffer != nullptr) {
+        auto& engine(mFlinger->getRenderEngine());
+        engine.unbindExternalTextureBuffer(mActiveBuffer->getId());
+    }
+}
 
 // -----------------------------------------------------------------------
 // Interface implementation for Layer
@@ -468,7 +473,7 @@
     const State& s(getDrawingState());
     auto& engine(mFlinger->getRenderEngine());
 
-    return engine.bindExternalTextureBuffer(mTextureName, s.buffer, s.acquireFence, false);
+    return engine.bindExternalTextureBuffer(mTextureName, s.buffer, s.acquireFence);
 }
 
 status_t BufferStateLayer::updateTexImage(bool& /*recomputeVisibleRegions*/, nsecs_t latchTime) {
@@ -542,6 +547,11 @@
         return BAD_VALUE;
     }
 
+    if (mActiveBuffer != nullptr) {
+        // todo: get this to work with BufferStateLayerCache
+        auto& engine(mFlinger->getRenderEngine());
+        engine.unbindExternalTextureBuffer(mActiveBuffer->getId());
+    }
     mActiveBuffer = s.buffer;
     mActiveBufferFence = s.acquireFence;
     auto& layerCompositionState = getCompositionLayer()->editState().frontEnd;
@@ -551,11 +561,6 @@
     return NO_ERROR;
 }
 
-bool BufferStateLayer::useCachedBufferForClientComposition() const {
-    // TODO: Store a proper staleness bit to support EGLImage caching.
-    return false;
-}
-
 status_t BufferStateLayer::updateFrameNumber(nsecs_t /*latchTime*/) {
     // TODO(marissaw): support frame history events
     mCurrentFrameNumber = mFrameNumber;
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index 0b03f49..97662e8 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -96,9 +96,6 @@
     // -----------------------------------------------------------------------
     bool fenceHasSignaled() const override;
 
-protected:
-    bool useCachedBufferForClientComposition() const override;
-
 private:
     nsecs_t getDesiredPresentTime() override;
     std::shared_ptr<FenceTime> getCurrentFenceTime() const override;
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 6deec29..30ae764 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -592,8 +592,6 @@
                     renderengine::LayerSettings layer = layerSettings.back();
                     EXPECT_THAT(layer.source.buffer.buffer, Not(IsNull()));
                     EXPECT_THAT(layer.source.buffer.fence, Not(IsNull()));
-                    EXPECT_EQ(renderengine::Buffer::CachingHint::NO_CACHE,
-                              layer.source.buffer.cacheHint);
                     EXPECT_EQ(DEFAULT_TEXTURE_ID, layer.source.buffer.textureName);
                     EXPECT_EQ(false, layer.source.buffer.isY410BT2020);
                     EXPECT_EQ(true, layer.source.buffer.usePremultipliedAlpha);