Create EGLImages during buffer allocation

EGLImage creation is now performed on an async binder thread, so now GPU
composition should rarely be stalled by expensive image creation.

Bug: 129008989
Test: systrace
Change-Id: I9732f866933a8950a4c69ff51d5ac1622bbb3470

14 files changed, 152 insertions, 25 deletions

diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp
index 5fb3f0b80f..d87228fbbd 100644
--- a/libs/gui/BufferQueue.cpp
+++ b/libs/gui/BufferQueue.cpp
@@ -59,6 +59,13 @@ void BufferQueue::ProxyConsumerListener::onFrameReplaced(
     }
 }
 
+void BufferQueue::ProxyConsumerListener::onBufferAllocated(const BufferItem& item) {
+    sp<ConsumerListener> listener(mConsumerListener.promote());
+    if (listener != nullptr) {
+        listener->onBufferAllocated(item);
+    }
+}
+
 void BufferQueue::ProxyConsumerListener::onBuffersReleased() {
     sp<ConsumerListener> listener(mConsumerListener.promote());
     if (listener != nullptr) {
diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index a462b0362f..e657488969 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -530,6 +530,13 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp<android::Fence>* ou
                 return NO_INIT;
             }
 
+            if (mCore->mConsumerListener != nullptr) {
+                BufferItem item;
+                item.mGraphicBuffer = graphicBuffer;
+                item.mSlot = *outSlot;
+                mCore->mConsumerListener->onBufferAllocated(item);
+            }
+
             VALIDATE_CONSISTENCY();
         } // Autolock scope
     }
@@ -1414,6 +1421,13 @@ void BufferQueueProducer::allocateBuffers(uint32_t width, uint32_t height,
                 BQ_LOGV("allocateBuffers: allocated a new buffer in slot %d",
                         *slot);
 
+                if (mCore->mConsumerListener != nullptr) {
+                    BufferItem item;
+                    item.mGraphicBuffer = buffers[i];
+                    item.mSlot = *slot;
+                    mCore->mConsumerListener->onBufferAllocated(item);
+                }
+
                 // Make sure the erase is done after all uses of the slot
                 // iterator since it will be invalid after this point.
                 mCore->mFreeSlots.erase(slot);
diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp
index abd9921fa9..1e94cc13cd 100644
--- a/libs/gui/ConsumerBase.cpp
+++ b/libs/gui/ConsumerBase.cpp
@@ -131,6 +131,8 @@ void ConsumerBase::onFrameReplaced(const BufferItem &item) {
     }
 }
 
+void ConsumerBase::onBufferAllocated(const BufferItem& /*item*/) {}
+
 void ConsumerBase::onBuffersReleased() {
     Mutex::Autolock lock(mMutex);
 
diff --git a/libs/gui/IConsumerListener.cpp b/libs/gui/IConsumerListener.cpp
index 85ac304ab8..ea9045cad9 100644
--- a/libs/gui/IConsumerListener.cpp
+++ b/libs/gui/IConsumerListener.cpp
@@ -28,7 +28,8 @@ enum class Tag : uint32_t {
     ON_FRAME_REPLACED,
     ON_BUFFERS_RELEASED,
     ON_SIDEBAND_STREAM_CHANGED,
-    LAST = ON_SIDEBAND_STREAM_CHANGED,
+    ON_BUFFER_ALLOCATED,
+    LAST = ON_BUFFER_ALLOCATED,
 };
 
 } // Anonymous namespace
@@ -54,6 +55,11 @@ public:
                                                                        item);
     }
 
+    void onBufferAllocated(const BufferItem& item) override {
+        callRemoteAsync<decltype(&IConsumerListener::onBufferAllocated)>(Tag::ON_BUFFER_ALLOCATED,
+                                                                         item);
+    }
+
     void onBuffersReleased() override {
         callRemoteAsync<decltype(&IConsumerListener::onBuffersReleased)>(Tag::ON_BUFFERS_RELEASED);
     }
