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/libs/renderengine/gl/GLESRenderEngine.cpp b/libs/renderengine/gl/GLESRenderEngine.cpp
index 5d0aa1e..8069a1a 100644
--- a/libs/renderengine/gl/GLESRenderEngine.cpp
+++ b/libs/renderengine/gl/GLESRenderEngine.cpp
@@ -626,22 +626,13 @@
 }
 
 status_t GLESRenderEngine::bindExternalTextureBuffer(uint32_t texName, sp<GraphicBuffer> buffer,
-                                                     sp<Fence> bufferFence, bool readCache) {
-    return bindExternalTextureBuffer(texName, buffer, bufferFence, readCache,
-                                     /*persistCache=*/false);
-}
-
-status_t GLESRenderEngine::bindExternalTextureBuffer(uint32_t texName, sp<GraphicBuffer> buffer,
-                                                     sp<Fence> bufferFence, bool readCache,
-                                                     bool persistCache) {
+                                                     sp<Fence> bufferFence) {
     ATRACE_CALL();
-    if (readCache) {
-        auto cachedImage = mImageCache.find(buffer->getId());
+    auto cachedImage = mImageCache.find(buffer->getId());
 
-        if (cachedImage != mImageCache.end()) {
-            bindExternalTextureImage(texName, *cachedImage->second);
-            return NO_ERROR;
-        }
+    if (cachedImage != mImageCache.end()) {
+        bindExternalTextureImage(texName, *cachedImage->second);
+        return NO_ERROR;
     }
 
     std::unique_ptr<Image> newImage = createImage();
@@ -678,35 +669,19 @@
             }
         }
     }
-
-    // We don't always want to persist to the cache, e.g. on older devices we
-    // might bind for synchronization purposes, but that might leak if we never
-    // call drawLayers again, so it's just better to recreate the image again
-    // if needed when we draw.
-    if (persistCache) {
-        mImageCache.insert(std::make_pair(buffer->getId(), std::move(newImage)));
-    }
+    mImageCache.insert(std::make_pair(buffer->getId(), std::move(newImage)));
 
     return NO_ERROR;
 }
 
