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

Commit 732ff442 authored by Alec Mouri's avatar Alec Mouri Committed by Android (Google) Code Review
Browse files

Merge "Don't leak flattened buffers from cached sets." into sc-dev

parents 99ebe3f9 49da70dc
Loading
Loading
Loading
Loading
+29 −7
Original line number Original line Diff line number Diff line
@@ -17,15 +17,12 @@
#pragma once
#pragma once


#include <compositionengine/impl/planner/LayerState.h>
#include <compositionengine/impl/planner/LayerState.h>
#include <renderengine/RenderEngine.h>


#include <chrono>
#include <chrono>


namespace android {
namespace android {


namespace renderengine {
class RenderEngine;
} // namespace renderengine

namespace compositionengine::impl::planner {
namespace compositionengine::impl::planner {


std::string durationString(std::chrono::milliseconds duration);
std::string durationString(std::chrono::milliseconds duration);
@@ -63,7 +60,7 @@ public:
    const Layer& getFirstLayer() const { return mLayers[0]; }
    const Layer& getFirstLayer() const { return mLayers[0]; }
    const Rect& getBounds() const { return mBounds; }
    const Rect& getBounds() const { return mBounds; }
    size_t getAge() const { return mAge; }
    size_t getAge() const { return mAge; }
    const sp<GraphicBuffer>& getBuffer() const { return mBuffer; }
    const sp<GraphicBuffer>& getBuffer() const { return mTexture.getBuffer(); }
    const sp<Fence>& getDrawFence() const { return mDrawFence; }
    const sp<Fence>& getDrawFence() const { return mDrawFence; }


    NonBufferHash getNonBufferHash() const;
    NonBufferHash getNonBufferHash() const;
@@ -82,7 +79,7 @@ public:


