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>
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
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;