Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 86770e53 authored by Alec Mouri's avatar Alec Mouri
Browse files

Remove RenderEngine::flush from latchBuffer() 

Fence fd will be owned by SurfaceFlinger to be propagated down to
latchBuffer for merging sync fences. We can't store the fd at the
Layer-level & flush on every draw() invocation for some reason (I don't
fully understand what the gl driver does under the hood, but from what
it looks like file descriptors are reused when the command stream is
flushed too often which causes a memory leak when a buffer's sync fence
is merged), so we'll let SF backend store the flush fence for the
previous frame.

Bug: 116277151
Change-Id: I7901b0178aa0f11505650bf5e1df6f085a5d93bf
Test: SurfaceFlinger_test, libsurfaceflinger_unittest, go/wm-smoke
parent 3221a897

services/surfaceflinger/BufferLayer.cpp

+3 −2
Original line number Diff line number Diff line
@@ -327,7 +327,8 @@ bool BufferLayer::onPostComposition(const std::shared_ptr<FenceTime>& glDoneFenc 
    return true;

}



Region BufferLayer::latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime) {

Region BufferLayer::latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime,

                                const sp<Fence>& releaseFence) {

    ATRACE_CALL();



    std::optional<Region> sidebandStreamDirtyRegion = latchSidebandStream(recomputeVisibleRegions);
@@ -368,7 +369,7 @@ Region BufferLayer::latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime 
        return dirtyRegion;

    }



    status_t err = updateTexImage(recomputeVisibleRegions, latchTime);

    status_t err = updateTexImage(recomputeVisibleRegions, latchTime, releaseFence);

    if (err != NO_ERROR) {

        return dirtyRegion;

    }

services/surfaceflinger/BufferLayer.h

+7 −2
Original line number Diff line number Diff line
@@ -91,7 +91,11 @@ public: 
    // the visible regions need to be recomputed (this is a fairly heavy

    // operation, so this should be set only if needed). Typically this is used

    // to figure out if the content or size of a surface has changed.

    Region latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime) override;

    // If there was a GL composition step rendering the previous frame, then

    // releaseFence will be populated with a native fence that fires when

    // composition has completed.

    Region latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime,

                       const sp<Fence>& releaseFence) override;



    bool isBufferLatched() const override { return mRefreshPending; }
@@ -135,7 +139,8 @@ private: 
    virtual void setFilteringEnabled(bool enabled) = 0;



    virtual status_t bindTextureImage() const = 0;

    virtual status_t updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime) = 0;

    virtual status_t updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime,

                                    const sp<Fence>& flushFence) = 0;



    virtual status_t updateActiveBuffer() = 0;

    virtual status_t updateFrameNumber(nsecs_t latchTime) = 0;

services/surfaceflinger/BufferLayerConsumer.cpp