@@ -89,6 +95,8 @@ status_t BnConsumerListener::onTransact(uint32_t code, const Parcel& data, Parce
             return callLocalAsync(data, reply, &IConsumerListener::onFrameAvailable);
         case Tag::ON_FRAME_REPLACED:
             return callLocalAsync(data, reply, &IConsumerListener::onFrameReplaced);
+        case Tag::ON_BUFFER_ALLOCATED:
+            return callLocalAsync(data, reply, &IConsumerListener::onBufferAllocated);
         case Tag::ON_BUFFERS_RELEASED:
             return callLocalAsync(data, reply, &IConsumerListener::onBuffersReleased);
         case Tag::ON_SIDEBAND_STREAM_CHANGED:
diff --git a/libs/gui/include/gui/BufferQueue.h b/libs/gui/include/gui/BufferQueue.h
index da952744f3..721427be7b 100644
--- a/libs/gui/include/gui/BufferQueue.h
+++ b/libs/gui/include/gui/BufferQueue.h
@@ -61,6 +61,7 @@ public:
         void onDisconnect() override;
         void onFrameAvailable(const BufferItem& item) override;
         void onFrameReplaced(const BufferItem& item) override;
+        void onBufferAllocated(const BufferItem& item) override;
         void onBuffersReleased() override;
         void onSidebandStreamChanged() override;
         void addAndGetFrameTimestamps(
diff --git a/libs/gui/include/gui/ConsumerBase.h b/libs/gui/include/gui/ConsumerBase.h
index 366ced380b..7c26482509 100644
--- a/libs/gui/include/gui/ConsumerBase.h
+++ b/libs/gui/include/gui/ConsumerBase.h
@@ -141,6 +141,7 @@ protected:
     // classes if they want the notification.
     virtual void onFrameAvailable(const BufferItem& item) override;
     virtual void onFrameReplaced(const BufferItem& item) override;
+    virtual void onBufferAllocated(const BufferItem& item) override;
     virtual void onBuffersReleased() override;
     virtual void onSidebandStreamChanged() override;
 
diff --git a/libs/gui/include/gui/IConsumerListener.h b/libs/gui/include/gui/IConsumerListener.h
index c0828820e3..03fefbe90c 100644
--- a/libs/gui/include/gui/IConsumerListener.h
+++ b/libs/gui/include/gui/IConsumerListener.h
@@ -61,6 +61,13 @@ public:
     // This is called without any lock held and can be called concurrently by multiple threads.
     virtual void onFrameReplaced(const BufferItem& /* item */) {} /* Asynchronous */
 
+    // onBufferAllocated is called to notify the buffer consumer that the BufferQueue has allocated
+    // a GraphicBuffer for a particular slot. Only the GraphicBuffer pointer and the slot ID will
+    // be populated.
+    //
+    // This is called without any lock held and can be called concurrently by multiple threads.
+    virtual void onBufferAllocated(const BufferItem& /* item */) {} /* Asynchronous */
+
     // onBuffersReleased is called to notify the buffer consumer that the BufferQueue has released
     // its references to one or more GraphicBuffers contained in its slots. The buffer consumer
     // should then call BufferQueue::getReleasedBuffers to retrieve the list of buffers.
diff --git a/libs/renderengine/gl/GLESRenderEngine.cpp b/libs/renderengine/gl/GLESRenderEngine.cpp
index 1980f50bac..fb71ce5332 100644
--- a/libs/renderengine/gl/GLESRenderEngine.cpp
+++ b/libs/renderengine/gl/GLESRenderEngine.cpp
@@ -626,23 +626,26 @@ void GLESRenderEngine::bindExternalTextureImage(uint32_t texName, const Image& i
     }
 }
 
-status_t GLESRenderEngine::bindExternalTextureBuffer(uint32_t texName, sp<GraphicBuffer> buffer,
-                                                     sp<Fence> bufferFence) {
+status_t GLESRenderEngine::cacheExternalTextureBuffer(const sp<GraphicBuffer>& buffer) {
+    std::lock_guard<std::mutex> lock(mRenderingMutex);
+    return cacheExternalTextureBufferLocked(buffer);
+}
+
+status_t GLESRenderEngine::bindExternalTextureBuffer(uint32_t texName,
+                                                     const sp<GraphicBuffer>& buffer,
+                                                     const sp<Fence>& bufferFence) {
     std::lock_guard<std::mutex> lock(mRenderingMutex);
     return bindExternalTextureBufferLocked(texName, buffer, bufferFence);
 }
 
-status_t GLESRenderEngine::bindExternalTextureBufferLocked(uint32_t texName,
-                                                           sp<GraphicBuffer> buffer,
-                                                           sp<Fence> bufferFence) {
+status_t GLESRenderEngine::cacheExternalTextureBufferLocked(const sp<GraphicBuffer>& buffer) {
     if (buffer == nullptr) {
         return BAD_VALUE;
     }
+
     ATRACE_CALL();
-    auto cachedImage = mImageCache.find(buffer->getId());
 
-    if (cachedImage != mImageCache.end()) {
-        bindExternalTextureImage(texName, *cachedImage->second);
+    if (mImageCache.count(buffer->getId()) > 0) {
         return NO_ERROR;
     }
 
@@ -654,11 +657,32 @@ status_t GLESRenderEngine::bindExternalTextureBufferLocked(uint32_t texName,
         ALOGE("Failed to create image. size=%ux%u st=%u usage=%#" PRIx64 " fmt=%d",
               buffer->getWidth(), buffer->getHeight(), buffer->getStride(), buffer->getUsage(),
               buffer->getPixelFormat());
+        return NO_INIT;
+    }
+    mImageCache.insert(std::make_pair(buffer->getId(), std::move(newImage)));
+
+    return NO_ERROR;
+}
+
+status_t GLESRenderEngine::bindExternalTextureBufferLocked(uint32_t texName,
+                                                           const sp<GraphicBuffer>& buffer,
+                                                           const sp<Fence>& bufferFence) {
+    ATRACE_CALL();
+    status_t cacheResult = cacheExternalTextureBufferLocked(buffer);
+
+    if (cacheResult != NO_ERROR) {
+        return cacheResult;
+    }
+
+    auto cachedImage = mImageCache.find(buffer->getId());
+
+    if (cachedImage == mImageCache.end()) {
+        // We failed creating the image if we got here, so bail out.
         bindExternalTextureImage(texName, *createImage());
         return NO_INIT;
     }
 
-    bindExternalTextureImage(texName, *newImage);
+    bindExternalTextureImage(texName, *cachedImage->second);
 
     // Wait for the new buffer to be ready.
     if (bufferFence != nullptr && bufferFence->isValid()) {
@@ -680,7 +704,6 @@ status_t GLESRenderEngine::bindExternalTextureBufferLocked(uint32_t texName,
             }
         }
     }
-    mImageCache.insert(std::make_pair(buffer->getId(), std::move(newImage)));
 
     return NO_ERROR;
 }
diff --git a/libs/renderengine/gl/GLESRenderEngine.h b/libs/renderengine/gl/GLESRenderEngine.h
index 8c8f308d3b..613629e4a1 100644
--- a/libs/renderengine/gl/GLESRenderEngine.h
+++ b/libs/renderengine/gl/GLESRenderEngine.h
@@ -72,8 +72,9 @@ public:
     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)
-            EXCLUDES(mRenderingMutex);
+    status_t bindExternalTextureBuffer(uint32_t texName, const sp<GraphicBuffer>& buffer,
+                                       const sp<Fence>& fence) EXCLUDES(mRenderingMutex);
+    status_t cacheExternalTextureBuffer(const sp<GraphicBuffer>& buffer) EXCLUDES(mRenderingMutex);
     void unbindExternalTextureBuffer(uint64_t bufferId) EXCLUDES(mRenderingMutex);
     status_t bindFrameBuffer(Framebuffer* framebuffer) override;
     void unbindFrameBuffer(Framebuffer* framebuffer) override;
@@ -219,8 +220,12 @@ private:
 
     // See bindExternalTextureBuffer above, but requiring that mRenderingMutex
     // is held.
-    status_t bindExternalTextureBufferLocked(uint32_t texName, sp<GraphicBuffer> buffer,
-                                             sp<Fence> fence) REQUIRES(mRenderingMutex);
+    status_t bindExternalTextureBufferLocked(uint32_t texName, const sp<GraphicBuffer>& buffer,
+                                             const sp<Fence>& fence) REQUIRES(mRenderingMutex);
+    // See cacheExternalTextureBuffer above, but requiring that mRenderingMutex
+    // is held.
+    status_t cacheExternalTextureBufferLocked(const sp<GraphicBuffer>& buffer)
+            REQUIRES(mRenderingMutex);
 
     std::unique_ptr<Framebuffer> mDrawingBuffer;
 
diff --git a/libs/renderengine/include/renderengine/RenderEngine.h b/libs/renderengine/include/renderengine/RenderEngine.h
index b211551348..6773859eb7 100644
--- a/libs/renderengine/include/renderengine/RenderEngine.h
+++ b/libs/renderengine/include/renderengine/RenderEngine.h
@@ -106,8 +106,14 @@ public:
     virtual void genTextures(size_t count, uint32_t* names) = 0;
     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) = 0;
+    // Legacy public method used by devices that don't support native fence
+    // synchronization in their GPU driver, as this method provides implicit
+    // synchronization for latching buffers.
+    virtual status_t bindExternalTextureBuffer(uint32_t texName, const sp<GraphicBuffer>& buffer,
+                                               const sp<Fence>& fence) = 0;
+    // Caches Image resources for this buffer, but does not bind the buffer to
+    // a particular texture.
+    virtual status_t cacheExternalTextureBuffer(const sp<GraphicBuffer>& buffer) = 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.
diff --git a/libs/renderengine/include/renderengine/mock/RenderEngine.h b/libs/renderengine/include/renderengine/mock/RenderEngine.h
index 479c7ac035..4f7d9f4352 100644
--- a/libs/renderengine/include/renderengine/mock/RenderEngine.h
+++ b/libs/renderengine/include/renderengine/mock/RenderEngine.h
@@ -51,7 +51,9 @@ public:
     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_METHOD3(bindExternalTextureBuffer, status_t(uint32_t, sp<GraphicBuffer>, sp<Fence>));
+    MOCK_METHOD1(cacheExternalTextureBuffer, status_t(const sp<GraphicBuffer>&));
+    MOCK_METHOD3(bindExternalTextureBuffer,
+                 status_t(uint32_t, const sp<GraphicBuffer>&, const sp<Fence>&));
     MOCK_METHOD1(unbindExternalTextureBuffer, void(uint64_t));
     MOCK_CONST_METHOD0(checkErrors, void());
     MOCK_METHOD4(setViewportAndProjection,
diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp
index 8c93cf432c..24904cfaa7 100644
--- a/libs/renderengine/tests/RenderEngineTest.cpp
+++ b/libs/renderengine/tests/RenderEngineTest.cpp
@@ -1003,4 +1003,18 @@ TEST_F(RenderEngineTest, drawLayers_bindExternalBufferCachesImages) {
     EXPECT_FALSE(sRE->isImageCachedForTesting(bufferId));
 }
 
+TEST_F(RenderEngineTest, drawLayers_cacheExternalBufferWithNullBuffer) {
+    status_t result = sRE->cacheExternalTextureBuffer(nullptr);
+    ASSERT_EQ(BAD_VALUE, result);
+}
+
+TEST_F(RenderEngineTest, drawLayers_cacheExternalBufferCachesImages) {
+    sp<GraphicBuffer> buf = allocateSourceBuffer(1, 1);
+    uint64_t bufferId = buf->getId();
+    sRE->cacheExternalTextureBuffer(buf);
+    EXPECT_TRUE(sRE->isImageCachedForTesting(bufferId));
+    sRE->unbindExternalTextureBuffer(bufferId);
+    EXPECT_FALSE(sRE->isImageCachedForTesting(bufferId));
+}
+
 } // namespace android
diff --git a/services/surfaceflinger/BufferLayerConsumer.cpp b/services/surfaceflinger/BufferLayerConsumer.cpp
index f2d4c5113f..fc98dc836a 100644
--- a/services/surfaceflinger/BufferLayerConsumer.cpp
+++ b/services/surfaceflinger/BufferLayerConsumer.cpp
@@ -217,7 +217,11 @@ status_t BufferLayerConsumer::acquireBufferLocked(BufferItem* item, nsecs_t pres
     // If item->mGraphicBuffer is not null, this buffer has not been acquired
     // before, so we need to clean up old references.
     if (item->mGraphicBuffer != nullptr) {
-        mImages[item->mSlot] = std::make_shared<Image>(item->mGraphicBuffer, mRE);
+        std::lock_guard<std::mutex> lock(mImagesMutex);
+        if (mImages[item->mSlot] == nullptr || mImages[item->mSlot]->graphicBuffer() == nullptr ||
+            mImages[item->mSlot]->graphicBuffer()->getId() != item->mGraphicBuffer->getId()) {
+            mImages[item->mSlot] = std::make_shared<Image>(item->mGraphicBuffer, mRE);
+        }
     }
 
     return NO_ERROR;
@@ -238,7 +242,12 @@ status_t BufferLayerConsumer::updateAndReleaseLocked(const BufferItem& item,
     // 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.
-    std::shared_ptr<Image> nextTextureBuffer = mImages[slot];
+
+    std::shared_ptr<Image> nextTextureBuffer;
+    {
+        std::lock_guard<std::mutex> lock(mImagesMutex);
+        nextTextureBuffer = mImages[slot];
+    }
 
     // release old buffer
     if (mCurrentTexture != BufferQueue::INVALID_BUFFER_SLOT) {
@@ -436,6 +445,7 @@ status_t BufferLayerConsumer::doFenceWaitLocked() const {
 
 void BufferLayerConsumer::freeBufferLocked(int slotIndex) {
     BLC_LOGV("freeBufferLocked: slotIndex=%d", slotIndex);
+    std::lock_guard<std::mutex> lock(mImagesMutex);
     if (slotIndex == mCurrentTexture) {
         mCurrentTexture = BufferQueue::INVALID_BUFFER_SLOT;
     }
@@ -468,6 +478,23 @@ void BufferLayerConsumer::onSidebandStreamChanged() {
     }
 }
 
+void BufferLayerConsumer::onBufferAllocated(const BufferItem& item) {
+    if (item.mGraphicBuffer != nullptr) {
+        std::shared_ptr<Image> image = std::make_shared<Image>(item.mGraphicBuffer, mRE);
+        std::shared_ptr<Image> oldImage;
+        {
+            std::lock_guard<std::mutex> lock(mImagesMutex);
+            oldImage = mImages[item.mSlot];
+            if (oldImage == nullptr || oldImage->graphicBuffer() == nullptr ||
+                oldImage->graphicBuffer()->getId() != item.mGraphicBuffer->getId()) {
+                mImages[item.mSlot] = std::make_shared<Image>(item.mGraphicBuffer, mRE);
+            }
+            image = mImages[item.mSlot];
+        }
+        mRE.cacheExternalTextureBuffer(image->graphicBuffer());
+    }
+}
+
 void BufferLayerConsumer::addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps,
                                                    FrameEventHistoryDelta* outDelta) {
     sp<Layer> l = mLayer.promote();
@@ -480,6 +507,7 @@ void BufferLayerConsumer::abandonLocked() {
     BLC_LOGV("abandonLocked");
     mCurrentTextureBuffer = nullptr;
     for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) {
+        std::lock_guard<std::mutex> lock(mImagesMutex);
         mImages[i] = nullptr;
     }
     ConsumerBase::abandonLocked();
diff --git a/services/surfaceflinger/BufferLayerConsumer.h b/services/surfaceflinger/BufferLayerConsumer.h
index f4ca846ea3..0f0655d705 100644
--- a/services/surfaceflinger/BufferLayerConsumer.h
+++ b/services/surfaceflinger/BufferLayerConsumer.h
@@ -17,6 +17,7 @@
 #ifndef ANDROID_BUFFERLAYERCONSUMER_H
 #define ANDROID_BUFFERLAYERCONSUMER_H
 
+#include <android-base/thread_annotations.h>
 #include <gui/BufferQueueDefs.h>
 #include <gui/ConsumerBase.h>
 #include <gui/HdrMetadata.h>
@@ -179,7 +180,7 @@ public:
 protected:
     // abandonLocked overrides the ConsumerBase method to clear
     // mCurrentTextureImage in addition to the ConsumerBase behavior.
-    virtual void abandonLocked();
+    virtual void abandonLocked() EXCLUDES(mImagesMutex);
 
     // dumpLocked overrides the ConsumerBase method to dump BufferLayerConsumer-
     // specific info in addition to the ConsumerBase behavior.
@@ -187,7 +188,8 @@ protected:
 
     // See ConsumerBase::acquireBufferLocked
     virtual status_t acquireBufferLocked(BufferItem* item, nsecs_t presentWhen,
-                                         uint64_t maxFrameNumber = 0) override;
+                                         uint64_t maxFrameNumber = 0) override
+            EXCLUDES(mImagesMutex);
 
     bool canUseImageCrop(const Rect& crop) const;
 
@@ -206,7 +208,8 @@ protected:
     // completion of the method will instead be returned to the caller, so that
     // it may call releaseBufferLocked itself later.
     status_t updateAndReleaseLocked(const BufferItem& item,
-                                    PendingRelease* pendingRelease = nullptr);
+                                    PendingRelease* pendingRelease = nullptr)
+            EXCLUDES(mImagesMutex);
 
     // Binds mTexName and the current buffer to TEXTURE_EXTERNAL target.
     // If the bind succeeds, this calls doFenceWait.
@@ -234,10 +237,11 @@ private:
     // that slot.  Otherwise it has no effect.
     //
     // This method must be called with mMutex locked.
-    virtual void freeBufferLocked(int slotIndex);
+    virtual void freeBufferLocked(int slotIndex) EXCLUDES(mImagesMutex);
 
     // IConsumerListener interface
     void onDisconnect() override;
+    void onBufferAllocated(const BufferItem& item) override EXCLUDES(mImagesMutex);
     void onSidebandStreamChanged() override;
     void addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps,
                                   FrameEventHistoryDelta* outDelta) override;
@@ -344,7 +348,12 @@ private:
     int mCurrentTexture;
 
     // Shadow buffer cache for cleaning up renderengine references.
-    std::shared_ptr<Image> mImages[BufferQueueDefs::NUM_BUFFER_SLOTS];
+    std::shared_ptr<Image> mImages[BufferQueueDefs::NUM_BUFFER_SLOTS] GUARDED_BY(mImagesMutex);
+
+    // Separate mutex guarding the shadow buffer cache.
+    // mImagesMutex can be manipulated with binder threads (e.g. onBuffersAllocated)
+    // which is contentious enough that we can't just use mMutex.
+    mutable std::mutex mImagesMutex;
 
     // A release that is pending on the receipt of a new release fence from
     // presentDisplay