refactor high-contrast filter

Rewrite as one cacheable runtime effect with uniforms paralleling
SkHighContrastConfig.  This is mostly for style, in the sense of
readability but also how I'd recommend writing these effects generally:
one program body controlled by uniforms.  Callers needing specialization
can write their own effect (or use our TBD uniform->constant facility).

Test isAlphaUnchanged(), and make it so by not saturating c.a.

MaliT880 was producing seriously bad results until I switched to mix().

Change-Id: Ia4cfef25fe26643f6832a5fa0843b28dafc5b284
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372295
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>

src/effects/SkHighContrastFilter.cpp

@@ -11,56 +11,57 @@
 #include "include/private/SkTPin.h"
 #include "src/core/SkRuntimeEffectPriv.h"
 
-sk_sp<SkColorFilter> SkHighContrastFilter::Make(const SkHighContrastConfig& userConfig) {
+sk_sp<SkColorFilter> SkHighContrastFilter::Make(const SkHighContrastConfig& config) {
-    if (!userConfig.isValid()) {
+    if (!config.isValid()) {
         return nullptr;
     }
 
-    // A contrast setting of exactly +1 would divide by zero (1+c)/(1-c), so pull in to +1-ε.
+    struct Uniforms { float grayscale, invertStyle, contrast; };
-    // I'm not exactly sure why we've historically pinned -1 up to -1+ε, maybe just symmetry?
-    SkHighContrastConfig config = userConfig;
-    config.fContrast = SkTPin(config.fContrast,
-                              -1.0f + FLT_EPSILON,
-                              +1.0f - FLT_EPSILON);
-
-    struct { float M; } uniforms;
-    SkString code{
-        "uniform shader input;"
-        "uniform half M;"
-    };
 
+    static SkRuntimeEffect* effect = []{
+        SkString code{R"(
+            uniform shader input;
+            uniform half grayscale, invertStyle, contrast;
+        )"};
         code += kRGB_to_HSL_sksl;
         code += kHSL_to_RGB_sksl;
-
+        code += R"(
-    code += "half4 main() {";
+            half4 main() {
-    if (true) {
+                half4 c = sample(input);  // linear unpremul RGBA in dst gamut.
-        code += "half4 c = sample(input);"; // c is linear unpremul RGBA in the dst gamut.
+                if (grayscale == 1) {
+                    c.rgb = dot(half3(0.2126, 0.7152, 0.0722), c.rgb).rrr;
                 }
-    if (config.fGrayscale) {
+                if (invertStyle == 1/*brightness*/) {
-        code += "c.rgb = dot(half3(0.2126, 0.7152, 0.0722), c.rgb).rrr;";
+                    c.rgb = 1 - c.rgb;
+                } else if (invertStyle == 2/*lightness*/) {
+                    c.rgb = rgb_to_hsl(c.rgb);
+                    c.b = 1 - c.b;
+                    c.rgb = hsl_to_rgb(c.rgb);
                 }
-    if (config.fInvertStyle == SkHighContrastConfig::InvertStyle::kInvertBrightness) {
+                c.rgb = mix(half3(0.5), c.rgb, contrast);
-        code += "c.rgb = 1 - c.rgb;";
+                return half4(saturate(c.rgb), c.a);
             }
-    if (config.fInvertStyle == SkHighContrastConfig::InvertStyle::kInvertLightness) {
+        )";
-        code += "c.rgb = rgb_to_hsl(c.rgb);";
-        code += "c.b = 1 - c.b;";
-        code += "c.rgb = hsl_to_rgb(c.rgb);";
-    }
-    if (float c = config.fContrast) {
-        uniforms.M = (1+c)/(1-c);
-        code += "c.rgb = (c.rgb - 0.5)*M + 0.5;";
-    }
-    if (true) {
-        code += "return saturate(c);";
-    }
-    code += "}";
 
         auto [effect, err] = SkRuntimeEffect::Make(code);
         if (!err.isEmpty()) {
             SkDebugf("%s\n%s\n", code.c_str(), err.c_str());
         }
         SkASSERT(effect && err.isEmpty());
+        return effect.release();
+    }();
+
+    // A contrast setting of exactly +1 would divide by zero (1+c)/(1-c), so pull in to +1-ε.
+    // I'm not exactly sure why we've historically pinned -1 up to -1+ε, maybe just symmetry?
+    float c = SkTPin(config.fContrast,
+                     -1.0f + FLT_EPSILON,
+                     +1.0f - FLT_EPSILON);
+
+    Uniforms uniforms = {
+        config.fGrayscale ? 1.0f : 0.0f,
+        (float)config.fInvertStyle,  // 0.0f for none, 1.0f for brightness, 2.0f for lightness
+        (1+c)/(1-c),
+    };
 
     sk_sp<SkColorFilter>    input = nullptr;
     skcms_TransferFunction linear = SkNamedTransferFn::kLinear;

tests/HighContrastFilterTest.cpp

@@ -45,10 +45,11 @@ DEF_TEST(HighContrastFilter_FilterImage, reporter) {
     }
 }
 
-DEF_TEST(HighContrastFilter_SanityCheck, reporter) {
+DEF_TEST(HighContrastFilter_SmokeTest, reporter) {
     SkHighContrastConfig config;
     config.fInvertStyle = SkHighContrastConfig::InvertStyle::kInvertLightness;
     sk_sp<SkColorFilter> filter = SkHighContrastFilter::Make(config);
+    REPORTER_ASSERT(reporter, filter->isAlphaUnchanged());
 
     SkColor white_inverted = filter->filterColor(SK_ColorWHITE);
     REPORTER_ASSERT(reporter, white_inverted == SK_ColorBLACK);