From 6ba7f2b4870df952ff8bb470fd52d91d9f206ae6 Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Thu, 2 Jun 2022 22:55:05 +0000 Subject: Use the correct dataspace in shaders.cpp This improves the quality of HDR->SDR tone-mapping in TextureView by removing an incorrect gamut mapping which was causing undesired hue shift. The technical detail is that in TextureView, skia will color manage the input image to the destination colorspace prior to tone-mapping. The tone-mapping shader library compensates when authoring the shader to use the correct EOTF when re-decoding the image, but did not compensate when binding uniforms. This patch compensates accordingly so that the correct gamut->gamut mapping is applied. This patch adds a brief test suite for libshaders to verify that the correct color gamut matrices are being selected. Bug: 234355355 Test: Observe accurate HLG colors in Photos and Instagram where TextureView is used for media playback. Test: libshaders_test Change-Id: I801349cfe1780880a55528fd7e91ff1ac553281b --- libs/shaders/TEST_MAPPING | 10 ++++ libs/shaders/shaders.cpp | 9 +++- libs/shaders/tests/Android.bp | 48 ++++++++++++++++++ libs/shaders/tests/shaders_test.cpp | 98 +++++++++++++++++++++++++++++++++++++ 4 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 libs/shaders/TEST_MAPPING create mode 100644 libs/shaders/tests/Android.bp create mode 100644 libs/shaders/tests/shaders_test.cpp diff --git a/libs/shaders/TEST_MAPPING b/libs/shaders/TEST_MAPPING new file mode 100644 index 0000000000..ad6514d201 --- /dev/null +++ b/libs/shaders/TEST_MAPPING @@ -0,0 +1,10 @@ +{ + "presubmit": [ + { + "name": "librenderengine_test" + }, + { + "name": "libshaders_test" + } + ] +} diff --git a/libs/shaders/shaders.cpp b/libs/shaders/shaders.cpp index 62745dc8d5..f80e93f6f8 100644 --- a/libs/shaders/shaders.cpp +++ b/libs/shaders/shaders.cpp @@ -469,12 +469,17 @@ std::vector buildLinearEffectUniforms( float currentDisplayLuminanceNits, float maxLuminance, AHardwareBuffer* buffer, aidl::android::hardware::graphics::composer3::RenderIntent renderIntent) { std::vector uniforms; - if (linearEffect.inputDataspace == linearEffect.outputDataspace) { + + const ui::Dataspace inputDataspace = linearEffect.fakeInputDataspace == ui::Dataspace::UNKNOWN + ? linearEffect.inputDataspace + : linearEffect.fakeInputDataspace; + + if (inputDataspace == linearEffect.outputDataspace) { uniforms.push_back({.name = "in_rgbToXyz", .value = buildUniformValue(mat4())}); uniforms.push_back( {.name = "in_xyzToRgb", .value = buildUniformValue(colorTransform)}); } else { - ColorSpace inputColorSpace = toColorSpace(linearEffect.inputDataspace); + ColorSpace inputColorSpace = toColorSpace(inputDataspace); ColorSpace outputColorSpace = toColorSpace(linearEffect.outputDataspace); uniforms.push_back({.name = "in_rgbToXyz", .value = buildUniformValue(mat4(inputColorSpace.getRGBtoXYZ()))}); diff --git a/libs/shaders/tests/Android.bp b/libs/shaders/tests/Android.bp new file mode 100644 index 0000000000..cf671bcb7a --- /dev/null +++ b/libs/shaders/tests/Android.bp @@ -0,0 +1,48 @@ +// Copyright 2022 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. + +package { + // See: http://go/android-license-faq + // A large-scale-change added 'default_applicable_licenses' to import + // all of the 'license_kinds' from "frameworks_native_license" + // to get the below license kinds: + // SPDX-license-identifier-Apache-2.0 + default_applicable_licenses: ["frameworks_native_license"], +} + +cc_test { + name: "libshaders_test", + test_suites: ["device-tests"], + srcs: [ + "shaders_test.cpp", + ], + header_libs: [ + "libtonemap_headers", + ], + shared_libs: [ + "android.hardware.graphics.common-V3-ndk", + "android.hardware.graphics.composer3-V1-ndk", + "android.hardware.graphics.common@1.2", + "libnativewindow", + ], + static_libs: [ + "libarect", + "libgmock", + "libgtest", + "libmath", + "libshaders", + "libtonemap", + "libui-types", + ], +} diff --git a/libs/shaders/tests/shaders_test.cpp b/libs/shaders/tests/shaders_test.cpp new file mode 100644 index 0000000000..d45fb246c7 --- /dev/null +++ b/libs/shaders/tests/shaders_test.cpp @@ -0,0 +1,98 @@ +/* + * Copyright 2022 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 "shaders/shaders.h" +#include +#include +#include +#include +#include +#include + +namespace android { + +using testing::Contains; +using testing::HasSubstr; + +struct ShadersTest : public ::testing::Test {}; + +namespace { + +MATCHER_P2(UniformEq, name, value, "") { + return arg.name == name && arg.value == value; +} + +template ::value, bool> = true> +std::vector buildUniformValue(T value) { + std::vector result; + result.resize(sizeof(value)); + std::memcpy(result.data(), &value, sizeof(value)); + return result; +} + +} // namespace + +TEST_F(ShadersTest, buildLinearEffectUniforms_selectsNoOpGamutMatrices) { + shaders::LinearEffect effect = + shaders::LinearEffect{.inputDataspace = ui::Dataspace::V0_SRGB_LINEAR, + .outputDataspace = ui::Dataspace::V0_SRGB_LINEAR, + .fakeInputDataspace = ui::Dataspace::UNKNOWN}; + + mat4 colorTransform = mat4::scale(vec4(.9, .9, .9, 1.)); + auto uniforms = + shaders::buildLinearEffectUniforms(effect, colorTransform, 1.f, 1.f, 1.f, nullptr, + aidl::android::hardware::graphics::composer3:: + RenderIntent::COLORIMETRIC); + EXPECT_THAT(uniforms, Contains(UniformEq("in_rgbToXyz", buildUniformValue(mat4())))); + EXPECT_THAT(uniforms, + Contains(UniformEq("in_xyzToRgb", buildUniformValue(colorTransform)))); +} + +TEST_F(ShadersTest, buildLinearEffectUniforms_selectsGamutTransformMatrices) { + shaders::LinearEffect effect = + shaders::LinearEffect{.inputDataspace = ui::Dataspace::V0_SRGB, + .outputDataspace = ui::Dataspace::DISPLAY_P3, + .fakeInputDataspace = ui::Dataspace::UNKNOWN}; + + ColorSpace inputColorSpace = ColorSpace::sRGB(); + ColorSpace outputColorSpace = ColorSpace::DisplayP3(); + auto uniforms = + shaders::buildLinearEffectUniforms(effect, mat4(), 1.f, 1.f, 1.f, nullptr, + aidl::android::hardware::graphics::composer3:: + RenderIntent::COLORIMETRIC); + EXPECT_THAT(uniforms, + Contains(UniformEq("in_rgbToXyz", + buildUniformValue(mat4(inputColorSpace.getRGBtoXYZ()))))); + EXPECT_THAT(uniforms, + Contains(UniformEq("in_xyzToRgb", + buildUniformValue(mat4(outputColorSpace.getXYZtoRGB()))))); +} + +TEST_F(ShadersTest, buildLinearEffectUniforms_respectsFakeInputDataspace) { + shaders::LinearEffect effect = + shaders::LinearEffect{.inputDataspace = ui::Dataspace::V0_SRGB, + .outputDataspace = ui::Dataspace::DISPLAY_P3, + .fakeInputDataspace = ui::Dataspace::DISPLAY_P3}; + + auto uniforms = + shaders::buildLinearEffectUniforms(effect, mat4(), 1.f, 1.f, 1.f, nullptr, + aidl::android::hardware::graphics::composer3:: + RenderIntent::COLORIMETRIC); + EXPECT_THAT(uniforms, Contains(UniformEq("in_rgbToXyz", buildUniformValue(mat4())))); + EXPECT_THAT(uniforms, Contains(UniformEq("in_xyzToRgb", buildUniformValue(mat4())))); +} + +} // namespace android -- cgit v1.2.3-59-g8ed1b