From 6183c97e5f317ad52ad16fe50e40129e2c7b2150 Mon Sep 17 00:00:00 2001 From: Romain Guy Date: Thu, 16 Mar 2017 12:24:55 -0700 Subject: [PATCH] Bowing my head in shame, going back to gamma interpolated gradients Frankengradients (linearly interpolated RGB, gamma interpolated alpha) look fantastic but unfortunately create sligh compatibility issues. For instance, a gradient from 0xffea1030 to 0x00ea1030 (opaque to alpha, with a single color) blended on top of 0xff101010 would not look the same as a single opaque gradient from 0xffea1030 to 0xff101010. The difference is hardly noticeable on simple gradients but it could cause confusion amongst app developers. Their life is hard enough as it is, let's be good to them. My crusade against the gamma world is not over and one day I shall be the victor. I am patience. Bug: 35485208 Test: UiRendering.ShaderTests, UiRendering.GradientTests, manual testing Change-Id: I8204e60cdf0a6b12dfe22638d30ca9622687000e --- libs/hwui/FloatColor.h | 13 ++++++------ libs/hwui/GradientCache.cpp | 23 +++++++++------------ libs/hwui/ProgramCache.cpp | 41 ++++++++++++++++++++----------------- libs/hwui/SkiaShader.cpp | 4 ++-- 4 files changed, 41 insertions(+), 40 deletions(-) diff --git a/libs/hwui/FloatColor.h b/libs/hwui/FloatColor.h index d8afa35d32bf1..a738ba4fe97e0 100644 --- a/libs/hwui/FloatColor.h +++ b/libs/hwui/FloatColor.h @@ -38,13 +38,14 @@ struct FloatColor { } // "color" is a gamma-encoded sRGB color - // After calling this method, the color is stored as a linear color. The color - // is not pre-multiplied. - void setUnPreMultipliedSRGB(uint32_t color) { + // After calling this method, the color is stored as a un-premultiplied linear color + // if linear blending is enabled. Otherwise, the color is stored as a un-premultiplied + // gamma-encoded sRGB color + void setUnPreMultiplied(uint32_t color) { a = ((color >> 24) & 0xff) / 255.0f; - r = EOCF_sRGB(((color >> 16) & 0xff) / 255.0f); - g = EOCF_sRGB(((color >> 8) & 0xff) / 255.0f); - b = EOCF_sRGB(((color ) & 0xff) / 255.0f); + r = EOCF(((color >> 16) & 0xff) / 255.0f); + g = EOCF(((color >> 8) & 0xff) / 255.0f); + b = EOCF(((color ) & 0xff) / 255.0f); } bool isNotBlack() { diff --git a/libs/hwui/GradientCache.cpp b/libs/hwui/GradientCache.cpp index 18bfcc2bbddf4..dceb28518db32 100644 --- a/libs/hwui/GradientCache.cpp +++ b/libs/hwui/GradientCache.cpp @@ -189,9 +189,9 @@ void GradientCache::mixBytes(const FloatColor& start, const FloatColor& end, float amount, uint8_t*& dst) const { float oppAmount = 1.0f - amount; float a = start.a * oppAmount + end.a * amount; - *dst++ = uint8_t(a * OECF_sRGB((start.r * oppAmount + end.r * amount)) * 255.0f); - *dst++ = uint8_t(a * OECF_sRGB((start.g * oppAmount + end.g * amount)) * 255.0f); - *dst++ = uint8_t(a * OECF_sRGB((start.b * oppAmount + end.b * amount)) * 255.0f); + *dst++ = uint8_t(a * OECF(start.r * oppAmount + end.r * amount) * 255.0f); + *dst++ = uint8_t(a * OECF(start.g * oppAmount + end.g * amount) * 255.0f); + *dst++ = uint8_t(a * OECF(start.b * oppAmount + end.b * amount) * 255.0f); *dst++ = uint8_t(a * 255.0f); } @@ -201,17 +201,14 @@ void GradientCache::mixFloats(const FloatColor& start, const FloatColor& end, float a = start.a * oppAmount + end.a * amount; float* d = (float*) dst; #ifdef ANDROID_ENABLE_LINEAR_BLENDING + // We want to stay linear *d++ = a * (start.r * oppAmount + end.r * amount); *d++ = a * (start.g * oppAmount + end.g * amount); *d++ = a * (start.b * oppAmount + end.b * amount); #else - // What we're doing to the alpha channel here is technically incorrect - // but reproduces Android's old behavior when the alpha was pre-multiplied - // with gamma-encoded colors - a = EOCF_sRGB(a); - *d++ = a * OECF_sRGB(start.r * oppAmount + end.r * amount); - *d++ = a * OECF_sRGB(start.g * oppAmount + end.g * amount); - *d++ = a * OECF_sRGB(start.b * oppAmount + end.b * amount); + *d++ = a * OECF(start.r * oppAmount + end.r * amount); + *d++ = a * OECF(start.g * oppAmount + end.g * amount); + *d++ = a * OECF(start.b * oppAmount + end.b * amount); #endif *d++ = a; dst += 4 * sizeof(float); @@ -232,10 +229,10 @@ void GradientCache::generateTexture(uint32_t* colors, float* positions, ChannelMixer mix = gMixers[mUseFloatTexture]; FloatColor start; - start.setUnPreMultipliedSRGB(colors[0]); + start.setUnPreMultiplied(colors[0]); FloatColor end; - end.setUnPreMultipliedSRGB(colors[1]); + end.setUnPreMultiplied(colors[1]); int currentPos = 1; float startPos = positions[0]; @@ -250,7 +247,7 @@ void GradientCache::generateTexture(uint32_t* colors, float* positions, currentPos++; - end.setUnPreMultipliedSRGB(colors[currentPos]); + end.setUnPreMultiplied(colors[currentPos]); distance = positions[currentPos] - startPos; } diff --git a/libs/hwui/ProgramCache.cpp b/libs/hwui/ProgramCache.cpp index 3f842e0b5679d..38c23e4babe8d 100644 --- a/libs/hwui/ProgramCache.cpp +++ b/libs/hwui/ProgramCache.cpp @@ -202,23 +202,26 @@ const char* gFS_Gradient_Functions = R"__SHADER__( )__SHADER__"; const char* gFS_Gradient_Preamble[2] = { // Linear framebuffer - "\nvec4 dither(const vec4 color) {\n" - " return color + (triangleNoise(gl_FragCoord.xy * screenSize.xy) / 255.0);\n" - "}\n" - "\nvec4 gammaMix(const vec4 a, const vec4 b, float v) {\n" - " vec4 c = mix(a, b, v);\n" - " c.a = EOTF_sRGB(c.a);\n" // This is technically incorrect but preserves compatibility - " return vec4(OETF_sRGB(c.rgb) * c.a, c.a);\n" - "}\n", + R"__SHADER__( + vec4 dither(const vec4 color) { + return color + (triangleNoise(gl_FragCoord.xy * screenSize.xy) / 255.0); + } + vec4 gradientMix(const vec4 a, const vec4 b, float v) { + vec4 c = mix(a, b, v); + return vec4(c.rgb * c.a, c.a); + } + )__SHADER__", // sRGB framebuffer - "\nvec4 dither(const vec4 color) {\n" - " vec3 dithered = sqrt(color.rgb) + (triangleNoise(gl_FragCoord.xy * screenSize.xy) / 255.0);\n" - " return vec4(dithered * dithered, color.a);\n" - "}\n" - "\nvec4 gammaMix(const vec4 a, const vec4 b, float v) {\n" - " vec4 c = mix(a, b, v);\n" - " return vec4(c.rgb * c.a, c.a);\n" - "}\n" + R"__SHADER__( + vec4 dither(const vec4 color) { + vec3 dithered = sqrt(color.rgb) + (triangleNoise(gl_FragCoord.xy * screenSize.xy) / 255.0); + return vec4(dithered * dithered, color.a); + } + vec4 gradientMixMix(const vec4 a, const vec4 b, float v) { + vec4 c = mix(a, b, v); + return vec4(c.rgb * c.a, c.a); + } + )__SHADER__", }; // Uses luminance coefficients from Rec.709 to choose the appropriate gamma @@ -272,19 +275,19 @@ const char* gFS_Main_FetchGradient[6] = { // Linear " vec4 gradientColor = texture2D(gradientSampler, linear);\n", - " vec4 gradientColor = gammaMix(startColor, endColor, clamp(linear, 0.0, 1.0));\n", + " vec4 gradientColor = gradientMix(startColor, endColor, clamp(linear, 0.0, 1.0));\n", // Circular " vec4 gradientColor = texture2D(gradientSampler, vec2(length(circular), 0.5));\n", - " vec4 gradientColor = gammaMix(startColor, endColor, clamp(length(circular), 0.0, 1.0));\n", + " vec4 gradientColor = gradientMix(startColor, endColor, clamp(length(circular), 0.0, 1.0));\n", // Sweep " highp float index = atan(sweep.y, sweep.x) * 0.15915494309; // inv(2 * PI)\n" " vec4 gradientColor = texture2D(gradientSampler, vec2(index - floor(index), 0.5));\n", " highp float index = atan(sweep.y, sweep.x) * 0.15915494309; // inv(2 * PI)\n" - " vec4 gradientColor = gammaMix(startColor, endColor, clamp(index - floor(index), 0.0, 1.0));\n" + " vec4 gradientColor = gradientMix(startColor, endColor, clamp(index - floor(index), 0.0, 1.0));\n" }; const char* gFS_Main_FetchBitmap = " vec4 bitmapColor = OETF(texture2D(bitmapSampler, outBitmapTexCoords));\n"; diff --git a/libs/hwui/SkiaShader.cpp b/libs/hwui/SkiaShader.cpp index 760c10c6130ee..4f7f9d7f9b9a3 100644 --- a/libs/hwui/SkiaShader.cpp +++ b/libs/hwui/SkiaShader.cpp @@ -173,8 +173,8 @@ bool tryStoreGradient(Caches& caches, const SkShader& shader, const Matrix4 mode outData->gradientSampler = 0; outData->gradientTexture = nullptr; - outData->startColor.setUnPreMultipliedSRGB(gradInfo.fColors[0]); - outData->endColor.setUnPreMultipliedSRGB(gradInfo.fColors[1]); + outData->startColor.setUnPreMultiplied(gradInfo.fColors[0]); + outData->endColor.setUnPreMultiplied(gradInfo.fColors[1]); } return true;