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

Commit 3d7c5617 authored by Alec Mouri's avatar Alec Mouri
Browse files

[RenderEngine] relax locking conditions of image cache. 

RenderEngine::drawLayers is currently only called on the main thread,
and layers should not be destroyed while the main thread is running, so
we don't need to lock during the entirety of rendering.

Note that with this change there is still some jank because GPU
rendering does take a long time, but it's not nearly as bad as before.

Bug: 136806342
Test: open photos a bunch of times
Change-Id: I66d52c07f31ccf350dec4df665364868350e13c0
parent 769ab6f9

libs/renderengine/gl/GLESRenderEngine.cpp

+145 −145
Original line number Diff line number Diff line
@@ -614,54 +614,18 @@ void GLESRenderEngine::bindExternalTextureImage(uint32_t texName, const Image& i 
    }

}



status_t GLESRenderEngine::cacheExternalTextureBuffer(const sp<GraphicBuffer>& buffer) {

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

    return cacheExternalTextureBufferLocked(buffer);

}



status_t GLESRenderEngine::bindExternalTextureBuffer(uint32_t texName,

                                                     const sp<GraphicBuffer>& buffer,

                                                     const sp<Fence>& bufferFence) {

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

    return bindExternalTextureBufferLocked(texName, buffer, bufferFence);

}



status_t GLESRenderEngine::cacheExternalTextureBufferLocked(const sp<GraphicBuffer>& buffer) {

    if (buffer == nullptr) {

        return BAD_VALUE;

    }



    ATRACE_CALL();



    if (mImageCache.count(buffer->getId()) > 0) {

        return NO_ERROR;

    }



    std::unique_ptr<Image> newImage = createImage();



    bool created = newImage->setNativeWindowBuffer(buffer->getNativeBuffer(),

                                                   buffer->getUsage() & GRALLOC_USAGE_PROTECTED);

    if (!created) {

        ALOGE("Failed to create image. size=%ux%u st=%u usage=%#" PRIx64 " fmt=%d",

              buffer->getWidth(), buffer->getHeight(), buffer->getStride(), buffer->getUsage(),

              buffer->getPixelFormat());

        return NO_INIT;

    }

    mImageCache.insert(std::make_pair(buffer->getId(), std::move(newImage)));



    return NO_ERROR;

}



