bufferqueues: Entirely remove support for passing GL fences into BQs

There are very few if any real clients (i.e. non goldfish) that use
these fences with bufferqueues. We can simplify the overall API by just
removing it and doing explicit sync (which is what would happen anyway)
where we would have passed them into the bufferqueue.

Bug: 339705065
Flag: com.android.graphics.libgui.flags.bq_gl_fence_cleanup
Test: old tests

Change-Id: I1f3973c78aafe278708f203ef46a2b91b138eba7

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
     };
 
     /**