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

Commit 05289c2b authored by Pablo Ceballos's avatar Pablo Ceballos
Browse files

SF: Don't pop pending state into mCurrentState 

Let mCurrentState always represent the most up to date state, even if
there are transactions we don't want to commit yet. Past snapshots of
the state are kept in mPendingState. In doTransaction(), grab the
correct pending state from mPendingState, but save it to a local copy
that gets committed rather than overriding mCurrentState.

Bug 27205755

Change-Id: Ib0ea809da1954409787c52b8f41d7963a57a6a4c
parent 9ffa1a4d

services/surfaceflinger/Layer.cpp

+15 −16
Original line number Original line Diff line number Diff line
@@ -1309,16 +1309,16 @@ void Layer::pushPendingState() { 
    mPendingStates.push_back(mCurrentState);

    mPendingStates.push_back(mCurrentState);

}

}





void Layer::popPendingState() {

void Layer::popPendingState(State* stateToCommit) {

    auto oldFlags = mCurrentState.flags;

    auto oldFlags = stateToCommit->flags;

    mCurrentState = mPendingStates[0];

    *stateToCommit = mPendingStates[0];

    mCurrentState.flags = (oldFlags & ~mCurrentState.mask) |

    stateToCommit->flags = (oldFlags & ~stateToCommit->mask) |

            (mCurrentState.flags & mCurrentState.mask);

            (stateToCommit->flags & stateToCommit->mask);





    mPendingStates.removeAt(0);

    mPendingStates.removeAt(0);

}

}