status_t GLESRenderEngine::bindExternalTextureBufferLocked(uint32_t texName,

                                                           const sp<GraphicBuffer>& buffer,

                                                           const sp<Fence>& bufferFence) {

    ATRACE_CALL();

    status_t cacheResult = cacheExternalTextureBufferLocked(buffer);

    status_t cacheResult = cacheExternalTextureBuffer(buffer);



    if (cacheResult != NO_ERROR) {

        return cacheResult;

    }



    {

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

        auto cachedImage = mImageCache.find(buffer->getId());



        if (cachedImage == mImageCache.end()) {
@@ -671,6 +635,7 @@ status_t GLESRenderEngine::bindExternalTextureBufferLocked(uint32_t texName, 
        }



        bindExternalTextureImage(texName, *cachedImage->second);

    }



    // Wait for the new buffer to be ready.

    if (bufferFence != nullptr && bufferFence->isValid()) {
@@ -696,6 +661,45 @@ status_t GLESRenderEngine::bindExternalTextureBufferLocked(uint32_t texName, 
    return NO_ERROR;

}



status_t GLESRenderEngine::cacheExternalTextureBuffer(const sp<GraphicBuffer>& buffer) {

    if (buffer == nullptr) {

        return BAD_VALUE;

    }



    {

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

        if (mImageCache.count(buffer->getId()) > 0) {

            // If there's already an image then fail fast here.

            return NO_ERROR;

        }

    }

    ATRACE_CALL();



    // Create the image without holding a lock so that we don't block anything.

    std::unique_ptr<Image> newImage = createImage();



    bool created = newImage->setNativeWindowBuffer(buffer->getNativeBuffer(),

                                                   buffer->getUsage() & GRALLOC_USAGE_PROTECTED);

    if (!created) {

        ALOGE("Failed to create image. size=%ux%u st=%u usage=%#" PRIx64 " fmt=%d",

              buffer->getWidth(), buffer->getHeight(), buffer->getStride(), buffer->getUsage(),

              buffer->getPixelFormat());

        return NO_INIT;

    }



    {

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

        if (mImageCache.count(buffer->getId()) > 0) {

            // In theory it's possible for another thread to recache the image,

            // so bail out if another thread won.

            return NO_ERROR;

        }

        mImageCache.insert(std::make_pair(buffer->getId(), std::move(newImage)));

    }



    return NO_ERROR;

}



void GLESRenderEngine::unbindExternalTextureBuffer(uint64_t bufferId) {

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

    const auto& cachedImage = mImageCache.find(bufferId);
@@ -889,9 +893,6 @@ status_t GLESRenderEngine::drawLayers(const DisplaySettings& display, 
        return BAD_VALUE;

    }



    {

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



    BindNativeBufferAsFramebuffer fbo(*this, buffer, useFramebufferCache);



    if (fbo.getStatus() != NO_ERROR) {
@@ -943,7 +944,7 @@ status_t GLESRenderEngine::drawLayers(const DisplaySettings& display, 
            isOpaque = layer.source.buffer.isOpaque;



            sp<GraphicBuffer> gBuf = layer.source.buffer.buffer;

                bindExternalTextureBufferLocked(layer.source.buffer.textureName, gBuf,

            bindExternalTextureBuffer(layer.source.buffer.textureName, gBuf,

                                      layer.source.buffer.fence);



            usePremultipliedAlpha = layer.source.buffer.usePremultipliedAlpha;
@@ -1009,7 +1010,6 @@ status_t GLESRenderEngine::drawLayers(const DisplaySettings& display, 
    }



    checkErrors();

    }

    return NO_ERROR;

}

libs/renderengine/gl/GLESRenderEngine.h

+1 −11
Original line number Diff line number Diff line
@@ -85,8 +85,7 @@ public: 
    bool useProtectedContext(bool useProtectedContext) override;

    status_t drawLayers(const DisplaySettings& display, const std::vector<LayerSettings>& layers,

                        ANativeWindowBuffer* buffer, const bool useFramebufferCache,

                        base::unique_fd&& bufferFence, base::unique_fd* drawFence)

            EXCLUDES(mRenderingMutex) override;

                        base::unique_fd&& bufferFence, base::unique_fd* drawFence) override;



    // internal to RenderEngine

    EGLDisplay getEGLDisplay() const { return mEGLDisplay; }
@@ -220,15 +219,6 @@ private: 
    // multiple threads is guaranteed thread-safe.

    std::mutex mRenderingMutex;



    // See bindExternalTextureBuffer above, but requiring that mRenderingMutex

    // is held.

    status_t bindExternalTextureBufferLocked(uint32_t texName, const sp<GraphicBuffer>& buffer,

                                             const sp<Fence>& fence) REQUIRES(mRenderingMutex);

    // See cacheExternalTextureBuffer above, but requiring that mRenderingMutex

    // is held.

    status_t cacheExternalTextureBufferLocked(const sp<GraphicBuffer>& buffer)

            REQUIRES(mRenderingMutex);



    std::unique_ptr<Framebuffer> mDrawingBuffer;



    class FlushTracer {

libs/renderengine/include/renderengine/RenderEngine.h

+11 −0
Original line number Diff line number Diff line
@@ -176,6 +176,17 @@ public: 
    // should be called for every display that needs to be rendered via the GPU.

    // @param display The display-wide settings that should be applied prior to

    // drawing any layers.

    //

    // Assumptions when calling this method:

    // 1. There is exactly one caller - i.e. multi-threading is not supported.

    // 2. Additional threads may be calling the {bind,cache}ExternalTexture

    // methods above. But the main thread is responsible for holding resources

    // such that Image destruction does not occur while this method is called.

    //

    // TODO(b/136806342): This should behavior should ideally be fixed since

    // the above two assumptions are brittle, as conditional thread safetyness

    // may be insufficient when maximizing rendering performance in the future.

    //

    // @param layers The layers to draw onto the display, in Z-order.

    // @param buffer The buffer which will be drawn to. This buffer will be

    // ready once drawFence fires.