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

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

Clean up and fix skia-RenderEngine tests

Skia-RenderEngine tests were accidentally exercising solely
GLESRenderEngine, so the wrong backend was being used. This patch:
* Correctly enables Skia-Renderengine tests
* Adds a downcasted pointer to GLESRenderEngine to access test-only
methods so that there's no loss of test coverage
* Disables Skia tests that rely on those GLES-specific test-only
methods, since those tests were behavior-specific anyways
* Correctly destroy the Skia RenderEngine instance
* Add fixes for the remaining failing tests. Fixes include:
** Fix SkImage caching to...actually cache
** Forcing the opaque bit must occur on the SkShader, rather than on the
SkColorFilter, so that the alpha channel is not sampled when computing
the src colors.
** When only a color transform is applied but color management is
enabled, converting between RGB primaries and linear XYZ reduces down to
an identity matrix, which avoids clamping artifacts in the shader.
** Apply source dataspace properly when there are solid colors
** Support for black clearRegions for SurfaceView

Bug: 173416417
Test: librenderengine_test
Change-Id: I530441a80039c7807931985665892017e4c48e76
parent 90f29fbd
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -79,6 +79,7 @@ filegroup {
    name: "librenderengine_skia_sources",
    srcs: [
        "skia/AutoBackendTexture.cpp",
        "skia/ColorSpaces.cpp",
        "skia/SkiaRenderEngine.cpp",
        "skia/SkiaGLRenderEngine.cpp",
        "skia/debug/CaptureTimer.cpp",
+1 −43
Original line number Diff line number Diff line
@@ -20,8 +20,7 @@
#define LOG_TAG "RenderEngine"
#define ATRACE_TAG ATRACE_TAG_GRAPHICS

#include <utils/Trace.h>

#include "ColorSpaces.h"
#include "log/log_main.h"
#include "utils/Trace.h"

@@ -29,47 +28,6 @@ namespace android {
namespace renderengine {
namespace skia {

// Converts an android dataspace to a supported SkColorSpace
// Supported dataspaces are
// 1. sRGB
// 2. Display P3
// 3. BT2020 PQ
// 4. BT2020 HLG
// Unknown primaries are mapped to BT709, and unknown transfer functions
// are mapped to sRGB.
static sk_sp<SkColorSpace> toSkColorSpace(ui::Dataspace dataspace) {
    skcms_Matrix3x3 gamut;
    switch (dataspace & HAL_DATASPACE_STANDARD_MASK) {
        case HAL_DATASPACE_STANDARD_BT709:
            gamut = SkNamedGamut::kSRGB;
            break;
        case HAL_DATASPACE_STANDARD_BT2020:
            gamut = SkNamedGamut::kRec2020;
            break;
        case HAL_DATASPACE_STANDARD_DCI_P3:
            gamut = SkNamedGamut::kDisplayP3;
            break;
        default:
            ALOGV("Unsupported Gamut: %d, defaulting to sRGB", dataspace);
            gamut = SkNamedGamut::kSRGB;
            break;
    }

    switch (dataspace & HAL_DATASPACE_TRANSFER_MASK) {
        case HAL_DATASPACE_TRANSFER_LINEAR:
            return SkColorSpace::MakeRGB(SkNamedTransferFn::kLinear, gamut);
        case HAL_DATASPACE_TRANSFER_SRGB:
            return SkColorSpace::MakeRGB(SkNamedTransferFn::kSRGB, gamut);
        case HAL_DATASPACE_TRANSFER_ST2084:
            return SkColorSpace::MakeRGB(SkNamedTransferFn::kPQ, gamut);
        case HAL_DATASPACE_TRANSFER_HLG:
            return SkColorSpace::MakeRGB(SkNamedTransferFn::kHLG, gamut);
        default:
            ALOGV("Unsupported Gamma: %d, defaulting to sRGB transfer", dataspace);
            return SkColorSpace::MakeRGB(SkNamedTransferFn::kSRGB, gamut);
    }
}

AutoBackendTexture::AutoBackendTexture(GrDirectContext* context, AHardwareBuffer* buffer,
                                       bool isRender) {
    ATRACE_CALL();
+56 −0
Original line number Diff line number Diff line
/*
 * Copyright 2021 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

#include "ColorSpaces.h"

namespace android {
namespace renderengine {
namespace skia {

sk_sp<SkColorSpace> toSkColorSpace(ui::Dataspace dataspace) {
    skcms_Matrix3x3 gamut;
    switch (dataspace & HAL_DATASPACE_STANDARD_MASK) {
        case HAL_DATASPACE_STANDARD_BT709:
            gamut = SkNamedGamut::kSRGB;
            break;
        case HAL_DATASPACE_STANDARD_BT2020:
            gamut = SkNamedGamut::kRec2020;
            break;
        case HAL_DATASPACE_STANDARD_DCI_P3:
            gamut = SkNamedGamut::kDisplayP3;
            break;
        default:
            gamut = SkNamedGamut::kSRGB;
            break;
    }

    switch (dataspace & HAL_DATASPACE_TRANSFER_MASK) {
        case HAL_DATASPACE_TRANSFER_LINEAR:
            return SkColorSpace::MakeRGB(SkNamedTransferFn::kLinear, gamut);
        case HAL_DATASPACE_TRANSFER_SRGB:
            return SkColorSpace::MakeRGB(SkNamedTransferFn::kSRGB, gamut);
        case HAL_DATASPACE_TRANSFER_ST2084:
            return SkColorSpace::MakeRGB(SkNamedTransferFn::kPQ, gamut);
        case HAL_DATASPACE_TRANSFER_HLG:
            return SkColorSpace::MakeRGB(SkNamedTransferFn::kHLG, gamut);
        default:
            return SkColorSpace::MakeRGB(SkNamedTransferFn::kSRGB, gamut);
    }
}

} // namespace skia
} // namespace renderengine
} // namespace android
 No newline at end of file
+38 −0
Original line number Diff line number Diff line
/*
 * Copyright 2021 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

#pragma once

#include "SkColorSpace.h"
#include "ui/GraphicTypes.h"

namespace android {
namespace renderengine {
namespace skia {

// Converts an android dataspace to a supported SkColorSpace
// Supported dataspaces are
// 1. sRGB
// 2. Display P3
// 3. BT2020 PQ
// 4. BT2020 HLG
// Unknown primaries are mapped to BT709, and unknown transfer functions
// are mapped to sRGB.
sk_sp<SkColorSpace> toSkColorSpace(ui::Dataspace dataspace);

} // namespace skia
} // namespace renderengine
} // namespace android
 No newline at end of file
+117 −56
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@
#include <SkColorSpace.h>
#include <SkImage.h>
#include <SkImageFilters.h>
#include <SkRegion.h>
#include <SkShadowUtils.h>
#include <SkSurface.h>
#include <gl/GrGLInterface.h>
@@ -43,6 +44,7 @@
#include <memory>

#include "../gl/GLExtensions.h"
#include "ColorSpaces.h"
#include "SkBlendMode.h"
#include "SkImageInfo.h"
#include "filters/BlurFilter.h"
@@ -281,6 +283,44 @@ SkiaGLRenderEngine::SkiaGLRenderEngine(const RenderEngineCreationArgs& args, EGL
    if (args.supportsBackgroundBlur) {
        mBlurFilter = new BlurFilter();
    }
    mCapture = std::make_unique<SkiaCapture>();
}

SkiaGLRenderEngine::~SkiaGLRenderEngine() {
    std::lock_guard<std::mutex> lock(mRenderingMutex);
    mRuntimeEffects.clear();
    mProtectedTextureCache.clear();
    mTextureCache.clear();

    if (mBlurFilter) {
        delete mBlurFilter;
    }

    mCapture = nullptr;

    mGrContext->flushAndSubmit(true);
    mGrContext->abandonContext();

    if (mProtectedGrContext) {
        mProtectedGrContext->flushAndSubmit(true);
        mProtectedGrContext->abandonContext();
    }

    if (mPlaceholderSurface != EGL_NO_SURFACE) {
        eglDestroySurface(mEGLDisplay, mPlaceholderSurface);
    }
    if (mProtectedPlaceholderSurface != EGL_NO_SURFACE) {
        eglDestroySurface(mEGLDisplay, mProtectedPlaceholderSurface);
    }
    if (mEGLContext != EGL_NO_CONTEXT) {
        eglDestroyContext(mEGLDisplay, mEGLContext);
    }
    if (mProtectedEGLContext != EGL_NO_CONTEXT) {
        eglDestroyContext(mEGLDisplay, mProtectedEGLContext);
    }
    eglMakeCurrent(mEGLDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
    eglTerminate(mEGLDisplay);
    eglReleaseThread();
}

bool SkiaGLRenderEngine::supportsProtectedContent() const {
@@ -298,6 +338,7 @@ bool SkiaGLRenderEngine::useProtectedContext(bool useProtectedContext) {
            useProtectedContext ? mProtectedPlaceholderSurface : mPlaceholderSurface;
    const EGLContext context = useProtectedContext ? mProtectedEGLContext : mEGLContext;
    const bool success = eglMakeCurrent(mEGLDisplay, surface, surface, context) == EGL_TRUE;

    if (success) {
        mInProtectedContext = useProtectedContext;
    }
@@ -450,6 +491,7 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display,
                                        const bool useFramebufferCache,
                                        base::unique_fd&& bufferFence, base::unique_fd* drawFence) {
    ATRACE_NAME("SkiaGL::drawLayers");

    std::lock_guard<std::mutex> lock(mRenderingMutex);
    if (layers.empty()) {
        ALOGV("Drawing empty layer stack");
@@ -488,7 +530,7 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display,
    if (surfaceTextureRef == nullptr || surfaceTextureRef->getTexture() == nullptr) {
        surfaceTextureRef = std::make_shared<AutoBackendTexture::LocalRef>();
        surfaceTextureRef->setTexture(
                new AutoBackendTexture(mGrContext.get(), buffer->toAHardwareBuffer(), true));
                new AutoBackendTexture(grContext.get(), buffer->toAHardwareBuffer(), true));
        if (useFramebufferCache) {
            ALOGD("Adding to cache");
            cache.insert({buffer->getId(), surfaceTextureRef});
@@ -498,10 +540,10 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display,
    sk_sp<SkSurface> surface =
            surfaceTextureRef->getTexture()->getOrCreateSurface(mUseColorManagement
                                                                        ? display.outputDataspace
                                                                        : ui::Dataspace::SRGB,
                                                                mGrContext.get());
                                                                        : ui::Dataspace::UNKNOWN,
                                                                grContext.get());

    SkCanvas* canvas = mCapture.tryCapture(surface.get());
    SkCanvas* canvas = mCapture->tryCapture(surface.get());
    if (canvas == nullptr) {
        ALOGE("Cannot acquire canvas from Skia.");
        return BAD_VALUE;
@@ -510,7 +552,7 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display,
    canvas->clear(SK_ColorTRANSPARENT);
    canvas->save();

    if (mCapture.isCaptureRunning()) {
    if (mCapture->isCaptureRunning()) {
        // Record display settings when capture is running.
        std::stringstream displaySettings;
        PrintTo(display, &displaySettings);
@@ -546,10 +588,33 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display,
    canvas->rotate(toDegrees(display.orientation));
    canvas->translate(-clipWidth / 2, -clipHeight / 2);
    canvas->translate(-display.clip.left, -display.clip.top);

    // TODO: clearRegion was required for SurfaceView when a buffer is not yet available but the
    // view is still on-screen. The clear region could be re-specified as a black color layer,
    // however.
    if (!display.clearRegion.isEmpty()) {
        size_t numRects = 0;
        Rect const* rects = display.clearRegion.getArray(&numRects);
        SkIRect skRects[numRects];
        for (int i = 0; i < numRects; ++i) {
            skRects[i] =
                    SkIRect::MakeLTRB(rects[i].left, rects[i].top, rects[i].right, rects[i].bottom);
        }
        SkRegion clearRegion;
        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));
        paint.setShader(shader);
        clearRegion.setRects(skRects, numRects);
        canvas->drawRegion(clearRegion, paint);
    }

    for (const auto& layer : layers) {
        canvas->save();

        if (mCapture.isCaptureRunning()) {
        if (mCapture->isCaptureRunning()) {
            // Record the name of the layer if the capture is running.
            std::stringstream layerSettings;
            PrintTo(*layer, &layerSettings);
@@ -588,6 +653,16 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display,
            }
        }

        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
                           // management is a no-op.
                           ? display.outputDataspace
                           : layer->sourceDataspace)
                : ui::Dataspace::UNKNOWN;

        if (layer->source.buffer.buffer) {
            ATRACE_NAME("DrawImage");
            const auto& item = layer->source.buffer;
@@ -597,31 +672,18 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display,
                imageTextureRef = iter->second;
            } else {
                imageTextureRef = std::make_shared<AutoBackendTexture::LocalRef>();
                imageTextureRef->setTexture(new AutoBackendTexture(mGrContext.get(),
                imageTextureRef->setTexture(new AutoBackendTexture(grContext.get(),
                                                                   item.buffer->toAHardwareBuffer(),
                                                                   false));
                mTextureCache.insert({buffer->getId(), imageTextureRef});
                mTextureCache.insert({item.buffer->getId(), imageTextureRef});
            }

            sk_sp<SkImage> image =
                    imageTextureRef->getTexture()
                            ->makeImage(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
                                                           // management is a no-op.
                                                           ? display.outputDataspace
                                                           : layer->sourceDataspace)
                                                : ui::Dataspace::SRGB,
                                        item.isOpaque ? kOpaque_SkAlphaType
                                                      : (item.usePremultipliedAlpha
                    imageTextureRef->getTexture()->makeImage(targetDataspace,
                                                             item.usePremultipliedAlpha
                                                                     ? kPremul_SkAlphaType
                                                                 : kUnpremul_SkAlphaType),
                                        mGrContext.get());
                                                                     : kUnpremul_SkAlphaType,
                                                             grContext.get());

            auto texMatrix = getSkM44(item.textureTransform).asM33();
            // textureTansform was intended to be passed directly into a shader, so when
@@ -650,11 +712,35 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display,
                shader = image->makeShader(SkSamplingOptions(), matrix);
            }

            // Handle opaque images - it's a little nonstandard how we do this.
            // Fundamentally we need to support SurfaceControl.Builder#setOpaque:
            // https://developer.android.com/reference/android/view/SurfaceControl.Builder#setOpaque(boolean)
            // The important language is that when isOpaque is set, opacity is not sampled from the
            // alpha channel, but blending may still be supported on a transaction via setAlpha. So,
            // here's the conundrum:
            // 1. We can't force the SkImage alpha type to kOpaque_SkAlphaType, because it's treated
            // as an internal hint - composition is undefined when there are alpha bits present.
            // 2. We can try to lie about the pixel layout, but that only works for RGBA8888
            // buffers, i.e., treating them as RGBx8888 instead. But we can't do the same for
            // RGBA1010102 because RGBx1010102 is not supported as a pixel layout for SkImages. It's
            // also not clear what to use for F16 either, and lying about the pixel layout is a bit
            // of a hack anyways.
            // 3. We can't change the blendmode to src, because while this satisfies the requirement
            // for ignoring the alpha channel, it doesn't quite satisfy the blending requirement
            // because src always clobbers the destination content.
            //
            // So, what we do here instead is an additive blend mode where we compose the input
            // image with a solid black. This might need to be reassess if this does not support
            // FP16 incredibly well, but FP16 end-to-end isn't well supported anyway at the moment.
            if (item.isOpaque) {
                shader = SkShaders::Blend(SkBlendMode::kPlus, shader,
                                          SkShaders::Color(SkColors::kBlack,
                                                           toSkColorSpace(targetDataspace)));
            }

            paint.setShader(
                    createRuntimeEffectShader(shader, layer, display,
                                              !item.isOpaque && item.usePremultipliedAlpha));

            // Make sure to take into the account the alpha set on the layer.
            paint.setAlphaf(layer->alpha);
        } else {
            ATRACE_NAME("DrawColor");
@@ -662,8 +748,8 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display,
            sk_sp<SkShader> shader = SkShaders::Color(SkColor4f{.fR = color.r,
                                                                .fG = color.g,
                                                                .fB = color.b,
                                                                layer->alpha},
                                                      nullptr);
                                                                .fA = layer->alpha},
                                                      toSkColorSpace(targetDataspace));
            paint.setShader(createRuntimeEffectShader(shader, layer, display,
                                                      /* undoPremultipliedAlpha */ false));
        }
@@ -671,31 +757,6 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display,
        sk_sp<SkColorFilter> filter =
                SkColorFilters::Matrix(toSkColorMatrix(display.colorTransform));

        // Handle opaque images - it's a little nonstandard how we do this.
        // Fundamentally we need to support SurfaceControl.Builder#setOpaque:
        // https://developer.android.com/reference/android/view/SurfaceControl.Builder#setOpaque(boolean)
        // The important language is that when isOpaque is set, opacity is not sampled from the
        // alpha channel, but blending may still be supported on a transaction via setAlpha. So,
        // here's the conundrum:
        // 1. We can't force the SkImage alpha type to kOpaque_SkAlphaType, because it's treated as
        // an internal hint - composition is undefined when there are alpha bits present.
        // 2. We can try to lie about the pixel layout, but that only works for RGBA8888 buffers,
        // i.e., treating them as RGBx8888 instead. But we can't do the same for RGBA1010102 because
        // RGBx1010102 is not supported as a pixel layout for SkImages. It's also not clear what to
        // use for F16 either, and lying about the pixel layout is a bit of a hack anyways.
        // 3. We can't change the blendmode to src, because while this satisfies the requirement for
        // ignoring the alpha channel, it doesn't quite satisfy the blending requirement because
        // src always clobbers the destination content.
        //
        // So, what we do here instead is an additive blend mode where we compose the input image
        // with a solid black. This might need to be reassess if this does not support FP16
        // incredibly well, but FP16 end-to-end isn't well supported anyway at the moment.
        if (layer->source.buffer.buffer && layer->source.buffer.isOpaque) {
            filter = SkColorFilters::Compose(filter,
                                             SkColorFilters::Blend(SK_ColorBLACK,
                                                                   SkBlendMode::kPlus));
        }

        paint.setColorFilter(filter);

        for (const auto effectRegion : layer->blurRegions) {
@@ -721,7 +782,7 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display,
        canvas->restore();
    }
    canvas->restore();
    mCapture.endCapture();
    mCapture->endCapture();
    {
        ATRACE_NAME("flush surface");
        surface->flush();
Loading