Merge "bufferqueues: Entirely remove support for passing GL fences into BQs" into main

Author: Jim Shargo

libs/gui/BufferQueueConsumer.cpp

@@ -477,9 +477,14 @@ status_t BufferQueueConsumer::attachBuffer(int* outSlot,
     return NO_ERROR;
 }
 
+#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
+status_t BufferQueueConsumer::releaseBuffer(int slot, uint64_t frameNumber,
+                                            const sp<Fence>& releaseFence) {
+#else
 status_t BufferQueueConsumer::releaseBuffer(int slot, uint64_t frameNumber,
         const sp<Fence>& releaseFence, EGLDisplay eglDisplay,
         EGLSyncKHR eglFence) {
+#endif
     ATRACE_CALL();
     ATRACE_BUFFER_INDEX(slot);
 
@@ -493,27 +498,6 @@ status_t BufferQueueConsumer::releaseBuffer(int slot, uint64_t frameNumber,
         return BAD_VALUE;
     }
 
-#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
-    if (eglFence != EGL_NO_SYNC_KHR) {
-        // Most platforms will be using native fences, so it's unlikely that we'll ever have to
-        // process an eglFence. Ideally we can remove this code eventually. In the mean time, do our
-        // best to wait for it so the buffer stays valid, otherwise return an error to the caller.
-        //
-        // EGL_SYNC_FLUSH_COMMANDS_BIT_KHR so that we don't wait forever on a fence that hasn't
-        // shown up on the GPU yet.
-        EGLint result = eglClientWaitSyncKHR(eglDisplay, eglFence, EGL_SYNC_FLUSH_COMMANDS_BIT_KHR,
-                                             1000000000);
-        if (result == EGL_FALSE) {
-            BQ_LOGE("releaseBuffer: error %#x waiting for fence", eglGetError());
-            return UNKNOWN_ERROR;
-        } else if (result == EGL_TIMEOUT_EXPIRED_KHR) {
-            BQ_LOGE("releaseBuffer: timeout waiting for fence");
-            return UNKNOWN_ERROR;
-        }
-        eglDestroySyncKHR(eglDisplay, eglFence);
-    }
-#endif
-
     sp<IProducerListener> listener;
     { // Autolock scope
         std::lock_guard<std::mutex> lock(mCore->mMutex);

libs/gui/ConsumerBase.cpp

@@ -656,9 +656,13 @@ status_t ConsumerBase::addReleaseFenceLocked(int slot,
     return OK;
 }
 
+#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
+status_t ConsumerBase::releaseBufferLocked(int slot, const sp<GraphicBuffer> graphicBuffer) {
+#else
 status_t ConsumerBase::releaseBufferLocked(
         int slot, const sp<GraphicBuffer> graphicBuffer,
         EGLDisplay display, EGLSyncKHR eglFence) {
+#endif
     if (mAbandoned) {
         CB_LOGE("releaseBufferLocked: ConsumerBase is abandoned!");
         return NO_INIT;
@@ -675,8 +679,12 @@ status_t ConsumerBase::releaseBufferLocked(
 
     CB_LOGV("releaseBufferLocked: slot=%d/%" PRIu64,
             slot, mSlots[slot].mFrameNumber);
+#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
+    status_t err = mConsumer->releaseBuffer(slot, mSlots[slot].mFrameNumber, mSlots[slot].mFence);
+#else
     status_t err = mConsumer->releaseBuffer(slot, mSlots[slot].mFrameNumber,
             display, eglFence, mSlots[slot].mFence);
+#endif
     if (err == IGraphicBufferConsumer::STALE_BUFFER_SLOT) {
         freeBufferLocked(slot);
     }

libs/gui/GLConsumer.cpp

@@ -417,18 +417,18 @@ void GLConsumer::onSlotCountChanged(int slotCount) {
 }
 #endif
 
-status_t GLConsumer::releaseBufferLocked(int buf,
-        sp<GraphicBuffer> graphicBuffer,
-        EGLDisplay display, EGLSyncKHR eglFence) {
+#if !COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
+status_t GLConsumer::releaseBufferLocked(int buf, sp<GraphicBuffer> graphicBuffer,
+                                         EGLDisplay display, EGLSyncKHR eglFence) {
     // release the buffer if it hasn't already been discarded by the
     // BufferQueue. This can happen, for example, when the producer of this
     // buffer has reallocated the original buffer slot after this buffer
     // was acquired.
-    status_t err = ConsumerBase::releaseBufferLocked(
-            buf, graphicBuffer, display, eglFence);
+    status_t err = ConsumerBase::releaseBufferLocked(buf, graphicBuffer, display, eglFence);
     mEglSlots[buf].mEglFence = EGL_NO_SYNC_KHR;
     return err;
 }
+#endif
 
 status_t GLConsumer::updateAndReleaseLocked(const BufferItem& item,
         PendingRelease* pendingRelease)
@@ -490,9 +490,14 @@ status_t GLConsumer::updateAndReleaseLocked(const BufferItem& item,
     // release old buffer
     if (mCurrentTexture != BufferQueue::INVALID_BUFFER_SLOT) {
         if (pendingRelease == nullptr) {
+#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
+            status_t status =
+                    releaseBufferLocked(mCurrentTexture, mCurrentTextureImage->graphicBuffer());
+#else
             status_t status = releaseBufferLocked(
                     mCurrentTexture, mCurrentTextureImage->graphicBuffer(),
                     mEglDisplay, mEglSlots[mCurrentTexture].mEglFence);
+#endif
             if (status < NO_ERROR) {
                 GLC_LOGE("updateAndRelease: failed to release buffer: %s (%d)",
                         strerror(-status), status);
@@ -501,10 +506,7 @@ status_t GLConsumer::updateAndReleaseLocked(const BufferItem& item,
             }
         } else {
             pendingRelease->currentTexture = mCurrentTexture;
-            pendingRelease->graphicBuffer =
-                    mCurrentTextureImage->graphicBuffer();
-            pendingRelease->display = mEglDisplay;
-            pendingRelease->fence = mEglSlots[mCurrentTexture].mEglFence;
+            pendingRelease->graphicBuffer = mCurrentTextureImage->graphicBuffer();
             pendingRelease->isPending = true;
         }
     }
@@ -744,6 +746,11 @@ status_t GLConsumer::syncForReleaseLocked(EGLDisplay dpy) {
                 return err;
             }
         } else if (mUseFenceSync && SyncFeatures::getInstance().useFenceSync()) {
+#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
+            // Basically all clients are using native fence syncs. If they aren't, we lose nothing
+            // by waiting here, because the alternative can cause deadlocks (b/339705065).
+            glFinish();
+#else
             EGLSyncKHR fence = mEglSlots[mCurrentTexture].mEglFence;
             if (fence != EGL_NO_SYNC_KHR) {
                 // There is already a fence for the current slot.  We need to
@@ -773,6 +780,7 @@ status_t GLConsumer::syncForReleaseLocked(EGLDisplay dpy) {
             }
             glFlush();
             mEglSlots[mCurrentTexture].mEglFence = fence;
+#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
         }
     }
 

libs/gui/IGraphicBufferConsumer.cpp

@@ -85,13 +85,20 @@ class BpGraphicBufferConsumer : public SafeBpInterface<IGraphicBufferConsumer> {
         return callRemote<Signature>(Tag::ATTACH_BUFFER, slot, buffer);
     }
 
+#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
+    status_t releaseBuffer(int buf, uint64_t frameNumber, const sp<Fence>& releaseFence) override {
+        using Signature = status_t (IGraphicBufferConsumer::*)(int, uint64_t, const sp<Fence>&);
+        return callRemote<Signature>(Tag::RELEASE_BUFFER, buf, frameNumber, releaseFence);
+    }
+#else
     status_t releaseBuffer(int buf, uint64_t frameNumber,
                            EGLDisplay display __attribute__((unused)),
                            EGLSyncKHR fence __attribute__((unused)),
                            const sp<Fence>& releaseFence) override {
         using Signature = status_t (IGraphicBufferConsumer::*)(int, uint64_t, const sp<Fence>&);
         return callRemote<Signature>(Tag::RELEASE_BUFFER, buf, frameNumber, releaseFence);
     }
+#endif
 
     status_t consumerConnect(const sp<IConsumerListener>& consumer, bool controlledByApp) override {
         using Signature = decltype(&IGraphicBufferConsumer::consumerConnect);

libs/gui/include/gui/BufferQueueConsumer.h

@@ -65,13 +65,14 @@ class BufferQueueConsumer : public BnGraphicBufferConsumer {
     // any references to the just-released buffer that it might have, as if it
     // had received a onBuffersReleased() call with a mask set for the released
     // buffer.
-    //
-    // Note that the dependencies on EGL will be removed once we switch to using
-    // the Android HW Sync HAL.
+#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
+    virtual status_t releaseBuffer(int slot, uint64_t frameNumber,
+                                   const sp<Fence>& releaseFence) override;
+#else
     virtual status_t releaseBuffer(int slot, uint64_t frameNumber,
             const sp<Fence>& releaseFence, EGLDisplay display,
             EGLSyncKHR fence);
-
+#endif
     // connect connects a consumer to the BufferQueue.  Only one
     // consumer may be connected, and when that consumer disconnects the
     // BufferQueue is placed into the "abandoned" state, causing most
@@ -167,6 +168,7 @@ class BufferQueueConsumer : public BnGraphicBufferConsumer {
     // dump our state in a String
     status_t dumpState(const String8& prefix, String8* outResult) const override;
 
+#if !COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
     // Functions required for backwards compatibility.
     // These will be modified/renamed in IGraphicBufferConsumer and will be
     // removed from this class at that time. See b/13306289.
@@ -176,6 +178,7 @@ class BufferQueueConsumer : public BnGraphicBufferConsumer {
             const sp<Fence>& releaseFence) {
         return releaseBuffer(buf, frameNumber, releaseFence, display, fence);
     }
+#endif
 
     virtual status_t consumerConnect(const sp<IConsumerListener>& consumer,
             bool controlledByApp) {

libs/gui/include/gui/ConsumerBase.h

@@ -245,10 +245,13 @@ class ConsumerBase : public virtual RefBase,
     // must take place when a buffer is released back to the BufferQueue.  If
     // it is overridden the derived class's implementation must call
     // ConsumerBase::releaseBufferLocked.
+#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
+    virtual status_t releaseBufferLocked(int slot, const sp<GraphicBuffer> graphicBuffer);
+#else
     virtual status_t releaseBufferLocked(int slot,
             const sp<GraphicBuffer> graphicBuffer,
             EGLDisplay display = EGL_NO_DISPLAY, EGLSyncKHR eglFence = EGL_NO_SYNC_KHR);
-
+#endif
     // returns true iff the slot still has the graphicBuffer in it.
     bool stillTracking(int slot, const sp<GraphicBuffer> graphicBuffer);
 

libs/gui/include/gui/GLConsumer.h

@@ -271,6 +271,7 @@ class GLConsumer : public ConsumerBase {
 #endif
     // releaseBufferLocked overrides the ConsumerBase method to update the
     // mEglSlots array in addition to the ConsumerBase.
+#if !COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
     virtual status_t releaseBufferLocked(int slot, const sp<GraphicBuffer> graphicBuffer,
                                          EGLDisplay display = EGL_NO_DISPLAY,
                                          EGLSyncKHR eglFence = EGL_NO_SYNC_KHR) override;
@@ -279,16 +280,14 @@ class GLConsumer : public ConsumerBase {
             const sp<GraphicBuffer> graphicBuffer, EGLSyncKHR eglFence) {
         return releaseBufferLocked(slot, graphicBuffer, mEglDisplay, eglFence);
     }
+#endif
 
     struct PendingRelease {
-        PendingRelease() : isPending(false), currentTexture(-1),
-                graphicBuffer(), display(nullptr), fence(nullptr) {}
+        PendingRelease() : isPending(false), currentTexture(-1), graphicBuffer() {}
 
         bool isPending;
         int currentTexture;
         sp<GraphicBuffer> graphicBuffer;
-        EGLDisplay display;
-        EGLSyncKHR fence;
     };
 
     // This releases the buffer in the slot referenced by mCurrentTexture,
@@ -468,16 +467,18 @@ class GLConsumer : public ConsumerBase {
     // EGLSlot contains the information and object references that
     // GLConsumer maintains about a BufferQueue buffer slot.
     struct EglSlot {
+#if !COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
         EglSlot() : mEglFence(EGL_NO_SYNC_KHR) {}
-
+#endif
         // mEglImage is the EGLImage created from mGraphicBuffer.
         sp<EglImage> mEglImage;
-
+#if !COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
         // mFence is the EGL sync object that must signal before the buffer
         // associated with this buffer slot may be dequeued. It is initialized
         // to EGL_NO_SYNC_KHR when the buffer is created and (optionally, based
         // on a compile-time option) set to a new sync object in updateTexImage.
         EGLSyncKHR mEglFence;
+#endif
     };
 
     // mEglDisplay is the EGLDisplay with which this GLConsumer is currently

libs/gui/include/gui/IGraphicBufferConsumer.h

@@ -139,12 +139,17 @@ class IGraphicBufferConsumer : public RefBase {
     //               * the buffer slot was invalid
     //               * the fence was NULL
     //               * the buffer slot specified is not in the acquired state
+#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
+    virtual status_t releaseBuffer(int buf, uint64_t frameNumber,
+                                   const sp<Fence>& releaseFence) = 0;
+#else
     virtual status_t releaseBuffer(int buf, uint64_t frameNumber, EGLDisplay display,
                                    EGLSyncKHR fence, const sp<Fence>& releaseFence) = 0;
 
     status_t releaseBuffer(int buf, uint64_t frameNumber, const sp<Fence>& releaseFence) {
         return releaseBuffer(buf, frameNumber, EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, releaseFence);
     }
+#endif
 
     // consumerConnect connects a consumer to the BufferQueue. Only one consumer may be connected,
     // and when that consumer disconnects the BufferQueue is placed into the "abandoned" state,

libs/gui/include/gui/mock/GraphicBufferConsumer.h

@@ -18,6 +18,7 @@
 
 #include <gmock/gmock.h>
 
+#include <com_android_graphics_libgui_flags.h>
 #include <gui/IGraphicBufferConsumer.h>
 
 #include <utils/RefBase.h>
@@ -33,7 +34,11 @@ class GraphicBufferConsumer : public BnGraphicBufferConsumer, public virtual and
     MOCK_METHOD3(acquireBuffer, status_t(BufferItem*, nsecs_t, uint64_t));
     MOCK_METHOD1(detachBuffer, status_t(int));
     MOCK_METHOD2(attachBuffer, status_t(int*, const sp<GraphicBuffer>&));
+#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
+    MOCK_METHOD3(releaseBuffer, status_t(int, uint64_t, const sp<Fence>&));
+#else
     MOCK_METHOD5(releaseBuffer, status_t(int, uint64_t, EGLDisplay, EGLSyncKHR, const sp<Fence>&));
+#endif
     MOCK_METHOD2(consumerConnect, status_t(const sp<IConsumerListener>&, bool));
     MOCK_METHOD0(consumerDisconnect, status_t());
     MOCK_METHOD1(getReleasedBuffers, status_t(uint64_t*));

libs/nativedisplay/include/surfacetexture/EGLConsumer.h

@@ -113,18 +113,11 @@ class EGLConsumer {
 
 protected:
     struct PendingRelease {
-        PendingRelease()
-              : isPending(false),
-                currentTexture(-1),
-                graphicBuffer(),
-                display(nullptr),
-                fence(nullptr) {}
+        PendingRelease() : isPending(false), currentTexture(-1), graphicBuffer() {}
 
         bool isPending;
         int currentTexture;
         sp<GraphicBuffer> graphicBuffer;
-        EGLDisplay display;
-        EGLSyncKHR fence;
     };
 
     /**
@@ -250,20 +243,24 @@ class EGLConsumer {
      * EGLConsumer maintains about a BufferQueue buffer slot.
      */
     struct EglSlot {
-        EglSlot() : mEglFence(EGL_NO_SYNC_KHR) {}
+#if !COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
 
+        EglSlot() : mEglFence(EGL_NO_SYNC_KHR) {}
+#endif
         /**
          * mEglImage is the EGLImage created from mGraphicBuffer.
          */
         sp<EglImage> mEglImage;
 
+#if !COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_GL_FENCE_CLEANUP)
         /**
          * mFence is the EGL sync object that must signal before the buffer
          * associated with this buffer slot may be dequeued. It is initialized
          * to EGL_NO_SYNC_KHR when the buffer is created and (optionally, based
          * on a compile-time option) set to a new sync object in updateTexImage.
          */
         EGLSyncKHR mEglFence;
+#endif
     };
 
     /**