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

Commit 7ee4f464 authored by Vishnu Nair's avatar Vishnu Nair
Browse files

[sf] Release the currently presented buffer when setBuffer is called 

with null

Fixes a regression introduced in T which ignores a setBuffer call
with a null buffer. The expected behavior should be to release the
currently presented buffer from surfaceflinger. The subsequent frame
will not present this layer so the region behind the layer will be
composited instead.

Bug: 241271897
Test: presubmit
Change-Id: Ie06025c59c58cc75a267b783729996a3cbceef45
parent 9402f4fa

libs/gui/SurfaceComposerClient.cpp

+34 −18
Original line number Diff line number Diff line
@@ -898,7 +898,7 @@ status_t SurfaceComposerClient::Transaction::writeToParcel(Parcel* parcel) const 
}



void SurfaceComposerClient::Transaction::releaseBufferIfOverwriting(const layer_state_t& state) {

    if (!(state.what & layer_state_t::eBufferChanged)) {

    if (!(state.what & layer_state_t::eBufferChanged) || !state.bufferData->hasBuffer()) {

        return;

    }
@@ -1642,14 +1642,9 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffe 


    releaseBufferIfOverwriting(*s);



    if (buffer == nullptr) {

        s->what &= ~layer_state_t::eBufferChanged;

        s->bufferData = nullptr;

        return *this;

    }



    std::shared_ptr<BufferData> bufferData = std::make_shared<BufferData>();

    bufferData->buffer = buffer;

    if (buffer) {

        uint64_t frameNumber = sc->resolveFrameNumber(optFrameNumber);

        bufferData->frameNumber = frameNumber;

        bufferData->producerId = producerId;
@@ -1660,10 +1655,12 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffe 
        }

        bufferData->releaseBufferEndpoint =

                IInterface::asBinder(TransactionCompletedListener::getIInstance());

        setReleaseBufferCallback(bufferData.get(), callback);

    }



    if (mIsAutoTimestamp) {

        mDesiredPresentTime = systemTime();

    }

    setReleaseBufferCallback(bufferData.get(), callback);

    s->what |= layer_state_t::eBufferChanged;

    s->bufferData = std::move(bufferData);

    registerSurfaceControlForCallback(sc);
@@ -1684,6 +1681,25 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffe 
    return *this;

}



SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::unsetBuffer(

        const sp<SurfaceControl>& sc) {

    layer_state_t* s = getLayerState(sc);

    if (!s) {

        mStatus = BAD_INDEX;

        return *this;

    }



    if (!(s->what & layer_state_t::eBufferChanged)) {

        return *this;

    }



    releaseBufferIfOverwriting(*s);



    s->what &= ~layer_state_t::eBufferChanged;

    s->bufferData = nullptr;

    return *this;

}



void SurfaceComposerClient::Transaction::setReleaseBufferCallback(BufferData* bufferData,

                                                                  ReleaseBufferCallback callback) {

    if (!callback) {

libs/gui/include/gui/SurfaceComposerClient.h

+1 −0
Original line number Diff line number Diff line
@@ -541,6 +541,7 @@ public: 
                               const std::optional<sp<Fence>>& fence = std::nullopt,

                               const std::optional<uint64_t>& frameNumber = std::nullopt,

                               uint32_t producerId = 0, ReleaseBufferCallback callback = nullptr);

        Transaction& unsetBuffer(const sp<SurfaceControl>& sc);

        std::shared_ptr<BufferData> getAndClearBuffer(const sp<SurfaceControl>& sc);



        /**

libs/gui/include/gui/test/CallbackUtils.h

+21 −9
Original line number Diff line number Diff line
@@ -51,6 +51,7 @@ public: 
    enum Buffer {

        NOT_ACQUIRED = 0,

        ACQUIRED,

        ACQUIRED_NULL,

    };



    enum PreviousBuffer {
@@ -133,17 +134,28 @@ private: 
              : mBufferResult(bufferResult), mPreviousBufferResult(previousBufferResult) {}



        void verifySurfaceControlStats(const SurfaceControlStats& surfaceControlStats,

                                       nsecs_t latchTime) const {

                                       nsecs_t /* latchTime */) const {

            const auto& [surfaceControl, latch, acquireTimeOrFence, presentFence,

                         previousReleaseFence, transformHint, frameEvents, ignore] =

                    surfaceControlStats;



            ASSERT_TRUE(std::holds_alternative<nsecs_t>(acquireTimeOrFence));

            ASSERT_EQ(std::get<nsecs_t>(acquireTimeOrFence) > 0,

                      mBufferResult == ExpectedResult::Buffer::ACQUIRED)

                    << "bad acquire time";

            ASSERT_LE(std::get<nsecs_t>(acquireTimeOrFence), latchTime)

                    << "acquire time should be <= latch time";

            nsecs_t acquireTime = -1;

            if (std::holds_alternative<nsecs_t>(acquireTimeOrFence)) {

                acquireTime = std::get<nsecs_t>(acquireTimeOrFence);

            } else {

                auto fence = std::get<sp<Fence>>(acquireTimeOrFence);

                if (fence) {

                    ASSERT_EQ(fence->wait(3000), NO_ERROR);

                    acquireTime = fence->getSignalTime();

                }

            }



            if (mBufferResult == ExpectedResult::Buffer::ACQUIRED) {

                ASSERT_GT(acquireTime, 0) << "acquire time should be valid";

            } else {

                ASSERT_LE(acquireTime, 0) << "acquire time should not be valid";

            }

            ASSERT_EQ(acquireTime > 0, mBufferResult == ExpectedResult::Buffer::ACQUIRED);



            if (mPreviousBufferResult == ExpectedResult::PreviousBuffer::RELEASED) {

                ASSERT_NE(previousReleaseFence, nullptr)

services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h

+2 −1
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ 
#include <optional>

#include <ostream>

#include <unordered_set>

#include "ui/LayerStack.h"



// TODO(b/129481165): remove the #pragma below and fix conversion issues

#pragma clang diagnostic push
@@ -140,7 +141,7 @@ public: 
            ClientCompositionTargetSettings&) const = 0;



    // Called after the layer is displayed to update the presentation fence

    virtual void onLayerDisplayed(ftl::SharedFuture<FenceResult>) = 0;

    virtual void onLayerDisplayed(ftl::SharedFuture<FenceResult>, ui::LayerStack layerStack) = 0;



    // Gets some kind of identifier for the layer for debug purposes.

    virtual const char* getDebugName() const = 0;

services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h

+2 −1
Original line number Diff line number Diff line
@@ -49,7 +49,8 @@ public: 
                       std::optional<compositionengine::LayerFE::LayerSettings>(

                               compositionengine::LayerFE::ClientCompositionTargetSettings&));



    MOCK_METHOD(void, onLayerDisplayed, (ftl::SharedFuture<FenceResult>), (override));

    MOCK_METHOD(void, onLayerDisplayed, (ftl::SharedFuture<FenceResult>, ui::LayerStack),

                (override));



    MOCK_CONST_METHOD0(getDebugName, const char*());

    MOCK_CONST_METHOD0(getSequence, int32_t());