summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlec Mouri <alecmouri@google.com>2022-06-02 22:55:05 +0000
committerAlec Mouri <alecmouri@google.com>2022-06-06 16:40:33 +0000
commit6ba7f2b4870df952ff8bb470fd52d91d9f206ae6 (patch)
tree546acf824881a4a51a01fd27836d7ece0b0efd5d
parent1821df6cd73fd31ef0b3d0c6e8fc4219bdce2002 (diff)
downloadnative-6ba7f2b4870df952ff8bb470fd52d91d9f206ae6.tar.gz
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
-rw-r--r--libs/shaders/TEST_MAPPING10
-rw-r--r--libs/shaders/shaders.cpp9
-rw-r--r--libs/shaders/tests/Android.bp48
-rw-r--r--libs/shaders/tests/shaders_test.cpp98
4 files changed, 163 insertions, 2 deletions
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<tonemap::ShaderUniform> buildLinearEffectUniforms(
float currentDisplayLuminanceNits, float maxLuminance, AHardwareBuffer* buffer,
aidl::android::hardware::graphics::composer3::RenderIntent renderIntent) {
std::vector<tonemap::ShaderUniform> 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>(mat4())});
uniforms.push_back(
{.name = "in_xyzToRgb", .value = buildUniformValue<mat4>(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>(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 <gmock/gmock.h>
+#include <gtest/gtest.h>
+#include <math/mat4.h>
+#include <tonemap/tonemap.h>
+#include <ui/ColorSpace.h>
+#include <cmath>
+
+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 <typename T, std::enable_if_t<std::is_trivially_copyable<T>::value, bool> = true>
+std::vector<uint8_t> buildUniformValue(T value) {
+ std::vector<uint8_t> 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>(mat4()))));
+ EXPECT_THAT(uniforms,
+ Contains(UniformEq("in_xyzToRgb", buildUniformValue<mat4>(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>(mat4(inputColorSpace.getRGBtoXYZ())))));
+ EXPECT_THAT(uniforms,
+ Contains(UniformEq("in_xyzToRgb",
+ buildUniformValue<mat4>(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>(mat4()))));
+ EXPECT_THAT(uniforms, Contains(UniformEq("in_xyzToRgb", buildUniformValue<mat4>(mat4()))));
+}
+
+} // namespace android