+13 −11
Original line number Diff line number Diff line
@@ -147,7 +147,8 @@ nsecs_t BufferLayerConsumer::computeExpectedPresent(const DispSync& dispSync) { 


status_t BufferLayerConsumer::updateTexImage(BufferRejecter* rejecter, const DispSync& dispSync,

                                             bool* autoRefresh, bool* queuedBuffer,

                                             uint64_t maxFrameNumber) {

                                             uint64_t maxFrameNumber,

                                             const sp<Fence>& releaseFence) {

    ATRACE_CALL();

    BLC_LOGV("updateTexImage");

    Mutex::Autolock lock(mMutex);
@@ -198,7 +199,7 @@ status_t BufferLayerConsumer::updateTexImage(BufferRejecter* rejecter, const Dis 
    }



    // Release the previous buffer.

    err = updateAndReleaseLocked(item, &mPendingRelease);

    err = updateAndReleaseLocked(item, &mPendingRelease, releaseFence);

    if (err != NO_ERROR) {

        return err;

    }
@@ -278,14 +279,15 @@ status_t BufferLayerConsumer::acquireBufferLocked(BufferItem* item, nsecs_t pres 
}



status_t BufferLayerConsumer::updateAndReleaseLocked(const BufferItem& item,

                                                     PendingRelease* pendingRelease) {

                                                     PendingRelease* pendingRelease,

                                                     const sp<Fence>& releaseFence) {

    status_t err = NO_ERROR;



    int slot = item.mSlot;



    // Do whatever sync ops we need to do before releasing the old slot.

    if (slot != mCurrentTexture) {

        err = syncForReleaseLocked();

        err = syncForReleaseLocked(releaseFence);

        if (err != NO_ERROR) {

            // Release the buffer we just acquired.  It's not safe to

            // release the old buffer, so instead we just drop the new frame.
@@ -367,19 +369,19 @@ status_t BufferLayerConsumer::bindTextureImageLocked() { 
    return doFenceWaitLocked();

}



status_t BufferLayerConsumer::syncForReleaseLocked() {

status_t BufferLayerConsumer::syncForReleaseLocked(const sp<Fence>& releaseFence) {

    BLC_LOGV("syncForReleaseLocked");



    if (mCurrentTexture != BufferQueue::INVALID_BUFFER_SLOT) {

        if (mRE.useNativeFenceSync()) {

            base::unique_fd fenceFd = mRE.flush();

            if (fenceFd == -1) {

        if (mRE.useNativeFenceSync() && releaseFence != Fence::NO_FENCE) {

            // TODO(alecmouri): fail further upstream if the fence is invalid

            if (!releaseFence->isValid()) {

                BLC_LOGE("syncForReleaseLocked: failed to flush RenderEngine");

                return UNKNOWN_ERROR;

            }

            sp<Fence> fence(new Fence(std::move(fenceFd)));

            status_t err = addReleaseFenceLocked(mCurrentTexture,

                                                 mCurrentTextureImage->graphicBuffer(), fence);

            status_t err =

                    addReleaseFenceLocked(mCurrentTexture, mCurrentTextureImage->graphicBuffer(),

                                          releaseFence);

            if (err != OK) {

                BLC_LOGE("syncForReleaseLocked: error adding release fence: "

                         "%s (%d)",

services/surfaceflinger/BufferLayerConsumer.h

+5 −3
Original line number Diff line number Diff line
@@ -94,7 +94,8 @@ public: 
    // used to reject the newly acquired buffer.  It also does not bind the

    // RenderEngine texture until bindTextureImage is called.

    status_t updateTexImage(BufferRejecter* rejecter, const DispSync& dispSync, bool* autoRefresh,

                            bool* queuedBuffer, uint64_t maxFrameNumber);

                            bool* queuedBuffer, uint64_t maxFrameNumber,

                            const sp<Fence>& releaseFence);



    // See BufferLayerConsumer::bindTextureImageLocked().

    status_t bindTextureImage();
@@ -208,7 +209,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,

                                    const sp<Fence>& releaseFence = Fence::NO_FENCE);



    // Binds mTexName and the current buffer to TEXTURE_EXTERNAL target.  Uses

    // mCurrentTexture if it's set, mCurrentTextureImage if not.  If the
@@ -282,7 +284,7 @@ private: 
    // current slot from RenderEngine.  If needed it will set the current

    // slot's fence to guard against a producer accessing the buffer before

    // the outstanding accesses have completed.

    status_t syncForReleaseLocked();

    status_t syncForReleaseLocked(const sp<Fence>& releaseFence);



    // The default consumer usage flags that BufferLayerConsumer always sets on its

    // BufferQueue instance; these will be OR:d with any additional flags passed

services/surfaceflinger/BufferQueueLayer.cpp

+3 −2
Original line number Diff line number Diff line
@@ -236,7 +236,8 @@ status_t BufferQueueLayer::bindTextureImage() const { 
    return mConsumer->bindTextureImage();

}



status_t BufferQueueLayer::updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime) {

status_t BufferQueueLayer::updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime,

                                          const sp<Fence>& releaseFence) {

    // This boolean is used to make sure that SurfaceFlinger's shadow copy

    // of the buffer queue isn't modified when the buffer queue is returning

    // BufferItem's that weren't actually queued. This can happen in shared
@@ -247,7 +248,7 @@ status_t BufferQueueLayer::updateTexImage(bool& recomputeVisibleRegions, nsecs_t 
                    getTransformToDisplayInverse(), mFreezeGeometryUpdates);

    status_t updateResult =

            mConsumer->updateTexImage(&r, *mFlinger->mPrimaryDispSync, &mAutoRefresh, &queuedBuffer,

                                      mLastFrameNumberReceived);

                                      mLastFrameNumberReceived, releaseFence);

    if (updateResult == BufferQueue::PRESENT_LATER) {

        // Producer doesn't want buffer to be displayed yet.  Signal a

        // layer update so we check again at the next opportunity.