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

Commit d3f6065b authored by Derek Sollenberger's avatar Derek Sollenberger
Browse files

Defer deleting ExternalTextures that go out of scope during DrawLayers 

Some buffers (e.g. protected content) are not pre-mapped into a
texture.  Their textures are created in drawLayers and go out of scope
at the end of drawLayers drawLayers when those temporary textures are
sent to the GPU.

This CL ensures that in those cases we defer unbinding and deleting the
temporary texture until the cleanupPostRender method is called. Also to
avoid unecessary thread hops into cleanupPostRender this CL also adds a
thread-safe check to see if cleanup is necessary. Finally, we also make
the threaded variant asynchronous to further improve the availability of
SurfaceFlinger.

Bug: 190628682
Bug: 191132989
Test: atest librenderengine_test and perfetto traces
Change-Id: Ic2f4384cd1957c928a0ef656a98eb0041e29622c
parent 2c34aa61

libs/renderengine/gl/GLESRenderEngine.cpp

+8 −29
Original line number Diff line number Diff line
@@ -970,37 +970,17 @@ void GLESRenderEngine::unbindFrameBuffer(Framebuffer* /*framebuffer*/) { 
    glBindFramebuffer(GL_FRAMEBUFFER, 0);

}



bool GLESRenderEngine::cleanupPostRender(CleanupMode mode) {

bool GLESRenderEngine::canSkipPostRenderCleanup() const {

    return mPriorResourcesCleaned ||

            (mLastDrawFence != nullptr && mLastDrawFence->getStatus() != Fence::Status::Signaled);

}



void GLESRenderEngine::cleanupPostRender() {

    ATRACE_CALL();



    if (mPriorResourcesCleaned ||

        (mLastDrawFence != nullptr && mLastDrawFence->getStatus() != Fence::Status::Signaled)) {

    if (canSkipPostRenderCleanup()) {

        // If we don't have a prior frame needing cleanup, then don't do anything.

        return false;

    }



    // This is a bit of a band-aid fix for FrameCaptureProcessor, as we should

    // not need to keep memory around if we don't need to do so.

    if (mode == CleanupMode::CLEAN_ALL) {

        // TODO: SurfaceFlinger memory utilization may benefit from resetting

        // texture bindings as well. Assess if it does and there's no performance regression

        // when rebinding the same image data to the same texture, and if so then its mode

        // behavior can be tweaked.

        if (mPlaceholderImage != EGL_NO_IMAGE_KHR) {

            for (auto [textureName, bufferId] : mTextureView) {

                if (bufferId && mPlaceholderImage != EGL_NO_IMAGE_KHR) {

                    glBindTexture(GL_TEXTURE_EXTERNAL_OES, textureName);

                    glEGLImageTargetTexture2DOES(GL_TEXTURE_EXTERNAL_OES,

                                                 static_cast<GLeglImageOES>(mPlaceholderImage));

                    mTextureView[textureName] = std::nullopt;

                    checkErrors();

                }

            }

        }

        {

            std::lock_guard<std::mutex> lock(mRenderingMutex);

            mImageCache.clear();

        }

        return;

    }



    // Bind the texture to placeholder so that backing image data can be freed.
@@ -1011,7 +991,6 @@ bool GLESRenderEngine::cleanupPostRender(CleanupMode mode) { 
    // we could no-op repeated calls of this method instead.

    mLastDrawFence = nullptr;

    mPriorResourcesCleaned = true;

    return true;

}



void GLESRenderEngine::cleanFramebufferCache() {

libs/renderengine/gl/GLESRenderEngine.h

+2 −1
Original line number Diff line number Diff line
@@ -68,7 +68,7 @@ public: 
                        const std::shared_ptr<ExternalTexture>& buffer,

                        const bool useFramebufferCache, base::unique_fd&& bufferFence,

                        base::unique_fd* drawFence) override;

    bool cleanupPostRender(CleanupMode mode) override;

    void cleanupPostRender() override;

    int getContextPriority() override;

    bool supportsBackgroundBlur() override { return mBlurFilter != nullptr; }

    void onPrimaryDisplaySizeChanged(ui::Size size) override {}
@@ -106,6 +106,7 @@ protected: 
    void mapExternalTextureBuffer(const sp<GraphicBuffer>& buffer, bool isRenderable)

            EXCLUDES(mRenderingMutex);

    void unmapExternalTextureBuffer(const sp<GraphicBuffer>& buffer) EXCLUDES(mRenderingMutex);

    bool canSkipPostRenderCleanup() const override;



private:

    friend class BindNativeBufferAsFramebuffer;

libs/renderengine/include/renderengine/RenderEngine.h

+14 −19
Original line number Diff line number Diff line
@@ -114,25 +114,6 @@ public: 
    virtual void genTextures(size_t count, uint32_t* names) = 0;

    virtual void deleteTextures(size_t count, uint32_t const* names) = 0;



    enum class CleanupMode {

        CLEAN_OUTPUT_RESOURCES,

        CLEAN_ALL,

    };

    // Clean-up method that should be called on the main thread after the

    // drawFence returned by drawLayers fires. This method will free up

    // resources used by the most recently drawn frame. If the frame is still

    // being drawn, then this call is silently ignored.

    //

    // If mode is CLEAN_OUTPUT_RESOURCES, then only resources related to the

    // output framebuffer are cleaned up, including the sibling texture.

    //

    // If mode is CLEAN_ALL, then we also cleanup resources related to any input

    // buffers.

    //

    // Returns true if resources were cleaned up, and false if we didn't need to

    // do any work.

    virtual bool cleanupPostRender(CleanupMode mode = CleanupMode::CLEAN_OUTPUT_RESOURCES) = 0;



    // queries that are required to be thread safe

    virtual size_t getMaxTextureSize() const = 0;

    virtual size_t getMaxViewportDims() const = 0;
@@ -186,6 +167,13 @@ public: 
                                const std::shared_ptr<ExternalTexture>& buffer,

                                const bool useFramebufferCache, base::unique_fd&& bufferFence,

                                base::unique_fd* drawFence) = 0;



    // Clean-up method that should be called on the main thread after the

    // drawFence returned by drawLayers fires. This method will free up

    // resources used by the most recently drawn frame. If the frame is still

    // being drawn, then the implementation is free to silently ignore this call.

    virtual void cleanupPostRender() = 0;



    virtual void cleanFramebufferCache() = 0;

    // Returns the priority this context was actually created with. Note: this may not be

    // the same as specified at context creation time, due to implementation limits on the
@@ -234,8 +222,15 @@ protected: 
    // that's conflict serializable, i.e. unmap a buffer should never occur before binding the

    // buffer if the caller called mapExternalTextureBuffer before calling unmap.

    virtual void unmapExternalTextureBuffer(const sp<GraphicBuffer>& buffer) = 0;



    // A thread safe query to determine if any post rendering cleanup is necessary.  Returning true

    // is a signal that calling the postRenderCleanup method would be a no-op and that callers can

    // avoid any thread synchronization that may be required by directly calling postRenderCleanup.

    virtual bool canSkipPostRenderCleanup() const = 0;



    friend class ExternalTexture;

    friend class threaded::RenderEngineThreaded;

    friend class RenderEngineTest_cleanupPostRender_cleansUpOnce_Test;

    const RenderEngineType mRenderEngineType;

};

libs/renderengine/include/renderengine/mock/RenderEngine.h

+2 −1
Original line number Diff line number Diff line
@@ -45,7 +45,8 @@ public: 
    MOCK_CONST_METHOD0(isProtected, bool());

    MOCK_CONST_METHOD0(supportsProtectedContent, bool());

    MOCK_METHOD1(useProtectedContext, bool(bool));

    MOCK_METHOD1(cleanupPostRender, bool(CleanupMode mode));

    MOCK_METHOD0(cleanupPostRender, void());

    MOCK_CONST_METHOD0(canSkipPostRenderCleanup, bool());

    MOCK_METHOD6(drawLayers,

                 status_t(const DisplaySettings&, const std::vector<const LayerSettings*>&,

                          const std::shared_ptr<ExternalTexture>&, const bool, base::unique_fd&&,

libs/renderengine/skia/AutoBackendTexture.cpp

+10 −7
Original line number Diff line number Diff line
@@ -29,8 +29,8 @@ namespace renderengine { 
namespace skia {



AutoBackendTexture::AutoBackendTexture(GrDirectContext* context, AHardwareBuffer* buffer,

                                       bool isOutputBuffer)

      : mIsOutputBuffer(isOutputBuffer) {

                                       bool isOutputBuffer, CleanupManager& cleanupMgr)

      : mCleanupMgr(cleanupMgr), mIsOutputBuffer(isOutputBuffer) {

    ATRACE_CALL();

    AHardwareBuffer_Desc desc;

    AHardwareBuffer_describe(buffer, &desc);
@@ -49,6 +49,13 @@ AutoBackendTexture::AutoBackendTexture(GrDirectContext* context, AHardwareBuffer 
             this, desc.width, desc.height, createProtectedImage, isOutputBuffer, desc.format);

}



AutoBackendTexture::~AutoBackendTexture() {

    if (mBackendTexture.isValid()) {

        mDeleteProc(mImageCtx);

        mBackendTexture = {};

    }

}



void AutoBackendTexture::unref(bool releaseLocalResources) {

    if (releaseLocalResources) {

        mSurface = nullptr;
@@ -57,11 +64,7 @@ void AutoBackendTexture::unref(bool releaseLocalResources) { 


    mUsageCount--;

    if (mUsageCount <= 0) {

        if (mBackendTexture.isValid()) {

            mDeleteProc(mImageCtx);

            mBackendTexture = {};

        }

        delete this;

        mCleanupMgr.add(this);

    }

}