bool Layer::applyPendingStates() {

bool Layer::applyPendingStates(State* stateToCommit) {

    bool stateUpdateAvailable = false;

    bool stateUpdateAvailable = false;

    while (!mPendingStates.empty()) {

    while (!mPendingStates.empty()) {

        if (mPendingStates[0].handle != nullptr) {

        if (mPendingStates[0].handle != nullptr) {
@@ -1327,7 +1327,7 @@ bool Layer::applyPendingStates() { 
                // will be visually wrong, but it should keep us from getting

                // will be visually wrong, but it should keep us from getting

                // into too much trouble.

                // into too much trouble.

                ALOGE("[%s] No local sync point found", mName.string());

                ALOGE("[%s] No local sync point found", mName.string());

                popPendingState();

                popPendingState(stateToCommit);

                stateUpdateAvailable = true;

                stateUpdateAvailable = true;

                continue;

                continue;

            }

            }
@@ -1345,7 +1345,7 @@ bool Layer::applyPendingStates() { 




            if (mRemoteSyncPoints.front()->frameIsAvailable()) {

            if (mRemoteSyncPoints.front()->frameIsAvailable()) {

                // Apply the state update

                // Apply the state update

                popPendingState();

                popPendingState(stateToCommit);

                stateUpdateAvailable = true;

                stateUpdateAvailable = true;





                // Signal our end of the sync point and then dispose of it

                // Signal our end of the sync point and then dispose of it
@@ -1355,7 +1355,7 @@ bool Layer::applyPendingStates() { 
                break;

                break;

            }

            }

        } else {

        } else {

            popPendingState();

            popPendingState(stateToCommit);

            stateUpdateAvailable = true;

            stateUpdateAvailable = true;

        }

        }

    }

    }
@@ -1385,12 +1385,12 @@ uint32_t Layer::doTransaction(uint32_t flags) { 
    ATRACE_CALL();

    ATRACE_CALL();





    pushPendingState();

    pushPendingState();

    if (!applyPendingStates()) {

    Layer::State c = getCurrentState();

    if (!applyPendingStates(&c)) {

        return 0;

        return 0;

    }

    }





    const Layer::State& s(getDrawingState());

    const Layer::State& s(getDrawingState());

    const Layer::State& c(getCurrentState());





    const bool sizeChanged = (c.requested.w != s.requested.w) ||

    const bool sizeChanged = (c.requested.w != s.requested.w) ||

                             (c.requested.h != s.requested.h);

                             (c.requested.h != s.requested.h);
@@ -1454,8 +1454,7 @@ uint32_t Layer::doTransaction(uint32_t flags) { 
    // this is used by Layer, which special cases resizes.

    // this is used by Layer, which special cases resizes.

    if (flags & eDontUpdateGeometryState)  {

    if (flags & eDontUpdateGeometryState)  {

    } else {

    } else {

        Layer::State& editCurrentState(getCurrentState());

        c.active = c.requested;

        editCurrentState.active = c.requested;

    }

    }





    if (s.active != c.active) {

    if (s.active != c.active) {
@@ -1475,12 +1474,12 @@ uint32_t Layer::doTransaction(uint32_t flags) { 
    }

    }





    // Commit the transaction

    // Commit the transaction

    commitTransaction();

    commitTransaction(c);

    return flags;

    return flags;

}

}





void Layer::commitTransaction() {

void Layer::commitTransaction(const State& stateToCommit) {

    mDrawingState = mCurrentState;

    mDrawingState = stateToCommit;

}

}





uint32_t Layer::getTransactionFlags(uint32_t flags) {

uint32_t Layer::getTransactionFlags(uint32_t flags) {

services/surfaceflinger/Layer.h

+3 −3
Original line number Original line Diff line number Diff line
@@ -431,7 +431,7 @@ private: 
    virtual void onFrameReplaced(const BufferItem& item) override;

    virtual void onFrameReplaced(const BufferItem& item) override;

    virtual void onSidebandStreamChanged() override;

    virtual void onSidebandStreamChanged() override;





    void commitTransaction();

    void commitTransaction(const State& stateToCommit);





    // needsLinearFiltering - true if this surface's state requires filtering

    // needsLinearFiltering - true if this surface's state requires filtering

    bool needsFiltering(const sp<const DisplayDevice>& hw) const;

    bool needsFiltering(const sp<const DisplayDevice>& hw) const;
@@ -500,8 +500,8 @@ private: 
    bool addSyncPoint(const std::shared_ptr<SyncPoint>& point);

    bool addSyncPoint(const std::shared_ptr<SyncPoint>& point);





    void pushPendingState();

    void pushPendingState();

    void popPendingState();

    void popPendingState(State* stateToCommit);

    bool applyPendingStates();

    bool applyPendingStates(State* stateToCommit);

public:

public:

    void notifyAvailableFrames();

    void notifyAvailableFrames();

private:

private:

services/surfaceflinger/tests/Transaction_test.cpp

+57 −0
Original line number Original line Diff line number Diff line
@@ -459,4 +459,61 @@ TEST_F(LayerUpdateTest, LayerSetMatrixWorks) { 
    }

    }

}

}





TEST_F(LayerUpdateTest, DeferredTransactionTest) {

    sp<ScreenCapture> sc;

    {

        SCOPED_TRACE("before anything");

        ScreenCapture::captureScreen(&sc);

        sc->checkPixel( 32,  32,  63,  63, 195);

        sc->checkPixel( 96,  96, 195,  63,  63);

        sc->checkPixel(160, 160,  63,  63, 195);

    }



    // set up two deferred transactions on different frames

    SurfaceComposerClient::openGlobalTransaction();

    ASSERT_EQ(NO_ERROR, mFGSurfaceControl->setAlpha(0.75));

    mFGSurfaceControl->deferTransactionUntil(mSyncSurfaceControl->getHandle(),

            mSyncSurfaceControl->getSurface()->getNextFrameNumber());

    SurfaceComposerClient::closeGlobalTransaction(true);



    SurfaceComposerClient::openGlobalTransaction();

    ASSERT_EQ(NO_ERROR, mFGSurfaceControl->setPosition(128,128));

    mFGSurfaceControl->deferTransactionUntil(mSyncSurfaceControl->getHandle(),

            mSyncSurfaceControl->getSurface()->getNextFrameNumber() + 1);

    SurfaceComposerClient::closeGlobalTransaction(true);



    {

        SCOPED_TRACE("before any trigger");

        ScreenCapture::captureScreen(&sc);

        sc->checkPixel( 32,  32,  63,  63, 195);

        sc->checkPixel( 96,  96, 195,  63,  63);

        sc->checkPixel(160, 160,  63,  63, 195);

    }



    // should trigger the first deferred transaction, but not the second one

    fillSurfaceRGBA8(mSyncSurfaceControl, 31, 31, 31);

    {

        SCOPED_TRACE("after first trigger");

        ScreenCapture::captureScreen(&sc);

        sc->checkPixel( 32,  32,  63,  63, 195);

        sc->checkPixel( 96,  96, 162,  63,  96);

        sc->checkPixel(160, 160,  63,  63, 195);

    }



    // should show up immediately since it's not deferred

    SurfaceComposerClient::openGlobalTransaction();

    ASSERT_EQ(NO_ERROR, mFGSurfaceControl->setAlpha(1.0));

    SurfaceComposerClient::closeGlobalTransaction(true);



    // trigger the second deferred transaction

    fillSurfaceRGBA8(mSyncSurfaceControl, 31, 31, 31);

    {

        SCOPED_TRACE("after second trigger");

        ScreenCapture::captureScreen(&sc);

        sc->checkPixel( 32,  32,  63,  63, 195);

        sc->checkPixel( 96,  96,  63,  63, 195);

        sc->checkPixel(160, 160, 195,  63,  63);

    }

}



}
 
}