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

Commit 74af6e28 authored by Chris Craik's avatar Chris Craik
Browse files

Fix OffscreenBuffer leak 

Fixes: 27941148

Make OffscreenBuffer lifecycle an explicit (and tested) contract between
FrameBuilder and BakedOpRenderer, entirely separate from dispatch. This
makes it safe to reject any rendering work via overdraw content
rejection (before it gets to a BakedOpDispatcher).

Adds a couple tests around OffscreenBuffer leaks, and switches
OffscreenBuffer tests to RENDERTHREAD_TEST macro, as appropriate.

Change-Id: Id114b835d042708ae921028fb4b17e5fa485fe64
parent 505a8d9d

libs/hwui/BakedOpDispatcher.cpp

+0 −4
Original line number Diff line number Diff line
@@ -809,10 +809,6 @@ void BakedOpDispatcher::onLayerOp(BakedOpRenderer& renderer, const LayerOp& op, 
                        Rect(op.unmappedBounds.getWidth(), op.unmappedBounds.getHeight()))

                .build();

        renderer.renderGlop(state, glop);



        if (op.destroy) {

            renderer.renderState().layerPool().putOrDelete(buffer);

        }

    }

}

libs/hwui/BakedOpRenderer.cpp

+4 −0
Original line number Diff line number Diff line
@@ -37,6 +37,10 @@ OffscreenBuffer* BakedOpRenderer::startTemporaryLayer(uint32_t width, uint32_t h 
    return buffer;

}



void BakedOpRenderer::recycleTemporaryLayer(OffscreenBuffer* offscreenBuffer) {

    mRenderState.layerPool().putOrDelete(offscreenBuffer);

}



void BakedOpRenderer::startRepaintLayer(OffscreenBuffer* offscreenBuffer, const Rect& repaintRect) {

    LOG_ALWAYS_FATAL_IF(mRenderTarget.offscreenBuffer, "already has layer...");

libs/hwui/BakedOpRenderer.h

+1 −0
Original line number Diff line number Diff line
@@ -69,6 +69,7 @@ public: 
    void startFrame(uint32_t width, uint32_t height, const Rect& repaintRect);

    void endFrame(const Rect& repaintRect);

    WARN_UNUSED_RESULT OffscreenBuffer* startTemporaryLayer(uint32_t width, uint32_t height);

    void recycleTemporaryLayer(OffscreenBuffer* offscreenBuffer);

    void startRepaintLayer(OffscreenBuffer* offscreenBuffer, const Rect& repaintRect);

    void endLayer();

    WARN_UNUSED_RESULT OffscreenBuffer* copyToLayer(const Rect& area);

libs/hwui/FrameBuilder.h

+6 −0
Original line number Diff line number Diff line
@@ -86,6 +86,7 @@ public: 
     */

    template <typename StaticDispatcher, typename Renderer>

    void replayBakedOps(Renderer& renderer) {

        std::vector<OffscreenBuffer*> temporaryLayers;

        finishDefer();

        /**

         * Defines a LUT of lambdas which allow a recorded BakedOpState to use state->op->opId to
@@ -129,6 +130,7 @@ public: 
            } else if (!layer.empty()) {

                // save layer - skip entire layer if empty (in which case, LayerOp has null layer).

                layer.offscreenBuffer = renderer.startTemporaryLayer(layer.width, layer.height);

                temporaryLayers.push_back(layer.offscreenBuffer);

                GL_CHECKPOINT(MODERATE);

                layer.replayBakedOpsImpl((void*)&renderer, unmergedReceivers, mergedReceivers);

                GL_CHECKPOINT(MODERATE);
@@ -145,6 +147,10 @@ public: 
            GL_CHECKPOINT(MODERATE);

            renderer.endFrame(fbo0.repaintRect);

        }



        for (auto& temporaryLayer : temporaryLayers) {

            renderer.recycleTemporaryLayer(temporaryLayer);

        }

    }



    void dump() const {

libs/hwui/RecordedOp.h

+3 −8
Original line number Diff line number Diff line
@@ -501,22 +501,20 @@ struct CopyFromLayerOp : RecordedOp { 
 * when creating/tracking a SkPaint* during defer isn't worth the bother.

 */

struct LayerOp : RecordedOp {

    // Records a one-use (saveLayer) layer for drawing. Once drawn, the layer will be destroyed.

    // Records a one-use (saveLayer) layer for drawing.

    LayerOp(BASE_PARAMS, OffscreenBuffer** layerHandle)

            : SUPER_PAINTLESS(LayerOp)

            , layerHandle(layerHandle)

            , alpha(paint ? paint->getAlpha() / 255.0f : 1.0f)

            , mode(PaintUtils::getXfermodeDirect(paint))

            , colorFilter(paint ? paint->getColorFilter() : nullptr)

            , destroy(true) {}

            , colorFilter(paint ? paint->getColorFilter() : nullptr) {}



    LayerOp(RenderNode& node)

            : RecordedOp(RecordedOpId::LayerOp, Rect(node.getWidth(), node.getHeight()), Matrix4::identity(), nullptr, nullptr)

            , layerHandle(node.getLayerHandle())

            , alpha(node.properties().layerProperties().alpha() / 255.0f)

            , mode(node.properties().layerProperties().xferMode())

            , colorFilter(node.properties().layerProperties().colorFilter())

            , destroy(false) {}

            , colorFilter(node.properties().layerProperties().colorFilter()) {}



    // Records a handle to the Layer object, since the Layer itself won't be

    // constructed until after this operation is constructed.
@@ -527,9 +525,6 @@ struct LayerOp : RecordedOp { 
    // pointer to object owned by either LayerProperties, or a recorded Paint object in a

    // BeginLayerOp. Lives longer than LayerOp in either case, so no skia ref counting is used.

    SkColorFilter* colorFilter;



    // whether to destroy the layer, once rendered

    const bool destroy;

};



}; // namespace uirenderer