commit 28698696f1b34a8df28f1428686efd4eadc241a9
author Mike Klein <mtklein@google.com> Fri Feb 12 07:59:55 2021 -0600
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Fri Feb 12 16:46:37 2021 +0000
tree d7c1c2630292d0d9321b6309c42f1532d304be03
parent 5ad759065d6c674ab1921e5e3568baf9bebddbe7

revise how we handle invalid configs

Move the contrast pin to Make() so it affects all implementations.

Bug: oss-fuzz:30859
Change-Id: Iaf7854701bc6a148976f653066d7bf8f27080d2f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/369698
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>

src/effects/SkHighContrastFilter.cpp

diff --git a/src/effects/SkHighContrastFilter.cpp b/src/effects/SkHighContrastFilter.cpp
index c2c7e7d..cb0c0ca 100644
--- a/src/effects/SkHighContrastFilter.cpp
+++ b/src/effects/SkHighContrastFilter.cpp
@@ -30,13 +30,7 @@
 
 class SkHighContrast_Filter : public SkColorFilterBase {
 public:
-    SkHighContrast_Filter(const SkHighContrastConfig& config) {
-        fConfig = config;
-        // Clamp contrast to just inside -1 to 1 to avoid division by zero.
-        fConfig.fContrast = SkTPin(fConfig.fContrast,
-                                   -1.0f + FLT_EPSILON,
-                                   1.0f - FLT_EPSILON);
-    }
+    SkHighContrast_Filter(const SkHighContrastConfig& config) : fConfig(config) {}
 
     ~SkHighContrast_Filter() override {}
 
@@ -204,10 +198,18 @@
     return SkHighContrastFilter::Make(config);
 }
 
-sk_sp<SkColorFilter> SkHighContrastFilter::Make(const SkHighContrastConfig& config) {
-    if (!config.isValid()) {
+sk_sp<SkColorFilter> SkHighContrastFilter::Make(const SkHighContrastConfig& userConfig) {
+    if (!userConfig.isValid()) {
         return nullptr;
     }
+
+    // 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?
+    SkHighContrastConfig config = userConfig;
+    config.fContrast = SkTPin(config.fContrast,
+                              -1.0f + FLT_EPSILON,
+                              +1.0f - FLT_EPSILON);
+
 #if defined(SK_SUPPORT_LEGACY_RUNTIME_EFFECTS)
     return sk_make_sp<SkHighContrast_Filter>(config);
 #else

tests/HighContrastFilterTest.cpp

diff --git a/tests/HighContrastFilterTest.cpp b/tests/HighContrastFilterTest.cpp
index 052811d..d3c4488 100644
--- a/tests/HighContrastFilterTest.cpp
+++ b/tests/HighContrastFilterTest.cpp
@@ -76,11 +76,13 @@
     REPORTER_ASSERT(reporter, !filter);
 
     // Valid contrast
-    config.fInvertStyle = SkHighContrastConfig::InvertStyle::kInvertBrightness;
-    config.fContrast = 0.5f;
-    REPORTER_ASSERT(reporter, config.isValid());
-    filter = SkHighContrastFilter::Make(config);
-    REPORTER_ASSERT(reporter, filter);
+    for (float contrast : {0.5f, +1.0f, -1.0f}) {
+        config.fInvertStyle = SkHighContrastConfig::InvertStyle::kInvertBrightness;
+        config.fContrast = contrast;
+        REPORTER_ASSERT(reporter, config.isValid());
+        filter = SkHighContrastFilter::Make(config);
+        REPORTER_ASSERT(reporter, filter);
+    }
 
     // Invalid contrast
     config.fContrast = 1.1f;