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

Commit b158a5c5 authored by Brian Lindahl's avatar Brian Lindahl
Browse files

Maintain the active buffer when clearing buffer slots 

When an app discards graphic buffers, for example when a MediaCodec
disconnects from a surface, those buffers will be uncached and removed
from HWC buffer slots. The active buffer, however, should still remain
active until the next buffer is queued up.

Bug: 262037933
Bug: 258196272
Test: atest OutputLayerUncacheBufferTest
Test: atest VtsHalGraphicsComposer3_TargetTest
Test: atest VtsHalGraphicsComposerV2_2TargetTest
Change-Id: I7c4eefb17e8bad694d698f9ad6d1d289f4af8d2c
parent 90553da7

services/surfaceflinger/CompositionEngine/include/compositionengine/impl/HwcBufferCache.h

+1 −1
Original line number Diff line number Diff line
@@ -83,7 +83,7 @@ public: 
    // If the buffer is already in the cache, the buffer is null to optimize away sending HWC the

    // buffer handle.

    //

    HwcSlotAndBuffer getHwcSlotAndBufferForOverride(const sp<GraphicBuffer>& buffer);

    HwcSlotAndBuffer getOverrideHwcSlotAndBuffer(const sp<GraphicBuffer>& buffer);



    //

    // When a client process discards a buffer, it needs to be purged from the HWC cache.

services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h