-void GLESRenderEngine::evictImages(const std::vector<LayerSettings>& layers) {
-    ATRACE_CALL();
-    // destroy old image references that we're not going to draw with.
-    std::unordered_set<uint64_t> bufIds;
-    for (auto layer : layers) {
-        if (layer.source.buffer.buffer != nullptr) {
-            bufIds.emplace(layer.source.buffer.buffer->getId());
-        }
+void GLESRenderEngine::unbindExternalTextureBuffer(uint64_t bufferId) {
+    const auto& cachedImage = mImageCache.find(bufferId);
+    if (cachedImage != mImageCache.end()) {
+        ALOGV("Destroying image for buffer: %" PRIu64, bufferId);
+        mImageCache.erase(bufferId);
+        return;
     }
-
-    for (auto it = mImageCache.begin(); it != mImageCache.end();) {
-        if (bufIds.count(it->first) == 0) {
-            it = mImageCache.erase(it);
-        } else {
-            it++;
-        }
-    }
+    ALOGV("Failed to find image for buffer: %" PRIu64, bufferId);
 }
 
 FloatRect GLESRenderEngine::setupLayerCropping(const LayerSettings& layer, Mesh& mesh) {
@@ -844,8 +819,6 @@
         return fbo.getStatus();
     }
 
-    evictImages(layers);
-
     // clear the entire buffer, sometimes when we reuse buffers we'd persist
     // ghost images otherwise.
     // we also require a full transparent framebuffer for overlays. This is
@@ -888,10 +861,8 @@
             isOpaque = layer.source.buffer.isOpaque;
 
             sp<GraphicBuffer> gBuf = layer.source.buffer.buffer;
-
-            bool readCache = layer.source.buffer.cacheHint == Buffer::CachingHint::USE_CACHE;
             bindExternalTextureBuffer(layer.source.buffer.textureName, gBuf,
-                                      layer.source.buffer.fence, readCache, /*persistCache=*/true);
+                                      layer.source.buffer.fence);
 
             usePremultipliedAlpha = layer.source.buffer.usePremultipliedAlpha;
             Texture texture(Texture::TEXTURE_EXTERNAL, layer.source.buffer.textureName);
diff --git a/libs/renderengine/gl/GLESRenderEngine.h b/libs/renderengine/gl/GLESRenderEngine.h
index e37c91d..728882a 100644
--- a/libs/renderengine/gl/GLESRenderEngine.h
+++ b/libs/renderengine/gl/GLESRenderEngine.h
@@ -74,8 +74,8 @@
     void genTextures(size_t count, uint32_t* names) override;
     void deleteTextures(size_t count, uint32_t const* names) override;
     void bindExternalTextureImage(uint32_t texName, const Image& image) override;
-    status_t bindExternalTextureBuffer(uint32_t texName, sp<GraphicBuffer> buffer, sp<Fence> fence,
-                                       bool readCache);
+    status_t bindExternalTextureBuffer(uint32_t texName, sp<GraphicBuffer> buffer, sp<Fence> fence);
+    void unbindExternalTextureBuffer(uint64_t bufferId);
     status_t bindFrameBuffer(Framebuffer* framebuffer) override;
     void unbindFrameBuffer(Framebuffer* framebuffer) override;
     void checkErrors() const override;
@@ -143,8 +143,6 @@
     // Defines the viewport, and sets the projection matrix to the projection
     // defined by the clip.
     void setViewportAndProjection(Rect viewport, Rect clip);
-    status_t bindExternalTextureBuffer(uint32_t texName, sp<GraphicBuffer> buffer, sp<Fence> fence,
-                                       bool readCache, bool persistCache);
     // Evicts stale images from the buffer cache.
     void evictImages(const std::vector<LayerSettings>& layers);
     // Computes the cropping window for the layer and sets up cropping
diff --git a/libs/renderengine/include/renderengine/LayerSettings.h b/libs/renderengine/include/renderengine/LayerSettings.h
index aa45ed8..b8bf801 100644
--- a/libs/renderengine/include/renderengine/LayerSettings.h
+++ b/libs/renderengine/include/renderengine/LayerSettings.h
@@ -32,16 +32,6 @@
 
 // Metadata describing the input buffer to render from.
 struct Buffer {
-    // Hint for whether to use the Image cache or not.
-    // If NO_CACHE is specified, then upload the contents of the GraphicBuffer
-    // to the GPU, without checking against any implementation defined cache.
-    // If USE_CACHE is specified, then check against an implementation defined
-    // cache first. If there is an Image cached for the given GraphicBuffer id,
-    // then use that instead of the provided buffer contents. If there is no
-    // cached image or the RenderEngine implementation does not support caching,
-    // then use the GraphicBuffer contents.
-    enum class CachingHint { NO_CACHE, USE_CACHE };
-
     // Buffer containing the image that we will render.
     // If buffer == nullptr, then the rest of the fields in this struct will be
     // ignored.
@@ -50,9 +40,6 @@
     // Fence that will fire when the buffer is ready to be bound.
     sp<Fence> fence = nullptr;
 
-    // Caching hint to use when uploading buffer contents.
-    CachingHint cacheHint = CachingHint::NO_CACHE;
-
     // Texture identifier to bind the external texture to.
     // TODO(alecmouri): This is GL-specific...make the type backend-agnostic.
     uint32_t textureName = 0;
diff --git a/libs/renderengine/include/renderengine/RenderEngine.h b/libs/renderengine/include/renderengine/RenderEngine.h
index 2a2b48f..ab34274 100644
--- a/libs/renderengine/include/renderengine/RenderEngine.h
+++ b/libs/renderengine/include/renderengine/RenderEngine.h
@@ -110,7 +110,11 @@
     virtual void deleteTextures(size_t count, uint32_t const* names) = 0;
     virtual void bindExternalTextureImage(uint32_t texName, const Image& image) = 0;
     virtual status_t bindExternalTextureBuffer(uint32_t texName, sp<GraphicBuffer> buffer,
-                                               sp<Fence> fence, bool cleanCache) = 0;
+                                               sp<Fence> fence) = 0;
+    // Removes internal resources referenced by the bufferId. This method should be
+    // invoked when the caller will no longer hold a reference to a GraphicBuffer
+    // and needs to clean up its resources.
+    virtual void unbindExternalTextureBuffer(uint64_t bufferId) = 0;
     // When binding a native buffer, it must be done before setViewportAndProjection
     // Returns NO_ERROR when binds successfully, NO_MEMORY when there's no memory for allocation.
     virtual status_t bindFrameBuffer(Framebuffer* framebuffer) = 0;
diff --git a/libs/renderengine/include/renderengine/mock/RenderEngine.h b/libs/renderengine/include/renderengine/mock/RenderEngine.h
index 5956c46..ddf7420 100644
--- a/libs/renderengine/include/renderengine/mock/RenderEngine.h
+++ b/libs/renderengine/include/renderengine/mock/RenderEngine.h
@@ -53,7 +53,8 @@
     MOCK_METHOD2(genTextures, void(size_t, uint32_t*));
     MOCK_METHOD2(deleteTextures, void(size_t, uint32_t const*));
     MOCK_METHOD2(bindExternalTextureImage, void(uint32_t, const renderengine::Image&));
-    MOCK_METHOD4(bindExternalTextureBuffer, status_t(uint32_t, sp<GraphicBuffer>, sp<Fence>, bool));
+    MOCK_METHOD3(bindExternalTextureBuffer, status_t(uint32_t, sp<GraphicBuffer>, sp<Fence>));
+    MOCK_METHOD1(unbindExternalTextureBuffer, void(uint64_t));
     MOCK_CONST_METHOD0(checkErrors, void());
     MOCK_METHOD4(setViewportAndProjection,
                  void(size_t, size_t, Rect, ui::Transform::orientation_flags));
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);