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

Commit f0497270 authored by Alec Mouri's avatar Alec Mouri
Browse files

Add locking within RenderEngine during rendering 

Multiple threads are allowed to access RenderEngine, which poses several
issues:
1. The image cache needs to be synchronized
2. It's theoretically possible for a thread other than the SF
main-thread to call drawLayers, which can cause undesirable behavior.

Bug: 128306587
Test: manual tests
Change-Id: Ic18b71924aa3c4d72f02f95ebf928257fb9e50ed
parent dd50a3cf

libs/renderengine/gl/GLESRenderEngine.cpp

+98 −86
Original line number Diff line number Diff line
@@ -627,6 +627,13 @@ void GLESRenderEngine::bindExternalTextureImage(uint32_t texName, const Image& i 


status_t GLESRenderEngine::bindExternalTextureBuffer(uint32_t texName, sp<GraphicBuffer> buffer,

                                                     sp<Fence> bufferFence) {

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

    return bindExternalTextureBufferLocked(texName, buffer, bufferFence);

}



status_t GLESRenderEngine::bindExternalTextureBufferLocked(uint32_t texName,

                                                           sp<GraphicBuffer> buffer,

                                                           sp<Fence> bufferFence) {

    ATRACE_CALL();

    auto cachedImage = mImageCache.find(buffer->getId());
@@ -675,6 +682,7 @@ status_t GLESRenderEngine::bindExternalTextureBuffer(uint32_t texName, sp<Graphi 
}



void GLESRenderEngine::unbindExternalTextureBuffer(uint64_t bufferId) {

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

    const auto& cachedImage = mImageCache.find(bufferId);

    if (cachedImage != mImageCache.end()) {

        ALOGV("Destroying image for buffer: %" PRIu64, bufferId);
@@ -810,6 +818,9 @@ status_t GLESRenderEngine::drawLayers(const DisplaySettings& display, 
        sync_wait(bufferFence.get(), -1);

    }



    {

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



        BindNativeBufferAsFramebuffer fbo(*this, buffer);



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



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

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

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

                                                layer.source.buffer.fence);



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



        checkErrors();

    }

    return NO_ERROR;

}

libs/renderengine/gl/GLESRenderEngine.h

+15 −7
Original line number Diff line number Diff line
@@ -74,8 +74,9 @@ public: 
    void genTextures(size_t count, uint32_t* names) override;

    void deleteTextures(size_t count, uint32_t const* names) override;

    void bindExternalTextureImage(uint32_t texName, const Image& image) override;

    status_t bindExternalTextureBuffer(uint32_t texName, sp<GraphicBuffer> buffer, sp<Fence> fence);

    void unbindExternalTextureBuffer(uint64_t bufferId);

    status_t bindExternalTextureBuffer(uint32_t texName, sp<GraphicBuffer> buffer, sp<Fence> fence)

            EXCLUDES(mRenderingMutex);

    void unbindExternalTextureBuffer(uint64_t bufferId) EXCLUDES(mRenderingMutex);

    status_t bindFrameBuffer(Framebuffer* framebuffer) override;

    void unbindFrameBuffer(Framebuffer* framebuffer) override;

    void checkErrors() const override;
@@ -85,7 +86,7 @@ public: 
    bool useProtectedContext(bool useProtectedContext) override;

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

                        ANativeWindowBuffer* buffer, base::unique_fd&& bufferFence,

                        base::unique_fd* drawFence) override;

                        base::unique_fd* drawFence) EXCLUDES(mRenderingMutex) override;



    // internal to RenderEngine

    EGLDisplay getEGLDisplay() const { return mEGLDisplay; }
@@ -197,10 +198,17 @@ private: 
    const bool mUseColorManagement = false;



    // Cache of GL images that we'll store per GraphicBuffer ID

    // TODO: Layer should call back on destruction instead to clean this up,

    // as it has better system utilization at the potential expense of a

    // more complicated interface.

    std::unordered_map<uint64_t, std::unique_ptr<Image>> mImageCache;

    std::unordered_map<uint64_t, std::unique_ptr<Image>> mImageCache GUARDED_BY(mRenderingMutex);

    // Mutex guarding rendering operations, so that:

    // 1. GL operations aren't interleaved, and

    // 2. Internal state related to rendering that is potentially modified by

    // multiple threads is guaranteed thread-safe.

    std::mutex mRenderingMutex;



    // See bindExternalTextureBuffer above, but requiring that mRenderingMutex

    // is held.

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

                                             sp<Fence> fence) REQUIRES(mRenderingMutex);



    std::unique_ptr<Framebuffer> mDrawingBuffer;