+3 −0
Original line number Diff line number Diff line
@@ -136,6 +136,9 @@ struct OutputLayerCompositionState { 
        // cost of sending reused buffers to the HWC.

        HwcBufferCache hwcBufferCache;



        // The previously-active buffer for this layer.

        uint32_t activeBufferSlot;



        // Set to true when overridden info has been sent to HW composer

        bool stateOverridden = false;

services/surfaceflinger/CompositionEngine/src/HwcBufferCache.cpp

+1 −1
Original line number Diff line number Diff line
@@ -42,7 +42,7 @@ HwcSlotAndBuffer HwcBufferCache::getHwcSlotAndBuffer(const sp<GraphicBuffer>& bu 
    return {cache(buffer), buffer};

}



HwcSlotAndBuffer HwcBufferCache::getHwcSlotAndBufferForOverride(const sp<GraphicBuffer>& buffer) {

HwcSlotAndBuffer HwcBufferCache::getOverrideHwcSlotAndBuffer(const sp<GraphicBuffer>& buffer) {

    if (buffer == mLastOverrideBuffer) {

        return {kOverrideBufferSlot, nullptr};

    }

services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp

+31 −15
Original line number Diff line number Diff line
@@ -610,20 +610,26 @@ void OutputLayer::writeSidebandStateToHWC(HWC2::Layer* hwcLayer, 
    }

}



void OutputLayer::uncacheBuffers(std::vector<uint64_t> const& bufferIdsToUncache) {

void OutputLayer::uncacheBuffers(const std::vector<uint64_t>& bufferIdsToUncache) {

    auto& state = editState();

    // Skip doing this if there is no HWC interface

    if (!state.hwc) {

        return;

    }



    for (auto bufferId : bufferIdsToUncache) {

    std::vector<uint32_t> slotsToClear;

    for (uint64_t bufferId : bufferIdsToUncache) {

        uint32_t slot = state.hwc->hwcBufferCache.uncache(bufferId);

        if (slot != UINT32_MAX && state.hwc->hwcLayer) {

            hal::Error error = state.hwc->hwcLayer->clearBufferSlot(slot);

            ALOGE("[%s] Failed to clear buffer slot %d: %s (%d)", getLayerFE().getDebugName(), slot,

                  to_string(error).c_str(), static_cast<int32_t>(error));

        if (slot != UINT32_MAX) {

            slotsToClear.push_back(slot);

        }

    }



    hal::Error error =

            state.hwc->hwcLayer->setBufferSlotsToClear(slotsToClear, state.hwc->activeBufferSlot);

    if (error != hal::Error::NONE) {

        ALOGE("[%s] Failed to clear buffer slots: %s (%d)", getLayerFE().getDebugName(),

              to_string(error).c_str(), static_cast<int32_t>(error));

    }

}
@@ -641,17 +647,27 @@ void OutputLayer::writeBufferStateToHWC(HWC2::Layer* hwcLayer, 


    HwcSlotAndBuffer hwcSlotAndBuffer;

    sp<Fence> hwcFence;

    {

        // Editing the state only because we update the HWC buffer cache and active buffer.

        auto& state = editState();

        // Override buffers use a special cache slot so that they don't evict client buffers.

    if (getState().overrideInfo.buffer != nullptr && !skipLayer) {

        hwcSlotAndBuffer = editState().hwc->hwcBufferCache.getHwcSlotAndBufferForOverride(

                getState().overrideInfo.buffer->getBuffer());

        hwcFence = getState().overrideInfo.acquireFence;

        if (state.overrideInfo.buffer != nullptr && !skipLayer) {

            hwcSlotAndBuffer = state.hwc->hwcBufferCache.getOverrideHwcSlotAndBuffer(

                    state.overrideInfo.buffer->getBuffer());

            hwcFence = state.overrideInfo.acquireFence;

        } else {

            hwcSlotAndBuffer =

                editState().hwc->hwcBufferCache.getHwcSlotAndBuffer(outputIndependentState.buffer);

                    state.hwc->hwcBufferCache.getHwcSlotAndBuffer(outputIndependentState.buffer);

            hwcFence = outputIndependentState.acquireFence;

        }



        // Keep track of the active buffer slot, so we can restore it after clearing other buffer

        // slots.

        if (hwcSlotAndBuffer.buffer) {

            state.hwc->activeBufferSlot = hwcSlotAndBuffer.slot;

        }

    }



    if (auto error = hwcLayer->setBuffer(hwcSlotAndBuffer.slot, hwcSlotAndBuffer.buffer, hwcFence);

        error != hal::Error::NONE) {

        ALOGE("[%s] Failed to set buffer %p: %s (%d)", getLayerFE().getDebugName(),

services/surfaceflinger/CompositionEngine/tests/HwcBufferCacheTest.cpp

+7 −7
Original line number Diff line number Diff line
@@ -115,20 +115,20 @@ TEST_F(HwcBufferCacheTest, uncache_whenUncached_returnsInvalidSlotNumber) { 
    EXPECT_EQ(cache.uncache(mBuffer2->getId()), UINT32_MAX);

}



TEST_F(HwcBufferCacheTest, getHwcSlotAndBufferForOverride_whenCached_returnsSameSlotAndNullBuffer) {

TEST_F(HwcBufferCacheTest, getOverrideHwcSlotAndBuffer_whenCached_returnsSameSlotAndNullBuffer) {

    HwcBufferCache cache;

    sp<GraphicBuffer> outBuffer;



    HwcSlotAndBuffer originalSlotAndBuffer = cache.getHwcSlotAndBufferForOverride(mBuffer1);

    HwcSlotAndBuffer originalSlotAndBuffer = cache.getOverrideHwcSlotAndBuffer(mBuffer1);

    EXPECT_NE(originalSlotAndBuffer.slot, UINT32_MAX);

    EXPECT_EQ(originalSlotAndBuffer.buffer, mBuffer1);



    HwcSlotAndBuffer finalSlotAndBuffer = cache.getHwcSlotAndBufferForOverride(mBuffer1);

    HwcSlotAndBuffer finalSlotAndBuffer = cache.getOverrideHwcSlotAndBuffer(mBuffer1);

    EXPECT_EQ(finalSlotAndBuffer.slot, originalSlotAndBuffer.slot);

    EXPECT_EQ(finalSlotAndBuffer.buffer, nullptr);

}



TEST_F(HwcBufferCacheTest, getHwcSlotAndBufferForOverride_whenSlotsFull_returnsIndependentSlot) {

TEST_F(HwcBufferCacheTest, getOverrideHwcSlotAndBuffer_whenSlotsFull_returnsIndependentSlot) {

    HwcBufferCache cache;

    sp<GraphicBuffer> outBuffer;
@@ -149,7 +149,7 @@ TEST_F(HwcBufferCacheTest, getHwcSlotAndBufferForOverride_whenSlotsFull_returnsI 


    sp<GraphicBuffer> overrideBuffer =

            sp<GraphicBuffer>::make(1u, 1u, HAL_PIXEL_FORMAT_RGBA_8888, 1u, 0u);

    HwcSlotAndBuffer overrideSlotAndBuffer = cache.getHwcSlotAndBufferForOverride(overrideBuffer);

    HwcSlotAndBuffer overrideSlotAndBuffer = cache.getOverrideHwcSlotAndBuffer(overrideBuffer);

    // expect us to have a slot number

    EXPECT_NE(overrideSlotAndBuffer.slot, UINT32_MAX);

    // expect this to be the first time we cached the buffer
@@ -173,7 +173,7 @@ TEST_F(HwcBufferCacheTest, uncache_whenOverrideCached_returnsSlotNumber) { 
    HwcBufferCache cache;

    sp<GraphicBuffer> outBuffer;



    HwcSlotAndBuffer hwcSlotAndBuffer = cache.getHwcSlotAndBufferForOverride(mBuffer1);

    HwcSlotAndBuffer hwcSlotAndBuffer = cache.getOverrideHwcSlotAndBuffer(mBuffer1);

    ASSERT_NE(hwcSlotAndBuffer.slot, UINT32_MAX);



    EXPECT_EQ(cache.uncache(mBuffer1->getId()), hwcSlotAndBuffer.slot);
@@ -184,7 +184,7 @@ TEST_F(HwcBufferCacheTest, uncache_whenOverrideUncached_returnsInvalidSlotNumber 
    HwcBufferCache cache;

    sp<GraphicBuffer> outBuffer;



    HwcSlotAndBuffer hwcSlotAndBuffer = cache.getHwcSlotAndBufferForOverride(mBuffer1);

    HwcSlotAndBuffer hwcSlotAndBuffer = cache.getOverrideHwcSlotAndBuffer(mBuffer1);

    ASSERT_NE(hwcSlotAndBuffer.slot, UINT32_MAX);



    EXPECT_EQ(cache.uncache(mBuffer2->getId()), UINT32_MAX);