    void setLastUpdate(std::chrono::steady_clock::time_point now) { mLastUpdate = now; }
    void setLastUpdate(std::chrono::steady_clock::time_point now) { mLastUpdate = now; }
    void append(const CachedSet& other) {
    void append(const CachedSet& other) {
        mBuffer = nullptr;
        mTexture.setBuffer(nullptr, nullptr);
        mDrawFence = nullptr;
        mDrawFence = nullptr;


        mLayers.insert(mLayers.end(), other.mLayers.cbegin(), other.mLayers.cend());
        mLayers.insert(mLayers.end(), other.mLayers.cbegin(), other.mLayers.cend());
@@ -105,7 +102,32 @@ private:
    std::vector<Layer> mLayers;
    std::vector<Layer> mLayers;
    Rect mBounds = Rect::EMPTY_RECT;
    Rect mBounds = Rect::EMPTY_RECT;
    size_t mAge = 0;
    size_t mAge = 0;
    sp<GraphicBuffer> mBuffer;

    class Texture {
    public:
        ~Texture() { setBuffer(nullptr, nullptr); }

        void setBuffer(const sp<GraphicBuffer>& buffer, renderengine::RenderEngine* re) {
            if (mRE && mBuffer) {
                mRE->unbindExternalTextureBuffer(mBuffer->getId());
            }

            mBuffer = buffer;
            mRE = re;

            if (mRE && mBuffer) {
                mRE->cacheExternalTextureBuffer(mBuffer);
            }
        }

        const sp<GraphicBuffer>& getBuffer() const { return mBuffer; }

    private:
        sp<GraphicBuffer> mBuffer = nullptr;
        renderengine::RenderEngine* mRE = nullptr;
    };

    Texture mTexture;
    sp<Fence> mDrawFence;
    sp<Fence> mDrawFence;


    static const bool sDebugHighlighLayers;
    static const bool sDebugHighlighLayers;
+3 −2
Original line number Original line Diff line number Diff line
@@ -126,7 +126,7 @@ bool CachedSet::hasBufferUpdate(std::vector<const LayerState*>::const_iterator l
}
}


bool CachedSet::hasReadyBuffer() const {
bool CachedSet::hasReadyBuffer() const {
    return mBuffer != nullptr && mDrawFence->getStatus() == Fence::Status::Signaled;
    return mTexture.getBuffer() != nullptr && mDrawFence->getStatus() == Fence::Status::Signaled;
}
}


std::vector<CachedSet> CachedSet::decompose() const {
std::vector<CachedSet> CachedSet::decompose() const {
@@ -209,11 +209,12 @@ void CachedSet::render(renderengine::RenderEngine& renderEngine) {
                                                 HAL_PIXEL_FORMAT_RGBA_8888, 1, usageFlags);
                                                 HAL_PIXEL_FORMAT_RGBA_8888, 1, usageFlags);
    LOG_ALWAYS_FATAL_IF(buffer->initCheck() != OK);
    LOG_ALWAYS_FATAL_IF(buffer->initCheck() != OK);
    base::unique_fd drawFence;
    base::unique_fd drawFence;

    status_t result = renderEngine.drawLayers(displaySettings, layerSettingsPointers, buffer, false,
    status_t result = renderEngine.drawLayers(displaySettings, layerSettingsPointers, buffer, false,
                                              base::unique_fd(), &drawFence);
                                              base::unique_fd(), &drawFence);


    if (result == NO_ERROR) {
    if (result == NO_ERROR) {
        mBuffer = buffer;
        mTexture.setBuffer(buffer, &renderEngine);
        mDrawFence = new Fence(drawFence.release());
        mDrawFence = new Fence(drawFence.release());
    }
    }
}
}
+6 −0
Original line number Original line Diff line number Diff line
@@ -310,8 +310,14 @@ TEST_F(CachedSetTest, render) {
    EXPECT_CALL(*layerFE1, prepareClientCompositionList(_)).WillOnce(Return(clientCompList1));
    EXPECT_CALL(*layerFE1, prepareClientCompositionList(_)).WillOnce(Return(clientCompList1));
    EXPECT_CALL(*layerFE2, prepareClientCompositionList(_)).WillOnce(Return(clientCompList2));
    EXPECT_CALL(*layerFE2, prepareClientCompositionList(_)).WillOnce(Return(clientCompList2));
    EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _, _)).WillOnce(Invoke(drawLayers));
    EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _, _)).WillOnce(Invoke(drawLayers));
    EXPECT_CALL(mRenderEngine, cacheExternalTextureBuffer(_));
    cachedSet.render(mRenderEngine);
    cachedSet.render(mRenderEngine);
    expectReadyBuffer(cachedSet);
    expectReadyBuffer(cachedSet);

    // Now check that appending a new cached set properly cleans up RenderEngine resources.
    EXPECT_CALL(mRenderEngine, unbindExternalTextureBuffer(_));
    CachedSet::Layer& layer3 = *mTestLayers[2]->cachedSetLayer.get();
    cachedSet.append(CachedSet(layer3));
}
}


} // namespace
} // namespace
+3 −1
Original line number Original line Diff line number Diff line
@@ -55,8 +55,10 @@ protected:
    // TODO(b/181192467): Once Flattener starts to do something useful with Predictor,
    // TODO(b/181192467): Once Flattener starts to do something useful with Predictor,
    // mPredictor should be mocked and checked for expectations.
    // mPredictor should be mocked and checked for expectations.
    Predictor mPredictor;
    Predictor mPredictor;
    std::unique_ptr<Flattener> mFlattener;

    // mRenderEngine may be held as a pointer to mFlattener, so mFlattener must be destroyed first.
    renderengine::mock::RenderEngine mRenderEngine;
    renderengine::mock::RenderEngine mRenderEngine;
    std::unique_ptr<Flattener> mFlattener;


    const std::chrono::steady_clock::time_point kStartTime = std::chrono::steady_clock::now();
    const std::chrono::steady_clock::time_point kStartTime = std::chrono::steady_clock::now();
    std::chrono::steady_clock::time_point mTime = kStartTime;
    std::chrono::steady_clock::time_point mTime = kStartTime;