Adding triple buffering without wait

Bug: 141939236
Test: build, boot, libgui_test
Change-Id: Ibffe3f10f07493700c0ead97a96548bd13cae953

5 files changed, 145 insertions, 67 deletions

diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index 3c31d7439f..06a5f06e31 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -14,6 +14,9 @@
  * limitations under the License.
  */
 
+#undef LOG_TAG
+#define LOG_TAG "BLASTBufferQueue"
+
 #include <gui/BLASTBufferQueue.h>
 #include <gui/BufferItemConsumer.h>
 
@@ -24,8 +27,14 @@ using namespace std::chrono_literals;
 namespace android {
 
 BLASTBufferQueue::BLASTBufferQueue(const sp<SurfaceControl>& surface, int width, int height)
-      : mSurfaceControl(surface), mWidth(width), mHeight(height) {
+      : mSurfaceControl(surface),
+        mPendingCallbacks(0),
+        mWidth(width),
+        mHeight(height),
+        mNextTransaction(nullptr) {
     BufferQueue::createBufferQueue(&mProducer, &mConsumer);
+    mConsumer->setMaxBufferCount(MAX_BUFFERS);
+    mProducer->setMaxDequeuedBufferCount(MAX_BUFFERS - 1);
     mBufferItemConsumer =
             new BufferItemConsumer(mConsumer, AHARDWAREBUFFER_USAGE_GPU_FRAMEBUFFER, 1, true);
     mBufferItemConsumer->setName(String8("BLAST Consumer"));
@@ -34,6 +43,8 @@ BLASTBufferQueue::BLASTBufferQueue(const sp<SurfaceControl>& surface, int width,
     mBufferItemConsumer->setDefaultBufferSize(mWidth, mHeight);
     mBufferItemConsumer->setDefaultBufferFormat(PIXEL_FORMAT_RGBA_8888);
     mBufferItemConsumer->setTransformHint(mSurfaceControl->getTransformHint());
+
+    mAcquired = false;
 }
 
 void BLASTBufferQueue::update(const sp<SurfaceControl>& surface, int width, int height) {
@@ -59,20 +70,32 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp<Fence
                                            const std::vector<SurfaceControlStats>& stats) {
     std::unique_lock _lock{mMutex};
 
-    if (stats.size() > 0 && mNextCallbackBufferItem.mGraphicBuffer != nullptr) {
+    if (stats.size() > 0 && !mShadowQueue.empty()) {
         mBufferItemConsumer->releaseBuffer(mNextCallbackBufferItem,
                                            stats[0].previousReleaseFence
                                                    ? stats[0].previousReleaseFence
                                                    : Fence::NO_FENCE);
+        mAcquired = false;
         mNextCallbackBufferItem = BufferItem();
         mBufferItemConsumer->setTransformHint(stats[0].transformHint);
     }
-    mDequeueWaitCV.notify_all();
+    mPendingCallbacks--;
+    processNextBufferLocked();
+    mCallbackCV.notify_all();
     decStrong((void*)transactionCallbackThunk);
 }
 
-void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) {
-    std::unique_lock _lock{mMutex};
+void BLASTBufferQueue::processNextBufferLocked() {
+    if (mShadowQueue.empty()) {
+        return;
+    }
+
+    if (mAcquired) {
+        return;
+    }
+
+    BufferItem item = std::move(mShadowQueue.front());
+    mShadowQueue.pop();
 
     SurfaceComposerClient::Transaction localTransaction;
     bool applyTransaction = true;
@@ -83,11 +106,11 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) {
         applyTransaction = false;
     }
 
-    int status = OK;
     mNextCallbackBufferItem = mLastSubmittedBufferItem;
-
     mLastSubmittedBufferItem = BufferItem();
-    status = mBufferItemConsumer->acquireBuffer(&mLastSubmittedBufferItem, -1, false);
+
+    status_t status = mBufferItemConsumer->acquireBuffer(&mLastSubmittedBufferItem, -1, false);
+    mAcquired = true;
     if (status != OK) {
         ALOGE("Failed to acquire?");
     }
@@ -99,7 +122,6 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) {
         return;
     }
 
-
     // Ensure BLASTBufferQueue stays alive until we receive the transaction complete callback.
     incStrong((void*)transactionCallbackThunk);
 
@@ -112,17 +134,21 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) {
     t->setCrop(mSurfaceControl, {0, 0, (int32_t)buffer->getWidth(), (int32_t)buffer->getHeight()});
 
     if (applyTransaction) {
-        ALOGE("Apply transaction");
         t->apply();
-
-        if (mNextCallbackBufferItem.mGraphicBuffer != nullptr) {
-            mDequeueWaitCV.wait_for(_lock, 5000ms);
-        }
     }
 }
 
+void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) {
+    std::lock_guard _lock{mMutex};
+
+    // add to shadow queue
+    mShadowQueue.push(item);
+    processNextBufferLocked();
+    mPendingCallbacks++;
+}
+
 void BLASTBufferQueue::setNextTransaction(SurfaceComposerClient::Transaction* t) {
-    std::unique_lock _lock{mMutex};
+    std::lock_guard _lock{mMutex};
     mNextTransaction = t;
 }
 
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index a538e14dfc..2c8d4caae7 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -189,48 +189,49 @@ void TransactionCompletedListener::addSurfaceControlToCallbacks(
 }
 
 void TransactionCompletedListener::onTransactionCompleted(ListenerStats listenerStats) {
-    std::lock_guard<std::mutex> lock(mMutex);
+    std::unordered_map<CallbackId, CallbackTranslation> callbacksMap;
+    {
+        std::lock_guard<std::mutex> lock(mMutex);
 
-    /* This listener knows all the sp<IBinder> to sp<SurfaceControl> for all its registered
-     * callbackIds, except for when Transactions are merged together. This probably cannot be
-     * solved before this point because the Transactions could be merged together and applied in a
-     * different process.
-     *
-     * Fortunately, we get all the callbacks for this listener for the same frame together at the
-     * same time. This means if any Transactions were merged together, we will get their callbacks
-     * at the same time. We can combine all the sp<IBinder> to sp<SurfaceControl> maps for all the
-     * callbackIds to generate one super map that contains all the sp<IBinder> to sp<SurfaceControl>
-     * that could possibly exist for the callbacks.
-     */
-    std::unordered_map<sp<IBinder>, sp<SurfaceControl>, SurfaceComposerClient::IBinderHash>
-            surfaceControls;
-    for (const auto& transactionStats : listenerStats.transactionStats) {
-        for (auto callbackId : transactionStats.callbackIds) {
-            auto& [callbackFunction, callbackSurfaceControls] = mCallbacks[callbackId];
-            surfaceControls.insert(callbackSurfaceControls.begin(), callbackSurfaceControls.end());
+        /* This listener knows all the sp<IBinder> to sp<SurfaceControl> for all its registered
+         * callbackIds, except for when Transactions are merged together. This probably cannot be
+         * solved before this point because the Transactions could be merged together and applied in
+         * a different process.
+         *
+         * Fortunately, we get all the callbacks for this listener for the same frame together at
+         * the same time. This means if any Transactions were merged together, we will get their
+         * callbacks at the same time. We can combine all the sp<IBinder> to sp<SurfaceControl> maps
+         * for all the callbackIds to generate one super map that contains all the sp<IBinder> to
+         * sp<SurfaceControl> that could possibly exist for the callbacks.
+         */
+        callbacksMap = mCallbacks;
+        for (const auto& transactionStats : listenerStats.transactionStats) {
+            for (auto& callbackId : transactionStats.callbackIds) {
+                mCallbacks.erase(callbackId);
+            }
         }
     }
-
     for (const auto& transactionStats : listenerStats.transactionStats) {
         for (auto callbackId : transactionStats.callbackIds) {
-            auto& [callbackFunction, callbackSurfaceControls] = mCallbacks[callbackId];
+            auto& [callbackFunction, callbackSurfaceControls] = callbacksMap[callbackId];
             if (!callbackFunction) {
                 ALOGE("cannot call null callback function, skipping");
                 continue;
             }
             std::vector<SurfaceControlStats> surfaceControlStats;
             for (const auto& surfaceStats : transactionStats.surfaceStats) {
-                surfaceControlStats.emplace_back(surfaceControls[surfaceStats.surfaceControl],
-                                                 surfaceStats.acquireTime,
-                                                 surfaceStats.previousReleaseFence,
-                                                 surfaceStats.transformHint);
-                surfaceControls[surfaceStats.surfaceControl]->setTransformHint(
-                        surfaceStats.transformHint);
+                surfaceControlStats
+                        .emplace_back(callbacksMap[callbackId]
+                                              .surfaceControls[surfaceStats.surfaceControl],
+                                      surfaceStats.acquireTime, surfaceStats.previousReleaseFence,
+                                      surfaceStats.transformHint);
+                callbacksMap[callbackId]
+                        .surfaceControls[surfaceStats.surfaceControl]
+                        ->setTransformHint(surfaceStats.transformHint);
             }
 
             callbackFunction(transactionStats.latchTime, transactionStats.presentFence,
                              surfaceControlStats);
-            mCallbacks.erase(callbackId);
         }
     }
 }
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index 6320556289..f1758a220b 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -27,6 +27,7 @@
 #include <utils/RefBase.h>
 
 #include <system/window.h>
+#include <thread>
 
 namespace android {
 
@@ -50,7 +51,6 @@ public:
     void setNextTransaction(SurfaceComposerClient::Transaction *t);
 
     void update(const sp<SurfaceControl>& surface, int width, int height);
-    
 
     virtual ~BLASTBufferQueue() = default;
 
@@ -61,32 +61,35 @@ private:
     BLASTBufferQueue& operator = (const BLASTBufferQueue& rhs);
     BLASTBufferQueue(const BLASTBufferQueue& rhs);
 
+    void processNextBufferLocked() REQUIRES(mMutex);
+
     sp<SurfaceControl> mSurfaceControl;
-    
-    mutable std::mutex mMutex;
 
-    static const int MAX_BUFFERS = 2;
+    std::mutex mMutex;
+    std::condition_variable mCallbackCV;
+    uint64_t mPendingCallbacks GUARDED_BY(mMutex);
+
+    static const int MAX_BUFFERS = 3;
     struct BufferInfo {
         sp<GraphicBuffer> buffer;
         int fence;
     };
-    
-    int mDequeuedBuffers = 0;
 
-    int mWidth;
-    int mHeight;
+    std::queue<const BufferItem> mShadowQueue GUARDED_BY(mMutex);
+    bool mAcquired GUARDED_BY(mMutex);
 
-    BufferItem mLastSubmittedBufferItem;
-    BufferItem mNextCallbackBufferItem;
-    sp<Fence> mLastFence;
+    int mWidth GUARDED_BY(mMutex);
+    int mHeight GUARDED_BY(mMutex);
 
-    std::condition_variable mDequeueWaitCV;
+    BufferItem mLastSubmittedBufferItem GUARDED_BY(mMutex);
+    BufferItem mNextCallbackBufferItem GUARDED_BY(mMutex);
+    sp<Fence> mLastFence GUARDED_BY(mMutex);
 
     sp<IGraphicBufferConsumer> mConsumer;
     sp<IGraphicBufferProducer> mProducer;
     sp<BufferItemConsumer> mBufferItemConsumer;
 
-    SurfaceComposerClient::Transaction* mNextTransaction = nullptr;
+    SurfaceComposerClient::Transaction* mNextTransaction GUARDED_BY(mMutex);
 };
 
 } // namespace android
diff --git a/libs/gui/tests/BLASTBufferQueue_test.cpp b/libs/gui/tests/BLASTBufferQueue_test.cpp
index ff22913226..6ecdae5fd7 100644
--- a/libs/gui/tests/BLASTBufferQueue_test.cpp
+++ b/libs/gui/tests/BLASTBufferQueue_test.cpp
@@ -19,6 +19,8 @@
 #include <gui/BLASTBufferQueue.h>
 
 #include <android/hardware/graphics/common/1.2/types.h>
+#include <gui/BufferQueueCore.h>
+#include <gui/BufferQueueProducer.h>
 #include <gui/IGraphicBufferProducer.h>
 #include <gui/IProducerListener.h>
 #include <gui/SurfaceComposerClient.h>
@@ -65,9 +67,11 @@ public:
         return mBlastBufferQueueAdapter->mSurfaceControl;
     }
 
-    void waitForCallback() {
+    void waitForCallbacks() {
         std::unique_lock lock{mBlastBufferQueueAdapter->mMutex};
-        mBlastBufferQueueAdapter->mDequeueWaitCV.wait_for(lock, 1s);
+        while (mBlastBufferQueueAdapter->mPendingCallbacks > 0) {
+            mBlastBufferQueueAdapter->mCallbackCV.wait(lock);
+        }
     }
 
 private:
@@ -116,6 +120,17 @@ protected:
                 .apply();
     }
 
+    void setUpProducer(BLASTBufferQueueHelper adapter, sp<IGraphicBufferProducer>& producer) {
+        auto igbProducer = adapter.getIGraphicBufferProducer();
+        ASSERT_NE(nullptr, igbProducer.get());
+        IGraphicBufferProducer::QueueBufferOutput qbOutput;
+        ASSERT_EQ(NO_ERROR,
+                  igbProducer->connect(new DummyProducerListener, NATIVE_WINDOW_API_CPU, false,
+                                       &qbOutput));
+        ASSERT_NE(ui::Transform::orientation_flags::ROT_INVALID, qbOutput.transformHint);
+        producer = igbProducer;
+    }
+
     void fillBuffer(uint32_t* bufData, uint32_t width, uint32_t height, uint32_t stride, uint8_t r,
                     uint8_t g, uint8_t b) {
         for (uint32_t row = 0; row < height; row++) {
@@ -195,14 +210,8 @@ TEST_F(BLASTBufferQueueTest, onFrameAvailable_Apply) {
     uint8_t b = 0;
 
     BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight);
-    auto igbProducer = adapter.getIGraphicBufferProducer();
-    ASSERT_NE(nullptr, igbProducer.get());
-    IGraphicBufferProducer::QueueBufferOutput qbOutput;
-    ASSERT_EQ(NO_ERROR,
-              igbProducer->connect(new DummyProducerListener, NATIVE_WINDOW_API_CPU, false,
-                                   &qbOutput));
-    ASSERT_EQ(NO_ERROR, igbProducer->setMaxDequeuedBufferCount(3));
-    ASSERT_NE(ui::Transform::orientation_flags::ROT_INVALID, qbOutput.transformHint);
+    sp<IGraphicBufferProducer> igbProducer;
+    setUpProducer(adapter, igbProducer);
 
     int slot;
     sp<Fence> fence;
@@ -219,6 +228,7 @@ TEST_F(BLASTBufferQueueTest, onFrameAvailable_Apply) {
     fillBuffer(bufData, buf->getWidth(), buf->getHeight(), buf->getStride(), r, g, b);
     buf->unlock();
 
+    IGraphicBufferProducer::QueueBufferOutput qbOutput;
     IGraphicBufferProducer::QueueBufferInput input(systemTime(), false, HAL_DATASPACE_UNKNOWN,
                                                    Rect(mDisplayWidth, mDisplayHeight),
                                                    NATIVE_WINDOW_SCALING_MODE_FREEZE, 0,
@@ -226,7 +236,7 @@ TEST_F(BLASTBufferQueueTest, onFrameAvailable_Apply) {
     igbProducer->queueBuffer(slot, input, &qbOutput);
     ASSERT_NE(ui::Transform::orientation_flags::ROT_INVALID, qbOutput.transformHint);
 
-    adapter.waitForCallback();
+    sleep(1);
 
     // capture screen and verify that it is red
     bool capturedSecureLayers;
@@ -237,4 +247,43 @@ TEST_F(BLASTBufferQueueTest, onFrameAvailable_Apply) {
                                        /*useIdentityTransform*/ false));
     ASSERT_NO_FATAL_FAILURE(checkScreenCapture(r, g, b));
 }
+
+TEST_F(BLASTBufferQueueTest, TripleBuffering) {
+    BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight);
+    sp<IGraphicBufferProducer> igbProducer;
+    setUpProducer(adapter, igbProducer);
+
+    std::vector<std::pair<int, sp<Fence>>> allocated;
+    for (int i = 0; i < 3; i++) {
+        int slot;
+        sp<Fence> fence;
+        sp<GraphicBuffer> buf;
+        auto ret = igbProducer->dequeueBuffer(&slot, &fence, mDisplayWidth, mDisplayHeight,
+                                              PIXEL_FORMAT_RGBA_8888, GRALLOC_USAGE_SW_WRITE_OFTEN,
+                                              nullptr, nullptr);
+        ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, ret);
+        ASSERT_EQ(OK, igbProducer->requestBuffer(slot, &buf));
+        allocated.push_back({slot, fence});
+    }
+    for (int i = 0; i < allocated.size(); i++) {
+        igbProducer->cancelBuffer(allocated[i].first, allocated[i].second);
+    }
+
+    for (int i = 0; i < 10; i++) {
+        int slot;
+        sp<Fence> fence;
+        sp<GraphicBuffer> buf;
+        auto ret = igbProducer->dequeueBuffer(&slot, &fence, mDisplayWidth, mDisplayHeight,
+                                              PIXEL_FORMAT_RGBA_8888, GRALLOC_USAGE_SW_WRITE_OFTEN,
+                                              nullptr, nullptr);
+        ASSERT_EQ(NO_ERROR, ret);
+        IGraphicBufferProducer::QueueBufferOutput qbOutput;
+        IGraphicBufferProducer::QueueBufferInput input(systemTime(), false, HAL_DATASPACE_UNKNOWN,
+                                                       Rect(mDisplayWidth, mDisplayHeight),
+                                                       NATIVE_WINDOW_SCALING_MODE_FREEZE, 0,
+                                                       Fence::NO_FENCE);
+        igbProducer->queueBuffer(slot, input, &qbOutput);
+    }
+    adapter.waitForCallbacks();
+}
 } // namespace android
diff --git a/libs/gui/tests/IGraphicBufferProducer_test.cpp b/libs/gui/tests/IGraphicBufferProducer_test.cpp
index aef7aed52c..103f7753c8 100644
--- a/libs/gui/tests/IGraphicBufferProducer_test.cpp
+++ b/libs/gui/tests/IGraphicBufferProducer_test.cpp
@@ -543,7 +543,6 @@ TEST_P(IGraphicBufferProducerTest, SetMaxDequeuedBufferCount_Succeeds) {
     // Should now be able to dequeue up to minBuffers times
     DequeueBufferResult result;
     for (int i = 0; i < minBuffers; ++i) {
-
         EXPECT_EQ(OK, ~IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION &
                 (dequeueBuffer(DEFAULT_WIDTH, DEFAULT_HEIGHT, DEFAULT_FORMAT,
                               TEST_PRODUCER_USAGE_BITS, &result)))