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

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

Cleanup Skia RE display and layer colorTransforms 

In the general case we don't set the display's colorMatrix on the paint
if we know it is the identity matrix.  We also now abort early if
the layer has no alpha and no colortransform or colorspace conversion
will change that.

The layer's colortransform is now respected regardless of whether or
not color management is enabled for the RE instance

Also includes some small code refactors to avoid computing the same
values in multiple places.

Test: librenderengine_test
Bug: 180712498
Bug: 181028875
Change-Id: I6e236efc5987d00cb464e8798c48f2b3b21635c5
parent 4c331c85

libs/renderengine/skia/SkiaGLRenderEngine.cpp

+45 −36
Original line number Diff line number Diff line
@@ -454,11 +454,6 @@ static bool needsToneMapping(ui::Dataspace sourceDataspace, ui::Dataspace destin 
            sourceTransfer != destTransfer;

}



static bool needsLinearEffect(const mat4& colorTransform, ui::Dataspace sourceDataspace,

                              ui::Dataspace destinationDataspace) {

    return colorTransform != mat4() || needsToneMapping(sourceDataspace, destinationDataspace);

}



void SkiaGLRenderEngine::cacheExternalTextureBuffer(const sp<GraphicBuffer>& buffer) {

    // Only run this if RE is running on its own thread. This way the access to GL

    // operations is guaranteed to be happening on the same thread.
@@ -491,14 +486,19 @@ void SkiaGLRenderEngine::unbindExternalTextureBuffer(uint64_t bufferId) { 
sk_sp<SkShader> SkiaGLRenderEngine::createRuntimeEffectShader(sk_sp<SkShader> shader,

                                                              const LayerSettings* layer,

                                                              const DisplaySettings& display,

                                                              bool undoPremultipliedAlpha) {

                                                              bool undoPremultipliedAlpha,

                                                              bool requiresLinearEffect) {

    if (layer->stretchEffect.hasEffect()) {

        // TODO: Implement

    }

    if (mUseColorManagement &&

        needsLinearEffect(layer->colorTransform, layer->sourceDataspace, display.outputDataspace)) {

        LinearEffect effect = LinearEffect{.inputDataspace = layer->sourceDataspace,

                                           .outputDataspace = display.outputDataspace,

    if (requiresLinearEffect) {

        const ui::Dataspace inputDataspace =

                mUseColorManagement ? layer->sourceDataspace : ui::Dataspace::UNKNOWN;

        const ui::Dataspace outputDataspace =

                mUseColorManagement ? display.outputDataspace : ui::Dataspace::UNKNOWN;



        LinearEffect effect = LinearEffect{.inputDataspace = inputDataspace,

                                           .outputDataspace = outputDataspace,

                                           .undoPremultipliedAlpha = undoPremultipliedAlpha};



        auto effectIter = mRuntimeEffects.find(effect);
@@ -630,11 +630,10 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, 
        }

    }



    const ui::Dataspace dstDataspace =

            mUseColorManagement ? display.outputDataspace : ui::Dataspace::UNKNOWN;

    sk_sp<SkSurface> dstSurface =

            surfaceTextureRef->getTexture()->getOrCreateSurface(mUseColorManagement

                                                                        ? display.outputDataspace

                                                                        : ui::Dataspace::UNKNOWN,

                                                                grContext.get());

            surfaceTextureRef->getTexture()->getOrCreateSurface(dstDataspace, grContext.get());



    SkCanvas* dstCanvas = mCapture->tryCapture(dstSurface.get());

    if (dstCanvas == nullptr) {
@@ -691,13 +690,18 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, 
        SkPaint paint;

        sk_sp<SkShader> shader =

                SkShaders::Color(SkColor4f{.fR = 0., .fG = 0., .fB = 0., .fA = 1.0},

                                 toSkColorSpace(mUseColorManagement ? display.outputDataspace

                                                                    : ui::Dataspace::UNKNOWN));

                                 toSkColorSpace(dstDataspace));

        paint.setShader(shader);

        clearRegion.setRects(skRects, numRects);

        canvas->drawRegion(clearRegion, paint);

    }



    // setup color filter if necessary

    sk_sp<SkColorFilter> displayColorTransform;

    if (display.colorTransform != mat4()) {

        displayColorTransform = SkColorFilters::Matrix(toSkColorMatrix(display.colorTransform));

    }



    for (const auto& layer : layers) {

        ATRACE_NAME("DrawLayer");
@@ -797,15 +801,22 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, 
            continue;

        }



        const ui::Dataspace targetDataspace = mUseColorManagement

                ? (needsLinearEffect(layer->colorTransform, layer->sourceDataspace,

                                     display.outputDataspace)

                           // If we need to map to linear space, then mark the source image with the

                           // same colorspace as the destination surface so that Skia's color

        const bool requiresLinearEffect = layer->colorTransform != mat4() ||

                (mUseColorManagement &&

                 needsToneMapping(layer->sourceDataspace, display.outputDataspace));



        // quick abort from drawing the remaining portion of the layer

        if (layer->alpha == 0 && !requiresLinearEffect &&

            (!displayColorTransform || displayColorTransform->isAlphaUnchanged())) {

            continue;

        }



        // If we need to map to linear space or color management is disabled, then mark the source

        // image with the same colorspace as the destination surface so that Skia's color

        // management is a no-op.

                           ? display.outputDataspace

                           : layer->sourceDataspace)

                : ui::Dataspace::UNKNOWN;

        const ui::Dataspace layerDataspace = (!mUseColorManagement || requiresLinearEffect)

                ? dstDataspace

                : layer->sourceDataspace;



        SkPaint paint;

        if (layer->source.buffer.buffer) {
@@ -824,7 +835,7 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, 
            }



            sk_sp<SkImage> image =

                    imageTextureRef->getTexture()->makeImage(targetDataspace,

                    imageTextureRef->getTexture()->makeImage(layerDataspace,

                                                             item.usePremultipliedAlpha

                                                                     ? kPremul_SkAlphaType

                                                                     : kUnpremul_SkAlphaType,
@@ -880,12 +891,12 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, 
            if (item.isOpaque) {

                shader = SkShaders::Blend(SkBlendMode::kPlus, shader,

                                          SkShaders::Color(SkColors::kBlack,

                                                           toSkColorSpace(targetDataspace)));

                                                           toSkColorSpace(layerDataspace)));

            }



            paint.setShader(

                    createRuntimeEffectShader(shader, layer, display,

                                              !item.isOpaque && item.usePremultipliedAlpha));

            paint.setShader(createRuntimeEffectShader(shader, layer, display,

                                                      !item.isOpaque && item.usePremultipliedAlpha,

                                                      requiresLinearEffect));

            paint.setAlphaf(layer->alpha);

        } else {

            ATRACE_NAME("DrawColor");
@@ -894,15 +905,13 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, 
                                                                .fG = color.g,

                                                                .fB = color.b,

                                                                .fA = layer->alpha},

                                                      toSkColorSpace(targetDataspace));

                                                      toSkColorSpace(layerDataspace));

            paint.setShader(createRuntimeEffectShader(shader, layer, display,

                                                      /* undoPremultipliedAlpha */ false));

                                                      /* undoPremultipliedAlpha */ false,

                                                      requiresLinearEffect));

        }



        sk_sp<SkColorFilter> filter =

                SkColorFilters::Matrix(toSkColorMatrix(display.colorTransform));



        paint.setColorFilter(filter);

        paint.setColorFilter(displayColorTransform);



        if (layer->geometry.roundedCornersRadius > 0) {

            paint.setAntiAlias(true);

libs/renderengine/skia/SkiaGLRenderEngine.h

+4 −3
Original line number Diff line number Diff line
@@ -92,11 +92,12 @@ private: 
    void initCanvas(SkCanvas* canvas, const DisplaySettings& display);

    void drawShadow(SkCanvas* canvas, const SkRect& casterRect, float casterCornerRadius,

                    const ShadowSettings& shadowSettings);

    // If mUseColorManagement is correct and layer needsLinearEffect, it returns a linear runtime

    // shader. Otherwise it returns the input shader.

    // If requiresLinearEffect is true or the layer has a stretchEffect a new shader is returned.

    // Otherwise it returns the input shader.

    sk_sp<SkShader> createRuntimeEffectShader(sk_sp<SkShader> shader, const LayerSettings* layer,

                                              const DisplaySettings& display,

                                              bool undoPremultipliedAlpha);

                                              bool undoPremultipliedAlpha,

                                              bool requiresLinearEffect);



    EGLDisplay mEGLDisplay;

    EGLContext mEGLContext;