From ba2f0bc333ad71e3a4b97709c200c6d4aceb3f8b Mon Sep 17 00:00:00 2001 From: KaiChieh Chuang Date: Mon, 7 Sep 2020 13:48:42 +0800 Subject: [RenderEngine] Introduce non-linear display color transform. A display color transform is a non-linear color matrix that should be applied in gamma space. Previously the non-linear display color transform is mixed into the linear color matrix that results in incorrect color inversion behaviour. RenderEngineTest fix fillBufferColorTransform calling wrong function fillBufferColorTransform display color transform set to 0.9f add renderengine object for useColorManagement enabled, to test color management affected on display and layer color transform Bug: 157203983 Test: color inversion Test: run RenderEngineTest Change-Id: Ic731f014a14795b80323eeaf929761c828150f91 Merged-In: Ic731f014a14795b80323eeaf929761c828150f91 --- libs/renderengine/Description.cpp | 5 ++ libs/renderengine/gl/GLESRenderEngine.cpp | 7 +- libs/renderengine/gl/GLESRenderEngine.h | 1 + libs/renderengine/gl/Program.cpp | 4 + libs/renderengine/gl/Program.h | 1 + libs/renderengine/gl/ProgramCache.cpp | 30 ++++++- libs/renderengine/gl/ProgramCache.h | 8 ++ .../include/renderengine/private/Description.h | 3 + libs/renderengine/tests/RenderEngineTest.cpp | 100 +++++++++++++++------ 9 files changed, 129 insertions(+), 30 deletions(-) diff --git a/libs/renderengine/Description.cpp b/libs/renderengine/Description.cpp index b9cea1071f..245c9e1e30 100644 --- a/libs/renderengine/Description.cpp +++ b/libs/renderengine/Description.cpp @@ -52,5 +52,10 @@ bool Description::hasColorMatrix() const { return colorMatrix != identity; } +bool Description::hasDisplayColorMatrix() const { + const mat4 identity; + return displayColorMatrix != identity; +} + } // namespace renderengine } // namespace android diff --git a/libs/renderengine/gl/GLESRenderEngine.cpp b/libs/renderengine/gl/GLESRenderEngine.cpp index 2139acb715..02021c9cea 100644 --- a/libs/renderengine/gl/GLESRenderEngine.cpp +++ b/libs/renderengine/gl/GLESRenderEngine.cpp @@ -1048,6 +1048,7 @@ status_t GLESRenderEngine::drawLayers(const DisplaySettings& display, setOutputDataSpace(display.outputDataspace); setDisplayMaxLuminance(display.maxLuminance); + setDisplayColorTransform(display.colorTransform); const mat4 projectionMatrix = ui::Transform(display.orientation).asMatrix4() * mState.projectionMatrix; @@ -1114,7 +1115,7 @@ status_t GLESRenderEngine::drawLayers(const DisplaySettings& display, position[3] = vec2(bounds.right, bounds.top); setupLayerCropping(*layer, mesh); - setColorTransform(display.colorTransform * layer->colorTransform); + setColorTransform(layer->colorTransform); bool usePremultipliedAlpha = true; bool disableTexture = true; @@ -1271,6 +1272,10 @@ void GLESRenderEngine::setColorTransform(const mat4& colorTransform) { mState.colorMatrix = colorTransform; } +void GLESRenderEngine::setDisplayColorTransform(const mat4& colorTransform) { + mState.displayColorMatrix = colorTransform; +} + void GLESRenderEngine::disableTexturing() { mState.textureEnabled = false; } diff --git a/libs/renderengine/gl/GLESRenderEngine.h b/libs/renderengine/gl/GLESRenderEngine.h index 61986ff721..0e003a05fe 100644 --- a/libs/renderengine/gl/GLESRenderEngine.h +++ b/libs/renderengine/gl/GLESRenderEngine.h @@ -159,6 +159,7 @@ private: void setupLayerTexturing(const Texture& texture); void setupFillWithColor(float r, float g, float b, float a); void setColorTransform(const mat4& colorTransform); + void setDisplayColorTransform(const mat4& colorTransform); void disableTexturing(); void disableBlending(); void setupCornerRadiusCropSize(float width, float height); diff --git a/libs/renderengine/gl/Program.cpp b/libs/renderengine/gl/Program.cpp index f4fbf3524a..a172c562f4 100644 --- a/libs/renderengine/gl/Program.cpp +++ b/libs/renderengine/gl/Program.cpp @@ -66,6 +66,7 @@ Program::Program(const ProgramCache::Key& /*needs*/, const char* vertex, const c mTextureMatrixLoc = glGetUniformLocation(programId, "texture"); mSamplerLoc = glGetUniformLocation(programId, "sampler"); mColorLoc = glGetUniformLocation(programId, "color"); + mDisplayColorMatrixLoc = glGetUniformLocation(programId, "displayColorMatrix"); mDisplayMaxLuminanceLoc = glGetUniformLocation(programId, "displayMaxLuminance"); mMaxMasteringLuminanceLoc = glGetUniformLocation(programId, "maxMasteringLuminance"); mMaxContentLuminanceLoc = glGetUniformLocation(programId, "maxContentLuminance"); @@ -129,6 +130,9 @@ void Program::setUniforms(const Description& desc) { const float color[4] = {desc.color.r, desc.color.g, desc.color.b, desc.color.a}; glUniform4fv(mColorLoc, 1, color); } + if (mDisplayColorMatrixLoc >= 0) { + glUniformMatrix4fv(mDisplayColorMatrixLoc, 1, GL_FALSE, desc.displayColorMatrix.asArray()); + } if (mInputTransformMatrixLoc >= 0) { mat4 inputTransformMatrix = desc.inputTransformMatrix; glUniformMatrix4fv(mInputTransformMatrixLoc, 1, GL_FALSE, inputTransformMatrix.asArray()); diff --git a/libs/renderengine/gl/Program.h b/libs/renderengine/gl/Program.h index fc3755e78f..429264531f 100644 --- a/libs/renderengine/gl/Program.h +++ b/libs/renderengine/gl/Program.h @@ -104,6 +104,7 @@ private: /* location of transform matrix */ GLint mInputTransformMatrixLoc; GLint mOutputTransformMatrixLoc; + GLint mDisplayColorMatrixLoc; /* location of corner radius uniform */ GLint mCornerRadiusLoc; diff --git a/libs/renderengine/gl/ProgramCache.cpp b/libs/renderengine/gl/ProgramCache.cpp index 3ae35ec4e9..dc8ce54736 100644 --- a/libs/renderengine/gl/ProgramCache.cpp +++ b/libs/renderengine/gl/ProgramCache.cpp @@ -180,6 +180,10 @@ ProgramCache::Key ProgramCache::computeKey(const Description& description) { description.hasOutputTransformMatrix() || description.hasColorMatrix() ? Key::OUTPUT_TRANSFORM_MATRIX_ON : Key::OUTPUT_TRANSFORM_MATRIX_OFF) + .set(Key::Key::DISPLAY_COLOR_TRANSFORM_MATRIX_MASK, + description.hasDisplayColorMatrix() + ? Key::DISPLAY_COLOR_TRANSFORM_MATRIX_ON + : Key::DISPLAY_COLOR_TRANSFORM_MATRIX_OFF) .set(Key::ROUNDED_CORNERS_MASK, description.cornerRadius > 0 ? Key::ROUNDED_CORNERS_ON : Key::ROUNDED_CORNERS_OFF) .set(Key::SHADOW_MASK, description.drawShadows ? Key::SHADOW_ON : Key::SHADOW_OFF); @@ -661,7 +665,9 @@ String8 ProgramCache::generateFragmentShader(const Key& needs) { )__SHADER__"; } - if (needs.hasTransformMatrix() || (needs.getInputTF() != needs.getOutputTF())) { + if (needs.hasTransformMatrix() || + (needs.getInputTF() != needs.getOutputTF()) || + needs.hasDisplayColorMatrix()) { if (needs.needsToneMapping()) { fs << "uniform float displayMaxLuminance;"; fs << "uniform float maxMasteringLuminance;"; @@ -700,6 +706,21 @@ String8 ProgramCache::generateFragmentShader(const Key& needs) { )__SHADER__"; } + if (needs.hasDisplayColorMatrix()) { + fs << "uniform mat4 displayColorMatrix;"; + fs << R"__SHADER__( + highp vec3 DisplayColorMatrix(const highp vec3 color) { + return clamp(vec3(displayColorMatrix * vec4(color, 1.0)), 0.0, 1.0); + } + )__SHADER__"; + } else { + fs << R"__SHADER__( + highp vec3 DisplayColorMatrix(const highp vec3 color) { + return color; + } + )__SHADER__"; + } + generateEOTF(fs, needs); generateOOTF(fs, needs); generateOETF(fs, needs); @@ -732,14 +753,17 @@ String8 ProgramCache::generateFragmentShader(const Key& needs) { } } - if (needs.hasTransformMatrix() || (needs.getInputTF() != needs.getOutputTF())) { + if (needs.hasTransformMatrix() || + (needs.getInputTF() != needs.getOutputTF()) || + needs.hasDisplayColorMatrix()) { if (!needs.isOpaque() && needs.isPremultiplied()) { // un-premultiply if needed before linearization // avoid divide by 0 by adding 0.5/256 to the alpha channel fs << "gl_FragColor.rgb = gl_FragColor.rgb / (gl_FragColor.a + 0.0019);"; } fs << "gl_FragColor.rgb = " - "OETF(OutputTransform(OOTF(InputTransform(EOTF(gl_FragColor.rgb)))));"; + "DisplayColorMatrix(OETF(OutputTransform(OOTF(InputTransform(EOTF(gl_FragColor.rgb))))));"; + if (!needs.isOpaque() && needs.isPremultiplied()) { // and re-premultiply if needed after gamma correction fs << "gl_FragColor.rgb = gl_FragColor.rgb * (gl_FragColor.a + 0.0019);"; diff --git a/libs/renderengine/gl/ProgramCache.h b/libs/renderengine/gl/ProgramCache.h index 901e6315a0..b492cb384b 100644 --- a/libs/renderengine/gl/ProgramCache.h +++ b/libs/renderengine/gl/ProgramCache.h @@ -117,6 +117,11 @@ public: SHADOW_MASK = 1 << SHADOW_SHIFT, SHADOW_OFF = 0 << SHADOW_SHIFT, SHADOW_ON = 1 << SHADOW_SHIFT, + + DISPLAY_COLOR_TRANSFORM_MATRIX_SHIFT = 14, + DISPLAY_COLOR_TRANSFORM_MATRIX_MASK = 1 << DISPLAY_COLOR_TRANSFORM_MATRIX_SHIFT, + DISPLAY_COLOR_TRANSFORM_MATRIX_OFF = 0 << DISPLAY_COLOR_TRANSFORM_MATRIX_SHIFT, + DISPLAY_COLOR_TRANSFORM_MATRIX_ON = 1 << DISPLAY_COLOR_TRANSFORM_MATRIX_SHIFT, }; inline Key() : mKey(0) {} @@ -143,6 +148,9 @@ public: inline bool hasOutputTransformMatrix() const { return (mKey & OUTPUT_TRANSFORM_MATRIX_MASK) == OUTPUT_TRANSFORM_MATRIX_ON; } + inline bool hasDisplayColorMatrix() const { + return (mKey & DISPLAY_COLOR_TRANSFORM_MATRIX_MASK) == DISPLAY_COLOR_TRANSFORM_MATRIX_ON; + } inline bool hasTransformMatrix() const { return hasInputTransformMatrix() || hasOutputTransformMatrix(); } diff --git a/libs/renderengine/include/renderengine/private/Description.h b/libs/renderengine/include/renderengine/private/Description.h index a62161a8a8..fa6ec10b6e 100644 --- a/libs/renderengine/include/renderengine/private/Description.h +++ b/libs/renderengine/include/renderengine/private/Description.h @@ -44,6 +44,7 @@ struct Description { bool hasInputTransformMatrix() const; bool hasOutputTransformMatrix() const; bool hasColorMatrix() const; + bool hasDisplayColorMatrix() const; // whether textures are premultiplied bool isPremultipliedAlpha = false; @@ -79,6 +80,8 @@ struct Description { // The color matrix will be applied in linear space right before OETF. mat4 colorMatrix; + // The display color matrix will be applied in gamma space after OETF + mat4 displayColorMatrix; mat4 inputTransformMatrix; mat4 outputTransformMatrix; diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp index 0b5b1e4be8..a720a27ed9 100644 --- a/libs/renderengine/tests/RenderEngineTest.cpp +++ b/libs/renderengine/tests/RenderEngineTest.cpp @@ -38,22 +38,27 @@ namespace android { struct RenderEngineTest : public ::testing::Test { static void SetUpTestSuite() { - sRE = renderengine::gl::GLESRenderEngine::create( - renderengine::RenderEngineCreationArgs::Builder() - .setPixelFormat(static_cast(ui::PixelFormat::RGBA_8888)) - .setImageCacheSize(1) - .setUseColorManagerment(false) - .setEnableProtectedContext(false) - .setPrecacheToneMapperShaderOnly(false) - .setSupportsBackgroundBlur(true) - .setContextPriority(renderengine::RenderEngine::ContextPriority::MEDIUM) - .build()); + renderengine::RenderEngineCreationArgs reCreationArgs = + renderengine::RenderEngineCreationArgs::Builder() + .setPixelFormat(static_cast(ui::PixelFormat::RGBA_8888)) + .setImageCacheSize(1) + .setUseColorManagerment(false) + .setEnableProtectedContext(false) + .setPrecacheToneMapperShaderOnly(false) + .setSupportsBackgroundBlur(true) + .setContextPriority(renderengine::RenderEngine::ContextPriority::MEDIUM) + .build(); + sRE = renderengine::gl::GLESRenderEngine::create(reCreationArgs); + + reCreationArgs.useColorManagement = true; + sRECM = renderengine::gl::GLESRenderEngine::create(reCreationArgs); } static void TearDownTestSuite() { // The ordering here is important - sCurrentBuffer must live longer // than RenderEngine to avoid a null reference on tear-down. sRE = nullptr; + sRECM = nullptr; sCurrentBuffer = nullptr; } @@ -82,6 +87,9 @@ struct RenderEngineTest : public ::testing::Test { for (uint32_t texName : mTexNames) { sRE->deleteTextures(1, &texName); } + for (uint32_t texName : mTexNamesCM) { + sRECM->deleteTextures(1, &texName); + } } void writeBufferToFile(const char* basename) { @@ -250,9 +258,13 @@ struct RenderEngineTest : public ::testing::Test { void invokeDraw(renderengine::DisplaySettings settings, std::vector layers, - sp buffer) { + sp buffer, + bool useColorManagement = false) { base::unique_fd fence; - status_t status = sRE->drawLayers(settings, layers, buffer->getNativeBuffer(), true, + status_t status = useColorManagement ? + sRECM ->drawLayers(settings, layers, buffer->getNativeBuffer(), true, + base::unique_fd(), &fence) : + sRE->drawLayers(settings, layers, buffer->getNativeBuffer(), true, base::unique_fd(), &fence); sCurrentBuffer = buffer; @@ -264,7 +276,11 @@ struct RenderEngineTest : public ::testing::Test { ASSERT_EQ(NO_ERROR, status); if (layers.size() > 0) { - ASSERT_TRUE(sRE->isFramebufferImageCachedForTesting(buffer->getId())); + if (useColorManagement) { + ASSERT_TRUE(sRECM->isFramebufferImageCachedForTesting(buffer->getId())); + } else { + ASSERT_TRUE(sRE->isFramebufferImageCachedForTesting(buffer->getId())); + } } } @@ -319,11 +335,14 @@ struct RenderEngineTest : public ::testing::Test { void fillBufferLayerTransform(); template - void fillBufferWithColorTransform(); + void fillBufferWithColorTransform(bool useColorManagement = false); template void fillBufferColorTransform(); + template + void fillBufferColorTransformCM(); + template void fillRedBufferWithRoundedCorners(); @@ -363,6 +382,8 @@ struct RenderEngineTest : public ::testing::Test { // For now, exercise the GL backend directly so that some caching specifics // can be tested without changing the interface. static std::unique_ptr sRE; + // renderengine object with Color Management enabled + static std::unique_ptr sRECM; // Dumb hack to avoid NPE in the EGL driver: the GraphicBuffer needs to // be freed *after* RenderEngine is destroyed, so that the EGL image is // destroyed first. @@ -371,14 +392,17 @@ struct RenderEngineTest : public ::testing::Test { sp mBuffer; std::vector mTexNames; + std::vector mTexNamesCM; }; std::unique_ptr RenderEngineTest::sRE = nullptr; +std::unique_ptr RenderEngineTest::sRECM = nullptr; + sp RenderEngineTest::sCurrentBuffer = nullptr; struct ColorSourceVariant { static void fillColor(renderengine::LayerSettings& layer, half r, half g, half b, - RenderEngineTest* /*fixture*/) { + RenderEngineTest* /*fixture*/, bool /*useColorManagement*/ = false) { layer.source.solidColor = half3(r, g, b); } }; @@ -406,11 +430,17 @@ struct ForceOpaqueBufferVariant { template struct BufferSourceVariant { static void fillColor(renderengine::LayerSettings& layer, half r, half g, half b, - RenderEngineTest* fixture) { + RenderEngineTest* fixture, + bool useColorManagement = false) { sp buf = RenderEngineTest::allocateSourceBuffer(1, 1); uint32_t texName; - fixture->sRE->genTextures(1, &texName); - fixture->mTexNames.push_back(texName); + if (useColorManagement) { + fixture->sRECM->genTextures(1, &texName); + fixture->mTexNamesCM.push_back(texName); + } else { + fixture->sRE->genTextures(1, &texName); + fixture->mTexNames.push_back(texName); + } uint8_t* pixels; buf->lock(GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_OFTEN, @@ -640,7 +670,7 @@ void RenderEngineTest::fillBufferLayerTransform() { } template -void RenderEngineTest::fillBufferWithColorTransform() { +void RenderEngineTest::fillBufferWithColorTransform(bool useColorManagement) { renderengine::DisplaySettings settings; settings.physicalDisplay = fullscreenRect(); settings.clip = Rect(1, 1); @@ -649,12 +679,12 @@ void RenderEngineTest::fillBufferWithColorTransform() { renderengine::LayerSettings layer; layer.geometry.boundaries = Rect(1, 1).toFloatRect(); - SourceVariant::fillColor(layer, 0.5f, 0.25f, 0.125f, this); + SourceVariant::fillColor(layer, 0.5f, 0.25f, 0.125f, this, useColorManagement); layer.alpha = 1.0f; // construct a fake color matrix // annihilate green and blue channels - settings.colorTransform = mat4::scale(vec4(1, 0, 0, 1)); + settings.colorTransform = mat4::scale(vec4(0.9f, 0, 0, 1)); // set red channel to red + green layer.colorTransform = mat4(1, 0, 0, 0, 1, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1); @@ -663,13 +693,19 @@ void RenderEngineTest::fillBufferWithColorTransform() { layers.push_back(&layer); - invokeDraw(settings, layers, mBuffer); + invokeDraw(settings, layers, mBuffer, useColorManagement); } template void RenderEngineTest::fillBufferColorTransform() { fillBufferWithColorTransform(); - expectBufferColor(fullscreenRect(), 191, 0, 0, 255); + expectBufferColor(fullscreenRect(), 172, 0, 0, 255, 1); +} + +template +void RenderEngineTest::fillBufferColorTransformCM() { + fillBufferWithColorTransform(true); + expectBufferColor(fullscreenRect(), 126, 0, 0, 255, 1); } template @@ -1072,7 +1108,11 @@ TEST_F(RenderEngineTest, drawLayers_fillBufferLayerTransform_colorSource) { } TEST_F(RenderEngineTest, drawLayers_fillBufferColorTransform_colorSource) { - fillBufferLayerTransform(); + fillBufferColorTransform(); +} + +TEST_F(RenderEngineTest, drawLayers_fillBufferColorTransformCM_colorSource) { + fillBufferColorTransformCM(); } TEST_F(RenderEngineTest, drawLayers_fillBufferRoundedCorners_colorSource) { @@ -1128,7 +1168,11 @@ TEST_F(RenderEngineTest, drawLayers_fillBufferLayerTransform_opaqueBufferSource) } TEST_F(RenderEngineTest, drawLayers_fillBufferColorTransform_opaqueBufferSource) { - fillBufferLayerTransform>(); + fillBufferColorTransform>(); +} + +TEST_F(RenderEngineTest, drawLayers_fillBufferColorTransformCM_opaqueBufferSource) { + fillBufferColorTransformCM>(); } TEST_F(RenderEngineTest, drawLayers_fillBufferRoundedCorners_opaqueBufferSource) { @@ -1184,7 +1228,11 @@ TEST_F(RenderEngineTest, drawLayers_fillBufferLayerTransform_bufferSource) { } TEST_F(RenderEngineTest, drawLayers_fillBufferColorTransform_bufferSource) { - fillBufferLayerTransform>(); + fillBufferColorTransform>(); +} + +TEST_F(RenderEngineTest, drawLayers_fillBufferColorTransformCM_bufferSource) { + fillBufferColorTransformCM>(); } TEST_F(RenderEngineTest, drawLayers_fillBufferRoundedCorners_bufferSource) { -- cgit v1.2.3-59-g8ed1b