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

Commit b8651c35 authored by Cairn Overturf's avatar Cairn Overturf
Browse files

Fix logging when buffers are out of order 

frameNumber should be reset when producer id changes

Bug: 419339746
Flag: EXEMPT bug fix
Test: presubmit, ran a few apps didn't see any unexpected logs.
Change-Id: I5a5fdfb07f0eac4915474875e683290a082f3a99
parent dd21a004

libs/gui/include/gui/LayerState.h

+5 −0
Original line number Diff line number Diff line
@@ -626,6 +626,11 @@ static inline int compare_type(const DisplayState& lhs, const DisplayState& rhs) 
    return compare_type(lhs.token, rhs.token);

}



static inline bool isFrameBarrierNewer(uint32_t producerIdA, uint64_t frameNumberA,

                                       uint32_t producerIdB, uint64_t frameNumberB) {

    return producerIdA > producerIdB || (producerIdA == producerIdB && frameNumberA > frameNumberB);

}



}; // namespace android



#endif // ANDROID_SF_LAYER_STATE_H

services/surfaceflinger/FrontEnd/RequestedLayerState.cpp

+6 −7
Original line number Diff line number Diff line
@@ -217,19 +217,18 @@ void RequestedLayerState::merge(const ResolvedComposerState& resolvedComposerSta 
                    frameNumberChanged ? bufferData->frameNumber : oldFramenumber + 1;

            bufferData->frameNumber = frameNumber;



            if ((barrierProducerId > bufferData->producerId) ||

                ((barrierProducerId == bufferData->producerId) &&

                 (barrierFrameNumber > bufferData->frameNumber))) {

                ALOGE("Out of order buffers detected for %s producedId=%d frameNumber=%" PRIu64

            if (isFrameBarrierNewer(barrierProducerId, barrierFrameNumber, bufferData->producerId,

                                    bufferData->frameNumber)) {

                ALOGE("Out-of-order buffers detected for %s producedId=%d frameNumber=%" PRIu64

                      " -> producedId=%d frameNumber=%" PRIu64,

                      getDebugString().c_str(), barrierProducerId, barrierFrameNumber,

                      bufferData->producerId, frameNumber);

                TransactionTraceWriter::getInstance().invoke("out_of_order_buffers_",

                                                             /*overwrite=*/false);

            } else {

                barrierProducerId = bufferData->producerId;

                barrierFrameNumber = bufferData->frameNumber;

            }



            barrierProducerId = std::max(bufferData->producerId, barrierProducerId);

            barrierFrameNumber = std::max(bufferData->frameNumber, barrierFrameNumber);

        }



        const bool newBufferFormatOpaque = LayerSnapshot::isOpaqueFormat(

services/surfaceflinger/Layer.cpp

+4 −25
Original line number Diff line number Diff line
@@ -143,9 +143,7 @@ Layer::Layer(const surfaceflinger::LayerCreationArgs& args) 
    mDrawingState.transform.set(0, 0);

    mDrawingState.frameNumber = 0;

    mDrawingState.previousFrameNumber = 0;

    mDrawingState.barrierFrameNumber = 0;

    mDrawingState.producerId = 0;

    mDrawingState.barrierProducerId = 0;

    mDrawingState.bufferTransform = 0;

    mDrawingState.transformToDisplayInverse = false;

    mDrawingState.acquireFence = sp<Fence>::make(-1);
@@ -902,12 +900,6 @@ bool Layer::setBuffer(std::shared_ptr<renderengine::ExternalTexture>& buffer, 
        REQUIRES(mFlinger->mStateLock) {

    SFTRACE_FORMAT("setBuffer %s - hasBuffer=%s", getDebugName(), (buffer ? "true" : "false"));



    const bool frameNumberChanged =

            bufferData.flags.test(BufferData::BufferDataChange::frameNumberChanged);

    const uint64_t frameNumber =

            frameNumberChanged ? bufferData.frameNumber : mDrawingState.frameNumber + 1;

    SFTRACE_FORMAT_INSTANT("setBuffer %s - %" PRIu64, getDebugName(), frameNumber);



    if (mDrawingState.buffer) {

        releasePreviousBuffer();

    } else if (buffer) {
@@ -933,22 +925,9 @@ bool Layer::setBuffer(std::shared_ptr<renderengine::ExternalTexture>& buffer, 
        }

    }



    if ((mDrawingState.producerId > bufferData.producerId) ||

        ((mDrawingState.producerId == bufferData.producerId) &&

         (mDrawingState.frameNumber > frameNumber))) {

        ALOGE("Out of order buffers detected for %s producedId=%d frameNumber=%" PRIu64

              " -> producedId=%d frameNumber=%" PRIu64,

              getDebugName(), mDrawingState.producerId, mDrawingState.frameNumber,

              bufferData.producerId, frameNumber);

        TransactionTraceWriter::getInstance().invoke("out_of_order_buffers_", /*overwrite=*/false);

    }



    mDrawingState.producerId = bufferData.producerId;

    mDrawingState.barrierProducerId =

            std::max(mDrawingState.producerId, mDrawingState.barrierProducerId);

    mDrawingState.frameNumber = frameNumber;

    mDrawingState.barrierFrameNumber =

            std::max(mDrawingState.frameNumber, mDrawingState.barrierFrameNumber);

    mDrawingState.frameNumber = bufferData.frameNumber;

    SFTRACE_FORMAT_INSTANT("setBuffer %s - %" PRIu64, getDebugName(), bufferData.frameNumber);



    mDrawingState.releaseBufferListener = bufferData.releaseBufferListener;

    mDrawingState.previousBuffer = std::move(mDrawingState.buffer);
@@ -976,10 +955,10 @@ bool Layer::setBuffer(std::shared_ptr<renderengine::ExternalTexture>& buffer, 
    if (bufferData.dequeueTime > 0) {

        const uint64_t bufferId = mDrawingState.buffer->getId();

        mFlinger->mFrameTracer->traceNewLayer(layerId, getName().c_str());

        mFlinger->mFrameTracer->traceTimestamp(layerId, bufferId, frameNumber,

        mFlinger->mFrameTracer->traceTimestamp(layerId, bufferId, bufferData.frameNumber,

                                               bufferData.dequeueTime,

                                               FrameTracer::FrameEvent::DEQUEUE);

        mFlinger->mFrameTracer->traceTimestamp(layerId, bufferId, frameNumber, postTime,

        mFlinger->mFrameTracer->traceTimestamp(layerId, bufferId, bufferData.frameNumber, postTime,

                                               FrameTracer::FrameEvent::QUEUE);

    }

services/surfaceflinger/Layer.h

+0 −6
Original line number Diff line number Diff line
@@ -104,15 +104,9 @@ public: 


        uint64_t frameNumber;

        uint64_t previousFrameNumber;

        // high watermark framenumber to use to check for barriers to protect ourselves

        // from out of order transactions

        uint64_t barrierFrameNumber;

        ui::Transform transform;



        uint32_t producerId = 0;

        // high watermark producerId to use to check for barriers to protect ourselves

        // from out of order transactions

        uint32_t barrierProducerId = 0;



        uint32_t bufferTransform;

        bool transformToDisplayInverse;